From: Rob Herring <robh@kernel.org>
To: Christian Marangi <ansuelsmth@gmail.com>
Cc: Andrew Lunn <andrew@lunn.ch>,
Florian Fainelli <f.fainelli@gmail.com>,
Vladimir Oltean <olteanv@gmail.com>,
"David S. Miller" <davem@davemloft.net>,
Eric Dumazet <edumazet@google.com>,
Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
Heiner Kallweit <hkallweit1@gmail.com>,
Russell King <linux@armlinux.org.uk>,
Gregory Clement <gregory.clement@bootlin.com>,
Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>,
Andy Gross <agross@kernel.org>,
Bjorn Andersson <andersson@kernel.org>,
Konrad Dybcio <konrad.dybcio@linaro.org>,
Pavel Machek <pavel@ucw.cz>, Lee Jones <lee@kernel.org>,
John Crispin <john@phrozen.org>,
netdev@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-arm-msm@vger.kernel.org, linux-leds@vger.kernel.org
Subject: Re: [net-next PATCH v5 10/15] dt-bindings: net: ethernet-controller: Document support for LEDs node
Date: Fri, 24 Mar 2023 17:06:41 -0500 [thread overview]
Message-ID: <20230324220641.GA54972-robh@kernel.org> (raw)
In-Reply-To: <641a35b8.1c0a0220.25419.2b4d@mx.google.com>
On Tue, Mar 21, 2023 at 11:54:46PM +0100, Christian Marangi wrote:
> On Tue, Mar 21, 2023 at 04:19:53PM -0500, Rob Herring wrote:
> > On Sun, Mar 19, 2023 at 08:18:09PM +0100, Christian Marangi wrote:
> > > Document support for LEDs node in ethernet-controller.
> > > Ethernet Controller may support different LEDs that can be configured
> > > for different operation like blinking on traffic event or port link.
> > >
> > > Also add some Documentation to describe the difference of these nodes
> > > compared to PHY LEDs, since ethernet-controller LEDs are controllable
> > > by the ethernet controller regs and the possible intergated PHY doesn't
> > > have control on them.
> > >
> > > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> > > ---
> > > .../bindings/net/ethernet-controller.yaml | 21 +++++++++++++++++++
> > > 1 file changed, 21 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/net/ethernet-controller.yaml b/Documentation/devicetree/bindings/net/ethernet-controller.yaml
> > > index 00be387984ac..a93673592314 100644
> > > --- a/Documentation/devicetree/bindings/net/ethernet-controller.yaml
> > > +++ b/Documentation/devicetree/bindings/net/ethernet-controller.yaml
> > > @@ -222,6 +222,27 @@ properties:
> > > required:
> > > - speed
> > >
> > > + leds:
> > > + type: object
> > > + description:
> > > + Describes the LEDs associated by Ethernet Controller.
> > > + These LEDs are not integrated in the PHY and PHY doesn't have any
> > > + control on them. Ethernet Controller regs are used to control
> > > + these defined LEDs.
> > > +
> > > + properties:
> > > + '#address-cells':
> > > + const: 1
> > > +
> > > + '#size-cells':
> > > + const: 0
> > > +
> > > + patternProperties:
> > > + '^led(@[a-f0-9]+)?$':
> > > + $ref: /schemas/leds/common.yaml#
> >
> > Are specific ethernet controllers allowed to add their own properties in
> > led nodes? If so, this doesn't work. As-is, this allows any other
> > properties. You need 'unevaluatedProperties: false' here to prevent
> > that. But then no one can add properties. If you want to support that,
> > then you need this to be a separate schema that devices can optionally
> > include if they don't extend the properties, and then devices that
> > extend the binding would essentially have the above with:
> >
> > $ref: /schemas/leds/common.yaml#
> > unevaluatedProperties: false
> > properties:
> > a-custom-device-prop: ...
> >
> >
> > If you wanted to define both common ethernet LED properties and
> > device specific properties, then you'd need to replace leds/common.yaml
> > above with the ethernet one.
> >
> > This is all the same reasons the DSA/switch stuff and graph bindings are
> > structured the way they are.
> >
>
> Hi Rob, thanks for the review/questions.
>
> The idea of all of this is to keep leds node as standard as possible.
> It was asked to add unevaluatedProperties: False but I didn't understood
> it was needed also for the led nodes.
>
> leds/common.yaml have additionalProperties set to true but I guess that
> is not OK for the final schema and we need something more specific.
Yes, every node needs a schema with all possible properties and then
'unevaluatedProperties: false' to not allow any other properties.
> Looking at the common.yaml schema reg binding is missing so an
> additional schema is needed.
>
> Reg is needed for ethernet LEDs and PHY but I think we should also permit
> to skip that if the device actually have just one LED. (if this wouldn't
> complicate the implementation. Maybe some hints from Andrew about this
> decision?)
>
> If we decide that reg is a must, if I understood it correctly we should
> create something like leds-ethernet.yaml that would reference common and
> add reg binding? Is it correct? This schema should be laded in leds
> directory and not in the net/ethernet.
You need 'reg' in properties, but whether it is required or not just
depends on putting it in 'required'. I don't have a strong opinion on
that, but generally it's only use 'reg' when there's more than 1.
Rob
next prev parent reply other threads:[~2023-03-24 22:06 UTC|newest]
Thread overview: 48+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-03-19 19:17 [net-next PATCH v5 00/15] net: Add basic LED support for switch/phy Christian Marangi
2023-03-19 19:18 ` [net-next PATCH v5 01/15] net: dsa: qca8k: move qca8k_port_to_phy() to header Christian Marangi
2023-03-20 16:32 ` Michal Kubiak
2023-03-19 19:18 ` [net-next PATCH v5 02/15] net: dsa: qca8k: add LEDs basic support Christian Marangi
2023-03-20 16:30 ` Michal Kubiak
2023-03-20 16:33 ` Christian Marangi
2023-03-20 17:48 ` Michal Kubiak
2023-03-20 18:05 ` Christian Marangi
2023-03-19 19:18 ` [net-next PATCH v5 03/15] net: dsa: qca8k: add LEDs blink_set() support Christian Marangi
2023-03-23 11:55 ` Pavel Machek
2023-03-19 19:18 ` [net-next PATCH v5 04/15] leds: Provide stubs for when CLASS_LED is disabled Christian Marangi
2023-03-19 22:49 ` Andrew Lunn
2023-03-20 17:52 ` Christian Marangi
2023-03-20 19:31 ` Andrew Lunn
2023-03-21 7:55 ` Christian Marangi
2023-03-21 15:58 ` Andrew Lunn
2023-03-21 16:02 ` Andrew Lunn
2023-03-21 16:13 ` Christian Marangi
2023-03-19 19:18 ` [net-next PATCH v5 05/15] net: phy: Add a binding for PHY LEDs Christian Marangi
2023-03-20 16:34 ` Michal Kubiak
2023-03-19 19:18 ` [net-next PATCH v5 06/15] net: phy: phy_device: Call into the PHY driver to set LED brightness Christian Marangi
2023-03-20 16:36 ` Michal Kubiak
2023-03-19 19:18 ` [net-next PATCH v5 07/15] net: phy: marvell: Add software control of the LEDs Christian Marangi
2023-03-23 11:55 ` Pavel Machek
2023-03-19 19:18 ` [net-next PATCH v5 08/15] net: phy: phy_device: Call into the PHY driver to set LED blinking Christian Marangi
2023-03-23 11:56 ` Pavel Machek
2023-03-19 19:18 ` [net-next PATCH v5 09/15] net: phy: marvell: Implement led_blink_set() Christian Marangi
2023-03-23 11:57 ` Pavel Machek
2023-03-19 19:18 ` [net-next PATCH v5 10/15] dt-bindings: net: ethernet-controller: Document support for LEDs node Christian Marangi
2023-03-21 21:19 ` Rob Herring
2023-03-21 22:54 ` Christian Marangi
2023-03-21 23:23 ` Andrew Lunn
2023-03-21 23:39 ` Christian Marangi
2023-03-24 21:59 ` Rob Herring
2023-03-24 22:06 ` Rob Herring [this message]
2023-03-19 19:18 ` [net-next PATCH v5 11/15] dt-bindings: net: dsa: qca8k: add LEDs definition example Christian Marangi
2023-03-21 21:27 ` Rob Herring
2023-03-19 19:18 ` [net-next PATCH v5 12/15] arm: qcom: dt: Drop unevaluated properties in switch nodes for rb3011 Christian Marangi
2023-03-24 8:54 ` Krzysztof Kozlowski
2023-03-19 19:18 ` [net-next PATCH v5 13/15] arm: qcom: dt: Add Switch LED for each port " Christian Marangi
2023-03-23 12:00 ` Pavel Machek
2023-03-19 19:18 ` [net-next PATCH v5 14/15] dt-bindings: net: phy: Document support for LEDs node Christian Marangi
2023-03-21 21:29 ` Rob Herring
2023-03-19 19:18 ` [net-next PATCH v5 15/15] arm: mvebu: dt: Add PHY LED support for 370-rd WAN port Christian Marangi
2023-03-23 12:04 ` Pavel Machek
2023-03-23 17:02 ` Andrew Lunn
2023-03-23 19:11 ` Pavel Machek
2023-03-23 19:53 ` 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=20230324220641.GA54972-robh@kernel.org \
--to=robh@kernel.org \
--cc=agross@kernel.org \
--cc=andersson@kernel.org \
--cc=andrew@lunn.ch \
--cc=ansuelsmth@gmail.com \
--cc=davem@davemloft.net \
--cc=devicetree@vger.kernel.org \
--cc=edumazet@google.com \
--cc=f.fainelli@gmail.com \
--cc=gregory.clement@bootlin.com \
--cc=hkallweit1@gmail.com \
--cc=john@phrozen.org \
--cc=konrad.dybcio@linaro.org \
--cc=krzysztof.kozlowski+dt@linaro.org \
--cc=kuba@kernel.org \
--cc=lee@kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-leds@vger.kernel.org \
--cc=linux@armlinux.org.uk \
--cc=netdev@vger.kernel.org \
--cc=olteanv@gmail.com \
--cc=pabeni@redhat.com \
--cc=pavel@ucw.cz \
--cc=sebastian.hesselbarth@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).