| Message ID | 20220116211529.25604-2-tobias@waldekranz.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | net/fsl: xgmac_mdio: Add workaround for erratum A-009885 | expand |
On Sun, Jan 16, 2022 at 10:15:26PM +0100, Tobias Waldekranz wrote: > Once an MDIO read transaction is initiated, we must read back the data > register within 16 MDC cycles after the transaction completes. Outside > of this window, reads may return corrupt data. > > Therefore, disable local interrupts in the critical section, to > maximize the probability that we can satisfy this requirement. Since this is for net, a Fixes: tag would be nice. Maybe that would be for the commit which added this driver, or maybe when the DTSI files for the SOCs which have this errata we added? Reviewed-by: Andrew Lunn <andrew@lunn.ch> Andrew
On Sun, Jan 16, 2022 at 23:02, Andrew Lunn <andrew@lunn.ch> wrote: > On Sun, Jan 16, 2022 at 10:15:26PM +0100, Tobias Waldekranz wrote: >> Once an MDIO read transaction is initiated, we must read back the data >> register within 16 MDC cycles after the transaction completes. Outside >> of this window, reads may return corrupt data. >> >> Therefore, disable local interrupts in the critical section, to >> maximize the probability that we can satisfy this requirement. > > Since this is for net, a Fixes: tag would be nice. Maybe that would be > for the commit which added this driver, or maybe when the DTSI files > for the SOCs which have this errata we added? Alright, I wasn't sure how to tag WAs for errata since it is more about the hardware than the driver. Should I send a v2 even if nothing else pops up, or is this more of a if-you're-sending-a-v2-anyway type of comment?
On Mon, Jan 17, 2022 at 08:24:22AM +0100, Tobias Waldekranz wrote: > On Sun, Jan 16, 2022 at 23:02, Andrew Lunn <andrew@lunn.ch> wrote: > > On Sun, Jan 16, 2022 at 10:15:26PM +0100, Tobias Waldekranz wrote: > >> Once an MDIO read transaction is initiated, we must read back the data > >> register within 16 MDC cycles after the transaction completes. Outside > >> of this window, reads may return corrupt data. > >> > >> Therefore, disable local interrupts in the critical section, to > >> maximize the probability that we can satisfy this requirement. > > > > Since this is for net, a Fixes: tag would be nice. Maybe that would be > > for the commit which added this driver, or maybe when the DTSI files > > for the SOCs which have this errata we added? > > Alright, I wasn't sure how to tag WAs for errata since it is more about > the hardware than the driver. The tag gives the backporter an idea how far back to go. If support for this SoC has only recently been added, there is no need to backport a long way. If it is an old SoC, then maybe more effort should be put into the backport? > Should I send a v2 even if nothing else > pops up, or is this more of a if-you're-sending-a-v2-anyway type of > comment? If you reply with a Fixes: patchwork will automagically append it like it does Reviewed-by, Tested-by etc. Andrew
On Sun, Jan 16, 2022 at 22:15, Tobias Waldekranz <tobias@waldekranz.com> wrote: > Once an MDIO read transaction is initiated, we must read back the data > register within 16 MDC cycles after the transaction completes. Outside > of this window, reads may return corrupt data. > > Therefore, disable local interrupts in the critical section, to > maximize the probability that we can satisfy this requirement. > > Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com> Fixes: d55ad2967d89 ("powerpc/mpc85xx: Create dts components for the FSL QorIQ DPAA FMan")
On Mon, 17 Jan 2022 15:00:41 +0100 Andrew Lunn wrote: > > Should I send a v2 even if nothing else > > pops up, or is this more of a if-you're-sending-a-v2-anyway type of > > comment? > > If you reply with a Fixes: patchwork will automagically append it like > it does Reviewed-by, Tested-by etc. That part is pretty finicky, it's supposed to work but when I apply these I only get review tags from Andrew and a Fixes tag already present on the last patch :( A v2 with Fixes tags included in the posting would be best after all. Thanks!
diff --git a/drivers/net/ethernet/freescale/xgmac_mdio.c b/drivers/net/ethernet/freescale/xgmac_mdio.c index 5b8b9bcf41a2..bf566ac3195b 100644 --- a/drivers/net/ethernet/freescale/xgmac_mdio.c +++ b/drivers/net/ethernet/freescale/xgmac_mdio.c @@ -51,6 +51,7 @@ struct tgec_mdio_controller { struct mdio_fsl_priv { struct tgec_mdio_controller __iomem *mdio_base; bool is_little_endian; + bool has_a009885; bool has_a011043; }; @@ -186,10 +187,10 @@ static int xgmac_mdio_read(struct mii_bus *bus, int phy_id, int regnum) { struct mdio_fsl_priv *priv = (struct mdio_fsl_priv *)bus->priv; struct tgec_mdio_controller __iomem *regs = priv->mdio_base; + unsigned long flags; uint16_t dev_addr; uint32_t mdio_stat; uint32_t mdio_ctl; - uint16_t value; int ret; bool endian = priv->is_little_endian; @@ -221,12 +222,18 @@ static int xgmac_mdio_read(struct mii_bus *bus, int phy_id, int regnum) return ret; } + if (priv->has_a009885) + /* Once the operation completes, i.e. MDIO_STAT_BSY clears, we + * must read back the data register within 16 MDC cycles. + */ + local_irq_save(flags); + /* Initiate the read */ xgmac_write32(mdio_ctl | MDIO_CTL_READ, ®s->mdio_ctl, endian); ret = xgmac_wait_until_done(&bus->dev, regs, endian); if (ret) - return ret; + goto irq_restore; /* Return all Fs if nothing was there */ if ((xgmac_read32(®s->mdio_stat, endian) & MDIO_STAT_RD_ER) && @@ -234,13 +241,17 @@ static int xgmac_mdio_read(struct mii_bus *bus, int phy_id, int regnum) dev_dbg(&bus->dev, "Error while reading PHY%d reg at %d.%hhu\n", phy_id, dev_addr, regnum); - return 0xffff; + ret = 0xffff; + } else { + ret = xgmac_read32(®s->mdio_data, endian) & 0xffff; + dev_dbg(&bus->dev, "read %04x\n", ret); } - value = xgmac_read32(®s->mdio_data, endian) & 0xffff; - dev_dbg(&bus->dev, "read %04x\n", value); +irq_restore: + if (priv->has_a009885) + local_irq_restore(flags); - return value; + return ret; } static int xgmac_mdio_probe(struct platform_device *pdev) @@ -287,6 +298,8 @@ static int xgmac_mdio_probe(struct platform_device *pdev) priv->is_little_endian = device_property_read_bool(&pdev->dev, "little-endian"); + priv->has_a009885 = device_property_read_bool(&pdev->dev, + "fsl,erratum-a009885"); priv->has_a011043 = device_property_read_bool(&pdev->dev, "fsl,erratum-a011043");
Once an MDIO read transaction is initiated, we must read back the data register within 16 MDC cycles after the transaction completes. Outside of this window, reads may return corrupt data. Therefore, disable local interrupts in the critical section, to maximize the probability that we can satisfy this requirement. Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com> --- drivers/net/ethernet/freescale/xgmac_mdio.c | 25 ++++++++++++++++----- 1 file changed, 19 insertions(+), 6 deletions(-)