linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next 0/1] net: fec: add C45 MDIO read/write support
@ 2019-08-19 17:11 Marco Hartmann
  2019-08-19 17:11 ` [PATCH net-next 1/1] " Marco Hartmann
  2019-08-19 22:54 ` [PATCH net-next 0/1] net: " Andrew Lunn
  0 siblings, 2 replies; 9+ messages in thread
From: Marco Hartmann @ 2019-08-19 17:11 UTC (permalink / raw)
  To: Marco Hartmann, Andy Duan, davem, netdev, linux-kernel, Christian Herber

As of yet, the Fast Ethernet Controller (FEC) driver only supports Clause 22
conform MDIO transactions. IEEE 802.3ae Clause 45 defines a modified MDIO
protocol that uses a two staged access model in order to increase the address
space.

This patch adds support for Clause 45 conform MDIO read and write operations to
the FEC driver.

Marco Hartmann (1):
  fec: add C45 MDIO read/write support

 drivers/net/ethernet/freescale/fec_main.c | 65 ++++++++++++++++++++++++++++---
 1 file changed, 59 insertions(+), 6 deletions(-)

-- 
2.7.4


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH net-next 1/1] fec: add C45 MDIO read/write support
  2019-08-19 17:11 [PATCH net-next 0/1] net: fec: add C45 MDIO read/write support Marco Hartmann
@ 2019-08-19 17:11 ` Marco Hartmann
  2019-08-20  1:35   ` David Miller
  2019-08-20  2:08   ` Andy Duan
  2019-08-19 22:54 ` [PATCH net-next 0/1] net: " Andrew Lunn
  1 sibling, 2 replies; 9+ messages in thread
From: Marco Hartmann @ 2019-08-19 17:11 UTC (permalink / raw)
  To: Marco Hartmann, Andy Duan, davem, netdev, linux-kernel, Christian Herber

IEEE 802.3ae clause 45 defines a modified MDIO protocol that uses a two
staged access model in order to increase the address space.

This patch adds support for C45 MDIO read and write accesses, which are
used whenever the MII_ADDR_C45 flag in the regnum argument is set.
In case it is not set, C22 accesses are used as before.

Co-developed-by: Christian Herber <christian.herber@nxp.com>
Signed-off-by: Christian Herber <christian.herber@nxp.com>
Signed-off-by: Marco Hartmann <marco.hartmann@nxp.com>
---
 drivers/net/ethernet/freescale/fec_main.c | 65 ++++++++++++++++++++++++++++---
 1 file changed, 59 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index c01d3ec3e9af..73f8f9a149a1 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -208,8 +208,11 @@ MODULE_PARM_DESC(macaddr, "FEC Ethernet MAC address");
 
 /* FEC MII MMFR bits definition */
 #define FEC_MMFR_ST		(1 << 30)
+#define FEC_MMFR_ST_C45		(0)
 #define FEC_MMFR_OP_READ	(2 << 28)
+#define FEC_MMFR_OP_READ_C45	(3 << 28)
 #define FEC_MMFR_OP_WRITE	(1 << 28)
+#define FEC_MMFR_OP_ADDR_WRITE	(0)
 #define FEC_MMFR_PA(v)		((v & 0x1f) << 23)
 #define FEC_MMFR_RA(v)		((v & 0x1f) << 18)
 #define FEC_MMFR_TA		(2 << 16)
@@ -1767,7 +1770,7 @@ static int fec_enet_mdio_read(struct mii_bus *bus, int mii_id, int regnum)
 	struct fec_enet_private *fep = bus->priv;
 	struct device *dev = &fep->pdev->dev;
 	unsigned long time_left;
-	int ret = 0;
+	int ret = 0, frame_start, frame_addr, frame_op;
 
 	ret = pm_runtime_get_sync(dev);
 	if (ret < 0)
@@ -1775,9 +1778,36 @@ static int fec_enet_mdio_read(struct mii_bus *bus, int mii_id, int regnum)
 
 	reinit_completion(&fep->mdio_done);
 
+	if (MII_ADDR_C45 & regnum) {
+		frame_start = FEC_MMFR_ST_C45;
+
+		/* write address */
+		frame_addr = (regnum >> 16);
+		writel(frame_start | FEC_MMFR_OP_ADDR_WRITE |
+		       FEC_MMFR_PA(mii_id) | FEC_MMFR_RA(frame_addr) |
+		       FEC_MMFR_TA | (regnum & 0xFFFF),
+		       fep->hwp + FEC_MII_DATA);
+
+		/* wait for end of transfer */
+		time_left = wait_for_completion_timeout(&fep->mdio_done,
+				usecs_to_jiffies(FEC_MII_TIMEOUT));
+		if (time_left == 0) {
+			netdev_err(fep->netdev, "MDIO address write timeout\n");
+			ret  = -ETIMEDOUT;
+		}
+
+		frame_op = FEC_MMFR_OP_READ_C45;
+
+	} else {
+		/* C22 read */
+		frame_op = FEC_MMFR_OP_READ;
+		frame_start = FEC_MMFR_ST;
+		frame_addr = regnum;
+	}
+
 	/* start a read op */
-	writel(FEC_MMFR_ST | FEC_MMFR_OP_READ |
-		FEC_MMFR_PA(mii_id) | FEC_MMFR_RA(regnum) |
+	writel(frame_start | frame_op |
+		FEC_MMFR_PA(mii_id) | FEC_MMFR_RA(frame_addr) |
 		FEC_MMFR_TA, fep->hwp + FEC_MII_DATA);
 
 	/* wait for end of transfer */
@@ -1804,7 +1834,7 @@ static int fec_enet_mdio_write(struct mii_bus *bus, int mii_id, int regnum,
 	struct fec_enet_private *fep = bus->priv;
 	struct device *dev = &fep->pdev->dev;
 	unsigned long time_left;
-	int ret;
+	int ret, frame_start, frame_addr;
 
 	ret = pm_runtime_get_sync(dev);
 	if (ret < 0)
@@ -1814,9 +1844,32 @@ static int fec_enet_mdio_write(struct mii_bus *bus, int mii_id, int regnum,
 
 	reinit_completion(&fep->mdio_done);
 
+	if (MII_ADDR_C45 & regnum) {
+		frame_start = FEC_MMFR_ST_C45;
+
+		/* write address */
+		frame_addr = (regnum >> 16);
+		writel(frame_start | FEC_MMFR_OP_ADDR_WRITE |
+		       FEC_MMFR_PA(mii_id) | FEC_MMFR_RA(frame_addr) |
+		       FEC_MMFR_TA | (regnum & 0xFFFF),
+		       fep->hwp + FEC_MII_DATA);
+
+		/* wait for end of transfer */
+		time_left = wait_for_completion_timeout(&fep->mdio_done,
+			usecs_to_jiffies(FEC_MII_TIMEOUT));
+		if (time_left == 0) {
+			netdev_err(fep->netdev, "MDIO address write timeout\n");
+			ret  = -ETIMEDOUT;
+		}
+	} else {
+		/* C22 write */
+		frame_start = FEC_MMFR_ST;
+		frame_addr = regnum;
+	}
+
 	/* start a write op */
-	writel(FEC_MMFR_ST | FEC_MMFR_OP_WRITE |
-		FEC_MMFR_PA(mii_id) | FEC_MMFR_RA(regnum) |
+	writel(frame_start | FEC_MMFR_OP_WRITE |
+		FEC_MMFR_PA(mii_id) | FEC_MMFR_RA(frame_addr) |
 		FEC_MMFR_TA | FEC_MMFR_DATA(value),
 		fep->hwp + FEC_MII_DATA);
 
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH net-next 0/1] net: fec: add C45 MDIO read/write support
  2019-08-19 17:11 [PATCH net-next 0/1] net: fec: add C45 MDIO read/write support Marco Hartmann
  2019-08-19 17:11 ` [PATCH net-next 1/1] " Marco Hartmann
@ 2019-08-19 22:54 ` Andrew Lunn
  2019-08-20  2:32   ` [EXT] " Andy Duan
  1 sibling, 1 reply; 9+ messages in thread
From: Andrew Lunn @ 2019-08-19 22:54 UTC (permalink / raw)
  To: Marco Hartmann; +Cc: Andy Duan, davem, netdev, linux-kernel, Christian Herber

On Mon, Aug 19, 2019 at 05:11:14PM +0000, Marco Hartmann wrote:
> As of yet, the Fast Ethernet Controller (FEC) driver only supports Clause 22
> conform MDIO transactions. IEEE 802.3ae Clause 45 defines a modified MDIO
> protocol that uses a two staged access model in order to increase the address
> space.
> 
> This patch adds support for Clause 45 conform MDIO read and write operations to
> the FEC driver.

Hi Marco

Do all versions of the FEC hardware support C45? Or do we need to make
use of the quirk support in this driver to just enable it for some
revisions of FEC?

Thanks
	Andrew

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH net-next 1/1] fec: add C45 MDIO read/write support
  2019-08-19 17:11 ` [PATCH net-next 1/1] " Marco Hartmann
@ 2019-08-20  1:35   ` David Miller
  2019-08-20  2:08   ` Andy Duan
  1 sibling, 0 replies; 9+ messages in thread
From: David Miller @ 2019-08-20  1:35 UTC (permalink / raw)
  To: marco.hartmann; +Cc: fugang.duan, netdev, linux-kernel, christian.herber

From: Marco Hartmann <marco.hartmann@nxp.com>
Date: Mon, 19 Aug 2019 17:11:14 +0000

> @@ -1767,7 +1770,7 @@ static int fec_enet_mdio_read(struct mii_bus *bus, int mii_id, int regnum)
>  	struct fec_enet_private *fep = bus->priv;
>  	struct device *dev = &fep->pdev->dev;
>  	unsigned long time_left;
> -	int ret = 0;
> +	int ret = 0, frame_start, frame_addr, frame_op;
>  

Please retain the reverse christmas tree ordering of local variables
here, thank you.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [PATCH net-next 1/1] fec: add C45 MDIO read/write support
  2019-08-19 17:11 ` [PATCH net-next 1/1] " Marco Hartmann
  2019-08-20  1:35   ` David Miller
@ 2019-08-20  2:08   ` Andy Duan
  2019-08-21 11:44     ` Marco Hartmann
  1 sibling, 1 reply; 9+ messages in thread
From: Andy Duan @ 2019-08-20  2:08 UTC (permalink / raw)
  To: Marco Hartmann, davem, netdev, linux-kernel, Christian Herber

From: Marco Hartmann Sent: Tuesday, August 20, 2019 1:11 AM
> IEEE 802.3ae clause 45 defines a modified MDIO protocol that uses a two
> staged access model in order to increase the address space.
> 
> This patch adds support for C45 MDIO read and write accesses, which are
> used whenever the MII_ADDR_C45 flag in the regnum argument is set.
> In case it is not set, C22 accesses are used as before.
> 
> Co-developed-by: Christian Herber <christian.herber@nxp.com>
> Signed-off-by: Christian Herber <christian.herber@nxp.com>
> Signed-off-by: Marco Hartmann <marco.hartmann@nxp.com>
> ---
>  drivers/net/ethernet/freescale/fec_main.c | 65
> ++++++++++++++++++++++++++++---
>  1 file changed, 59 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/ethernet/freescale/fec_main.c
> b/drivers/net/ethernet/freescale/fec_main.c
> index c01d3ec3e9af..73f8f9a149a1 100644
> --- a/drivers/net/ethernet/freescale/fec_main.c
> +++ b/drivers/net/ethernet/freescale/fec_main.c
> @@ -208,8 +208,11 @@ MODULE_PARM_DESC(macaddr, "FEC Ethernet
> MAC address");
> 
>  /* FEC MII MMFR bits definition */
>  #define FEC_MMFR_ST		(1 << 30)
> +#define FEC_MMFR_ST_C45		(0)
>  #define FEC_MMFR_OP_READ	(2 << 28)
> +#define FEC_MMFR_OP_READ_C45	(3 << 28)
>  #define FEC_MMFR_OP_WRITE	(1 << 28)
> +#define FEC_MMFR_OP_ADDR_WRITE	(0)
>  #define FEC_MMFR_PA(v)		((v & 0x1f) << 23)
>  #define FEC_MMFR_RA(v)		((v & 0x1f) << 18)
>  #define FEC_MMFR_TA		(2 << 16)
> @@ -1767,7 +1770,7 @@ static int fec_enet_mdio_read(struct mii_bus *bus,
> int mii_id, int regnum)
>  	struct fec_enet_private *fep = bus->priv;
>  	struct device *dev = &fep->pdev->dev;
>  	unsigned long time_left;
> -	int ret = 0;
> +	int ret = 0, frame_start, frame_addr, frame_op;

Add bool variable:

bool is_c45 = !!(regnum & MII_ADDR_C45);
> 
>  	ret = pm_runtime_get_sync(dev);
>  	if (ret < 0)
> @@ -1775,9 +1778,36 @@ static int fec_enet_mdio_read(struct mii_bus
> *bus, int mii_id, int regnum)
> 
>  	reinit_completion(&fep->mdio_done);
> 
> +	if (MII_ADDR_C45 & regnum) {
if (is_c45)

> +		frame_start = FEC_MMFR_ST_C45;
> +
> +		/* write address */
> +		frame_addr = (regnum >> 16);
> +		writel(frame_start | FEC_MMFR_OP_ADDR_WRITE |
> +		       FEC_MMFR_PA(mii_id) | FEC_MMFR_RA(frame_addr) |
> +		       FEC_MMFR_TA | (regnum & 0xFFFF),
> +		       fep->hwp + FEC_MII_DATA);
> +
> +		/* wait for end of transfer */
> +		time_left = wait_for_completion_timeout(&fep->mdio_done,
> +				usecs_to_jiffies(FEC_MII_TIMEOUT));
> +		if (time_left == 0) {
> +			netdev_err(fep->netdev, "MDIO address write timeout\n");
> +			ret  = -ETIMEDOUT;

Should be:
goto out;
> +		}
> +
> +		frame_op = FEC_MMFR_OP_READ_C45;
> +
> +	} else {
> +		/* C22 read */
> +		frame_op = FEC_MMFR_OP_READ;
> +		frame_start = FEC_MMFR_ST;
> +		frame_addr = regnum;
> +	}
> +
>  	/* start a read op */
> -	writel(FEC_MMFR_ST | FEC_MMFR_OP_READ |
> -		FEC_MMFR_PA(mii_id) | FEC_MMFR_RA(regnum) |
> +	writel(frame_start | frame_op |
> +		FEC_MMFR_PA(mii_id) | FEC_MMFR_RA(frame_addr) |
>  		FEC_MMFR_TA, fep->hwp + FEC_MII_DATA);
> 
>  	/* wait for end of transfer */
> @@ -1804,7 +1834,7 @@ static int fec_enet_mdio_write(struct mii_bus *bus,
> int mii_id, int regnum,
>  	struct fec_enet_private *fep = bus->priv;
>  	struct device *dev = &fep->pdev->dev;
>  	unsigned long time_left;
> -	int ret;
> +	int ret, frame_start, frame_addr;
> 
>  	ret = pm_runtime_get_sync(dev);
>  	if (ret < 0)
> @@ -1814,9 +1844,32 @@ static int fec_enet_mdio_write(struct mii_bus
> *bus, int mii_id, int regnum,

bool is_c45 = !!(regnum & MII_ADDR_C45);
> 
>  	reinit_completion(&fep->mdio_done);
> 
> +	if (MII_ADDR_C45 & regnum) {

if (!is_c45) {
> +		frame_start = FEC_MMFR_ST_C45;
> +
> +		/* write address */
> +		frame_addr = (regnum >> 16);
> +		writel(frame_start | FEC_MMFR_OP_ADDR_WRITE |
> +		       FEC_MMFR_PA(mii_id) | FEC_MMFR_RA(frame_addr) |
> +		       FEC_MMFR_TA | (regnum & 0xFFFF),
> +		       fep->hwp + FEC_MII_DATA);
> +
> +		/* wait for end of transfer */
> +		time_left = wait_for_completion_timeout(&fep->mdio_done,
> +			usecs_to_jiffies(FEC_MII_TIMEOUT));
> +		if (time_left == 0) {
> +			netdev_err(fep->netdev, "MDIO address write timeout\n");
> +			ret  = -ETIMEDOUT;
Like mdio read, it should be:
goto out; 
> +		}
> +	} else {
> +		/* C22 write */
> +		frame_start = FEC_MMFR_ST;
> +		frame_addr = regnum;
> +	}
> +
>  	/* start a write op */
> -	writel(FEC_MMFR_ST | FEC_MMFR_OP_WRITE |
> -		FEC_MMFR_PA(mii_id) | FEC_MMFR_RA(regnum) |
> +	writel(frame_start | FEC_MMFR_OP_WRITE |
> +		FEC_MMFR_PA(mii_id) | FEC_MMFR_RA(frame_addr) |
>  		FEC_MMFR_TA | FEC_MMFR_DATA(value),
>  		fep->hwp + FEC_MII_DATA);
> 
> --
> 2.7.4


^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [EXT] Re: [PATCH net-next 0/1] net: fec: add C45 MDIO read/write support
  2019-08-19 22:54 ` [PATCH net-next 0/1] net: " Andrew Lunn
@ 2019-08-20  2:32   ` Andy Duan
  2019-08-20 13:04     ` Andrew Lunn
  0 siblings, 1 reply; 9+ messages in thread
From: Andy Duan @ 2019-08-20  2:32 UTC (permalink / raw)
  To: Andrew Lunn, Marco Hartmann; +Cc: davem, netdev, linux-kernel, Christian Herber

From: Andrew Lunn <andrew@lunn.ch>
> On Mon, Aug 19, 2019 at 05:11:14PM +0000, Marco Hartmann wrote:
> > As of yet, the Fast Ethernet Controller (FEC) driver only supports
> > Clause 22 conform MDIO transactions. IEEE 802.3ae Clause 45 defines a
> > modified MDIO protocol that uses a two staged access model in order to
> > increase the address space.
> >
> > This patch adds support for Clause 45 conform MDIO read and write
> > operations to the FEC driver.
> 
> Hi Marco
> 
> Do all versions of the FEC hardware support C45? Or do we need to make use
> of the quirk support in this driver to just enable it for some revisions of FEC?
> 
> Thanks
>         Andrew

i.MX legacy platforms like i.MX6/7 series, they doesn't support Write & Read Increment.
But for i.MX8MQ/MM series, it support C45 full features like Write & Read Increment.

For the patch itself, it doesn't support Write & Read Increment, so I think the patch doesn't
need to add quirk support.

Andy

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [EXT] Re: [PATCH net-next 0/1] net: fec: add C45 MDIO read/write support
  2019-08-20  2:32   ` [EXT] " Andy Duan
@ 2019-08-20 13:04     ` Andrew Lunn
  2019-08-21  5:55       ` Andy Duan
  0 siblings, 1 reply; 9+ messages in thread
From: Andrew Lunn @ 2019-08-20 13:04 UTC (permalink / raw)
  To: Andy Duan; +Cc: Marco Hartmann, davem, netdev, linux-kernel, Christian Herber

On Tue, Aug 20, 2019 at 02:32:26AM +0000, Andy Duan wrote:
> From: Andrew Lunn <andrew@lunn.ch>
> > On Mon, Aug 19, 2019 at 05:11:14PM +0000, Marco Hartmann wrote:
> > > As of yet, the Fast Ethernet Controller (FEC) driver only supports
> > > Clause 22 conform MDIO transactions. IEEE 802.3ae Clause 45 defines a
> > > modified MDIO protocol that uses a two staged access model in order to
> > > increase the address space.
> > >
> > > This patch adds support for Clause 45 conform MDIO read and write
> > > operations to the FEC driver.
> > 
> > Hi Marco
> > 
> > Do all versions of the FEC hardware support C45? Or do we need to make use
> > of the quirk support in this driver to just enable it for some revisions of FEC?
> > 
> > Thanks
> >         Andrew
> 
> i.MX legacy platforms like i.MX6/7 series, they doesn't support Write & Read Increment.
> But for i.MX8MQ/MM series, it support C45 full features like Write & Read Increment.
> 
> For the patch itself, it doesn't support Write & Read Increment, so I think the patch doesn't
> need to add quirk support.

Hi Andy

So what happens with something older than a i.MX8MQ/MM when a C45
transfer is attempted? This patch adds a new write. Does that write
immediately trigger a completion interrupt? Does it never trigger an
interrupt, and we have to wait FEC_MII_TIMEOUT?

Ideally, if the hardware does not support C45, we want it to return
EOPNOTSUPP.

Thanks
	Andrew

^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [EXT] Re: [PATCH net-next 0/1] net: fec: add C45 MDIO read/write support
  2019-08-20 13:04     ` Andrew Lunn
@ 2019-08-21  5:55       ` Andy Duan
  0 siblings, 0 replies; 9+ messages in thread
From: Andy Duan @ 2019-08-21  5:55 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: Marco Hartmann, davem, netdev, linux-kernel, Christian Herber

From: Andrew Lunn <andrew@lunn.ch> Sent: Tuesday, August 20, 2019 9:04 PM
> On Tue, Aug 20, 2019 at 02:32:26AM +0000, Andy Duan wrote:
> > From: Andrew Lunn <andrew@lunn.ch>
> > > On Mon, Aug 19, 2019 at 05:11:14PM +0000, Marco Hartmann wrote:
> > > > As of yet, the Fast Ethernet Controller (FEC) driver only supports
> > > > Clause 22 conform MDIO transactions. IEEE 802.3ae Clause 45
> > > > defines a modified MDIO protocol that uses a two staged access
> > > > model in order to increase the address space.
> > > >
> > > > This patch adds support for Clause 45 conform MDIO read and write
> > > > operations to the FEC driver.
> > >
> > > Hi Marco
> > >
> > > Do all versions of the FEC hardware support C45? Or do we need to
> > > make use of the quirk support in this driver to just enable it for some
> revisions of FEC?
> > >
> > > Thanks
> > >         Andrew
> >
> > i.MX legacy platforms like i.MX6/7 series, they doesn't support Write & Read
> Increment.
> > But for i.MX8MQ/MM series, it support C45 full features like Write & Read
> Increment.
> >
> > For the patch itself, it doesn't support Write & Read Increment, so I
> > think the patch doesn't need to add quirk support.
> 
> Hi Andy
> 
> So what happens with something older than a i.MX8MQ/MM when a C45
> transfer is attempted? This patch adds a new write. Does that write
> immediately trigger a completion interrupt? Does it never trigger an interrupt,
> and we have to wait FEC_MII_TIMEOUT?
> 
> Ideally, if the hardware does not support C45, we want it to return
> EOPNOTSUPP.
> 
> Thanks
>         Andrew

It still trigger an interrupt to wakeup the completion, we have to wait FEC_MII_TIMEOUT.
Older chips just support part of C45 feature just like the patch implementation. 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH net-next 1/1] fec: add C45 MDIO read/write support
  2019-08-20  2:08   ` Andy Duan
@ 2019-08-21 11:44     ` Marco Hartmann
  0 siblings, 0 replies; 9+ messages in thread
From: Marco Hartmann @ 2019-08-21 11:44 UTC (permalink / raw)
  To: Andy Duan, davem, netdev, linux-kernel, Christian Herber

On 20.08.19 04:08, Andy Duan wrote:
> From: Marco Hartmann Sent: Tuesday, August 20, 2019 1:11 AM
>> IEEE 802.3ae clause 45 defines a modified MDIO protocol that uses a two
>> staged access model in order to increase the address space.
>>
>> This patch adds support for C45 MDIO read and write accesses, which are
>> used whenever the MII_ADDR_C45 flag in the regnum argument is set.
>> In case it is not set, C22 accesses are used as before.
>>
>> Co-developed-by: Christian Herber <christian.herber@nxp.com>
>> Signed-off-by: Christian Herber <christian.herber@nxp.com>
>> Signed-off-by: Marco Hartmann <marco.hartmann@nxp.com>
>> ---
>>   drivers/net/ethernet/freescale/fec_main.c | 65
>> ++++++++++++++++++++++++++++---
>>   1 file changed, 59 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/freescale/fec_main.c
>> b/drivers/net/ethernet/freescale/fec_main.c
>> index c01d3ec3e9af..73f8f9a149a1 100644
>> --- a/drivers/net/ethernet/freescale/fec_main.c
>> +++ b/drivers/net/ethernet/freescale/fec_main.c
>> @@ -208,8 +208,11 @@ MODULE_PARM_DESC(macaddr, "FEC Ethernet
>> MAC address");
>>
>>   /* FEC MII MMFR bits definition */
>>   #define FEC_MMFR_ST		(1 << 30)
>> +#define FEC_MMFR_ST_C45		(0)
>>   #define FEC_MMFR_OP_READ	(2 << 28)
>> +#define FEC_MMFR_OP_READ_C45	(3 << 28)
>>   #define FEC_MMFR_OP_WRITE	(1 << 28)
>> +#define FEC_MMFR_OP_ADDR_WRITE	(0)
>>   #define FEC_MMFR_PA(v)		((v & 0x1f) << 23)
>>   #define FEC_MMFR_RA(v)		((v & 0x1f) << 18)
>>   #define FEC_MMFR_TA		(2 << 16)
>> @@ -1767,7 +1770,7 @@ static int fec_enet_mdio_read(struct mii_bus *bus,
>> int mii_id, int regnum)
>>   	struct fec_enet_private *fep = bus->priv;
>>   	struct device *dev = &fep->pdev->dev;
>>   	unsigned long time_left;
>> -	int ret = 0;
>> +	int ret = 0, frame_start, frame_addr, frame_op;
> 
> Add bool variable:
> 
> bool is_c45 = !!(regnum & MII_ADDR_C45);
>>
>>   	ret = pm_runtime_get_sync(dev);
>>   	if (ret < 0)
>> @@ -1775,9 +1778,36 @@ static int fec_enet_mdio_read(struct mii_bus
>> *bus, int mii_id, int regnum)
>>
>>   	reinit_completion(&fep->mdio_done);
>>
>> +	if (MII_ADDR_C45 & regnum) {
> if (is_c45)
> 
>> +		frame_start = FEC_MMFR_ST_C45;
>> +
>> +		/* write address */
>> +		frame_addr = (regnum >> 16);
>> +		writel(frame_start | FEC_MMFR_OP_ADDR_WRITE |
>> +		       FEC_MMFR_PA(mii_id) | FEC_MMFR_RA(frame_addr) |
>> +		       FEC_MMFR_TA | (regnum & 0xFFFF),
>> +		       fep->hwp + FEC_MII_DATA);
>> +
>> +		/* wait for end of transfer */
>> +		time_left = wait_for_completion_timeout(&fep->mdio_done,
>> +				usecs_to_jiffies(FEC_MII_TIMEOUT));
>> +		if (time_left == 0) {
>> +			netdev_err(fep->netdev, "MDIO address write timeout\n");
>> +			ret  = -ETIMEDOUT;
> 
> Should be:
> goto out;
>> +		}
>> +
>> +		frame_op = FEC_MMFR_OP_READ_C45;
>> +
>> +	} else {
>> +		/* C22 read */
>> +		frame_op = FEC_MMFR_OP_READ;
>> +		frame_start = FEC_MMFR_ST;
>> +		frame_addr = regnum;
>> +	}
>> +
>>   	/* start a read op */
>> -	writel(FEC_MMFR_ST | FEC_MMFR_OP_READ |
>> -		FEC_MMFR_PA(mii_id) | FEC_MMFR_RA(regnum) |
>> +	writel(frame_start | frame_op |
>> +		FEC_MMFR_PA(mii_id) | FEC_MMFR_RA(frame_addr) |
>>   		FEC_MMFR_TA, fep->hwp + FEC_MII_DATA);
>>
>>   	/* wait for end of transfer */
>> @@ -1804,7 +1834,7 @@ static int fec_enet_mdio_write(struct mii_bus *bus,
>> int mii_id, int regnum,
>>   	struct fec_enet_private *fep = bus->priv;
>>   	struct device *dev = &fep->pdev->dev;
>>   	unsigned long time_left;
>> -	int ret;
>> +	int ret, frame_start, frame_addr;
>>
>>   	ret = pm_runtime_get_sync(dev);
>>   	if (ret < 0)
>> @@ -1814,9 +1844,32 @@ static int fec_enet_mdio_write(struct mii_bus
>> *bus, int mii_id, int regnum,
> 
> bool is_c45 = !!(regnum & MII_ADDR_C45);
>>
>>   	reinit_completion(&fep->mdio_done);
>>
>> +	if (MII_ADDR_C45 & regnum) {
> 
> if (!is_c45) {
>> +		frame_start = FEC_MMFR_ST_C45;
>> +
>> +		/* write address */
>> +		frame_addr = (regnum >> 16);
>> +		writel(frame_start | FEC_MMFR_OP_ADDR_WRITE |
>> +		       FEC_MMFR_PA(mii_id) | FEC_MMFR_RA(frame_addr) |
>> +		       FEC_MMFR_TA | (regnum & 0xFFFF),
>> +		       fep->hwp + FEC_MII_DATA);
>> +
>> +		/* wait for end of transfer */
>> +		time_left = wait_for_completion_timeout(&fep->mdio_done,
>> +			usecs_to_jiffies(FEC_MII_TIMEOUT));
>> +		if (time_left == 0) {
>> +			netdev_err(fep->netdev, "MDIO address write timeout\n");
>> +			ret  = -ETIMEDOUT;
> Like mdio read, it should be:
> goto out;
>> +		}
>> +	} else {
>> +		/* C22 write */
>> +		frame_start = FEC_MMFR_ST;
>> +		frame_addr = regnum;
>> +	}
>> +
>>   	/* start a write op */
>> -	writel(FEC_MMFR_ST | FEC_MMFR_OP_WRITE |
>> -		FEC_MMFR_PA(mii_id) | FEC_MMFR_RA(regnum) |
>> +	writel(frame_start | FEC_MMFR_OP_WRITE |
>> +		FEC_MMFR_PA(mii_id) | FEC_MMFR_RA(frame_addr) |
>>   		FEC_MMFR_TA | FEC_MMFR_DATA(value),
>>   		fep->hwp + FEC_MII_DATA);
>>
>> --
>> 2.7.4
> 

Thank you for your feedback,
the fixes are included in v2 of the patch.

Regards,
Marco

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2019-08-21 11:45 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-19 17:11 [PATCH net-next 0/1] net: fec: add C45 MDIO read/write support Marco Hartmann
2019-08-19 17:11 ` [PATCH net-next 1/1] " Marco Hartmann
2019-08-20  1:35   ` David Miller
2019-08-20  2:08   ` Andy Duan
2019-08-21 11:44     ` Marco Hartmann
2019-08-19 22:54 ` [PATCH net-next 0/1] net: " Andrew Lunn
2019-08-20  2:32   ` [EXT] " Andy Duan
2019-08-20 13:04     ` Andrew Lunn
2019-08-21  5:55       ` Andy Duan

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).