Skip to content

Commit

Permalink
leds: turris-omnia: Do not use SMBUS calls
Browse files Browse the repository at this point in the history
The leds-turris-omnia driver uses three function for I2C access:
- i2c_smbus_write_byte_data() and i2c_smbus_read_byte_data(), which
  cause an emulated SMBUS transfer,
- i2c_master_send(), which causes an ordinary I2C transfer.

The Turris Omnia MCU LED controller is not semantically SMBUS, it
operates as a simple I2C bus. It does not implement any of the SMBUS
specific features, like PEC, or procedure calls, or anything. Moreover
the I2C controller driver also does not implement SMBUS, and so the
emulated SMBUS procedure from drivers/i2c/i2c-core-smbus.c is used for
the SMBUS calls, which gives an unnecessary overhead.

When I first wrote the driver, I was unaware of these facts, and I
simply used the first function that worked.

Drop the I2C SMBUS calls and instead use simple I2C transfers.

Fixes: 089381b ("leds: initial support for Turris Omnia LEDs")
Signed-off-by: Marek Behún <kabel@kernel.org>
Link: https://lore.kernel.org/r/20230918161104.20860-2-kabel@kernel.org
Signed-off-by: Lee Jones <lee@kernel.org>
  • Loading branch information
Marek Behún authored and Lee Jones committed Nov 1, 2023
1 parent 7c97701 commit 6de283b
Showing 1 changed file with 42 additions and 12 deletions.
54 changes: 42 additions & 12 deletions drivers/leds/leds-turris-omnia.c
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
/*
* CZ.NIC's Turris Omnia LEDs driver
*
* 2020 by Marek Behún <kabel@kernel.org>
* 2020, 2023 by Marek Behún <kabel@kernel.org>
*/

#include <linux/i2c.h>
Expand Down Expand Up @@ -41,6 +41,37 @@ struct omnia_leds {
struct omnia_led leds[];
};

static int omnia_cmd_write_u8(const struct i2c_client *client, u8 cmd, u8 val)
{
u8 buf[2] = { cmd, val };

return i2c_master_send(client, buf, sizeof(buf));
}

static int omnia_cmd_read_u8(const struct i2c_client *client, u8 cmd)
{
struct i2c_msg msgs[2];
u8 reply;
int ret;

msgs[0].addr = client->addr;
msgs[0].flags = 0;
msgs[0].len = 1;
msgs[0].buf = &cmd;
msgs[1].addr = client->addr;
msgs[1].flags = I2C_M_RD;
msgs[1].len = 1;
msgs[1].buf = &reply;

ret = i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));
if (likely(ret == ARRAY_SIZE(msgs)))
return reply;
else if (ret < 0)
return ret;
else
return -EIO;
}

static int omnia_led_brightness_set_blocking(struct led_classdev *cdev,
enum led_brightness brightness)
{
Expand All @@ -64,7 +95,7 @@ static int omnia_led_brightness_set_blocking(struct led_classdev *cdev,
if (buf[2] || buf[3] || buf[4])
state |= CMD_LED_STATE_ON;

ret = i2c_smbus_write_byte_data(leds->client, CMD_LED_STATE, state);
ret = omnia_cmd_write_u8(leds->client, CMD_LED_STATE, state);
if (ret >= 0 && (state & CMD_LED_STATE_ON))
ret = i2c_master_send(leds->client, buf, 5);

Expand Down Expand Up @@ -114,18 +145,18 @@ static int omnia_led_register(struct i2c_client *client, struct omnia_led *led,
cdev->brightness_set_blocking = omnia_led_brightness_set_blocking;

/* put the LED into software mode */
ret = i2c_smbus_write_byte_data(client, CMD_LED_MODE,
CMD_LED_MODE_LED(led->reg) |
CMD_LED_MODE_USER);
ret = omnia_cmd_write_u8(client, CMD_LED_MODE,
CMD_LED_MODE_LED(led->reg) |
CMD_LED_MODE_USER);
if (ret < 0) {
dev_err(dev, "Cannot set LED %pOF to software mode: %i\n", np,
ret);
return ret;
}

/* disable the LED */
ret = i2c_smbus_write_byte_data(client, CMD_LED_STATE,
CMD_LED_STATE_LED(led->reg));
ret = omnia_cmd_write_u8(client, CMD_LED_STATE,
CMD_LED_STATE_LED(led->reg));
if (ret < 0) {
dev_err(dev, "Cannot set LED %pOF brightness: %i\n", np, ret);
return ret;
Expand Down Expand Up @@ -158,7 +189,7 @@ static ssize_t brightness_show(struct device *dev, struct device_attribute *a,
struct i2c_client *client = to_i2c_client(dev);
int ret;

ret = i2c_smbus_read_byte_data(client, CMD_LED_GET_BRIGHTNESS);
ret = omnia_cmd_read_u8(client, CMD_LED_GET_BRIGHTNESS);

if (ret < 0)
return ret;
Expand All @@ -179,8 +210,7 @@ static ssize_t brightness_store(struct device *dev, struct device_attribute *a,
if (brightness > 100)
return -EINVAL;

ret = i2c_smbus_write_byte_data(client, CMD_LED_SET_BRIGHTNESS,
(u8)brightness);
ret = omnia_cmd_write_u8(client, CMD_LED_SET_BRIGHTNESS, brightness);

return ret < 0 ? ret : count;
}
Expand Down Expand Up @@ -237,8 +267,8 @@ static void omnia_leds_remove(struct i2c_client *client)
u8 buf[5];

/* put all LEDs into default (HW triggered) mode */
i2c_smbus_write_byte_data(client, CMD_LED_MODE,
CMD_LED_MODE_LED(OMNIA_BOARD_LEDS));
omnia_cmd_write_u8(client, CMD_LED_MODE,
CMD_LED_MODE_LED(OMNIA_BOARD_LEDS));

/* set all LEDs color to [255, 255, 255] */
buf[0] = CMD_LED_COLOR;
Expand Down

0 comments on commit 6de283b

Please sign in to comment.