[PATCH] dt-bindings:leds: Add function define for POE

Tony O'Brien posted 1 patch 10 months, 1 week ago
include/dt-bindings/leds/common.h | 1 +
1 file changed, 1 insertion(+)
[PATCH] dt-bindings:leds: Add function define for POE
Posted by Tony O'Brien 10 months, 1 week ago
Add a define so network devices with a PoE PSE feature can provide
status indications of connected PoE PDs.

Reviewed-by: Ben Hoelker <ben.hoelker@alliedtelesis.co.nz>
Reviewed-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
Signed-off-by: Tony O'Brien <tony.obrien@alliedtelesis.co.nz>
---
 include/dt-bindings/leds/common.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/dt-bindings/leds/common.h b/include/dt-bindings/leds/common.h
index 4f017bea0123..3a8958b81c87 100644
--- a/include/dt-bindings/leds/common.h
+++ b/include/dt-bindings/leds/common.h
@@ -94,6 +94,7 @@
 #define LED_FUNCTION_MOBILE "mobile"
 #define LED_FUNCTION_MTD "mtd"
 #define LED_FUNCTION_PANIC "panic"
+#define LED_FUNCTION_POE "poe"
 #define LED_FUNCTION_PROGRAMMING "programming"
 #define LED_FUNCTION_RX "rx"
 #define LED_FUNCTION_SD "sd"
-- 
2.48.1
Re: [PATCH] dt-bindings:leds: Add function define for POE
Posted by Krzysztof Kozlowski 10 months, 1 week ago
On Thu, Feb 13, 2025 at 01:58:41PM +1300, Tony O'Brien wrote:
> Add a define so network devices with a PoE PSE feature can provide
> status indications of connected PoE PDs.
> 
> Reviewed-by: Ben Hoelker <ben.hoelker@alliedtelesis.co.nz>
> Reviewed-by: Chris Packham <chris.packham@alliedtelesis.co.nz>

Where did these two reviews happen?

> Signed-off-by: Tony O'Brien <tony.obrien@alliedtelesis.co.nz>
> ---
>  include/dt-bindings/leds/common.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/include/dt-bindings/leds/common.h b/include/dt-bindings/leds/common.h
> index 4f017bea0123..3a8958b81c87 100644
> --- a/include/dt-bindings/leds/common.h
> +++ b/include/dt-bindings/leds/common.h
> @@ -94,6 +94,7 @@
>  #define LED_FUNCTION_MOBILE "mobile"
>  #define LED_FUNCTION_MTD "mtd"
>  #define LED_FUNCTION_PANIC "panic"
> +#define LED_FUNCTION_POE "poe"

Where is any user of this?

Best regards,
Krzysztof
Re: [PATCH] dt-bindings:leds: Add function define for POE
Posted by Tony O'Brien 10 months ago
Hi Krzysztof -

> Where did these two reviews happen?
They were in-house reviews.  Please feel free to remove them from the
patch.

> Where is any user of this?
We are adding Kernel control of PoE LEDs and thought this might be
useful to others, maybe those working on netdev, or anyone implementing
PoE on their devices.  Also, the Kernel >> Docs >> LEDs page states:
"If required color or function is missing, please submit a patch to
linux-leds@vger.kernel.org", which is included here.

Cheers,
Tony O'Brien

On Thu, 2025-02-13 at 10:31 +0100, Krzysztof Kozlowski wrote:
> On Thu, Feb 13, 2025 at 01:58:41PM +1300, Tony O'Brien wrote:
> > Add a define so network devices with a PoE PSE feature can provide
> > status indications of connected PoE PDs.
> > 
> > Reviewed-by: Ben Hoelker <ben.hoelker@alliedtelesis.co.nz>
> > Reviewed-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> 
> Where did these two reviews happen?
> 
> > Signed-off-by: Tony O'Brien <tony.obrien@alliedtelesis.co.nz>
> > ---
> >  include/dt-bindings/leds/common.h | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/include/dt-bindings/leds/common.h b/include/dt-
> > bindings/leds/common.h
> > index 4f017bea0123..3a8958b81c87 100644
> > --- a/include/dt-bindings/leds/common.h
> > +++ b/include/dt-bindings/leds/common.h
> > @@ -94,6 +94,7 @@
> >  #define LED_FUNCTION_MOBILE "mobile"
> >  #define LED_FUNCTION_MTD "mtd"
> >  #define LED_FUNCTION_PANIC "panic"
> > +#define LED_FUNCTION_POE "poe"
> 
> Where is any user of this?
> 
> Best regards,
> Krzysztof
> 

Re: [PATCH] dt-bindings:leds: Add function define for POE
Posted by Krzysztof Kozlowski 10 months ago
On 16/02/2025 22:37, Tony O'Brien wrote:
> Hi Krzysztof -
> 
>> Where did these two reviews happen?
> They were in-house reviews.  Please feel free to remove them from the
> patch.
> 
>> Where is any user of this?
> We are adding Kernel control of PoE LEDs and thought this might be
> useful to others, maybe those working on netdev, or anyone implementing
> PoE on their devices.  Also, the Kernel >> Docs >> LEDs page states:
> "If required color or function is missing, please submit a patch to
> linux-leds@vger.kernel.org", which is included here.
You did not answer: where is the user? By "we are adding" you mean
downstream?

Best regards,
Krzysztof
Re: [PATCH] dt-bindings:leds: Add function define for POE
Posted by Chris Packham 9 months, 4 weeks ago
Hi Krzysztof,

On 17/02/2025 22:13, Krzysztof Kozlowski wrote:
> On 16/02/2025 22:37, Tony O'Brien wrote:
>> Hi Krzysztof -
>>
>>> Where did these two reviews happen?
>> They were in-house reviews.  Please feel free to remove them from the
>> patch.
>>
>>> Where is any user of this?
>> We are adding Kernel control of PoE LEDs and thought this might be
>> useful to others, maybe those working on netdev, or anyone implementing
>> PoE on their devices.  Also, the Kernel >> Docs >> LEDs page states:
>> "If required color or function is missing, please submit a patch to
>> linux-leds@vger.kernel.org", which is included here.
> You did not answer: where is the user? By "we are adding" you mean
> downstream?

It's downstream. One of our PoE switches in development which uses a 
pca9552 LED controller. I suggested to Tony that we get this upstream as 
I know there is some work going on to support PoE PSEs in netdev. I did 
wonder if we wanted to make this more specific i.e. have "pse" and "pd" 
as different functions but something like "poe" seemed fine as you're 
not going to have something that is both a PSE and a PD on the same port.

Re: [PATCH] dt-bindings:leds: Add function define for POE
Posted by Krzysztof Kozlowski 9 months, 4 weeks ago
On 23/02/2025 21:54, Chris Packham wrote:
> Hi Krzysztof,
> 
> On 17/02/2025 22:13, Krzysztof Kozlowski wrote:
>> On 16/02/2025 22:37, Tony O'Brien wrote:
>>> Hi Krzysztof -
>>>
>>>> Where did these two reviews happen?
>>> They were in-house reviews.  Please feel free to remove them from the
>>> patch.
>>>
>>>> Where is any user of this?
>>> We are adding Kernel control of PoE LEDs and thought this might be
>>> useful to others, maybe those working on netdev, or anyone implementing
>>> PoE on their devices.  Also, the Kernel >> Docs >> LEDs page states:
>>> "If required color or function is missing, please submit a patch to
>>> linux-leds@vger.kernel.org", which is included here.
>> You did not answer: where is the user? By "we are adding" you mean
>> downstream?
> 
> It's downstream. One of our PoE switches in development which uses a 


So for me that's a no. We don't care about downstream. Otherwise we
would need to accept whatever else people invented and never bothered
with upstreaming.

> pca9552 LED controller. I suggested to Tony that we get this upstream as 
> I know there is some work going on to support PoE PSEs in netdev. I did 
> wonder if we wanted to make this more specific i.e. have "pse" and "pd" 
> as different functions but something like "poe" seemed fine as you're 
> not going to have something that is both a PSE and a PD on the same port.

To me, this is not a catalog of all possible LED functions. Come with
any sort of user - either driver or DTS (and I still remember discussion
with Hans de Goede about taking such patches without DTS user, but that
was under condition there is driver user).

Best regards,
Krzysztof
Re: [PATCH] dt-bindings:leds: Add function define for POE
Posted by Chris Packham 9 months, 4 weeks ago
Hi Krzysztof,

On 24/02/2025 21:11, Krzysztof Kozlowski wrote:
> On 23/02/2025 21:54, Chris Packham wrote:
>> Hi Krzysztof,
>>
>> On 17/02/2025 22:13, Krzysztof Kozlowski wrote:
>>> On 16/02/2025 22:37, Tony O'Brien wrote:
>>>> Hi Krzysztof -
>>>>
>>>>> Where did these two reviews happen?
>>>> They were in-house reviews.  Please feel free to remove them from the
>>>> patch.
>>>>
>>>>> Where is any user of this?
>>>> We are adding Kernel control of PoE LEDs and thought this might be
>>>> useful to others, maybe those working on netdev, or anyone implementing
>>>> PoE on their devices.  Also, the Kernel >> Docs >> LEDs page states:
>>>> "If required color or function is missing, please submit a patch to
>>>> linux-leds@vger.kernel.org", which is included here.
>>> You did not answer: where is the user? By "we are adding" you mean
>>> downstream?
>> It's downstream. One of our PoE switches in development which uses a
>
> So for me that's a no. We don't care about downstream. Otherwise we
> would need to accept whatever else people invented and never bothered
> with upstreaming.
>
>> pca9552 LED controller. I suggested to Tony that we get this upstream as
>> I know there is some work going on to support PoE PSEs in netdev. I did
>> wonder if we wanted to make this more specific i.e. have "pse" and "pd"
>> as different functions but something like "poe" seemed fine as you're
>> not going to have something that is both a PSE and a PD on the same port.
> To me, this is not a catalog of all possible LED functions. Come with
> any sort of user - either driver or DTS (and I still remember discussion
> with Hans de Goede about taking such patches without DTS user, but that
> was under condition there is driver user).

OK fair enough.

I do plan on getting more of our boards landed upstream. There's a few 
CN9130 based ones that should be trivial to do (one of which is the user 
of this LED function). Until then we can carry a local definition of 
that function.