[PATCH] i2c: Make I2C_ATR invisible

Geert Uytterhoeven posted 1 patch 2 years, 4 months ago
drivers/i2c/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH] i2c: Make I2C_ATR invisible
Posted by Geert Uytterhoeven 2 years, 4 months ago
I2C Address Translator (ATR) support is not a stand-alone driver, but a
library.  All of its users select I2C_ATR.  Hence there is no need for
the user to enable this symbol manually, except when compile-testing.

Fixes: a076a860acae77bb ("media: i2c: add I2C Address Translator (ATR) support")
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
Do we care yet about out-of-tree drivers that need this functionality?
---
 drivers/i2c/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/i2c/Kconfig b/drivers/i2c/Kconfig
index c6d1a345ea6d8aee..9388823bb0bb960c 100644
--- a/drivers/i2c/Kconfig
+++ b/drivers/i2c/Kconfig
@@ -72,7 +72,7 @@ config I2C_MUX
 source "drivers/i2c/muxes/Kconfig"
 
 config I2C_ATR
-	tristate "I2C Address Translator (ATR) support"
+	tristate "I2C Address Translator (ATR) support" if COMPILE_TEST
 	help
 	  Enable support for I2C Address Translator (ATR) chips.
 
-- 
2.34.1
Re: [PATCH] i2c: Make I2C_ATR invisible
Posted by Wolfram Sang 2 years, 3 months ago
On Tue, Aug 15, 2023 at 05:29:11PM +0200, Geert Uytterhoeven wrote:
> I2C Address Translator (ATR) support is not a stand-alone driver, but a
> library.  All of its users select I2C_ATR.  Hence there is no need for
> the user to enable this symbol manually, except when compile-testing.
> 
> Fixes: a076a860acae77bb ("media: i2c: add I2C Address Translator (ATR) support")
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

Fixed checkpatch warning:

WARNING: Please use correct Fixes: style 'Fixes: <12 chars of sha1> ("<title line>")' - ie: 'Fixes: a076a860acae ("media: i2c: add I2C Address Translator (ATR) support")'

and applied to for-current, thanks!

Re: [PATCH] i2c: Make I2C_ATR invisible
Posted by Wolfram Sang 2 years, 3 months ago
On Tue, Aug 15, 2023 at 05:29:11PM +0200, Geert Uytterhoeven wrote:
> I2C Address Translator (ATR) support is not a stand-alone driver, but a
> library.  All of its users select I2C_ATR.  Hence there is no need for
> the user to enable this symbol manually, except when compile-testing.
> 
> Fixes: a076a860acae77bb ("media: i2c: add I2C Address Translator (ATR) support")
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

Acked-by: Wolfram Sang <wsa@kernel.org>

I like it but I can only apply it once the ATR hits upstream. Or it goes
via the same tree than I2C_ATR. I don't mind which way.

Re: [PATCH] i2c: Make I2C_ATR invisible
Posted by Luca Ceresoli 2 years, 4 months ago
Hi Geert,

On Tue, 15 Aug 2023 17:29:11 +0200
Geert Uytterhoeven <geert+renesas@glider.be> wrote:

> I2C Address Translator (ATR) support is not a stand-alone driver, but a
> library.  All of its users select I2C_ATR.  Hence there is no need for
> the user to enable this symbol manually, except when compile-testing.
> 
> Fixes: a076a860acae77bb ("media: i2c: add I2C Address Translator (ATR) support")
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> Do we care yet about out-of-tree drivers that need this functionality?
> ---
>  drivers/i2c/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/i2c/Kconfig b/drivers/i2c/Kconfig
> index c6d1a345ea6d8aee..9388823bb0bb960c 100644
> --- a/drivers/i2c/Kconfig
> +++ b/drivers/i2c/Kconfig
> @@ -72,7 +72,7 @@ config I2C_MUX
>  source "drivers/i2c/muxes/Kconfig"
>  
>  config I2C_ATR
> -	tristate "I2C Address Translator (ATR) support"
> +	tristate "I2C Address Translator (ATR) support" if COMPILE_TEST

Either as-is, or with an anonymous tristate:

Reviewed-by: Luca Ceresoli <luca.ceresoli@bootlin.com>

Luca

-- 
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Re: [PATCH] i2c: Make I2C_ATR invisible
Posted by Tomi Valkeinen 2 years, 4 months ago
On 17/08/2023 10:45, Luca Ceresoli wrote:
> Hi Geert,
> 
> On Tue, 15 Aug 2023 17:29:11 +0200
> Geert Uytterhoeven <geert+renesas@glider.be> wrote:
> 
>> I2C Address Translator (ATR) support is not a stand-alone driver, but a
>> library.  All of its users select I2C_ATR.  Hence there is no need for
>> the user to enable this symbol manually, except when compile-testing.
>>
>> Fixes: a076a860acae77bb ("media: i2c: add I2C Address Translator (ATR) support")
>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>> ---
>> Do we care yet about out-of-tree drivers that need this functionality?
>> ---
>>   drivers/i2c/Kconfig | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/i2c/Kconfig b/drivers/i2c/Kconfig
>> index c6d1a345ea6d8aee..9388823bb0bb960c 100644
>> --- a/drivers/i2c/Kconfig
>> +++ b/drivers/i2c/Kconfig
>> @@ -72,7 +72,7 @@ config I2C_MUX
>>   source "drivers/i2c/muxes/Kconfig"
>>   
>>   config I2C_ATR
>> -	tristate "I2C Address Translator (ATR) support"
>> +	tristate "I2C Address Translator (ATR) support" if COMPILE_TEST
> 
> Either as-is, or with an anonymous tristate:
> 
> Reviewed-by: Luca Ceresoli <luca.ceresoli@bootlin.com>

Yes, I'm also fine either way:

Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>

  Tomi
Re: [PATCH] i2c: Make I2C_ATR invisible
Posted by Tomi Valkeinen 2 years, 4 months ago
On 15/08/2023 18:29, Geert Uytterhoeven wrote:
> I2C Address Translator (ATR) support is not a stand-alone driver, but a
> library.  All of its users select I2C_ATR.  Hence there is no need for
> the user to enable this symbol manually, except when compile-testing.
> 
> Fixes: a076a860acae77bb ("media: i2c: add I2C Address Translator (ATR) support")
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> Do we care yet about out-of-tree drivers that need this functionality?
> ---
>   drivers/i2c/Kconfig | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/i2c/Kconfig b/drivers/i2c/Kconfig
> index c6d1a345ea6d8aee..9388823bb0bb960c 100644
> --- a/drivers/i2c/Kconfig
> +++ b/drivers/i2c/Kconfig
> @@ -72,7 +72,7 @@ config I2C_MUX
>   source "drivers/i2c/muxes/Kconfig"
>   
>   config I2C_ATR
> -	tristate "I2C Address Translator (ATR) support"
> +	tristate "I2C Address Translator (ATR) support" if COMPILE_TEST
>   	help
>   	  Enable support for I2C Address Translator (ATR) chips.
>   

Isn't this normally done with just "tristate", without the text? Is 
there a need to make configs manually selectable when compile-test is 
enabled?

  Tomi
Re: [PATCH] i2c: Make I2C_ATR invisible
Posted by Geert Uytterhoeven 2 years, 4 months ago
Hi Tomi,

On Tue, Aug 15, 2023 at 6:00 PM Tomi Valkeinen
<tomi.valkeinen@ideasonboard.com> wrote:
> On 15/08/2023 18:29, Geert Uytterhoeven wrote:
> > I2C Address Translator (ATR) support is not a stand-alone driver, but a
> > library.  All of its users select I2C_ATR.  Hence there is no need for
> > the user to enable this symbol manually, except when compile-testing.
> >
> > Fixes: a076a860acae77bb ("media: i2c: add I2C Address Translator (ATR) support")
> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > ---
> > Do we care yet about out-of-tree drivers that need this functionality?
> > ---
> >   drivers/i2c/Kconfig | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/i2c/Kconfig b/drivers/i2c/Kconfig
> > index c6d1a345ea6d8aee..9388823bb0bb960c 100644
> > --- a/drivers/i2c/Kconfig
> > +++ b/drivers/i2c/Kconfig
> > @@ -72,7 +72,7 @@ config I2C_MUX
> >   source "drivers/i2c/muxes/Kconfig"
> >
> >   config I2C_ATR
> > -     tristate "I2C Address Translator (ATR) support"
> > +     tristate "I2C Address Translator (ATR) support" if COMPILE_TEST
> >       help
> >         Enable support for I2C Address Translator (ATR) chips.
> >
>
> Isn't this normally done with just "tristate", without the text? Is
> there a need to make configs manually selectable when compile-test is
> enabled?

"tristate" without the text would make the symbol invisible, too.
However, then the user has no way to enable it for compile-testing
(unless also enabling one of the symbols that select it, which may
 not be possible due to other dependencies).

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
Re: [PATCH] i2c: Make I2C_ATR invisible
Posted by Tomi Valkeinen 2 years, 4 months ago
Hi Geert,

On 16/08/2023 11:17, Geert Uytterhoeven wrote:
> Hi Tomi,
> 
> On Tue, Aug 15, 2023 at 6:00 PM Tomi Valkeinen
> <tomi.valkeinen@ideasonboard.com> wrote:
>> On 15/08/2023 18:29, Geert Uytterhoeven wrote:
>>> I2C Address Translator (ATR) support is not a stand-alone driver, but a
>>> library.  All of its users select I2C_ATR.  Hence there is no need for
>>> the user to enable this symbol manually, except when compile-testing.
>>>
>>> Fixes: a076a860acae77bb ("media: i2c: add I2C Address Translator (ATR) support")
>>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>>> ---
>>> Do we care yet about out-of-tree drivers that need this functionality?
>>> ---
>>>    drivers/i2c/Kconfig | 2 +-
>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/i2c/Kconfig b/drivers/i2c/Kconfig
>>> index c6d1a345ea6d8aee..9388823bb0bb960c 100644
>>> --- a/drivers/i2c/Kconfig
>>> +++ b/drivers/i2c/Kconfig
>>> @@ -72,7 +72,7 @@ config I2C_MUX
>>>    source "drivers/i2c/muxes/Kconfig"
>>>
>>>    config I2C_ATR
>>> -     tristate "I2C Address Translator (ATR) support"
>>> +     tristate "I2C Address Translator (ATR) support" if COMPILE_TEST
>>>        help
>>>          Enable support for I2C Address Translator (ATR) chips.
>>>
>>
>> Isn't this normally done with just "tristate", without the text? Is
>> there a need to make configs manually selectable when compile-test is
>> enabled?
> 
> "tristate" without the text would make the symbol invisible, too.
> However, then the user has no way to enable it for compile-testing
> (unless also enabling one of the symbols that select it, which may
>   not be possible due to other dependencies).

Yes. My point/question is, i2c-atr isn't different than any other 
selectable kconfig (afaics), like, say, DRM_KMS_HELPER. So is the 
"official" way (if there is such a thing) to add selectable kconfigs 
with just "tristate", or tristate with "if COMPILE_TEST". I thought it 
was the former, but I can see value with the latter too.

  Tomi