diff mbox series

[U-Boot] driver: net: ldpaa_eth: Add PHY-less SGMII support

Message ID 1509959398-5710-1-git-send-email-Ashish.Kumar@nxp.com
State Superseded
Delegated to: York Sun
Headers show
Series [U-Boot] driver: net: ldpaa_eth: Add PHY-less SGMII support | expand

Commit Message

Ashish Kumar Nov. 6, 2017, 9:09 a.m. UTC
In case of PHY-less mode, there is no interaction with PHY
so auto-neg etc is not required and link will have fixed
attributes

Signed-off-by: Ashish Kumar <Ashish.Kumar@nxp.com>
Signed-off-by: Prabhakar Kushwaha <prabhakar.kushwaha@nxp.com>
---
 drivers/net/ldpaa_eth/ldpaa_eth.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

Comments

York Sun Nov. 8, 2017, 8:37 p.m. UTC | #1
On 11/06/2017 01:09 AM, Ashish Kumar wrote:
> In case of PHY-less mode, there is no interaction with PHY
> so auto-neg etc is not required and link will have fixed
> attributes
> 
> Signed-off-by: Ashish Kumar <Ashish.Kumar@nxp.com>
> Signed-off-by: Prabhakar Kushwaha <prabhakar.kushwaha@nxp.com>
> ---
>  drivers/net/ldpaa_eth/ldpaa_eth.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ldpaa_eth/ldpaa_eth.c b/drivers/net/ldpaa_eth/ldpaa_eth.c
> index 21be79a..85e7faa 100644
> --- a/drivers/net/ldpaa_eth/ldpaa_eth.c
> +++ b/drivers/net/ldpaa_eth/ldpaa_eth.c
> @@ -440,12 +440,17 @@ static int ldpaa_eth_open(struct eth_device *net_dev, bd_t *bd)
>  	bus = wriop_get_mdio(priv->dpmac_id);
>  	enet_if = wriop_get_enet_if(priv->dpmac_id);
>  	if ((bus == NULL) &&
> -	    (enet_if == PHY_INTERFACE_MODE_XGMII)) {
> +	   ((enet_if == PHY_INTERFACE_MODE_XGMII) ||
> +	   (enet_if == PHY_INTERFACE_MODE_SGMII))) {
>  		priv->phydev = (struct phy_device *)
>  				malloc(sizeof(struct phy_device));
>  		memset(priv->phydev, 0, sizeof(struct phy_device));
>  
> -		priv->phydev->speed = SPEED_10000;
> +		if (enet_if == PHY_INTERFACE_MODE_XGMII)
> +			priv->phydev->speed = SPEED_10000;
> +		else
> +			priv->phydev->speed = SPEED_1000;
> +
>  		priv->phydev->link = 1;
>  		priv->phydev->duplex = DUPLEX_FULL;
>  	}
> 

How was this patch tested?

A little off-topic. I remember a similar discussion somewhere (maybe
internal) for PowerPC-based boards regarding fixed SGMII port. When the
port doesn't have a PHY, it fails due to PHY id check. I don't find the
change in U-Boot code. Does anyone remember it?

York
Ashish Kumar Nov. 9, 2017, 5:16 a.m. UTC | #2
-----Original Message-----
From: York Sun 
Sent: Thursday, November 09, 2017 2:07 AM
To: Ashish Kumar <ashish.kumar@nxp.com>; u-boot@lists.denx.de
Cc: joe.hershberger@ni.com; Prabhakar Kushwaha <prabhakar.kushwaha@nxp.com>
Subject: Re: [PATCH] driver: net: ldpaa_eth: Add PHY-less SGMII support

On 11/06/2017 01:09 AM, Ashish Kumar wrote:
> In case of PHY-less mode, there is no interaction with PHY so auto-neg 
> etc is not required and link will have fixed attributes
> 
> Signed-off-by: Ashish Kumar <Ashish.Kumar@nxp.com>
> Signed-off-by: Prabhakar Kushwaha <prabhakar.kushwaha@nxp.com>
> ---
>  drivers/net/ldpaa_eth/ldpaa_eth.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ldpaa_eth/ldpaa_eth.c 
> b/drivers/net/ldpaa_eth/ldpaa_eth.c
> index 21be79a..85e7faa 100644
> --- a/drivers/net/ldpaa_eth/ldpaa_eth.c
> +++ b/drivers/net/ldpaa_eth/ldpaa_eth.c
> @@ -440,12 +440,17 @@ static int ldpaa_eth_open(struct eth_device *net_dev, bd_t *bd)
>  	bus = wriop_get_mdio(priv->dpmac_id);
>  	enet_if = wriop_get_enet_if(priv->dpmac_id);
>  	if ((bus == NULL) &&
> -	    (enet_if == PHY_INTERFACE_MODE_XGMII)) {
> +	   ((enet_if == PHY_INTERFACE_MODE_XGMII) ||
> +	   (enet_if == PHY_INTERFACE_MODE_SGMII))) {
>  		priv->phydev = (struct phy_device *)
>  				malloc(sizeof(struct phy_device));
>  		memset(priv->phydev, 0, sizeof(struct phy_device));
>  
> -		priv->phydev->speed = SPEED_10000;
> +		if (enet_if == PHY_INTERFACE_MODE_XGMII)
> +			priv->phydev->speed = SPEED_10000;
> +		else
> +			priv->phydev->speed = SPEED_1000;
> +
>  		priv->phydev->link = 1;
>  		priv->phydev->duplex = DUPLEX_FULL;
>  	}
> 

How was this patch tested?

>It can be tested with PHY-less SGMII riser cards after adding patches attached. 
>With the existing Riser card, which are PHY enabled, the test is that PHYs are not configured by adding these to patches.

>Do you suggest to add these patches in upstream?

Regards
Ashish

A little off-topic. I remember a similar discussion somewhere (maybe
internal) for PowerPC-based boards regarding fixed SGMII port. When the port doesn't have a PHY, it fails due to PHY id check. I don't find the change in U-Boot code. Does anyone remember it?

York
York Sun Nov. 9, 2017, 5:48 p.m. UTC | #3
On 11/08/2017 09:16 PM, Ashish Kumar wrote:
> -----Original Message-----
> From: York Sun 
> Sent: Thursday, November 09, 2017 2:07 AM
> To: Ashish Kumar <ashish.kumar@nxp.com>; u-boot@lists.denx.de
> Cc: joe.hershberger@ni.com; Prabhakar Kushwaha <prabhakar.kushwaha@nxp.com>
> Subject: Re: [PATCH] driver: net: ldpaa_eth: Add PHY-less SGMII support
> 
> On 11/06/2017 01:09 AM, Ashish Kumar wrote:
>> In case of PHY-less mode, there is no interaction with PHY so auto-neg 
>> etc is not required and link will have fixed attributes
>>
>> Signed-off-by: Ashish Kumar <Ashish.Kumar@nxp.com>
>> Signed-off-by: Prabhakar Kushwaha <prabhakar.kushwaha@nxp.com>
>> ---
>>  drivers/net/ldpaa_eth/ldpaa_eth.c | 9 +++++++--
>>  1 file changed, 7 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/ldpaa_eth/ldpaa_eth.c 
>> b/drivers/net/ldpaa_eth/ldpaa_eth.c
>> index 21be79a..85e7faa 100644
>> --- a/drivers/net/ldpaa_eth/ldpaa_eth.c
>> +++ b/drivers/net/ldpaa_eth/ldpaa_eth.c
>> @@ -440,12 +440,17 @@ static int ldpaa_eth_open(struct eth_device *net_dev, bd_t *bd)
>>  	bus = wriop_get_mdio(priv->dpmac_id);
>>  	enet_if = wriop_get_enet_if(priv->dpmac_id);
>>  	if ((bus == NULL) &&
>> -	    (enet_if == PHY_INTERFACE_MODE_XGMII)) {
>> +	   ((enet_if == PHY_INTERFACE_MODE_XGMII) ||
>> +	   (enet_if == PHY_INTERFACE_MODE_SGMII))) {
>>  		priv->phydev = (struct phy_device *)
>>  				malloc(sizeof(struct phy_device));
>>  		memset(priv->phydev, 0, sizeof(struct phy_device));
>>  
>> -		priv->phydev->speed = SPEED_10000;
>> +		if (enet_if == PHY_INTERFACE_MODE_XGMII)
>> +			priv->phydev->speed = SPEED_10000;
>> +		else
>> +			priv->phydev->speed = SPEED_1000;
>> +
>>  		priv->phydev->link = 1;
>>  		priv->phydev->duplex = DUPLEX_FULL;
>>  	}
>>
> 
> How was this patch tested?
> 
>> It can be tested with PHY-less SGMII riser cards after adding patches attached. 
>> With the existing Riser card, which are PHY enabled, the test is that PHYs are not configured by adding these to patches.
> 
>> Do you suggest to add these patches in upstream?
> 

Ashish,

This patch seems OK. But it is only effective with the other two patches
you mentioned, isn't it? Please find a proper way to enable SGMII_PHYLIB
using Kconfig.

York
Ashish Kumar Jan. 2, 2018, 5:44 a.m. UTC | #4
> -----Original Message-----
> From: U-Boot [mailto:u-boot-bounces@lists.denx.de] On Behalf Of Ashish
> Kumar
> Sent: Thursday, November 09, 2017 10:46 AM
> To: York Sun <york.sun@nxp.com>; u-boot@lists.denx.de; Prabhakar Kushwaha
> <prabhakar.kushwaha@nxp.com>
> Cc: joe.hershberger@ni.com
> Subject: Re: [U-Boot] [PATCH] driver: net: ldpaa_eth: Add PHY-less SGMII
> support
> 
> -----Original Message-----
> From: York Sun
> Sent: Thursday, November 09, 2017 2:07 AM
> To: Ashish Kumar <ashish.kumar@nxp.com>; u-boot@lists.denx.de
> Cc: joe.hershberger@ni.com; Prabhakar Kushwaha
> <prabhakar.kushwaha@nxp.com>
> Subject: Re: [PATCH] driver: net: ldpaa_eth: Add PHY-less SGMII support
> 
> On 11/06/2017 01:09 AM, Ashish Kumar wrote:
> > In case of PHY-less mode, there is no interaction with PHY so auto-neg
> > etc is not required and link will have fixed attributes
> >
> > Signed-off-by: Ashish Kumar <Ashish.Kumar@nxp.com>
> > Signed-off-by: Prabhakar Kushwaha <prabhakar.kushwaha@nxp.com>
> > ---
> >  drivers/net/ldpaa_eth/ldpaa_eth.c | 9 +++++++--
> >  1 file changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/ldpaa_eth/ldpaa_eth.c
> > b/drivers/net/ldpaa_eth/ldpaa_eth.c
> > index 21be79a..85e7faa 100644
> > --- a/drivers/net/ldpaa_eth/ldpaa_eth.c
> > +++ b/drivers/net/ldpaa_eth/ldpaa_eth.c
> > @@ -440,12 +440,17 @@ static int ldpaa_eth_open(struct eth_device
> *net_dev, bd_t *bd)
> >  	bus = wriop_get_mdio(priv->dpmac_id);
> >  	enet_if = wriop_get_enet_if(priv->dpmac_id);
> >  	if ((bus == NULL) &&
> > -	    (enet_if == PHY_INTERFACE_MODE_XGMII)) {
> > +	   ((enet_if == PHY_INTERFACE_MODE_XGMII) ||
> > +	   (enet_if == PHY_INTERFACE_MODE_SGMII))) {
> >  		priv->phydev = (struct phy_device *)
> >  				malloc(sizeof(struct phy_device));
> >  		memset(priv->phydev, 0, sizeof(struct phy_device));
> >
> > -		priv->phydev->speed = SPEED_10000;
> > +		if (enet_if == PHY_INTERFACE_MODE_XGMII)
> > +			priv->phydev->speed = SPEED_10000;
> > +		else
> > +			priv->phydev->speed = SPEED_1000;
> > +
> >  		priv->phydev->link = 1;
> >  		priv->phydev->duplex = DUPLEX_FULL;
> >  	}
> >
> 
> How was this patch tested?
> 
> >It can be tested with PHY-less SGMII riser cards after adding patches attached.
> >With the existing Riser card, which are PHY enabled, the test is that PHYs are
> not configured by adding these to patches.
> 
> >Do you suggest to add these patches in upstream?
> 
> Regards
> Ashish
> 
> A little off-topic. I remember a similar discussion somewhere (maybe
> internal) for PowerPC-based boards regarding fixed SGMII port. When the port
> doesn't have a PHY, it fails due to PHY id check. I don't find the change in U-Boot
> code. Does anyone remember it?

Hello York,

Please provide more details of the PowerPC target in which this these changes may be required?

Regards
Ashish
> 
> York
diff mbox series

Patch

diff --git a/drivers/net/ldpaa_eth/ldpaa_eth.c b/drivers/net/ldpaa_eth/ldpaa_eth.c
index 21be79a..85e7faa 100644
--- a/drivers/net/ldpaa_eth/ldpaa_eth.c
+++ b/drivers/net/ldpaa_eth/ldpaa_eth.c
@@ -440,12 +440,17 @@  static int ldpaa_eth_open(struct eth_device *net_dev, bd_t *bd)
 	bus = wriop_get_mdio(priv->dpmac_id);
 	enet_if = wriop_get_enet_if(priv->dpmac_id);
 	if ((bus == NULL) &&
-	    (enet_if == PHY_INTERFACE_MODE_XGMII)) {
+	   ((enet_if == PHY_INTERFACE_MODE_XGMII) ||
+	   (enet_if == PHY_INTERFACE_MODE_SGMII))) {
 		priv->phydev = (struct phy_device *)
 				malloc(sizeof(struct phy_device));
 		memset(priv->phydev, 0, sizeof(struct phy_device));
 
-		priv->phydev->speed = SPEED_10000;
+		if (enet_if == PHY_INTERFACE_MODE_XGMII)
+			priv->phydev->speed = SPEED_10000;
+		else
+			priv->phydev->speed = SPEED_1000;
+
 		priv->phydev->link = 1;
 		priv->phydev->duplex = DUPLEX_FULL;
 	}