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

on 2025/2/26 14:49, Arnd Bergmann wrote:

On Wed, Feb 26, 2025, at 04:21, Jijie Shao wrote:

quoted

on 2025/2/26 0:33, Arnd Bergmann wrote:

quoted

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