Re: [PATCH 2/2] net: ti: icssg-prueth: Add ICSSG ethernet driver

[Andrew Lunn <andrew@lunn.ch>]

> +static int prueth_config_rgmiidelay(struct prueth *prueth,
> +				    struct device_node *eth_np,
> +				    phy_interface_t phy_if)
> +{
> +	struct device *dev = prueth->dev;
> +	struct regmap *ctrl_mmr;
> +	u32 rgmii_tx_id = 0;
> +	u32 icssgctrl_reg;
> +
> +	if (!phy_interface_mode_is_rgmii(phy_if))
> +		return 0;
> +
> +	ctrl_mmr = syscon_regmap_lookup_by_phandle(eth_np, "ti,syscon-rgmii-delay");
> +	if (IS_ERR(ctrl_mmr)) {
> +		dev_err(dev, "couldn't get ti,syscon-rgmii-delay\n");
> +		return -ENODEV;
> +	}
> +
> +	if (of_property_read_u32_index(eth_np, "ti,syscon-rgmii-delay", 1,
> +				       &icssgctrl_reg)) {
> +		dev_err(dev, "couldn't get ti,rgmii-delay reg. offset\n");
> +		return -ENODEV;
> +	}
> +
> +	if (phy_if == PHY_INTERFACE_MODE_RGMII_ID ||
> +	    phy_if == PHY_INTERFACE_MODE_RGMII_TXID)
> +		rgmii_tx_id |= ICSSG_CTRL_RGMII_ID_MODE;
> +
> +	regmap_update_bits(ctrl_mmr, icssgctrl_reg, ICSSG_CTRL_RGMII_ID_MODE, rgmii_tx_id);

I know we discussed this before. Why are you adding a delay here in
the MAC? If you do add the delay here, you need to mask what you pass
to phy_connect(), otherwise the PHY is also going to add a delay for
"id" and "txid".

In general, it is best to leave all delays to the PHY.

     Andrew