| Message ID | 20241029135621.12546-3-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 warnings:
[auto build test WARNING on linus/master]
[also build test WARNING on v6.12-rc5]
[cannot apply to lee-leds/for-leds-next robh/for-next next-20241030]
[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/20241029-215858
base: linus/master
patch link: https://lore.kernel.org/r/20241029135621.12546-3-kabel%40kernel.org
patch subject: [PATCH leds v4 02/12] leds: turris-omnia: Use command execution functions from the MCU driver
config: riscv-allmodconfig (https://download.01.org/0day-ci/archive/20241031/202410311612.0OkxKVgC-lkp@intel.com/config)
compiler: clang version 20.0.0git (https://github.com/llvm/llvm-project 639a7ac648f1e50ccd2556e17d401c04f9cce625)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241031/202410311612.0OkxKVgC-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/202410311612.0OkxKVgC-lkp@intel.com/
All warnings (new ones prefixed by >>):
In file included from drivers/leds/leds-turris-omnia.c:8:
In file included from include/linux/i2c.h:19:
In file included from include/linux/regulator/consumer.h:35:
In file included from include/linux/suspend.h:5:
In file included from include/linux/swap.h:9:
In file included from include/linux/memcontrol.h:13:
In file included from include/linux/cgroup.h:26:
In file included from include/linux/kernel_stat.h:8:
In file included from include/linux/interrupt.h:22:
In file included from arch/riscv/include/asm/sections.h:9:
In file included from include/linux/mm.h:2213:
include/linux/vmstat.h:504:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
504 | return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
| ~~~~~~~~~~~~~~~~~~~~~ ^
505 | item];
| ~~~~
include/linux/vmstat.h:511:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
511 | return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
| ~~~~~~~~~~~~~~~~~~~~~ ^
512 | NR_VM_NUMA_EVENT_ITEMS +
| ~~~~~~~~~~~~~~~~~~~~~~
include/linux/vmstat.h:518:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
518 | return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
| ~~~~~~~~~~~ ^ ~~~
include/linux/vmstat.h:524:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
524 | return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
| ~~~~~~~~~~~~~~~~~~~~~ ^
525 | NR_VM_NUMA_EVENT_ITEMS +
| ~~~~~~~~~~~~~~~~~~~~~~
>> drivers/leds/leds-turris-omnia.c:409:12: warning: stack frame size (2064) exceeds limit (2048) in 'omnia_leds_probe' [-Wframe-larger-than]
409 | static int omnia_leds_probe(struct i2c_client *client)
| ^
5 warnings generated.
vim +/omnia_leds_probe +409 drivers/leds/leds-turris-omnia.c
43e9082fbccc7d Marek Behún 2023-09-18 408
4934630409cb60 Uwe Kleine-König 2022-11-18 @409 static int omnia_leds_probe(struct i2c_client *client)
089381b27abe28 Marek Behún 2020-07-23 410 {
089381b27abe28 Marek Behún 2020-07-23 411 struct device *dev = &client->dev;
122d57e2960c81 Krzysztof Kozlowski 2024-08-16 412 struct device_node *np = dev_of_node(dev);
089381b27abe28 Marek Behún 2020-07-23 413 struct omnia_leds *leds;
089381b27abe28 Marek Behún 2020-07-23 414 struct omnia_led *led;
089381b27abe28 Marek Behún 2020-07-23 415 int ret, count;
089381b27abe28 Marek Behún 2020-07-23 416
089381b27abe28 Marek Behún 2020-07-23 417 count = of_get_available_child_count(np);
089381b27abe28 Marek Behún 2020-07-23 418 if (!count) {
089381b27abe28 Marek Behún 2020-07-23 419 dev_err(dev, "LEDs are not defined in device tree!\n");
089381b27abe28 Marek Behún 2020-07-23 420 return -ENODEV;
089381b27abe28 Marek Behún 2020-07-23 421 } else if (count > OMNIA_BOARD_LEDS) {
089381b27abe28 Marek Behún 2020-07-23 422 dev_err(dev, "Too many LEDs defined in device tree!\n");
089381b27abe28 Marek Behún 2020-07-23 423 return -EINVAL;
089381b27abe28 Marek Behún 2020-07-23 424 }
089381b27abe28 Marek Behún 2020-07-23 425
089381b27abe28 Marek Behún 2020-07-23 426 leds = devm_kzalloc(dev, struct_size(leds, leds, count), GFP_KERNEL);
089381b27abe28 Marek Behún 2020-07-23 427 if (!leds)
089381b27abe28 Marek Behún 2020-07-23 428 return -ENOMEM;
089381b27abe28 Marek Behún 2020-07-23 429
089381b27abe28 Marek Behún 2020-07-23 430 leds->client = client;
089381b27abe28 Marek Behún 2020-07-23 431 i2c_set_clientdata(client, leds);
089381b27abe28 Marek Behún 2020-07-23 432
43e9082fbccc7d Marek Behún 2023-09-18 433 ret = omnia_mcu_get_features(client);
43e9082fbccc7d Marek Behún 2023-09-18 434 if (ret < 0) {
43e9082fbccc7d Marek Behún 2023-09-18 435 dev_err(dev, "Cannot determine MCU supported features: %d\n",
43e9082fbccc7d Marek Behún 2023-09-18 436 ret);
43e9082fbccc7d Marek Behún 2023-09-18 437 return ret;
43e9082fbccc7d Marek Behún 2023-09-18 438 }
43e9082fbccc7d Marek Behún 2023-09-18 439
43e9082fbccc7d Marek Behún 2023-09-18 440 leds->has_gamma_correction = ret & FEAT_LED_GAMMA_CORRECTION;
43e9082fbccc7d Marek Behún 2023-09-18 441 if (!leds->has_gamma_correction) {
43e9082fbccc7d Marek Behún 2023-09-18 442 dev_info(dev,
43e9082fbccc7d Marek Behún 2023-09-18 443 "Your board's MCU firmware does not support the LED gamma correction feature.\n");
43e9082fbccc7d Marek Behún 2023-09-18 444 dev_info(dev,
43e9082fbccc7d Marek Behún 2023-09-18 445 "Consider upgrading MCU firmware with the omnia-mcutool utility.\n");
43e9082fbccc7d Marek Behún 2023-09-18 446 }
43e9082fbccc7d Marek Behún 2023-09-18 447
089381b27abe28 Marek Behún 2020-07-23 448 mutex_init(&leds->lock);
089381b27abe28 Marek Behún 2020-07-23 449
cbd6954fecbeb8 Marek Behún 2023-09-18 450 ret = devm_led_trigger_register(dev, &omnia_hw_trigger);
cbd6954fecbeb8 Marek Behún 2023-09-18 451 if (ret < 0) {
cbd6954fecbeb8 Marek Behún 2023-09-18 452 dev_err(dev, "Cannot register private LED trigger: %d\n", ret);
cbd6954fecbeb8 Marek Behún 2023-09-18 453 return ret;
cbd6954fecbeb8 Marek Behún 2023-09-18 454 }
cbd6954fecbeb8 Marek Behún 2023-09-18 455
089381b27abe28 Marek Behún 2020-07-23 456 led = &leds->leds[0];
122d57e2960c81 Krzysztof Kozlowski 2024-08-16 457 for_each_available_child_of_node_scoped(np, child) {
089381b27abe28 Marek Behún 2020-07-23 458 ret = omnia_led_register(client, led, child);
122d57e2960c81 Krzysztof Kozlowski 2024-08-16 459 if (ret < 0)
089381b27abe28 Marek Behún 2020-07-23 460 return ret;
089381b27abe28 Marek Behún 2020-07-23 461
089381b27abe28 Marek Behún 2020-07-23 462 led += ret;
089381b27abe28 Marek Behún 2020-07-23 463 }
089381b27abe28 Marek Behún 2020-07-23 464
089381b27abe28 Marek Behún 2020-07-23 465 return 0;
089381b27abe28 Marek Behún 2020-07-23 466 }
089381b27abe28 Marek Behún 2020-07-23 467
On Thu, Oct 31, 2024, at 10:01, kernel test robot wrote: > > kernel test robot noticed the following build warnings: >>> drivers/leds/leds-turris-omnia.c:409:12: warning: stack frame size (2064) exceeds limit (2048) in 'omnia_leds_probe' [-Wframe-larger-than] > 409 | static int omnia_leds_probe(struct i2c_client *client) > | ^ The problem here is the i2c_client on the stack, you can't do that: static int omnia_mcu_get_features(const struct i2c_client *client) { + struct i2c_client mcu_client = *client; u16 reply; I haven't looked at this in detail, but there is usually a trivial alternative. Arnd
On Fri, Nov 01, 2024 at 10:06:40AM +0100, Arnd Bergmann wrote: > On Thu, Oct 31, 2024, at 10:01, kernel test robot wrote: > > > > kernel test robot noticed the following build warnings: > > >>> drivers/leds/leds-turris-omnia.c:409:12: warning: stack frame size (2064) exceeds limit (2048) in 'omnia_leds_probe' [-Wframe-larger-than] > > 409 | static int omnia_leds_probe(struct i2c_client *client) > > | ^ > > The problem here is the i2c_client on the stack, you can't > do that: > > static int omnia_mcu_get_features(const struct i2c_client *client) > { > + struct i2c_client mcu_client = *client; > u16 reply; OMG, I see. struct i2c_client contains struct device. OK, I will do this correctly by finding the MCU device on the I2C bus. Marek
diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig index b784bb74a837..fcbe93b61e49 100644 --- a/drivers/leds/Kconfig +++ b/drivers/leds/Kconfig @@ -217,6 +217,7 @@ config LEDS_TURRIS_OMNIA depends on I2C depends on MACH_ARMADA_38X || COMPILE_TEST depends on OF + depends on TURRIS_OMNIA_MCU 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 4cff8c4b020c..0b71848930dd 100644 --- a/drivers/leds/leds-turris-omnia.c +++ b/drivers/leds/leds-turris-omnia.c @@ -2,7 +2,7 @@ /* * CZ.NIC's Turris Omnia LEDs driver * - * 2020, 2023 by Marek Behún <kabel@kernel.org> + * 2020, 2023, 2024 by Marek Behún <kabel@kernel.org> */ #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,66 +58,21 @@ struct omnia_leds { struct omnia_led leds[]; }; -static int omnia_cmd_write_u8(const struct i2c_client *client, u8 cmd, u8 val) +static int omnia_cmd_set_color(const struct i2c_client *client, u8 led, u8 r, u8 g, u8 b) { - u8 buf[2] = { cmd, val }; - int ret; - - ret = i2c_master_send(client, buf, sizeof(buf)); + u8 buf[5] = { CMD_LED_COLOR, led, r, g, b }; - 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; + return omnia_cmd_write(client, buf, sizeof(buf)); } static int omnia_led_send_color_cmd(const struct i2c_client *client, struct omnia_led *led) { - char cmd[5]; int ret; - cmd[0] = CMD_LED_COLOR; - cmd[1] = led->reg; - cmd[2] = led->subled_info[0].brightness; - cmd[3] = led->subled_info[1].brightness; - cmd[4] = led->subled_info[2].brightness; - /* Send the color change command */ - ret = i2c_master_send(client, cmd, 5); + ret = omnia_cmd_set_color(client, led->reg, led->subled_info[0].brightness, + led->subled_info[1].brightness, led->subled_info[2].brightness); if (ret < 0) return ret; @@ -352,14 +308,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 +342,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 +384,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) @@ -510,20 +467,11 @@ static int omnia_leds_probe(struct i2c_client *client) 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, CMD_LED_MODE, CMD_LED_MODE_LED(OMNIA_BOARD_LEDS)); /* set all LEDs color to [255, 255, 255] */ - buf[0] = CMD_LED_COLOR; - buf[1] = OMNIA_BOARD_LEDS; - buf[2] = 255; - buf[3] = 255; - buf[4] = 255; - - i2c_master_send(client, buf, 5); + omnia_cmd_set_color(client, OMNIA_BOARD_LEDS, 255, 255, 255); } static const struct of_device_id of_omnia_leds_match[] = {
Use 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. Signed-off-by: Marek Behún <kabel@kernel.org> --- drivers/leds/Kconfig | 1 + drivers/leds/leds-turris-omnia.c | 110 ++++++++----------------------- 2 files changed, 30 insertions(+), 81 deletions(-)