Re: [PATCH 6/6] ARM: dts: aspeed: rainier: Add leds that are on optional PCI cable cards

[Andrew Jeffery]

On Wed, 10 Feb 2021, at 21:46, vishwanatha subbanna wrote:
> 
> 
> > On 16-Nov-2020, at 11:43 AM, Joel Stanley <joel at jms.id.au> wrote:
> > 
> > On Fri, 13 Nov 2020 at 05:59, Vishwanatha Subbanna
> > <vishwa at linux.vnet.ibm.com> wrote:
> >> 
> >> These are LEDs on the cable cards that plug into PCIE slots.
> >> The LEDs are controlled by PCA9552 I2C expander
> >> 
> >> Signed-off-by: Vishwanatha Subbanna <vishwa at linux.vnet.ibm.com>
> >> ---
> >> arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts | 288 +++++++++++++++++++++++++++
> >> 1 file changed, 288 insertions(+)
> >> 
> >> diff --git a/arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts b/arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts
> >> index 67c8c40..7de5f76 100644
> >> --- a/arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts
> >> +++ b/arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts
> >> @@ -696,6 +696,70 @@
> >>                        gpios = <&pca4 7 GPIO_ACTIVE_LOW>;
> >>                };
> >>        };
> >> +
> >> +       leds-optional-cablecard0 {
> > 
> > Is it necessary to have separate nodes for each of the different GPIO devices?
> > 
> > Would it make sense to combine them, or is it better to be separate?
> > 
> > Andrew, Eddie, Brad: please review this one before I merge it.
> 
> I answered this in previous patch set.  If I express ‘em all in one 
> node that is “leds {", then if any of the GPIO is not seen because of 
> not having the card, then the current leds-gpio driver knocks off all 
> the ones on which it successfully acquired the GPIOs also, leaving 
> nothing.

I'm struggling to follow this sentence. Can you please explain what you're 
trying to say in a less colloquial way?

> I did speak to the maintainer and it looked like the behaviour 
> was existing since long time and changing it would break old code.
> 

Break how?

Andrew