linux-kernel - Re: [PATCH] ARM: dts: aspeed: clemente: Add HDD LED GPIO

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: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <deb2dda58b923f2ae362dc19164dc845c4757e12.camel@codeconstruct.com.au>
Date: Mon, 29 Sep 2025 12:30:41 +0930
From: Andrew Jeffery <andrew@...econstruct.com.au>
To: Leo Wang <leo.jt.wang@...il.com>, Rob Herring <robh@...nel.org>, 
 Krzysztof Kozlowski
	 <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, Joel Stanley
	 <joel@....id.au>
Cc: devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org, 
	linux-aspeed@...ts.ozlabs.org, linux-kernel@...r.kernel.org, 
	george.kw.lee@...-foxconn.com, bruce.jy.hung@...-foxconn.com, 
	leo.jt.wang@...-foxconn.com
Subject: Re: [PATCH] ARM: dts: aspeed: clemente: Add HDD LED GPIO

Hi Leo,

On Fri, 2025-09-26 at 17:37 +0800, Leo Wang wrote:
> Define a GPIO expander pin for the HDD LED and expose it via the
> LED subsystem. This allows the BMC to control the front panel
> HDD activity LED.
> 
> Changes include:
>  - Add `led-hdd` node under the LED definitions
>  - Name the expander pin "HDD_LED_N" in io_expander13

Can you please drop this "Changes include" section? What you've written
here is plain to see from the diff below. The commit message should
explain motivations, concerns and trade-offs, and not be a wrote
description of the change. The documentation on submitting patches
explores what's required:

https://docs.kernel.org/process/submitting-patches.html#describe-your-changes

Cheers,

Andrew

> 
> Signed-off-by: Leo Wang <leo.jt.wang@...il.com>
> ---
>  arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-clemente.dts | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-clemente.dts b/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-clemente.dts
> index a614f947dafc9a6628c3f3d45b62ff3ca9d2b618..450446913e36b1418fab901cde44280468990c7a 100644
> --- a/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-clemente.dts
> +++ b/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-clemente.dts
> @@ -95,6 +95,11 @@ led-3 {
>  			label = "bmc_ready_cpld_noled";
>  			gpios = <&gpio0 ASPEED_GPIO(P, 5) (GPIO_ACTIVE_HIGH|GPIO_TRANSITORY)>;
>  		};
> +
> +		led-hdd {
> +			label = "hdd_led";
> +			gpios = <&io_expander13 1 GPIO_ACTIVE_LOW>;
> +		};
>  	};
>  
>  	memory@...00000 {
> @@ -1199,7 +1204,7 @@ io_expander13: gpio@14 {
>  		#gpio-cells = <2>;
>  		gpio-line-names =
>  			"rmc_en_dc_pwr_on",
> -			"",
> +			"HDD_LED_N",
>  			"",
>  			"",
>  			"",
> 
> ---
> base-commit: c65261717599d419e9c683d85f515d3ca2261549
> change-id: 20250926-leo-dts-add-shunt-resistor-0cba43f75bce
> 
> Best regards,