Hi Lee Jones, Am Donnerstag, dem 28.09.2023 um 15:42 +0100 schrieb Lee Jones: > On Sat, 23 Sep 2023, André Apitzsch wrote: > > > This commit adds support for Kinetic KTD2026/7 RGB/White LED > > driver. > > > > Signed-off-by: André Apitzsch <git@xxxxxxxxxxx> > > --- > > drivers/leds/rgb/Kconfig | 13 + > > drivers/leds/rgb/Makefile | 1 + > > drivers/leds/rgb/leds-ktd202x.c | 625 > > ++++++++++++++++++++++++++++++++++++++++ > > 3 files changed, 639 insertions(+) > > > > diff --git a/drivers/leds/rgb/Kconfig b/drivers/leds/rgb/Kconfig > > index 183bccc06cf3..a6a21f564673 100644 > > --- a/drivers/leds/rgb/Kconfig > > +++ b/drivers/leds/rgb/Kconfig > > @@ -14,6 +14,19 @@ config LEDS_GROUP_MULTICOLOR > > To compile this driver as a module, choose M here: the > > module > > will be called 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 > > + RGB/White LED driver found in different BQ mobile phones. > > + It is a 3 or 4 channel LED driver programmed via an I2C > > interface. > > + > > + To compile this driver as a module, choose M here: the > > module > > + will be called leds-ktd202x. > > + > > config LEDS_PWM_MULTICOLOR > > tristate "PWM driven multi-color LED Support" > > depends on PWM > > diff --git a/drivers/leds/rgb/Makefile b/drivers/leds/rgb/Makefile > > index c11cc56384e7..243f31e4d70d 100644 > > --- a/drivers/leds/rgb/Makefile > > +++ b/drivers/leds/rgb/Makefile > > @@ -1,6 +1,7 @@ > > # SPDX-License-Identifier: GPL-2.0 > > > > obj-$(CONFIG_LEDS_GROUP_MULTICOLOR) += leds-group-multicolor.o > > +obj-$(CONFIG_LEDS_KTD202X) += leds-ktd202x.o > > obj-$(CONFIG_LEDS_PWM_MULTICOLOR) += leds-pwm-multicolor.o > > obj-$(CONFIG_LEDS_QCOM_LPG) += leds-qcom-lpg.o > > obj-$(CONFIG_LEDS_MT6370_RGB) += leds-mt6370-rgb.o > > diff --git a/drivers/leds/rgb/leds-ktd202x.c > > b/drivers/leds/rgb/leds-ktd202x.c > > new file mode 100644 > > index 000000000000..b328ecd34664 > > --- /dev/null > > +++ b/drivers/leds/rgb/leds-ktd202x.c > > @@ -0,0 +1,625 @@ > > +// SPDX-License-Identifier: GPL-2.0-or-later > > +/* > > + * Kinetic KTD2026/7 RGB/White LED driver with I2C interface > > + * > > + * Copyright 2023 André Apitzsch <git@xxxxxxxxxxx> > > + * > > + * Datasheet: https://www.kinet-ic.com/uploads/KTD2026-7-04h.pdf > > + */ > > + > > [..] > > + > > +static int ktd202x_probe_dt(struct ktd202x *chip) > > +{ > > + struct device_node *np = dev_of_node(chip->dev), *child; > > + unsigned int i; > > + int count, ret; > > + > > + chip->num_leds = (int)(unsigned long)of_device_get_match_data(chip->dev); > > There are a bunch of recent patches converting these to: > > i2c_get_match_data() > > ... is that also applicable here? > I don't think there is a benefit in using it here. i2c_get_match_data() requires a "struct i2c_client client", so we would have to pass that to ktd202x_probe_dt(). Only to replace chip->num_leds = (int)(unsigned long)of_device_get_match_data(chip->dev); by chip->num_leds = (int)(unsigned long)i2c_get_match_data(client); Best regards, André > > + count = of_get_available_child_count(np); > > + if (!count || count > chip->num_leds) > > + return -EINVAL; > > + > > + regmap_write(chip->regmap, KTD202X_REG_RESET_CONTROL, > > + KTD202X_RSTR_RESET); > > + > > + /* Allow the device to execute the complete reset */ > > + usleep_range(200, 300); > > + > > + i = 0; > > + for_each_available_child_of_node(np, child) { > > + ret = ktd202x_add_led(chip, child, i); > > + if (ret) > > + return ret; > > + i++; > > + } > > + > > + return 0; > > +} > > + > > +static const struct regmap_config ktd202x_regmap_config = { > > + .reg_bits = 8, > > + .val_bits = 8, > > + .max_register = 0x09, > > + .cache_type = REGCACHE_FLAT, > > + .reg_defaults = ktd202x_reg_defaults, > > + .num_reg_defaults = ARRAY_SIZE(ktd202x_reg_defaults), > > +}; > > + > > +static int ktd202x_probe(struct i2c_client *client) > > +{ > > + struct device *dev = &client->dev; > > + struct ktd202x *chip; > > + int count; > > + int ret; > > + > > + count = device_get_child_node_count(dev); > > + if (!count || count > KTD202X_MAX_LEDS) > > + return dev_err_probe(dev, -EINVAL, > > + "Incorrect number of leds > > (%d)", count); > > + > > + chip = devm_kzalloc(dev, struct_size(chip, leds, count), > > GFP_KERNEL); > > + if (!chip) > > + return -ENOMEM; > > + > > + mutex_init(&chip->mutex); > > + > > + chip->dev = dev; > > + i2c_set_clientdata(client, chip); > > + > > + chip->regmap = devm_regmap_init_i2c(client, > > &ktd202x_regmap_config); > > + if (IS_ERR(chip->regmap)) { > > + ret = dev_err_probe(dev, PTR_ERR(chip->regmap), > > + "Failed to allocate register > > map.\n"); > > + goto error; > > Where is the mutex first used? > > I'd suggest moving mutex_init() as late as possible and omitting as > many > of these gotos as you can. > > > + } > > + > > + chip->regulators[0].supply = "vin"; > > + chip->regulators[1].supply = "vio"; > > + ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(chip- > > >regulators), > > + chip->regulators); > > + if (ret < 0) { > > + dev_err_probe(dev, ret, "Failed to request > > regulators.\n"); > > + goto error; > > + } > > + > > + ret = regulator_bulk_enable(ARRAY_SIZE(chip->regulators), > > + chip->regulators); > > + if (ret) { > > + dev_err_probe(dev, ret, "Failed to enable > > regulators.\n"); > > + goto error; > > + } > > + > > + ret = ktd202x_probe_dt(chip); > > + if (ret < 0) > > + goto error_reg; > > + > > + ret = regulator_bulk_disable(ARRAY_SIZE(chip->regulators), > > + chip->regulators); > > + if (ret) { > > + dev_err_probe(dev, ret, "Failed to disable > > regulators.\n"); > > + goto error; > > + } > > + > > + return 0; > > + > > +error_reg: > > + regulator_bulk_disable(ARRAY_SIZE(chip->regulators), > > + chip->regulators); > > + > > +error: > > + mutex_destroy(&chip->mutex); > > + return ret; > > +} > > + > > +static void ktd202x_remove(struct i2c_client *client) > > +{ > > + struct ktd202x *chip = i2c_get_clientdata(client); > > + > > + ktd202x_chip_disable(chip); > > + > > + mutex_destroy(&chip->mutex); > > +} > > + > > +static void ktd202x_shutdown(struct i2c_client *client) > > +{ > > + struct ktd202x *chip = i2c_get_clientdata(client); > > + > > + /* Reset registers to make sure all off before shutdown */ > > Grammar. > > > + regmap_write(chip->regmap, KTD202X_REG_RESET_CONTROL, > > + KTD202X_RSTR_RESET); > > +} > > + > > +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 }, > > + {}, > > +}; > > + > > Remove these line. > > > +MODULE_DEVICE_TABLE(of, ktd202x_match_table); > > + > > +static struct i2c_driver ktd202x_driver = { > > + .driver = { > > + .name = "leds-ktd202x", > > + .of_match_table = ktd202x_match_table, > > + }, > > + .probe = ktd202x_probe, > > + .remove = ktd202x_remove, > > + .shutdown = ktd202x_shutdown, > > +}; > > +module_i2c_driver(ktd202x_driver); > > + > > +MODULE_AUTHOR("André Apitzsch <git@xxxxxxxxxxx>"); > > +MODULE_DESCRIPTION("Kinetic KTD2026/7 LED driver"); > > +MODULE_LICENSE("GPL"); > > > > -- > > 2.42.0 > > >
