| Message ID | 20201026165113.3723159-1-arnd@kernel.org |
|---|---|
| State | New |
| Headers | show |
| Series | mdio: use inline functions for to_mdio_device() etc | expand |
On Mon, Oct 26, 2020 at 05:51:09PM +0100, Arnd Bergmann wrote: > From: Arnd Bergmann <arnd@arndb.de> .. > Redefine the macros in linux/mdio.h as inline functions to avoid this > problem. > > Fixes: a9049e0c513c ("mdio: Add support for mdio drivers.") > Signed-off-by: Arnd Bergmann <arnd@arndb.de> Hi Arnd It is nice to have the additional type checking. Since you added a Fixes: tag, do you want this in stable? netdev puts into the Subject line the tree it is intended for: [PATCH net v1] or [PATCH net-next v1] Anyway: Reviewed-by: Andrew Lunn <andrew@lunn.ch> Andrew
On Mon, 26 Oct 2020 17:51:09 +0100 Arnd Bergmann wrote: > From: Arnd Bergmann <arnd@arndb.de> > > Nesting container_of() causes warnings with W=2, and doing this > in a header means we see a lot of them, like: > > In file included from drivers/net/mdio/of_mdio.c:11: > drivers/net/mdio/of_mdio.c: In function 'of_phy_find_device': > include/linux/kernel.h:852:8: warning: declaration of '__mptr' shadows a previous local [-Wshadow] > 852 | void *__mptr = (void *)(ptr); \ > | ^~~~~~ > include/linux/phy.h:655:26: note: in expansion of macro 'container_of' > 655 | #define to_phy_device(d) container_of(to_mdio_device(d), \ > | ^~~~~~~~~~~~ > include/linux/mdio.h:52:27: note: in expansion of macro 'container_of' > 52 | #define to_mdio_device(d) container_of(d, struct mdio_device, dev) > | ^~~~~~~~~~~~ > include/linux/phy.h:655:39: note: in expansion of macro 'to_mdio_device' > 655 | #define to_phy_device(d) container_of(to_mdio_device(d), \ > | ^~~~~~~~~~~~~~ > drivers/net/mdio/of_mdio.c:379:10: note: in expansion of macro 'to_phy_device' > 379 | return to_phy_device(&mdiodev->dev); > | ^~~~~~~~~~~~~ > include/linux/kernel.h:852:8: note: shadowed declaration is here > 852 | void *__mptr = (void *)(ptr); \ > | ^~~~~~ > include/linux/phy.h:655:26: note: in expansion of macro 'container_of' > 655 | #define to_phy_device(d) container_of(to_mdio_device(d), \ > | ^~~~~~~~~~~~ > drivers/net/mdio/of_mdio.c:379:10: note: in expansion of macro 'to_phy_device' > 379 | return to_phy_device(&mdiodev->dev); > | ^~~~~~~~~~~~~ > > Redefine the macros in linux/mdio.h as inline functions to avoid this > problem. > > Fixes: a9049e0c513c ("mdio: Add support for mdio drivers.") I feel like this is slightly onerous, please drop the tag. Harmless W=2 warnings hardly call for getting this into the cogs of the stable machinery. > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > --- > include/linux/mdio.h | 20 +++++++++++++++----- > 1 file changed, 15 insertions(+), 5 deletions(-) > > diff --git a/include/linux/mdio.h b/include/linux/mdio.h > index dbd69b3d170b..7c059cb8e069 100644 > --- a/include/linux/mdio.h > +++ b/include/linux/mdio.h > @@ -49,7 +49,10 @@ struct mdio_device { > unsigned int reset_assert_delay; > unsigned int reset_deassert_delay; > }; > -#define to_mdio_device(d) container_of(d, struct mdio_device, dev) empty line > +static inline struct mdio_device *to_mdio_device(struct device *dev) > +{ > + return container_of(dev, struct mdio_device, dev); > +} > > /* struct mdio_driver_common: Common to all MDIO drivers */ > struct mdio_driver_common { > @@ -57,8 +60,11 @@ struct mdio_driver_common { > int flags; > }; > #define MDIO_DEVICE_FLAG_PHY 1 > -#define to_mdio_common_driver(d) \ > - container_of(d, struct mdio_driver_common, driver) and here > +static inline struct mdio_driver_common * > +to_mdio_common_driver(struct device_driver *drv) > +{ > + return container_of(drv, struct mdio_driver_common, driver); > +} > > /* struct mdio_driver: Generic MDIO driver */ > struct mdio_driver { > @@ -73,8 +79,12 @@ struct mdio_driver { > /* Clears up any memory if needed */ > void (*remove)(struct mdio_device *mdiodev); > }; > -#define to_mdio_driver(d) \ > - container_of(to_mdio_common_driver(d), struct mdio_driver, mdiodrv) > + > +static inline struct mdio_driver *to_mdio_driver(struct device_driver *drv) > +{ > + struct mdio_driver_common *common = to_mdio_common_driver(drv); and here Otherwise we're trading W=2 warnings for checkpatch warnings :) Thanks! > + return container_of(common, struct mdio_driver, mdiodrv); > +} > > /* device driver data */ > static inline void mdiodev_set_drvdata(struct mdio_device *mdio, void *data)
On Wed, Oct 28, 2020 at 2:16 AM Jakub Kicinski <kuba@kernel.org> wrote: > On Mon, 26 Oct 2020 17:51:09 +0100 Arnd Bergmann wrote: > > > Fixes: a9049e0c513c ("mdio: Add support for mdio drivers.") > > I feel like this is slightly onerous, please drop the tag. > > Harmless W=2 warnings hardly call for getting this into the cogs of > the stable machinery. > ... > > unsigned int reset_deassert_delay; > > }; > > -#define to_mdio_device(d) container_of(d, struct mdio_device, dev) > > empty line I fixed all of those now, plan to resend next week. Thanks for taking a look! Arnd
diff --git a/include/linux/mdio.h b/include/linux/mdio.h index dbd69b3d170b..7c059cb8e069 100644 --- a/include/linux/mdio.h +++ b/include/linux/mdio.h @@ -49,7 +49,10 @@ struct mdio_device { unsigned int reset_assert_delay; unsigned int reset_deassert_delay; }; -#define to_mdio_device(d) container_of(d, struct mdio_device, dev) +static inline struct mdio_device *to_mdio_device(struct device *dev) +{ + return container_of(dev, struct mdio_device, dev); +} /* struct mdio_driver_common: Common to all MDIO drivers */ struct mdio_driver_common { @@ -57,8 +60,11 @@ struct mdio_driver_common { int flags; }; #define MDIO_DEVICE_FLAG_PHY 1 -#define to_mdio_common_driver(d) \ - container_of(d, struct mdio_driver_common, driver) +static inline struct mdio_driver_common * +to_mdio_common_driver(struct device_driver *drv) +{ + return container_of(drv, struct mdio_driver_common, driver); +} /* struct mdio_driver: Generic MDIO driver */ struct mdio_driver { @@ -73,8 +79,12 @@ struct mdio_driver { /* Clears up any memory if needed */ void (*remove)(struct mdio_device *mdiodev); }; -#define to_mdio_driver(d) \ - container_of(to_mdio_common_driver(d), struct mdio_driver, mdiodrv) + +static inline struct mdio_driver *to_mdio_driver(struct device_driver *drv) +{ + struct mdio_driver_common *common = to_mdio_common_driver(drv); + return container_of(common, struct mdio_driver, mdiodrv); +} /* device driver data */ static inline void mdiodev_set_drvdata(struct mdio_device *mdio, void *data)