Turris Omnia device-tree changes
v2 cover Turris Omnia device-tree changes
Hi Gregory,
v2 of series with changes for Turris Omnia device tree. It applies on mvebu-dt branch.
Changes from v1:
- added patch which adds description for switch interrupt
- removed patch adding ethernet-phy interrupt: the PHY interrupt is asserted by level low, but the GPIO expander driver supports only edge rising/falling, and even then it may not be correct when an interrupt storm occurs. So keep polling the PHY
- added Andrew's Reviewed-by tags
Marek
Marek Behún (6):
ARM: dts: turris-omnia: enable HW buffer management
ARM: dts: turris-omnia: add comphy handle to eth2
ARM: dts: turris-omnia: describe switch interrupt
ARM: dts: turris-omnia: add SFP node
ARM: dts: turris-omnia: add LED controller node
ARM: dts: turris-omnia: update ethernet-phy node and handle name
arch/arm/boot/dts/armada-385-turris-omnia.dts | 162 +++++++++++++++++- 1 file changed, 157 insertions(+), 5 deletions(-)
Signature · 1 line
2.26.2
PATCH v2 1/6 ARM: dts: turris-omnia: enable HW buffer management
The buffer manager is available on Turris Omnia but needs to be described in device-tree to be used.
--- arch/arm/boot/dts/armada-385-turris-omnia.dts | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)
arch/arm/boot/dts/armada-385-turris-omnia.dts +17 -0
@@ -84,12 +84,23 @@ pcie@3,0 { }; }; +&bm { + status = "okay"; +}; + +&bm_bppi { + status = "okay"; +}; + /* Connected to 88E6176 switch, port 6 */ ð0 { pinctrl-names = "default"; pinctrl-0 = <&ge0_rgmii_pins>; status = "okay"; phy-mode = "rgmii"; + buffer-manager = <&bm>; + bm,pool-long = <0>; + bm,pool-short = <3>; fixed-link { speed = <1000>;
@@ -103,6 +114,9 @@ ð1 { pinctrl-0 = <&ge1_rgmii_pins>; status = "okay"; phy-mode = "rgmii"; + buffer-manager = <&bm>; + bm,pool-long = <1>; + bm,pool-short = <3>; fixed-link { speed = <1000>;
@@ -115,6 +129,9 @@ ð2 { status = "okay"; phy-mode = "sgmii"; phy = <&phy1>; + buffer-manager = <&bm>; + bm,pool-long = <2>; + bm,pool-short = <3>; }; &i2c0 {
Signature · 1 line
2.26.2
PATCH v2 2/6 ARM: dts: turris-omnia: add comphy handle to eth2
In reply to Marek BehúnThe eth2 controller on Turris Omnia is connected to SerDes. For SFP to be able to switch between 1G ans 2.5G modes the comphy link has to be defined.
--- arch/arm/boot/dts/armada-385-turris-omnia.dts | 1 + 1 file changed, 1 insertion(+)
arch/arm/boot/dts/armada-385-turris-omnia.dts +1 -0
@@ -129,6 +129,7 @@ ð2 { status = "okay"; phy-mode = "sgmii"; phy = <&phy1>; + phys = <&comphy5 2>; buffer-manager = <&bm>; bm,pool-long = <2>; bm,pool-short = <3>;
Signature · 1 line
2.26.2
Re: [PATCH mvebu-dt v2 2/6] ARM: dts: turris-omnia: add comphy handle to eth2
Quoted 2 lines · Marek Behún
The eth2 controller on Turris Omnia is connected to SerDes. For SFP to be able to switch between 1G ans 2.5G modes the comphy link has to be
Typo "ans" is still present in v2, despite pointed out by Andrew.
Quoted 20 lines
defined.
Signed-off-by Marek Behún <[email protected]>Fixes f3a6a9f3704a ("ARM: dts: add description for Armada 38x ...")Reviewed-by Andrew Lunn <[email protected]>Cc Uwe Kleine-König <[email protected]>Cc Jason Cooper <[email protected]>Cc Gregory CLEMENT <[email protected]>Cc Andreas Färber <[email protected]>Cc Rob Herring <[email protected]>--- arch/arm/boot/dts/armada-385-turris-omnia.dts | 1 + 1 file changed, 1 insertion(+)arch/arm/boot/dts/armada-385-turris-omnia.dts +0
-0
arch/arm/boot/dts/armada-385-turris-omnia.dts ð2 { · line 135
Quoted 7 lines
arch/arm/boot/dts/armada-385-turris-omnia.dts +1
-0@@ -129,6 +129,7 @@ ð2 { status = "okay"; phy-mode = "sgmii"; phy = <&phy1>; + phys = <&comphy5 2>; buffer-manager = <&bm>; bm,pool-long = <2>; bm,pool-short = <3>;
Property matches what I've come up with,
Thanks,
Andreas
Signature · 4 lines
SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Felix Imendörffer HRB 36809 (AG Nürnberg)
PATCH v2 3/6 ARM: dts: turris-omnia: describe switch interrupt
In reply to Marek BehúnDescribe switch interrupt for Turris Omnia so that the CPU does not have to poll the switch. We also need to to set mpp45 pin to gpio function for this.
--- arch/arm/boot/dts/armada-385-turris-omnia.dts | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-)
arch/arm/boot/dts/armada-385-turris-omnia.dts +11 -1
@@ -260,13 +260,18 @@ phy1: phy@1 { /* Switch MV88E6176 at address 0x10 */ switch@10 { + pinctrl-names = "default"; + pinctrl-0 = <&swint_pins>; compatible = "marvell,mv88e6085"; #address-cells = <1>; #size-cells = <0>; - dsa,member = <0 0>; + dsa,member = <0 0>; reg = <0x10>; + interrupt-parent = <&gpio1>; + interrupts = <13 IRQ_TYPE_LEVEL_LOW>; + ports { #address-cells = <1>; #size-cells = <0>;
@@ -319,6 +324,11 @@ pcawan_pins: pcawan-pins { marvell,function = "gpio"; }; + swint_pins: swint-pins { + marvell,pins = "mpp45"; + marvell,function = "gpio"; + }; + spi0cs0_pins: spi0cs0-pins { marvell,pins = "mpp25"; marvell,function = "spi0";
Signature · 1 line
2.26.2
PATCH v2 4/6 ARM: dts: turris-omnia: add SFP node
In reply to Marek BehúnTurris Omnia has a SFP cage that, together with WAN PHY is connected to eth2 SerDes via a SerDes multiplexor. Describe the SFP cage, but leave it disabled. Until phylink has support for such multiplexor we will leave it to U-Boot to enable SFP and disable WAN PHY at boot time depending on whether a SFP module is present.
--- arch/arm/boot/dts/armada-385-turris-omnia.dts | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-)
arch/arm/boot/dts/armada-385-turris-omnia.dts +18 -1
@@ -82,6 +82,22 @@ pcie@3,0 { }; }; }; + + sfp: sfp { + compatible = "sff,sfp"; + i2c-bus = <&sfp_i2c>; + tx-fault-gpios = <&pcawan 0 GPIO_ACTIVE_HIGH>; + tx-disable-gpios = <&pcawan 1 GPIO_ACTIVE_HIGH>; + rate-select0-gpios = <&pcawan 2 GPIO_ACTIVE_HIGH>; + los-gpios = <&pcawan 3 GPIO_ACTIVE_HIGH>; + mod-def0-gpios = <&pcawan 4 GPIO_ACTIVE_LOW>; + + /* + * For now this has to be enabled at boot time by U-Boot when + * a SFP module is present. + */ + status = "disabled"; + }; }; &bm {
@@ -130,6 +146,7 @@ ð2 { phy-mode = "sgmii"; phy = <&phy1>; phys = <&comphy5 2>; + sfp = <&sfp>; buffer-manager = <&bm>; bm,pool-long = <2>; bm,pool-short = <3>;
@@ -195,7 +212,7 @@ i2c@3 { /* routed to PCIe2 connector (CN62A) */ }; - i2c@4 { + sfp_i2c: i2c@4 { #address-cells = <1>; #size-cells = <0>; reg = <4>;
Signature · 1 line
2.26.2
Re: [PATCH mvebu-dt v2 4/6] ARM: dts: turris-omnia: add SFP node
Hi Marek,
Quoted 1 line · Marek Behún
Turris Omnia has a SFP cage that, together with WAN PHY is connected to
"an SFP"
Comma missing after PHY (or drop before together).
Quoted 4 lines
eth2 SerDes via a SerDes multiplexor. Describe the SFP cage, but leave it disabled. Until phylink has support for such multiplexor we will leave it to U-Boot to enable SFP and disable WAN PHY at boot time depending on whether a SFP module is present.
multiplexor vs. multiplexer may be a British thing? Thunderbird underlines it fwiw.
Quoted 50 lines
Signed-off-by Marek Behún <[email protected]>Fixes 26ca8b52d6e1 ("ARM: dts: add support for Turris Omnia")Reviewed-by Andrew Lunn <[email protected]>Cc Uwe Kleine-König <[email protected]>Cc Jason Cooper <[email protected]>Cc Gregory CLEMENT <[email protected]>Cc Andreas Färber <[email protected]>Cc Rob Herring <[email protected]>--- arch/arm/boot/dts/armada-385-turris-omnia.dts | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-)arch/arm/boot/dts/armada-385-turris-omnia.dts +17
-0@@ -82,6 +82,22 @@ pcie@3,0 { }; }; }; + + sfp: sfp { + compatible = "sff,sfp"; + i2c-bus = <&sfp_i2c>; + tx-fault-gpios = <&pcawan 0 GPIO_ACTIVE_HIGH>; + tx-disable-gpios = <&pcawan 1 GPIO_ACTIVE_HIGH>; + rate-select0-gpios = <&pcawan 2 GPIO_ACTIVE_HIGH>; + los-gpios = <&pcawan 3 GPIO_ACTIVE_HIGH>; + mod-def0-gpios = <&pcawan 4 GPIO_ACTIVE_LOW>; + + /* + * For now this has to be enabled at boot time by U-Boot when + * a SFP module is present. + */ + status = "disabled"; + }; }; &bm {@@ -130,6 +146,7 @@ ð2 { phy-mode = "sgmii"; phy = <&phy1>; phys = <&comphy5 2>; + sfp = <&sfp>; buffer-manager = <&bm>; bm,pool-long = <2>; bm,pool-short = <3>;
arch/arm/boot/dts/armada-385-turris-omnia.dts i2c@3 { · line 218
Quoted 8 lines
arch/arm/boot/dts/armada-385-turris-omnia.dts +1
-1@@ -195,7 +212,7 @@ i2c@3 { /* routed to PCIe2 connector (CN62A) */ }; - i2c@4 { + sfp_i2c: i2c@4 { #address-cells = <1>; #size-cells = <0>; reg = <4>;
Matches what I've come up with,
However, I also needed to set managed = "in-band-status" when enabling SFP node and removing phy property. Shall we prepare it with its default value of "auto" and add a comment? (unlike disabled -> okay, in-band-status is longer than auto, so not sure whether it helps U-Boot, but it may help humans.
Thanks,
Andreas
Signature · 4 lines
SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Felix Imendörffer HRB 36809 (AG Nürnberg)
Re: [PATCH mvebu-dt v2 4/6] ARM: dts: turris-omnia: add SFP node
Quoted 8 lines · Andreas Färber
Hi Marek,
Quoted 1 line · Marek Behún
Turris Omnia has a SFP cage that, together with WAN PHY is connected to
"an SFP"
Comma missing after PHY (or drop before together).
Quoted 7 lines
Quoted 4 lines
eth2 SerDes via a SerDes multiplexor. Describe the SFP cage, but leave it disabled. Until phylink has support for such multiplexor we will leave it to U-Boot to enable SFP and disable WAN PHY at boot time depending on whether a SFP module is present.
multiplexor vs. multiplexer may be a British thing? Thunderbird underlines it fwiw.
Why doesn't someone who has a Turris Omnia come up with the support needed for this board, such as sending a patch to add support for this multiplexer?
Signature · 2 lines
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH mvebu-dt v2 4/6] ARM: dts: turris-omnia: add SFP node
Quoted 14 lines · Russell King - ARM Linux admin
Quoted 13 lines · Andreas Färber
Hi Marek,
Quoted 1 line · Marek Behún
Turris Omnia has a SFP cage that, together with WAN PHY is connected to
"an SFP"
Comma missing after PHY (or drop before together).Quoted 4 lines
eth2 SerDes via a SerDes multiplexor. Describe the SFP cage, but leave it disabled. Until phylink has support for such multiplexor we will leave it to U-Boot to enable SFP and disable WAN PHY at boot time depending on whether a SFP module is present.
Quoted 6 lines
Quoted 2 lines
multiplexor vs. multiplexer may be a British thing? Thunderbird underlines it fwiw.
Why doesn't someone who has a Turris Omnia come up with the support needed for this board, such as sending a patch to add support for this multiplexer?
Since you have me in To: Because I've already spent over a week on other patches just to get a 2.5G SFP recognized, which still doesn't work.
Cheers,
Andreas
Signature · 4 lines
SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Felix Imendörffer HRB 36809 (AG Nürnberg)
Re: [PATCH mvebu-dt v2 4/6] ARM: dts: turris-omnia: add SFP node
In reply to Russell King - ARM Linux adminQuoted 14 lines · On Sat, 14 Nov 2020 21:42:04 +0000 Russell King - ARM Linux admin…
Quoted 13 lines · Andreas Färber
Hi Marek,
Quoted 1 line · Marek Behún
Turris Omnia has a SFP cage that, together with WAN PHY is connected to
"an SFP"
Comma missing after PHY (or drop before together).Quoted 4 lines
eth2 SerDes via a SerDes multiplexor. Describe the SFP cage, but leave it disabled. Until phylink has support for such multiplexor we will leave it to U-Boot to enable SFP and disable WAN PHY at boot time depending on whether a SFP module is present.
Quoted 7 lines
Quoted 2 lines
multiplexor vs. multiplexer may be a British thing? Thunderbird underlines it fwiw.
Why doesn't someone who has a Turris Omnia come up with the support needed for this board, such as sending a patch to add support for this multiplexer?
Russell, I have this planned, but it is a bit complicated. We discussed this maybe 1 or 2 years ago.
The thing is that phylink does not support such a thing nor is there a simple way to add such support to it.
One problem we discussed last time is the correct DT binding. Should it be something like this?
eth2 {
phy-mode = "sgmii";
multiplexer {
gpio = <&mod_def0_gpio>;
option@0 {
reg = <0>;
phy-handle = <&phy1>;
};
option@1 {
reg = <1>;
sfp = <&sfp>;
managed = "in-band-status";
};
};};
But who should handle this? Phylink, SFP, or maybe this should be handled generically in OF / fwnode subsystem? I.e. a change in GPIO value could change device's properties/children?
Also the &mod_def0_gpio is already used by the sfp node. Can this multiplexor node also refer to it?
Marek
Re: [PATCH mvebu-dt v2 4/6] ARM: dts: turris-omnia: add SFP node
Quoted 25 lines · Marek Behún
Quoted 14 lines · On Sat, 14 Nov 2020 21:42:04 +0000 Russell King - ARM Linux admin…
Quoted 13 lines · Andreas Färber
Hi Marek,
Quoted 1 line · Marek Behún
Turris Omnia has a SFP cage that, together with WAN PHY is connected to
"an SFP"
Comma missing after PHY (or drop before together).Quoted 4 lines
eth2 SerDes via a SerDes multiplexor. Describe the SFP cage, but leave it disabled. Until phylink has support for such multiplexor we will leave it to U-Boot to enable SFP and disable WAN PHY at boot time depending on whether a SFP module is present.
Quoted 7 lines
Quoted 2 lines
multiplexor vs. multiplexer may be a British thing? Thunderbird underlines it fwiw.
Why doesn't someone who has a Turris Omnia come up with the support needed for this board, such as sending a patch to add support for this multiplexer?
Quoted 5 lines
Russell, I have this planned, but it is a bit complicated. We discussed this maybe 1 or 2 years ago.
The thing is that phylink does not support such a thing nor is there a simple way to add such support to it.
What I'm saying is that hacking around this problem doesn't actually solve it, and phylink is not going to suddenly grow support for this. It's not something I'm planning to work on adding support for.
Quoted 19 lines
One problem we discussed last time is the correct DT binding. Should it be something like this?
eth2 { phy-mode = "sgmii"; multiplexer { gpio = <&mod_def0_gpio>; option@0 { reg = <0>; phy-handle = <&phy1>; }; option@1 { reg = <1>; sfp = <&sfp>; managed = "in-band-status"; }; };
Quoted 1 line
};
See comments below.
Quoted 3 lines
But who should handle this? Phylink, SFP, or maybe this should be handled generically in OF / fwnode subsystem? I.e. a change in GPIO value could change device's properties/children?
The DT folk will object to it being handled in the OF specific code, it seems very obvious to me that would be the case. The OF specific code is really just a library to parse the DT descriptions, it has very little complexity to it, and it makes no decisions about what to parse depending on external inputs. Also, it's technically infeasible when you consider what it would take to implement it at that level.
Let's say for argument that it was implemented in the OF specific code. What would it take? Well, when the GPIO changes state, we would have to cause the code parsing this eth2 node to be re-run. The only way to do that today would be to unbind the driver for eth2 and rebind it. That will mean that anything setup with eth2 (e.g. interface renaming) would be lost. It would also be very noisy in the kernel log, and would likely take quite some time to process. The OF specific code would need to know to redirect a lookup of "managed", "sfp", "phy" etc in the eth2 node into the relative sub-node.
This is clearly not a feasible idea for something that dynamically changes.
It doesn't make sense to try to do any of this at the SFP layer. The SFP layer just concerns itself with the SFP cage, the inserted module, and nothing more. It doesn't have any knowledge of what it is connected to on the host side, nor should it.
That leaves phylink, which is told via the SFP layer when a module is inserted or removed. It isn't told in real-time as the MOD_DEF0 signal changes state however, only after the module has been successfully detected. This leads to a race condition where phylink can think that a module is not inserted, but it really is, and the link comes up on your PHY, but that PHY is no longer connected to the ethernet controller because MOD_DEF0 has changed state.
The only way around that is to have phylink check the state of MOD_DEF0 itself each time _before_ it uses the results of the on-board PHY. However, phylink can't know the state of MOD_DEF0 (see below.)
Quoted 2 lines
Also the
&mod_def0_gpiois already used by the sfp node. Can this multiplexor node also refer to it?
The GPIO library does not allow multiple users of a single GPIO, so this would not work.
I know this is not an easy problem to solve, but I don't think hacking around the problem in the way that is being done in this patch set is really helping.
Having an ethernet node with both a SFP cage and a phy will cause problems when a SFP is detected containing a PHY. The two struct phy_device pointers, one that is maintained by phylink and another in struct net_device, will both be overwritten with the SFP PHY, losing the on-board PHY. This will result in a memory and resource leak at minimum.
At minimum, the first thing that needs to happen in phylink for this is we split the pointer for the on-board PHY from the SFP module PHY, so we can keep track of both. We then need to deal with the struct net_device phy pointer, setting it as appropriate depending on whether a module is inserted. That should get us to a point where your proposed DT patch containing both the SFP and PHY can be used, and the kernel will make the right decision, except for the MOD_DEF0 issue I mention above. Solving that is going to be hard, because it seems specific to your platform, and I don't have an idea that would be feasible at the moment.
Signature · 2 lines
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH mvebu-dt v2 4/6] ARM: dts: turris-omnia: add SFP node
In reply to Andreas FärberQuoted 77 lines · On Sat, 14 Nov 2020 22:36:09 +0100 Andreas Färber <[email protected]> wrote:
Hi Marek,
Quoted 1 line · Marek Behún
Turris Omnia has a SFP cage that, together with WAN PHY is connected to
"an SFP"
Comma missing after PHY (or drop before together).Quoted 4 lines
eth2 SerDes via a SerDes multiplexor. Describe the SFP cage, but leave it disabled. Until phylink has support for such multiplexor we will leave it to U-Boot to enable SFP and disable WAN PHY at boot time depending on whether a SFP module is present.
multiplexor vs. multiplexer may be a British thing? Thunderbird underlines it fwiw.
Quoted 50 lines
Signed-off-by Marek Behún <[email protected]>Fixes 26ca8b52d6e1 ("ARM: dts: add support for Turris Omnia")Reviewed-by Andrew Lunn <[email protected]>Cc Uwe Kleine-König <[email protected]>Cc Jason Cooper <[email protected]>Cc Gregory CLEMENT <[email protected]>Cc Andreas Färber <[email protected]>Cc Rob Herring <[email protected]>--- arch/arm/boot/dts/armada-385-turris-omnia.dts | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-)arch/arm/boot/dts/armada-385-turris-omnia.dts +17
-0@@ -82,6 +82,22 @@ pcie@3,0 { }; }; }; + + sfp: sfp { + compatible = "sff,sfp"; + i2c-bus = <&sfp_i2c>; + tx-fault-gpios = <&pcawan 0 GPIO_ACTIVE_HIGH>; + tx-disable-gpios = <&pcawan 1 GPIO_ACTIVE_HIGH>; + rate-select0-gpios = <&pcawan 2 GPIO_ACTIVE_HIGH>; + los-gpios = <&pcawan 3 GPIO_ACTIVE_HIGH>; + mod-def0-gpios = <&pcawan 4 GPIO_ACTIVE_LOW>; + + /* + * For now this has to be enabled at boot time by U-Boot when + * a SFP module is present. + */ + status = "disabled"; + }; }; &bm {@@ -130,6 +146,7 @@ ð2 { phy-mode = "sgmii"; phy = <&phy1>; phys = <&comphy5 2>; + sfp = <&sfp>; buffer-manager = <&bm>; bm,pool-long = <2>; bm,pool-short = <3>;arch/arm/boot/dts/armada-385-turris-omnia.dts i2c@3 { · line 218
Quoted 8 lines
arch/arm/boot/dts/armada-385-turris-omnia.dts +1
-1@@ -195,7 +212,7 @@ i2c@3 { /* routed to PCIe2 connector (CN62A) */ }; - i2c@4 { + sfp_i2c: i2c@4 { #address-cells = <1>; #size-cells = <0>; reg = <4>;Matches what I've come up with,
Quoted 7 lines
Reviewed-by Andreas Färber <[email protected]>However, I also needed to set managed = "in-band-status" when enabling SFP node and removing phy property. Shall we prepare it with its default value of "auto" and add a comment? (unlike disabled -> okay, in-band-status is longer than auto, so not sure whether it helps U-Boot, but it may help humans.
The idea is that for now when U-Boot detects that SFP is present, it shall change the device tree accordingly:
remove phy-handle
add managed = in-band-status
enable sfp node
This is the only way to support this reasobaly until the multiplexer is somehow supported by kernel.
Marek
Re: [PATCH mvebu-dt v2 4/6] ARM: dts: turris-omnia: add SFP node
Quoted 88 lines · Marek Behún
Quoted 77 lines · On Sat, 14 Nov 2020 22:36:09 +0100 Andreas Färber <[email protected]> wrote:
Hi Marek,
Quoted 1 line · Marek Behún
Turris Omnia has a SFP cage that, together with WAN PHY is connected to
"an SFP"
Comma missing after PHY (or drop before together).Quoted 4 lines
eth2 SerDes via a SerDes multiplexor. Describe the SFP cage, but leave it disabled. Until phylink has support for such multiplexor we will leave it to U-Boot to enable SFP and disable WAN PHY at boot time depending on whether a SFP module is present.
multiplexor vs. multiplexer may be a British thing? Thunderbird underlines it fwiw.
Quoted 50 lines
Signed-off-by Marek Behún <[email protected]>Fixes 26ca8b52d6e1 ("ARM: dts: add support for Turris Omnia")Reviewed-by Andrew Lunn <[email protected]>Cc Uwe Kleine-König <[email protected]>Cc Jason Cooper <[email protected]>Cc Gregory CLEMENT <[email protected]>Cc Andreas Färber <[email protected]>Cc Rob Herring <[email protected]>--- arch/arm/boot/dts/armada-385-turris-omnia.dts | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-)arch/arm/boot/dts/armada-385-turris-omnia.dts +17
-0@@ -82,6 +82,22 @@ pcie@3,0 { }; }; }; + + sfp: sfp { + compatible = "sff,sfp"; + i2c-bus = <&sfp_i2c>; + tx-fault-gpios = <&pcawan 0 GPIO_ACTIVE_HIGH>; + tx-disable-gpios = <&pcawan 1 GPIO_ACTIVE_HIGH>; + rate-select0-gpios = <&pcawan 2 GPIO_ACTIVE_HIGH>; + los-gpios = <&pcawan 3 GPIO_ACTIVE_HIGH>; + mod-def0-gpios = <&pcawan 4 GPIO_ACTIVE_LOW>; + + /* + * For now this has to be enabled at boot time by U-Boot when + * a SFP module is present. + */ + status = "disabled"; + }; }; &bm {@@ -130,6 +146,7 @@ ð2 { phy-mode = "sgmii"; phy = <&phy1>; phys = <&comphy5 2>; + sfp = <&sfp>; buffer-manager = <&bm>; bm,pool-long = <2>; bm,pool-short = <3>;arch/arm/boot/dts/armada-385-turris-omnia.dts i2c@3 { · line 218
Quoted 8 lines
arch/arm/boot/dts/armada-385-turris-omnia.dts +1
-1@@ -195,7 +212,7 @@ i2c@3 { /* routed to PCIe2 connector (CN62A) */ }; - i2c@4 { + sfp_i2c: i2c@4 { #address-cells = <1>; #size-cells = <0>; reg = <4>;Matches what I've come up with,
Quoted 7 lines
Reviewed-by Andreas Färber <[email protected]>However, I also needed to set managed = "in-band-status" when enabling SFP node and removing phy property. Shall we prepare it with its default value of "auto" and add a comment? (unlike disabled -> okay, in-band-status is longer than auto, so not sure whether it helps U-Boot, but it may help humans.
Quoted 8 lines
The idea is that for now when U-Boot detects that SFP is present, it shall change the device tree accordingly:
remove phy-handle
add managed = in-band-status
enable sfp nodeThis is the only way to support this reasobaly until the multiplexer is somehow supported by kernel.
I do understand the idea. My point was that you added a 4-line comment about status property further above, but no comments about phy-handle nor managed properties down here.
It might also be a good idea to explain in a comment why they are mutually exclusive (mod-def0, multiplexer).
Have you done any debugging as to why we can't just leave the sfp node enabled? Does it toggle mod-def0-gpios on probe even if no SFP is physically present on i2c? Maybe it can be simplified over in sfp code?
Regards,
Andreas
Signature · 4 lines
SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Felix Imendörffer HRB 36809 (AG Nürnberg)
Re: [PATCH mvebu-dt v2 4/6] ARM: dts: turris-omnia: add SFP node
Quoted 9 lines · On Sun, 15 Nov 2020 00:16:48 +0100 Andreas Färber <[email protected]> wrote:
I do understand the idea. My point was that you added a 4-line comment about status property further above, but no comments about phy-handle nor managed properties down here.
It might also be a good idea to explain in a comment why they are mutually exclusive (mod-def0, multiplexer).
Have you done any debugging as to why we can't just leave the sfp node enabled?
Current phylink code will, in such situation, try to connect both phy and sfp. It will try to configure settings from both, depending on when last interrupt came from PHY and when SFP module state changed. It will break.
Also if there is another PHY in the SFP module, it won't be able to get connected.
It is simpler to leave sfp node disabled and get full support for this configuration into kernel first.
Quoted 2 lines
Does it toggle mod-def0-gpios on probe even if no SFP is physically present on i2c? Maybe it can be simplified over in sfp code?
mod-deg0-gpio is not toggled by kernel. It is an input gpio. Its state determines whether sfp module is present.
Re: [PATCH mvebu-dt v2 4/6] ARM: dts: turris-omnia: add SFP node
In reply to Andreas FärberAndreas, is the version below satisfactory?
Marek
arch/arm/boot/dts/armada-385-turris-omnia.dts +28 -1
@@ -82,6 +82,23 @@ pcie@3,0 { }; }; }; + + sfp: sfp { + compatible = "sff,sfp"; + i2c-bus = <&sfp_i2c>; + tx-fault-gpios = <&pcawan 0 GPIO_ACTIVE_HIGH>; + tx-disable-gpios = <&pcawan 1 GPIO_ACTIVE_HIGH>; + rate-select0-gpios = <&pcawan 2 GPIO_ACTIVE_HIGH>; + los-gpios = <&pcawan 3 GPIO_ACTIVE_HIGH>; + mod-def0-gpios = <&pcawan 4 GPIO_ACTIVE_LOW>; + + /* + * For now this has to be enabled at boot time by U-Boot when + * a SFP module is present. Read more in the comment in the + * eth2 node below. + */ + status = "disabled"; + }; }; &bm {
@@ -126,10 +143,20 @@ fixed-link { /* WAN port */ ð2 { + /* + * eth2 is connected via a multiplexor to both the SFP cage and to + * ethernet-phy@1. The multiplexor switches the signal to SFP cage when + * a SFP module is present, as determined by the mode-def0 GPIO. + * + * Until kernel supports this configuration properly, in case SFP module + * is present, U-Boot has to enable the sfp node above, remove phy + * handle and add managed = "in-band-status" property. + */ status = "okay"; phy-mode = "sgmii"; phy = <&phy1>; phys = <&comphy5 2>; + sfp = <&sfp>; buffer-manager = <&bm>; bm,pool-long = <2>; bm,pool-short = <3>;
@@ -195,7 +222,7 @@ i2c@3 { /* routed to PCIe2 connector (CN62A) */ }; - i2c@4 { + sfp_i2c: i2c@4 { #address-cells = <1>; #size-cells = <0>; reg = <4>;
Re: [PATCH mvebu-dt v2 4/6] ARM: dts: turris-omnia: add SFP node
In reply to Andreas FärberQuoted 42 lines · Andreas Färber
Hi Marek,
Quoted 39 lines · Marek Behún
arch/arm/boot/dts/armada-385-turris-omnia.dts +17
-0@@ -82,6 +82,22 @@ pcie@3,0 { }; }; }; + + sfp: sfp { + compatible = "sff,sfp"; + i2c-bus = <&sfp_i2c>; + tx-fault-gpios = <&pcawan 0 GPIO_ACTIVE_HIGH>; + tx-disable-gpios = <&pcawan 1 GPIO_ACTIVE_HIGH>; + rate-select0-gpios = <&pcawan 2 GPIO_ACTIVE_HIGH>; + los-gpios = <&pcawan 3 GPIO_ACTIVE_HIGH>; + mod-def0-gpios = <&pcawan 4 GPIO_ACTIVE_LOW>; + + /* + * For now this has to be enabled at boot time by U-Boot when + * a SFP module is present. + */ + status = "disabled"; + }; }; &bm {@@ -130,6 +146,7 @@ ð2 { phy-mode = "sgmii"; phy = <&phy1>; phys = <&comphy5 2>; + sfp = <&sfp>; buffer-manager = <&bm>; bm,pool-long = <2>; bm,pool-short = <3>;@@ -195,7 +212,7 @@ i2c@3 { /* routed to PCIe2 connector (CN62A) */ };
Quoted 7 lines
arch/arm/boot/dts/armada-385-turris-omnia.dts i2c@3 { · line 218
Quoted 5 lines
arch/arm/boot/dts/armada-385-turris-omnia.dts +1
-1- i2c@4 { + sfp_i2c: i2c@4 { #address-cells = <1>; #size-cells = <0>; reg = <4>;Matches what I've come up with,
Actually one minor diff: As I had pointed out to you privately before, I set maximum-power-milliwatt to its max of 2000.
Does your hardware only support the default of 1000, or are you still unsure of the limit that you've omitted it here? Not sure if it makes a difference in practice, but it does show up in dmesg.
Regards,
Andreas
Signature · 4 lines
SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Felix Imendörffer HRB 36809 (AG Nürnberg)
Re: [PATCH mvebu-dt v2 4/6] ARM: dts: turris-omnia: add SFP node
Quoted 6 lines
Actually one minor diff: As I had pointed out to you privately before, I set maximum-power-milliwatt to its max of 2000.
Does your hardware only support the default of 1000, or are you still unsure of the limit that you've omitted it here? Not sure if it makes a difference in practice, but it does show up in dmesg.
I am unsure, I have to ask HW guys what power is guaranteed.
Marek
PATCH v2 5/6 ARM: dts: turris-omnia: add LED controller node
In reply to Marek BehúnLinux now has incomplete support for the LED controller on Turris Omnia:
it can set brightness and colors for each LED.
The controller can also put these LEDs into HW controlled mode, in which the LEDs are controlled by HW: for example the WAN LED is connected via MCU to the WAN PHY LED pin.
The driver does not support these HW controlled modes yet, and on probe puts the LEDs into SW controlled mode.
Add node describing the LED controller, but disable it for now.
--- arch/arm/boot/dts/armada-385-turris-omnia.dts | 107 ++++++++++++++++++ 1 file changed, 107 insertions(+)
arch/arm/boot/dts/armada-385-turris-omnia.dts +107 -0
@@ -12,6 +12,7 @@ #include <dt-bindings/gpio/gpio.h> #include <dt-bindings/input/input.h> +#include <dt-bindings/leds/common.h> #include "armada-385.dtsi" / {
@@ -172,6 +173,112 @@ i2c@0 { /* STM32F0 command interface at address 0x2a */ /* leds device (in STM32F0) at address 0x2b */ + led-controller@2b { + compatible = "cznic,turris-omnia-leds"; + reg = <0x2b>; + #address-cells = <1>; + #size-cells = <0>; + + /* + * The driver does not support HW control mode + * for the LEDs yet. Disable the LEDs for now. + * + * Also LED functions are not stable yet: + * - there are 3 LEDs connected via MCU to PCIe + * ports. One of these ports supports mSATA. + * There is no mSATA nor PCIe function. + * For now we use LED_FUNCITION_WLAN, since + * in most cases users have wifi cards in + * these slots + * - there are 2 LEDs dedicated for user: A and + * B. Again there is no such function defined. + * For now we use LED_FUNCTION_DEBUG + */ + status = "disabled"; + + multi-led@0 { + reg = <0x0>; + color = <LED_COLOR_ID_RGB>; + function = LED_FUNCTION_DEBUG; + function-enumerator = <2>; + }; + + multi-led@1 { + reg = <0x1>; + color = <LED_COLOR_ID_RGB>; + function = LED_FUNCTION_DEBUG; + function-enumerator = <1>; + }; + + multi-led@2 { + reg = <0x2>; + color = <LED_COLOR_ID_RGB>; + function = LED_FUNCTION_WLAN; + function-enumerator = <3>; + }; + + multi-led@3 { + reg = <0x3>; + color = <LED_COLOR_ID_RGB>; + function = LED_FUNCTION_WLAN; + function-enumerator = <2>; + }; + + multi-led@4 { + reg = <0x4>; + color = <LED_COLOR_ID_RGB>; + function = LED_FUNCTION_WLAN; + function-enumerator = <1>; + }; + + multi-led@5 { + reg = <0x5>; + color = <LED_COLOR_ID_RGB>; + function = LED_FUNCTION_WAN; + }; + + multi-led@6 { + reg = <0x6>; + color = <LED_COLOR_ID_RGB>; + function = LED_FUNCTION_LAN; + function-enumerator = <4>; + }; + + multi-led@7 { + reg = <0x7>; + color = <LED_COLOR_ID_RGB>; + function = LED_FUNCTION_LAN; + function-enumerator = <3>; + }; + + multi-led@8 { + reg = <0x8>; + color = <LED_COLOR_ID_RGB>; + function = LED_FUNCTION_LAN; + function-enumerator = <2>; + }; + + multi-led@9 { + reg = <0x9>; + color = <LED_COLOR_ID_RGB>; + function = LED_FUNCTION_LAN; + function-enumerator = <1>; + }; + + multi-led@a { + reg = <0xa>; + color = <LED_COLOR_ID_RGB>; + function = LED_FUNCTION_LAN; + function-enumerator = <0>; + }; + + multi-led@b { + reg = <0xb>; + color = <LED_COLOR_ID_RGB>; + function = LED_FUNCTION_POWER; + }; + }; + eeprom@54 { compatible = "atmel,24c64"; reg = <0x54>;
Signature · 1 line
2.26.2
Re: [PATCH mvebu-dt v2 5/6] ARM: dts: turris-omnia: add LED controller node
Hi Marek,
Quoted 36 lines · Marek Behún
Linux now has incomplete support for the LED controller on Turris Omnia:
it can set brightness and colors for each LED.The controller can also put these LEDs into HW controlled mode, in which the LEDs are controlled by HW: for example the WAN LED is connected via MCU to the WAN PHY LED pin.
The driver does not support these HW controlled modes yet, and on probe puts the LEDs into SW controlled mode.
Add node describing the LED controller, but disable it for now.
Signed-off-by Marek Behún <[email protected]>Cc Uwe Kleine-König <[email protected]>Cc Jason Cooper <[email protected]>Cc Gregory CLEMENT <[email protected]>Cc Andreas Färber <[email protected]>Cc Rob Herring <[email protected]>--- arch/arm/boot/dts/armada-385-turris-omnia.dts | 107 ++++++++++++++++++ 1 file changed, 107 insertions(+)arch/arm/boot/dts/armada-385-turris-omnia.dts +1
-0@@ -12,6 +12,7 @@ #include <dt-bindings/gpio/gpio.h> #include <dt-bindings/input/input.h> +#include <dt-bindings/leds/common.h> #include "armada-385.dtsi" / {
arch/arm/boot/dts/armada-385-turris-omnia.dts i2c@0 { · line 174
Quoted 2 lines
arch/arm/boot/dts/armada-385-turris-omnia.dts +0
-0@@ -172,6 +173,112 @@ i2c@0 { /* STM32F0 command interface at address 0x2a */ /* leds device (in STM32F0) at address 0x2b */
Update and move this comment now that the node documents the address?
Quoted 6 lines
arch/arm/boot/dts/armada-385-turris-omnia.dts +5
-0+ led-controller@2b { + compatible = "cznic,turris-omnia-leds"; + reg = <0x2b>; + #address-cells = <1>; + #size-cells = <0>;
arch/arm/boot/dts/armada-385-turris-omnia.dts i2c@0 { · line 190
Quoted 10 lines
arch/arm/boot/dts/armada-385-turris-omnia.dts +10
-0+ + /* + * The driver does not support HW control mode + * for the LEDs yet. Disable the LEDs for now. + * + * Also LED functions are not stable yet: + * - there are 3 LEDs connected via MCU to PCIe + * ports. One of these ports supports mSATA. + * There is no mSATA nor PCIe function. + * For now we use LED_FUNCITION_WLAN, since
FUNCTION
arch/arm/boot/dts/armada-385-turris-omnia.dts i2c@0 { · line 192
Quoted 2 lines
arch/arm/boot/dts/armada-385-turris-omnia.dts +2
-0+ * in most cases users have wifi cards in + * these slots
Doesn't U-Boot detect mSATA and switches SerDes configuration? You could then have it set LED_FUNCTION_DISK in case of mSATA detected.
I recently didn't find any DT binding for the netdev LED trigger, but you could set trigger-sources to associate the LEDS with PCIe nodes even if unused. Same for the LAN LEDs and switch port nodes, if you give them labels.
arch/arm/boot/dts/armada-385-turris-omnia.dts i2c@0 { · line 195
Quoted 3 lines
arch/arm/boot/dts/armada-385-turris-omnia.dts +3
-0+ * - there are 2 LEDs dedicated for user: A and + * B. Again there is no such function defined. + * For now we use LED_FUNCTION_DEBUG
I'd suggest the more neutral LED_FUNCTION_INDICATOR.
Cheers,
Andreas
Quoted 90 lines
arch/arm/boot/dts/armada-385-turris-omnia.dts +86
-0+ */ + status = "disabled"; + + multi-led@0 { + reg = <0x0>; + color = <LED_COLOR_ID_RGB>; + function = LED_FUNCTION_DEBUG; + function-enumerator = <2>; + }; + + multi-led@1 { + reg = <0x1>; + color = <LED_COLOR_ID_RGB>; + function = LED_FUNCTION_DEBUG; + function-enumerator = <1>; + }; + + multi-led@2 { + reg = <0x2>; + color = <LED_COLOR_ID_RGB>; + function = LED_FUNCTION_WLAN; + function-enumerator = <3>; + }; + + multi-led@3 { + reg = <0x3>; + color = <LED_COLOR_ID_RGB>; + function = LED_FUNCTION_WLAN; + function-enumerator = <2>; + }; + + multi-led@4 { + reg = <0x4>; + color = <LED_COLOR_ID_RGB>; + function = LED_FUNCTION_WLAN; + function-enumerator = <1>; + }; + + multi-led@5 { + reg = <0x5>; + color = <LED_COLOR_ID_RGB>; + function = LED_FUNCTION_WAN; + }; + + multi-led@6 { + reg = <0x6>; + color = <LED_COLOR_ID_RGB>; + function = LED_FUNCTION_LAN; + function-enumerator = <4>; + }; + + multi-led@7 { + reg = <0x7>; + color = <LED_COLOR_ID_RGB>; + function = LED_FUNCTION_LAN; + function-enumerator = <3>; + }; + + multi-led@8 { + reg = <0x8>; + color = <LED_COLOR_ID_RGB>; + function = LED_FUNCTION_LAN; + function-enumerator = <2>; + }; + + multi-led@9 { + reg = <0x9>; + color = <LED_COLOR_ID_RGB>; + function = LED_FUNCTION_LAN; + function-enumerator = <1>; + }; + + multi-led@a { + reg = <0xa>; + color = <LED_COLOR_ID_RGB>; + function = LED_FUNCTION_LAN; + function-enumerator = <0>; + }; + + multi-led@b { + reg = <0xb>; + color = <LED_COLOR_ID_RGB>; + function = LED_FUNCTION_POWER; + }; + }; + eeprom@54 { compatible = "atmel,24c64"; reg = <0x54>;
Signature · 4 lines
SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Felix Imendörffer HRB 36809 (AG Nürnberg)
Re: [PATCH mvebu-dt v2 5/6] ARM: dts: turris-omnia: add LED controller node
On Sat, 14 Nov 2020 22:58:45 +0100
Andreas Färber <[email protected]> wrote:
Hi Andreas,
Quoted 4 lines
arch/arm/boot/dts/armada-385-turris-omnia.dts i2c@0 { · line 174
Quoted 2 lines
arch/arm/boot/dts/armada-385-turris-omnia.dts +0
-0/* STM32F0 command interface at address 0x2a */ /* leds device (in STM32F0) at address 0x2b */Update and move this comment now that the node documents the address?
Sounds reasonable.
Quoted 5 lines
arch/arm/boot/dts/armada-385-turris-omnia.dts i2c@0 { · line 192
Quoted 2 lines
arch/arm/boot/dts/armada-385-turris-omnia.dts +2
-0+ * in most cases users have wifi cards in + * these slotsDoesn't U-Boot detect mSATA and switches SerDes configuration? You could then have it set
LED_FUNCTION_DISKin case of mSATA detected.
Yes, but this also needs to be changed by u-boot. And current version of MCU firmware on Omnia doesn't connect the mSATA/PCI3 LED when in HW controlled mode, so the LED has to be blinked in software anyway, for now.
Another problem is that user can put non wireless/disk PCIe device into this slot. What should the LED function be then? ...
Quoted 4 lines
I recently didn't find any DT binding for the netdev LED trigger, but you could set trigger-sources to associate the LEDS with PCIe nodes even if unused. Same for the LAN LEDs and switch port nodes, if you give them labels.
I am also working with Pavel in LED subsystem. Trigger-sources does not work currently - you can put it in device tree, but the drivers ignore it. I am currently also working on transparent HW offloading of LED
triggers: if you set netdev trigger for lan1 LED, the system will just enable HW controlled mode on Omnia, instead of blinking this LED in software.
So please wait till this is done.
BTW, another issue is the devicename and function part of LED:
The linux,default-trigger property is deprecated in favor of function.
So somehow the system should enable netdev trigger on the LED is trigger-source points to a network device and function is compatible with netdev trigger. What should this functions be? We have:
LED_FUNCTION_ACTIVITY
LED_FUNCTION_RX
LED_FUNCTION_TX
LED_FUNCTION_WAN
LED_FUNCTION_LAN
Jacek thinks that
LED_FUNCTION_ACTIVITY should be used for system activity trigger
LED_FUNCTION_RX/TX on uart
LED_FUNCTION_LAN on a network device
But I and Pavel think that if the LED_FUNCTION_ACTIVITY is used, the trigger should be selected depending on trigger-source:
- if it points to a network device, "netdev"
- if it points to a block device, a potential "blkdev" trigger which does not exist now
- ...
Also RX/TX should be IMO used this way: for the netdev trigger you can use whether it should blink only on rx, only on tx, or on both.
Please look at:
https://www.spinics.net/lists/linux-leds/msg16632.html
Quoted 6 lines
arch/arm/boot/dts/armada-385-turris-omnia.dts i2c@0 { · line 195
Quoted 4 lines
arch/arm/boot/dts/armada-385-turris-omnia.dts +3
-0+ * - there are 2 LEDs dedicated for user: A and + * B. Again there is no such function defined. + * For now we use LED_FUNCTION_DEBUGI'd suggest the more neutral
LED_FUNCTION_INDICATOR.
Hmm, that sounds reasonable.
Thanks.
Marek
PATCH v2 6/6 ARM: dts: turris-omnia: update ethernet-phy node and handle name
In reply to Marek BehúnUse property name phy-handle instead of the deprecated phy to connect eth2 to the PHY.
Rename the node from "phy@1" to "ethernet-phy@1", since "phy@1" is incorrect according to device-tree bindings documentation. Also remove the "ethernet-phy-id0141.0DD1" compatible string, it is not needed. Kernel can read the PHY identifier itself.
--- arch/arm/boot/dts/armada-385-turris-omnia.dts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
arch/arm/boot/dts/armada-385-turris-omnia.dts +3 -3
@@ -145,7 +145,7 @@ fixed-link { ð2 { status = "okay"; phy-mode = "sgmii"; - phy = <&phy1>; + phy-handle = <&phy1>; phys = <&comphy5 2>; sfp = <&sfp>; buffer-manager = <&bm>;
@@ -374,9 +374,9 @@ &mdio { pinctrl-0 = <&mdio_pins>; status = "okay"; - phy1: phy@1 { + phy1: ethernet-phy@1 { status = "okay"; - compatible = "ethernet-phy-id0141.0DD1", "ethernet-phy-ieee802.3-c22"; + compatible = "ethernet-phy-ieee802.3-c22"; reg = <1>; /* irq is connected to &pcawan pin 7 */
Signature · 1 line
2.26.2
Re: [PATCH mvebu-dt v2 6/6] ARM: dts: turris-omnia: update ethernet-phy node and handle name
Hi Marek,
Quoted 33 lines · Marek Behún
Use property name
phy-handleinstead of the deprecatedphyto connect eth2 to the PHY.
Rename the node from "phy@1" to "ethernet-phy@1", since "phy@1" is incorrect according to device-tree bindings documentation. Also remove the "ethernet-phy-id0141.0DD1" compatible string, it is not needed. Kernel can read the PHY identifier itself.Signed-off-by Marek Behún <[email protected]>Reviewed-by Andrew Lunn <[email protected]>Cc Uwe Kleine-König <[email protected]>Cc Jason Cooper <[email protected]>Cc Gregory CLEMENT <[email protected]>Cc Andreas Färber <[email protected]>Cc Rob Herring <[email protected]>--- arch/arm/boot/dts/armada-385-turris-omnia.dts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)arch/arm/boot/dts/armada-385-turris-omnia.dts +1
-1@@ -145,7 +145,7 @@ fixed-link { ð2 { status = "okay"; phy-mode = "sgmii"; - phy = <&phy1>; + phy-handle = <&phy1>; phys = <&comphy5 2>; sfp = <&sfp>; buffer-manager = <&bm>;
arch/arm/boot/dts/armada-385-turris-omnia.dts &mdio { · line 377
Quoted 5 lines
arch/arm/boot/dts/armada-385-turris-omnia.dts +1
-1@@ -374,9 +374,9 @@ &mdio { pinctrl-0 = <&mdio_pins>; status = "okay"; - phy1: phy@1 { + phy1: ethernet-phy@1 {
This one I had noticed in the DT binding and verified that mainline U-Boot does not rely on it. So ACK for this.
arch/arm/boot/dts/armada-385-turris-omnia.dts &mdio { · line 378
Quoted 1 line
arch/arm/boot/dts/armada-385-turris-omnia.dts +0
-0status = "okay";
Unrelated: This property is theoretically superfluous, as unlike eth2 this node is new and doesn't overwrite a pre-existing property.
I believe in my testing overriding with status = "disabled" was not enough to get the SFP to work, I needed to comment out the referencing phy(-handle) property.
arch/arm/boot/dts/armada-385-turris-omnia.dts &mdio { · line 379
Quoted 2 lines
arch/arm/boot/dts/armada-385-turris-omnia.dts +1
-1- compatible = "ethernet-phy-id0141.0DD1", "ethernet-phy-ieee802.3-c22"; + compatible = "ethernet-phy-ieee802.3-c22";
Does it do any harm to leave it in though?
arch/arm/boot/dts/armada-385-turris-omnia.dts &mdio { · line 382
Quoted 3 lines
arch/arm/boot/dts/armada-385-turris-omnia.dts +0
-0reg = <1>; /* irq is connected to &pcawan pin 7 */
Regards,
Andreas
Signature · 4 lines
SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Felix Imendörffer HRB 36809 (AG Nürnberg)
Re: [PATCH mvebu-dt v2 6/6] ARM: dts: turris-omnia: update ethernet-phy node and handle name
On Sat, 14 Nov 2020 23:04:41 +0100
Andreas Färber <[email protected]> wrote:
Hi Andreas,
Quoted 10 lines
arch/arm/boot/dts/armada-385-turris-omnia.dts &mdio { · line 377
Quoted 2 lines
arch/arm/boot/dts/armada-385-turris-omnia.dts +1
-1- phy1: phy@1 { + phy1: ethernet-phy@1 {This one I had noticed in the DT binding and verified that mainline U-Boot does not rely on it. So ACK for this.
Quoted 1 line
status = "okay";
Unrelated: This property is theoretically superfluous, as unlike eth2 this node is new and doesn't overwrite a pre-existing property.
Yes, status = "okay" is not needed if there isn't status = "disabled" before (somewhere in include files). Maybe I will send a patch removing all unneeded status="okay" properties in the future.
Quoted 3 lines
I believe in my testing overriding with status = "disabled" was not enough to get the SFP to work, I needed to comment out the referencing phy(-handle) property.
Yes, as I wrote in reply to your question in patch 4/6, U-Boot needs to:
- remove phy-handle property
- add managed = in-band-status property
- enable sfp node
Quoted 5 lines
arch/arm/boot/dts/armada-385-turris-omnia.dts &mdio { · line 379
Quoted 3 lines
arch/arm/boot/dts/armada-385-turris-omnia.dts +1
-1- compatible = "ethernet-phy-id0141.0DD1", "ethernet-phy-ieee802.3-c22"; + compatible = "ethernet-phy-ieee802.3-c22";Does it do any harm to leave it in though?
No, but lets remove it anyway :)
Marek