| Message ID | 20240207061006.407066-3-hpa@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | KTD2026 indicator LED for X86 Xiaomi Pad2 | expand |
Hi, Am Mittwoch, dem 07.02.2024 um 14:10 +0800 schrieb Kate Hsuan: > This LED controller also 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 device tree so this work would affect the original would -> won't ? > implementations. > > Signed-off-by: Kate Hsuan <hpa@redhat.com> > --- > drivers/leds/rgb/Kconfig | 1 - > drivers/leds/rgb/leds-ktd202x.c | 68 ++++++++++++++++++++++--------- > -- > 2 files changed, 45 insertions(+), 24 deletions(-) > > diff --git a/drivers/leds/rgb/Kconfig b/drivers/leds/rgb/Kconfig > index a6a21f564673..f245dbd9a163 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..a1aef62e3db5 100644 > --- a/drivers/leds/rgb/leds-ktd202x.c > +++ b/drivers/leds/rgb/leds-ktd202x.c > @@ -112,6 +112,8 @@ static int ktd202x_chip_disable(struct ktd202x > *chip) > > regmap_write(chip->regmap, KTD202X_REG_RESET_CONTROL, > KTD202X_ENABLE_CTRL_SLEEP); > > + usleep_range(500, 600); Why have you added the sleep? > + > ret = regulator_bulk_disable(ARRAY_SIZE(chip->regulators), > chip->regulators); > if (ret) { > dev_err(chip->dev, "Failed to disable regulators: > %d\n", ret); > @@ -381,16 +383,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 *np, > struct ktd202x_led *led, struct > led_init_data *init_data) > { > struct led_classdev *cdev; > - struct device_node *child; > + struct fwnode_handle *child; Nit, swap the above lines for reverse Christmas tree. > 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(np, child) { > + num_channels++; > + } > if (!num_channels || num_channels > chip->num_leds) > return -EINVAL; > > @@ -398,22 +402,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(np, child) { > u32 mono_color; > u32 reg; > int ret; > > - ret = of_property_read_u32(child, "reg", ®); > + ret = fwnode_property_read_u32(child, "reg", ®); > if (ret != 0 || reg >= chip->num_leds) { > dev_err(chip->dev, "invalid 'reg' of > %pOFn\n", child); > - of_node_put(child); > + fwnode_handle_put(child); > return -EINVAL; > } > > - 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); > + fwnode_handle_put(child); > return ret; > } > > @@ -433,14 +437,14 @@ 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 *np, > 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", ®); > + ret = fwnode_property_read_u32(np, "reg", ®); > if (ret != 0 || reg >= chip->num_leds) { > dev_err(chip->dev, "invalid 'reg' of %pOFn\n", np); > return -EINVAL; > @@ -454,7 +458,7 @@ 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 *np, unsigned int index) > { > struct ktd202x_led *led = &chip->leds[index]; > struct led_init_data init_data = {}; > @@ -463,14 +467,14 @@ 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(np, "color", &color); > if (ret < 0 && ret != -EINVAL) { > dev_err(chip->dev, "failed to parse 'color' of > %pOF\n", np); > return ret; > } > > led->chip = chip; > - init_data.fwnode = of_fwnode_handle(np); > + init_data.fwnode = np; > > if (color == LED_COLOR_ID_RGB) { > cdev = &led->mcdev.led_cdev; > @@ -492,26 +496,30 @@ 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; > - int count; > + struct device *dev = chip->dev; > + struct fwnode_handle *np, *child; Nit, swap the above lines. > + int count = 0; Un-needed init. > int i = 0; > > - chip->num_leds = (int)(unsigned > long)of_device_get_match_data(chip->dev); > + count = device_get_child_node_count(dev); > > - count = of_get_available_child_count(np); > if (!count || count > chip->num_leds) > return -EINVAL; > > + np = dev_fwnode(chip->dev); > + if (!np) > + return -ENODEV; > + > 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(np, child) { > int ret = ktd202x_add_led(chip, child, i); > > if (ret) { > - of_node_put(child); > + fwnode_handle_put(child); > return ret; > } > i++; > @@ -533,7 +541,7 @@ static int ktd202x_probe(struct i2c_client > *client) > { > struct device *dev = &client->dev; > struct ktd202x *chip; > - int count; > + int count = 0; Un-needed init. > int ret; > > count = device_get_child_node_count(dev); > @@ -568,6 +576,8 @@ static int ktd202x_probe(struct i2c_client > *client) > return ret; > } > > + chip->num_leds = (int) (unsigned > long)i2c_get_match_data(client); > + > ret = ktd202x_probe_dt(chip); > if (ret < 0) { > regulator_bulk_disable(ARRAY_SIZE(chip->regulators), > chip->regulators); > @@ -602,21 +612,33 @@ 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); > + > +#ifndef CONFIG_ACPI > 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); > +#endif > > static struct i2c_driver ktd202x_driver = { > .driver = { > - .name = "leds-ktd202x", > - .of_match_table = ktd202x_match_table, > + .name = "leds-ktd202x", Why was the space after "name" replaced by tab? > +#ifndef CONFIG_ACPI > + .of_match_table = ktd20xx_match_table, Typo: ktd20xx_match_table -> ktd202x_match_table André > +#endif > }, > .probe = ktd202x_probe, > .remove = ktd202x_remove, > .shutdown = ktd202x_shutdown, > + .id_table = ktd202x_id, > }; > module_i2c_driver(ktd202x_driver); >
Hi, Thank you for your review. Sorry for the late reply. I came back from holiday. On Thu, Feb 8, 2024 at 5:03 AM André Apitzsch <git@apitzsch.eu> wrote: > > Hi, > > Am Mittwoch, dem 07.02.2024 um 14:10 +0800 schrieb Kate Hsuan: > > This LED controller also 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 device tree so this work would affect the original > would -> won't ? won't > > > implementations. > > > > Signed-off-by: Kate Hsuan <hpa@redhat.com> > > --- > > drivers/leds/rgb/Kconfig | 1 - > > drivers/leds/rgb/leds-ktd202x.c | 68 ++++++++++++++++++++++--------- > > -- > > 2 files changed, 45 insertions(+), 24 deletions(-) > > > > diff --git a/drivers/leds/rgb/Kconfig b/drivers/leds/rgb/Kconfig > > index a6a21f564673..f245dbd9a163 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..a1aef62e3db5 100644 > > --- a/drivers/leds/rgb/leds-ktd202x.c > > +++ b/drivers/leds/rgb/leds-ktd202x.c > > @@ -112,6 +112,8 @@ static int ktd202x_chip_disable(struct ktd202x > > *chip) > > > > regmap_write(chip->regmap, KTD202X_REG_RESET_CONTROL, > > KTD202X_ENABLE_CTRL_SLEEP); > > > > + usleep_range(500, 600); > Why have you added the sleep? When removing the module, a WARN_ON() shown as the following URL will be triggered. It happens randomly. I tried to mitigate it through a sleep. https://elixir.bootlin.com/linux/latest/source/drivers/regulator/core.c#L2396 > > > + > > ret = regulator_bulk_disable(ARRAY_SIZE(chip->regulators), > > chip->regulators); > > if (ret) { > > dev_err(chip->dev, "Failed to disable regulators: > > %d\n", ret); > > @@ -381,16 +383,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 *np, > > struct ktd202x_led *led, struct > > led_init_data *init_data) > > { > > struct led_classdev *cdev; > > - struct device_node *child; > > + struct fwnode_handle *child; > Nit, swap the above lines for reverse Christmas tree. okay > > > 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(np, child) { > > + num_channels++; > > + } > > if (!num_channels || num_channels > chip->num_leds) > > return -EINVAL; > > > > @@ -398,22 +402,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(np, child) { > > u32 mono_color; > > u32 reg; > > int ret; > > > > - ret = of_property_read_u32(child, "reg", ®); > > + ret = fwnode_property_read_u32(child, "reg", ®); > > if (ret != 0 || reg >= chip->num_leds) { > > dev_err(chip->dev, "invalid 'reg' of > > %pOFn\n", child); > > - of_node_put(child); > > + fwnode_handle_put(child); > > return -EINVAL; > > } > > > > - 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); > > + fwnode_handle_put(child); > > return ret; > > } > > > > @@ -433,14 +437,14 @@ 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 *np, > > 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", ®); > > + ret = fwnode_property_read_u32(np, "reg", ®); > > if (ret != 0 || reg >= chip->num_leds) { > > dev_err(chip->dev, "invalid 'reg' of %pOFn\n", np); > > return -EINVAL; > > @@ -454,7 +458,7 @@ 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 *np, unsigned int index) > > { > > struct ktd202x_led *led = &chip->leds[index]; > > struct led_init_data init_data = {}; > > @@ -463,14 +467,14 @@ 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(np, "color", &color); > > if (ret < 0 && ret != -EINVAL) { > > dev_err(chip->dev, "failed to parse 'color' of > > %pOF\n", np); > > return ret; > > } > > > > led->chip = chip; > > - init_data.fwnode = of_fwnode_handle(np); > > + init_data.fwnode = np; > > > > if (color == LED_COLOR_ID_RGB) { > > cdev = &led->mcdev.led_cdev; > > @@ -492,26 +496,30 @@ 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; > > - int count; > > + struct device *dev = chip->dev; > > + struct fwnode_handle *np, *child; > Nit, swap the above lines. > > > + int count = 0; > Un-needed init. Okay > > > int i = 0; > > > > - chip->num_leds = (int)(unsigned > > long)of_device_get_match_data(chip->dev); > > + count = device_get_child_node_count(dev); > > > > - count = of_get_available_child_count(np); > > if (!count || count > chip->num_leds) > > return -EINVAL; > > > > + np = dev_fwnode(chip->dev); > > + if (!np) > > + return -ENODEV; > > + > > 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(np, child) { > > int ret = ktd202x_add_led(chip, child, i); > > > > if (ret) { > > - of_node_put(child); > > + fwnode_handle_put(child); > > return ret; > > } > > i++; > > @@ -533,7 +541,7 @@ static int ktd202x_probe(struct i2c_client > > *client) > > { > > struct device *dev = &client->dev; > > struct ktd202x *chip; > > - int count; > > + int count = 0; > Un-needed init. Okay. > > > int ret; > > > > count = device_get_child_node_count(dev); > > @@ -568,6 +576,8 @@ static int ktd202x_probe(struct i2c_client > > *client) > > return ret; > > } > > > > + chip->num_leds = (int) (unsigned > > long)i2c_get_match_data(client); > > + > > ret = ktd202x_probe_dt(chip); > > if (ret < 0) { > > regulator_bulk_disable(ARRAY_SIZE(chip->regulators), > > chip->regulators); > > @@ -602,21 +612,33 @@ 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); > > + > > +#ifndef CONFIG_ACPI > > 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); > > +#endif > > > > static struct i2c_driver ktd202x_driver = { > > .driver = { > > - .name = "leds-ktd202x", > > - .of_match_table = ktd202x_match_table, > > + .name = "leds-ktd202x", > Why was the space after "name" replaced by tab? I'll remove it. > > > +#ifndef CONFIG_ACPI > > + .of_match_table = ktd20xx_match_table, > Typo: ktd20xx_match_table -> ktd202x_match_table Ops. I'll fix it. > > André > > > +#endif > > }, > > .probe = ktd202x_probe, > > .remove = ktd202x_remove, > > .shutdown = ktd202x_shutdown, > > + .id_table = ktd202x_id, > > }; > > module_i2c_driver(ktd202x_driver); > > > I'll propose a v2 patch to improve the code according to your suggestions.
diff --git a/drivers/leds/rgb/Kconfig b/drivers/leds/rgb/Kconfig index a6a21f564673..f245dbd9a163 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..a1aef62e3db5 100644 --- a/drivers/leds/rgb/leds-ktd202x.c +++ b/drivers/leds/rgb/leds-ktd202x.c @@ -112,6 +112,8 @@ static int ktd202x_chip_disable(struct ktd202x *chip) regmap_write(chip->regmap, KTD202X_REG_RESET_CONTROL, KTD202X_ENABLE_CTRL_SLEEP); + usleep_range(500, 600); + ret = regulator_bulk_disable(ARRAY_SIZE(chip->regulators), chip->regulators); if (ret) { dev_err(chip->dev, "Failed to disable regulators: %d\n", ret); @@ -381,16 +383,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 *np, struct ktd202x_led *led, struct led_init_data *init_data) { struct led_classdev *cdev; - struct device_node *child; + struct fwnode_handle *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(np, child) { + num_channels++; + } if (!num_channels || num_channels > chip->num_leds) return -EINVAL; @@ -398,22 +402,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(np, child) { u32 mono_color; u32 reg; int ret; - ret = of_property_read_u32(child, "reg", ®); + ret = fwnode_property_read_u32(child, "reg", ®); if (ret != 0 || reg >= chip->num_leds) { dev_err(chip->dev, "invalid 'reg' of %pOFn\n", child); - of_node_put(child); + fwnode_handle_put(child); return -EINVAL; } - 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); + fwnode_handle_put(child); return ret; } @@ -433,14 +437,14 @@ 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 *np, 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", ®); + ret = fwnode_property_read_u32(np, "reg", ®); if (ret != 0 || reg >= chip->num_leds) { dev_err(chip->dev, "invalid 'reg' of %pOFn\n", np); return -EINVAL; @@ -454,7 +458,7 @@ 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 *np, unsigned int index) { struct ktd202x_led *led = &chip->leds[index]; struct led_init_data init_data = {}; @@ -463,14 +467,14 @@ 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(np, "color", &color); if (ret < 0 && ret != -EINVAL) { dev_err(chip->dev, "failed to parse 'color' of %pOF\n", np); return ret; } led->chip = chip; - init_data.fwnode = of_fwnode_handle(np); + init_data.fwnode = np; if (color == LED_COLOR_ID_RGB) { cdev = &led->mcdev.led_cdev; @@ -492,26 +496,30 @@ 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; - int count; + struct device *dev = chip->dev; + struct fwnode_handle *np, *child; + int count = 0; int i = 0; - chip->num_leds = (int)(unsigned long)of_device_get_match_data(chip->dev); + count = device_get_child_node_count(dev); - count = of_get_available_child_count(np); if (!count || count > chip->num_leds) return -EINVAL; + np = dev_fwnode(chip->dev); + if (!np) + return -ENODEV; + 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(np, child) { int ret = ktd202x_add_led(chip, child, i); if (ret) { - of_node_put(child); + fwnode_handle_put(child); return ret; } i++; @@ -533,7 +541,7 @@ static int ktd202x_probe(struct i2c_client *client) { struct device *dev = &client->dev; struct ktd202x *chip; - int count; + int count = 0; int ret; count = device_get_child_node_count(dev); @@ -568,6 +576,8 @@ static int ktd202x_probe(struct i2c_client *client) return ret; } + chip->num_leds = (int) (unsigned long)i2c_get_match_data(client); + ret = ktd202x_probe_dt(chip); if (ret < 0) { regulator_bulk_disable(ARRAY_SIZE(chip->regulators), chip->regulators); @@ -602,21 +612,33 @@ 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); + +#ifndef CONFIG_ACPI 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); +#endif static struct i2c_driver ktd202x_driver = { .driver = { - .name = "leds-ktd202x", - .of_match_table = ktd202x_match_table, + .name = "leds-ktd202x", +#ifndef CONFIG_ACPI + .of_match_table = ktd20xx_match_table, +#endif }, .probe = ktd202x_probe, .remove = ktd202x_remove, .shutdown = ktd202x_shutdown, + .id_table = ktd202x_id, }; module_i2c_driver(ktd202x_driver);
This LED controller also 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 device tree so this work would affect the original implementations. Signed-off-by: Kate Hsuan <hpa@redhat.com> --- drivers/leds/rgb/Kconfig | 1 - drivers/leds/rgb/leds-ktd202x.c | 68 ++++++++++++++++++++++----------- 2 files changed, 45 insertions(+), 24 deletions(-)