Re: [PATCH 3/3] macintosh/ams: Fix unused variable warning

[Christophe Leroy <christophe.leroy@csgroup.eu>]

Le 07/03/2024 à 06:32, Michael Ellerman a écrit :
> Christophe Leroy <christophe.leroy@csgroup.eu> writes:
>> Le 06/03/2024 à 13:58, Michael Ellerman a écrit :
>>> If both CONFIG_SENSORS_AMS_PMU and CONFIG_SENSORS_AMS_I2C are unset,
>>> there is an unused variable warning in the ams driver:
>>>
>>>     drivers/macintosh/ams/ams-core.c: In function 'ams_init':
>>>     drivers/macintosh/ams/ams-core.c:181:29: warning: unused variable 'np'
>>>       181 |         struct device_node *np;
>>>
>>> Fix it by using IS_ENABLED() to create a block for each case, and move
>>> the variable declartion in there.
>>>
>>> Probably the dependencies should be changed so that the driver can't be
>>> built with both variants disabled, but that would be a larger change.
>>
>> Can be done easily that way I think:
>>
>> diff --git a/drivers/macintosh/Kconfig b/drivers/macintosh/Kconfig
>> index a0e717a986dc..fb38f684444f 100644
>> --- a/drivers/macintosh/Kconfig
>> +++ b/drivers/macintosh/Kconfig
>> @@ -262,7 +262,7 @@ config SENSORS_AMS
>>    	  will be called ams.
>>
>>    config SENSORS_AMS_PMU
>> -	bool "PMU variant"
>> +	bool "PMU variant" if SENSORS_AMS_I2C
>>    	depends on SENSORS_AMS && ADB_PMU
>>    	default y
>>    	help
> 
> Thanks. It's a little clunky. For example if you answer no to both
> prompts, it still selects SENSORS_AMS_PMU, but I guess it doesn't really
> matter.
> 
>    $ make oldconfig
>    ...
>      Apple Motion Sensor driver (SENSORS_AMS) [N/m/y/?] (NEW) y
>        PMU variant (SENSORS_AMS_PMU) [Y/n/?] (NEW) n
>        I2C variant (SENSORS_AMS_I2C) [Y/n/?] (NEW) n
>    #
>    # configuration written to .config
>    #
>    make[1]: Leaving directory '/home/michael/linux/.build'
>    
>    $ grep SENSORS_AMS .build/.config
>    CONFIG_SENSORS_AMS=y
>    CONFIG_SENSORS_AMS_PMU=y
>    # CONFIG_SENSORS_AMS_I2C is not set
> 
> 
> I'll turn to this into a patch and add your SoB?

That's fine for me.

You can alternatively use Suggested-by: , I don't really mind.

Thanks
Christophe