* [EXT] [PATCH u-boot-marvell] arm64: dts: armada-3720-espressobin: fix COMPHY nodes
2020-08-19 9:57 [PATCH u-boot-marvell] arm64: dts: armada-3720-espressobin: fix COMPHY nodes Marek Behún
@ 2020-08-19 10:10 ` Kostya Porotchkin
2020-08-19 10:33 ` Stefan Roese
` (4 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: Kostya Porotchkin @ 2020-08-19 10:10 UTC (permalink / raw)
To: u-boot
> -----Original Message-----
> From: Marek Beh?n <marek.behun@nic.cz>
> Sent: Wednesday, August 19, 2020 12:57
> To: u-boot at 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>
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH u-boot-marvell] arm64: dts: armada-3720-espressobin: fix COMPHY nodes
2020-08-19 9:57 [PATCH u-boot-marvell] arm64: dts: armada-3720-espressobin: fix COMPHY nodes Marek Behún
2020-08-19 10:10 ` [EXT] " Kostya Porotchkin
@ 2020-08-19 10:33 ` Stefan Roese
2020-08-26 7:12 ` Andre Heider
` (3 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: Stefan Roese @ 2020-08-19 10:33 UTC (permalink / raw)
To: u-boot
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
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH u-boot-marvell] arm64: dts: armada-3720-espressobin: fix COMPHY nodes
2020-08-19 9:57 [PATCH u-boot-marvell] arm64: dts: armada-3720-espressobin: fix COMPHY nodes Marek Behún
2020-08-19 10:10 ` [EXT] " Kostya Porotchkin
2020-08-19 10:33 ` Stefan Roese
@ 2020-08-26 7:12 ` Andre Heider
2020-08-28 8:18 ` Andre Heider
` (2 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: Andre Heider @ 2020-08-26 7:12 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH u-boot-marvell] arm64: dts: armada-3720-espressobin: fix COMPHY nodes
2020-08-19 9:57 [PATCH u-boot-marvell] arm64: dts: armada-3720-espressobin: fix COMPHY nodes Marek Behún
` (2 preceding siblings ...)
2020-08-26 7:12 ` Andre Heider
@ 2020-08-28 8:18 ` Andre Heider
2020-08-28 14:59 ` Andre Heider
2020-08-30 6:38 ` Andre Heider
2020-08-31 13:02 ` Stefan Roese
5 siblings, 1 reply; 9+ messages in thread
From: Andre Heider @ 2020-08-28 8:18 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH u-boot-marvell] arm64: dts: armada-3720-espressobin: fix COMPHY nodes
2020-08-28 8:18 ` Andre Heider
@ 2020-08-28 14:59 ` Andre Heider
0 siblings, 0 replies; 9+ messages in thread
From: Andre Heider @ 2020-08-28 14:59 UTC (permalink / raw)
To: u-boot
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.heider at gmail.com/
Regards,
Andre
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH u-boot-marvell] arm64: dts: armada-3720-espressobin: fix COMPHY nodes
2020-08-19 9:57 [PATCH u-boot-marvell] arm64: dts: armada-3720-espressobin: fix COMPHY nodes Marek Behún
` (3 preceding siblings ...)
2020-08-28 8:18 ` Andre Heider
@ 2020-08-30 6:38 ` Andre Heider
2020-08-30 22:57 ` Marek Behun
2020-08-31 13:02 ` Stefan Roese
5 siblings, 1 reply; 9+ messages in thread
From: Andre Heider @ 2020-08-30 6:38 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH u-boot-marvell] arm64: dts: armada-3720-espressobin: fix COMPHY nodes
2020-08-30 6:38 ` Andre Heider
@ 2020-08-30 22:57 ` Marek Behun
0 siblings, 0 replies; 9+ messages in thread
From: Marek Behun @ 2020-08-30 22:57 UTC (permalink / raw)
To: u-boot
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 :( ....
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH u-boot-marvell] arm64: dts: armada-3720-espressobin: fix COMPHY nodes
2020-08-19 9:57 [PATCH u-boot-marvell] arm64: dts: armada-3720-espressobin: fix COMPHY nodes Marek Behún
` (4 preceding siblings ...)
2020-08-30 6:38 ` Andre Heider
@ 2020-08-31 13:02 ` Stefan Roese
5 siblings, 0 replies; 9+ messages in thread
From: Stefan Roese @ 2020-08-31 13:02 UTC (permalink / raw)
To: u-boot
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
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de
^ permalink raw reply [flat|nested] 9+ messages in thread