Re: [PATCH RFC net-next 8/8] dsa: qca8k: Use DSA common code for LEDs

[Christian Marangi <ansuelsmth@gmail.com>]

On Wed, Nov 29, 2023 at 03:16:16AM +0100, Andrew Lunn wrote:
> > Hi,
> > 
> > I attached a fixup patch for this to correctly work. Since we are using
> > port_num the thing needs to be decrememted by one.
> 
> I thought this might happen.
> 
> How about this fix instead? It fits better with the naming of
> parameters, and just does the offset once for each API function.
>

Yep also works. My idea was to act on the function that parse the
port_num and gives regs and mask to prevent problem in the future but I
don't think it will change that much.

With your proposed fix,

Reviewed-by: Christian Marangi <ansuelsmth@gmail.com>

> 
> From 0789b95345bfa5086365051f95531fdb3d053e3e Mon Sep 17 00:00:00 2001
> From: Andrew Lunn <andrew@lunn.ch>
> Date: Tue, 28 Nov 2023 20:11:42 -0600
> Subject: [PATCH] dsa: qca8k: Fix off-by-one for LEDs.
> 
> ---
>  drivers/net/dsa/qca/qca8k-leds.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/dsa/qca/qca8k-leds.c b/drivers/net/dsa/qca/qca8k-leds.c
> index febae23b65a9..0aa209b84251 100644
> --- a/drivers/net/dsa/qca/qca8k-leds.c
> +++ b/drivers/net/dsa/qca/qca8k-leds.c
> @@ -80,9 +80,10 @@ qca8k_parse_netdev(unsigned long rules, u32 *offload_trigger)
>  }
>  
>  int
> -qca8k_led_brightness_set(struct dsa_switch *ds, int port_num,
> +qca8k_led_brightness_set(struct dsa_switch *ds, int port,
>  			 u8 led_num, enum led_brightness brightness)
>  {
> +	int port_num = qca8k_port_to_phy(port);
>  	struct qca8k_led_pattern_en reg_info;
>  	struct qca8k_priv *priv = ds->priv;
>  	u32 mask, val;
> @@ -140,11 +141,12 @@ qca8k_led_brightness_set(struct dsa_switch *ds, int port_num,
>  }
>  
>  int
> -qca8k_led_blink_set(struct dsa_switch *ds, int port_num, u8 led_num,
> +qca8k_led_blink_set(struct dsa_switch *ds, int port, u8 led_num,
>  		    unsigned long *delay_on,
>  		    unsigned long *delay_off)
>  {
>  	u32 mask, val = QCA8K_LED_ALWAYS_BLINK_4HZ;
> +	int port_num = qca8k_port_to_phy(port);
>  	struct qca8k_led_pattern_en reg_info;
>  	struct qca8k_priv *priv = ds->priv;
>  
> @@ -231,9 +233,10 @@ qca8k_led_hw_control_is_supported(struct dsa_switch *ds,
>  }
>  
>  int
> -qca8k_led_hw_control_set(struct dsa_switch *ds, int port_num, u8 led_num,
> +qca8k_led_hw_control_set(struct dsa_switch *ds, int port, u8 led_num,
>  			 unsigned long rules)
>  {
> +	int port_num = qca8k_port_to_phy(port);
>  	struct qca8k_led_pattern_en reg_info;
>  	struct qca8k_priv *priv = ds->priv;
>  	u32 offload_trigger = 0;
> @@ -255,9 +258,10 @@ qca8k_led_hw_control_set(struct dsa_switch *ds, int port_num, u8 led_num,
>  }
>  
>  int
> -qca8k_led_hw_control_get(struct dsa_switch *ds, int port_num, u8 led_num,
> +qca8k_led_hw_control_get(struct dsa_switch *ds, int port, u8 led_num,
>  			 unsigned long *rules)
>  {
> +	int port_num = qca8k_port_to_phy(port);
>  	struct qca8k_led_pattern_en reg_info;
>  	struct qca8k_priv *priv = ds->priv;
>  	u32 val;
> -- 
> 2.42.0
> 

-- 
	Ansuel