diff mbox series

[1/2] regmap: soundwire: fix Kconfig select/depend issue

Message ID 20190411192814.10663-2-pierre-louis.bossart@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series soundwire: fix Kconfig select/depend issues | expand

Commit Message

Pierre-Louis Bossart April 11, 2019, 7:28 p.m. UTC
The mechanism should be

config CODEC_XYX_SDW
       depends on SOUNDWIRE
       select REGMAP_SOUNDWIRE

config REGMAP_SOUNDWIRE
       depends on SOUNDWIRE
       select SOUNDWIRE_BUS

SOUNDWIRE_BUS can be independently selected by the SOC driver. The SOC
driver should not know or care about REGMAP_SOUNDWIRE.

Fixes: 7c22ce6e2184 ('03fc8746f7915b5a391d8227f7e1')
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 drivers/base/regmap/Kconfig | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Takashi Iwai April 12, 2019, 8:38 a.m. UTC | #1
On Thu, 11 Apr 2019 21:28:13 +0200,
Pierre-Louis Bossart wrote:
> 
> The mechanism should be
> 
> config CODEC_XYX_SDW
>        depends on SOUNDWIRE
>        select REGMAP_SOUNDWIRE
> 
> config REGMAP_SOUNDWIRE
>        depends on SOUNDWIRE
>        select SOUNDWIRE_BUS

To be noted, in general you can't do put both depends-on and select.
The select always wins.  So the depends-on in REGMAP_SOUNDWIRE is more
or less moot.


thanks,

Takashi

> SOUNDWIRE_BUS can be independently selected by the SOC driver. The SOC
> driver should not know or care about REGMAP_SOUNDWIRE.
> 
> Fixes: 7c22ce6e2184 ('03fc8746f7915b5a391d8227f7e1')
> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> ---
>  drivers/base/regmap/Kconfig | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/base/regmap/Kconfig b/drivers/base/regmap/Kconfig
> index 6ad5ef48b61e..4e422afe3c0d 100644
> --- a/drivers/base/regmap/Kconfig
> +++ b/drivers/base/regmap/Kconfig
> @@ -44,7 +44,8 @@ config REGMAP_IRQ
>  
>  config REGMAP_SOUNDWIRE
>  	tristate
> -	depends on SOUNDWIRE_BUS
> +	depends on SOUNDWIRE
> +	select SOUNDWIRE_BUS
>  
>  config REGMAP_SCCB
>  	tristate
> -- 
> 2.17.1
>
Mark Brown April 12, 2019, 8:46 a.m. UTC | #2
On Fri, Apr 12, 2019 at 10:38:19AM +0200, Takashi Iwai wrote:
> Pierre-Louis Bossart wrote:

> > config REGMAP_SOUNDWIRE
> >        depends on SOUNDWIRE
> >        select SOUNDWIRE_BUS

> To be noted, in general you can't do put both depends-on and select.
> The select always wins.  So the depends-on in REGMAP_SOUNDWIRE is more
> or less moot.

To be clear the issue is that the dependencies of selected symbols just
get ignored (IIRC their selects too).  It can be useful for
documentation but it does get a bit confusing sometimes.
Pierre-Louis Bossart April 12, 2019, 2:07 p.m. UTC | #3
Thanks for the reviews

>> The mechanism should be
>>
>> config CODEC_XYX_SDW
>>         depends on SOUNDWIRE
>>         select REGMAP_SOUNDWIRE
>>
>> config REGMAP_SOUNDWIRE
>>         depends on SOUNDWIRE
>>         select SOUNDWIRE_BUS
> 
> To be noted, in general you can't do put both depends-on and select.
> The select always wins.  So the depends-on in REGMAP_SOUNDWIRE is more
> or less moot.


ok. To double-check, the example below would be legit, yes?

  config CODEC_XYX_SDW
	 tristate "XYZ SDW Codec"
          depends on SOUNDWIRE
          select REGMAP_SOUNDWIRE

  config REGMAP_SOUNDWIRE
          select SOUNDWIRE_BUS

it'd follow the typical pattern seen in sound/soc/codecs/Kconfig:

config SND_SOC_CS4265
	tristate "Cirrus Logic CS4265 CODEC"
	depends on I2C
	select REGMAP_I2C

Thanks
-Pierre
Mark Brown April 12, 2019, 2:18 p.m. UTC | #4
On Fri, Apr 12, 2019 at 09:07:41AM -0500, Pierre-Louis Bossart wrote:

>  config CODEC_XYX_SDW
> 	 tristate "XYZ SDW Codec"
>          depends on SOUNDWIRE
>          select REGMAP_SOUNDWIRE

That looks good.

>  config REGMAP_SOUNDWIRE
>          select SOUNDWIRE_BUS

> it'd follow the typical pattern seen in sound/soc/codecs/Kconfig:

IIRC the select here won't actually do anything.
Takashi Iwai April 12, 2019, 2:21 p.m. UTC | #5
On Fri, 12 Apr 2019 16:18:41 +0200,
Mark Brown wrote:
> 
> On Fri, Apr 12, 2019 at 09:07:41AM -0500, Pierre-Louis Bossart wrote:
> 
> >  config CODEC_XYX_SDW
> > 	 tristate "XYZ SDW Codec"
> >          depends on SOUNDWIRE
> >          select REGMAP_SOUNDWIRE
> 
> That looks good.
> 
> >  config REGMAP_SOUNDWIRE
> >          select SOUNDWIRE_BUS
> 
> > it'd follow the typical pattern seen in sound/soc/codecs/Kconfig:
> 
> IIRC the select here won't actually do anything.

I thought it'd do select SOUNDWIRE_BUS.  The depends-on here would be
ignored instead.


Takashi
Vinod Koul April 14, 2019, 10:20 a.m. UTC | #6
On 12-04-19, 16:21, Takashi Iwai wrote:
> On Fri, 12 Apr 2019 16:18:41 +0200,
> Mark Brown wrote:
> > 
> > On Fri, Apr 12, 2019 at 09:07:41AM -0500, Pierre-Louis Bossart wrote:
> > 
> > >  config CODEC_XYX_SDW
> > > 	 tristate "XYZ SDW Codec"
> > >          depends on SOUNDWIRE
> > >          select REGMAP_SOUNDWIRE
> > 
> > That looks good.
> > 
> > >  config REGMAP_SOUNDWIRE
> > >          select SOUNDWIRE_BUS
> > 
> > > it'd follow the typical pattern seen in sound/soc/codecs/Kconfig:
> > 
> > IIRC the select here won't actually do anything.
> 
> I thought it'd do select SOUNDWIRE_BUS.  The depends-on here would be
> ignored instead.

yeah this all is bit complicated and should not be so. As Srini pointed
out, we have two symbols, SOUNDWIRE as a menuconfig item which enables
the subsystem and then SOUNDWIRE_BUS which compiles in the code.

Unfortunately I dont seem to recall the advantages of this approach so
it might be easier to drop SOUNDWIRE_BUS and then let codecs do select
REGMAP_SOUNDWIRE and depends on SOUNDWIRE

Thanks
diff mbox series

Patch

diff --git a/drivers/base/regmap/Kconfig b/drivers/base/regmap/Kconfig
index 6ad5ef48b61e..4e422afe3c0d 100644
--- a/drivers/base/regmap/Kconfig
+++ b/drivers/base/regmap/Kconfig
@@ -44,7 +44,8 @@  config REGMAP_IRQ
 
 config REGMAP_SOUNDWIRE
 	tristate
-	depends on SOUNDWIRE_BUS
+	depends on SOUNDWIRE
+	select SOUNDWIRE_BUS
 
 config REGMAP_SCCB
 	tristate