Hi Lee,
On 7/11/24 10:30, Lee Jones wrote:
On Wed, 10 Jul 2024, Bastien Curutchet wrote:
Hi Lee,
On 6/17/24 16:39, Bastien Curutchet wrote:
+static int pca9532_update_hw_blink(struct pca9532_led *led,
+ unsigned long delay_on, unsigned long delay_off)
+{
+ struct pca9532_data *data = i2c_get_clientdata(led->client);
+ unsigned int psc;
+ int i;
+
+ /* Look for others LEDs that already use PWM1 */
+ for (i = 0; i < data->chip_info->num_leds; i++) {
+ struct pca9532_led *other = &data->leds[i];
+
+ if (other == led)
+ continue;
+
+ if (other->state == PCA9532_PWM1) {
+ if (other->ldev.blink_delay_on != delay_on ||
+ other->ldev.blink_delay_off != delay_off) {
+ dev_err(&led->client->dev,
+ "HW can handle only one blink configuration at a time\n");
I'm having some second thoughts about this dev_err().
It was dev_dbg() in V1, but based on your suggestion, I changed it to
dev_err() because an error was returned after.
I've worked more with this patch since it got applied, these error messages
appear frequently, though they don’t seem to be 'real' errors to me as the
software callback is used afterwards and the LED blinks at the expected
period.
Don't you think a dev_dbg() would be more appropriate in this case ? Or
perhaps a comment instead of a message ?
If it's not an error, then don't return an error message.
Maybe drop the message for a comment and return -EBUSY instead?
OK I'll do this, thank you.
+ return -EINVAL;
+ }
+ }
+ }
+
Best regards,
Bastien
