[PATCH v3] net: fsl: Fix busy flag polling register
NXP's mEMAC reference manual, Chapter 6.5.5 "MDIO Ethernet Management
Interface usage", specifies to poll the BSY (0) bit in the CFG/STAT
register to wait until a transaction has finished, not bit 31 in the
data register.
In the Linux kernel, this has already been fixed in commit 26eee0210ad7
("net/fsl: fix a bug in xgmac_mdio").
This patch changes the register in the fman_mdio and fsl_ls_mdio
drivers.
As the MDIO_DATA_BSY define is no longer in use, this patch also removes
its definition from the fsl_memac header.
Signed-off-by: Markus Koch
---
v1->v2:
* Fix register
v2->v3:
* Also apply fix to fsl_ls_mdio
* Add note about define-removal in commit message
Thanks, Camelia!
drivers/net/fm/memac_phy.c | 2 +-
drivers/net/fsl_ls_mdio.c | 4 ++--
include/fsl_memac.h| 1 -
3 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/net/fm/memac_phy.c b/drivers/net/fm/memac_phy.c
index 72b500a6d1..3ddae97e09 100644
--- a/drivers/net/fm/memac_phy.c
+++ b/drivers/net/fm/memac_phy.c
@@ -64,7 +64,7 @@ static int memac_wait_until_done(struct memac_mdio_controller
*regs)
{
unsigned int timeout = MAX_NUM_RETRIES;
- while ((memac_in_32(®s->mdio_data) & MDIO_DATA_BSY) && timeout--)
+ while ((memac_in_32(®s->mdio_stat) & MDIO_STAT_BSY) && timeout--)
;
if (!timeout) {
diff --git a/drivers/net/fsl_ls_mdio.c b/drivers/net/fsl_ls_mdio.c
index 6d4e682fdf..f213e0dd85 100644
--- a/drivers/net/fsl_ls_mdio.c
+++ b/drivers/net/fsl_ls_mdio.c
@@ -84,7 +84,7 @@ static int dm_fsl_ls_mdio_read(struct udevice *dev, int addr,
memac_out_32(®s->mdio_ctl, mdio_ctl);
/* Wait till the MDIO write is complete */
- while ((memac_in_32(®s->mdio_data)) & MDIO_DATA_BSY)
+ while ((memac_in_32(®s->mdio_stat)) & MDIO_STAT_BSY)
;
/* Return all Fs if nothing was there */
@@ -107,7 +107,7 @@ static int dm_fsl_ls_mdio_write(struct udevice *dev, int
addr, int devad,
memac_out_32(®s->mdio_data, MDIO_DATA(val));
/* Wait till the MDIO write is complete */
- while ((memac_in_32(®s->mdio_data)) & MDIO_DATA_BSY)
+ while ((memac_in_32(®s->mdio_stat)) & MDIO_STAT_BSY)
;
return 0;
diff --git a/include/fsl_memac.h b/include/fsl_memac.h
index d067f1511c..6ac1e558b9 100644
--- a/include/fsl_memac.h
+++ b/include/fsl_memac.h
@@ -254,7 +254,6 @@ struct memac_mdio_controller {
#define MDIO_CTL_READ (1 << 15)
#define MDIO_DATA(x) (x & 0x)
-#define MDIO_DATA_BSY (1 << 31)
struct fsl_enet_mac;
--
2.34.1
Re: [PATCH v2] net: fsl_mdio: Fix busy flag polling register
On 1/7/22 17:23, Camelia Alexandra Groza (OSS) wrote:
-Original Message-
From: U-Boot On Behalf Of Markus Koch
Sent: Tuesday, January 4, 2022 17:42
To: Ioana Ciornei ; joe.hershber...@ni.com;
rfried@gmail.com
Cc: Madalin Bucur (OSS) ; Camelia Alexandra
Groza ; u-boot@lists.denx.de; Markus Koch
Subject: [PATCH v2] net: fsl_mdio: Fix busy flag polling register
NXP's mEMAC reference manual, Chapter 6.5.5 "MDIO Ethernet
Management
Interface usage", specifies to poll the BSY (0) bit in the CFG/STAT
register to wait until a transaction has finished, not bit 31 in the
data register.
In the Linux kernel, this has already been fixed in commit 26eee0210ad7
("net/fsl: fix a bug in xgmac_mdio").
Signed-off-by: Markus Koch
I am ok with the change for the mEMAC driver but MDIO_DATA_BSY is still
used by the fsl_ls_mdio driver. The MDIO driver suffers from the same issue.
Please send a v3 and either patch both drivers or don't remove the define.
Also, if removing the define, please mention it explicitly in the patch
description.
I'll change the other driver as well.
There's also the fm/tgec_phy driver that uses the same construct, but it has
its own define for MDIO_DATA_BSY. Should I also change that? I think it
corresponds to Linux' xgmac_mdio.c which also uses the STAT_BSY register, but
I don't have a datasheet to verify the register definition.
Thanks,
Markus
Thanks,
Camelia
---
Changed to use the mdio_stat register. Thanks, Ioana!
drivers/net/fm/memac_phy.c | 2 +-
include/fsl_memac.h| 1 -
2 files changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/fm/memac_phy.c b/drivers/net/fm/memac_phy.c
index 72b500a6d1..3ddae97e09 100644
--- a/drivers/net/fm/memac_phy.c
+++ b/drivers/net/fm/memac_phy.c
@@ -64,7 +64,7 @@ static int memac_wait_until_done(struct
memac_mdio_controller *regs)
{
unsigned int timeout = MAX_NUM_RETRIES;
- while ((memac_in_32(®s->mdio_data) & MDIO_DATA_BSY) &&
timeout--)
+ while ((memac_in_32(®s->mdio_stat) & MDIO_STAT_BSY) &&
timeout--)
;
if (!timeout) {
diff --git a/include/fsl_memac.h b/include/fsl_memac.h
index d067f1511c..6ac1e558b9 100644
--- a/include/fsl_memac.h
+++ b/include/fsl_memac.h
@@ -254,7 +254,6 @@ struct memac_mdio_controller {
#define MDIO_CTL_READ (1 << 15)
#define MDIO_DATA(x) (x & 0x)
-#define MDIO_DATA_BSY (1 << 31)
struct fsl_enet_mac;
--
2.34.1
[PATCH v2] net: fsl_mdio: Fix busy flag polling register
NXP's mEMAC reference manual, Chapter 6.5.5 "MDIO Ethernet Management
Interface usage", specifies to poll the BSY (0) bit in the CFG/STAT
register to wait until a transaction has finished, not bit 31 in the
data register.
In the Linux kernel, this has already been fixed in commit 26eee0210ad7
("net/fsl: fix a bug in xgmac_mdio").
Signed-off-by: Markus Koch
---
Changed to use the mdio_stat register. Thanks, Ioana!
drivers/net/fm/memac_phy.c | 2 +-
include/fsl_memac.h| 1 -
2 files changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/fm/memac_phy.c b/drivers/net/fm/memac_phy.c
index 72b500a6d1..3ddae97e09 100644
--- a/drivers/net/fm/memac_phy.c
+++ b/drivers/net/fm/memac_phy.c
@@ -64,7 +64,7 @@ static int memac_wait_until_done(struct memac_mdio_controller
*regs)
{
unsigned int timeout = MAX_NUM_RETRIES;
- while ((memac_in_32(®s->mdio_data) & MDIO_DATA_BSY) && timeout--)
+ while ((memac_in_32(®s->mdio_stat) & MDIO_STAT_BSY) && timeout--)
;
if (!timeout) {
diff --git a/include/fsl_memac.h b/include/fsl_memac.h
index d067f1511c..6ac1e558b9 100644
--- a/include/fsl_memac.h
+++ b/include/fsl_memac.h
@@ -254,7 +254,6 @@ struct memac_mdio_controller {
#define MDIO_CTL_READ (1 << 15)
#define MDIO_DATA(x) (x & 0x)
-#define MDIO_DATA_BSY (1 << 31)
struct fsl_enet_mac;
--
2.34.1
[PATCH] net: fsl_mdio: Fix busy flag polling register
NXP's mEMAC reference manual, Chapter 6.5.5 "MDIO Ethernet Management
Interface usage", specifies to poll the BSY (0) bit in the CFG (we call
it CTL) register to wait until a transaction has finished, not bit 31 in
the data register.
In the Linux kernel, this has already been fixed in commit 26eee0210ad7
("net/fsl: fix a bug in xgmac_mdio").
Signed-off-by: Markus Koch
---
I only stumbled over this section of code while looking at something else, but
I'm surprised this even works the way it is now. Maybe it's luck.
Sadly I have not yet had the chance to test this change on actual hardware, and
I'm not sure I will anytime soon, so I'm asking whether there's anyone who
could compile and run my code to see whether MDIO transactions work as expected.
Thanks!
Markus
drivers/net/fm/memac_phy.c | 2 +-
include/fsl_memac.h| 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/fm/memac_phy.c b/drivers/net/fm/memac_phy.c
index 72b500a6d1..0af6e83a8f 100644
--- a/drivers/net/fm/memac_phy.c
+++ b/drivers/net/fm/memac_phy.c
@@ -64,7 +64,7 @@ static int memac_wait_until_done(struct memac_mdio_controller
*regs)
{
unsigned int timeout = MAX_NUM_RETRIES;
- while ((memac_in_32(®s->mdio_data) & MDIO_DATA_BSY) && timeout--)
+ while ((memac_in_32(®s->mdio_ctl) & MDIO_CTL_BSY) && timeout--)
;
if (!timeout) {
diff --git a/include/fsl_memac.h b/include/fsl_memac.h
index d067f1511c..d973fc0a5e 100644
--- a/include/fsl_memac.h
+++ b/include/fsl_memac.h
@@ -246,6 +246,7 @@ struct memac_mdio_controller {
#define MDIO_STAT_HOLD_15_CLK (7 << 2)
#define MDIO_STAT_NEG (1 << 23)
+#define MDIO_CTL_BSY (1 << 0)
#define MDIO_CTL_DEV_ADDR(x) (x & 0x1f)
#define MDIO_CTL_PORT_ADDR(x) ((x & 0x1f) << 5)
#define MDIO_CTL_PRE_DIS (1 << 10)
@@ -254,7 +255,6 @@ struct memac_mdio_controller {
#define MDIO_CTL_READ (1 << 15)
#define MDIO_DATA(x) (x & 0x)
-#define MDIO_DATA_BSY (1 << 31)
struct fsl_enet_mac;
--
2.34.1
