From: Kurt Kanzenbach <kurt@linutronix.de>
To: Rob Herring <robh@kernel.org>, Florian Fainelli <f.fainelli@gmail.com>
Cc: Andrew Lunn <andrew@lunn.ch>,
Vivien Didelot <vivien.didelot@gmail.com>,
"David S. Miller" <davem@davemloft.net>,
Jakub Kicinski <kuba@kernel.org>, netdev <netdev@vger.kernel.org>,
devicetree@vger.kernel.org
Subject: Re: [PATCH v1 1/1] dt-bindings: net: dsa: Add DSA yaml binding
Date: Sat, 11 Jul 2020 13:59:33 +0200 [thread overview]
Message-ID: <871rliw9cq.fsf@kurt> (raw)
In-Reply-To: <CAL_Jsq+zP9++MftM+Dh2Fe-OdKq6EiGA_tASEbBwA_jEdwoFCA@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 5642 bytes --]
Hi,
On Fri Jul 10 2020, Rob Herring wrote:
> On Fri, Jul 10, 2020 at 11:20 AM Florian Fainelli <f.fainelli@gmail.com> 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 <f.fainelli@gmail.com>
>> >> Signed-off-by: Kurt Kanzenbach <kurt@linutronix.de>
>> >> ---
>> >> .../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 <andrew@lunn.ch>
>> >> + - Florian Fainelli <f.fainelli@gmail.com>
>> >> + - Vivien Didelot <vivien.didelot@gmail.com>
>> >> +
>> >> +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 "<prop>: 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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
next prev parent reply other threads:[~2020-07-11 11:59 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-10 9:06 [PATCH v1 0/1] dt-bindings: net: dsa: Add DSA yaml binding Kurt Kanzenbach
2020-07-10 9:06 ` [PATCH v1 1/1] " Kurt Kanzenbach
2020-07-10 16:39 ` Rob Herring
2020-07-11 11:35 ` Kurt Kanzenbach
2020-07-11 16:52 ` Andrew Lunn
2020-07-12 10:29 ` Kurt Kanzenbach
2020-07-13 20:56 ` Rob Herring
2020-07-10 16:45 ` Rob Herring
2020-07-10 17:20 ` Florian Fainelli
2020-07-10 19:38 ` Rob Herring
2020-07-11 11:59 ` Kurt Kanzenbach [this message]
2020-07-11 16:42 ` Andrew Lunn
2020-07-13 20:41 ` Rob Herring
2020-07-14 6:18 ` Kurt Kanzenbach
2020-07-10 17:39 ` Andrew Lunn
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=871rliw9cq.fsf@kurt \
--to=kurt@linutronix.de \
--cc=andrew@lunn.ch \
--cc=davem@davemloft.net \
--cc=devicetree@vger.kernel.org \
--cc=f.fainelli@gmail.com \
--cc=kuba@kernel.org \
--cc=netdev@vger.kernel.org \
--cc=robh@kernel.org \
--cc=vivien.didelot@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).