diff mbox series

[leds,v3,05/11] dt-bindings: leds: cznic,turris-omnia-leds: Allow interrupts property

Message ID 20240913123103.21226-6-kabel@kernel.org (mailing list archive)
State Superseded
Headers show
Series Turris Omnia LED driver changes | expand

Commit Message

Marek Behún Sept. 13, 2024, 12:30 p.m. UTC
Extend the cznic,turris-omnia-leds binding with interrupts property,
specifying the global LED brightness changed by button press interrupt.

Signed-off-by: Marek Behún <kabel@kernel.org>
---
 .../devicetree/bindings/leds/cznic,turris-omnia-leds.yaml | 8 ++++++++
 1 file changed, 8 insertions(+)

Comments

Krzysztof Kozlowski Sept. 16, 2024, 2:33 p.m. UTC | #1
On Fri, Sep 13, 2024 at 02:30:57PM +0200, Marek Behún wrote:
> Extend the cznic,turris-omnia-leds binding with interrupts property,
> specifying the global LED brightness changed by button press interrupt.
> 
> Signed-off-by: Marek Behún <kabel@kernel.org>
> ---
>  .../devicetree/bindings/leds/cznic,turris-omnia-leds.yaml | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/leds/cznic,turris-omnia-leds.yaml b/Documentation/devicetree/bindings/leds/cznic,turris-omnia-leds.yaml
> index 34ef5215c150..f52f6304c79e 100644
> --- a/Documentation/devicetree/bindings/leds/cznic,turris-omnia-leds.yaml
> +++ b/Documentation/devicetree/bindings/leds/cznic,turris-omnia-leds.yaml
> @@ -23,6 +23,12 @@ properties:
>      description: I2C slave address of the microcontroller.
>      maxItems: 1
>  
> +  interrupts:
> +    description:
> +      Specifier for the global LED brightness changed by front button press
> +      interrupt.

This "front button press" concerns me that you just hooked here
gpio-key. Are you sure that this is the physical interrupt line going to
this device?

Best regards,
Krzysztof
Marek Behún Sept. 17, 2024, 7:08 a.m. UTC | #2
On Mon, Sep 16, 2024 at 04:33:13PM +0200, Krzysztof Kozlowski wrote:
> On Fri, Sep 13, 2024 at 02:30:57PM +0200, Marek Behún wrote:
> > Extend the cznic,turris-omnia-leds binding with interrupts property,
> > specifying the global LED brightness changed by button press interrupt.
> > 
> > Signed-off-by: Marek Behún <kabel@kernel.org>
> > ---
> >  .../devicetree/bindings/leds/cznic,turris-omnia-leds.yaml | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/leds/cznic,turris-omnia-leds.yaml b/Documentation/devicetree/bindings/leds/cznic,turris-omnia-leds.yaml
> > index 34ef5215c150..f52f6304c79e 100644
> > --- a/Documentation/devicetree/bindings/leds/cznic,turris-omnia-leds.yaml
> > +++ b/Documentation/devicetree/bindings/leds/cznic,turris-omnia-leds.yaml
> > @@ -23,6 +23,12 @@ properties:
> >      description: I2C slave address of the microcontroller.
> >      maxItems: 1
> >  
> > +  interrupts:
> > +    description:
> > +      Specifier for the global LED brightness changed by front button press
> > +      interrupt.
> 
> This "front button press" concerns me that you just hooked here
> gpio-key. Are you sure that this is the physical interrupt line going to
> this device?

No no no, that is a different interrupt from the gpio-key.

The button can be configure in two modes:

- it can act like a GPIO to the CPU, forwarding press and release events
  via the GPIO chip (so button press it is handled by CPU)
  - can be set with
      echo cpu >/sys/bus/i2c/devices/1-002a/front_button_mode
  - pressing it will generate the INT_BUTTON_PRESSED interrupt from the
    MCU

- it can change LED panel brightness (so button press is handled by MCU)
  - this is the default mode, configured after boot
  - can be set with
      echo mcu >/sys/bus/i2c/devices/1-002a/front_button_mode
  - pressing it will generate the INT_BRIGHTNESS_CHANGED interrupt

The INT_BUTTON_PRESSED and INT_BRIGHTNESS_CHANGED interrupt semantically
different (and also literally, they are at different bits). See
  https://gitlab.nic.cz/turris/hw/omnia_hw_ctrl/-/blob/master/src/drivers/i2c_iface.h?ref_type=heads#L289-L321

Marek
Krzysztof Kozlowski Sept. 20, 2024, 2:54 p.m. UTC | #3
On 13/09/2024 14:30, Marek Behún wrote:
> Extend the cznic,turris-omnia-leds binding with interrupts property,
> specifying the global LED brightness changed by button press interrupt.
> 
> Signed-off-by: Marek Behún <kabel@kernel.org>
> ---


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

Best regards,
Krzysztof
Krzysztof Kozlowski Sept. 20, 2024, 2:54 p.m. UTC | #4
On 17/09/2024 09:08, Marek Behún wrote:
> On Mon, Sep 16, 2024 at 04:33:13PM +0200, Krzysztof Kozlowski wrote:
>> On Fri, Sep 13, 2024 at 02:30:57PM +0200, Marek Behún wrote:
>>> Extend the cznic,turris-omnia-leds binding with interrupts property,
>>> specifying the global LED brightness changed by button press interrupt.
>>>
>>> Signed-off-by: Marek Behún <kabel@kernel.org>
>>> ---
>>>  .../devicetree/bindings/leds/cznic,turris-omnia-leds.yaml | 8 ++++++++
>>>  1 file changed, 8 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/leds/cznic,turris-omnia-leds.yaml b/Documentation/devicetree/bindings/leds/cznic,turris-omnia-leds.yaml
>>> index 34ef5215c150..f52f6304c79e 100644
>>> --- a/Documentation/devicetree/bindings/leds/cznic,turris-omnia-leds.yaml
>>> +++ b/Documentation/devicetree/bindings/leds/cznic,turris-omnia-leds.yaml
>>> @@ -23,6 +23,12 @@ properties:
>>>      description: I2C slave address of the microcontroller.
>>>      maxItems: 1
>>>  
>>> +  interrupts:
>>> +    description:
>>> +      Specifier for the global LED brightness changed by front button press
>>> +      interrupt.
>>
>> This "front button press" concerns me that you just hooked here
>> gpio-key. Are you sure that this is the physical interrupt line going to
>> this device?
> 
> No no no, that is a different interrupt from the gpio-key.
> 
> The button can be configure in two modes:
> 
> - it can act like a GPIO to the CPU, forwarding press and release events
>   via the GPIO chip (so button press it is handled by CPU)
>   - can be set with
>       echo cpu >/sys/bus/i2c/devices/1-002a/front_button_mode
>   - pressing it will generate the INT_BUTTON_PRESSED interrupt from the
>     MCU
> 
> - it can change LED panel brightness (so button press is handled by MCU)
>   - this is the default mode, configured after boot
>   - can be set with
>       echo mcu >/sys/bus/i2c/devices/1-002a/front_button_mode
>   - pressing it will generate the INT_BRIGHTNESS_CHANGED interrupt
> 
> The INT_BUTTON_PRESSED and INT_BRIGHTNESS_CHANGED interrupt semantically
> different (and also literally, they are at different bits). See
>   https://gitlab.nic.cz/turris/hw/omnia_hw_ctrl/-/blob/master/src/drivers/i2c_iface.h?ref_type=heads#L289-L321

That's fine, thanks for the explanation.

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/leds/cznic,turris-omnia-leds.yaml b/Documentation/devicetree/bindings/leds/cznic,turris-omnia-leds.yaml
index 34ef5215c150..f52f6304c79e 100644
--- a/Documentation/devicetree/bindings/leds/cznic,turris-omnia-leds.yaml
+++ b/Documentation/devicetree/bindings/leds/cznic,turris-omnia-leds.yaml
@@ -23,6 +23,12 @@  properties:
     description: I2C slave address of the microcontroller.
     maxItems: 1
 
+  interrupts:
+    description:
+      Specifier for the global LED brightness changed by front button press
+      interrupt.
+    maxItems: 1
+
   "#address-cells":
     const: 1
 
@@ -56,6 +62,7 @@  additionalProperties: false
 examples:
   - |
 
+    #include <dt-bindings/interrupt-controller/irq.h>
     #include <dt-bindings/leds/common.h>
 
     i2c {
@@ -65,6 +72,7 @@  examples:
         led-controller@2b {
             compatible = "cznic,turris-omnia-leds";
             reg = <0x2b>;
+            interrupts-extended = <&mcu 11 IRQ_TYPE_NONE>;
             #address-cells = <1>;
             #size-cells = <0>;