On Mon, Mar 20, 2023 at 05:30:05PM +0100, Michal Kubiak wrote:
> 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"
>
Oh god... I added the extra comment and totally missed the other.
Sorry!
Btw ok for the description of the LED mapping? It's a bit complex so
tried to do my best to describe them.
--
Ansuel
