| Message ID | 20231121202835.28152-1-ddrokosov@salutedevices.com |
|---|---|
| Headers | show |
| Series | leds: aw200xx: several driver updates | expand |
On Tue, 21 Nov 2023, Dmitry Rokosov wrote: > HWEN is hardware control, which is used for enable/disable aw200xx chip. > It's high active, internally pulled down to GND. > > After HWEN pin set high the chip begins to load the OTP information, > which takes 200us to complete. About 200us wait time is needed for > internal oscillator startup and display SRAM initialization. After > display SRAM initialization, the registers in page 1 to page 5 can be > configured via i2c interface. > > Signed-off-by: Dmitry Rokosov <ddrokosov@salutedevices.com> > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com> > --- > drivers/leds/leds-aw200xx.c | 33 +++++++++++++++++++++++++++++++++ > 1 file changed, 33 insertions(+) > > diff --git a/drivers/leds/leds-aw200xx.c b/drivers/leds/leds-aw200xx.c > index 842a22087b16..7762b3a132ac 100644 > --- a/drivers/leds/leds-aw200xx.c > +++ b/drivers/leds/leds-aw200xx.c > @@ -10,6 +10,7 @@ > #include <linux/bitfield.h> > #include <linux/bits.h> > #include <linux/container_of.h> > +#include <linux/gpio/consumer.h> > #include <linux/i2c.h> > #include <linux/leds.h> > #include <linux/mod_devicetable.h> > @@ -116,6 +117,7 @@ struct aw200xx { > struct mutex mutex; > u32 num_leds; > u32 display_rows; > + struct gpio_desc *hwen; > struct aw200xx_led leds[] __counted_by(num_leds); > }; > > @@ -358,6 +360,25 @@ static int aw200xx_chip_check(const struct aw200xx *const chip) > return 0; > } > > +static void aw200xx_enable(const struct aw200xx *const chip) > +{ > + gpiod_set_value_cansleep(chip->hwen, 1); > + > + /* > + * After HWEN pin set high the chip begins to load the OTP information, > + * which takes 200us to complete. About 200us wait time is needed for > + * internal oscillator startup and display SRAM initialization. After > + * display SRAM initialization, the registers in page1 to page5 can be > + * configured via i2c interface. > + */ > + fsleep(400); > +} > + > +static void aw200xx_disable(const struct aw200xx *const chip) > +{ > + return gpiod_set_value_cansleep(chip->hwen, 0); > +} > + > static int aw200xx_probe_fw(struct device *dev, struct aw200xx *chip) > { > struct fwnode_handle *child; > @@ -517,6 +538,14 @@ static int aw200xx_probe(struct i2c_client *client) > if (IS_ERR(chip->regmap)) > return PTR_ERR(chip->regmap); > > + chip->hwen = devm_gpiod_get_optional(&client->dev, "enable", > + GPIOD_OUT_HIGH); > + if (IS_ERR(chip->hwen)) > + return dev_err_probe(&client->dev, PTR_ERR(chip->hwen), > + "Cannot get enable gpio"); Nit: "GPIO" > + > + aw200xx_enable(chip); > + > ret = aw200xx_chip_check(chip); > if (ret) > return ret; > @@ -537,6 +566,9 @@ static int aw200xx_probe(struct i2c_client *client) > ret = aw200xx_chip_init(chip); > > out_unlock: > + if (ret) > + aw200xx_disable(chip); > + > mutex_unlock(&chip->mutex); > return ret; > } > @@ -546,6 +578,7 @@ static void aw200xx_remove(struct i2c_client *client) > struct aw200xx *chip = i2c_get_clientdata(client); > > aw200xx_chip_reset(chip); > + aw200xx_disable(chip); > mutex_destroy(&chip->mutex); > } > > -- > 2.36.0 >
On Tue, 21 Nov 2023, Dmitry Rokosov wrote: > From: George Stark <gnstark@salutedevices.com> > > Get rid of device tree property "awinic,display-rows". The property > value actually means number of current switches and depends on how leds Nit: LEDs > are connected to the device. It should be calculated manually by max > used led number. In the same way it is computed automatically now. As above - I won't mention this again. > Max used led is taken from led definition subnodes. > > Signed-off-by: George Stark <gnstark@salutedevices.com> > Signed-off-by: Dmitry Rokosov <ddrokosov@salutedevices.com> > --- > drivers/leds/leds-aw200xx.c | 39 +++++++++++++++++++++++++------------ > 1 file changed, 27 insertions(+), 12 deletions(-) > > diff --git a/drivers/leds/leds-aw200xx.c b/drivers/leds/leds-aw200xx.c > index 7762b3a132ac..4bce5e7381c0 100644 > --- a/drivers/leds/leds-aw200xx.c > +++ b/drivers/leds/leds-aw200xx.c > @@ -379,6 +379,30 @@ static void aw200xx_disable(const struct aw200xx *const chip) > return gpiod_set_value_cansleep(chip->hwen, 0); > } > > +static bool aw200xx_probe_get_display_rows(struct device *dev, struct aw200xx *chip) > +{ > + struct fwnode_handle *child; > + u32 max_source = 0; > + > + device_for_each_child_node(dev, child) { > + u32 source; > + int ret; > + > + ret = fwnode_property_read_u32(child, "reg", &source); > + if (ret || source >= chip->cdef->channels) Shouldn't the second clause fail instantly? > + continue; > + > + max_source = max(max_source, source); > + } > + > + if (!max_source) Since max_source is an integer, please use an '== 0' comparison. > + return false; > + > + chip->display_rows = max_source / chip->cdef->display_size_columns + 1; > + > + return true; > +} > + > static int aw200xx_probe_fw(struct device *dev, struct aw200xx *chip) > { > struct fwnode_handle *child; > @@ -386,18 +410,9 @@ static int aw200xx_probe_fw(struct device *dev, struct aw200xx *chip) > int ret; > int i; > > - ret = device_property_read_u32(dev, "awinic,display-rows", > - &chip->display_rows); > - if (ret) > - return dev_err_probe(dev, ret, > - "Failed to read 'display-rows' property\n"); > - > - if (!chip->display_rows || > - chip->display_rows > chip->cdef->display_size_rows_max) { > - return dev_err_probe(dev, ret, > - "Invalid leds display size %u\n", > - chip->display_rows); > - } > + if (!aw200xx_probe_get_display_rows(dev, chip)) Function calls in side if() statements in general is rough. Please break it out and use 'ret' as we usually do. > + return dev_err_probe(dev, -EINVAL, Make this the return value from aw200xx_probe_get_display_rows() and use 'ret' instead. > + "No valid led definitions found\n"); > > current_max = aw200xx_imax_from_global(chip, AW200XX_IMAX_MAX_uA); > current_min = aw200xx_imax_from_global(chip, AW200XX_IMAX_MIN_uA); > -- > 2.36.0 >
On Tue, 21 Nov 2023, Dmitry Rokosov wrote: > From: George Stark <gnstark@salutedevices.com> > > According to datasheets of aw200xx devices software reset takes at > least 1ms so add delay after reset before issuing commands to device. Are you able to use an auto-correct tool to sharpen the grammar a little? > Signed-off-by: George Stark <gnstark@salutedevices.com> > Signed-off-by: Dmitry Rokosov <ddrokosov@salutedevices.com> > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com> > --- > drivers/leds/leds-aw200xx.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/leds/leds-aw200xx.c b/drivers/leds/leds-aw200xx.c > index 4bce5e7381c0..bb17e48b3e2a 100644 > --- a/drivers/leds/leds-aw200xx.c > +++ b/drivers/leds/leds-aw200xx.c > @@ -321,6 +321,9 @@ static int aw200xx_chip_reset(const struct aw200xx *const chip) > if (ret) > return ret; > > + /* according to datasheet software reset takes at least 1ms */ Please start sentences with an uppercase char. "According to the datasheet, software resets take at least 1ms" ^ ^ ^ > + fsleep(1000); > + > regcache_mark_dirty(chip->regmap); > return regmap_write(chip->regmap, AW200XX_REG_FCD, AW200XX_FCD_CLEAR); > } > -- > 2.36.0 >
On Tue, 21 Nov 2023, Dmitry Rokosov wrote: > From: George Stark <gnstark@salutedevices.com> > > Add support for Awinic aw20108 device from the same LED drivers family. > New device supports 108 LEDs using a matrix of 12x9 outputs. > > Signed-off-by: George Stark <gnstark@salutedevices.com> > Signed-off-by: Dmitry Rokosov <ddrokosov@salutedevices.com> > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com> > --- > drivers/leds/Kconfig | 14 +++++++++----- > drivers/leds/leds-aw200xx.c | 10 +++++++++- > 2 files changed, 18 insertions(+), 6 deletions(-) > > diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig > index 6046dfeca16f..a879628e985c 100644 > --- a/drivers/leds/Kconfig > +++ b/drivers/leds/Kconfig > @@ -95,14 +95,18 @@ config LEDS_ARIEL > Say Y to if your machine is a Dell Wyse 3020 thin client. > > config LEDS_AW200XX > - tristate "LED support for Awinic AW20036/AW20054/AW20072" > + tristate "LED support for Awinic AW20036/AW20054/AW20072/AW20108" > depends on LEDS_CLASS > depends on I2C > help > - This option enables support for the AW20036/AW20054/AW20072 LED driver. > - It is a 3x12/6x9/6x12 matrix LED driver programmed via > - an I2C interface, up to 36/54/72 LEDs or 12/18/24 RGBs, > - 3 pattern controllers for auto breathing or group dimming control. > + This option enables support for Awinic AW200XX LED controller. "for ..." THE or AN. Or put an 's' at the end of "controller". > + It is a matrix LED driver programmed via an I2C interface. Devices have > + a set of individually controlled leds and support 3 pattern controllers LEDs > + for auto breathing or group dimming control. Supported devices: > + - AW20036 (3x12) 36 LEDs > + - AW20054 (6x9) 54 LEDs > + - AW20072 (6x12) 72 LEDs > + - AW20108 (9x12) 108 LEDs > > To compile this driver as a module, choose M here: the module > will be called leds-aw200xx. > diff --git a/drivers/leds/leds-aw200xx.c b/drivers/leds/leds-aw200xx.c > index c48aa11fd411..4b5036360887 100644 > --- a/drivers/leds/leds-aw200xx.c > +++ b/drivers/leds/leds-aw200xx.c > @@ -1,6 +1,6 @@ > // SPDX-License-Identifier: GPL-2.0 > /* > - * Awinic AW20036/AW20054/AW20072 LED driver > + * Awinic AW20036/AW20054/AW20072/AW20108 LED driver > * > * Copyright (c) 2023, SberDevices. All Rights Reserved. > * > @@ -620,10 +620,17 @@ static const struct aw200xx_chipdef aw20072_cdef = { > .display_size_columns = 12, > }; > > +static const struct aw200xx_chipdef aw20108_cdef = { > + .channels = 108, > + .display_size_rows_max = 9, > + .display_size_columns = 12, > +}; > + > static const struct i2c_device_id aw200xx_id[] = { > { "aw20036" }, > { "aw20054" }, > { "aw20072" }, > + { "aw20108" }, > {} > }; > MODULE_DEVICE_TABLE(i2c, aw200xx_id); > @@ -632,6 +639,7 @@ static const struct of_device_id aw200xx_match_table[] = { > { .compatible = "awinic,aw20036", .data = &aw20036_cdef, }, > { .compatible = "awinic,aw20054", .data = &aw20054_cdef, }, > { .compatible = "awinic,aw20072", .data = &aw20072_cdef, }, > + { .compatible = "awinic,aw20108", .data = &aw20108_cdef, }, > {} > }; > MODULE_DEVICE_TABLE(of, aw200xx_match_table); > -- > 2.36.0 >
Hello Lee, Thank you for the detailed review! Please find my answer below. On Thu, Nov 23, 2023 at 04:38:16PM +0000, Lee Jones wrote: > On Tue, 21 Nov 2023, Dmitry Rokosov wrote: > > > From: George Stark <gnstark@salutedevices.com> > > > > According to datasheets of aw200xx devices software reset takes at > > least 1ms so add delay after reset before issuing commands to device. > > Are you able to use an auto-correct tool to sharpen the grammar a little? > > > Signed-off-by: George Stark <gnstark@salutedevices.com> > > Signed-off-by: Dmitry Rokosov <ddrokosov@salutedevices.com> > > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com> > > --- > > drivers/leds/leds-aw200xx.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/drivers/leds/leds-aw200xx.c b/drivers/leds/leds-aw200xx.c > > index 4bce5e7381c0..bb17e48b3e2a 100644 > > --- a/drivers/leds/leds-aw200xx.c > > +++ b/drivers/leds/leds-aw200xx.c > > @@ -321,6 +321,9 @@ static int aw200xx_chip_reset(const struct aw200xx *const chip) > > if (ret) > > return ret; > > > > + /* according to datasheet software reset takes at least 1ms */ > > Please start sentences with an uppercase char. > > "According to the datasheet, software resets take at least 1ms" > ^ ^ ^ > Here it's only one 'software reset' mentioned. > > + fsleep(1000); > > + > > regcache_mark_dirty(chip->regmap); > > return regmap_write(chip->regmap, AW200XX_REG_FCD, AW200XX_FCD_CLEAR); > > } > > -- > > 2.36.0 > > > > -- > Lee Jones [李琼斯]
On Thu, Nov 23, 2023 at 04:32:52PM +0000, Lee Jones wrote: > On Tue, 21 Nov 2023, Dmitry Rokosov wrote: > > > From: George Stark <gnstark@salutedevices.com> > > > > Get rid of device tree property "awinic,display-rows". The property > > value actually means number of current switches and depends on how leds > > Nit: LEDs > > > are connected to the device. It should be calculated manually by max > > used led number. In the same way it is computed automatically now. > > As above - I won't mention this again. > > > Max used led is taken from led definition subnodes. > > > > Signed-off-by: George Stark <gnstark@salutedevices.com> > > Signed-off-by: Dmitry Rokosov <ddrokosov@salutedevices.com> > > --- > > drivers/leds/leds-aw200xx.c | 39 +++++++++++++++++++++++++------------ > > 1 file changed, 27 insertions(+), 12 deletions(-) > > > > diff --git a/drivers/leds/leds-aw200xx.c b/drivers/leds/leds-aw200xx.c > > index 7762b3a132ac..4bce5e7381c0 100644 > > --- a/drivers/leds/leds-aw200xx.c > > +++ b/drivers/leds/leds-aw200xx.c > > @@ -379,6 +379,30 @@ static void aw200xx_disable(const struct aw200xx *const chip) > > return gpiod_set_value_cansleep(chip->hwen, 0); > > } > > > > +static bool aw200xx_probe_get_display_rows(struct device *dev, struct aw200xx *chip) > > +{ > > + struct fwnode_handle *child; > > + u32 max_source = 0; > > + > > + device_for_each_child_node(dev, child) { > > + u32 source; > > + int ret; > > + > > + ret = fwnode_property_read_u32(child, "reg", &source); > > + if (ret || source >= chip->cdef->channels) > > Shouldn't the second clause fail instantly? > We already have such logic in the aw200xx_probe_fw() function, which skips the LED node with the wrong reg value too. Furthermore, we have strict reg constraints in the dt-bindings parts (in the current patch series), so we assume that the DT developer will not create an LED with the wrong reg value. > > + continue; > > + > > + max_source = max(max_source, source); > > + } > > + > > + if (!max_source) > > Since max_source is an integer, please use an '== 0' comparison. > Okay > > + return false; > > + > > + chip->display_rows = max_source / chip->cdef->display_size_columns + 1; > > + > > + return true; > > +} > > + > > static int aw200xx_probe_fw(struct device *dev, struct aw200xx *chip) > > { > > struct fwnode_handle *child; > > @@ -386,18 +410,9 @@ static int aw200xx_probe_fw(struct device *dev, struct aw200xx *chip) > > int ret; > > int i; > > > > - ret = device_property_read_u32(dev, "awinic,display-rows", > > - &chip->display_rows); > > - if (ret) > > - return dev_err_probe(dev, ret, > > - "Failed to read 'display-rows' property\n"); > > - > > - if (!chip->display_rows || > > - chip->display_rows > chip->cdef->display_size_rows_max) { > > - return dev_err_probe(dev, ret, > > - "Invalid leds display size %u\n", > > - chip->display_rows); > > - } > > + if (!aw200xx_probe_get_display_rows(dev, chip)) > > Function calls in side if() statements in general is rough. > > Please break it out and use 'ret' as we usually do. > > > + return dev_err_probe(dev, -EINVAL, > > Make this the return value from aw200xx_probe_get_display_rows() and use > 'ret' instead. > No problem, I'll prepare a new version. > > + "No valid led definitions found\n"); > > > > current_max = aw200xx_imax_from_global(chip, AW200XX_IMAX_MAX_uA); > > current_min = aw200xx_imax_from_global(chip, AW200XX_IMAX_MIN_uA); > > -- > > 2.36.0 > > > > -- > Lee Jones [李琼斯]
On Fri, 24 Nov 2023, Dmitry Rokosov wrote: > On Thu, Nov 23, 2023 at 04:32:52PM +0000, Lee Jones wrote: > > On Tue, 21 Nov 2023, Dmitry Rokosov wrote: > > > > > From: George Stark <gnstark@salutedevices.com> > > > > > > Get rid of device tree property "awinic,display-rows". The property > > > value actually means number of current switches and depends on how leds > > > > Nit: LEDs > > > > > are connected to the device. It should be calculated manually by max > > > used led number. In the same way it is computed automatically now. > > > > As above - I won't mention this again. > > > > > Max used led is taken from led definition subnodes. > > > > > > Signed-off-by: George Stark <gnstark@salutedevices.com> > > > Signed-off-by: Dmitry Rokosov <ddrokosov@salutedevices.com> > > > --- > > > drivers/leds/leds-aw200xx.c | 39 +++++++++++++++++++++++++------------ > > > 1 file changed, 27 insertions(+), 12 deletions(-) > > > > > > diff --git a/drivers/leds/leds-aw200xx.c b/drivers/leds/leds-aw200xx.c > > > index 7762b3a132ac..4bce5e7381c0 100644 > > > --- a/drivers/leds/leds-aw200xx.c > > > +++ b/drivers/leds/leds-aw200xx.c > > > @@ -379,6 +379,30 @@ static void aw200xx_disable(const struct aw200xx *const chip) > > > return gpiod_set_value_cansleep(chip->hwen, 0); > > > } > > > > > > +static bool aw200xx_probe_get_display_rows(struct device *dev, struct aw200xx *chip) > > > +{ > > > + struct fwnode_handle *child; > > > + u32 max_source = 0; > > > + > > > + device_for_each_child_node(dev, child) { > > > + u32 source; > > > + int ret; > > > + > > > + ret = fwnode_property_read_u32(child, "reg", &source); > > > + if (ret || source >= chip->cdef->channels) > > > > Shouldn't the second clause fail instantly? > > > > We already have such logic in the aw200xx_probe_fw() function, which > skips the LED node with the wrong reg value too. Furthermore, we have > strict reg constraints in the dt-bindings parts (in the current patch > series), so we assume that the DT developer will not create an LED with > the wrong reg value. Why is it being checked again then?
On Fri, 24 Nov 2023, Dmitry Rokosov wrote: > Hello Lee, > > Thank you for the detailed review! > > Please find my answer below. > > On Thu, Nov 23, 2023 at 04:38:16PM +0000, Lee Jones wrote: > > On Tue, 21 Nov 2023, Dmitry Rokosov wrote: > > > > > From: George Stark <gnstark@salutedevices.com> > > > > > > According to datasheets of aw200xx devices software reset takes at > > > least 1ms so add delay after reset before issuing commands to device. > > > > Are you able to use an auto-correct tool to sharpen the grammar a little? > > > > > Signed-off-by: George Stark <gnstark@salutedevices.com> > > > Signed-off-by: Dmitry Rokosov <ddrokosov@salutedevices.com> > > > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com> > > > --- > > > drivers/leds/leds-aw200xx.c | 3 +++ > > > 1 file changed, 3 insertions(+) > > > > > > diff --git a/drivers/leds/leds-aw200xx.c b/drivers/leds/leds-aw200xx.c > > > index 4bce5e7381c0..bb17e48b3e2a 100644 > > > --- a/drivers/leds/leds-aw200xx.c > > > +++ b/drivers/leds/leds-aw200xx.c > > > @@ -321,6 +321,9 @@ static int aw200xx_chip_reset(const struct aw200xx *const chip) > > > if (ret) > > > return ret; > > > > > > + /* according to datasheet software reset takes at least 1ms */ > > > > Please start sentences with an uppercase char. > > > > "According to the datasheet, software resets take at least 1ms" > > ^ ^ ^ > > > > Here it's only one 'software reset' mentioned. That's okay. The English is still 100% valid, since this describes them happening more than once; say per week, per year, per lifetime of the H/W or some such. If you *really* want to describe one reset happening once, ever, then you can say "a software reset takes". > > > + fsleep(1000); > > > + > > > regcache_mark_dirty(chip->regmap); > > > return regmap_write(chip->regmap, AW200XX_REG_FCD, AW200XX_FCD_CLEAR); > > > }
Lee, Thank you for the quick reply! On Mon, Nov 27, 2023 at 08:57:55AM +0000, Lee Jones wrote: > On Fri, 24 Nov 2023, Dmitry Rokosov wrote: > > > On Thu, Nov 23, 2023 at 04:32:52PM +0000, Lee Jones wrote: > > > On Tue, 21 Nov 2023, Dmitry Rokosov wrote: > > > > > > > From: George Stark <gnstark@salutedevices.com> > > > > > > > > Get rid of device tree property "awinic,display-rows". The property > > > > value actually means number of current switches and depends on how leds > > > > > > Nit: LEDs > > > > > > > are connected to the device. It should be calculated manually by max > > > > used led number. In the same way it is computed automatically now. > > > > > > As above - I won't mention this again. > > > > > > > Max used led is taken from led definition subnodes. > > > > > > > > Signed-off-by: George Stark <gnstark@salutedevices.com> > > > > Signed-off-by: Dmitry Rokosov <ddrokosov@salutedevices.com> > > > > --- > > > > drivers/leds/leds-aw200xx.c | 39 +++++++++++++++++++++++++------------ > > > > 1 file changed, 27 insertions(+), 12 deletions(-) > > > > > > > > diff --git a/drivers/leds/leds-aw200xx.c b/drivers/leds/leds-aw200xx.c > > > > index 7762b3a132ac..4bce5e7381c0 100644 > > > > --- a/drivers/leds/leds-aw200xx.c > > > > +++ b/drivers/leds/leds-aw200xx.c > > > > @@ -379,6 +379,30 @@ static void aw200xx_disable(const struct aw200xx *const chip) > > > > return gpiod_set_value_cansleep(chip->hwen, 0); > > > > } > > > > > > > > +static bool aw200xx_probe_get_display_rows(struct device *dev, struct aw200xx *chip) > > > > +{ > > > > + struct fwnode_handle *child; > > > > + u32 max_source = 0; > > > > + > > > > + device_for_each_child_node(dev, child) { > > > > + u32 source; > > > > + int ret; > > > > + > > > > + ret = fwnode_property_read_u32(child, "reg", &source); > > > > + if (ret || source >= chip->cdef->channels) > > > > > > Shouldn't the second clause fail instantly? > > > > > > > We already have such logic in the aw200xx_probe_fw() function, which > > skips the LED node with the wrong reg value too. Furthermore, we have > > strict reg constraints in the dt-bindings parts (in the current patch > > series), so we assume that the DT developer will not create an LED with > > the wrong reg value. > > Why is it being checked again then? Hmmm, aw200xx_probe_get_display_rows() executes before the old implementation... So we need to check it again. Do you think it should be reworked? I've already sent a new patchset. Could you please take a look at the other fixes?
The following patch series includes several updates for the AW200XX LED driver: - some small fixes and optimizations to the driver implementation: delays, autodimming calculation, disable_locking regmap flag, display_rows calculation in runtime; - fix LED device tree node pattern to accept LED names counting not only from 0 to f; - add missing reg constraints; - support HWEN hardware control, which allows enabling or disabling AW200XX RTL logic from the main SoC using a GPIO pin; - introduce the new AW20108 LED controller, the datasheet for this controller can be found at [1]. Changes v4 since v3 at [4] - properly handle max_source = 0 situations - fix Rob's dt_binding_check alerts Changes v3 since v2 at [3]: - handle all cases during hwen gpio get routine execution - rename 'hwen-gpios' to standard 'enable-gpios' - properly handle aw200xx_probe_get_display_rows() ret values - fix timestamp format in the comments and commit messages - expand LEDS_AW200XX config and dt-bindings description - describe reg constraints for all compatible variants - add Conor's Acked-by tag Changes v2 since v1 at [2]: - rebase on the latest aw200xx changes from lee/leds git repo - some commit messages rewording - replace legacy gpio_* API with gpiod_* and devm_gpiod_* API - rename dt property awinic,hwen-gpio to hwen-gpios according to gpiod API - use fsleep() instead of usleep_range() per Andy's suggestion - add max_brightness parameter to led cdev to restrict set_brightness() overflow - provide reg constraints as Rob suggested - move hwen-gpios to proper dt node in the bindings example Links: [1] https://doc.awinic.com/doc/20230609wm/8a9a9ac8-1d8f-4e75-bf7a-67a04465c153.pdf [2] https://lore.kernel.org/all/20231006160437.15627-1-ddrokosov@salutedevices.com/ [3] https://lore.kernel.org/all/20231018182943.18700-1-ddrokosov@salutedevices.com/ [4] https://lore.kernel.org/all/20231101142445.8753-1-ddrokosov@salutedevices.com/ Dmitry Rokosov (3): leds: aw200xx: support HWEN hardware control dt-bindings: leds: aw200xx: introduce optional enable-gpios property dt-bindings: leds: aw200xx: fix led pattern and add reg constraints George Stark (7): leds: aw200xx: calculate dts property display_rows in the driver dt-bindings: leds: aw200xx: remove property "awinic,display-rows" leds: aw200xx: add delay after software reset leds: aw200xx: enable disable_locking flag in regmap config leds: aw200xx: improve autodim calculation method leds: aw200xx: add support for aw20108 device dt-bindings: leds: awinic,aw200xx: add AW20108 device Martin Kurbanov (1): leds: aw200xx: fix write to DIM parameter .../bindings/leds/awinic,aw200xx.yaml | 95 ++++++++++++----- drivers/leds/Kconfig | 14 ++- drivers/leds/leds-aw200xx.c | 100 +++++++++++++++--- 3 files changed, 163 insertions(+), 46 deletions(-)