Re: [PATCH v2 3/4] ARM: dts: aspeed: bletchley: Add second source PCA9532 LED controllers

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The alternative PCA9532 LED controllers use incorrect GPIO active polarity flags, causing the LEDs to operate inverted compared to the primary PCA9552 controllers.
--

--- Patch [3]: [PATCH v2 3/4] ARM: dts: aspeed: bletchley: Add second source PCA9532 LED controllers ---
commit a1677ab804733ff8d7d1bbc1ba314032d50ddf82
Author: Cosmo Chou <chou.cosmo@gmail.com>

ARM: dts: aspeed: bletchley: Add second source PCA9532 LED controllers

This commit adds alternative PCA9532 LED controllers at address 0x66 to
support boards populated with the PCA9532 instead of the primary PCA9552
at 0x67.

> diff --git a/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-bletchley.dts b/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-bletchley.dts
> index e1fa3a18ea5cf..3da1cf9bbb990 100644
> --- a/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-bletchley.dts
> +++ b/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-bletchley.dts

[ ... ]

> @@ -86,6 +87,66 @@ led-7 {
>  		};
>  	};
>  
> +	fan_gpio_leds_alt {
> +		compatible = "gpio-leds";
> +
> +		led-0 {
> +			default-state = "on";
> +			gpios = <&fan_leds_alt 8 GPIO_ACTIVE_HIGH>;

[Severity: High]
Will this GPIO active polarity cause the LED to operate inverted compared to the
primary PCA9552 controller?

The leds-pca955x driver maps a raw value of 1 to High-Z and 0 to Output LOW,
whereas the leds-pca9532 driver maps a raw value of 1 to Output LOW and 0 to
High-Z.

Because the two drivers map raw GPIO values to opposite electrical states, does
this flag need to be GPIO_ACTIVE_LOW to produce the same physical state when
logically ON?

> +			label = "fan0_blue";
> +			retain-state-shutdown;
> +		};
> +

[ ... ]

> @@ -162,6 +223,24 @@ led-1 {
>  		};
>  	};
>  
> +	sled1_gpio_leds_alt {
> +		compatible = "gpio-leds";
> +
> +		led-0 {
> +			default-state = "keep";
> +			gpios = <&sled1_leds_alt 0 GPIO_ACTIVE_LOW>;

[Severity: High]
Similar to the fan LEDs above, does this polarity need to be GPIO_ACTIVE_HIGH
for the PCA9532 to output the same physical LOW state when logically ON?

The current GPIO_ACTIVE_LOW flag appears to be copied from the primary PCA9552
node, but due to the inverted semantics of the leds-pca9532 driver, this might
result in inverted electrical signals.

Does this polarity issue also apply to the alternative nodes for sled2 through
sled6?

> +			label = "sled1_amber";
> +			retain-state-shutdown;
> +		};
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260729114312.3947300-1-chou.cosmo@gmail.com?part=3