lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <d90f338ca7a07d3e95ee6da08d84a74aeb7d866d.camel@codeconstruct.com.au>
Date: Wed, 08 Jan 2025 11:49:57 +1030
From: Andrew Jeffery <andrew@...econstruct.com.au>
To: Delphine CC Chiu <delphine_cc_chiu@...ynn.com>, patrick@...cx.xyz, Rob
 Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor
 Dooley <conor+dt@...nel.org>, Joel Stanley <joel@....id.au>
Cc: Marshall Zhan <marshall.zhan.wiwynn@...il.com>, 
	devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org, 
	linux-aspeed@...ts.ozlabs.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1] ARM: dts: aspeed: yosemite4: add fan led config

Hi Marshall,

On Fri, 2024-12-20 at 12:38 +0800, Delphine CC Chiu wrote:
> From: Marshall Zhan <marshall.zhan.wiwynn@...il.com>
> 
> Set fan led config in yosemite4 DTS.
> 
> Signed-off-by: Marshall Zhan <marshall.zhan.wiwynn@...il.com>
> Signed-off-by: Delphine CC Chiu <delphine_cc_chiu@...ynn.com>
> ---
>  .../aspeed/aspeed-bmc-facebook-yosemite4.dts  | 166
> +++++++++++++++++-
>  1 file changed, 163 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-
> yosemite4.dts b/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-
> yosemite4.dts
> index ab4904cf2c0e..b5865efcc80c 100644
> --- a/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-yosemite4.dts
> +++ b/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-yosemite4.dts
> @@ -73,6 +73,160 @@ tpm@0 {
>                         spi-max-frequency = <33000000>;
>                 };
>         };
> +
> +       leds {
> +               compatible = "gpio-leds";
> +
> +               led_identify {

The binding prefers naming the led nodes differently:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/leds/leds-gpio.yaml?h=v6.12#n22

If you must name the nodes this way, can you document why in a comment
in the devicetree?

> +                       retain-state-shutdown;
> +                       default-state = "off";
> +                       gpios = <&identify_gpio 8 GPIO_ACTIVE_HIGH>;
> +               };
> +
> +               led_fan0_blue {
> +                       retain-state-shutdown;
> +                       default-state = "on";
> +                       gpios = <&led_gpio0 4 GPIO_ACTIVE_HIGH>;
> +               };
> +
> +               led_fan0_amber {
> +                       retain-state-shutdown;
> +                       default-state = "off";
> +                       gpios = <&led_gpio0 5 GPIO_ACTIVE_HIGH>;
> +               };
> +
> +               led_fan1_blue {
> +                       retain-state-shutdown;
> +                       default-state = "on";
> +                       gpios = <&led_gpio0 10 GPIO_ACTIVE_HIGH>;
> +               };
> +
> +               led_fan1_amber {
> +                       retain-state-shutdown;
> +                       default-state = "off";
> +                       gpios = <&led_gpio0 11 GPIO_ACTIVE_HIGH>;
> +               };
> +
> +               led_fan2_blue {
> +                       retain-state-shutdown;
> +                       default-state = "on";
> +                       gpios = <&led_gpio1 4 GPIO_ACTIVE_HIGH>;
> +               };
> +
> +               led_fan2_amber {
> +                       retain-state-shutdown;
> +                       default-state = "off";
> +                       gpios = <&led_gpio1 5 GPIO_ACTIVE_HIGH>;
> +               };
> +
> +               led_fan3_blue {
> +                       retain-state-shutdown;
> +                       default-state = "on";
> +                       gpios = <&led_gpio1 10 GPIO_ACTIVE_HIGH>;
> +               };
> +
> +               led_fan3_amber {
> +                       retain-state-shutdown;
> +                       default-state = "off";
> +                       gpios = <&led_gpio1 11 GPIO_ACTIVE_HIGH>;
> +               };
> +
> +               led_fan4_blue {
> +                       retain-state-shutdown;
> +                       default-state = "on";
> +                       gpios = <&led_gpio0 2 GPIO_ACTIVE_HIGH>;
> +               };
> +
> +               led_fan4_amber {
> +                       retain-state-shutdown;
> +                       default-state = "off";
> +                       gpios = <&led_gpio0 3 GPIO_ACTIVE_HIGH>;
> +               };
> +
> +               led_fan5_blue {
> +                       retain-state-shutdown;
> +                       default-state = "on";
> +                       gpios = <&led_gpio0 8 GPIO_ACTIVE_HIGH>;
> +               };
> +
> +               led_fan5_amber {
> +                       retain-state-shutdown;
> +                       default-state = "off";
> +                       gpios = <&led_gpio0 9 GPIO_ACTIVE_HIGH>;
> +               };
> +
> +               led_fan6_blue {
> +                       retain-state-shutdown;
> +                       default-state = "on";
> +                       gpios = <&led_gpio1 2 GPIO_ACTIVE_HIGH>;
> +               };
> +
> +               led_fan6_amber {
> +                       retain-state-shutdown;
> +                       default-state = "off";
> +                       gpios = <&led_gpio1 3 GPIO_ACTIVE_HIGH>;
> +               };
> +
> +               led_fan7_blue {
> +                       retain-state-shutdown;
> +                       default-state = "on";
> +                       gpios = <&led_gpio1 8 GPIO_ACTIVE_HIGH>;
> +               };
> +
> +               led_fan7_amber {
> +                       retain-state-shutdown;
> +                       default-state = "off";
> +                       gpios = <&led_gpio1 9 GPIO_ACTIVE_HIGH>;
> +               };
> +
> +               led_fan8_blue {
> +                       retain-state-shutdown;
> +                       default-state = "on";
> +                       gpios = <&led_gpio0 0 GPIO_ACTIVE_HIGH>;
> +               };
> +
> +               led_fan8_amber {
> +                       retain-state-shutdown;
> +                       default-state = "off";
> +                       gpios = <&led_gpio0 1 GPIO_ACTIVE_HIGH>;
> +               };
> +
> +               led_fan9_blue {
> +                       retain-state-shutdown;
> +                       default-state = "on";
> +                       gpios = <&led_gpio0 6 GPIO_ACTIVE_HIGH>;
> +               };
> +
> +               led_fan9_amber {
> +                       retain-state-shutdown;
> +                       default-state = "off";
> +                       gpios = <&led_gpio0 7 GPIO_ACTIVE_HIGH>;
> +               };
> +
> +               led_fan10_blue {
> +                       retain-state-shutdown;
> +                       default-state = "on";
> +                       gpios = <&led_gpio1 0 GPIO_ACTIVE_HIGH>;
> +               };
> +
> +               led_fan10_amber {
> +                       retain-state-shutdown;
> +                       default-state = "off";
> +                       gpios = <&led_gpio1 1 GPIO_ACTIVE_HIGH>;
> +               };
> +
> +               led_fan11_blue {
> +                       retain-state-shutdown;
> +                       default-state = "on";
> +                       gpios = <&led_gpio1 6 GPIO_ACTIVE_HIGH>;
> +               };
> +
> +               led_fan11_amber {
> +                       retain-state-shutdown;
> +                       default-state = "off";
> +                       gpios = <&led_gpio1 7 GPIO_ACTIVE_HIGH>;
> +               };
> +       };
>  };
>  
>  &uart1 {
> @@ -995,11 +1149,17 @@ gpio@20 {
>                                 #gpio-cells = <2>;
>                         };
>  
> -                       gpio@21 {
> +                       identify_gpio: gpio@21 {

Does this expander really only expose the identify led?

>                                 compatible = "nxp,pca9506";
>                                 reg = <0x21>;
>                                 gpio-controller;
>                                 #gpio-cells = <2>;
> +                               gpio-line-names = "","","","",
> +                                                 "","","","",
> +                                                 "LED_IDENTIFY",
> +                                                
> "","","","","","","",
> +                                                
> "","","","","","","","",
> +                                                
> "","","","","","","","";
>                         };
>  
>                         gpio@22 {
> @@ -1173,7 +1333,7 @@ eeprom@52 {
>                                 reg = <0x52>;
>                         };
>  
> -                       gpio@61 {
> +                       led_gpio0: gpio@61 {

Bit of nitpick, but perhaps the node label could be improved?
fan_leds0? And fan_leds1 below?

>                                 compatible = "nxp,pca9552";
>                                 reg = <0x61>;
>                                 #address-cells = <1>;
> @@ -1221,7 +1381,7 @@ eeprom@52 {
>                                 reg = <0x52>;
>                         };
>  
> -                       gpio@61 {
> +                       led_gpio1: gpio@61 {
>                                 compatible = "nxp,pca9552";
>                                 reg = <0x61>;
>                                 #address-cells = <1>;

Cheers,

Andrew

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ