Re: [PATCH v1 1/1] drivers: phy: sr-usb: do not use internal fsm for USB2 phy init

From: Vinod Koul
Date: Tue May 19 2020 - 02:53:00 EST


On 13-05-20, 23:09, Rayagonda Kokatanur wrote:
> From: Bharat Gooty <bharat.gooty@xxxxxxxxxxxx>
>
> 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@xxxxxxxxxxxx>
> Signed-off-by: Rayagonda Kokatanur <rayagonda.kokatanur@xxxxxxxxxxxx>
> ---
> 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