From 1490cbb9dbfd0eabfe45f9b674097aea7e6760fc Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Mon, 10 Mar 2025 16:54:51 +0200 Subject: [PATCH 01/31] device property: Split fwnode_get_child_node_count() The new helper is introduced to allow counting the child firmware nodes of their parent without requiring a device to be passed. This also makes the fwnode and device property API more symmetrical with the rest. Signed-off-by: Andy Shevchenko Acked-by: "Rafael J. Wysocki" Reviewed-by: Sakari Ailus Reviewed-by: Heikki Krogerus Reviewed-by: Jonathan Cameron Link: https://lore.kernel.org/r/20250310150835.3139322-2-andriy.shevchenko@linux.intel.com Signed-off-by: Lee Jones --- drivers/base/property.c | 12 ++++++------ include/linux/property.h | 7 ++++++- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/drivers/base/property.c b/drivers/base/property.c index c1392743df9c..805f75b35115 100644 --- a/drivers/base/property.c +++ b/drivers/base/property.c @@ -928,22 +928,22 @@ bool fwnode_device_is_available(const struct fwnode_handle *fwnode) EXPORT_SYMBOL_GPL(fwnode_device_is_available); /** - * device_get_child_node_count - return the number of child nodes for device - * @dev: Device to count the child nodes for + * fwnode_get_child_node_count - return the number of child nodes for a given firmware node + * @fwnode: Pointer to the parent firmware node * - * Return: the number of child nodes for a given device. + * Return: the number of child nodes for a given firmware node. */ -unsigned int device_get_child_node_count(const struct device *dev) +unsigned int fwnode_get_child_node_count(const struct fwnode_handle *fwnode) { struct fwnode_handle *child; unsigned int count = 0; - device_for_each_child_node(dev, child) + fwnode_for_each_child_node(fwnode, child) count++; return count; } -EXPORT_SYMBOL_GPL(device_get_child_node_count); +EXPORT_SYMBOL_GPL(fwnode_get_child_node_count); bool device_dma_supported(const struct device *dev) { diff --git a/include/linux/property.h b/include/linux/property.h index e214ecd241eb..bc5bfc98176b 100644 --- a/include/linux/property.h +++ b/include/linux/property.h @@ -208,7 +208,12 @@ DEFINE_FREE(fwnode_handle, struct fwnode_handle *, fwnode_handle_put(_T)) int fwnode_irq_get(const struct fwnode_handle *fwnode, unsigned int index); int fwnode_irq_get_byname(const struct fwnode_handle *fwnode, const char *name); -unsigned int device_get_child_node_count(const struct device *dev); +unsigned int fwnode_get_child_node_count(const struct fwnode_handle *fwnode); + +static inline unsigned int device_get_child_node_count(const struct device *dev) +{ + return fwnode_get_child_node_count(dev_fwnode(dev)); +} static inline int device_property_read_u8(const struct device *dev, const char *propname, u8 *val) From 4623cc4e9a5f1b7ad7e6599dfc2a5a4d9d4f5d72 Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Mon, 10 Mar 2025 16:54:52 +0200 Subject: [PATCH 02/31] leds: pwm-multicolor: Use fwnode_get_child_node_count() Since fwnode_get_child_node_count() was split from its device property counterpart, we may utilise it in the driver and drop custom implementation. Signed-off-by: Andy Shevchenko Reviewed-by: Jonathan Cameron Link: https://lore.kernel.org/r/20250310150835.3139322-3-andriy.shevchenko@linux.intel.com Signed-off-by: Lee Jones --- drivers/leds/rgb/leds-pwm-multicolor.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/drivers/leds/rgb/leds-pwm-multicolor.c b/drivers/leds/rgb/leds-pwm-multicolor.c index 1c7705bafdfc..e0d7d3c9215c 100644 --- a/drivers/leds/rgb/leds-pwm-multicolor.c +++ b/drivers/leds/rgb/leds-pwm-multicolor.c @@ -107,12 +107,12 @@ static int iterate_subleds(struct device *dev, struct pwm_mc_led *priv, static int led_pwm_mc_probe(struct platform_device *pdev) { - struct fwnode_handle *mcnode, *fwnode; + struct fwnode_handle *mcnode; struct led_init_data init_data = {}; struct led_classdev *cdev; struct mc_subled *subled; struct pwm_mc_led *priv; - int count = 0; + unsigned int count; int ret = 0; mcnode = device_get_named_child_node(&pdev->dev, "multi-led"); @@ -121,8 +121,7 @@ static int led_pwm_mc_probe(struct platform_device *pdev) "expected multi-led node\n"); /* count the nodes inside the multi-led node */ - fwnode_for_each_child_node(mcnode, fwnode) - count++; + count = fwnode_get_child_node_count(mcnode); priv = devm_kzalloc(&pdev->dev, struct_size(priv, leds, count), GFP_KERNEL); From 53762bb44b0659e79a3e3177372e823ec4afcc8a Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Mon, 10 Mar 2025 16:54:53 +0200 Subject: [PATCH 03/31] leds: ncp5623: Use fwnode_get_child_node_count() Since fwnode_get_child_node_count() was split from its device property counterpart, we may utilise it in the driver and drop custom implementation. Signed-off-by: Andy Shevchenko Reviewed-by: Jonathan Cameron Link: https://lore.kernel.org/r/20250310150835.3139322-4-andriy.shevchenko@linux.intel.com Signed-off-by: Lee Jones --- drivers/leds/rgb/leds-ncp5623.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/leds/rgb/leds-ncp5623.c b/drivers/leds/rgb/leds-ncp5623.c index f18156683375..7c7d44623a9e 100644 --- a/drivers/leds/rgb/leds-ncp5623.c +++ b/drivers/leds/rgb/leds-ncp5623.c @@ -155,9 +155,9 @@ static int ncp5623_probe(struct i2c_client *client) struct device *dev = &client->dev; struct fwnode_handle *mc_node, *led_node; struct led_init_data init_data = { }; - int num_subleds = 0; struct ncp5623 *ncp; struct mc_subled *subled_info; + unsigned int num_subleds; u32 color_index; u32 reg; int ret; @@ -172,8 +172,7 @@ static int ncp5623_probe(struct i2c_client *client) if (!mc_node) return -EINVAL; - fwnode_for_each_child_node(mc_node, led_node) - num_subleds++; + num_subleds = fwnode_get_child_node_count(mc_node); subled_info = devm_kcalloc(dev, num_subleds, sizeof(*subled_info), GFP_KERNEL); if (!subled_info) { From 08ca89e98620c08d68b7e7aed6c9294698e214e1 Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Mon, 10 Mar 2025 16:54:54 +0200 Subject: [PATCH 04/31] usb: typec: tcpm: Use fwnode_get_child_node_count() Since fwnode_get_child_node_count() was split from its device property counterpart, we may utilise it in the driver and drop custom implementation. Signed-off-by: Andy Shevchenko Reviewed-by: Heikki Krogerus Acked-by: Kyle Tso Reviewed-by: Jonathan Cameron Link: https://lore.kernel.org/r/20250310150835.3139322-5-andriy.shevchenko@linux.intel.com Signed-off-by: Lee Jones --- drivers/usb/typec/tcpm/tcpm.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c index a99db4e025cd..f967ba8e0b63 100644 --- a/drivers/usb/typec/tcpm/tcpm.c +++ b/drivers/usb/typec/tcpm/tcpm.c @@ -7166,7 +7166,7 @@ static void tcpm_fw_get_timings(struct tcpm_port *port, struct fwnode_handle *fw static int tcpm_fw_get_caps(struct tcpm_port *port, struct fwnode_handle *fwnode) { - struct fwnode_handle *capabilities, *child, *caps = NULL; + struct fwnode_handle *capabilities, *caps = NULL; unsigned int nr_src_pdo, nr_snk_pdo; const char *opmode_str; u32 *src_pdo, *snk_pdo; @@ -7232,9 +7232,7 @@ static int tcpm_fw_get_caps(struct tcpm_port *port, struct fwnode_handle *fwnode if (!capabilities) { port->pd_count = 1; } else { - fwnode_for_each_child_node(capabilities, child) - port->pd_count++; - + port->pd_count = fwnode_get_child_node_count(capabilities); if (!port->pd_count) { ret = -ENODATA; goto put_capabilities; From 06d99fcf1f87d5b6bf1c7dd0f7ec422304a47f31 Mon Sep 17 00:00:00 2001 From: Craig McQueen Date: Mon, 17 Mar 2025 13:26:30 +1100 Subject: [PATCH 05/31] leds: led-triggers: Improvements for default trigger Accept "default" written to sysfs trigger attr. If the text "default" is written to the LED's sysfs 'trigger' attr, then call led_trigger_set_default() to set the LED to its default trigger. If the default trigger is set to "none", then led_trigger_set_default() will remove a trigger. This is in contrast to the default trigger being unset, in which case led_trigger_set_default() does nothing. Signed-off-by: Craig McQueen Reviewed-by: Jacek Anaszewski Link: https://lore.kernel.org/r/20250317022630.424015-1-craig@mcqueen.au Signed-off-by: Lee Jones --- Documentation/ABI/testing/sysfs-class-led | 6 ++++++ drivers/leds/led-triggers.c | 13 +++++++++++++ 2 files changed, 19 insertions(+) diff --git a/Documentation/ABI/testing/sysfs-class-led b/Documentation/ABI/testing/sysfs-class-led index 2e24ac3bd7ef..0313b82644f2 100644 --- a/Documentation/ABI/testing/sysfs-class-led +++ b/Documentation/ABI/testing/sysfs-class-led @@ -72,6 +72,12 @@ Description: /sys/class/leds/ once a given trigger is selected. For their documentation see `sysfs-class-led-trigger-*`. + Writing "none" removes the trigger for this LED. + + Writing "default" sets the trigger to the LED's default trigger + (which would often be configured in the device tree for the + hardware). + What: /sys/class/leds//inverted Date: January 2011 KernelVersion: 2.6.38 diff --git a/drivers/leds/led-triggers.c b/drivers/leds/led-triggers.c index b2d40f87a5ff..3799dcc1cf07 100644 --- a/drivers/leds/led-triggers.c +++ b/drivers/leds/led-triggers.c @@ -54,6 +54,11 @@ ssize_t led_trigger_write(struct file *filp, struct kobject *kobj, goto unlock; } + if (sysfs_streq(buf, "default")) { + led_trigger_set_default(led_cdev); + goto unlock; + } + down_read(&triggers_list_lock); list_for_each_entry(trig, &trigger_list, next_trig) { if (sysfs_streq(buf, trig->name) && trigger_relevant(led_cdev, trig)) { @@ -98,6 +103,9 @@ static int led_trigger_format(char *buf, size_t size, int len = led_trigger_snprintf(buf, size, "%s", led_cdev->trigger ? "none" : "[none]"); + if (led_cdev->default_trigger) + len += led_trigger_snprintf(buf + len, size - len, " default"); + list_for_each_entry(trig, &trigger_list, next_trig) { bool hit; @@ -281,6 +289,11 @@ void led_trigger_set_default(struct led_classdev *led_cdev) if (!led_cdev->default_trigger) return; + if (!strcmp(led_cdev->default_trigger, "none")) { + led_trigger_remove(led_cdev); + return; + } + down_read(&triggers_list_lock); down_write(&led_cdev->trigger_lock); list_for_each_entry(trig, &trigger_list, next_trig) { From ee44a1def7ee4c6f9f04a02bfa1a106ee41434d3 Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Tue, 18 Mar 2025 18:04:29 +0200 Subject: [PATCH 06/31] leds: core: Bail out when composed name can't fit the buffer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GCC compiler complains about snprintf() calls that may potentially cut the output: drivers/leds/led-core.c:551:78: error: ‘snprintf’ output may be truncated before the last format character [-Werror=format-truncation=] drivers/leds/led-core.c:554:78: error: ‘snprintf’ output may be truncated before the last format character [-Werror=format-truncation=] ... Fix these by checking for the potential overflow. This requires to align all the branches to use the same callee, i.e. snprintf(), otherwise the code will be blown up and return different error codes for the different branches. Signed-off-by: Andy Shevchenko Link: https://lore.kernel.org/r/20250318160524.2979982-1-andriy.shevchenko@linux.intel.com Signed-off-by: Lee Jones --- drivers/leds/led-core.c | 45 +++++++++++++++++++++++------------------ 1 file changed, 25 insertions(+), 20 deletions(-) diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c index 907fc703e0c5..1a59a4f38479 100644 --- a/drivers/leds/led-core.c +++ b/drivers/leds/led-core.c @@ -529,6 +529,7 @@ int led_compose_name(struct device *dev, struct led_init_data *init_data, struct led_properties props = {}; struct fwnode_handle *fwnode = init_data->fwnode; const char *devicename = init_data->devicename; + int n; if (!led_classdev_name) return -EINVAL; @@ -542,45 +543,49 @@ int led_compose_name(struct device *dev, struct led_init_data *init_data, * Otherwise the label is prepended with devicename to compose * the final LED class device name. */ - if (!devicename) { - strscpy(led_classdev_name, props.label, - LED_MAX_NAME_SIZE); + if (devicename) { + n = snprintf(led_classdev_name, LED_MAX_NAME_SIZE, "%s:%s", + devicename, props.label); } else { - snprintf(led_classdev_name, LED_MAX_NAME_SIZE, "%s:%s", - devicename, props.label); + n = snprintf(led_classdev_name, LED_MAX_NAME_SIZE, "%s", props.label); } } else if (props.function || props.color_present) { char tmp_buf[LED_MAX_NAME_SIZE]; if (props.func_enum_present) { - snprintf(tmp_buf, LED_MAX_NAME_SIZE, "%s:%s-%d", - props.color_present ? led_colors[props.color] : "", - props.function ?: "", props.func_enum); + n = snprintf(tmp_buf, LED_MAX_NAME_SIZE, "%s:%s-%d", + props.color_present ? led_colors[props.color] : "", + props.function ?: "", props.func_enum); } else { - snprintf(tmp_buf, LED_MAX_NAME_SIZE, "%s:%s", - props.color_present ? led_colors[props.color] : "", - props.function ?: ""); + n = snprintf(tmp_buf, LED_MAX_NAME_SIZE, "%s:%s", + props.color_present ? led_colors[props.color] : "", + props.function ?: ""); } - if (init_data->devname_mandatory) { - snprintf(led_classdev_name, LED_MAX_NAME_SIZE, "%s:%s", - devicename, tmp_buf); - } else { - strscpy(led_classdev_name, tmp_buf, LED_MAX_NAME_SIZE); + if (n >= LED_MAX_NAME_SIZE) + return -E2BIG; + if (init_data->devname_mandatory) { + n = snprintf(led_classdev_name, LED_MAX_NAME_SIZE, "%s:%s", + devicename, tmp_buf); + } else { + n = snprintf(led_classdev_name, LED_MAX_NAME_SIZE, "%s", tmp_buf); } } else if (init_data->default_label) { if (!devicename) { dev_err(dev, "Legacy LED naming requires devicename segment"); return -EINVAL; } - snprintf(led_classdev_name, LED_MAX_NAME_SIZE, "%s:%s", - devicename, init_data->default_label); + n = snprintf(led_classdev_name, LED_MAX_NAME_SIZE, "%s:%s", + devicename, init_data->default_label); } else if (is_of_node(fwnode)) { - strscpy(led_classdev_name, to_of_node(fwnode)->name, - LED_MAX_NAME_SIZE); + n = snprintf(led_classdev_name, LED_MAX_NAME_SIZE, "%s", + to_of_node(fwnode)->name); } else return -EINVAL; + if (n >= LED_MAX_NAME_SIZE) + return -E2BIG; + return 0; } EXPORT_SYMBOL_GPL(led_compose_name); From ca72d5ef59513c448f03d20a58e1942af6441b44 Mon Sep 17 00:00:00 2001 From: Manuel Fombuena Date: Tue, 25 Feb 2025 22:02:28 +0000 Subject: [PATCH 07/31] Documentation: leds: Remove .rst extension for leds-st1202 on index No other LED driver is listed on index.rst with the extension .rst. Remove it. Fixes: b1816b22381b ("Documentation:leds: Add leds-st1202.rst") Signed-off-by: Manuel Fombuena Link: https://lore.kernel.org/r/CWLP123MB5473137572529F99746F4AC4C5C32@CWLP123MB5473.GBRP123.PROD.OUTLOOK.COM Signed-off-by: Lee Jones --- Documentation/leds/index.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/leds/index.rst b/Documentation/leds/index.rst index 0ab0a2128a11..76fae171039c 100644 --- a/Documentation/leds/index.rst +++ b/Documentation/leds/index.rst @@ -28,5 +28,5 @@ LEDs leds-mlxcpld leds-mt6370-rgb leds-sc27xx - leds-st1202.rst + leds-st1202 leds-qcom-lpg From b2661df9febda90b2bc7e5b867431c8433f49e79 Mon Sep 17 00:00:00 2001 From: "Gustavo A. R. Silva" Date: Mon, 31 Mar 2025 11:01:38 -0600 Subject: [PATCH 08/31] leds: leds-cros_ec: Avoid -Wflex-array-member-not-at-end warning MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit -Wflex-array-member-not-at-end was introduced in GCC-14, and we are getting ready to enable it, globally. Replace an on-stack definition of a flexible structure with a call to utility function cros_ec_cmd(). So, with these changes, fix the following warning: drivers/leds/leds-cros_ec.c:70:40: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end] Signed-off-by: "Gustavo A. R. Silva" Acked-by: Thomas Weißschuh Link: https://lore.kernel.org/r/Z-rKcgFjsyKvd58q@kspp Signed-off-by: Lee Jones --- drivers/leds/leds-cros_ec.c | 19 +++---------------- 1 file changed, 3 insertions(+), 16 deletions(-) diff --git a/drivers/leds/leds-cros_ec.c b/drivers/leds/leds-cros_ec.c index 275522b81ea5..377cf04e202a 100644 --- a/drivers/leds/leds-cros_ec.c +++ b/drivers/leds/leds-cros_ec.c @@ -60,31 +60,18 @@ static inline struct cros_ec_led_priv *cros_ec_led_cdev_to_priv(struct led_class union cros_ec_led_cmd_data { struct ec_params_led_control req; struct ec_response_led_control resp; -} __packed; +}; static int cros_ec_led_send_cmd(struct cros_ec_device *cros_ec, union cros_ec_led_cmd_data *arg) { int ret; - struct { - struct cros_ec_command msg; - union cros_ec_led_cmd_data data; - } __packed buf = { - .msg = { - .version = 1, - .command = EC_CMD_LED_CONTROL, - .insize = sizeof(arg->resp), - .outsize = sizeof(arg->req), - }, - .data.req = arg->req - }; - ret = cros_ec_cmd_xfer_status(cros_ec, &buf.msg); + ret = cros_ec_cmd(cros_ec, 1, EC_CMD_LED_CONTROL, &arg->req, + sizeof(arg->req), &arg->resp, sizeof(arg->resp)); if (ret < 0) return ret; - arg->resp = buf.data.resp; - return 0; } From e35ca991a777ef513040cbb36bc8245a031a2633 Mon Sep 17 00:00:00 2001 From: Sven Schwermer Date: Fri, 4 Apr 2025 20:40:36 +0200 Subject: [PATCH 09/31] leds: multicolor: Fix intensity setting while SW blinking When writing to the multi_intensity file, don't unconditionally call led_set_brightness. By only doing this if blinking is inactive we prevent blinking from stopping if the blinking is in its off phase while the file is written. Instead, if blinking is active, the changed intensity values are applied upon the next blink. This is consistent with changing the brightness on monochrome LEDs with active blinking. Suggested-by: Jacek Anaszewski Acked-by: Jacek Anaszewski Acked-by: Pavel Machek Reviewed-by: Tobias Deiminger Tested-by: Sven Schuchmann Signed-off-by: Sven Schwermer Link: https://lore.kernel.org/r/20250404184043.227116-1-sven@svenschwermer.de Signed-off-by: Lee Jones --- drivers/leds/led-class-multicolor.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/leds/led-class-multicolor.c b/drivers/leds/led-class-multicolor.c index b2a87c994816..fd66d2bdeace 100644 --- a/drivers/leds/led-class-multicolor.c +++ b/drivers/leds/led-class-multicolor.c @@ -59,7 +59,8 @@ static ssize_t multi_intensity_store(struct device *dev, for (i = 0; i < mcled_cdev->num_colors; i++) mcled_cdev->subled_info[i].intensity = intensity_value[i]; - led_set_brightness(led_cdev, led_cdev->brightness); + if (!test_bit(LED_BLINK_SW, &led_cdev->work_flags)) + led_set_brightness(led_cdev, led_cdev->brightness); ret = size; err_out: mutex_unlock(&led_cdev->led_access); From bd3d14932923a717ebe475122ca7e17200f87a0c Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Mon, 7 Apr 2025 18:13:26 +0300 Subject: [PATCH 10/31] leds: pca955x: Avoid potential overflow when filling default_label MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GCC compiler (Debian 14.2.0-17) is not happy about printing into a too short buffer (when build with `make W=1`): drivers/leds/leds-pca955x.c:554:33: note: ‘snprintf’ output between 2 and 12 bytes into a destination of size 8 Indeed, the buffer size is chosen based on some assumptions, while in general the assigned value might not fit (GCC can't prove it does). Fix this by changing the bits field in the struct pca955x_chipdef to u8, with a positive side effect of the better memory footprint, and convert loop iterator to be unsigned. With that done, update format specifiers accordingly. In one case join back string literal as it improves the grepping over the code based on the message and remove duplicating information (the driver name is printed as pert of the dev_*() output [1]) as we touch the same line anyway. Link: https://lore.kernel.org/r/4ac527f2-c59e-70a2-efd4-da52370ea557@dave.eu/ [1] Signed-off-by: Andy Shevchenko Link: https://lore.kernel.org/r/20250407151441.706378-1-andriy.shevchenko@linux.intel.com Signed-off-by: Lee Jones --- drivers/leds/leds-pca955x.c | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/drivers/leds/leds-pca955x.c b/drivers/leds/leds-pca955x.c index e9cfde9fe4b1..24a40a1cdb15 100644 --- a/drivers/leds/leds-pca955x.c +++ b/drivers/leds/leds-pca955x.c @@ -73,7 +73,7 @@ enum pca955x_type { }; struct pca955x_chipdef { - int bits; + u8 bits; u8 slv_addr; /* 7-bit slave address mask */ int slv_addr_shift; /* Number of bits to ignore */ int blink_div; /* PSC divider */ @@ -142,13 +142,13 @@ struct pca955x_platform_data { }; /* 8 bits per input register */ -static inline int pca955x_num_input_regs(int bits) +static inline u8 pca955x_num_input_regs(u8 bits) { return (bits + 7) / 8; } /* 4 bits per LED selector register */ -static inline int pca955x_num_led_regs(int bits) +static inline u8 pca955x_num_led_regs(u8 bits) { return (bits + 3) / 4; } @@ -581,14 +581,14 @@ static int pca955x_probe(struct i2c_client *client) struct led_classdev *led; struct led_init_data init_data; struct i2c_adapter *adapter; - int i, bit, err, nls, reg; + u8 i, nls, psc0; u8 ls1[4]; u8 ls2[4]; struct pca955x_platform_data *pdata; - u8 psc0; bool keep_psc0 = false; bool set_default_label = false; char default_label[8]; + int bit, err, reg; chip = i2c_get_match_data(client); if (!chip) @@ -610,16 +610,15 @@ static int pca955x_probe(struct i2c_client *client) return -ENODEV; } - dev_info(&client->dev, "leds-pca955x: Using %s %d-bit LED driver at " - "slave address 0x%02x\n", client->name, chip->bits, - client->addr); + dev_info(&client->dev, "Using %s %u-bit LED driver at slave address 0x%02x\n", + client->name, chip->bits, client->addr); if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA)) return -EIO; if (pdata->num_leds != chip->bits) { dev_err(&client->dev, - "board info claims %d LEDs on a %d-bit chip\n", + "board info claims %d LEDs on a %u-bit chip\n", pdata->num_leds, chip->bits); return -ENODEV; } @@ -694,8 +693,7 @@ static int pca955x_probe(struct i2c_client *client) } if (set_default_label) { - snprintf(default_label, sizeof(default_label), - "%d", i); + snprintf(default_label, sizeof(default_label), "%u", i); init_data.default_label = default_label; } else { init_data.default_label = NULL; From 4bab18dcb452e00e28e181eb2d7a3a3a6a5d1413 Mon Sep 17 00:00:00 2001 From: Andrew Davis Date: Mon, 7 Apr 2025 13:35:50 -0500 Subject: [PATCH 11/31] leds: lp8860: Use regmap_multi_reg_write for EEPROM writes This helper does the same thing as manual looping, use it instead. Signed-off-by: Andrew Davis Link: https://lore.kernel.org/r/20250407183555.409687-1-afd@ti.com Signed-off-by: Lee Jones --- drivers/leds/leds-lp8860.c | 21 ++++++--------------- 1 file changed, 6 insertions(+), 15 deletions(-) diff --git a/drivers/leds/leds-lp8860.c b/drivers/leds/leds-lp8860.c index 995f2adf8569..2b1c68e60949 100644 --- a/drivers/leds/leds-lp8860.c +++ b/drivers/leds/leds-lp8860.c @@ -103,12 +103,7 @@ struct lp8860_led { struct regulator *regulator; }; -struct lp8860_eeprom_reg { - uint8_t reg; - uint8_t value; -}; - -static struct lp8860_eeprom_reg lp8860_eeprom_disp_regs[] = { +static const struct reg_sequence lp8860_eeprom_disp_regs[] = { { LP8860_EEPROM_REG_0, 0xed }, { LP8860_EEPROM_REG_1, 0xdf }, { LP8860_EEPROM_REG_2, 0xdc }, @@ -238,7 +233,7 @@ static int lp8860_brightness_set(struct led_classdev *led_cdev, static int lp8860_init(struct lp8860_led *led) { unsigned int read_buf; - int ret, i, reg_count; + int ret, reg_count; if (led->regulator) { ret = regulator_enable(led->regulator); @@ -266,14 +261,10 @@ static int lp8860_init(struct lp8860_led *led) } reg_count = ARRAY_SIZE(lp8860_eeprom_disp_regs); - for (i = 0; i < reg_count; i++) { - ret = regmap_write(led->eeprom_regmap, - lp8860_eeprom_disp_regs[i].reg, - lp8860_eeprom_disp_regs[i].value); - if (ret) { - dev_err(&led->client->dev, "Failed writing EEPROM\n"); - goto out; - } + ret = regmap_multi_reg_write(led->eeprom_regmap, lp8860_eeprom_disp_regs, reg_count); + if (ret) { + dev_err(&led->client->dev, "Failed writing EEPROM\n"); + goto out; } ret = lp8860_unlock_eeprom(led, LP8860_LOCK_EEPROM); From 87a59548af95db9b6b3a19fdd5b4e6c49bdbc285 Mon Sep 17 00:00:00 2001 From: Andrew Davis Date: Mon, 7 Apr 2025 13:35:51 -0500 Subject: [PATCH 12/31] leds: lp8860: Use new mutex guards to cleanup function exits Use scoped mutex guards to simplify return paths. While here use devm_mutex_init() to register the muxex so it also is cleaned up automatically. Signed-off-by: Andrew Davis Link: https://lore.kernel.org/r/20250407183555.409687-2-afd@ti.com Signed-off-by: Lee Jones --- drivers/leds/leds-lp8860.c | 27 +++++++++++---------------- 1 file changed, 11 insertions(+), 16 deletions(-) diff --git a/drivers/leds/leds-lp8860.c b/drivers/leds/leds-lp8860.c index 2b1c68e60949..2d91f476f0b7 100644 --- a/drivers/leds/leds-lp8860.c +++ b/drivers/leds/leds-lp8860.c @@ -135,7 +135,7 @@ static int lp8860_unlock_eeprom(struct lp8860_led *led, int lock) { int ret; - mutex_lock(&led->lock); + guard(mutex)(&led->lock); if (lock == LP8860_UNLOCK_EEPROM) { ret = regmap_write(led->regmap, @@ -143,7 +143,7 @@ static int lp8860_unlock_eeprom(struct lp8860_led *led, int lock) LP8860_EEPROM_CODE_1); if (ret) { dev_err(&led->client->dev, "EEPROM Unlock failed\n"); - goto out; + return ret; } ret = regmap_write(led->regmap, @@ -151,14 +151,14 @@ static int lp8860_unlock_eeprom(struct lp8860_led *led, int lock) LP8860_EEPROM_CODE_2); if (ret) { dev_err(&led->client->dev, "EEPROM Unlock failed\n"); - goto out; + return ret; } ret = regmap_write(led->regmap, LP8860_EEPROM_UNLOCK, LP8860_EEPROM_CODE_3); if (ret) { dev_err(&led->client->dev, "EEPROM Unlock failed\n"); - goto out; + return ret; } } else { ret = regmap_write(led->regmap, @@ -166,8 +166,6 @@ static int lp8860_unlock_eeprom(struct lp8860_led *led, int lock) LP8860_LOCK_EEPROM); } -out: - mutex_unlock(&led->lock); return ret; } @@ -204,30 +202,29 @@ static int lp8860_brightness_set(struct led_classdev *led_cdev, int disp_brightness = brt_val * 255; int ret; - mutex_lock(&led->lock); + guard(mutex)(&led->lock); ret = lp8860_fault_check(led); if (ret) { dev_err(&led->client->dev, "Cannot read/clear faults\n"); - goto out; + return ret; } ret = regmap_write(led->regmap, LP8860_DISP_CL1_BRT_MSB, (disp_brightness & 0xff00) >> 8); if (ret) { dev_err(&led->client->dev, "Cannot write CL1 MSB\n"); - goto out; + return ret; } ret = regmap_write(led->regmap, LP8860_DISP_CL1_BRT_LSB, disp_brightness & 0xff); if (ret) { dev_err(&led->client->dev, "Cannot write CL1 LSB\n"); - goto out; + return ret; } -out: - mutex_unlock(&led->lock); - return ret; + + return 0; } static int lp8860_init(struct lp8860_led *led) @@ -392,7 +389,7 @@ static int lp8860_probe(struct i2c_client *client) led->client = client; led->led_dev.brightness_set_blocking = lp8860_brightness_set; - mutex_init(&led->lock); + devm_mutex_init(&client->dev, &led->lock); i2c_set_clientdata(client, led); @@ -443,8 +440,6 @@ static void lp8860_remove(struct i2c_client *client) dev_err(&led->client->dev, "Failed to disable regulator\n"); } - - mutex_destroy(&led->lock); } static const struct i2c_device_id lp8860_id[] = { From 0cb55e16bd842364340d24e2e368b9d6c5800423 Mon Sep 17 00:00:00 2001 From: Andrew Davis Date: Mon, 7 Apr 2025 13:35:52 -0500 Subject: [PATCH 13/31] leds: lp8860: Remove default regs when not caching If we are not using regmap caches, then the value will be read in every time, having a default value does not change anything in that case. Remove the unused defaults. Signed-off-by: Andrew Davis Link: https://lore.kernel.org/r/20250407183555.409687-3-afd@ti.com Signed-off-by: Lee Jones --- drivers/leds/leds-lp8860.c | 52 -------------------------------------- 1 file changed, 52 deletions(-) diff --git a/drivers/leds/leds-lp8860.c b/drivers/leds/leds-lp8860.c index 2d91f476f0b7..4cd1b960d504 100644 --- a/drivers/leds/leds-lp8860.c +++ b/drivers/leds/leds-lp8860.c @@ -292,61 +292,11 @@ static int lp8860_init(struct lp8860_led *led) return ret; } -static const struct reg_default lp8860_reg_defs[] = { - { LP8860_DISP_CL1_BRT_MSB, 0x00}, - { LP8860_DISP_CL1_BRT_LSB, 0x00}, - { LP8860_DISP_CL1_CURR_MSB, 0x00}, - { LP8860_DISP_CL1_CURR_LSB, 0x00}, - { LP8860_CL2_BRT_MSB, 0x00}, - { LP8860_CL2_BRT_LSB, 0x00}, - { LP8860_CL2_CURRENT, 0x00}, - { LP8860_CL3_BRT_MSB, 0x00}, - { LP8860_CL3_BRT_LSB, 0x00}, - { LP8860_CL3_CURRENT, 0x00}, - { LP8860_CL4_BRT_MSB, 0x00}, - { LP8860_CL4_BRT_LSB, 0x00}, - { LP8860_CL4_CURRENT, 0x00}, - { LP8860_CONFIG, 0x00}, - { LP8860_FAULT_CLEAR, 0x00}, - { LP8860_EEPROM_CNTRL, 0x80}, - { LP8860_EEPROM_UNLOCK, 0x00}, -}; - static const struct regmap_config lp8860_regmap_config = { .reg_bits = 8, .val_bits = 8, .max_register = LP8860_EEPROM_UNLOCK, - .reg_defaults = lp8860_reg_defs, - .num_reg_defaults = ARRAY_SIZE(lp8860_reg_defs), -}; - -static const struct reg_default lp8860_eeprom_defs[] = { - { LP8860_EEPROM_REG_0, 0x00 }, - { LP8860_EEPROM_REG_1, 0x00 }, - { LP8860_EEPROM_REG_2, 0x00 }, - { LP8860_EEPROM_REG_3, 0x00 }, - { LP8860_EEPROM_REG_4, 0x00 }, - { LP8860_EEPROM_REG_5, 0x00 }, - { LP8860_EEPROM_REG_6, 0x00 }, - { LP8860_EEPROM_REG_7, 0x00 }, - { LP8860_EEPROM_REG_8, 0x00 }, - { LP8860_EEPROM_REG_9, 0x00 }, - { LP8860_EEPROM_REG_10, 0x00 }, - { LP8860_EEPROM_REG_11, 0x00 }, - { LP8860_EEPROM_REG_12, 0x00 }, - { LP8860_EEPROM_REG_13, 0x00 }, - { LP8860_EEPROM_REG_14, 0x00 }, - { LP8860_EEPROM_REG_15, 0x00 }, - { LP8860_EEPROM_REG_16, 0x00 }, - { LP8860_EEPROM_REG_17, 0x00 }, - { LP8860_EEPROM_REG_18, 0x00 }, - { LP8860_EEPROM_REG_19, 0x00 }, - { LP8860_EEPROM_REG_20, 0x00 }, - { LP8860_EEPROM_REG_21, 0x00 }, - { LP8860_EEPROM_REG_22, 0x00 }, - { LP8860_EEPROM_REG_23, 0x00 }, - { LP8860_EEPROM_REG_24, 0x00 }, }; static const struct regmap_config lp8860_eeprom_regmap_config = { @@ -354,8 +304,6 @@ static const struct regmap_config lp8860_eeprom_regmap_config = { .val_bits = 8, .max_register = LP8860_EEPROM_REG_24, - .reg_defaults = lp8860_eeprom_defs, - .num_reg_defaults = ARRAY_SIZE(lp8860_eeprom_defs), }; static int lp8860_probe(struct i2c_client *client) From b0d6394094ee657f50b42b44f4903014d90e912a Mon Sep 17 00:00:00 2001 From: Andrew Davis Date: Mon, 7 Apr 2025 13:35:53 -0500 Subject: [PATCH 14/31] leds: lp8860: Enable regulator using enable_optional helper This allows the regulator to be optional which is the same as done here with all the checks for NULL. This also disables on remove for us, so remove the manual disabling. Signed-off-by: Andrew Davis Link: https://lore.kernel.org/r/20250407183555.409687-4-afd@ti.com Signed-off-by: Lee Jones --- drivers/leds/leds-lp8860.c | 32 ++++---------------------------- 1 file changed, 4 insertions(+), 28 deletions(-) diff --git a/drivers/leds/leds-lp8860.c b/drivers/leds/leds-lp8860.c index 4cd1b960d504..2ceebdb5820e 100644 --- a/drivers/leds/leds-lp8860.c +++ b/drivers/leds/leds-lp8860.c @@ -91,7 +91,6 @@ * @regmap: Devices register map * @eeprom_regmap: EEPROM register map * @enable_gpio: VDDIO/EN gpio to enable communication interface - * @regulator: LED supply regulator pointer */ struct lp8860_led { struct mutex lock; @@ -100,7 +99,6 @@ struct lp8860_led { struct regmap *regmap; struct regmap *eeprom_regmap; struct gpio_desc *enable_gpio; - struct regulator *regulator; }; static const struct reg_sequence lp8860_eeprom_disp_regs[] = { @@ -232,15 +230,6 @@ static int lp8860_init(struct lp8860_led *led) unsigned int read_buf; int ret, reg_count; - if (led->regulator) { - ret = regulator_enable(led->regulator); - if (ret) { - dev_err(&led->client->dev, - "Failed to enable regulator\n"); - return ret; - } - } - gpiod_direction_output(led->enable_gpio, 1); ret = lp8860_fault_check(led); @@ -282,13 +271,6 @@ static int lp8860_init(struct lp8860_led *led) if (ret) gpiod_direction_output(led->enable_gpio, 0); - if (led->regulator) { - ret = regulator_disable(led->regulator); - if (ret) - dev_err(&led->client->dev, - "Failed to disable regulator\n"); - } - return ret; } @@ -330,9 +312,10 @@ static int lp8860_probe(struct i2c_client *client) return ret; } - led->regulator = devm_regulator_get(&client->dev, "vled"); - if (IS_ERR(led->regulator)) - led->regulator = NULL; + ret = devm_regulator_get_enable_optional(&client->dev, "vled"); + if (ret && ret != -ENODEV) + return dev_err_probe(&client->dev, ret, + "Failed to enable vled regulator\n"); led->client = client; led->led_dev.brightness_set_blocking = lp8860_brightness_set; @@ -381,13 +364,6 @@ static void lp8860_remove(struct i2c_client *client) int ret; gpiod_direction_output(led->enable_gpio, 0); - - if (led->regulator) { - ret = regulator_disable(led->regulator); - if (ret) - dev_err(&led->client->dev, - "Failed to disable regulator\n"); - } } static const struct i2c_device_id lp8860_id[] = { From e0b95ba33c0f31ac91741921cc89986536595862 Mon Sep 17 00:00:00 2001 From: Andrew Davis Date: Mon, 7 Apr 2025 13:35:54 -0500 Subject: [PATCH 15/31] leds: lp8860: Only unlock in lp8860_unlock_eeprom() Locking is a single register write, so no need to have the unlock function also lock. This removes the need to pass in the option and reduces the nesting level in the function. Signed-off-by: Andrew Davis Link: https://lore.kernel.org/r/20250407183555.409687-5-afd@ti.com Signed-off-by: Lee Jones --- drivers/leds/leds-lp8860.c | 46 ++++++++++++++------------------------ 1 file changed, 17 insertions(+), 29 deletions(-) diff --git a/drivers/leds/leds-lp8860.c b/drivers/leds/leds-lp8860.c index 2ceebdb5820e..8b3660f80f4c 100644 --- a/drivers/leds/leds-lp8860.c +++ b/drivers/leds/leds-lp8860.c @@ -129,39 +129,27 @@ static const struct reg_sequence lp8860_eeprom_disp_regs[] = { { LP8860_EEPROM_REG_24, 0x3E }, }; -static int lp8860_unlock_eeprom(struct lp8860_led *led, int lock) +static int lp8860_unlock_eeprom(struct lp8860_led *led) { int ret; guard(mutex)(&led->lock); - if (lock == LP8860_UNLOCK_EEPROM) { - ret = regmap_write(led->regmap, - LP8860_EEPROM_UNLOCK, - LP8860_EEPROM_CODE_1); - if (ret) { - dev_err(&led->client->dev, "EEPROM Unlock failed\n"); - return ret; - } + ret = regmap_write(led->regmap, LP8860_EEPROM_UNLOCK, LP8860_EEPROM_CODE_1); + if (ret) { + dev_err(&led->client->dev, "EEPROM Unlock failed\n"); + return ret; + } - ret = regmap_write(led->regmap, - LP8860_EEPROM_UNLOCK, - LP8860_EEPROM_CODE_2); - if (ret) { - dev_err(&led->client->dev, "EEPROM Unlock failed\n"); - return ret; - } - ret = regmap_write(led->regmap, - LP8860_EEPROM_UNLOCK, - LP8860_EEPROM_CODE_3); - if (ret) { - dev_err(&led->client->dev, "EEPROM Unlock failed\n"); - return ret; - } - } else { - ret = regmap_write(led->regmap, - LP8860_EEPROM_UNLOCK, - LP8860_LOCK_EEPROM); + ret = regmap_write(led->regmap, LP8860_EEPROM_UNLOCK, LP8860_EEPROM_CODE_2); + if (ret) { + dev_err(&led->client->dev, "EEPROM Unlock failed\n"); + return ret; + } + ret = regmap_write(led->regmap, LP8860_EEPROM_UNLOCK, LP8860_EEPROM_CODE_3); + if (ret) { + dev_err(&led->client->dev, "EEPROM Unlock failed\n"); + return ret; } return ret; @@ -240,7 +228,7 @@ static int lp8860_init(struct lp8860_led *led) if (ret) goto out; - ret = lp8860_unlock_eeprom(led, LP8860_UNLOCK_EEPROM); + ret = lp8860_unlock_eeprom(led); if (ret) { dev_err(&led->client->dev, "Failed unlocking EEPROM\n"); goto out; @@ -253,7 +241,7 @@ static int lp8860_init(struct lp8860_led *led) goto out; } - ret = lp8860_unlock_eeprom(led, LP8860_LOCK_EEPROM); + ret = regmap_write(led->regmap, LP8860_EEPROM_UNLOCK, LP8860_LOCK_EEPROM); if (ret) goto out; From 982e0f042542fc0c2fbcb2b67fbedc431624ae59 Mon Sep 17 00:00:00 2001 From: Andrew Davis Date: Mon, 7 Apr 2025 13:35:55 -0500 Subject: [PATCH 16/31] leds: lp8860: Disable GPIO with devm action This helps prevent mistakes like disable out of order in cleanup functions and forgetting to free on error paths (as was done here). This was the last thing the .remove() function did, so remove that too. Signed-off-by: Andrew Davis Link: https://lore.kernel.org/r/20250407183555.409687-6-afd@ti.com Signed-off-by: Lee Jones --- drivers/leds/leds-lp8860.c | 38 +++++++++++++------------------------- 1 file changed, 13 insertions(+), 25 deletions(-) diff --git a/drivers/leds/leds-lp8860.c b/drivers/leds/leds-lp8860.c index 8b3660f80f4c..52b97c9f2a03 100644 --- a/drivers/leds/leds-lp8860.c +++ b/drivers/leds/leds-lp8860.c @@ -90,7 +90,6 @@ * @led_dev: led class device pointer * @regmap: Devices register map * @eeprom_regmap: EEPROM register map - * @enable_gpio: VDDIO/EN gpio to enable communication interface */ struct lp8860_led { struct mutex lock; @@ -98,7 +97,6 @@ struct lp8860_led { struct led_classdev led_dev; struct regmap *regmap; struct regmap *eeprom_regmap; - struct gpio_desc *enable_gpio; }; static const struct reg_sequence lp8860_eeprom_disp_regs[] = { @@ -218,8 +216,6 @@ static int lp8860_init(struct lp8860_led *led) unsigned int read_buf; int ret, reg_count; - gpiod_direction_output(led->enable_gpio, 1); - ret = lp8860_fault_check(led); if (ret) goto out; @@ -256,9 +252,6 @@ static int lp8860_init(struct lp8860_led *led) return ret; out: - if (ret) - gpiod_direction_output(led->enable_gpio, 0); - return ret; } @@ -276,6 +269,13 @@ static const struct regmap_config lp8860_eeprom_regmap_config = { .max_register = LP8860_EEPROM_REG_24, }; +static void lp8860_disable_gpio(void *data) +{ + struct gpio_desc *gpio = data; + + gpiod_set_value(gpio, 0); +} + static int lp8860_probe(struct i2c_client *client) { int ret; @@ -283,6 +283,7 @@ static int lp8860_probe(struct i2c_client *client) struct device_node *np = dev_of_node(&client->dev); struct device_node *child_node; struct led_init_data init_data = {}; + struct gpio_desc *enable_gpio; led = devm_kzalloc(&client->dev, sizeof(*led), GFP_KERNEL); if (!led) @@ -292,13 +293,11 @@ static int lp8860_probe(struct i2c_client *client) if (!child_node) return -EINVAL; - led->enable_gpio = devm_gpiod_get_optional(&client->dev, - "enable", GPIOD_OUT_LOW); - if (IS_ERR(led->enable_gpio)) { - ret = PTR_ERR(led->enable_gpio); - dev_err(&client->dev, "Failed to get enable gpio: %d\n", ret); - return ret; - } + enable_gpio = devm_gpiod_get_optional(&client->dev, "enable", GPIOD_OUT_LOW); + if (IS_ERR(enable_gpio)) + return dev_err_probe(&client->dev, PTR_ERR(enable_gpio), + "Failed to get enable GPIO\n"); + devm_add_action_or_reset(&client->dev, lp8860_disable_gpio, enable_gpio); ret = devm_regulator_get_enable_optional(&client->dev, "vled"); if (ret && ret != -ENODEV) @@ -310,8 +309,6 @@ static int lp8860_probe(struct i2c_client *client) devm_mutex_init(&client->dev, &led->lock); - i2c_set_clientdata(client, led); - led->regmap = devm_regmap_init_i2c(client, &lp8860_regmap_config); if (IS_ERR(led->regmap)) { ret = PTR_ERR(led->regmap); @@ -346,14 +343,6 @@ static int lp8860_probe(struct i2c_client *client) return 0; } -static void lp8860_remove(struct i2c_client *client) -{ - struct lp8860_led *led = i2c_get_clientdata(client); - int ret; - - gpiod_direction_output(led->enable_gpio, 0); -} - static const struct i2c_device_id lp8860_id[] = { { "lp8860" }, { } @@ -372,7 +361,6 @@ static struct i2c_driver lp8860_driver = { .of_match_table = of_lp8860_leds_match, }, .probe = lp8860_probe, - .remove = lp8860_remove, .id_table = lp8860_id, }; module_i2c_driver(lp8860_driver); From f9a2eacb9107365c3f1e3061e2c435cafd554fc8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Beh=C3=BAn?= Date: Thu, 17 Apr 2025 09:05:07 +0200 Subject: [PATCH 17/31] leds: turris-omnia: Drop commas in the terminator entries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Drop commas in terminator entries of `struct attribute` array and `struct of_device_id` array. Signed-off-by: Marek Behún Link: https://lore.kernel.org/r/20250417070507.24929-1-kabel@kernel.org Signed-off-by: Lee Jones --- drivers/leds/leds-turris-omnia.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/leds/leds-turris-omnia.c b/drivers/leds/leds-turris-omnia.c index 4fe1a9c0bc1b..25ee5c1eb820 100644 --- a/drivers/leds/leds-turris-omnia.c +++ b/drivers/leds/leds-turris-omnia.c @@ -361,7 +361,7 @@ static DEVICE_ATTR_RW(gamma_correction); static struct attribute *omnia_led_controller_attrs[] = { &dev_attr_brightness.attr, &dev_attr_gamma_correction.attr, - NULL, + NULL }; ATTRIBUTE_GROUPS(omnia_led_controller); @@ -527,7 +527,7 @@ static void omnia_leds_remove(struct i2c_client *client) static const struct of_device_id of_omnia_leds_match[] = { { .compatible = "cznic,turris-omnia-leds", }, - {}, + { } }; MODULE_DEVICE_TABLE(of, of_omnia_leds_match); From 4c6c3ca07b7a70e7b8fe5c25cbf0650c422a3a28 Mon Sep 17 00:00:00 2001 From: Krzysztof Kozlowski Date: Thu, 17 Apr 2025 09:46:56 +0200 Subject: [PATCH 18/31] leds: Do not enable by default during compile testing Enabling the compile test should not cause automatic enabling of all drivers, but only allow to choose to compile them. Signed-off-by: Krzysztof Kozlowski Link: https://lore.kernel.org/r/20250417074656.81626-1-krzysztof.kozlowski@linaro.org Signed-off-by: Lee Jones --- drivers/leds/Kconfig | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig index a104cbb0a001..b107dbe1fa90 100644 --- a/drivers/leds/Kconfig +++ b/drivers/leds/Kconfig @@ -735,7 +735,7 @@ config LEDS_NS2 tristate "LED support for Network Space v2 GPIO LEDs" depends on LEDS_CLASS depends on MACH_KIRKWOOD || MACH_ARMADA_370 || COMPILE_TEST - default y + default y if MACH_KIRKWOOD || MACH_ARMADA_370 help This option enables support for the dual-GPIO LEDs found on the following LaCie/Seagate boards: @@ -750,7 +750,7 @@ config LEDS_NETXBIG depends on LEDS_CLASS depends on MACH_KIRKWOOD || COMPILE_TEST depends on OF_GPIO - default y + default MACH_KIRKWOOD help This option enables support for LEDs found on the LaCie 2Big and 5Big Network v2 boards. The LEDs are wired to a CPLD and are From ee08ec51a0a01ed8702a7f91e5102dad92319172 Mon Sep 17 00:00:00 2001 From: Bartosz Golaszewski Date: Wed, 23 Apr 2025 09:53:50 +0200 Subject: [PATCH 19/31] leds: lgm-sso: Use new GPIO line value setter callbacks struct gpio_chip now has callbacks for setting line values that return an integer, allowing to indicate failures. Convert the driver to using them. Signed-off-by: Bartosz Golaszewski Reviewed-by: Linus Walleij Link: https://lore.kernel.org/r/20250423-gpiochip-set-rv-leds-v1-1-2f42d8fbb525@linaro.org Signed-off-by: Lee Jones --- drivers/leds/blink/leds-lgm-sso.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/leds/blink/leds-lgm-sso.c b/drivers/leds/blink/leds-lgm-sso.c index effaaaf302b5..c9027f9c4bb7 100644 --- a/drivers/leds/blink/leds-lgm-sso.c +++ b/drivers/leds/blink/leds-lgm-sso.c @@ -450,7 +450,7 @@ static int sso_gpio_get(struct gpio_chip *chip, unsigned int offset) return !!(reg_val & BIT(offset)); } -static void sso_gpio_set(struct gpio_chip *chip, unsigned int offset, int value) +static int sso_gpio_set(struct gpio_chip *chip, unsigned int offset, int value) { struct sso_led_priv *priv = gpiochip_get_data(chip); @@ -458,6 +458,8 @@ static void sso_gpio_set(struct gpio_chip *chip, unsigned int offset, int value) if (!priv->gpio.freq) regmap_update_bits(priv->mmap, SSO_CON0, SSO_CON0_SWU, SSO_CON0_SWU); + + return 0; } static int sso_gpio_gc_init(struct device *dev, struct sso_led_priv *priv) @@ -469,7 +471,7 @@ static int sso_gpio_gc_init(struct device *dev, struct sso_led_priv *priv) gc->get_direction = sso_gpio_get_dir; gc->direction_output = sso_gpio_dir_out; gc->get = sso_gpio_get; - gc->set = sso_gpio_set; + gc->set_rv = sso_gpio_set; gc->label = "lgm-sso"; gc->base = -1; From 2aafd2e41cf1434c680ea7410333a079a15747c3 Mon Sep 17 00:00:00 2001 From: Bartosz Golaszewski Date: Wed, 23 Apr 2025 09:53:51 +0200 Subject: [PATCH 20/31] leds: pca955x: Use new GPIO line value setter callbacks struct gpio_chip now has callbacks for setting line values that return an integer, allowing to indicate failures. Convert the driver to using them. Signed-off-by: Bartosz Golaszewski Reviewed-by: Linus Walleij Link: https://lore.kernel.org/r/20250423-gpiochip-set-rv-leds-v1-2-2f42d8fbb525@linaro.org Signed-off-by: Lee Jones --- drivers/leds/leds-pca955x.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/leds/leds-pca955x.c b/drivers/leds/leds-pca955x.c index 24a40a1cdb15..42fe056b1c74 100644 --- a/drivers/leds/leds-pca955x.c +++ b/drivers/leds/leds-pca955x.c @@ -495,10 +495,10 @@ static int pca955x_set_value(struct gpio_chip *gc, unsigned int offset, return pca955x_led_set(&led->led_cdev, PCA955X_GPIO_LOW); } -static void pca955x_gpio_set_value(struct gpio_chip *gc, unsigned int offset, - int val) +static int pca955x_gpio_set_value(struct gpio_chip *gc, unsigned int offset, + int val) { - pca955x_set_value(gc, offset, val); + return pca955x_set_value(gc, offset, val); } static int pca955x_gpio_get_value(struct gpio_chip *gc, unsigned int offset) @@ -737,7 +737,7 @@ static int pca955x_probe(struct i2c_client *client) pca955x->gpio.label = "gpio-pca955x"; pca955x->gpio.direction_input = pca955x_gpio_direction_input; pca955x->gpio.direction_output = pca955x_gpio_direction_output; - pca955x->gpio.set = pca955x_gpio_set_value; + pca955x->gpio.set_rv = pca955x_gpio_set_value; pca955x->gpio.get = pca955x_gpio_get_value; pca955x->gpio.request = pca955x_gpio_request_pin; pca955x->gpio.free = pca955x_gpio_free_pin; From e1cc2c8cc7ccccb52814d10c98de1252f8a7ae61 Mon Sep 17 00:00:00 2001 From: Bartosz Golaszewski Date: Wed, 23 Apr 2025 09:53:52 +0200 Subject: [PATCH 21/31] leds: pca9532: Use new GPIO line value setter callbacks struct gpio_chip now has callbacks for setting line values that return an integer, allowing to indicate failures. Convert the driver to using them. Signed-off-by: Bartosz Golaszewski Reviewed-by: Linus Walleij Link: https://lore.kernel.org/r/20250423-gpiochip-set-rv-leds-v1-3-2f42d8fbb525@linaro.org Signed-off-by: Lee Jones --- drivers/leds/leds-pca9532.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/drivers/leds/leds-pca9532.c b/drivers/leds/leds-pca9532.c index 1b47acf54720..7d4c071a6cd0 100644 --- a/drivers/leds/leds-pca9532.c +++ b/drivers/leds/leds-pca9532.c @@ -318,7 +318,8 @@ static int pca9532_gpio_request_pin(struct gpio_chip *gc, unsigned offset) return -EBUSY; } -static void pca9532_gpio_set_value(struct gpio_chip *gc, unsigned offset, int val) +static int pca9532_gpio_set_value(struct gpio_chip *gc, unsigned int offset, + int val) { struct pca9532_data *data = gpiochip_get_data(gc); struct pca9532_led *led = &data->leds[offset]; @@ -329,6 +330,8 @@ static void pca9532_gpio_set_value(struct gpio_chip *gc, unsigned offset, int va led->state = PCA9532_OFF; pca9532_setled(led); + + return 0; } static int pca9532_gpio_get_value(struct gpio_chip *gc, unsigned offset) @@ -351,9 +354,7 @@ static int pca9532_gpio_direction_input(struct gpio_chip *gc, unsigned offset) static int pca9532_gpio_direction_output(struct gpio_chip *gc, unsigned offset, int val) { - pca9532_gpio_set_value(gc, offset, val); - - return 0; + return pca9532_gpio_set_value(gc, offset, val); } #endif /* CONFIG_LEDS_PCA9532_GPIO */ @@ -472,7 +473,7 @@ static int pca9532_configure(struct i2c_client *client, data->gpio.label = "gpio-pca9532"; data->gpio.direction_input = pca9532_gpio_direction_input; data->gpio.direction_output = pca9532_gpio_direction_output; - data->gpio.set = pca9532_gpio_set_value; + data->gpio.set_rv = pca9532_gpio_set_value; data->gpio.get = pca9532_gpio_get_value; data->gpio.request = pca9532_gpio_request_pin; data->gpio.can_sleep = 1; From d1d3205730735b652303a82ba49fcfef7c20510d Mon Sep 17 00:00:00 2001 From: Bartosz Golaszewski Date: Wed, 23 Apr 2025 09:53:53 +0200 Subject: [PATCH 22/31] leds: tca6507: Use new GPIO line value setter callbacks struct gpio_chip now has callbacks for setting line values that return an integer, allowing to indicate failures. Convert the driver to using them. Signed-off-by: Bartosz Golaszewski Reviewed-by: Linus Walleij Link: https://lore.kernel.org/r/20250423-gpiochip-set-rv-leds-v1-4-2f42d8fbb525@linaro.org Signed-off-by: Lee Jones --- drivers/leds/leds-tca6507.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/drivers/leds/leds-tca6507.c b/drivers/leds/leds-tca6507.c index acbd8169723c..89c165c8ee9c 100644 --- a/drivers/leds/leds-tca6507.c +++ b/drivers/leds/leds-tca6507.c @@ -588,8 +588,8 @@ static int tca6507_blink_set(struct led_classdev *led_cdev, } #ifdef CONFIG_GPIOLIB -static void tca6507_gpio_set_value(struct gpio_chip *gc, - unsigned offset, int val) +static int tca6507_gpio_set_value(struct gpio_chip *gc, unsigned int offset, + int val) { struct tca6507_chip *tca = gpiochip_get_data(gc); unsigned long flags; @@ -604,13 +604,14 @@ static void tca6507_gpio_set_value(struct gpio_chip *gc, spin_unlock_irqrestore(&tca->lock, flags); if (tca->reg_set) schedule_work(&tca->work); + + return 0; } static int tca6507_gpio_direction_output(struct gpio_chip *gc, unsigned offset, int val) { - tca6507_gpio_set_value(gc, offset, val); - return 0; + return tca6507_gpio_set_value(gc, offset, val); } static int tca6507_probe_gpios(struct device *dev, @@ -636,7 +637,7 @@ static int tca6507_probe_gpios(struct device *dev, tca->gpio.base = -1; tca->gpio.owner = THIS_MODULE; tca->gpio.direction_output = tca6507_gpio_direction_output; - tca->gpio.set = tca6507_gpio_set_value; + tca->gpio.set_rv = tca6507_gpio_set_value; tca->gpio.parent = dev; err = devm_gpiochip_add_data(dev, &tca->gpio, tca); if (err) { From 5039a33fed8851fcf384fae2bcb8fd4858edd597 Mon Sep 17 00:00:00 2001 From: Lee Jones Date: Thu, 24 Apr 2025 15:45:38 +0100 Subject: [PATCH 23/31] leds: Provide skeleton KUnit testing for the LEDs framework Apply a very basic implementation of KUnit LED testing. More tests / use-cases will be added steadily over time. CMD: tools/testing/kunit/kunit.py run --kunitconfig drivers/leds OUTPUT: [15:34:19] Configuring KUnit Kernel ... [15:34:19] Building KUnit Kernel ... Populating config with: $ make ARCH=um O=.kunit olddefconfig Building with: $ make all compile_commands.json scripts_gdb ARCH=um O=.kunit --jobs=20 [15:34:22] Starting KUnit Kernel (1/1)... [15:34:22] ============================================================ Running tests with: $ .kunit/linux kunit.enable=1 mem=1G console=tty kunit_shutdown=halt [15:34:23] ===================== led (1 subtest) ====================== [15:34:23] [PASSED] led_test_class_register [15:34:23] ======================= [PASSED] led ======================= [15:34:23] ============================================================ [15:34:23] Testing complete. Ran 1 tests: passed: 1 [15:34:23] Elapsed time: 4.268s total, 0.001s configuring, 3.048s building, 1.214s running Link: https://lore.kernel.org/r/20250424144544.1438584-1-lee@kernel.org Signed-off-by: Lee Jones --- drivers/leds/.kunitconfig | 4 +++ drivers/leds/Kconfig | 7 ++++ drivers/leds/Makefile | 1 + drivers/leds/led-test.c | 76 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 88 insertions(+) create mode 100644 drivers/leds/.kunitconfig create mode 100644 drivers/leds/led-test.c diff --git a/drivers/leds/.kunitconfig b/drivers/leds/.kunitconfig new file mode 100644 index 000000000000..5180f77910a1 --- /dev/null +++ b/drivers/leds/.kunitconfig @@ -0,0 +1,4 @@ +CONFIG_KUNIT=y +CONFIG_NEW_LEDS=y +CONFIG_LEDS_CLASS=y +CONFIG_LEDS_KUNIT_TEST=y diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig index b107dbe1fa90..6e3dce7e35a4 100644 --- a/drivers/leds/Kconfig +++ b/drivers/leds/Kconfig @@ -55,6 +55,13 @@ config LEDS_BRIGHTNESS_HW_CHANGED See Documentation/ABI/testing/sysfs-class-led for details. +config LEDS_KUNIT_TEST + tristate "KUnit tests for LEDs" + depends on KUNIT && LEDS_CLASS + default KUNIT_ALL_TESTS + help + Say Y here to enable KUnit testing for the LEDs framework. + comment "LED drivers" config LEDS_88PM860X diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile index 2f170d69dcbf..9a0333ec1a86 100644 --- a/drivers/leds/Makefile +++ b/drivers/leds/Makefile @@ -6,6 +6,7 @@ obj-$(CONFIG_LEDS_CLASS) += led-class.o obj-$(CONFIG_LEDS_CLASS_FLASH) += led-class-flash.o obj-$(CONFIG_LEDS_CLASS_MULTICOLOR) += led-class-multicolor.o obj-$(CONFIG_LEDS_TRIGGERS) += led-triggers.o +obj-$(CONFIG_LEDS_KUNIT_TEST) += led-test.o # LED Platform Drivers (keep this sorted, M-| sort) obj-$(CONFIG_LEDS_88PM860X) += leds-88pm860x.o diff --git a/drivers/leds/led-test.c b/drivers/leds/led-test.c new file mode 100644 index 000000000000..068c9d0eb683 --- /dev/null +++ b/drivers/leds/led-test.c @@ -0,0 +1,76 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Copyright (C) 2025 Google LLC + * + * Author: Lee Jones + */ + +#include +#include +#include +#include + +struct led_test_ddata { + struct led_classdev cdev; + struct device *dev; +}; + +static void led_test_class_register(struct kunit *test) +{ + struct led_test_ddata *ddata = test->priv; + struct led_classdev *cdev = &ddata->cdev; + struct device *dev = ddata->dev; + int ret; + + cdev->name = "led-test"; + + ret = devm_led_classdev_register(dev, cdev); + KUNIT_ASSERT_EQ(test, ret, 0); + if (ret) + return; +} + +static struct kunit_case led_test_cases[] = { + KUNIT_CASE(led_test_class_register), + { } +}; + +static int led_test_init(struct kunit *test) +{ + struct led_test_ddata *ddata; + struct device *dev; + + ddata = kunit_kzalloc(test, sizeof(*ddata), GFP_KERNEL); + if (!ddata) + return -ENOMEM; + + test->priv = ddata; + + dev = kunit_device_register(test, "led_test"); + if (IS_ERR(dev)) + return PTR_ERR(dev); + + ddata->dev = get_device(dev); + + return 0; +} + +static void led_test_exit(struct kunit *test) +{ + struct led_test_ddata *ddata = test->priv; + + if (ddata && ddata->dev) + put_device(ddata->dev); +} + +static struct kunit_suite led_test_suite = { + .name = "led", + .init = led_test_init, + .exit = led_test_exit, + .test_cases = led_test_cases, +}; +kunit_test_suite(led_test_suite); + +MODULE_AUTHOR("Lee Jones "); +MODULE_DESCRIPTION("KUnit tests for the LED framework"); +MODULE_LICENSE("GPL"); From b441b95a592c42f8448f9bd912b91e1e9b4262ff Mon Sep 17 00:00:00 2001 From: Jesse Karjalainen Date: Sat, 26 Apr 2025 05:04:54 +0300 Subject: [PATCH 24/31] leds: pca995x: Fix typo in pca995x_of_match's of_device_id entry Remove the stray space between the '.' and the 'data' field name in the PCA995x device-tree match table. Signed-off-by: Jesse Karjalainen Link: https://lore.kernel.org/r/20250426020454.47059-1-jesse@ponkila.com Signed-off-by: Lee Jones --- drivers/leds/leds-pca995x.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/leds/leds-pca995x.c b/drivers/leds/leds-pca995x.c index 11c7bb69573e..6ad06ce2bf64 100644 --- a/drivers/leds/leds-pca995x.c +++ b/drivers/leds/leds-pca995x.c @@ -197,7 +197,7 @@ MODULE_DEVICE_TABLE(i2c, pca995x_id); static const struct of_device_id pca995x_of_match[] = { { .compatible = "nxp,pca9952", .data = &pca9952_chipdef }, - { .compatible = "nxp,pca9955b", . data = &pca9955b_chipdef }, + { .compatible = "nxp,pca9955b", .data = &pca9955b_chipdef }, { .compatible = "nxp,pca9956b", .data = &pca9956b_chipdef }, {}, }; From 1d7f25483c8791f395dd857cce8a6a65bcfa295f Mon Sep 17 00:00:00 2001 From: Lee Jones Date: Thu, 1 May 2025 09:19:11 +0100 Subject: [PATCH 25/31] leds: led-test: Remove standard error checking after KUNIT_ASSERT_*() If a KUNIT_ASSERT_*() call ends up in an assertion, the test is marked as a failure and the subsequent error checking is never executed, making it superfluous. Remove it for simplicity and to avoid confusion. Link: https://lore.kernel.org/r/20250501081918.3621432-1-lee@kernel.org Signed-off-by: Lee Jones --- drivers/leds/led-test.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/leds/led-test.c b/drivers/leds/led-test.c index 068c9d0eb683..23820189abe3 100644 --- a/drivers/leds/led-test.c +++ b/drivers/leds/led-test.c @@ -26,8 +26,6 @@ static void led_test_class_register(struct kunit *test) ret = devm_led_classdev_register(dev, cdev); KUNIT_ASSERT_EQ(test, ret, 0); - if (ret) - return; } static struct kunit_case led_test_cases[] = { From eb58933b78cdad9b879dc2dd248e7284341a1cfc Mon Sep 17 00:00:00 2001 From: Lee Jones Date: Thu, 1 May 2025 09:19:12 +0100 Subject: [PATCH 26/31] leds: led-test: Fill out the registration test to cover more test cases Upon successful LED class device registration, it is expected that certain attributes are filled out in pre-defined ways. For instance, if provided, the .brightness_get() call-back should be called to populate the class device 'brightness' attribute, 'max_brightness' should be initialised as LED_FULL (at least until we can rid these pesky enums) and the sysfs group should be created with the class device name supplied by the caller. If subsequent registrations take place that would result in name conflicts, various outcomes are expected depending on which flags are set. If LED_REJECT_NAME_CONFLICT is disabled, registration should succeed resulting in an iteration on the provided name. Conversely, if it's enabled, then registration is expected to fail outright. We test for all of these scenarios here. Link: https://lore.kernel.org/r/20250501081918.3621432-2-lee@kernel.org Signed-off-by: Lee Jones --- drivers/leds/led-test.c | 31 ++++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/drivers/leds/led-test.c b/drivers/leds/led-test.c index 23820189abe3..bc85e4513745 100644 --- a/drivers/leds/led-test.c +++ b/drivers/leds/led-test.c @@ -10,22 +10,51 @@ #include #include +#define LED_TEST_POST_REG_BRIGHTNESS 10 + struct led_test_ddata { struct led_classdev cdev; struct device *dev; }; +static enum led_brightness led_test_brightness_get(struct led_classdev *cdev) +{ + return LED_TEST_POST_REG_BRIGHTNESS; +} + static void led_test_class_register(struct kunit *test) { struct led_test_ddata *ddata = test->priv; - struct led_classdev *cdev = &ddata->cdev; + struct led_classdev *cdev_clash, *cdev = &ddata->cdev; struct device *dev = ddata->dev; int ret; + /* Register a LED class device */ cdev->name = "led-test"; + cdev->brightness_get = led_test_brightness_get; + cdev->brightness = 0; ret = devm_led_classdev_register(dev, cdev); KUNIT_ASSERT_EQ(test, ret, 0); + + KUNIT_EXPECT_EQ(test, cdev->max_brightness, LED_FULL); + KUNIT_EXPECT_EQ(test, cdev->brightness, LED_TEST_POST_REG_BRIGHTNESS); + KUNIT_EXPECT_STREQ(test, cdev->dev->kobj.name, "led-test"); + + /* Register again with the same name - expect it to pass with the LED renamed */ + cdev_clash = devm_kmemdup(dev, cdev, sizeof(*cdev), GFP_KERNEL); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, cdev_clash); + + ret = devm_led_classdev_register(dev, cdev_clash); + KUNIT_ASSERT_EQ(test, ret, 0); + + KUNIT_EXPECT_STREQ(test, cdev_clash->dev->kobj.name, "led-test_1"); + KUNIT_EXPECT_STREQ(test, cdev_clash->name, "led-test"); + + /* Enable name conflict rejection and register with the same name again - expect failure */ + cdev_clash->flags |= LED_REJECT_NAME_CONFLICT; + ret = devm_led_classdev_register(dev, cdev_clash); + KUNIT_EXPECT_EQ(test, ret, -EEXIST); } static struct kunit_case led_test_cases[] = { From cfa40f29df08c7ad296c81a2fac677830af83ec7 Mon Sep 17 00:00:00 2001 From: Lee Jones Date: Thu, 1 May 2025 09:19:13 +0100 Subject: [PATCH 27/31] leds: led-test: Provide tests for the lookup and get infrastructure This API allows providers to offer an specific LED to be looked-up by a consumer. Consumers are then able to describe the aforementioned LED and take a reference on it. For convenience, we're testing both sides of the API in just one test function here. In reality, both the provider and the consumer would be logistically orthogonal. CMD: tools/testing/kunit/kunit.py run --kunitconfig drivers/leds RESULTS: [16:38:57] Configuring KUnit Kernel ... [16:38:57] Building KUnit Kernel ... Populating config with: $ make ARCH=um O=.kunit olddefconfig Building with: $ make all compile_commands.json scripts_gdb ARCH=um O=.kunit --jobs=20 [16:39:02] Starting KUnit Kernel (1/1)... [16:39:02] ============================================================ Running tests with: $ .kunit/linux kunit.enable=1 mem=1G console=tty kunit_shutdown=halt [16:39:03] ===================== led (2 subtests) ===================== [16:39:03] [PASSED] led_test_class_register [16:39:03] [PASSED] led_test_class_add_lookup_and_get [16:39:03] ======================= [PASSED] led ======================= [16:39:03] ============================================================ [16:39:03] Testing complete. Ran 2 tests: passed: 2 [16:39:03] Elapsed time: 6.255s total, 0.001s configuring, 5.131s building, 1.106s running Link: https://lore.kernel.org/r/20250501081918.3621432-3-lee@kernel.org Signed-off-by: Lee Jones --- drivers/leds/led-test.c | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/drivers/leds/led-test.c b/drivers/leds/led-test.c index bc85e4513745..ddf9aa967a6a 100644 --- a/drivers/leds/led-test.c +++ b/drivers/leds/led-test.c @@ -57,8 +57,37 @@ static void led_test_class_register(struct kunit *test) KUNIT_EXPECT_EQ(test, ret, -EEXIST); } +static void led_test_class_add_lookup_and_get(struct kunit *test) +{ + struct led_test_ddata *ddata = test->priv; + struct led_classdev *cdev = &ddata->cdev, *cdev_get; + struct device *dev = ddata->dev; + struct led_lookup_data lookup; + int ret; + + /* First, register a LED class device */ + cdev->name = "led-test"; + ret = devm_led_classdev_register(dev, cdev); + KUNIT_ASSERT_EQ(test, ret, 0); + + /* Then make the LED available for lookup */ + lookup.provider = cdev->name; + lookup.dev_id = dev_name(dev); + lookup.con_id = "led-test-1"; + led_add_lookup(&lookup); + + /* Finally, attempt to look it up via the API - imagine this was an orthogonal driver */ + cdev_get = devm_led_get(dev, "led-test-1"); + KUNIT_ASSERT_FALSE(test, IS_ERR(cdev_get)); + + KUNIT_EXPECT_STREQ(test, cdev_get->name, cdev->name); + + led_remove_lookup(&lookup); +} + static struct kunit_case led_test_cases[] = { KUNIT_CASE(led_test_class_register), + KUNIT_CASE(led_test_class_add_lookup_and_get), { } }; From f1c86ab98640e9288049673ca2e4517a9df7ed83 Mon Sep 17 00:00:00 2001 From: Christophe JAILLET Date: Fri, 2 May 2025 16:51:22 +0200 Subject: [PATCH 28/31] leds: rgb: leds-mt6370-rgb: Improve definition of some struct linear_range Use LINEAR_RANGE() instead of hand-writing it. It is more robust, should the layout of the structure change one day. Signed-off-by: Christophe JAILLET Link: https://lore.kernel.org/r/1ce4245107e0a51dce502a007a69899bda018d5f.1746197460.git.christophe.jaillet@wanadoo.fr Signed-off-by: Lee Jones --- drivers/leds/rgb/leds-mt6370-rgb.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/leds/rgb/leds-mt6370-rgb.c b/drivers/leds/rgb/leds-mt6370-rgb.c index ebd3ba878dd5..c5927d0eb830 100644 --- a/drivers/leds/rgb/leds-mt6370-rgb.c +++ b/drivers/leds/rgb/leds-mt6370-rgb.c @@ -199,17 +199,17 @@ static const struct reg_field mt6372_reg_fields[F_MAX_FIELDS] = { /* Current unit: microamp, time unit: millisecond */ static const struct linear_range common_led_ranges[R_MAX_RANGES] = { - [R_LED123_CURR] = { 4000, 1, 6, 4000 }, - [R_LED4_CURR] = { 2000, 1, 3, 2000 }, - [R_LED_TRFON] = { 125, 0, 15, 200 }, - [R_LED_TOFF] = { 250, 0, 15, 400 }, + [R_LED123_CURR] = LINEAR_RANGE(4000, 1, 6, 4000), + [R_LED4_CURR] = LINEAR_RANGE(2000, 1, 3, 2000), + [R_LED_TRFON] = LINEAR_RANGE(125, 0, 15, 200), + [R_LED_TOFF] = LINEAR_RANGE(250, 0, 15, 400), }; static const struct linear_range mt6372_led_ranges[R_MAX_RANGES] = { - [R_LED123_CURR] = { 2000, 1, 14, 2000 }, - [R_LED4_CURR] = { 2000, 1, 14, 2000 }, - [R_LED_TRFON] = { 125, 0, 15, 250 }, - [R_LED_TOFF] = { 250, 0, 15, 500 }, + [R_LED123_CURR] = LINEAR_RANGE(2000, 1, 14, 2000), + [R_LED4_CURR] = LINEAR_RANGE(2000, 1, 14, 2000), + [R_LED_TRFON] = LINEAR_RANGE(125, 0, 15, 250), + [R_LED_TOFF] = LINEAR_RANGE(250, 0, 15, 500), }; static const unsigned int common_tfreqs[] = { From 6a09ae8281986d5fb5ce0115135c05a5afb8718e Mon Sep 17 00:00:00 2001 From: Richard Leitner Date: Wed, 7 May 2025 09:51:31 +0200 Subject: [PATCH 29/31] leds: flash: Add support for flash/strobe duration Add support for the new V4L2_CID_FLASH_DURATION control to the LEDs driver. Signed-off-by: Richard Leitner Link: https://lore.kernel.org/r/20250507-ov9282-flash-strobe-v4-2-72b299c1b7c9@linux.dev Signed-off-by: Lee Jones --- drivers/leds/led-class-flash.c | 15 +++++++++++++++ include/linux/led-class-flash.h | 16 ++++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/drivers/leds/led-class-flash.c b/drivers/leds/led-class-flash.c index f4e26ce84862..165035a8826c 100644 --- a/drivers/leds/led-class-flash.c +++ b/drivers/leds/led-class-flash.c @@ -440,6 +440,21 @@ int led_update_flash_brightness(struct led_classdev_flash *fled_cdev) } EXPORT_SYMBOL_GPL(led_update_flash_brightness); +int led_set_flash_duration(struct led_classdev_flash *fled_cdev, u32 duration) +{ + struct led_classdev *led_cdev = &fled_cdev->led_cdev; + struct led_flash_setting *s = &fled_cdev->duration; + + s->val = duration; + led_clamp_align(s); + + if (!(led_cdev->flags & LED_SUSPENDED)) + return call_flash_op(fled_cdev, duration_set, s->val); + + return 0; +} +EXPORT_SYMBOL_GPL(led_set_flash_duration); + MODULE_AUTHOR("Jacek Anaszewski "); MODULE_DESCRIPTION("LED Flash class interface"); MODULE_LICENSE("GPL v2"); diff --git a/include/linux/led-class-flash.h b/include/linux/led-class-flash.h index 36df927ec4b7..21ec856c36bc 100644 --- a/include/linux/led-class-flash.h +++ b/include/linux/led-class-flash.h @@ -45,6 +45,8 @@ struct led_flash_ops { int (*timeout_set)(struct led_classdev_flash *fled_cdev, u32 timeout); /* get the flash LED fault */ int (*fault_get)(struct led_classdev_flash *fled_cdev, u32 *fault); + /* set flash duration */ + int (*duration_set)(struct led_classdev_flash *fled_cdev, u32 duration); }; /* @@ -75,6 +77,9 @@ struct led_classdev_flash { /* flash timeout value in microseconds along with its constraints */ struct led_flash_setting timeout; + /* flash timeout value in microseconds along with its constraints */ + struct led_flash_setting duration; + /* LED Flash class sysfs groups */ const struct attribute_group *sysfs_groups[LED_FLASH_SYSFS_GROUPS_SIZE]; }; @@ -209,4 +214,15 @@ int led_set_flash_timeout(struct led_classdev_flash *fled_cdev, u32 timeout); */ int led_get_flash_fault(struct led_classdev_flash *fled_cdev, u32 *fault); +/** + * led_set_flash_duration - set flash LED duration + * @fled_cdev: the flash LED to set + * @timeout: the flash duration to set it to + * + * Set the flash strobe duration. + * + * Returns: 0 on success or negative error value on failure + */ +int led_set_flash_duration(struct led_classdev_flash *fled_cdev, u32 duration); + #endif /* __LINUX_FLASH_LEDS_H_INCLUDED */ From 0d12bb1a7fb6aee144cc2c710573763221b4c245 Mon Sep 17 00:00:00 2001 From: Matthias Fend Date: Wed, 14 May 2025 12:10:07 +0200 Subject: [PATCH 30/31] dt-bindings: leds: Add Texas Instruments TPS6131x flash LED driver Document Texas Instruments TPS61310/TPS61311 flash LED driver devicetree bindings. Signed-off-by: Matthias Fend Reviewed-by: Krzysztof Kozlowski Link: https://lore.kernel.org/r/20250514-leds-tps6131x-v5-1-a4fb9e7f2c47@emfend.at Signed-off-by: Lee Jones --- .../devicetree/bindings/leds/ti,tps61310.yaml | 120 ++++++++++++++++++ 1 file changed, 120 insertions(+) create mode 100644 Documentation/devicetree/bindings/leds/ti,tps61310.yaml diff --git a/Documentation/devicetree/bindings/leds/ti,tps61310.yaml b/Documentation/devicetree/bindings/leds/ti,tps61310.yaml new file mode 100644 index 000000000000..118f9c8bfdf7 --- /dev/null +++ b/Documentation/devicetree/bindings/leds/ti,tps61310.yaml @@ -0,0 +1,120 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/leds/ti,tps61310.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Texas Instruments TPS6131X flash LED driver + +maintainers: + - Matthias Fend + +description: | + The TPS61310/TPS61311 is a flash LED driver with I2C interface. + Its power stage is capable of supplying a maximum total current of roughly 1500mA. + The TPS6131x provides three constant-current sinks, capable of sinking + up to 2 x 400mA (LED1 and LED3) and 800mA (LED2) in flash mode. + In torch mode, each sink (LED1, LED2, LED3) supports currents up to 175mA. + Since the three current sinks share most of the control components such as + flash timer, control logic, safety timer and the operating mode, they cannot + be used completely independently of each other. Therefore, only one LED is + supported, but the current sinks can be combined accordingly. + + The data sheet can be found at: + https://www.ti.com/lit/ds/symlink/tps61310.pdf + +properties: + compatible: + oneOf: + - items: + - enum: + - ti,tps61311 + - const: ti,tps61310 + - items: + - const: ti,tps61310 + + reg: + maxItems: 1 + + reset-gpios: + maxItems: 1 + description: GPIO connected to NRESET pin + + ti,valley-current-limit: + type: boolean + description: + Reduce the valley peak current limit from 1750mA to 1250mA (TPS61310) or + from 2480mA to 1800mA (TPS61311). + + led: + type: object + $ref: common.yaml# + unevaluatedProperties: false + + properties: + led-sources: + minItems: 1 + maxItems: 3 + items: + enum: [1, 2, 3] + + led-max-microamp: + oneOf: + - minimum: 50000 + maximum: 350000 + multipleOf: 50000 + - minimum: 25000 + maximum: 525000 + multipleOf: 25000 + + flash-max-microamp: + oneOf: + - minimum: 50000 + maximum: 800000 + multipleOf: 50000 + - minimum: 25000 + maximum: 1500000 + multipleOf: 25000 + + flash-max-timeout-us: + enum: [ 5300, 10700, 16000, 21300, 26600, 32000, 37300, 68200, 71500, + 102200, 136300, 170400, 204500, 340800, 579300, 852000 ] + + required: + - led-sources + - led-max-microamp + - flash-max-microamp + - flash-max-timeout-us + +required: + - compatible + - reg + - led + +additionalProperties: false + +examples: + - | + #include + #include + + i2c { + #address-cells = <1>; + #size-cells = <0>; + + led-controller@33 { + compatible = "ti,tps61311", "ti,tps61310"; + reg = <0x33>; + + reset-gpios = <&gpio1 0 GPIO_ACTIVE_LOW>; + + led { + function = LED_FUNCTION_FLASH; + color = ; + led-sources = <1>, <2>, <3>; + led-max-microamp = <525000>; + flash-max-microamp = <1500000>; + flash-max-timeout-us = <852000>; + }; + }; + }; From b338a2ae9b316df1d81b5289badcc8cbbbfe1b2b Mon Sep 17 00:00:00 2001 From: Matthias Fend Date: Wed, 14 May 2025 12:10:08 +0200 Subject: [PATCH 31/31] leds: tps6131x: Add support for Texas Instruments TPS6131X flash LED driver The TPS61310/TPS61311 is a flash LED driver with I2C interface. Its power stage is capable of supplying a maximum total current of roughly 1500mA. The TPS6131x provides three constant-current sinks, capable of sinking up to 2 x 400mA (LED1 and LED3) and 800mA (LED2) in flash mode. In torch mode each sink (LED1, LED2, LED3) supports currents up to 175mA. Signed-off-by: Matthias Fend Link: https://lore.kernel.org/r/20250514-leds-tps6131x-v5-2-a4fb9e7f2c47@emfend.at Signed-off-by: Lee Jones --- MAINTAINERS | 7 + drivers/leds/flash/Kconfig | 11 + drivers/leds/flash/Makefile | 1 + drivers/leds/flash/leds-tps6131x.c | 815 +++++++++++++++++++++++++++++ 4 files changed, 834 insertions(+) create mode 100644 drivers/leds/flash/leds-tps6131x.c diff --git a/MAINTAINERS b/MAINTAINERS index 96b827049501..b55fc0632e0b 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -23925,6 +23925,13 @@ F: Documentation/devicetree/bindings/hwmon/ti,tps23861.yaml F: Documentation/hwmon/tps23861.rst F: drivers/hwmon/tps23861.c +TEXAS INSTRUMENTS TPS6131X FLASH LED DRIVER +M: Matthias Fend +L: linux-leds@vger.kernel.org +S: Maintained +F: Documentation/devicetree/bindings/leds/ti,tps6131x.yaml +F: drivers/leds/flash/leds-tps6131x.c + TEXAS INSTRUMENTS' DAC7612 DAC DRIVER M: Ricardo Ribalda L: linux-iio@vger.kernel.org diff --git a/drivers/leds/flash/Kconfig b/drivers/leds/flash/Kconfig index f39f0bfe6eef..55ca663ca506 100644 --- a/drivers/leds/flash/Kconfig +++ b/drivers/leds/flash/Kconfig @@ -132,4 +132,15 @@ config LEDS_SY7802 This driver can be built as a module, it will be called "leds-sy7802". +config LEDS_TPS6131X + tristate "LED support for TI TPS6131x flash LED driver" + depends on I2C && OF + depends on GPIOLIB + select REGMAP_I2C + help + This option enables support for Texas Instruments TPS61310/TPS61311 + flash LED driver. + + This driver can be built as a module, it will be called "leds-tps6131x". + endif # LEDS_CLASS_FLASH diff --git a/drivers/leds/flash/Makefile b/drivers/leds/flash/Makefile index 48860eeced79..712fb737a428 100644 --- a/drivers/leds/flash/Makefile +++ b/drivers/leds/flash/Makefile @@ -12,3 +12,4 @@ obj-$(CONFIG_LEDS_RT4505) += leds-rt4505.o obj-$(CONFIG_LEDS_RT8515) += leds-rt8515.o obj-$(CONFIG_LEDS_SGM3140) += leds-sgm3140.o obj-$(CONFIG_LEDS_SY7802) += leds-sy7802.o +obj-$(CONFIG_LEDS_TPS6131X) += leds-tps6131x.o diff --git a/drivers/leds/flash/leds-tps6131x.c b/drivers/leds/flash/leds-tps6131x.c new file mode 100644 index 000000000000..6f4d4fd55361 --- /dev/null +++ b/drivers/leds/flash/leds-tps6131x.c @@ -0,0 +1,815 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * Texas Instruments TPS61310/TPS61311 flash LED driver with I2C interface + * + * Copyright 2025 Matthias Fend + */ + +#include +#include +#include +#include +#include +#include +#include + +#define TPS6131X_REG_0 0x00 +#define TPS6131X_REG_0_RESET BIT(7) +#define TPS6131X_REG_0_DCLC13 GENMASK(5, 3) +#define TPS6131X_REG_0_DCLC13_SHIFT 3 +#define TPS6131X_REG_0_DCLC2 GENMASK(2, 0) +#define TPS6131X_REG_0_DCLC2_SHIFT 0 + +#define TPS6131X_REG_1 0x01 +#define TPS6131X_REG_1_MODE GENMASK(7, 6) +#define TPS6131X_REG_1_MODE_SHIFT 6 +#define TPS6131X_REG_1_FC2 GENMASK(5, 0) +#define TPS6131X_REG_1_FC2_SHIFT 0 + +#define TPS6131X_REG_2 0x02 +#define TPS6131X_REG_2_MODE GENMASK(7, 6) +#define TPS6131X_REG_2_MODE_SHIFT 6 +#define TPS6131X_REG_2_ENVM BIT(5) +#define TPS6131X_REG_2_FC13 GENMASK(4, 0) +#define TPS6131X_REG_2_FC13_SHIFT 0 + +#define TPS6131X_REG_3 0x03 +#define TPS6131X_REG_3_STIM GENMASK(7, 5) +#define TPS6131X_REG_3_STIM_SHIFT 5 +#define TPS6131X_REG_3_HPFL BIT(4) +#define TPS6131X_REG_3_SELSTIM_TO BIT(3) +#define TPS6131X_REG_3_STT BIT(2) +#define TPS6131X_REG_3_SFT BIT(1) +#define TPS6131X_REG_3_TXMASK BIT(0) + +#define TPS6131X_REG_4 0x04 +#define TPS6131X_REG_4_PG BIT(7) +#define TPS6131X_REG_4_HOTDIE_HI BIT(6) +#define TPS6131X_REG_4_HOTDIE_LO BIT(5) +#define TPS6131X_REG_4_ILIM BIT(4) +#define TPS6131X_REG_4_INDC GENMASK(3, 0) +#define TPS6131X_REG_4_INDC_SHIFT 0 + +#define TPS6131X_REG_5 0x05 +#define TPS6131X_REG_5_SELFCAL BIT(7) +#define TPS6131X_REG_5_ENPSM BIT(6) +#define TPS6131X_REG_5_STSTRB1_DIR BIT(5) +#define TPS6131X_REG_5_GPIO BIT(4) +#define TPS6131X_REG_5_GPIOTYPE BIT(3) +#define TPS6131X_REG_5_ENLED3 BIT(2) +#define TPS6131X_REG_5_ENLED2 BIT(1) +#define TPS6131X_REG_5_ENLED1 BIT(0) + +#define TPS6131X_REG_6 0x06 +#define TPS6131X_REG_6_ENTS BIT(7) +#define TPS6131X_REG_6_LEDHOT BIT(6) +#define TPS6131X_REG_6_LEDWARN BIT(5) +#define TPS6131X_REG_6_LEDHDR BIT(4) +#define TPS6131X_REG_6_OV GENMASK(3, 0) +#define TPS6131X_REG_6_OV_SHIFT 0 + +#define TPS6131X_REG_7 0x07 +#define TPS6131X_REG_7_ENBATMON BIT(7) +#define TPS6131X_REG_7_BATDROOP GENMASK(6, 4) +#define TPS6131X_REG_7_BATDROOP_SHIFT 4 +#define TPS6131X_REG_7_REVID GENMASK(2, 0) +#define TPS6131X_REG_7_REVID_SHIFT 0 + +#define TPS6131X_MAX_CHANNELS 3 + +#define TPS6131X_FLASH_MAX_I_CHAN13_MA 400 +#define TPS6131X_FLASH_MAX_I_CHAN2_MA 800 +#define TPS6131X_FLASH_STEP_I_MA 25 + +#define TPS6131X_TORCH_MAX_I_CHAN13_MA 175 +#define TPS6131X_TORCH_MAX_I_CHAN2_MA 175 +#define TPS6131X_TORCH_STEP_I_MA 25 + +/* The torch watchdog timer must be refreshed within an interval of 13 seconds. */ +#define TPS6131X_TORCH_REFRESH_INTERVAL_JIFFIES msecs_to_jiffies(10000) + +#define UA_TO_MA(UA) ((UA) / 1000) + +enum tps6131x_mode { + TPS6131X_MODE_SHUTDOWN = 0x0, + TPS6131X_MODE_TORCH = 0x1, + TPS6131X_MODE_FLASH = 0x2, +}; + +struct tps6131x { + struct device *dev; + struct regmap *regmap; + struct gpio_desc *reset_gpio; + /* + * Registers 0, 1, 2, and 3 control parts of the controller that are not completely + * independent of each other. Since some operations require the registers to be written in + * a specific order to avoid unwanted side effects, they are synchronized with a lock. + */ + struct mutex lock; /* Hardware access lock for register 0, 1, 2 and 3 */ + struct delayed_work torch_refresh_work; + bool valley_current_limit; + bool chan1_en; + bool chan2_en; + bool chan3_en; + struct fwnode_handle *led_node; + u32 max_flash_current_ma; + u32 step_flash_current_ma; + u32 max_torch_current_ma; + u32 step_torch_current_ma; + u32 max_timeout_us; + struct led_classdev_flash fled_cdev; + struct v4l2_flash *v4l2_flash; +}; + +static struct tps6131x *fled_cdev_to_tps6131x(struct led_classdev_flash *fled_cdev) +{ + return container_of(fled_cdev, struct tps6131x, fled_cdev); +} + +/* + * Register contents after a power on/reset. These values cannot be changed. + */ + +#define TPS6131X_DCLC2_50MA 2 +#define TPS6131X_DCLC13_25MA 1 +#define TPS6131X_FC2_400MA 16 +#define TPS6131X_FC13_200MA 8 +#define TPS6131X_STIM_0_579MS_1_37MS 6 +#define TPS6131X_SELSTIM_RANGE0 0 +#define TPS6131X_INDC_OFF 0 +#define TPS6131X_OV_4950MV 9 +#define TPS6131X_BATDROOP_150MV 4 + +static const struct reg_default tps6131x_regmap_defaults[] = { + { TPS6131X_REG_0, (TPS6131X_DCLC13_25MA << TPS6131X_REG_0_DCLC13_SHIFT) | + (TPS6131X_DCLC2_50MA << TPS6131X_REG_0_DCLC2_SHIFT) }, + { TPS6131X_REG_1, (TPS6131X_MODE_SHUTDOWN << TPS6131X_REG_1_MODE_SHIFT) | + (TPS6131X_FC2_400MA << TPS6131X_REG_1_FC2_SHIFT) }, + { TPS6131X_REG_2, (TPS6131X_MODE_SHUTDOWN << TPS6131X_REG_2_MODE_SHIFT) | + (TPS6131X_FC13_200MA << TPS6131X_REG_2_FC13_SHIFT) }, + { TPS6131X_REG_3, (TPS6131X_STIM_0_579MS_1_37MS << TPS6131X_REG_3_STIM_SHIFT) | + (TPS6131X_SELSTIM_RANGE0 << TPS6131X_REG_3_SELSTIM_TO) | + TPS6131X_REG_3_TXMASK }, + { TPS6131X_REG_4, (TPS6131X_INDC_OFF << TPS6131X_REG_4_INDC_SHIFT) }, + { TPS6131X_REG_5, TPS6131X_REG_5_ENPSM | TPS6131X_REG_5_STSTRB1_DIR | + TPS6131X_REG_5_GPIOTYPE | TPS6131X_REG_5_ENLED2 }, + { TPS6131X_REG_6, (TPS6131X_OV_4950MV << TPS6131X_REG_6_OV_SHIFT) }, + { TPS6131X_REG_7, (TPS6131X_BATDROOP_150MV << TPS6131X_REG_7_BATDROOP_SHIFT) }, +}; + +/* + * These registers contain flags that are reset when read. + */ +static bool tps6131x_regmap_precious(struct device *dev, unsigned int reg) +{ + switch (reg) { + case TPS6131X_REG_3: + case TPS6131X_REG_4: + case TPS6131X_REG_6: + return true; + default: + return false; + } +} + +static const struct regmap_config tps6131x_regmap = { + .reg_bits = 8, + .val_bits = 8, + .max_register = TPS6131X_REG_7, + .reg_defaults = tps6131x_regmap_defaults, + .num_reg_defaults = ARRAY_SIZE(tps6131x_regmap_defaults), + .cache_type = REGCACHE_FLAT, + .precious_reg = &tps6131x_regmap_precious, +}; + +struct tps6131x_timer_config { + u8 val; + u8 range; + u32 time_us; +}; + +static const struct tps6131x_timer_config tps6131x_timer_configs[] = { + { .val = 0, .range = 1, .time_us = 5300 }, + { .val = 1, .range = 1, .time_us = 10700 }, + { .val = 2, .range = 1, .time_us = 16000 }, + { .val = 3, .range = 1, .time_us = 21300 }, + { .val = 4, .range = 1, .time_us = 26600 }, + { .val = 5, .range = 1, .time_us = 32000 }, + { .val = 6, .range = 1, .time_us = 37300 }, + { .val = 0, .range = 0, .time_us = 68200 }, + { .val = 7, .range = 1, .time_us = 71500 }, + { .val = 1, .range = 0, .time_us = 102200 }, + { .val = 2, .range = 0, .time_us = 136300 }, + { .val = 3, .range = 0, .time_us = 170400 }, + { .val = 4, .range = 0, .time_us = 204500 }, + { .val = 5, .range = 0, .time_us = 340800 }, + { .val = 6, .range = 0, .time_us = 579300 }, + { .val = 7, .range = 0, .time_us = 852000 }, +}; + +static const struct tps6131x_timer_config *tps6131x_find_closest_timer_config(u32 timeout_us) +{ + const struct tps6131x_timer_config *timer_config = &tps6131x_timer_configs[0]; + u32 diff, min_diff = U32_MAX; + int i; + + for (i = 0; i < ARRAY_SIZE(tps6131x_timer_configs); i++) { + diff = abs(tps6131x_timer_configs[i].time_us - timeout_us); + if (diff < min_diff) { + timer_config = &tps6131x_timer_configs[i]; + min_diff = diff; + if (!min_diff) + break; + } + } + + return timer_config; +} + +static int tps6131x_reset_chip(struct tps6131x *tps6131x) +{ + int ret; + + if (tps6131x->reset_gpio) { + gpiod_set_value_cansleep(tps6131x->reset_gpio, 1); + fsleep(10); + gpiod_set_value_cansleep(tps6131x->reset_gpio, 0); + fsleep(100); + } else { + ret = regmap_update_bits(tps6131x->regmap, TPS6131X_REG_0, TPS6131X_REG_0_RESET, + TPS6131X_REG_0_RESET); + if (ret) + return ret; + + fsleep(100); + + ret = regmap_update_bits(tps6131x->regmap, TPS6131X_REG_0, TPS6131X_REG_0_RESET, 0); + if (ret) + return ret; + } + + return 0; +} + +static int tps6131x_init_chip(struct tps6131x *tps6131x) +{ + u32 val; + int ret; + + val = tps6131x->valley_current_limit ? TPS6131X_REG_4_ILIM : 0; + + ret = regmap_write(tps6131x->regmap, TPS6131X_REG_4, val); + if (ret) + return ret; + + val = TPS6131X_REG_5_ENPSM | TPS6131X_REG_5_STSTRB1_DIR | TPS6131X_REG_5_GPIOTYPE; + + if (tps6131x->chan1_en) + val |= TPS6131X_REG_5_ENLED1; + + if (tps6131x->chan2_en) + val |= TPS6131X_REG_5_ENLED2; + + if (tps6131x->chan3_en) + val |= TPS6131X_REG_5_ENLED3; + + ret = regmap_write(tps6131x->regmap, TPS6131X_REG_5, val); + if (ret) + return ret; + + val = TPS6131X_REG_6_ENTS; + + ret = regmap_write(tps6131x->regmap, TPS6131X_REG_6, val); + if (ret) + return ret; + + return 0; +} + +static int tps6131x_set_mode(struct tps6131x *tps6131x, enum tps6131x_mode mode, bool force) +{ + u8 val = mode << TPS6131X_REG_1_MODE_SHIFT; + + return regmap_update_bits_base(tps6131x->regmap, TPS6131X_REG_1, TPS6131X_REG_1_MODE, val, + NULL, false, force); +} + +static void tps6131x_torch_refresh_handler(struct work_struct *work) +{ + struct tps6131x *tps6131x = container_of(work, struct tps6131x, torch_refresh_work.work); + int ret; + + guard(mutex)(&tps6131x->lock); + + ret = tps6131x_set_mode(tps6131x, TPS6131X_MODE_TORCH, true); + if (ret < 0) { + dev_err(tps6131x->dev, "Failed to refresh torch watchdog timer\n"); + return; + } + + schedule_delayed_work(&tps6131x->torch_refresh_work, + TPS6131X_TORCH_REFRESH_INTERVAL_JIFFIES); +} + +static int tps6131x_brightness_set(struct led_classdev *cdev, enum led_brightness brightness) +{ + struct led_classdev_flash *fled_cdev = lcdev_to_flcdev(cdev); + struct tps6131x *tps6131x = fled_cdev_to_tps6131x(fled_cdev); + u32 num_chans, steps_chan13, steps_chan2, steps_remaining; + u8 reg0; + int ret; + + cancel_delayed_work_sync(&tps6131x->torch_refresh_work); + + /* + * The brightness parameter uses the number of current steps as the unit (not the current + * value itself). Since the reported step size can vary depending on the configuration, + * this value must be converted into actual register steps. + */ + steps_remaining = (brightness * tps6131x->step_torch_current_ma) / TPS6131X_TORCH_STEP_I_MA; + + num_chans = tps6131x->chan1_en + tps6131x->chan2_en + tps6131x->chan3_en; + + /* + * The currents are distributed as evenly as possible across the activated channels. + * Since channels 1 and 3 share the same register setting, they always use the same current + * value. Channel 2 supports higher currents and thus takes over the remaining additional + * portion that cannot be covered by the other channels. + */ + steps_chan13 = min_t(u32, steps_remaining / num_chans, + TPS6131X_TORCH_MAX_I_CHAN13_MA / TPS6131X_TORCH_STEP_I_MA); + if (tps6131x->chan1_en) + steps_remaining -= steps_chan13; + if (tps6131x->chan3_en) + steps_remaining -= steps_chan13; + + steps_chan2 = min_t(u32, steps_remaining, + TPS6131X_TORCH_MAX_I_CHAN2_MA / TPS6131X_TORCH_STEP_I_MA); + + guard(mutex)(&tps6131x->lock); + + reg0 = (steps_chan13 << TPS6131X_REG_0_DCLC13_SHIFT) | + (steps_chan2 << TPS6131X_REG_0_DCLC2_SHIFT); + ret = regmap_update_bits(tps6131x->regmap, TPS6131X_REG_0, + TPS6131X_REG_0_DCLC13 | TPS6131X_REG_0_DCLC2, reg0); + if (ret < 0) + return ret; + + ret = tps6131x_set_mode(tps6131x, brightness ? TPS6131X_MODE_TORCH : TPS6131X_MODE_SHUTDOWN, + true); + if (ret < 0) + return ret; + + /* + * In order to use both the flash and the video light functions purely via the I2C + * interface, STRB1 must be low. If STRB1 is low, then the video light watchdog timer + * is also active, which puts the device into the shutdown state after around 13 seconds. + * To prevent this, the mode must be refreshed within the watchdog timeout. + */ + if (brightness) + schedule_delayed_work(&tps6131x->torch_refresh_work, + TPS6131X_TORCH_REFRESH_INTERVAL_JIFFIES); + + return 0; +} + +static int tps6131x_strobe_set(struct led_classdev_flash *fled_cdev, bool state) +{ + struct tps6131x *tps6131x = fled_cdev_to_tps6131x(fled_cdev); + int ret; + + guard(mutex)(&tps6131x->lock); + + ret = tps6131x_set_mode(tps6131x, state ? TPS6131X_MODE_FLASH : TPS6131X_MODE_SHUTDOWN, + true); + if (ret < 0) + return ret; + + if (state) { + ret = regmap_update_bits_base(tps6131x->regmap, TPS6131X_REG_3, TPS6131X_REG_3_SFT, + TPS6131X_REG_3_SFT, NULL, false, true); + if (ret) + return ret; + } + + ret = regmap_update_bits_base(tps6131x->regmap, TPS6131X_REG_3, TPS6131X_REG_3_SFT, 0, NULL, + false, true); + if (ret) + return ret; + + return 0; +} + +static int tps6131x_flash_brightness_set(struct led_classdev_flash *fled_cdev, u32 brightness) +{ + struct tps6131x *tps6131x = fled_cdev_to_tps6131x(fled_cdev); + u32 num_chans; + u32 steps_chan13, steps_chan2; + u32 steps_remaining; + int ret; + + steps_remaining = brightness / TPS6131X_FLASH_STEP_I_MA; + num_chans = tps6131x->chan1_en + tps6131x->chan2_en + tps6131x->chan3_en; + steps_chan13 = min_t(u32, steps_remaining / num_chans, + TPS6131X_FLASH_MAX_I_CHAN13_MA / TPS6131X_FLASH_STEP_I_MA); + if (tps6131x->chan1_en) + steps_remaining -= steps_chan13; + if (tps6131x->chan3_en) + steps_remaining -= steps_chan13; + steps_chan2 = min_t(u32, steps_remaining, + TPS6131X_FLASH_MAX_I_CHAN2_MA / TPS6131X_FLASH_STEP_I_MA); + + guard(mutex)(&tps6131x->lock); + + ret = regmap_update_bits(tps6131x->regmap, TPS6131X_REG_2, TPS6131X_REG_2_FC13, + steps_chan13 << TPS6131X_REG_2_FC13_SHIFT); + if (ret < 0) + return ret; + + ret = regmap_update_bits(tps6131x->regmap, TPS6131X_REG_1, TPS6131X_REG_1_FC2, + steps_chan2 << TPS6131X_REG_1_FC2_SHIFT); + if (ret < 0) + return ret; + + fled_cdev->brightness.val = brightness; + + return 0; +} + +static int tps6131x_flash_timeout_set(struct led_classdev_flash *fled_cdev, u32 timeout_us) +{ + const struct tps6131x_timer_config *timer_config; + struct tps6131x *tps6131x = fled_cdev_to_tps6131x(fled_cdev); + u8 reg3; + int ret; + + guard(mutex)(&tps6131x->lock); + + timer_config = tps6131x_find_closest_timer_config(timeout_us); + + reg3 = timer_config->val << TPS6131X_REG_3_STIM_SHIFT; + if (timer_config->range) + reg3 |= TPS6131X_REG_3_SELSTIM_TO; + + ret = regmap_update_bits(tps6131x->regmap, TPS6131X_REG_3, + TPS6131X_REG_3_STIM | TPS6131X_REG_3_SELSTIM_TO, reg3); + if (ret < 0) + return ret; + + fled_cdev->timeout.val = timer_config->time_us; + + return 0; +} + +static int tps6131x_strobe_get(struct led_classdev_flash *fled_cdev, bool *state) +{ + struct tps6131x *tps6131x = fled_cdev_to_tps6131x(fled_cdev); + unsigned int reg3; + int ret; + + ret = regmap_read_bypassed(tps6131x->regmap, TPS6131X_REG_3, ®3); + if (ret) + return ret; + + *state = !!(reg3 & TPS6131X_REG_3_SFT); + + return 0; +} + +static int tps6131x_flash_fault_get(struct led_classdev_flash *fled_cdev, u32 *fault) +{ + struct tps6131x *tps6131x = fled_cdev_to_tps6131x(fled_cdev); + unsigned int reg3, reg4, reg6; + int ret; + + *fault = 0; + + ret = regmap_read_bypassed(tps6131x->regmap, TPS6131X_REG_3, ®3); + if (ret < 0) + return ret; + + ret = regmap_read_bypassed(tps6131x->regmap, TPS6131X_REG_4, ®4); + if (ret < 0) + return ret; + + ret = regmap_read_bypassed(tps6131x->regmap, TPS6131X_REG_6, ®6); + if (ret < 0) + return ret; + + if (reg3 & TPS6131X_REG_3_HPFL) + *fault |= LED_FAULT_SHORT_CIRCUIT; + + if (reg3 & TPS6131X_REG_3_SELSTIM_TO) + *fault |= LED_FAULT_TIMEOUT; + + if (reg4 & TPS6131X_REG_4_HOTDIE_HI) + *fault |= LED_FAULT_OVER_TEMPERATURE; + + if (reg6 & (TPS6131X_REG_6_LEDHOT | TPS6131X_REG_6_LEDWARN)) + *fault |= LED_FAULT_LED_OVER_TEMPERATURE; + + if (!(reg6 & TPS6131X_REG_6_LEDHDR)) + *fault |= LED_FAULT_UNDER_VOLTAGE; + + if (reg6 & TPS6131X_REG_6_LEDHOT) { + ret = regmap_update_bits_base(tps6131x->regmap, TPS6131X_REG_6, + TPS6131X_REG_6_LEDHOT, 0, NULL, false, true); + if (ret < 0) + return ret; + } + + return 0; +} + +static const struct led_flash_ops flash_ops = { + .flash_brightness_set = tps6131x_flash_brightness_set, + .strobe_set = tps6131x_strobe_set, + .strobe_get = tps6131x_strobe_get, + .timeout_set = tps6131x_flash_timeout_set, + .fault_get = tps6131x_flash_fault_get, +}; + +static int tps6131x_parse_node(struct tps6131x *tps6131x) +{ + const struct tps6131x_timer_config *timer_config; + struct device *dev = tps6131x->dev; + u32 channels[TPS6131X_MAX_CHANNELS]; + u32 current_step_multiplier; + u32 current_ua; + u32 max_current_flash_ma, max_current_torch_ma; + u32 timeout_us; + int num_channels; + int i; + int ret; + + tps6131x->valley_current_limit = device_property_read_bool(dev, "ti,valley-current-limit"); + + tps6131x->led_node = fwnode_get_next_available_child_node(dev->fwnode, NULL); + if (!tps6131x->led_node) { + dev_err(dev, "Missing LED node\n"); + return -EINVAL; + } + + num_channels = fwnode_property_count_u32(tps6131x->led_node, "led-sources"); + if (num_channels <= 0) { + dev_err(dev, "Failed to read led-sources property\n"); + return -EINVAL; + } + + if (num_channels > TPS6131X_MAX_CHANNELS) { + dev_err(dev, "led-sources count %u exceeds maximum channel count %u\n", + num_channels, TPS6131X_MAX_CHANNELS); + return -EINVAL; + } + + ret = fwnode_property_read_u32_array(tps6131x->led_node, "led-sources", channels, + num_channels); + if (ret < 0) { + dev_err(dev, "Failed to read led-sources property\n"); + return ret; + } + + max_current_flash_ma = 0; + max_current_torch_ma = 0; + for (i = 0; i < num_channels; i++) { + switch (channels[i]) { + case 1: + tps6131x->chan1_en = true; + max_current_flash_ma += TPS6131X_FLASH_MAX_I_CHAN13_MA; + max_current_torch_ma += TPS6131X_TORCH_MAX_I_CHAN13_MA; + break; + case 2: + tps6131x->chan2_en = true; + max_current_flash_ma += TPS6131X_FLASH_MAX_I_CHAN2_MA; + max_current_torch_ma += TPS6131X_TORCH_MAX_I_CHAN2_MA; + break; + case 3: + tps6131x->chan3_en = true; + max_current_flash_ma += TPS6131X_FLASH_MAX_I_CHAN13_MA; + max_current_torch_ma += TPS6131X_TORCH_MAX_I_CHAN13_MA; + break; + default: + dev_err(dev, "led-source out of range [1-3]\n"); + return -EINVAL; + } + } + + /* + * If only channels 1 and 3 are used, the step size is doubled because the two channels + * share the same current control register. + */ + current_step_multiplier = + (tps6131x->chan1_en && tps6131x->chan3_en && !tps6131x->chan2_en) ? 2 : 1; + tps6131x->step_flash_current_ma = current_step_multiplier * TPS6131X_FLASH_STEP_I_MA; + tps6131x->step_torch_current_ma = current_step_multiplier * TPS6131X_TORCH_STEP_I_MA; + + ret = fwnode_property_read_u32(tps6131x->led_node, "led-max-microamp", ¤t_ua); + if (ret < 0) { + dev_err(dev, "Failed to read led-max-microamp property\n"); + return ret; + } + + tps6131x->max_torch_current_ma = UA_TO_MA(current_ua); + + if (!tps6131x->max_torch_current_ma || + tps6131x->max_torch_current_ma > max_current_torch_ma || + (tps6131x->max_torch_current_ma % tps6131x->step_torch_current_ma)) { + dev_err(dev, "led-max-microamp out of range or not a multiple of %u\n", + tps6131x->step_torch_current_ma); + return -EINVAL; + } + + ret = fwnode_property_read_u32(tps6131x->led_node, "flash-max-microamp", ¤t_ua); + if (ret < 0) { + dev_err(dev, "Failed to read flash-max-microamp property\n"); + return ret; + } + + tps6131x->max_flash_current_ma = UA_TO_MA(current_ua); + + if (!tps6131x->max_flash_current_ma || + tps6131x->max_flash_current_ma > max_current_flash_ma || + (tps6131x->max_flash_current_ma % tps6131x->step_flash_current_ma)) { + dev_err(dev, "flash-max-microamp out of range or not a multiple of %u\n", + tps6131x->step_flash_current_ma); + return -EINVAL; + } + + ret = fwnode_property_read_u32(tps6131x->led_node, "flash-max-timeout-us", &timeout_us); + if (ret < 0) { + dev_err(dev, "Failed to read flash-max-timeout-us property\n"); + return ret; + } + + timer_config = tps6131x_find_closest_timer_config(timeout_us); + tps6131x->max_timeout_us = timer_config->time_us; + + if (tps6131x->max_timeout_us != timeout_us) + dev_warn(dev, "flash-max-timeout-us %u not supported (using %u)\n", timeout_us, + tps6131x->max_timeout_us); + + return 0; +} + +static int tps6131x_led_class_setup(struct tps6131x *tps6131x) +{ + const struct tps6131x_timer_config *timer_config; + struct led_classdev *led_cdev; + struct led_flash_setting *setting; + struct led_init_data init_data = {}; + int ret; + + tps6131x->fled_cdev.ops = &flash_ops; + + setting = &tps6131x->fled_cdev.timeout; + timer_config = tps6131x_find_closest_timer_config(0); + setting->min = timer_config->time_us; + setting->max = tps6131x->max_timeout_us; + setting->step = 1; /* Only some specific time periods are supported. No fixed step size. */ + setting->val = setting->min; + + setting = &tps6131x->fled_cdev.brightness; + setting->min = tps6131x->step_flash_current_ma; + setting->max = tps6131x->max_flash_current_ma; + setting->step = tps6131x->step_flash_current_ma; + setting->val = setting->min; + + led_cdev = &tps6131x->fled_cdev.led_cdev; + led_cdev->brightness_set_blocking = tps6131x_brightness_set; + led_cdev->max_brightness = tps6131x->max_torch_current_ma; + led_cdev->flags |= LED_DEV_CAP_FLASH; + + init_data.fwnode = tps6131x->led_node; + init_data.devicename = NULL; + init_data.default_label = NULL; + init_data.devname_mandatory = false; + + ret = devm_led_classdev_flash_register_ext(tps6131x->dev, &tps6131x->fled_cdev, + &init_data); + if (ret) + return ret; + + return 0; +} + +static int tps6131x_flash_external_strobe_set(struct v4l2_flash *v4l2_flash, bool enable) +{ + struct led_classdev_flash *fled_cdev = v4l2_flash->fled_cdev; + struct tps6131x *tps6131x = fled_cdev_to_tps6131x(fled_cdev); + + guard(mutex)(&tps6131x->lock); + + return tps6131x_set_mode(tps6131x, enable ? TPS6131X_MODE_FLASH : TPS6131X_MODE_SHUTDOWN, + false); +} + +static const struct v4l2_flash_ops tps6131x_v4l2_flash_ops = { + .external_strobe_set = tps6131x_flash_external_strobe_set, +}; + +static int tps6131x_v4l2_setup(struct tps6131x *tps6131x) +{ + struct v4l2_flash_config v4l2_cfg = { 0 }; + struct led_flash_setting *intensity = &v4l2_cfg.intensity; + + intensity->min = tps6131x->step_torch_current_ma; + intensity->max = tps6131x->max_torch_current_ma; + intensity->step = tps6131x->step_torch_current_ma; + intensity->val = intensity->min; + + strscpy(v4l2_cfg.dev_name, tps6131x->fled_cdev.led_cdev.dev->kobj.name, + sizeof(v4l2_cfg.dev_name)); + + v4l2_cfg.has_external_strobe = true; + v4l2_cfg.flash_faults = LED_FAULT_TIMEOUT | LED_FAULT_OVER_TEMPERATURE | + LED_FAULT_SHORT_CIRCUIT | LED_FAULT_UNDER_VOLTAGE | + LED_FAULT_LED_OVER_TEMPERATURE; + + tps6131x->v4l2_flash = v4l2_flash_init(tps6131x->dev, tps6131x->led_node, + &tps6131x->fled_cdev, &tps6131x_v4l2_flash_ops, + &v4l2_cfg); + if (IS_ERR(tps6131x->v4l2_flash)) { + dev_err(tps6131x->dev, "Failed to initialize v4l2 flash LED\n"); + return PTR_ERR(tps6131x->v4l2_flash); + } + + return 0; +} + +static int tps6131x_probe(struct i2c_client *client) +{ + struct tps6131x *tps6131x; + int ret; + + tps6131x = devm_kzalloc(&client->dev, sizeof(*tps6131x), GFP_KERNEL); + if (!tps6131x) + return -ENOMEM; + + tps6131x->dev = &client->dev; + i2c_set_clientdata(client, tps6131x); + mutex_init(&tps6131x->lock); + INIT_DELAYED_WORK(&tps6131x->torch_refresh_work, tps6131x_torch_refresh_handler); + + ret = tps6131x_parse_node(tps6131x); + if (ret) + return ret; + + tps6131x->regmap = devm_regmap_init_i2c(client, &tps6131x_regmap); + if (IS_ERR(tps6131x->regmap)) { + ret = PTR_ERR(tps6131x->regmap); + return dev_err_probe(&client->dev, ret, "Failed to allocate register map\n"); + } + + tps6131x->reset_gpio = devm_gpiod_get_optional(&client->dev, "reset", GPIOD_OUT_HIGH); + if (IS_ERR(tps6131x->reset_gpio)) { + ret = PTR_ERR(tps6131x->reset_gpio); + return dev_err_probe(&client->dev, ret, "Failed to get reset GPIO\n"); + } + + ret = tps6131x_reset_chip(tps6131x); + if (ret) + return dev_err_probe(&client->dev, ret, "Failed to reset LED controller\n"); + + ret = tps6131x_init_chip(tps6131x); + if (ret) + return dev_err_probe(&client->dev, ret, "Failed to initialize LED controller\n"); + + ret = tps6131x_led_class_setup(tps6131x); + if (ret) + return dev_err_probe(&client->dev, ret, "Failed to setup LED class\n"); + + ret = tps6131x_v4l2_setup(tps6131x); + if (ret) + return dev_err_probe(&client->dev, ret, "Failed to setup v4l2 flash\n"); + + return 0; +} + +static void tps6131x_remove(struct i2c_client *client) +{ + struct tps6131x *tps6131x = i2c_get_clientdata(client); + + v4l2_flash_release(tps6131x->v4l2_flash); + + cancel_delayed_work_sync(&tps6131x->torch_refresh_work); +} + +static const struct of_device_id of_tps6131x_leds_match[] = { + { .compatible = "ti,tps61310" }, + {} +}; +MODULE_DEVICE_TABLE(of, of_tps6131x_leds_match); + +static struct i2c_driver tps6131x_i2c_driver = { + .driver = { + .name = "tps6131x", + .of_match_table = of_tps6131x_leds_match, + }, + .probe = tps6131x_probe, + .remove = tps6131x_remove, +}; +module_i2c_driver(tps6131x_i2c_driver); + +MODULE_DESCRIPTION("Texas Instruments TPS6131X flash LED driver"); +MODULE_AUTHOR("Matthias Fend "); +MODULE_LICENSE("GPL");