| Message ID | 20241104141924.18816-8-kabel@kernel.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Turris Omnia LED driver changes | expand |
Hi Marek,
kernel test robot noticed the following build errors:
[auto build test ERROR on linus/master]
[also build test ERROR on v6.12-rc6]
[cannot apply to lee-leds/for-leds-next robh/for-next next-20241104]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Marek-Beh-n/turris-omnia-mcu-interface-h-Move-command-execution-function-to-global-header/20241104-223435
base: linus/master
patch link: https://lore.kernel.org/r/20241104141924.18816-8-kabel%40kernel.org
patch subject: [PATCH leds v5 07/12] leds: turris-omnia: Notify sysfs on MCU global LEDs brightness change
config: arm-randconfig-003-20241105 (https://download.01.org/0day-ci/archive/20241105/202411051138.jzDE6sBH-lkp@intel.com/config)
compiler: arm-linux-gnueabi-gcc (GCC) 14.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241105/202411051138.jzDE6sBH-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202411051138.jzDE6sBH-lkp@intel.com/
All errors (new ones prefixed by >>):
arm-linux-gnueabi-ld: drivers/leds/leds-turris-omnia.o: in function `omnia_cmd_set_color':
>> include/linux/turris-omnia-mcu-interface.h:275:(.text+0x5a): undefined reference to `omnia_cmd_write_read'
arm-linux-gnueabi-ld: drivers/leds/leds-turris-omnia.o: in function `omnia_cmd_read_u16':
include/linux/turris-omnia-mcu-interface.h:311:(.text+0xaa): undefined reference to `omnia_cmd_write_read'
arm-linux-gnueabi-ld: drivers/leds/leds-turris-omnia.o: in function `gamma_correction_show':
include/linux/turris-omnia-mcu-interface.h:311:(.text+0x1a2): undefined reference to `omnia_cmd_write_read'
arm-linux-gnueabi-ld: drivers/leds/leds-turris-omnia.o: in function `omnia_cmd_write_u8':
include/linux/turris-omnia-mcu-interface.h:275:(.text+0x1c8): undefined reference to `omnia_cmd_write_read'
arm-linux-gnueabi-ld: drivers/leds/leds-turris-omnia.o: in function `brightness_show':
include/linux/turris-omnia-mcu-interface.h:311:(.text+0x4c2): undefined reference to `omnia_cmd_write_read'
vim +275 include/linux/turris-omnia-mcu-interface.h
9f74fe5691025f Marek Behún 2024-11-04 267
9f74fe5691025f Marek Behún 2024-11-04 268 int omnia_cmd_write_read(const struct i2c_client *client,
9f74fe5691025f Marek Behún 2024-11-04 269 void *cmd, unsigned int cmd_len,
9f74fe5691025f Marek Behún 2024-11-04 270 void *reply, unsigned int reply_len);
9f74fe5691025f Marek Behún 2024-11-04 271
9f74fe5691025f Marek Behún 2024-11-04 272 static inline int omnia_cmd_write(const struct i2c_client *client, void *cmd,
9f74fe5691025f Marek Behún 2024-11-04 273 unsigned int len)
9f74fe5691025f Marek Behún 2024-11-04 274 {
9f74fe5691025f Marek Behún 2024-11-04 @275 return omnia_cmd_write_read(client, cmd, len, NULL, 0);
9f74fe5691025f Marek Behún 2024-11-04 276 }
9f74fe5691025f Marek Behún 2024-11-04 277
On Tue, 05 Nov 2024, kernel test robot wrote: > Hi Marek, > > kernel test robot noticed the following build errors: > > [auto build test ERROR on linus/master] > [also build test ERROR on v6.12-rc6] > [cannot apply to lee-leds/for-leds-next robh/for-next next-20241104] > [If your patch is applied to the wrong git tree, kindly drop us a note. > And when submitting patch, we suggest to use '--base' as documented in > https://git-scm.com/docs/git-format-patch#_base_tree_information] > > url: https://github.com/intel-lab-lkp/linux/commits/Marek-Beh-n/turris-omnia-mcu-interface-h-Move-command-execution-function-to-global-header/20241104-223435 > base: linus/master > patch link: https://lore.kernel.org/r/20241104141924.18816-8-kabel%40kernel.org > patch subject: [PATCH leds v5 07/12] leds: turris-omnia: Notify sysfs on MCU global LEDs brightness change > config: arm-randconfig-003-20241105 (https://download.01.org/0day-ci/archive/20241105/202411051138.jzDE6sBH-lkp@intel.com/config) > compiler: arm-linux-gnueabi-gcc (GCC) 14.1.0 > reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241105/202411051138.jzDE6sBH-lkp@intel.com/reproduce) > > If you fix the issue in a separate patch/commit (i.e. not just a new version of > the same patch/commit), kindly add following tags > | Reported-by: kernel test robot <lkp@intel.com> > | Closes: https://lore.kernel.org/oe-kbuild-all/202411051138.jzDE6sBH-lkp@intel.com/ > > All errors (new ones prefixed by >>): > > arm-linux-gnueabi-ld: drivers/leds/leds-turris-omnia.o: in function `omnia_cmd_set_color': > >> include/linux/turris-omnia-mcu-interface.h:275:(.text+0x5a): undefined reference to `omnia_cmd_write_read' > arm-linux-gnueabi-ld: drivers/leds/leds-turris-omnia.o: in function `omnia_cmd_read_u16': > include/linux/turris-omnia-mcu-interface.h:311:(.text+0xaa): undefined reference to `omnia_cmd_write_read' > arm-linux-gnueabi-ld: drivers/leds/leds-turris-omnia.o: in function `gamma_correction_show': > include/linux/turris-omnia-mcu-interface.h:311:(.text+0x1a2): undefined reference to `omnia_cmd_write_read' > arm-linux-gnueabi-ld: drivers/leds/leds-turris-omnia.o: in function `omnia_cmd_write_u8': > include/linux/turris-omnia-mcu-interface.h:275:(.text+0x1c8): undefined reference to `omnia_cmd_write_read' > arm-linux-gnueabi-ld: drivers/leds/leds-turris-omnia.o: in function `brightness_show': > include/linux/turris-omnia-mcu-interface.h:311:(.text+0x4c2): undefined reference to `omnia_cmd_write_read' > > > vim +275 include/linux/turris-omnia-mcu-interface.h > > 9f74fe5691025f Marek Behún 2024-11-04 267 > 9f74fe5691025f Marek Behún 2024-11-04 268 int omnia_cmd_write_read(const struct i2c_client *client, > 9f74fe5691025f Marek Behún 2024-11-04 269 void *cmd, unsigned int cmd_len, > 9f74fe5691025f Marek Behún 2024-11-04 270 void *reply, unsigned int reply_len); > 9f74fe5691025f Marek Behún 2024-11-04 271 > 9f74fe5691025f Marek Behún 2024-11-04 272 static inline int omnia_cmd_write(const struct i2c_client *client, void *cmd, > 9f74fe5691025f Marek Behún 2024-11-04 273 unsigned int len) > 9f74fe5691025f Marek Behún 2024-11-04 274 { > 9f74fe5691025f Marek Behún 2024-11-04 @275 return omnia_cmd_write_read(client, cmd, len, NULL, 0); > 9f74fe5691025f Marek Behún 2024-11-04 276 } > 9f74fe5691025f Marek Behún 2024-11-04 277 Is this real or a false positive?
On Mon, 04 Nov 2024, Marek Behún wrote: > 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. > > Signed-off-by: Marek Behún <kabel@kernel.org> > --- > drivers/leds/Kconfig | 2 +- > drivers/leds/leds-turris-omnia.c | 48 ++++++++++++++++++++++++++++++++ > 2 files changed, 49 insertions(+), 1 deletion(-) > > diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig > index fcbe93b61e49..148384aacdcc 100644 > --- a/drivers/leds/Kconfig > +++ b/drivers/leds/Kconfig > @@ -217,7 +217,7 @@ config LEDS_TURRIS_OMNIA > depends on I2C > depends on MACH_ARMADA_38X || COMPILE_TEST > depends on OF > - depends on TURRIS_OMNIA_MCU > + depends on TURRIS_OMNIA_MCU_GPIO > select LEDS_TRIGGERS > help > This option enables basic support for the LEDs found on the front > diff --git a/drivers/leds/leds-turris-omnia.c b/drivers/leds/leds-turris-omnia.c > index 7d2ed0c6500a..168ce362fd14 100644 > --- a/drivers/leds/leds-turris-omnia.c > +++ b/drivers/leds/leds-turris-omnia.c > @@ -43,12 +43,15 @@ struct omnia_led { > * @client: I2C client device > * @lock: mutex to protect > * @has_gamma_correction: whether the MCU firmware supports gamma correction > + * @brightness_knode: kernel node of the "brightness" device sysfs attribute (this is the > + * driver specific global brightness, not the LED classdev brightness) > * @leds: flexible array of per-LED data > */ > struct omnia_leds { > struct i2c_client *client; > struct mutex lock; > bool has_gamma_correction; > + struct kernfs_node *brightness_knode; > struct omnia_led leds[]; > }; > > @@ -373,6 +376,30 @@ static struct attribute *omnia_led_controller_attrs[] = { > }; > ATTRIBUTE_GROUPS(omnia_led_controller); > > +static irqreturn_t omnia_brightness_changed_threaded_fn(int irq, void *data) > +{ > + struct omnia_leds *leds = data; > + > + if (unlikely(!leds->brightness_knode)) { > + /* > + * It would be nicer to get this dirent in the driver probe method, before the IRQ > + * is requested. But the really_probe() function in drivers/base/dd.c registers > + * driver's .dev_groups only after probe is finished, so during driver probe the > + * "brightness" sysfs node is not yet present. Right, but this is known and therefore never called from probe making this comment superfluous. Either do something about it or remove the comment and carry-on working with what you have. :) > + * > + * Note that sysfs_get_dirent() may sleep. This is okay, because we are in threaded > + * context. > + */ > + leds->brightness_knode = sysfs_get_dirent(leds->client->dev.kobj.sd, "brightness"); > + if (!leds->brightness_knode) > + return IRQ_NONE; > + } > + > + sysfs_notify_dirent(leds->brightness_knode); > + > + return IRQ_HANDLED; > +} > + > static int omnia_mcu_get_features(const struct i2c_client *mcu_client) > { > u16 reply; > @@ -459,6 +486,14 @@ static int omnia_leds_probe(struct i2c_client *client) > "Consider upgrading MCU firmware with the omnia-mcutool utility.\n"); > } > > + if (client->irq && (ret & OMNIA_FEAT_BRIGHTNESS_INT)) { > + ret = devm_request_threaded_irq(dev, client->irq, NULL, > + omnia_brightness_changed_threaded_fn, IRQF_ONESHOT, > + "leds-turris-omnia", leds); > + if (ret < 0) > + return dev_err_probe(dev, ret, "Cannot request brightness IRQ\n"); > + } > + > mutex_init(&leds->lock); > > ret = devm_led_trigger_register(dev, &omnia_hw_trigger); > @@ -481,6 +516,19 @@ static int omnia_leds_probe(struct i2c_client *client) > > static void omnia_leds_remove(struct i2c_client *client) > { > + struct omnia_leds *leds = i2c_get_clientdata(client); > + > + /* > + * We need to free the brightness IRQ here, before putting away the brightness sysfs node. > + * Otherwise devres would free the interrupt only after the sysfs node is removed, and if > + * an interrupt occurred between those two events, it would use a removed sysfs node. > + */ > + devm_free_irq(&client->dev, client->irq, leds); > + > + /* Now put away the sysfs node we got the first time the interrupt handler was called */ > + if (leds->brightness_knode) > + sysfs_put(leds->brightness_knode); > + > /* put all LEDs into default (HW triggered) mode */ > omnia_cmd_write_u8(client, OMNIA_CMD_LED_MODE, OMNIA_CMD_LED_MODE_LED(OMNIA_BOARD_LEDS)); > > -- > 2.45.2 >
On Wed, Nov 06, 2024 at 10:29:19AM +0000, Lee Jones wrote: > On Tue, 05 Nov 2024, kernel test robot wrote: > > > Hi Marek, > > > > kernel test robot noticed the following build errors: > > > > [auto build test ERROR on linus/master] > > [also build test ERROR on v6.12-rc6] > > [cannot apply to lee-leds/for-leds-next robh/for-next next-20241104] > > [If your patch is applied to the wrong git tree, kindly drop us a note. > > And when submitting patch, we suggest to use '--base' as documented in > > https://git-scm.com/docs/git-format-patch#_base_tree_information] > > > > url: https://github.com/intel-lab-lkp/linux/commits/Marek-Beh-n/turris-omnia-mcu-interface-h-Move-command-execution-function-to-global-header/20241104-223435 > > base: linus/master > > patch link: https://lore.kernel.org/r/20241104141924.18816-8-kabel%40kernel.org > > patch subject: [PATCH leds v5 07/12] leds: turris-omnia: Notify sysfs on MCU global LEDs brightness change > > config: arm-randconfig-003-20241105 (https://download.01.org/0day-ci/archive/20241105/202411051138.jzDE6sBH-lkp@intel.com/config) > > compiler: arm-linux-gnueabi-gcc (GCC) 14.1.0 > > reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241105/202411051138.jzDE6sBH-lkp@intel.com/reproduce) > > > > If you fix the issue in a separate patch/commit (i.e. not just a new version of > > the same patch/commit), kindly add following tags > > | Reported-by: kernel test robot <lkp@intel.com> > > | Closes: https://lore.kernel.org/oe-kbuild-all/202411051138.jzDE6sBH-lkp@intel.com/ > > > > All errors (new ones prefixed by >>): > > > > arm-linux-gnueabi-ld: drivers/leds/leds-turris-omnia.o: in function `omnia_cmd_set_color': > > >> include/linux/turris-omnia-mcu-interface.h:275:(.text+0x5a): undefined reference to `omnia_cmd_write_read' > > arm-linux-gnueabi-ld: drivers/leds/leds-turris-omnia.o: in function `omnia_cmd_read_u16': > > include/linux/turris-omnia-mcu-interface.h:311:(.text+0xaa): undefined reference to `omnia_cmd_write_read' > > arm-linux-gnueabi-ld: drivers/leds/leds-turris-omnia.o: in function `gamma_correction_show': > > include/linux/turris-omnia-mcu-interface.h:311:(.text+0x1a2): undefined reference to `omnia_cmd_write_read' > > arm-linux-gnueabi-ld: drivers/leds/leds-turris-omnia.o: in function `omnia_cmd_write_u8': > > include/linux/turris-omnia-mcu-interface.h:275:(.text+0x1c8): undefined reference to `omnia_cmd_write_read' > > arm-linux-gnueabi-ld: drivers/leds/leds-turris-omnia.o: in function `brightness_show': > > include/linux/turris-omnia-mcu-interface.h:311:(.text+0x4c2): undefined reference to `omnia_cmd_write_read' > > > > > > vim +275 include/linux/turris-omnia-mcu-interface.h > > > > 9f74fe5691025f Marek Behún 2024-11-04 267 > > 9f74fe5691025f Marek Behún 2024-11-04 268 int omnia_cmd_write_read(const struct i2c_client *client, > > 9f74fe5691025f Marek Behún 2024-11-04 269 void *cmd, unsigned int cmd_len, > > 9f74fe5691025f Marek Behún 2024-11-04 270 void *reply, unsigned int reply_len); > > 9f74fe5691025f Marek Behún 2024-11-04 271 > > 9f74fe5691025f Marek Behún 2024-11-04 272 static inline int omnia_cmd_write(const struct i2c_client *client, void *cmd, > > 9f74fe5691025f Marek Behún 2024-11-04 273 unsigned int len) > > 9f74fe5691025f Marek Behún 2024-11-04 274 { > > 9f74fe5691025f Marek Behún 2024-11-04 @275 return omnia_cmd_write_read(client, cmd, len, NULL, 0); > > 9f74fe5691025f Marek Behún 2024-11-04 276 } > > 9f74fe5691025f Marek Behún 2024-11-04 277 > > Is this real or a false positive? It is real. Happens when LEDS_TURRIS_OMNIA=y and TURRIS_OMNIA_MCU=m. LEDS_TURRIS_OMNIA must depend on both TURRIS_OMNIA_MCU and TURRIS_OMNIA_MCU_GPIO, not just the latter.
diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig index fcbe93b61e49..148384aacdcc 100644 --- a/drivers/leds/Kconfig +++ b/drivers/leds/Kconfig @@ -217,7 +217,7 @@ config LEDS_TURRIS_OMNIA depends on I2C depends on MACH_ARMADA_38X || COMPILE_TEST depends on OF - depends on TURRIS_OMNIA_MCU + depends on TURRIS_OMNIA_MCU_GPIO select LEDS_TRIGGERS help This option enables basic support for the LEDs found on the front diff --git a/drivers/leds/leds-turris-omnia.c b/drivers/leds/leds-turris-omnia.c index 7d2ed0c6500a..168ce362fd14 100644 --- a/drivers/leds/leds-turris-omnia.c +++ b/drivers/leds/leds-turris-omnia.c @@ -43,12 +43,15 @@ struct omnia_led { * @client: I2C client device * @lock: mutex to protect * @has_gamma_correction: whether the MCU firmware supports gamma correction + * @brightness_knode: kernel node of the "brightness" device sysfs attribute (this is the + * driver specific global brightness, not the LED classdev brightness) * @leds: flexible array of per-LED data */ struct omnia_leds { struct i2c_client *client; struct mutex lock; bool has_gamma_correction; + struct kernfs_node *brightness_knode; struct omnia_led leds[]; }; @@ -373,6 +376,30 @@ static struct attribute *omnia_led_controller_attrs[] = { }; ATTRIBUTE_GROUPS(omnia_led_controller); +static irqreturn_t omnia_brightness_changed_threaded_fn(int irq, void *data) +{ + struct omnia_leds *leds = data; + + if (unlikely(!leds->brightness_knode)) { + /* + * It would be nicer to get this dirent in the driver probe method, before the IRQ + * is requested. But the really_probe() function in drivers/base/dd.c registers + * driver's .dev_groups only after probe is finished, so during driver probe the + * "brightness" sysfs node is not yet present. + * + * Note that sysfs_get_dirent() may sleep. This is okay, because we are in threaded + * context. + */ + leds->brightness_knode = sysfs_get_dirent(leds->client->dev.kobj.sd, "brightness"); + if (!leds->brightness_knode) + return IRQ_NONE; + } + + sysfs_notify_dirent(leds->brightness_knode); + + return IRQ_HANDLED; +} + static int omnia_mcu_get_features(const struct i2c_client *mcu_client) { u16 reply; @@ -459,6 +486,14 @@ static int omnia_leds_probe(struct i2c_client *client) "Consider upgrading MCU firmware with the omnia-mcutool utility.\n"); } + if (client->irq && (ret & OMNIA_FEAT_BRIGHTNESS_INT)) { + ret = devm_request_threaded_irq(dev, client->irq, NULL, + omnia_brightness_changed_threaded_fn, IRQF_ONESHOT, + "leds-turris-omnia", leds); + if (ret < 0) + return dev_err_probe(dev, ret, "Cannot request brightness IRQ\n"); + } + mutex_init(&leds->lock); ret = devm_led_trigger_register(dev, &omnia_hw_trigger); @@ -481,6 +516,19 @@ static int omnia_leds_probe(struct i2c_client *client) static void omnia_leds_remove(struct i2c_client *client) { + struct omnia_leds *leds = i2c_get_clientdata(client); + + /* + * We need to free the brightness IRQ here, before putting away the brightness sysfs node. + * Otherwise devres would free the interrupt only after the sysfs node is removed, and if + * an interrupt occurred between those two events, it would use a removed sysfs node. + */ + devm_free_irq(&client->dev, client->irq, leds); + + /* Now put away the sysfs node we got the first time the interrupt handler was called */ + if (leds->brightness_knode) + sysfs_put(leds->brightness_knode); + /* put all LEDs into default (HW triggered) mode */ omnia_cmd_write_u8(client, OMNIA_CMD_LED_MODE, OMNIA_CMD_LED_MODE_LED(OMNIA_BOARD_LEDS));
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. Signed-off-by: Marek Behún <kabel@kernel.org> --- drivers/leds/Kconfig | 2 +- drivers/leds/leds-turris-omnia.c | 48 ++++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 1 deletion(-)