diff mbox series

[net] net: dsa: microchip: ksz8795: Correctly handle huge frame configuration

Message ID 43107d9e8b5b8b05f0cbd4e1f47a2bb88c8747b2.1681755535.git.christophe.jaillet@wanadoo.fr
State New
Headers
Series [net] net: dsa: microchip: ksz8795: Correctly handle huge frame configuration |

Commit Message

Christophe JAILLET April 17, 2023, 6:19 p.m. UTC 
  Because of the logic in place, SW_HUGE_PACKET can never be set.
(If the first condition is true, then the 2nd one is also true, but is not
executed)

Change the logic and update each bit individually.

Fixes: 29d1e85f45e0 ("net: dsa: microchip: ksz8: add MTU configuration support")
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
Untested.
---
 drivers/net/dsa/microchip/ksz8795.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Oleksij Rempel April 18, 2023, 4:21 a.m. UTC | #1 
Hi Christophe,

On Mon, Apr 17, 2023 at 08:19:33PM +0200, Christophe JAILLET wrote:
> Because of the logic in place, SW_HUGE_PACKET can never be set.
> (If the first condition is true, then the 2nd one is also true, but is not
> executed)
> 
> Change the logic and update each bit individually.
>
> Fixes: 29d1e85f45e0 ("net: dsa: microchip: ksz8: add MTU configuration support")
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>

Reviewed-by: Oleksij Rempel <o.rempel@pengutronix.de>

Thank you!

> ---
> Untested.

I do not have access to this HW too.

> ---
>  drivers/net/dsa/microchip/ksz8795.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/dsa/microchip/ksz8795.c b/drivers/net/dsa/microchip/ksz8795.c
> index 23614a937cc3..f56fca1b1a22 100644
> --- a/drivers/net/dsa/microchip/ksz8795.c
> +++ b/drivers/net/dsa/microchip/ksz8795.c
> @@ -96,7 +96,7 @@ static int ksz8795_change_mtu(struct ksz_device *dev, int frame_size)
>  
>  	if (frame_size > KSZ8_LEGAL_PACKET_SIZE)
>  		ctrl2 |= SW_LEGAL_PACKET_DISABLE;
> -	else if (frame_size > KSZ8863_NORMAL_PACKET_SIZE)
> +	if (frame_size > KSZ8863_NORMAL_PACKET_SIZE)
>  		ctrl1 |= SW_HUGE_PACKET;
>  
>  	ret = ksz_rmw8(dev, REG_SW_CTRL_1, SW_HUGE_PACKET, ctrl1);
> -- 
> 2.34.1
> 
>
Simon Horman April 19, 2023, 3:14 p.m. UTC | #2 
On Mon, Apr 17, 2023 at 08:19:33PM +0200, Christophe JAILLET wrote:
> Because of the logic in place, SW_HUGE_PACKET can never be set.
> (If the first condition is true, then the 2nd one is also true, but is not
> executed)
> 
> Change the logic and update each bit individually.
> 
> Fixes: 29d1e85f45e0 ("net: dsa: microchip: ksz8: add MTU configuration support")
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> ---
> Untested.
> ---
>  drivers/net/dsa/microchip/ksz8795.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Simon Horman <simon.horman@corigine.com>
Vladimir Oltean April 19, 2023, 4:14 p.m. UTC | #3 
On Mon, Apr 17, 2023 at 08:19:33PM +0200, Christophe JAILLET wrote:
> Because of the logic in place, SW_HUGE_PACKET can never be set.
> (If the first condition is true, then the 2nd one is also true, but is not
> executed)
> 
> Change the logic and update each bit individually.
> 
> Fixes: 29d1e85f45e0 ("net: dsa: microchip: ksz8: add MTU configuration support")
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> ---
> Untested.
> ---
>  drivers/net/dsa/microchip/ksz8795.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/dsa/microchip/ksz8795.c b/drivers/net/dsa/microchip/ksz8795.c
> index 23614a937cc3..f56fca1b1a22 100644
> --- a/drivers/net/dsa/microchip/ksz8795.c
> +++ b/drivers/net/dsa/microchip/ksz8795.c
> @@ -96,7 +96,7 @@ static int ksz8795_change_mtu(struct ksz_device *dev, int frame_size)
>  
>  	if (frame_size > KSZ8_LEGAL_PACKET_SIZE)
>  		ctrl2 |= SW_LEGAL_PACKET_DISABLE;
> -	else if (frame_size > KSZ8863_NORMAL_PACKET_SIZE)
> +	if (frame_size > KSZ8863_NORMAL_PACKET_SIZE)
>  		ctrl1 |= SW_HUGE_PACKET;
>  
>  	ret = ksz_rmw8(dev, REG_SW_CTRL_1, SW_HUGE_PACKET, ctrl1);
> -- 
> 2.34.1
> 

After checking with the datasheet, the change looks indeed correct.

Reviewed-by: Vladimir Oltean <olteanv@gmail.com>
Florian Fainelli April 19, 2023, 5:46 p.m. UTC | #4 
On 4/17/23 11:19, Christophe JAILLET wrote:
> Because of the logic in place, SW_HUGE_PACKET can never be set.
> (If the first condition is true, then the 2nd one is also true, but is not
> executed)
> 
> Change the logic and update each bit individually.
> 
> Fixes: 29d1e85f45e0 ("net: dsa: microchip: ksz8: add MTU configuration support")
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
patchwork-bot+netdevbpf@kernel.org April 19, 2023, 9:20 p.m. UTC | #5 
Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Mon, 17 Apr 2023 20:19:33 +0200 you wrote:
> Because of the logic in place, SW_HUGE_PACKET can never be set.
> (If the first condition is true, then the 2nd one is also true, but is not
> executed)
> 
> Change the logic and update each bit individually.
> 
> Fixes: 29d1e85f45e0 ("net: dsa: microchip: ksz8: add MTU configuration support")
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> 
> [...]

Here is the summary with links:
  - [net] net: dsa: microchip: ksz8795: Correctly handle huge frame configuration
    https://git.kernel.org/netdev/net/c/3d2f8f1f184c

You are awesome, thank you!
diff mbox series

Patch

diff --git a/drivers/net/dsa/microchip/ksz8795.c b/drivers/net/dsa/microchip/ksz8795.c
index 23614a937cc3..f56fca1b1a22 100644
--- a/drivers/net/dsa/microchip/ksz8795.c
+++ b/drivers/net/dsa/microchip/ksz8795.c
@@ -96,7 +96,7 @@  static int ksz8795_change_mtu(struct ksz_device *dev, int frame_size)
 
 	if (frame_size > KSZ8_LEGAL_PACKET_SIZE)
 		ctrl2 |= SW_LEGAL_PACKET_DISABLE;
-	else if (frame_size > KSZ8863_NORMAL_PACKET_SIZE)
+	if (frame_size > KSZ8863_NORMAL_PACKET_SIZE)
 		ctrl1 |= SW_HUGE_PACKET;
 
 	ret = ksz_rmw8(dev, REG_SW_CTRL_1, SW_HUGE_PACKET, ctrl1);