[PATCH 1/2] net: hisilicon: hns_mdio: remove incorrect ACPI_PTR annotation

[PATCH 1/2] net: hisilicon: hns_mdio: remove incorrect ACPI_PTR annotation

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

Building with W=1 shows a warning about hns_mdio_acpi_match being unused when
CONFIG_ACPI is disabled:

drivers/net/ethernet/hisilicon/hns_mdio.c:631:36: error: unused variable 'hns_mdio_acpi_match' [-Werror,-Wunused-const-variable]

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/net/ethernet/hisilicon/hns_mdio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/hisilicon/hns_mdio.c b/drivers/net/ethernet/hisilicon/hns_mdio.c
index a1aa6c1f966e..6812be8dc64f 100644
--- a/drivers/net/ethernet/hisilicon/hns_mdio.c
+++ b/drivers/net/ethernet/hisilicon/hns_mdio.c
@@ -640,7 +640,7 @@ static struct platform_driver hns_mdio_driver = {
 	.driver = {
 		   .name = MDIO_DRV_NAME,
 		   .of_match_table = hns_mdio_match,
-		   .acpi_match_table = ACPI_PTR(hns_mdio_acpi_match),
+		   .acpi_match_table = hns_mdio_acpi_match,
 		   },
 };
 
-- 
2.39.5

[PATCH 2/2] net: xgene-v2: remove incorrect ACPI_PTR annotation

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

Building with W=1 shows a warning about xge_acpi_match being unused when
CONFIG_ACPI is disabled:

drivers/net/ethernet/apm/xgene-v2/main.c:723:36: error: unused variable 'xge_acpi_match' [-Werror,-Wunused-const-variable]

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/net/ethernet/apm/xgene-v2/main.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/apm/xgene-v2/main.c b/drivers/net/ethernet/apm/xgene-v2/main.c
index 2a91c84aebdb..d7ca847d44c7 100644
--- a/drivers/net/ethernet/apm/xgene-v2/main.c
+++ b/drivers/net/ethernet/apm/xgene-v2/main.c
@@ -9,8 +9,6 @@
 
 #include "main.h"
 
-static const struct acpi_device_id xge_acpi_match[];
-
 static int xge_get_resources(struct xge_pdata *pdata)
 {
 	struct platform_device *pdev;
@@ -731,7 +729,7 @@ MODULE_DEVICE_TABLE(acpi, xge_acpi_match);
 static struct platform_driver xge_driver = {
 	.driver = {
 		   .name = "xgene-enet-v2",
-		   .acpi_match_table = ACPI_PTR(xge_acpi_match),
+		   .acpi_match_table = xge_acpi_match,
 	},
 	.probe = xge_probe,
 	.remove = xge_remove,
-- 
2.39.5

Re: [PATCH 1/2] net: hisilicon: hns_mdio: remove incorrect ACPI_PTR annotation

[Jijie Shao]

on 2025/2/26 0:33, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
>
> Building with W=1 shows a warning about hns_mdio_acpi_match being unused when
> CONFIG_ACPI is disabled:
>
> drivers/net/ethernet/hisilicon/hns_mdio.c:631:36: error: unused variable 'hns_mdio_acpi_match' [-Werror,-Wunused-const-variable]
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>   drivers/net/ethernet/hisilicon/hns_mdio.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/hisilicon/hns_mdio.c b/drivers/net/ethernet/hisilicon/hns_mdio.c
> index a1aa6c1f966e..6812be8dc64f 100644
> --- a/drivers/net/ethernet/hisilicon/hns_mdio.c
> +++ b/drivers/net/ethernet/hisilicon/hns_mdio.c
> @@ -640,7 +640,7 @@ static struct platform_driver hns_mdio_driver = {
>   	.driver = {
>   		   .name = MDIO_DRV_NAME,
>   		   .of_match_table = hns_mdio_match,
> -		   .acpi_match_table = ACPI_PTR(hns_mdio_acpi_match),
> +		   .acpi_match_table = hns_mdio_acpi_match,
>   		   },
>   };
>   


Thank you.


But I think it can be changed to:

+ #ifdef CONFIG_ACPI
static const struct acpi_device_id hns_mdio_acpi_match[] = {
	{ "HISI0141", 0 },
	{ },
};
MODULE_DEVICE_TABLE(acpi, hns_mdio_acpi_match);
+ #endif

static struct platform_driver hns_mdio_driver = {
	.probe = hns_mdio_probe,
	.remove = hns_mdio_remove,
	.driver = {
		   .name = MDIO_DRV_NAME,
		   .of_match_table = hns_mdio_match,
		   .acpi_match_table = ACPI_PTR(hns_mdio_acpi_match),
		   },
};

Thansk,
Jijie Shao

Re: [PATCH 1/2] net: hisilicon: hns_mdio: remove incorrect ACPI_PTR annotation

[Arnd Bergmann]

On Wed, Feb 26, 2025, at 04:21, Jijie Shao wrote:
> on 2025/2/26 0:33, Arnd Bergmann wrote:
>> From: Arnd Bergmann <arnd@arndb.de>
>>
>> Building with W=1 shows a warning about hns_mdio_acpi_match being unused when
>> CONFIG_ACPI is disabled:
>>
>> drivers/net/ethernet/hisilicon/hns_mdio.c:631:36: error: unused variable 'hns_mdio_acpi_match' [-Werror,-Wunused-const-variable]
>>
>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>> ---
>>   drivers/net/ethernet/hisilicon/hns_mdio.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/ethernet/hisilicon/hns_mdio.c b/drivers/net/ethernet/hisilicon/hns_mdio.c
>> index a1aa6c1f966e..6812be8dc64f 100644
>> --- a/drivers/net/ethernet/hisilicon/hns_mdio.c
>> +++ b/drivers/net/ethernet/hisilicon/hns_mdio.c
>> @@ -640,7 +640,7 @@ static struct platform_driver hns_mdio_driver = {
>>   	.driver = {
>>   		   .name = MDIO_DRV_NAME,
>>   		   .of_match_table = hns_mdio_match,
>> -		   .acpi_match_table = ACPI_PTR(hns_mdio_acpi_match),
>> +		   .acpi_match_table = hns_mdio_acpi_match,
>>   		   },
>>   };
>
>
> But I think it can be changed to:
>
> + #ifdef CONFIG_ACPI
> static const struct acpi_device_id hns_mdio_acpi_match[] = {
> 	{ "HISI0141", 0 },
> 	{ },
> };
> MODULE_DEVICE_TABLE(acpi, hns_mdio_acpi_match);
> + #endif
>

That would of course avoid the build warning, but otherwise
would be worse: the only reason ACPI_PTR()/of_match_ptr() exist
is to work around drivers that have to put their device ID
table inside of an #ifdef for some other reason. Adding the
#ifdef to work around an incorrect ACPI_PTR() makes no sense.

     Arnd

Re: [PATCH 1/2] net: hisilicon: hns_mdio: remove incorrect ACPI_PTR annotation

[Jijie Shao]

on 2025/2/26 14:49, Arnd Bergmann wrote:
> On Wed, Feb 26, 2025, at 04:21, Jijie Shao wrote:
>> on 2025/2/26 0:33, Arnd Bergmann wrote:
>>> From: Arnd Bergmann <arnd@arndb.de>
>>>
>>> Building with W=1 shows a warning about hns_mdio_acpi_match being unused when
>>> CONFIG_ACPI is disabled:
>>>
>>> drivers/net/ethernet/hisilicon/hns_mdio.c:631:36: error: unused variable 'hns_mdio_acpi_match' [-Werror,-Wunused-const-variable]
>>>
>>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>>> ---
>>>    drivers/net/ethernet/hisilicon/hns_mdio.c | 2 +-
>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/net/ethernet/hisilicon/hns_mdio.c b/drivers/net/ethernet/hisilicon/hns_mdio.c
>>> index a1aa6c1f966e..6812be8dc64f 100644
>>> --- a/drivers/net/ethernet/hisilicon/hns_mdio.c
>>> +++ b/drivers/net/ethernet/hisilicon/hns_mdio.c
>>> @@ -640,7 +640,7 @@ static struct platform_driver hns_mdio_driver = {
>>>    	.driver = {
>>>    		   .name = MDIO_DRV_NAME,
>>>    		   .of_match_table = hns_mdio_match,
>>> -		   .acpi_match_table = ACPI_PTR(hns_mdio_acpi_match),
>>> +		   .acpi_match_table = hns_mdio_acpi_match,
>>>    		   },
>>>    };
>>
>> But I think it can be changed to:
>>
>> + #ifdef CONFIG_ACPI
>> static const struct acpi_device_id hns_mdio_acpi_match[] = {
>> 	{ "HISI0141", 0 },
>> 	{ },
>> };
>> MODULE_DEVICE_TABLE(acpi, hns_mdio_acpi_match);
>> + #endif
>>
> That would of course avoid the build warning, but otherwise
> would be worse: the only reason ACPI_PTR()/of_match_ptr() exist
> is to work around drivers that have to put their device ID
> table inside of an #ifdef for some other reason. Adding the
> #ifdef to work around an incorrect ACPI_PTR() makes no sense.
>
>       Arnd

if CONFIG_ACPI is disabled, ACPI_PTR() will return NULL, so
hns_mdio_acpi_match is unused variable.


So use #ifdef is possible and has no side effects, and many drivers do so.



Of course, it also seems possible to remove ACPI_PTR(),
But I'm not sure if it's okay to set a value to acpi_match_table if CONFIG_ACPI is disabled.
It need maintainer to look at this.

Thanks,
Jijie Shao

Re: [PATCH 1/2] net: hisilicon: hns_mdio: remove incorrect ACPI_PTR annotation

[Arnd Bergmann]

On Thu, Feb 27, 2025, at 12:53, Jijie Shao wrote:
>
> if CONFIG_ACPI is disabled, ACPI_PTR() will return NULL, so
> hns_mdio_acpi_match is unused variable.
>
>
> So use #ifdef is possible and has no side effects, and many drivers do so.

Those should be cleaned up eventually, but that is separate from
the build warning.

> Of course, it also seems possible to remove ACPI_PTR(),
> But I'm not sure if it's okay to set a value to acpi_match_table if 
> CONFIG_ACPI is disabled.

Setting .acpi_match_table and .of_match_table unconditionally
is the normal case. Historically we had some drivers that
used of_match_ptr() to assign the .of_match_table in order
to allow drivers to #ifdef out the CONFIG_OF portion of the
driver for platforms that did not already use devicetree
based probing.

There are basically no platforms left that have not been
converted to devicetree yet, so there is no point in
micro-optimizing the kernel size for that case, but the
(mis)use of of_match_ptr() has been copied into drivers
after that, and most of the ACPI_PTR() users unfortunately
copied from that when drivers started supporting both.

     Arnd

Re: [PATCH 1/2] net: hisilicon: hns_mdio: remove incorrect ACPI_PTR annotation

[Paolo Abeni]

On 2/27/25 1:03 PM, Arnd Bergmann wrote:
> On Thu, Feb 27, 2025, at 12:53, Jijie Shao wrote:
>>
>> if CONFIG_ACPI is disabled, ACPI_PTR() will return NULL, so
>> hns_mdio_acpi_match is unused variable.
>>
>>
>> So use #ifdef is possible and has no side effects, and many drivers do so.
> 
> Those should be cleaned up eventually, but that is separate from
> the build warning.
> 
>> Of course, it also seems possible to remove ACPI_PTR(),
>> But I'm not sure if it's okay to set a value to acpi_match_table if 
>> CONFIG_ACPI is disabled.
> 
> Setting .acpi_match_table and .of_match_table unconditionally
> is the normal case. Historically we had some drivers that
> used of_match_ptr() to assign the .of_match_table in order
> to allow drivers to #ifdef out the CONFIG_OF portion of the
> driver for platforms that did not already use devicetree
> based probing.
> 
> There are basically no platforms left that have not been
> converted to devicetree yet, so there is no point in
> micro-optimizing the kernel size for that case, but the
> (mis)use of of_match_ptr() has been copied into drivers
> after that, and most of the ACPI_PTR() users unfortunately
> copied from that when drivers started supporting both.

Makes sense, thanks!

Paolo

Re: [PATCH 1/2] net: hisilicon: hns_mdio: remove incorrect ACPI_PTR annotation

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (main)
by Paolo Abeni <pabeni@redhat.com>:

On Tue, 25 Feb 2025 17:33:32 +0100 you wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> Building with W=1 shows a warning about hns_mdio_acpi_match being unused when
> CONFIG_ACPI is disabled:
> 
> drivers/net/ethernet/hisilicon/hns_mdio.c:631:36: error: unused variable 'hns_mdio_acpi_match' [-Werror,-Wunused-const-variable]
> 
> [...]

Here is the summary with links:
  - [1/2] net: hisilicon: hns_mdio: remove incorrect ACPI_PTR annotation
    https://git.kernel.org/netdev/net-next/c/9355f7277d69
  - [2/2] net: xgene-v2: remove incorrect ACPI_PTR annotation
    https://git.kernel.org/netdev/net-next/c/01358e8fe922

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html