On Sun, Mar 19, 2023 at 08:18:01PM +0100, Christian Marangi wrote: > Add LEDs basic support for qca8k Switch Family by adding basic > brightness_set() support. > > Since these LEDs refelect port status, the default label is set to > ":port". DT binding should describe the color, function and number of > the leds using standard LEDs api. > > These LEDs supports only blocking variant of the brightness_set() > function since they can sleep during access of the switch leds to set > the brightness. > > While at it add to the qca8k header file each mode defined by the Switch > Documentation for future use. > > Signed-off-by: Christian Marangi <ansuelsmth@xxxxxxxxx> > --- Hi Christian, The patch looks good to me. I just found one nitpick in the comment. Thanks, Michal > +static int > +qca8k_parse_port_leds(struct qca8k_priv *priv, struct fwnode_handle *port, int port_num) > +{ > + struct fwnode_handle *led = NULL, *leds = NULL; > + struct led_init_data init_data = { }; > + enum led_default_state state; > + struct qca8k_led *port_led; > + int led_num, led_index; > + int ret; > + > + leds = fwnode_get_named_child_node(port, "leds"); > + if (!leds) { > + dev_dbg(priv->dev, "No Leds node specified in device tree for port %d!\n", > + port_num); > + return 0; > + } > + > + fwnode_for_each_child_node(leds, led) { > + /* Reg represent the led number of the port. > + * Each port can have at least 3 leds attached Nitpick: "at least" -> "at most"