netdev - Re: [PATCH net-next v3] net: phy: add driver for MediaTek SoC built-in GE 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] [day] [month] [year] [list]

Message-ID: <ZFzb9Fnc3mJAQal9@corigine.com>
Date: Thu, 11 May 2023 14:13:40 +0200
From: Simon Horman <simon.horman@...igine.com>
To: Daniel Golle <daniel@...rotopia.org>
Cc: netdev@...r.kernel.org, linux-mediatek@...ts.infradead.org,
	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
	Andrew Lunn <andrew@...n.ch>,
	Heiner Kallweit <hkallweit1@...il.com>,
	Russell King <linux@...linux.org.uk>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Qingfang Deng <dqfext@...il.com>,
	SkyLake Huang <SkyLake.Huang@...iatek.com>,
	Matthias Brugger <matthias.bgg@...il.com>,
	AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
Subject: Re: [PATCH net-next v3] net: phy: add driver for MediaTek SoC
 built-in GE PHYs

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@...rotopia.org>

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;
> +	}

...