All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1 1/1] drivers: phy: sr-usb: do not use internal fsm for USB2 phy init
@ 2020-05-13 17:39 Rayagonda Kokatanur
  2020-05-19  6:52 ` Vinod Koul
  0 siblings, 1 reply; 3+ messages in thread
From: Rayagonda Kokatanur @ 2020-05-13 17:39 UTC (permalink / raw)
  To: Kishon Vijay Abraham I, Vinod Koul, Srinath Mannam, linux-kernel,
	Bharat Gooty
  Cc: Rayagonda Kokatanur

From: Bharat Gooty <bharat.gooty@broadcom.com>

During different reboot cycles, USB PHY PLL may not always lock
during initialization and therefore can cause USB to be not usable.

Hence do not use internal FSM programming sequence for the USB
PHY initialization.

Fixes: 4dcddbb38b64 ("phy: sr-usb: Add Stingray USB PHY driver")
Signed-off-by: Bharat Gooty <bharat.gooty@broadcom.com>
Signed-off-by: Rayagonda Kokatanur <rayagonda.kokatanur@broadcom.com>
---
 drivers/phy/broadcom/phy-bcm-sr-usb.c | 55 +--------------------------
 1 file changed, 2 insertions(+), 53 deletions(-)

diff --git a/drivers/phy/broadcom/phy-bcm-sr-usb.c b/drivers/phy/broadcom/phy-bcm-sr-usb.c
index fe6c58910e4c..7c7862b4f41f 100644
--- a/drivers/phy/broadcom/phy-bcm-sr-usb.c
+++ b/drivers/phy/broadcom/phy-bcm-sr-usb.c
@@ -16,8 +16,6 @@ enum bcm_usb_phy_version {
 };
 
 enum bcm_usb_phy_reg {
-	PLL_NDIV_FRAC,
-	PLL_NDIV_INT,
 	PLL_CTRL,
 	PHY_CTRL,
 	PHY_PLL_CTRL,
@@ -31,18 +29,11 @@ static const u8 bcm_usb_combo_phy_ss[] = {
 };
 
 static const u8 bcm_usb_combo_phy_hs[] = {
-	[PLL_NDIV_FRAC]	= 0x04,
-	[PLL_NDIV_INT]	= 0x08,
 	[PLL_CTRL]	= 0x0c,
 	[PHY_CTRL]	= 0x10,
 };
 
-#define HSPLL_NDIV_INT_VAL	0x13
-#define HSPLL_NDIV_FRAC_VAL	0x1005
-
 static const u8 bcm_usb_hs_phy[] = {
-	[PLL_NDIV_FRAC]	= 0x0,
-	[PLL_NDIV_INT]	= 0x4,
 	[PLL_CTRL]	= 0x8,
 	[PHY_CTRL]	= 0xc,
 };
@@ -52,7 +43,6 @@ enum pll_ctrl_bits {
 	SSPLL_SUSPEND_EN,
 	PLL_SEQ_START,
 	PLL_LOCK,
-	PLL_PDIV,
 };
 
 static const u8 u3pll_ctrl[] = {
@@ -66,29 +56,17 @@ static const u8 u3pll_ctrl[] = {
 #define HSPLL_PDIV_VAL		0x1
 
 static const u8 u2pll_ctrl[] = {
-	[PLL_PDIV]	= 1,
 	[PLL_RESETB]	= 5,
 	[PLL_LOCK]	= 6,
 };
 
 enum bcm_usb_phy_ctrl_bits {
 	CORERDY,
-	AFE_LDO_PWRDWNB,
-	AFE_PLL_PWRDWNB,
-	AFE_BG_PWRDWNB,
-	PHY_ISO,
 	PHY_RESETB,
 	PHY_PCTL,
 };
 
 #define PHY_PCTL_MASK	0xffff
-/*
- * 0x0806 of PCTL_VAL has below bits set
- * BIT-8 : refclk divider 1
- * BIT-3:2: device mode; mode is not effect
- * BIT-1: soft reset active low
- */
-#define HSPHY_PCTL_VAL	0x0806
 #define SSPHY_PCTL_VAL	0x0006
 
 static const u8 u3phy_ctrl[] = {
@@ -98,10 +76,6 @@ static const u8 u3phy_ctrl[] = {
 
 static const u8 u2phy_ctrl[] = {
 	[CORERDY]		= 0,
-	[AFE_LDO_PWRDWNB]	= 1,
-	[AFE_PLL_PWRDWNB]	= 2,
-	[AFE_BG_PWRDWNB]	= 3,
-	[PHY_ISO]		= 4,
 	[PHY_RESETB]		= 5,
 	[PHY_PCTL]		= 6,
 };
@@ -186,38 +160,13 @@ static int bcm_usb_hs_phy_init(struct bcm_usb_phy_cfg *phy_cfg)
 	int ret = 0;
 	void __iomem *regs = phy_cfg->regs;
 	const u8 *offset;
-	u32 rd_data;
 
 	offset = phy_cfg->offset;
 
-	writel(HSPLL_NDIV_INT_VAL, regs + offset[PLL_NDIV_INT]);
-	writel(HSPLL_NDIV_FRAC_VAL, regs + offset[PLL_NDIV_FRAC]);
-
-	rd_data = readl(regs + offset[PLL_CTRL]);
-	rd_data &= ~(HSPLL_PDIV_MASK << u2pll_ctrl[PLL_PDIV]);
-	rd_data |= (HSPLL_PDIV_VAL << u2pll_ctrl[PLL_PDIV]);
-	writel(rd_data, regs + offset[PLL_CTRL]);
-
-	/* Set Core Ready high */
-	bcm_usb_reg32_setbits(regs + offset[PHY_CTRL],
-			      BIT(u2phy_ctrl[CORERDY]));
-
-	/* Maximum timeout for Core Ready done */
-	msleep(30);
-
+	bcm_usb_reg32_clrbits(regs + offset[PLL_CTRL],
+			      BIT(u2pll_ctrl[PLL_RESETB]));
 	bcm_usb_reg32_setbits(regs + offset[PLL_CTRL],
 			      BIT(u2pll_ctrl[PLL_RESETB]));
-	bcm_usb_reg32_setbits(regs + offset[PHY_CTRL],
-			      BIT(u2phy_ctrl[PHY_RESETB]));
-
-
-	rd_data = readl(regs + offset[PHY_CTRL]);
-	rd_data &= ~(PHY_PCTL_MASK << u2phy_ctrl[PHY_PCTL]);
-	rd_data |= (HSPHY_PCTL_VAL << u2phy_ctrl[PHY_PCTL]);
-	writel(rd_data, regs + offset[PHY_CTRL]);
-
-	/* Maximum timeout for PLL reset done */
-	msleep(30);
 
 	ret = bcm_usb_pll_lock_check(regs + offset[PLL_CTRL],
 				     BIT(u2pll_ctrl[PLL_LOCK]));
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread
* Re: [PATCH v1 1/1] drivers: phy: sr-usb: do not use internal fsm for USB2 phy init
  2020-05-13 17:39 [PATCH v1 1/1] drivers: phy: sr-usb: do not use internal fsm for USB2 phy init Rayagonda Kokatanur
@ 2020-05-19  6:52 ` Vinod Koul
  2020-05-19  7:14   ` Rayagonda Kokatanur
  0 siblings, 1 reply; 3+ messages in thread
From: Vinod Koul @ 2020-05-19  6:52 UTC (permalink / raw)
  To: Rayagonda Kokatanur
  Cc: Kishon Vijay Abraham I, Srinath Mannam, linux-kernel, Bharat Gooty

On 13-05-20, 23:09, Rayagonda Kokatanur wrote:
> From: Bharat Gooty <bharat.gooty@broadcom.com>
> 
> During different reboot cycles, USB PHY PLL may not always lock
> during initialization and therefore can cause USB to be not usable.

Ok

> Hence do not use internal FSM programming sequence for the USB
> PHY initialization.

And what is the impact of not using FSM programming sequence? If not
impact why was it added in the first place ?

> Fixes: 4dcddbb38b64 ("phy: sr-usb: Add Stingray USB PHY driver")
> Signed-off-by: Bharat Gooty <bharat.gooty@broadcom.com>
> Signed-off-by: Rayagonda Kokatanur <rayagonda.kokatanur@broadcom.com>
> ---
>  drivers/phy/broadcom/phy-bcm-sr-usb.c | 55 +--------------------------
>  1 file changed, 2 insertions(+), 53 deletions(-)
> 
> diff --git a/drivers/phy/broadcom/phy-bcm-sr-usb.c b/drivers/phy/broadcom/phy-bcm-sr-usb.c
> index fe6c58910e4c..7c7862b4f41f 100644
> --- a/drivers/phy/broadcom/phy-bcm-sr-usb.c
> +++ b/drivers/phy/broadcom/phy-bcm-sr-usb.c
> @@ -16,8 +16,6 @@ enum bcm_usb_phy_version {
>  };
>  
>  enum bcm_usb_phy_reg {
> -	PLL_NDIV_FRAC,
> -	PLL_NDIV_INT,
>  	PLL_CTRL,
>  	PHY_CTRL,
>  	PHY_PLL_CTRL,
> @@ -31,18 +29,11 @@ static const u8 bcm_usb_combo_phy_ss[] = {
>  };
>  
>  static const u8 bcm_usb_combo_phy_hs[] = {
> -	[PLL_NDIV_FRAC]	= 0x04,
> -	[PLL_NDIV_INT]	= 0x08,
>  	[PLL_CTRL]	= 0x0c,
>  	[PHY_CTRL]	= 0x10,
>  };
>  
> -#define HSPLL_NDIV_INT_VAL	0x13
> -#define HSPLL_NDIV_FRAC_VAL	0x1005
> -
>  static const u8 bcm_usb_hs_phy[] = {
> -	[PLL_NDIV_FRAC]	= 0x0,
> -	[PLL_NDIV_INT]	= 0x4,
>  	[PLL_CTRL]	= 0x8,
>  	[PHY_CTRL]	= 0xc,
>  };
> @@ -52,7 +43,6 @@ enum pll_ctrl_bits {
>  	SSPLL_SUSPEND_EN,
>  	PLL_SEQ_START,
>  	PLL_LOCK,
> -	PLL_PDIV,
>  };
>  
>  static const u8 u3pll_ctrl[] = {
> @@ -66,29 +56,17 @@ static const u8 u3pll_ctrl[] = {
>  #define HSPLL_PDIV_VAL		0x1
>  
>  static const u8 u2pll_ctrl[] = {
> -	[PLL_PDIV]	= 1,
>  	[PLL_RESETB]	= 5,
>  	[PLL_LOCK]	= 6,
>  };
>  
>  enum bcm_usb_phy_ctrl_bits {
>  	CORERDY,
> -	AFE_LDO_PWRDWNB,
> -	AFE_PLL_PWRDWNB,
> -	AFE_BG_PWRDWNB,
> -	PHY_ISO,
>  	PHY_RESETB,
>  	PHY_PCTL,
>  };
>  
>  #define PHY_PCTL_MASK	0xffff
> -/*
> - * 0x0806 of PCTL_VAL has below bits set
> - * BIT-8 : refclk divider 1
> - * BIT-3:2: device mode; mode is not effect
> - * BIT-1: soft reset active low
> - */
> -#define HSPHY_PCTL_VAL	0x0806
>  #define SSPHY_PCTL_VAL	0x0006
>  
>  static const u8 u3phy_ctrl[] = {
> @@ -98,10 +76,6 @@ static const u8 u3phy_ctrl[] = {
>  
>  static const u8 u2phy_ctrl[] = {
>  	[CORERDY]		= 0,
> -	[AFE_LDO_PWRDWNB]	= 1,
> -	[AFE_PLL_PWRDWNB]	= 2,
> -	[AFE_BG_PWRDWNB]	= 3,
> -	[PHY_ISO]		= 4,
>  	[PHY_RESETB]		= 5,
>  	[PHY_PCTL]		= 6,
>  };
> @@ -186,38 +160,13 @@ static int bcm_usb_hs_phy_init(struct bcm_usb_phy_cfg *phy_cfg)
>  	int ret = 0;
>  	void __iomem *regs = phy_cfg->regs;
>  	const u8 *offset;
> -	u32 rd_data;
>  
>  	offset = phy_cfg->offset;
>  
> -	writel(HSPLL_NDIV_INT_VAL, regs + offset[PLL_NDIV_INT]);
> -	writel(HSPLL_NDIV_FRAC_VAL, regs + offset[PLL_NDIV_FRAC]);
> -
> -	rd_data = readl(regs + offset[PLL_CTRL]);
> -	rd_data &= ~(HSPLL_PDIV_MASK << u2pll_ctrl[PLL_PDIV]);
> -	rd_data |= (HSPLL_PDIV_VAL << u2pll_ctrl[PLL_PDIV]);
> -	writel(rd_data, regs + offset[PLL_CTRL]);
> -
> -	/* Set Core Ready high */
> -	bcm_usb_reg32_setbits(regs + offset[PHY_CTRL],
> -			      BIT(u2phy_ctrl[CORERDY]));
> -
> -	/* Maximum timeout for Core Ready done */
> -	msleep(30);
> -
> +	bcm_usb_reg32_clrbits(regs + offset[PLL_CTRL],
> +			      BIT(u2pll_ctrl[PLL_RESETB]));
>  	bcm_usb_reg32_setbits(regs + offset[PLL_CTRL],
>  			      BIT(u2pll_ctrl[PLL_RESETB]));
> -	bcm_usb_reg32_setbits(regs + offset[PHY_CTRL],
> -			      BIT(u2phy_ctrl[PHY_RESETB]));
> -
> -
> -	rd_data = readl(regs + offset[PHY_CTRL]);
> -	rd_data &= ~(PHY_PCTL_MASK << u2phy_ctrl[PHY_PCTL]);
> -	rd_data |= (HSPHY_PCTL_VAL << u2phy_ctrl[PHY_PCTL]);
> -	writel(rd_data, regs + offset[PHY_CTRL]);
> -
> -	/* Maximum timeout for PLL reset done */
> -	msleep(30);
>  
>  	ret = bcm_usb_pll_lock_check(regs + offset[PLL_CTRL],
>  				     BIT(u2pll_ctrl[PLL_LOCK]));
> -- 
> 2.17.1

-- 
~Vinod

^ permalink raw reply	[flat|nested] 3+ messages in thread
* Re: [PATCH v1 1/1] drivers: phy: sr-usb: do not use internal fsm for USB2 phy init
  2020-05-19  6:52 ` Vinod Koul
@ 2020-05-19  7:14   ` Rayagonda Kokatanur
  0 siblings, 0 replies; 3+ messages in thread
From: Rayagonda Kokatanur @ 2020-05-19  7:14 UTC (permalink / raw)
  To: Vinod Koul
  Cc: Kishon Vijay Abraham I, Srinath Mannam,
	Linux Kernel Mailing List, Bharat Gooty

Hi Vinod,

On Tue, May 19, 2020 at 12:22 PM Vinod Koul <vkoul@kernel.org> wrote:
>
> On 13-05-20, 23:09, Rayagonda Kokatanur wrote:
> > From: Bharat Gooty <bharat.gooty@broadcom.com>
> >
> > During different reboot cycles, USB PHY PLL may not always lock
> > during initialization and therefore can cause USB to be not usable.
>
> Ok
>
> > Hence do not use internal FSM programming sequence for the USB
> > PHY initialization.
>
> And what is the impact of not using FSM programming sequence? If not
> impact why was it added in the first place ?

We have two methods for PHY bring-up. One is the current method and
the other is the FSM programming sequence. As we have observed PHY pll
lock is not happening after long reboots, we need to use the other
method. Using current method we have tested for 500,000 reboot
iterations and always USB PHY pll lock has happened. Connected USB
devices are detected and enumerated.

Best regards,
Rayagonda
>
> > Fixes: 4dcddbb38b64 ("phy: sr-usb: Add Stingray USB PHY driver")
> > Signed-off-by: Bharat Gooty <bharat.gooty@broadcom.com>
> > Signed-off-by: Rayagonda Kokatanur <rayagonda.kokatanur@broadcom.com>
> > ---
> >  drivers/phy/broadcom/phy-bcm-sr-usb.c | 55 +--------------------------
> >  1 file changed, 2 insertions(+), 53 deletions(-)
> >
> > diff --git a/drivers/phy/broadcom/phy-bcm-sr-usb.c b/drivers/phy/broadcom/phy-bcm-sr-usb.c
> > index fe6c58910e4c..7c7862b4f41f 100644
> > --- a/drivers/phy/broadcom/phy-bcm-sr-usb.c
> > +++ b/drivers/phy/broadcom/phy-bcm-sr-usb.c
> > @@ -16,8 +16,6 @@ enum bcm_usb_phy_version {
> >  };
> >
> >  enum bcm_usb_phy_reg {
> > -     PLL_NDIV_FRAC,
> > -     PLL_NDIV_INT,
> >       PLL_CTRL,
> >       PHY_CTRL,
> >       PHY_PLL_CTRL,
> > @@ -31,18 +29,11 @@ static const u8 bcm_usb_combo_phy_ss[] = {
> >  };
> >
> >  static const u8 bcm_usb_combo_phy_hs[] = {
> > -     [PLL_NDIV_FRAC] = 0x04,
> > -     [PLL_NDIV_INT]  = 0x08,
> >       [PLL_CTRL]      = 0x0c,
> >       [PHY_CTRL]      = 0x10,
> >  };
> >
> > -#define HSPLL_NDIV_INT_VAL   0x13
> > -#define HSPLL_NDIV_FRAC_VAL  0x1005
> > -
> >  static const u8 bcm_usb_hs_phy[] = {
> > -     [PLL_NDIV_FRAC] = 0x0,
> > -     [PLL_NDIV_INT]  = 0x4,
> >       [PLL_CTRL]      = 0x8,
> >       [PHY_CTRL]      = 0xc,
> >  };
> > @@ -52,7 +43,6 @@ enum pll_ctrl_bits {
> >       SSPLL_SUSPEND_EN,
> >       PLL_SEQ_START,
> >       PLL_LOCK,
> > -     PLL_PDIV,
> >  };
> >
> >  static const u8 u3pll_ctrl[] = {
> > @@ -66,29 +56,17 @@ static const u8 u3pll_ctrl[] = {
> >  #define HSPLL_PDIV_VAL               0x1
> >
> >  static const u8 u2pll_ctrl[] = {
> > -     [PLL_PDIV]      = 1,
> >       [PLL_RESETB]    = 5,
> >       [PLL_LOCK]      = 6,
> >  };
> >
> >  enum bcm_usb_phy_ctrl_bits {
> >       CORERDY,
> > -     AFE_LDO_PWRDWNB,
> > -     AFE_PLL_PWRDWNB,
> > -     AFE_BG_PWRDWNB,
> > -     PHY_ISO,
> >       PHY_RESETB,
> >       PHY_PCTL,
> >  };
> >
> >  #define PHY_PCTL_MASK        0xffff
> > -/*
> > - * 0x0806 of PCTL_VAL has below bits set
> > - * BIT-8 : refclk divider 1
> > - * BIT-3:2: device mode; mode is not effect
> > - * BIT-1: soft reset active low
> > - */
> > -#define HSPHY_PCTL_VAL       0x0806
> >  #define SSPHY_PCTL_VAL       0x0006
> >
> >  static const u8 u3phy_ctrl[] = {
> > @@ -98,10 +76,6 @@ static const u8 u3phy_ctrl[] = {
> >
> >  static const u8 u2phy_ctrl[] = {
> >       [CORERDY]               = 0,
> > -     [AFE_LDO_PWRDWNB]       = 1,
> > -     [AFE_PLL_PWRDWNB]       = 2,
> > -     [AFE_BG_PWRDWNB]        = 3,
> > -     [PHY_ISO]               = 4,
> >       [PHY_RESETB]            = 5,
> >       [PHY_PCTL]              = 6,
> >  };
> > @@ -186,38 +160,13 @@ static int bcm_usb_hs_phy_init(struct bcm_usb_phy_cfg *phy_cfg)
> >       int ret = 0;
> >       void __iomem *regs = phy_cfg->regs;
> >       const u8 *offset;
> > -     u32 rd_data;
> >
> >       offset = phy_cfg->offset;
> >
> > -     writel(HSPLL_NDIV_INT_VAL, regs + offset[PLL_NDIV_INT]);
> > -     writel(HSPLL_NDIV_FRAC_VAL, regs + offset[PLL_NDIV_FRAC]);
> > -
> > -     rd_data = readl(regs + offset[PLL_CTRL]);
> > -     rd_data &= ~(HSPLL_PDIV_MASK << u2pll_ctrl[PLL_PDIV]);
> > -     rd_data |= (HSPLL_PDIV_VAL << u2pll_ctrl[PLL_PDIV]);
> > -     writel(rd_data, regs + offset[PLL_CTRL]);
> > -
> > -     /* Set Core Ready high */
> > -     bcm_usb_reg32_setbits(regs + offset[PHY_CTRL],
> > -                           BIT(u2phy_ctrl[CORERDY]));
> > -
> > -     /* Maximum timeout for Core Ready done */
> > -     msleep(30);
> > -
> > +     bcm_usb_reg32_clrbits(regs + offset[PLL_CTRL],
> > +                           BIT(u2pll_ctrl[PLL_RESETB]));
> >       bcm_usb_reg32_setbits(regs + offset[PLL_CTRL],
> >                             BIT(u2pll_ctrl[PLL_RESETB]));
> > -     bcm_usb_reg32_setbits(regs + offset[PHY_CTRL],
> > -                           BIT(u2phy_ctrl[PHY_RESETB]));
> > -
> > -
> > -     rd_data = readl(regs + offset[PHY_CTRL]);
> > -     rd_data &= ~(PHY_PCTL_MASK << u2phy_ctrl[PHY_PCTL]);
> > -     rd_data |= (HSPHY_PCTL_VAL << u2phy_ctrl[PHY_PCTL]);
> > -     writel(rd_data, regs + offset[PHY_CTRL]);
> > -
> > -     /* Maximum timeout for PLL reset done */
> > -     msleep(30);
> >
> >       ret = bcm_usb_pll_lock_check(regs + offset[PLL_CTRL],
> >                                    BIT(u2pll_ctrl[PLL_LOCK]));
> > --
> > 2.17.1
>
> --
> ~Vinod

^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-05-19  7:14 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-13 17:39 [PATCH v1 1/1] drivers: phy: sr-usb: do not use internal fsm for USB2 phy init Rayagonda Kokatanur
2020-05-19  6:52 ` Vinod Koul
2020-05-19  7:14   ` Rayagonda Kokatanur

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.