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

[Krzysztof Kozlowski <krzk@kernel.org>]

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