On 2/14/23 18:31, Elmar Psilog wrote:
Am 13.02.23 um 22:20 schrieb Marek Vasut:
On 2/13/23 22:04, Fabio Estevam wrote:
Adding Marek, who has sent some EQOS patches recently.
On Mon, Feb 13, 2023 at 6:02 PM Elmar Psilog <[email protected]> wrote:
Hello,
Think I found a regression in EQOS driver with fixed-phy. Maybe someone
with a imx8mp board might check that use case to confirm? That would be
great.
While ethernet was working in v2022.04 a "ping" in v2023.01 returns
ERROR: no/invalid <fixed-link> property!
invalid speed 0 eqos_adjust_link() failed: -22 FAILED
although devicetree/hardware kept unchanged.
This happens because in file fixed.c in in function fixedphy_config()
the call
val = ofnode_read_u32_default(node, "speed", 0);
returns 0 instead of 1000 and also the duplex is not set. Found
that in
file/function dwc_eth_qos.c / eqos_start() the line
eqos->phy->node = eqos->phy_of_node;
is responsible for losing the information. Don't know what magic
happens
here - so I can't fix it - I just followed the data. So all works well
and even the parsing of old and new fixed-link devicetree works til
that
line. After that I don't get speed anymore. Maybe you can have a
look at
this?
Try this patch (needs CONFIG_FIXED_PHY=y) :
diff --git a/drivers/net/dwc_eth_qos.c b/drivers/net/dwc_eth_qos.c
index d488bd0c288..592af53b352 100644
--- a/drivers/net/dwc_eth_qos.c
+++ b/drivers/net/dwc_eth_qos.c
@@ -791,9 +791,21 @@ static int eqos_start(struct udevice *dev)
*/
if (!eqos->phy) {
int addr = -1;
- addr = eqos_get_phy_addr(eqos, dev);
- eqos->phy = phy_connect(eqos->mii, addr, dev,
- eqos->config->interface(dev));
+ ofnode fixed_node;
+
+ if (IS_ENABLED(CONFIG_PHY_FIXED)) {
+ fixed_node = ofnode_find_subnode(dev_ofnode(dev),
+ "fixed-link");
+ if (ofnode_valid(fixed_node)) {
+ eqos->phy =
fixed_phy_create(dev_ofnode(dev));
+ }
+
+ if (!eqos->phy) {
+ addr = eqos_get_phy_addr(eqos, dev);
+ eqos->phy = phy_connect(eqos->mii, addr, dev,
+ eqos->config->interface(dev));
+ }
+
if (!eqos->phy) {
pr_err("phy_connect() failed");
goto err_stop_resets;
Thanks Fabio for forwarding and Marek for the quick patch! Don't get
this exactly.
Bit confused about the closing "}" in if(IS_ENABLED) seems to be missed
(or an else path)?
Where exactly should this be? From the insertion it looks like you mean:
Yes, the { shouldn't be at the end of 'if (ofnode_valid(fixed_node)) {' .
+ if (IS_ENABLED(CONFIG_PHY_FIXED)) {
+ fixed_node = ofnode_find_subnode(dev_ofnode(dev),
+ "fixed-link");
+ if (ofnode_valid(fixed_node)) {
+ eqos->phy =
fixed_phy_create(dev_ofnode(dev));
Add this here (*) too (see below):
eqos->phy_of_node = fixed_node;
+ }
This works too.
+ }
+ if (!eqos->phy) {
+ addr = eqos_get_phy_addr(eqos, dev);
+ eqos->phy = phy_connect(eqos->mii, addr, dev,
eqos->config->interface(dev));
+ }
....
But that doesn't solve the issue. The magic line
eqos->phy->node = eqos->phy_of_node;
will still executed.
See (*) above , does that help ?
Likely a different issue but connected to EQOS: I am wondering why the
clocks for ethernet look so much different if FEC is not configured.
Shouldn't be the case, right? At least imx8mp.dtsi doesn't tell about
any 24MHz clock. Also counter is "0"? Well, EQOS works with uncommenting
the line above and this clock, but why?
Each device is probed on first use, so the clock are left unconfigured
until the device is actually used (e.g. for ethernet transfer). Note
that there is a huge series which overhauls the EQoS and FEC clock on
the ML, see:
[PATCH v3 01/14] clk: imx8mp: Add EQoS MAC clock
