Message ID | ea0a35ed686e6dace77e25cb70a8f39fdd1ea8ad.1594668793.git.mnhagan88@gmail.com |
---|---|
State | Changes Requested |
Delegated to: | David Miller |
Headers | show |
Series | [1/2] net: dsa: qca8k: Add additional PORT0_PAD_CTRL options | expand |
On Mon, 13 Jul 2020 21:50:26 +0100 Matthew Hagan wrote: > Add names and decriptions of additional PORT0_PAD_CTRL properties. > > Signed-off-by: Matthew Hagan <mnhagan88@gmail.com> > --- > Documentation/devicetree/bindings/net/dsa/qca8k.txt | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/Documentation/devicetree/bindings/net/dsa/qca8k.txt b/Documentation/devicetree/bindings/net/dsa/qca8k.txt > index ccbc6d89325d..3d34c4f2e891 100644 > --- a/Documentation/devicetree/bindings/net/dsa/qca8k.txt > +++ b/Documentation/devicetree/bindings/net/dsa/qca8k.txt > @@ -13,6 +13,14 @@ Optional properties: > > - reset-gpios: GPIO to be used to reset the whole device > > +Optional MAC configuration properties: > + > +- qca,exchange-mac0-mac6: If present, internally swaps MAC0 and MAC6. Perhaps we can say a little more here? > +- qca,sgmii-rxclk-falling-edge: If present, sets receive clock phase to > + falling edge. > +- qca,sgmii-txclk-falling-edge: If present, sets transmit clock phase to > + falling edge. These are not something that other vendors may implement and therefore something we may want to make generic? Andrew? > Subnodes: > > The integrated switch subnode should be specified according to the binding
On 7/13/2020 1:50 PM, Matthew Hagan wrote: > Add names and decriptions of additional PORT0_PAD_CTRL properties. > > Signed-off-by: Matthew Hagan <mnhagan88@gmail.com> > --- > Documentation/devicetree/bindings/net/dsa/qca8k.txt | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/Documentation/devicetree/bindings/net/dsa/qca8k.txt b/Documentation/devicetree/bindings/net/dsa/qca8k.txt > index ccbc6d89325d..3d34c4f2e891 100644 > --- a/Documentation/devicetree/bindings/net/dsa/qca8k.txt > +++ b/Documentation/devicetree/bindings/net/dsa/qca8k.txt > @@ -13,6 +13,14 @@ Optional properties: > > - reset-gpios: GPIO to be used to reset the whole device > > +Optional MAC configuration properties: > + > +- qca,exchange-mac0-mac6: If present, internally swaps MAC0 and MAC6. > +- qca,sgmii-rxclk-falling-edge: If present, sets receive clock phase to > + falling edge. > +- qca,sgmii-txclk-falling-edge: If present, sets transmit clock phase to > + falling edge. Are not these two mutually exclusive, that is the presence of one implies the absence of the other?
On Thu, Jul 16, 2020 at 03:09:25PM -0700, Jakub Kicinski wrote: > On Mon, 13 Jul 2020 21:50:26 +0100 Matthew Hagan wrote: > > Add names and decriptions of additional PORT0_PAD_CTRL properties. > > > > Signed-off-by: Matthew Hagan <mnhagan88@gmail.com> > > --- > > Documentation/devicetree/bindings/net/dsa/qca8k.txt | 8 ++++++++ > > 1 file changed, 8 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/net/dsa/qca8k.txt b/Documentation/devicetree/bindings/net/dsa/qca8k.txt > > index ccbc6d89325d..3d34c4f2e891 100644 > > --- a/Documentation/devicetree/bindings/net/dsa/qca8k.txt > > +++ b/Documentation/devicetree/bindings/net/dsa/qca8k.txt > > @@ -13,6 +13,14 @@ Optional properties: > > > > - reset-gpios: GPIO to be used to reset the whole device > > > > +Optional MAC configuration properties: > > + > > +- qca,exchange-mac0-mac6: If present, internally swaps MAC0 and MAC6. > > Perhaps we can say a little more here? > > > +- qca,sgmii-rxclk-falling-edge: If present, sets receive clock phase to > > + falling edge. > > +- qca,sgmii-txclk-falling-edge: If present, sets transmit clock phase to > > + falling edge. > > These are not something that other vendors may implement and therefore > something we may want to make generic? Andrew? I've never seen any other vendor implement this. Which to me makes me think this is a vendor extension, to Ciscos vendor extension of 1000BaseX. Matthew, do you have a real use cases of these? I don't see a DT patch making use of them. And if you do, what is the PHY on the other end which also allows you to invert the clocks? Andrew
On Thu, Jul 16, 2020 at 03:09:25PM -0700, Jakub Kicinski wrote: > On Mon, 13 Jul 2020 21:50:26 +0100 Matthew Hagan wrote: > > Add names and decriptions of additional PORT0_PAD_CTRL properties. > > > > Signed-off-by: Matthew Hagan <mnhagan88@gmail.com> > > --- > > Documentation/devicetree/bindings/net/dsa/qca8k.txt | 8 ++++++++ > > 1 file changed, 8 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/net/dsa/qca8k.txt b/Documentation/devicetree/bindings/net/dsa/qca8k.txt > > index ccbc6d89325d..3d34c4f2e891 100644 > > --- a/Documentation/devicetree/bindings/net/dsa/qca8k.txt > > +++ b/Documentation/devicetree/bindings/net/dsa/qca8k.txt > > @@ -13,6 +13,14 @@ Optional properties: > > > > - reset-gpios: GPIO to be used to reset the whole device > > > > +Optional MAC configuration properties: > > + > > +- qca,exchange-mac0-mac6: If present, internally swaps MAC0 and MAC6. > > Perhaps we can say a little more here? > > > +- qca,sgmii-rxclk-falling-edge: If present, sets receive clock phase to > > + falling edge. > > +- qca,sgmii-txclk-falling-edge: If present, sets transmit clock phase to > > + falling edge. > > These are not something that other vendors may implement and therefore > something we may want to make generic? Andrew? > It was asked before whether this device uses source-synchronous clock for SGMII or if it recovers the clock from the data stream. Just "pass" was given for a response. https://patchwork.ozlabs.org/project/netdev/patch/8ddd76e484e1bedd12c87ea0810826b60e004a65.1591380105.git.noodles@earth.li/ One can, in principle, tell easily by examining schematics. If the SGMII is only connected via RX_P, RX_N, TX_P, TX_N (and optionally there might be external reference clocks for the SERDES lanes, but these are not part of the data connection itself), then the clock is recovered from the serial data stream, and we have no idea what "SGMII delays" are. If the schematic shows 2 extra clock signals, one in each transmit direction, then this is, in Russell King's words, "a new world of RGMII delay pain but for SGMII". In principle I would fully expect clock skews to be necessary for any high-speed protocol with source-synchronous clocking. The problem, really, is that we aren't ready to deal with this properly. We aren't distinguishing "SGMII with clock" from "SGMII without clock" in any way. We have no idea who else is using such a thing. Depending on the magnitude of this new world, it may be wise to let these bindings go in as-is, or do something more kernel-wide... One simple question to ask Matthew is what are you connecting to these SGMII lanes, and if you need any special configuration on the other end of those lanes too (and what is the configuration you are using on the qca8k: enable the "SGMII delays" in both directions?). > > Subnodes: > > > > The integrated switch subnode should be specified according to the binding > Thanks, -Vladimir
On Fri, Jul 17, 2020 at 01:38:22AM +0300, Vladimir Oltean wrote: > On Thu, Jul 16, 2020 at 03:09:25PM -0700, Jakub Kicinski wrote: > > On Mon, 13 Jul 2020 21:50:26 +0100 Matthew Hagan wrote: > > > +- qca,sgmii-rxclk-falling-edge: If present, sets receive clock phase to > > > + falling edge. > > > +- qca,sgmii-txclk-falling-edge: If present, sets transmit clock phase to > > > + falling edge. > > > > These are not something that other vendors may implement and therefore > > something we may want to make generic? Andrew? > > > > It was asked before whether this device uses source-synchronous clock > for SGMII or if it recovers the clock from the data stream. Just "pass" > was given for a response. > > https://patchwork.ozlabs.org/project/netdev/patch/8ddd76e484e1bedd12c87ea0810826b60e004a65.1591380105.git.noodles@earth.li/ > > One can, in principle, tell easily by examining schematics. If the SGMII > is only connected via RX_P, RX_N, TX_P, TX_N (and optionally there might > be external reference clocks for the SERDES lanes, but these are not > part of the data connection itself), then the clock is recovered from > the serial data stream, and we have no idea what "SGMII delays" are. > > If the schematic shows 2 extra clock signals, one in each transmit > direction, then this is, in Russell King's words, "a new world of RGMII > delay pain but for SGMII". In principle I would fully expect clock skews > to be necessary for any high-speed protocol with source-synchronous > clocking. The problem, really, is that we aren't ready to deal with this > properly. We aren't distinguishing "SGMII with clock" from "SGMII > without clock" in any way. We have no idea who else is using such a > thing. Depending on the magnitude of this new world, it may be wise to > let these bindings go in as-is, or do something more kernel-wide... I don't have the schematic for the device I've been working with, but the switch data sheet just shows 2 differential pairs (input/output) for the SerDes Interface (whereas the RGMII interfaces *are* listed with their clocks). J.
On 16/07/2020 23:32, Andrew Lunn wrote: > On Thu, Jul 16, 2020 at 03:09:25PM -0700, Jakub Kicinski wrote: >> On Mon, 13 Jul 2020 21:50:26 +0100 Matthew Hagan wrote: >>> Add names and decriptions of additional PORT0_PAD_CTRL properties. >>> >>> Signed-off-by: Matthew Hagan <mnhagan88@gmail.com> >>> --- >>> Documentation/devicetree/bindings/net/dsa/qca8k.txt | 8 ++++++++ >>> 1 file changed, 8 insertions(+) >>> >>> diff --git a/Documentation/devicetree/bindings/net/dsa/qca8k.txt b/Documentation/devicetree/bindings/net/dsa/qca8k.txt >>> index ccbc6d89325d..3d34c4f2e891 100644 >>> --- a/Documentation/devicetree/bindings/net/dsa/qca8k.txt >>> +++ b/Documentation/devicetree/bindings/net/dsa/qca8k.txt >>> @@ -13,6 +13,14 @@ Optional properties: >>> >>> - reset-gpios: GPIO to be used to reset the whole device >>> >>> +Optional MAC configuration properties: >>> + >>> +- qca,exchange-mac0-mac6: If present, internally swaps MAC0 and MAC6. >> >> Perhaps we can say a little more here? >> >>> +- qca,sgmii-rxclk-falling-edge: If present, sets receive clock phase to >>> + falling edge. >>> +- qca,sgmii-txclk-falling-edge: If present, sets transmit clock phase to >>> + falling edge. >> >> These are not something that other vendors may implement and therefore >> something we may want to make generic? Andrew? > > I've never seen any other vendor implement this. Which to me makes me > think this is a vendor extension, to Ciscos vendor extension of > 1000BaseX. > > Matthew, do you have a real use cases of these? I don't see a DT patch > making use of them. And if you do, what is the PHY on the other end > which also allows you to invert the clocks? > The use case I am working on is the Cisco Meraki MX65 which requires bit 18 set (qca,sgmii-txclk-falling-edge). On the other side is a BCM58625 SRAB with ports 4 and 5 in SGMII mode. There is no special polarity configuration set on this side though I do have very limited info on what is available. The settings I have replicate the vendor configuration extracted from the device. The qca,sgmii-rxclk-falling-edge option (bit 19) is commonly used according to the device trees found in the OpenWrt, which is still using the ar8216 driver. With a count through the ar8327-initvals I see bit 19 set on 18 of 22 devices using SGMII on MAC0. > Andrew > Matthew
On 7/17/2020 12:26 PM, Matthew Hagan wrote: > > > On 16/07/2020 23:32, Andrew Lunn wrote: >> On Thu, Jul 16, 2020 at 03:09:25PM -0700, Jakub Kicinski wrote: >>> On Mon, 13 Jul 2020 21:50:26 +0100 Matthew Hagan wrote: >>>> Add names and decriptions of additional PORT0_PAD_CTRL properties. >>>> >>>> Signed-off-by: Matthew Hagan <mnhagan88@gmail.com> >>>> --- >>>> Documentation/devicetree/bindings/net/dsa/qca8k.txt | 8 ++++++++ >>>> 1 file changed, 8 insertions(+) >>>> >>>> diff --git a/Documentation/devicetree/bindings/net/dsa/qca8k.txt b/Documentation/devicetree/bindings/net/dsa/qca8k.txt >>>> index ccbc6d89325d..3d34c4f2e891 100644 >>>> --- a/Documentation/devicetree/bindings/net/dsa/qca8k.txt >>>> +++ b/Documentation/devicetree/bindings/net/dsa/qca8k.txt >>>> @@ -13,6 +13,14 @@ Optional properties: >>>> >>>> - reset-gpios: GPIO to be used to reset the whole device >>>> >>>> +Optional MAC configuration properties: >>>> + >>>> +- qca,exchange-mac0-mac6: If present, internally swaps MAC0 and MAC6. >>> >>> Perhaps we can say a little more here? >>> >>>> +- qca,sgmii-rxclk-falling-edge: If present, sets receive clock phase to >>>> + falling edge. >>>> +- qca,sgmii-txclk-falling-edge: If present, sets transmit clock phase to >>>> + falling edge. >>> >>> These are not something that other vendors may implement and therefore >>> something we may want to make generic? Andrew? >> >> I've never seen any other vendor implement this. Which to me makes me >> think this is a vendor extension, to Ciscos vendor extension of >> 1000BaseX. >> >> Matthew, do you have a real use cases of these? I don't see a DT patch >> making use of them. And if you do, what is the PHY on the other end >> which also allows you to invert the clocks? >> > The use case I am working on is the Cisco Meraki MX65 which requires bit > 18 set (qca,sgmii-txclk-falling-edge). On the other side is a BCM58625 > SRAB with ports 4 and 5 in SGMII mode. There is no special polarity > configuration set on this side though I do have very limited info on > what is available. The settings I have replicate the vendor > configuration extracted from the device. The only polarity change that I am aware of on the BCM58625 side is to allow for the TXDP/TXDN to be swapped, this is achieved by setting bit 5 in the TX_ACONTROL0 register (block address is 0x8060), that does look different than what this is controlling though.
On 16/07/2020 23:09, Jakub Kicinski wrote: > On Mon, 13 Jul 2020 21:50:26 +0100 Matthew Hagan wrote: >> Add names and decriptions of additional PORT0_PAD_CTRL properties. >> >> Signed-off-by: Matthew Hagan <mnhagan88@gmail.com> >> --- >> Documentation/devicetree/bindings/net/dsa/qca8k.txt | 8 ++++++++ >> 1 file changed, 8 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/net/dsa/qca8k.txt b/Documentation/devicetree/bindings/net/dsa/qca8k.txt >> index ccbc6d89325d..3d34c4f2e891 100644 >> --- a/Documentation/devicetree/bindings/net/dsa/qca8k.txt >> +++ b/Documentation/devicetree/bindings/net/dsa/qca8k.txt >> @@ -13,6 +13,14 @@ Optional properties: >> >> - reset-gpios: GPIO to be used to reset the whole device >> >> +Optional MAC configuration properties: >> + >> +- qca,exchange-mac0-mac6: If present, internally swaps MAC0 and MAC6. > > Perhaps we can say a little more here? > From John's patch: "The switch allows us to swap the internal wirering of the two cpu ports. For the HW offloading to work the ethernet MAC conencting to the LAN ports must be wired to cpu port 0. There is HW in the wild that does not fulfill this requirement. On these boards we need to swap the cpu ports." This option is somewhat linked to instances where both MAC0 and MAC6 are used as CPU ports. I may omit this for now since support for this hasn't been added and MAC0 is hard-coded as the CPU port. The initial intention here was to cover options commonly set by OpenWrt devices, based upon their ar8327-initvals, to allow migration to qca8k. >> +- qca,sgmii-rxclk-falling-edge: If present, sets receive clock phase to >> + falling edge. >> +- qca,sgmii-txclk-falling-edge: If present, sets transmit clock phase to >> + falling edge. > > These are not something that other vendors may implement and therefore > something we may want to make generic? Andrew? > >> Subnodes: >> >> The integrated switch subnode should be specified according to the binding > Matthew
On 7/17/2020 1:29 PM, Matthew Hagan wrote: > > > On 16/07/2020 23:09, Jakub Kicinski wrote: >> On Mon, 13 Jul 2020 21:50:26 +0100 Matthew Hagan wrote: >>> Add names and decriptions of additional PORT0_PAD_CTRL properties. >>> >>> Signed-off-by: Matthew Hagan <mnhagan88@gmail.com> >>> --- >>> Documentation/devicetree/bindings/net/dsa/qca8k.txt | 8 ++++++++ >>> 1 file changed, 8 insertions(+) >>> >>> diff --git a/Documentation/devicetree/bindings/net/dsa/qca8k.txt b/Documentation/devicetree/bindings/net/dsa/qca8k.txt >>> index ccbc6d89325d..3d34c4f2e891 100644 >>> --- a/Documentation/devicetree/bindings/net/dsa/qca8k.txt >>> +++ b/Documentation/devicetree/bindings/net/dsa/qca8k.txt >>> @@ -13,6 +13,14 @@ Optional properties: >>> >>> - reset-gpios: GPIO to be used to reset the whole device >>> >>> +Optional MAC configuration properties: >>> + >>> +- qca,exchange-mac0-mac6: If present, internally swaps MAC0 and MAC6. >> >> Perhaps we can say a little more here? >> > From John's patch: > "The switch allows us to swap the internal wirering of the two cpu ports. > For the HW offloading to work the ethernet MAC conencting to the LAN > ports must be wired to cpu port 0. There is HW in the wild that does not > fulfill this requirement. On these boards we need to swap the cpu ports." > > This option is somewhat linked to instances where both MAC0 and MAC6 are > used as CPU ports. I may omit this for now since support for this hasn't > been added and MAC0 is hard-coded as the CPU port. The initial intention > here was to cover options commonly set by OpenWrt devices, based upon > their ar8327-initvals, to allow migration to qca8k. If you update the description of the property, I do not see a reason why this should not be supported as of today, sooner or later you will need it to convert more devices to qca8k as you say.
On 17.07.20 22:29, Matthew Hagan wrote: > > On 16/07/2020 23:09, Jakub Kicinski wrote: >> On Mon, 13 Jul 2020 21:50:26 +0100 Matthew Hagan wrote: >>> Add names and decriptions of additional PORT0_PAD_CTRL properties. >>> >>> Signed-off-by: Matthew Hagan <mnhagan88@gmail.com> >>> --- >>> Documentation/devicetree/bindings/net/dsa/qca8k.txt | 8 ++++++++ >>> 1 file changed, 8 insertions(+) >>> >>> diff --git a/Documentation/devicetree/bindings/net/dsa/qca8k.txt b/Documentation/devicetree/bindings/net/dsa/qca8k.txt >>> index ccbc6d89325d..3d34c4f2e891 100644 >>> --- a/Documentation/devicetree/bindings/net/dsa/qca8k.txt >>> +++ b/Documentation/devicetree/bindings/net/dsa/qca8k.txt >>> @@ -13,6 +13,14 @@ Optional properties: >>> >>> - reset-gpios: GPIO to be used to reset the whole device >>> >>> +Optional MAC configuration properties: >>> + >>> +- qca,exchange-mac0-mac6: If present, internally swaps MAC0 and MAC6. >> Perhaps we can say a little more here? >> > From John's patch: > "The switch allows us to swap the internal wirering of the two cpu ports. > For the HW offloading to work the ethernet MAC conencting to the LAN > ports must be wired to cpu port 0. There is HW in the wild that does not > fulfill this requirement. On these boards we need to swap the cpu ports." > > This option is somewhat linked to instances where both MAC0 and MAC6 are > used as CPU ports. I may omit this for now since support for this hasn't > been added and MAC0 is hard-coded as the CPU port. The initial intention > here was to cover options commonly set by OpenWrt devices, based upon > their ar8327-initvals, to allow migration to qca8k. > > correct, specifically quantenna designs do this, also saw ciscos swap mac0/6 for cpu port, that part of the patch is definitely safe to go. I stumbled across this while making qca8k work for g-fiber on a quantenna SoC. in regards to the sgmii clk skew. I never understood the electrics fully I am afraid, but without the patch it simply does not work. my eletcric foo is unfortunately is not sufficient to understand the "whys" I am afraid. John
On 17.07.20 22:39, Florian Fainelli wrote: > > On 7/17/2020 1:29 PM, Matthew Hagan wrote: >> >> On 16/07/2020 23:09, Jakub Kicinski wrote: >>> On Mon, 13 Jul 2020 21:50:26 +0100 Matthew Hagan wrote: >>>> Add names and decriptions of additional PORT0_PAD_CTRL properties. >>>> >>>> Signed-off-by: Matthew Hagan <mnhagan88@gmail.com> >>>> --- >>>> Documentation/devicetree/bindings/net/dsa/qca8k.txt | 8 ++++++++ >>>> 1 file changed, 8 insertions(+) >>>> >>>> diff --git a/Documentation/devicetree/bindings/net/dsa/qca8k.txt b/Documentation/devicetree/bindings/net/dsa/qca8k.txt >>>> index ccbc6d89325d..3d34c4f2e891 100644 >>>> --- a/Documentation/devicetree/bindings/net/dsa/qca8k.txt >>>> +++ b/Documentation/devicetree/bindings/net/dsa/qca8k.txt >>>> @@ -13,6 +13,14 @@ Optional properties: >>>> >>>> - reset-gpios: GPIO to be used to reset the whole device >>>> >>>> +Optional MAC configuration properties: >>>> + >>>> +- qca,exchange-mac0-mac6: If present, internally swaps MAC0 and MAC6. >>> Perhaps we can say a little more here? >>> >> From John's patch: >> "The switch allows us to swap the internal wirering of the two cpu ports. >> For the HW offloading to work the ethernet MAC conencting to the LAN >> ports must be wired to cpu port 0. There is HW in the wild that does not >> fulfill this requirement. On these boards we need to swap the cpu ports." >> >> This option is somewhat linked to instances where both MAC0 and MAC6 are >> used as CPU ports. I may omit this for now since support for this hasn't >> been added and MAC0 is hard-coded as the CPU port. The initial intention >> here was to cover options commonly set by OpenWrt devices, based upon >> their ar8327-initvals, to allow migration to qca8k. > If you update the description of the property, I do not see a reason why > this should not be supported as of today, sooner or later you will need > it to convert more devices to qca8k as you say. correct, there will be patches soonish to make qcom dakota and hawkeye/cypress use qca8k as their switch fabric is 95% identical. we already started working on it. it is mmio based rather than mdio based, so the patch is quite a large rework right now. John
On Fri, Jul 17, 2020 at 08:26:02PM +0100, Matthew Hagan wrote: > > > On 16/07/2020 23:32, Andrew Lunn wrote: > > On Thu, Jul 16, 2020 at 03:09:25PM -0700, Jakub Kicinski wrote: > >> On Mon, 13 Jul 2020 21:50:26 +0100 Matthew Hagan wrote: > >>> Add names and decriptions of additional PORT0_PAD_CTRL properties. > >>> > >>> Signed-off-by: Matthew Hagan <mnhagan88@gmail.com> > >>> --- > >>> Documentation/devicetree/bindings/net/dsa/qca8k.txt | 8 ++++++++ > >>> 1 file changed, 8 insertions(+) > >>> > >>> diff --git a/Documentation/devicetree/bindings/net/dsa/qca8k.txt b/Documentation/devicetree/bindings/net/dsa/qca8k.txt > >>> index ccbc6d89325d..3d34c4f2e891 100644 > >>> --- a/Documentation/devicetree/bindings/net/dsa/qca8k.txt > >>> +++ b/Documentation/devicetree/bindings/net/dsa/qca8k.txt > >>> @@ -13,6 +13,14 @@ Optional properties: > >>> > >>> - reset-gpios: GPIO to be used to reset the whole device > >>> > >>> +Optional MAC configuration properties: > >>> + > >>> +- qca,exchange-mac0-mac6: If present, internally swaps MAC0 and MAC6. > >> > >> Perhaps we can say a little more here? > >> > >>> +- qca,sgmii-rxclk-falling-edge: If present, sets receive clock phase to > >>> + falling edge. > >>> +- qca,sgmii-txclk-falling-edge: If present, sets transmit clock phase to > >>> + falling edge. > >> > >> These are not something that other vendors may implement and therefore > >> something we may want to make generic? Andrew? > > > > I've never seen any other vendor implement this. Which to me makes me > > think this is a vendor extension, to Ciscos vendor extension of > > 1000BaseX. > > > > Matthew, do you have a real use cases of these? I don't see a DT patch > > making use of them. And if you do, what is the PHY on the other end > > which also allows you to invert the clocks? > > > The use case I am working on is the Cisco Meraki MX65 which requires bit > 18 set (qca,sgmii-txclk-falling-edge). On the other side is a BCM58625 > SRAB with ports 4 and 5 in SGMII mode. There is no special polarity > configuration set on this side though I do have very limited info on > what is available. The settings I have replicate the vendor > configuration extracted from the device. > > The qca,sgmii-rxclk-falling-edge option (bit 19) is commonly used > according to the device trees found in the OpenWrt, which is still using > the ar8216 driver. With a count through the ar8327-initvals I see bit 19 > set on 18 of 22 devices using SGMII on MAC0. > > Andrew > > > > Matthew Let's say I'm a user. When would I need to set qca,sgmii-txclk-falling-edge and/or qca,sgmii-rxclk-falling-edge, and wwhen would I need not to? Thanks, -Vladimir
On Fri, Jul 17, 2020 at 10:44:19PM +0200, John Crispin wrote: > in regards to the sgmii clk skew. I never understood the electrics fully I > am afraid, but without the patch it simply does not work. my eletcric foo is > unfortunately is not sufficient to understand the "whys" I am afraid. Do you happen to know what frequency the clock is? Is it 1.25GHz or 625MHz? It sounds like it may be 1.25GHz if the edge is important. If the clock is 1.25GHz, the "why" is because of hazards (it has nothing to do with delays in RGMII being propagated to SGMII). Quite simply, a flip-flop suffers from metastability if the clock and data inputs change at about the same time. Amongst the parametrics of flip-flops will be a data setup time, and a data hold time, referenced to the clock signal. If the data changes within the setup and hold times of the clock changing, then the output of the flip-flop is unpredictable - it can latch a logic 1 or a logic 0, or oscillate between the two until settling on one state. So, if data is clocked out on the rising edge of a clock signal, and clocked in on the rising edge of a clock signal - and the data and clock edges arrive within the setup and hold times at the flip-flop that is clocking the data in, there is a metastability hazard, and the data bit that is latched is unpredictable. One way to solve this is to clock data out on one edge, and clock data in on the opposite edge - this is used on buses such as SPI. Other buses such as I2C define minimum separation between transitions between the SDA and SCL signals. These solutions don't work with RGMII - the RGMII TXC clocks data on both edges. The only solution there is to ensure a delay is introduced between the data and clock changes seen at the receiver - which can be done by introducing delays at the transmitter or at the receiver, or by serpentine routing of the traces to induce delays to separate the clock and data transitions sufficiently to avoid metastability. If the clock is 625MHz (as with some Marvell devices for SGMII) then both clock edges are used, and both edges are used just like RGMII. Therefore, the same considerations as RGMII apply there to ensure that the data setup and hold times are not violated.
On Sat, Jul 18, 2020 at 02:20:11PM +0100, Russell King - ARM Linux admin wrote: > On Fri, Jul 17, 2020 at 10:44:19PM +0200, John Crispin wrote: > > in regards to the sgmii clk skew. I never understood the electrics fully I > > am afraid, but without the patch it simply does not work. my eletcric foo is > > unfortunately is not sufficient to understand the "whys" I am afraid. > > Do you happen to know what frequency the clock is? Is it 1.25GHz or > 625MHz? It sounds like it may be 1.25GHz if the edge is important. I'm also a bit clueless when it comes to these systems. I thought the clock was embedded into the SERDES signal? You recover it from the signal? Florian, does the switch have a separate clock input/output? Andrew
On Sat, Jul 18, 2020 at 04:44:35PM +0200, Andrew Lunn wrote: > On Sat, Jul 18, 2020 at 02:20:11PM +0100, Russell King - ARM Linux admin wrote: > > On Fri, Jul 17, 2020 at 10:44:19PM +0200, John Crispin wrote: > > > in regards to the sgmii clk skew. I never understood the electrics fully I > > > am afraid, but without the patch it simply does not work. my eletcric foo is > > > unfortunately is not sufficient to understand the "whys" I am afraid. > > > > Do you happen to know what frequency the clock is? Is it 1.25GHz or > > 625MHz? It sounds like it may be 1.25GHz if the edge is important. > > I'm also a bit clueless when it comes to these systems. > > I thought the clock was embedded into the SERDES signal? You recover > it from the signal? Indeed it is. An external clock can be used to avoid needing clock recovery in the SERDES receiver. For example, with the 88E1111, it only accepts the datastream from the MAC with no provision for a separate clock, but it can be configured to generate a 625MHz clock for the data stream to the MAC if required. Being 625MHz (half the data rate), both edges of the clock are used, with a delay to help avoid the metastability hazard I previously described at the receiver.
On 18/07/2020 14:20, Russell King - ARM Linux admin wrote: > On Fri, Jul 17, 2020 at 10:44:19PM +0200, John Crispin wrote: >> in regards to the sgmii clk skew. I never understood the electrics fully I >> am afraid, but without the patch it simply does not work. my eletcric foo is >> unfortunately is not sufficient to understand the "whys" I am afraid. > > Do you happen to know what frequency the clock is? Is it 1.25GHz or > 625MHz? It sounds like it may be 1.25GHz if the edge is important. > > If the clock is 1.25GHz, the "why" is because of hazards (it has > nothing to do with delays in RGMII being propagated to SGMII). > > Quite simply, a flip-flop suffers from metastability if the clock and > data inputs change at about the same time. Amongst the parametrics of > flip-flops will be a data setup time, and a data hold time, referenced > to the clock signal. > > If the data changes within the setup and hold times of the clock > changing, then the output of the flip-flop is unpredictable - it can > latch a logic 1 or a logic 0, or oscillate between the two until > settling on one state. > > So, if data is clocked out on the rising edge of a clock signal, and > clocked in on the rising edge of a clock signal - and the data and > clock edges arrive within the setup and hold times at the flip-flop > that is clocking the data in, there is a metastability hazard, and > the data bit that is latched is unpredictable. > With default settings, in my case, the device will work at first, though eventually problems arise with loss of connectivity, but constant activity on the individual port led. > One way to solve this is to clock data out on one edge, and clock data > in on the opposite edge - this is used on buses such as SPI. Other > buses such as I2C define minimum separation between transitions between > the SDA and SCL signals. > Is there any case where it would matter which way round the clocks are, or is it only relevant that they are on opposite edges? Why not do this by default for qca8k devices? > These solutions don't work with RGMII - the RGMII TXC clocks data on > both edges. The only solution there is to ensure a delay is introduced > between the data and clock changes seen at the receiver - which can be > done by introducing delays at the transmitter or at the receiver, or by > serpentine routing of the traces to induce delays to separate the clock > and data transitions sufficiently to avoid metastability. > > If the clock is 625MHz (as with some Marvell devices for SGMII) then > both clock edges are used, and both edges are used just like RGMII. > Therefore, the same considerations as RGMII apply there to ensure that > the data setup and hold times are not violated. > By default, both tx and rx are set to rising edge. Matthew
On Fri, Jul 17, 2020 at 08:26:02PM +0100, Matthew Hagan wrote: > > > On 16/07/2020 23:32, Andrew Lunn wrote: > > On Thu, Jul 16, 2020 at 03:09:25PM -0700, Jakub Kicinski wrote: > >> On Mon, 13 Jul 2020 21:50:26 +0100 Matthew Hagan wrote: > >>> Add names and decriptions of additional PORT0_PAD_CTRL properties. > >>> > >>> Signed-off-by: Matthew Hagan <mnhagan88@gmail.com> > >>> --- > >>> Documentation/devicetree/bindings/net/dsa/qca8k.txt | 8 ++++++++ > >>> 1 file changed, 8 insertions(+) > >>> > >>> diff --git a/Documentation/devicetree/bindings/net/dsa/qca8k.txt b/Documentation/devicetree/bindings/net/dsa/qca8k.txt > >>> index ccbc6d89325d..3d34c4f2e891 100644 > >>> --- a/Documentation/devicetree/bindings/net/dsa/qca8k.txt > >>> +++ b/Documentation/devicetree/bindings/net/dsa/qca8k.txt > >>> @@ -13,6 +13,14 @@ Optional properties: > >>> > >>> - reset-gpios: GPIO to be used to reset the whole device > >>> > >>> +Optional MAC configuration properties: > >>> + > >>> +- qca,exchange-mac0-mac6: If present, internally swaps MAC0 and MAC6. > >> > >> Perhaps we can say a little more here? > >> > >>> +- qca,sgmii-rxclk-falling-edge: If present, sets receive clock phase to > >>> + falling edge. > >>> +- qca,sgmii-txclk-falling-edge: If present, sets transmit clock phase to > >>> + falling edge. > >> > >> These are not something that other vendors may implement and therefore > >> something we may want to make generic? Andrew? > > > > I've never seen any other vendor implement this. Which to me makes me > > think this is a vendor extension, to Ciscos vendor extension of > > 1000BaseX. > > > > Matthew, do you have a real use cases of these? I don't see a DT patch > > making use of them. And if you do, what is the PHY on the other end > > which also allows you to invert the clocks? > > > The use case I am working on is the Cisco Meraki MX65 which requires bit > 18 set (qca,sgmii-txclk-falling-edge). On the other side is a BCM58625 > SRAB with ports 4 and 5 in SGMII mode. There is no special polarity > configuration set on this side though I do have very limited info on > what is available. The settings I have replicate the vendor > configuration extracted from the device. > > The qca,sgmii-rxclk-falling-edge option (bit 19) is commonly used > according to the device trees found in the OpenWrt, which is still using > the ar8216 driver. With a count through the ar8327-initvals I see bit 19 > set on 18 of 22 devices using SGMII on MAC0. Can't you identify the device and configure the setting based on that? After all, MDIO devices are discoverable. Or there's no MDIO here? Rob
diff --git a/Documentation/devicetree/bindings/net/dsa/qca8k.txt b/Documentation/devicetree/bindings/net/dsa/qca8k.txt index ccbc6d89325d..3d34c4f2e891 100644 --- a/Documentation/devicetree/bindings/net/dsa/qca8k.txt +++ b/Documentation/devicetree/bindings/net/dsa/qca8k.txt @@ -13,6 +13,14 @@ Optional properties: - reset-gpios: GPIO to be used to reset the whole device +Optional MAC configuration properties: + +- qca,exchange-mac0-mac6: If present, internally swaps MAC0 and MAC6. +- qca,sgmii-rxclk-falling-edge: If present, sets receive clock phase to + falling edge. +- qca,sgmii-txclk-falling-edge: If present, sets transmit clock phase to + falling edge. + Subnodes: The integrated switch subnode should be specified according to the binding
Add names and decriptions of additional PORT0_PAD_CTRL properties. Signed-off-by: Matthew Hagan <mnhagan88@gmail.com> --- Documentation/devicetree/bindings/net/dsa/qca8k.txt | 8 ++++++++ 1 file changed, 8 insertions(+)