[PATCH v2 0/3] Add support for DPHY TX on J721E

[PATCH v2 0/3] Add support for DPHY TX on J721E

[Rahul T R]

Following series of patches adds support for DPHY TX on TI's J721E
SoC. New compatible is added and required cdns dphy ops are implemented.
The series also adds band ctrl configuration required for dphy tx

v2:
-Fix a build error reported by kernel test robot <lkp@intel.com>
 which uses clang compiler. Did not get the error with GNU Toolchain
 9.2-2019.12

Rahul T R (3):
  phy: dt-bindings: cdns,dphy: Add compatible for dphy on j721e
  phy: cdns-dphy: Add band config for dphy tx
  phy: cdns-dphy: Add support for DPHY TX on J721e

 .../devicetree/bindings/phy/cdns,dphy.yaml    |   5 +-
 drivers/phy/cadence/Kconfig                   |  10 ++
 drivers/phy/cadence/cdns-dphy.c               | 114 +++++++++++++++++-
 3 files changed, 126 insertions(+), 3 deletions(-)

-- 
2.36.1

[PATCH v2 1/3] phy: dt-bindings: cdns,dphy: Add compatible for dphy on j721e

[Rahul T R]

Add compatible to support dphy tx on j721e

Signed-off-by: Rahul T R <r-ravikumar@ti.com>
---
 Documentation/devicetree/bindings/phy/cdns,dphy.yaml | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/phy/cdns,dphy.yaml b/Documentation/devicetree/bindings/phy/cdns,dphy.yaml
index c50629bd1b51..f0e9ca8427bb 100644
--- a/Documentation/devicetree/bindings/phy/cdns,dphy.yaml
+++ b/Documentation/devicetree/bindings/phy/cdns,dphy.yaml
@@ -11,8 +11,9 @@ maintainers:
 
 properties:
   compatible:
-    items:
-      - const: cdns,dphy
+    enum:
+      - cdns,dphy
+      - ti,j721e-dphy
 
   reg:
     maxItems: 1
-- 
2.36.1

[PATCH v2 2/3] phy: cdns-dphy: Add band config for dphy tx

[Rahul T R]

Add support for band ctrl config for dphy tx.

Signed-off-by: Rahul T R <r-ravikumar@ti.com>
---
 drivers/phy/cadence/cdns-dphy.c | 52 ++++++++++++++++++++++++++++++++-
 1 file changed, 51 insertions(+), 1 deletion(-)

diff --git a/drivers/phy/cadence/cdns-dphy.c b/drivers/phy/cadence/cdns-dphy.c
index ba042e39cfaf..ddfa524d8ce7 100644
--- a/drivers/phy/cadence/cdns-dphy.c
+++ b/drivers/phy/cadence/cdns-dphy.c
@@ -4,6 +4,7 @@
  */
 
 #include <linux/bitops.h>
+#include <linux/bitfield.h>
 #include <linux/clk.h>
 #include <linux/io.h>
 #include <linux/module.h>
@@ -45,6 +46,10 @@
 #define DPHY_CMN_OPDIV_FROM_REG		BIT(6)
 #define DPHY_CMN_OPDIV(x)		((x) << 7)
 
+#define DPHY_BAND_CFG			DPHY_PCS(0x0)
+#define DPHY_BAND_CFG_LEFT_BAND		GENMASK(4, 0)
+#define DPHY_BAND_CFG_RIGHT_BAND	GENMASK(9, 5)
+
 #define DPHY_PSM_CFG			DPHY_PCS(0x4)
 #define DPHY_PSM_CFG_FROM_REG		BIT(0)
 #define DPHY_PSM_CLK_DIV(x)		((x) << 1)
@@ -92,6 +97,22 @@ struct cdns_dphy {
 	struct phy *phy;
 };
 
+struct cdns_dphy_band {
+	unsigned int min_rate;
+	unsigned int max_rate;
+};
+
+/* Order of bands is important since the index is the band number. */
+static struct cdns_dphy_band tx_bands[] = {
+	{80, 100}, {100, 120}, {120, 160}, {160, 200}, {200, 240},
+	{240, 320}, {320, 390}, {390, 450}, {450, 510}, {510, 560},
+	{560, 640}, {640, 690}, {690, 770}, {770, 870}, {870, 950},
+	{950, 1000}, {1000, 1200}, {1200, 1400}, {1400, 1600}, {1600, 1800},
+	{1800, 2000}, {2000, 2200}, {2200, 2500}
+};
+
+static int num_tx_bands = ARRAY_SIZE(tx_bands);
+
 static int cdns_dsi_get_dphy_pll_cfg(struct cdns_dphy *dphy,
 				     struct cdns_dphy_cfg *cfg,
 				     struct phy_configure_opts_mipi_dphy *opts,
@@ -232,6 +253,26 @@ static int cdns_dphy_config_from_opts(struct phy *phy,
 	return 0;
 }
 
+static int cdns_dphy_tx_get_band_ctrl(unsigned long hs_clk_rate)
+{
+	unsigned int rate;
+	int i;
+
+	rate = hs_clk_rate / 1000000UL;
+
+	if (rate < tx_bands[0].min_rate || rate >= tx_bands[num_tx_bands - 1].max_rate)
+		return -EOPNOTSUPP;
+
+	for (i = 0; i < num_tx_bands; i++) {
+		if (rate >= tx_bands[i].min_rate && rate < tx_bands[i].max_rate)
+			return i;
+	}
+
+	/* Unreachable. */
+	WARN(1, "Reached unreachable code.");
+	return -EINVAL;
+}
+
 static int cdns_dphy_validate(struct phy *phy, enum phy_mode mode, int submode,
 			      union phy_configure_opts *opts)
 {
@@ -247,7 +288,8 @@ static int cdns_dphy_configure(struct phy *phy, union phy_configure_opts *opts)
 {
 	struct cdns_dphy *dphy = phy_get_drvdata(phy);
 	struct cdns_dphy_cfg cfg = { 0 };
-	int ret;
+	int ret, band_ctrl;
+	unsigned int reg;
 
 	ret = cdns_dphy_config_from_opts(phy, &opts->mipi_dphy, &cfg);
 	if (ret)
@@ -276,6 +318,14 @@ static int cdns_dphy_configure(struct phy *phy, union phy_configure_opts *opts)
 	 */
 	cdns_dphy_set_pll_cfg(dphy, &cfg);
 
+	band_ctrl = cdns_dphy_tx_get_band_ctrl(opts->mipi_dphy.hs_clk_rate);
+	if (band_ctrl < 0)
+		return band_ctrl;
+
+	reg = FIELD_PREP(DPHY_BAND_CFG_LEFT_BAND, band_ctrl) |
+	      FIELD_PREP(DPHY_BAND_CFG_RIGHT_BAND, band_ctrl);
+	writel(reg, dphy->regs + DPHY_BAND_CFG);
+
 	return 0;
 }
 
-- 
2.36.1

[PATCH v2 3/3] phy: cdns-dphy: Add support for DPHY TX on J721e

[Rahul T R]

Add support new compatible for dphy-tx on j721e
and implement dphy ops required.

Signed-off-by: Rahul T R <r-ravikumar@ti.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 ddfa524d8ce7..cba8221eb193 100644
--- a/drivers/phy/cadence/cdns-dphy.c
+++ b/drivers/phy/cadence/cdns-dphy.c
@@ -7,6 +7,7 @@
 #include <linux/bitfield.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;
@@ -220,6 +234,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
+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),
+	       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
@@ -231,6 +282,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)
@@ -420,6 +479,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

Re: [PATCH v2 1/3] phy: dt-bindings: cdns,dphy: Add compatible for dphy on j721e

[Laurent Pinchart]

Hi Rahul,

Thank you for the patch.

On Wed, Jun 22, 2022 at 01:23:38PM +0530, Rahul T R wrote:
> Add compatible to support dphy tx on j721e
> 
> Signed-off-by: Rahul T R <r-ravikumar@ti.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  Documentation/devicetree/bindings/phy/cdns,dphy.yaml | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/phy/cdns,dphy.yaml b/Documentation/devicetree/bindings/phy/cdns,dphy.yaml
> index c50629bd1b51..f0e9ca8427bb 100644
> --- a/Documentation/devicetree/bindings/phy/cdns,dphy.yaml
> +++ b/Documentation/devicetree/bindings/phy/cdns,dphy.yaml
> @@ -11,8 +11,9 @@ maintainers:
>  
>  properties:
>    compatible:
> -    items:
> -      - const: cdns,dphy
> +    enum:
> +      - cdns,dphy
> +      - ti,j721e-dphy
>  
>    reg:
>      maxItems: 1

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 1/3] phy: dt-bindings: cdns,dphy: Add compatible for dphy on j721e

[Laurent Pinchart]

Hi Rahul,

Thank you for the patch.

On Wed, Jun 22, 2022 at 01:23:38PM +0530, Rahul T R wrote:
> Add compatible to support dphy tx on j721e
> 
> Signed-off-by: Rahul T R <r-ravikumar@ti.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  Documentation/devicetree/bindings/phy/cdns,dphy.yaml | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/phy/cdns,dphy.yaml b/Documentation/devicetree/bindings/phy/cdns,dphy.yaml
> index c50629bd1b51..f0e9ca8427bb 100644
> --- a/Documentation/devicetree/bindings/phy/cdns,dphy.yaml
> +++ b/Documentation/devicetree/bindings/phy/cdns,dphy.yaml
> @@ -11,8 +11,9 @@ maintainers:
>  
>  properties:
>    compatible:
> -    items:
> -      - const: cdns,dphy
> +    enum:
> +      - cdns,dphy
> +      - ti,j721e-dphy
>  
>    reg:
>      maxItems: 1

-- 
Regards,

Laurent Pinchart

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v2 2/3] phy: cdns-dphy: Add band config for dphy tx

[Laurent Pinchart]

Hi Rahul,

Thank you for the patch.

On Wed, Jun 22, 2022 at 01:23:39PM +0530, Rahul T R wrote:
> Add support for band ctrl config for dphy tx.
> 
> Signed-off-by: Rahul T R <r-ravikumar@ti.com>
> ---
>  drivers/phy/cadence/cdns-dphy.c | 52 ++++++++++++++++++++++++++++++++-
>  1 file changed, 51 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/phy/cadence/cdns-dphy.c b/drivers/phy/cadence/cdns-dphy.c
> index ba042e39cfaf..ddfa524d8ce7 100644
> --- a/drivers/phy/cadence/cdns-dphy.c
> +++ b/drivers/phy/cadence/cdns-dphy.c
> @@ -4,6 +4,7 @@
>   */
>  
>  #include <linux/bitops.h>
> +#include <linux/bitfield.h>

Nitpicking, bitfield goes before bitops :-)

>  #include <linux/clk.h>
>  #include <linux/io.h>
>  #include <linux/module.h>
> @@ -45,6 +46,10 @@
>  #define DPHY_CMN_OPDIV_FROM_REG		BIT(6)
>  #define DPHY_CMN_OPDIV(x)		((x) << 7)
>  
> +#define DPHY_BAND_CFG			DPHY_PCS(0x0)
> +#define DPHY_BAND_CFG_LEFT_BAND		GENMASK(4, 0)
> +#define DPHY_BAND_CFG_RIGHT_BAND	GENMASK(9, 5)
> +
>  #define DPHY_PSM_CFG			DPHY_PCS(0x4)
>  #define DPHY_PSM_CFG_FROM_REG		BIT(0)
>  #define DPHY_PSM_CLK_DIV(x)		((x) << 1)
> @@ -92,6 +97,22 @@ struct cdns_dphy {
>  	struct phy *phy;
>  };
>  
> +struct cdns_dphy_band {
> +	unsigned int min_rate;
> +	unsigned int max_rate;
> +};
> +
> +/* Order of bands is important since the index is the band number. */
> +static struct cdns_dphy_band tx_bands[] = {

static const

> +	{80, 100}, {100, 120}, {120, 160}, {160, 200}, {200, 240},
> +	{240, 320}, {320, 390}, {390, 450}, {450, 510}, {510, 560},
> +	{560, 640}, {640, 690}, {690, 770}, {770, 870}, {870, 950},
> +	{950, 1000}, {1000, 1200}, {1200, 1400}, {1400, 1600}, {1600, 1800},
> +	{1800, 2000}, {2000, 2200}, {2200, 2500}

The max_rate value of band N is always equal to the min_rate value of
band N+1. Could we store one only ?

> +};
> +
> +static int num_tx_bands = ARRAY_SIZE(tx_bands);

You can use ARRAY_SIZE(tx_bands) directly below and drop this.

> +
>  static int cdns_dsi_get_dphy_pll_cfg(struct cdns_dphy *dphy,
>  				     struct cdns_dphy_cfg *cfg,
>  				     struct phy_configure_opts_mipi_dphy *opts,
> @@ -232,6 +253,26 @@ static int cdns_dphy_config_from_opts(struct phy *phy,
>  	return 0;
>  }
>  
> +static int cdns_dphy_tx_get_band_ctrl(unsigned long hs_clk_rate)
> +{
> +	unsigned int rate;
> +	int i;
> +
> +	rate = hs_clk_rate / 1000000UL;
> +
> +	if (rate < tx_bands[0].min_rate || rate >= tx_bands[num_tx_bands - 1].max_rate)
> +		return -EOPNOTSUPP;
> +
> +	for (i = 0; i < num_tx_bands; i++) {
> +		if (rate >= tx_bands[i].min_rate && rate < tx_bands[i].max_rate)
> +			return i;
> +	}
> +
> +	/* Unreachable. */
> +	WARN(1, "Reached unreachable code.");

I'd drop the WARN() if it's really unreachable.

> +	return -EINVAL;
> +}
> +
>  static int cdns_dphy_validate(struct phy *phy, enum phy_mode mode, int submode,
>  			      union phy_configure_opts *opts)
>  {
> @@ -247,7 +288,8 @@ static int cdns_dphy_configure(struct phy *phy, union phy_configure_opts *opts)
>  {
>  	struct cdns_dphy *dphy = phy_get_drvdata(phy);
>  	struct cdns_dphy_cfg cfg = { 0 };
> -	int ret;
> +	int ret, band_ctrl;
> +	unsigned int reg;
>  
>  	ret = cdns_dphy_config_from_opts(phy, &opts->mipi_dphy, &cfg);
>  	if (ret)
> @@ -276,6 +318,14 @@ static int cdns_dphy_configure(struct phy *phy, union phy_configure_opts *opts)
>  	 */
>  	cdns_dphy_set_pll_cfg(dphy, &cfg);
>  
> +	band_ctrl = cdns_dphy_tx_get_band_ctrl(opts->mipi_dphy.hs_clk_rate);
> +	if (band_ctrl < 0)
> +		return band_ctrl;
> +
> +	reg = FIELD_PREP(DPHY_BAND_CFG_LEFT_BAND, band_ctrl) |
> +	      FIELD_PREP(DPHY_BAND_CFG_RIGHT_BAND, band_ctrl);
> +	writel(reg, dphy->regs + DPHY_BAND_CFG);
> +
>  	return 0;
>  }
>  

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 2/3] phy: cdns-dphy: Add band config for dphy tx

[Laurent Pinchart]

Hi Rahul,

Thank you for the patch.

On Wed, Jun 22, 2022 at 01:23:39PM +0530, Rahul T R wrote:
> Add support for band ctrl config for dphy tx.
> 
> Signed-off-by: Rahul T R <r-ravikumar@ti.com>
> ---
>  drivers/phy/cadence/cdns-dphy.c | 52 ++++++++++++++++++++++++++++++++-
>  1 file changed, 51 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/phy/cadence/cdns-dphy.c b/drivers/phy/cadence/cdns-dphy.c
> index ba042e39cfaf..ddfa524d8ce7 100644
> --- a/drivers/phy/cadence/cdns-dphy.c
> +++ b/drivers/phy/cadence/cdns-dphy.c
> @@ -4,6 +4,7 @@
>   */
>  
>  #include <linux/bitops.h>
> +#include <linux/bitfield.h>

Nitpicking, bitfield goes before bitops :-)

>  #include <linux/clk.h>
>  #include <linux/io.h>
>  #include <linux/module.h>
> @@ -45,6 +46,10 @@
>  #define DPHY_CMN_OPDIV_FROM_REG		BIT(6)
>  #define DPHY_CMN_OPDIV(x)		((x) << 7)
>  
> +#define DPHY_BAND_CFG			DPHY_PCS(0x0)
> +#define DPHY_BAND_CFG_LEFT_BAND		GENMASK(4, 0)
> +#define DPHY_BAND_CFG_RIGHT_BAND	GENMASK(9, 5)
> +
>  #define DPHY_PSM_CFG			DPHY_PCS(0x4)
>  #define DPHY_PSM_CFG_FROM_REG		BIT(0)
>  #define DPHY_PSM_CLK_DIV(x)		((x) << 1)
> @@ -92,6 +97,22 @@ struct cdns_dphy {
>  	struct phy *phy;
>  };
>  
> +struct cdns_dphy_band {
> +	unsigned int min_rate;
> +	unsigned int max_rate;
> +};
> +
> +/* Order of bands is important since the index is the band number. */
> +static struct cdns_dphy_band tx_bands[] = {

static const

> +	{80, 100}, {100, 120}, {120, 160}, {160, 200}, {200, 240},
> +	{240, 320}, {320, 390}, {390, 450}, {450, 510}, {510, 560},
> +	{560, 640}, {640, 690}, {690, 770}, {770, 870}, {870, 950},
> +	{950, 1000}, {1000, 1200}, {1200, 1400}, {1400, 1600}, {1600, 1800},
> +	{1800, 2000}, {2000, 2200}, {2200, 2500}

The max_rate value of band N is always equal to the min_rate value of
band N+1. Could we store one only ?

> +};
> +
> +static int num_tx_bands = ARRAY_SIZE(tx_bands);

You can use ARRAY_SIZE(tx_bands) directly below and drop this.

> +
>  static int cdns_dsi_get_dphy_pll_cfg(struct cdns_dphy *dphy,
>  				     struct cdns_dphy_cfg *cfg,
>  				     struct phy_configure_opts_mipi_dphy *opts,
> @@ -232,6 +253,26 @@ static int cdns_dphy_config_from_opts(struct phy *phy,
>  	return 0;
>  }
>  
> +static int cdns_dphy_tx_get_band_ctrl(unsigned long hs_clk_rate)
> +{
> +	unsigned int rate;
> +	int i;
> +
> +	rate = hs_clk_rate / 1000000UL;
> +
> +	if (rate < tx_bands[0].min_rate || rate >= tx_bands[num_tx_bands - 1].max_rate)
> +		return -EOPNOTSUPP;
> +
> +	for (i = 0; i < num_tx_bands; i++) {
> +		if (rate >= tx_bands[i].min_rate && rate < tx_bands[i].max_rate)
> +			return i;
> +	}
> +
> +	/* Unreachable. */
> +	WARN(1, "Reached unreachable code.");

I'd drop the WARN() if it's really unreachable.

> +	return -EINVAL;
> +}
> +
>  static int cdns_dphy_validate(struct phy *phy, enum phy_mode mode, int submode,
>  			      union phy_configure_opts *opts)
>  {
> @@ -247,7 +288,8 @@ static int cdns_dphy_configure(struct phy *phy, union phy_configure_opts *opts)
>  {
>  	struct cdns_dphy *dphy = phy_get_drvdata(phy);
>  	struct cdns_dphy_cfg cfg = { 0 };
> -	int ret;
> +	int ret, band_ctrl;
> +	unsigned int reg;
>  
>  	ret = cdns_dphy_config_from_opts(phy, &opts->mipi_dphy, &cfg);
>  	if (ret)
> @@ -276,6 +318,14 @@ static int cdns_dphy_configure(struct phy *phy, union phy_configure_opts *opts)
>  	 */
>  	cdns_dphy_set_pll_cfg(dphy, &cfg);
>  
> +	band_ctrl = cdns_dphy_tx_get_band_ctrl(opts->mipi_dphy.hs_clk_rate);
> +	if (band_ctrl < 0)
> +		return band_ctrl;
> +
> +	reg = FIELD_PREP(DPHY_BAND_CFG_LEFT_BAND, band_ctrl) |
> +	      FIELD_PREP(DPHY_BAND_CFG_RIGHT_BAND, band_ctrl);
> +	writel(reg, dphy->regs + DPHY_BAND_CFG);
> +
>  	return 0;
>  }
>  

-- 
Regards,

Laurent Pinchart

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v2 2/3] phy: cdns-dphy: Add band config for dphy tx

[Rahul T R]

On 11:16-20220622, Laurent Pinchart wrote:
> Hi Rahul,
> 
> Thank you for the patch.
> 
> On Wed, Jun 22, 2022 at 01:23:39PM +0530, Rahul T R wrote:
> > Add support for band ctrl config for dphy tx.
> > 
> > Signed-off-by: Rahul T R <r-ravikumar@ti.com>
> > ---
> >  drivers/phy/cadence/cdns-dphy.c | 52 ++++++++++++++++++++++++++++++++-
> >  1 file changed, 51 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/phy/cadence/cdns-dphy.c b/drivers/phy/cadence/cdns-dphy.c
> > index ba042e39cfaf..ddfa524d8ce7 100644
> > --- a/drivers/phy/cadence/cdns-dphy.c
> > +++ b/drivers/phy/cadence/cdns-dphy.c
> > @@ -4,6 +4,7 @@
> >   */
> >  
> >  #include <linux/bitops.h>
> > +#include <linux/bitfield.h>
> 
> Nitpicking, bitfield goes before bitops :-)
> 
> >  #include <linux/clk.h>
> >  #include <linux/io.h>
> >  #include <linux/module.h>
> > @@ -45,6 +46,10 @@
> >  #define DPHY_CMN_OPDIV_FROM_REG		BIT(6)
> >  #define DPHY_CMN_OPDIV(x)		((x) << 7)
> >  
> > +#define DPHY_BAND_CFG			DPHY_PCS(0x0)
> > +#define DPHY_BAND_CFG_LEFT_BAND		GENMASK(4, 0)
> > +#define DPHY_BAND_CFG_RIGHT_BAND	GENMASK(9, 5)
> > +
> >  #define DPHY_PSM_CFG			DPHY_PCS(0x4)
> >  #define DPHY_PSM_CFG_FROM_REG		BIT(0)
> >  #define DPHY_PSM_CLK_DIV(x)		((x) << 1)
> > @@ -92,6 +97,22 @@ struct cdns_dphy {
> >  	struct phy *phy;
> >  };
> >  
> > +struct cdns_dphy_band {
> > +	unsigned int min_rate;
> > +	unsigned int max_rate;
> > +};
> > +
> > +/* Order of bands is important since the index is the band number. */
> > +static struct cdns_dphy_band tx_bands[] = {
> 
> static const
> 
> > +	{80, 100}, {100, 120}, {120, 160}, {160, 200}, {200, 240},
> > +	{240, 320}, {320, 390}, {390, 450}, {450, 510}, {510, 560},
> > +	{560, 640}, {640, 690}, {690, 770}, {770, 870}, {870, 950},
> > +	{950, 1000}, {1000, 1200}, {1200, 1400}, {1400, 1600}, {1600, 1800},
> > +	{1800, 2000}, {2000, 2200}, {2200, 2500}
> 
> The max_rate value of band N is always equal to the min_rate value of
> band N+1. Could we store one only ?
> 
> > +};
> > +
> > +static int num_tx_bands = ARRAY_SIZE(tx_bands);
> 
> You can use ARRAY_SIZE(tx_bands) directly below and drop this.
> 
> > +
> >  static int cdns_dsi_get_dphy_pll_cfg(struct cdns_dphy *dphy,
> >  				     struct cdns_dphy_cfg *cfg,
> >  				     struct phy_configure_opts_mipi_dphy *opts,
> > @@ -232,6 +253,26 @@ static int cdns_dphy_config_from_opts(struct phy *phy,
> >  	return 0;
> >  }
> >  
> > +static int cdns_dphy_tx_get_band_ctrl(unsigned long hs_clk_rate)
> > +{
> > +	unsigned int rate;
> > +	int i;
> > +
> > +	rate = hs_clk_rate / 1000000UL;
> > +
> > +	if (rate < tx_bands[0].min_rate || rate >= tx_bands[num_tx_bands - 1].max_rate)
> > +		return -EOPNOTSUPP;
> > +
> > +	for (i = 0; i < num_tx_bands; i++) {
> > +		if (rate >= tx_bands[i].min_rate && rate < tx_bands[i].max_rate)
> > +			return i;
> > +	}
> > +
> > +	/* Unreachable. */
> > +	WARN(1, "Reached unreachable code.");
> 
> I'd drop the WARN() if it's really unreachable.
>

Hi Laurent,

Thanks for the review!
I have sent a v3, addressing
all the comments

Please review

Regards
Rahul T R

> > +	return -EINVAL;
> > +}
> > +
> >  static int cdns_dphy_validate(struct phy *phy, enum phy_mode mode, int submode,
> >  			      union phy_configure_opts *opts)
> >  {
> > @@ -247,7 +288,8 @@ static int cdns_dphy_configure(struct phy *phy, union phy_configure_opts *opts)
> >  {
> >  	struct cdns_dphy *dphy = phy_get_drvdata(phy);
> >  	struct cdns_dphy_cfg cfg = { 0 };
> > -	int ret;
> > +	int ret, band_ctrl;
> > +	unsigned int reg;
> >  
> >  	ret = cdns_dphy_config_from_opts(phy, &opts->mipi_dphy, &cfg);
> >  	if (ret)
> > @@ -276,6 +318,14 @@ static int cdns_dphy_configure(struct phy *phy, union phy_configure_opts *opts)
> >  	 */
> >  	cdns_dphy_set_pll_cfg(dphy, &cfg);
> >  
> > +	band_ctrl = cdns_dphy_tx_get_band_ctrl(opts->mipi_dphy.hs_clk_rate);
> > +	if (band_ctrl < 0)
> > +		return band_ctrl;
> > +
> > +	reg = FIELD_PREP(DPHY_BAND_CFG_LEFT_BAND, band_ctrl) |
> > +	      FIELD_PREP(DPHY_BAND_CFG_RIGHT_BAND, band_ctrl);
> > +	writel(reg, dphy->regs + DPHY_BAND_CFG);
> > +
> >  	return 0;
> >  }
> >  
> 
> -- 
> Regards,
> 
> Laurent Pinchart