linux/drivers/leds/trigger, branch v6.7

leds: trigger: netdev: fix RTNL handling to prevent potential deadlock

2023-12-06T15:36:55Z

When working on LED support for r8169 I got the following lockdep warning. Easiest way to prevent this scenario seems to be to take the RTNL lock before the trigger_data lock in set_device_name(). ====================================================== WARNING: possible circular locking dependency detected 6.7.0-rc2-next-20231124+ #2 Not tainted ------------------------------------------------------ bash/383 is trying to acquire lock: ffff888103aa1c68 (&trigger_data->lock){+.+.}-{3:3}, at: netdev_trig_notify+0xec/0x190 [ledtrig_netdev] but task is already holding lock: ffffffff8cddf808 (rtnl_mutex){+.+.}-{3:3}, at: rtnl_lock+0x12/0x20 which lock already depends on the new lock. the existing dependency chain (in reverse order) is: -> #1 (rtnl_mutex){+.+.}-{3:3}: __mutex_lock+0x9b/0xb50 mutex_lock_nested+0x16/0x20 rtnl_lock+0x12/0x20 set_device_name+0xa9/0x120 [ledtrig_netdev] netdev_trig_activate+0x1a1/0x230 [ledtrig_netdev] led_trigger_set+0x172/0x2c0 led_trigger_write+0xf1/0x140 sysfs_kf_bin_write+0x5d/0x80 kernfs_fop_write_iter+0x15d/0x210 vfs_write+0x1f0/0x510 ksys_write+0x6c/0xf0 __x64_sys_write+0x14/0x20 do_syscall_64+0x3f/0xf0 entry_SYSCALL_64_after_hwframe+0x6c/0x74 -> #0 (&trigger_data->lock){+.+.}-{3:3}: __lock_acquire+0x1459/0x25a0 lock_acquire+0xc8/0x2d0 __mutex_lock+0x9b/0xb50 mutex_lock_nested+0x16/0x20 netdev_trig_notify+0xec/0x190 [ledtrig_netdev] call_netdevice_register_net_notifiers+0x5a/0x100 register_netdevice_notifier+0x85/0x120 netdev_trig_activate+0x1d4/0x230 [ledtrig_netdev] led_trigger_set+0x172/0x2c0 led_trigger_write+0xf1/0x140 sysfs_kf_bin_write+0x5d/0x80 kernfs_fop_write_iter+0x15d/0x210 vfs_write+0x1f0/0x510 ksys_write+0x6c/0xf0 __x64_sys_write+0x14/0x20 do_syscall_64+0x3f/0xf0 entry_SYSCALL_64_after_hwframe+0x6c/0x74 other info that might help us debug this: Possible unsafe locking scenario: CPU0 CPU1 ---- ---- lock(rtnl_mutex); lock(&trigger_data->lock); lock(rtnl_mutex); lock(&trigger_data->lock); *** DEADLOCK *** 8 locks held by bash/383: #0: ffff888103ff33f0 (sb_writers#3){.+.+}-{0:0}, at: ksys_write+0x6c/0xf0 #1: ffff888103aa1e88 (&of->mutex){+.+.}-{3:3}, at: kernfs_fop_write_iter+0x114/0x210 #2: ffff8881036f1890 (kn->active#82){.+.+}-{0:0}, at: kernfs_fop_write_iter+0x11d/0x210 #3: ffff888108e2c358 (&led_cdev->led_access){+.+.}-{3:3}, at: led_trigger_write+0x30/0x140 #4: ffffffff8cdd9e10 (triggers_list_lock){++++}-{3:3}, at: led_trigger_write+0x75/0x140 #5: ffff888108e2c270 (&led_cdev->trigger_lock){++++}-{3:3}, at: led_trigger_write+0xe3/0x140 #6: ffffffff8cdde3d0 (pernet_ops_rwsem){++++}-{3:3}, at: register_netdevice_notifier+0x1c/0x120 #7: ffffffff8cddf808 (rtnl_mutex){+.+.}-{3:3}, at: rtnl_lock+0x12/0x20 stack backtrace: CPU: 0 PID: 383 Comm: bash Not tainted 6.7.0-rc2-next-20231124+ #2 Hardware name: Default string Default string/Default string, BIOS ADLN.M6.SODIMM.ZB.CY.015 08/08/2023 Call Trace: dump_stack_lvl+0x5c/0xd0 dump_stack+0x10/0x20 print_circular_bug+0x2dd/0x410 check_noncircular+0x131/0x150 __lock_acquire+0x1459/0x25a0 lock_acquire+0xc8/0x2d0 ? netdev_trig_notify+0xec/0x190 [ledtrig_netdev] __mutex_lock+0x9b/0xb50 ? netdev_trig_notify+0xec/0x190 [ledtrig_netdev] ? __this_cpu_preempt_check+0x13/0x20 ? netdev_trig_notify+0xec/0x190 [ledtrig_netdev] ? __cancel_work_timer+0x11c/0x1b0 ? __mutex_lock+0x123/0xb50 mutex_lock_nested+0x16/0x20 ? mutex_lock_nested+0x16/0x20 netdev_trig_notify+0xec/0x190 [ledtrig_netdev] call_netdevice_register_net_notifiers+0x5a/0x100 register_netdevice_notifier+0x85/0x120 netdev_trig_activate+0x1d4/0x230 [ledtrig_netdev] led_trigger_set+0x172/0x2c0 ? preempt_count_add+0x49/0xc0 led_trigger_write+0xf1/0x140 sysfs_kf_bin_write+0x5d/0x80 kernfs_fop_write_iter+0x15d/0x210 vfs_write+0x1f0/0x510 ksys_write+0x6c/0xf0 __x64_sys_write+0x14/0x20 do_syscall_64+0x3f/0xf0 entry_SYSCALL_64_after_hwframe+0x6c/0x74 RIP: 0033:0x7f269055d034 Code: c7 00 16 00 00 00 b8 ff ff ff ff c3 66 2e 0f 1f 84 00 00 00 00 00 f3 0f 1e fa 80 3d 35 c3 0d 00 00 74 13 b8 01 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 54 c3 0f 1f 00 48 83 ec 28 48 89 54 24 18 48 RSP: 002b:00007ffddb7ef748 EFLAGS: 00000202 ORIG_RAX: 0000000000000001 RAX: ffffffffffffffda RBX: 0000000000000007 RCX: 00007f269055d034 RDX: 0000000000000007 RSI: 000055bf5f4af3c0 RDI: 0000000000000001 RBP: 000055bf5f4af3c0 R08: 0000000000000073 R09: 0000000000000001 R10: 0000000000000000 R11: 0000000000000202 R12: 0000000000000007 R13: 00007f26906325c0 R14: 00007f269062ff20 R15: 0000000000000000 Fixes: d5e01266e7f5 ("leds: trigger: netdev: add additional specific link speed mode") Cc: stable@vger.kernel.org Signed-off-by: Heiner Kallweit Reviewed-by: Andrew Lunn Acked-by: Lee Jones Link: https://lore.kernel.org/r/fb5c8294-2a10-4bf5-8f10-3d2b77d2757e@gmail.com Signed-off-by: Jakub Kicinski

leds: trigger: netdev: Move size check in set_device_name

2023-11-01T11:29:14Z

GCC 13.2 complains about array subscript 17 is above array bounds of 'char[16]' with IFNAMSIZ set to 16. The warning is correct but this scenario is impossible. set_device_name is called by device_name_store (store sysfs entry) and netdev_trig_activate. device_name_store already check if size is >= of IFNAMSIZ and return -EINVAL. (making the warning scenario impossible) netdev_trig_activate works on already defined interface, where the name has already been checked and should already follow the condition of strlen() < IFNAMSIZ. Aside from the scenario being impossible, set_device_name can be improved to both mute the warning and make the function safer. To make it safer, move size check from device_name_store directly to set_device_name and prevent any out of bounds scenario. Cc: stable@vger.kernel.org Fixes: 28a6a2ef18ad ("leds: trigger: netdev: refactor code setting device name") Reported-by: kernel test robot Closes: https://lore.kernel.org/oe-kbuild-all/202309192035.GTJEEbem-lkp@intel.com/ Signed-off-by: Christian Marangi Link: https://lore.kernel.org/r/20231007131042.15032-1-ansuelsmth@gmail.com Signed-off-by: Lee Jones

leds: triggers: gpio: Rewrite to use trigger-sources

2023-11-01T11:28:58Z

By providing a GPIO line as "trigger-sources" in the FWNODE (such as from the device tree) and combining with the GPIO trigger, we can support a GPIO LED trigger in a natural way from the hardware description instead of using the custom sysfs and deprecated global GPIO numberspace. Example: gpio: gpio@0 { compatible "my-gpio"; gpio-controller; #gpio-cells = <2>; interrupt-controller; #interrupt-cells = <2>; #trigger-source-cells = <2>; }; leds { compatible = "gpio-leds"; led-my-gpio { label = "device:blue:myled"; gpios = <&gpio 0 GPIO_ACTIVE_HIGH>; default-state = "off"; linux,default-trigger = "gpio"; trigger-sources = <&gpio 1 GPIO_ACTIVE_HIGH>; }; }; Make this the norm, unmark the driver as broken. Delete the sysfs handling of GPIOs. Since GPIO descriptors inherently can describe inversion, the inversion handling can just be deleted. Signed-off-by: Linus Walleij Link: https://lore.kernel.org/r/20230926-gpio-led-trigger-dt-v2-3-e06e458b788e@linaro.org Signed-off-by: Lee Jones

leds: trigger: ledtrig-cpu:: Fix 'output may be truncated' issue for 'cpu'

2023-11-01T11:28:51Z

In order to teach the compiler that 'trig->name' will never be truncated, we need to tell it that 'cpu' is not negative. When building with W=1, this fixes the following warnings: drivers/leds/trigger/ledtrig-cpu.c: In function ‘ledtrig_cpu_init’: drivers/leds/trigger/ledtrig-cpu.c:155:56: error: ‘%d’ directive output may be truncated writing between 1 and 11 bytes into a region of size 5 [-Werror=format-truncation=] 155 | snprintf(trig->name, MAX_NAME_LEN, "cpu%d", cpu); | ^~ drivers/leds/trigger/ledtrig-cpu.c:155:52: note: directive argument in the range [-2147483648, 7] 155 | snprintf(trig->name, MAX_NAME_LEN, "cpu%d", cpu); | ^~~~~~~ drivers/leds/trigger/ledtrig-cpu.c:155:17: note: ‘snprintf’ output between 5 and 15 bytes into a destination of size 8 155 | snprintf(trig->name, MAX_NAME_LEN, "cpu%d", cpu); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Fixes: 8f88731d052d ("led-triggers: create a trigger for CPU activity") Signed-off-by: Christophe JAILLET Link: https://lore.kernel.org/r/3f4be7a99933cf8566e630da54f6ab913caac432.1695453322.git.christophe.jaillet@wanadoo.fr Signed-off-by: Lee Jones

Merge tag 'leds-next-6.6' of git://git.kernel.org/pub/scm/linux/kernel/git/lee/leds

2023-09-04T20:52:58Z

Pull LED updates from Lee Jones: "Core Frameworks: - Add new framework to support Group Multi-Color (GMC) LEDs - Offer an 'optional' API for non-essential LEDs - Support obtaining 'max brightness' values from Device Tree - Provide new led_classdev member 'color' (settable via DT and SYFS) - Stop TTY Trigger from using the old LED_ON constraints - Statically allocate leds_class New Drivers: - Add support for NXP PCA995x I2C Constant Current LED Driver New Device Support: - Add support for Siemens Simatic IPC BX-21 to Simatic IPC Fix-ups: - Some dependency / Kconfig tweaking - Move final probe() functions back over from .probe_new() - Simplify obtaining resources (memory, device data) using unified API helpers - Bunch of Device Tree additions, conversions and adaptions - Fix trivial styling issues; comments - Ensure correct includes are present and remove some that are not required - Omit the use of redundant casts and if relevant replace with better ones - Use purpose-built APIs for various actions; sysfs_emit(), module_led_trigger() - Remove a bunch of superfluous locking Bug Fixes: - Ensure error codes are correctly propagated back up the call chain - Fix incorrect error values from being returned (missing '-') - Ensure get'ed resources are put'ed to prevent leaks - Use correct class when exporting module resources - Fixing rounding (or lack there of) issues - Fix 'always false' LED_COLOR_ID_MULTI BUG() check" * tag 'leds-next-6.6' of git://git.kernel.org/pub/scm/linux/kernel/git/lee/leds: (40 commits) leds: aw2013: Enable pull-up supply for interrupt and I2C dt-bindings: leds: Document pull-up supply for interrupt and I2C dt-bindings: leds: aw2013: Document interrupt leds: uleds: Use module_misc_device macro to simplify the code leds: trigger: netdev: Use module_led_trigger macro to simplify the code dt-bindings: leds: Fix reference to definition of default-state leds: turris-omnia: Drop unnecessary mutex locking leds: turris-omnia: Use sysfs_emit() instead of sprintf() leds: Make leds_class a static const structure leds: Remove redundant of_match_ptr() dt-bindings: leds: Add gpio-line-names to PCA9532 GPIO leds: trigger: tty: Do not use LED_ON/OFF constants, use led_blink_set_oneshot instead dt-bindings: leds: rohm,bd71828: Drop select:false leds: Fix BUG_ON check for LED_COLOR_ID_MULTI that is always false leds: multicolor: Use rounded division when calculating color components leds: rgb: Add a multicolor LED driver to group monochromatic LEDs dt-bindings: leds: Add binding for a multicolor group of LEDs leds: class: Store the color index in struct led_classdev leds: Provide devm_of_led_get_optional() leds: pca995x: Fix MODULE_DEVICE_TABLE for OF ...

Merge git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net

2023-08-24T17:51:39Z

Cross-merge networking fixes after downstream PR. Conflicts: include/net/inet_sock.h f866fbc842de ("ipv4: fix data-races around inet->inet_id") c274af224269 ("inet: introduce inet->inet_flags") https://lore.kernel.org/all/679ddff6-db6e-4ff6-b177-574e90d0103d@tessares.net/ Adjacent changes: drivers/net/bonding/bond_alb.c e74216b8def3 ("bonding: fix macvlan over alb bond support") f11e5bd159b0 ("bonding: support balance-alb with openvswitch") drivers/net/ethernet/broadcom/bgmac.c d6499f0b7c7c ("net: bgmac: Return PTR_ERR() for fixed_phy_register()") 23a14488ea58 ("net: bgmac: Fix return value check for fixed_phy_register()") drivers/net/ethernet/broadcom/genet/bcmmii.c 32bbe64a1386 ("net: bcmgenet: Fix return value check for fixed_phy_register()") acf50d1adbf4 ("net: bcmgenet: Return PTR_ERR() for fixed_phy_register()") net/sctp/socket.c f866fbc842de ("ipv4: fix data-races around inet->inet_id") b09bde5c3554 ("inet: move inet->mc_loop to inet->inet_frags") Signed-off-by: Jakub Kicinski

leds: trigger: netdev: rename 'hw_control' sysfs entry to 'offloaded'

2023-08-22T18:02:29Z

Commit b655892ffd6d ("leds: trigger: netdev: expose hw_control status via sysfs") exposed to sysfs the flag that tells whether the LED trigger is offloaded to hardware, under the name "hw_control", since that is the name under which this setting is called in the code. Everywhere else in kernel when some work that is normally done in software can be made to be done by hardware instead, we use the word "offloading" to describe this, e.g. "LED blinking is offloaded to hardware". Normally renaming sysfs entries is a no-go because of backwards compatibility. But since this patch was not yet released in a stable kernel, I think it is still possible to rename it, if there is consensus. Fixes: b655892ffd6d ("leds: trigger: netdev: expose hw_control status via sysfs") Signed-off-by: Marek Behún Reviewed-by: Andrew Lunn Link: https://lore.kernel.org/r/20230821121453.30203-1-kabel@kernel.org Signed-off-by: Jakub Kicinski

leds: trigger: netdev: Use module_led_trigger macro to simplify the code

2023-08-18T10:42:05Z

Use the module_led_trigger macro to simplify the code, which is the same as declaring with module_init() and module_exit(). Signed-off-by: Li Zetao Reviewed-by: Andrew Lunn Link: https://lore.kernel.org/r/20230815075944.1089298-2-lizetao1@huawei.com Signed-off-by: Lee Jones

leds: trigger: tty: Do not use LED_ON/OFF constants, use led_blink_set_oneshot instead

2023-08-17T10:58:32Z

The tty LED trigger uses the obsolete LED_ON & LED_OFF constants when setting LED brightness. This is bad because the LED_ON constant is equal to 1, and so when activating the tty LED trigger on a LED class device with max_brightness greater than 1, the LED is dimmer than it can be (when max_brightness is 255, the LED is very dimm indeed; some devices translate 1/255 to 0, so the LED is OFF all the time). Instead of directly setting brightness to a specific value, use the led_blink_set_oneshot() function from LED core to configure the blink. This function takes the current configured brightness as blink brightness if not zero, and max brightness otherwise. This also changes the behavior of the TTY LED trigger. Previously if rx/tx stats kept changing, the LED was ON all the time they kept changing. With this patch the LED will blink on TTY activity. Fixes: fd4a641ac88f ("leds: trigger: implement a tty trigger") Signed-off-by: Marek Behún Link: https://lore.kernel.org/r/20230802090753.13611-1-kabel@kernel.org Signed-off-by: Lee Jones

leds: trig-netdev: Disable offload on deactivation of trigger

2023-08-11T00:35:00Z

Ensure that the offloading of blinking is stopped when the trigger is deactivated. Calling led_set_brightness() is documented as stopping offload and setting the LED to a constant brightness. Suggested-by: Daniel Golle Signed-off-by: Andrew Lunn Reviewed-by: Simon Horman Tested-by: Daniel Golle Link: https://lore.kernel.org/r/20230808210436.838995-5-andrew@lunn.ch Signed-off-by: Jakub Kicinski