Re: [PATCH net-next v3] net: phy: add driver for MediaTek SoC built-in GE PHYs

[Simon Horman]

On Thu, May 11, 2023 at 12:08:02AM +0200, Daniel Golle wrote:
> Some of MediaTek's Filogic SoCs come with built-in gigabit Ethernet
> PHYs which require calibration data from the SoC's efuse.
> Despite the similar design the driver doesn't share any code with the
> existing mediatek-ge.c, so add support for these PHYs by introducing a
> new driver for only MediaTek's ARM64 SoCs.
> 
> Signed-off-by: Daniel Golle <daniel@xxxxxxxxxxxxxx>

Hi Daniel,

some minor nits from my side.

...

> diff --git a/drivers/net/phy/mediatek-ge-soc.c b/drivers/net/phy/mediatek-ge-soc.c

...

> +#define MTK_PHY_RG_BG_RASEL			0x115
> +#define   MTK_PHY_RG_BG_RASEL_MASK		GENMASK(2, 0)
> +
> +/* These macro privides efuse parsing for internal phy. */

nit: s/privides/provides/

     Please consider using checkpatch --codespell

...

> +static int tx_amp_fill_result(struct phy_device *phydev, u16 *buf)
> +{
> +	int i;
> +	int bias[16] = {};
> +	const int vals_9461[16] = { 7, 1, 4, 7,
> +				    7, 1, 4, 7,
> +				    7, 1, 4, 7,
> +				    7, 1, 4, 7 };
> +	const int vals_9481[16] = { 10, 6, 6, 10,
> +				    10, 6, 6, 10,
> +				    10, 6, 6, 10,
> +				    10, 6, 6, 10 };

nits: Please put a blank line here.
      Please arrange local variables in networking code in reverse xmas
      tree order.

Link: https://github.com/ecree-solarflare/xmastree

> +	switch (phydev->drv->phy_id) {
> +	case MTK_GPHY_ID_MT7981:
> +		/* We add some calibration to efuse values
> +		 * due to board level influence.
> +		 * GBE: +7, TBT: +1, HBT: +4, TST: +7
> +		 */
> +		memcpy(bias, (const void *)vals_9461, sizeof(bias));
> +		break;
> +	case MTK_GPHY_ID_MT7988:
> +		memcpy(bias, (const void *)vals_9481, sizeof(bias));
> +		break;
> +	}

...