Re: [PATCH net-next 7/9] net: stmmac: Integrate XGMAC into main driver flow
> Looks like a plan. So, I will remove the adjust_link speed > selection for > 1g and the C45 support. I will leave the SS > selection in dwxgmac2_core_init (patch 2/9) as this makes no > difference because only 1g will be selected for now. I will also > clearly refer in cover letter the BW results for 1g tests. > > Later on I will add support for 10g once shipping arrives. > > Looks okay? Hi Jose Yes, this is good. Thanks Andrew
Re: [PATCH net-next 7/9] net: stmmac: Integrate XGMAC into main driver flow
On 02-08-2018 15:36, Andrew Lunn wrote: >> Sorry, I made a mistake. Where it reads SGMII in my reply I was >> referring to XGMII. > So you have XGMII between the MAC and the PHY. That should support > 2.5G, 5G and 10G. What i don't know is if you can also do 10/100/1000 > over XGMII? Acording to databook I can only do 1G/2.5G and 10G. > > How are you currently connecting your 1G PHY to the MAC? XGMII is a > big parallel bus, where as SGMII is a small serial bus. I will check with HW team because I've no idea how is this connected ... > > I would say, before this patchset goes anywhere, you need to test > 10/100/1000/2.5G/10G, with at least one PHY. > > Alternatively, take out support for 2.5G/10G and C45, and post patches > for just > 1G and C22. That you can test and you know works. You can > add the rest later. Looks like a plan. So, I will remove the adjust_link speed selection for > 1g and the C45 support. I will leave the SS selection in dwxgmac2_core_init (patch 2/9) as this makes no difference because only 1g will be selected for now. I will also clearly refer in cover letter the BW results for 1g tests. Later on I will add support for 10g once shipping arrives. Looks okay? Thanks and Best Regards, Jose Miguel Abreu > > Andrew
Re: [PATCH net-next 7/9] net: stmmac: Integrate XGMAC into main driver flow
> Sorry, I made a mistake. Where it reads SGMII in my reply I was > referring to XGMII. So you have XGMII between the MAC and the PHY. That should support 2.5G, 5G and 10G. What i don't know is if you can also do 10/100/1000 over XGMII? How are you currently connecting your 1G PHY to the MAC? XGMII is a big parallel bus, where as SGMII is a small serial bus. I would say, before this patchset goes anywhere, you need to test 10/100/1000/2.5G/10G, with at least one PHY. Alternatively, take out support for 2.5G/10G and C45, and post patches for just > 1G and C22. That you can test and you know works. You can add the rest later. Andrew
Re: [PATCH net-next 7/9] net: stmmac: Integrate XGMAC into main driver flow
Hi Andrew,
On 02-08-2018 15:03, Andrew Lunn wrote:
> On Thu, Aug 02, 2018 at 09:26:28AM +0100, Jose Abreu wrote:
>> Hi Andrew,
>>
>> Thanks for the review!
>>
>> On 01-08-2018 16:23, Andrew Lunn wrote:
@@ -842,6 +863,12 @@ static void stmmac_adjust_link(struct net_device *dev)
new_state = true;
ctrl &= ~priv->hw->link.speed_mask;
switch (phydev->speed) {
+ case SPEED_1:
+ ctrl |= priv->hw->link.speed1;
+ break;
+ case SPEED_2500:
+ ctrl |= priv->hw->link.speed2500;
+ break;
case SPEED_1000:
ctrl |= priv->hw->link.speed1000;
break;
>>> Hi Jose
>>>
>>> What PHY did you test this with?
>> We had some shipping issues with the 10G phy so right now I'm
>> using a 1G phy ...
> Please add that to the commit message. It is useful for people to know
> this is untested above 1G, and so probably broken
Ok, will do.
>
>> I would expect that as MDIO is used in both
>> phys then phylib would take care of everything as long as I
>> specify in the DT the right interface (SGMII) ... Am I making a
>> wrong assumption?
>>
>>> 10G phys change the interface mode when the speed change. In general,
>>> 10/100/1000G copper uses SGMII. A 1G SFP optical module generally
>>> wants 1000Base-X. 2.5G wants 2500Base-X, 10G copper wants 10GKR, etc.
>>>
>>> So your adjust link callback needs to look at phydev->interface and
>>> reconfigure the MAC as requested.
>> Sorry, I'm not a phy expert but as long as I use MDIO shouldn't
>> this be transparent to MAC? I mean, there are no registers about
>> the interface to use in XGMAC2, there is only this speed
>> selection register that its implemented already in the
>> stmmac_adjust_link.
> MDIO is the control plane used to manage the PHY. But here we are
> talking about the data plane. As i said, the link between the MAC and
> PHY will need to change depending on what the PHY is doing. SGMII will
> work for 10/100/1000, but nothing above that.
Sorry, I made a mistake. Where it reads SGMII in my reply I was
referring to XGMII.
> It could be this speed
> register also changes the SERDES configuration, but you really should
> confirm this and find out exactly what it is doing. There can be
> multiple ways of doing one speed, e.g. SGMII at 1G. So if the PHY
> wants you to do 1000Base-X and the MAC can only do SGMII, you need to
> be raising an error. phylink makes this simpler. It ask the MAC driver
> for all the modes it supports. It will then not ask the MAC to swap to
> something it does not support.
Ok. XGMII support is optional in the MAC so I will need to add a
check for that.
Thanks and Best Regards,
Jose Miguel Abreu
>
> I suggest you get the datasheet for the PHY you are expecting to get,
> once shipping is fixed. See what it says about its MAC side interface.
> You can also look at the Marvell 10G driver, e.g.
> mv3310_update_interface().
>
> Andrew
Re: [PATCH net-next 7/9] net: stmmac: Integrate XGMAC into main driver flow
On Thu, Aug 02, 2018 at 09:26:28AM +0100, Jose Abreu wrote:
> Hi Andrew,
>
> Thanks for the review!
>
> On 01-08-2018 16:23, Andrew Lunn wrote:
> >> @@ -842,6 +863,12 @@ static void stmmac_adjust_link(struct net_device *dev)
> >>new_state = true;
> >>ctrl &= ~priv->hw->link.speed_mask;
> >>switch (phydev->speed) {
> >> + case SPEED_1:
> >> + ctrl |= priv->hw->link.speed1;
> >> + break;
> >> + case SPEED_2500:
> >> + ctrl |= priv->hw->link.speed2500;
> >> + break;
> >>case SPEED_1000:
> >>ctrl |= priv->hw->link.speed1000;
> >>break;
> > Hi Jose
> >
> > What PHY did you test this with?
>
> We had some shipping issues with the 10G phy so right now I'm
> using a 1G phy ...
Please add that to the commit message. It is useful for people to know
this is untested above 1G, and so probably broken
> I would expect that as MDIO is used in both
> phys then phylib would take care of everything as long as I
> specify in the DT the right interface (SGMII) ... Am I making a
> wrong assumption?
>
> >
> > 10G phys change the interface mode when the speed change. In general,
> > 10/100/1000G copper uses SGMII. A 1G SFP optical module generally
> > wants 1000Base-X. 2.5G wants 2500Base-X, 10G copper wants 10GKR, etc.
> >
> > So your adjust link callback needs to look at phydev->interface and
> > reconfigure the MAC as requested.
>
> Sorry, I'm not a phy expert but as long as I use MDIO shouldn't
> this be transparent to MAC? I mean, there are no registers about
> the interface to use in XGMAC2, there is only this speed
> selection register that its implemented already in the
> stmmac_adjust_link.
MDIO is the control plane used to manage the PHY. But here we are
talking about the data plane. As i said, the link between the MAC and
PHY will need to change depending on what the PHY is doing. SGMII will
work for 10/100/1000, but nothing above that. It could be this speed
register also changes the SERDES configuration, but you really should
confirm this and find out exactly what it is doing. There can be
multiple ways of doing one speed, e.g. SGMII at 1G. So if the PHY
wants you to do 1000Base-X and the MAC can only do SGMII, you need to
be raising an error. phylink makes this simpler. It ask the MAC driver
for all the modes it supports. It will then not ask the MAC to swap to
something it does not support.
I suggest you get the datasheet for the PHY you are expecting to get,
once shipping is fixed. See what it says about its MAC side interface.
You can also look at the Marvell 10G driver, e.g.
mv3310_update_interface().
Andrew
Re: [PATCH net-next 7/9] net: stmmac: Integrate XGMAC into main driver flow
Hi Andrew,
Thanks for the review!
On 01-08-2018 16:23, Andrew Lunn wrote:
>> @@ -842,6 +863,12 @@ static void stmmac_adjust_link(struct net_device *dev)
>> new_state = true;
>> ctrl &= ~priv->hw->link.speed_mask;
>> switch (phydev->speed) {
>> +case SPEED_1:
>> +ctrl |= priv->hw->link.speed1;
>> +break;
>> +case SPEED_2500:
>> +ctrl |= priv->hw->link.speed2500;
>> +break;
>> case SPEED_1000:
>> ctrl |= priv->hw->link.speed1000;
>> break;
> Hi Jose
>
> What PHY did you test this with?
We had some shipping issues with the 10G phy so right now I'm
using a 1G phy ... I would expect that as MDIO is used in both
phys then phylib would take care of everything as long as I
specify in the DT the right interface (SGMII) ... Am I making a
wrong assumption?
>
> 10G phys change the interface mode when the speed change. In general,
> 10/100/1000G copper uses SGMII. A 1G SFP optical module generally
> wants 1000Base-X. 2.5G wants 2500Base-X, 10G copper wants 10GKR, etc.
>
> So your adjust link callback needs to look at phydev->interface and
> reconfigure the MAC as requested.
Sorry, I'm not a phy expert but as long as I use MDIO shouldn't
this be transparent to MAC? I mean, there are no registers about
the interface to use in XGMAC2, there is only this speed
selection register that its implemented already in the
stmmac_adjust_link.
>
> You might also want to consider moving from phylib to phylink. It has
> a better interface for things like this, and makes support for SFP
> interfaces much easier. A MAC which supports 10G is likely to be used
> with SFPs...
Ok, I will take a look into it.
Thanks and Best Regards,
Jose Miguel Abreu
>
> Andrew
Re: [PATCH net-next 7/9] net: stmmac: Integrate XGMAC into main driver flow
> @@ -842,6 +863,12 @@ static void stmmac_adjust_link(struct net_device *dev)
> new_state = true;
> ctrl &= ~priv->hw->link.speed_mask;
> switch (phydev->speed) {
> + case SPEED_1:
> + ctrl |= priv->hw->link.speed1;
> + break;
> + case SPEED_2500:
> + ctrl |= priv->hw->link.speed2500;
> + break;
> case SPEED_1000:
> ctrl |= priv->hw->link.speed1000;
> break;
Hi Jose
What PHY did you test this with?
10G phys change the interface mode when the speed change. In general,
10/100/1000G copper uses SGMII. A 1G SFP optical module generally
wants 1000Base-X. 2.5G wants 2500Base-X, 10G copper wants 10GKR, etc.
So your adjust link callback needs to look at phydev->interface and
reconfigure the MAC as requested.
You might also want to consider moving from phylib to phylink. It has
a better interface for things like this, and makes support for SFP
interfaces much easier. A MAC which supports 10G is likely to be used
with SFPs...
Andrew
[PATCH net-next 7/9] net: stmmac: Integrate XGMAC into main driver flow
Now that we have all the XGMAC related callbacks, lets start integrating
this IP block into main driver.
Signed-off-by: Jose Abreu
Cc: David S. Miller
Cc: Joao Pinto
Cc: Giuseppe Cavallaro
Cc: Alexandre Torgue
---
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 55 ++-
1 file changed, 45 insertions(+), 10 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index d9e60cfd8a85..2c7a571140bb 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -51,6 +51,7 @@
#include
#include
#include "dwmac1000.h"
+#include "dwxgmac2.h"
#include "hwif.h"
#define STMMAC_ALIGN(x)L1_CACHE_ALIGN(x)
@@ -262,6 +263,21 @@ static void stmmac_clk_csr_set(struct stmmac_priv *priv)
else
priv->clk_csr = 0;
}
+
+ if (priv->plat->has_xgmac) {
+ if (clk_rate > 4)
+ priv->clk_csr = 0x5;
+ else if (clk_rate > 35000)
+ priv->clk_csr = 0x4;
+ else if (clk_rate > 3)
+ priv->clk_csr = 0x3;
+ else if (clk_rate > 25000)
+ priv->clk_csr = 0x2;
+ else if (clk_rate > 15000)
+ priv->clk_csr = 0x1;
+ else
+ priv->clk_csr = 0x0;
+ }
}
static void print_pkt(unsigned char *buf, int len)
@@ -498,7 +514,7 @@ static void stmmac_get_rx_hwtstamp(struct stmmac_priv
*priv, struct dma_desc *p,
if (!priv->hwts_rx_en)
return;
/* For GMAC4, the valid timestamp is from CTX next desc. */
- if (priv->plat->has_gmac4)
+ if (priv->plat->has_gmac4 || priv->plat->has_xgmac)
desc = np;
/* Check if timestamp is available */
@@ -540,6 +556,9 @@ static int stmmac_hwtstamp_ioctl(struct net_device *dev,
struct ifreq *ifr)
u32 ts_event_en = 0;
u32 value = 0;
u32 sec_inc;
+ bool xmac;
+
+ xmac = priv->plat->has_gmac4 || priv->plat->has_xgmac;
if (!(priv->dma_cap.time_stamp || priv->adv_ts)) {
netdev_alert(priv->dev, "No support for HW time stamping\n");
@@ -575,7 +594,7 @@ static int stmmac_hwtstamp_ioctl(struct net_device *dev,
struct ifreq *ifr)
/* PTP v1, UDP, any kind of event packet */
config.rx_filter = HWTSTAMP_FILTER_PTP_V1_L4_EVENT;
/* take time stamp for all event messages */
- if (priv->plat->has_gmac4)
+ if (xmac)
snap_type_sel = PTP_GMAC4_TCR_SNAPTYPSEL_1;
else
snap_type_sel = PTP_TCR_SNAPTYPSEL_1;
@@ -610,7 +629,7 @@ static int stmmac_hwtstamp_ioctl(struct net_device *dev,
struct ifreq *ifr)
config.rx_filter = HWTSTAMP_FILTER_PTP_V2_L4_EVENT;
ptp_v2 = PTP_TCR_TSVER2ENA;
/* take time stamp for all event messages */
- if (priv->plat->has_gmac4)
+ if (xmac)
snap_type_sel = PTP_GMAC4_TCR_SNAPTYPSEL_1;
else
snap_type_sel = PTP_TCR_SNAPTYPSEL_1;
@@ -647,7 +666,7 @@ static int stmmac_hwtstamp_ioctl(struct net_device *dev,
struct ifreq *ifr)
config.rx_filter = HWTSTAMP_FILTER_PTP_V2_EVENT;
ptp_v2 = PTP_TCR_TSVER2ENA;
/* take time stamp for all event messages */
- if (priv->plat->has_gmac4)
+ if (xmac)
snap_type_sel = PTP_GMAC4_TCR_SNAPTYPSEL_1;
else
snap_type_sel = PTP_TCR_SNAPTYPSEL_1;
@@ -718,7 +737,7 @@ static int stmmac_hwtstamp_ioctl(struct net_device *dev,
struct ifreq *ifr)
/* program Sub Second Increment reg */
stmmac_config_sub_second_increment(priv,
priv->ptpaddr, priv->plat->clk_ptp_rate,
- priv->plat->has_gmac4, _inc);
+ xmac, _inc);
temp = div_u64(10ULL, sec_inc);
/* Store sub second increment and flags for later use */
@@ -755,12 +774,14 @@ static int stmmac_hwtstamp_ioctl(struct net_device *dev,
struct ifreq *ifr)
*/
static int stmmac_init_ptp(struct stmmac_priv *priv)
{
+ bool xmac = priv->plat->has_gmac4 || priv->plat->has_xgmac;
+
if (!(priv->dma_cap.time_stamp || priv->dma_cap.atime_stamp))
return -EOPNOTSUPP;
priv->adv_ts = 0;
- /* Check if adv_ts can be enabled for dwmac 4.x core */
- if
