netdev - Re: [PATCH net-next 2/2] net: phy: microchip_t1s: add support for Microchip LAN865x Rev.B0 PHYs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <c9d06cfe-8834-4ffa-81a8-097c883cc960@lunn.ch>
Date:   Wed, 26 Apr 2023 23:52:16 +0200
From:   Andrew Lunn <andrew@...n.ch>
To:     Parthiban Veerasooran <Parthiban.Veerasooran@...rochip.com>
Cc:     netdev@...r.kernel.org, davem@...emloft.net,
        jan.huber@...rochip.com, thorsten.kummermehr@...rochip.com,
        ramon.nordin.rodriguez@...roamp.se
Subject: Re: [PATCH net-next 2/2] net: phy: microchip_t1s: add support for
 Microchip LAN865x Rev.B0 PHYs

> -#define LAN867X_REG_IRQ_1_CTL 0x001C
> -#define LAN867X_REG_IRQ_2_CTL 0x001D
> +#define LAN86XX_REG_IRQ_1_CTL 0x001C
> +#define LAN86XX_REG_IRQ_2_CTL 0x001D

This patch is pretty big. Please split to LAN867X to LAN86XX rename
out, to make the patches smaller and easier to review.

> +static int lan865x_revb0_plca_set_cfg(struct phy_device *phydev,
> +				      const struct phy_plca_cfg *plca_cfg)
> +{
> +	int ret;
> +
> +	ret = genphy_c45_plca_set_cfg(phydev, plca_cfg);
> +	if (ret)
> +		return ret;
> +
> +	/* Disable the collision detection when PLCA is enabled and enable
> +	 * collision detection when CSMA/CD mode is enabled.
> +	 */
> +	if (plca_cfg->enabled)
> +		return phy_write_mmd(phydev, MDIO_MMD_VEND2, 0x0087, 0x0000);
> +	else
> +		return phy_write_mmd(phydev, MDIO_MMD_VEND2, 0x0087, 0x0083);
> +}
> +

This could be in a patch of its own, with a good commit message
explaining why it is needed.

Once you replace the magic numbers with #defines, the comment becomes
pointless. But what is missing is the answer to the question Why?

	   Andrew