Hi Vinod,
> -----Original Message-----
> From: Vinod Koul <vkoul@xxxxxxxxxx>
> Sent: 12 August 2025 07:52 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 6/6] phy: exynos5-usbdrd: support SS combo phy for
> ExynosAutov920
>
> On 05-08-25, 17:22, Pritam Manohar Sutar wrote:
> > Add required change in phy driver to support combo SS phy for this SoC.
> >
> > Signed-off-by: Pritam Manohar Sutar <pritam.sutar@xxxxxxxxxxx>
> > ---
> > drivers/phy/samsung/phy-exynos5-usbdrd.c | 327
> +++++++++++++++++++-
> > include/linux/soc/samsung/exynos-regs-pmu.h | 1 +
> > 2 files changed, 324 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/phy/samsung/phy-exynos5-usbdrd.c
> > b/drivers/phy/samsung/phy-exynos5-usbdrd.c
> > index c22f4de7d094..1108f0c07755 100644
> > --- a/drivers/phy/samsung/phy-exynos5-usbdrd.c
> > +++ b/drivers/phy/samsung/phy-exynos5-usbdrd.c
> > @@ -273,6 +273,36 @@
> > #define EXYNOSAUTOV920_DRD_HSPPLLTUNE 0x110
> > #define HSPPLLTUNE_FSEL GENMASK(18, 16)
> >
> > +/* ExynosAutov920 phy usb31drd port reg */
> > +#define EXYNOSAUTOV920_USB31DRD_PHY_RST_CTRL 0x000
> > +#define PHY_RST_CTRL_PIPE_LANE0_RESET_N_OVRD_EN BIT(5)
> > +#define PHY_RST_CTRL_PIPE_LANE0_RESET_N BIT(4)
> > +#define PHY_RST_CTRL_PHY_RESET_OVRD_EN BIT(1)
> > +#define PHY_RST_CTRL_PHY_RESET BIT(0)
> > +
> > +#define EXYNOSAUTOV920_USB31DRD_PHY_CR_PARA_CON0
> 0x0004
> > +#define PHY_CR_PARA_CON0_PHY0_CR_PARA_ADDR
> GENMASK(31, 16)
> > +#define PHY_CR_PARA_CON0_PHY0_CR_PARA_CLK BIT(8)
> > +#define PHY_CR_PARA_CON0_PHY0_CR_PARA_ACK BIT(4)
> > +#define PHY_CR_PARA_CON0_PHY0_CR_PARA_SEL BIT(0)
> > +
> > +#define EXYNOSAUTOV920_USB31DRD_PHY_CR_PARA_CON1
> 0x0008
> > +
> > +#define EXYNOSAUTOV920_USB31DRD_PHY_CR_PARA_CON2
> 0x000c
> > +#define PHY_CR_PARA_CON2_PHY0_CR_PARA_WR_EN BIT(0)
> > +#define PHY_CR_PARA_CON2_PHY0_CR_PARA_WR_DATA
> GENMASK(31, 16)
> > +
> > +#define EXYNOSAUTOV920_USB31DRD_PHY_CONFIG0 0x100
> > +#define PHY_CONFIG0_PHY0_PMA_PWR_STABLE BIT(14)
> > +#define PHY_CONFIG0_PHY0_PCS_PWR_STABLE BIT(13)
> > +#define PHY_CONFIG0_PHY0_ANA_PWR_EN BIT(1)
> > +
> > +#define EXYNOSAUTOV920_USB31DRD_PHY_CONFIG7 0x11c
> > +#define PHY_CONFIG7_PHY_TEST_POWERDOWN BIT(24)
> > +
> > +#define EXYNOSAUTOV920_USB31DRD_PHY_CONFIG4 0x110
> > +#define PHY_CONFIG4_PIPE_RX0_SRIS_MODE_EN BIT(2)
> > +
> > /* Exynos9 - GS101 */
> > #define EXYNOS850_DRD_SECPMACTL 0x48
> > #define SECPMACTL_PMA_ROPLL_REF_CLK_SEL
> GENMASK(13, 12)
> > @@ -2077,6 +2107,253 @@ static const struct
> exynos5_usbdrd_phy_drvdata exynos990_usbdrd_phy = {
> > .n_regulators = ARRAY_SIZE(exynos5_regulator_names),
> > };
> >
> > +static void
> > +exynosautov920_usb31drd_cr_clk(struct exynos5_usbdrd_phy *phy_drd,
> > +bool high) {
> > + void __iomem *reg_phy = phy_drd->reg_phy;
> > + u32 reg = 0;
>
> again..
>
> > +
> > + reg = readl(reg_phy +
> EXYNOSAUTOV920_USB31DRD_PHY_CR_PARA_CON0);
> > + if (high)
> > + reg |= PHY_CR_PARA_CON0_PHY0_CR_PARA_CLK;
> > + else
> > + reg &= ~PHY_CR_PARA_CON0_PHY0_CR_PARA_CLK;
> > +
> > + writel(reg, reg_phy +
> EXYNOSAUTOV920_USB31DRD_PHY_CR_PARA_CON0);
> > + fsleep(1);
> > +}
> > +
> > +static void
> > +exynosautov920_usb31drd_port_phy_ready(struct exynos5_usbdrd_phy
> > +*phy_drd) {
> > + struct device *dev = phy_drd->dev;
> > + void __iomem *reg_phy = phy_drd->reg_phy;
> > + static const unsigned int timeout_us = 20000;
> > + static const unsigned int sleep_us = 40;
> > + u32 reg = 0;
>
> here too
>
> > + int err;
> > +
> > + /* Clear cr_para_con */
> > + reg &= ~(PHY_CR_PARA_CON0_PHY0_CR_PARA_CLK |
> > + PHY_CR_PARA_CON0_PHY0_CR_PARA_ADDR);
> > + reg |= PHY_CR_PARA_CON0_PHY0_CR_PARA_SEL;
> > + writel(reg, reg_phy +
> EXYNOSAUTOV920_USB31DRD_PHY_CR_PARA_CON0);
> > + writel(0x0, reg_phy +
> EXYNOSAUTOV920_USB31DRD_PHY_CR_PARA_CON1);
> > + writel(0x0, reg_phy +
> EXYNOSAUTOV920_USB31DRD_PHY_CR_PARA_CON2);
> > +
> > + exynosautov920_usb31drd_cr_clk(phy_drd, true);
> > + exynosautov920_usb31drd_cr_clk(phy_drd, false);
> > +
> > + /*
> > + * The maximum time from phy reset de-assertion to de-assertion of
> > + * tx/rx_ack can be as high as 5ms in fast simulation mode.
> > + * Time to phy ready is < 20ms
> > + */
> > + err = readl_poll_timeout(reg_phy +
> > +
> EXYNOSAUTOV920_USB31DRD_PHY_CR_PARA_CON0,
> > + reg, !(reg &
> PHY_CR_PARA_CON0_PHY0_CR_PARA_ACK),
> > + sleep_us, timeout_us);
> > + if (err)
> > + dev_err(dev, "timed out waiting for rx/tx_ack: %#.8x\n",
> reg);
> > +
> > + reg &= ~PHY_CR_PARA_CON0_PHY0_CR_PARA_CLK;
> > + writel(reg, reg_phy +
> EXYNOSAUTOV920_USB31DRD_PHY_CR_PARA_CON0);
> > +}
> > +
> > +static void
> > +exynosautov920_usb31drd_cr_write(struct exynos5_usbdrd_phy
> *phy_drd,
> > + u16 addr, u16 data)
> > +{
> > + struct device *dev = phy_drd->dev;
> > + void __iomem *reg_phy = phy_drd->reg_phy;
> > + u32 cnt = 0;
> > + u32 reg = 0;
>
> this one, former is okay
>
> > +
> > + /* Pre Clocking */
> > + reg = readl(reg_phy +
> EXYNOSAUTOV920_USB31DRD_PHY_CR_PARA_CON0);
> > + reg |= PHY_CR_PARA_CON0_PHY0_CR_PARA_SEL;
> > + writel(reg, reg_phy +
> EXYNOSAUTOV920_USB31DRD_PHY_CR_PARA_CON0);
> > +
> > + /*
> > + * tx clks must be available prior to assertion of tx req.
> > + * tx pstate p2 to p0 transition directly is not permitted.
> > + * tx clk ready must be asserted synchronously on tx clk prior
> > + * to internal transmit clk alignment sequence in the phy
> > + * when entering from p2 to p1 to p0.
> > + */
> > + do {
> > + exynosautov920_usb31drd_cr_clk(phy_drd, true);
> > + exynosautov920_usb31drd_cr_clk(phy_drd, false);
> > + cnt++;
> > + } while (cnt < 15);
> > +
> > + reg &= ~PHY_CR_PARA_CON0_PHY0_CR_PARA_SEL;
> > + writel(reg, reg_phy +
> EXYNOSAUTOV920_USB31DRD_PHY_CR_PARA_CON0);
> > +
> > + /*
> > + * tx data path is active when tx lane is in p0 state
> > + * and tx data en asserted. enable cr_para_wr_en.
> > + */
> > + reg = readl(reg_phy +
> EXYNOSAUTOV920_USB31DRD_PHY_CR_PARA_CON2);
> > + reg &= ~PHY_CR_PARA_CON2_PHY0_CR_PARA_WR_DATA;
> > + reg |=
> FIELD_PREP(PHY_CR_PARA_CON2_PHY0_CR_PARA_WR_DATA, data) |
> > + PHY_CR_PARA_CON2_PHY0_CR_PARA_WR_EN;
> > + writel(reg, reg_phy +
> EXYNOSAUTOV920_USB31DRD_PHY_CR_PARA_CON2);
> > +
> > + /* write addr */
> > + reg = readl(reg_phy +
> EXYNOSAUTOV920_USB31DRD_PHY_CR_PARA_CON0);
> > + reg &= ~PHY_CR_PARA_CON0_PHY0_CR_PARA_ADDR;
> > + reg |= FIELD_PREP(PHY_CR_PARA_CON0_PHY0_CR_PARA_ADDR,
> addr) |
> > + PHY_CR_PARA_CON0_PHY0_CR_PARA_CLK |
> > + PHY_CR_PARA_CON0_PHY0_CR_PARA_SEL;
> > + writel(reg, reg_phy +
> EXYNOSAUTOV920_USB31DRD_PHY_CR_PARA_CON0);
> > +
> > + /* check cr_para_ack*/
> > + cnt = 0;
> > + do {
> > + /*
> > + * data symbols are captured by phy on rising edge of the
> > + * tx_clk when tx data enabled.
> > + * completion of the write cycle is acknowledged by
assertion
> > + * of the cr_para_ack.
> > + */
> > + exynosautov920_usb31drd_cr_clk(phy_drd, true);
> > + reg = readl(reg_phy +
> EXYNOSAUTOV920_USB31DRD_PHY_CR_PARA_CON0);
> > + if ((reg & PHY_CR_PARA_CON0_PHY0_CR_PARA_ACK))
> > + break;
> > +
> > + exynosautov920_usb31drd_cr_clk(phy_drd, false);
> > +
> > + /*
> > + * wait for minimum of 10 cr_para_clk cycles after phy reset
> > + * is negated, before accessing control regs to allow for
> > + * internal resets.
> > + */
> > + cnt++;
> > + } while (cnt < 10);
> > +
> > + if (cnt == 10)
> > + dev_dbg(dev, "CR write failed to 0x%04x\n", addr);
>
> Not error?
This is only for debugging purpose. It is not considered as error.
> --
> ~Vinod
Will address other comments in next version of the patch-set (v6).
Thank you.
Regards,
Pritam
