All of lore.kernel.org
 help / color / mirror / Atom feed
From: Bartosz Golaszewski <brgl@bgdev.pl>
To: Jonathan Corbet <corbet@lwn.net>,
	Jeff Kirsher <jeffrey.t.kirsher@intel.com>,
	"David S . Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>, John Crispin <john@phrozen.org>,
	Sean Wang <sean.wang@mediatek.com>,
	Mark Lee <Mark-MC.Lee@mediatek.com>,
	Matthias Brugger <matthias.bgg@gmail.com>,
	Realtek linux nic maintainers <nic_swsd@realtek.com>,
	Heiner Kallweit <hkallweit1@gmail.com>,
	Andrew Lunn <andrew@lunn.ch>,
	Florian Fainelli <f.fainelli@gmail.com>,
	Russell King <linux@armlinux.org.uk>,
	Rob Herring <robh+dt@kernel.org>,
	Frank Rowand <frowand.list@gmail.com>
Cc: linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
	intel-wired-lan@lists.osuosl.org, netdev@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-mediatek@lists.infradead.org, devicetree@vger.kernel.org,
	Fabien Parent <fparent@baylibre.com>,
	Stephane Le Provost <stephane.leprovost@mediatek.com>,
	Pedro Tsai <pedro.tsai@mediatek.com>,
	Andrew Perepech <andrew.perepech@mediatek.com>,
	Bartosz Golaszewski <bgolaszewski@baylibre.com>
Subject: [PATCH 00/11] net: improve devres helpers
Date: Mon, 22 Jun 2020 12:00:45 +0200	[thread overview]
Message-ID: <20200622100056.10151-1-brgl@bgdev.pl> (raw)

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

When I first submitted the series adding devm_register_netdev() I was
told during review that it should check if the underlying struct net_device
is managed too before proceeding. I initially accepted this as the right
approach but in the back of my head something seemed wrong about this.
I started looking around and noticed how devm_mdiobus_register()
is implemented.

It turned out that struct mii_bus contains information about whether it's
managed or not and the release callback of devm_mdiobus_alloc() is responsible
for calling mdiobus_unregister(). This seems wrong to me as managed structures
shouldn't care about who manages them. It's devres' code task to correctly undo
whatever it registers/allocates.

With this series I propose to make the release callbacks of mdiobus devm
helpers only release the resources they actually allocate themselves as it the
standard in devm routines. I also propose to not check whether the structures
passed to devm_mdiobus_register() and devm_register_netdev() are already
managed as they could have been allocated over devres as part of bigger
memory chunk. I see this as an unnecessary limitation.

First two patches aim at removing the only use of devm_mdiobus_free(). It
modifies the ixgbe driver. I only compile tested it as I don't have the
relevant hw.

Next two patches relax devm_register_netdev() - we stop checking whether
struct net_device was registered using devm_etherdev_alloc().

We then document the mdio devres helper that's missing in devres.rst list
and un-inline the current implementation of devm_mdiobus_register().

Patch 8 re-implements the devres helpers for mdio conforming to common
devres patterns.

Patches 9 and 10 provide devm_of_mdiobus_register() and the last patch
adds its first user.

Bartosz Golaszewski (11):
  net: ethernet: ixgbe: check the return value of ixgbe_mii_bus_init()
  net: ethernet: ixgbe: don't call devm_mdiobus_free()
  net: devres: relax devm_register_netdev()
  net: devres: rename the release callback of devm_register_netdev()
  Documentation: devres: add missing mdio helper
  phy: un-inline devm_mdiobus_register()
  phy: mdio: add kerneldoc for __devm_mdiobus_register()
  net: phy: don't abuse devres in devm_mdiobus_register()
  of: mdio: remove the 'extern' keyword from function declarations
  of: mdio: provide devm_of_mdiobus_register()
  net: ethernet: mtk-star-emac: use devm_of_mdiobus_register()

 .../driver-api/driver-model/devres.rst        |  3 +-
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |  6 +-
 drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c  | 14 +---
 drivers/net/ethernet/mediatek/mtk_star_emac.c | 13 +--
 drivers/net/ethernet/realtek/r8169_main.c     |  2 +-
 drivers/net/phy/Makefile                      |  2 +-
 drivers/net/phy/mdio_bus.c                    | 73 ----------------
 drivers/net/phy/mdio_devres.c                 | 83 +++++++++++++++++++
 drivers/of/of_mdio.c                          | 43 ++++++++++
 include/linux/of_mdio.h                       | 40 ++++-----
 include/linux/phy.h                           | 21 +----
 net/devres.c                                  | 23 +----
 12 files changed, 167 insertions(+), 156 deletions(-)
 create mode 100644 drivers/net/phy/mdio_devres.c

-- 
2.26.1


WARNING: multiple messages have this Message-ID (diff)
From: Bartosz Golaszewski <brgl@bgdev.pl>
To: intel-wired-lan@osuosl.org
Subject: [Intel-wired-lan] [PATCH 00/11] net: improve devres helpers
Date: Mon, 22 Jun 2020 12:00:45 +0200	[thread overview]
Message-ID: <20200622100056.10151-1-brgl@bgdev.pl> (raw)

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

When I first submitted the series adding devm_register_netdev() I was
told during review that it should check if the underlying struct net_device
is managed too before proceeding. I initially accepted this as the right
approach but in the back of my head something seemed wrong about this.
I started looking around and noticed how devm_mdiobus_register()
is implemented.

It turned out that struct mii_bus contains information about whether it's
managed or not and the release callback of devm_mdiobus_alloc() is responsible
for calling mdiobus_unregister(). This seems wrong to me as managed structures
shouldn't care about who manages them. It's devres' code task to correctly undo
whatever it registers/allocates.

With this series I propose to make the release callbacks of mdiobus devm
helpers only release the resources they actually allocate themselves as it the
standard in devm routines. I also propose to not check whether the structures
passed to devm_mdiobus_register() and devm_register_netdev() are already
managed as they could have been allocated over devres as part of bigger
memory chunk. I see this as an unnecessary limitation.

First two patches aim at removing the only use of devm_mdiobus_free(). It
modifies the ixgbe driver. I only compile tested it as I don't have the
relevant hw.

Next two patches relax devm_register_netdev() - we stop checking whether
struct net_device was registered using devm_etherdev_alloc().

We then document the mdio devres helper that's missing in devres.rst list
and un-inline the current implementation of devm_mdiobus_register().

Patch 8 re-implements the devres helpers for mdio conforming to common
devres patterns.

Patches 9 and 10 provide devm_of_mdiobus_register() and the last patch
adds its first user.

Bartosz Golaszewski (11):
  net: ethernet: ixgbe: check the return value of ixgbe_mii_bus_init()
  net: ethernet: ixgbe: don't call devm_mdiobus_free()
  net: devres: relax devm_register_netdev()
  net: devres: rename the release callback of devm_register_netdev()
  Documentation: devres: add missing mdio helper
  phy: un-inline devm_mdiobus_register()
  phy: mdio: add kerneldoc for __devm_mdiobus_register()
  net: phy: don't abuse devres in devm_mdiobus_register()
  of: mdio: remove the 'extern' keyword from function declarations
  of: mdio: provide devm_of_mdiobus_register()
  net: ethernet: mtk-star-emac: use devm_of_mdiobus_register()

 .../driver-api/driver-model/devres.rst        |  3 +-
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |  6 +-
 drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c  | 14 +---
 drivers/net/ethernet/mediatek/mtk_star_emac.c | 13 +--
 drivers/net/ethernet/realtek/r8169_main.c     |  2 +-
 drivers/net/phy/Makefile                      |  2 +-
 drivers/net/phy/mdio_bus.c                    | 73 ----------------
 drivers/net/phy/mdio_devres.c                 | 83 +++++++++++++++++++
 drivers/of/of_mdio.c                          | 43 ++++++++++
 include/linux/of_mdio.h                       | 40 ++++-----
 include/linux/phy.h                           | 21 +----
 net/devres.c                                  | 23 +----
 12 files changed, 167 insertions(+), 156 deletions(-)
 create mode 100644 drivers/net/phy/mdio_devres.c

-- 
2.26.1


             reply	other threads:[~2020-06-22 10:01 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-22 10:00 Bartosz Golaszewski [this message]
2020-06-22 10:00 ` [Intel-wired-lan] [PATCH 00/11] net: improve devres helpers Bartosz Golaszewski
2020-06-22 10:00 ` [PATCH 01/11] net: ethernet: ixgbe: check the return value of ixgbe_mii_bus_init() Bartosz Golaszewski
2020-06-22 10:00   ` [Intel-wired-lan] " Bartosz Golaszewski
2020-06-22 10:00 ` [PATCH 02/11] net: ethernet: ixgbe: don't call devm_mdiobus_free() Bartosz Golaszewski
2020-06-22 10:00   ` [Intel-wired-lan] " Bartosz Golaszewski
2020-06-22 10:00 ` [PATCH 03/11] net: devres: relax devm_register_netdev() Bartosz Golaszewski
2020-06-22 10:00   ` [Intel-wired-lan] " Bartosz Golaszewski
2020-06-22 22:49   ` Jakub Kicinski
2020-06-22 22:49     ` [Intel-wired-lan] " Jakub Kicinski
2020-06-23  9:12     ` Bartosz Golaszewski
2020-06-23  9:12       ` [Intel-wired-lan] " Bartosz Golaszewski
2020-06-23 20:16       ` Jakub Kicinski
2020-06-23 20:16         ` [Intel-wired-lan] " Jakub Kicinski
2020-06-23 20:16         ` Jakub Kicinski
2020-06-23 20:16         ` Jakub Kicinski
2020-06-22 10:00 ` [PATCH 04/11] net: devres: rename the release callback of devm_register_netdev() Bartosz Golaszewski
2020-06-22 10:00   ` [Intel-wired-lan] " Bartosz Golaszewski
2020-06-22 10:00 ` [PATCH 05/11] Documentation: devres: add missing mdio helper Bartosz Golaszewski
2020-06-22 10:00   ` [Intel-wired-lan] " Bartosz Golaszewski
2020-06-22 10:00 ` [PATCH 06/11] phy: un-inline devm_mdiobus_register() Bartosz Golaszewski
2020-06-22 10:00   ` [Intel-wired-lan] " Bartosz Golaszewski
2020-06-22 23:55   ` Florian Fainelli
2020-06-22 23:55     ` [Intel-wired-lan] " Florian Fainelli
2020-06-25  9:16     ` Bartosz Golaszewski
2020-06-25  9:16       ` [Intel-wired-lan] " Bartosz Golaszewski
2020-06-25  9:16       ` Bartosz Golaszewski
2020-06-25  9:16       ` Bartosz Golaszewski
2020-06-22 10:00 ` [PATCH 07/11] phy: mdio: add kerneldoc for __devm_mdiobus_register() Bartosz Golaszewski
2020-06-22 10:00   ` [Intel-wired-lan] " Bartosz Golaszewski
2020-06-22 10:00 ` [PATCH 08/11] net: phy: don't abuse devres in devm_mdiobus_register() Bartosz Golaszewski
2020-06-22 10:00   ` [Intel-wired-lan] " Bartosz Golaszewski
2020-06-22 10:00 ` [PATCH 09/11] of: mdio: remove the 'extern' keyword from function declarations Bartosz Golaszewski
2020-06-22 10:00   ` [Intel-wired-lan] " Bartosz Golaszewski
2020-06-22 10:00 ` [PATCH 10/11] of: mdio: provide devm_of_mdiobus_register() Bartosz Golaszewski
2020-06-22 10:00   ` [Intel-wired-lan] " Bartosz Golaszewski
2020-06-22 10:00 ` [PATCH 11/11] net: ethernet: mtk-star-emac: use devm_of_mdiobus_register() Bartosz Golaszewski
2020-06-22 10:00   ` [Intel-wired-lan] " Bartosz Golaszewski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200622100056.10151-1-brgl@bgdev.pl \
    --to=brgl@bgdev.pl \
    --cc=Mark-MC.Lee@mediatek.com \
    --cc=andrew.perepech@mediatek.com \
    --cc=andrew@lunn.ch \
    --cc=bgolaszewski@baylibre.com \
    --cc=corbet@lwn.net \
    --cc=davem@davemloft.net \
    --cc=devicetree@vger.kernel.org \
    --cc=f.fainelli@gmail.com \
    --cc=fparent@baylibre.com \
    --cc=frowand.list@gmail.com \
    --cc=hkallweit1@gmail.com \
    --cc=intel-wired-lan@lists.osuosl.org \
    --cc=jeffrey.t.kirsher@intel.com \
    --cc=john@phrozen.org \
    --cc=kuba@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mediatek@lists.infradead.org \
    --cc=linux@armlinux.org.uk \
    --cc=matthias.bgg@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=nic_swsd@realtek.com \
    --cc=pedro.tsai@mediatek.com \
    --cc=robh+dt@kernel.org \
    --cc=sean.wang@mediatek.com \
    --cc=stephane.leprovost@mediatek.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.