[U-Boot] [PATCH] net: lpc32xx: Fix MDIO busy wait
MESSIER, ALEXANDRE
amessier at Tycoint.com
Tue Dec 22 23:41:28 CET 2015
Hello Vladimir,
> Hi Alexandre,
>
> On 16.12.2015 20:37, amessier.tyco at gmail.com wrote:
> > From: Alexandre Messier <amessier at 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 at 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),
> + ®s->madr);
> +
> + /* write data to the MII write register */
> + writel(data, ®s->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),
> - ®s->madr);
> -
> - /* write data to the MII write register */
> - writel(data, ®s->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.
More information about the U-Boot
mailing list