On Wed, 02 Aug 2023, Marek Behún wrote:
> If the MCU on Turris Omnia is running newer firmware versions, the LED
> controller supports RGB gamma correction (and enables it by default for
> newer boards).
>
> Determine whether the gamma correction setting feature is supported and
> add the ability to set it via sysfs attribute file.
>
> Signed-off-by: Marek Behún <kabel@xxxxxxxxxx>
> ---
> .../sysfs-class-led-driver-turris-omnia | 14 ++
> drivers/leds/leds-turris-omnia.c | 135 +++++++++++++++---
> 2 files changed, 132 insertions(+), 17 deletions(-)
>
> diff --git a/Documentation/ABI/testing/sysfs-class-led-driver-turris-omnia b/Documentation/ABI/testing/sysfs-class-led-driver-turris-omnia
> index c4d46970c1cf..369b4ae8be5f 100644
> --- a/Documentation/ABI/testing/sysfs-class-led-driver-turris-omnia
> +++ b/Documentation/ABI/testing/sysfs-class-led-driver-turris-omnia
> @@ -12,3 +12,17 @@ Description: (RW) On the front panel of the Turris Omnia router there is also
> able to change this setting from software.
>
> Format: %i
> +
> +What: /sys/class/leds/<led>/device/gamma_correction
> +Date: August 2023
> +KernelVersion: 6.6
> +Contact: Marek Behún <kabel@xxxxxxxxxx>
> +Description: (RW) Newer versions of the microcontroller firmware of the
> + Turris Omnia router support gamma correction for the RGB LEDs.
> + This feature can be enabled/disabled by writing to this file.
> +
> + If the feature is not supported because the MCU firmware is too
> + old, the file always reads as 0, and writing to the file results
> + in the EOPNOTSUPP error.
> +
> + Format: %i
> diff --git a/drivers/leds/leds-turris-omnia.c b/drivers/leds/leds-turris-omnia.c
> index 180b0cbeb92e..75cc7d2cf6d1 100644
> --- a/drivers/leds/leds-turris-omnia.c
> +++ b/drivers/leds/leds-turris-omnia.c
> @@ -15,17 +15,30 @@
> #define OMNIA_BOARD_LEDS 12
> #define OMNIA_LED_NUM_CHANNELS 3
>
> -#define CMD_LED_MODE 3
> -#define CMD_LED_MODE_LED(l) ((l) & 0x0f)
> -#define CMD_LED_MODE_USER 0x10
> +/* MCU controller commands at I2C address 0x2a */
> +#define OMNIA_MCU_I2C_ADDR 0x2a
>
> -#define CMD_LED_STATE 4
> -#define CMD_LED_STATE_LED(l) ((l) & 0x0f)
> -#define CMD_LED_STATE_ON 0x10
> +#define CMD_GET_STATUS_WORD 0x01
> +#define STS_FEATURES_SUPPORTED BIT(2)
>
> -#define CMD_LED_COLOR 5
> -#define CMD_LED_SET_BRIGHTNESS 7
> -#define CMD_LED_GET_BRIGHTNESS 8
> +#define CMD_GET_FEATURES 0x10
> +#define FEAT_LED_GAMMA_CORRECTION BIT(5)
> +
> +/* LED controller commands at I2C address 0x2b */
> +#define CMD_LED_MODE 0x03
> +#define CMD_LED_MODE_LED(l) ((l) & 0x0f)
> +#define CMD_LED_MODE_USER 0x10
> +
> +#define CMD_LED_STATE 0x04
> +#define CMD_LED_STATE_LED(l) ((l) & 0x0f)
> +#define CMD_LED_STATE_ON 0x10
> +
> +#define CMD_LED_COLOR 0x05
> +#define CMD_LED_SET_BRIGHTNESS 0x07
> +#define CMD_LED_GET_BRIGHTNESS 0x08
> +
> +#define CMD_SET_GAMMA_CORRECTION 0x30
> +#define CMD_GET_GAMMA_CORRECTION 0x31
>
> struct omnia_led {
> struct led_classdev_mc mc_cdev;
> @@ -40,6 +53,7 @@ struct omnia_led {
> struct omnia_leds {
> struct i2c_client *client;
> struct mutex lock;
> + bool has_gamma_correction;
> struct omnia_led leds[];
> };
>
> @@ -53,30 +67,42 @@ static int omnia_cmd_write(const struct i2c_client *client, u8 cmd, u8 val)
> return ret < 0 ? ret : 0;
> }
>
> -static int omnia_cmd_read(const struct i2c_client *client, u8 cmd)
> +static int omnia_cmd_read_raw(struct i2c_adapter *adapter, u8 addr, u8 cmd,
> + void *reply, size_t len)
> {
> struct i2c_msg msgs[2];
> - u8 reply;
> int ret;
>
> - msgs[0].addr = client->addr;
> + msgs[0].addr = addr;
> msgs[0].flags = 0;
> msgs[0].len = 1;
> msgs[0].buf = &cmd;
> - msgs[1].addr = client->addr;
> + msgs[1].addr = addr;
> msgs[1].flags = I2C_M_RD;
> - msgs[1].len = 1;
> - msgs[1].buf = &reply;
> + msgs[1].len = len;
> + msgs[1].buf = reply;
>
> - ret = i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));
> + ret = i2c_transfer(adapter, msgs, ARRAY_SIZE(msgs));
> if (likely(ret == ARRAY_SIZE(msgs)))
> - return reply;
> + return 0;
Why not return ret and use that throughout?
> else if (ret < 0)
> return ret;
> else
> return -EIO;
> }
[...]
--
Lee Jones [李琼斯]
