Messages in this thread | | | From | Andy Shevchenko <> | Date | Tue, 16 Apr 2024 20:29:05 +0300 | Subject | Re: [PATCH v6 2/5] leds: rgb: leds-ktd202x: Get device properties through fwnode to support ACPI |
| |
On Tue, Apr 16, 2024 at 8:39 AM Kate Hsuan <hpa@redhat.com> wrote: > > This LED controller is installed on a Xiaomi pad2 and it is an x86 > platform. The original driver is based on the device tree and can't be > used for this ACPI based system. This patch migrated the driver to use > fwnode to access the properties. Moreover, the fwnode API supports the > device tree so this work won't affect the original implementations.
..
> - int num_channels; > + int num_channels = 0;
Split this assignment, so...
> int i = 0;
> - num_channels = of_get_available_child_count(np);
..it become
num_channels = 0;
here.
> + fwnode_for_each_available_child_node(fwnode, child) > + num_channels++;
..
> -static int ktd202x_add_led(struct ktd202x *chip, struct device_node *np, unsigned int index) > +static int ktd202x_add_led(struct ktd202x *chip, > + struct fwnode_handle *fwnode, > + unsigned int index)
Why split over 3 lines? I believe it can be still two or one (depending if you use a relaxed limit).
..
> static int ktd202x_probe_dt(struct ktd202x *chip)
Perhaps you want to rename this to something like ktd202x_probe_fw().
..
> + fwnode = dev_fwnode(dev);
Will be no use if the bellow applied, right?
..
> - for_each_available_child_of_node(np, child) { > + fwnode_for_each_available_child_node(fwnode, child) {
Use device_for_each_child_node() instead.
> }
..
> - .shutdown = ktd202x_shutdown, > + .shutdown = ktd202x_shutdown
Stray change.
-- With Best Regards, Andy Shevchenko
| |