[PATCH] net: tulip: winbond-840: remove never-true 486 CPU check

[PATCH] net: tulip: winbond-840: remove never-true 486 CPU check

[Ethan Nelson-Moore]

The winbond-840 driver contains a check for boot_cpu_data.x86 <= 4.
This field encodes the x86 generation (3 = 386, 4 = 486, etc).
Therefore, since 486 support was removed from the kernel in commit
8b793a92d862 ("x86/cpu: Remove M486/M486SX/ELAN support"), the
condition is always true. Remove the check.

Signed-off-by: Ethan Nelson-Moore <enelsonmoore@gmail.com>
---
 drivers/net/ethernet/dec/tulip/winbond-840.c | 14 ++------------
 1 file changed, 2 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/dec/tulip/winbond-840.c b/drivers/net/ethernet/dec/tulip/winbond-840.c
index a5581f1d0141..c42eaf0eef64 100644
--- a/drivers/net/ethernet/dec/tulip/winbond-840.c
+++ b/drivers/net/ethernet/dec/tulip/winbond-840.c
@@ -876,24 +876,14 @@ static void init_registers(struct net_device *dev)
 	i |= 0x02;		/* give Rx priority */
 
 	/* Configure the PCI bus bursts and FIFO thresholds.
-	   486: Set 8 longword cache alignment, 8 longword burst.
-	   586: Set 16 longword cache alignment, no burst limit.
+	   Set 16 longword cache alignment, no burst limit.
 	   Cache alignment bits 15:14	     Burst length 13:8
 		0000	<not allowed> 		0000 align to cache	0800 8 longwords
 		4000	8  longwords		0100 1 longword		1000 16 longwords
 		8000	16 longwords		0200 2 longwords	2000 32 longwords
 		C000	32  longwords		0400 4 longwords */
 
-#if defined (__i386__) && !defined(MODULE) && !defined(CONFIG_UML)
-	/* When not a module we can work around broken '486 PCI boards. */
-	if (boot_cpu_data.x86 <= 4) {
-		i |= 0x4800;
-		dev_info(&dev->dev,
-			 "This is a 386/486 PCI system, setting cache alignment to 8 longwords\n");
-	} else {
-		i |= 0xE000;
-	}
-#elif defined(__powerpc__) || defined(__i386__) || defined(__alpha__) || defined(__ia64__) || defined(__x86_64__)
+#if defined(__powerpc__) || defined(__i386__) || defined(__alpha__) || defined(__ia64__) || defined(__x86_64__)
 	i |= 0xE000;
 #elif defined(CONFIG_SPARC) || defined (CONFIG_PARISC) || defined(CONFIG_ARM)
 	i |= 0x4800;
-- 
2.43.0

Re: [PATCH] net: tulip: winbond-840: remove never-true 486 CPU check

[Ethan Nelson-Moore]

On Mon, Sep 14, 2026 at 7:46 PM Ethan Nelson-Moore
<enelsonmoore@gmail.com> wrote:
> The winbond-840 driver contains a check for boot_cpu_data.x86 <= 4.
> This field encodes the x86 generation (3 = 386, 4 = 486, etc).
> Therefore, since 486 support was removed from the kernel in commit
> 8b793a92d862 ("x86/cpu: Remove M486/M486SX/ELAN support"), the
> condition is always true. Remove the check.

Oops, never true*. Should I send a v2, or will whoever merges this fix it?

Re: [PATCH] net: tulip: winbond-840: remove never-true 486 CPU check

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 3 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 1 · Low: 2

- [Medium] Inverted rationale in the commit message.
- [Low] Stale `#include <asm/processor.h>` with a now-false self-
  documenting comment.
- [Low] Comment/code mismatch introduced by the comment rewrite in
  init_registers().

--

net: tulip: winbond-840: remove never-true 486 CPU check

This removes the runtime boot_cpu_data.x86 <= 4 test in init_registers()
of the winbond-840 driver and collapses the surrounding preprocessor
chain so that the i386 non-module case now falls into the generic
architecture branch.  The comment describing the two cache alignment
cases is reworded at the same time.

> net: tulip: winbond-840: remove never-true 486 CPU check

[Severity: Medium]
The subject and the message body say the condition is always true:

    Therefore, since 486 support was removed from the kernel in commit
    8b793a92d862 ("x86/cpu: Remove M486/M486SX/ELAN support"), the
    condition is always true. Remove the check.

Is the polarity backwards here?  boot_cpu_data.x86 holds the CPU family
(3 = 386, 4 = 486, 5 = 586), and after 8b793a92d862 the minimum family
for 32-bit builds is 5:

arch/x86/Kconfig.cpu:
    config X86_MINIMUM_CPU_FAMILY
            int
            default "64" if X86_64
            default "6" if X86_32 && (MPENTIUM4 || ...)
            default "5"

So a family <= 4 CPU can no longer boot the kernel and the predicate is
never true, which is consistent with the patch keeping i |= 0xE000 (the
else branch) rather than i |= 0x4800.

Should the wording be something like "the condition is never true, so
the else branch is always taken"?  As written, a reader or a stable
backporter would expect 0x4800 to be the value preserved for i386
built-in builds.

> diff --git a/drivers/net/ethernet/dec/tulip/winbond-840.c b/drivers/net/ethernet/dec/tulip/winbond-840.c
> index a5581f1d01417..c42eaf0eef642 100644
> --- a/drivers/net/ethernet/dec/tulip/winbond-840.c
> +++ b/drivers/net/ethernet/dec/tulip/winbond-840.c
> @@ -876,24 +876,14 @@ static void init_registers(struct net_device *dev)
>  	i |= 0x02;		/* give Rx priority */
>  
>  	/* Configure the PCI bus bursts and FIFO thresholds.
> -	   486: Set 8 longword cache alignment, 8 longword burst.
> -	   586: Set 16 longword cache alignment, no burst limit.
> +	   Set 16 longword cache alignment, no burst limit.

[Severity: Low]
Does this single unconditional line still match the code below it?  Two
different values are still programmed, and neither one is "16 longword
cache alignment, no burst limit" (which the table just below decodes as
0x8000):

    #if defined(__powerpc__) || defined(__i386__) || ...
            i |= 0xE000;    /* C000 = 32 longwords | 2000 = 32 longword burst */
    #elif defined(CONFIG_SPARC) || defined (CONFIG_PARISC) || defined(CONFIG_ARM)
            i |= 0x4800;    /* 4000 = 8 longwords | 0800 = 8 longword burst */
    #else
            i |= 0x4800;

The deleted "486:" line is the one that described 0x4800 accurately, and
that value is still programmed on SPARC, PARISC, ARM and the unknown-arch
fallback.  Would it be better to drop the prose entirely, or restate it
for both remaining cases?

The 0xE000 half of the mismatch predates this patch; the new part is that
the wording is now unconditional and also covers the 0x4800 branches.

>  	   Cache alignment bits 15:14	     Burst length 13:8
>  		0000	<not allowed> 		0000 align to cache	0800 8 longwords
>  		4000	8  longwords		0100 1 longword		1000 16 longwords
>  		8000	16 longwords		0200 2 longwords	2000 32 longwords
>  		C000	32  longwords		0400 4 longwords */
>  
> -#if defined (__i386__) && !defined(MODULE) && !defined(CONFIG_UML)
> -	/* When not a module we can work around broken '486 PCI boards. */
> -	if (boot_cpu_data.x86 <= 4) {
> -		i |= 0x4800;
> -		dev_info(&dev->dev,
> -			 "This is a 386/486 PCI system, setting cache alignment to 8 longwords\n");
> -	} else {
> -		i |= 0xE000;
> -	}
> -#elif defined(__powerpc__) || defined(__i386__) || defined(__alpha__) || defined(__ia64__) || defined(__x86_64__)
> +#if defined(__powerpc__) || defined(__i386__) || defined(__alpha__) || defined(__ia64__) || defined(__x86_64__)
>  	i |= 0xE000;

[Severity: Low]
With boot_cpu_data gone, is the header include at the top of
drivers/net/ethernet/dec/tulip/winbond-840.c now stale?

    #include <asm/processor.h>		/* Processor type for cache alignment. */

That was the only user of asm/processor.h in this file, and after the
patch the cache alignment selection in init_registers() is purely
compile-time arch conditionals, so the comment no longer describes
anything the code does.  Should the include (or at least the comment) be
dropped in the same change?

> #elif defined(CONFIG_SPARC) || defined (CONFIG_PARISC) || defined(CONFIG_ARM)
>  	i |= 0x4800;

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260915024551.196126-1-enelsonmoore%40gmail.com

RE: [PATCH] net: tulip: winbond-840: remove never-true 486 CPU check

[Loktionov, Aleksandr]

> -----Original Message-----
> From: Ethan Nelson-Moore <enelsonmoore@gmail.com>
> Sent: Tuesday, September 15, 2026 4:46 AM
> To: Arend van Spriel <arend.vanspriel@broadcom.com>; Loktionov,
> Aleksandr <aleksandr.loktionov@intel.com>; Jijie Shao
> <shaojijie@huawei.com>; Petr Machata <petrm@nvidia.com>; Keller, Jacob
> E <jacob.e.keller@intel.com>; Uwe Kleine-König (The Capable Hub)
> <u.kleine-koenig@baylibre.com>; netdev@vger.kernel.org; linux-
> parisc@vger.kernel.org
> Cc: Ethan Nelson-Moore <enelsonmoore@gmail.com>; Andrew Lunn
> <andrew+netdev@lunn.ch>; David S. Miller <davem@davemloft.net>; Eric
> Dumazet <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>; Paolo
> Abeni <pabeni@redhat.com>; Michael Grzeschik <mgr@kernel.org>
> Subject: [PATCH] net: tulip: winbond-840: remove never-true 486 CPU
> check
> 
> The winbond-840 driver contains a check for boot_cpu_data.x86 <= 4.
> This field encodes the x86 generation (3 = 386, 4 = 486, etc).
> Therefore, since 486 support was removed from the kernel in commit
> 8b793a92d862 ("x86/cpu: Remove M486/M486SX/ELAN support"), the
> condition is always true. Remove the check.
It looks like condition is always false, isn't it?

Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>


> 
> Signed-off-by: Ethan Nelson-Moore <enelsonmoore@gmail.com>
> ---
>  drivers/net/ethernet/dec/tulip/winbond-840.c | 14 ++------------
>  1 file changed, 2 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/net/ethernet/dec/tulip/winbond-840.c
> b/drivers/net/ethernet/dec/tulip/winbond-840.c
> index a5581f1d0141..c42eaf0eef64 100644
> --- a/drivers/net/ethernet/dec/tulip/winbond-840.c
> +++ b/drivers/net/ethernet/dec/tulip/winbond-840.c
> @@ -876,24 +876,14 @@ static void init_registers(struct net_device
> *dev)
>  	i |= 0x02;		/* give Rx priority */
> 
>  	/* Configure the PCI bus bursts and FIFO thresholds.
> -	   486: Set 8 longword cache alignment, 8 longword burst.
> -	   586: Set 16 longword cache alignment, no burst limit.
> +	   Set 16 longword cache alignment, no burst limit.
>  	   Cache alignment bits 15:14	     Burst length 13:8
>  		0000	<not allowed> 		0000 align to cache
> 	0800 8 longwords
>  		4000	8  longwords		0100 1 longword
> 	1000 16 longwords
>  		8000	16 longwords		0200 2 longwords	2000 32
> longwords
>  		C000	32  longwords		0400 4 longwords */
> 
> -#if defined (__i386__) && !defined(MODULE) && !defined(CONFIG_UML)
> -	/* When not a module we can work around broken '486 PCI boards.
> */
> -	if (boot_cpu_data.x86 <= 4) {
> -		i |= 0x4800;
> -		dev_info(&dev->dev,
> -			 "This is a 386/486 PCI system, setting cache
> alignment to 8 longwords\n");
> -	} else {
> -		i |= 0xE000;
> -	}
> -#elif defined(__powerpc__) || defined(__i386__) || defined(__alpha__)
> || defined(__ia64__) || defined(__x86_64__)
> +#if defined(__powerpc__) || defined(__i386__) || defined(__alpha__)
> ||
> +defined(__ia64__) || defined(__x86_64__)
>  	i |= 0xE000;
>  #elif defined(CONFIG_SPARC) || defined (CONFIG_PARISC) ||
> defined(CONFIG_ARM)
>  	i |= 0x4800;
> --
> 2.43.0