Messages in this thread | | | Date | Mon, 20 Mar 2023 12:06:31 -0700 | Subject | Re: [RFC PATCH] drivers: net: dsa: b53: mmap: add phy ops | From | Florian Fainelli <> |
| |
On 3/20/23 11:28, Álvaro Fernández Rojas wrote: > 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.
Looks good for the most part, just a few questions below.
> > 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)
For some reason, there was no bit introduced to tell us when a transaction has finished, so we have to poll after a certain delay has elapsed...
> + > +#define REG_MDIOD 0xb4 > + > struct b53_mmap_priv { > void __iomem *regs; > + struct mii_bus *bus; > }; > > 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);
Since you make the 'mii-bus' property and 'priv->bus' necessary prerequisites for the driver to finish probing successfully, when shall we not have valid priv->bus reference to work with? Do we end-up taking the other path at all? -- Florian
| |