All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1] leds: pca9532: correct shift computation in pca9532_getled
@ 2020-09-22 19:28 Markus Moll
  2020-09-24 12:24 ` Pavel Machek
  0 siblings, 1 reply; 4+ messages in thread
From: Markus Moll @ 2020-09-22 19:28 UTC (permalink / raw)
  To: Riku Voipio, Pavel Machek, Dan Murphy; +Cc: linux-leds

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(-)

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;
 }
-- 
2.25.1

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v1] leds: pca9532: correct shift computation in pca9532_getled
  2020-09-22 19:28 [PATCH v1] leds: pca9532: correct shift computation in pca9532_getled Markus Moll
@ 2020-09-24 12:24 ` Pavel Machek
  2020-09-24 13:07   ` Markus Moll
  0 siblings, 1 reply; 4+ messages in thread
From: Pavel Machek @ 2020-09-24 12:24 UTC (permalink / raw)
  To: Markus Moll; +Cc: Riku Voipio, Dan Murphy, linux-leds

[-- Attachment #1: Type: text/plain, Size: 653 bytes --]

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
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v1] leds: pca9532: correct shift computation in pca9532_getled
  2020-09-24 12:24 ` Pavel Machek
@ 2020-09-24 13:07   ` Markus Moll
  2020-09-24 13:35     ` Pavel Machek
  0 siblings, 1 reply; 4+ messages in thread
From: Markus Moll @ 2020-09-24 13:07 UTC (permalink / raw)
  To: Pavel Machek; +Cc: Riku Voipio, Dan Murphy, linux-leds

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




^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v1] leds: pca9532: correct shift computation in pca9532_getled
  2020-09-24 13:07   ` Markus Moll
@ 2020-09-24 13:35     ` Pavel Machek
  0 siblings, 0 replies; 4+ messages in thread
From: Pavel Machek @ 2020-09-24 13:35 UTC (permalink / raw)
  To: Markus Moll; +Cc: Riku Voipio, Dan Murphy, linux-leds

[-- Attachment #1: Type: text/plain, Size: 712 bytes --]

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2020-09-24 13:35 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-22 19:28 [PATCH v1] leds: pca9532: correct shift computation in pca9532_getled Markus Moll
2020-09-24 12:24 ` Pavel Machek
2020-09-24 13:07   ` Markus Moll
2020-09-24 13:35     ` Pavel Machek

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.