Hi Rahul, On 22/06/22 04:23PM, Rahul T R wrote: > Add support new compatible for dphy-tx on j721e > and implement dphy ops required. > > Signed-off-by: Rahul T R <r-ravikumar@xxxxxx> > --- > drivers/phy/cadence/Kconfig | 10 ++++++ > drivers/phy/cadence/cdns-dphy.c | 62 +++++++++++++++++++++++++++++++++ > 2 files changed, 72 insertions(+) > > diff --git a/drivers/phy/cadence/Kconfig b/drivers/phy/cadence/Kconfig > index 1adde2d99ae7..18024ac6d511 100644 > --- a/drivers/phy/cadence/Kconfig > +++ b/drivers/phy/cadence/Kconfig > @@ -22,6 +22,16 @@ config PHY_CADENCE_DPHY > system. If M is selected, the module will be called > cdns-dphy. > > +if PHY_CADENCE_DPHY > + > +config PHY_CADENCE_DPHY_J721E > + depends on ARCH_K3 || COMPILE_TEST > + bool "J721E DPHY TX Wiz support" > + default y > + help > + Support J721E Cadence DPHY TX Wiz configuration. > +endif > + > config PHY_CADENCE_DPHY_RX > tristate "Cadence D-PHY Rx Support" > depends on HAS_IOMEM && OF > diff --git a/drivers/phy/cadence/cdns-dphy.c b/drivers/phy/cadence/cdns-dphy.c > index 14f951013b4f..a6b7d696f77a 100644 > --- a/drivers/phy/cadence/cdns-dphy.c > +++ b/drivers/phy/cadence/cdns-dphy.c > @@ -7,6 +7,7 @@ > #include <linux/bitops.h> > #include <linux/clk.h> > #include <linux/io.h> > +#include <linux/iopoll.h> > #include <linux/module.h> > #include <linux/of_address.h> > #include <linux/of_device.h> > @@ -18,6 +19,7 @@ > > #define REG_WAKEUP_TIME_NS 800 > #define DPHY_PLL_RATE_HZ 108000000 > +#define POLL_TIMEOUT_US 1000 > > /* DPHY registers */ > #define DPHY_PMA_CMN(reg) (reg) > @@ -62,6 +64,18 @@ > #define DSI_NULL_FRAME_OVERHEAD 6 > #define DSI_EOT_PKT_SIZE 4 > > +#define DPHY_TX_J721E_WIZ_PLL_CTRL 0xF04 > +#define DPHY_TX_J721E_WIZ_STATUS 0xF08 > +#define DPHY_TX_J721E_WIZ_RST_CTRL 0xF0C > +#define DPHY_TX_J721E_WIZ_PSM_FREQ 0xF10 > + > +#define DPHY_TX_J721E_WIZ_IPDIV GENMASK(4, 0) > +#define DPHY_TX_J721E_WIZ_OPDIV GENMASK(13, 8) > +#define DPHY_TX_J721E_WIZ_FBDIV GENMASK(25, 16) > +#define DPHY_TX_J721E_WIZ_LANE_RSTB BIT(31) > +#define DPHY_TX_WIZ_PLL_LOCK BIT(31) > +#define DPHY_TX_WIZ_O_CMN_READY BIT(31) > + > struct cdns_dphy_cfg { > u8 pll_ipdiv; > u8 pll_opdiv; > @@ -210,6 +224,43 @@ static void cdns_dphy_ref_set_psm_div(struct cdns_dphy *dphy, u8 div) > dphy->regs + DPHY_PSM_CFG); > } > > +#ifdef CONFIG_PHY_CADENCE_DPHY_J721E Honestly, I don't think there is much use for using this config here. I prefer to have as little ifdefs scattered in code as possible. And I don't think the code you add makes much of a difference in terms of size. I have not looked much into the WIZ module but the code looks fine to me. > +static unsigned long cdns_dphy_j721e_get_wakeup_time_ns(struct cdns_dphy *dphy) > +{ > + return 1000000; > +} > + > +static void cdns_dphy_j721e_set_pll_cfg(struct cdns_dphy *dphy, > + const struct cdns_dphy_cfg *cfg) > +{ > + u32 status; > + > + writel(DPHY_CMN_PWM_HIGH(6) | DPHY_CMN_PWM_LOW(0x101) | > + DPHY_CMN_PWM_DIV(0x8), Please avoid using magic numbers. Or if you are going to use them, at least explain in a comment what they are doing. > + dphy->regs + DPHY_CMN_PWM); > + > + writel((FIELD_PREP(DPHY_TX_J721E_WIZ_IPDIV, cfg->pll_ipdiv) | > + FIELD_PREP(DPHY_TX_J721E_WIZ_OPDIV, cfg->pll_opdiv) | > + FIELD_PREP(DPHY_TX_J721E_WIZ_FBDIV, cfg->pll_fbdiv)), > + dphy->regs + DPHY_TX_J721E_WIZ_PLL_CTRL); > + > + writel(DPHY_TX_J721E_WIZ_LANE_RSTB, > + dphy->regs + DPHY_TX_J721E_WIZ_RST_CTRL); > + > + readl_poll_timeout(dphy->regs + DPHY_TX_J721E_WIZ_PLL_CTRL, status, > + (status & DPHY_TX_WIZ_PLL_LOCK), 0, POLL_TIMEOUT_US); > + > + readl_poll_timeout(dphy->regs + DPHY_TX_J721E_WIZ_STATUS, status, > + (status & DPHY_TX_WIZ_O_CMN_READY), 0, > + POLL_TIMEOUT_US); > +} > + > +static void cdns_dphy_j721e_set_psm_div(struct cdns_dphy *dphy, u8 div) > +{ > + writel(div, dphy->regs + DPHY_TX_J721E_WIZ_PSM_FREQ); > +} > +#endif /* !CONFIG_PHY_CADENCE_DPHY_J721E */ > + > /* > * This is the reference implementation of DPHY hooks. Specific integration of > * this IP may have to re-implement some of them depending on how they decided > @@ -221,6 +272,14 @@ static const struct cdns_dphy_ops ref_dphy_ops = { > .set_psm_div = cdns_dphy_ref_set_psm_div, > }; > > +#ifdef CONFIG_PHY_CADENCE_DPHY_J721E > +static const struct cdns_dphy_ops j721e_dphy_ops = { > + .get_wakeup_time_ns = cdns_dphy_j721e_get_wakeup_time_ns, > + .set_pll_cfg = cdns_dphy_j721e_set_pll_cfg, > + .set_psm_div = cdns_dphy_j721e_set_psm_div, > +}; > +#endif /* !CONFIG_PHY_CADENCE_DPHY_J721E */ > + > static int cdns_dphy_config_from_opts(struct phy *phy, > struct phy_configure_opts_mipi_dphy *opts, > struct cdns_dphy_cfg *cfg) > @@ -408,6 +467,9 @@ static int cdns_dphy_remove(struct platform_device *pdev) > > static const struct of_device_id cdns_dphy_of_match[] = { > { .compatible = "cdns,dphy", .data = &ref_dphy_ops }, > +#ifdef CONFIG_PHY_CADENCE_DPHY_J721E > + { .compatible = "ti,j721e-dphy", .data = &j721e_dphy_ops }, > +#endif /* !CONFIG_PHY_CADENCE_DPHY_J721E */ > { /* sentinel */ }, > }; > MODULE_DEVICE_TABLE(of, cdns_dphy_of_match); > -- > 2.36.1 > -- Regards, Pratyush Yadav Texas Instruments Inc.