Re: [RESEND PATCH net-next v8 1/1] net: mdio: Add RTL9300 MDIO driver
From: Paolo Abeni <pabeni@redhat.com>
Date: 2025-03-06 09:30:39
Also in:
lkml
Hi, On 3/4/25 2:52 AM, Chris Packham wrote:
quoted hunk ↗ jump to hunk
Add a driver for the MDIO controller on the RTL9300 family of Ethernet switches with integrated SoC. There are 4 physical SMI interfaces on the RTL9300 however access is done using the switch ports. The driver takes the MDIO bus hierarchy from the DTS and uses this to configure the switch ports so they are associated with the correct PHY. This mapping is also used when dealing with software requests from phylib. Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz> --- Notes: Changes in v8: - Fix typo in user visible string Changes in v7: - Update out of date comment - Use for_each_set_bit() instead of open-coded iteration - Use FIELD_PREP() in a few more places - Add #defines for register field masks Changes in v6: - Parse port->phy mapping from devicetree removing the need for the realtek,port property - Remove erroneous code dealing with SMI_POLL_CTRL. When actually implemented this stops the LED unit from updating correctly. Changes in v5: - Reword out of date comment - Use GENMASK/FIELD_PREP where appropriate - Introduce port validity bitmap. - Use more obvious names for PHY_CTRL_READ/WRITE and PHY_CTRL_TYPE_C45/C22 Changes in v4: - rename to realtek-rtl9300 - s/realtek_/rtl9300_/ - add locking to support concurrent access - The dtbinding now represents the MDIO bus hierarchy so we consume this information and use it to configure the switch port to MDIO bus+addr. Changes in v3: - Fix (another) off-by-one error Changes in v2: - Add clause 22 support - Remove commented out code - Formatting cleanup - Set MAX_PORTS correctly for MDIO interface - Fix off-by-one error in pn check drivers/net/mdio/Kconfig | 7 + drivers/net/mdio/Makefile | 1 + drivers/net/mdio/mdio-realtek-rtl9300.c | 475 ++++++++++++++++++++++++ 3 files changed, 483 insertions(+) create mode 100644 drivers/net/mdio/mdio-realtek-rtl9300.cdiff --git a/drivers/net/mdio/Kconfig b/drivers/net/mdio/Kconfig index 4a7a303be2f7..058fcdaf6c18 100644 --- a/drivers/net/mdio/Kconfig +++ b/drivers/net/mdio/Kconfig@@ -185,6 +185,13 @@ config MDIO_IPQ8064 This driver supports the MDIO interface found in the network interface units of the IPQ8064 SoC +config MDIO_REALTEK_RTL9300 + tristate "Realtek RTL9300 MDIO interface support" + depends on MACH_REALTEK_RTL || COMPILE_TEST + help + This driver supports the MDIO interface found in the Realtek + RTL9300 family of Ethernet switches with integrated SoC. + config MDIO_REGMAP tristate helpdiff --git a/drivers/net/mdio/Makefile b/drivers/net/mdio/Makefile index 1015f0db4531..c23778e73890 100644 --- a/drivers/net/mdio/Makefile +++ b/drivers/net/mdio/Makefile@@ -19,6 +19,7 @@ obj-$(CONFIG_MDIO_MOXART) += mdio-moxart.o obj-$(CONFIG_MDIO_MSCC_MIIM) += mdio-mscc-miim.o obj-$(CONFIG_MDIO_MVUSB) += mdio-mvusb.o obj-$(CONFIG_MDIO_OCTEON) += mdio-octeon.o +obj-$(CONFIG_MDIO_REALTEK_RTL9300) += mdio-realtek-rtl9300.o obj-$(CONFIG_MDIO_REGMAP) += mdio-regmap.o obj-$(CONFIG_MDIO_SUN4I) += mdio-sun4i.o obj-$(CONFIG_MDIO_THUNDER) += mdio-thunder.odiff --git a/drivers/net/mdio/mdio-realtek-rtl9300.c b/drivers/net/mdio/mdio-realtek-rtl9300.c new file mode 100644 index 000000000000..0a97c2a9c46d --- /dev/null +++ b/drivers/net/mdio/mdio-realtek-rtl9300.c@@ -0,0 +1,475 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * MDIO controller for RTL9300 switches with integrated SoC. + * + * The MDIO communication is abstracted by the switch. At the software level + * communication uses the switch port to address the PHY. We work out the + * mapping based on the MDIO bus described in device tree and phandles on the + * ethernet-ports property. + */ + +#include <linux/bitfield.h> +#include <linux/bitmap.h> +#include <linux/bits.h> +#include <linux/cleanup.h> +#include <linux/find.h> +#include <linux/mdio.h> +#include <linux/mfd/syscon.h> +#include <linux/mod_devicetable.h> +#include <linux/mutex.h> +#include <linux/of_mdio.h> +#include <linux/phy.h> +#include <linux/platform_device.h> +#include <linux/property.h> +#include <linux/regmap.h> + +#define SMI_GLB_CTRL 0xca00 +#define GLB_CTRL_INTF_SEL(intf) BIT(16 + (intf)) +#define SMI_PORT0_15_POLLING_SEL 0xca08 +#define SMI_ACCESS_PHY_CTRL_0 0xcb70 +#define SMI_ACCESS_PHY_CTRL_1 0xcb74 +#define PHY_CTRL_REG_ADDR GENMASK(24, 20) +#define PHY_CTRL_PARK_PAGE GENMASK(19, 15) +#define PHY_CTRL_MAIN_PAGE GENMASK(14, 3) +#define PHY_CTRL_WRITE BIT(2) +#define PHY_CTRL_READ 0 +#define PHY_CTRL_TYPE_C45 BIT(1) +#define PHY_CTRL_TYPE_C22 0 +#define PHY_CTRL_CMD BIT(0) +#define PHY_CTRL_FAIL BIT(25) +#define SMI_ACCESS_PHY_CTRL_2 0xcb78 +#define PHY_CTRL_INDATA GENMASK(31, 16) +#define PHY_CTRL_DATA GENMASK(15, 0) +#define SMI_ACCESS_PHY_CTRL_3 0xcb7c +#define PHY_CTRL_MMD_DEVAD GENMASK(20, 16) +#define PHY_CTRL_MMD_REG GENMASK(15, 0) +#define SMI_PORT0_5_ADDR_CTRL 0xcb80 + +#define MAX_PORTS 28 +#define MAX_SMI_BUSSES 4 +#define MAX_SMI_ADDR 0x1f + +struct rtl9300_mdio_priv { + struct regmap *regmap; + struct mutex lock; /* protect HW access */ + DECLARE_BITMAP(valid_ports, MAX_PORTS); + u8 smi_bus[MAX_PORTS]; + u8 smi_addr[MAX_PORTS]; + bool smi_bus_is_c45[MAX_SMI_BUSSES]; + struct mii_bus *bus[MAX_SMI_BUSSES]; +}; + +struct rtl9300_mdio_chan { + struct rtl9300_mdio_priv *priv; + u8 mdio_bus; +}; + +static int rtl9300_mdio_phy_to_port(struct mii_bus *bus, int phy_id) +{ + struct rtl9300_mdio_chan *chan = bus->priv; + struct rtl9300_mdio_priv *priv = chan->priv; + int i; + + for_each_set_bit(i, priv->valid_ports, MAX_PORTS) + if (priv->smi_bus[i] == chan->mdio_bus && + priv->smi_addr[i] == phy_id) + return i; + + return -ENOENT; +} + +static int rtl9300_mdio_wait_ready(struct rtl9300_mdio_priv *priv) +{ + struct regmap *regmap = priv->regmap; + u32 val; + + lockdep_assert_held(&priv->lock); + + return regmap_read_poll_timeout(regmap, SMI_ACCESS_PHY_CTRL_1, + val, !(val & PHY_CTRL_CMD), 10, 1000); +} + +static int rtl9300_mdio_read_c22(struct mii_bus *bus, int phy_id, int regnum) +{ + struct rtl9300_mdio_chan *chan = bus->priv; + struct rtl9300_mdio_priv *priv = chan->priv; + struct regmap *regmap = priv->regmap; + int port; + u32 val; + int err; + + guard(mutex)(&priv->lock);
I'm sorry for the late feedback but quoting Documentation/process/maintainer-netdev.rst: """ Use of ``guard()`` is discouraged within any function longer than 20 lines, """ I suggest plain mutex_lock()/unlock() usage in a Also please respect the reverse christmass tree in the variable definiton. You will have to do something alike: struct rtl9300_mdio_chan *chan = bus->priv; struct rtl9300_mdio_priv *priv; struct regmap *regmap; int port; u32 val; int err; priv = chan->priv; regmap = priv->regmap; Finaly, the cover letter is not required for a single patch. Thanks, Paolo