-----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