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@xxxxxxxx>
> > >
> > > 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@xxxxxxxx>
> > > ---
> > >    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