| Message ID | 20241119095141.1236414-1-jacky_chou@aspeedtech.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net,v2] net: mdio: aspeed: Add dummy read for fire control | expand |
On Tue, Nov 19, 2024 at 05:51:41PM +0800, Jacky Chou wrote: > When the command bus is sometimes busy, it may cause the command is not > arrived to MDIO controller immediately. On software, the driver issues a > write command to the command bus does not wait for command complete and > it returned back to code immediately. But a read command will wait for > the data back, once a read command was back indicates the previous write > command had arrived to controller. > Add a dummy read to ensure triggering mdio controller before starting > polling the status of mdio controller to avoid polling unexpected timeout. Please have another attempt at writing the commit message. > Fixes: a9770eac511a ("net: mdio: Move MDIO drivers into a new subdirectory") > Signed-off-by: Jacky Chou <jacky_chou@aspeedtech.com> > --- > drivers/net/mdio/mdio-aspeed.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/net/mdio/mdio-aspeed.c b/drivers/net/mdio/mdio-aspeed.c > index c2170650415c..373902d33b96 100644 > --- a/drivers/net/mdio/mdio-aspeed.c > +++ b/drivers/net/mdio/mdio-aspeed.c > @@ -62,6 +62,8 @@ static int aspeed_mdio_op(struct mii_bus *bus, u8 st, u8 op, u8 phyad, u8 regad, > | FIELD_PREP(ASPEED_MDIO_DATA_MIIRDATA, data); > > iowrite32(ctrl, ctx->base + ASPEED_MDIO_CTRL); > + /* Add dummy read to ensure triggering mdio controller */ > + (void)ioread32(ctx->base + ASPEED_MDIO_CTRL); Maybe: /* Dummy read to flush previous write to controller */ Andrew --- pw-bot: cr
Hi Andrew Lunn, Thank you for your reply. > > When the command bus is sometimes busy, it may cause the command is > > not arrived to MDIO controller immediately. On software, the driver > > issues a write command to the command bus does not wait for command > > complete and it returned back to code immediately. But a read command > > will wait for the data back, once a read command was back indicates > > the previous write command had arrived to controller. > > Add a dummy read to ensure triggering mdio controller before starting > > polling the status of mdio controller to avoid polling unexpected timeout. > > Please have another attempt at writing the commit message. > > > Fixes: a9770eac511a ("net: mdio: Move MDIO drivers into a new > > subdirectory") > > Signed-off-by: Jacky Chou <jacky_chou@aspeedtech.com> > > --- > > drivers/net/mdio/mdio-aspeed.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/drivers/net/mdio/mdio-aspeed.c > > b/drivers/net/mdio/mdio-aspeed.c index c2170650415c..373902d33b96 > > 100644 > > --- a/drivers/net/mdio/mdio-aspeed.c > > +++ b/drivers/net/mdio/mdio-aspeed.c > > @@ -62,6 +62,8 @@ static int aspeed_mdio_op(struct mii_bus *bus, u8 st, > u8 op, u8 phyad, u8 regad, > > | FIELD_PREP(ASPEED_MDIO_DATA_MIIRDATA, data); > > > > iowrite32(ctrl, ctx->base + ASPEED_MDIO_CTRL); > > + /* Add dummy read to ensure triggering mdio controller */ > > + (void)ioread32(ctx->base + ASPEED_MDIO_CTRL); > > Maybe: /* Dummy read to flush previous write to controller */ Agree. I will change the title in next version. Thanks, Jacky
On Tue, 2024-11-19 at 17:51 +0800, Jacky Chou wrote: > When the command bus is sometimes busy, it may cause the command is > not > arrived to MDIO controller immediately. On software, the driver > issues a > write command to the command bus does not wait for command complete > and > it returned back to code immediately. But a read command will wait > for > the data back, once a read command was back indicates the previous > write > command had arrived to controller. > Add a dummy read to ensure triggering mdio controller before starting > polling the status of mdio controller to avoid polling unexpected > timeout. Why use the explicit dummy read rather than adjust the poll interval or duration? I still don't think that's been adequately explained given the hardware-clear of the fire bit on completion, which is what we're polling for. Andrew
Hi Andrew Jeffery, Thank you for your reply. > > When the command bus is sometimes busy, it may cause the command is > > not arrived to MDIO controller immediately. On software, the driver > > issues a write command to the command bus does not wait for command > > complete and it returned back to code immediately. But a read command > > will wait for the data back, once a read command was back indicates > > the previous write command had arrived to controller. > > Add a dummy read to ensure triggering mdio controller before starting > > polling the status of mdio controller to avoid polling unexpected > > timeout. > > Why use the explicit dummy read rather than adjust the poll interval or > duration? I still don't think that's been adequately explained given the > hardware-clear of the fire bit on completion, which is what we're polling for. We cannot know exactly what the timeout value of polling should be set to. Because no one could know when the write command has arrived MDIO controller. A dummy read can ensure the previous write command had arrived MDIO controller Therefore, we chose to use a dummy read instead of increasing the timeout value. Thanks, Jacky
diff --git a/drivers/net/mdio/mdio-aspeed.c b/drivers/net/mdio/mdio-aspeed.c index c2170650415c..373902d33b96 100644 --- a/drivers/net/mdio/mdio-aspeed.c +++ b/drivers/net/mdio/mdio-aspeed.c @@ -62,6 +62,8 @@ static int aspeed_mdio_op(struct mii_bus *bus, u8 st, u8 op, u8 phyad, u8 regad, | FIELD_PREP(ASPEED_MDIO_DATA_MIIRDATA, data); iowrite32(ctrl, ctx->base + ASPEED_MDIO_CTRL); + /* Add dummy read to ensure triggering mdio controller */ + (void)ioread32(ctx->base + ASPEED_MDIO_CTRL); return readl_poll_timeout(ctx->base + ASPEED_MDIO_CTRL, ctrl, !(ctrl & ASPEED_MDIO_CTRL_FIRE),
When the command bus is sometimes busy, it may cause the command is not arrived to MDIO controller immediately. On software, the driver issues a write command to the command bus does not wait for command complete and it returned back to code immediately. But a read command will wait for the data back, once a read command was back indicates the previous write command had arrived to controller. Add a dummy read to ensure triggering mdio controller before starting polling the status of mdio controller to avoid polling unexpected timeout. Fixes: a9770eac511a ("net: mdio: Move MDIO drivers into a new subdirectory") Signed-off-by: Jacky Chou <jacky_chou@aspeedtech.com> --- drivers/net/mdio/mdio-aspeed.c | 2 ++ 1 file changed, 2 insertions(+)