Messages in this thread | | | From | "Liao, Bard" <> | Subject | RE: [PATCH 2/2] soundwire: intel: transition to 3 steps initialization | Date | Thu, 21 May 2020 02:23:54 +0000 |
| |
> -----Original Message----- > From: Vinod Koul <vkoul@kernel.org> > Sent: Wednesday, May 20, 2020 9:54 PM > To: Bard Liao <yung-chuan.liao@linux.intel.com> > Cc: alsa-devel@alsa-project.org; linux-kernel@vger.kernel.org; tiwai@suse.de; > broonie@kernel.org; gregkh@linuxfoundation.org; jank@cadence.com; > srinivas.kandagatla@linaro.org; rander.wang@linux.intel.com; > ranjani.sridharan@linux.intel.com; hui.wang@canonical.com; pierre- > louis.bossart@linux.intel.com; Kale, Sanyog R <sanyog.r.kale@intel.com>; > Blauciak, Slawomir <slawomir.blauciak@intel.com>; Lin, Mengdong > <mengdong.lin@intel.com>; Liao, Bard <bard.liao@intel.com> > 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 <pierre-louis.bossart@linux.intel.com> > > > > 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, &sdw- > >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
| |