Messages in this thread | | | Date | Tue, 12 Apr 2022 12:56:18 +0300 | Subject | Re: [RFC 13/13] net: ti: icssg-prueth: Add ICSSG ethernet driver | From | Grygorii Strashko <> |
| |
On 12/04/2022 12:45, Puranjay Mohan wrote: > + Roger, Grygorii > > On 07/04/22 00:07, Andrew Lunn wrote: >>> +static int emac_phy_connect(struct prueth_emac *emac) >>> +{ >>> + struct prueth *prueth = emac->prueth; >>> + >>> + /* connect PHY */ >>> + emac->phydev = of_phy_connect(emac->ndev, emac->phy_node, >>> + &emac_adjust_link, 0, emac->phy_if); >> >>> +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); >>> + >>> + return 0; >>> +} >>> >> >> O.K, so this does not do what i initially thought it was doing. I was >> thinking it was to fine tune the delay, ti,syscon-rgmii-delay would be >> a small pico second value to allow the 2ns delay to be tuned to the >> board. >> >> But now i think this is actually inserting the full 2ns delay? >> >> The problem is, you also pass phy_if to of_phy_connect() so the PHY >> will also insert the delay if requested. So you end up with double >> delays for rgmii_id and rgmii_txid.
It's misunderstanding here. The bit field name in TRM is RGMII0_ID_MODE and meaning: 0h - Internal transmit delay is enabled 1h - Internal transmit delay is not enabled.
So here internal delay will be disabled for RGMII_ID/RGMII_TXID.
>> >> The general recommendation is that the PHY inserts the delay, based on >> phy-mode. The MAC does not add a delay, so i suggest you always write >> 0 here, just to ensure the system is in a deterministic state, and the >> bootloader and not being messing around with things. >> >> Andrew
-- Best regards, Grygorii, Ukraine
| |