Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

leds: turris-omnia: Fix brightness setting and trigger activating

I have improperly refactored commits
4d5ed2621c24 ("leds: turris-omnia: Make set_brightness() more efficient")
and
aaf38273cf76 ("leds: turris-omnia: Support HW controlled mode via private trigger")
after Lee requested a change in API semantics of the new functions I
introduced in commit
28350bc0ac77 ("leds: turris-omnia: Do not use SMBUS calls").

Before the change, the function omnia_cmd_write_u8() returned 0 on
success, and afterwards it returned a positive value (number of bytes
written). The latter version was applied, but the following commits did
not properly account for this change.

This results in non-functional LED's .brightness_set_blocking() and
trigger's .activate() methods.

The main reasoning behind the semantics change was that read/write
methods should return the number of read/written bytes on success.
It was pointed to me [1] that this is not always true (for example the
regmap API does not do so), and since the driver never uses this number
of read/written bytes information, I decided to fix this issue by
changing the functions to the original semantics (return 0 on success).

[1] https://lore.kernel.org/linux-gpio/ZQnn+Gi0xVlsGCYA@smile.fi.intel.com/

Fixes: 28350bc0ac77 ("leds: turris-omnia: Do not use SMBUS calls")
Signed-off-by: Marek Behún <kabel@kernel.org>
Link: https://lore.kernel.org/r/20231016141538.30037-1-kabel@kernel.org
Signed-off-by: Lee Jones <lee@kernel.org>

authored by

Marek Behún and committed by
Lee Jones
78cbcfd8 50be9e02

+20 -17
+20 -17
drivers/leds/leds-turris-omnia.c
··· 60 60 static int omnia_cmd_write_u8(const struct i2c_client *client, u8 cmd, u8 val) 61 61 { 62 62 u8 buf[2] = { cmd, val }; 63 + int ret; 63 64 64 - return i2c_master_send(client, buf, sizeof(buf)); 65 + ret = i2c_master_send(client, buf, sizeof(buf)); 66 + 67 + return ret < 0 ? ret : 0; 65 68 } 66 69 67 70 static int omnia_cmd_read_raw(struct i2c_adapter *adapter, u8 addr, u8 cmd, ··· 84 81 85 82 ret = i2c_transfer(adapter, msgs, ARRAY_SIZE(msgs)); 86 83 if (likely(ret == ARRAY_SIZE(msgs))) 87 - return len; 84 + return 0; 88 85 else if (ret < 0) 89 86 return ret; 90 87 else ··· 94 91 static int omnia_cmd_read_u8(const struct i2c_client *client, u8 cmd) 95 92 { 96 93 u8 reply; 97 - int ret; 94 + int err; 98 95 99 - ret = omnia_cmd_read_raw(client->adapter, client->addr, cmd, &reply, 1); 100 - if (ret < 0) 101 - return ret; 96 + err = omnia_cmd_read_raw(client->adapter, client->addr, cmd, &reply, 1); 97 + if (err) 98 + return err; 102 99 103 100 return reply; 104 101 } ··· 239 236 240 237 mutex_unlock(&leds->lock); 241 238 242 - if (err < 0) 239 + if (err) 243 240 dev_err(cdev->dev, "Cannot put LED to software mode: %i\n", 244 241 err); 245 242 } ··· 305 302 ret = omnia_cmd_write_u8(client, CMD_LED_MODE, 306 303 CMD_LED_MODE_LED(led->reg) | 307 304 CMD_LED_MODE_USER); 308 - if (ret < 0) { 305 + if (ret) { 309 306 dev_err(dev, "Cannot set LED %pOF to software mode: %i\n", np, 310 307 ret); 311 308 return ret; ··· 314 311 /* disable the LED */ 315 312 ret = omnia_cmd_write_u8(client, CMD_LED_STATE, 316 313 CMD_LED_STATE_LED(led->reg)); 317 - if (ret < 0) { 314 + if (ret) { 318 315 dev_err(dev, "Cannot set LED %pOF brightness: %i\n", np, ret); 319 316 return ret; 320 317 } ··· 367 364 { 368 365 struct i2c_client *client = to_i2c_client(dev); 369 366 unsigned long brightness; 370 - int ret; 367 + int err; 371 368 372 369 if (kstrtoul(buf, 10, &brightness)) 373 370 return -EINVAL; ··· 375 372 if (brightness > 100) 376 373 return -EINVAL; 377 374 378 - ret = omnia_cmd_write_u8(client, CMD_LED_SET_BRIGHTNESS, brightness); 375 + err = omnia_cmd_write_u8(client, CMD_LED_SET_BRIGHTNESS, brightness); 379 376 380 - return ret < 0 ? ret : count; 377 + return err ?: count; 381 378 } 382 379 static DEVICE_ATTR_RW(brightness); 383 380 ··· 406 403 struct i2c_client *client = to_i2c_client(dev); 407 404 struct omnia_leds *leds = i2c_get_clientdata(client); 408 405 bool val; 409 - int ret; 406 + int err; 410 407 411 408 if (!leds->has_gamma_correction) 412 409 return -EOPNOTSUPP; ··· 414 411 if (kstrtobool(buf, &val) < 0) 415 412 return -EINVAL; 416 413 417 - ret = omnia_cmd_write_u8(client, CMD_SET_GAMMA_CORRECTION, val); 414 + err = omnia_cmd_write_u8(client, CMD_SET_GAMMA_CORRECTION, val); 418 415 419 - return ret < 0 ? ret : count; 416 + return err ?: count; 420 417 } 421 418 static DEVICE_ATTR_RW(gamma_correction); 422 419 ··· 434 431 435 432 err = omnia_cmd_read_raw(client->adapter, OMNIA_MCU_I2C_ADDR, 436 433 CMD_GET_STATUS_WORD, &reply, sizeof(reply)); 437 - if (err < 0) 434 + if (err) 438 435 return err; 439 436 440 437 /* Check whether MCU firmware supports the CMD_GET_FEAUTRES command */ ··· 443 440 444 441 err = omnia_cmd_read_raw(client->adapter, OMNIA_MCU_I2C_ADDR, 445 442 CMD_GET_FEATURES, &reply, sizeof(reply)); 446 - if (err < 0) 443 + if (err) 447 444 return err; 448 445 449 446 return le16_to_cpu(reply);