[PATCH 1/3] phy: marvell: a3700: Set TXDCLK_2X_SEL bit during PCIe initialization

[PATCH 1/3] phy: marvell: a3700: Set TXDCLK_2X_SEL bit during PCIe initialization

[Pali Rohár]

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>
---
 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)
 
-- 
2.20.1

[PATCH 2/3] phy: marvell: a3700: Fix configuring polarity invert bits

[Pali Rohár]

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

[PATCH 3/3] phy: marvell: a3700: Return correct error code when power up fails

[Pali Rohár]

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

Re: [PATCH 1/3] phy: marvell: a3700: Set TXDCLK_2X_SEL bit during PCIe initialization

[Stefan Roese]

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

Re: [PATCH 2/3] phy: marvell: a3700: Fix configuring polarity invert bits

[Stefan Roese]

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

Re: [PATCH 3/3] phy: marvell: a3700: Return correct error code when power up fails

[Stefan Roese]

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

Re: [PATCH 1/3] phy: marvell: a3700: Set TXDCLK_2X_SEL bit during PCIe initialization

[Pali Rohár]

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

Re: [PATCH 1/3] phy: marvell: a3700: Set TXDCLK_2X_SEL bit during PCIe initialization

[Stefan Roese]

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

Re: [PATCH 2/3] phy: marvell: a3700: Fix configuring polarity invert bits

[Stefan Roese]

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

Re: [PATCH 3/3] phy: marvell: a3700: Return correct error code when power up fails

[Stefan Roese]

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