| Message ID | 61b71d12-4e3a-edbe-1d69-c66e7cf46d8e@sigmadesigns.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Marc Gonzalez <marc_gonzalez@sigmadesigns.com> writes: > According to commit e5f3a4a56ce2a707b2fb8ce37e4414dcac89c672 > ("Documentation: devicetree: clarify usage of the RGMII phy-modes") > there are 4 RGMII phy-modes to handle: > > "rgmii" (RX and TX delays are added by the MAC when required) > "rgmii-id" (RGMII with internal RX and TX delays provided by the PHY, > the MAC should not add the RX or TX delays in this case) > "rgmii-rxid" (RGMII with internal RX delay provided by the PHY, > the MAC should not add an RX delay in this case) > "rgmii-txid" (RGMII with internal TX delay provided by the PHY, > the MAC should not add an TX delay in this case) > > Let the MAC handle TX clock delay for rgmii and rgmii-rxid. > > Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com> > --- > drivers/net/ethernet/aurora/nb8800.c | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/ethernet/aurora/nb8800.c b/drivers/net/ethernet/aurora/nb8800.c > index 041cfb7952f8..f3ed320eb4ad 100644 > --- a/drivers/net/ethernet/aurora/nb8800.c > +++ b/drivers/net/ethernet/aurora/nb8800.c > @@ -609,7 +609,7 @@ static void nb8800_mac_config(struct net_device *dev) > mac_mode |= HALF_DUPLEX; > > if (gigabit) { > - if (priv->phy_mode == PHY_INTERFACE_MODE_RGMII) > + if (phy_interface_is_rgmii(dev->phydev)) > mac_mode |= RGMII_MODE; > > mac_mode |= GMAC_MODE; This is a separate issue, and the change is obviously correct. > @@ -1268,11 +1268,13 @@ static int nb8800_tangox_init(struct net_device *dev) > break; > > case PHY_INTERFACE_MODE_RGMII: > - pad_mode = PAD_MODE_RGMII; > + case PHY_INTERFACE_MODE_RGMII_RXID: > + pad_mode = PAD_MODE_RGMII | PAD_MODE_GTX_CLK_DELAY; > break; > > + case PHY_INTERFACE_MODE_RGMII_ID: > case PHY_INTERFACE_MODE_RGMII_TXID: > - pad_mode = PAD_MODE_RGMII | PAD_MODE_GTX_CLK_DELAY; > + pad_mode = PAD_MODE_RGMII; > break; Won't this just make it break in a different set of circumstances? I think the only sane solution to this mess is to never configure the MAC delay based on the existing phy-connection-type property. If some board requires this delay (because the PHY can't do it), a new property should probably be introduced for that.
On 19/07/2017 19:17, Måns Rullgård wrote: > Marc Gonzalez writes: > >> According to commit e5f3a4a56ce2a707b2fb8ce37e4414dcac89c672 >> ("Documentation: devicetree: clarify usage of the RGMII phy-modes") >> there are 4 RGMII phy-modes to handle: >> >> "rgmii" (RX and TX delays are added by the MAC when required) >> "rgmii-id" (RGMII with internal RX and TX delays provided by the PHY, >> the MAC should not add the RX or TX delays in this case) >> "rgmii-rxid" (RGMII with internal RX delay provided by the PHY, >> the MAC should not add an RX delay in this case) >> "rgmii-txid" (RGMII with internal TX delay provided by the PHY, >> the MAC should not add an TX delay in this case) >> >> Let the MAC handle TX clock delay for rgmii and rgmii-rxid. >> >> Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com> >> --- >> drivers/net/ethernet/aurora/nb8800.c | 8 +++++--- >> 1 file changed, 5 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/net/ethernet/aurora/nb8800.c b/drivers/net/ethernet/aurora/nb8800.c >> index 041cfb7952f8..f3ed320eb4ad 100644 >> --- a/drivers/net/ethernet/aurora/nb8800.c >> +++ b/drivers/net/ethernet/aurora/nb8800.c >> @@ -609,7 +609,7 @@ static void nb8800_mac_config(struct net_device *dev) >> mac_mode |= HALF_DUPLEX; >> >> if (gigabit) { >> - if (priv->phy_mode == PHY_INTERFACE_MODE_RGMII) >> + if (phy_interface_is_rgmii(dev->phydev)) >> mac_mode |= RGMII_MODE; >> >> mac_mode |= GMAC_MODE; > > This is a separate issue, and the change is obviously correct. > >> @@ -1268,11 +1268,13 @@ static int nb8800_tangox_init(struct net_device *dev) >> break; >> >> case PHY_INTERFACE_MODE_RGMII: >> - pad_mode = PAD_MODE_RGMII; >> + case PHY_INTERFACE_MODE_RGMII_RXID: >> + pad_mode = PAD_MODE_RGMII | PAD_MODE_GTX_CLK_DELAY; >> break; >> >> + case PHY_INTERFACE_MODE_RGMII_ID: >> case PHY_INTERFACE_MODE_RGMII_TXID: >> - pad_mode = PAD_MODE_RGMII | PAD_MODE_GTX_CLK_DELAY; >> + pad_mode = PAD_MODE_RGMII; >> break; > > Won't this just make it break in a different set of circumstances? I don't think so, and here's my reasoning: AFAIU, the HW block always requires a TX clock delay (I don't know what the "safe" interval is. PHY adds 2.4 ns, MAC adds ~1 ns, both work.) RX clock delay seems to be "Don't Care" (tested both enabled and disabled by PHY) By "tested", I mean ability to ping remote system. If phy-mode is RGMII or RGMII_RXID, then don't add TX clock delay from PHY, therefore add it from MAC. If phy_mode is RGMII_ID or RGMII_TXID, then do add TX clock delay from PHY, therefore don't add it from MAC. What set of circumstances would create an issue? Regards.
On 07/19/2017 10:36 AM, Mason wrote: > On 19/07/2017 19:17, Måns Rullgård wrote: > >> Marc Gonzalez writes: >> >>> According to commit e5f3a4a56ce2a707b2fb8ce37e4414dcac89c672 >>> ("Documentation: devicetree: clarify usage of the RGMII phy-modes") >>> there are 4 RGMII phy-modes to handle: >>> >>> "rgmii" (RX and TX delays are added by the MAC when required) >>> "rgmii-id" (RGMII with internal RX and TX delays provided by the PHY, >>> the MAC should not add the RX or TX delays in this case) >>> "rgmii-rxid" (RGMII with internal RX delay provided by the PHY, >>> the MAC should not add an RX delay in this case) >>> "rgmii-txid" (RGMII with internal TX delay provided by the PHY, >>> the MAC should not add an TX delay in this case) >>> >>> Let the MAC handle TX clock delay for rgmii and rgmii-rxid. >>> >>> Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com> >>> --- >>> drivers/net/ethernet/aurora/nb8800.c | 8 +++++--- >>> 1 file changed, 5 insertions(+), 3 deletions(-) >>> >>> diff --git a/drivers/net/ethernet/aurora/nb8800.c b/drivers/net/ethernet/aurora/nb8800.c >>> index 041cfb7952f8..f3ed320eb4ad 100644 >>> --- a/drivers/net/ethernet/aurora/nb8800.c >>> +++ b/drivers/net/ethernet/aurora/nb8800.c >>> @@ -609,7 +609,7 @@ static void nb8800_mac_config(struct net_device *dev) >>> mac_mode |= HALF_DUPLEX; >>> >>> if (gigabit) { >>> - if (priv->phy_mode == PHY_INTERFACE_MODE_RGMII) >>> + if (phy_interface_is_rgmii(dev->phydev)) >>> mac_mode |= RGMII_MODE; >>> >>> mac_mode |= GMAC_MODE; >> >> This is a separate issue, and the change is obviously correct. >> >>> @@ -1268,11 +1268,13 @@ static int nb8800_tangox_init(struct net_device *dev) >>> break; >>> >>> case PHY_INTERFACE_MODE_RGMII: >>> - pad_mode = PAD_MODE_RGMII; >>> + case PHY_INTERFACE_MODE_RGMII_RXID: >>> + pad_mode = PAD_MODE_RGMII | PAD_MODE_GTX_CLK_DELAY; >>> break; >>> >>> + case PHY_INTERFACE_MODE_RGMII_ID: >>> case PHY_INTERFACE_MODE_RGMII_TXID: >>> - pad_mode = PAD_MODE_RGMII | PAD_MODE_GTX_CLK_DELAY; >>> + pad_mode = PAD_MODE_RGMII; >>> break; >> >> Won't this just make it break in a different set of circumstances? > > I don't think so, and here's my reasoning: > > AFAIU, the HW block always requires a TX clock delay > (I don't know what the "safe" interval is. PHY adds > 2.4 ns, MAC adds ~1 ns, both work.) The nominal delay should be 2ns because that's exactly what a 90 degrees shift at a 125Mhz would be. The RGMII specification defines the following: TskewT - Data to Clock output Skew (At Transmitter) Min: -500ns, Nom: 0, Max: + 500 ns TskewR - Data to Clock input Skew (At Receiver) Min: 1ns, Nom: 0, Max: 2.6ns (see note 1) note 1: This implies that PC board design will require clocks to be routed such that an additional trace delay of greater than 1.5ns and less than 2.0ns will be added to the associated clock signal. For 10/100 the Max value is unspecified. So it seems to me like you are borderline spec in both delays you gave here and the "HW block always requires a TX clock delay" statement is true for a given board design only. > RX clock delay seems to be "Don't Care" (tested both > enabled and disabled by PHY) > By "tested", I mean ability to ping remote system. Can you do something a bit more stressful than just a ping, also if you have the ability to change the inter-packet gap, do it, and see if you start seeing FCS or any other decoding errors. > > If phy-mode is RGMII or RGMII_RXID, then don't add > TX clock delay from PHY, therefore add it from MAC. > > If phy_mode is RGMII_ID or RGMII_TXID, then do add > TX clock delay from PHY, therefore don't add it from MAC. > > What set of circumstances would create an issue? Existing Device Tree sources that do not correspond to that description you just did, I suppose they are all out of tree?
On 19/07/2017 20:30, Florian Fainelli wrote: > On 07/19/2017 10:36 AM, Mason wrote: >> On 19/07/2017 19:17, Måns Rullgård wrote: >> >>> Marc Gonzalez writes: >>> >>>> According to commit e5f3a4a56ce2a707b2fb8ce37e4414dcac89c672 >>>> ("Documentation: devicetree: clarify usage of the RGMII phy-modes") >>>> there are 4 RGMII phy-modes to handle: >>>> >>>> "rgmii" (RX and TX delays are added by the MAC when required) >>>> "rgmii-id" (RGMII with internal RX and TX delays provided by the PHY, >>>> the MAC should not add the RX or TX delays in this case) >>>> "rgmii-rxid" (RGMII with internal RX delay provided by the PHY, >>>> the MAC should not add an RX delay in this case) >>>> "rgmii-txid" (RGMII with internal TX delay provided by the PHY, >>>> the MAC should not add an TX delay in this case) >>>> >>>> Let the MAC handle TX clock delay for rgmii and rgmii-rxid. >>>> >>>> Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com> >>>> --- >>>> drivers/net/ethernet/aurora/nb8800.c | 8 +++++--- >>>> 1 file changed, 5 insertions(+), 3 deletions(-) >>>> >>>> diff --git a/drivers/net/ethernet/aurora/nb8800.c b/drivers/net/ethernet/aurora/nb8800.c >>>> index 041cfb7952f8..f3ed320eb4ad 100644 >>>> --- a/drivers/net/ethernet/aurora/nb8800.c >>>> +++ b/drivers/net/ethernet/aurora/nb8800.c >>>> @@ -609,7 +609,7 @@ static void nb8800_mac_config(struct net_device *dev) >>>> mac_mode |= HALF_DUPLEX; >>>> >>>> if (gigabit) { >>>> - if (priv->phy_mode == PHY_INTERFACE_MODE_RGMII) >>>> + if (phy_interface_is_rgmii(dev->phydev)) >>>> mac_mode |= RGMII_MODE; >>>> >>>> mac_mode |= GMAC_MODE; >>> >>> This is a separate issue, and the change is obviously correct. >>> >>>> @@ -1268,11 +1268,13 @@ static int nb8800_tangox_init(struct net_device *dev) >>>> break; >>>> >>>> case PHY_INTERFACE_MODE_RGMII: >>>> - pad_mode = PAD_MODE_RGMII; >>>> + case PHY_INTERFACE_MODE_RGMII_RXID: >>>> + pad_mode = PAD_MODE_RGMII | PAD_MODE_GTX_CLK_DELAY; >>>> break; >>>> >>>> + case PHY_INTERFACE_MODE_RGMII_ID: >>>> case PHY_INTERFACE_MODE_RGMII_TXID: >>>> - pad_mode = PAD_MODE_RGMII | PAD_MODE_GTX_CLK_DELAY; >>>> + pad_mode = PAD_MODE_RGMII; >>>> break; >>> >>> Won't this just make it break in a different set of circumstances? >> >> I don't think so, and here's my reasoning: >> >> AFAIU, the HW block always requires a TX clock delay >> (I don't know what the "safe" interval is. PHY adds >> 2.4 ns, MAC adds ~1 ns, both work.) > > The nominal delay should be 2ns because that's exactly what a 90 degrees > shift at a 125Mhz would be. The RGMII specification defines the following: > > TskewT - Data to Clock output Skew (At Transmitter) Min: -500ns, Nom: 0, > Max: + 500 ns > TskewR - Data to Clock input Skew (At Receiver) Min: 1ns, Nom: 0, Max: > 2.6ns (see note 1) > > note 1: This implies that PC board design will require clocks to be > routed such that an additional trace delay of greater than 1.5ns and > less than 2.0ns will be added to the associated clock signal. For 10/100 > the Max value is unspecified. > > So it seems to me like you are borderline spec in both delays you gave > here and the "HW block always requires a TX clock delay" statement is > true for a given board design only. I must confess that my understanding of clock delays, clock skew, routing, traces, etc is nil. Is TskewT the TX clock delay? And TskewR the RX clock delay? Doesn't wire delay factor in too? (So longer wires require more delay.) >> RX clock delay seems to be "Don't Care" (tested both >> enabled and disabled by PHY) >> By "tested", I mean ability to ping remote system. > > Can you do something a bit more stressful than just a ping, also if you > have the ability to change the inter-packet gap, do it, and see if you > start seeing FCS or any other decoding errors. Errr... "Inter-packet gap"? Is there supposed to be a HW knob to tweak how long the HW waits between sending two frames? >> If phy-mode is RGMII or RGMII_RXID, then don't add >> TX clock delay from PHY, therefore add it from MAC. >> >> If phy_mode is RGMII_ID or RGMII_TXID, then do add >> TX clock delay from PHY, therefore don't add it from MAC. >> >> What set of circumstances would create an issue? > > Existing Device Tree sources that do not correspond to that description > you just did, I suppose they are all out of tree? The problem with PHY drivers is that there is no simple compatible string to grep for. The tango boards use "ethernet-phy-id004d.d072" but not a single other DT uses that string. For example, am335x-evm.dts doesn't seem to name the PHY. Hmmm, how does the at803x probe function match for that board? How does one estimate the impact of driver changes in the eth PHY layer? Regards.
On 07/19/2017 02:15 PM, Mason wrote: > On 19/07/2017 20:30, Florian Fainelli wrote: >> On 07/19/2017 10:36 AM, Mason wrote: >>> On 19/07/2017 19:17, Måns Rullgård wrote: >>> >>>> Marc Gonzalez writes: >>>> >>>>> According to commit e5f3a4a56ce2a707b2fb8ce37e4414dcac89c672 >>>>> ("Documentation: devicetree: clarify usage of the RGMII phy-modes") >>>>> there are 4 RGMII phy-modes to handle: >>>>> >>>>> "rgmii" (RX and TX delays are added by the MAC when required) >>>>> "rgmii-id" (RGMII with internal RX and TX delays provided by the PHY, >>>>> the MAC should not add the RX or TX delays in this case) >>>>> "rgmii-rxid" (RGMII with internal RX delay provided by the PHY, >>>>> the MAC should not add an RX delay in this case) >>>>> "rgmii-txid" (RGMII with internal TX delay provided by the PHY, >>>>> the MAC should not add an TX delay in this case) >>>>> >>>>> Let the MAC handle TX clock delay for rgmii and rgmii-rxid. >>>>> >>>>> Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com> >>>>> --- >>>>> drivers/net/ethernet/aurora/nb8800.c | 8 +++++--- >>>>> 1 file changed, 5 insertions(+), 3 deletions(-) >>>>> >>>>> diff --git a/drivers/net/ethernet/aurora/nb8800.c b/drivers/net/ethernet/aurora/nb8800.c >>>>> index 041cfb7952f8..f3ed320eb4ad 100644 >>>>> --- a/drivers/net/ethernet/aurora/nb8800.c >>>>> +++ b/drivers/net/ethernet/aurora/nb8800.c >>>>> @@ -609,7 +609,7 @@ static void nb8800_mac_config(struct net_device *dev) >>>>> mac_mode |= HALF_DUPLEX; >>>>> >>>>> if (gigabit) { >>>>> - if (priv->phy_mode == PHY_INTERFACE_MODE_RGMII) >>>>> + if (phy_interface_is_rgmii(dev->phydev)) >>>>> mac_mode |= RGMII_MODE; >>>>> >>>>> mac_mode |= GMAC_MODE; >>>> >>>> This is a separate issue, and the change is obviously correct. >>>> >>>>> @@ -1268,11 +1268,13 @@ static int nb8800_tangox_init(struct net_device *dev) >>>>> break; >>>>> >>>>> case PHY_INTERFACE_MODE_RGMII: >>>>> - pad_mode = PAD_MODE_RGMII; >>>>> + case PHY_INTERFACE_MODE_RGMII_RXID: >>>>> + pad_mode = PAD_MODE_RGMII | PAD_MODE_GTX_CLK_DELAY; >>>>> break; >>>>> >>>>> + case PHY_INTERFACE_MODE_RGMII_ID: >>>>> case PHY_INTERFACE_MODE_RGMII_TXID: >>>>> - pad_mode = PAD_MODE_RGMII | PAD_MODE_GTX_CLK_DELAY; >>>>> + pad_mode = PAD_MODE_RGMII; >>>>> break; >>>> >>>> Won't this just make it break in a different set of circumstances? >>> >>> I don't think so, and here's my reasoning: >>> >>> AFAIU, the HW block always requires a TX clock delay >>> (I don't know what the "safe" interval is. PHY adds >>> 2.4 ns, MAC adds ~1 ns, both work.) >> >> The nominal delay should be 2ns because that's exactly what a 90 degrees >> shift at a 125Mhz would be. The RGMII specification defines the following: >> >> TskewT - Data to Clock output Skew (At Transmitter) Min: -500ns, Nom: 0, >> Max: + 500 ns >> TskewR - Data to Clock input Skew (At Receiver) Min: 1ns, Nom: 0, Max: >> 2.6ns (see note 1) >> >> note 1: This implies that PC board design will require clocks to be >> routed such that an additional trace delay of greater than 1.5ns and >> less than 2.0ns will be added to the associated clock signal. For 10/100 >> the Max value is unspecified. >> >> So it seems to me like you are borderline spec in both delays you gave >> here and the "HW block always requires a TX clock delay" statement is >> true for a given board design only. > > I must confess that my understanding of clock delays, > clock skew, routing, traces, etc is nil. > > Is TskewT the TX clock delay? > And TskewR the RX clock delay? > > Doesn't wire delay factor in too? > (So longer wires require more delay.) How about you start reading the RGMII specification so we can at least, if nothing else agree on the terminology? It's public: http://web.archive.org/web/20160303171328/http://www.hp.com/rnd/pdfs/RGMIIv2_0_final_hp.pdf > >>> RX clock delay seems to be "Don't Care" (tested both >>> enabled and disabled by PHY) >>> By "tested", I mean ability to ping remote system. >> >> Can you do something a bit more stressful than just a ping, also if you >> have the ability to change the inter-packet gap, do it, and see if you >> start seeing FCS or any other decoding errors. > > Errr... "Inter-packet gap"? > Is there supposed to be a HW knob to tweak how long > the HW waits between sending two frames? Some Ethernet controllers let you change it, some don't, if nb8800 allows it, it's good for testing in that it packs more frames per quantum of time. If not, do you have at least a FCS error counter? > >>> If phy-mode is RGMII or RGMII_RXID, then don't add >>> TX clock delay from PHY, therefore add it from MAC. >>> >>> If phy_mode is RGMII_ID or RGMII_TXID, then do add >>> TX clock delay from PHY, therefore don't add it from MAC. >>> >>> What set of circumstances would create an issue? >> >> Existing Device Tree sources that do not correspond to that description >> you just did, I suppose they are all out of tree? > > The problem with PHY drivers is that there is no > simple compatible string to grep for. That does not help for sure. > > The tango boards use "ethernet-phy-id004d.d072" > but not a single other DT uses that string. > For example, am335x-evm.dts doesn't seem to name the PHY. > Hmmm, how does the at803x probe function match for that > board? of_mdiobus_register() scans the Device Tree and for each populated PHY node, reads its OUI register and and matches the PHY OUI with the driver list that it has. Quite similar to PCI or USB in fact. > > How does one estimate the impact of driver changes in > the eth PHY layer? It's hard, because there are many Ethernet MACs and PHY drivers that are all inter-operable with each other, which is both a blessing and a curse. I completely understand what you want to solve but I suspect you will have to do it in a way where you either accept that you may not be fully compliant with the now clarified "phy-mode" description, in order not to break other people's set up that were already non-compliant (can't blame them, they did not know back then), or you will have to use additional MAC properties to override the delay settings on the MAC or PHY side.
On 19/07/2017 23:34, Florian Fainelli wrote: > How about you start reading the RGMII specification so we can at least, > if nothing else agree on the terminology? It's public: > > http://web.archive.org/web/20160303171328/http://www.hp.com/rnd/pdfs/RGMIIv2_0_final_hp.pdf Thanks for linking the spec. Having no EE training, I am ill-equipped to interpret the timings table. As you pointed out, the spec states that the "Data to Clock input Skew (at Receiver)" must be within [ 1.0, 2.6 ] ns. I understand that 2 ns is 1/4 of a 125 MHz period, but it's not clear to me why the above interval is centered at 1.8 instead of 2.0 ns. Also, the AR8035 PHY offers 4 possible TX clock delays: { 0.25, 1.3, 2.4, 3.4 } according to their doc. The two extremes are outside the interval, when would they be useful? In case the transmitter adds "bad" skew? Why doesn't the PHY support 1.8/2.0? Is it perhaps unable to, because of PLL limitations? It's also not clear to me if wire length has any influence on the required skew. I would say "no". I think signal propagation time has nothing to do with clock skew (as long as both wires are roughly the same length). > Some Ethernet controllers let you change it, some don't, if nb8800 > allows it, it's good for testing in that it packs more frames per > quantum of time. If not, do you have at least a FCS error counter? I'll have a closer look, and test with iPerf3. Or is there a better benchmark? I will look for an inter-packet gap knob and FCS error counter. > I completely understand what you want to solve but I suspect you will > have to do it in a way where you either accept that you may not be fully > compliant with the now clarified "phy-mode" description, in order not to > break other people's set up that were already non-compliant (can't blame > them, they did not know back then), or you will have to use additional > MAC properties to override the delay settings on the MAC or PHY side. I think I need to give up the notion of "fixing" the at803x driver. Some boards rely on the fact that RX clock delay is enabled by default, like am335x-evm using "rgmii-txid" instead of "rgmii-id". My board needs to enable both internal delays, so I don't need the PHY patch. I will only fix the MAC driver and the DTS. Regards.
Mason <slash.tmp@free.fr> writes:
> I will look for an inter-packet gap knob and FCS error counter.
There is an FCS error counter. Use "ethtool -S" and look for
rx_bad_fcs_frames. Reading the stats counters automatically resets
them to zero.
On 20/07/2017 14:33, Mason wrote: > As [Florian] pointed out, the spec states that the > "Data to Clock input Skew (at Receiver)" > must be within [ 1.0, 2.6 ] ns. > > I understand that 2 ns is 1/4 of a 125 MHz period, > but it's not clear to me why the above interval is > centered at 1.8 instead of 2.0 ns. > > Also, the AR8035 PHY offers 4 possible TX clock delays: > { 0.25, 1.3, 2.4, 3.4 } according to their doc. > The two extremes are outside the interval, when would > they be useful? In case the transmitter adds "bad" skew? > > Why doesn't the PHY support 1.8/2.0? Is it perhaps > unable to, because of PLL limitations? I haven't yet found answers for these questions. - Why is the interval centered at 1.8 instead of 2.0 ns? - What use are 0.25 ns and 3.4 ns skew? - Why doesn't the PHY support a "recommended" value like 1.8 ns? Does anyone have pointers to good resources? Regards.
On 07/24/2017 02:21 PM, Mason wrote: > On 20/07/2017 14:33, Mason wrote: > >> As [Florian] pointed out, the spec states that the >> "Data to Clock input Skew (at Receiver)" >> must be within [ 1.0, 2.6 ] ns. >> >> I understand that 2 ns is 1/4 of a 125 MHz period, >> but it's not clear to me why the above interval is >> centered at 1.8 instead of 2.0 ns. >> >> Also, the AR8035 PHY offers 4 possible TX clock delays: >> { 0.25, 1.3, 2.4, 3.4 } according to their doc. >> The two extremes are outside the interval, when would >> they be useful? In case the transmitter adds "bad" skew? >> >> Why doesn't the PHY support 1.8/2.0? Is it perhaps >> unable to, because of PLL limitations? > > I haven't yet found answers for these questions. > > - Why is the interval centered at 1.8 instead of 2.0 ns? Presumably because this is almost the middle of the available range and it still provides a value that is within the specification... > - What use are 0.25 ns and 3.4 ns skew? Accounting for extreme PCB traces lengths possibly, or just exposing the raw values that the HW supports by increments of 0.25 ns, just because the HW supports it. > - Why doesn't the PHY support a "recommended" value like 1.8 ns? > > Does anyone have pointers to good resources? The PHY datasheet and the RGMII specification really ought to be the starting points, there is not much more to it. Maybe go ask your support person at Qualcomm/Atheros about their PHY design?
On 24/07/2017 23:49, Florian Fainelli wrote: > On 07/24/2017 02:21 PM, Mason wrote: >> On 20/07/2017 14:33, Mason wrote: >> >>> As [Florian] pointed out, the spec states that the >>> "Data to Clock input Skew (at Receiver)" >>> must be within [ 1.0, 2.6 ] ns. >>> >>> I understand that 2 ns is 1/4 of a 125 MHz period, >>> but it's not clear to me why the above interval is >>> centered at 1.8 instead of 2.0 ns. >>> >>> Also, the AR8035 PHY offers 4 possible TX clock delays: >>> { 0.25, 1.3, 2.4, 3.4 } according to their doc. >>> The two extremes are outside the interval, when would >>> they be useful? In case the transmitter adds "bad" skew? >>> >>> Why doesn't the PHY support 1.8/2.0? Is it perhaps >>> unable to, because of PLL limitations? >> >> I haven't yet found answers for these questions. >> >> - Why is the interval centered at 1.8 instead of 2.0 ns? > > Presumably because this is almost the middle of the available range and > it still provides a value that is within the specification... I was talking about the RGMII spec. If - theoretically - the best results are achieved by having a 2 ns skew between clock and data, it seems odd for the RGMII spec to define an interval of [ 1.0, 2.6 ] ns for acceptable values. I would have expected [ 1.2, 2.8 ] ns. >> - What use are 0.25 ns and 3.4 ns skew? > > Accounting for extreme PCB traces lengths possibly, or just exposing the > raw values that the HW supports by increments of 0.25 ns, just because > the HW supports it. The AR8035 doesn't support increments of 0.25 ns, it supports just 4 values: 0.25, 1.3, 2.4, 3.4 Two of which are outside the acceptable range defined in the RGMII spec. Odd. Giving it more thought, I don't think trace length factors in, unless the data and clock lines have very different length (signal propagation). >> - Why doesn't the PHY support a "recommended" value like 1.8 ns? >> >> Does anyone have pointers to good resources? > > The PHY datasheet and the RGMII specification really ought to be the > starting points, there is not much more to it. Maybe go ask your support > person at Qualcomm/Atheros about their PHY design? Sadly, I rarely have access to support for the blocks we use. I had to download the datasheet off the internet. But I was only asking out of personal curiosity, since this is outside my field. I don't think any customer has complained about the default settings. Regards.
diff --git a/drivers/net/ethernet/aurora/nb8800.c b/drivers/net/ethernet/aurora/nb8800.c index 041cfb7952f8..f3ed320eb4ad 100644 --- a/drivers/net/ethernet/aurora/nb8800.c +++ b/drivers/net/ethernet/aurora/nb8800.c @@ -609,7 +609,7 @@ static void nb8800_mac_config(struct net_device *dev) mac_mode |= HALF_DUPLEX; if (gigabit) { - if (priv->phy_mode == PHY_INTERFACE_MODE_RGMII) + if (phy_interface_is_rgmii(dev->phydev)) mac_mode |= RGMII_MODE; mac_mode |= GMAC_MODE; @@ -1268,11 +1268,13 @@ static int nb8800_tangox_init(struct net_device *dev) break; case PHY_INTERFACE_MODE_RGMII: - pad_mode = PAD_MODE_RGMII; + case PHY_INTERFACE_MODE_RGMII_RXID: + pad_mode = PAD_MODE_RGMII | PAD_MODE_GTX_CLK_DELAY; break; + case PHY_INTERFACE_MODE_RGMII_ID: case PHY_INTERFACE_MODE_RGMII_TXID: - pad_mode = PAD_MODE_RGMII | PAD_MODE_GTX_CLK_DELAY; + pad_mode = PAD_MODE_RGMII; break; default:
According to commit e5f3a4a56ce2a707b2fb8ce37e4414dcac89c672 ("Documentation: devicetree: clarify usage of the RGMII phy-modes") there are 4 RGMII phy-modes to handle: "rgmii" (RX and TX delays are added by the MAC when required) "rgmii-id" (RGMII with internal RX and TX delays provided by the PHY, the MAC should not add the RX or TX delays in this case) "rgmii-rxid" (RGMII with internal RX delay provided by the PHY, the MAC should not add an RX delay in this case) "rgmii-txid" (RGMII with internal TX delay provided by the PHY, the MAC should not add an TX delay in this case) Let the MAC handle TX clock delay for rgmii and rgmii-rxid. Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com> --- drivers/net/ethernet/aurora/nb8800.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)