diff mbox series

[v3,04/11] leds: aw200xx: calculate dts property display_rows in the driver

Message ID 20231101142445.8753-5-ddrokosov@salutedevices.com
State New
Headers
Series leds: aw200xx: several driver updates |

Commit Message

Dmitry Rokosov Nov. 1, 2023, 2:24 p.m. UTC 
  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
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 <gnstark@salutedevices.com>
Signed-off-by: Dmitry Rokosov <ddrokosov@salutedevices.com>
---
 drivers/leds/leds-aw200xx.c | 35 +++++++++++++++++++++++------------
 1 file changed, 23 insertions(+), 12 deletions(-)

Comments

Andy Shevchenko Nov. 18, 2023, 4:46 p.m. UTC | #1 
On Wed, Nov 1, 2023 at 4:24 PM Dmitry Rokosov
<ddrokosov@salutedevices.com> 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
> 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;

?

> +}
diff mbox series

Patch

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);