* dwc_eth_qos driver for tegra
@ 2022-05-23 9:17 Rasmus Villemoes
2022-05-23 10:57 ` Marek Vasut
0 siblings, 1 reply; 4+ messages in thread
From: Rasmus Villemoes @ 2022-05-23 9:17 UTC (permalink / raw)
To: Christophe Roullier, Stephen Warren; +Cc: u-boot, Marek Vasut, Patrick Delaunay
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.
Rasmus
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: dwc_eth_qos driver for tegra
2022-05-23 9:17 dwc_eth_qos driver for tegra Rasmus Villemoes
@ 2022-05-23 10:57 ` Marek Vasut
2022-05-23 11:46 ` Rasmus Villemoes
0 siblings, 1 reply; 4+ messages in thread
From: Marek Vasut @ 2022-05-23 10:57 UTC (permalink / raw)
To: Rasmus Villemoes, Christophe Roullier, Stephen Warren
Cc: u-boot, Patrick Delaunay, Ramon Fried
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 ?
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: dwc_eth_qos driver for tegra
2022-05-23 10:57 ` Marek Vasut
@ 2022-05-23 11:46 ` Rasmus Villemoes
2022-05-23 12:09 ` Marek Vasut
0 siblings, 1 reply; 4+ messages in thread
From: Rasmus Villemoes @ 2022-05-23 11:46 UTC (permalink / raw)
To: Marek Vasut, Christophe Roullier, Stephen Warren
Cc: u-boot, Patrick Delaunay, Ramon Fried
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: dwc_eth_qos driver for tegra
2022-05-23 11:46 ` Rasmus Villemoes
@ 2022-05-23 12:09 ` Marek Vasut
0 siblings, 0 replies; 4+ messages in thread
From: Marek Vasut @ 2022-05-23 12:09 UTC (permalink / raw)
To: Rasmus Villemoes, Christophe Roullier, Stephen Warren
Cc: u-boot, Patrick Delaunay, Ramon Fried
On 5/23/22 13:46, Rasmus Villemoes wrote:
> 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"?
I think we wait for nvidia to answer all this, I don't have that SoC
available.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-05-23 12:09 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-23 9:17 dwc_eth_qos driver for tegra Rasmus Villemoes
2022-05-23 10:57 ` Marek Vasut
2022-05-23 11:46 ` Rasmus Villemoes
2022-05-23 12:09 ` Marek Vasut
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.