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

[Jijie Shao <shaojijie@huawei.com>]

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