diff mbox series

[1/2] dt-bindings: leds: register-bit-led: Add active-low property

Message ID 20220818172528.23062-1-pali@kernel.org
State Not Applicable, archived
Headers show
Series [1/2] dt-bindings: leds: register-bit-led: Add active-low property | expand

Checks

Context Check Description
robh/checkpatch success
robh/patch-applied success
robh/dtbs-check warning build log
robh/dt-meta-schema success

Commit Message

Pali Rohár Aug. 18, 2022, 5:25 p.m. UTC
Allow to define inverted logic (0 - enable LED, 1 - disable LED) via
active-low property.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 Documentation/devicetree/bindings/leds/register-bit-led.yaml | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Krzysztof Kozlowski Aug. 19, 2022, 6:46 a.m. UTC | #1
On 18/08/2022 20:25, Pali Rohár wrote:
> Allow to define inverted logic (0 - enable LED, 1 - disable LED) via
> active-low property.
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>
> ---
>  Documentation/devicetree/bindings/leds/register-bit-led.yaml | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/leds/register-bit-led.yaml b/Documentation/devicetree/bindings/leds/register-bit-led.yaml
> index 79b8fc0f9d23..5c6ef26f1a94 100644
> --- a/Documentation/devicetree/bindings/leds/register-bit-led.yaml
> +++ b/Documentation/devicetree/bindings/leds/register-bit-led.yaml
> @@ -43,6 +43,11 @@ properties:
>          0x100000, 0x200000, 0x400000, 0x800000, 0x1000000, 0x2000000, 0x4000000,
>          0x8000000, 0x10000000, 0x20000000, 0x40000000, 0x80000000 ]
>  
> +  active-low:
> +    $ref: /schemas/types.yaml#/definitions/flag

This could be shorter: type: boolean

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Although the question is - where is the user of it?

Best regards,
Krzysztof
Pali Rohár Aug. 19, 2022, 6:56 a.m. UTC | #2
On Friday 19 August 2022 09:46:56 Krzysztof Kozlowski wrote:
> Although the question is - where is the user of it?

I was planning to send updated powerpc DTS files with these
register-bit-led definitions after accepting dt bindings.
Pavel Machek Aug. 19, 2022, 8:08 a.m. UTC | #3
Hi!

> > Although the question is - where is the user of it?
> 
> I was planning to send updated powerpc DTS files with these
> register-bit-led definitions after accepting dt bindings.

We need device tree people to ack them, first. But a note saying "this
is for Turris Omnia router" would be welcome.

Best regards,
								Pavel
Linus Walleij Aug. 19, 2022, 8:35 a.m. UTC | #4
On Thu, Aug 18, 2022 at 7:25 PM Pali Rohár <pali@kernel.org> wrote:

> Allow to define inverted logic (0 - enable LED, 1 - disable LED) via
> active-low property.
>
> Signed-off-by: Pali Rohár <pali@kernel.org>

This looks helpful and solves a real-world problem.
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij
Krzysztof Kozlowski Aug. 19, 2022, 8:38 a.m. UTC | #5
On 19/08/2022 11:08, Pavel Machek wrote:
> Hi!
> 
>>> Although the question is - where is the user of it?
>>
>> I was planning to send updated powerpc DTS files with these
>> register-bit-led definitions after accepting dt bindings.
> 
> We need device tree people to ack them, first. But a note saying "this
> is for Turris Omnia router" would be welcome.

In general the process is one of:
1. Send DT bindings with driver and DTS changes,
2. Send DT bindings with driver in one patchset, DTS in second but you
mention the dependency.

You should not wait with DTS till bindings got accepted. Why? Because
for example we do not want bindings for stuff which never is going to be
upstreamed (with several exceptions, e.g. for other systems). Also
because we want to be able to compare bindings with your DTS
implementing them, so we are sure you described everything (especially
that you said running one command to install dtchema and second command
to make the check is not possible in your system).

Without DTS here how can anyone be sure your DTS actually follows the
bindings?

Best regards,
Krzysztof
Pali Rohár Aug. 19, 2022, 8:42 a.m. UTC | #6
On Friday 19 August 2022 11:38:29 Krzysztof Kozlowski wrote:
> On 19/08/2022 11:08, Pavel Machek wrote:
> > Hi!
> > 
> >>> Although the question is - where is the user of it?
> >>
> >> I was planning to send updated powerpc DTS files with these
> >> register-bit-led definitions after accepting dt bindings.
> > 
> > We need device tree people to ack them, first. But a note saying "this
> > is for Turris Omnia router" would be welcome.
> 
> In general the process is one of:
> 1. Send DT bindings with driver and DTS changes,
> 2. Send DT bindings with driver in one patchset, DTS in second but you
> mention the dependency.
> 
> You should not wait with DTS till bindings got accepted. Why? Because
> for example we do not want bindings for stuff which never is going to be
> upstreamed (with several exceptions, e.g. for other systems). Also
> because we want to be able to compare bindings with your DTS
> implementing them, so we are sure you described everything (especially
> that you said running one command to install dtchema and second command
> to make the check is not possible in your system).
> 
> Without DTS here how can anyone be sure your DTS actually follows the
> bindings?
> 
> Best regards,
> Krzysztof

Well, last time I was told that first needs to be accepted bindings
documentation and then device tree files. So I did it like this. And now
it is again feasible and different steps and ordering is needed...
Sorry I cannot known all requirements which are moreover changing every
day.
Krzysztof Kozlowski Aug. 19, 2022, 9:08 a.m. UTC | #7
On 19/08/2022 11:42, Pali Rohár wrote:
> On Friday 19 August 2022 11:38:29 Krzysztof Kozlowski wrote:
>> On 19/08/2022 11:08, Pavel Machek wrote:
>>> Hi!
>>>
>>>>> Although the question is - where is the user of it?
>>>>
>>>> I was planning to send updated powerpc DTS files with these
>>>> register-bit-led definitions after accepting dt bindings.
>>>
>>> We need device tree people to ack them, first. But a note saying "this
>>> is for Turris Omnia router" would be welcome.
>>
>> In general the process is one of:
>> 1. Send DT bindings with driver and DTS changes,
>> 2. Send DT bindings with driver in one patchset, DTS in second but you
>> mention the dependency.
>>
>> You should not wait with DTS till bindings got accepted. Why? Because
>> for example we do not want bindings for stuff which never is going to be
>> upstreamed (with several exceptions, e.g. for other systems). Also
>> because we want to be able to compare bindings with your DTS
>> implementing them, so we are sure you described everything (especially
>> that you said running one command to install dtchema and second command
>> to make the check is not possible in your system).
>>
>> Without DTS here how can anyone be sure your DTS actually follows the
>> bindings?
>>
>> Best regards,
>> Krzysztof
> 
> Well, last time I was told that first needs to be accepted bindings
> documentation and then device tree files. So I did it like this. And now
> it is again feasible and different steps and ordering is needed...
> Sorry I cannot known all requirements which are moreover changing every
> day.

The rule is the same from years and no one was changing it. All driver
submitters, all DTS developers follow this. You are literally the one of
very few which is surprised by that generic rule and call it "a change".
The patches should be accepted by maintainers in such order (so without
bindings acceptance, the driver and DTS should not go), but not the
sending order.

It's the same with sysfs ABI. Exactly the same.

Best regards,
Krzysztof
Pali Rohár Aug. 19, 2022, 9:11 a.m. UTC | #8
On Friday 19 August 2022 10:42:22 Pali Rohár wrote:
> On Friday 19 August 2022 11:38:29 Krzysztof Kozlowski wrote:
> > On 19/08/2022 11:08, Pavel Machek wrote:
> > > Hi!
> > > 
> > >>> Although the question is - where is the user of it?
> > >>
> > >> I was planning to send updated powerpc DTS files with these
> > >> register-bit-led definitions after accepting dt bindings.
> > > 
> > > We need device tree people to ack them, first. But a note saying "this
> > > is for Turris Omnia router" would be welcome.
> > 
> > In general the process is one of:
> > 1. Send DT bindings with driver and DTS changes,
> > 2. Send DT bindings with driver in one patchset, DTS in second but you
> > mention the dependency.
> > 
> > You should not wait with DTS till bindings got accepted. Why? Because
> > for example we do not want bindings for stuff which never is going to be
> > upstreamed (with several exceptions, e.g. for other systems). Also
> > because we want to be able to compare bindings with your DTS
> > implementing them, so we are sure you described everything (especially
> > that you said running one command to install dtchema and second command
> > to make the check is not possible in your system).
> > 
> > Without DTS here how can anyone be sure your DTS actually follows the
> > bindings?
> > 
> > Best regards,
> > Krzysztof
> 
> Well, last time I was told that first needs to be accepted bindings
> documentation and then device tree files. So I did it like this. And now
> it is again feasible and different steps and ordering is needed...
> Sorry I cannot known all requirements which are moreover changing every
> day.

Here it is:
https://lore.kernel.org/linuxppc-dev/20220819084433.26011-1-pali@kernel.org/
Linus Walleij Aug. 26, 2022, 8:05 a.m. UTC | #9
On Thu, Aug 18, 2022 at 7:25 PM Pali Rohár <pali@kernel.org> wrote:

> This new active-low property specify that LED has inverted logic
> (0 - enable LED, 1 - disable LED).
>
> Signed-off-by: Pali Rohár <pali@kernel.org>

Acked-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij
Pali Rohár Aug. 31, 2022, 11:49 a.m. UTC | #10
On Friday 26 August 2022 10:05:26 Linus Walleij wrote:
> On Thu, Aug 18, 2022 at 7:25 PM Pali Rohár <pali@kernel.org> wrote:
> 
> > This new active-low property specify that LED has inverted logic
> > (0 - enable LED, 1 - disable LED).
> >
> > Signed-off-by: Pali Rohár <pali@kernel.org>
> 
> Acked-by: Linus Walleij <linus.walleij@linaro.org>
> 
> Yours,
> Linus Walleij

Is something else needed for this patch series?
Pali Rohár Oct. 9, 2022, 11:40 a.m. UTC | #11
On Wednesday 31 August 2022 13:49:36 Pali Rohár wrote:
> On Friday 26 August 2022 10:05:26 Linus Walleij wrote:
> > On Thu, Aug 18, 2022 at 7:25 PM Pali Rohár <pali@kernel.org> wrote:
> > 
> > > This new active-low property specify that LED has inverted logic
> > > (0 - enable LED, 1 - disable LED).
> > >
> > > Signed-off-by: Pali Rohár <pali@kernel.org>
> > 
> > Acked-by: Linus Walleij <linus.walleij@linaro.org>
> > 
> > Yours,
> > Linus Walleij
> 
> Is something else needed for this patch series?

PING?
Pali Rohár Nov. 1, 2022, 10:32 p.m. UTC | #12
On Sunday 09 October 2022 13:40:43 Pali Rohár wrote:
> On Wednesday 31 August 2022 13:49:36 Pali Rohár wrote:
> > On Friday 26 August 2022 10:05:26 Linus Walleij wrote:
> > > On Thu, Aug 18, 2022 at 7:25 PM Pali Rohár <pali@kernel.org> wrote:
> > > 
> > > > This new active-low property specify that LED has inverted logic
> > > > (0 - enable LED, 1 - disable LED).
> > > >
> > > > Signed-off-by: Pali Rohár <pali@kernel.org>
> > > 
> > > Acked-by: Linus Walleij <linus.walleij@linaro.org>
> > > 
> > > Yours,
> > > Linus Walleij
> > 
> > Is something else needed for this patch series?
> 
> PING?

PING?
Linus Walleij Nov. 8, 2022, 12:42 p.m. UTC | #13
On Tue, Nov 1, 2022 at 11:33 PM Pali Rohár <pali@kernel.org> wrote:
> On Sunday 09 October 2022 13:40:43 Pali Rohár wrote:
> > On Wednesday 31 August 2022 13:49:36 Pali Rohár wrote:
> > > On Friday 26 August 2022 10:05:26 Linus Walleij wrote:
> > > > On Thu, Aug 18, 2022 at 7:25 PM Pali Rohár <pali@kernel.org> wrote:
> > > >
> > > > > This new active-low property specify that LED has inverted logic
> > > > > (0 - enable LED, 1 - disable LED).
> > > > >
> > > > > Signed-off-by: Pali Rohár <pali@kernel.org>
> > > >
> > > > Acked-by: Linus Walleij <linus.walleij@linaro.org>
> > > >
> > > > Yours,
> > > > Linus Walleij
> > >
> > > Is something else needed for this patch series?
> >
> > PING?
>
> PING?

If the LED subsystem isn't working consider sending the patch to the
SoC tree with a notice that they apply it instead and why.

Yours,
Linus Walleij
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/leds/register-bit-led.yaml b/Documentation/devicetree/bindings/leds/register-bit-led.yaml
index 79b8fc0f9d23..5c6ef26f1a94 100644
--- a/Documentation/devicetree/bindings/leds/register-bit-led.yaml
+++ b/Documentation/devicetree/bindings/leds/register-bit-led.yaml
@@ -43,6 +43,11 @@  properties:
         0x100000, 0x200000, 0x400000, 0x800000, 0x1000000, 0x2000000, 0x4000000,
         0x8000000, 0x10000000, 0x20000000, 0x40000000, 0x80000000 ]
 
+  active-low:
+    $ref: /schemas/types.yaml#/definitions/flag
+    description:
+      LED is ON when bit in register is not set
+
   offset:
     description:
       register offset to the register controlling this LED