Re: [PATCH 2/4] dt-bindings: net: dsa: document internal MDIO bus

From: "Russell King (Oracle)" <linux@armlinux.org.uk>
To: Vladimir Oltean <olteanv@gmail.com>
Cc: "Arınç ÜNAL" <arinc.unal@arinc9.com>,
	"Andrew Lunn" <andrew@lunn.ch>,
	"Florian Fainelli" <f.fainelli@gmail.com>,
	"David S. Miller" <davem@davemloft.net>,
	"Eric Dumazet" <edumazet@google.com>,
	"Jakub Kicinski" <kuba@kernel.org>,
	"Paolo Abeni" <pabeni@redhat.com>,
	"Rob Herring" <robh+dt@kernel.org>,
	"Krzysztof Kozlowski" <krzysztof.kozlowski+dt@linaro.org>,
	"Conor Dooley" <conor+dt@kernel.org>,
	"Woojung Huh" <woojung.huh@microchip.com>,
	UNGLinuxDriver@microchip.com,
	"Linus Walleij" <linus.walleij@linaro.org>,
	"Alvin Šipraga" <alsi@bang-olufsen.dk>,
	"Daniel Golle" <daniel@makrotopia.org>,
	"Landen Chao" <Landen.Chao@mediatek.com>,
	"DENG Qingfang" <dqfext@gmail.com>,
	"Sean Wang" <sean.wang@mediatek.com>,
	"Matthias Brugger" <matthias.bgg@gmail.com>,
	"AngeloGioacchino Del Regno"
	<angelogioacchino.delregno@collabora.com>,
	mithat.guner@xeront.com, erkin.bozoglu@xeront.com,
	netdev@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-mediatek@lists.infradead.org
Subject: Re: [PATCH 2/4] dt-bindings: net: dsa: document internal MDIO bus
Date: Wed, 13 Sep 2023 16:59:19 +0100	[thread overview]
Message-ID: <ZQHcV5DUfcCYkkTi@shell.armlinux.org.uk> (raw)
In-Reply-To: <20230913074231.5azwxqjuv2wp5nik@skbuf>

On Wed, Sep 13, 2023 at 10:42:31AM +0300, Vladimir Oltean wrote:
> On Wed, Sep 13, 2023 at 08:52:37AM +0300, Arınç ÜNAL wrote:
> > On 12.09.2023 22:34, Vladimir Oltean wrote:
> > > On Tue, Sep 12, 2023 at 10:23:51PM +0300, Arınç ÜNAL wrote:
> > > > The phylink bindings for user ports I ended up making by looking up the
> > > > existing devicetrees are different than the phylink bindings for the shared
> > > > (CPU and DSA) ports currently enforced on all switches.
> > > > 
> > > > My phylink bindings for user ports:
> > > > 
> > > >              allOf:
> > > >                - anyOf:
> > > >                    - required: [ fixed-link ]
> > > >                    - required: [ phy-handle ]
> > > >                    - required: [ managed ]
> > > > 
> > > >                - if:
> > > >                    required: [ fixed-link ]
> > > >                  then:
> > > >                    not:
> > > >                      required: [ managed ]
> > > 
> > > Right, it should have been anyOf and not oneOf.. my mistake. It is a bug
> > > which should be fixed. It's the same phylink that gets used in both cases,
> > > user ports and shared ports :)
> > 
> > One more thing, I don't recall phy-mode being required to be defined for
> > user ports as it will default to GMII. I don't believe this is the same
> > case for shared ports so phy-mode is required only for them?
> 
> phy-mode is not strictly required, but I think there is a strong
> preference to set it. IIRC, when looking at the DSA device trees, there
> was no case where phy-mode would be absent on CPU/DSA ports if the other
> link properties were also present, so we required it too. There were no
> complaints in 1 year since dsa_shared_port_validate_of() is there. The
> requirement can be relaxed to just a warning and no error in the kernel,
> and the removal of "required" in the schema, if it helps making it
> common with user ports.

However, phylink pretty much requires phy-mode to be specified to be
something sane for shared ports, so I wouldn't be in favour of relaxing
the checkinng in dsa_shared_port_validate_of()... not unless you're
now going to accept the approach I originally proposed to have DSA
drivers tell the core (and thus phylink) what phy-mode and other link
parameters should be used when they are missing from DT.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!