Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Fri, 17 Jul 2020 01:38:22 +0300
From:   Vladimir Oltean <olteanv@gmail.com>
To:     Jakub Kicinski <kuba@kernel.org>
Cc:     Matthew Hagan <mnhagan88@gmail.com>, Andrew Lunn <andrew@lunn.ch>,
        Vivien Didelot <vivien.didelot@gmail.com>,
        Florian Fainelli <f.fainelli@gmail.com>,
        "David S. Miller" <davem@davemloft.net>, linux@armlinux.org.uk,
        netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
        John Crispin <john@phrozen.org>,
        Jonathan McDowell <noodles@earth.li>,
        Rob Herring <robh+dt@kernel.org>, devicetree@vger.kernel.org
Subject: Re: [PATCH 2/2] dt-bindings: net: dsa: qca8k: Add PORT0_PAD_CTRL
 properties
Message-ID: <20200716223822.yptldqqn36fbp2i7@skbuf>
References: <2e1776f997441792a44cd35a16f1e69f848816ce.1594668793.git.mnhagan88@gmail.com>
 <ea0a35ed686e6dace77e25cb70a8f39fdd1ea8ad.1594668793.git.mnhagan88@gmail.com>
 <20200716150925.0f3e01b8@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20200716150925.0f3e01b8@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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