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

["Russell King (Oracle)" <linux@armlinux.org.uk>]

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!