Re: Aw: Re: Re: [RFC v1] dt-bindings: net: dsa: convert binding for mediatek switches

From: Frank Wunderlich <frank-w@public-files.de>
To: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Cc: "Greg Ungerer" <gerg@kernel.org>,
	"René van Dorst" <opensource@vdorst.com>,
	"Mauro Carvalho Chehab" <mchehab+samsung@kernel.org>,
	"Frank Wunderlich" <linux@fw-web.de>,
	"Andrew Lunn" <andrew@lunn.ch>,
	"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>,
	"Paolo Abeni" <pabeni@redhat.com>,
	"Rob Herring" <robh+dt@kernel.org>,
	"Krzysztof Kozlowski" <krzysztof.kozlowski+dt@linaro.org>,
	"Matthias Brugger" <matthias.bgg@gmail.com>,
	"Sean Wang" <sean.wang@mediatek.com>,
	"Landen Chao" <Landen.Chao@mediatek.com>,
	"DENG Qingfang" <dqfext@gmail.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: Aw: Re: Re: [RFC v1] dt-bindings: net: dsa: convert binding for mediatek switches
Date: Wed, 04 May 2022 09:44:29 +0200	[thread overview]
Message-ID: <69290DD3-0179-49C2-8E7D-9F8DBDEBC96F@public-files.de> (raw)
In-Reply-To: <95aea078-3e85-79c3-79c0-430bd7c0fbae@linaro.org>

m 4. Mai 2022 08:51:41 MESZ schrieb Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>:
>On 03/05/2022 17:03, Frank Wunderlich wrote:
>> 
>> have not posted this version as it was failing in dtbs_check, this
>was how i tried:
>> 
>>
>https://github.com/frank-w/BPI-R2-4.14/blob/8f2033eb6fcae273580263c3f0b31f0d48821740/Documentation/devicetree/bindings/net/dsa/mediatek.yaml#L177
>
>You have mixed up indentation of the second if (and missing -).

The "compatible if" should be a child of the "if" above,because phy-mode property only exists for cpu-port. I can try with additional "-" (but i guess this is only needed for allOf)

Rob told me that i cannot check compatible in subnode and this check will be always true...just like my experience.
I can only make the compatible check at top-level and then need to define substructure based on this (so define structure twice). He suggested me adding this to description for now.

Imho this can be added later if really needed...did not found any example checking for compatible in a subnode. All were in top level. Afair these properties are handled by dsa-core/phylink and driver only compares constants set there.

>(...)
>
>>>>
>>>> basicly this "ports"-property should be required too, right?
>>>
>>> Previous binding did not enforce it, I think, but it is reasonable
>to
>>> require ports.
>> 
>> basicly it is required in dsa.yaml, so it will be redundant here
>> 
>>
>https://elixir.bootlin.com/linux/v5.18-rc5/source/Documentation/devicetree/bindings/net/dsa/dsa.yaml#L55
>> 
>> this defines it as pattern "^(ethernet-)?ports$" and should be
>processed by dsa-core. so maybe changing it to same pattern instead of
>moving up as normal property?
>
>Just keep what is already used in existing DTS.

Currently only "ports" is used...so i will change it to "normal" property.

>>>> for 33 there seem no constant..all other references to pio node are
>with numbers too and there seem no binding
>>>> header defining the gpio pins (only functions in
>include/dt-bindings/pinctrl/mt7623-pinfunc.h)
>>>
>>> ok, then my comment
>> 
>> you mean adding a comment to the example that GPIO-flags/constants
>should be used instead of magic numbers?
>
>I think something was cut from my reply. I wanted to say:
>"ok, then my comment can be skipped"

Ok

>But I think your check was not correct. I looked at bpi-r2 DTS
>(mt7623n)
>and pio controller uses GPIO flags.

I see only same as in the example

https://elixir.bootlin.com/linux/latest/source/arch/arm/boot/dts/mt7623n-bananapi-bpi-r2.dts#L196

>Best regards,
>Krzysztof
regards Frank