* [PATCH] net/fsl: remove func xgmac_wait_until_free() as duplicate
@ 2017-05-08 11:31 Alexandru Ardelean
2017-05-08 19:34 ` David Miller
0 siblings, 1 reply; 3+ messages in thread
From: Alexandru Ardelean @ 2017-05-08 11:31 UTC (permalink / raw)
To: netdev; +Cc: Shaohui.Xie, davem, Alexandru Ardelean
Looking at xgmac_wait_until_done() and xgmac_wait_until_free()
functions, they seem to have turned out completely identical.
Though, judging from the git history it seems they
initially weren't.
Remove xgmac_wait_until_free() in favor of xgmac_wait_until_done().
Signed-off-by: Alexandru Ardelean <ardeleanalex@gmail.com>
---
drivers/net/ethernet/freescale/xgmac_mdio.c | 33 ++++-------------------------
1 file changed, 4 insertions(+), 29 deletions(-)
diff --git a/drivers/net/ethernet/freescale/xgmac_mdio.c b/drivers/net/ethernet/freescale/xgmac_mdio.c
index e03b30c..54597a8 100644
--- a/drivers/net/ethernet/freescale/xgmac_mdio.c
+++ b/drivers/net/ethernet/freescale/xgmac_mdio.c
@@ -71,31 +71,6 @@ static void xgmac_write32(u32 value,
}
/*
- * Wait until the MDIO bus is free
- */
-static int xgmac_wait_until_free(struct device *dev,
- struct tgec_mdio_controller __iomem *regs,
- bool is_little_endian)
-{
- unsigned int timeout;
-
- /* Wait till the bus is free */
- timeout = TIMEOUT;
- while ((xgmac_read32(®s->mdio_stat, is_little_endian) &
- MDIO_STAT_BSY) && timeout) {
- cpu_relax();
- timeout--;
- }
-
- if (!timeout) {
- dev_err(dev, "timeout waiting for bus to be free\n");
- return -ETIMEDOUT;
- }
-
- return 0;
-}
-
-/*
* Wait till the MDIO read or write operation is complete
*/
static int xgmac_wait_until_done(struct device *dev,
@@ -147,7 +122,7 @@ static int xgmac_mdio_write(struct mii_bus *bus, int phy_id, int regnum, u16 val
xgmac_write32(mdio_stat, ®s->mdio_stat, endian);
- ret = xgmac_wait_until_free(&bus->dev, regs, endian);
+ ret = xgmac_wait_until_done(&bus->dev, regs, endian);
if (ret)
return ret;
@@ -159,7 +134,7 @@ static int xgmac_mdio_write(struct mii_bus *bus, int phy_id, int regnum, u16 val
if (regnum & MII_ADDR_C45) {
xgmac_write32(regnum & 0xffff, ®s->mdio_addr, endian);
- ret = xgmac_wait_until_free(&bus->dev, regs, endian);
+ ret = xgmac_wait_until_done(&bus->dev, regs, endian);
if (ret)
return ret;
}
@@ -201,7 +176,7 @@ static int xgmac_mdio_read(struct mii_bus *bus, int phy_id, int regnum)
xgmac_write32(mdio_stat, ®s->mdio_stat, endian);
- ret = xgmac_wait_until_free(&bus->dev, regs, endian);
+ ret = xgmac_wait_until_done(&bus->dev, regs, endian);
if (ret)
return ret;
@@ -213,7 +188,7 @@ static int xgmac_mdio_read(struct mii_bus *bus, int phy_id, int regnum)
if (regnum & MII_ADDR_C45) {
xgmac_write32(regnum & 0xffff, ®s->mdio_addr, endian);
- ret = xgmac_wait_until_free(&bus->dev, regs, endian);
+ ret = xgmac_wait_until_done(&bus->dev, regs, endian);
if (ret)
return ret;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] net/fsl: remove func xgmac_wait_until_free() as duplicate
2017-05-08 11:31 [PATCH] net/fsl: remove func xgmac_wait_until_free() as duplicate Alexandru Ardelean
@ 2017-05-08 19:34 ` David Miller
2017-05-09 6:56 ` Alexandru Ardelean
0 siblings, 1 reply; 3+ messages in thread
From: David Miller @ 2017-05-08 19:34 UTC (permalink / raw)
To: ardeleanalex; +Cc: netdev, Shaohui.Xie
From: Alexandru Ardelean <ardeleanalex@gmail.com>
Date: Mon, 8 May 2017 14:31:18 +0300
> Looking at xgmac_wait_until_done() and xgmac_wait_until_free()
> functions, they seem to have turned out completely identical.
>
> Though, judging from the git history it seems they
> initially weren't.
>
> Remove xgmac_wait_until_free() in favor of xgmac_wait_until_done().
>
> Signed-off-by: Alexandru Ardelean <ardeleanalex@gmail.com>
This situation got created by the commit below, I wonder if it
is even correct.
Certainly the original author of this code intended the MDIO
_data_ rather than the _status_ register to be polled.
Otherwise, why in the world have two functions which are in
every other regard identical?
commit 26eee0210ad72a29eb4a70b34320bda266f91a0d
Author: Shaohui Xie <Shaohui.Xie@freescale.com>
Date: Mon Mar 16 18:55:50 2015 +0800
net/fsl: fix a bug in xgmac_mdio
There is a bug in xgmac_wait_until_done() which mdio_stat should be used
instead of mdio_data when checking if busy bit is cleared.
Signed-off-by: Shaohui Xie <Shaohui.Xie@freescale.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
diff --git a/drivers/net/ethernet/freescale/xgmac_mdio.c b/drivers/net/ethernet/freescale/xgmac_mdio.c
index 3a83bc2..5f691f2 100644
--- a/drivers/net/ethernet/freescale/xgmac_mdio.c
+++ b/drivers/net/ethernet/freescale/xgmac_mdio.c
@@ -79,7 +79,7 @@ static int xgmac_wait_until_done(struct device *dev,
/* Wait till the MDIO write is complete */
timeout = TIMEOUT;
- while ((ioread32be(®s->mdio_data) & MDIO_DATA_BSY) && timeout) {
+ while ((ioread32be(®s->mdio_stat) & MDIO_STAT_BSY) && timeout) {
cpu_relax();
timeout--;
}
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] net/fsl: remove func xgmac_wait_until_free() as duplicate
2017-05-08 19:34 ` David Miller
@ 2017-05-09 6:56 ` Alexandru Ardelean
0 siblings, 0 replies; 3+ messages in thread
From: Alexandru Ardelean @ 2017-05-09 6:56 UTC (permalink / raw)
To: David Miller; +Cc: netdev
On Mon, May 8, 2017 at 10:34 PM, David Miller <davem@davemloft.net> wrote:
> From: Alexandru Ardelean <ardeleanalex@gmail.com>
> Date: Mon, 8 May 2017 14:31:18 +0300
>
>> Looking at xgmac_wait_until_done() and xgmac_wait_until_free()
>> functions, they seem to have turned out completely identical.
>>
>> Though, judging from the git history it seems they
>> initially weren't.
>>
>> Remove xgmac_wait_until_free() in favor of xgmac_wait_until_done().
>>
>> Signed-off-by: Alexandru Ardelean <ardeleanalex@gmail.com>
>
> This situation got created by the commit below, I wonder if it
> is even correct.
>
> Certainly the original author of this code intended the MDIO
> _data_ rather than the _status_ register to be polled.
>
> Otherwise, why in the world have two functions which are in
> every other regard identical?
[ Removed Shaohui.Xie@nxp.com email from thread ; seems email addr no
longer valid ].
I should have added an RFC tag.
Thing is, we just use this driver, so, I can't offer much input here.
I was debugging [a while ago] some other issue related to this and
stumbled over this.
I haven't seem any obvious issues yet, to suggest one thing or or the
other [regarding polling status or data registers].
I would have liked some input from some NXP/Freescale people on this email list.
Or maybe I could forward this to more people that contribute the NXP eco-system.
If you have any suggestions, please advise and I'll adapt.
Thanks
Alex
>
> commit 26eee0210ad72a29eb4a70b34320bda266f91a0d
> Author: Shaohui Xie <Shaohui.Xie@freescale.com>
> Date: Mon Mar 16 18:55:50 2015 +0800
>
> net/fsl: fix a bug in xgmac_mdio
>
> There is a bug in xgmac_wait_until_done() which mdio_stat should be used
> instead of mdio_data when checking if busy bit is cleared.
>
> Signed-off-by: Shaohui Xie <Shaohui.Xie@freescale.com>
> Signed-off-by: David S. Miller <davem@davemloft.net>
>
> diff --git a/drivers/net/ethernet/freescale/xgmac_mdio.c b/drivers/net/ethernet/freescale/xgmac_mdio.c
> index 3a83bc2..5f691f2 100644
> --- a/drivers/net/ethernet/freescale/xgmac_mdio.c
> +++ b/drivers/net/ethernet/freescale/xgmac_mdio.c
> @@ -79,7 +79,7 @@ static int xgmac_wait_until_done(struct device *dev,
>
> /* Wait till the MDIO write is complete */
> timeout = TIMEOUT;
> - while ((ioread32be(®s->mdio_data) & MDIO_DATA_BSY) && timeout) {
> + while ((ioread32be(®s->mdio_stat) & MDIO_STAT_BSY) && timeout) {
> cpu_relax();
> timeout--;
> }
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-05-09 6:57 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-08 11:31 [PATCH] net/fsl: remove func xgmac_wait_until_free() as duplicate Alexandru Ardelean
2017-05-08 19:34 ` David Miller
2017-05-09 6:56 ` Alexandru Ardelean
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).