On 23/05/2022 12.57, Marek Vasut wrote: > On 5/23/22 11:17, Rasmus Villemoes wrote: >> Hi > > Hi, > >> I'm looking at switching the dwc_eth_qos driver over to use >> dm_eth_phy_connect(). However, I'm a little puzzled by the code for the >> tegra variant. The comment at the top of the file, as well as >> tegra186.dtsi, says >> >> phy-mode = "rgmii"; >> >> But eqos_get_interface_tegra186() returns a hard-coded >> PHY_INTERFACE_MODE_MII. Now the commit which introduced the ->interface >> abstraction, ac2d4efb16e (net: dwc_eth_qos: add Ethernet stm32mp1 >> support), and that eqos_get_interface_tegra186() function, changed >> >> - eqos->phy = phy_connect(eqos->mii, 0, dev, 0); >> >> to >> >> + eqos->phy = phy_connect(eqos->mii, 0, dev, >> + eqos->config->interface(dev)); >> >> and that last hard-coded 0 in the former phy_connect() is indeed >> equivalent to PHY_INTERFACE_MODE_MII. >> >> So which is it? It would be nice if one could just rely on >> dm_eth_phy_connect() picking up the correct value from device tree, and >> drop all the code which duplicates parsing of phy-mode from the ethernet >> driver. > > linux-2.6$ git grep mii arch/arm64/boot/dts/nvidia/tegra186* > arch/arm64/boot/dts/nvidia/tegra186-p3310.dtsi: phy-mode = "rgmii"; > arch/arm64/boot/dts/nvidia/tegra186-p3509-0000+p3636-0001.dts: phy-mode > = "rgmii-id"; > > So probably RGMII ?
Well, yes, I also did check the linux device tree files which also says rgmii, but that doesn't explain why the U-Boot driver code seems to ignore that entirely and use mii hardcoded, both before and after ac2d4efb16e. So another way of asking: does this driver actually work today, and/or has it worked at some point? I assume the answer is yes - after all, the very first commit "supports the specific configuration used in NVIDIA's Tegra186 chip", but that commit also did that phy_connect() with a last argument of 0 aka PHY_INTERFACE_MODE_MII. And would it break if one started taking the phy-mode from device tree? If so, should device tree be updated to say "mii"? Rasmus
