On Wed, Oct 4, 2023 at 5:59 PM Jerome Brunet <jbrunet@xxxxxxxxxxxx> wrote: > > > On Wed 04 Oct 2023 at 11:20, Neil Armstrong <neil.armstrong@xxxxxxxxxx> wrote: > > > On 02/10/2023 16:10, Jerome Brunet wrote: > >> Add support for the Libretech cottonwood board family. > >> These 2 boards are based on the same PCB, with an RPi B form factor. > >> The "Alta" board uses an a311d while the "Solitude" variant uses an > >> s905d3. > >> Co-developed-by: Da Xue <da.xue@xxxxxxxxxxxx> > >> Signed-off-by: Da Xue <da.xue@xxxxxxxxxxxx> > >> Signed-off-by: Jerome Brunet <jbrunet@xxxxxxxxxxxx> > >> --- > >> arch/arm64/boot/dts/amlogic/Makefile | 2 + > >> .../amlogic/meson-g12b-a311d-libretech-cc.dts | 133 ++++ > >> .../amlogic/meson-libretech-cottonwood.dtsi | 610 ++++++++++++++++++ > >> .../amlogic/meson-sm1-s905d3-libretech-cc.dts | 89 +++ > >> 4 files changed, 834 insertions(+) > >> create mode 100644 arch/arm64/boot/dts/amlogic/meson-g12b-a311d-libretech-cc.dts > >> create mode 100644 arch/arm64/boot/dts/amlogic/meson-libretech-cottonwood.dtsi > >> create mode 100644 arch/arm64/boot/dts/amlogic/meson-sm1-s905d3-libretech-cc.dts > >> > > > > <snip> > > > >> + > >> + leds-pwm { > >> + compatible = "pwm-leds"; > >> + > >> + led-green { > >> + color = <LED_COLOR_ID_GREEN>; > >> + function = LED_FUNCTION_STATUS; > >> + linux,default-trigger = "default-on"; > >> + panic-indicator; > >> + max-brightness = <255>; > >> + pwms = <&pwm_cd 1 1250 0>; > >> + active-low; > >> + }; > >> + > >> + led-blue { > >> + color = <LED_COLOR_ID_BLUE>; > >> + function = LED_FUNCTION_ACTIVITY; > >> + linux,default-trigger = "activity"; > > > > "activity" isn't documented, perhaps heartbeat instead ? > > > > The trigger does exist though. The other way is to extend the DT doc. > I don't really care one way or the other We can and should use heartbeat to both be consistent and also show Linux has started for developers. > > I'll defer to Da on this one > > >> + max-brightness = <255>; > >> + pwms = <&pwm_ab 1 1250 0>; > >> + active-low; > >> + }; > > > > leds subnodes should be named as led(-[0-9a-f]+) > > > > see Documentation/devicetree/bindings/leds/leds-pwm.yaml > > That I do care. The schematics refer to the leds by name. There is no > number assigned, much less hex. Making one up makes no sense. > > User should be able to quickly (and easily) link what they see in the > schematics with DT. > > So I'd prefer to submit a change for the regex rather than changing this I don't get why the LEDs should be hex either. It's contradictory to https://www.kernel.org/doc/html/latest/leds/leds-class.html I did a simple grep and didn't find any references to LED_COLOR_ID_ORANGE. We can debug this later. > > > > >> + }; > >> + > >> + leds-gpio { > >> + compatible = "gpio-leds"; > >> + > >> + led-orange { > >> + color = <LED_COLOR_ID_AMBER>; > >> + function = LED_FUNCTION_STANDBY; > >> + gpios = <&gpio GPIOX_6 GPIO_ACTIVE_LOW>; > >> + }; > > > > Ditto, but you can simply use "led" since it's the only one. > > > > See Documentation/devicetree/bindings/leds/leds-gpio.yaml > > > > Neil > > > > > > <snip> > > > _______________________________________________ > linux-amlogic mailing list > linux-amlogic@xxxxxxxxxxxxxxxxxxx > http://lists.infradead.org/mailman/listinfo/linux-amlogic