[PATCH u-boot-marvell] arm64: dts: armada-3720-espressobin: fix COMPHY nodes
This commit fixes initialization of COMPHY on EspressoBin.
Commit 22f418935be4 ("phy: marvell: a3700: Use comphy_mux on Armada 37xx.") introduced usage of comphy_mux on Armada 37xx comphy driver. The lanes are defined in comphy_a3700.c as described in functional specification, that is: lane 0 is SGMII1 or USB3 lane 1 is PCIe or SGMII0 lane 2 is SATA or USB3
But the DTS for EspressoBin configures PCIe on lane 0 and USB3 on lane 1, which is wrong in the sense of the specification and doesn't work with the comphy_mux code, which is 2 years now (the aardvark driver causes synchronous abort in U-Boot).
It worked till the above mentioned commit, because the code for powering up PCIe PHY doesn't work with lane number at all, and the code for powering up USB3 PHY works differently only if USB3 is on lane 2, ie. the check goes like: if (lane == 2) something else something else so it does not differentiate between lanes 0 and 1.
In the future I shall post patches that remove the comphy_a3700 driver and add comphy driver which uses calls to ATF, like Linux' driver does. This will have the advantage of same DTS bindings as Linux', but till this is done, we need this patch.
Signed-off-by: Marek Behún marek.behun@nic.cz Tested-by: Pali Rohár pali@kernel.org Cc: Stefan Roese sr@denx.de --- arch/arm/dts/armada-3720-espressobin.dts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/arm/dts/armada-3720-espressobin.dts b/arch/arm/dts/armada-3720-espressobin.dts index 84e2c2adba..50381e979e 100644 --- a/arch/arm/dts/armada-3720-espressobin.dts +++ b/arch/arm/dts/armada-3720-espressobin.dts @@ -72,13 +72,13 @@ &comphy { max-lanes = <3>; phy0 { - phy-type = <PHY_TYPE_PEX0>; - phy-speed = <PHY_SPEED_2_5G>; + phy-type = <PHY_TYPE_USB3_HOST0>; + phy-speed = <PHY_SPEED_5G>; };
phy1 { - phy-type = <PHY_TYPE_USB3_HOST0>; - phy-speed = <PHY_SPEED_5G>; + phy-type = <PHY_TYPE_PEX0>; + phy-speed = <PHY_SPEED_2_5G>; };
phy2 {
-----Original Message----- From: Marek Behún marek.behun@nic.cz Sent: Wednesday, August 19, 2020 12:57 To: u-boot@lists.denx.de Cc: Kostya Porotchkin kostap@marvell.com; Marek Behún marek.behun@nic.cz; Pali Rohár pali@kernel.org; Stefan Roese sr@denx.de Subject: [EXT] [PATCH u-boot-marvell] arm64: dts: armada-3720-espressobin: fix COMPHY nodes
External Email
This commit fixes initialization of COMPHY on EspressoBin.
Commit 22f418935be4 ("phy: marvell: a3700: Use comphy_mux on Armada 37xx.") introduced usage of comphy_mux on Armada 37xx comphy driver. The lanes are defined in comphy_a3700.c as described in functional specification, that is: lane 0 is SGMII1 or USB3 lane 1 is PCIe or SGMII0 lane 2 is SATA or USB3
But the DTS for EspressoBin configures PCIe on lane 0 and USB3 on lane 1, which is wrong in the sense of the specification and doesn't work with the comphy_mux code, which is 2 years now (the aardvark driver causes synchronous abort in U-Boot).
It worked till the above mentioned commit, because the code for powering up PCIe PHY doesn't work with lane number at all, and the code for powering up USB3 PHY works differently only if USB3 is on lane 2, ie. the check goes like: if (lane == 2) something else something else so it does not differentiate between lanes 0 and 1.
In the future I shall post patches that remove the comphy_a3700 driver and add comphy driver which uses calls to ATF, like Linux' driver does. This will have the advantage of same DTS bindings as Linux', but till this is done, we need this patch.
Signed-off-by: Marek Behún marek.behun@nic.cz Tested-by: Pali Rohár pali@kernel.org Cc: Stefan Roese sr@denx.de
arch/arm/dts/armada-3720-espressobin.dts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/arm/dts/armada-3720-espressobin.dts b/arch/arm/dts/armada-3720-espressobin.dts index 84e2c2adba..50381e979e 100644 --- a/arch/arm/dts/armada-3720-espressobin.dts +++ b/arch/arm/dts/armada-3720-espressobin.dts @@ -72,13 +72,13 @@ &comphy { max-lanes = <3>; phy0 {
phy-type = <PHY_TYPE_PEX0>;phy-speed = <PHY_SPEED_2_5G>;
phy-type = <PHY_TYPE_USB3_HOST0>;phy-speed = <PHY_SPEED_5G>;};
phy1 {
phy-type = <PHY_TYPE_USB3_HOST0>;phy-speed = <PHY_SPEED_5G>;
phy-type = <PHY_TYPE_PEX0>;phy-speed = <PHY_SPEED_2_5G>;};
phy2 {
-- 2.26.2
Reviewed by Konstantin Porotchkin kostap@marvell.com
On 19.08.20 11:57, Marek Behún wrote:
This commit fixes initialization of COMPHY on EspressoBin.
Commit 22f418935be4 ("phy: marvell: a3700: Use comphy_mux on Armada 37xx.") introduced usage of comphy_mux on Armada 37xx comphy driver. The lanes are defined in comphy_a3700.c as described in functional specification, that is: lane 0 is SGMII1 or USB3 lane 1 is PCIe or SGMII0 lane 2 is SATA or USB3
But the DTS for EspressoBin configures PCIe on lane 0 and USB3 on lane 1, which is wrong in the sense of the specification and doesn't work with the comphy_mux code, which is 2 years now (the aardvark driver causes synchronous abort in U-Boot).
It worked till the above mentioned commit, because the code for powering up PCIe PHY doesn't work with lane number at all, and the code for powering up USB3 PHY works differently only if USB3 is on lane 2, ie. the check goes like: if (lane == 2) something else something else so it does not differentiate between lanes 0 and 1.
In the future I shall post patches that remove the comphy_a3700 driver and add comphy driver which uses calls to ATF, like Linux' driver does. This will have the advantage of same DTS bindings as Linux',
That's good. Thanks.
but till this is done, we need this patch.
Signed-off-by: Marek Behún marek.behun@nic.cz Tested-by: Pali Rohár pali@kernel.org Cc: Stefan Roese sr@denx.de
Reviewed-by: Stefan Roese sr@denx.de
Thanks, Stefan
arch/arm/dts/armada-3720-espressobin.dts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/arm/dts/armada-3720-espressobin.dts b/arch/arm/dts/armada-3720-espressobin.dts index 84e2c2adba..50381e979e 100644 --- a/arch/arm/dts/armada-3720-espressobin.dts +++ b/arch/arm/dts/armada-3720-espressobin.dts @@ -72,13 +72,13 @@ &comphy { max-lanes = <3>; phy0 {
phy-type = <PHY_TYPE_PEX0>;phy-speed = <PHY_SPEED_2_5G>;
phy-type = <PHY_TYPE_USB3_HOST0>;phy-speed = <PHY_SPEED_5G>;};
phy1 {
phy-type = <PHY_TYPE_USB3_HOST0>;phy-speed = <PHY_SPEED_5G>;
phy-type = <PHY_TYPE_PEX0>;phy-speed = <PHY_SPEED_2_5G>;};
phy2 {
Viele Grüße, Stefan
On 19/08/2020 11:57, Marek Behún wrote:
This commit fixes initialization of COMPHY on EspressoBin.
Commit 22f418935be4 ("phy: marvell: a3700: Use comphy_mux on Armada 37xx.") introduced usage of comphy_mux on Armada 37xx comphy driver. The lanes are defined in comphy_a3700.c as described in functional specification, that is: lane 0 is SGMII1 or USB3 lane 1 is PCIe or SGMII0 lane 2 is SATA or USB3
But the DTS for EspressoBin configures PCIe on lane 0 and USB3 on lane 1, which is wrong in the sense of the specification and doesn't work with the comphy_mux code, which is 2 years now (the aardvark driver causes synchronous abort in U-Boot).
It worked till the above mentioned commit, because the code for powering up PCIe PHY doesn't work with lane number at all, and the code for powering up USB3 PHY works differently only if USB3 is on lane 2, ie. the check goes like: if (lane == 2) something else something else so it does not differentiate between lanes 0 and 1.
In the future I shall post patches that remove the comphy_a3700 driver and add comphy driver which uses calls to ATF, like Linux' driver does. This will have the advantage of same DTS bindings as Linux', but till this is done, we need this patch.
Signed-off-by: Marek Behún marek.behun@nic.cz Tested-by: Pali Rohár pali@kernel.org Cc: Stefan Roese sr@denx.de
Tested-by: Andre Heider a.heider@gmail.com
Nice, thank you!
I remember getting these data aborts more than a year ago...
Andre
Hi Marek,
On 19/08/2020 11:57, Marek Behún wrote:
This commit fixes initialization of COMPHY on EspressoBin.
Commit 22f418935be4 ("phy: marvell: a3700: Use comphy_mux on Armada 37xx.") introduced usage of comphy_mux on Armada 37xx comphy driver. The lanes are defined in comphy_a3700.c as described in functional specification, that is: lane 0 is SGMII1 or USB3 lane 1 is PCIe or SGMII0 lane 2 is SATA or USB3
But the DTS for EspressoBin configures PCIe on lane 0 and USB3 on lane 1, which is wrong in the sense of the specification and doesn't work with the comphy_mux code, which is 2 years now (the aardvark driver causes synchronous abort in U-Boot).
It worked till the above mentioned commit, because the code for powering up PCIe PHY doesn't work with lane number at all, and the code for powering up USB3 PHY works differently only if USB3 is on lane 2, ie. the check goes like: if (lane == 2) something else something else so it does not differentiate between lanes 0 and 1.
In the future I shall post patches that remove the comphy_a3700 driver and add comphy driver which uses calls to ATF, like Linux' driver does. This will have the advantage of same DTS bindings as Linux', but till this is done, we need this patch.
Signed-off-by: Marek Behún marek.behun@nic.cz Tested-by: Pali Rohár pali@kernel.org Cc: Stefan Roese sr@denx.de
now that I have a working mainline firmware, I think I have a related problem: my sata ssd doesn't get detected, I just get this:
TIM-1.0 WTMI-devel-18.12.1- WTMI: system early-init CPU VDD voltage default value: 1.108V NOTICE: Booting Trusted Firmware NOTICE: BL1: v2.3(): (Marvell-devel-18.12.0) NOTICE: BL1: Built : 06:12:46, Aug 26 2020 NOTICE: BL1: Booting BL2 NOTICE: BL2: v2.3(): (Marvell-devel-18.12.0) NOTICE: BL2: Built : 06:12:46, Aug 26 2020 NOTICE: BL1: Booting BL31 NOTICE: BL31: v2.3(): (Marvell-devel-18.12.0) NOTICE: BL31: Built : 06:12:46
U-Boot 2020.07 (Aug 26 2020 - 06:12:46 +0000)
DRAM: 1 GiB Comphy-0: USB3_HOST0 5 Gbps Comphy-1: PEX0 2.5 Gbps Comphy-2: SATA0 5 Gbps SATA link 0 timeout.
Any idea what's missing here? Do you think this may also be comphy related?
Please cc: me on your comphy/atf patches, I'd be willing to test those!
Thanks, Andre
On 28/08/2020 10:18, Andre Heider wrote:
Hi Marek,
On 19/08/2020 11:57, Marek Behún wrote:
This commit fixes initialization of COMPHY on EspressoBin.
Commit 22f418935be4 ("phy: marvell: a3700: Use comphy_mux on Armada 37xx.") introduced usage of comphy_mux on Armada 37xx comphy driver. The lanes are defined in comphy_a3700.c as described in functional specification, that is: lane 0 is SGMII1 or USB3 lane 1 is PCIe or SGMII0 lane 2 is SATA or USB3
But the DTS for EspressoBin configures PCIe on lane 0 and USB3 on lane 1, which is wrong in the sense of the specification and doesn't work with the comphy_mux code, which is 2 years now (the aardvark driver causes synchronous abort in U-Boot).
It worked till the above mentioned commit, because the code for powering up PCIe PHY doesn't work with lane number at all, and the code for powering up USB3 PHY works differently only if USB3 is on lane 2, ie. the check goes like: if (lane == 2) something else something else so it does not differentiate between lanes 0 and 1.
In the future I shall post patches that remove the comphy_a3700 driver and add comphy driver which uses calls to ATF, like Linux' driver does. This will have the advantage of same DTS bindings as Linux', but till this is done, we need this patch.
Signed-off-by: Marek Behún marek.behun@nic.cz Tested-by: Pali Rohár pali@kernel.org Cc: Stefan Roese sr@denx.de
now that I have a working mainline firmware, I think I have a related problem: my sata ssd doesn't get detected, I just get this:
TIM-1.0 WTMI-devel-18.12.1- WTMI: system early-init CPU VDD voltage default value: 1.108V NOTICE: Booting Trusted Firmware NOTICE: BL1: v2.3(): (Marvell-devel-18.12.0) NOTICE: BL1: Built : 06:12:46, Aug 26 2020 NOTICE: BL1: Booting BL2 NOTICE: BL2: v2.3(): (Marvell-devel-18.12.0) NOTICE: BL2: Built : 06:12:46, Aug 26 2020 NOTICE: BL1: Booting BL31 NOTICE: BL31: v2.3(): (Marvell-devel-18.12.0) NOTICE: BL31: Built : 06:12:46
U-Boot 2020.07 (Aug 26 2020 - 06:12:46 +0000)
DRAM: 1 GiB Comphy-0: USB3_HOST0 5 Gbps Comphy-1: PEX0 2.5 Gbps Comphy-2: SATA0 5 Gbps SATA link 0 timeout.
Any idea what's missing here? Do you think this may also be comphy related?
Please cc: me on your comphy/atf patches, I'd be willing to test those!
Nevermind, Pali figured it out: https://patchwork.ozlabs.org/project/uboot/patch/20200828145629.540954-1-a.h...
Regards, Andre
Hi Marek,
On 19/08/2020 11:57, Marek Behún wrote:
This commit fixes initialization of COMPHY on EspressoBin.
Commit 22f418935be4 ("phy: marvell: a3700: Use comphy_mux on Armada 37xx.") introduced usage of comphy_mux on Armada 37xx comphy driver. The lanes are defined in comphy_a3700.c as described in functional specification, that is: lane 0 is SGMII1 or USB3 lane 1 is PCIe or SGMII0 lane 2 is SATA or USB3
But the DTS for EspressoBin configures PCIe on lane 0 and USB3 on lane 1, which is wrong in the sense of the specification and doesn't work with the comphy_mux code, which is 2 years now (the aardvark driver causes synchronous abort in U-Boot).
I think armada-3720-db.dts requires the same fix?
Regards, Andre
On Sun, 30 Aug 2020 08:38:32 +0200 Andre Heider a.heider@gmail.com wrote:
Hi Marek,
On 19/08/2020 11:57, Marek Behún wrote:
This commit fixes initialization of COMPHY on EspressoBin.
Commit 22f418935be4 ("phy: marvell: a3700: Use comphy_mux on Armada 37xx.") introduced usage of comphy_mux on Armada 37xx comphy driver. The lanes are defined in comphy_a3700.c as described in functional specification, that is: lane 0 is SGMII1 or USB3 lane 1 is PCIe or SGMII0 lane 2 is SATA or USB3
But the DTS for EspressoBin configures PCIe on lane 0 and USB3 on lane 1, which is wrong in the sense of the specification and doesn't work with the comphy_mux code, which is 2 years now (the aardvark driver causes synchronous abort in U-Boot).
I think armada-3720-db.dts requires the same fix?
Regards, Andre
You are right. Sigh :( ....
On 19.08.20 11:57, Marek Behún wrote:
This commit fixes initialization of COMPHY on EspressoBin.
Commit 22f418935be4 ("phy: marvell: a3700: Use comphy_mux on Armada 37xx.") introduced usage of comphy_mux on Armada 37xx comphy driver. The lanes are defined in comphy_a3700.c as described in functional specification, that is: lane 0 is SGMII1 or USB3 lane 1 is PCIe or SGMII0 lane 2 is SATA or USB3
But the DTS for EspressoBin configures PCIe on lane 0 and USB3 on lane 1, which is wrong in the sense of the specification and doesn't work with the comphy_mux code, which is 2 years now (the aardvark driver causes synchronous abort in U-Boot).
It worked till the above mentioned commit, because the code for powering up PCIe PHY doesn't work with lane number at all, and the code for powering up USB3 PHY works differently only if USB3 is on lane 2, ie. the check goes like: if (lane == 2) something else something else so it does not differentiate between lanes 0 and 1.
In the future I shall post patches that remove the comphy_a3700 driver and add comphy driver which uses calls to ATF, like Linux' driver does. This will have the advantage of same DTS bindings as Linux', but till this is done, we need this patch.
Signed-off-by: Marek Behún marek.behun@nic.cz Tested-by: Pali Rohár pali@kernel.org Cc: Stefan Roese sr@denx.de
Applied to u-boot-marvell/master
Thanks, Stefan
arch/arm/dts/armada-3720-espressobin.dts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/arm/dts/armada-3720-espressobin.dts b/arch/arm/dts/armada-3720-espressobin.dts index 84e2c2adba..50381e979e 100644 --- a/arch/arm/dts/armada-3720-espressobin.dts +++ b/arch/arm/dts/armada-3720-espressobin.dts @@ -72,13 +72,13 @@ &comphy { max-lanes = <3>; phy0 {
phy-type = <PHY_TYPE_PEX0>;phy-speed = <PHY_SPEED_2_5G>;
phy-type = <PHY_TYPE_USB3_HOST0>;phy-speed = <PHY_SPEED_5G>;};
phy1 {
phy-type = <PHY_TYPE_USB3_HOST0>;phy-speed = <PHY_SPEED_5G>;
phy-type = <PHY_TYPE_PEX0>;phy-speed = <PHY_SPEED_2_5G>;};
phy2 {
Viele Grüße, Stefan
participants (5)
-
Andre Heider -
Kostya Porotchkin -
Marek Behun -
Marek Behún
-
Stefan Roese