| Message ID | 1522168309-12338-4-git-send-email-m-karicheri2@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Mar 27, 2018 at 12:31:42PM -0400, Murali Karicheri wrote: > As a preparatory patch to add support for 2u cpsw hardware found on > K2G SoC, make call to gbe_sgmii_config() conditional. This is required > since 2u uses RGMII interface instead of SGMII and to allow for driver > re-use. > > Signed-off-by: Murali Karicheri <m-karicheri2@ti.com> > --- > drivers/net/ethernet/ti/netcp_ethss.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/ethernet/ti/netcp_ethss.c b/drivers/net/ethernet/ti/netcp_ethss.c > index 56dbc0b..1dea891 100644 > --- a/drivers/net/ethernet/ti/netcp_ethss.c > +++ b/drivers/net/ethernet/ti/netcp_ethss.c > @@ -2271,7 +2271,8 @@ static int gbe_slave_open(struct gbe_intf *gbe_intf) > > void (*hndlr)(struct net_device *) = gbe_adjust_link; > > - gbe_sgmii_config(priv, slave); > + if ((priv->ss_version == GBE_SS_VERSION_14) || IS_SS_ID_NU(priv)) > + gbe_sgmii_config(priv, slave); Hi Murali So you have: #define IS_SS_ID_NU(d) \ (GBE_IDENT((d)->ss_version) == GBE_SS_ID_NU) Does version 14 have a name? Could you add another IS_SS_ID_XX(d) macro for it? That would make these statements more consistent. Andrew
On Tue, Mar 27, 2018 at 12:31:42PM -0400, Murali Karicheri wrote: > As a preparatory patch to add support for 2u cpsw hardware found on > K2G SoC, make call to gbe_sgmii_config() conditional. This is required > since 2u uses RGMII interface instead of SGMII and to allow for driver > re-use. > > Signed-off-by: Murali Karicheri <m-karicheri2@ti.com> > --- > drivers/net/ethernet/ti/netcp_ethss.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/ethernet/ti/netcp_ethss.c b/drivers/net/ethernet/ti/netcp_ethss.c > index 56dbc0b..1dea891 100644 > --- a/drivers/net/ethernet/ti/netcp_ethss.c > +++ b/drivers/net/ethernet/ti/netcp_ethss.c > @@ -2271,7 +2271,8 @@ static int gbe_slave_open(struct gbe_intf *gbe_intf) > > void (*hndlr)(struct net_device *) = gbe_adjust_link; > > - gbe_sgmii_config(priv, slave); > + if ((priv->ss_version == GBE_SS_VERSION_14) || IS_SS_ID_NU(priv)) > + gbe_sgmii_config(priv, slave); Or maybe: if (slave->phy_node == PHY_INTERFACE_MODE_SGMII) gbe_sgmii_config(priv, slave); Andrew
On 03/27/2018 01:13 PM, Andrew Lunn wrote: > On Tue, Mar 27, 2018 at 12:31:42PM -0400, Murali Karicheri wrote: >> As a preparatory patch to add support for 2u cpsw hardware found on >> K2G SoC, make call to gbe_sgmii_config() conditional. This is required >> since 2u uses RGMII interface instead of SGMII and to allow for driver >> re-use. >> >> Signed-off-by: Murali Karicheri <m-karicheri2@ti.com> >> --- >> drivers/net/ethernet/ti/netcp_ethss.c | 7 +++++-- >> 1 file changed, 5 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/net/ethernet/ti/netcp_ethss.c b/drivers/net/ethernet/ti/netcp_ethss.c >> index 56dbc0b..1dea891 100644 >> --- a/drivers/net/ethernet/ti/netcp_ethss.c >> +++ b/drivers/net/ethernet/ti/netcp_ethss.c >> @@ -2271,7 +2271,8 @@ static int gbe_slave_open(struct gbe_intf *gbe_intf) >> >> void (*hndlr)(struct net_device *) = gbe_adjust_link; >> >> - gbe_sgmii_config(priv, slave); >> + if ((priv->ss_version == GBE_SS_VERSION_14) || IS_SS_ID_NU(priv)) >> + gbe_sgmii_config(priv, slave); > > Hi Murali > > So you have: > > #define IS_SS_ID_NU(d) \ > (GBE_IDENT((d)->ss_version) == GBE_SS_ID_NU) > > > Does version 14 have a name? Could you add another IS_SS_ID_XX(d) > macro for it? That would make these statements more consistent. > unfortunately not being the first version :) Probably we can add #define IS_SS_ID_VER_14(d) \ (GBE_IDENT((d)->ss_version) == GBE_SS_VERSION_14) and replace all instances of (priv->ss_version == GBE_SS_VERSION_14) with if (IS_SS_ID_VER_14(priv)) or equivalent. > Andrew >
On 03/27/2018 01:18 PM, Andrew Lunn wrote: > On Tue, Mar 27, 2018 at 12:31:42PM -0400, Murali Karicheri wrote: >> As a preparatory patch to add support for 2u cpsw hardware found on >> K2G SoC, make call to gbe_sgmii_config() conditional. This is required >> since 2u uses RGMII interface instead of SGMII and to allow for driver >> re-use. >> >> Signed-off-by: Murali Karicheri <m-karicheri2@ti.com> >> --- >> drivers/net/ethernet/ti/netcp_ethss.c | 7 +++++-- >> 1 file changed, 5 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/net/ethernet/ti/netcp_ethss.c b/drivers/net/ethernet/ti/netcp_ethss.c >> index 56dbc0b..1dea891 100644 >> --- a/drivers/net/ethernet/ti/netcp_ethss.c >> +++ b/drivers/net/ethernet/ti/netcp_ethss.c >> @@ -2271,7 +2271,8 @@ static int gbe_slave_open(struct gbe_intf *gbe_intf) >> >> void (*hndlr)(struct net_device *) = gbe_adjust_link; >> >> - gbe_sgmii_config(priv, slave); >> + if ((priv->ss_version == GBE_SS_VERSION_14) || IS_SS_ID_NU(priv)) >> + gbe_sgmii_config(priv, slave); > > Or maybe: > > if (slave->phy_node == PHY_INTERFACE_MODE_SGMII) > gbe_sgmii_config(priv, slave); Yeah. Based on my response to your other comment, this would become if ((slave->link_interface == SGMII_LINK_MAC_PHY) && (IS_SS_ID_VER_14(priv) || IS_SS_ID_NU(priv))) gbe_sgmii_config(priv, slave); We can't solely depends on phy_mode here. Phy interface is one of several interface possible. There is MAC_TO_MAC_FORCED, NO_MDIO etc. So we check the link_interface above. If we can agree, here is what will appear in v3 1) Add another patch to do conversion of priv->ss_version == GBE_SS_VERSION_14 check with a macro, IS_SS_ID_VER_14 2) modify this patch as above. Murali > > Andrew >
On 03/27/2018 01:18 PM, Andrew Lunn wrote: > On Tue, Mar 27, 2018 at 12:31:42PM -0400, Murali Karicheri wrote: >> As a preparatory patch to add support for 2u cpsw hardware found on >> K2G SoC, make call to gbe_sgmii_config() conditional. This is required >> since 2u uses RGMII interface instead of SGMII and to allow for driver >> re-use. >> >> Signed-off-by: Murali Karicheri <m-karicheri2@ti.com> >> --- >> drivers/net/ethernet/ti/netcp_ethss.c | 7 +++++-- >> 1 file changed, 5 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/net/ethernet/ti/netcp_ethss.c b/drivers/net/ethernet/ti/netcp_ethss.c >> index 56dbc0b..1dea891 100644 >> --- a/drivers/net/ethernet/ti/netcp_ethss.c >> +++ b/drivers/net/ethernet/ti/netcp_ethss.c >> @@ -2271,7 +2271,8 @@ static int gbe_slave_open(struct gbe_intf *gbe_intf) >> >> void (*hndlr)(struct net_device *) = gbe_adjust_link; >> >> - gbe_sgmii_config(priv, slave); >> + if ((priv->ss_version == GBE_SS_VERSION_14) || IS_SS_ID_NU(priv)) >> + gbe_sgmii_config(priv, slave); > > Or maybe: > > if (slave->phy_node == PHY_INTERFACE_MODE_SGMII) > gbe_sgmii_config(priv, slave); > > Andrew > Actually, on specific cpsw hardware, driver supports specific interface. Driver also uses link-interface DT property to configure this and link-interface can be without phy as well. So essentially the above code looks good IMO. I need to change this as if (IS_SS_ID_VER_14(priv) || IS_SS_ID_NU(priv)) gbe_sgmii_config(priv, slave); Based on the link-interface, driver selects the phy mode and pass it to of_phy_connect(). I will update the above in v3.
diff --git a/drivers/net/ethernet/ti/netcp_ethss.c b/drivers/net/ethernet/ti/netcp_ethss.c index 56dbc0b..1dea891 100644 --- a/drivers/net/ethernet/ti/netcp_ethss.c +++ b/drivers/net/ethernet/ti/netcp_ethss.c @@ -2271,7 +2271,8 @@ static int gbe_slave_open(struct gbe_intf *gbe_intf) void (*hndlr)(struct net_device *) = gbe_adjust_link; - gbe_sgmii_config(priv, slave); + if ((priv->ss_version == GBE_SS_VERSION_14) || IS_SS_ID_NU(priv)) + gbe_sgmii_config(priv, slave); gbe_port_reset(slave); gbe_sgmii_rtreset(priv, slave, false); gbe_port_config(priv, slave, priv->rx_packet_max); @@ -3039,7 +3040,9 @@ static void init_secondary_ports(struct gbe_priv *gbe_dev, continue; } - gbe_sgmii_config(gbe_dev, slave); + if ((gbe_dev->ss_version == GBE_SS_VERSION_14) || + IS_SS_ID_NU(gbe_dev)) + gbe_sgmii_config(gbe_dev, slave); gbe_port_reset(slave); gbe_port_config(gbe_dev, slave, gbe_dev->rx_packet_max); list_add_tail(&slave->slave_list, &gbe_dev->secondary_slaves);
As a preparatory patch to add support for 2u cpsw hardware found on K2G SoC, make call to gbe_sgmii_config() conditional. This is required since 2u uses RGMII interface instead of SGMII and to allow for driver re-use. Signed-off-by: Murali Karicheri <m-karicheri2@ti.com> --- drivers/net/ethernet/ti/netcp_ethss.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)