At Mon, 15 Jun 2015 17:42:02 +0100, Mark Brown wrote:
On Mon, Jun 15, 2015 at 06:35:16PM +0200, Takashi Iwai wrote:
Mark Brown wrote:
On Thu, Jun 11, 2015 at 10:03:56PM +0530, Vinod Koul wrote:
- for (i = 0; i < num_stream; i++) {
struct hdac_ext_stream *stream =kzalloc(sizeof(*stream), GFP_KERNEL);Still not sure why these are Sky Lake specific?
Currently the allocation and the free of each HDA(-ext) stream are left to each controller driver. (See the stream object is embedded.)
It looks awfully like it's dynamically allocated here...
And, yes, the allocation (especially the assignment of the stream tag) *is* SKL specific. SKL has some twists in the interpretation of HD-audio spec.
I can see the thing calling these functions being driver specific but as far as I can see all these are doing is allocating structs, initialising them with passed in parameters, putting them on a list and calling a core function on them. It's not the bit taking the decisions, it's just doing mechanical things.
Actually, this can be done a bit more cleanly -- if there will be no more additions to the stream object. It wasn't clear, so the allocation and free are left to the driver, so far.
- snd_hdac_bus_stop_chip(bus);
- snd_hdac_bus_enter_link_reset(bus);
- if (bus->irq >= 0) {
free_irq(bus->irq, bus);Why are we freeing the interrupt over suspend? That is very unusual behaviour.
Believe or not, a PCI controller may reassign to a different IRQ number after hibernation. This really happened in the past for some old stuff, hence we have this in the legacy driver.
But SKL won't need this, I suppose.
Do you mean PCI controller or BIOS here?
Weird combination of both.
+static const struct hdac_io_ops skl_io_ops = {
- .reg_writel = skl_pci_writel,
- .reg_readl = skl_pci_readl,
- .reg_writew = skl_pci_writew,
- .reg_readw = skl_pci_readw,
- .reg_writeb = skl_pci_writeb,
- .reg_readb = skl_pci_readb,
- .dma_alloc_pages = skl_dma_alloc_pages,
- .dma_free_pages = skl_dma_free_pages,
+};
Still not thrilled at open coding these wrappers.
This is not so sexy, right, but be patient until the next HDA core code rewrite.
Or we could add the ops to the core now and convert the drivers later?
This should be feasible.
Takashi