Re: [PATCH v2 04/11] leds: aw200xx: calculate dts property display_rows in driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hello Andy

On 10/19/23 12:01, Andy Shevchenko wrote:
On Wed, Oct 18, 2023 at 9:30 PM Dmitry Rokosov
<ddrokosov@xxxxxxxxxxxxxxxxx> wrote:

From: George Stark <gnstark@xxxxxxxxxxxxxxxxx>

Get rid of device tree property "awinic,display-rows" and calculate it
in driver using led definition nodes. display-row actually means number
of current switches and depends on how leds are connected to the device.

Still the commit message does not answer the question why it's safe
for the users that have this property enabled in their DTBs (note B
letter).



...

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

Perhaps a warning?

     dev_warn(dev, "Unable to read from %pfw or apply a source channel
number\n", child);

I skipped the warning intentionally because we have just the same loop several steps below and with the same check. There we have all proper warnings and aw200xx_probe_get_display_rows was intended to be short and lightweight. I'll redesign it to be even more simple.


+                       continue;
+
+               max_source = max(max_source, source);
+       }

...

+       chip->display_rows = max_source / chip->cdef->display_size_columns + 1;
+       if (!chip->display_rows) {
+               dev_err(dev, "No valid led definitions found\n");
+               return -EINVAL;

So, this part is in ->probe() flow only, correct? If so,
   return dev_err_probe(...);

+       }

...

+       if (aw200xx_probe_get_display_rows(dev, chip))
+               return -EINVAL;

Why is the error code shadowed?


--
Best regards
George



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux