Messages in this thread | ![/](/images/icornerl.gif) | | From | Jonas Gorski <> | Date | Tue, 21 Mar 2023 11:35:56 +0100 | Subject | Re: [RFC PATCH] drivers: net: dsa: b53: mmap: add phy ops |
| |
On Mon, 20 Mar 2023 at 19:28, Álvaro Fernández Rojas <noltari@gmail.com> 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. > > 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; > }; > > 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)
On nitpick here: AFACT, what you are actually getting there as phy_id isn't the phy_id but the port_id, it just happens to be identical for internal ports.
So in theory you would first need to convert this to the appropriate phy_id (+ which bus) first, else you risk reading from the wrong device (and/or bus).
See how the phys_mii_mask is based on the indexes of the user ports, not their actual phy_ids. [1] [2]
[1] https://elixir.bootlin.com/linux/latest/source/net/dsa/dsa.c#L660 [2] https://elixir.bootlin.com/linux/latest/source/include/net/dsa.h#L596
Regards Jonas
| ![\](/images/icornerr.gif) |