[PATCH v6 0/5] KTD2026 indicator LED for X86 Xiaomi Pad2

[PATCH v6 0/5] KTD2026 indicator LED for X86 Xiaomi Pad2

[Kate Hsuan]

This patch added the support for Xiaomi Pad2 indicator LED. This work
included:
1. Added the KTD2026 swnode description to describe the LED controller.
2. Migrated the original driver to fwnode to support x86 platform.
3. Support for multi-color LED trigger event.
4. The LED shows orange  when charging and the LED shows green when the
   battery is full.

Moreover, the LED trigger is set to the new trigger, called
"bq27520-0-charging-orange-full-green" for Xiaomi Pad2 so the LED shows
orange when charging and the LED shows green when the battery is full.

The new LED API led_mc_trigger_event() can be found in the following
URL.
https://lore.kernel.org/linux-leds/f40a0b1a-ceac-e269-c2dd-0158c5b4a1ad@gmail.com/

--
Changes in v6:
1. The I2C ID table was moved to a separate patch.
2. The LED shows orange when charging.
3. The trigger name was renamed to charging-orange-full-green.
4. The default trigger of Xiaomi Pad2 is
   "bq27520-0-charging-orange-full-green".

Changes in v5:
1. Fix swnode LED color settings.
2. Improve the driver based on the comments.
3. Introduce a LED new API- led_mc_trigger_event() to make the LED
   color can be changed according to the trigger.
4. Introduced a new trigger "charging-red-full-green". The LED will be
   red when charging and the the LED will be green when the battery is
   full.
5. Set the default trigger to "bq27520-0-charging-red-full-green" for
   Xiaomi Pad2.

Changes in v4:
1. Fix double casting.
2. Since force casting a pointer value to int will trigger a compiler
   warning, the type of num_leds was changed to unsigned long.

Changes in v3:
1. Drop the patch "leds-ktd202x: Skip regulator settings for Xiaomi
   pad2"

Changes in v2:
1. Typo and style fixes.
2. The patch 0003 skips all the regulator setup for Xiaomi pad2 since
   KTD2026 on Xiaomi pad2 is already powered by BP25890RTWR. So, the
   sleep can be removed when removing the module.


Kate Hsuan (5):
  platform: x86-android-tablets: other: Add swnode for Xiaomi pad2
    indicator LED
  leds: rgb: leds-ktd202x: Get device properties through fwnode to
    support ACPI
  leds: rgb: leds-ktd202x: I2C ID tables for KTD2026 and 2027
  power: supply: power-supply-leds: Add charging_orange_full_green
    trigger for RGB LED
  platform: x86-android-tablets: others: Set the LED trigger to
    charging_orange_full_green for Xiaomi pad2

 drivers/leds/rgb/Kconfig                      |  1 -
 drivers/leds/rgb/leds-ktd202x.c               | 73 ++++++++++-------
 .../platform/x86/x86-android-tablets/other.c  | 82 +++++++++++++++++++
 .../x86/x86-android-tablets/shared-psy-info.h |  2 +
 drivers/power/supply/power_supply_leds.c      | 26 ++++++
 include/linux/power_supply.h                  |  2 +
 6 files changed, 156 insertions(+), 30 deletions(-)

-- 
2.44.0

[PATCH v6 1/5] platform: x86-android-tablets: other: Add swnode for Xiaomi pad2 indicator LED

[Kate Hsuan]

KTD2026 LED controller manages the indicator LED for Xiaomi pad2. The ACPI
for it is not properly made so the kernel can't get a correct description.

This work adds a description for this RGB LED controller and also sets a
trigger to indicate the changing event (bq27520-0-charging). When it is
charging, the indicator LED will be turned on.

Signed-off-by: Kate Hsuan <hpa@redhat.com>
---
 .../platform/x86/x86-android-tablets/other.c  | 82 +++++++++++++++++++
 .../x86/x86-android-tablets/shared-psy-info.h |  2 +
 2 files changed, 84 insertions(+)

diff --git a/drivers/platform/x86/x86-android-tablets/other.c b/drivers/platform/x86/x86-android-tablets/other.c
index bc6bbf7ec6ea..c77d56454f2d 100644
--- a/drivers/platform/x86/x86-android-tablets/other.c
+++ b/drivers/platform/x86/x86-android-tablets/other.c
@@ -13,6 +13,8 @@
 #include <linux/input.h>
 #include <linux/platform_device.h>
 
+#include <dt-bindings/leds/common.h>
+
 #include "shared-psy-info.h"
 #include "x86-android-tablets.h"
 
@@ -593,6 +595,83 @@ const struct x86_dev_info whitelabel_tm800a550l_info __initconst = {
 	.gpiod_lookup_tables = whitelabel_tm800a550l_gpios,
 };
 
+/*
+ * The fwnode for ktd2026 on Xaomi pad2. It composed of a RGB LED node
+ * with three subnodes for each color. The RGB LED node is named
+ * "multi-led" to align with the name in the device tree.
+ */
+
+/* main fwnode for ktd2026 */
+static const struct software_node ktd2026_node = {
+	.name = "ktd2026"
+};
+
+static const struct property_entry ktd2026_rgb_led_props[] = {
+	PROPERTY_ENTRY_U32("reg", 0),
+	PROPERTY_ENTRY_U32("color", LED_COLOR_ID_RGB),
+	PROPERTY_ENTRY_STRING("function", "indicator"),
+	PROPERTY_ENTRY_STRING("linux,default-trigger", "bq27520-0-charging"),
+	{ }
+};
+
+static const struct software_node ktd2026_rgb_led_node = {
+	.name = "multi-led",
+	.properties = ktd2026_rgb_led_props,
+	.parent = &ktd2026_node,
+};
+
+static const struct property_entry ktd2026_blue_led_props[] = {
+	PROPERTY_ENTRY_U32("reg", 0),
+	PROPERTY_ENTRY_U32("color", LED_COLOR_ID_BLUE),
+	{ }
+};
+
+static const struct software_node ktd2026_blue_led_node = {
+	.properties = ktd2026_blue_led_props,
+	.parent = &ktd2026_rgb_led_node,
+};
+
+static const struct property_entry ktd2026_green_led_props[] = {
+	PROPERTY_ENTRY_U32("reg", 1),
+	PROPERTY_ENTRY_U32("color", LED_COLOR_ID_GREEN),
+	{ }
+};
+
+static const struct software_node ktd2026_green_led_node = {
+	.properties = ktd2026_green_led_props,
+	.parent = &ktd2026_rgb_led_node,
+};
+
+static const struct property_entry ktd2026_red_led_props[] = {
+	PROPERTY_ENTRY_U32("reg", 2),
+	PROPERTY_ENTRY_U32("color", LED_COLOR_ID_RED),
+	{ }
+};
+
+static const struct software_node ktd2026_red_led_node = {
+	.properties = ktd2026_red_led_props,
+	.parent = &ktd2026_rgb_led_node,
+};
+
+static const struct software_node *ktd2026_node_group[] = {
+	&ktd2026_node,
+	&ktd2026_rgb_led_node,
+	&ktd2026_green_led_node,
+	&ktd2026_blue_led_node,
+	&ktd2026_red_led_node,
+	NULL
+};
+
+static int __init xiaomi_mipad2_init(void)
+{
+	return software_node_register_node_group(ktd2026_node_group);
+}
+
+static void xiaomi_mipad2_exit(void)
+{
+	software_node_unregister_node_group(ktd2026_node_group);
+}
+
 /*
  * If the EFI bootloader is not Xiaomi's own signed Android loader, then the
  * Xiaomi Mi Pad 2 X86 tablet sets OSID in the DSDT to 1 (Windows), causing
@@ -616,6 +695,7 @@ static const struct x86_i2c_client_info xiaomi_mipad2_i2c_clients[] __initconst
 			.type = "ktd2026",
 			.addr = 0x30,
 			.dev_name = "ktd2026",
+			.swnode = &ktd2026_node,
 		},
 		.adapter_path = "\\_SB_.PCI0.I2C3",
 	},
@@ -624,4 +704,6 @@ static const struct x86_i2c_client_info xiaomi_mipad2_i2c_clients[] __initconst
 const struct x86_dev_info xiaomi_mipad2_info __initconst = {
 	.i2c_client_info = xiaomi_mipad2_i2c_clients,
 	.i2c_client_count = ARRAY_SIZE(xiaomi_mipad2_i2c_clients),
+	.init = xiaomi_mipad2_init,
+	.exit = xiaomi_mipad2_exit,
 };
diff --git a/drivers/platform/x86/x86-android-tablets/shared-psy-info.h b/drivers/platform/x86/x86-android-tablets/shared-psy-info.h
index c2d2968cddc2..8c33ec47ee12 100644
--- a/drivers/platform/x86/x86-android-tablets/shared-psy-info.h
+++ b/drivers/platform/x86/x86-android-tablets/shared-psy-info.h
@@ -29,4 +29,6 @@ extern const char * const bq24190_modules[];
 extern const struct platform_device_info int3496_pdevs[];
 extern struct gpiod_lookup_table int3496_reference_gpios;
 
+extern const struct software_node ktd2026_leds_node;
+
 #endif
-- 
2.44.0

[PATCH v6 2/5] leds: rgb: leds-ktd202x: Get device properties through fwnode to support ACPI

[Kate Hsuan]

This LED controller is installed on a Xiaomi pad2 and it is an x86
platform. The original driver is based on the device tree and can't be
used for this ACPI based system. This patch migrated the driver to use
fwnode to access the properties. Moreover, the fwnode API supports the
device tree so this work won't affect the original implementations.

Signed-off-by: Kate Hsuan <hpa@redhat.com>
---
 drivers/leds/rgb/Kconfig        |  1 -
 drivers/leds/rgb/leds-ktd202x.c | 67 ++++++++++++++++++---------------
 2 files changed, 37 insertions(+), 31 deletions(-)

diff --git a/drivers/leds/rgb/Kconfig b/drivers/leds/rgb/Kconfig
index e66bd21b9852..14d6b294a786 100644
--- a/drivers/leds/rgb/Kconfig
+++ b/drivers/leds/rgb/Kconfig
@@ -17,7 +17,6 @@ config LEDS_GROUP_MULTICOLOR
 config LEDS_KTD202X
 	tristate "LED support for KTD202x Chips"
 	depends on I2C
-	depends on OF
 	select REGMAP_I2C
 	help
 	  This option enables support for the Kinetic KTD2026/KTD2027
diff --git a/drivers/leds/rgb/leds-ktd202x.c b/drivers/leds/rgb/leds-ktd202x.c
index 514965795a10..8d0ed1a95a9f 100644
--- a/drivers/leds/rgb/leds-ktd202x.c
+++ b/drivers/leds/rgb/leds-ktd202x.c
@@ -99,7 +99,7 @@ struct ktd202x {
 	struct device *dev;
 	struct regmap *regmap;
 	bool enabled;
-	int num_leds;
+	unsigned long num_leds;
 	struct ktd202x_led leds[] __counted_by(num_leds);
 };
 
@@ -381,16 +381,18 @@ static int ktd202x_blink_mc_set(struct led_classdev *cdev,
 				 mc->num_colors);
 }
 
-static int ktd202x_setup_led_rgb(struct ktd202x *chip, struct device_node *np,
+static int ktd202x_setup_led_rgb(struct ktd202x *chip, struct fwnode_handle *fwnode,
 				 struct ktd202x_led *led, struct led_init_data *init_data)
 {
+	struct fwnode_handle *child;
 	struct led_classdev *cdev;
-	struct device_node *child;
 	struct mc_subled *info;
-	int num_channels;
+	int num_channels = 0;
 	int i = 0;
 
-	num_channels = of_get_available_child_count(np);
+	fwnode_for_each_available_child_node(fwnode, child)
+		num_channels++;
+
 	if (!num_channels || num_channels > chip->num_leds)
 		return -EINVAL;
 
@@ -398,22 +400,22 @@ static int ktd202x_setup_led_rgb(struct ktd202x *chip, struct device_node *np,
 	if (!info)
 		return -ENOMEM;
 
-	for_each_available_child_of_node(np, child) {
+	fwnode_for_each_available_child_node(fwnode, child) {
 		u32 mono_color;
 		u32 reg;
 		int ret;
 
-		ret = of_property_read_u32(child, "reg", &reg);
+		ret = fwnode_property_read_u32(child, "reg", &reg);
 		if (ret != 0 || reg >= chip->num_leds) {
-			dev_err(chip->dev, "invalid 'reg' of %pOFn\n", child);
-			of_node_put(child);
-			return -EINVAL;
+			dev_err(chip->dev, "invalid 'reg' of %pfw\n", child);
+			fwnode_handle_put(child);
+			return ret;
 		}
 
-		ret = of_property_read_u32(child, "color", &mono_color);
+		ret = fwnode_property_read_u32(child, "color", &mono_color);
 		if (ret < 0 && ret != -EINVAL) {
-			dev_err(chip->dev, "failed to parse 'color' of %pOF\n", child);
-			of_node_put(child);
+			dev_err(chip->dev, "failed to parse 'color' of %pfw\n", child);
+			fwnode_handle_put(child);
 			return ret;
 		}
 
@@ -433,16 +435,16 @@ static int ktd202x_setup_led_rgb(struct ktd202x *chip, struct device_node *np,
 	return devm_led_classdev_multicolor_register_ext(chip->dev, &led->mcdev, init_data);
 }
 
-static int ktd202x_setup_led_single(struct ktd202x *chip, struct device_node *np,
+static int ktd202x_setup_led_single(struct ktd202x *chip, struct fwnode_handle *fwnode,
 				    struct ktd202x_led *led, struct led_init_data *init_data)
 {
 	struct led_classdev *cdev;
 	u32 reg;
 	int ret;
 
-	ret = of_property_read_u32(np, "reg", &reg);
+	ret = fwnode_property_read_u32(fwnode, "reg", &reg);
 	if (ret != 0 || reg >= chip->num_leds) {
-		dev_err(chip->dev, "invalid 'reg' of %pOFn\n", np);
+		dev_err(chip->dev, "invalid 'reg' of %pfw\n", fwnode);
 		return -EINVAL;
 	}
 	led->index = reg;
@@ -454,7 +456,9 @@ static int ktd202x_setup_led_single(struct ktd202x *chip, struct device_node *np
 	return devm_led_classdev_register_ext(chip->dev, &led->cdev, init_data);
 }
 
-static int ktd202x_add_led(struct ktd202x *chip, struct device_node *np, unsigned int index)
+static int ktd202x_add_led(struct ktd202x *chip,
+			   struct fwnode_handle *fwnode,
+			   unsigned int index)
 {
 	struct ktd202x_led *led = &chip->leds[index];
 	struct led_init_data init_data = {};
@@ -463,21 +467,21 @@ static int ktd202x_add_led(struct ktd202x *chip, struct device_node *np, unsigne
 	int ret;
 
 	/* Color property is optional in single color case */
-	ret = of_property_read_u32(np, "color", &color);
+	ret = fwnode_property_read_u32(fwnode, "color", &color);
 	if (ret < 0 && ret != -EINVAL) {
-		dev_err(chip->dev, "failed to parse 'color' of %pOF\n", np);
+		dev_err(chip->dev, "failed to parse 'color' of %pfw\n", fwnode);
 		return ret;
 	}
 
 	led->chip = chip;
-	init_data.fwnode = of_fwnode_handle(np);
+	init_data.fwnode = fwnode;
 
 	if (color == LED_COLOR_ID_RGB) {
 		cdev = &led->mcdev.led_cdev;
-		ret = ktd202x_setup_led_rgb(chip, np, led, &init_data);
+		ret = ktd202x_setup_led_rgb(chip, fwnode, led, &init_data);
 	} else {
 		cdev = &led->cdev;
-		ret = ktd202x_setup_led_single(chip, np, led, &init_data);
+		ret = ktd202x_setup_led_single(chip, fwnode, led, &init_data);
 	}
 
 	if (ret) {
@@ -492,26 +496,27 @@ static int ktd202x_add_led(struct ktd202x *chip, struct device_node *np, unsigne
 
 static int ktd202x_probe_dt(struct ktd202x *chip)
 {
-	struct device_node *np = dev_of_node(chip->dev), *child;
+	struct fwnode_handle *child, *fwnode;
+	struct device *dev = chip->dev;
 	int count;
 	int i = 0;
 
-	chip->num_leds = (int)(unsigned long)of_device_get_match_data(chip->dev);
-
-	count = of_get_available_child_count(np);
+	count = device_get_child_node_count(dev);
 	if (!count || count > chip->num_leds)
 		return -EINVAL;
 
+	fwnode = dev_fwnode(dev);
+
 	regmap_write(chip->regmap, KTD202X_REG_RESET_CONTROL, KTD202X_RSTR_RESET);
 
 	/* Allow the device to execute the complete reset */
 	usleep_range(200, 300);
 
-	for_each_available_child_of_node(np, child) {
+	fwnode_for_each_available_child_node(fwnode, child) {
 		int ret = ktd202x_add_led(chip, child, i);
 
 		if (ret) {
-			of_node_put(child);
+			fwnode_handle_put(child);
 			return ret;
 		}
 		i++;
@@ -554,6 +559,8 @@ static int ktd202x_probe(struct i2c_client *client)
 		return ret;
 	}
 
+	chip->num_leds = (unsigned long)i2c_get_match_data(client);
+
 	chip->regulators[0].supply = "vin";
 	chip->regulators[1].supply = "vio";
 	ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(chip->regulators), chip->regulators);
@@ -605,7 +612,7 @@ static void ktd202x_shutdown(struct i2c_client *client)
 static const struct of_device_id ktd202x_match_table[] = {
 	{ .compatible = "kinetic,ktd2026", .data = (void *)KTD2026_NUM_LEDS },
 	{ .compatible = "kinetic,ktd2027", .data = (void *)KTD2027_NUM_LEDS },
-	{},
+	{}
 };
 MODULE_DEVICE_TABLE(of, ktd202x_match_table);
 
@@ -616,7 +623,7 @@ static struct i2c_driver ktd202x_driver = {
 	},
 	.probe = ktd202x_probe,
 	.remove = ktd202x_remove,
-	.shutdown = ktd202x_shutdown,
+	.shutdown = ktd202x_shutdown
 };
 module_i2c_driver(ktd202x_driver);
 
-- 
2.44.0

[PATCH v6 3/5] leds: rgb: leds-ktd202x: I2C ID tables for KTD2026 and 2027

[Kate Hsuan]

This table shows the maximum support LED channel for KTD2026 and KTD-2027.
The 3-channel LED controller KTD2026 controls R/G/B three LEDs. The
4-channel LED controller KTD2027 controls R/G/B and a flashing LEDs.

Link: https://www.kinet-ic.com/uploads/KTD2026-7-04h.pdf

Signed-off-by: Kate Hsuan <hpa@redhat.com>
---
 drivers/leds/rgb/leds-ktd202x.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/leds/rgb/leds-ktd202x.c b/drivers/leds/rgb/leds-ktd202x.c
index 8d0ed1a95a9f..2c47b0405961 100644
--- a/drivers/leds/rgb/leds-ktd202x.c
+++ b/drivers/leds/rgb/leds-ktd202x.c
@@ -609,6 +609,13 @@ static void ktd202x_shutdown(struct i2c_client *client)
 	regmap_write(chip->regmap, KTD202X_REG_RESET_CONTROL, KTD202X_RSTR_RESET);
 }
 
+static const struct i2c_device_id ktd202x_id[] = {
+	{"ktd2026", KTD2026_NUM_LEDS},
+	{"ktd2027", KTD2027_NUM_LEDS},
+	{}
+};
+MODULE_DEVICE_TABLE(i2c, ktd202x_id);
+
 static const struct of_device_id ktd202x_match_table[] = {
 	{ .compatible = "kinetic,ktd2026", .data = (void *)KTD2026_NUM_LEDS },
 	{ .compatible = "kinetic,ktd2027", .data = (void *)KTD2027_NUM_LEDS },
@@ -623,7 +630,8 @@ static struct i2c_driver ktd202x_driver = {
 	},
 	.probe = ktd202x_probe,
 	.remove = ktd202x_remove,
-	.shutdown = ktd202x_shutdown
+	.shutdown = ktd202x_shutdown,
+	.id_table = ktd202x_id
 };
 module_i2c_driver(ktd202x_driver);
 
-- 
2.44.0

[PATCH v6 4/5] power: supply: power-supply-leds: Add charging_orange_full_green trigger for RGB LED

[Kate Hsuan]

Add a charging_orange_full_green LED trigger and the trigger is based on
led_mc_trigger_event() which can set an RGB LED when the trigger is
triggered. The LED will show orange when the battery status is charging.
The LED will show green when the battery status is full.

Link: https://lore.kernel.org/linux-leds/f40a0b1a-ceac-e269-c2dd-0158c5b4a1ad@gmail.com/

Signed-off-by: Kate Hsuan <hpa@redhat.com>
---
 drivers/power/supply/power_supply_leds.c | 26 ++++++++++++++++++++++++
 include/linux/power_supply.h             |  2 ++
 2 files changed, 28 insertions(+)

diff --git a/drivers/power/supply/power_supply_leds.c b/drivers/power/supply/power_supply_leds.c
index c7db29d5fcb8..8dd99199c65b 100644
--- a/drivers/power/supply/power_supply_leds.c
+++ b/drivers/power/supply/power_supply_leds.c
@@ -22,6 +22,9 @@
 static void power_supply_update_bat_leds(struct power_supply *psy)
 {
 	union power_supply_propval status;
+	unsigned int intensity_green[3] = {255, 0, 0};
+	unsigned int intensity_orange[3] = {128, 0, 255};
+	unsigned int intensity_red[3] = {0, 0, 255};
 
 	if (power_supply_get_property(psy, POWER_SUPPLY_PROP_STATUS, &status))
 		return;
@@ -36,12 +39,20 @@ static void power_supply_update_bat_leds(struct power_supply *psy)
 		/* Going from blink to LED on requires a LED_OFF event to stop blink */
 		led_trigger_event(psy->charging_blink_full_solid_trig, LED_OFF);
 		led_trigger_event(psy->charging_blink_full_solid_trig, LED_FULL);
+		led_mc_trigger_event(psy->charging_orange_full_green_trig,
+				     intensity_green,
+				     ARRAY_SIZE(intensity_green),
+				     LED_FULL);
 		break;
 	case POWER_SUPPLY_STATUS_CHARGING:
 		led_trigger_event(psy->charging_full_trig, LED_FULL);
 		led_trigger_event(psy->charging_trig, LED_FULL);
 		led_trigger_event(psy->full_trig, LED_OFF);
 		led_trigger_blink(psy->charging_blink_full_solid_trig, 0, 0);
+		led_mc_trigger_event(psy->charging_orange_full_green_trig,
+				     intensity_orange,
+				     ARRAY_SIZE(intensity_orange),
+				     LED_FULL);
 		break;
 	default:
 		led_trigger_event(psy->charging_full_trig, LED_OFF);
@@ -49,6 +60,10 @@ static void power_supply_update_bat_leds(struct power_supply *psy)
 		led_trigger_event(psy->full_trig, LED_OFF);
 		led_trigger_event(psy->charging_blink_full_solid_trig,
 			LED_OFF);
+		led_mc_trigger_event(psy->charging_orange_full_green_trig,
+				     intensity_red,
+				     ARRAY_SIZE(intensity_red),
+				     LED_OFF);
 		break;
 	}
 }
@@ -74,6 +89,11 @@ static int power_supply_create_bat_triggers(struct power_supply *psy)
 	if (!psy->charging_blink_full_solid_trig_name)
 		goto charging_blink_full_solid_failed;
 
+	psy->charging_orange_full_green_trig_name = kasprintf(GFP_KERNEL,
+		"%s-charging-orange-full-green", psy->desc->name);
+	if (!psy->charging_orange_full_green_trig_name)
+		goto charging_red_full_green_failed;
+
 	led_trigger_register_simple(psy->charging_full_trig_name,
 				    &psy->charging_full_trig);
 	led_trigger_register_simple(psy->charging_trig_name,
@@ -82,9 +102,13 @@ static int power_supply_create_bat_triggers(struct power_supply *psy)
 				    &psy->full_trig);
 	led_trigger_register_simple(psy->charging_blink_full_solid_trig_name,
 				    &psy->charging_blink_full_solid_trig);
+	led_trigger_register_simple(psy->charging_orange_full_green_trig_name,
+				    &psy->charging_orange_full_green_trig);
 
 	return 0;
 
+charging_red_full_green_failed:
+	kfree(psy->charging_blink_full_solid_trig_name);
 charging_blink_full_solid_failed:
 	kfree(psy->full_trig_name);
 full_failed:
@@ -101,10 +125,12 @@ static void power_supply_remove_bat_triggers(struct power_supply *psy)
 	led_trigger_unregister_simple(psy->charging_trig);
 	led_trigger_unregister_simple(psy->full_trig);
 	led_trigger_unregister_simple(psy->charging_blink_full_solid_trig);
+	led_trigger_unregister_simple(psy->charging_orange_full_green_trig);
 	kfree(psy->charging_blink_full_solid_trig_name);
 	kfree(psy->full_trig_name);
 	kfree(psy->charging_trig_name);
 	kfree(psy->charging_full_trig_name);
+	kfree(psy->charging_orange_full_green_trig_name);
 }
 
 /* Generated power specific LEDs triggers. */
diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
index c0992a77feea..9b6898085224 100644
--- a/include/linux/power_supply.h
+++ b/include/linux/power_supply.h
@@ -318,6 +318,8 @@ struct power_supply {
 	char *online_trig_name;
 	struct led_trigger *charging_blink_full_solid_trig;
 	char *charging_blink_full_solid_trig_name;
+	struct led_trigger *charging_orange_full_green_trig;
+	char *charging_orange_full_green_trig_name;
 #endif
 };
 
-- 
2.44.0

[PATCH v6 5/5] platform: x86-android-tablets: others: Set the LED trigger to charging_orange_full_green for Xiaomi pad2

[Kate Hsuan]

Set the default trigger to bq27520-0-charging-orange-full-green. The LED
will show orange when the battery is charging. The LED will show green
when the battery status is full.

Signed-off-by: Kate Hsuan <hpa@redhat.com>
---
 drivers/platform/x86/x86-android-tablets/other.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/platform/x86/x86-android-tablets/other.c b/drivers/platform/x86/x86-android-tablets/other.c
index c77d56454f2d..52032a874b7f 100644
--- a/drivers/platform/x86/x86-android-tablets/other.c
+++ b/drivers/platform/x86/x86-android-tablets/other.c
@@ -610,7 +610,7 @@ static const struct property_entry ktd2026_rgb_led_props[] = {
 	PROPERTY_ENTRY_U32("reg", 0),
 	PROPERTY_ENTRY_U32("color", LED_COLOR_ID_RGB),
 	PROPERTY_ENTRY_STRING("function", "indicator"),
-	PROPERTY_ENTRY_STRING("linux,default-trigger", "bq27520-0-charging"),
+	PROPERTY_ENTRY_STRING("linux,default-trigger", "bq27520-0-charging-orange-full-green"),
 	{ }
 };
 
-- 
2.44.0

Re: [PATCH v6 1/5] platform: x86-android-tablets: other: Add swnode for Xiaomi pad2 indicator LED

[Andy Shevchenko]

On Tue, Apr 16, 2024 at 8:39 AM Kate Hsuan <hpa@redhat.com> wrote:
>
> KTD2026 LED controller manages the indicator LED for Xiaomi pad2. The ACPI
> for it is not properly made so the kernel can't get a correct description.
>
> This work adds a description for this RGB LED controller and also sets a
> trigger to indicate the changing event (bq27520-0-charging). When it is
> charging, the indicator LED will be turned on.

...

> +/*
> + * The fwnode for ktd2026 on Xaomi pad2. It composed of a RGB LED node

is composed

> + * with three subnodes for each color. The RGB LED node is named
> + * "multi-led" to align with the name in the device tree.
> + */

...

> +static const struct software_node ktd2026_node = {
> +       .name = "ktd2026"

Please, leave a trailing comma as it's not a termination entry.

> +};

(TBH I'm still unsure that having a name is a good idea even if it's
supposed to be only a single device on the platform, but it's up to
Hans who has an experience with those.)

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v6 4/5] power: supply: power-supply-leds: Add charging_orange_full_green trigger for RGB LED

[kernel test robot]

Hi Kate,

kernel test robot noticed the following build errors:

[auto build test ERROR on sre-power-supply/for-next]
[also build test ERROR on lee-leds/for-leds-next linus/master v6.9-rc4]
[cannot apply to pavel-leds/for-next next-20240416]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Kate-Hsuan/platform-x86-android-tablets-other-Add-swnode-for-Xiaomi-pad2-indicator-LED/20240416-134240
base:   https://git.kernel.org/pub/scm/linux/kernel/git/sre/linux-power-supply.git for-next
patch link:    https://lore.kernel.org/r/20240416053909.256319-5-hpa%40redhat.com
patch subject: [PATCH v6 4/5] power: supply: power-supply-leds: Add charging_orange_full_green trigger for RGB LED
config: parisc64-defconfig (https://download.01.org/0day-ci/archive/20240417/202404170023.1zEGO9ja-lkp@intel.com/config)
compiler: hppa64-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240417/202404170023.1zEGO9ja-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202404170023.1zEGO9ja-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/power/supply/power_supply_leds.c: In function 'power_supply_update_bat_leds':
>> drivers/power/supply/power_supply_leds.c:42:17: error: implicit declaration of function 'led_mc_trigger_event'; did you mean 'led_trigger_event'? [-Werror=implicit-function-declaration]
      42 |                 led_mc_trigger_event(psy->charging_orange_full_green_trig,
         |                 ^~~~~~~~~~~~~~~~~~~~
         |                 led_trigger_event
   cc1: some warnings being treated as errors


vim +42 drivers/power/supply/power_supply_leds.c

    21	
    22	static void power_supply_update_bat_leds(struct power_supply *psy)
    23	{
    24		union power_supply_propval status;
    25		unsigned int intensity_green[3] = {255, 0, 0};
    26		unsigned int intensity_orange[3] = {128, 0, 255};
    27		unsigned int intensity_red[3] = {0, 0, 255};
    28	
    29		if (power_supply_get_property(psy, POWER_SUPPLY_PROP_STATUS, &status))
    30			return;
    31	
    32		dev_dbg(&psy->dev, "%s %d\n", __func__, status.intval);
    33	
    34		switch (status.intval) {
    35		case POWER_SUPPLY_STATUS_FULL:
    36			led_trigger_event(psy->charging_full_trig, LED_FULL);
    37			led_trigger_event(psy->charging_trig, LED_OFF);
    38			led_trigger_event(psy->full_trig, LED_FULL);
    39			/* Going from blink to LED on requires a LED_OFF event to stop blink */
    40			led_trigger_event(psy->charging_blink_full_solid_trig, LED_OFF);
    41			led_trigger_event(psy->charging_blink_full_solid_trig, LED_FULL);
  > 42			led_mc_trigger_event(psy->charging_orange_full_green_trig,
    43					     intensity_green,
    44					     ARRAY_SIZE(intensity_green),
    45					     LED_FULL);
    46			break;
    47		case POWER_SUPPLY_STATUS_CHARGING:
    48			led_trigger_event(psy->charging_full_trig, LED_FULL);
    49			led_trigger_event(psy->charging_trig, LED_FULL);
    50			led_trigger_event(psy->full_trig, LED_OFF);
    51			led_trigger_blink(psy->charging_blink_full_solid_trig, 0, 0);
    52			led_mc_trigger_event(psy->charging_orange_full_green_trig,
    53					     intensity_orange,
    54					     ARRAY_SIZE(intensity_orange),
    55					     LED_FULL);
    56			break;
    57		default:
    58			led_trigger_event(psy->charging_full_trig, LED_OFF);
    59			led_trigger_event(psy->charging_trig, LED_OFF);
    60			led_trigger_event(psy->full_trig, LED_OFF);
    61			led_trigger_event(psy->charging_blink_full_solid_trig,
    62				LED_OFF);
    63			led_mc_trigger_event(psy->charging_orange_full_green_trig,
    64					     intensity_red,
    65					     ARRAY_SIZE(intensity_red),
    66					     LED_OFF);
    67			break;
    68		}
    69	}
    70	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v6 2/5] leds: rgb: leds-ktd202x: Get device properties through fwnode to support ACPI

[Andy Shevchenko]

On Tue, Apr 16, 2024 at 8:39 AM Kate Hsuan <hpa@redhat.com> wrote:
>
> This LED controller is installed on a Xiaomi pad2 and it is an x86
> platform. The original driver is based on the device tree and can't be
> used for this ACPI based system. This patch migrated the driver to use
> fwnode to access the properties. Moreover, the fwnode API supports the
> device tree so this work won't affect the original implementations.

...

> -       int num_channels;
> +       int num_channels = 0;

Split this assignment, so...

>         int i = 0;

> -       num_channels = of_get_available_child_count(np);

...it become

  num_channels = 0;

here.

> +       fwnode_for_each_available_child_node(fwnode, child)
> +               num_channels++;

...

> -static int ktd202x_add_led(struct ktd202x *chip, struct device_node *np, unsigned int index)
> +static int ktd202x_add_led(struct ktd202x *chip,
> +                          struct fwnode_handle *fwnode,
> +                          unsigned int index)

Why split over 3 lines? I believe it can be still two or one
(depending if you use a relaxed limit).

...

>  static int ktd202x_probe_dt(struct ktd202x *chip)

Perhaps you want to rename this to something like ktd202x_probe_fw().

...

> +       fwnode = dev_fwnode(dev);

Will be no use if the bellow applied, right?

...

> -       for_each_available_child_of_node(np, child) {
> +       fwnode_for_each_available_child_node(fwnode, child) {

Use device_for_each_child_node() instead.

>         }

...

> -       .shutdown = ktd202x_shutdown,
> +       .shutdown = ktd202x_shutdown

Stray change.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v6 3/5] leds: rgb: leds-ktd202x: I2C ID tables for KTD2026 and 2027

[Andy Shevchenko]

On Tue, Apr 16, 2024 at 8:40 AM Kate Hsuan <hpa@redhat.com> wrote:
>
> This table shows the maximum support LED channel for KTD2026 and KTD-2027.
> The 3-channel LED controller KTD2026 controls R/G/B three LEDs. The
> 4-channel LED controller KTD2027 controls R/G/B and a flashing LEDs.

and flashing

...

> Link: https://www.kinet-ic.com/uploads/KTD2026-7-04h.pdf

Make it Datasheet: tag

>

and drop this blank line (to ensure the Datasheet will be recognised as a tag).

> Signed-off-by: Kate Hsuan <hpa@redhat.com>

...

> -       .shutdown = ktd202x_shutdown
> +       .shutdown = ktd202x_shutdown,
> +       .id_table = ktd202x_id

Exactly good example to show what the difference is between
"terminator entry" and "last field in the initialiser" and why in the
latter it's better to keep a trailing comma. And hence why the stray
change in the previous patch. So, id_table also should keep a trailing
comma.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v6 4/5] power: supply: power-supply-leds: Add charging_orange_full_green trigger for RGB LED

[Andy Shevchenko]

On Tue, Apr 16, 2024 at 8:03 PM kernel test robot <lkp@intel.com> wrote:
>
> Hi Kate,
>
> kernel test robot noticed the following build errors:

> All errors (new ones prefixed by >>):
>
>    drivers/power/supply/power_supply_leds.c: In function 'power_supply_update_bat_leds':
> >> drivers/power/supply/power_supply_leds.c:42:17: error: implicit declaration of function 'led_mc_trigger_event'; did you mean 'led_trigger_event'? [-Werror=implicit-function-declaration]
>       42 |                 led_mc_trigger_event(psy->charging_orange_full_green_trig,
>          |                 ^~~~~~~~~~~~~~~~~~~~
>          |                 led_trigger_event
>    cc1: some warnings being treated as errors

Probably you need a new dependency or so.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v6 4/5] power: supply: power-supply-leds: Add charging_orange_full_green trigger for RGB LED

[kernel test robot]

Hi Kate,

kernel test robot noticed the following build errors:

[auto build test ERROR on sre-power-supply/for-next]
[also build test ERROR on lee-leds/for-leds-next linus/master v6.9-rc4]
[cannot apply to next-20240416]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Kate-Hsuan/platform-x86-android-tablets-other-Add-swnode-for-Xiaomi-pad2-indicator-LED/20240416-134240
base:   https://git.kernel.org/pub/scm/linux/kernel/git/sre/linux-power-supply.git for-next
patch link:    https://lore.kernel.org/r/20240416053909.256319-5-hpa%40redhat.com
patch subject: [PATCH v6 4/5] power: supply: power-supply-leds: Add charging_orange_full_green trigger for RGB LED
config: arm-defconfig (https://download.01.org/0day-ci/archive/20240417/202404170705.Or8MFKNi-lkp@intel.com/config)
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project.git f28c006a5895fc0e329fe15fead81e37457cb1d1)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240417/202404170705.Or8MFKNi-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202404170705.Or8MFKNi-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/power/supply/power_supply_leds.c:42:3: error: implicit declaration of function 'led_mc_trigger_event' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
                   led_mc_trigger_event(psy->charging_orange_full_green_trig,
                   ^
   drivers/power/supply/power_supply_leds.c:42:3: note: did you mean 'led_trigger_event'?
   include/linux/leds.h:489:6: note: 'led_trigger_event' declared here
   void led_trigger_event(struct led_trigger *trigger,  enum led_brightness event);
        ^
   1 error generated.


vim +/led_mc_trigger_event +42 drivers/power/supply/power_supply_leds.c

    21	
    22	static void power_supply_update_bat_leds(struct power_supply *psy)
    23	{
    24		union power_supply_propval status;
    25		unsigned int intensity_green[3] = {255, 0, 0};
    26		unsigned int intensity_orange[3] = {128, 0, 255};
    27		unsigned int intensity_red[3] = {0, 0, 255};
    28	
    29		if (power_supply_get_property(psy, POWER_SUPPLY_PROP_STATUS, &status))
    30			return;
    31	
    32		dev_dbg(&psy->dev, "%s %d\n", __func__, status.intval);
    33	
    34		switch (status.intval) {
    35		case POWER_SUPPLY_STATUS_FULL:
    36			led_trigger_event(psy->charging_full_trig, LED_FULL);
    37			led_trigger_event(psy->charging_trig, LED_OFF);
    38			led_trigger_event(psy->full_trig, LED_FULL);
    39			/* Going from blink to LED on requires a LED_OFF event to stop blink */
    40			led_trigger_event(psy->charging_blink_full_solid_trig, LED_OFF);
    41			led_trigger_event(psy->charging_blink_full_solid_trig, LED_FULL);
  > 42			led_mc_trigger_event(psy->charging_orange_full_green_trig,
    43					     intensity_green,
    44					     ARRAY_SIZE(intensity_green),
    45					     LED_FULL);
    46			break;
    47		case POWER_SUPPLY_STATUS_CHARGING:
    48			led_trigger_event(psy->charging_full_trig, LED_FULL);
    49			led_trigger_event(psy->charging_trig, LED_FULL);
    50			led_trigger_event(psy->full_trig, LED_OFF);
    51			led_trigger_blink(psy->charging_blink_full_solid_trig, 0, 0);
    52			led_mc_trigger_event(psy->charging_orange_full_green_trig,
    53					     intensity_orange,
    54					     ARRAY_SIZE(intensity_orange),
    55					     LED_FULL);
    56			break;
    57		default:
    58			led_trigger_event(psy->charging_full_trig, LED_OFF);
    59			led_trigger_event(psy->charging_trig, LED_OFF);
    60			led_trigger_event(psy->full_trig, LED_OFF);
    61			led_trigger_event(psy->charging_blink_full_solid_trig,
    62				LED_OFF);
    63			led_mc_trigger_event(psy->charging_orange_full_green_trig,
    64					     intensity_red,
    65					     ARRAY_SIZE(intensity_red),
    66					     LED_OFF);
    67			break;
    68		}
    69	}
    70	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v6 4/5] power: supply: power-supply-leds: Add charging_orange_full_green trigger for RGB LED

[Kate Hsuan]

Hi,

On Wed, Apr 17, 2024 at 1:34 AM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Tue, Apr 16, 2024 at 8:03 PM kernel test robot <lkp@intel.com> wrote:
> >
> > Hi Kate,
> >
> > kernel test robot noticed the following build errors:
>
> > All errors (new ones prefixed by >>):
> >
> >    drivers/power/supply/power_supply_leds.c: In function 'power_supply_update_bat_leds':
> > >> drivers/power/supply/power_supply_leds.c:42:17: error: implicit declaration of function 'led_mc_trigger_event'; did you mean 'led_trigger_event'? [-Werror=implicit-function-declaration]
> >       42 |                 led_mc_trigger_event(psy->charging_orange_full_green_trig,
> >          |                 ^~~~~~~~~~~~~~~~~~~~
> >          |                 led_trigger_event
> >    cc1: some warnings being treated as errors
>
> Probably you need a new dependency or so.

I will include Hans' two patches for led_mc_trigger_event() in v7 patch.

>
> --
> With Best Regards,
> Andy Shevchenko
>


-- 
BR,
Kate

Re: [PATCH v6 3/5] leds: rgb: leds-ktd202x: I2C ID tables for KTD2026 and 2027

[Kate Hsuan]

Hi Andy

On Wed, Apr 17, 2024 at 1:32 AM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Tue, Apr 16, 2024 at 8:40 AM Kate Hsuan <hpa@redhat.com> wrote:
> >
> > This table shows the maximum support LED channel for KTD2026 and KTD-2027.
> > The 3-channel LED controller KTD2026 controls R/G/B three LEDs. The
> > 4-channel LED controller KTD2027 controls R/G/B and a flashing LEDs.
>
> and flashing

LEDs should be LED.

>
> ...
>
> > Link: https://www.kinet-ic.com/uploads/KTD2026-7-04h.pdf
>
> Make it Datasheet: tag
Okay
>
> >
>
> and drop this blank line (to ensure the Datasheet will be recognised as a tag).
Okay
>
> > Signed-off-by: Kate Hsuan <hpa@redhat.com>
>
> ...
>
> > -       .shutdown = ktd202x_shutdown
> > +       .shutdown = ktd202x_shutdown,
> > +       .id_table = ktd202x_id
>
> Exactly good example to show what the difference is between
> "terminator entry" and "last field in the initialiser" and why in the
> latter it's better to keep a trailing comma. And hence why the stray
> change in the previous patch. So, id_table also should keep a trailing
> comma.

Thank you for reviewing it.
Lesson learned. Now I know how to manipulate the comma correctly.

>
> --
> With Best Regards,
> Andy Shevchenko
>


-- 
BR,
Kate

Re: [PATCH v6 2/5] leds: rgb: leds-ktd202x: Get device properties through fwnode to support ACPI

[Kate Hsuan]

Hi Andy,

Thank you for reviewing.

On Wed, Apr 17, 2024 at 1:29 AM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Tue, Apr 16, 2024 at 8:39 AM Kate Hsuan <hpa@redhat.com> wrote:
> >
> > This LED controller is installed on a Xiaomi pad2 and it is an x86
> > platform. The original driver is based on the device tree and can't be
> > used for this ACPI based system. This patch migrated the driver to use
> > fwnode to access the properties. Moreover, the fwnode API supports the
> > device tree so this work won't affect the original implementations.
>
> ...
>
> > -       int num_channels;
> > +       int num_channels = 0;
>
> Split this assignment, so...
>
> >         int i = 0;
>
> > -       num_channels = of_get_available_child_count(np);
>
> ...it become
>
>   num_channels = 0;
>
> here.
>
> > +       fwnode_for_each_available_child_node(fwnode, child)
> > +               num_channels++;
>

It will look like this:
       num_channels = 0;
       fwnode_for_each_available_child_node(fwnode, child)
              num_channels++;

> ...
>
> > -static int ktd202x_add_led(struct ktd202x *chip, struct device_node *np, unsigned int index)
> > +static int ktd202x_add_led(struct ktd202x *chip,
> > +                          struct fwnode_handle *fwnode,
> > +                          unsigned int index)
>
> Why split over 3 lines? I believe it can be still two or one
> (depending if you use a relaxed limit).

Make it to be one line.

>
> ...
>
> >  static int ktd202x_probe_dt(struct ktd202x *chip)
>
> Perhaps you want to rename this to something like ktd202x_probe_fw().

Sounds good.

>
> ...
>
> > +       fwnode = dev_fwnode(dev);
>
> Will be no use if the bellow applied, right?

Right. It can be dropped.

>
> ...
>
> > -       for_each_available_child_of_node(np, child) {
> > +       fwnode_for_each_available_child_node(fwnode, child) {
>
> Use device_for_each_child_node() instead.

Okay.

>
> >         }
>
> ...
>
> > -       .shutdown = ktd202x_shutdown,
> > +       .shutdown = ktd202x_shutdown
>
> Stray change.

I know the reason  :)

>
> --
> With Best Regards,
> Andy Shevchenko
>


-- 
BR,
Kate

Re: [PATCH v6 1/5] platform: x86-android-tablets: other: Add swnode for Xiaomi pad2 indicator LED

[Kate Hsuan]

Hi Andy,

Thank you for reviewing.

On Tue, Apr 16, 2024 at 9:46 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Tue, Apr 16, 2024 at 8:39 AM Kate Hsuan <hpa@redhat.com> wrote:
> >
> > KTD2026 LED controller manages the indicator LED for Xiaomi pad2. The ACPI
> > for it is not properly made so the kernel can't get a correct description.
> >
> > This work adds a description for this RGB LED controller and also sets a
> > trigger to indicate the changing event (bq27520-0-charging). When it is
> > charging, the indicator LED will be turned on.
>
> ...
>
> > +/*
> > + * The fwnode for ktd2026 on Xaomi pad2. It composed of a RGB LED node
>
> is composed

Okay.

>
> > + * with three subnodes for each color. The RGB LED node is named
> > + * "multi-led" to align with the name in the device tree.
> > + */
>
> ...
>
> > +static const struct software_node ktd2026_node = {
> > +       .name = "ktd2026"
>
> Please, leave a trailing comma as it's not a termination entry.

Okay.

>
> > +};
>
> (TBH I'm still unsure that having a name is a good idea even if it's
> supposed to be only a single device on the platform, but it's up to
> Hans who has an experience with those.)

Hans gave me an example without a name. I think, it can be dropped.
Moreover, Only one KTD2026 is on Xiaomi Pad2 so having a name is ok too.

>
> --
> With Best Regards,
> Andy Shevchenko
>


-- 
BR,
Kate

Re: [PATCH v6 1/5] platform: x86-android-tablets: other: Add swnode for Xiaomi pad2 indicator LED

[Hans de Goede]

Hi Kate,

On 4/16/24 7:39 AM, Kate Hsuan wrote:
> KTD2026 LED controller manages the indicator LED for Xiaomi pad2. The ACPI
> for it is not properly made so the kernel can't get a correct description.
> 
> This work adds a description for this RGB LED controller and also sets a
> trigger to indicate the changing event (bq27520-0-charging). When it is
> charging, the indicator LED will be turned on.
> 
> Signed-off-by: Kate Hsuan <hpa@redhat.com>

Since this patch is more or less done and since which fwnodes there should
be and with which contents is prescribed by the existing devicetree
bindings which are not being changes I have already merged this patch
into pdx86/for-next:

https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/commit/?h=for-next&id=fcc6220ddc7e54d8442287273d0cb8c415ada022

So there is no reason to resend this. Please drop this patch from v7
of the patch-set.

Regards,

Hans




> ---
>  .../platform/x86/x86-android-tablets/other.c  | 82 +++++++++++++++++++
>  .../x86/x86-android-tablets/shared-psy-info.h |  2 +
>  2 files changed, 84 insertions(+)
> 
> diff --git a/drivers/platform/x86/x86-android-tablets/other.c b/drivers/platform/x86/x86-android-tablets/other.c
> index bc6bbf7ec6ea..c77d56454f2d 100644
> --- a/drivers/platform/x86/x86-android-tablets/other.c
> +++ b/drivers/platform/x86/x86-android-tablets/other.c
> @@ -13,6 +13,8 @@
>  #include <linux/input.h>
>  #include <linux/platform_device.h>
>  
> +#include <dt-bindings/leds/common.h>
> +
>  #include "shared-psy-info.h"
>  #include "x86-android-tablets.h"
>  
> @@ -593,6 +595,83 @@ const struct x86_dev_info whitelabel_tm800a550l_info __initconst = {
>  	.gpiod_lookup_tables = whitelabel_tm800a550l_gpios,
>  };
>  
> +/*
> + * The fwnode for ktd2026 on Xaomi pad2. It composed of a RGB LED node
> + * with three subnodes for each color. The RGB LED node is named
> + * "multi-led" to align with the name in the device tree.
> + */
> +
> +/* main fwnode for ktd2026 */
> +static const struct software_node ktd2026_node = {
> +	.name = "ktd2026"
> +};
> +
> +static const struct property_entry ktd2026_rgb_led_props[] = {
> +	PROPERTY_ENTRY_U32("reg", 0),
> +	PROPERTY_ENTRY_U32("color", LED_COLOR_ID_RGB),
> +	PROPERTY_ENTRY_STRING("function", "indicator"),
> +	PROPERTY_ENTRY_STRING("linux,default-trigger", "bq27520-0-charging"),
> +	{ }
> +};
> +
> +static const struct software_node ktd2026_rgb_led_node = {
> +	.name = "multi-led",
> +	.properties = ktd2026_rgb_led_props,
> +	.parent = &ktd2026_node,
> +};
> +
> +static const struct property_entry ktd2026_blue_led_props[] = {
> +	PROPERTY_ENTRY_U32("reg", 0),
> +	PROPERTY_ENTRY_U32("color", LED_COLOR_ID_BLUE),
> +	{ }
> +};
> +
> +static const struct software_node ktd2026_blue_led_node = {
> +	.properties = ktd2026_blue_led_props,
> +	.parent = &ktd2026_rgb_led_node,
> +};
> +
> +static const struct property_entry ktd2026_green_led_props[] = {
> +	PROPERTY_ENTRY_U32("reg", 1),
> +	PROPERTY_ENTRY_U32("color", LED_COLOR_ID_GREEN),
> +	{ }
> +};
> +
> +static const struct software_node ktd2026_green_led_node = {
> +	.properties = ktd2026_green_led_props,
> +	.parent = &ktd2026_rgb_led_node,
> +};
> +
> +static const struct property_entry ktd2026_red_led_props[] = {
> +	PROPERTY_ENTRY_U32("reg", 2),
> +	PROPERTY_ENTRY_U32("color", LED_COLOR_ID_RED),
> +	{ }
> +};
> +
> +static const struct software_node ktd2026_red_led_node = {
> +	.properties = ktd2026_red_led_props,
> +	.parent = &ktd2026_rgb_led_node,
> +};
> +
> +static const struct software_node *ktd2026_node_group[] = {
> +	&ktd2026_node,
> +	&ktd2026_rgb_led_node,
> +	&ktd2026_green_led_node,
> +	&ktd2026_blue_led_node,
> +	&ktd2026_red_led_node,
> +	NULL
> +};
> +
> +static int __init xiaomi_mipad2_init(void)
> +{
> +	return software_node_register_node_group(ktd2026_node_group);
> +}
> +
> +static void xiaomi_mipad2_exit(void)
> +{
> +	software_node_unregister_node_group(ktd2026_node_group);
> +}
> +
>  /*
>   * If the EFI bootloader is not Xiaomi's own signed Android loader, then the
>   * Xiaomi Mi Pad 2 X86 tablet sets OSID in the DSDT to 1 (Windows), causing
> @@ -616,6 +695,7 @@ static const struct x86_i2c_client_info xiaomi_mipad2_i2c_clients[] __initconst
>  			.type = "ktd2026",
>  			.addr = 0x30,
>  			.dev_name = "ktd2026",
> +			.swnode = &ktd2026_node,
>  		},
>  		.adapter_path = "\\_SB_.PCI0.I2C3",
>  	},
> @@ -624,4 +704,6 @@ static const struct x86_i2c_client_info xiaomi_mipad2_i2c_clients[] __initconst
>  const struct x86_dev_info xiaomi_mipad2_info __initconst = {
>  	.i2c_client_info = xiaomi_mipad2_i2c_clients,
>  	.i2c_client_count = ARRAY_SIZE(xiaomi_mipad2_i2c_clients),
> +	.init = xiaomi_mipad2_init,
> +	.exit = xiaomi_mipad2_exit,
>  };
> diff --git a/drivers/platform/x86/x86-android-tablets/shared-psy-info.h b/drivers/platform/x86/x86-android-tablets/shared-psy-info.h
> index c2d2968cddc2..8c33ec47ee12 100644
> --- a/drivers/platform/x86/x86-android-tablets/shared-psy-info.h
> +++ b/drivers/platform/x86/x86-android-tablets/shared-psy-info.h
> @@ -29,4 +29,6 @@ extern const char * const bq24190_modules[];
>  extern const struct platform_device_info int3496_pdevs[];
>  extern struct gpiod_lookup_table int3496_reference_gpios;
>  
> +extern const struct software_node ktd2026_leds_node;
> +
>  #endif

Re: [PATCH v6 4/5] power: supply: power-supply-leds: Add charging_orange_full_green trigger for RGB LED

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 5121 bytes --]

Hi,

On Tue, Apr 16, 2024 at 01:39:08PM +0800, Kate Hsuan wrote:
> Add a charging_orange_full_green LED trigger and the trigger is based on
> led_mc_trigger_event() which can set an RGB LED when the trigger is
> triggered. The LED will show orange when the battery status is charging.
> The LED will show green when the battery status is full.
> 
> Link: https://lore.kernel.org/linux-leds/f40a0b1a-ceac-e269-c2dd-0158c5b4a1ad@gmail.com/
> 
> Signed-off-by: Kate Hsuan <hpa@redhat.com>
> ---

Acked-by: Sebastian Reichel <sebastian.reichel@collabora.com>

-- Sebastian

>  drivers/power/supply/power_supply_leds.c | 26 ++++++++++++++++++++++++
>  include/linux/power_supply.h             |  2 ++
>  2 files changed, 28 insertions(+)
> 
> diff --git a/drivers/power/supply/power_supply_leds.c b/drivers/power/supply/power_supply_leds.c
> index c7db29d5fcb8..8dd99199c65b 100644
> --- a/drivers/power/supply/power_supply_leds.c
> +++ b/drivers/power/supply/power_supply_leds.c
> @@ -22,6 +22,9 @@
>  static void power_supply_update_bat_leds(struct power_supply *psy)
>  {
>  	union power_supply_propval status;
> +	unsigned int intensity_green[3] = {255, 0, 0};
> +	unsigned int intensity_orange[3] = {128, 0, 255};
> +	unsigned int intensity_red[3] = {0, 0, 255};
>  
>  	if (power_supply_get_property(psy, POWER_SUPPLY_PROP_STATUS, &status))
>  		return;
> @@ -36,12 +39,20 @@ static void power_supply_update_bat_leds(struct power_supply *psy)
>  		/* Going from blink to LED on requires a LED_OFF event to stop blink */
>  		led_trigger_event(psy->charging_blink_full_solid_trig, LED_OFF);
>  		led_trigger_event(psy->charging_blink_full_solid_trig, LED_FULL);
> +		led_mc_trigger_event(psy->charging_orange_full_green_trig,
> +				     intensity_green,
> +				     ARRAY_SIZE(intensity_green),
> +				     LED_FULL);
>  		break;
>  	case POWER_SUPPLY_STATUS_CHARGING:
>  		led_trigger_event(psy->charging_full_trig, LED_FULL);
>  		led_trigger_event(psy->charging_trig, LED_FULL);
>  		led_trigger_event(psy->full_trig, LED_OFF);
>  		led_trigger_blink(psy->charging_blink_full_solid_trig, 0, 0);
> +		led_mc_trigger_event(psy->charging_orange_full_green_trig,
> +				     intensity_orange,
> +				     ARRAY_SIZE(intensity_orange),
> +				     LED_FULL);
>  		break;
>  	default:
>  		led_trigger_event(psy->charging_full_trig, LED_OFF);
> @@ -49,6 +60,10 @@ static void power_supply_update_bat_leds(struct power_supply *psy)
>  		led_trigger_event(psy->full_trig, LED_OFF);
>  		led_trigger_event(psy->charging_blink_full_solid_trig,
>  			LED_OFF);
> +		led_mc_trigger_event(psy->charging_orange_full_green_trig,
> +				     intensity_red,
> +				     ARRAY_SIZE(intensity_red),
> +				     LED_OFF);
>  		break;
>  	}
>  }
> @@ -74,6 +89,11 @@ static int power_supply_create_bat_triggers(struct power_supply *psy)
>  	if (!psy->charging_blink_full_solid_trig_name)
>  		goto charging_blink_full_solid_failed;
>  
> +	psy->charging_orange_full_green_trig_name = kasprintf(GFP_KERNEL,
> +		"%s-charging-orange-full-green", psy->desc->name);
> +	if (!psy->charging_orange_full_green_trig_name)
> +		goto charging_red_full_green_failed;
> +
>  	led_trigger_register_simple(psy->charging_full_trig_name,
>  				    &psy->charging_full_trig);
>  	led_trigger_register_simple(psy->charging_trig_name,
> @@ -82,9 +102,13 @@ static int power_supply_create_bat_triggers(struct power_supply *psy)
>  				    &psy->full_trig);
>  	led_trigger_register_simple(psy->charging_blink_full_solid_trig_name,
>  				    &psy->charging_blink_full_solid_trig);
> +	led_trigger_register_simple(psy->charging_orange_full_green_trig_name,
> +				    &psy->charging_orange_full_green_trig);
>  
>  	return 0;
>  
> +charging_red_full_green_failed:
> +	kfree(psy->charging_blink_full_solid_trig_name);
>  charging_blink_full_solid_failed:
>  	kfree(psy->full_trig_name);
>  full_failed:
> @@ -101,10 +125,12 @@ static void power_supply_remove_bat_triggers(struct power_supply *psy)
>  	led_trigger_unregister_simple(psy->charging_trig);
>  	led_trigger_unregister_simple(psy->full_trig);
>  	led_trigger_unregister_simple(psy->charging_blink_full_solid_trig);
> +	led_trigger_unregister_simple(psy->charging_orange_full_green_trig);
>  	kfree(psy->charging_blink_full_solid_trig_name);
>  	kfree(psy->full_trig_name);
>  	kfree(psy->charging_trig_name);
>  	kfree(psy->charging_full_trig_name);
> +	kfree(psy->charging_orange_full_green_trig_name);
>  }
>  
>  /* Generated power specific LEDs triggers. */
> diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
> index c0992a77feea..9b6898085224 100644
> --- a/include/linux/power_supply.h
> +++ b/include/linux/power_supply.h
> @@ -318,6 +318,8 @@ struct power_supply {
>  	char *online_trig_name;
>  	struct led_trigger *charging_blink_full_solid_trig;
>  	char *charging_blink_full_solid_trig_name;
> +	struct led_trigger *charging_orange_full_green_trig;
> +	char *charging_orange_full_green_trig_name;
>  #endif
>  };
>  
> -- 
> 2.44.0
> 
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v6 4/5] power: supply: power-supply-leds: Add charging_orange_full_green trigger for RGB LED

[Hans de Goede]

Hi,

On 4/18/24 2:34 PM, Sebastian Reichel wrote:
> Hi,
> 
> On Tue, Apr 16, 2024 at 01:39:08PM +0800, Kate Hsuan wrote:
>> Add a charging_orange_full_green LED trigger and the trigger is based on
>> led_mc_trigger_event() which can set an RGB LED when the trigger is
>> triggered. The LED will show orange when the battery status is charging.
>> The LED will show green when the battery status is full.
>>
>> Link: https://lore.kernel.org/linux-leds/f40a0b1a-ceac-e269-c2dd-0158c5b4a1ad@gmail.com/
>>
>> Signed-off-by: Kate Hsuan <hpa@redhat.com>
>> ---
> 
> Acked-by: Sebastian Reichel <sebastian.reichel@collabora.com>

Thanks. Does this mean your ok with routing this change through the LED tree
together with the 2 LED core patches adding the new led_mc_trigger_event()
function this uses ?

Regards,

Hans




> -- Sebastian
> 
>>  drivers/power/supply/power_supply_leds.c | 26 ++++++++++++++++++++++++
>>  include/linux/power_supply.h             |  2 ++
>>  2 files changed, 28 insertions(+)
>>
>> diff --git a/drivers/power/supply/power_supply_leds.c b/drivers/power/supply/power_supply_leds.c
>> index c7db29d5fcb8..8dd99199c65b 100644
>> --- a/drivers/power/supply/power_supply_leds.c
>> +++ b/drivers/power/supply/power_supply_leds.c
>> @@ -22,6 +22,9 @@
>>  static void power_supply_update_bat_leds(struct power_supply *psy)
>>  {
>>  	union power_supply_propval status;
>> +	unsigned int intensity_green[3] = {255, 0, 0};
>> +	unsigned int intensity_orange[3] = {128, 0, 255};
>> +	unsigned int intensity_red[3] = {0, 0, 255};
>>  
>>  	if (power_supply_get_property(psy, POWER_SUPPLY_PROP_STATUS, &status))
>>  		return;
>> @@ -36,12 +39,20 @@ static void power_supply_update_bat_leds(struct power_supply *psy)
>>  		/* Going from blink to LED on requires a LED_OFF event to stop blink */
>>  		led_trigger_event(psy->charging_blink_full_solid_trig, LED_OFF);
>>  		led_trigger_event(psy->charging_blink_full_solid_trig, LED_FULL);
>> +		led_mc_trigger_event(psy->charging_orange_full_green_trig,
>> +				     intensity_green,
>> +				     ARRAY_SIZE(intensity_green),
>> +				     LED_FULL);
>>  		break;
>>  	case POWER_SUPPLY_STATUS_CHARGING:
>>  		led_trigger_event(psy->charging_full_trig, LED_FULL);
>>  		led_trigger_event(psy->charging_trig, LED_FULL);
>>  		led_trigger_event(psy->full_trig, LED_OFF);
>>  		led_trigger_blink(psy->charging_blink_full_solid_trig, 0, 0);
>> +		led_mc_trigger_event(psy->charging_orange_full_green_trig,
>> +				     intensity_orange,
>> +				     ARRAY_SIZE(intensity_orange),
>> +				     LED_FULL);
>>  		break;
>>  	default:
>>  		led_trigger_event(psy->charging_full_trig, LED_OFF);
>> @@ -49,6 +60,10 @@ static void power_supply_update_bat_leds(struct power_supply *psy)
>>  		led_trigger_event(psy->full_trig, LED_OFF);
>>  		led_trigger_event(psy->charging_blink_full_solid_trig,
>>  			LED_OFF);
>> +		led_mc_trigger_event(psy->charging_orange_full_green_trig,
>> +				     intensity_red,
>> +				     ARRAY_SIZE(intensity_red),
>> +				     LED_OFF);
>>  		break;
>>  	}
>>  }
>> @@ -74,6 +89,11 @@ static int power_supply_create_bat_triggers(struct power_supply *psy)
>>  	if (!psy->charging_blink_full_solid_trig_name)
>>  		goto charging_blink_full_solid_failed;
>>  
>> +	psy->charging_orange_full_green_trig_name = kasprintf(GFP_KERNEL,
>> +		"%s-charging-orange-full-green", psy->desc->name);
>> +	if (!psy->charging_orange_full_green_trig_name)
>> +		goto charging_red_full_green_failed;
>> +
>>  	led_trigger_register_simple(psy->charging_full_trig_name,
>>  				    &psy->charging_full_trig);
>>  	led_trigger_register_simple(psy->charging_trig_name,
>> @@ -82,9 +102,13 @@ static int power_supply_create_bat_triggers(struct power_supply *psy)
>>  				    &psy->full_trig);
>>  	led_trigger_register_simple(psy->charging_blink_full_solid_trig_name,
>>  				    &psy->charging_blink_full_solid_trig);
>> +	led_trigger_register_simple(psy->charging_orange_full_green_trig_name,
>> +				    &psy->charging_orange_full_green_trig);
>>  
>>  	return 0;
>>  
>> +charging_red_full_green_failed:
>> +	kfree(psy->charging_blink_full_solid_trig_name);
>>  charging_blink_full_solid_failed:
>>  	kfree(psy->full_trig_name);
>>  full_failed:
>> @@ -101,10 +125,12 @@ static void power_supply_remove_bat_triggers(struct power_supply *psy)
>>  	led_trigger_unregister_simple(psy->charging_trig);
>>  	led_trigger_unregister_simple(psy->full_trig);
>>  	led_trigger_unregister_simple(psy->charging_blink_full_solid_trig);
>> +	led_trigger_unregister_simple(psy->charging_orange_full_green_trig);
>>  	kfree(psy->charging_blink_full_solid_trig_name);
>>  	kfree(psy->full_trig_name);
>>  	kfree(psy->charging_trig_name);
>>  	kfree(psy->charging_full_trig_name);
>> +	kfree(psy->charging_orange_full_green_trig_name);
>>  }
>>  
>>  /* Generated power specific LEDs triggers. */
>> diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
>> index c0992a77feea..9b6898085224 100644
>> --- a/include/linux/power_supply.h
>> +++ b/include/linux/power_supply.h
>> @@ -318,6 +318,8 @@ struct power_supply {
>>  	char *online_trig_name;
>>  	struct led_trigger *charging_blink_full_solid_trig;
>>  	char *charging_blink_full_solid_trig_name;
>> +	struct led_trigger *charging_orange_full_green_trig;
>> +	char *charging_orange_full_green_trig_name;
>>  #endif
>>  };
>>  
>> -- 
>> 2.44.0
>>
>>

Re: [PATCH v6 4/5] power: supply: power-supply-leds: Add charging_orange_full_green trigger for RGB LED

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 928 bytes --]

Hi,

On Fri, Apr 19, 2024 at 09:37:56AM +0200, Hans de Goede wrote:
> On 4/18/24 2:34 PM, Sebastian Reichel wrote:
> > On Tue, Apr 16, 2024 at 01:39:08PM +0800, Kate Hsuan wrote:
> >> Add a charging_orange_full_green LED trigger and the trigger is based on
> >> led_mc_trigger_event() which can set an RGB LED when the trigger is
> >> triggered. The LED will show orange when the battery status is charging.
> >> The LED will show green when the battery status is full.
> >>
> >> Link: https://lore.kernel.org/linux-leds/f40a0b1a-ceac-e269-c2dd-0158c5b4a1ad@gmail.com/
> >>
> >> Signed-off-by: Kate Hsuan <hpa@redhat.com>
> >> ---
> > 
> > Acked-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> 
> Thanks. Does this mean your ok with routing this change through the LED tree
> together with the 2 LED core patches adding the new led_mc_trigger_event()
> function this uses ?

Yes.

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]