Re: [net-next PATCH v2 08/15] dt-bindings: net: dsa: qca8k: Add MAC swap and clock phase properties

From: Ansuel Smith <ansuelsmth@gmail.com>
To: Andrew Lunn <andrew@lunn.ch>
Cc: Vivien Didelot <vivien.didelot@gmail.com>,
	Florian Fainelli <f.fainelli@gmail.com>,
	Vladimir Oltean <olteanv@gmail.com>,
	"David S. Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>,
	Rob Herring <robh+dt@kernel.org>,
	Heiner Kallweit <hkallweit1@gmail.com>,
	Russell King <linux@armlinux.org.uk>,
	netdev@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, Matthew Hagan <mnhagan88@gmail.com>
Subject: Re: [net-next PATCH v2 08/15] dt-bindings: net: dsa: qca8k: Add MAC swap and clock phase properties
Date: Sat, 9 Oct 2021 20:08:24 +0200	[thread overview]
Message-ID: <YWHamNcXmxuaVgB+@Ansuel-xps.localdomain> (raw)
In-Reply-To: <YWHMRMTSa8xP4SKK@lunn.ch>

On Sat, Oct 09, 2021 at 07:07:16PM +0200, Andrew Lunn wrote:
> On Fri, Oct 08, 2021 at 02:22:18AM +0200, Ansuel Smith wrote:
> > Add names and decriptions of additional PORT0_PAD_CTRL properties.
> > Document new binding qca,mac6_exchange that exchange the mac0 port
> > with mac6.
> > qca,sgmii-(rx|tx)clk-falling-edge are for setting the respective clock
> > phase to failling edge.
> > 
> > Signed-off-by: Matthew Hagan <mnhagan88@gmail.com>
> > Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> > ---
> >  Documentation/devicetree/bindings/net/dsa/qca8k.txt | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/net/dsa/qca8k.txt b/Documentation/devicetree/bindings/net/dsa/qca8k.txt
> > index 9383d6bf2426..208ee5bc1bbb 100644
> > --- a/Documentation/devicetree/bindings/net/dsa/qca8k.txt
> > +++ b/Documentation/devicetree/bindings/net/dsa/qca8k.txt
> > @@ -13,6 +13,11 @@ Required properties:
> >  Optional properties:
> >  
> >  - reset-gpios: GPIO to be used to reset the whole device
> > +- qca,mac6-exchange: Internally swap MAC0 with MAC6.
> > +- qca,sgmii-rxclk-falling-edge: Set the receive clock phase to falling edge.
> > +                                Mostly used in qca8327 with CPU port 0 set to
> > +                                sgmii.
> > +- qca,sgmii-txclk-falling-edge: Set the transmit clock phase to falling edge.
> >  - qca,rgmii0-1-8v: Set the internal regulator to supply 1.8v for MAC0 port.
> >                     This is needed for qca8337 and toggles the supply voltage
> >                     from 1.5v to 1.8v. For the specific regs it was observed
> 
> The edge configuration is a port configuration. So it should be inside
> the port DT node it applies to. That also gives a clean way forward
> when a new switch appears with more SGMII interfaces, each with its
> own edge configuration.
> 

Problem here is that from Documentation falling edge can be set only on
PAD0. PAD5 and PAD6 have the related bit reserved.
But anyway qca8k support only single sgmii and it's not supported a
config with multiple sgmii. Do we have standard binding for this?

> But that then leads into the MAC0/MAC6 swap mess. I need to think
> about that some more, how do we cleanly describe that in DT.
> 
>       Andrew

About the mac swap. Do we really need to implement a complex thing for
something that is really implemented internally to the switch? With this
option MAC6 is swapped with MAC0. But with the port configuration in DT
it doesn't change anything. Same reg, no change. It's really that some
OEM connect the secondary port instead of the primary (for some reason,
hw choice?) and swap them internally.

Anyway some question.
I will move the falling binding to the port DT node and move the
configuration to mac_config. Should I keep the
dedicated function to setup PAD0 swap or I can directly add the check in
the qca8k_setup for only the bit related to enable the swap?

-- 
	Ansuel