This patch modify MDIO read/write functions to support
communication with C45 PHY in Cadence ethernet controller driver.
Signed-off-by: Parshuram Thombare <[email protected]>
---
drivers/net/ethernet/cadence/macb.h | 15 +++++--
drivers/net/ethernet/cadence/macb_main.c | 61 ++++++++++++++++++++++++-----
2 files changed, 61 insertions(+), 15 deletions(-)
diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
index bed4ded..59c23e0 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -636,10 +636,17 @@
#define GEM_CLK_DIV96 5
/* Constants for MAN register */
-#define MACB_MAN_SOF 1
-#define MACB_MAN_WRITE 1
-#define MACB_MAN_READ 2
-#define MACB_MAN_CODE 2
+#define MACB_MAN_C22_SOF 1
+#define MACB_MAN_C22_WRITE 1
+#define MACB_MAN_C22_READ 2
+#define MACB_MAN_C22_CODE 2
+
+#define MACB_MAN_C45_SOF 0
+#define MACB_MAN_C45_ADDR 0
+#define MACB_MAN_C45_WRITE 1
+#define MACB_MAN_C45_POST_READ_INCR 2
+#define MACB_MAN_C45_READ 3
+#define MACB_MAN_C45_CODE 2
/* Capability mask bits */
#define MACB_CAPS_ISR_CLEAR_ON_WRITE 0x00000001
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 4f4f8e5..2494abf 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -323,11 +323,30 @@ static int macb_mdio_read(struct mii_bus *bus, int mii_id, int regnum)
struct macb *bp = bus->priv;
int value;
- macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_SOF)
- | MACB_BF(RW, MACB_MAN_READ)
- | MACB_BF(PHYA, mii_id)
- | MACB_BF(REGA, regnum)
- | MACB_BF(CODE, MACB_MAN_CODE)));
+ if (regnum & MII_ADDR_C45) {
+ macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_C45_SOF)
+ | MACB_BF(RW, MACB_MAN_C45_ADDR)
+ | MACB_BF(PHYA, mii_id)
+ | MACB_BF(REGA, (regnum >> 16) & 0x1F)
+ | MACB_BF(DATA, regnum & 0xFFFF)
+ | MACB_BF(CODE, MACB_MAN_C45_CODE)));
+
+ /* wait for end of transfer */
+ while (!MACB_BFEXT(IDLE, macb_readl(bp, NSR)))
+ cpu_relax();
+
+ macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_C45_SOF)
+ | MACB_BF(RW, MACB_MAN_C45_READ)
+ | MACB_BF(PHYA, mii_id)
+ | MACB_BF(REGA, (regnum >> 16) & 0x1F)
+ | MACB_BF(CODE, MACB_MAN_C45_CODE)));
+ } else {
+ macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_C22_SOF)
+ | MACB_BF(RW, MACB_MAN_C22_READ)
+ | MACB_BF(PHYA, mii_id)
+ | MACB_BF(REGA, regnum)
+ | MACB_BF(CODE, MACB_MAN_C22_CODE)));
+ }
/* wait for end of transfer */
while (!MACB_BFEXT(IDLE, macb_readl(bp, NSR)))
@@ -343,12 +362,32 @@ static int macb_mdio_write(struct mii_bus *bus, int mii_id, int regnum,
{
struct macb *bp = bus->priv;
- macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_SOF)
- | MACB_BF(RW, MACB_MAN_WRITE)
- | MACB_BF(PHYA, mii_id)
- | MACB_BF(REGA, regnum)
- | MACB_BF(CODE, MACB_MAN_CODE)
- | MACB_BF(DATA, value)));
+ if (regnum & MII_ADDR_C45) {
+ macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_C45_SOF)
+ | MACB_BF(RW, MACB_MAN_C45_ADDR)
+ | MACB_BF(PHYA, mii_id)
+ | MACB_BF(REGA, (regnum >> 16) & 0x1F)
+ | MACB_BF(DATA, regnum & 0xFFFF)
+ | MACB_BF(CODE, MACB_MAN_C45_CODE)));
+
+ /* wait for end of transfer */
+ while (!MACB_BFEXT(IDLE, macb_readl(bp, NSR)))
+ cpu_relax();
+
+ macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_C45_SOF)
+ | MACB_BF(RW, MACB_MAN_C45_WRITE)
+ | MACB_BF(PHYA, mii_id)
+ | MACB_BF(REGA, (regnum >> 16) & 0x1F)
+ | MACB_BF(CODE, MACB_MAN_C45_CODE)
+ | MACB_BF(DATA, value))); //Data
+ } else {
+ macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_C22_SOF)
+ | MACB_BF(RW, MACB_MAN_C22_WRITE)
+ | MACB_BF(PHYA, mii_id)
+ | MACB_BF(REGA, regnum)
+ | MACB_BF(CODE, MACB_MAN_C22_CODE)
+ | MACB_BF(DATA, value)));
+ }
/* wait for end of transfer */
while (!MACB_BFEXT(IDLE, macb_readl(bp, NSR)))
--
1.7.1
On Fri, Feb 22, 2019 at 08:12:42PM +0000, Parshuram Thombare wrote:
> This patch modify MDIO read/write functions to support
> communication with C45 PHY in Cadence ethernet controller driver.
Hi Parshuram
Are all versions of the MDIO controller capable of doing C45?
Andrew
Regards,
Parshuram Thombare
>-----Original Message-----
>From: Andrew Lunn <[email protected]>
>Sent: Saturday, February 23, 2019 3:12 AM
>To: Parshuram Raju Thombare <[email protected]>
>Cc: [email protected]; [email protected];
>[email protected]; [email protected]; [email protected]; linux-
>[email protected]; Rafal Ciepiela <[email protected]>; Piotr Sroka
><[email protected]>; Jan Kotas <[email protected]>
>Subject: Re: [PATCH 2/3] net: ethernet: add c45 PHY support in MDIO read/write
>functions.
>
>EXTERNAL MAIL
>
>
>On Fri, Feb 22, 2019 at 08:12:42PM +0000, Parshuram Thombare wrote:
>> This patch modify MDIO read/write functions to support communication
>> with C45 PHY in Cadence ethernet controller driver.
>
>Hi Parshuram
>
>Are all versions of the MDIO controller capable of doing C45?
>
> Andrew
Now driver support c22 and c45 PHY.
Are you suggesting to add check for C45 PHY using is_c45 in phydev ?
Regards,
Parshuram Thombare
> >On Fri, Feb 22, 2019 at 08:12:42PM +0000, Parshuram Thombare wrote:
> >> This patch modify MDIO read/write functions to support communication
> >> with C45 PHY in Cadence ethernet controller driver.
> >
> >Hi Parshuram
> >
> >Are all versions of the MDIO controller capable of doing C45?
> >
> > Andrew
> Now driver support c22 and c45 PHY.
> Are you suggesting to add check for C45 PHY using is_c45 in phydev ?
You are unconditionally supporting C45. Are there versions of the
hardware which don't actually support C45? You have this endless loop:
+ /* wait for end of transfer */
+ while (!MACB_BFEXT(IDLE, macb_readl(bp, NSR)))
+ cpu_relax();
If there is hardware which does not support C45, will this loop
forever?
Andrew
On Fri, Feb 22, 2019 at 08:12:42PM +0000, Parshuram Thombare wrote:
> This patch modify MDIO read/write functions to support
> communication with C45 PHY in Cadence ethernet controller driver.
>
> Signed-off-by: Parshuram Thombare <[email protected]>
> ---
> drivers/net/ethernet/cadence/macb.h | 15 +++++--
> drivers/net/ethernet/cadence/macb_main.c | 61 ++++++++++++++++++++++++-----
> 2 files changed, 61 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
> index bed4ded..59c23e0 100644
> --- a/drivers/net/ethernet/cadence/macb.h
> +++ b/drivers/net/ethernet/cadence/macb.h
> @@ -636,10 +636,17 @@
> #define GEM_CLK_DIV96 5
>
> /* Constants for MAN register */
> -#define MACB_MAN_SOF 1
> -#define MACB_MAN_WRITE 1
> -#define MACB_MAN_READ 2
> -#define MACB_MAN_CODE 2
> +#define MACB_MAN_C22_SOF 1
> +#define MACB_MAN_C22_WRITE 1
> +#define MACB_MAN_C22_READ 2
> +#define MACB_MAN_C22_CODE 2
> +
> +#define MACB_MAN_C45_SOF 0
> +#define MACB_MAN_C45_ADDR 0
> +#define MACB_MAN_C45_WRITE 1
> +#define MACB_MAN_C45_POST_READ_INCR 2
> +#define MACB_MAN_C45_READ 3
> +#define MACB_MAN_C45_CODE 2
>
> /* Capability mask bits */
> #define MACB_CAPS_ISR_CLEAR_ON_WRITE 0x00000001
> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
> index 4f4f8e5..2494abf 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -323,11 +323,30 @@ static int macb_mdio_read(struct mii_bus *bus, int mii_id, int regnum)
> struct macb *bp = bus->priv;
> int value;
>
> - macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_SOF)
> - | MACB_BF(RW, MACB_MAN_READ)
> - | MACB_BF(PHYA, mii_id)
> - | MACB_BF(REGA, regnum)
> - | MACB_BF(CODE, MACB_MAN_CODE)));
> + if (regnum & MII_ADDR_C45) {
> + macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_C45_SOF)
> + | MACB_BF(RW, MACB_MAN_C45_ADDR)
> + | MACB_BF(PHYA, mii_id)
> + | MACB_BF(REGA, (regnum >> 16) & 0x1F)
> + | MACB_BF(DATA, regnum & 0xFFFF)
> + | MACB_BF(CODE, MACB_MAN_C45_CODE)));
> +
> + /* wait for end of transfer */
> + while (!MACB_BFEXT(IDLE, macb_readl(bp, NSR)))
> + cpu_relax();
You need a timeout here, and anywhere you wait for the hardware to
complete. Try to make use of readx_poll_timeout() variants.
Andrew
>> >On Fri, Feb 22, 2019 at 08:12:42PM +0000, Parshuram Thombare wrote:
>> >> This patch modify MDIO read/write functions to support
>> >> communication with C45 PHY in Cadence ethernet controller driver.
>> >
>> >Hi Parshuram
>> >
>> >Are all versions of the MDIO controller capable of doing C45?
>> >
>> > Andrew
>> Now driver support c22 and c45 PHY.
>> Are you suggesting to add check for C45 PHY using is_c45 in phydev ?
>
>You are unconditionally supporting C45. Are there versions of the hardware which
>don't actually support C45? You have this endless loop:
There is controller which don't support C45. I will add check for that using is_c45.
>
>+ /* wait for end of transfer */
>+ while (!MACB_BFEXT(IDLE, macb_readl(bp, NSR)))
>+ cpu_relax();
>
>If there is hardware which does not support C45, will this loop forever?
>
> Andrew
Yes, this bit is supposed to be set. I will add timeout here.
Regards,
Parshuram Thombare
>On Fri, Feb 22, 2019 at 08:12:42PM +0000, Parshuram Thombare wrote:
>> This patch modify MDIO read/write functions to support communication
>> with C45 PHY in Cadence ethernet controller driver.
>>
>> Signed-off-by: Parshuram Thombare <[email protected]>
>> ---
>> drivers/net/ethernet/cadence/macb.h | 15 +++++--
>> drivers/net/ethernet/cadence/macb_main.c | 61
>++++++++++++++++++++++++-----
>> 2 files changed, 61 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/cadence/macb.h
>> b/drivers/net/ethernet/cadence/macb.h
>> index bed4ded..59c23e0 100644
>> --- a/drivers/net/ethernet/cadence/macb.h
>> +++ b/drivers/net/ethernet/cadence/macb.h
>> @@ -636,10 +636,17 @@
>> #define GEM_CLK_DIV96 5
>>
>> /* Constants for MAN register */
>> -#define MACB_MAN_SOF 1
>> -#define MACB_MAN_WRITE 1
>> -#define MACB_MAN_READ 2
>> -#define MACB_MAN_CODE 2
>> +#define MACB_MAN_C22_SOF 1
>> +#define MACB_MAN_C22_WRITE 1
>> +#define MACB_MAN_C22_READ 2
>> +#define MACB_MAN_C22_CODE 2
>> +
>> +#define MACB_MAN_C45_SOF 0
>> +#define MACB_MAN_C45_ADDR 0
>> +#define MACB_MAN_C45_WRITE 1
>> +#define MACB_MAN_C45_POST_READ_INCR 2
>> +#define MACB_MAN_C45_READ 3
>> +#define MACB_MAN_C45_CODE 2
>>
>> /* Capability mask bits */
>> #define MACB_CAPS_ISR_CLEAR_ON_WRITE 0x00000001
>> diff --git a/drivers/net/ethernet/cadence/macb_main.c
>> b/drivers/net/ethernet/cadence/macb_main.c
>> index 4f4f8e5..2494abf 100644
>> --- a/drivers/net/ethernet/cadence/macb_main.c
>> +++ b/drivers/net/ethernet/cadence/macb_main.c
>> @@ -323,11 +323,30 @@ static int macb_mdio_read(struct mii_bus *bus, int
>mii_id, int regnum)
>> struct macb *bp = bus->priv;
>> int value;
>>
>> - macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_SOF)
>> - | MACB_BF(RW, MACB_MAN_READ)
>> - | MACB_BF(PHYA, mii_id)
>> - | MACB_BF(REGA, regnum)
>> - | MACB_BF(CODE, MACB_MAN_CODE)));
>> + if (regnum & MII_ADDR_C45) {
>> + macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_C45_SOF)
>> + | MACB_BF(RW, MACB_MAN_C45_ADDR)
>> + | MACB_BF(PHYA, mii_id)
>> + | MACB_BF(REGA, (regnum >> 16) & 0x1F)
>> + | MACB_BF(DATA, regnum & 0xFFFF)
>> + | MACB_BF(CODE, MACB_MAN_C45_CODE)));
>> +
>> + /* wait for end of transfer */
>> + while (!MACB_BFEXT(IDLE, macb_readl(bp, NSR)))
>> + cpu_relax();
>
>You need a timeout here, and anywhere you wait for the hardware to complete.
>Try to make use of readx_poll_timeout() variants.
>
> Andrew
Yes, I will add timeout here.
Regards,
Parshuram Thombare
Sorry for sending this patch again, but I didn't sent previous
email --in-reply-to last comment on v1 of this patch. So
rectifying this mistake.
This version 2 of patch to modify MDIO read/write functions to support
communication with C45 PHY in Cadence ethernet controller driver.
Changes:
1. Added timeout
2. Removed unused operation macro MACB_MAN_C45_POST_READ_INCR
I thought of starting with relatively smaller, independant and simpler changes.
This patch is independant of patch series and looks relatively straight forward
with aim of supporting C45 PHY for support of high speed PHY's.
Signed-off-by: Parshuram Thombare <[email protected]>
---
drivers/net/ethernet/cadence/macb.h | 14 +++++--
drivers/net/ethernet/cadence/macb_main.c | 61 ++++++++++++++++++++++++-----
2 files changed, 60 insertions(+), 15 deletions(-)
diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
index acc66a7..d25fa03 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -629,10 +629,16 @@
#define GEM_CLK_DIV96 5
/* Constants for MAN register */
-#define MACB_MAN_SOF 1
-#define MACB_MAN_WRITE 1
-#define MACB_MAN_READ 2
-#define MACB_MAN_CODE 2
+#define MACB_MAN_C22_SOF 1
+#define MACB_MAN_C22_WRITE 1
+#define MACB_MAN_C22_READ 2
+#define MACB_MAN_C22_CODE 2
+
+#define MACB_MAN_C45_SOF 0
+#define MACB_MAN_C45_ADDR 0
+#define MACB_MAN_C45_WRITE 1
+#define MACB_MAN_C45_READ 3
+#define MACB_MAN_C45_CODE 2
/* Capability mask bits */
#define MACB_CAPS_ISR_CLEAR_ON_WRITE 0x00000001
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index ad099fd..17072fd 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -345,11 +345,30 @@ static int macb_mdio_read(struct mii_bus *bus, int mii_id, int regnum)
if (status < 0)
goto mdio_read_exit;
- macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_SOF)
- | MACB_BF(RW, MACB_MAN_READ)
- | MACB_BF(PHYA, mii_id)
- | MACB_BF(REGA, regnum)
- | MACB_BF(CODE, MACB_MAN_CODE)));
+ if (regnum & MII_ADDR_C45) {
+ macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_C45_SOF)
+ | MACB_BF(RW, MACB_MAN_C45_ADDR)
+ | MACB_BF(PHYA, mii_id)
+ | MACB_BF(REGA, (regnum >> 16) & 0x1F)
+ | MACB_BF(DATA, regnum & 0xFFFF)
+ | MACB_BF(CODE, MACB_MAN_C45_CODE)));
+
+ status = macb_mdio_wait_for_idle(bp);
+ if (status < 0)
+ goto mdio_read_exit;
+
+ macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_C45_SOF)
+ | MACB_BF(RW, MACB_MAN_C45_READ)
+ | MACB_BF(PHYA, mii_id)
+ | MACB_BF(REGA, (regnum >> 16) & 0x1F)
+ | MACB_BF(CODE, MACB_MAN_C45_CODE)));
+ } else {
+ macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_C22_SOF)
+ | MACB_BF(RW, MACB_MAN_C22_READ)
+ | MACB_BF(PHYA, mii_id)
+ | MACB_BF(REGA, regnum)
+ | MACB_BF(CODE, MACB_MAN_C22_CODE)));
+ }
status = macb_mdio_wait_for_idle(bp);
if (status < 0)
@@ -378,12 +397,32 @@ static int macb_mdio_write(struct mii_bus *bus, int mii_id, int regnum,
if (status < 0)
goto mdio_write_exit;
- macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_SOF)
- | MACB_BF(RW, MACB_MAN_WRITE)
- | MACB_BF(PHYA, mii_id)
- | MACB_BF(REGA, regnum)
- | MACB_BF(CODE, MACB_MAN_CODE)
- | MACB_BF(DATA, value)));
+ if (regnum & MII_ADDR_C45) {
+ macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_C45_SOF)
+ | MACB_BF(RW, MACB_MAN_C45_ADDR)
+ | MACB_BF(PHYA, mii_id)
+ | MACB_BF(REGA, (regnum >> 16) & 0x1F)
+ | MACB_BF(DATA, regnum & 0xFFFF)
+ | MACB_BF(CODE, MACB_MAN_C45_CODE)));
+
+ status = macb_mdio_wait_for_idle(bp);
+ if (status < 0)
+ goto mdio_write_exit;
+
+ macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_C45_SOF)
+ | MACB_BF(RW, MACB_MAN_C45_WRITE)
+ | MACB_BF(PHYA, mii_id)
+ | MACB_BF(REGA, (regnum >> 16) & 0x1F)
+ | MACB_BF(CODE, MACB_MAN_C45_CODE)
+ | MACB_BF(DATA, value)));
+ } else {
+ macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_C22_SOF)
+ | MACB_BF(RW, MACB_MAN_C22_WRITE)
+ | MACB_BF(PHYA, mii_id)
+ | MACB_BF(REGA, regnum)
+ | MACB_BF(CODE, MACB_MAN_C22_CODE)
+ | MACB_BF(DATA, value)));
+ }
status = macb_mdio_wait_for_idle(bp);
if (status < 0)
--
1.7.1
On 3/18/19 10:42 AM, Parshuram Thombare wrote:
> Sorry for sending this patch again, but I didn't sent previous
> email --in-reply-to last comment on v1 of this patch. So
> rectifying this mistake.
You have 3 patches in your series, you need to resend all of them even
if there is only one to which you are making changes, this is not
documented in netdev-FAQ.rst though, let's update that.
>
> This version 2 of patch to modify MDIO read/write functions to support
> communication with C45 PHY in Cadence ethernet controller driver.
>
> Changes:
> 1. Added timeout
> 2. Removed unused operation macro MACB_MAN_C45_POST_READ_INCR
>
> I thought of starting with relatively smaller, independant and simpler changes.
> This patch is independant of patch series and looks relatively straight forward
> with aim of supporting C45 PHY for support of high speed PHY's.
>
> Signed-off-by: Parshuram Thombare <[email protected]>
> ---
> drivers/net/ethernet/cadence/macb.h | 14 +++++--
> drivers/net/ethernet/cadence/macb_main.c | 61 ++++++++++++++++++++++++-----
> 2 files changed, 60 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
> index acc66a7..d25fa03 100644
> --- a/drivers/net/ethernet/cadence/macb.h
> +++ b/drivers/net/ethernet/cadence/macb.h
> @@ -629,10 +629,16 @@
> #define GEM_CLK_DIV96 5
>
> /* Constants for MAN register */
> -#define MACB_MAN_SOF 1
> -#define MACB_MAN_WRITE 1
> -#define MACB_MAN_READ 2
> -#define MACB_MAN_CODE 2
> +#define MACB_MAN_C22_SOF 1
> +#define MACB_MAN_C22_WRITE 1
> +#define MACB_MAN_C22_READ 2
> +#define MACB_MAN_C22_CODE 2
> +
> +#define MACB_MAN_C45_SOF 0
> +#define MACB_MAN_C45_ADDR 0
> +#define MACB_MAN_C45_WRITE 1
> +#define MACB_MAN_C45_READ 3
> +#define MACB_MAN_C45_CODE 2
>
> /* Capability mask bits */
> #define MACB_CAPS_ISR_CLEAR_ON_WRITE 0x00000001
> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
> index ad099fd..17072fd 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -345,11 +345,30 @@ static int macb_mdio_read(struct mii_bus *bus, int mii_id, int regnum)
> if (status < 0)
> goto mdio_read_exit;
>
> - macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_SOF)
> - | MACB_BF(RW, MACB_MAN_READ)
> - | MACB_BF(PHYA, mii_id)
> - | MACB_BF(REGA, regnum)
> - | MACB_BF(CODE, MACB_MAN_CODE)));
> + if (regnum & MII_ADDR_C45) {
> + macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_C45_SOF)
> + | MACB_BF(RW, MACB_MAN_C45_ADDR)
> + | MACB_BF(PHYA, mii_id)
> + | MACB_BF(REGA, (regnum >> 16) & 0x1F)
> + | MACB_BF(DATA, regnum & 0xFFFF)
> + | MACB_BF(CODE, MACB_MAN_C45_CODE)));
> +
> + status = macb_mdio_wait_for_idle(bp);
> + if (status < 0)
> + goto mdio_read_exit;
> +
> + macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_C45_SOF)
> + | MACB_BF(RW, MACB_MAN_C45_READ)
> + | MACB_BF(PHYA, mii_id)
> + | MACB_BF(REGA, (regnum >> 16) & 0x1F)
> + | MACB_BF(CODE, MACB_MAN_C45_CODE)));
> + } else {
> + macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_C22_SOF)
> + | MACB_BF(RW, MACB_MAN_C22_READ)
> + | MACB_BF(PHYA, mii_id)
> + | MACB_BF(REGA, regnum)
> + | MACB_BF(CODE, MACB_MAN_C22_CODE)));
> + }
>
> status = macb_mdio_wait_for_idle(bp);
> if (status < 0)
> @@ -378,12 +397,32 @@ static int macb_mdio_write(struct mii_bus *bus, int mii_id, int regnum,
> if (status < 0)
> goto mdio_write_exit;
>
> - macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_SOF)
> - | MACB_BF(RW, MACB_MAN_WRITE)
> - | MACB_BF(PHYA, mii_id)
> - | MACB_BF(REGA, regnum)
> - | MACB_BF(CODE, MACB_MAN_CODE)
> - | MACB_BF(DATA, value)));
> + if (regnum & MII_ADDR_C45) {
> + macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_C45_SOF)
> + | MACB_BF(RW, MACB_MAN_C45_ADDR)
> + | MACB_BF(PHYA, mii_id)
> + | MACB_BF(REGA, (regnum >> 16) & 0x1F)
> + | MACB_BF(DATA, regnum & 0xFFFF)
> + | MACB_BF(CODE, MACB_MAN_C45_CODE)));
> +
> + status = macb_mdio_wait_for_idle(bp);
> + if (status < 0)
> + goto mdio_write_exit;
> +
> + macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_C45_SOF)
> + | MACB_BF(RW, MACB_MAN_C45_WRITE)
> + | MACB_BF(PHYA, mii_id)
> + | MACB_BF(REGA, (regnum >> 16) & 0x1F)
> + | MACB_BF(CODE, MACB_MAN_C45_CODE)
> + | MACB_BF(DATA, value)));
> + } else {
> + macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_C22_SOF)
> + | MACB_BF(RW, MACB_MAN_C22_WRITE)
> + | MACB_BF(PHYA, mii_id)
> + | MACB_BF(REGA, regnum)
> + | MACB_BF(CODE, MACB_MAN_C22_CODE)
> + | MACB_BF(DATA, value)));
> + }
>
> status = macb_mdio_wait_for_idle(bp);
> if (status < 0)
>
--
Florian
Thanks for quick reply.
I am still working on other patches, so I think I will send all of them in another single mail chain.
Regards,
Parshuram Thombare
>-----Original Message-----
>From: Florian Fainelli <[email protected]>
>Sent: Monday, March 18, 2019 11:15 PM
>To: Parshuram Raju Thombare <[email protected]>; [email protected]
>Cc: [email protected]; [email protected];
>[email protected]; [email protected]; [email protected];
>Rafal Ciepiela <[email protected]>; Piotr Sroka <[email protected]>; Jan
>Kotas <[email protected]>
>Subject: Re: [PATCH v2 2/3] net: ethernet: cadence: add c45 PHY support in MDIO
>read/write functions.
>
>EXTERNAL MAIL
>
>
>On 3/18/19 10:42 AM, Parshuram Thombare wrote:
>> Sorry for sending this patch again, but I didn't sent previous email
>> --in-reply-to last comment on v1 of this patch. So rectifying this
>> mistake.
>
>You have 3 patches in your series, you need to resend all of them even if there is
>only one to which you are making changes, this is not documented in netdev-
>FAQ.rst though, let's update that.
>
>>
>> This version 2 of patch to modify MDIO read/write functions to support
>> communication with C45 PHY in Cadence ethernet controller driver.
>>
>> Changes:
>> 1. Added timeout
>> 2. Removed unused operation macro
>MACB_MAN_C45_POST_READ_INCR
>>
>> I thought of starting with relatively smaller, independant and simpler changes.
>> This patch is independant of patch series and looks relatively
>> straight forward with aim of supporting C45 PHY for support of high speed
>PHY's.
>>
>> Signed-off-by: Parshuram Thombare <[email protected]>
>> ---
>> drivers/net/ethernet/cadence/macb.h | 14 +++++--
>> drivers/net/ethernet/cadence/macb_main.c | 61
>++++++++++++++++++++++++-----
>> 2 files changed, 60 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/cadence/macb.h
>> b/drivers/net/ethernet/cadence/macb.h
>> index acc66a7..d25fa03 100644
>> --- a/drivers/net/ethernet/cadence/macb.h
>> +++ b/drivers/net/ethernet/cadence/macb.h
>> @@ -629,10 +629,16 @@
>> #define GEM_CLK_DIV96 5
>>
>> /* Constants for MAN register */
>> -#define MACB_MAN_SOF 1
>> -#define MACB_MAN_WRITE 1
>> -#define MACB_MAN_READ 2
>> -#define MACB_MAN_CODE 2
>> +#define MACB_MAN_C22_SOF 1
>> +#define MACB_MAN_C22_WRITE 1
>> +#define MACB_MAN_C22_READ 2
>> +#define MACB_MAN_C22_CODE 2
>> +
>> +#define MACB_MAN_C45_SOF 0
>> +#define MACB_MAN_C45_ADDR 0
>> +#define MACB_MAN_C45_WRITE 1
>> +#define MACB_MAN_C45_READ 3
>> +#define MACB_MAN_C45_CODE 2
>>
>> /* Capability mask bits */
>> #define MACB_CAPS_ISR_CLEAR_ON_WRITE 0x00000001
>> diff --git a/drivers/net/ethernet/cadence/macb_main.c
>> b/drivers/net/ethernet/cadence/macb_main.c
>> index ad099fd..17072fd 100644
>> --- a/drivers/net/ethernet/cadence/macb_main.c
>> +++ b/drivers/net/ethernet/cadence/macb_main.c
>> @@ -345,11 +345,30 @@ static int macb_mdio_read(struct mii_bus *bus, int
>mii_id, int regnum)
>> if (status < 0)
>> goto mdio_read_exit;
>>
>> - macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_SOF)
>> - | MACB_BF(RW, MACB_MAN_READ)
>> - | MACB_BF(PHYA, mii_id)
>> - | MACB_BF(REGA, regnum)
>> - | MACB_BF(CODE, MACB_MAN_CODE)));
>> + if (regnum & MII_ADDR_C45) {
>> + macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_C45_SOF)
>> + | MACB_BF(RW, MACB_MAN_C45_ADDR)
>> + | MACB_BF(PHYA, mii_id)
>> + | MACB_BF(REGA, (regnum >> 16) & 0x1F)
>> + | MACB_BF(DATA, regnum & 0xFFFF)
>> + | MACB_BF(CODE, MACB_MAN_C45_CODE)));
>> +
>> + status = macb_mdio_wait_for_idle(bp);
>> + if (status < 0)
>> + goto mdio_read_exit;
>> +
>> + macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_C45_SOF)
>> + | MACB_BF(RW, MACB_MAN_C45_READ)
>> + | MACB_BF(PHYA, mii_id)
>> + | MACB_BF(REGA, (regnum >> 16) & 0x1F)
>> + | MACB_BF(CODE, MACB_MAN_C45_CODE)));
>> + } else {
>> + macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_C22_SOF)
>> + | MACB_BF(RW, MACB_MAN_C22_READ)
>> + | MACB_BF(PHYA, mii_id)
>> + | MACB_BF(REGA, regnum)
>> + | MACB_BF(CODE, MACB_MAN_C22_CODE)));
>> + }
>>
>> status = macb_mdio_wait_for_idle(bp);
>> if (status < 0)
>> @@ -378,12 +397,32 @@ static int macb_mdio_write(struct mii_bus *bus, int
>mii_id, int regnum,
>> if (status < 0)
>> goto mdio_write_exit;
>>
>> - macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_SOF)
>> - | MACB_BF(RW, MACB_MAN_WRITE)
>> - | MACB_BF(PHYA, mii_id)
>> - | MACB_BF(REGA, regnum)
>> - | MACB_BF(CODE, MACB_MAN_CODE)
>> - | MACB_BF(DATA, value)));
>> + if (regnum & MII_ADDR_C45) {
>> + macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_C45_SOF)
>> + | MACB_BF(RW, MACB_MAN_C45_ADDR)
>> + | MACB_BF(PHYA, mii_id)
>> + | MACB_BF(REGA, (regnum >> 16) & 0x1F)
>> + | MACB_BF(DATA, regnum & 0xFFFF)
>> + | MACB_BF(CODE, MACB_MAN_C45_CODE)));
>> +
>> + status = macb_mdio_wait_for_idle(bp);
>> + if (status < 0)
>> + goto mdio_write_exit;
>> +
>> + macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_C45_SOF)
>> + | MACB_BF(RW, MACB_MAN_C45_WRITE)
>> + | MACB_BF(PHYA, mii_id)
>> + | MACB_BF(REGA, (regnum >> 16) & 0x1F)
>> + | MACB_BF(CODE, MACB_MAN_C45_CODE)
>> + | MACB_BF(DATA, value)));
>> + } else {
>> + macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_C22_SOF)
>> + | MACB_BF(RW, MACB_MAN_C22_WRITE)
>> + | MACB_BF(PHYA, mii_id)
>> + | MACB_BF(REGA, regnum)
>> + | MACB_BF(CODE, MACB_MAN_C22_CODE)
>> + | MACB_BF(DATA, value)));
>> + }
>>
>> status = macb_mdio_wait_for_idle(bp);
>> if (status < 0)
>>
>
>
>--
>Florian
$subject line should be:
"net: macb: Add c45 PHY support in MDIO read/write functions"
On 18/03/2019 at 18:42, Parshuram Thombare wrote:
> Sorry for sending this patch again, but I didn't sent previous
> email --in-reply-to last comment on v1 of this patch. So
> rectifying this mistake.
>
> This version 2 of patch to modify MDIO read/write functions to support
> communication with C45 PHY in Cadence ethernet controller driver.
>
> Changes:
> 1. Added timeout
> 2. Removed unused operation macro MACB_MAN_C45_POST_READ_INCR
>
> I thought of starting with relatively smaller, independant and simpler changes.
> This patch is independant of patch series and looks relatively straight forward
> with aim of supporting C45 PHY for support of high speed PHY's.
Most of this must go below the "---" line hereunder...
>
> Signed-off-by: Parshuram Thombare <[email protected]>
> ---
There ^.
And the commit message before should look like:
"
Modify MDIO read/write functions to support communication with C45 PHY
in Cadence ethernet controller driver.
"
> drivers/net/ethernet/cadence/macb.h | 14 +++++--
> drivers/net/ethernet/cadence/macb_main.c | 61 ++++++++++++++++++++++++-----
> 2 files changed, 60 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
> index acc66a7..d25fa03 100644
> --- a/drivers/net/ethernet/cadence/macb.h
> +++ b/drivers/net/ethernet/cadence/macb.h
> @@ -629,10 +629,16 @@
> #define GEM_CLK_DIV96 5
>
> /* Constants for MAN register */
> -#define MACB_MAN_SOF 1
> -#define MACB_MAN_WRITE 1
> -#define MACB_MAN_READ 2
> -#define MACB_MAN_CODE 2
> +#define MACB_MAN_C22_SOF 1
> +#define MACB_MAN_C22_WRITE 1
> +#define MACB_MAN_C22_READ 2
> +#define MACB_MAN_C22_CODE 2
> +
> +#define MACB_MAN_C45_SOF 0
> +#define MACB_MAN_C45_ADDR 0
> +#define MACB_MAN_C45_WRITE 1
> +#define MACB_MAN_C45_READ 3
> +#define MACB_MAN_C45_CODE 2
You changed tabs to spaces here: please conform to preceding driver's
style and other lines of this .h file.
>
> /* Capability mask bits */
> #define MACB_CAPS_ISR_CLEAR_ON_WRITE 0x00000001
> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
> index ad099fd..17072fd 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -345,11 +345,30 @@ static int macb_mdio_read(struct mii_bus *bus, int mii_id, int regnum)
> if (status < 0)
> goto mdio_read_exit;
>
> - macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_SOF)
> - | MACB_BF(RW, MACB_MAN_READ)
> - | MACB_BF(PHYA, mii_id)
> - | MACB_BF(REGA, regnum)
> - | MACB_BF(CODE, MACB_MAN_CODE)));
> + if (regnum & MII_ADDR_C45) {
> + macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_C45_SOF)
> + | MACB_BF(RW, MACB_MAN_C45_ADDR)
> + | MACB_BF(PHYA, mii_id)
> + | MACB_BF(REGA, (regnum >> 16) & 0x1F)
> + | MACB_BF(DATA, regnum & 0xFFFF)
> + | MACB_BF(CODE, MACB_MAN_C45_CODE)));
> +
> + status = macb_mdio_wait_for_idle(bp);
> + if (status < 0)
> + goto mdio_read_exit;
> +
> + macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_C45_SOF)
> + | MACB_BF(RW, MACB_MAN_C45_READ)
> + | MACB_BF(PHYA, mii_id)
> + | MACB_BF(REGA, (regnum >> 16) & 0x1F)
> + | MACB_BF(CODE, MACB_MAN_C45_CODE)));
> + } else {
> + macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_C22_SOF)
> + | MACB_BF(RW, MACB_MAN_C22_READ)
> + | MACB_BF(PHYA, mii_id)
> + | MACB_BF(REGA, regnum)
> + | MACB_BF(CODE, MACB_MAN_C22_CODE)));
> + }
>
> status = macb_mdio_wait_for_idle(bp);
> if (status < 0)
> @@ -378,12 +397,32 @@ static int macb_mdio_write(struct mii_bus *bus, int mii_id, int regnum,
> if (status < 0)
> goto mdio_write_exit;
>
> - macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_SOF)
> - | MACB_BF(RW, MACB_MAN_WRITE)
> - | MACB_BF(PHYA, mii_id)
> - | MACB_BF(REGA, regnum)
> - | MACB_BF(CODE, MACB_MAN_CODE)
> - | MACB_BF(DATA, value)));
> + if (regnum & MII_ADDR_C45) {
> + macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_C45_SOF)
> + | MACB_BF(RW, MACB_MAN_C45_ADDR)
> + | MACB_BF(PHYA, mii_id)
> + | MACB_BF(REGA, (regnum >> 16) & 0x1F)
> + | MACB_BF(DATA, regnum & 0xFFFF)
> + | MACB_BF(CODE, MACB_MAN_C45_CODE)));
> +
> + status = macb_mdio_wait_for_idle(bp);
> + if (status < 0)
> + goto mdio_write_exit;
> +
> + macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_C45_SOF)
> + | MACB_BF(RW, MACB_MAN_C45_WRITE)
> + | MACB_BF(PHYA, mii_id)
> + | MACB_BF(REGA, (regnum >> 16) & 0x1F)
> + | MACB_BF(CODE, MACB_MAN_C45_CODE)
> + | MACB_BF(DATA, value)));
> + } else {
> + macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_C22_SOF)
> + | MACB_BF(RW, MACB_MAN_C22_WRITE)
> + | MACB_BF(PHYA, mii_id)
> + | MACB_BF(REGA, regnum)
> + | MACB_BF(CODE, MACB_MAN_C22_CODE)
> + | MACB_BF(DATA, value)));
> + }
>
> status = macb_mdio_wait_for_idle(bp);
> if (status < 0)
Otherwise it looks correct to me on the macb point-of-view.
Please correct the little things noted before, re-send independently as
a v3 and we also make sure that things are good on the phy side.
Best regards,
--
Nicolas Ferre
On Mon, Mar 18, 2019 at 05:42:28PM +0000, Parshuram Thombare wrote:
> Sorry for sending this patch again, but I didn't sent previous
> email --in-reply-to last comment on v1 of this patch. So
> rectifying this mistake.
>
> This version 2 of patch to modify MDIO read/write functions to support
> communication with C45 PHY in Cadence ethernet controller driver.
>
> Changes:
> 1. Added timeout
> 2. Removed unused operation macro MACB_MAN_C45_POST_READ_INCR
>
> I thought of starting with relatively smaller, independant and simpler changes.
> This patch is independant of patch series and looks relatively straight forward
> with aim of supporting C45 PHY for support of high speed PHY's.
Hi Parshuram
Something i asked last time, but i'm not sure i got an answer. Do all
generations of the MDIO controller support C45? If the older versions
only support C22, we need to be sure it does the right thing when
asked to do a C45 transfer.
Andrew
>-----Original Message-----
>From: [email protected] <[email protected]>
>Sent: Thursday, March 21, 2019 7:43 PM
>To: Parshuram Raju Thombare <[email protected]>; [email protected]
>Cc: [email protected]; [email protected]; [email protected];
>[email protected]; [email protected]; Rafal Ciepiela
><[email protected]>; Piotr Sroka <[email protected]>; Jan Kotas
><[email protected]>
>Subject: Re: [PATCH v2 2/3] net: ethernet: cadence: add c45 PHY support in MDIO
>read/write functions.
>
>EXTERNAL MAIL
>
>
>$subject line should be:
>
>"net: macb: Add c45 PHY support in MDIO read/write functions"
>
>
Ok, I will change subject in v3 of this patch.
>On 18/03/2019 at 18:42, Parshuram Thombare wrote:
>> Sorry for sending this patch again, but I didn't sent previous email
>> --in-reply-to last comment on v1 of this patch. So rectifying this
>> mistake.
>>
>> This version 2 of patch to modify MDIO read/write functions to support
>> communication with C45 PHY in Cadence ethernet controller driver.
>>
>> Changes:
>> 1. Added timeout
>> 2. Removed unused operation macro
>MACB_MAN_C45_POST_READ_INCR
>>
>> I thought of starting with relatively smaller, independant and simpler changes.
>> This patch is independant of patch series and looks relatively
>> straight forward with aim of supporting C45 PHY for support of high speed
>PHY's.
>
>Most of this must go below the "---" line hereunder...
>
>
>>
>> Signed-off-by: Parshuram Thombare <[email protected]>
>> ---
>
>There ^.
>
>And the commit message before should look like:
>"
>Modify MDIO read/write functions to support communication with C45 PHY in
>Cadence ethernet controller driver.
>"
>
Ok.
>> drivers/net/ethernet/cadence/macb.h | 14 +++++--
>> drivers/net/ethernet/cadence/macb_main.c | 61
>++++++++++++++++++++++++-----
>> 2 files changed, 60 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/cadence/macb.h
>> b/drivers/net/ethernet/cadence/macb.h
>> index acc66a7..d25fa03 100644
>> --- a/drivers/net/ethernet/cadence/macb.h
>> +++ b/drivers/net/ethernet/cadence/macb.h
>> @@ -629,10 +629,16 @@
>> #define GEM_CLK_DIV96 5
>>
>> /* Constants for MAN register */
>> -#define MACB_MAN_SOF 1
>> -#define MACB_MAN_WRITE 1
>> -#define MACB_MAN_READ 2
>> -#define MACB_MAN_CODE 2
>> +#define MACB_MAN_C22_SOF 1
>> +#define MACB_MAN_C22_WRITE 1
>> +#define MACB_MAN_C22_READ 2
>> +#define MACB_MAN_C22_CODE 2
>> +
>> +#define MACB_MAN_C45_SOF 0
>> +#define MACB_MAN_C45_ADDR 0
>> +#define MACB_MAN_C45_WRITE 1
>> +#define MACB_MAN_C45_READ 3
>> +#define MACB_MAN_C45_CODE 2
>
>
>You changed tabs to spaces here: please conform to preceding driver's style and
>other lines of this .h file.
>
I will change spaces by tab.
>>
>> /* Capability mask bits */
>> #define MACB_CAPS_ISR_CLEAR_ON_WRITE 0x00000001
>> diff --git a/drivers/net/ethernet/cadence/macb_main.c
>> b/drivers/net/ethernet/cadence/macb_main.c
>> index ad099fd..17072fd 100644
>> --- a/drivers/net/ethernet/cadence/macb_main.c
>> +++ b/drivers/net/ethernet/cadence/macb_main.c
>> @@ -345,11 +345,30 @@ static int macb_mdio_read(struct mii_bus *bus, int
>mii_id, int regnum)
>> if (status < 0)
>> goto mdio_read_exit;
>>
>> - macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_SOF)
>> - | MACB_BF(RW, MACB_MAN_READ)
>> - | MACB_BF(PHYA, mii_id)
>> - | MACB_BF(REGA, regnum)
>> - | MACB_BF(CODE, MACB_MAN_CODE)));
>> + if (regnum & MII_ADDR_C45) {
>> + macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_C45_SOF)
>> + | MACB_BF(RW, MACB_MAN_C45_ADDR)
>> + | MACB_BF(PHYA, mii_id)
>> + | MACB_BF(REGA, (regnum >> 16) & 0x1F)
>> + | MACB_BF(DATA, regnum & 0xFFFF)
>> + | MACB_BF(CODE, MACB_MAN_C45_CODE)));
>> +
>> + status = macb_mdio_wait_for_idle(bp);
>> + if (status < 0)
>> + goto mdio_read_exit;
>> +
>> + macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_C45_SOF)
>> + | MACB_BF(RW, MACB_MAN_C45_READ)
>> + | MACB_BF(PHYA, mii_id)
>> + | MACB_BF(REGA, (regnum >> 16) & 0x1F)
>> + | MACB_BF(CODE, MACB_MAN_C45_CODE)));
>> + } else {
>> + macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_C22_SOF)
>> + | MACB_BF(RW, MACB_MAN_C22_READ)
>> + | MACB_BF(PHYA, mii_id)
>> + | MACB_BF(REGA, regnum)
>> + | MACB_BF(CODE, MACB_MAN_C22_CODE)));
>> + }
>>
>> status = macb_mdio_wait_for_idle(bp);
>> if (status < 0)
>> @@ -378,12 +397,32 @@ static int macb_mdio_write(struct mii_bus *bus, int
>mii_id, int regnum,
>> if (status < 0)
>> goto mdio_write_exit;
>>
>> - macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_SOF)
>> - | MACB_BF(RW, MACB_MAN_WRITE)
>> - | MACB_BF(PHYA, mii_id)
>> - | MACB_BF(REGA, regnum)
>> - | MACB_BF(CODE, MACB_MAN_CODE)
>> - | MACB_BF(DATA, value)));
>> + if (regnum & MII_ADDR_C45) {
>> + macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_C45_SOF)
>> + | MACB_BF(RW, MACB_MAN_C45_ADDR)
>> + | MACB_BF(PHYA, mii_id)
>> + | MACB_BF(REGA, (regnum >> 16) & 0x1F)
>> + | MACB_BF(DATA, regnum & 0xFFFF)
>> + | MACB_BF(CODE, MACB_MAN_C45_CODE)));
>> +
>> + status = macb_mdio_wait_for_idle(bp);
>> + if (status < 0)
>> + goto mdio_write_exit;
>> +
>> + macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_C45_SOF)
>> + | MACB_BF(RW, MACB_MAN_C45_WRITE)
>> + | MACB_BF(PHYA, mii_id)
>> + | MACB_BF(REGA, (regnum >> 16) & 0x1F)
>> + | MACB_BF(CODE, MACB_MAN_C45_CODE)
>> + | MACB_BF(DATA, value)));
>> + } else {
>> + macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_C22_SOF)
>> + | MACB_BF(RW, MACB_MAN_C22_WRITE)
>> + | MACB_BF(PHYA, mii_id)
>> + | MACB_BF(REGA, regnum)
>> + | MACB_BF(CODE, MACB_MAN_C22_CODE)
>> + | MACB_BF(DATA, value)));
>> + }
>>
>> status = macb_mdio_wait_for_idle(bp);
>> if (status < 0)
>
>Otherwise it looks correct to me on the macb point-of-view.
>
>Please correct the little things noted before, re-send independently as a v3 and
>we also make sure that things are good on the phy side.
>
Thank you for comments. I will make above mentioned changes and re-send this
patch independently as a v3.
>Best regards,
>--
>Nicolas Ferre
Regards,
Parshuram Thombare
>-----Original Message-----
>From: Andrew Lunn <[email protected]>
>Sent: Thursday, March 21, 2019 7:55 PM
>To: Parshuram Raju Thombare <[email protected]>
>Cc: [email protected]; [email protected];
>[email protected]; [email protected]; [email protected]; linux-
>[email protected]; Rafal Ciepiela <[email protected]>; Piotr Sroka
><[email protected]>; Jan Kotas <[email protected]>
>Subject: Re: [PATCH v2 2/3] net: ethernet: cadence: add c45 PHY support in MDIO
>read/write functions.
>
>EXTERNAL MAIL
>
>
>On Mon, Mar 18, 2019 at 05:42:28PM +0000, Parshuram Thombare wrote:
>> Sorry for sending this patch again, but I didn't sent previous email
>> --in-reply-to last comment on v1 of this patch. So rectifying this
>> mistake.
>>
>> This version 2 of patch to modify MDIO read/write functions to support
>> communication with C45 PHY in Cadence ethernet controller driver.
>>
>> Changes:
>> 1. Added timeout
>> 2. Removed unused operation macro
>MACB_MAN_C45_POST_READ_INCR
>>
>> I thought of starting with relatively smaller, independant and simpler changes.
>> This patch is independant of patch series and looks relatively
>> straight forward with aim of supporting C45 PHY for support of high speed
>PHY's.
>
>Hi Parshuram
>
>Something i asked last time, but i'm not sure i got an answer. Do all generations
>of the MDIO controller support C45? If the older versions only support C22, we
>need to be sure it does the right thing when asked to do a C45 transfer.
Hi Andrew,
There are some older versions of controller which doesn't support C45. I will make
sure that driver return error for C45 transfer requests when it is not supported by
controller.
>
> Andrew
Regards,
Parshuram Thombare