RE: [PATCH 2/2] soundwire: intel: transition to 3 steps initialization
> -Original Message- > From: Vinod Koul > Sent: Thursday, May 21, 2020 12:37 PM > To: Liao, Bard > Cc: Bard Liao ; alsa-de...@alsa-project.org; > linux-kernel@vger.kernel.org; ti...@suse.de; broo...@kernel.org; > gre...@linuxfoundation.org; j...@cadence.com; > srinivas.kandaga...@linaro.org; rander.w...@linux.intel.com; > ranjani.sridha...@linux.intel.com; hui.w...@canonical.com; pierre- > louis.boss...@linux.intel.com; Kale, Sanyog R ; > Blauciak, Slawomir ; Lin, Mengdong > > Subject: Re: [PATCH 2/2] soundwire: intel: transition to 3 steps > initialization > > On 21-05-20, 02:23, Liao, Bard wrote: > > > -Original Message- > > > From: Vinod Koul > > > Sent: Wednesday, May 20, 2020 9:54 PM > > > To: Bard Liao > > > Cc: alsa-de...@alsa-project.org; linux-kernel@vger.kernel.org; > > > ti...@suse.de; broo...@kernel.org; gre...@linuxfoundation.org; > > > j...@cadence.com; srinivas.kandaga...@linaro.org; > > > rander.w...@linux.intel.com; ranjani.sridha...@linux.intel.com; > > > hui.w...@canonical.com; pierre- louis.boss...@linux.intel.com; Kale, > > > Sanyog R ; Blauciak, Slawomir > > > ; Lin, Mengdong > > > ; Liao, Bard > > > Subject: Re: [PATCH 2/2] soundwire: intel: transition to 3 steps > > > initialization > > > > > > On 20-05-20, 03:19, Bard Liao wrote: > > > > From: Pierre-Louis Bossart > > > > > > > > Rather than a plain-vanilla init/exit, this patch provides 3 steps > > > > in the initialization (ACPI scan, probe, startup) which makes it > > > > easier to detect platform support for SoundWire, allocate required > > > > resources as early as possible, and conversely help make the > > > > startup() callback lighter-weight with only hardware register setup. > > > > > > Okay but can you add details in changelog on what each step would do? > > > > Sure. Will do. > > > > > > > > > @@ -1134,25 +1142,15 @@ static int intel_probe(struct > > > > platform_device > > > *pdev) > > > > > > > > intel_pdi_ch_update(sdw); > > > > > > > > - /* Acquire IRQ */ > > > > - ret = request_threaded_irq(sdw->link_res->irq, > > > > - sdw_cdns_irq, sdw_cdns_thread, > > > > - IRQF_SHARED, KBUILD_MODNAME, > > > >cdns); > > > > > > This is removed here but not added anywhere else, do we have no irq > > > after this patch? > > > > We use a single irq for all Intel Audio DSP events and it will be > > requested in the SOF driver. > > And how will the irq be propagated to sdw/cdns drivers here? We export the handler and call it on SOF driver. > > -- > ~Vinod
Re: [PATCH 2/2] soundwire: intel: transition to 3 steps initialization
On 21-05-20, 02:23, Liao, Bard wrote: > > -Original Message- > > From: Vinod Koul > > Sent: Wednesday, May 20, 2020 9:54 PM > > To: Bard Liao > > Cc: alsa-de...@alsa-project.org; linux-kernel@vger.kernel.org; > > ti...@suse.de; > > broo...@kernel.org; gre...@linuxfoundation.org; j...@cadence.com; > > srinivas.kandaga...@linaro.org; rander.w...@linux.intel.com; > > ranjani.sridha...@linux.intel.com; hui.w...@canonical.com; pierre- > > louis.boss...@linux.intel.com; Kale, Sanyog R ; > > Blauciak, Slawomir ; Lin, Mengdong > > ; Liao, Bard > > Subject: Re: [PATCH 2/2] soundwire: intel: transition to 3 steps > > initialization > > > > On 20-05-20, 03:19, Bard Liao wrote: > > > From: Pierre-Louis Bossart > > > > > > Rather than a plain-vanilla init/exit, this patch provides 3 steps in > > > the initialization (ACPI scan, probe, startup) which makes it easier to > > > detect platform support for SoundWire, allocate required resources as > > > early as possible, and conversely help make the startup() callback > > > lighter-weight with only hardware register setup. > > > > Okay but can you add details in changelog on what each step would do? > > Sure. Will do. > > > > > > @@ -1134,25 +1142,15 @@ static int intel_probe(struct platform_device > > *pdev) > > > > > > intel_pdi_ch_update(sdw); > > > > > > - /* Acquire IRQ */ > > > - ret = request_threaded_irq(sdw->link_res->irq, > > > -sdw_cdns_irq, sdw_cdns_thread, > > > -IRQF_SHARED, KBUILD_MODNAME, > > >cdns); > > > > This is removed here but not added anywhere else, do we have no irq > > after this patch? > > We use a single irq for all Intel Audio DSP events and it will > be requested in the SOF driver. And how will the irq be propagated to sdw/cdns drivers here? -- ~Vinod
RE: [PATCH 2/2] soundwire: intel: transition to 3 steps initialization
> -Original Message-
> From: Vinod Koul
> Sent: Wednesday, May 20, 2020 9:54 PM
> To: Bard Liao
> Cc: alsa-de...@alsa-project.org; linux-kernel@vger.kernel.org; ti...@suse.de;
> broo...@kernel.org; gre...@linuxfoundation.org; j...@cadence.com;
> srinivas.kandaga...@linaro.org; rander.w...@linux.intel.com;
> ranjani.sridha...@linux.intel.com; hui.w...@canonical.com; pierre-
> louis.boss...@linux.intel.com; Kale, Sanyog R ;
> Blauciak, Slawomir ; Lin, Mengdong
> ; Liao, Bard
> Subject: Re: [PATCH 2/2] soundwire: intel: transition to 3 steps
> initialization
>
> On 20-05-20, 03:19, Bard Liao wrote:
> > From: Pierre-Louis Bossart
> >
> > Rather than a plain-vanilla init/exit, this patch provides 3 steps in
> > the initialization (ACPI scan, probe, startup) which makes it easier to
> > detect platform support for SoundWire, allocate required resources as
> > early as possible, and conversely help make the startup() callback
> > lighter-weight with only hardware register setup.
>
> Okay but can you add details in changelog on what each step would do?
Sure. Will do.
>
> > @@ -1134,25 +1142,15 @@ static int intel_probe(struct platform_device
> *pdev)
> >
> > intel_pdi_ch_update(sdw);
> >
> > - /* Acquire IRQ */
> > - ret = request_threaded_irq(sdw->link_res->irq,
> > - sdw_cdns_irq, sdw_cdns_thread,
> > - IRQF_SHARED, KBUILD_MODNAME,
> >cdns);
>
> This is removed here but not added anywhere else, do we have no irq
> after this patch?
We use a single irq for all Intel Audio DSP events and it will
be requested in the SOF driver.
>
> > @@ -1205,5 +1201,5 @@ static struct platform_driver sdw_intel_drv = {
> > module_platform_driver(sdw_intel_drv);
> >
> > MODULE_LICENSE("Dual BSD/GPL");
> > -MODULE_ALIAS("platform:int-sdw");
> > +MODULE_ALIAS("sdw:intel-sdw");
>
> it is still a platform device, so does sdw: tag make sense?
> This is used by modprobe to load the driver!
Will fix it
>
> > +/**
> > + * sdw_intel_probe() - SoundWire Intel probe routine
> > + * @res: resource data
> > + *
> > + * This creates SoundWire Master and Slave devices below the controller.
>
> I dont think the comment is correct, this is done in intel_master_probe
> which is platform device probe...
Thanks. Will fix it.
>
> > + * All the information necessary is stored in the context, and the res
> > + * argument pointer can be freed after this step.
> > + */
> > +struct sdw_intel_ctx
> > +*sdw_intel_probe(struct sdw_intel_res *res)
> > +{
> > + return sdw_intel_probe_controller(res);
> > +}
> > +EXPORT_SYMBOL(sdw_intel_probe);
>
> I guess this would be called by SOF driver, question is when..?
Will document it, thanks.
>
> > +/**
> > + * sdw_intel_startup() - SoundWire Intel startup
> > + * @ctx: SoundWire context allocated in the probe
> > + *
> > + */
> > +int sdw_intel_startup(struct sdw_intel_ctx *ctx)
> > +{
> > + return sdw_intel_startup_controller(ctx);
> > +}
> > +EXPORT_SYMBOL(sdw_intel_startup);
>
> when is this called, pls do document that
Will document it, thanks.
>
> --
> ~Vinod
Re: [PATCH 2/2] soundwire: intel: transition to 3 steps initialization
On 20-05-20, 03:19, Bard Liao wrote:
> From: Pierre-Louis Bossart
>
> Rather than a plain-vanilla init/exit, this patch provides 3 steps in
> the initialization (ACPI scan, probe, startup) which makes it easier to
> detect platform support for SoundWire, allocate required resources as
> early as possible, and conversely help make the startup() callback
> lighter-weight with only hardware register setup.
Okay but can you add details in changelog on what each step would do?
> @@ -1134,25 +1142,15 @@ static int intel_probe(struct platform_device *pdev)
>
> intel_pdi_ch_update(sdw);
>
> - /* Acquire IRQ */
> - ret = request_threaded_irq(sdw->link_res->irq,
> -sdw_cdns_irq, sdw_cdns_thread,
> -IRQF_SHARED, KBUILD_MODNAME, >cdns);
This is removed here but not added anywhere else, do we have no irq
after this patch?
> @@ -1205,5 +1201,5 @@ static struct platform_driver sdw_intel_drv = {
> module_platform_driver(sdw_intel_drv);
>
> MODULE_LICENSE("Dual BSD/GPL");
> -MODULE_ALIAS("platform:int-sdw");
> +MODULE_ALIAS("sdw:intel-sdw");
it is still a platform device, so does sdw: tag make sense?
This is used by modprobe to load the driver!
> +/**
> + * sdw_intel_probe() - SoundWire Intel probe routine
> + * @res: resource data
> + *
> + * This creates SoundWire Master and Slave devices below the controller.
I dont think the comment is correct, this is done in intel_master_probe
which is platform device probe...
> + * All the information necessary is stored in the context, and the res
> + * argument pointer can be freed after this step.
> + */
> +struct sdw_intel_ctx
> +*sdw_intel_probe(struct sdw_intel_res *res)
> +{
> + return sdw_intel_probe_controller(res);
> +}
> +EXPORT_SYMBOL(sdw_intel_probe);
I guess this would be called by SOF driver, question is when..?
> +/**
> + * sdw_intel_startup() - SoundWire Intel startup
> + * @ctx: SoundWire context allocated in the probe
> + *
> + */
> +int sdw_intel_startup(struct sdw_intel_ctx *ctx)
> +{
> + return sdw_intel_startup_controller(ctx);
> +}
> +EXPORT_SYMBOL(sdw_intel_startup);
when is this called, pls do document that
--
~Vinod
