Re: [PATCH v4 5/7] ASoC: intel - add Skylake HDA audio driver

[Vinod Koul <vinod.koul@intel.com>]

On Fri, May 29, 2015 at 06:41:15PM +0100, Mark Brown wrote:
> On Mon, May 11, 2015 at 04:24:03PM +0530, Vinod Koul wrote:
> 
> > +static void azx_free_streams(struct soc_hdac_bus *sbus)
> > +{
> > +	struct hdac_stream *s;
> > +	struct soc_hdac_stream *stream;
> > +	struct hdac_bus *bus = hdac_bus(sbus);
> > +
> > +	while (!list_empty(&bus->stream_list)) {
> > +		s = list_first_entry(&bus->stream_list, struct hdac_stream, list);
> > +		stream = stream_to_soc_hdac_stream(s);
> > +		list_del(&s->list);
> > +		kfree(stream);
> > +	}
> > +}
> 
> Why is this (and the equivalent link function) device specific code?
> They look very generic...
In HDA we will have only one stream, but here we can have multiple links so
need to free each of them.
Also if you notice most of the code in this patch uses hdac_xxx helpers to
do the actual job

> 
> > +static int azx_acquire_irq(struct soc_hdac_bus *sbus, int do_disconnect)
> > +{
> > +	struct hda_skl *hda = to_hda_skl(sbus);
> > +	struct hdac_bus *bus = hdac_bus(sbus);
> > +	int ret = 0;
> > +
> > +	ret = request_threaded_irq(hda->pci->irq, azx_interrupt,
> > +			azx_threaded_handler,
> > +			hda->msi ? 0 : IRQF_SHARED,
> > +			KBUILD_MODNAME, sbus);
> 
> Why not just always request the interrupt as shared - we don't seem to
> care in the interrupt handling code?
I think we can move this to shared only

-- 
~Vinod