Hi Matt, Ricardo,
Matt - thanks for the update.
Ricardo - thanks for the review.
On 10/28/2016 09:20 AM, Matt Ranostay wrote:
On Fri, Oct 28, 2016 at 12:04 AM, Ricardo Ribalda Delgado
<ricardo.ribalda@xxxxxxxxx> wrote:
Hi Matt
Have you verified that led_set cannot be called in "parallel" ? There
might be a race condition otherwise
You are correct I should be using mutex_lock(&pca963x->chip->mutex);
Right, the locking has to be moved from the pca963x_brightness() to
the pca963x_led_set().
On Fri, Oct 28, 2016 at 4:53 AM, Matt Ranostay <mranostay@xxxxxxxxx> wrote:
+static int pca963x_power_state(struct pca963x_led *pca963x)
What about:
if (pca963x->led_cdev.brightness)
set_bit(pca963x->led_num, leds_on);
else
clear_bit(pca963x->led_num, leds_on);
if (!(*leds_on) != !cached_leds)
return i2c_smbus_write_byte_data(pca963x->chip->client,
PCA963X_MODE1, *leds_on ? 0: BIT(4));
return 0
I believe it is simpler, but it is just a matter of taste.
Don't really matter to which way we do it, but will wait for everyone's
input before spamming with v5 (with mutex fixes above)
Solution proposed by Ricardo looks indeed simpler.
--
Best regards,
Jacek Anaszewski
--
To unsubscribe from this list: send the line "unsubscribe linux-leds" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
