[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20220623094523.ccrsmw477a5btgcf@ti.com>
Date: Thu, 23 Jun 2022 15:15:23 +0530
From: Pratyush Yadav <p.yadav@...com>
To: Rahul T R <r-ravikumar@...com>
CC: <linux-phy@...ts.infradead.org>, <kishon@...com>,
<vkoul@...nel.org>, <robh+dt@...nel.org>,
<krzysztof.kozlowski+dt@...aro.org>,
<laurent.pinchart@...asonboard.com>,
<tomi.valkeinen@...asonboard.com>, <linux-kernel@...r.kernel.org>,
<jpawar@...ence.com>, <sjakhade@...ence.com>, <mparab@...ence.com>,
<devicetree@...r.kernel.org>, <vigneshr@...com>,
<lee.jones@...aro.org>
Subject: Re: [PATCH v3 3/3] phy: cdns-dphy: Add support for DPHY TX on J721e
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@...com>
> ---
> 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.
Powered by blists - more mailing lists