Messages in this thread | | | From | Parshuram Raju Thombare <> | Subject | RE: [PATCH v2 2/3] net: ethernet: cadence: add c45 PHY support in MDIO read/write functions. | Date | Sat, 23 Mar 2019 04:15:31 +0000 |
| |
>-----Original Message----- >From: Nicolas.Ferre@microchip.com <Nicolas.Ferre@microchip.com> >Sent: Thursday, March 21, 2019 7:43 PM >To: Parshuram Raju Thombare <pthombar@cadence.com>; andrew@lunn.ch >Cc: davem@davemloft.net; netdev@vger.kernel.org; f.fainelli@gmail.com; >hkallweit1@gmail.com; linux-kernel@vger.kernel.org; Rafal Ciepiela ><rafalc@cadence.com>; Piotr Sroka <piotrs@cadence.com>; Jan Kotas ><jank@cadence.com> >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 <pthombar@cadence.com> >> --- > >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
| |