* [PATCH v2] net: fsl_mdio: Fix busy flag polling register
@ 2022-01-04 15:41 Markus Koch
2022-01-07 16:23 ` Camelia Alexandra Groza (OSS)
0 siblings, 1 reply; 4+ messages in thread
From: Markus Koch @ 2022-01-04 15:41 UTC (permalink / raw)
To: ioana.ciornei, joe.hershberger, rfried.dev
Cc: madalin.bucur, camelia.groza, u-boot, Markus Koch
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 <markus@notsyncing.net>
---
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 & 0xffff)
-#define MDIO_DATA_BSY (1 << 31)
struct fsl_enet_mac;
--
2.34.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* RE: [PATCH v2] net: fsl_mdio: Fix busy flag polling register
2022-01-04 15:41 [PATCH v2] net: fsl_mdio: Fix busy flag polling register Markus Koch
@ 2022-01-07 16:23 ` Camelia Alexandra Groza (OSS)
2022-01-07 17:49 ` Markus Koch
0 siblings, 1 reply; 4+ messages in thread
From: Camelia Alexandra Groza (OSS) @ 2022-01-07 16:23 UTC (permalink / raw)
To: Markus Koch, Ioana Ciornei, joe.hershberger, rfried.dev
Cc: Madalin Bucur (OSS), u-boot
> -----Original Message-----
> From: U-Boot <u-boot-bounces@lists.denx.de> On Behalf Of Markus Koch
> Sent: Tuesday, January 4, 2022 17:42
> To: Ioana Ciornei <ioana.ciornei@nxp.com>; joe.hershberger@ni.com;
> rfried.dev@gmail.com
> Cc: Madalin Bucur (OSS) <madalin.bucur@oss.nxp.com>; Camelia Alexandra
> Groza <camelia.groza@nxp.com>; u-boot@lists.denx.de; Markus Koch
> <markus@notsyncing.net>
> 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 <markus@notsyncing.net>
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.
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 & 0xffff)
> -#define MDIO_DATA_BSY (1 << 31)
>
> struct fsl_enet_mac;
>
> --
> 2.34.1
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] net: fsl_mdio: Fix busy flag polling register
2022-01-07 16:23 ` Camelia Alexandra Groza (OSS)
@ 2022-01-07 17:49 ` Markus Koch
2022-01-10 17:42 ` Camelia Alexandra Groza (OSS)
0 siblings, 1 reply; 4+ messages in thread
From: Markus Koch @ 2022-01-07 17:49 UTC (permalink / raw)
To: Camelia Alexandra Groza (OSS),
joe.hershberger, rfried.dev, Ioana Ciornei
Cc: Madalin Bucur (OSS), u-boot
On 1/7/22 17:23, Camelia Alexandra Groza (OSS) wrote:
>> -----Original Message-----
>> From: U-Boot <u-boot-bounces@lists.denx.de> On Behalf Of Markus Koch
>> Sent: Tuesday, January 4, 2022 17:42
>> To: Ioana Ciornei <ioana.ciornei@nxp.com>; joe.hershberger@ni.com;
>> rfried.dev@gmail.com
>> Cc: Madalin Bucur (OSS) <madalin.bucur@oss.nxp.com>; Camelia Alexandra
>> Groza <camelia.groza@nxp.com>; u-boot@lists.denx.de; Markus Koch
>> <markus@notsyncing.net>
>> 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 <markus@notsyncing.net>
>
> 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 & 0xffff)
>> -#define MDIO_DATA_BSY (1 << 31)
>>
>> struct fsl_enet_mac;
>>
>> --
>> 2.34.1
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: [PATCH v2] net: fsl_mdio: Fix busy flag polling register
2022-01-07 17:49 ` Markus Koch
@ 2022-01-10 17:42 ` Camelia Alexandra Groza (OSS)
0 siblings, 0 replies; 4+ messages in thread
From: Camelia Alexandra Groza (OSS) @ 2022-01-10 17:42 UTC (permalink / raw)
To: Markus Koch, Camelia Alexandra Groza (OSS),
joe.hershberger, rfried.dev, Ioana Ciornei
Cc: Madalin Bucur (OSS), u-boot
> -----Original Message-----
> From: Markus Koch <markus@notsyncing.net>
> Sent: Friday, January 7, 2022 19:49
> To: Camelia Alexandra Groza (OSS) <camelia.groza@oss.nxp.com>;
> joe.hershberger@ni.com; rfried.dev@gmail.com; Ioana Ciornei
> <ioana.ciornei@nxp.com>
> Cc: Madalin Bucur (OSS) <madalin.bucur@oss.nxp.com>; u-
> boot@lists.denx.de
> Subject: 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 <u-boot-bounces@lists.denx.de> On Behalf Of Markus
> Koch
> >> Sent: Tuesday, January 4, 2022 17:42
> >> To: Ioana Ciornei <ioana.ciornei@nxp.com>; joe.hershberger@ni.com;
> >> rfried.dev@gmail.com
> >> Cc: Madalin Bucur (OSS) <madalin.bucur@oss.nxp.com>; Camelia
> Alexandra
> >> Groza <camelia.groza@nxp.com>; u-boot@lists.denx.de; Markus Koch
> >> <markus@notsyncing.net>
> >> 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 <markus@notsyncing.net>
> >
> > 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.
The TGEC controller has BUSY bits mirroring each other in both the
MDIO_DATA and MDIO_CFG_STAT registers. The tgec_phy driver doesn't need
changing.
Regards,
Camelia
> 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 & 0xffff)
> >> -#define MDIO_DATA_BSY (1 << 31)
> >>
> >> struct fsl_enet_mac;
> >>
> >> --
> >> 2.34.1
> >
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-01-10 17:42 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-04 15:41 [PATCH v2] net: fsl_mdio: Fix busy flag polling register Markus Koch
2022-01-07 16:23 ` Camelia Alexandra Groza (OSS)
2022-01-07 17:49 ` Markus Koch
2022-01-10 17:42 ` Camelia Alexandra Groza (OSS)
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.