On 20-05-20, 03:19, Bard Liao wrote: > From: Pierre-Louis Bossart <pierre-louis.bossart@xxxxxxxxxxxxxxx> > > 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, &sdw->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