| Message ID | 718260256.101908.1600802915869@mail.vodafone.de |
|---|---|
| State | New |
| Headers | show |
| Series | [v1] leds: pca9532: correct shift computation in pca9532_getled | expand |
On Tue 2020-09-22 21:28:35, Markus Moll wrote: > Each led setting occupies two bits in a corresponding led register. > Accessing these bits requires shifting and masking, which was > implemented incorrectly in pca9532_getled. Two new helper macros > concentrate the computation of those masks in one place. > > Signed-off-by: Markus Moll <mmoll@de.pepperl-fuchs.com> Thanks, applied. Note that default-state = keep should _not_ be supported unless someone needs it. Best regards, Pavel
Hi Am Donnerstag, 24. September 2020, 14:24:21 CEST schrieb Pavel Machek: > > Note that default-state = keep should _not_ be supported unless > someone needs it. > We are using it (hence the patch), does that count? (We actually need it, as on our board U-Boot determines whether the leds should blink during boot, and glitch-free blinking wouldn't otherwise be possible). Markus
Hi! > Am Donnerstag, 24. September 2020, 14:24:21 CEST schrieb Pavel Machek: > > > > Note that default-state = keep should _not_ be supported unless > > someone needs it. > > > > We are using it (hence the patch), does that count? Yes :-). > (We actually need it, as on our board U-Boot determines whether the leds > should blink during boot, and glitch-free blinking wouldn't otherwise be > possible). Okay with me. I'd just prefer to avoid "there's possibility to do that in device tree, so let's support it". Best regards, Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
diff --git a/drivers/leds/leds-pca9532.c b/drivers/leds/leds-pca9532.c index 41229f775d3..d37fd9577d4 100644 --- a/drivers/leds/leds-pca9532.c +++ b/drivers/leds/leds-pca9532.c @@ -27,6 +27,8 @@ #define PCA9532_REG_PWM(m, i) (PCA9532_REG_OFFSET(m) + 0x2 + (i) * 2) #define LED_REG(m, led) (PCA9532_REG_OFFSET(m) + 0x5 + (led >> 2)) #define LED_NUM(led) (led & 0x3) +#define LED_SHIFT(led) (LED_NUM(led) * 2) +#define LED_MASK(led) (0x3 << LED_SHIFT(led)) #define ldev_to_led(c) container_of(c, struct pca9532_led, ldev) @@ -162,9 +164,9 @@ static void pca9532_setled(struct pca9532_led *led) mutex_lock(&data->update_lock); reg = i2c_smbus_read_byte_data(client, LED_REG(maxleds, led->id)); /* zero led bits */ - reg = reg & ~(0x3<<LED_NUM(led->id)*2); + reg = reg & ~LED_MASK(led->id); /* set the new value */ - reg = reg | (led->state << LED_NUM(led->id)*2); + reg = reg | (led->state << LED_SHIFT(led->id)); i2c_smbus_write_byte_data(client, LED_REG(maxleds, led->id), reg); mutex_unlock(&data->update_lock); } @@ -260,7 +262,7 @@ static enum pca9532_state pca9532_getled(struct pca9532_led *led) mutex_lock(&data->update_lock); reg = i2c_smbus_read_byte_data(client, LED_REG(maxleds, led->id)); - ret = reg >> LED_NUM(led->id)/2; + ret = (reg & LED_MASK(led->id)) >> LED_SHIFT(led->id); mutex_unlock(&data->update_lock); return ret; }
Each led setting occupies two bits in a corresponding led register. Accessing these bits requires shifting and masking, which was implemented incorrectly in pca9532_getled. Two new helper macros concentrate the computation of those masks in one place. Signed-off-by: Markus Moll <mmoll@de.pepperl-fuchs.com> --- drivers/leds/leds-pca9532.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)