| Message ID | 20240306025801.8814-2-hpa@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | KTD2026 indicator LED for X86 Xiaomi Pad2 | expand |
Wed, Mar 06, 2024 at 10:58:00AM +0800, Kate Hsuan kirjoitti: > There is a KTD2026 LED controller to manage the indicator LED for Xiaomi > pad2. The ACPI for it is not properly made so the kernel can't get > a correct description of it. > > This work add a description for this RGB LED controller and also set a > trigger to indicate the chaging event (bq27520-0-charging). When it is > charging, the indicator LED will be turn on. Ilpo, Kate, please consider the following remarks. ... > #include <linux/gpio/machine.h> > #include <linux/input.h> > #include <linux/platform_device.h> + Blank line? > +#include <dt-bindings/leds/common.h> Not sure where to place this, some drivers put it the first, some after linux/*. ... > +/* main fwnode for ktd2026 */ > +static const struct software_node ktd2026_node = { No name? > +}; > + > +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"), It's less than 80, why not on a single line? > + Redundant blank line. > + { } > +}; ... > +/* B */ B for red?! > +static const struct property_entry ktd2026_red_led_props[] = { > + PROPERTY_ENTRY_U32("reg", 0), > + PROPERTY_ENTRY_U32("color", LED_COLOR_ID_BLUE), > + { } > +}; ... > +/* R */ R for blue?! > +static const struct property_entry ktd2026_blue_led_props[] = { > + PROPERTY_ENTRY_U32("reg", 2), > + PROPERTY_ENTRY_U32("color", LED_COLOR_ID_RED), > + { } > +}; ... > +static int __init xiaomi_mipad2_init(void) > +{ > + return software_node_register_node_group(ktd2026_node_group); > +} > + > +static void xiaomi_mipad2_exit(void) Don't you want to have __exit here? > +{ > + software_node_unregister_node_group(ktd2026_node_group); > +}
On Tue, 12 Mar 2024, Andy Shevchenko wrote: > Wed, Mar 06, 2024 at 10:58:00AM +0800, Kate Hsuan kirjoitti: > > There is a KTD2026 LED controller to manage the indicator LED for Xiaomi > > pad2. The ACPI for it is not properly made so the kernel can't get > > a correct description of it. > > > > This work add a description for this RGB LED controller and also set a > > trigger to indicate the chaging event (bq27520-0-charging). When it is > > charging, the indicator LED will be turn on. > > Ilpo, Kate, please consider the following remarks. > > ... > > > #include <linux/gpio/machine.h> > > #include <linux/input.h> > > #include <linux/platform_device.h> > > + Blank line? > > > +#include <dt-bindings/leds/common.h> > > Not sure where to place this, some drivers put it the first, some after linux/*. > > ... > > > +/* main fwnode for ktd2026 */ > > +static const struct software_node ktd2026_node = { > > No name? > > > +}; > > + > > +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"), > > It's less than 80, why not on a single line? > > > + > > Redundant blank line. > > > + { } > > +}; > > ... > > > +/* B */ > > B for red?! > > > +static const struct property_entry ktd2026_red_led_props[] = { > > + PROPERTY_ENTRY_U32("reg", 0), > > + PROPERTY_ENTRY_U32("color", LED_COLOR_ID_BLUE), > > + { } > > +}; The name with "red" and LED_COLOR_ID_BLUE are also inconsistent. > ... > > > +/* R */ > > R for blue?! > > > +static const struct property_entry ktd2026_blue_led_props[] = { > > + PROPERTY_ENTRY_U32("reg", 2), > > + PROPERTY_ENTRY_U32("color", LED_COLOR_ID_RED), > > + { } > > +}; Here as well. I think it's better I drop this patch (it's only in review-ilpo) and wait for v5 version as there's some much still to correct.
On Tue, Mar 12, 2024 at 6:35 PM Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> wrote: > > On Tue, 12 Mar 2024, Andy Shevchenko wrote: > > > Wed, Mar 06, 2024 at 10:58:00AM +0800, Kate Hsuan kirjoitti: > > > There is a KTD2026 LED controller to manage the indicator LED for Xiaomi > > > pad2. The ACPI for it is not properly made so the kernel can't get > > > a correct description of it. > > > > > > This work add a description for this RGB LED controller and also set a > > > trigger to indicate the chaging event (bq27520-0-charging). When it is > > > charging, the indicator LED will be turn on. > > > > Ilpo, Kate, please consider the following remarks. > > > > ... > > > > > #include <linux/gpio/machine.h> > > > #include <linux/input.h> > > > #include <linux/platform_device.h> > > > > + Blank line? > > > > > +#include <dt-bindings/leds/common.h> > > > > Not sure where to place this, some drivers put it the first, some after linux/*. > > > > ... > > > > > +/* main fwnode for ktd2026 */ > > > +static const struct software_node ktd2026_node = { > > > > No name? > > > > > +}; > > > + > > > +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"), > > > > It's less than 80, why not on a single line? > > > > > + > > > > Redundant blank line. > > > > > + { } > > > +}; > > > > ... > > > > > +/* B */ > > > > B for red?! > > > > > +static const struct property_entry ktd2026_red_led_props[] = { > > > + PROPERTY_ENTRY_U32("reg", 0), > > > + PROPERTY_ENTRY_U32("color", LED_COLOR_ID_BLUE), > > > + { } > > > +}; > > The name with "red" and LED_COLOR_ID_BLUE are also inconsistent. > > > ... > > > > > +/* R */ > > > > R for blue?! > > > > > +static const struct property_entry ktd2026_blue_led_props[] = { > > > + PROPERTY_ENTRY_U32("reg", 2), > > > + PROPERTY_ENTRY_U32("color", LED_COLOR_ID_RED), > > > + { } > > > +}; > > Here as well. > > I think it's better I drop this patch (it's only in review-ilpo) and wait > for v5 version as there's some much still to correct. > Hi, Thank you for reviewing it. I'll propose the v5 patch to improve them :) > > -- > i. >
diff --git a/drivers/platform/x86/x86-android-tablets/other.c b/drivers/platform/x86/x86-android-tablets/other.c index bc6bbf7ec6ea..542ef6667b7b 100644 --- a/drivers/platform/x86/x86-android-tablets/other.c +++ b/drivers/platform/x86/x86-android-tablets/other.c @@ -12,6 +12,7 @@ #include <linux/gpio/machine.h> #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 +594,87 @@ 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 (B/G/R). 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 = { +}; + +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, +}; + +/* B */ +static const struct property_entry ktd2026_red_led_props[] = { + PROPERTY_ENTRY_U32("reg", 0), + PROPERTY_ENTRY_U32("color", LED_COLOR_ID_BLUE), + { } +}; + +static const struct software_node ktd2026_red_led_node = { + .properties = ktd2026_red_led_props, + .parent = &ktd2026_rgb_led_node, +}; + +/* G */ +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, +}; + +/* R */ +static const struct property_entry ktd2026_blue_led_props[] = { + PROPERTY_ENTRY_U32("reg", 2), + PROPERTY_ENTRY_U32("color", LED_COLOR_ID_RED), + { } +}; + +static const struct software_node ktd2026_blue_led_node = { + .properties = ktd2026_blue_led_props, + .parent = &ktd2026_rgb_led_node, +}; + +static const struct software_node *ktd2026_node_group[] = { + &ktd2026_node, + &ktd2026_rgb_led_node, + &ktd2026_red_led_node, + &ktd2026_green_led_node, + &ktd2026_blue_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 +698,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 +707,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
There is a KTD2026 LED controller to manage the indicator LED for Xiaomi pad2. The ACPI for it is not properly made so the kernel can't get a correct description of it. This work add a description for this RGB LED controller and also set a trigger to indicate the chaging event (bq27520-0-charging). When it is charging, the indicator LED will be turn on. Signed-off-by: Kate Hsuan <hpa@redhat.com> --- .../platform/x86/x86-android-tablets/other.c | 85 +++++++++++++++++++ .../x86/x86-android-tablets/shared-psy-info.h | 2 + 2 files changed, 87 insertions(+)