2023-05-24 13:22:42

by Maxime Chevallier

[permalink] [raw]
Subject: [PATCH net-next 0/4] net: add a regmap-based mdio driver and drop TSE PCS

Hello everyone,

This series follows-up on the worki [1] aiming at dropping the altera TSE PCS
driver, as it turns out to be a version of the Lynx PCS exposed as a
memory-mapped block, instead of living on an MDIO bus.

One step of this removal involved creating a regmap-based mdio driver
that translates MDIO accesses into the actual underlying bus that
exposes the register. The register layout must of course match the
standard MDIO layout, but we can now account for differences in stride
with recent work on the regmap subsystem [2].

Mark, Net maintainers, this series depends on the patch e12ff2876493 that was
recently merged into the regmap tree [3].

For this series to be usable in net-next, this patch must be applied
beforehand. Should Mark create a tag that would then be merged into
net-next ?

This series introduces a new MDIO driver, and uses it to convert Altera
TSE from the actual TSE PCS driver to Lynx PCS.

Since it turns out dwmac_socfpga also uses a TSE PCS block, port that
driver to Lynx as well.

Thanks,

Maxime

[1] : https://lore.kernel.org/all/[email protected]/
[2] : https://lore.kernel.org/all/[email protected]/#t
[3] : https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regmap.git/commit/?id=e12ff28764937dd58c8613f16065da60da149048

Maxime Chevallier (4):
net: mdio: Introduce a regmap-based mdio driver
net: ethernet: altera-tse: Convert to mdio-regmap and use PCS Lynx
net: pcs: Drop the TSE PCS driver
net: stmmac: dwmac-sogfpga: use the lynx pcs driver

MAINTAINERS | 14 +-
drivers/net/ethernet/altera/Kconfig | 2 +
drivers/net/ethernet/altera/altera_tse.h | 1 +
drivers/net/ethernet/altera/altera_tse_main.c | 57 +++-
drivers/net/ethernet/stmicro/stmmac/Kconfig | 1 +
drivers/net/ethernet/stmicro/stmmac/Makefile | 2 +-
.../ethernet/stmicro/stmmac/altr_tse_pcs.c | 257 ------------------
.../ethernet/stmicro/stmmac/altr_tse_pcs.h | 29 --
drivers/net/ethernet/stmicro/stmmac/common.h | 1 +
.../ethernet/stmicro/stmmac/dwmac-socfpga.c | 90 ++++--
.../net/ethernet/stmicro/stmmac/stmmac_main.c | 12 +-
drivers/net/mdio/Kconfig | 10 +
drivers/net/mdio/Makefile | 1 +
drivers/net/mdio/mdio-regmap.c | 85 ++++++
drivers/net/pcs/Kconfig | 6 -
drivers/net/pcs/Makefile | 1 -
drivers/net/pcs/pcs-altera-tse.c | 160 -----------
include/linux/mdio/mdio-regmap.h | 25 ++
include/linux/pcs-altera-tse.h | 17 --
19 files changed, 258 insertions(+), 513 deletions(-)
delete mode 100644 drivers/net/ethernet/stmicro/stmmac/altr_tse_pcs.c
delete mode 100644 drivers/net/ethernet/stmicro/stmmac/altr_tse_pcs.h
create mode 100644 drivers/net/mdio/mdio-regmap.c
delete mode 100644 drivers/net/pcs/pcs-altera-tse.c
create mode 100644 include/linux/mdio/mdio-regmap.h
delete mode 100644 include/linux/pcs-altera-tse.h

--
2.40.1



2023-05-24 13:22:52

by Maxime Chevallier

[permalink] [raw]
Subject: [PATCH net-next 2/4] net: ethernet: altera-tse: Convert to mdio-regmap and use PCS Lynx

The newly introduced regmap-based MDIO driver allows for an easy mapping
of an mdiodevice onto the memory-mapped TSE PCS, which is actually a
Lynx PCS.

Convert Altera TSE to use this PCS instead of the pcs-altera-tse, which
is nothing more than a memory-mapped Lynx PCS.

Signed-off-by: Maxime Chevallier <[email protected]>
---
drivers/net/ethernet/altera/altera_tse.h | 1 +
drivers/net/ethernet/altera/altera_tse_main.c | 57 +++++++++++++++++--
include/linux/mdio/mdio-regmap.h | 2 +
3 files changed, 54 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/altera/altera_tse.h b/drivers/net/ethernet/altera/altera_tse.h
index db5eed06e92d..d50cf440d01b 100644
--- a/drivers/net/ethernet/altera/altera_tse.h
+++ b/drivers/net/ethernet/altera/altera_tse.h
@@ -477,6 +477,7 @@ struct altera_tse_private {
struct phylink *phylink;
struct phylink_config phylink_config;
struct phylink_pcs *pcs;
+ struct mdio_device *pcs_mdiodev;
};

/* Function prototypes
diff --git a/drivers/net/ethernet/altera/altera_tse_main.c b/drivers/net/ethernet/altera/altera_tse_main.c
index 190ff1bcd94e..66db6a7d0b22 100644
--- a/drivers/net/ethernet/altera/altera_tse_main.c
+++ b/drivers/net/ethernet/altera/altera_tse_main.c
@@ -27,14 +27,16 @@
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/mii.h>
+#include <linux/mdio/mdio-regmap.h>
#include <linux/netdevice.h>
#include <linux/of_device.h>
#include <linux/of_mdio.h>
#include <linux/of_net.h>
#include <linux/of_platform.h>
-#include <linux/pcs-altera-tse.h>
+#include <linux/pcs-lynx.h>
#include <linux/phy.h>
#include <linux/platform_device.h>
+#include <linux/regmap.h>
#include <linux/skbuff.h>
#include <asm/cacheflush.h>

@@ -1134,13 +1136,21 @@ static int altera_tse_probe(struct platform_device *pdev)
const struct of_device_id *of_id = NULL;
struct altera_tse_private *priv;
struct resource *control_port;
+ struct regmap *pcs_regmap;
struct resource *dma_res;
struct resource *pcs_res;
+ struct mii_bus *pcs_bus;
struct net_device *ndev;
void __iomem *descmap;
- int pcs_reg_width = 2;
int ret = -ENODEV;

+ struct regmap_config pcs_regmap_cfg;
+
+ struct mdio_regmap_config mrc = {
+ .parent = &pdev->dev,
+ .valid_addr = 0x0,
+ };
+
ndev = alloc_etherdev(sizeof(struct altera_tse_private));
if (!ndev) {
dev_err(&pdev->dev, "Could not allocate network device\n");
@@ -1258,10 +1268,29 @@ static int altera_tse_probe(struct platform_device *pdev)
ret = request_and_map(pdev, "pcs", &pcs_res,
&priv->pcs_base);
if (ret) {
+ /* If we can't find a dedicated resource for the PCS, fallback
+ * to the internal PCS, that has a different address stride
+ */
priv->pcs_base = priv->mac_dev + tse_csroffs(mdio_phy0);
- pcs_reg_width = 4;
+ pcs_regmap_cfg.reg_bits = 32;
+ /* Values are MDIO-like values, on 16 bits */
+ pcs_regmap_cfg.val_bits = 16;
+ pcs_regmap_cfg.reg_shift = REGMAP_UPSHIFT(2);
+ } else {
+ pcs_regmap_cfg.reg_bits = 16;
+ pcs_regmap_cfg.val_bits = 16;
+ pcs_regmap_cfg.reg_shift = REGMAP_UPSHIFT(1);
}

+ /* Create a regmap for the PCS so that it can be used by the PCS driver */
+ pcs_regmap = devm_regmap_init_mmio(&pdev->dev, priv->pcs_base,
+ &pcs_regmap_cfg);
+ if (IS_ERR(pcs_regmap)) {
+ ret = PTR_ERR(pcs_regmap);
+ goto err_free_netdev;
+ }
+ mrc.regmap = pcs_regmap;
+
/* Rx IRQ */
priv->rx_irq = platform_get_irq_byname(pdev, "rx_irq");
if (priv->rx_irq == -ENXIO) {
@@ -1384,7 +1413,20 @@ static int altera_tse_probe(struct platform_device *pdev)
(unsigned long) control_port->start, priv->rx_irq,
priv->tx_irq);

- priv->pcs = alt_tse_pcs_create(ndev, priv->pcs_base, pcs_reg_width);
+ snprintf(mrc.name, MII_BUS_ID_SIZE, "%s-pcs-mii", ndev->name);
+ pcs_bus = devm_mdio_regmap_register(&pdev->dev, &mrc);
+ if (IS_ERR(pcs_bus)) {
+ ret = PTR_ERR(pcs_bus);
+ goto err_init_phy;
+ }
+
+ priv->pcs_mdiodev = mdio_device_create(pcs_bus, 0);
+
+ priv->pcs = lynx_pcs_create(priv->pcs_mdiodev);
+ if (!priv->pcs) {
+ ret = -ENODEV;
+ goto err_init_phy;
+ }

priv->phylink_config.dev = &ndev->dev;
priv->phylink_config.type = PHYLINK_NETDEV;
@@ -1407,11 +1449,12 @@ static int altera_tse_probe(struct platform_device *pdev)
if (IS_ERR(priv->phylink)) {
dev_err(&pdev->dev, "failed to create phylink\n");
ret = PTR_ERR(priv->phylink);
- goto err_init_phy;
+ goto err_pcs;
}

return 0;
-
+err_pcs:
+ mdio_device_free(priv->pcs_mdiodev);
err_init_phy:
unregister_netdev(ndev);
err_register_netdev:
@@ -1433,6 +1476,8 @@ static int altera_tse_remove(struct platform_device *pdev)
altera_tse_mdio_destroy(ndev);
unregister_netdev(ndev);
phylink_destroy(priv->phylink);
+ mdio_device_free(priv->pcs_mdiodev);
+
free_netdev(ndev);

return 0;
diff --git a/include/linux/mdio/mdio-regmap.h b/include/linux/mdio/mdio-regmap.h
index 536a5bfcce35..d09968bbc4ce 100644
--- a/include/linux/mdio/mdio-regmap.h
+++ b/include/linux/mdio/mdio-regmap.h
@@ -7,6 +7,8 @@
#ifndef MDIO_REGMAP_H
#define MDIO_REGMAP_H

+#include <linux/phy.h>
+
struct device;
struct regmap;

--
2.40.1


2023-05-24 13:23:06

by Maxime Chevallier

[permalink] [raw]
Subject: [PATCH net-next 1/4] net: mdio: Introduce a regmap-based mdio driver

There exists several examples today of devices that embed an ethernet
PHY or PCS directly inside an SoC. In this situation, either the device
is controlled through a vendor-specific register set, or sometimes
exposes the standard 802.3 registers that are typically accessed over
MDIO.

As phylib and phylink are designed to use mdiodevices, this driver
allows creating a virtual MDIO bus, that translates mdiodev register
accesses to regmap accesses.

The reason we use regmap is because there are at least 3 such devices
known today, 2 of them are Altera TSE PCS's, memory-mapped, exposed
with a 4-byte stride in stmmac's dwmac-socfpga variant, and a 2-byte
stride in altera-tse. The other one (nxp,sja1110-base-tx-mdio) is
exposed over SPI.

Signed-off-by: Maxime Chevallier <[email protected]>
---
MAINTAINERS | 7 +++
drivers/net/ethernet/altera/Kconfig | 2 +
drivers/net/mdio/Kconfig | 10 ++++
drivers/net/mdio/Makefile | 1 +
drivers/net/mdio/mdio-regmap.c | 85 +++++++++++++++++++++++++++++
include/linux/mdio/mdio-regmap.h | 23 ++++++++
6 files changed, 128 insertions(+)
create mode 100644 drivers/net/mdio/mdio-regmap.c
create mode 100644 include/linux/mdio/mdio-regmap.h

diff --git a/MAINTAINERS b/MAINTAINERS
index c25172d6471a..ef8362aa93b3 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12840,6 +12840,13 @@ F: Documentation/devicetree/bindings/net/ieee802154/mcr20a.txt
F: drivers/net/ieee802154/mcr20a.c
F: drivers/net/ieee802154/mcr20a.h

+MDIO REGMAP DRIVER
+M: Maxime Chevallier <[email protected]>
+L: [email protected]
+S: Maintained
+F: drivers/net/mdio/mdio-regmap.c
+F: include/linux/mdio/mdio-regmap.h
+
MEASUREMENT COMPUTING CIO-DAC IIO DRIVER
M: William Breathitt Gray <[email protected]>
L: [email protected]
diff --git a/drivers/net/ethernet/altera/Kconfig b/drivers/net/ethernet/altera/Kconfig
index dd7fd41ccde5..0a7c0a217536 100644
--- a/drivers/net/ethernet/altera/Kconfig
+++ b/drivers/net/ethernet/altera/Kconfig
@@ -5,6 +5,8 @@ config ALTERA_TSE
select PHYLIB
select PHYLINK
select PCS_ALTERA_TSE
+ select MDIO_REGMAP
+ depends on REGMAP
help
This driver supports the Altera Triple-Speed (TSE) Ethernet MAC.

diff --git a/drivers/net/mdio/Kconfig b/drivers/net/mdio/Kconfig
index 9ff2e6f22f3f..aef39c89cf44 100644
--- a/drivers/net/mdio/Kconfig
+++ b/drivers/net/mdio/Kconfig
@@ -185,6 +185,16 @@ config MDIO_IPQ8064
This driver supports the MDIO interface found in the network
interface units of the IPQ8064 SoC

+config MDIO_REGMAP
+ tristate
+ help
+ This driver allows using MDIO devices that are not sitting on a
+ regular MDIO bus, but still exposes the standard 802.3 register
+ layout. It's regmap-based so that it can be used on integrated,
+ memory-mapped PHYs, SPI PHYs and so on. A new virtual MDIO bus is
+ created, and its read/write operations are mapped to the underlying
+ regmap.
+
config MDIO_THUNDER
tristate "ThunderX SOCs MDIO buses"
depends on 64BIT
diff --git a/drivers/net/mdio/Makefile b/drivers/net/mdio/Makefile
index 7d4cb4c11e4e..1015f0db4531 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_REGMAP) += mdio-regmap.o
obj-$(CONFIG_MDIO_SUN4I) += mdio-sun4i.o
obj-$(CONFIG_MDIO_THUNDER) += mdio-thunder.o
obj-$(CONFIG_MDIO_XGENE) += mdio-xgene.o
diff --git a/drivers/net/mdio/mdio-regmap.c b/drivers/net/mdio/mdio-regmap.c
new file mode 100644
index 000000000000..9b8845b18536
--- /dev/null
+++ b/drivers/net/mdio/mdio-regmap.c
@@ -0,0 +1,85 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/* Driver for MMIO-Mapped MDIO devices. Some IPs expose internal PHYs or PCS
+ * within the MMIO-mapped area
+ *
+ * Copyright (C) 2023 Maxime Chevallier <[email protected]>
+ */
+#include <linux/bitfield.h>
+#include <linux/delay.h>
+#include <linux/mdio.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_mdio.h>
+#include <linux/phy.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/mdio/mdio-regmap.h>
+
+#define DRV_NAME "mdio-regmap"
+
+static int mdio_regmap_read_c22(struct mii_bus *bus, int addr, int regnum)
+{
+ struct mdio_regmap_config *ctx = bus->priv;
+ unsigned int val;
+ int ret;
+
+ if (ctx->valid_addr != addr)
+ return -ENODEV;
+
+ ret = regmap_read(ctx->regmap, regnum, &val);
+ if (ret < 0)
+ return ret;
+
+ return val;
+}
+
+static int mdio_regmap_write_c22(struct mii_bus *bus, int addr, int regnum,
+ u16 val)
+{
+ struct mdio_regmap_config *ctx = bus->priv;
+
+ if (ctx->valid_addr != addr)
+ return -ENODEV;
+
+ return regmap_write(ctx->regmap, regnum, val);
+}
+
+struct mii_bus *devm_mdio_regmap_register(struct device *dev,
+ const struct mdio_regmap_config *config)
+{
+ struct mdio_regmap_config *mrc;
+ struct mii_bus *mii;
+ int rc;
+
+ if (!config->parent)
+ return ERR_PTR(-EINVAL);
+
+ mii = devm_mdiobus_alloc_size(config->parent, sizeof(*mrc));
+ if (!mii)
+ return ERR_PTR(-ENOMEM);
+
+ mrc = mii->priv;
+ memcpy(mrc, config, sizeof(*mrc));
+
+ mrc->regmap = config->regmap;
+ mrc->valid_addr = config->valid_addr;
+
+ mii->name = DRV_NAME;
+ strscpy(mii->id, config->name, MII_BUS_ID_SIZE);
+ mii->parent = config->parent;
+ mii->read = mdio_regmap_read_c22;
+ mii->write = mdio_regmap_write_c22;
+
+ rc = devm_mdiobus_register(dev, mii);
+ if (rc) {
+ dev_err(config->parent, "Cannot register MDIO bus![%s] (%d)\n", mii->id, rc);
+ return ERR_PTR(rc);
+ }
+
+ return mii;
+}
+EXPORT_SYMBOL_GPL(devm_mdio_regmap_register);
+
+MODULE_DESCRIPTION("MDIO API over regmap");
+MODULE_AUTHOR("Maxime Chevallier <[email protected]>");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/mdio/mdio-regmap.h b/include/linux/mdio/mdio-regmap.h
new file mode 100644
index 000000000000..536a5bfcce35
--- /dev/null
+++ b/include/linux/mdio/mdio-regmap.h
@@ -0,0 +1,23 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Driver for MMIO-Mapped MDIO devices. Some IPs expose internal PHYs or PCS
+ * within the MMIO-mapped area
+ *
+ * Copyright (C) 2023 Maxime Chevallier <[email protected]>
+ */
+#ifndef MDIO_REGMAP_H
+#define MDIO_REGMAP_H
+
+struct device;
+struct regmap;
+
+struct mdio_regmap_config {
+ struct device *parent;
+ struct regmap *regmap;
+ char name[MII_BUS_ID_SIZE];
+ u32 valid_addr;
+};
+
+struct mii_bus *devm_mdio_regmap_register(struct device *dev,
+ const struct mdio_regmap_config *config);
+
+#endif
--
2.40.1


2023-05-24 17:48:23

by Vladimir Oltean

[permalink] [raw]
Subject: Re: [PATCH net-next 1/4] net: mdio: Introduce a regmap-based mdio driver

On Wed, May 24, 2023 at 07:30:51PM +0200, Andrew Lunn wrote:
> > + mii->name = DRV_NAME;
> > + strscpy(mii->id, config->name, MII_BUS_ID_SIZE);
> > + mii->parent = config->parent;
> > + mii->read = mdio_regmap_read_c22;
> > + mii->write = mdio_regmap_write_c22;
>
> Since there is only one valid address on the bus, you can set
> mii->phy_mask to make the scanning of the bus a little faster.

Sorry, I didn't reach this thread yet, I don't have the full context.
Just wanted to add: if the caller knows that there's only a PCS and not
a PHY on this bus, you don't want auto-scanning at all, since that will
create an unconnected phy_device. It would be good if the caller provided
the phy_mask.

2023-05-24 17:55:23

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH net-next 1/4] net: mdio: Introduce a regmap-based mdio driver

> + mii->name = DRV_NAME;
> + strscpy(mii->id, config->name, MII_BUS_ID_SIZE);
> + mii->parent = config->parent;
> + mii->read = mdio_regmap_read_c22;
> + mii->write = mdio_regmap_write_c22;

Since there is only one valid address on the bus, you can set
mii->phy_mask to make the scanning of the bus a little faster.

Andrew

2023-05-25 09:26:53

by Maxime Chevallier

[permalink] [raw]
Subject: Re: [PATCH net-next 1/4] net: mdio: Introduce a regmap-based mdio driver

Hello Vlad, Andrew,

On Wed, 24 May 2023 20:41:45 +0300
Vladimir Oltean <[email protected]> wrote:

> On Wed, May 24, 2023 at 07:30:51PM +0200, Andrew Lunn wrote:
> > > + mii->name = DRV_NAME;
> > > + strscpy(mii->id, config->name, MII_BUS_ID_SIZE);
> > > + mii->parent = config->parent;
> > > + mii->read = mdio_regmap_read_c22;
> > > + mii->write = mdio_regmap_write_c22;
> >
> > Since there is only one valid address on the bus, you can set
> > mii->phy_mask to make the scanning of the bus a little faster.

Good point, I'll add that.

> Sorry, I didn't reach this thread yet, I don't have the full context.
> Just wanted to add: if the caller knows that there's only a PCS and
> not a PHY on this bus, you don't want auto-scanning at all, since
> that will create an unconnected phy_device. It would be good if the
> caller provided the phy_mask.

As there can only be one mdiodevice on that bus, and we are already
passing the address of the only available device in struct
mdio_rgmap_config.valid_addr, I guess we can simply add a flag to
indicate if autoscan needs to be enabled or not, and set phy_mask to
either unmask the only valid address, or plain ~0UL if we don't want
autoscan at all.

Thanks both of you for the reviews,

Maxime