Re: [net-next PATCH v5 02/15] net: dsa: qca8k: add LEDs basic support

[Michal Kubiak]

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"