[net-next,v6,00/11] net: dsa: realtek: variants to drivers, interfaces to a common module

Message ID 20240209-realtek_reverse-v6-0-0662f8cbc7b5@gmail.com
Headers
Series net: dsa: realtek: variants to drivers, interfaces to a common module |

Message

Luiz Angelo Daros de Luca Feb. 9, 2024, 5:03 a.m. UTC
  The current driver consists of two interface modules (SMI and MDIO) and
two family/variant modules (RTL8365MB and RTL8366RB). The SMI and MDIO
modules serve as the platform and MDIO drivers, respectively, calling
functions from the variant modules. In this setup, one interface module
can be loaded independently of the other, but both variants must be
loaded (if not disabled at build time) for any type of interface. This
approach doesn't scale well, especially with the addition of more switch
variants (e.g., RTL8366B), leading to loaded but unused modules.
Additionally, this also seems upside down, as the specific driver code
normally depends on the more generic functions and not the other way
around.

Each variant module was converted into real drivers, serving as both a
platform driver (for switches connected using the SMI interface) and an
MDIO driver (for MDIO-connected switches). The relationship between the
variant and interface modules is reversed, with the variant module now
calling both interface functions (if not disabled at build time). While
in most devices only one interface is likely used, the interface code is
significantly smaller than a variant module, consuming fewer resources
than the previous code. With variant modules now functioning as real
drivers, compatible strings are published only in a single variant
module, preventing conflicts.

The patch series introduces a new common module for functions shared by
both variants. This module also absorbs the two previous interface
modules, as they would always be loaded anyway.

The series relocates the user MII driver from realtek-smi to rtl83xx. It
is now used by MDIO-connected switches instead of the generic DSA
driver. There's a change in how this driver locates the MDIO node. It
now only searches for a child node named "mdio".

The dsa_switch in realtek_priv->ds is now embedded in the struct. It is
always in use and avoids dynamic memory allocation.

Testing has been performed with an RTL8367S (rtl8365mb) using MDIO
interface and an RTL8366RB (rtl8366) with SMI interface.

Luiz

---

Changes in v2:
- Renamed realtek_common module to realtek-dsa.
- Removed the warning when the MDIO node is not named "mdio."
- ds->user_mii_bus is only assigned if all user ports do not have a
  phy-handle.
- of_node_put is now back to the driver remove method.
- Renamed realtek_common_probe_{pre,post} to
  realtek_common_{probe,register_switch}.
- Added some comments for realtek_common_{probe,register_switch}.
- Using dev_err_probe whenever possible.
- Embedded priv->ds into realtek_priv, removing its dynamic
  allocation.
- Fixed realtek-common.h macros.
- Save and check the return value in functions, even when it is the
  last one.
- Added the #if expression as a comment to #else and #endif in header
  files.
- Unregister the platform and the MDIO driver in the reverse order
  they are registered.
- Unregister the first driver if the second one failed to register.
- Added the revert patch for "net: dsa: OF-ware slave_mii_bus."
- Link to v1: https://lore.kernel.org/r/20231208045054.27966-1-luizluca@gmail.com

Changes in v3:
- Look for the MDIO bus searching for a child node named "mdio" instead
  of the compatible string.
- Removed the check for a phy-handle in ports. ds->user_mii_bus will
  not be used anymore.
- Dropped comments for realtek_common_{probe,register_switch}.
- Fixed a compile error in "net: dsa: OF-ware slave_mii_bus".
- Used the wrapper realtek_smi_driver_register instead of
  platform_driver_register.
- Link to v2: https://lore.kernel.org/r/20231220042632.26825-1-luizluca@gmail.com/

Changes in v4:
- Changed Makefile to use ifdef instead of dynamic variable names.
- Added comments for all exported symbols.
- Migrated exported symbols to REALTEK_DSA namespace.
- renamed realtek_common to rtl83xx.
- put the mdio node just after registration and not in driver remove.
- rtl83xx_probe now receives a struct with regmap read/write functions
  and build regmap_config dynamically.
- pulled into a new patch the realtek_priv change from "common
  realtek-dsa module".
- pulled into a new patch the user_mii_bus setup changes from "migrate
  user_mii_bus setup to realtek-dsa".
- removed the revert "net: dsa: OF-ware slave_mii_bus" patch from the
  series.
- Link to v3: https://lore.kernel.org/r/20231223005253.17891-1-luizluca@gmail.com/

Changes in v5:
- Removed empty line at the end of files
- Multiple kdoc fixes
- added rtl83xx_unregister_switch and rtl83xx_shutdown
- moved include <linux/regmap.h> from rtl83xx.h to rtl83xx.c
- moved dev_set_drvdata to the end of rtl83xx_probe()
- changed mii_user bus is name to devicename(switch):user_mii
- Link to v4: https://lore.kernel.org/all/20240123215606.26716-1-luizluca@gmail.com/

Changes in v6:
- Renamed realtek-dsa to realtek_dsa to match loaded module name
- More kdocs fixes
- Calls rtl83xx_remove on error after a successful rtl83xx_probe
- Use struct dsa_switch *ds instead of priv->ds
- Link to v5: https://lore.kernel.org/r/20240130-realtek_reverse-v5-0-ecafd9283a07@gmail.com

---
Luiz Angelo Daros de Luca (11):
      net: dsa: realtek: drop cleanup from realtek_ops
      net: dsa: realtek: introduce REALTEK_DSA namespace
      net: dsa: realtek: convert variants into real drivers
      net: dsa: realtek: keep variant reference in realtek_priv
      net: dsa: realtek: common rtl83xx module
      net: dsa: realtek: merge rtl83xx and interface modules into realtek_dsa
      net: dsa: realtek: get internal MDIO node by name
      net: dsa: realtek: clean user_mii_bus setup
      net: dsa: realtek: migrate user_mii_bus setup to realtek_dsa
      net: dsa: realtek: use the same mii bus driver for both interfaces
      net: dsa: realtek: embed dsa_switch into realtek_priv

 drivers/net/dsa/realtek/Kconfig        |  20 +--
 drivers/net/dsa/realtek/Makefile       |  13 +-
 drivers/net/dsa/realtek/realtek-mdio.c | 205 ++++++----------------
 drivers/net/dsa/realtek/realtek-mdio.h |  48 ++++++
 drivers/net/dsa/realtek/realtek-smi.c  | 279 +++++++-----------------------
 drivers/net/dsa/realtek/realtek-smi.h  |  48 ++++++
 drivers/net/dsa/realtek/realtek.h      |  12 +-
 drivers/net/dsa/realtek/rtl8365mb.c    | 128 ++++++++------
 drivers/net/dsa/realtek/rtl8366-core.c |  22 +--
 drivers/net/dsa/realtek/rtl8366rb.c    | 119 +++++++------
 drivers/net/dsa/realtek/rtl83xx.c      | 303 +++++++++++++++++++++++++++++++++
 drivers/net/dsa/realtek/rtl83xx.h      |  22 +++
 12 files changed, 705 insertions(+), 514 deletions(-)
---
base-commit: 2121c43f88f593eea51d483bedd638cb0623c7e2
change-id: 20240127-realtek_reverse-1b6021490c85

Best regards,
  

Comments

Linus Walleij Feb. 9, 2024, 1:03 p.m. UTC | #1
On Fri, Feb 9, 2024 at 6:04 AM Luiz Angelo Daros de Luca
<luizluca@gmail.com> wrote:

> Create a namespace to group the exported symbols.
>
> Signed-off-by: Luiz Angelo Daros de Luca <luizluca@gmail.com>
> Reviewed-by: Vladimir Oltean <olteanv@gmail.com>
> Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij
  
Linus Walleij Feb. 9, 2024, 1:17 p.m. UTC | #2
On Fri, Feb 9, 2024 at 6:04 AM Luiz Angelo Daros de Luca
<luizluca@gmail.com> wrote:

> The current driver consists of two interface modules (SMI and MDIO) and
> two family/variant modules (RTL8365MB and RTL8366RB). The SMI and MDIO
> modules serve as the platform and MDIO drivers, respectively, calling
> functions from the variant modules. In this setup, one interface module
> can be loaded independently of the other, but both variants must be
> loaded (if not disabled at build time) for any type of interface. This
> approach doesn't scale well, especially with the addition of more switch
> variants (e.g., RTL8366B), leading to loaded but unused modules.
> Additionally, this also seems upside down, as the specific driver code
> normally depends on the more generic functions and not the other way
> around.
>
> Each variant module was converted into real drivers, serving as both a
> platform driver (for switches connected using the SMI interface) and an
> MDIO driver (for MDIO-connected switches). The relationship between the
> variant and interface modules is reversed, with the variant module now
> calling both interface functions (if not disabled at build time). While
> in most devices only one interface is likely used, the interface code is
> significantly smaller than a variant module, consuming fewer resources
> than the previous code. With variant modules now functioning as real
> drivers, compatible strings are published only in a single variant
> module, preventing conflicts.
>
> The patch series introduces a new common module for functions shared by
> both variants. This module also absorbs the two previous interface
> modules, as they would always be loaded anyway.
>
> The series relocates the user MII driver from realtek-smi to rtl83xx. It
> is now used by MDIO-connected switches instead of the generic DSA
> driver. There's a change in how this driver locates the MDIO node. It
> now only searches for a child node named "mdio".
>
> The dsa_switch in realtek_priv->ds is now embedded in the struct. It is
> always in use and avoids dynamic memory allocation.
>
> Testing has been performed with an RTL8367S (rtl8365mb) using MDIO
> interface and an RTL8366RB (rtl8366) with SMI interface.

Tested on my RTL8366RB on a D-Link DIR-685 as well, works like a
charm:
Tested-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij
  
patchwork-bot+netdevbpf@kernel.org Feb. 12, 2024, 10:50 a.m. UTC | #3
Hello:

This series was applied to netdev/net-next.git (main)
by David S. Miller <davem@davemloft.net>:

On Fri, 09 Feb 2024 02:03:36 -0300 you wrote:
> The current driver consists of two interface modules (SMI and MDIO) and
> two family/variant modules (RTL8365MB and RTL8366RB). The SMI and MDIO
> modules serve as the platform and MDIO drivers, respectively, calling
> functions from the variant modules. In this setup, one interface module
> can be loaded independently of the other, but both variants must be
> loaded (if not disabled at build time) for any type of interface. This
> approach doesn't scale well, especially with the addition of more switch
> variants (e.g., RTL8366B), leading to loaded but unused modules.
> Additionally, this also seems upside down, as the specific driver code
> normally depends on the more generic functions and not the other way
> around.
> 
> [...]

Here is the summary with links:
  - [net-next,v6,01/11] net: dsa: realtek: drop cleanup from realtek_ops
    https://git.kernel.org/netdev/net-next/c/33f4336cbd32
  - [net-next,v6,02/11] net: dsa: realtek: introduce REALTEK_DSA namespace
    https://git.kernel.org/netdev/net-next/c/ded3813b44fe
  - [net-next,v6,03/11] net: dsa: realtek: convert variants into real drivers
    https://git.kernel.org/netdev/net-next/c/bce254b839ab
  - [net-next,v6,04/11] net: dsa: realtek: keep variant reference in realtek_priv
    https://git.kernel.org/netdev/net-next/c/4667a1db2f55
  - [net-next,v6,05/11] net: dsa: realtek: common rtl83xx module
    https://git.kernel.org/netdev/net-next/c/8be040ecd94c
  - [net-next,v6,06/11] net: dsa: realtek: merge rtl83xx and interface modules into realtek_dsa
    https://git.kernel.org/netdev/net-next/c/98b75c1c149c
  - [net-next,v6,07/11] net: dsa: realtek: get internal MDIO node by name
    https://git.kernel.org/netdev/net-next/c/8685c98d45c5
  - [net-next,v6,08/11] net: dsa: realtek: clean user_mii_bus setup
    https://git.kernel.org/netdev/net-next/c/68c66d8d8a19
  - [net-next,v6,09/11] net: dsa: realtek: migrate user_mii_bus setup to realtek_dsa
    https://git.kernel.org/netdev/net-next/c/b4bd77971f3c
  - [net-next,v6,10/11] net: dsa: realtek: use the same mii bus driver for both interfaces
    https://git.kernel.org/netdev/net-next/c/bba140a566ed
  - [net-next,v6,11/11] net: dsa: realtek: embed dsa_switch into realtek_priv
    https://git.kernel.org/netdev/net-next/c/9fc469b2943d

You are awesome, thank you!
  
Luiz Angelo Daros de Luca Feb. 12, 2024, 12:43 p.m. UTC | #4
> Tested on my RTL8366RB on a D-Link DIR-685 as well, works like a
> charm:

Thanks, Linus. An independent test is always welcome.

> Tested-by: Linus Walleij <linus.walleij@linaro.org>


Regards,

Luiz
  
Luiz Angelo Daros de Luca Feb. 12, 2024, 12:45 p.m. UTC | #5
> Hello:
>
> This series was applied to netdev/net-next.git (main)
> by David S. Miller <davem@davemloft.net>:
>
> On Fri, 09 Feb 2024 02:03:36 -0300 you wrote:
> > The current driver consists of two interface modules (SMI and MDIO) and
> > two family/variant modules (RTL8365MB and RTL8366RB). The SMI and MDIO
> > modules serve as the platform and MDIO drivers, respectively, calling
> > functions from the variant modules. In this setup, one interface module
> > can be loaded independently of the other, but both variants must be
> > loaded (if not disabled at build time) for any type of interface. This
> > approach doesn't scale well, especially with the addition of more switch
> > variants (e.g., RTL8366B), leading to loaded but unused modules.
> > Additionally, this also seems upside down, as the specific driver code
> > normally depends on the more generic functions and not the other way
> > around.
> >
> > [...]
>
> Here is the summary with links:
>   - [net-next,v6,01/11] net: dsa: realtek: drop cleanup from realtek_ops
>     https://git.kernel.org/netdev/net-next/c/33f4336cbd32
>   - [net-next,v6,02/11] net: dsa: realtek: introduce REALTEK_DSA namespace
>     https://git.kernel.org/netdev/net-next/c/ded3813b44fe
>   - [net-next,v6,03/11] net: dsa: realtek: convert variants into real drivers
>     https://git.kernel.org/netdev/net-next/c/bce254b839ab
>   - [net-next,v6,04/11] net: dsa: realtek: keep variant reference in realtek_priv
>     https://git.kernel.org/netdev/net-next/c/4667a1db2f55
>   - [net-next,v6,05/11] net: dsa: realtek: common rtl83xx module
>     https://git.kernel.org/netdev/net-next/c/8be040ecd94c
>   - [net-next,v6,06/11] net: dsa: realtek: merge rtl83xx and
 interface modules into realtek_dsa
>     https://git.kernel.org/netdev/net-next/c/98b75c1c149c
>   - [net-next,v6,07/11] net: dsa: realtek: get internal MDIO node by name
>     https://git.kernel.org/netdev/net-next/c/8685c98d45c5
>   - [net-next,v6,08/11] net: dsa: realtek: clean user_mii_bus setup
>     https://git.kernel.org/netdev/net-next/c/68c66d8d8a19
>   - [net-next,v6,09/11] net: dsa: realtek: migrate user_mii_bus setup to realtek_dsa
>     https://git.kernel.org/netdev/net-next/c/b4bd77971f3c
>   - [net-next,v6,10/11] net: dsa: realtek: use the same mii bus driver for both interfaces
>     https://git.kernel.org/netdev/net-next/c/bba140a566ed
>   - [net-next,v6,11/11] net: dsa: realtek: embed dsa_switch into realtek_priv
>     https://git.kernel.org/netdev/net-next/c/9fc469b2943d
>
> You are awesome, thank you!

Thank you all involved, especially Vladmir who invested quite some time on this.

Regards,

Luiz
  
Vladimir Oltean Feb. 16, 2024, 1:20 a.m. UTC | #6
On Mon, Feb 12, 2024 at 09:45:19AM -0300, Luiz Angelo Daros de Luca wrote:
> Thank you all involved, especially Vladmir who invested quite some time on this.

Thank you as well for being persistent. Unfortunately, due to some
personal things that need my attention, I have been reading my emails on
and off in the past few weeks, and will continue to do so for some weeks
to come still. Sorry for the delays.