[PATCH 1/3] powerpc/64s: Fix get_hugepd_cache_index() build failure

[PATCH 1/3] powerpc/64s: Fix get_hugepd_cache_index() build failure

[Michael Ellerman]

With CONFIG_BUG=n, the 64-bit Book3S build fails with:

  arch/powerpc/include/asm/book3s/64/pgtable-64k.h: In function 'get_hugepd_cache_index':
  arch/powerpc/include/asm/book3s/64/pgtable-64k.h:51:1: error: no return statement in function returning non-void

Currently the body of the function is just BUG(), so when CONFIG_BUG=n
it is an empty function, leading to the error.

get_hugepd_cache_index() should never be called, the only call is behind
an is_hugepd() check, which is always false for this configuration.

Instead mark it as always inline, and change the BUG() to BUILD_BUG().
That should allow the compiler to see that the function is never called,
and therefore that it never returns, fixing the build error.

Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 arch/powerpc/include/asm/book3s/64/pgtable-64k.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/include/asm/book3s/64/pgtable-64k.h b/arch/powerpc/include/asm/book3s/64/pgtable-64k.h
index 2fce3498b000..ced7ee8b42fc 100644
--- a/arch/powerpc/include/asm/book3s/64/pgtable-64k.h
+++ b/arch/powerpc/include/asm/book3s/64/pgtable-64k.h
@@ -45,9 +45,9 @@ static inline int hugepd_ok(hugepd_t hpd)
 /*
  * This should never get called
  */
-static inline int get_hugepd_cache_index(int index)
+static __always_inline int get_hugepd_cache_index(int index)
 {
-	BUG();
+	BUILD_BUG();
 }
 
 #endif /* CONFIG_HUGETLB_PAGE */
-- 
2.43.2

[PATCH 2/3] powerpc/83xx: Fix build failure with FPU=n

[Michael Ellerman]

Building eg. 83xx/mpc832x_rdb_defconfig with FPU=n, fails with:

  arch/powerpc/platforms/83xx/suspend.c: In function 'mpc83xx_suspend_enter':
  arch/powerpc/platforms/83xx/suspend.c:209:17: error: implicit declaration of function 'enable_kernel_fp'
    209 |                 enable_kernel_fp();

Fix it by providing an enable_kernel_fp() stub for FPU=n builds,
which allows using IS_ENABLED(CONFIG_PPC_FPU) around the call to
enable_kernel_fp().

Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 arch/powerpc/include/asm/switch_to.h  | 4 ++++
 arch/powerpc/platforms/83xx/suspend.c | 3 ++-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/include/asm/switch_to.h b/arch/powerpc/include/asm/switch_to.h
index aee25e3ebf96..fc933807ddc8 100644
--- a/arch/powerpc/include/asm/switch_to.h
+++ b/arch/powerpc/include/asm/switch_to.h
@@ -48,6 +48,10 @@ static inline void disable_kernel_fp(void)
 #else
 static inline void save_fpu(struct task_struct *t) { }
 static inline void flush_fp_to_thread(struct task_struct *t) { }
+static inline void enable_kernel_fp(void)
+{
+	BUILD_BUG();
+}
 #endif
 
 #ifdef CONFIG_ALTIVEC
diff --git a/arch/powerpc/platforms/83xx/suspend.c b/arch/powerpc/platforms/83xx/suspend.c
index c9664e46b03d..99bd4355f28e 100644
--- a/arch/powerpc/platforms/83xx/suspend.c
+++ b/arch/powerpc/platforms/83xx/suspend.c
@@ -206,7 +206,8 @@ static int mpc83xx_suspend_enter(suspend_state_t state)
 		out_be32(&pmc_regs->config1,
 		         in_be32(&pmc_regs->config1) | PMCCR1_POWER_OFF);
 
-		enable_kernel_fp();
+		if (IS_ENABLED(CONFIG_PPC_FPU))
+			enable_kernel_fp();
 
 		mpc83xx_enter_deep_sleep(immrbase);
 
-- 
2.43.2

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

[Michael Ellerman]

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.

Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 drivers/macintosh/ams/ams-core.c | 29 ++++++++++++++---------------
 1 file changed, 14 insertions(+), 15 deletions(-)

diff --git a/drivers/macintosh/ams/ams-core.c b/drivers/macintosh/ams/ams-core.c
index c978b4272daa..22d3e6605287 100644
--- a/drivers/macintosh/ams/ams-core.c
+++ b/drivers/macintosh/ams/ams-core.c
@@ -178,25 +178,24 @@ int ams_sensor_attach(void)
 
 static int __init ams_init(void)
 {
-	struct device_node *np;
-
 	spin_lock_init(&ams_info.irq_lock);
 	mutex_init(&ams_info.lock);
 	INIT_WORK(&ams_info.worker, ams_worker);
 
-#ifdef CONFIG_SENSORS_AMS_I2C
-	np = of_find_node_by_name(NULL, "accelerometer");
-	if (np && of_device_is_compatible(np, "AAPL,accelerometer_1"))
-		/* Found I2C motion sensor */
-		return ams_i2c_init(np);
-#endif
-
-#ifdef CONFIG_SENSORS_AMS_PMU
-	np = of_find_node_by_name(NULL, "sms");
-	if (np && of_device_is_compatible(np, "sms"))
-		/* Found PMU motion sensor */
-		return ams_pmu_init(np);
-#endif
+	if (IS_ENABLED(CONFIG_SENSORS_AMS_I2C)) {
+		struct device_node *np = of_find_node_by_name(NULL, "accelerometer");
+		if (np && of_device_is_compatible(np, "AAPL,accelerometer_1"))
+			/* Found I2C motion sensor */
+			return ams_i2c_init(np);
+	}
+
+	if (IS_ENABLED(CONFIG_SENSORS_AMS_PMU)) {
+		struct device_node *np = of_find_node_by_name(NULL, "sms");
+		if (np && of_device_is_compatible(np, "sms"))
+			/* Found PMU motion sensor */
+			return ams_pmu_init(np);
+	}
+
 	return -ENODEV;
 }
 
-- 
2.43.2

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

[Christophe Leroy]

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


> 
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> ---
>   drivers/macintosh/ams/ams-core.c | 29 ++++++++++++++---------------
>   1 file changed, 14 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/macintosh/ams/ams-core.c b/drivers/macintosh/ams/ams-core.c
> index c978b4272daa..22d3e6605287 100644
> --- a/drivers/macintosh/ams/ams-core.c
> +++ b/drivers/macintosh/ams/ams-core.c
> @@ -178,25 +178,24 @@ int ams_sensor_attach(void)
>   
>   static int __init ams_init(void)
>   {
> -	struct device_node *np;
> -
>   	spin_lock_init(&ams_info.irq_lock);
>   	mutex_init(&ams_info.lock);
>   	INIT_WORK(&ams_info.worker, ams_worker);
>   
> -#ifdef CONFIG_SENSORS_AMS_I2C
> -	np = of_find_node_by_name(NULL, "accelerometer");
> -	if (np && of_device_is_compatible(np, "AAPL,accelerometer_1"))
> -		/* Found I2C motion sensor */
> -		return ams_i2c_init(np);
> -#endif
> -
> -#ifdef CONFIG_SENSORS_AMS_PMU
> -	np = of_find_node_by_name(NULL, "sms");
> -	if (np && of_device_is_compatible(np, "sms"))
> -		/* Found PMU motion sensor */
> -		return ams_pmu_init(np);
> -#endif
> +	if (IS_ENABLED(CONFIG_SENSORS_AMS_I2C)) {
> +		struct device_node *np = of_find_node_by_name(NULL, "accelerometer");
> +		if (np && of_device_is_compatible(np, "AAPL,accelerometer_1"))
> +			/* Found I2C motion sensor */
> +			return ams_i2c_init(np);
> +	}
> +
> +	if (IS_ENABLED(CONFIG_SENSORS_AMS_PMU)) {
> +		struct device_node *np = of_find_node_by_name(NULL, "sms");
> +		if (np && of_device_is_compatible(np, "sms"))
> +			/* Found PMU motion sensor */
> +			return ams_pmu_init(np);
> +	}
> +
>   	return -ENODEV;
>   }
>   

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

[Michael Ellerman]

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?

cheers

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

[Christophe Leroy]

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

Re: (subset) [PATCH 1/3] powerpc/64s: Fix get_hugepd_cache_index() build failure

[Michael Ellerman]

On Wed, 06 Mar 2024 23:58:51 +1100, Michael Ellerman wrote:
> With CONFIG_BUG=n, the 64-bit Book3S build fails with:
> 
>   arch/powerpc/include/asm/book3s/64/pgtable-64k.h: In function 'get_hugepd_cache_index':
>   arch/powerpc/include/asm/book3s/64/pgtable-64k.h:51:1: error: no return statement in function returning non-void
> 
> Currently the body of the function is just BUG(), so when CONFIG_BUG=n
> it is an empty function, leading to the error.
> 
> [...]

Patches 1 & 2 applied to powerpc/next.

[1/3] powerpc/64s: Fix get_hugepd_cache_index() build failure
      https://git.kernel.org/powerpc/c/329105ce53437ff64b29f6c429dfe5dc2aa7b676
[2/3] powerpc/83xx: Fix build failure with FPU=n
      https://git.kernel.org/powerpc/c/c2e5d70cf05b48bfbd5b6625bbd0ec3052cecd5d

cheers