2023-11-01 14:26:45

by Dmitry Rokosov

[permalink] [raw]
Subject: [PATCH v3 00/11] leds: aw200xx: several driver updates

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 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/[email protected]/
[3] https://lore.kernel.org/all/[email protected]/

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 | 100 +++++++++++++-----
drivers/leds/Kconfig | 14 ++-
drivers/leds/leds-aw200xx.c | 96 ++++++++++++++---
3 files changed, 159 insertions(+), 51 deletions(-)

--
2.36.0


2023-11-01 14:27:45

by Dmitry Rokosov

[permalink] [raw]
Subject: [PATCH v3 04/11] leds: aw200xx: calculate dts property display_rows in the driver

From: George Stark <[email protected]>

Get rid of device tree property "awinic,display-rows". The property
value actually means number of current switches and depends on how 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.
Max used led is taken from led definition subnodes.

Signed-off-by: George Stark <[email protected]>
Signed-off-by: Dmitry Rokosov <[email protected]>
---
drivers/leds/leds-aw200xx.c | 35 +++++++++++++++++++++++------------
1 file changed, 23 insertions(+), 12 deletions(-)

diff --git a/drivers/leds/leds-aw200xx.c b/drivers/leds/leds-aw200xx.c
index 7762b3a132ac..aab8898b0330 100644
--- a/drivers/leds/leds-aw200xx.c
+++ b/drivers/leds/leds-aw200xx.c
@@ -379,6 +379,26 @@ 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)
+ continue;
+
+ max_source = max(max_source, source);
+ }
+
+ chip->display_rows = max_source / chip->cdef->display_size_columns + 1;
+ return !!chip->display_rows;
+}
+
static int aw200xx_probe_fw(struct device *dev, struct aw200xx *chip)
{
struct fwnode_handle *child;
@@ -386,18 +406,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))
+ return dev_err_probe(dev, -EINVAL,
+ "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

2023-11-18 16:47:34

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v3 04/11] leds: aw200xx: calculate dts property display_rows in the driver

On Wed, Nov 1, 2023 at 4:24 PM Dmitry Rokosov
<[email protected]> wrote:
>
> From: George Stark <[email protected]>
>
> Get rid of device tree property "awinic,display-rows". The property
> value actually means number of current switches and depends on how 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.
> Max used led is taken from led definition subnodes.

...

> +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)
> + continue;
> +
> + max_source = max(max_source, source);
> + }

> + chip->display_rows = max_source / chip->cdef->display_size_columns + 1;
> + return !!chip->display_rows;

This is a bit weird. Can we rewrite it as

if (max_source == 0)
return false;

->display_rows = ...
return true;

?

> +}


--
With Best Regards,
Andy Shevchenko

2023-11-18 16:58:48

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v3 00/11] leds: aw200xx: several driver updates

On Wed, Nov 1, 2023 at 4:24 PM Dmitry Rokosov
<[email protected]> wrote:
>
> 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].

For non device tree binding patches
Reviewed-by: Andy Shevchenko <[email protected]>
One nit I commented on the individual patch.

--
With Best Regards,
Andy Shevchenko