From: Florian Fainelli <f.fainelli@gmail.com>
To: Rob Herring <robh+dt@kernel.org>,
Matthias Kaehlcke <mka@chromium.org>,
Andrew Lunn <andrew@lunn.ch>,
Heiner Kallweit <hkallweit1@gmail.com>
Cc: "David S . Miller" <davem@davemloft.net>,
Mark Rutland <mark.rutland@arm.com>,
netdev <netdev@vger.kernel.org>,
devicetree@vger.kernel.org,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
Douglas Anderson <dianders@chromium.org>
Subject: Re: [PATCH v2 6/7] dt-bindings: net: realtek: Add property to configure LED mode
Date: Wed, 10 Jul 2019 09:28:39 -0700 [thread overview]
Message-ID: <7d102d81-750d-32d9-a554-95f018e69f2f@gmail.com> (raw)
In-Reply-To: <CAL_JsqL_AU+JV0c2mNbXiPh2pvfYbPbLV-2PHHX0hC3vUH4QWg@mail.gmail.com>
On 7/10/19 8:55 AM, Rob Herring wrote:
> On Wed, Jul 3, 2019 at 5:23 PM Matthias Kaehlcke <mka@chromium.org> wrote:
>>
>> Hi Florian,
>>
>> On Wed, Jul 03, 2019 at 02:37:47PM -0700, Florian Fainelli wrote:
>>> On 7/3/19 12:37 PM, Matthias Kaehlcke wrote:
>>>> The LED behavior of some Realtek PHYs is configurable. Add the
>>>> property 'realtek,led-modes' to specify the configuration of the
>>>> LEDs.
>>>>
>>>> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
>>>> ---
>>>> Changes in v2:
>>>> - patch added to the series
>>>> ---
>>>> .../devicetree/bindings/net/realtek.txt | 9 +++++++++
>>>> include/dt-bindings/net/realtek.h | 17 +++++++++++++++++
>>>> 2 files changed, 26 insertions(+)
>>>> create mode 100644 include/dt-bindings/net/realtek.h
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/net/realtek.txt b/Documentation/devicetree/bindings/net/realtek.txt
>>>> index 71d386c78269..40b0d6f9ee21 100644
>>>> --- a/Documentation/devicetree/bindings/net/realtek.txt
>>>> +++ b/Documentation/devicetree/bindings/net/realtek.txt
>>>> @@ -9,6 +9,12 @@ Optional properties:
>>>>
>>>> SSC is only available on some Realtek PHYs (e.g. RTL8211E).
>>>>
>>>> +- realtek,led-modes: LED mode configuration.
>>>> +
>>>> + A 0..3 element vector, with each element configuring the operating
>>>> + mode of an LED. Omitted LEDs are turned off. Allowed values are
>>>> + defined in "include/dt-bindings/net/realtek.h".
>>>
>>> This should probably be made more general and we should define LED modes
>>> that makes sense regardless of the PHY device, introduce a set of
>>> generic functions for validating and then add new function pointer for
>>> setting the LED configuration to the PHY driver. This would allow to be
>>> more future proof where each PHY driver could expose standard LEDs class
>>> devices to user-space, and it would also allow facilities like: ethtool
>>> -p to plug into that.
>>>
>>> Right now, each driver invents its own way of configuring LEDs, that
>>> does not scale, and there is not really a good reason for that other
>>> than reviewing drivers in isolation and therefore making it harder to
>>> extract the commonality. Yes, I realize that since you are the latest
>>> person submitting something in that area, you are being selected :)
>
> I agree.
>
>> I see the merit of your proposal to come up with a generic mechanism
>> to configure Ethernet LEDs, however I can't justify spending much of
>> my work time on this. If it is deemed useful I'm happy to send another
>> version of the current patchset that addresses the reviewer's comments,
>> but if the implementation of a generic LED configuration interface is
>> a requirement I will have to abandon at least the LED configuration
>> part of this series.
>
> Can you at least define a common binding for this. Maybe that's just
> removing 'realtek'. While the kernel side can evolve to a common
> infrastructure, the DT bindings can't.
That would be a great start, and that is actually what I had in mind
(should have been more specific), I was not going to have you Matthias
do the grand slam and convert all this LED configuration into the LEDs
class etc. that would not be fair.
It seems to be that we can fairly easily agree on a common binding for
LED configuration, I would define something along those lines to be
flexible:
phy-led-configuration = <LED_NUM_MASK LED_CFG_MASK>;
where LED_NUM_MASK is one of:
0 -> link
1 -> activity
2 -> speed
that way you can define single/dual/triple LED configurations by
updating the bitmask.
LED_CFG_MASK is one of:
0 -> LED_CFG_10
1 -> LED_CFG_100
2 -> LED_CFG_1000
(let's assume 1Gbps or less for now)
or this can be combined in a single cell with a left shift.
Andrew, Heiner, do you see that approach working correctly and scaling
appropriately?
--
Florian
next prev parent reply other threads:[~2019-07-10 16:28 UTC|newest]
Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-07-03 19:37 [PATCH v2 1/7] dt-bindings: net: Add bindings for Realtek PHYs Matthias Kaehlcke
2019-07-03 19:37 ` [PATCH v2 2/7] net: phy: realtek: Allow disabling RTL8211E EEE LED mode Matthias Kaehlcke
2019-07-03 20:09 ` Heiner Kallweit
2019-07-03 20:32 ` Matthias Kaehlcke
2019-07-03 19:37 ` [PATCH v2 3/7] dt-bindings: net: realtek: Add property to enable SSC Matthias Kaehlcke
2019-07-03 19:37 ` [PATCH v2 4/7] net: phy: realtek: Enable accessing RTL8211E extension pages Matthias Kaehlcke
2019-07-03 20:12 ` Heiner Kallweit
2019-07-03 20:36 ` Matthias Kaehlcke
2019-07-03 21:01 ` Heiner Kallweit
2019-07-03 21:24 ` Matthias Kaehlcke
2019-07-03 21:27 ` Heiner Kallweit
2019-07-03 22:56 ` Matthias Kaehlcke
2019-07-03 19:37 ` [PATCH v2 5/7] net: phy: realtek: Support SSC for the RTL8211E Matthias Kaehlcke
2019-07-03 19:37 ` [PATCH v2 6/7] dt-bindings: net: realtek: Add property to configure LED mode Matthias Kaehlcke
2019-07-03 20:07 ` Andrew Lunn
2019-07-03 20:13 ` Heiner Kallweit
2019-07-03 20:22 ` Heiner Kallweit
2019-07-03 21:37 ` Florian Fainelli
2019-07-03 23:23 ` Matthias Kaehlcke
2019-07-10 15:55 ` Rob Herring
2019-07-10 16:28 ` Florian Fainelli [this message]
2019-07-12 17:28 ` Matthias Kaehlcke
2019-07-12 17:20 ` Matthias Kaehlcke
2019-07-22 17:14 ` Matthias Kaehlcke
2019-07-22 19:01 ` Andrew Lunn
2019-07-22 19:14 ` Matthias Kaehlcke
2019-07-22 19:38 ` Andrew Lunn
2019-07-03 19:37 ` [PATCH v2 7/7] net: phy: realtek: configure RTL8211E LEDs Matthias Kaehlcke
2019-07-03 20:10 ` Andrew Lunn
2019-07-03 20:43 ` Matthias Kaehlcke
2019-07-03 20:28 ` Heiner Kallweit
2019-07-03 20:45 ` Matthias Kaehlcke
2019-07-03 20:21 ` [PATCH v2 1/7] dt-bindings: net: Add bindings for Realtek PHYs David Miller
2019-07-03 21:11 ` Rob Herring
2019-07-03 21:33 ` Andrew Lunn
2019-07-03 22:08 ` Matthias Kaehlcke
2019-07-05 16:18 ` Rob Herring
2019-07-05 16:17 ` Rob Herring
2019-07-05 16:29 ` Andrew Lunn
2019-07-05 17:07 ` Rob Herring
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=7d102d81-750d-32d9-a554-95f018e69f2f@gmail.com \
--to=f.fainelli@gmail.com \
--cc=andrew@lunn.ch \
--cc=davem@davemloft.net \
--cc=devicetree@vger.kernel.org \
--cc=dianders@chromium.org \
--cc=hkallweit1@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=mka@chromium.org \
--cc=netdev@vger.kernel.org \
--cc=robh+dt@kernel.org \
/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).