Re: [PATCH net-next v2 03/14] net: pcs: pcs-xpcs-regmap: support XPCS memory-mapped MDIO bus via regmap

[Maxime Chevallier]

Hi Alex,

On 6/5/26 03:00, Alex Elder wrote:
> From: Daniel Thompson <daniel@xxxxxxxxxxxx>
> 
> In some DesignWare XPCS implementatons the memory-mapped MDIO bus is
> allocated to a register window that does not align to a page boundary.
> This makes iomapping the registers problematic.
> 
> For example the Toshiba TC9564 (a PCIe Ethernet-AVB/TSN bridge) provides
> an "eMAC" subsystem with the XPCS base address cuddled up to XGMAC
> registers.
> 
> Let's introduce helpers to allow the driver that owns the eMAC to register
> an XPCS using is regmap for the memory-mapped MDIO bus.
> 
> Signed-off-by: Daniel Thompson <daniel@xxxxxxxxxxxx>
> Signed-off-by: Alex Elder <elder@xxxxxxxxxxxx>
> ---
>  MAINTAINERS                         |   2 +
>  drivers/net/pcs/Makefile            |   4 +-
>  drivers/net/pcs/pcs-xpcs-regmap.c   | 219 ++++++++++++++++++++++++++++
>  include/linux/pcs/pcs-xpcs-regmap.h |  20 +++
>  4 files changed, 243 insertions(+), 2 deletions(-)
>  create mode 100644 drivers/net/pcs/pcs-xpcs-regmap.c
>  create mode 100644 include/linux/pcs/pcs-xpcs-regmap.h
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index eb8cdcc76324f..2aa6ea012c848 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -25931,8 +25931,10 @@ F:	drivers/net/ethernet/synopsys/
>  SYNOPSYS DESIGNWARE ETHERNET XPCS DRIVER
>  L:	netdev@xxxxxxxxxxxxxxx
>  S:	Orphan
> +F:	drivers/net/pcs/pcs-xpcs-regmap.c
>  F:	drivers/net/pcs/pcs-xpcs.c
>  F:	drivers/net/pcs/pcs-xpcs.h
> +F	include/linux/pcs/pcs-xpcs-regmap.h
>  F:	include/linux/pcs/pcs-xpcs.h
>  
>  SYNOPSYS DESIGNWARE HDMI RX CONTROLLER DRIVER
> diff --git a/drivers/net/pcs/Makefile b/drivers/net/pcs/Makefile
> index 4f7920618b900..565f1b63fce0b 100644
> --- a/drivers/net/pcs/Makefile
> +++ b/drivers/net/pcs/Makefile
> @@ -1,8 +1,8 @@
>  # SPDX-License-Identifier: GPL-2.0
>  # Makefile for Linux PCS drivers
>  
> -pcs_xpcs-$(CONFIG_PCS_XPCS)	:= pcs-xpcs.o pcs-xpcs-plat.o \
> -				   pcs-xpcs-nxp.o pcs-xpcs-wx.o
> +pcs_xpcs-$(CONFIG_PCS_XPCS)	:= pcs-xpcs.o pcs-xpcs-nxp.o pcs-xpcs-regmap.o \
> +				   pcs-xpcs-plat.o pcs-xpcs-wx.o
>  
>  obj-$(CONFIG_PCS_XPCS)		+= pcs_xpcs.o
>  obj-$(CONFIG_PCS_LYNX)		+= pcs-lynx.o
> diff --git a/drivers/net/pcs/pcs-xpcs-regmap.c b/drivers/net/pcs/pcs-xpcs-regmap.c
> new file mode 100644
> index 0000000000000..55cd05d09c7db
> --- /dev/null
> +++ b/drivers/net/pcs/pcs-xpcs-regmap.c
> @@ -0,0 +1,219 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Synopsys DesignWare XPCS regmap helpers
> + *
> + * Copyright (C) 2026 RISCstar Solutions.
> + * Copyright (C) 2024 Serge Semin
> + */
> +
> +#include <linux/device.h>
> +#include <linux/kernel.h>
> +#include <linux/mdio.h>
> +#include <linux/pcs/pcs-xpcs.h>
> +#include <linux/pcs/pcs-xpcs-regmap.h>
> +#include <linux/regmap.h>
> +
> +#include "pcs-xpcs.h"
> +
> +/* Page select register for the indirect MMIO CSRs access */
> +#define DW_VR_CSR_VIEWPORT		0xff
> +
> +struct dw_xpcs_regmap {
> +	struct device *dev;
> +	struct mii_bus *bus;
> +	struct regmap *regmap;
> +	bool reg_indir;
> +};
> +
> +static ptrdiff_t xpcs_regmap_addr_format(int dev, int reg)
> +{
> +	return FIELD_PREP(0x1f0000, dev) | FIELD_PREP(0xffff, reg);
> +}
> +
> +static u16 xpcs_regmap_addr_page(ptrdiff_t csr)
> +{
> +	return FIELD_GET(0x1fff00, csr);
> +}
> +
> +static ptrdiff_t xpcs_regmap_addr_offset(ptrdiff_t csr)
> +{
> +	return FIELD_GET(0xff, csr);
> +}
> +
> +static int xpcs_regmap_read_reg_indirect(struct dw_xpcs_regmap *pxpcs, int dev,
> +					 int reg)
> +{
> +	ptrdiff_t csr, ofs;
> +	unsigned int val;
> +	u16 page;
> +	int res;
> +
> +	csr = xpcs_regmap_addr_format(dev, reg);
> +	page = xpcs_regmap_addr_page(csr);
> +	ofs = xpcs_regmap_addr_offset(csr);
> +
> +	res = regmap_write(pxpcs->regmap, DW_VR_CSR_VIEWPORT, page);
> +	if (res < 0)
> +		return res;
> +
> +	res = regmap_read(pxpcs->regmap, ofs, &val);
> +	if (res < 0)
> +		return res;
> +
> +	return val & 0xffff;
> +}
> +
> +static int xpcs_regmap_write_reg_indirect(struct dw_xpcs_regmap *pxpcs, int dev,
> +					  int reg, u16 val)
> +{
> +	ptrdiff_t csr, ofs;
> +	u16 page;
> +	int res;
> +
> +	csr = xpcs_regmap_addr_format(dev, reg);
> +	page = xpcs_regmap_addr_page(csr);
> +	ofs = xpcs_regmap_addr_offset(csr);
> +
> +	res = regmap_write(pxpcs->regmap, DW_VR_CSR_VIEWPORT, page);
> +	if (res < 0)
> +		return res;
> +
> +	return regmap_write(pxpcs->regmap, ofs, val);
> +}
> +
> +static int xpcs_regmap_read_reg_direct(struct dw_xpcs_regmap *pxpcs, int dev,
> +				       int reg)
> +{
> +	unsigned int val;
> +	ptrdiff_t csr;
> +	int res;
> +
> +	csr = xpcs_regmap_addr_format(dev, reg);
> +	res = regmap_read(pxpcs->regmap, csr, &val);
> +	if (res < 0)
> +		return res;
> +
> +	return val & 0xffff;
> +}
> +
> +static int xpcs_regmap_write_reg_direct(struct dw_xpcs_regmap *pxpcs, int dev,
> +					int reg, u16 val)
> +{
> +	ptrdiff_t csr = xpcs_regmap_addr_format(dev, reg);
> +
> +	return regmap_write(pxpcs->regmap, csr, val);
> +}
> +
> +static int xpcs_regmap_read_c22(struct mii_bus *bus, int addr, int reg)
> +{
> +	struct dw_xpcs_regmap *pxpcs = bus->priv;
> +
> +	if (addr != 0)
> +		return -ENODEV;
> +
> +	if (pxpcs->reg_indir)
> +		return xpcs_regmap_read_reg_indirect(pxpcs, MDIO_MMD_VEND2, reg);
> +	else
> +		return xpcs_regmap_read_reg_direct(pxpcs, MDIO_MMD_VEND2, reg);
> +}
> +
> +static int xpcs_regmap_write_c22(struct mii_bus *bus, int addr, int reg, u16 val)
> +{
> +	struct dw_xpcs_regmap *pxpcs = bus->priv;
> +
> +	if (addr != 0)
> +		return -ENODEV;
> +
> +	if (pxpcs->reg_indir)
> +		return xpcs_regmap_write_reg_indirect(pxpcs, MDIO_MMD_VEND2, reg, val);
> +	else
> +		return xpcs_regmap_write_reg_direct(pxpcs, MDIO_MMD_VEND2, reg, val);
> +}
> +
> +static int xpcs_regmap_read_c45(struct mii_bus *bus, int addr, int dev, int reg)
> +{
> +	struct dw_xpcs_regmap *pxpcs = bus->priv;
> +
> +	if (addr != 0)
> +		return -ENODEV;
> +
> +	if (pxpcs->reg_indir)
> +		return xpcs_regmap_read_reg_indirect(pxpcs, dev, reg);
> +	else
> +		return xpcs_regmap_read_reg_direct(pxpcs, dev, reg);
> +}
> +
> +static int xpcs_regmap_write_c45(struct mii_bus *bus, int addr, int dev,
> +				 int reg, u16 val)
> +{
> +	struct dw_xpcs_regmap *pxpcs = bus->priv;
> +
> +	if (addr != 0)
> +		return -ENODEV;
> +
> +	if (pxpcs->reg_indir)
> +		return xpcs_regmap_write_reg_indirect(pxpcs, dev, reg, val);
> +	else
> +		return xpcs_regmap_write_reg_direct(pxpcs, dev, reg, val);
> +}
> +
> +static void devm_xpcs_regmap_destroy(void *data)
> +{
> +	struct dw_xpcs *xpcs = data;
> +
> +	xpcs_destroy(xpcs);
> +}
> +
> +struct dw_xpcs *devm_xpcs_regmap_register(struct device *dev,
> +					  const struct xpcs_regmap_config *config)
> +{
> +	static atomic_t id = ATOMIC_INIT(-1);
> +	struct dw_xpcs_regmap *pxpcs;
> +	struct dw_xpcs *xpcs;
> +	int ret;
> +
> +	pxpcs = devm_kzalloc(dev, sizeof(*pxpcs), GFP_KERNEL);
> +	if (!pxpcs)
> +		return ERR_PTR(-ENOMEM);
> +
> +	pxpcs->dev = dev;
> +	pxpcs->regmap = config->regmap;
> +	pxpcs->reg_indir = config->reg_indir;

Looking at the overall series, is there any reason for this flag ?

Looks like the reg_indir=false path isn't used at all in this series.

Maybe just drop it and let anyone add it back should the need arise ?

Maxime