pon., 3 paź 2022 o 19:06 Michał Grzelak <mig@xxxxxxxxxxxx> napisał(a): > > On 02/10/2022 10:23, Marcin Wojtas wrote: > >niedz., 2 paź 2022 o 10:00 Krzysztof Kozlowski > ><krzysztof.kozlowski@xxxxxxxxxx> napisał(a): > >> > >> On 01/10/2022 17:53, Michał Grzelak wrote: > >> > 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. > >> > > >> >>> + > >> >>> + 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
