On Wed, Mar 06, 2024 at 04:40:08PM -0600, Bjorn Helgaas wrote: > On Thu, Feb 15, 2024 at 03:23:45PM +0100, Mariusz Tkaczyk wrote: > > The interface is ready to support enclosures where patterns are not > > mutually exclusive, driver may clear other LEDs if they cannot be enabled > > together. > > I don't think this code does anything like "clearing other LEDs," does > it? I agree that this code doesn't impose any constraints about > indications being mutually exclusive, etc. It merely sets or clears > indication bits, and the hardware implementation is free to interpret > that as it sees fit. I guess the paragraph needs to be rephrased. The point is that if this NPEM driver sets bit A and another bit B is already set, and the hardware is incapable of lighting up whatever is controlled by these two bits *simultaneously*, the hardware might clear bit B when setting bit A. The driver can cope with that because npem_set() reads back the register contents with npem_read_reg() after calling npem_set_active_patterns(). > > + /* > > + * For the case where a NPEM command has not completed immediately, > > + * it is recommended that software not continuously ???spin??? on polling > > + * the status register, but rather poll under interrupt at a reduced > > + * rate; for example at 10 ms intervals. > > + * > > + * PCIe r6.1 sec 6.28 "Implementation Note: Software Polling of NPEM > > + * Command Completed" > > The implementation note also recommends reversing the order, i.e., > polling for completion of previous command and then writing a new > command, but it looks like we don't use that strategy? I think the leds subsystem expects the LED to be lit up by the time the ->brightness_set_blocking() callback returns. If the driver waited for command completion *before* setting a bit instead of afterwards, it could happen that npem_set() (the ->brightness_set_blocking() callback) returns even though the command hasn't completed yet and the LED thus isn't lit up yet. Thanks, Lukas
