Re: [PATCH v2] dt-bindings: net: marvell,pp2: convert to json-schema
From: Marcin Wojtas <hidden>
Date: 2022-10-03 17:24:00
Also in:
linux-devicetree, lkml
pon., 3 paź 2022 o 19:06 Michał Grzelak [off-list ref] napisał(a):
On 02/10/2022 10:23, Marcin Wojtas wrote:quoted
niedz., 2 paź 2022 o 10:00 Krzysztof Kozlowski [off-list ref] napisał(a):quoted
On 01/10/2022 17:53, Michał Grzelak wrote:quoted
Hi Krzysztof, Thanks for your comments and time spent on reviewing my patch. All of those improvements will be included in next version. Also, I would like to know your opinion about one.quoted
quoted
+ + marvell,system-controller: + $ref: /schemas/types.yaml#/definitions/phandle + description: a phandle to the system controller. + +patternProperties: + '^eth[0-9a-f]*(@.*)?$':The name should be "(ethernet-)?port", unless anything depends on particular naming?What do you think about pattern "^(ethernet-)?eth[0-9a-f]+(@.*)?$"? It resembles pattern found in net/ethernet-phy.yaml like properties:$nodename:pattern:"^ethernet-phy(@[a-f0-9]+)?$", while still passing `dt_binding_check' and `dtbs_check'. It should also comply with your comment.Node names like ethernet-eth do not make much sense because they contain redundant ethernet or eth. AFAIK, all other bindings like that call these ethernet-ports (or sometimes shorter - ports). Unless this device is different than all others?IMO "^(ethernet-)?port@[0-9]+$" for the subnodes' names could be fine (as long as we don't have to modify the existing .dtsi files) - there is no dependency in the driver code on that.Indeed, driver's code isn't dependent; however, there is a dependency on 'eth[0-2]' name in all relevant .dts and .dtsi files, e.g.: https://github.com/torvalds/linux/blob/master/arch/arm/boot/dts/armada-375.dtsi#L190 https://github.com/torvalds/linux/blob/master/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi#L72 Ports under 'ethernet' node are named eth[0-2], thus those and all .dts files including the above would have to be modified to pass through `dtbs_check'.
Can you please double check? The .dts files use labels, the node name they relate to should be irrelevant: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/marvell/cn9130-db.dtsi?h=v6.0#n122 (BTW, for A7k8k/CN913x example please use updated names, i.e. s/cpm_/cp0_/ and s/cps_/cp1_/) https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/armada-375-db.dts?h=v6.0#n167 Best regards, Marcin