[U-Boot] [PATCH] net: lpc32xx: Fix MDIO busy wait

[U-Boot] [PATCH] net: lpc32xx: Fix MDIO busy wait

[amessier.tyco at gmail.com]

From: Alexandre Messier <amessier@tycoint.com>

The MDIO read function waits on the busy flag after issuing the read
command, while the MDIO write function waits on the busy flag before
issuing the write command.

This causes an issue when writing then immediately reading. As the MDIO
module is still busy, the read command is not processed. The wait on
busy flag in the read command passes because the previous write command
finishes. In the end, the value returned by the read function is
whatever was present in the MDIO data register.

Fix the issue by making sure the busy flag is cleared before issuing a
read command. This way, it is still possible to issue a write command
and continue executing u-boot code while the command is processed by
the hardware. Any following MDIO read/write commands after a write
command will wait for a cleared busy flag.

Signed-off-by: Alexandre Messier <amessier@tycoint.com>
---
 drivers/net/lpc32xx_eth.c |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/drivers/net/lpc32xx_eth.c b/drivers/net/lpc32xx_eth.c
index e76e9bc..25629a9 100644
--- a/drivers/net/lpc32xx_eth.c
+++ b/drivers/net/lpc32xx_eth.c
@@ -246,6 +246,20 @@ static int mii_reg_read(const char *devname, u8 phy_adr, u8 reg_ofs, u16 *data)
 		return -EFAULT;
 	}
 
+	/* wait till the MII is not busy */
+	timeout = MII_TIMEOUT;
+	do {
+		/* read MII indicators register */
+		mind_reg = readl(&regs->mind);
+		if (--timeout == 0)
+			break;
+	} while (mind_reg & MIND_BUSY);
+
+	if (timeout == 0) {
+		printf("%s:%u: MII busy timeout\n", __func__, __LINE__);
+		return -EFAULT;
+	}
+
 	/* write the phy and reg addressse into the MII address reg */
 	writel((phy_adr << MADR_PHY_OFFSET) | (reg_ofs << MADR_REG_OFFSET),
 	       &regs->madr);
-- 
1.7.1

[U-Boot] [PATCH] net: lpc32xx: Fix MDIO busy wait

[Vladimir Zapolskiy]

Hi Alexandre,

On 16.12.2015 20:37, amessier.tyco at gmail.com wrote:
> From: Alexandre Messier <amessier@tycoint.com>
> 
> The MDIO read function waits on the busy flag after issuing the read
> command, 

that's correct, after issuing the read command and before register read.

> while the MDIO write function waits on the busy flag before
> issuing the write command.

and that is not correct, I believe.

From the spec (MII Mgmt Indicators Register):

	For PHY Write if scan is not used:
	1. Write 0 to MCMD
	2. Write PHY address and register address to MADR
	3. Write data to MWTD
-->	4. Wait for busy bit to be cleared in MIND

	For PHY Read if scan is not used:
	1. Write 1 to MCMD
	2. Write PHY address and register address to MADR
-->	3. Wait for busy bit to be cleared in MIND
	4. Write 0 to MCMD
	5. Read data from MRDD

Could you please test/review an alternative fix? I believe it adds proper
serialization of all command sequences (read/read, read/write, write/read
and write/write). Thank you in advance.

diff --git a/drivers/net/lpc32xx_eth.c b/drivers/net/lpc32xx_eth.c
index e76e9bc..3ba5b4b 100644
--- a/drivers/net/lpc32xx_eth.c
+++ b/drivers/net/lpc32xx_eth.c
@@ -304,6 +304,13 @@ static int mii_reg_write(const char *devname, u8
phy_adr, u8 reg_ofs, u16 data)
 		return -EFAULT;
 	}

+	/* write the phy and reg addressse into the MII address reg */
+	writel((phy_adr << MADR_PHY_OFFSET) | (reg_ofs << MADR_REG_OFFSET),
+	       &regs->madr);
+
+	/* write data to the MII write register */
+	writel(data, &regs->mwtd);
+
 	/* wait till the MII is not busy */
 	timeout = MII_TIMEOUT;
 	do {
@@ -319,13 +326,6 @@ static int mii_reg_write(const char *devname, u8
phy_adr, u8 reg_ofs, u16 data)
 		return -EFAULT;
 	}

-	/* write the phy and reg addressse into the MII address reg */
-	writel((phy_adr << MADR_PHY_OFFSET) | (reg_ofs << MADR_REG_OFFSET),
-	       &regs->madr);
-
-	/* write data to the MII write register */
-	writel(data, &regs->mwtd);
-
 	/*debug("%s:(adr %d, off %d) <= %04x\n", __func__, phy_adr,
 		reg_ofs, data);*/


> This causes an issue when writing then immediately reading. As the MDIO
> module is still busy, the read command is not processed. The wait on
> busy flag in the read command passes because the previous write command
> finishes. In the end, the value returned by the read function is
> whatever was present in the MDIO data register.
> 
> Fix the issue by making sure the busy flag is cleared before issuing a
> read command. This way, it is still possible to issue a write command
> and continue executing u-boot code while the command is processed by
> the hardware. Any following MDIO read/write commands after a write
> command will wait for a cleared busy flag.

--
With best wishes,
Vladimir

[U-Boot] [PATCH] net: lpc32xx: Fix MDIO busy wait

[MESSIER, ALEXANDRE]

Hello Vladimir,

> Hi Alexandre,
>
> On 16.12.2015 20:37, amessier.tyco at gmail.com wrote:
> > From: Alexandre Messier <amessier@tycoint.com>
> >
> > The MDIO read function waits on the busy flag after issuing the read
> > command,
>
> that's correct, after issuing the read command and before register read.
>
> > while the MDIO write function waits on the busy flag before
> > issuing the write command.
>
> and that is not correct, I believe.
>
> From the spec (MII Mgmt Indicators Register):
>
>       For PHY Write if scan is not used:
>       1. Write 0 to MCMD
>       2. Write PHY address and register address to MADR
>       3. Write data to MWTD
> -->   4. Wait for busy bit to be cleared in MIND
>
>       For PHY Read if scan is not used:
>       1. Write 1 to MCMD
>       2. Write PHY address and register address to MADR
> -->   3. Wait for busy bit to be cleared in MIND
>       4. Write 0 to MCMD
>       5. Read data from MRDD
>
> Could you please test/review an alternative fix? I believe it adds proper
> serialization of all command sequences (read/read, read/write, write/read
> and write/write). Thank you in advance.

Yes, this approach is fine too. I tested it and it fixes the issue.

If you want to submit it as a new patch:
Tested-by: Alexandre Messier <amessier@tycoint.com>

>
> diff --git a/drivers/net/lpc32xx_eth.c b/drivers/net/lpc32xx_eth.c
> index e76e9bc..3ba5b4b 100644
> --- a/drivers/net/lpc32xx_eth.c
> +++ b/drivers/net/lpc32xx_eth.c
> @@ -304,6 +304,13 @@ static int mii_reg_write(const char *devname, u8
> phy_adr, u8 reg_ofs, u16 data)
>               return -EFAULT;
>       }
>
> +     /* write the phy and reg addressse into the MII address reg */
> +     writel((phy_adr << MADR_PHY_OFFSET) | (reg_ofs <<
> MADR_REG_OFFSET),
> +            &regs->madr);
> +
> +     /* write data to the MII write register */
> +     writel(data, &regs->mwtd);
> +
>       /* wait till the MII is not busy */
>       timeout = MII_TIMEOUT;
>       do {
> @@ -319,13 +326,6 @@ static int mii_reg_write(const char *devname, u8
> phy_adr, u8 reg_ofs, u16 data)
>               return -EFAULT;
>       }
>
> -     /* write the phy and reg addressse into the MII address reg */
> -     writel((phy_adr << MADR_PHY_OFFSET) | (reg_ofs <<
> MADR_REG_OFFSET),
> -            &regs->madr);
> -
> -     /* write data to the MII write register */
> -     writel(data, &regs->mwtd);
> -
>       /*debug("%s:(adr %d, off %d) <= %04x\n", __func__, phy_adr,
>               reg_ofs, data);*/
>
>
> > This causes an issue when writing then immediately reading. As the MDIO
> > module is still busy, the read command is not processed. The wait on
> > busy flag in the read command passes because the previous write
> command
> > finishes. In the end, the value returned by the read function is
> > whatever was present in the MDIO data register.
> >
> > Fix the issue by making sure the busy flag is cleared before issuing a
> > read command. This way, it is still possible to issue a write command
> > and continue executing u-boot code while the command is processed by
> > the hardware. Any following MDIO read/write commands after a write
> > command will wait for a cleared busy flag.
>
> --
> With best wishes,
> Vladimir
>
>


________________________________

This e-mail contains privileged and confidential information intended for the use of the addressees named above. If you are not the intended recipient of this e-mail, you are hereby notified that you must not disseminate, copy or take any action in respect of any information contained in it. If you have received this e-mail in error, please notify the sender immediately by e-mail and immediately destroy this e-mail and its attachments.