Messages in this thread | | | From | Álvaro Fernández Rojas <> | Date | Tue, 21 Mar 2023 10:32:06 +0100 | Subject | Re: [RFC PATCH] drivers: net: dsa: b53: mmap: add phy ops |
| |
2023-03-20 19:28 GMT+01:00, Álvaro Fernández Rojas <noltari@gmail.com>: > Currently, B53 MMAP BCM63xx devices with an external switch hang when > performing PHY read and write operations due to invalid registers access. > This adds support for PHY ops by using the internal bus from > mdio-mux-bcm6368 > when probed by device tree and also falls back to direct MDIO registers if > not. > > This is an alternative to: > - > https://patchwork.kernel.org/project/netdevbpf/cover/20230317113427.302162-1-noltari@gmail.com/ > - > https://patchwork.kernel.org/project/netdevbpf/patch/20230317113427.302162-2-noltari@gmail.com/ > - > https://patchwork.kernel.org/project/netdevbpf/patch/20230317113427.302162-3-noltari@gmail.com/ > - > https://patchwork.kernel.org/project/netdevbpf/patch/20230317113427.302162-4-noltari@gmail.com/ > As discussed, it was an ABI break and not the correct way of fixing the > issue. > > Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com> > --- > drivers/net/dsa/b53/b53_mmap.c | 86 +++++++++++++++++++++++++++++++ > include/linux/platform_data/b53.h | 1 + > 2 files changed, 87 insertions(+) > > diff --git a/drivers/net/dsa/b53/b53_mmap.c > b/drivers/net/dsa/b53/b53_mmap.c > index 706df04b6cee..7deca1c557c5 100644 > --- a/drivers/net/dsa/b53/b53_mmap.c > +++ b/drivers/net/dsa/b53/b53_mmap.c > @@ -19,14 +19,25 @@ > #include <linux/bits.h> > #include <linux/kernel.h> > #include <linux/module.h> > +#include <linux/of_mdio.h> > #include <linux/io.h> > #include <linux/platform_device.h> > #include <linux/platform_data/b53.h> > > #include "b53_priv.h" > > +#define REG_MDIOC 0xb0 > +#define REG_MDIOC_EXT_MASK BIT(16) > +#define REG_MDIOC_REG_SHIFT 20 > +#define REG_MDIOC_PHYID_SHIFT 25 > +#define REG_MDIOC_RD_MASK BIT(30) > +#define REG_MDIOC_WR_MASK BIT(31) > + > +#define REG_MDIOD 0xb4 > + > struct b53_mmap_priv { > void __iomem *regs; > + struct mii_bus *bus;
Can we reuse "bus" from b53_device instead of adding a new one for b53_mmap_priv?
> }; > > static int b53_mmap_read8(struct b53_device *dev, u8 page, u8 reg, u8 > *val) > @@ -216,6 +227,69 @@ static int b53_mmap_write64(struct b53_device *dev, u8 > page, u8 reg, > return 0; > } > > +static inline void b53_mmap_mdio_read(struct b53_device *dev, int phy_id, > + int loc, u16 *val) > +{ > + uint32_t reg; > + > + b53_mmap_write32(dev, 0, REG_MDIOC, 0); > + > + reg = REG_MDIOC_RD_MASK | > + (phy_id << REG_MDIOC_PHYID_SHIFT) | > + (loc << REG_MDIOC_REG_SHIFT); > + > + b53_mmap_write32(dev, 0, REG_MDIOC, reg); > + udelay(50); > + b53_mmap_read16(dev, 0, REG_MDIOD, val); > +} > + > +static inline int b53_mmap_mdio_write(struct b53_device *dev, int phy_id, > + int loc, u16 val) > +{ > + uint32_t reg; > + > + b53_mmap_write32(dev, 0, REG_MDIOC, 0); > + > + reg = REG_MDIOC_WR_MASK | > + (phy_id << REG_MDIOC_PHYID_SHIFT) | > + (loc << REG_MDIOC_REG_SHIFT) | > + val; > + > + b53_mmap_write32(dev, 0, REG_MDIOC, reg); > + udelay(50); > + > + return 0; > +} > + > +static int b53_mmap_phy_read16(struct b53_device *dev, int addr, int reg, > + u16 *value) > +{ > + struct b53_mmap_priv *priv = dev->priv; > + struct mii_bus *bus = priv->bus; > + > + if (bus) > + *value = mdiobus_read_nested(bus, addr, reg); > + else > + b53_mmap_mdio_read(dev, addr, reg, value); > + > + return 0; > +} > + > +static int b53_mmap_phy_write16(struct b53_device *dev, int addr, int reg, > + u16 value) > +{ > + struct b53_mmap_priv *priv = dev->priv; > + struct mii_bus *bus = priv->bus; > + int ret; > + > + if (bus) > + ret = mdiobus_write_nested(bus, addr, reg, value); > + else > + ret = b53_mmap_mdio_write(dev, addr, reg, value); > + > + return ret; > +} > + > static const struct b53_io_ops b53_mmap_ops = { > .read8 = b53_mmap_read8, > .read16 = b53_mmap_read16, > @@ -227,6 +301,8 @@ static const struct b53_io_ops b53_mmap_ops = { > .write32 = b53_mmap_write32, > .write48 = b53_mmap_write48, > .write64 = b53_mmap_write64, > + .phy_read16 = b53_mmap_phy_read16, > + .phy_write16 = b53_mmap_phy_write16, > }; > > static int b53_mmap_probe_of(struct platform_device *pdev, > @@ -234,6 +310,7 @@ static int b53_mmap_probe_of(struct platform_device > *pdev, > { > struct device_node *np = pdev->dev.of_node; > struct device_node *of_ports, *of_port; > + struct device_node *mdio; > struct device *dev = &pdev->dev; > struct b53_platform_data *pdata; > void __iomem *mem; > @@ -251,6 +328,14 @@ static int b53_mmap_probe_of(struct platform_device > *pdev, > pdata->chip_id = (u32)device_get_match_data(dev); > pdata->big_endian = of_property_read_bool(np, "big-endian"); > > + mdio = of_parse_phandle(np, "mii-bus", 0);
Is "mii-bus" OK or shall we use something like "brcm,mii-bus"?
> + if (!mdio) > + return -EINVAL; > + > + pdata->bus = of_mdio_find_bus(mdio); > + if (!pdata->bus) > + return -EPROBE_DEFER; > + > of_ports = of_get_child_by_name(np, "ports"); > if (!of_ports) { > dev_err(dev, "no ports child node found\n"); > @@ -297,6 +382,7 @@ static int b53_mmap_probe(struct platform_device *pdev) > return -ENOMEM; > > priv->regs = pdata->regs; > + priv->bus = pdata->bus; > > dev = b53_switch_alloc(&pdev->dev, &b53_mmap_ops, priv); > if (!dev) > diff --git a/include/linux/platform_data/b53.h > b/include/linux/platform_data/b53.h > index 6f6fed2b171d..be0c5bfdedad 100644 > --- a/include/linux/platform_data/b53.h > +++ b/include/linux/platform_data/b53.h > @@ -32,6 +32,7 @@ struct b53_platform_data { > /* only used by MMAP'd driver */ > unsigned big_endian:1; > void __iomem *regs; > + struct mii_bus *bus; > }; > > #endif > -- > 2.30.2 > >
| |