[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <71d11cdb-345f-43c7-b764-5aa43cdf1e1c@microchip.com>
Date: Tue, 16 Sep 2025 10:38:07 -0700
From: Ryan Wanner <ryan.wanner@...rochip.com>
To: <robh@...nel.org>, <krzk+dt@...nel.org>, <conor+dt@...nel.org>,
<nicolas.ferre@...rochip.com>, <alexandre.belloni@...tlin.com>,
<claudiu.beznea@...on.dev>, <devicetree@...r.kernel.org>,
<linux-arm-kernel@...ts.infradead.org>, <linux-kernel@...r.kernel.org>,
<linux-leds@...r.kernel.org>
Subject: Re: [PATCH] ARM: dts: microchip: sama7d65: Add GPIO buttons and LEDs
On 9/11/25 00:00, Alexander Dahl wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> Hello Ryan,
>
> Am Wed, Sep 10, 2025 at 10:16:03AM -0700 schrieb Ryan Wanner:
>> On 9/9/25 23:25, Alexander Dahl wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>
>>> Hello Ryan,
>>>
>>> Am Wed, Sep 10, 2025 at 08:20:28AM +0200 schrieb Alexander Dahl:
>>>> Hello Ryan,
>>>>
>>>> Am Tue, Sep 09, 2025 at 09:08:38AM -0700 schrieb Ryan.Wanner@...rochip.com:
>>>>> From: Ryan Wanner <Ryan.Wanner@...rochip.com>
>>>>>
>>>>> Add the USER button as a GPIO input as well as add the LEDs and enable
>>>>> the blue LED as a heartbeat.
>>>>>
>>>>> Signed-off-by: Ryan Wanner <Ryan.Wanner@...rochip.com>
>>>>> ---
>>>>> .../dts/microchip/at91-sama7d65_curiosity.dts | 49 +++++++++++++++++++
>>>>> 1 file changed, 49 insertions(+)
>>>>>
>>>>> diff --git a/arch/arm/boot/dts/microchip/at91-sama7d65_curiosity.dts b/arch/arm/boot/dts/microchip/at91-sama7d65_curiosity.dts
>>>>> index f091cc40a9f0..2fe34c59d942 100644
>>>>> --- a/arch/arm/boot/dts/microchip/at91-sama7d65_curiosity.dts
>>>>> +++ b/arch/arm/boot/dts/microchip/at91-sama7d65_curiosity.dts
>>>>> @@ -11,6 +11,7 @@
>>>>> #include "sama7d65-pinfunc.h"
>>>>> #include "sama7d65.dtsi"
>>>>> #include <dt-bindings/mfd/atmel-flexcom.h>
>>>>> +#include <dt-bindings/input/input.h>
>>>>> #include <dt-bindings/pinctrl/at91.h>
>>>>>
>>>>> / {
>>>>> @@ -26,6 +27,42 @@ chosen {
>>>>> stdout-path = "serial0:115200n8";
>>>>> };
>>>>>
>>>>> + gpio-keys {
>>>>> + compatible = "gpio-keys";
>>>>> +
>>>>> + pinctrl-names = "default";
>>>>> + pinctrl-0 = <&pinctrl_key_gpio_default>;
>>>>> +
>>>>> + button {
>>>>> + label = "PB_USER";
>>>>> + gpios = <&pioa PIN_PC10 GPIO_ACTIVE_LOW>;
>>>>> + linux,code = <KEY_PROG1>;
>>>>> + wakeup-source;
>>>>> + };
>>>>> + };
>>>>> +
>>>>> + leds {
>>>>> + compatible = "gpio-leds";
>>>>> + pinctrl-names = "default";
>>>>> + pinctrl-0 = <&pinctrl_led_gpio_default>;
>>>>> +
>>>>> + led-red {
>>>>> + label = "red";
>>>>> + gpios = <&pioa PIN_PB17 GPIO_ACTIVE_HIGH>; /* Conflict with pwm. */
>>>>> + };
>>>>> +
>>>>> + led-green {
>>>>> + label = "green";
>>>>> + gpios = <&pioa PIN_PB15 GPIO_ACTIVE_HIGH>; /* Conflict with pwm. */
>>>>> + };
>>>>> +
>>>>> + led-blue {
>>>>> + label = "blue";
>>>>> + gpios = <&pioa PIN_PA21 GPIO_ACTIVE_HIGH>;
>>>>> + linux,default-trigger = "heartbeat";
>>>>> + };
>>>>> + };
>>>>
>>>> The label property is deprecated. Please use the properties "color"
>>>> and "function" for new boards. See devicetree binding documentation
>>>> for LEDs.
>>>
>>> From a quick glance, this seems to be an RGB-LED, so I would suggest
>>> to not model it as three distinct LEDs, but make use of the
>>> "leds-group-multicolor" feature, example:
>>>
>>> 59 multi-led {
>>> 60 compatible = "leds-group-multicolor";
>>> 61 color = <LED_COLOR_ID_RGB>;
>>> 62 function = LED_FUNCTION_INDICATOR;
>>> 63 leds = <&led_red>, <&led_green>, <&led_blue>;
>>> 64 };
>>
>> I see, I was not aware of this feature. This would combine all of the
>> LED pins into one RGB light correct, it seems from sysfs that this is
>> the case.
>
> The group-multicolor feature was merged for kernel v6.6 so it's still
> quite new. I tried this some time ago, so this is from memory only.
> The three single color gpio leds are still visible in sysfs, but you
> can not control them independently anymore, only through the sysfs
> interface of that one multicolor led.
>
>> Would having the default-trigger="heartbeat" still be allowed for the
>> led-blue node or should that be moved into the multi-led node? From the
>> bindings it seems that the default trigger is still in the gpio-led nodes.
>
> Sorry, not sure here. I put linux-leds in Cc, maybe someone over
> there can answer. If this does not fit how Microchip wants to handle
> that LED on their boards I think that's fine, too. Just wanted to
> make people aware of the possibility.
It does not fit very well how we want to handle the LEDs on this board
as of right now. I am assuming it is fine for now to have these as
individual LEDs?
Best,
Ryan>
> Greets
> Alex
>
>>
>> Best,
>> Ryan
>>>
>>> Greets
>>> Alex
>>>
>>>>
>>>> Thanks and greetings
>>>> Alex
>>>>
>>>>> +
>>>>> memory@...00000 {
>>>>> device_type = "memory";
>>>>> reg = <0x60000000 0x40000000>;
>>>>> @@ -352,6 +389,18 @@ pinctrl_i2c10_default: i2c10-default {
>>>>> bias-pull-up;
>>>>> };
>>>>>
>>>>> + pinctrl_key_gpio_default: key-gpio-default {
>>>>> + pinmux = <PIN_PC10__GPIO>;
>>>>> + bias-pull-up;
>>>>> + };
>>>>> +
>>>>> + pinctrl_led_gpio_default: led-gpio-default {
>>>>> + pinmux = <PIN_PB15__GPIO>,
>>>>> + <PIN_PB17__GPIO>,
>>>>> + <PIN_PA21__GPIO>;
>>>>> + bias-pull-up;
>>>>> + };
>>>>> +
>>>>> pinctrl_sdmmc1_default: sdmmc1-default {
>>>>> cmd-data {
>>>>> pinmux = <PIN_PB22__SDMMC1_CMD>,
>>>>> --
>>>>> 2.43.0
>>>>>
>>>>>
>>>>
>>
>>
Powered by blists - more mailing lists