Messages in this thread | ![/](/images/icornerl.gif) | | From | Kate Hsuan <> | Date | Thu, 18 Apr 2024 15:22:59 +0800 | Subject | Re: [PATCH v6 2/5] leds: rgb: leds-ktd202x: Get device properties through fwnode to support ACPI |
| |
Hi Andy,
Thank you for reviewing.
On Wed, Apr 17, 2024 at 1:29 AM Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > > 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++; >
It will look like this: num_channels = 0; 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).
Make it to be one line.
> > ... > > > static int ktd202x_probe_dt(struct ktd202x *chip) > > Perhaps you want to rename this to something like ktd202x_probe_fw().
Sounds good.
> > ... > > > + fwnode = dev_fwnode(dev); > > Will be no use if the bellow applied, right?
Right. It can be dropped.
> > ... > > > - for_each_available_child_of_node(np, child) { > > + fwnode_for_each_available_child_node(fwnode, child) { > > Use device_for_each_child_node() instead.
Okay.
> > > } > > ... > > > - .shutdown = ktd202x_shutdown, > > + .shutdown = ktd202x_shutdown > > Stray change.
I know the reason :)
> > -- > With Best Regards, > Andy Shevchenko >
-- BR, Kate
| ![\](/images/icornerr.gif) |