linux-kernel - Re: [PATCH] soundwire: intel: move to auxiliary bus

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 23 Mar 2021 08:37:56 +0100
From:   Greg KH <gregkh@...uxfoundation.org>
To:     Vinod Koul <vkoul@...nel.org>
Cc:     Bard Liao <yung-chuan.liao@...ux.intel.com>,
        alsa-devel@...a-project.org, linux-kernel@...r.kernel.org,
        srinivas.kandagatla@...aro.org, rander.wang@...ux.intel.com,
        hui.wang@...onical.com, pierre-louis.bossart@...ux.intel.com,
        sanyog.r.kale@...el.com, bard.liao@...el.com
Subject: Re: [PATCH] soundwire: intel: move to auxiliary bus

On Tue, Mar 23, 2021 at 12:18:46PM +0530, Vinod Koul wrote:
> On 23-03-21, 08:43, Bard Liao wrote:
> > From: Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com>
> > 
> > Now that the auxiliary_bus exists, there's no reason to use platform
> > devices as children of a PCI device any longer.
> > 
> > This patch refactors the code by extending a basic auxiliary device
> > with Intel link-specific structures that need to be passed between
> > controller and link levels. This refactoring is much cleaner with no
> > need for cross-pointers between device and link structures.
> > 
> > Note that the auxiliary bus API has separate init and add steps, which
> > requires more attention in the error unwinding paths. The main loop
> > needs to deal with kfree() and auxiliary_device_uninit() for the
> > current iteration before jumping to the common label which releases
> > everything allocated in prior iterations.
> > 
> > Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com>
> > Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@...ux.intel.com>
> > Reviewed-by: Ranjani Sridharan <ranjani.sridharan@...ux.intel.com>
> > Signed-off-by: Bard Liao <yung-chuan.liao@...ux.intel.com>
> > ---
> >  drivers/soundwire/Kconfig           |   1 +
> >  drivers/soundwire/intel.c           |  52 ++++----
> >  drivers/soundwire/intel.h           |  14 +-
> >  drivers/soundwire/intel_init.c      | 190 +++++++++++++++++++---------
> >  include/linux/soundwire/sdw_intel.h |   6 +-
> >  5 files changed, 175 insertions(+), 88 deletions(-)
> > 
> > diff --git a/drivers/soundwire/Kconfig b/drivers/soundwire/Kconfig
> > index 016e74230bb7..2b7795233282 100644
> > --- a/drivers/soundwire/Kconfig
> > +++ b/drivers/soundwire/Kconfig
> > @@ -25,6 +25,7 @@ config SOUNDWIRE_INTEL
> >  	tristate "Intel SoundWire Master driver"
> >  	select SOUNDWIRE_CADENCE
> >  	select SOUNDWIRE_GENERIC_ALLOCATION
> > +	select AUXILIARY_BUS
> >  	depends on ACPI && SND_SOC
> >  	help
> >  	  SoundWire Intel Master driver.
> > diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
> > index d2254ee2fee2..039a101982c9 100644
> > --- a/drivers/soundwire/intel.c
> > +++ b/drivers/soundwire/intel.c
> > @@ -11,7 +11,7 @@
> >  #include <linux/module.h>
> >  #include <linux/interrupt.h>
> >  #include <linux/io.h>
> > -#include <linux/platform_device.h>
> > +#include <linux/auxiliary_bus.h>
> >  #include <sound/pcm_params.h>
> >  #include <linux/pm_runtime.h>
> >  #include <sound/soc.h>
> > @@ -1331,9 +1331,10 @@ static int intel_init(struct sdw_intel *sdw)
> >  /*
> >   * probe and init
> >   */
> > -static int intel_master_probe(struct platform_device *pdev)
> > +static int intel_link_probe(struct auxiliary_device *auxdev, const struct auxiliary_device_id *id)
> >  {
> > -	struct device *dev = &pdev->dev;
> > +	struct device *dev = &auxdev->dev;
> > +	struct sdw_intel_link_dev *ldev = auxiliary_dev_to_sdw_intel_link_dev(auxdev);
> 
> Do we need another abstractions for resources here, why not aux dev
> creation set the resources required and we skip this step...
> 
> >  	struct sdw_intel *sdw;
> >  	struct sdw_cdns *cdns;
> >  	struct sdw_bus *bus;
> > @@ -1346,14 +1347,14 @@ static int intel_master_probe(struct platform_device *pdev)
> >  	cdns = &sdw->cdns;
> >  	bus = &cdns->bus;
> >  
> > -	sdw->instance = pdev->id;
> > -	sdw->link_res = dev_get_platdata(dev);
> > +	sdw->instance = auxdev->id;
> 
> so auxdev has id and still we pass id as argument :( Not sure if folks
> can fix this now

That's odd, yeah, it should be fixed.

greg k-h