Whenever min(a, b) is used with a and b unsigned variables or literals,
`make W=2` complains. Change four min() calls into umin().

stderr extract (GCC 11.2.0, MIPS Codescape):

./include/linux/minmax.h:68:57: warning: comparison is always true due
                          to limited range of data type [-Wtype-limits]
   68 | #define __is_nonneg(ux) statically_true((long long)(ux) >= 0)
      |                                                         ^~
drivers/net/ethernet/cadence/macb_main.c:2299:26: note: in expansion of
                                                            macro ‘min’
 2299 |              hdrlen = min(skb_headlen(skb), bp->max_tx_length);
      |                       ^~~

Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 drivers/net/ethernet/cadence/macb_main.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 98e28d51a6e12c24ef27c939363eb43c0aec1951..6c6bc6aa23c718772b95b398e807f193a38e141a 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -2035,7 +2035,7 @@ static unsigned int macb_tx_map(struct macb *bp,
 		count++;
 		tx_head++;
 
-		size = min(len, bp->max_tx_length);
+		size = umin(len, bp->max_tx_length);
 	}
 
 	/* Then, map paged data from fragments */
@@ -2045,7 +2045,7 @@ static unsigned int macb_tx_map(struct macb *bp,
 		len = skb_frag_size(frag);
 		offset = 0;
 		while (len) {
-			size = min(len, bp->max_tx_length);
+			size = umin(len, bp->max_tx_length);
 			entry = macb_tx_ring_wrap(bp, tx_head);
 			tx_skb = &queue->tx_skb[entry];
 
@@ -2301,7 +2301,7 @@ static netdev_tx_t macb_start_xmit(struct sk_buff *skb, struct net_device *dev)
 			return NETDEV_TX_BUSY;
 		}
 	} else
-		hdrlen = min(skb_headlen(skb), bp->max_tx_length);
+		hdrlen = umin(skb_headlen(skb), bp->max_tx_length);
 
 #if defined(DEBUG) && defined(VERBOSE_DEBUG)
 	netdev_vdbg(bp->dev,
@@ -4573,8 +4573,8 @@ static int macb_init(struct platform_device *pdev)
 	 * each 4-tuple define requires 1 T2 screener reg + 3 compare regs
 	 */
 	reg = gem_readl(bp, DCFG8);
-	bp->max_tuples = min((GEM_BFEXT(SCR2CMP, reg) / 3),
-			GEM_BFEXT(T2SCR, reg));
+	bp->max_tuples = umin((GEM_BFEXT(SCR2CMP, reg) / 3),
+			      GEM_BFEXT(T2SCR, reg));
 	INIT_LIST_HEAD(&bp->rx_fs_list.list);
 	if (bp->max_tuples > 0) {
 		/* also needs one ethtype match to check IPv4 */

-- 
2.51.0

On Tue, 14 Oct 2025 17:25:12 +0200
Théo Lebrun <theo.lebrun@bootlin.com> wrote:

> Whenever min(a, b) is used with a and b unsigned variables or literals,
> `make W=2` complains. Change four min() calls into umin().

It will, and you'll get the same 'error' all over the place.
Basically -Wtype-limits is broken.

Don't remove valid checks because it bleats.

	David


> 
> stderr extract (GCC 11.2.0, MIPS Codescape):
> 
> ./include/linux/minmax.h:68:57: warning: comparison is always true due
>                           to limited range of data type [-Wtype-limits]
>    68 | #define __is_nonneg(ux) statically_true((long long)(ux) >= 0)
>       |                                                         ^~
> drivers/net/ethernet/cadence/macb_main.c:2299:26: note: in expansion of
>                                                             macro ‘min’
>  2299 |              hdrlen = min(skb_headlen(skb), bp->max_tx_length);
>       |                       ^~~
> 
> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
> ---
>  drivers/net/ethernet/cadence/macb_main.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
> index 98e28d51a6e12c24ef27c939363eb43c0aec1951..6c6bc6aa23c718772b95b398e807f193a38e141a 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -2035,7 +2035,7 @@ static unsigned int macb_tx_map(struct macb *bp,
>  		count++;
>  		tx_head++;
>  
> -		size = min(len, bp->max_tx_length);
> +		size = umin(len, bp->max_tx_length);
>  	}
>  
>  	/* Then, map paged data from fragments */
> @@ -2045,7 +2045,7 @@ static unsigned int macb_tx_map(struct macb *bp,
>  		len = skb_frag_size(frag);
>  		offset = 0;
>  		while (len) {
> -			size = min(len, bp->max_tx_length);
> +			size = umin(len, bp->max_tx_length);
>  			entry = macb_tx_ring_wrap(bp, tx_head);
>  			tx_skb = &queue->tx_skb[entry];
>  
> @@ -2301,7 +2301,7 @@ static netdev_tx_t macb_start_xmit(struct sk_buff *skb, struct net_device *dev)
>  			return NETDEV_TX_BUSY;
>  		}
>  	} else
> -		hdrlen = min(skb_headlen(skb), bp->max_tx_length);
> +		hdrlen = umin(skb_headlen(skb), bp->max_tx_length);
>  
>  #if defined(DEBUG) && defined(VERBOSE_DEBUG)
>  	netdev_vdbg(bp->dev,
> @@ -4573,8 +4573,8 @@ static int macb_init(struct platform_device *pdev)
>  	 * each 4-tuple define requires 1 T2 screener reg + 3 compare regs
>  	 */
>  	reg = gem_readl(bp, DCFG8);
> -	bp->max_tuples = min((GEM_BFEXT(SCR2CMP, reg) / 3),
> -			GEM_BFEXT(T2SCR, reg));
> +	bp->max_tuples = umin((GEM_BFEXT(SCR2CMP, reg) / 3),
> +			      GEM_BFEXT(T2SCR, reg));
>  	INIT_LIST_HEAD(&bp->rx_fs_list.list);
>  	if (bp->max_tuples > 0) {
>  		/* also needs one ethtype match to check IPv4 */
> 

On Sun Oct 19, 2025 at 4:10 PM CEST, David Laight wrote:
> On Tue, 14 Oct 2025 17:25:12 +0200
> Théo Lebrun <theo.lebrun@bootlin.com> wrote:
>
>> Whenever min(a, b) is used with a and b unsigned variables or literals,
>> `make W=2` complains. Change four min() calls into umin().
>
> It will, and you'll get the same 'error' all over the place.
> Basically -Wtype-limits is broken.
>
> Don't remove valid checks because it bleats.

In theory I agree. In practice, this patch leads to a more readable
`make W=2 drivers/net/ethernet/cadence/` stderr output, by removing a
few false positives, and that's my only desire (not quite).

I am not sure what you mean by "Don't remove valid checks"; could you
clarify? My understanding is that the warning checks are about the
signedness of unsigned integers. Are you implying that we lose
something (safety?) when switching from min(a, b) to umin(a, b) with
a/b both unsigned ints?

Thanks David,

[0]: https://lore.kernel.org/lkml/176066582948.1978978.752807229943547484.git-patchwork-notify@kernel.org/
[1]: https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/?id=f26c6438a285

--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

On Mon, 20 Oct 2025 13:44:43 +0200
Théo Lebrun <theo.lebrun@bootlin.com> wrote:

> On Sun Oct 19, 2025 at 4:10 PM CEST, David Laight wrote:
> > On Tue, 14 Oct 2025 17:25:12 +0200
> > Théo Lebrun <theo.lebrun@bootlin.com> wrote:
> >  
> >> Whenever min(a, b) is used with a and b unsigned variables or literals,
> >> `make W=2` complains. Change four min() calls into umin().  
> >
> > It will, and you'll get the same 'error' all over the place.
> > Basically -Wtype-limits is broken.
> >
> > Don't remove valid checks because it bleats.  
> 
> In theory I agree. In practice, this patch leads to a more readable
> `make W=2 drivers/net/ethernet/cadence/` stderr output, by removing a
> few false positives, and that's my only desire (not quite).
> 
> I am not sure what you mean by "Don't remove valid checks"; could you
> clarify? My understanding is that the warning checks are about the
> signedness of unsigned integers. Are you implying that we lose
> something (safety?) when switching from min(a, b) to umin(a, b) with
> a/b both unsigned ints?

The issue is that -Wtype-limits warns for every case where min() is
used with two unsigned values.
It is pretty much impossible to code around it as well.
It also warns for other #defines that are trying to check for invalid
constant values.
The checks are there to pick up invalid calls, using umin() (and worse
min_t()) to avoid the warnings is making the checks pointless.
So you may know the code is ok, but the compile-time checks are there
to ensure it is ok.

	David

> [0]: https://lore.kernel.org/lkml/176066582948.1978978.752807229943547484.git-patchwork-notify@kernel.org/
> [1]: https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/?id=f26c6438a285
> 
> --
> Théo Lebrun, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
>