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

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

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