Hi, On Fri Jul 10 2020, Rob Herring wrote: > On Fri, Jul 10, 2020 at 11:20 AM Florian Fainelli wrote: >> >> >> >> On 7/10/2020 9:45 AM, Rob Herring wrote: >> > On Fri, Jul 10, 2020 at 11:06:18AM +0200, Kurt Kanzenbach wrote: >> >> For future DSA drivers it makes sense to add a generic DSA yaml binding which >> >> can be used then. This was created using the properties from dsa.txt. It >> >> includes the ports and the dsa,member property. >> >> >> >> Suggested-by: Florian Fainelli >> >> Signed-off-by: Kurt Kanzenbach >> >> --- >> >> .../devicetree/bindings/net/dsa/dsa.yaml | 80 +++++++++++++++++++ >> >> 1 file changed, 80 insertions(+) >> >> create mode 100644 Documentation/devicetree/bindings/net/dsa/dsa.yaml >> >> >> >> diff --git a/Documentation/devicetree/bindings/net/dsa/dsa.yaml b/Documentation/devicetree/bindings/net/dsa/dsa.yaml >> >> new file mode 100644 >> >> index 000000000000..bec257231bf8 >> >> --- /dev/null >> >> +++ b/Documentation/devicetree/bindings/net/dsa/dsa.yaml >> >> @@ -0,0 +1,80 @@ >> >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >> >> +%YAML 1.2 >> >> +--- >> >> +$id: http://devicetree.org/schemas/net/dsa/dsa.yaml# >> >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> >> + >> >> +title: Distributed Switch Architecture Device Tree Bindings >> > >> > DSA is a Linuxism, right? >> >> Not really, it is a Marvell term that describes their proprietary >> switching protocol. Since then DSA within Linux expands well beyond just >> Marvell switches, so the terms have been blurred a little bit. > > Either way, sounds like the terminology here should be more general. How? > > Though I missed that this is really just a conversion of dsa.txt which > should be removed in this patch. Otherwise, you'll get me re-reviewing > the binding. Yes, it's a conversion of the dsa.txt. I should have stated that more clearly. I didn't remove the .txt file, because it's referenced in all the different switch bindings such as b53.txt, ksz.txt and so on. How to handle that? > >> >> + >> >> +maintainers: >> >> + - Andrew Lunn >> >> + - Florian Fainelli >> >> + - Vivien Didelot >> >> + >> >> +description: >> >> + Switches are true Linux devices and can be probed by any means. Once probed, >> > >> > Bindings are OS independent. OK. >> > >> >> + they register to the DSA framework, passing a node pointer. This node is >> >> + expected to fulfil the following binding, and may contain additional >> >> + properties as required by the device it is embedded within. >> > >> > Describe what type of h/w should use this binding. I took the description from the dsa.txt. However, it makes sense to adjust that description. Basically all Ethernet switches with a dedicated CPU port should use DSA and this binding. >> > >> >> + >> >> +properties: >> >> + $nodename: >> >> + pattern: "^switch(@.*)?$" >> >> + >> >> + dsa,member: >> >> + minItems: 2 >> >> + maxItems: 2 >> >> + description: >> >> + A two element list indicates which DSA cluster, and position within the >> >> + cluster a switch takes. <0 0> is cluster 0, switch 0. <0 1> is cluster 0, >> >> + switch 1. <1 0> is cluster 1, switch 0. A switch not part of any cluster >> >> + (single device hanging off a CPU port) must not specify this property >> >> + $ref: /schemas/types.yaml#/definitions/uint32-array >> >> + >> >> + ports: >> >> + type: object >> >> + properties: >> >> + '#address-cells': >> >> + const: 1 >> >> + '#size-cells': >> >> + const: 0 >> >> + >> >> + patternProperties: >> >> + "^port@[0-9]+$": >> > >> > As ports and port are OF graph nodes, it would be better if we >> > standardized on a different name for these. I think we've used >> > 'ethernet-port' some. >> >> Yes we did talk about that before, however when the original DSA binding >> was introduced about 7 years ago (or maybe more recently, my memory >> fails me now), "ports" was chosen as the encapsulating node. We should >> be accepting both ethernet-ports and ports. > > Yes, I'm aware of the history. Back then it was a free-for-all on node > names. Now we're trying to be more disciplined. Ideally, we pick > something unique to standardize on and fix the dts files to match as > long as the node name is generally a don't care for the OS. > > The schema says only port/ports is allowed, Yes, it does. > so at a minimum > ethernet-port/ethernet-ports needs to be added here. Just to be sure. Instead of ports { port@1 { ... } } The following should be possible as well? ethernet-ports { port@1 { ... } } Is there an easy way to add that alternative to the schema? Or does the ethernet-ports property has to be defined as well? > >> >> > >> >> + type: object >> >> + description: DSA switch ports >> >> + >> >> + allOf: >> >> + - $ref: ../ethernet-controller.yaml# >> > >> > How does this and 'ethernet' both apply? >> >> I think the intent here was to mean that some of the properties from the >> Ethernet controller such as phy-mode, phy-handle, fixed-link also apply >> here since the switch port is a simplified Ethernet MAC on a number of >> counts. > > Okay, it's good to explicitly define which of those apply as I imagine > some don't. Just need ": true" to do that. Yes, that was my intent. Only a few properties from the Ethernet controller are needed. I'll add them like you suggested. > > Rob Thanks, Kurt