* [PATCH 2/3] phy: marvell: a3700: Fix configuring polarity invert bits
2021-09-24 14:11 [PATCH 1/3] phy: marvell: a3700: Set TXDCLK_2X_SEL bit during PCIe initialization Pali Rohár
@ 2021-09-24 14:11 ` Pali Rohár
2021-09-24 14:19 ` Stefan Roese
2021-10-08 9:17 ` Stefan Roese
2021-09-24 14:11 ` [PATCH 3/3] phy: marvell: a3700: Return correct error code when power up fails Pali Rohár
` (2 subsequent siblings)
3 siblings, 2 replies; 10+ messages in thread
From: Pali Rohár @ 2021-09-24 14:11 UTC (permalink / raw)
To: Stefan Roese
Cc: Konstantin Porotchkin, Grzegorz Jaszczyk, Marek Behún, u-boot
phy_txd_inv or phy_rxd_inv needs to be set only in case when
appropriate polarity is inverted. Otherwise these bits should be
cleared.
Same change was included in TF-A project:
https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/9406
Signed-off-by: Pali Rohár <pali@kernel.org>
---
drivers/phy/marvell/comphy_a3700.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/drivers/phy/marvell/comphy_a3700.c b/drivers/phy/marvell/comphy_a3700.c
index afa1295bbdb8..47a1ebd50238 100644
--- a/drivers/phy/marvell/comphy_a3700.c
+++ b/drivers/phy/marvell/comphy_a3700.c
@@ -230,9 +230,13 @@ static int comphy_pcie_power_up(u32 speed, u32 invert)
*/
if (invert & COMPHY_POLARITY_TXD_INVERT)
reg_set16(phy_addr(PCIE, SYNC_PATTERN), phy_txd_inv, 0);
+ else
+ reg_set16(phy_addr(PCIE, SYNC_PATTERN), 0, phy_txd_inv);
if (invert & COMPHY_POLARITY_RXD_INVERT)
reg_set16(phy_addr(PCIE, SYNC_PATTERN), phy_rxd_inv, 0);
+ else
+ reg_set16(phy_addr(PCIE, SYNC_PATTERN), 0, phy_rxd_inv);
/*
* 11. Release SW reset
@@ -467,9 +471,13 @@ static int comphy_usb3_power_up(u32 lane, u32 type, u32 speed, u32 invert)
*/
if (invert & COMPHY_POLARITY_TXD_INVERT)
usb3_reg_set16(SYNC_PATTERN, phy_txd_inv, 0, lane);
+ else
+ usb3_reg_set16(SYNC_PATTERN, 0, phy_txd_inv, lane);
if (invert & COMPHY_POLARITY_RXD_INVERT)
usb3_reg_set16(SYNC_PATTERN, phy_rxd_inv, 0, lane);
+ else
+ usb3_reg_set16(SYNC_PATTERN, 0, phy_rxd_inv, lane);
/*
* 10. Set max speed generation to USB3.0 5Gbps
@@ -839,9 +847,13 @@ static int comphy_sgmii_power_up(u32 lane, u32 speed, u32 invert)
*/
if (invert & COMPHY_POLARITY_TXD_INVERT)
reg_set16(sgmiiphy_addr(lane, SYNC_PATTERN), phy_txd_inv, 0);
+ else
+ reg_set16(sgmiiphy_addr(lane, SYNC_PATTERN), 0, phy_txd_inv);
if (invert & COMPHY_POLARITY_RXD_INVERT)
reg_set16(sgmiiphy_addr(lane, SYNC_PATTERN), phy_rxd_inv, 0);
+ else
+ reg_set16(sgmiiphy_addr(lane, SYNC_PATTERN), 0, phy_rxd_inv);
/*
* 19. Set PHY input ports PIN_PU_PLL, PIN_PU_TX and PIN_PU_RX to 1
--
2.20.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH 2/3] phy: marvell: a3700: Fix configuring polarity invert bits
2021-09-24 14:11 ` [PATCH 2/3] phy: marvell: a3700: Fix configuring polarity invert bits Pali Rohár
@ 2021-09-24 14:19 ` Stefan Roese
2021-10-08 9:17 ` Stefan Roese
1 sibling, 0 replies; 10+ messages in thread
From: Stefan Roese @ 2021-09-24 14:19 UTC (permalink / raw)
To: Pali Rohár
Cc: Konstantin Porotchkin, Grzegorz Jaszczyk, Marek Behún, u-boot
On 24.09.21 16:11, Pali Rohár wrote:
> phy_txd_inv or phy_rxd_inv needs to be set only in case when
> appropriate polarity is inverted. Otherwise these bits should be
> cleared.
>
> Same change was included in TF-A project:
> https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/9406
>
> Signed-off-by: Pali Rohár <pali@kernel.org>
Reviewed-by: Stefan Roese <sr@denx.de>
Thanks,
Stefan
> ---
> drivers/phy/marvell/comphy_a3700.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/drivers/phy/marvell/comphy_a3700.c b/drivers/phy/marvell/comphy_a3700.c
> index afa1295bbdb8..47a1ebd50238 100644
> --- a/drivers/phy/marvell/comphy_a3700.c
> +++ b/drivers/phy/marvell/comphy_a3700.c
> @@ -230,9 +230,13 @@ static int comphy_pcie_power_up(u32 speed, u32 invert)
> */
> if (invert & COMPHY_POLARITY_TXD_INVERT)
> reg_set16(phy_addr(PCIE, SYNC_PATTERN), phy_txd_inv, 0);
> + else
> + reg_set16(phy_addr(PCIE, SYNC_PATTERN), 0, phy_txd_inv);
>
> if (invert & COMPHY_POLARITY_RXD_INVERT)
> reg_set16(phy_addr(PCIE, SYNC_PATTERN), phy_rxd_inv, 0);
> + else
> + reg_set16(phy_addr(PCIE, SYNC_PATTERN), 0, phy_rxd_inv);
>
> /*
> * 11. Release SW reset
> @@ -467,9 +471,13 @@ static int comphy_usb3_power_up(u32 lane, u32 type, u32 speed, u32 invert)
> */
> if (invert & COMPHY_POLARITY_TXD_INVERT)
> usb3_reg_set16(SYNC_PATTERN, phy_txd_inv, 0, lane);
> + else
> + usb3_reg_set16(SYNC_PATTERN, 0, phy_txd_inv, lane);
>
> if (invert & COMPHY_POLARITY_RXD_INVERT)
> usb3_reg_set16(SYNC_PATTERN, phy_rxd_inv, 0, lane);
> + else
> + usb3_reg_set16(SYNC_PATTERN, 0, phy_rxd_inv, lane);
>
> /*
> * 10. Set max speed generation to USB3.0 5Gbps
> @@ -839,9 +847,13 @@ static int comphy_sgmii_power_up(u32 lane, u32 speed, u32 invert)
> */
> if (invert & COMPHY_POLARITY_TXD_INVERT)
> reg_set16(sgmiiphy_addr(lane, SYNC_PATTERN), phy_txd_inv, 0);
> + else
> + reg_set16(sgmiiphy_addr(lane, SYNC_PATTERN), 0, phy_txd_inv);
>
> if (invert & COMPHY_POLARITY_RXD_INVERT)
> reg_set16(sgmiiphy_addr(lane, SYNC_PATTERN), phy_rxd_inv, 0);
> + else
> + reg_set16(sgmiiphy_addr(lane, SYNC_PATTERN), 0, phy_rxd_inv);
>
> /*
> * 19. Set PHY input ports PIN_PU_PLL, PIN_PU_TX and PIN_PU_RX to 1
>
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@denx.de
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] phy: marvell: a3700: Fix configuring polarity invert bits
2021-09-24 14:11 ` [PATCH 2/3] phy: marvell: a3700: Fix configuring polarity invert bits Pali Rohár
2021-09-24 14:19 ` Stefan Roese
@ 2021-10-08 9:17 ` Stefan Roese
1 sibling, 0 replies; 10+ messages in thread
From: Stefan Roese @ 2021-10-08 9:17 UTC (permalink / raw)
To: Pali Rohár
Cc: Konstantin Porotchkin, Grzegorz Jaszczyk, Marek Behún, u-boot
On 24.09.21 16:11, Pali Rohár wrote:
> phy_txd_inv or phy_rxd_inv needs to be set only in case when
> appropriate polarity is inverted. Otherwise these bits should be
> cleared.
>
> Same change was included in TF-A project:
> https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/9406
>
> Signed-off-by: Pali Rohár <pali@kernel.org>
Applied to u-boot-marvell/master
Thanks,
Stefan
> ---
> drivers/phy/marvell/comphy_a3700.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/drivers/phy/marvell/comphy_a3700.c b/drivers/phy/marvell/comphy_a3700.c
> index afa1295bbdb8..47a1ebd50238 100644
> --- a/drivers/phy/marvell/comphy_a3700.c
> +++ b/drivers/phy/marvell/comphy_a3700.c
> @@ -230,9 +230,13 @@ static int comphy_pcie_power_up(u32 speed, u32 invert)
> */
> if (invert & COMPHY_POLARITY_TXD_INVERT)
> reg_set16(phy_addr(PCIE, SYNC_PATTERN), phy_txd_inv, 0);
> + else
> + reg_set16(phy_addr(PCIE, SYNC_PATTERN), 0, phy_txd_inv);
>
> if (invert & COMPHY_POLARITY_RXD_INVERT)
> reg_set16(phy_addr(PCIE, SYNC_PATTERN), phy_rxd_inv, 0);
> + else
> + reg_set16(phy_addr(PCIE, SYNC_PATTERN), 0, phy_rxd_inv);
>
> /*
> * 11. Release SW reset
> @@ -467,9 +471,13 @@ static int comphy_usb3_power_up(u32 lane, u32 type, u32 speed, u32 invert)
> */
> if (invert & COMPHY_POLARITY_TXD_INVERT)
> usb3_reg_set16(SYNC_PATTERN, phy_txd_inv, 0, lane);
> + else
> + usb3_reg_set16(SYNC_PATTERN, 0, phy_txd_inv, lane);
>
> if (invert & COMPHY_POLARITY_RXD_INVERT)
> usb3_reg_set16(SYNC_PATTERN, phy_rxd_inv, 0, lane);
> + else
> + usb3_reg_set16(SYNC_PATTERN, 0, phy_rxd_inv, lane);
>
> /*
> * 10. Set max speed generation to USB3.0 5Gbps
> @@ -839,9 +847,13 @@ static int comphy_sgmii_power_up(u32 lane, u32 speed, u32 invert)
> */
> if (invert & COMPHY_POLARITY_TXD_INVERT)
> reg_set16(sgmiiphy_addr(lane, SYNC_PATTERN), phy_txd_inv, 0);
> + else
> + reg_set16(sgmiiphy_addr(lane, SYNC_PATTERN), 0, phy_txd_inv);
>
> if (invert & COMPHY_POLARITY_RXD_INVERT)
> reg_set16(sgmiiphy_addr(lane, SYNC_PATTERN), phy_rxd_inv, 0);
> + else
> + reg_set16(sgmiiphy_addr(lane, SYNC_PATTERN), 0, phy_rxd_inv);
>
> /*
> * 19. Set PHY input ports PIN_PU_PLL, PIN_PU_TX and PIN_PU_RX to 1
>
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@denx.de
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 3/3] phy: marvell: a3700: Return correct error code when power up fails
2021-09-24 14:11 [PATCH 1/3] phy: marvell: a3700: Set TXDCLK_2X_SEL bit during PCIe initialization Pali Rohár
2021-09-24 14:11 ` [PATCH 2/3] phy: marvell: a3700: Fix configuring polarity invert bits Pali Rohár
@ 2021-09-24 14:11 ` Pali Rohár
2021-09-24 14:19 ` Stefan Roese
2021-10-08 9:17 ` Stefan Roese
2021-09-24 14:18 ` [PATCH 1/3] phy: marvell: a3700: Set TXDCLK_2X_SEL bit during PCIe initialization Stefan Roese
2021-10-08 9:16 ` Stefan Roese
3 siblings, 2 replies; 10+ messages in thread
From: Pali Rohár @ 2021-09-24 14:11 UTC (permalink / raw)
To: Stefan Roese
Cc: Konstantin Porotchkin, Grzegorz Jaszczyk, Marek Behún, u-boot
Subroutines in comphy_usb2_power_up() and comphy_sgmii_power_up() functions
may fail. In this case, do not continue execution of current function and
instead jump to the end. Return value in 'ret' variable is already set.
Signed-off-by: Pali Rohár <pali@kernel.org>
---
drivers/phy/marvell/comphy_a3700.c | 26 ++++++++++++++++++++------
1 file changed, 20 insertions(+), 6 deletions(-)
diff --git a/drivers/phy/marvell/comphy_a3700.c b/drivers/phy/marvell/comphy_a3700.c
index 47a1ebd50238..b775db441890 100644
--- a/drivers/phy/marvell/comphy_a3700.c
+++ b/drivers/phy/marvell/comphy_a3700.c
@@ -594,24 +594,30 @@ static int comphy_usb2_power_up(u8 usb32)
rb_usb2phy_pllcal_done, /* value */
rb_usb2phy_pllcal_done, /* mask */
POLL_32B_REG); /* 32bit */
- if (!ret)
+ if (!ret) {
printf("Failed to end USB2 PLL calibration\n");
+ goto out;
+ }
/* Assert impedance calibration done */
ret = comphy_poll_reg(USB2_PHY_CAL_CTRL_ADDR(usb32),
rb_usb2phy_impcal_done, /* value */
rb_usb2phy_impcal_done, /* mask */
POLL_32B_REG); /* 32bit */
- if (!ret)
+ if (!ret) {
printf("Failed to end USB2 impedance calibration\n");
+ goto out;
+ }
/* Assert squetch calibration done */
ret = comphy_poll_reg(USB2_PHY_RX_CHAN_CTRL1_ADDR(usb32),
rb_usb2phy_sqcal_done, /* value */
rb_usb2phy_sqcal_done, /* mask */
POLL_32B_REG); /* 32bit */
- if (!ret)
+ if (!ret) {
printf("Failed to end USB2 unknown calibration\n");
+ goto out;
+ }
/* Assert PLL is ready */
ret = comphy_poll_reg(USB2_PHY_PLL_CTRL0_ADDR(usb32),
@@ -619,9 +625,12 @@ static int comphy_usb2_power_up(u8 usb32)
rb_usb2phy_pll_ready, /* mask */
POLL_32B_REG); /* 32bit */
- if (!ret)
+ if (!ret) {
printf("Failed to lock USB2 PLL\n");
+ goto out;
+ }
+out:
debug_exit();
return ret;
@@ -873,8 +882,10 @@ static int comphy_sgmii_power_up(u32 lane, u32 speed, u32 invert)
rb_pll_ready_tx | rb_pll_ready_rx, /* value */
rb_pll_ready_tx | rb_pll_ready_rx, /* mask */
POLL_32B_REG); /* 32bit */
- if (!ret)
+ if (!ret) {
printf("Failed to lock PLL for SGMII PHY %d\n", lane);
+ goto out;
+ }
/*
* 21. Set COMPHY input port PIN_TX_IDLE=0
@@ -895,14 +906,17 @@ static int comphy_sgmii_power_up(u32 lane, u32 speed, u32 invert)
rb_rx_init_done, /* value */
rb_rx_init_done, /* mask */
POLL_32B_REG); /* 32bit */
- if (!ret)
+ if (!ret) {
printf("Failed to init RX of SGMII PHY %d\n", lane);
+ goto out;
+ }
/*
* Restore saved selector.
*/
reg_set(COMPHY_SEL_ADDR, saved_selector, 0xFFFFFFFF);
+out:
debug_exit();
return ret;
--
2.20.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH 3/3] phy: marvell: a3700: Return correct error code when power up fails
2021-09-24 14:11 ` [PATCH 3/3] phy: marvell: a3700: Return correct error code when power up fails Pali Rohár
@ 2021-09-24 14:19 ` Stefan Roese
2021-10-08 9:17 ` Stefan Roese
1 sibling, 0 replies; 10+ messages in thread
From: Stefan Roese @ 2021-09-24 14:19 UTC (permalink / raw)
To: Pali Rohár
Cc: Konstantin Porotchkin, Grzegorz Jaszczyk, Marek Behún, u-boot
On 24.09.21 16:11, Pali Rohár wrote:
> Subroutines in comphy_usb2_power_up() and comphy_sgmii_power_up() functions
> may fail. In this case, do not continue execution of current function and
> instead jump to the end. Return value in 'ret' variable is already set.
>
> Signed-off-by: Pali Rohár <pali@kernel.org>
Reviewed-by: Stefan Roese <sr@denx.de>
Thanks,
Stefan
> ---
> drivers/phy/marvell/comphy_a3700.c | 26 ++++++++++++++++++++------
> 1 file changed, 20 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/phy/marvell/comphy_a3700.c b/drivers/phy/marvell/comphy_a3700.c
> index 47a1ebd50238..b775db441890 100644
> --- a/drivers/phy/marvell/comphy_a3700.c
> +++ b/drivers/phy/marvell/comphy_a3700.c
> @@ -594,24 +594,30 @@ static int comphy_usb2_power_up(u8 usb32)
> rb_usb2phy_pllcal_done, /* value */
> rb_usb2phy_pllcal_done, /* mask */
> POLL_32B_REG); /* 32bit */
> - if (!ret)
> + if (!ret) {
> printf("Failed to end USB2 PLL calibration\n");
> + goto out;
> + }
>
> /* Assert impedance calibration done */
> ret = comphy_poll_reg(USB2_PHY_CAL_CTRL_ADDR(usb32),
> rb_usb2phy_impcal_done, /* value */
> rb_usb2phy_impcal_done, /* mask */
> POLL_32B_REG); /* 32bit */
> - if (!ret)
> + if (!ret) {
> printf("Failed to end USB2 impedance calibration\n");
> + goto out;
> + }
>
> /* Assert squetch calibration done */
> ret = comphy_poll_reg(USB2_PHY_RX_CHAN_CTRL1_ADDR(usb32),
> rb_usb2phy_sqcal_done, /* value */
> rb_usb2phy_sqcal_done, /* mask */
> POLL_32B_REG); /* 32bit */
> - if (!ret)
> + if (!ret) {
> printf("Failed to end USB2 unknown calibration\n");
> + goto out;
> + }
>
> /* Assert PLL is ready */
> ret = comphy_poll_reg(USB2_PHY_PLL_CTRL0_ADDR(usb32),
> @@ -619,9 +625,12 @@ static int comphy_usb2_power_up(u8 usb32)
> rb_usb2phy_pll_ready, /* mask */
> POLL_32B_REG); /* 32bit */
>
> - if (!ret)
> + if (!ret) {
> printf("Failed to lock USB2 PLL\n");
> + goto out;
> + }
>
> +out:
> debug_exit();
>
> return ret;
> @@ -873,8 +882,10 @@ static int comphy_sgmii_power_up(u32 lane, u32 speed, u32 invert)
> rb_pll_ready_tx | rb_pll_ready_rx, /* value */
> rb_pll_ready_tx | rb_pll_ready_rx, /* mask */
> POLL_32B_REG); /* 32bit */
> - if (!ret)
> + if (!ret) {
> printf("Failed to lock PLL for SGMII PHY %d\n", lane);
> + goto out;
> + }
>
> /*
> * 21. Set COMPHY input port PIN_TX_IDLE=0
> @@ -895,14 +906,17 @@ static int comphy_sgmii_power_up(u32 lane, u32 speed, u32 invert)
> rb_rx_init_done, /* value */
> rb_rx_init_done, /* mask */
> POLL_32B_REG); /* 32bit */
> - if (!ret)
> + if (!ret) {
> printf("Failed to init RX of SGMII PHY %d\n", lane);
> + goto out;
> + }
>
> /*
> * Restore saved selector.
> */
> reg_set(COMPHY_SEL_ADDR, saved_selector, 0xFFFFFFFF);
>
> +out:
> debug_exit();
>
> return ret;
>
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@denx.de
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH 3/3] phy: marvell: a3700: Return correct error code when power up fails
2021-09-24 14:11 ` [PATCH 3/3] phy: marvell: a3700: Return correct error code when power up fails Pali Rohár
2021-09-24 14:19 ` Stefan Roese
@ 2021-10-08 9:17 ` Stefan Roese
1 sibling, 0 replies; 10+ messages in thread
From: Stefan Roese @ 2021-10-08 9:17 UTC (permalink / raw)
To: Pali Rohár
Cc: Konstantin Porotchkin, Grzegorz Jaszczyk, Marek Behún, u-boot
On 24.09.21 16:11, Pali Rohár wrote:
> Subroutines in comphy_usb2_power_up() and comphy_sgmii_power_up() functions
> may fail. In this case, do not continue execution of current function and
> instead jump to the end. Return value in 'ret' variable is already set.
>
> Signed-off-by: Pali Rohár <pali@kernel.org>
Applied to u-boot-marvell/master
Thanks,
Stefan
> ---
> drivers/phy/marvell/comphy_a3700.c | 26 ++++++++++++++++++++------
> 1 file changed, 20 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/phy/marvell/comphy_a3700.c b/drivers/phy/marvell/comphy_a3700.c
> index 47a1ebd50238..b775db441890 100644
> --- a/drivers/phy/marvell/comphy_a3700.c
> +++ b/drivers/phy/marvell/comphy_a3700.c
> @@ -594,24 +594,30 @@ static int comphy_usb2_power_up(u8 usb32)
> rb_usb2phy_pllcal_done, /* value */
> rb_usb2phy_pllcal_done, /* mask */
> POLL_32B_REG); /* 32bit */
> - if (!ret)
> + if (!ret) {
> printf("Failed to end USB2 PLL calibration\n");
> + goto out;
> + }
>
> /* Assert impedance calibration done */
> ret = comphy_poll_reg(USB2_PHY_CAL_CTRL_ADDR(usb32),
> rb_usb2phy_impcal_done, /* value */
> rb_usb2phy_impcal_done, /* mask */
> POLL_32B_REG); /* 32bit */
> - if (!ret)
> + if (!ret) {
> printf("Failed to end USB2 impedance calibration\n");
> + goto out;
> + }
>
> /* Assert squetch calibration done */
> ret = comphy_poll_reg(USB2_PHY_RX_CHAN_CTRL1_ADDR(usb32),
> rb_usb2phy_sqcal_done, /* value */
> rb_usb2phy_sqcal_done, /* mask */
> POLL_32B_REG); /* 32bit */
> - if (!ret)
> + if (!ret) {
> printf("Failed to end USB2 unknown calibration\n");
> + goto out;
> + }
>
> /* Assert PLL is ready */
> ret = comphy_poll_reg(USB2_PHY_PLL_CTRL0_ADDR(usb32),
> @@ -619,9 +625,12 @@ static int comphy_usb2_power_up(u8 usb32)
> rb_usb2phy_pll_ready, /* mask */
> POLL_32B_REG); /* 32bit */
>
> - if (!ret)
> + if (!ret) {
> printf("Failed to lock USB2 PLL\n");
> + goto out;
> + }
>
> +out:
> debug_exit();
>
> return ret;
> @@ -873,8 +882,10 @@ static int comphy_sgmii_power_up(u32 lane, u32 speed, u32 invert)
> rb_pll_ready_tx | rb_pll_ready_rx, /* value */
> rb_pll_ready_tx | rb_pll_ready_rx, /* mask */
> POLL_32B_REG); /* 32bit */
> - if (!ret)
> + if (!ret) {
> printf("Failed to lock PLL for SGMII PHY %d\n", lane);
> + goto out;
> + }
>
> /*
> * 21. Set COMPHY input port PIN_TX_IDLE=0
> @@ -895,14 +906,17 @@ static int comphy_sgmii_power_up(u32 lane, u32 speed, u32 invert)
> rb_rx_init_done, /* value */
> rb_rx_init_done, /* mask */
> POLL_32B_REG); /* 32bit */
> - if (!ret)
> + if (!ret) {
> printf("Failed to init RX of SGMII PHY %d\n", lane);
> + goto out;
> + }
>
> /*
> * Restore saved selector.
> */
> reg_set(COMPHY_SEL_ADDR, saved_selector, 0xFFFFFFFF);
>
> +out:
> debug_exit();
>
> return ret;
>
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@denx.de
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] phy: marvell: a3700: Set TXDCLK_2X_SEL bit during PCIe initialization
2021-09-24 14:11 [PATCH 1/3] phy: marvell: a3700: Set TXDCLK_2X_SEL bit during PCIe initialization Pali Rohár
2021-09-24 14:11 ` [PATCH 2/3] phy: marvell: a3700: Fix configuring polarity invert bits Pali Rohár
2021-09-24 14:11 ` [PATCH 3/3] phy: marvell: a3700: Return correct error code when power up fails Pali Rohár
@ 2021-09-24 14:18 ` Stefan Roese
2021-09-24 14:21 ` Pali Rohár
2021-10-08 9:16 ` Stefan Roese
3 siblings, 1 reply; 10+ messages in thread
From: Stefan Roese @ 2021-09-24 14:18 UTC (permalink / raw)
To: Pali Rohár
Cc: Konstantin Porotchkin, Grzegorz Jaszczyk, Marek Behún, u-boot
On 24.09.21 16:11, Pali Rohár wrote:
> Marvell Armada 3700 Functional Specifications, section 52.2 PCIe Link
> Initialization says that TXDCLK_2X_SEL bit needs to be enabled for PCIe
> Root Complex mode.
>
> Same change was included in TF-A project:
> https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/9408
>
> Signed-off-by: Pali Rohár <pali@kernel.org>
Did you experience some problems without this change? If yes, which?
Reviewed-by: Stefan Roese <sr@denx.de>
Thanks,
Stefan
> ---
> drivers/phy/marvell/comphy_a3700.c | 2 +-
> drivers/phy/marvell/comphy_a3700.h | 1 +
> 2 files changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/phy/marvell/comphy_a3700.c b/drivers/phy/marvell/comphy_a3700.c
> index 5eb137db4884..afa1295bbdb8 100644
> --- a/drivers/phy/marvell/comphy_a3700.c
> +++ b/drivers/phy/marvell/comphy_a3700.c
> @@ -200,7 +200,7 @@ static int comphy_pcie_power_up(u32 speed, u32 invert)
> * 6. Enable the output of 100M/125M/500M clock
> */
> reg_set16(phy_addr(PCIE, MISC_REG0),
> - 0xA00D | rb_clk500m_en | rb_clk100m_125m_en, 0xFFFF);
> + 0xA00D | rb_clk500m_en | rb_txdclk_2x_sel | rb_clk100m_125m_en, 0xFFFF);
>
> /*
> * 7. Enable TX
> diff --git a/drivers/phy/marvell/comphy_a3700.h b/drivers/phy/marvell/comphy_a3700.h
> index 8748c6c84ae6..23c8ffbff44d 100644
> --- a/drivers/phy/marvell/comphy_a3700.h
> +++ b/drivers/phy/marvell/comphy_a3700.h
> @@ -120,6 +120,7 @@ static inline void __iomem *phy_addr(enum phy_unit unit, u32 addr)
>
> #define MISC_REG0 0x4f
> #define rb_clk100m_125m_en BIT(4)
> +#define rb_txdclk_2x_sel BIT(6)
> #define rb_clk500m_en BIT(7)
> #define rb_ref_clk_sel BIT(10)
>
>
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@denx.de
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] phy: marvell: a3700: Set TXDCLK_2X_SEL bit during PCIe initialization
2021-09-24 14:18 ` [PATCH 1/3] phy: marvell: a3700: Set TXDCLK_2X_SEL bit during PCIe initialization Stefan Roese
@ 2021-09-24 14:21 ` Pali Rohár
0 siblings, 0 replies; 10+ messages in thread
From: Pali Rohár @ 2021-09-24 14:21 UTC (permalink / raw)
To: Stefan Roese
Cc: Konstantin Porotchkin, Grzegorz Jaszczyk, Marek Behún, u-boot
On Friday 24 September 2021 16:18:46 Stefan Roese wrote:
> On 24.09.21 16:11, Pali Rohár wrote:
> > Marvell Armada 3700 Functional Specifications, section 52.2 PCIe Link
> > Initialization says that TXDCLK_2X_SEL bit needs to be enabled for PCIe
> > Root Complex mode.
> >
> > Same change was included in TF-A project:
> > https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/9408
> >
> > Signed-off-by: Pali Rohár <pali@kernel.org>
>
> Did you experience some problems without this change?
I think I did not see problems neither with nor without this change.
Code just aligns with what is written in documentation guidelines.
> If yes, which?
>
> Reviewed-by: Stefan Roese <sr@denx.de>
>
> Thanks,
> Stefan
>
> > ---
> > drivers/phy/marvell/comphy_a3700.c | 2 +-
> > drivers/phy/marvell/comphy_a3700.h | 1 +
> > 2 files changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/phy/marvell/comphy_a3700.c b/drivers/phy/marvell/comphy_a3700.c
> > index 5eb137db4884..afa1295bbdb8 100644
> > --- a/drivers/phy/marvell/comphy_a3700.c
> > +++ b/drivers/phy/marvell/comphy_a3700.c
> > @@ -200,7 +200,7 @@ static int comphy_pcie_power_up(u32 speed, u32 invert)
> > * 6. Enable the output of 100M/125M/500M clock
> > */
> > reg_set16(phy_addr(PCIE, MISC_REG0),
> > - 0xA00D | rb_clk500m_en | rb_clk100m_125m_en, 0xFFFF);
> > + 0xA00D | rb_clk500m_en | rb_txdclk_2x_sel | rb_clk100m_125m_en, 0xFFFF);
> > /*
> > * 7. Enable TX
> > diff --git a/drivers/phy/marvell/comphy_a3700.h b/drivers/phy/marvell/comphy_a3700.h
> > index 8748c6c84ae6..23c8ffbff44d 100644
> > --- a/drivers/phy/marvell/comphy_a3700.h
> > +++ b/drivers/phy/marvell/comphy_a3700.h
> > @@ -120,6 +120,7 @@ static inline void __iomem *phy_addr(enum phy_unit unit, u32 addr)
> > #define MISC_REG0 0x4f
> > #define rb_clk100m_125m_en BIT(4)
> > +#define rb_txdclk_2x_sel BIT(6)
> > #define rb_clk500m_en BIT(7)
> > #define rb_ref_clk_sel BIT(10)
> >
>
>
> 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@denx.de
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] phy: marvell: a3700: Set TXDCLK_2X_SEL bit during PCIe initialization
2021-09-24 14:11 [PATCH 1/3] phy: marvell: a3700: Set TXDCLK_2X_SEL bit during PCIe initialization Pali Rohár
` (2 preceding siblings ...)
2021-09-24 14:18 ` [PATCH 1/3] phy: marvell: a3700: Set TXDCLK_2X_SEL bit during PCIe initialization Stefan Roese
@ 2021-10-08 9:16 ` Stefan Roese
3 siblings, 0 replies; 10+ messages in thread
From: Stefan Roese @ 2021-10-08 9:16 UTC (permalink / raw)
To: Pali Rohár
Cc: Konstantin Porotchkin, Grzegorz Jaszczyk, Marek Behún, u-boot
On 24.09.21 16:11, Pali Rohár wrote:
> Marvell Armada 3700 Functional Specifications, section 52.2 PCIe Link
> Initialization says that TXDCLK_2X_SEL bit needs to be enabled for PCIe
> Root Complex mode.
>
> Same change was included in TF-A project:
> https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/9408
>
> Signed-off-by: Pali Rohár <pali@kernel.org>
Applied to u-boot-marvell/master
Thanks,
Stefan
> ---
> drivers/phy/marvell/comphy_a3700.c | 2 +-
> drivers/phy/marvell/comphy_a3700.h | 1 +
> 2 files changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/phy/marvell/comphy_a3700.c b/drivers/phy/marvell/comphy_a3700.c
> index 5eb137db4884..afa1295bbdb8 100644
> --- a/drivers/phy/marvell/comphy_a3700.c
> +++ b/drivers/phy/marvell/comphy_a3700.c
> @@ -200,7 +200,7 @@ static int comphy_pcie_power_up(u32 speed, u32 invert)
> * 6. Enable the output of 100M/125M/500M clock
> */
> reg_set16(phy_addr(PCIE, MISC_REG0),
> - 0xA00D | rb_clk500m_en | rb_clk100m_125m_en, 0xFFFF);
> + 0xA00D | rb_clk500m_en | rb_txdclk_2x_sel | rb_clk100m_125m_en, 0xFFFF);
>
> /*
> * 7. Enable TX
> diff --git a/drivers/phy/marvell/comphy_a3700.h b/drivers/phy/marvell/comphy_a3700.h
> index 8748c6c84ae6..23c8ffbff44d 100644
> --- a/drivers/phy/marvell/comphy_a3700.h
> +++ b/drivers/phy/marvell/comphy_a3700.h
> @@ -120,6 +120,7 @@ static inline void __iomem *phy_addr(enum phy_unit unit, u32 addr)
>
> #define MISC_REG0 0x4f
> #define rb_clk100m_125m_en BIT(4)
> +#define rb_txdclk_2x_sel BIT(6)
> #define rb_clk500m_en BIT(7)
> #define rb_ref_clk_sel BIT(10)
>
>
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@denx.de
^ permalink raw reply [flat|nested] 10+ messages in thread