Turris Omnia LED driver changes
Subsystems (4)
Marek Behún
maintainercover Turris Omnia LED driver changes
Hello Lee,
this series is for 6.12, but it depends on changes that have been merged to 6.11-rc3. Your for-leds-next branch is based on 6.11-rc1, so it won't apply there.
This series does two things:
1. It drops the code for executing MCU commands from the LEDs driver and instead starts using the functions from the turris-omnia-mcu platform driver. This is so that we don't have the same code at two places.
2. It adds support for the global LED brightness change interrupt to the LED driver. This interrupt is exposed by the MCU driver. Userspace can poll for global LED brightness change (triggered by pressing the front button) by polling the corresponding sysfs file, /sys/class/leds/<led>/device/brightness, documented in Documentation/ABI/testing/sysfs-class-led-driver-turris-omnia.
Marek
Marek Behún (8):
turris-omnia-mcu-interface.h: Move command execution function to
global header
leds: turris-omnia: Use command execution functions from the MCU
driver
turris-omnia-mcu-interface.h: Add LED commands related definitions to
global header
leds: turris-omnia: Use global header for MCU command definitions
leds: turris-omnia: Notify sysfs on MCU global LEDs brightness change
platform: cznic: turris-omnia-mcu: Inform about missing LED panel
brightness change interrupt feature
leds: turris-omnia: Inform about missing LED gamma correction feature
in the MCU driver
ARM: dts: turris-omnia: Add global LED brightness change interrupt
.../dts/marvell/armada-385-turris-omnia.dts | 1 + drivers/leds/leds-turris-omnia.c | 228 +++++++++--------- .../platform/cznic/turris-omnia-mcu-base.c | 3 + drivers/platform/cznic/turris-omnia-mcu.h | 130 ---------- include/linux/turris-omnia-mcu-interface.h | 147 +++++++++++ 5 files changed, 259 insertions(+), 250 deletions(-)
Signature · 1 line
2.44.2
Marek Behún
maintainerPATCH 1/8 turris-omnia-mcu-interface.h: Move command execution function to global header
Move the command execution functions from the turris-omnia-mcu platform driver private header to the global turris-omnia-mcu-interface.h header, so that they can be used by the LED driver.
--- .../platform/cznic/turris-omnia-mcu-base.c | 1 + drivers/platform/cznic/turris-omnia-mcu.h | 130 ----------------- include/linux/turris-omnia-mcu-interface.h | 135 ++++++++++++++++++ 3 files changed, 136 insertions(+), 130 deletions(-)
drivers/platform/cznic/turris-omnia-mcu-base.c +1 -0
@@ -52,6 +52,7 @@ int omnia_cmd_write_read(const struct i2c_client *client,return 0 ; } +EXPORT_SYMBOL_GPL (omnia_cmd_write_read);static int omnia_get_version_hash (struct omnia_mcu *mcu,bool bootloader,char version[static OMNIA_FW_VERSION_HEX_LEN ])
drivers/platform/cznic/turris-omnia-mcu.h +0 -130
148 lines not on this page · raw message
include/linux/turris-omnia-mcu-interface.h +135 -0
146 lines not on this page · raw message
Signature · 1 line
2.44.2
Re: [PATCH leds 1/8] turris-omnia-mcu-interface.h: Move command execution function to global header
Quoted 3 lines · Marek Behún
Move the command execution functions from the turris-omnia-mcu platform driver private header to the global
turris-omnia-mcu-interface.hheader, so that they can be used by the LED driver.
...
Quoted 4 lines
#include <
linux/bitfield.h>
+#include <linux/bitops.h>#include <
linux/bits.h>
You may drop this one now, as guaranteed by bitops.h.
Quoted 3 lines
include/linux/turris-omnia-mcu-interface.h +3
-0+#include <linux/types.h> +#include <asm/byteorder.h> +#include <asm/unaligned.h>
Signature · 2 lines
With Best Regards, Andy Shevchenko
Marek Behún
maintainerRe: [PATCH leds 1/8] turris-omnia-mcu-interface.h: Move command execution function to global header
Quoted 5 lines · Andy Shevchenko
Quoted 3 lines · Marek Behún
Move the command execution functions from the turris-omnia-mcu platform driver private header to the global
turris-omnia-mcu-interface.hheader, so that they can be used by the LED driver.
Quoted 8 lines
...
Quoted 4 lines
#include <
linux/bitfield.h>
+#include <linux/bitops.h>#include <
linux/bits.h>You may drop this one now, as guaranteed by
bitops.h.
Hi Andy,
How do I know that including bit.sh won't be ever deleted from bitops.h?
Is there some database which symbols are supposed to be provided by which header, and not just randomly included from another header?
Marek
Re: [PATCH leds 1/8] turris-omnia-mcu-interface.h: Move command execution function to global header
Quoted 2 lines · Marek Behún
Quoted 1 line · Andy Shevchenko
On Mon, Sep 02, 2024 at 02:40:57PM +0200, Marek Behún wrote:
...
Quoted 9 lines
Quoted 6 lines
Quoted 4 lines
#include <
linux/bitfield.h>
+#include <linux/bitops.h>#include <
linux/bits.h>You may drop this one now, as guaranteed by
bitops.h.How do I know that including
bit.shwon't be ever deleted frombitops.h?
How do I?
Somebody should start writing documentation (and tooling?) for all this header dependency hell we have in the Linux kernel project.
Quoted 2 lines
Is there some database which symbols are supposed to be provided by which header, and not just randomly included from another header?
Bottom line, more explicit inclusions are fine, but here is a proverb:
"make a fool pray to God, he will break his forehead."
Means that either radical approach (none or all) is not good.
Signature · 2 lines
With Best Regards, Andy Shevchenko
Marek Behún
maintainerPATCH 2/8 leds: turris-omnia: Use command execution functions from the MCU driver
In reply to Marek BehúnUse the MCU command execution functions from the MCU driver instead of the ad-hoc implementation in the LED driver. This allows as to drop the LED driver implementation, which is a duplicate.
--- drivers/leds/leds-turris-omnia.c | 88 ++++++++------------------------ 1 file changed, 22 insertions(+), 66 deletions(-)
drivers/leds/leds-turris-omnia.c +22 -66
@@ -2,7 +2,7 @@/* * CZ.NIC's Turris Omnia LEDs driver * -* 2020, 2023 by Marek Behún <[email protected]> +* 2020, 2023, 2024 by Marek Behún <[email protected]> */ #include <linux/i2c.h>
@@ -10,6 +10,7 @@#include <linux/module.h> #include <linux/mutex.h> #include <linux/of.h> +#include <linux/turris-omnia-mcu-interface.h> #include "leds.h" #define OMNIA_BOARD_LEDS 12
@@ -57,52 +58,6 @@ 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 }; -int ret; - - ret =i2c_master_send (client, buf,sizeof (buf)); - -return ret <0 ? ret :0 ; -} - -static int omnia_cmd_read_raw (struct i2c_adapter *adapter,u8 addr,u8 cmd, -void *reply,size_t len) -{ -struct i2c_msg msgs[2 ]; -int ret; - - msgs[0 ].addr = addr; - msgs[0 ].flags =0 ; - msgs[0 ].len =1 ; - msgs[0 ].buf = &cmd; - msgs[1 ].addr = addr; - msgs[1 ].flags =I2C_M_RD ; - msgs[1 ].len = len; - msgs[1 ].buf = reply; - - ret =i2c_transfer (adapter, msgs,ARRAY_SIZE (msgs)); -if (likely (ret ==ARRAY_SIZE (msgs))) -return 0 ; -else if (ret <0 ) -return ret; -else -return -EIO ; -} - -static int omnia_cmd_read_u8 (const struct i2c_client *client,u8 cmd) -{ -u8 reply; -int err; - - err =omnia_cmd_read_raw (client->adapter, client->addr, cmd, &reply,1 ); -if (err) -return err; - -return reply; -} -static int omnia_led_send_color_cmd (const struct i2c_client *client,struct omnia_led *led) {
@@ -352,14 +307,14 @@ static ssize_t brightness_show(struct device *dev, struct device_attribute *a, char *buf) {struct i2c_client *client =to_i2c_client (dev); -int ret; - - ret =omnia_cmd_read_u8 (client,CMD_LED_GET_BRIGHTNESS ); +u8 reply; +int err; -if (ret <0 ) -return ret; + err =omnia_cmd_read_u8 (client,CMD_LED_GET_BRIGHTNESS , &reply); +if (err <0 ) +return err; -return sysfs_emit (buf,"%d\n" , ret); +return sysfs_emit (buf,"%d\n" , reply); } static ssize_t brightness_store(struct device *dev, struct device_attribute *a,
@@ -386,17 +341,16 @@ static ssize_t gamma_correction_show(struct device *dev, {struct i2c_client *client =to_i2c_client (dev);struct omnia_leds *leds =i2c_get_clientdata (client); -int ret; +u8 reply =0 ; +int err;if (leds->has_gamma_correction) { - ret =omnia_cmd_read_u8 (client,CMD_GET_GAMMA_CORRECTION ); -if (ret <0 ) -return ret; - }else { - ret =0 ; + err =omnia_cmd_read_u8 (client,CMD_GET_GAMMA_CORRECTION , &reply); +if (err <0 ) +return err; } -return sysfs_emit (buf,"%d\n" , !!ret); +return sysfs_emit (buf,"%d\n" , !!reply); } static ssize_t gamma_correction_store(struct device *dev,
@@ -429,24 +383,26 @@ ATTRIBUTE_GROUPS(omnia_led_controller);static int omnia_mcu_get_features (const struct i2c_client *client) { +struct i2c_client mcu_client = *client;u16 reply;int err; - err =omnia_cmd_read_raw (client->adapter,OMNIA_MCU_I2C_ADDR , -CMD_GET_STATUS_WORD , &reply,sizeof (reply)); +/* We have to read features from different I2C address */ + mcu_client.addr =OMNIA_MCU_I2C_ADDR ; + + err =omnia_cmd_read_u16 (&mcu_client,CMD_GET_STATUS_WORD , &reply);if (err)return err;/* Check whether MCU firmware supports the CMD_GET_FEAUTRES command */ -if (!(le16_to_cpu (reply) &STS_FEATURES_SUPPORTED )) +if (!(reply &STS_FEATURES_SUPPORTED ))return 0 ; - err =omnia_cmd_read_raw (client->adapter,OMNIA_MCU_I2C_ADDR , -CMD_GET_FEATURES , &reply,sizeof (reply)); + err =omnia_cmd_read_u16 (&mcu_client,CMD_GET_FEATURES , &reply);if (err)return err; -return le16_to_cpu (reply); +return reply; }static int omnia_leds_probe (struct i2c_client *client)
Signature · 1 line
2.44.2
Marek Behún
maintainerPATCH 3/8 turris-omnia-mcu-interface.h: Add LED commands related definitions to global header
In reply to Marek BehúnAdd definitions for contents of the OMNIA_CMD_LED_MODE and OMNIA_CMD_LED_STATE commands to the global turris-omnia-mcu-interface.h header.
--- include/linux/turris-omnia-mcu-interface.h | 12 ++++++++++++ 1 file changed, 12 insertions(+)
include/linux/turris-omnia-mcu-interface.h +12 -0
@@ -240,6 +240,18 @@ enum omnia_int_e { OMNIA_INT_LAN5_LED1 = BIT(31), }; +enum omnia_cmd_led_mode_e { +OMNIA_CMD_LED_MODE_LED_MASK =GENMASK (3 ,0 ), +#define OMNIA_CMD_LED_MODE_LED(_l) FIELD_PREP(OMNIA_CMD_LED_MODE_LED_MASK, _l) + OMNIA_CMD_LED_MODE_USER = BIT(4), +}; + +enum omnia_cmd_led_state_e { +OMNIA_CMD_LED_STATE_LED_MASK =GENMASK (3 ,0 ), +#define OMNIA_CMD_LED_STATE_LED(_l) FIELD_PREP(OMNIA_CMD_LED_STATE_LED_MASK, _l) + OMNIA_CMD_LED_STATE_ON = BIT(4), +}; +enum omnia_cmd_poweroff_e {OMNIA_CMD_POWER_OFF_POWERON_BUTTON =BIT (0 ),OMNIA_CMD_POWER_OFF_MAGIC =0xdead ,
Signature · 1 line
2.44.2
Marek Behún
maintainerPATCH 4/8 leds: turris-omnia: Use global header for MCU command definitions
In reply to Marek BehúnThe global turris-omnia-mcu-interface.h header file contains the definitions for MCU commands. Drop the driver-internal definitions and use the global ones.
--- drivers/leds/leds-turris-omnia.c | 78 ++++++++++++-------------------- 1 file changed, 28 insertions(+), 50 deletions(-)
drivers/leds/leds-turris-omnia.c +28 -50
@@ -16,30 +16,8 @@#define OMNIA_BOARD_LEDS 12#define OMNIA_LED_NUM_CHANNELS 3 -/* MCU controller commands at I2C address 0x2a */ -#define OMNIA_MCU_I2C_ADDR 0x2a - -#define CMD_GET_STATUS_WORD 0x01 -#define STS_FEATURES_SUPPORTED BIT(2) - -#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 +/* MCU controller I2C address 0x2a, needed for detecting MCU features */ +#define OMNIA_MCU_I2C_ADDR 0x2astruct omnia_led {struct led_classdev_mc mc_cdev;
@@ -64,7 +42,7 @@ static int omnia_led_send_color_cmd(const struct i2c_client *client,char cmd[5 ];int ret; - cmd[0 ] =CMD_LED_COLOR ; + cmd[0 ] =OMNIA_CMD_LED_COLOR ; cmd[1 ] = led->reg; cmd[2 ] = led->subled_info[0 ].brightness; cmd[3 ] = led->subled_info[1 ].brightness;
@@ -126,12 +104,12 @@ static int omnia_led_brightness_set_blocking(struct led_classdev *cdev,* is not being blinked by HW. */ if (!err && !led->hwtrig && !brightness != !led->on) { -u8 state =CMD_LED_STATE_LED (led->reg); +u8 state =OMNIA_CMD_LED_STATE_LED (led->reg);if (brightness) - state |=CMD_LED_STATE_ON ; + state |=OMNIA_CMD_LED_STATE_ON ; - err =omnia_cmd_write_u8 (leds->client,CMD_LED_STATE , state); + err =omnia_cmd_write_u8 (leds->client,OMNIA_CMD_LED_STATE , state);if (!err) led->on = !!brightness; }
@@ -166,8 +144,8 @@ static int omnia_hwtrig_activate(struct led_classdev *cdev)if (!err) {/* Put the LED into MCU controlled mode */ - err =omnia_cmd_write_u8 (leds->client,CMD_LED_MODE , -CMD_LED_MODE_LED (led->reg)); + err =omnia_cmd_write_u8 (leds->client,OMNIA_CMD_LED_MODE , +OMNIA_CMD_LED_MODE_LED (led->reg));if (!err) led->hwtrig = true; }
@@ -188,9 +166,9 @@ static void omnia_hwtrig_deactivate(struct led_classdev *cdev) led->hwtrig = false;/* Put the LED into software mode */ - err =omnia_cmd_write_u8 (leds->client,CMD_LED_MODE , -CMD_LED_MODE_LED (led->reg) | -CMD_LED_MODE_USER ); + err =omnia_cmd_write_u8 (leds->client,OMNIA_CMD_LED_MODE , +OMNIA_CMD_LED_MODE_LED (led->reg) | +OMNIA_CMD_LED_MODE_USER );mutex_unlock (&leds->lock);
@@ -257,9 +235,9 @@ static int omnia_led_register(struct i2c_client *client, struct omnia_led *led, cdev->default_trigger = omnia_hw_trigger.name;/* put the LED into software mode */ - ret =omnia_cmd_write_u8 (client,CMD_LED_MODE , -CMD_LED_MODE_LED (led->reg) | -CMD_LED_MODE_USER ); + ret =omnia_cmd_write_u8 (client,OMNIA_CMD_LED_MODE , +OMNIA_CMD_LED_MODE_LED (led->reg) | +OMNIA_CMD_LED_MODE_USER );if (ret) {dev_err (dev,"Cannot set LED %pOF to software mode: %i\n" , np, ret);
@@ -267,8 +245,8 @@ static int omnia_led_register(struct i2c_client *client, struct omnia_led *led, }/* disable the LED */ - ret =omnia_cmd_write_u8 (client,CMD_LED_STATE , -CMD_LED_STATE_LED (led->reg)); + ret =omnia_cmd_write_u8 (client,OMNIA_CMD_LED_STATE , +OMNIA_CMD_LED_STATE_LED (led->reg));if (ret) {dev_err (dev,"Cannot set LED %pOF brightness: %i\n" , np, ret);return ret;
@@ -310,7 +288,7 @@ static ssize_t brightness_show(struct device *dev, struct device_attribute *a,u8 reply;int err; - err =omnia_cmd_read_u8 (client,CMD_LED_GET_BRIGHTNESS , &reply); + err =omnia_cmd_read_u8 (client,OMNIA_CMD_GET_BRIGHTNESS , &reply);if (err <0 )return err;
@@ -330,7 +308,7 @@ static ssize_t brightness_store(struct device *dev, struct device_attribute *a,if (brightness >100 )return -EINVAL ; - err =omnia_cmd_write_u8 (client,CMD_LED_SET_BRIGHTNESS , brightness); + err =omnia_cmd_write_u8 (client,OMNIA_CMD_SET_BRIGHTNESS , brightness);return err ?: count; }
@@ -345,7 +323,7 @@ static ssize_t gamma_correction_show(struct device *dev,int err;if (leds->has_gamma_correction) { - err =omnia_cmd_read_u8 (client,CMD_GET_GAMMA_CORRECTION , &reply); + err =omnia_cmd_read_u8 (client,OMNIA_CMD_GET_GAMMA_CORRECTION , &reply);if (err <0 )return err; }
@@ -368,7 +346,7 @@ static ssize_t gamma_correction_store(struct device *dev,if (kstrtobool (buf, &val) <0 )return -EINVAL ; - err =omnia_cmd_write_u8 (client,CMD_SET_GAMMA_CORRECTION , val); + err =omnia_cmd_write_u8 (client,OMNIA_CMD_SET_GAMMA_CORRECTION , val);return err ?: count; }
@@ -390,15 +368,15 @@ static int omnia_mcu_get_features(const struct i2c_client *client)/* We have to read features from different I2C address */ mcu_client.addr =OMNIA_MCU_I2C_ADDR ; - err =omnia_cmd_read_u16 (&mcu_client,CMD_GET_STATUS_WORD , &reply); + err =omnia_cmd_read_u16 (&mcu_client,OMNIA_CMD_GET_STATUS_WORD , &reply);if (err)return err; -/* Check whether MCU firmware supports the CMD_GET_FEAUTRES command */ -if (!(reply &STS_FEATURES_SUPPORTED )) +/* Check whether MCU firmware supports the OMNIA_CMD_GET_FEAUTRES command */ +if (!(reply &OMNIA_STS_FEATURES_SUPPORTED ))return 0 ; - err =omnia_cmd_read_u16 (&mcu_client,CMD_GET_FEATURES , &reply); + err =omnia_cmd_read_u16 (&mcu_client,OMNIA_CMD_GET_FEATURES , &reply);if (err)return err;
@@ -436,7 +414,7 @@ static int omnia_leds_probe(struct i2c_client *client)return ret; } - leds->has_gamma_correction = ret &FEAT_LED_GAMMA_CORRECTION ; + leds->has_gamma_correction = ret &OMNIA_FEAT_LED_GAMMA_CORRECTION ;if (!leds->has_gamma_correction) {dev_info (dev,"Your board's MCU firmware does not support the LED gamma correction feature.\n" );
@@ -469,11 +447,11 @@ static void omnia_leds_remove(struct i2c_client *client)u8 buf[5 ];/* put all LEDs into default (HW triggered) mode */ -omnia_cmd_write_u8 (client,CMD_LED_MODE , -CMD_LED_MODE_LED (OMNIA_BOARD_LEDS )); +omnia_cmd_write_u8 (client,OMNIA_CMD_LED_MODE , +OMNIA_CMD_LED_MODE_LED (OMNIA_BOARD_LEDS ));/* set all LEDs color to [255, 255, 255] */ - buf[0 ] =CMD_LED_COLOR ; + buf[0 ] =OMNIA_CMD_LED_COLOR ; buf[1 ] =OMNIA_BOARD_LEDS ; buf[2 ] =255 ; buf[3 ] =255 ;
Signature · 1 line
2.44.2
Marek Behún
maintainerPATCH 5/8 leds: turris-omnia: Notify sysfs on MCU global LEDs brightness change
In reply to Marek BehúnRecall that on Turris Omnia, the LED controller has a global brightness property, which allows the user to make the front LED panel dimmer.
There is also a button on the front panel, which by default is configured so that pressing it changes the global brightness to a lower value (unless it is at 0%, in which case pressing the button changes the global brightness to 100%).
Newer versions of the MCU firmware support informing the SOC that the brightness was changed by button press event via an interrupt.
Now that we have the turris-omnia-mcu driver, which adds support for MCU interrupts, add the ability to inform the userspace (via a sysfs notification) that the global brightness was changed.
--- drivers/leds/leds-turris-omnia.c | 67 ++++++++++++++++++++++++++++++-- 1 file changed, 64 insertions(+), 3 deletions(-)
drivers/leds/leds-turris-omnia.c +64 -3
@@ -357,7 +357,57 @@ static struct attribute *omnia_led_controller_attrs[] = { &dev_attr_gamma_correction.attr, NULL, }; -ATTRIBUTE_GROUPS (omnia_led_controller); + +static const struct attribute_group omnia_led_controller_group = { + .attrs = omnia_led_controller_attrs, +}; + +static irqreturn_t omnia_brightness_changed_handler (int irq,void *dev_id) +{ +struct kernfs_node *brightness_kn = dev_id; + +sysfs_notify_dirent (brightness_kn); + +return IRQ_HANDLED ; +} + +static void brightness_kn_release (struct device *dev,void *res) +{ +struct kernfs_node **brightness_kn = res; + +sysfs_put (*brightness_kn); +} + +static int omnia_probe_brightness_interrupt (struct i2c_client *client) +{ +struct kernfs_node **brightness_kn; +struct device *dev = &client->dev; +int ret; + + brightness_kn =devres_alloc (brightness_kn_release, +sizeof (*brightness_kn),GFP_KERNEL ); +if (!brightness_kn) +return -ENOMEM ; + + *brightness_kn =sysfs_get_dirent (dev->kobj.sd,"brightness" ); +if (!*brightness_kn) { +devres_free (brightness_kn); +return -EIO ; + } + +devres_add (dev, brightness_kn); + + ret =devm_request_any_context_irq (dev, client->irq, + omnia_brightness_changed_handler, +IRQF_ONESHOT ,"leds-turris-omnia" , + *brightness_kn); +if (ret <0 ) { +dev_err (dev,"Cannot request IRQ: %d\n" , ret); +return ret; + } + +return 0 ; +}static int omnia_mcu_get_features (const struct i2c_client *client) {
@@ -387,6 +437,7 @@ static int omnia_leds_probe(struct i2c_client *client) {struct device *dev = &client->dev;struct device_node *np =dev_of_node (dev); +bool has_brightness_interrupt;struct omnia_leds *leds;struct omnia_led *led;int ret, count;
@@ -414,6 +465,8 @@ static int omnia_leds_probe(struct i2c_client *client)return ret; } + has_brightness_interrupt = ret &OMNIA_FEAT_BRIGHTNESS_INT ; + leds->has_gamma_correction = ret &OMNIA_FEAT_LED_GAMMA_CORRECTION ;if (!leds->has_gamma_correction) { dev_info(dev,
@@ -439,7 +492,16 @@ static int omnia_leds_probe(struct i2c_client *client) led += ret; } -return 0 ; + ret =devm_device_add_group (dev, &omnia_led_controller_group); +if (ret <0 ) { +dev_err (dev,"Cannot add sysfs attribute group: %d\n" , ret); +return ret; + } + +if (has_brightness_interrupt) + ret =omnia_probe_brightness_interrupt (client); + +return ret; }static void omnia_leds_remove (struct i2c_client *client)
@@ -479,7 +541,6 @@ static struct i2c_driver omnia_leds_driver = { .driver = { .name = "leds-turris-omnia", .of_match_table = of_omnia_leds_match, - .dev_groups = omnia_led_controller_groups, }, };
Signature · 1 line
2.44.2
Re: [PATCH leds 5/8] leds: turris-omnia: Notify sysfs on MCU global LEDs brightness change
Quoted 14 lines · Marek Behún
Recall that on Turris Omnia, the LED controller has a global brightness property, which allows the user to make the front LED panel dimmer.
There is also a button on the front panel, which by default is configured so that pressing it changes the global brightness to a lower value (unless it is at 0%, in which case pressing the button changes the global brightness to 100%).
Newer versions of the MCU firmware support informing the SOC that the brightness was changed by button press event via an interrupt.
Now that we have the turris-omnia-mcu driver, which adds support for MCU interrupts, add the ability to inform the userspace (via a sysfs notification) that the global brightness was changed.
...
drivers/leds/leds-turris-omnia.c static int omnia_leds_probe(struct i2c_client *client) · line 495
Quoted 1 line
drivers/leds/leds-turris-omnia.c +1
-0+ ret =devm_device_add_group (dev, &omnia_led_controller_group);
AFAIU the intention is to remove that API, hence this shall not be used. Greg?
https://lore.kernel.org/lkml/[email protected]/
Signature · 2 lines
With Best Regards, Andy Shevchenko
Marek Behún
maintainerRe: [PATCH leds 5/8] leds: turris-omnia: Notify sysfs on MCU global LEDs brightness change
Quoted 16 lines · Andy Shevchenko
Quoted 8 lines · Marek Behún
Recall that on Turris Omnia, the LED controller has a global brightness property, which allows the user to make the front LED panel dimmer.
There is also a button on the front panel, which by default is configured so that pressing it changes the global brightness to a lower value (unless it is at 0%, in which case pressing the button changes the global brightness to 100%).
Quoted 6 lines
Newer versions of the MCU firmware support informing the SOC that the brightness was changed by button press event via an interrupt.
Now that we have the turris-omnia-mcu driver, which adds support for MCU interrupts, add the ability to inform the userspace (via a sysfs notification) that the global brightness was changed.
Quoted 6 lines
...
drivers/leds/leds-turris-omnia.c static int omnia_leds_probe(struct i2c_client *client) · line 495
Quoted 1 line
drivers/leds/leds-turris-omnia.c +1
-0+ ret =devm_device_add_group (dev, &omnia_led_controller_group);AFAIU the intention is to remove that API, hence this shall not be used. Greg?
OMG yes this should be with the group .is_visible method. Sorry.
Marek Behún
maintainerPATCH 6/8 platform: cznic: turris-omnia-mcu: Inform about missing LED panel brightness change interrupt feature
In reply to Marek BehúnWhen reading MCU firmware features, check also for the LED panel brightness change interrupt feature, and suggest upgrading the firmware if it is missing.
--- drivers/platform/cznic/turris-omnia-mcu-base.c | 1 + 1 file changed, 1 insertion(+)
drivers/platform/cznic/turris-omnia-mcu-base.c +1 -0
@@ -258,6 +258,7 @@ static int omnia_mcu_read_features(struct omnia_mcu *mcu)_DEF_FEAT (NEW_INT_API ,"new interrupt API" ),_DEF_FEAT (POWEROFF_WAKEUP ,"poweroff and wakeup" ),_DEF_FEAT (TRNG ,"true random number generator" ), +_DEF_FEAT (BRIGHTNESS_INT ,"LED panel brightness change interrupt" ), #undef _DEF_FEAT };struct i2c_client *client = mcu->client;
Signature · 1 line
2.44.2
Marek Behún
maintainerPATCH 7/8 leds: turris-omnia: Inform about missing LED gamma correction feature in the MCU driver
In reply to Marek BehúnIf the LED gamma correction feature is missing in the MCU firmware, inform about this in the MCU firmware probe function instead of LED driver probe function, so that all the feature checks are in one place.
--- drivers/leds/leds-turris-omnia.c | 9 +-------- drivers/platform/cznic/turris-omnia-mcu-base.c | 1 + 2 files changed, 2 insertions(+), 8 deletions(-)
drivers/leds/leds-turris-omnia.c +1 -8
@@ -465,15 +465,8 @@ static int omnia_leds_probe(struct i2c_client *client)return ret; } - has_brightness_interrupt = ret &OMNIA_FEAT_BRIGHTNESS_INT ; - leds->has_gamma_correction = ret &OMNIA_FEAT_LED_GAMMA_CORRECTION ; -if (!leds->has_gamma_correction) { -dev_info (dev, -"Your board's MCU firmware does not support the LED gamma correction feature.\n" ); -dev_info (dev, -"Consider upgrading MCU firmware with the omnia-mcutool utility.\n" ); - } + has_brightness_interrupt = ret &OMNIA_FEAT_BRIGHTNESS_INT ;mutex_init (&leds->lock);
drivers/platform/cznic/turris-omnia-mcu-base.c +1 -0
@@ -259,6 +259,7 @@ static int omnia_mcu_read_features(struct omnia_mcu *mcu)_DEF_FEAT (POWEROFF_WAKEUP ,"poweroff and wakeup" ),_DEF_FEAT (TRNG ,"true random number generator" ),_DEF_FEAT (BRIGHTNESS_INT ,"LED panel brightness change interrupt" ), +_DEF_FEAT (LED_GAMMA_CORRECTION ,"LED gamma correction" ), #undef _DEF_FEAT };struct i2c_client *client = mcu->client;
Signature · 1 line
2.44.2
Marek Behún
maintainerPATCH 8/8 ARM: dts: turris-omnia: Add global LED brightness change interrupt
In reply to Marek BehúnWhen global LED brightness is changed by pressing the front button on Turris Omnia, the MCU can produce an interrupt to the CPU. Add the description of this interrupt to the LED controller node.
--- arch/arm/boot/dts/marvell/armada-385-turris-omnia.dts | 1 + 1 file changed, 1 insertion(+)
arch/arm/boot/dts/marvell/armada-385-turris-omnia.dts +1 -0
@@ -251,6 +251,7 @@ mcu: system-controller@2a { led-controller@2b { compatible = "cznic,turris-omnia-leds"; reg = <0x2b>; + interrupts-extended = <&mcu 11 IRQ_TYPE_NONE>; #address-cells = <1>; #size-cells = <0>; status = "okay";
Signature · 1 line
2.44.2