-
-
Notifications
You must be signed in to change notification settings - Fork 9.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ipq806x: Ethernet interface fixes for AP3935 #13629
base: main
Are you sure you want to change the base?
Conversation
6f5b5de
to
e8c4e2f
Compare
#size-cells = <0>; | ||
|
||
status = "okay"; | ||
&mdio0 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you sure it's ok? mdio0 is using the gmac0 mdio instead of the gpio bitbang way... so the defined gpio won't be used... Was this tested?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it was tested, and it does work. I realize (now) that we shouldn't need to configure the pins/gpios here. If there's any testing you'd like me to do, I have a device that runs the a current snapshot build and a build based off this branch, so happy to do so. I can update the patch to remove the extra config.
I've dumped dmesg, dtc -I dtb -O dts /sys/firmware/fdt, and some mdio-tools output from each device here: https://gist.github.com/berg/350c0d392f01636e1802dd30b03ba46c
The .pr
files are for the device running this PR; the .snapshot
files are for the device which netbooted the most recent snapshot as of an hour ago (r24112-67e465b32e).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BTW, I built a new version with the extraneous gpio bits removed from the gmac mdio config and the PHYs still work fine. I can push that but it'll probably wipe out this comment thread, so I'll wait to hear from you.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sorry for the delay. Yes push the new version. Comments are not lost so no worries!
|
||
phy-mode = "rgmii"; | ||
phy-mode = "rgmii-id"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we put this in another pr or just later when the patch is merged upstream?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately I sorta sold the issue short in the PR description. Let me attempt to explain a bit better. The interface doesn't work ~at all on a cold-boot in the current main branch.
When netbooting, the boot rom pokes the right bits into the phy to put it into rgmii-id mode. Then, the fixed-link
config in the current device tree keeps the phy from being initialized by the driver so the delay bits in the phy registers never get reset.
But if you power cycle the box and it boots from flash, then the LAN1 interface doesn't work (it links, it attempts to transmit frames, but they show up as errors on the uplinked switch, and it can't receive any frames). Removing the fixed-link config and wiring up the rgmii pins in this config causes the phy registers to be set by the driver, but by default the delays are disabled.
Without the kernel patch, the MAC driver won't initialize because it doesn't recognize the rgmii-id
phy mode. The phy driver supports this mode and will set the registers appropriately.
I'm happy to split the PR up further in any way you suggest, but the board doesn't work very well without this particular patch; this bit is kind of the point of the whole thing. I hope this explanation helps.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please split it... What about upstreaming? Do you want help? I can take care of sending this upstream!
Part 1 of openwrt#13629 split. * Sets the LAN 2 MAC address in the DTS by deriving it from LAN 1's address. The factory OS derives this from the `eth1addr` u-boot env variable, but the nvmem_u-boot-env driver doesn't support parsing MAC addresses from fields other than `ethaddr`. But for all of the device samples I've checked (~10) it derives the correct MAC. * Updates 02_network to ensure that interfaces are assigned to roles correctly and consistently.
Part 1 of openwrt#13629 split. * Sets the LAN 2 MAC address in the DTS by deriving it from LAN 1's address. The factory OS derives this from the `eth1addr` u-boot env variable, but the nvmem_u-boot-env driver doesn't support parsing MAC addresses from fields other than `ethaddr`. But for all of the device samples I've checked (~10) it derives the correct MAC. * Updates 02_network to ensure that interfaces are assigned to roles correctly and consistently. Signed-off-by: Bryan Berg <bdb@north-eastham.org>
Part 1 of openwrt#13629 split. * Sets the LAN 2 MAC address in the DTS by deriving it from LAN 1's address. The factory OS derives this from the `eth1addr` u-boot env variable, but the nvmem_u-boot-env driver doesn't support parsing MAC addresses from fields other than `ethaddr`. But for all of the device samples I've checked (~10) it derives the correct MAC. * Updates 02_network to ensure that interfaces are assigned to roles correctly and consistently. Signed-off-by: Bryan Berg <bdb@north-eastham.org>
Part 1 of openwrt#13629 split. * Sets the LAN 2 MAC address in the DTS by deriving it from LAN 1's address. The factory OS derives this from the `eth1addr` u-boot env variable, but the nvmem_u-boot-env driver doesn't support parsing MAC addresses from fields other than `ethaddr`. But for all of the device samples I've checked (~10) it derives the correct MAC. * Updates 02_network to ensure that interfaces are assigned to roles correctly and consistently. Signed-off-by: Bryan Berg <bdb@north-eastham.org>
Part 1 of openwrt#13629 split. * Sets the LAN 2 MAC address in the DTS by deriving it from LAN 1's address. The factory OS derives this from the `eth1addr` u-boot env variable, but the nvmem_u-boot-env driver doesn't support parsing MAC addresses from fields other than `ethaddr`. But for all of the device samples I've checked (~10) it derives the correct MAC. * Updates 02_network to ensure that interfaces are assigned to roles correctly and consistently. Signed-off-by: Bryan Berg <bdb@north-eastham.org>
Part 1 of openwrt#13629 split. * Sets the LAN 2 MAC address in the DTS by deriving it from LAN 1's address. The factory OS derives this from the `eth1addr` u-boot env variable, but the nvmem_u-boot-env driver doesn't support parsing MAC addresses from fields other than `ethaddr`. But for all of the device samples I've checked (~10) it derives the correct MAC. * Updates 02_network to ensure that interfaces are assigned to roles correctly and consistently. Signed-off-by: Bryan Berg <bdb@north-eastham.org>
Part 1 of openwrt#13629 split. * Sets the LAN 2 MAC address in the DTS by deriving it from LAN 1's address. The factory OS derives this from the `eth1addr` u-boot env variable, but the nvmem_u-boot-env driver doesn't support parsing MAC addresses from fields other than `ethaddr`. But for all of the device samples I've checked (~10) it derives the correct MAC. * Updates 02_network to ensure that interfaces are assigned to roles correctly and consistently. Signed-off-by: Bryan Berg <bdb@north-eastham.org>
Part 1 of openwrt#13629 split. * Sets the LAN 2 MAC address in the DTS by deriving it from LAN 1's address. The factory OS derives this from the `eth1addr` u-boot env variable, but the nvmem_u-boot-env driver doesn't support parsing MAC addresses from fields other than `ethaddr`. But for all of the device samples I've checked (~10) it derives the correct MAC. * Updates 02_network to ensure that interfaces are assigned to roles correctly and consistently. Signed-off-by: Bryan Berg <bdb@north-eastham.org>
|
||
nvmem-cells = <ðaddr>; | ||
nvmem-cell-names = "mac-address"; | ||
mac-address-increment = <1>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Deprecated please use nvmem-layout new way with nvmem cell cells. (you can check other example in other dts)
@berg any news with this? Can you rebase? Also again can you split the current commit in multiple commits? (not multiple pr. multiple commits.) |
@Ansuel I'll dig back into this, apologies—I moved and haven't brought my builder host back online. I will try to get this rebased, split up, and wrapped up by the weekend. |
@berg btw i pushed the patch upstream so it's only a matter of fixing the dts |
Updates ap3935 target to use the mdio controller built-in to gmac0, rather than having the SoC bit-bang mdio from gpio pins Signed-off-by: Bryan Berg <bdb@north-eastham.org>
Fix issue with LAN1 interface on ap3935 devices, where the LAN 1interface won't come up after boot unless the bootloader has initialized the network stack. Use of `fixed-link` in the prior code kept the driver from clobbering PHY settings, but now that the driver supports rgmii-id phy-mode, we can just use that. Signed-off-by: Bryan Berg <bdb@north-eastham.org>
Used by ethernet interfaces on AP3935i. Signed-off-by: Bryan Berg <bdb@north-eastham.org>
@Ansuel please take a look when you can, this should be ready to go. Feedback welcome though! Thank you! |
Some fixes to Ethernet interfaces on top of #13030.
Enable at803x phy driver under kernel 6.6.
Fix issue with LAN1 interface on ap3935 devices, where the LAN 1 interface won't come up after boot unless the bootloader has initialized the PHY. Use of
fixed-link
in the prior code kept the driver from clobbering PHY settings, but now that the driver supports rgmii-id phy-mode, we can just use that.Cleans up mdio0 in the DTS so it uses existing defined mdio.
Signed-off-by: Bryan Berg bdb@north-eastham.org