[PATCH] mdio: use inline functions for to_mdio_device() etc

[PATCH] mdio: use inline functions for to_mdio_device() etc

[Arnd Bergmann]

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.")
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)
+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)
-- 
2.27.0

Re: [PATCH] mdio: use inline functions for to_mdio_device() etc

[Andrew Lunn]

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

Re: [PATCH] mdio: use inline functions for to_mdio_device() etc

[Jakub Kicinski]

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)

Re: [PATCH] mdio: use inline functions for to_mdio_device() etc

[Arnd Bergmann]

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