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); > > 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) > > > Regards! -- 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