netdev - Re: [PATCH net-next v2 02/10] net: stmmac: Add support for Allwinner A523 GMAC200

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 for Android: free password hash cracker in your pocket

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

Message-ID: <aJyrECCOPTZP81Sx@shell.armlinux.org.uk>
Date: Wed, 13 Aug 2025 16:11:12 +0100
From: "Russell King (Oracle)" <linux@...linux.org.uk>
To: Chen-Yu Tsai <wens@...nel.org>
Cc: Andrew Lunn <andrew+netdev@...n.ch>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>, Chen-Yu Tsai <wens@...e.org>,
	Jernej Skrabec <jernej@...nel.org>,
	Samuel Holland <samuel@...lland.org>, netdev@...r.kernel.org,
	devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
	linux-sunxi@...ts.linux.dev, linux-kernel@...r.kernel.org,
	Andre Przywara <andre.przywara@....com>
Subject: Re: [PATCH net-next v2 02/10] net: stmmac: Add support for Allwinner
 A523 GMAC200

Hi,

A few comments on this...

On Wed, Aug 13, 2025 at 10:55:32PM +0800, Chen-Yu Tsai wrote:
> +#include "stmmac.h"
> +#include "stmmac_platform.h"
> +
> +#define SYSCON_REG		0x34
> +
> +/* RMII specific bits */
> +#define SYSCON_RMII_EN		BIT(13) /* 1: enable RMII (overrides EPIT) */
> +/* Generic system control EMAC_CLK bits */
> +#define SYSCON_ETXDC_MASK		GENMASK(12, 10)
> +#define SYSCON_ERXDC_MASK		GENMASK(9, 5)
> +/* EMAC PHY Interface Type */
> +#define SYSCON_EPIT			BIT(2) /* 1: RGMII, 0: MII */
> +#define SYSCON_ETCS_MASK		GENMASK(1, 0)
> +#define SYSCON_ETCS_MII		0x0
> +#define SYSCON_ETCS_EXT_GMII	0x1
> +#define SYSCON_ETCS_INT_GMII	0x2
> +
> +#define MASK_TO_VAL(mask)   ((mask) >> (__builtin_ffsll(mask) - 1))

Is FIELD_GET() not sufficient?

> +
> +static int sun55i_gmac200_set_syscon(struct device *dev,
> +				     struct plat_stmmacenet_data *plat)
> +{
> +	struct device_node *node = dev->of_node;
> +	struct regmap *regmap;
> +	u32 val, reg = 0;
> +	int ret;
> +
> +	regmap = syscon_regmap_lookup_by_phandle(node, "syscon");
> +	if (IS_ERR(regmap))
> +		return dev_err_probe(dev, PTR_ERR(regmap), "Unable to map syscon\n");
> +
> +	if (!of_property_read_u32(node, "tx-internal-delay-ps", &val)) {
> +		if (val % 100)
> +			return dev_err_probe(dev, -EINVAL,
> +					     "tx-delay must be a multiple of 100\n");

	"tx-delay must be a multiple of 100ps\n"

would be a bit better.

> +		val /= 100;
> +		dev_dbg(dev, "set tx-delay to %x\n", val);
> +		if (val > MASK_TO_VAL(SYSCON_ETXDC_MASK))
> +			return dev_err_probe(dev, -EINVAL,
> +					     "Invalid TX clock delay: %d\n",
> +					     val);

		if (!FIELD_FIT(SYSCON_ETXDC_MASK, val))
			return dev_err_probe(dev, -EINVAL,
					    "TX clock delay exceeds maximum (%d00ps > %d00ps)\n",
					    val, FIELD_MAX(SYSCON_ETXDC_MASK));

> +
> +		reg |= FIELD_PREP(SYSCON_ETXDC_MASK, val);
> +	}
> +
> +	if (!of_property_read_u32(node, "rx-internal-delay-ps", &val)) {
> +		if (val % 100)
> +			return dev_err_probe(dev, -EINVAL,
> +					     "rx-delay must be a multiple of 100\n");
> +		val /= 100;
> +		dev_dbg(dev, "set rx-delay to %x\n", val);
> +		if (val > MASK_TO_VAL(SYSCON_ERXDC_MASK))
> +			return dev_err_probe(dev, -EINVAL,
> +					     "Invalid RX clock delay: %d\n",
> +					     val);

all the same points in this block.

Thanks.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!