Re: [PATCH net-next 2/4] net: dsa: ks8955: Delete KSZ8864 and KSZ8795 support

[Simon Horman <horms@kernel.org>]

On Sun, Jan 18, 2026 at 11:07:32PM +0100, Linus Walleij wrote:

...

> @@ -346,79 +291,37 @@ static int ks8995_reset(struct ks8995_switch *ks)
>  static int ks8995_get_revision(struct ks8995_switch *ks)
>  {
>  	int err;
> -	u8 id0, id1, ksz8864_id;
> +	u8 id0, id1;
>  
>  	/* read family id */
>  	err = ks8995_read_reg(ks, KS8995_REG_ID0, &id0);
> -	if (err) {
> -		err = -EIO;
> -		goto err_out;
> -	}
> +	if (err)
> +		return -EIO;
>  
>  	/* verify family id */
> -	if (id0 != ks->chip->family_id) {
> +	if (id0 != FAMILY_KS8995) {
>  		dev_err(&ks->spi->dev, "chip family id mismatch: expected 0x%02x but 0x%02x read\n",
> -			ks->chip->family_id, id0);
> -		err = -ENODEV;
> -		goto err_out;
> +			FAMILY_KS8995, id0);
> +		return -ENODEV;
>  	}
>  
> -	switch (ks->chip->family_id) {
> -	case FAMILY_KS8995:
> -		/* try reading chip id at CHIP ID1 */
> -		err = ks8995_read_reg(ks, KS8995_REG_ID1, &id1);
> -		if (err) {
> -			err = -EIO;
> -			goto err_out;
> -		}
> -
> -		/* verify chip id */
> -		if ((get_chip_id(id1) == CHIPID_M) &&
> -		    (get_chip_id(id1) == ks->chip->chip_id)) {
> -			/* KS8995MA */
> -			ks->revision_id = get_chip_rev(id1);
> -		} else if (get_chip_id(id1) != CHIPID_M) {
> -			/* KSZ8864RMN */
> -			err = ks8995_read_reg(ks, KS8995_REG_ID1, &ksz8864_id);
> -			if (err) {
> -				err = -EIO;
> -				goto err_out;
> -			}
> -
> -			if ((ksz8864_id & 0x80) &&
> -			    (ks->chip->chip_id == KSZ8864_CHIP_ID)) {
> -				ks->revision_id = get_chip_rev(id1);
> -			}
> -
> -		} else {
> -			dev_err(&ks->spi->dev, "unsupported chip id for KS8995 family: 0x%02x\n",
> -				id1);
> -			err = -ENODEV;
> -		}
> -		break;
> -	case FAMILY_KSZ8795:
> -		/* try reading chip id at CHIP ID1 */
> -		err = ks8995_read_reg(ks, KS8995_REG_ID1, &id1);
> -		if (err) {
> -			err = -EIO;
> -			goto err_out;
> -		}
> +	/* try reading chip id at CHIP ID1 */
> +	err = ks8995_read_reg(ks, KS8995_REG_ID1, &id1);
> +	if (err)
> +		return -EIO;
>  
> -		if (get_chip_id(id1) == ks->chip->chip_id) {
> -			ks->revision_id = get_chip_rev(id1);
> -		} else {
> -			dev_err(&ks->spi->dev, "unsupported chip id for KSZ8795 family: 0x%02x\n",
> -				id1);
> -			err = -ENODEV;
> -		}
> -		break;
> -	default:
> -		dev_err(&ks->spi->dev, "unsupported family id: 0x%02x\n", id0);
> -		err = -ENODEV;
> -		break;
> +	/* verify chip id */
> +	if ((get_chip_id(id1) == CHIPID_M) &&
> +	    (get_chip_id(id1) == KS8995_CHIP_ID)) {

Hi Linus,

This seems a little odd to me.
The condition can be true, but only because the constants are equal (0).

What is the intention here?

Flagged by Smatch.

> +		/* KS8995MA */
> +		ks->revision_id = get_chip_rev(id1);
> +	} else {
> +		dev_err(&ks->spi->dev, "unsupported chip id for KS8995 family: 0x%02x\n",
> +			id1);
> +		return -ENODEV;
>  	}
> -err_out:
> -	return err;
> +
> +	return 0;
>  }

...