Hi!I will revise it.
The device has the ability to group LED output into control banksInversely?
so that multiple LED banks can be controlled with the same mixing and
brightness. Inversely the LEDs can also be controlled independently.
OK.
--- /dev/nullCan we get https here and in the binding document?
+++ b/drivers/leds/leds-lp50xx.c
@@ -0,0 +1,784 @@
+// SPDX-License-Identifier: GPL-2.0
+// TI LP50XX LED chip family driver
+// Copyright (C) 2018 Texas Instruments Incorporated - http://www.ti.com/
+
Please run this through checkpatch -- I believe it will have some
comments.
+is handle_put(child) needed here?
+ device_for_each_child_node(priv->dev, child) {
+ led = &priv->leds[i];
+ ret = fwnode_property_count_u32(child, "reg");
+ if (ret < 0) {
+ dev_err(&priv->client->dev,
+ "reg property is invalid\n");
+ return -EINVAL;
+ }Move this to subfunction to reduce the indentation? (Or, just refactor
+ if (ret > 1) {
+ priv->num_of_banked_leds = ret;
+ if (priv->num_of_banked_leds >
+ priv->chip_info->max_modules) {
+ dev_err(&priv->client->dev,
+ "reg property is invalid\n");
+ ret = -EINVAL;
+ fwnode_handle_put(child);
+ goto child_out;
+ }
+
+ ret = fwnode_property_read_u32_array(child,
+ "reg",
+ led_banks,
+ ret);
it somehow).
I will rework it for all+ if (ret) {Create label that does the handle_put so you don't need to repeat it
+ dev_err(&priv->client->dev,
+ "reg property is missing\n");
+ fwnode_handle_put(child);
+ goto child_out;
+ }
quite so often?
OK needs to goto to out.
+ fwnode_for_each_child_node(child, led_node) {This uses undefined value.
+ ret = fwnode_property_read_u32(led_node, "color",
+ &color_id);
+ if (ret)
+ dev_err(priv->dev, "Cannot read color\n");
+
+ mc_led_info[num_colors].color_index = color_id;
+ ret = lp50xx_reset(led);Does the GPIO need to be disabled before enabling it for reset?
Best regards,
Pavel