Re: [PATCH v3 3/4] leds: flash: lm3643: Describe the LED of ACPI boards without LED nodes

[Lee Jones <lee@kernel.org>]

Andy, et al., Please see below.

On Sun, 11 Oct 2026, Rillian Grant wrote:

> From: Ruslan Koreev <koreev.r@gmail.com>
>
> Lenovo ThinkPads enumerate the LM3643 through ACPI (HID TXNW3643)
> without a _DSD, so the device has no LED child nodes and the driver
> refuses to probe. On these laptops the chip drives the Windows Hello IR
> flood illuminator, with both current sources feeding a single LED, and
> Lenovo's Windows driver applies a fixed configuration to any LM3643 it
> finds.
>
> Add a DMI table that provides that description as a software node when
> the firmware offers none: one IR flash LED on both outputs, with torch
> and flash limits well below the chip's maximums. Start with the ThinkPad
> X1 Carbon Gen 14.
>
> The description is keyed by DMI instead of being applied to every
> TXNW3643 because the wiring is a board property: which output feeds
> which LED and how much current the LED tolerates cannot be read from the
> chip, and a wrong guess overdrives an LED. The firmware cannot be fixed
> from the driver side, and a per-board table is what x86-android-tablets
> does for devices whose ACPI tables describe nothing either. Boards with
> a _DSD are not affected, the fallback only runs when the device has no
> child nodes. The limits are conservative because the LED part is not
> documented; the illuminator is bright enough for face authentication at
> a fraction of them.
>
> Tested on that laptop: torch through /sys/class/leds/ir:flash lights the
> ST VD55G1 IR camera's field of view, and the resulting frames are good
> enough for face authentication.
>
> Signed-off-by: Ruslan Koreev <koreev.r@gmail.com>
> [rillian: add reg property to match the DT binding]
> [rillian: change DMI activation log to dev_dbg()]
> [rillian: use DMI_EXACT_MATCH]
> Signed-off-by: Rillian Grant <rillian.grant@gmail.com>
> ---
>  drivers/leds/flash/leds-lm3643.c | 93 ++++++++++++++++++++++++++++++++
>  1 file changed, 93 insertions(+)
>
> diff --git a/drivers/leds/flash/leds-lm3643.c b/drivers/leds/flash/leds-lm3643.c
> index cbe66e90d8e1..18ac75df72e2 100644
> --- a/drivers/leds/flash/leds-lm3643.c
> +++ b/drivers/leds/flash/leds-lm3643.c
> @@ -9,6 +9,7 @@
>  #include <linux/bitfield.h>
>  #include <linux/bits.h>
>  #include <linux/cleanup.h>
> +#include <linux/dmi.h>
>  #include <linux/gpio/consumer.h>
>  #include <linux/i2c.h>
>  #include <linux/led-class-flash.h>
> @@ -691,6 +692,92 @@ static int lm3643_register_led(struct device *dev, struct lm3643 *chip,
>  	return lm3643_register_v4l2(dev, led, fwnode);
>  }
>
> +/*
> + * Lenovo ThinkPads enumerate the chip through ACPI (HID TXNW3643) without a
> + * _DSD, so the LED is not described by firmware. On the boards below it drives
> + * the Windows Hello IR flood illuminator, with both current sources feeding a
> + * single LED. Describe that with a software node, with torch and flash limits
> + * (100 mA and 300 mA per output) well below the chip's maximums.
> + */
> +static const u32 lm3643_ir_flood_sources[] = { 0, 1 };
> +
> +static const struct property_entry lm3643_ir_flood_props[] = {
> +	PROPERTY_ENTRY_U32("reg", 0),
> +	PROPERTY_ENTRY_U32_ARRAY("led-sources", lm3643_ir_flood_sources),
> +	PROPERTY_ENTRY_STRING("function", LED_FUNCTION_FLASH),
> +	PROPERTY_ENTRY_U32("color", LED_COLOR_ID_IR),
> +	PROPERTY_ENTRY_U32("led-max-microamp", 200000),
> +	PROPERTY_ENTRY_U32("flash-max-microamp", 600000),
> +	PROPERTY_ENTRY_U32("flash-max-timeout-us", 100000),
> +	{ }
> +};
> +
> +static const struct software_node lm3643_ir_flood_root = {
> +	.name = "lm3643",
> +};
> +
> +static const struct software_node lm3643_ir_flood_led = {
> +	.name = "led-0",
> +	.parent = &lm3643_ir_flood_root,
> +	.properties = lm3643_ir_flood_props,
> +};
> +
> +static const struct software_node *lm3643_ir_flood_nodes[] = {
> +	&lm3643_ir_flood_root,
> +	&lm3643_ir_flood_led,
> +	NULL
> +};
> +
> +static const struct dmi_system_id lm3643_dmi_leds[] = {
> +	{
> +		/* Lenovo ThinkPad X1 Carbon Gen 14 */
> +		.matches = {
> +			DMI_EXACT_MATCH(DMI_SYS_VENDOR, "LENOVO"),
> +			DMI_EXACT_MATCH(DMI_PRODUCT_VERSION, "ThinkPad X1 Carbon Gen 14"),
> +		},
> +		.driver_data = (void *)lm3643_ir_flood_nodes,

Andy, et al.,

I don't usually allow data from other registration mechanisms to be
passed through another (MFD data via OF for instance).

Is this okay?

> +	},
> +	{ }
> +};
> +
> +static void lm3643_remove_dmi_leds(void *data)
> +{
> +	struct device *dev = data;
> +	const struct dmi_system_id *id = dmi_first_match(lm3643_dmi_leds);
> +
> +	set_secondary_fwnode(dev, NULL);
> +	if (id)
> +		software_node_unregister_node_group(id->driver_data);
> +}
> +
> +static int lm3643_add_dmi_leds(struct device *dev)
> +{
> +	const struct dmi_system_id *id;
> +	const struct software_node **nodes;
> +	int ret;
> +
> +	if (!has_acpi_companion(dev))
> +		return 0;
> +
> +	id = dmi_first_match(lm3643_dmi_leds);
> +	if (!id)
> +		return 0;
> +
> +	nodes = id->driver_data;
> +	/* The nodes are static: a second chip on the same board cannot reuse them */
> +	if (software_node_fwnode(nodes[0]))
> +		return dev_err_probe(dev, -EBUSY, "LED description already in use\n");
> +
> +	ret = software_node_register_node_group(nodes);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "failed to register the LED description\n");
> +
> +	set_secondary_fwnode(dev, software_node_fwnode(nodes[0]));
> +	dev_dbg(dev, "no LED nodes in firmware, using the DMI description\n");

Please remove temporary debug prints before submitting.

> +
> +	return devm_add_action_or_reset(dev, lm3643_remove_dmi_leds, dev);
> +}
> +
>  static int lm3643_probe(struct i2c_client *client)
>  {
>  	struct device *dev = &client->dev;
> @@ -701,6 +788,12 @@ static int lm3643_probe(struct i2c_client *client)
>  	int ret;
>
>  	count = device_get_child_node_count(dev);
> +	if (!count) {
> +		ret = lm3643_add_dmi_leds(dev);
> +		if (ret)
> +			return ret;
> +		count = device_get_child_node_count(dev);
> +	}
>  	if (!count || count > LM3643_NUM_CHANNELS)
>  		return dev_err_probe(dev, -EINVAL, "%u LED nodes found, expected 1 to %d\n",
>  					 count, LM3643_NUM_CHANNELS);
> -- 
> 2.56.0
>
>

-- 
Lee Jones