Closed Chadster766 closed 9 years ago
mmilburn
commented
9 years ago @Chadster766 Given 99.9% of the work has been done for us, I don't see why not. Even if I sit around and do nothing, it sounds like Kaloz would get to it. I'll start digging into it again.
@leitec IIRC it is operating in indirect mode. I'll have to look into it tomorrow to confirm.
Chadster766
commented
9 years ago Awesome!
gufus
commented
9 years ago Yes... VCOOL
From: Chad McCue [mailto:notifications@github.com] Sent: Wednesday, December 10, 2014 8:11 PM To: Chadster766/McWRT Subject: Re: [McWRT] swconfig doesn't work (#20)
Awesome! — Reply to this email directly or view it on GitHub.
mmilburn
commented
9 years ago So I'm still working on this. I'm kinda thinking out loud here so don't feel obligated to respond. My only barrier is understanding. Again, worst case is Kaloz updates the dts before I'm certain of what I'm doing. MV78230 datasheet: https://origin-www.marvell.com/embedded-processors/armada-xp/assets/HW_MV78230_OS.PDF Armada XP functional datasheet: https://origin-www.marvell.com/embedded-processors/armada-xp/assets/ARMADA-XP-Functional-SpecDatasheet.pdf Our dts: https://dev.openwrt.org/browser/trunk/target/linux/mvebu/files/arch/arm/boot/dts/armada-xp-mamba.dts?rev=41292 ea4500 dts: https://dev.openwrt.org/browser/trunk/target/linux/kirkwood/patches-3.14/160-ea4500.patch?rev=43487
We've got 2 ethernet interfaces in rgmii mode using the multi-purpose pins. This agrees with the MV78230 datasheet and our dts. Interestingly there is a third interface that could be in sgmii mode, but it goes through serdes, so it probably is not available (which would explain it is never configured or detected). So yay, that makes sense to me. Section 17.11 of the functional spec explains the MDIO interface for the SoC. Every MAC has an SMI register, so is it logical for both ethernet MACs to be able to control downstream PHYs? That seems like it could cause problems, for now I'll assume I only need to worry about ge0. It looks like the PHY address register is at 0x00072000. So using the I2C LED driver in our dts as an example, my statement for the phy reg could be: reg = < 0x2000 >;
And yes, I2C stuff is shown to be around 0x00011000 in the functional spec, so I'm not totally crazy.
Edit: text within "< >" was being eaten.
mmilburn
commented
9 years ago Feel like I'm close to getting the device tree file changes sorted
ERROR (phandle_references): Reference to non-existent node or label "mdio"I feel like the node very much exists, but I probably just need to look at it after some sleep.
--- target/linux/mvebu/files/arch/arm/boot/dts/armada-xp-mamba.dts.orig 2014-12-16 11:34:32.319339917 -0700
+++ target/linux/mvebu/files/arch/arm/boot/dts/armada-xp-mamba.dts 2014-12-17 00:07:30.857229716 -0700
@@ -117,7 +117,14 @@
};
mdio {
- status = "disabled";
+ status = "okay";
+ phy0: ethernet-phy@0 {
+ reg = <0>;
+ };
+
+ phy1: ethernet-phy@1 {
+ reg = <1>;
+ };
};
ethernet@70000 {
@@ -125,6 +132,7 @@
pinctrl-names = "default";
status = "okay";
phy-mode = "rgmii-id";
+ phy = <&phy0>;
fixed-link {
speed = <1000>;
full-duplex;
@@ -135,6 +143,7 @@
pinctrl-0 = <&pmx_ge1>;
pinctrl-names = "default";
status = "okay";
+ phy = <&phy1>;
phy-mode = "rgmii-id";
fixed-link {
speed = <1000>;
@@ -274,4 +283,15 @@
gpio-fan,speed-map = <0 0
4500 1>;
};
+
+ mvsw6171 {
+ compatible = "marvell,88e6171";
+ status = "okay";
+ reg = <1>;
+
+ mii-bus = <&mdio>;
+ cpu-port-0 = <5>;
+ cpu-port-1 = <6>;
+ is-indirect;
+ };
};
leitec
commented
9 years ago It exists, but doesn't have a label that can be referenced. I think mdio: mdio { would do the trick. If not here, then in armada-370-xp.dtsi, where it seems to be first defined.
Why the new PHY stuff? If Ethernet already works, you don't need anything other than
mdio: mdio {
status = "okay";
}and the mvsw6171 entry.
mmilburn
commented
9 years ago @leitec I yanked it from armada-xp-axpwifiap.dts. I was attempting to cobble the node together from known parts. I'm not sure if it's "known good" since I don't think anyone actually uses that dts, and it wouldn't be the first time broken code got committed to the kernel. I'll redeclare the mdio node as you suggested and give it a shot, thanks!
"If not here, then in armada-370-xp.dtsi, where it seems to be first defined." Totally. When I first started digging into this it really bothered me that I seemed to be plucking a bus from out of the ether and defining it without an mmio range or even designated pins. I felt better when I searched through the includes.
leitec
commented
9 years ago Ah, I see. The problem with that setup is that the kernel PHY subsystem would then treat those two defined PHYs as hooked up to the two ethernet ports. In turn, it'd query them for link status, etc., and if they are not actually there you won't get anything.
I believe the switch makes the actual PHY registers available per port, but you have to access it indirectly via another set of registers (similar to the indirect mode we use for the main switch) so it's easier to just declare it a fixed link and configure the switch similarly.
mmilburn
commented
9 years ago You're right, defining PHYs there is nonsensical. The SoC only has the two MIIs, and the PHYs on the board are controlled by the switch chip.
Doing mdio: mdio definitely does the trick.
Doing some more reading of http://devicetree.org/ and https://github.com/torvalds/linux/blob/master/Documentation/devicetree/ to make sure I know what I'm doing.
mmilburn
commented
9 years ago Whoops, not indirect.
[ 1.257383] mvsw6171 b00000003.mvsw6171: Found MV88E6171 at f1072004.mdio-mi:10
[ 1.257388] mvsw6171 b00000003.mvsw6171: Using direct addressingI'll test this a little before I check in changes.
Edit: hurr durr, did not mean to close.
mmilburn
commented
9 years ago Need to confirm cpu-ports, but here's the changes so far:
diff --git a/target/linux/mvebu/config-3.14 b/target/linux/mvebu/config-3.14
index 01bd9c1..17a5d64 100644
--- a/target/linux/mvebu/config-3.14
+++ b/target/linux/mvebu/config-3.14
@@ -209,6 +209,7 @@ CONFIG_MVEBU_DEVBUS=y
CONFIG_MVEBU_MBUS=y
CONFIG_MVMDIO=y
CONFIG_MVNETA=y
+CONFIG_MVSW6171_PHY=y
CONFIG_MV_XOR=y
CONFIG_NEED_DMA_MAP_STATE=y
# CONFIG_NEON is not set
@@ -266,6 +267,7 @@ CONFIG_SPI=y
CONFIG_SPI_MASTER=y
CONFIG_SPI_ORION=y
CONFIG_STOP_MACHINE=y
+CONFIG_SWCONFIG=y
CONFIG_SWIOTLB=y
# CONFIG_SWP_EMULATE is not set
CONFIG_SYS_SUPPORTS_APM_EMULATION=y
diff --git a/target/linux/mvebu/files/arch/arm/boot/dts/armada-xp-mamba.dts b/target/linux/mvebu/files/arch/arm/boot/dts/armada-xp-mamba.dts
index a3dc012..cae7d14 100644
--- a/target/linux/mvebu/files/arch/arm/boot/dts/armada-xp-mamba.dts
+++ b/target/linux/mvebu/files/arch/arm/boot/dts/armada-xp-mamba.dts
@@ -115,9 +115,9 @@
nr-ports = <1>;
status = "okay";
};
-
- mdio {
- status = "disabled";
+
+ mdio: mdio {
+ status = "okay";
};
ethernet@70000 {
@@ -274,4 +274,13 @@
gpio-fan,speed-map = <0 0
4500 1>;
};
+
+ mvsw6171 {
+ compatible = "marvell,88e6171";
+ status = "okay";
+
+ mii-bus = <&mdio>;
+ cpu-port-0 = <5>;
+ cpu-port-1 = <6>;
+ };
};
diff --git a/target/linux/mvebu/profiles/100-Generic.mk b/target/linux/mvebu/profiles/100-Generic.mk
index a692311..05d88b5 100644
--- a/target/linux/mvebu/profiles/100-Generic.mk
+++ b/target/linux/mvebu/profiles/100-Generic.mk
@@ -14,7 +14,7 @@ define Profile/Generic
kmod-rtc-marvell kmod-thermal-armada \
kmod-gpio-button-hotplug kmod-hwmon-tmp421 \
kmod-hwmon-gpiofan kmod-leds-tlc59116 \
- kmod-ledtrig-usbdev
+ kmod-ledtrig-usbdev swconfig
endef
define Profile/Generic/DescriptionGreat @mmilburn looking good!
leitec
commented
9 years ago Excellent. I forgot to mention this earlier, but I think this is also necessary for VLANs to work under 3.14:
otherwise you never get tagged packets. It's fixed in 3.18.
mmilburn
commented
9 years ago @leitec I'll check to see that these changes haven't been backported to 3.14.x (since it's LTS), and create a patch if necessary.
Given the default switch layout and the interface naming, it looks as though the cpu-port settings are correct. Port 5 is grouped with ports 1-3, the interface that has direct access to these ports is eth0. A similar case can be made for port 6. I have not thought of a better way to confirm the settings, but I think this reasoning is sufficient.
leitec
commented
9 years ago Whoops, I didn't think to check. Yeah, it looks like it's already fixed in newer 3.14.x.
mmilburn
commented
9 years ago Hmm, I can't seem to find it in 3.14.26. Made sure it wasn't applied as an OpenWRT patch with:
make target/linux/{clean,prepare} V=s QUILT=1
quilt push -aapplied platform specific patches are:
platform/001-build_mamba_dts.patch
platform/002-revert_i2c_delay.patch
platform/003-add_leds_tlc59116_driver.patch
platform/008-nand_warn_weak_ecc_strength.patch
platform/009-add_of_mtd_ecc_helpers.patch
platform/010-pxa3xx_nand_print_ECC_strength.patch
platform/011-pxa3xx_nand_clean_error_handling.patch
platform/012-pxa3xx_nand_use_ecc_info_from_dt.patch
platform/018-decouple_phy_id_and_address.patch
platform/019-add_fixed_phy_register.patch
platform/020-of_fixed_link_phy.patch
platform/021-mvneta_support_fixed_links.patch
platform/100-find_active_root.patchAlso:
git rev-parse HEAD
2420d1c6a3af7bcb8994d26ca01f3e6cb0602bc7Since I couldn't find evidence of the patch being applied to 3.14.x kernels, I'm building and testing a backport of the patch.
to be clear, I mean this patch: https://git.kernel.org/cgit/linux/kernel/git/stable/linux-stable.git/commit/drivers/net/ethernet/marvell/mvneta.c?id=817dbfa5d1bc276a72c1a577310382008e8aca0a
I'll be submitting my backport of the patch tonight to the openwrt-devel mailing list. If I don't get to it tonight, I'll submitting the patch for switch support to the openwrt-devel mailing list tomorrow.
Chadster766
commented
9 years ago Awesome!
mmilburn
commented
9 years ago backport submitted http://patchwork.ozlabs.org/patch/423845/
mmilburn
commented
9 years ago Ok, patch to turn on switch driver going out tomorrow.
leitec
commented
9 years ago Great.
The bad news is apparently 802.1q VLANs won't work anyway, since the 6171 driver doesn't handle port state/STU, a requirement of the 6172. Nikita Nazarenko pointed this out to me and posted a working driver:
http://patchwork.ozlabs.org/patch/423418/
The diff itself looks a little broken (long lines). I'll be merging it with the 6171 driver soon.
mmilburn
commented
9 years ago @leitec I will hold off on pushing the dts and config changes to openwrt-devel.
leitec
commented
9 years ago OK, I have four patches that should get things working posted here: http://staticky.com/mvsw/ These are patches against OpenWrt trunk. It shouldn't actually require changes to the DTS, but to keep things clean and consistent you can change mvsw6171 { to whatever you want (mvsw61xx { works, or anything else) and change compatible = "marvell,88e6171"; to compatible = "marvell,88e6172";
Everything works here on my 6171's, but they don't appear to require the STU stuff Nikita posted about. Hopefully this results in working 802.1q VLANs on the 6172.
mmilburn
commented
9 years ago Kaloz has added the dts changes: https://dev.openwrt.org/changeset/43854/
Chadster766
commented
9 years ago Nice, does that help with your switch driver or is that no longer needed.
mmilburn
commented
9 years ago CC already has @leitec's original mvsw6171 driver. The only thing left is to test his patches for 802.1q vlans.
mmilburn
commented
9 years ago Because it's taken me a while to get to this, I had to make a few trivial changes to the first patch:
(20:51:%) diff 0001-mvsw6171-rename-to-mvsw61xx.patch 0001-mvsw6171-rename-to-mvsw61xx.patch.new (2015/01/07)
2305c2305
< - obj-$(CONFIG_AR8216_PHY) += ar8216.o
---
> - obj-$(CONFIG_AR8216_PHY) += ar8216.o ar8327.o
2334c2334
< + obj-$(CONFIG_AR8216_PHY) += ar8216.o
---
> + obj-$(CONFIG_AR8216_PHY) += ar8216.o ar8327.o
2363c2363
< - obj-$(CONFIG_AR8216_PHY) += ar8216.o
---
> - obj-$(CONFIG_AR8216_PHY) += ar8216.o ar8327.o
2392c2392
< + obj-$(CONFIG_AR8216_PHY) += ar8216.o
---
> + obj-$(CONFIG_AR8216_PHY) += ar8216.o ar8327.oafter that everything applies cleanly. After I finish building an image, my plan for testing is as follows:
tcpdump -e -i to confirm tagged frames.Since I am not an expert on this, please let me know if you have a better way to test this. Or, if you think I am testing incorrectly.
Chadster766
commented
9 years ago If this is your driver mod than I will do VLAN testing with my equipment too.
leitec
commented
9 years ago Thanks for testing. I will refresh the first patch when I submit it.
Your testing procedure sounds good to me. Here's an excerpt from my config using a '6171 switch. I have tagged packets coming in and out from switch port 4 (WAN, on my hardware) and going tagged into port 6 (eth0 on my hardware) plus untagged on two of the LAN ports.
config switch
option name 'switch0'
option reset '1'
option enable_vlan '1'
config switch_vlan
option device 'switch0'
option vlan '11'
option ports '0 1 4t 6t'There's also a corresponding config for eth0.11 There's a further option, option vid under switch_vlan that sets the actual transmitted VID. It defaults to whatever vlan is set to, but in the case you want a VID > 64, you can set option vid '300' or whatever for eth0.300.
mmilburn
commented
9 years ago Did a simple test, tagging appears:
config switch
option name 'switch0'
option reset '1'
option enable_vlan '1'
config switch_vlan
option device 'switch0'
option vlan '1'
option ports '4 6t'tcpdump -eni eth1
...
03:25:02.315780 10:7b:ef:98:af:a4 > 01:00:5e:7f:ff:fa, ethertype 802.1Q (0x8100), length 406: vlan 1, p 0, ethertype IPv4, 192.168.0.1.1900 > 239.255.255.250.1900: UDP, length 360I was also able to set qmode and pvid with swconfig.
mmilburn
commented
9 years ago @leitec, let me know if I need to do more in depth testing. Things appear to work fine.
leitec
commented
9 years ago I think that's it. Thanks a lot for testing!
mmilburn
commented
9 years ago No problem. Thanks for all the help Claudio.
@Chadster766 I will close this ticket once Claudio's patches appear in trunk.
mmilburn
commented
9 years ago mmilburn
commented
9 years ago Changes applied to trunk. This is in CC now.
Hi Mark,
On Fri, Jan 09, 2015 at 04:21:10PM -0700, Mark Milburn wrote:
Use Claudio's renamed mvsw6171 driver. Depends upon https://patchwork.ozlabs.org/patch/427183/ patchset.
target/linux/mvebu/config-3.14 | 2 +- target/linux/mvebu/config-3.18 | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
Applied as a part of r43938. Thank you!
Luka
swconfig