Messages in this thread | | | Date | Fri, 1 Apr 2022 09:17:24 -0500 | Subject | Re: [PATCH] leds: pca955x: Add HW blink support | From | Eddie James <> |
| |
On 3/31/22 10:39, Patrick Williams wrote: > On Wed, Mar 30, 2022 at 03:33:18PM -0500, Eddie James wrote: >> Support blinking using the PCA955x chip. Use PWM0 for blinking >> instead of LED_HALF brightness. Since there is only one frequency >> and brightness register for any blinking LED, all blinked LEDs on >> the chip will have the same frequency and brightness. > The current implementation uses the PWM to control the "brightness" > with PWM0 being assigned 50% and PWM1 being configured as a single value > that isn't ON, OFF, or 50%. I suspect that most users of these 955x > chips care either about brightness or blinking but not both, but it is > possible I am wrong. It would be nice if we could use PWM1 as another > hardware blink control when it hasn't been used for brightness, but that > would require some additional state tracking I think. > > I like that we can now use the hardware to control blink rate, rather > than doing it in software, and, I really like that in theory if N LEDs on > the device are all blinking at the same rate they will actually turn on and > off at the same exact moment because it is done in hardware. I am really > concerned about this proposed change and the way it will change current > behavior though. > > It is not uncommon in a BMC design to use one of these 955x chips to control > 8 or 16 different LEDs reflecting the state of the system and at > different blink rates. An example LED policy might be that you have 1 LED > for "power status" and another LED for "system identify + health status". > When the system is powered off the "power status" LED flashes at a slow rate > and when the system is powered on it goes on solid. When the system is healthy > the "health status" is on, when it is unhealthy it blinks slowly, and when the > system is "identified" it blinks fast. > > My point of the above is that there are certainly system policies where > you'd want to flash two different LEDs at two different rates. In > today's implementation of this driver those both turn into > software-emulated blinking by the kernel. With your proposal we lose > this ability and instead whichever LED is configured second will affect > all other blinking LEDs.
Yep. I see your point, it could be problematic.
> > It looks like in led-core.c led_blink_setup that if the device > `blink_set` returns an error then software blinking is the fallback. Is > it possible for us to have this driver keep track of how many LEDs are > in blink state (and which speeds are allocated) and get led-core to > fallback to software blinking if we are unable to satisfy the new blink > rate without affecting an existing LED blink rate?
OK, I like this idea, I'll go ahead and implement it. Thanks for the suggestion!
Eddie
> > Looking at the tree it seems bcm6328 does what I am suggesting already > but I don't see any other drivers that obviously do. The PCA955x is > pretty widely used in BMC implementations: > > $ git grep -l pca955 arch/arm/boot/dts/aspeed* | wc -l > 13 >
| |