Hi Vinod,
> -----Original Message-----
> From: Vinod Koul <vkoul@xxxxxxxxxx>
> Sent: 12 August 2025 07:48 PM
> To: Pritam Manohar Sutar <pritam.sutar@xxxxxxxxxxx>
> Cc: kishon@xxxxxxxxxx; robh@xxxxxxxxxx; krzk+dt@xxxxxxxxxx;
> conor+dt@xxxxxxxxxx; alim.akhtar@xxxxxxxxxxx; andre.draszik@xxxxxxxxxx;
> peter.griffin@xxxxxxxxxx; kauschluss@xxxxxxxxxxx;
> ivo.ivanov.ivanov1@xxxxxxxxx; igor.belwon@xxxxxxxxxxxxxxxxxxxxxxxxxx;
> m.szyprowski@xxxxxxxxxxx; s.nawrocki@xxxxxxxxxxx; linux-
> phy@xxxxxxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx;
linux-samsung-
> soc@xxxxxxxxxxxxxxx; rosa.pila@xxxxxxxxxxx; dev.tailor@xxxxxxxxxxx;
> faraz.ata@xxxxxxxxxxx; muhammed.ali@xxxxxxxxxxx;
> selvarasu.g@xxxxxxxxxxx
> Subject: Re: [PATCH v5 4/6] phy: exynos5-usbdrd: support HS combo phy for
> ExynosAutov920
>
> On 05-08-25, 17:22, Pritam Manohar Sutar wrote:
> > Support UTMI+ combo phy for this SoC which is somewhat simmilar to
> > what the existing Exynos850 support does. The difference is that some
> > register offsets and bit fields are defferent from Exynos850.
> >
> > Add required change in phy driver to support combo HS phy for this SoC.
> >
> > Signed-off-by: Pritam Manohar Sutar <pritam.sutar@xxxxxxxxxxx>
> > ---
> > drivers/phy/samsung/phy-exynos5-usbdrd.c | 210
> > +++++++++++++++++++++++
> > 1 file changed, 210 insertions(+)
> >
> > diff --git a/drivers/phy/samsung/phy-exynos5-usbdrd.c
> > b/drivers/phy/samsung/phy-exynos5-usbdrd.c
> > index 5400dd23e500..c22f4de7d094 100644
> > --- a/drivers/phy/samsung/phy-exynos5-usbdrd.c
> > +++ b/drivers/phy/samsung/phy-exynos5-usbdrd.c
> > @@ -41,6 +41,13 @@
> > #define EXYNOS2200_CLKRST_LINK_PCLK_SEL BIT(1)
> >
> > #define EXYNOS2200_DRD_UTMI 0x10
> > +
> > +/* ExynosAutov920 bits */
> > +#define UTMICTL_FORCE_UTMI_SUSPEND BIT(13)
> > +#define UTMICTL_FORCE_UTMI_SLEEP BIT(12)
> > +#define UTMICTL_FORCE_DPPULLDOWN BIT(9)
> > +#define UTMICTL_FORCE_DMPULLDOWN BIT(8)
> > +
> > #define EXYNOS2200_UTMI_FORCE_VBUSVALID BIT(1)
> > #define EXYNOS2200_UTMI_FORCE_BVALID BIT(0)
> >
> > @@ -250,6 +257,22 @@
> > #define EXYNOS850_DRD_HSP_TEST 0x5c
> > #define HSP_TEST_SIDDQ BIT(24)
> >
> > +#define EXYNOSAUTOV920_DRD_HSP_CLKRST 0x100
> > +#define HSPCLKRST_PHY20_SW_PORTRESET BIT(3)
> > +#define HSPCLKRST_PHY20_SW_POR BIT(1)
> > +#define HSPCLKRST_PHY20_SW_POR_SEL BIT(0)
> > +
> > +#define EXYNOSAUTOV920_DRD_HSPCTL 0x104
> > +#define HSPCTRL_VBUSVLDEXTSEL BIT(13)
> > +#define HSPCTRL_VBUSVLDEXT BIT(12)
> > +#define HSPCTRL_EN_UTMISUSPEND BIT(9)
> > +#define HSPCTRL_COMMONONN BIT(8)
> > +
> > +#define EXYNOSAUTOV920_DRD_HSP_TEST 0x10c
> > +
> > +#define EXYNOSAUTOV920_DRD_HSPPLLTUNE 0x110
> > +#define HSPPLLTUNE_FSEL GENMASK(18, 16)
> > +
> > /* Exynos9 - GS101 */
> > #define EXYNOS850_DRD_SECPMACTL 0x48
> > #define SECPMACTL_PMA_ROPLL_REF_CLK_SEL GENMASK(13,
> 12)
> > @@ -2054,6 +2077,139 @@ static const struct exynos5_usbdrd_phy_drvdata
> exynos990_usbdrd_phy = {
> > .n_regulators = ARRAY_SIZE(exynos5_regulator_names),
> > };
> >
> > +static void
> > +exynosautov920_usbdrd_utmi_init(struct exynos5_usbdrd_phy *phy_drd) {
> > + void __iomem *reg_phy = phy_drd->reg_phy;
> > + u32 reg;
> > +
> > + /*
> > + * Disable HWACG (hardware auto clock gating control). This
> > + * forces QACTIVE signal in Q-Channel interface to HIGH level,
> > + * to make sure the PHY clock is not gated by the hardware.
> > + */
> > + reg = readl(reg_phy + EXYNOS850_DRD_LINKCTRL);
> > + reg |= LINKCTRL_FORCE_QACT;
> > + writel(reg, reg_phy + EXYNOS850_DRD_LINKCTRL);
>
> maybe add a read-modify-write helper, this is user a lot here
Used this convention for readability purpose. Other SoCs are also using
convention in this file.
Moreover, noted this and will consider to clean-up this file later.
>
> > +
> > + /* De-assert link reset */
> > + reg = readl(reg_phy + EXYNOS2200_DRD_CLKRST);
> > + reg &= ~CLKRST_LINK_SW_RST;
> > + writel(reg, reg_phy + EXYNOS2200_DRD_CLKRST);
> > +
> > + /* Set PHY POR High */
> > + reg = readl(reg_phy + EXYNOSAUTOV920_DRD_HSP_CLKRST);
> > + reg |= HSPCLKRST_PHY20_SW_POR | HSPCLKRST_PHY20_SW_POR_SEL;
> > + writel(reg, reg_phy + EXYNOSAUTOV920_DRD_HSP_CLKRST);
> > +
> > + /* Enable UTMI+ */
> > + reg = readl(reg_phy + EXYNOS2200_DRD_UTMI);
> > + reg &= ~(UTMICTL_FORCE_UTMI_SUSPEND |
> UTMICTL_FORCE_UTMI_SLEEP |
> > + UTMICTL_FORCE_DPPULLDOWN |
> UTMICTL_FORCE_DMPULLDOWN);
> > + writel(reg, reg_phy + EXYNOS2200_DRD_UTMI);
> > +
> > + /* set phy clock & control HS phy */
> > + reg = readl(reg_phy + EXYNOSAUTOV920_DRD_HSPCTL);
> > + reg |= HSPCTRL_EN_UTMISUSPEND | HSPCTRL_COMMONONN;
> > + writel(reg, reg_phy + EXYNOSAUTOV920_DRD_HSPCTL);
> > +
> > + fsleep(100);
> > +
> > + /* Set VBUS Valid and DP-Pull up control by VBUS pad usage */
> > + reg = readl(reg_phy + EXYNOS850_DRD_LINKCTRL);
> > + reg |= FIELD_PREP_CONST(LINKCTRL_BUS_FILTER_BYPASS, 0xf);
> > + writel(reg, reg_phy + EXYNOS850_DRD_LINKCTRL);
> > +
> > + reg = readl(reg_phy + EXYNOS2200_DRD_UTMI);
> > + reg |= EXYNOS2200_UTMI_FORCE_VBUSVALID |
> EXYNOS2200_UTMI_FORCE_BVALID;
> > + writel(reg, reg_phy + EXYNOS2200_DRD_UTMI);
> > +
> > + reg = readl(reg_phy + EXYNOSAUTOV920_DRD_HSPCTL);
> > + reg |= HSPCTRL_VBUSVLDEXTSEL | HSPCTRL_VBUSVLDEXT;
> > + writel(reg, reg_phy + EXYNOSAUTOV920_DRD_HSPCTL);
> > +
> > + /* Setting FSEL for refference clock */
> > + reg = readl(reg_phy + EXYNOSAUTOV920_DRD_HSPPLLTUNE);
> > + reg &= ~HSPPLLTUNE_FSEL;
>
> Empty line here please
Will add empty line.
>
> > + switch (phy_drd->extrefclk) {
> > + case EXYNOS5_FSEL_50MHZ:
> > + reg |= FIELD_PREP(HSPPLLTUNE_FSEL, 7);
> > + break;
> > + case EXYNOS5_FSEL_26MHZ:
> > + reg |= FIELD_PREP(HSPPLLTUNE_FSEL, 6);
> > + break;
> > + case EXYNOS5_FSEL_24MHZ:
> > + reg |= FIELD_PREP(HSPPLLTUNE_FSEL, 2);
> > + break;
> > + case EXYNOS5_FSEL_20MHZ:
> > + reg |= FIELD_PREP(HSPPLLTUNE_FSEL, 1);
> > + break;
> > + case EXYNOS5_FSEL_19MHZ2:
> > + reg |= FIELD_PREP(HSPPLLTUNE_FSEL, 0);
> > + break;
> > + default:
> > + dev_warn(phy_drd->dev, "unsupported ref clk: %#.2x\n",
> > + phy_drd->extrefclk);
>
> but we still continue?
This SoC supports 19.2Mhz refclk and it sets default reg value for this
refclk if it does not find clk.
> --
> ~Vinod
Thank you,
Regards,
Pritam
