soundwire: fix regmap dependencies and align with other serial links
diff mbox series

Message ID 20190718230215.18675-1-pierre-louis.bossart@linux.intel.com
State New
Headers show
Series
  • soundwire: fix regmap dependencies and align with other serial links
Related show

Commit Message

Pierre-Louis Bossart July 18, 2019, 11:02 p.m. UTC
The existing code has a mixed select/depend usage which makes no sense.

config SOUNDWIRE_BUS
       tristate
       select REGMAP_SOUNDWIRE

config REGMAP_SOUNDWIRE
        tristate
        depends on SOUNDWIRE_BUS

Let's remove one layer of Kconfig definitions and align with the
solutions used by all other serial links.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 drivers/base/regmap/Kconfig | 2 +-
 drivers/soundwire/Kconfig   | 7 +------
 drivers/soundwire/Makefile  | 2 +-
 3 files changed, 3 insertions(+), 8 deletions(-)

Comments

Rafael J. Wysocki July 19, 2019, 9:04 a.m. UTC | #1
On Fri, Jul 19, 2019 at 1:02 AM Pierre-Louis Bossart
<pierre-louis.bossart@linux.intel.com> wrote:
>
> The existing code has a mixed select/depend usage which makes no sense.
>
> config SOUNDWIRE_BUS
>        tristate
>        select REGMAP_SOUNDWIRE
>
> config REGMAP_SOUNDWIRE
>         tristate
>         depends on SOUNDWIRE_BUS
>
> Let's remove one layer of Kconfig definitions and align with the
> solutions used by all other serial links.
>
> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

No issues found:

Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

> ---
>  drivers/base/regmap/Kconfig | 2 +-
>  drivers/soundwire/Kconfig   | 7 +------
>  drivers/soundwire/Makefile  | 2 +-
>  3 files changed, 3 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/base/regmap/Kconfig b/drivers/base/regmap/Kconfig
> index 6ad5ef48b61e..8cd2ac650b50 100644
> --- a/drivers/base/regmap/Kconfig
> +++ b/drivers/base/regmap/Kconfig
> @@ -44,7 +44,7 @@ config REGMAP_IRQ
>
>  config REGMAP_SOUNDWIRE
>         tristate
> -       depends on SOUNDWIRE_BUS
> +       depends on SOUNDWIRE
>
>  config REGMAP_SCCB
>         tristate
> diff --git a/drivers/soundwire/Kconfig b/drivers/soundwire/Kconfig
> index 3a01cfd70fdc..f518273cfbe3 100644
> --- a/drivers/soundwire/Kconfig
> +++ b/drivers/soundwire/Kconfig
> @@ -4,7 +4,7 @@
>  #
>
>  menuconfig SOUNDWIRE
> -       bool "SoundWire support"
> +       tristate "SoundWire support"
>         help
>           SoundWire is a 2-Pin interface with data and clock line ratified
>           by the MIPI Alliance. SoundWire is used for transporting data
> @@ -17,17 +17,12 @@ if SOUNDWIRE
>
>  comment "SoundWire Devices"
>
> -config SOUNDWIRE_BUS
> -       tristate
> -       select REGMAP_SOUNDWIRE
> -
>  config SOUNDWIRE_CADENCE
>         tristate
>
>  config SOUNDWIRE_INTEL
>         tristate "Intel SoundWire Master driver"
>         select SOUNDWIRE_CADENCE
> -       select SOUNDWIRE_BUS
>         depends on X86 && ACPI && SND_SOC
>         help
>           SoundWire Intel Master driver.
> diff --git a/drivers/soundwire/Makefile b/drivers/soundwire/Makefile
> index fd99a831b92a..45b7e5001653 100644
> --- a/drivers/soundwire/Makefile
> +++ b/drivers/soundwire/Makefile
> @@ -5,7 +5,7 @@
>
>  #Bus Objs
>  soundwire-bus-objs := bus_type.o bus.o slave.o mipi_disco.o stream.o
> -obj-$(CONFIG_SOUNDWIRE_BUS) += soundwire-bus.o
> +obj-$(CONFIG_SOUNDWIRE) += soundwire-bus.o
>
>  #Cadence Objs
>  soundwire-cadence-objs := cadence_master.o
> --
> 2.20.1
>
Pierre-Louis Bossart Aug. 7, 2019, 3:09 p.m. UTC | #2
On 7/19/19 4:04 AM, Rafael J. Wysocki wrote:
> On Fri, Jul 19, 2019 at 1:02 AM Pierre-Louis Bossart
> <pierre-louis.bossart@linux.intel.com> wrote:
>>
>> The existing code has a mixed select/depend usage which makes no sense.
>>
>> config SOUNDWIRE_BUS
>>         tristate
>>         select REGMAP_SOUNDWIRE
>>
>> config REGMAP_SOUNDWIRE
>>          tristate
>>          depends on SOUNDWIRE_BUS
>>
>> Let's remove one layer of Kconfig definitions and align with the
>> solutions used by all other serial links.
>>
>> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> 
> No issues found:
> 
> Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Vinod, Mark, any feedback?

There will be a set of SoundWire codec drivers provided upstream soonish 
and we'll get a number of kbuild errors without this patch.

> 
>> ---
>>   drivers/base/regmap/Kconfig | 2 +-
>>   drivers/soundwire/Kconfig   | 7 +------
>>   drivers/soundwire/Makefile  | 2 +-
>>   3 files changed, 3 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/base/regmap/Kconfig b/drivers/base/regmap/Kconfig
>> index 6ad5ef48b61e..8cd2ac650b50 100644
>> --- a/drivers/base/regmap/Kconfig
>> +++ b/drivers/base/regmap/Kconfig
>> @@ -44,7 +44,7 @@ config REGMAP_IRQ
>>
>>   config REGMAP_SOUNDWIRE
>>          tristate
>> -       depends on SOUNDWIRE_BUS
>> +       depends on SOUNDWIRE
>>
>>   config REGMAP_SCCB
>>          tristate
>> diff --git a/drivers/soundwire/Kconfig b/drivers/soundwire/Kconfig
>> index 3a01cfd70fdc..f518273cfbe3 100644
>> --- a/drivers/soundwire/Kconfig
>> +++ b/drivers/soundwire/Kconfig
>> @@ -4,7 +4,7 @@
>>   #
>>
>>   menuconfig SOUNDWIRE
>> -       bool "SoundWire support"
>> +       tristate "SoundWire support"
>>          help
>>            SoundWire is a 2-Pin interface with data and clock line ratified
>>            by the MIPI Alliance. SoundWire is used for transporting data
>> @@ -17,17 +17,12 @@ if SOUNDWIRE
>>
>>   comment "SoundWire Devices"
>>
>> -config SOUNDWIRE_BUS
>> -       tristate
>> -       select REGMAP_SOUNDWIRE
>> -
>>   config SOUNDWIRE_CADENCE
>>          tristate
>>
>>   config SOUNDWIRE_INTEL
>>          tristate "Intel SoundWire Master driver"
>>          select SOUNDWIRE_CADENCE
>> -       select SOUNDWIRE_BUS
>>          depends on X86 && ACPI && SND_SOC
>>          help
>>            SoundWire Intel Master driver.
>> diff --git a/drivers/soundwire/Makefile b/drivers/soundwire/Makefile
>> index fd99a831b92a..45b7e5001653 100644
>> --- a/drivers/soundwire/Makefile
>> +++ b/drivers/soundwire/Makefile
>> @@ -5,7 +5,7 @@
>>
>>   #Bus Objs
>>   soundwire-bus-objs := bus_type.o bus.o slave.o mipi_disco.o stream.o
>> -obj-$(CONFIG_SOUNDWIRE_BUS) += soundwire-bus.o
>> +obj-$(CONFIG_SOUNDWIRE) += soundwire-bus.o
>>
>>   #Cadence Objs
>>   soundwire-cadence-objs := cadence_master.o
>> --
>> 2.20.1
>>
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
>
Mark Brown Aug. 7, 2019, 5:56 p.m. UTC | #3
On Wed, Aug 07, 2019 at 10:09:27AM -0500, Pierre-Louis Bossart wrote:

> Vinod, Mark, any feedback?

> There will be a set of SoundWire codec drivers provided upstream soonish and
> we'll get a number of kbuild errors without this patch.

I think I'm missing context here, I've basically been zoning out all the
soundwire stuff - the patch series are huge and generate a bunch of
discusion.  Is the patch below the full thing?  I don't see any obvious
problems.

> 
> > 
> > > ---
> > >   drivers/base/regmap/Kconfig | 2 +-
> > >   drivers/soundwire/Kconfig   | 7 +------
> > >   drivers/soundwire/Makefile  | 2 +-
> > >   3 files changed, 3 insertions(+), 8 deletions(-)
> > > 
> > > diff --git a/drivers/base/regmap/Kconfig b/drivers/base/regmap/Kconfig
> > > index 6ad5ef48b61e..8cd2ac650b50 100644
> > > --- a/drivers/base/regmap/Kconfig
> > > +++ b/drivers/base/regmap/Kconfig
> > > @@ -44,7 +44,7 @@ config REGMAP_IRQ
> > > 
> > >   config REGMAP_SOUNDWIRE
> > >          tristate
> > > -       depends on SOUNDWIRE_BUS
> > > +       depends on SOUNDWIRE
> > > 
> > >   config REGMAP_SCCB
> > >          tristate
> > > diff --git a/drivers/soundwire/Kconfig b/drivers/soundwire/Kconfig
> > > index 3a01cfd70fdc..f518273cfbe3 100644
> > > --- a/drivers/soundwire/Kconfig
> > > +++ b/drivers/soundwire/Kconfig
> > > @@ -4,7 +4,7 @@
> > >   #
> > > 
> > >   menuconfig SOUNDWIRE
> > > -       bool "SoundWire support"
> > > +       tristate "SoundWire support"
> > >          help
> > >            SoundWire is a 2-Pin interface with data and clock line ratified
> > >            by the MIPI Alliance. SoundWire is used for transporting data
> > > @@ -17,17 +17,12 @@ if SOUNDWIRE
> > > 
> > >   comment "SoundWire Devices"
> > > 
> > > -config SOUNDWIRE_BUS
> > > -       tristate
> > > -       select REGMAP_SOUNDWIRE
> > > -
> > >   config SOUNDWIRE_CADENCE
> > >          tristate
> > > 
> > >   config SOUNDWIRE_INTEL
> > >          tristate "Intel SoundWire Master driver"
> > >          select SOUNDWIRE_CADENCE
> > > -       select SOUNDWIRE_BUS
> > >          depends on X86 && ACPI && SND_SOC
> > >          help
> > >            SoundWire Intel Master driver.
> > > diff --git a/drivers/soundwire/Makefile b/drivers/soundwire/Makefile
> > > index fd99a831b92a..45b7e5001653 100644
> > > --- a/drivers/soundwire/Makefile
> > > +++ b/drivers/soundwire/Makefile
> > > @@ -5,7 +5,7 @@
> > > 
> > >   #Bus Objs
> > >   soundwire-bus-objs := bus_type.o bus.o slave.o mipi_disco.o stream.o
> > > -obj-$(CONFIG_SOUNDWIRE_BUS) += soundwire-bus.o
> > > +obj-$(CONFIG_SOUNDWIRE) += soundwire-bus.o
> > > 
> > >   #Cadence Objs
> > >   soundwire-cadence-objs := cadence_master.o
> > > --
> > > 2.20.1
> > > 
> > _______________________________________________
> > Alsa-devel mailing list
> > Alsa-devel@alsa-project.org
> > https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
> >
Pierre-Louis Bossart Aug. 7, 2019, 6:17 p.m. UTC | #4
Hi Mark,

> 
>> Vinod, Mark, any feedback?
> 
>> There will be a set of SoundWire codec drivers provided upstream soonish and
>> we'll get a number of kbuild errors without this patch.
> 
> I think I'm missing context here, I've basically been zoning out all the
> soundwire stuff - the patch series are huge and generate a bunch of
> discusion.  Is the patch below the full thing?  I don't see any obvious
> problems.

Here's a bit of context:

This patch is really independent from the 40-odd fixes I pushed about 10 
days ago. I provided an initial version back in April ('[PATCH v2 2/2] 
regmap: soundwire: fix Kconfig select/depend issue') during the first 
batch of updates. At the time, the suggested solution for the 
compilation issues was not agreed on, so the build errors remained - not 
a big deal they only show-up with codec drivers that were not upstreamed 
so far. It took me a while to come back to it but that was the first in 
my TODO list after my Summer break and now that we are almost ready to 
upstream those codec drivers it's a good time to revisit this issue.

Your initial feedback was:

"This now makes _SOUNDWIRE different to all the other bus types; if this
is a good change then surely the same thing should be done for all the
other bus types. "

and

"Alignment is a requirement.  If you want to optimize
this then it'd be better to optimize all the bus types rather than just
having the one weird bus type that does something different for no
documented reason."

I don't have the knowledge or means to test what I suggested initially 
for the other buses, and the optimization was minimal anyways, so this 
patch takes the path of least resistance and aligns with others.

if there are no objections it's probably easier to push this patch 
through the SoundWire tree, with the relevant Acks.
Mark Brown Aug. 7, 2019, 7:09 p.m. UTC | #5
On Wed, Aug 07, 2019 at 01:17:20PM -0500, Pierre-Louis Bossart wrote:

> I don't have the knowledge or means to test what I suggested initially for
> the other buses, and the optimization was minimal anyways, so this patch
> takes the path of least resistance and aligns with others.

> if there are no objections it's probably easier to push this patch through
> the SoundWire tree, with the relevant Acks.

Makes sense I think

Acked-by: Mark Brown <broonie@kernel.org>
Srinivas Kandagatla Aug. 8, 2019, 1:48 p.m. UTC | #6
On 19/07/2019 00:02, Pierre-Louis Bossart wrote:
> The existing code has a mixed select/depend usage which makes no sense.
> 
> config SOUNDWIRE_BUS
>         tristate
>         select REGMAP_SOUNDWIRE
> 
> config REGMAP_SOUNDWIRE
>          tristate
>          depends on SOUNDWIRE_BUS
> 
> Let's remove one layer of Kconfig definitions and align with the
> solutions used by all other serial links.
> 
> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> ---
>   drivers/base/regmap/Kconfig | 2 +-
>   drivers/soundwire/Kconfig   | 7 +------
>   drivers/soundwire/Makefile  | 2 +-
>   3 files changed, 3 insertions(+), 8 deletions(-)

I have been using some thing similar in my setup.
I did test this with Qualcomm WSA881x codec.

Tested-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

--srini
Vinod Koul Aug. 9, 2019, 4:51 a.m. UTC | #7
On 18-07-19, 18:02, Pierre-Louis Bossart wrote:
> The existing code has a mixed select/depend usage which makes no sense.
> 
> config SOUNDWIRE_BUS
>        tristate
>        select REGMAP_SOUNDWIRE
> 
> config REGMAP_SOUNDWIRE
>         tristate
>         depends on SOUNDWIRE_BUS
> 
> Let's remove one layer of Kconfig definitions and align with the
> solutions used by all other serial links.

Applied, thanks

Patch
diff mbox series

diff --git a/drivers/base/regmap/Kconfig b/drivers/base/regmap/Kconfig
index 6ad5ef48b61e..8cd2ac650b50 100644
--- a/drivers/base/regmap/Kconfig
+++ b/drivers/base/regmap/Kconfig
@@ -44,7 +44,7 @@  config REGMAP_IRQ
 
 config REGMAP_SOUNDWIRE
 	tristate
-	depends on SOUNDWIRE_BUS
+	depends on SOUNDWIRE
 
 config REGMAP_SCCB
 	tristate
diff --git a/drivers/soundwire/Kconfig b/drivers/soundwire/Kconfig
index 3a01cfd70fdc..f518273cfbe3 100644
--- a/drivers/soundwire/Kconfig
+++ b/drivers/soundwire/Kconfig
@@ -4,7 +4,7 @@ 
 #
 
 menuconfig SOUNDWIRE
-	bool "SoundWire support"
+	tristate "SoundWire support"
 	help
 	  SoundWire is a 2-Pin interface with data and clock line ratified
 	  by the MIPI Alliance. SoundWire is used for transporting data
@@ -17,17 +17,12 @@  if SOUNDWIRE
 
 comment "SoundWire Devices"
 
-config SOUNDWIRE_BUS
-	tristate
-	select REGMAP_SOUNDWIRE
-
 config SOUNDWIRE_CADENCE
 	tristate
 
 config SOUNDWIRE_INTEL
 	tristate "Intel SoundWire Master driver"
 	select SOUNDWIRE_CADENCE
-	select SOUNDWIRE_BUS
 	depends on X86 && ACPI && SND_SOC
 	help
 	  SoundWire Intel Master driver.
diff --git a/drivers/soundwire/Makefile b/drivers/soundwire/Makefile
index fd99a831b92a..45b7e5001653 100644
--- a/drivers/soundwire/Makefile
+++ b/drivers/soundwire/Makefile
@@ -5,7 +5,7 @@ 
 
 #Bus Objs
 soundwire-bus-objs := bus_type.o bus.o slave.o mipi_disco.o stream.o
-obj-$(CONFIG_SOUNDWIRE_BUS) += soundwire-bus.o
+obj-$(CONFIG_SOUNDWIRE) += soundwire-bus.o
 
 #Cadence Objs
 soundwire-cadence-objs := cadence_master.o