Re: [PATCH] phy: Change the configuration interface param to void* to make it more general

Re: [PATCH] phy: Change the configuration interface param to void* to make it more general

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 1064 bytes --]

On Fri, Jul 12, 2019 at 02:04:08AM +0800, Zeng Tao wrote:
> The phy framework now allows runtime configurations, but only limited
> to mipi now, and it's not reasonable to introduce user specified
> configurations into the union phy_configure_opts structure. An simple
> way is to replace with a void *.

I'm not sure why it's unreasonable?

> We have already got some phy drivers which introduce private phy API
> for runtime configurations, and with this patch, they can switch to
> the phy_configure as a replace.

If you have a custom mode of operation, then you'll need a custom
phy_mode as well, and surely you can have a custom set of parameters.

Since those functions are meant to provide a two-way negotiation of
the various parameters, you'll have to have that structure shared
between the two either way, so the only thing required in addition to
what you would have passing a void is one line to add that structure
in the union.

That's barely unreasonable.

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH] phy: Change the configuration interface param to void* to make it more general

[Zeng Tao]

The phy framework now allows runtime configurations, but only limited
to mipi now, and it's not reasonable to introduce user specified
configurations into the union phy_configure_opts structure. An simple
way is to replace with a void *.

We have already got some phy drivers which introduce private phy API
for runtime configurations, and with this patch, they can switch to
the phy_configure as a replace.

Signed-off-by: Zeng Tao <prime.zeng@hisilicon.com>
---
 drivers/phy/allwinner/phy-sun6i-mipi-dphy.c |  4 ++--
 drivers/phy/cadence/cdns-dphy.c             |  8 ++++----
 drivers/phy/phy-core.c                      |  4 ++--
 include/linux/phy/phy.h                     | 24 ++++++------------------
 4 files changed, 14 insertions(+), 26 deletions(-)

diff --git a/drivers/phy/allwinner/phy-sun6i-mipi-dphy.c b/drivers/phy/allwinner/phy-sun6i-mipi-dphy.c
index 79c8af5..6a60473 100644
--- a/drivers/phy/allwinner/phy-sun6i-mipi-dphy.c
+++ b/drivers/phy/allwinner/phy-sun6i-mipi-dphy.c
@@ -105,12 +105,12 @@ static int sun6i_dphy_init(struct phy *phy)
 	return 0;
 }
 
-static int sun6i_dphy_configure(struct phy *phy, union phy_configure_opts *opts)
+static int sun6i_dphy_configure(struct phy *phy, void *opts)
 {
 	struct sun6i_dphy *dphy = phy_get_drvdata(phy);
 	int ret;
 
-	ret = phy_mipi_dphy_config_validate(&opts->mipi_dphy);
+	ret = phy_mipi_dphy_config_validate(opts);
 	if (ret)
 		return ret;
 
diff --git a/drivers/phy/cadence/cdns-dphy.c b/drivers/phy/cadence/cdns-dphy.c
index 90c4e9b..0ec5013 100644
--- a/drivers/phy/cadence/cdns-dphy.c
+++ b/drivers/phy/cadence/cdns-dphy.c
@@ -233,23 +233,23 @@ static int cdns_dphy_config_from_opts(struct phy *phy,
 }
 
 static int cdns_dphy_validate(struct phy *phy, enum phy_mode mode, int submode,
-			      union phy_configure_opts *opts)
+			      void *opts)
 {
 	struct cdns_dphy_cfg cfg = { 0 };
 
 	if (mode != PHY_MODE_MIPI_DPHY)
 		return -EINVAL;
 
-	return cdns_dphy_config_from_opts(phy, &opts->mipi_dphy, &cfg);
+	return cdns_dphy_config_from_opts(phy, opts, &cfg);
 }
 
-static int cdns_dphy_configure(struct phy *phy, union phy_configure_opts *opts)
+static int cdns_dphy_configure(struct phy *phy, void *opts)
 {
 	struct cdns_dphy *dphy = phy_get_drvdata(phy);
 	struct cdns_dphy_cfg cfg = { 0 };
 	int ret;
 
-	ret = cdns_dphy_config_from_opts(phy, &opts->mipi_dphy, &cfg);
+	ret = cdns_dphy_config_from_opts(phy, opts, &cfg);
 	if (ret)
 		return ret;
 
diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
index e3880c4a1..048d4d6 100644
--- a/drivers/phy/phy-core.c
+++ b/drivers/phy/phy-core.c
@@ -420,7 +420,7 @@ EXPORT_SYMBOL_GPL(phy_calibrate);
  *
  * Returns: 0 if successful, an negative error code otherwise
  */
-int phy_configure(struct phy *phy, union phy_configure_opts *opts)
+int phy_configure(struct phy *phy, void *opts)
 {
 	int ret;
 
@@ -455,7 +455,7 @@ EXPORT_SYMBOL_GPL(phy_configure);
  * Returns: 0 if successful, an negative error code otherwise
  */
 int phy_validate(struct phy *phy, enum phy_mode mode, int submode,
-		 union phy_configure_opts *opts)
+		 void *opts)
 {
 	int ret;
 
diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index 15032f14..8948f94 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -16,8 +16,6 @@
 #include <linux/pm_runtime.h>
 #include <linux/regulator/consumer.h>
 
-#include <linux/phy/phy-mipi-dphy.h>
-
 struct phy;
 
 enum phy_mode {
@@ -41,15 +39,6 @@ enum phy_mode {
 	PHY_MODE_SATA
 };
 
-/**
- * union phy_configure_opts - Opaque generic phy configuration
- *
- * @mipi_dphy:	Configuration set applicable for phys supporting
- *		the MIPI_DPHY phy mode.
- */
-union phy_configure_opts {
-	struct phy_configure_opts_mipi_dphy	mipi_dphy;
-};
 
 /**
  * struct phy_ops - set of function pointers for performing phy operations
@@ -80,7 +69,7 @@ struct phy_ops {
 	 *
 	 * Returns: 0 if successful, an negative error code otherwise
 	 */
-	int	(*configure)(struct phy *phy, union phy_configure_opts *opts);
+	int	(*configure)(struct phy *phy, void *opts);
 
 	/**
 	 * @validate:
@@ -99,7 +88,7 @@ struct phy_ops {
 	 * error code otherwise
 	 */
 	int	(*validate)(struct phy *phy, enum phy_mode mode, int submode,
-			    union phy_configure_opts *opts);
+			    void *opts);
 	int	(*reset)(struct phy *phy);
 	int	(*calibrate)(struct phy *phy);
 	void	(*release)(struct phy *phy);
@@ -207,9 +196,9 @@ int phy_power_off(struct phy *phy);
 int phy_set_mode_ext(struct phy *phy, enum phy_mode mode, int submode);
 #define phy_set_mode(phy, mode) \
 	phy_set_mode_ext(phy, mode, 0)
-int phy_configure(struct phy *phy, union phy_configure_opts *opts);
+int phy_configure(struct phy *phy, void *opts);
 int phy_validate(struct phy *phy, enum phy_mode mode, int submode,
-		 union phy_configure_opts *opts);
+		 void *opts);
 
 static inline enum phy_mode phy_get_mode(struct phy *phy)
 {
@@ -354,8 +343,7 @@ static inline int phy_calibrate(struct phy *phy)
 	return -ENOSYS;
 }
 
-static inline int phy_configure(struct phy *phy,
-				union phy_configure_opts *opts)
+static inline int phy_configure(struct phy *phy, void *opts)
 {
 	if (!phy)
 		return 0;
@@ -364,7 +352,7 @@ static inline int phy_configure(struct phy *phy,
 }
 
 static inline int phy_validate(struct phy *phy, enum phy_mode mode, int submode,
-			       union phy_configure_opts *opts)
+			       void *opts)
 {
 	if (!phy)
 		return 0;
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] phy: Change the configuration interface param to void* to make it more general

[kbuild test robot]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=unknown-8bit, Size: 31849 bytes --]

Hi Zeng,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[cannot apply to v5.2 next-20190712]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Zeng-Tao/phy-Change-the-configuration-interface-param-to-void-to-make-it-more-general/20190713-213420
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.1-rc1-7-g2b96cd8-dirty
        make ARCH=x86_64 allmodconfig
        make C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>


sparse warnings: (new ones prefixed by >>)

   include/linux/sched.h:609:43: sparse: sparse: bad integer constant expression
   include/linux/sched.h:609:73: sparse: sparse: invalid named zero-width bitfield `value'
   include/linux/sched.h:610:43: sparse: sparse: bad integer constant expression
   include/linux/sched.h:610:67: sparse: sparse: invalid named zero-width bitfield `bucket_id'
   drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c:165:22: sparse: sparse: using member 'hs_clk_rate' in incomplete struct phy_configure_opts_mipi_dphy
   drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c:169:30: sparse: sparse: using member 'hs_clk_rate' in incomplete struct phy_configure_opts_mipi_dphy
   drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c:173:17: sparse: sparse: using member 'hs_clk_rate' in incomplete struct phy_configure_opts_mipi_dphy
   drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c:200:17: sparse: sparse: using member 'hs_clk_rate' in incomplete struct phy_configure_opts_mipi_dphy
   drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c:206:9: sparse: sparse: using member 'hs_clk_rate' in incomplete struct phy_configure_opts_mipi_dphy
   drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c:211:9: sparse: sparse: using member 'lp_clk_rate' in incomplete struct phy_configure_opts_mipi_dphy
   drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c:216:9: sparse: sparse: using member 'lp_clk_rate' in incomplete struct phy_configure_opts_mipi_dphy
   drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c:220:26: sparse: sparse: using member 'hs_prepare' in incomplete struct phy_configure_opts_mipi_dphy
   drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c:221:17: sparse: sparse: using member 'hs_prepare' in incomplete struct phy_configure_opts_mipi_dphy
   drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c:227:22: sparse: sparse: using member 'hs_prepare' in incomplete struct phy_configure_opts_mipi_dphy
   drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c:230:37: sparse: sparse: using member 'hs_prepare' in incomplete struct phy_configure_opts_mipi_dphy
   drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c:237:26: sparse: sparse: using member 'clk_prepare' in incomplete struct phy_configure_opts_mipi_dphy
   drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c:238:17: sparse: sparse: using member 'clk_prepare' in incomplete struct phy_configure_opts_mipi_dphy
   drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c:244:43: sparse: sparse: using member 'clk_prepare' in incomplete struct phy_configure_opts_mipi_dphy
   drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c:247:30: sparse: sparse: using member 'hs_clk_rate' in incomplete struct phy_configure_opts_mipi_dphy
   drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c:251:29: sparse: sparse: using member 'hs_clk_rate' in incomplete struct phy_configure_opts_mipi_dphy
   drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c:255:30: sparse: sparse: using member 'hs_clk_rate' in incomplete struct phy_configure_opts_mipi_dphy
   drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c:264:22: sparse: sparse: using member 'hs_clk_rate' in incomplete struct phy_configure_opts_mipi_dphy
   drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c:266:27: sparse: sparse: using member 'hs_clk_rate' in incomplete struct phy_configure_opts_mipi_dphy
   drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c:268:27: sparse: sparse: using member 'hs_clk_rate' in incomplete struct phy_configure_opts_mipi_dphy
   drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c:270:27: sparse: sparse: using member 'hs_clk_rate' in incomplete struct phy_configure_opts_mipi_dphy
   drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c:272:27: sparse: sparse: using member 'hs_clk_rate' in incomplete struct phy_configure_opts_mipi_dphy
   drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c:274:27: sparse: sparse: using member 'hs_clk_rate' in incomplete struct phy_configure_opts_mipi_dphy
>> drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c:326:53: sparse: sparse: using member 'mipi_dphy' in incomplete union phy_configure_opts
   drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c:356:54: sparse: sparse: using member 'mipi_dphy' in incomplete union phy_configure_opts
--
   include/linux/sched.h:609:43: sparse: sparse: bad integer constant expression
   include/linux/sched.h:609:73: sparse: sparse: invalid named zero-width bitfield `value'
   include/linux/sched.h:610:43: sparse: sparse: bad integer constant expression
   include/linux/sched.h:610:67: sparse: sparse: invalid named zero-width bitfield `bucket_id'
>> drivers/gpu/drm/bridge/cdns-dsi.c:609:73: sparse: sparse: using member 'mipi_dphy' in incomplete union phy_configure_opts
   drivers/gpu/drm/bridge/cdns-dsi.c:784:73: sparse: sparse: using member 'mipi_dphy' in incomplete union phy_configure_opts

vim +/mipi_dphy +326 drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c

f4c8116e294b12c Guido Günther 2019-06-20  153  
f4c8116e294b12c Guido Günther 2019-06-20  154  static int mixel_dphy_config_from_opts(struct phy *phy,
f4c8116e294b12c Guido Günther 2019-06-20  155  	       struct phy_configure_opts_mipi_dphy *dphy_opts,
f4c8116e294b12c Guido Günther 2019-06-20  156  	       struct mixel_dphy_cfg *cfg)
f4c8116e294b12c Guido Günther 2019-06-20  157  {
f4c8116e294b12c Guido Günther 2019-06-20  158  	struct mixel_dphy_priv *priv = dev_get_drvdata(phy->dev.parent);
f4c8116e294b12c Guido Günther 2019-06-20  159  	unsigned long ref_clk = clk_get_rate(priv->phy_ref_clk);
f4c8116e294b12c Guido Günther 2019-06-20  160  	u32 lp_t, numerator, denominator;
f4c8116e294b12c Guido Günther 2019-06-20  161  	unsigned long long tmp;
f4c8116e294b12c Guido Günther 2019-06-20  162  	u32 n;
f4c8116e294b12c Guido Günther 2019-06-20  163  	int i;
f4c8116e294b12c Guido Günther 2019-06-20  164  
f4c8116e294b12c Guido Günther 2019-06-20  165  	if (dphy_opts->hs_clk_rate > DATA_RATE_MAX_SPEED ||
f4c8116e294b12c Guido Günther 2019-06-20  166  	    dphy_opts->hs_clk_rate < DATA_RATE_MIN_SPEED)
f4c8116e294b12c Guido Günther 2019-06-20  167  		return -EINVAL;
f4c8116e294b12c Guido Günther 2019-06-20  168  
f4c8116e294b12c Guido Günther 2019-06-20  169  	numerator = dphy_opts->hs_clk_rate;
f4c8116e294b12c Guido Günther 2019-06-20  170  	denominator = ref_clk;
f4c8116e294b12c Guido Günther 2019-06-20  171  	get_best_ratio(&numerator, &denominator, 255, 256);
f4c8116e294b12c Guido Günther 2019-06-20  172  	if (!numerator || !denominator) {
f4c8116e294b12c Guido Günther 2019-06-20  173  		dev_err(&phy->dev, "Invalid %d/%d for %ld/%ld\n",
f4c8116e294b12c Guido Günther 2019-06-20  174  			numerator, denominator,
f4c8116e294b12c Guido Günther 2019-06-20  175  			dphy_opts->hs_clk_rate, ref_clk);
f4c8116e294b12c Guido Günther 2019-06-20  176  		return -EINVAL;
f4c8116e294b12c Guido Günther 2019-06-20  177  	}
f4c8116e294b12c Guido Günther 2019-06-20  178  
f4c8116e294b12c Guido Günther 2019-06-20  179  	while ((numerator < 16) && (denominator <= 128)) {
f4c8116e294b12c Guido Günther 2019-06-20  180  		numerator <<= 1;
f4c8116e294b12c Guido Günther 2019-06-20  181  		denominator <<= 1;
f4c8116e294b12c Guido Günther 2019-06-20  182  	}
f4c8116e294b12c Guido Günther 2019-06-20  183  	/*
f4c8116e294b12c Guido Günther 2019-06-20  184  	 * CM ranges between 16 and 255
f4c8116e294b12c Guido Günther 2019-06-20  185  	 * CN ranges between 1 and 32
f4c8116e294b12c Guido Günther 2019-06-20  186  	 * CO is power of 2: 1, 2, 4, 8
f4c8116e294b12c Guido Günther 2019-06-20  187  	 */
f4c8116e294b12c Guido Günther 2019-06-20  188  	i = __ffs(denominator);
f4c8116e294b12c Guido Günther 2019-06-20  189  	if (i > 3)
f4c8116e294b12c Guido Günther 2019-06-20  190  		i = 3;
f4c8116e294b12c Guido Günther 2019-06-20  191  	cfg->cn = denominator >> i;
f4c8116e294b12c Guido Günther 2019-06-20  192  	cfg->co = 1 << i;
f4c8116e294b12c Guido Günther 2019-06-20  193  	cfg->cm = numerator;
f4c8116e294b12c Guido Günther 2019-06-20  194  
f4c8116e294b12c Guido Günther 2019-06-20  195  	if (cfg->cm < 16 || cfg->cm > 255 ||
f4c8116e294b12c Guido Günther 2019-06-20  196  	    cfg->cn < 1 || cfg->cn > 32 ||
f4c8116e294b12c Guido Günther 2019-06-20  197  	    cfg->co < 1 || cfg->co > 8) {
f4c8116e294b12c Guido Günther 2019-06-20  198  		dev_err(&phy->dev, "Invalid CM/CN/CO values: %u/%u/%u\n",
f4c8116e294b12c Guido Günther 2019-06-20  199  			cfg->cm, cfg->cn, cfg->co);
f4c8116e294b12c Guido Günther 2019-06-20  200  		dev_err(&phy->dev, "for hs_clk/ref_clk=%ld/%ld ~ %d/%d\n",
f4c8116e294b12c Guido Günther 2019-06-20  201  			dphy_opts->hs_clk_rate, ref_clk,
f4c8116e294b12c Guido Günther 2019-06-20  202  			numerator, denominator);
f4c8116e294b12c Guido Günther 2019-06-20  203  		return -EINVAL;
f4c8116e294b12c Guido Günther 2019-06-20  204  	}
f4c8116e294b12c Guido Günther 2019-06-20  205  
f4c8116e294b12c Guido Günther 2019-06-20  206  	dev_dbg(&phy->dev, "hs_clk/ref_clk=%ld/%ld ~ %d/%d\n",
f4c8116e294b12c Guido Günther 2019-06-20  207  		dphy_opts->hs_clk_rate, ref_clk, numerator, denominator);
f4c8116e294b12c Guido Günther 2019-06-20  208  
f4c8116e294b12c Guido Günther 2019-06-20  209  	/* LP clock period */
f4c8116e294b12c Guido Günther 2019-06-20  210  	tmp = 1000000000000LL;
f4c8116e294b12c Guido Günther 2019-06-20  211  	do_div(tmp, dphy_opts->lp_clk_rate); /* ps */
f4c8116e294b12c Guido Günther 2019-06-20  212  	if (tmp > ULONG_MAX)
f4c8116e294b12c Guido Günther 2019-06-20  213  		return -EINVAL;
f4c8116e294b12c Guido Günther 2019-06-20  214  
f4c8116e294b12c Guido Günther 2019-06-20  215  	lp_t = tmp;
f4c8116e294b12c Guido Günther 2019-06-20  216  	dev_dbg(&phy->dev, "LP clock %lu, period: %u ps\n",
f4c8116e294b12c Guido Günther 2019-06-20  217  		dphy_opts->lp_clk_rate, lp_t);
f4c8116e294b12c Guido Günther 2019-06-20  218  
f4c8116e294b12c Guido Günther 2019-06-20  219  	/* hs_prepare: in lp clock periods */
f4c8116e294b12c Guido Günther 2019-06-20  220  	if (2 * dphy_opts->hs_prepare > 5 * lp_t) {
f4c8116e294b12c Guido Günther 2019-06-20  221  		dev_err(&phy->dev,
f4c8116e294b12c Guido Günther 2019-06-20  222  			"hs_prepare (%u) > 2.5 * lp clock period (%u)\n",
f4c8116e294b12c Guido Günther 2019-06-20  223  			dphy_opts->hs_prepare, lp_t);
f4c8116e294b12c Guido Günther 2019-06-20  224  		return -EINVAL;
f4c8116e294b12c Guido Günther 2019-06-20  225  	}
f4c8116e294b12c Guido Günther 2019-06-20  226  	/* 00: lp_t, 01: 1.5 * lp_t, 10: 2 * lp_t, 11: 2.5 * lp_t */
f4c8116e294b12c Guido Günther 2019-06-20  227  	if (dphy_opts->hs_prepare < lp_t) {
f4c8116e294b12c Guido Günther 2019-06-20  228  		n = 0;
f4c8116e294b12c Guido Günther 2019-06-20  229  	} else {
f4c8116e294b12c Guido Günther 2019-06-20  230  		tmp = 2 * (dphy_opts->hs_prepare - lp_t);
f4c8116e294b12c Guido Günther 2019-06-20  231  		do_div(tmp, lp_t);
f4c8116e294b12c Guido Günther 2019-06-20  232  		n = tmp;
f4c8116e294b12c Guido Günther 2019-06-20  233  	}
f4c8116e294b12c Guido Günther 2019-06-20  234  	cfg->m_prg_hs_prepare = n;
f4c8116e294b12c Guido Günther 2019-06-20  235  
f4c8116e294b12c Guido Günther 2019-06-20  236  	/* clk_prepare: in lp clock periods */
f4c8116e294b12c Guido Günther 2019-06-20  237  	if (2 * dphy_opts->clk_prepare > 3 * lp_t) {
f4c8116e294b12c Guido Günther 2019-06-20  238  		dev_err(&phy->dev,
f4c8116e294b12c Guido Günther 2019-06-20  239  			"clk_prepare (%u) > 1.5 * lp clock period (%u)\n",
f4c8116e294b12c Guido Günther 2019-06-20  240  			dphy_opts->clk_prepare, lp_t);
f4c8116e294b12c Guido Günther 2019-06-20  241  		return -EINVAL;
f4c8116e294b12c Guido Günther 2019-06-20  242  	}
f4c8116e294b12c Guido Günther 2019-06-20  243  	/* 00: lp_t, 01: 1.5 * lp_t */
f4c8116e294b12c Guido Günther 2019-06-20  244  	cfg->mc_prg_hs_prepare = dphy_opts->clk_prepare > lp_t ? 1 : 0;
f4c8116e294b12c Guido Günther 2019-06-20  245  
f4c8116e294b12c Guido Günther 2019-06-20  246  	/* hs_zero: formula from NXP BSP */
f4c8116e294b12c Guido Günther 2019-06-20 @247  	n = (144 * (dphy_opts->hs_clk_rate / 1000000) - 47500) / 10000;
f4c8116e294b12c Guido Günther 2019-06-20  248  	cfg->m_prg_hs_zero = n < 1 ? 1 : n;
f4c8116e294b12c Guido Günther 2019-06-20  249  
f4c8116e294b12c Guido Günther 2019-06-20  250  	/* clk_zero: formula from NXP BSP */
f4c8116e294b12c Guido Günther 2019-06-20  251  	n = (34 * (dphy_opts->hs_clk_rate / 1000000) - 2500) / 1000;
f4c8116e294b12c Guido Günther 2019-06-20  252  	cfg->mc_prg_hs_zero = n < 1 ? 1 : n;
f4c8116e294b12c Guido Günther 2019-06-20  253  
f4c8116e294b12c Guido Günther 2019-06-20  254  	/* clk_trail, hs_trail: formula from NXP BSP */
f4c8116e294b12c Guido Günther 2019-06-20  255  	n = (103 * (dphy_opts->hs_clk_rate / 1000000) + 10000) / 10000;
f4c8116e294b12c Guido Günther 2019-06-20  256  	if (n > 15)
f4c8116e294b12c Guido Günther 2019-06-20  257  		n = 15;
f4c8116e294b12c Guido Günther 2019-06-20  258  	if (n < 1)
f4c8116e294b12c Guido Günther 2019-06-20  259  		n = 1;
f4c8116e294b12c Guido Günther 2019-06-20  260  	cfg->m_prg_hs_trail = n;
f4c8116e294b12c Guido Günther 2019-06-20  261  	cfg->mc_prg_hs_trail = n;
f4c8116e294b12c Guido Günther 2019-06-20  262  
f4c8116e294b12c Guido Günther 2019-06-20  263  	/* rxhs_settle: formula from NXP BSP */
f4c8116e294b12c Guido Günther 2019-06-20  264  	if (dphy_opts->hs_clk_rate < MBPS(80))
f4c8116e294b12c Guido Günther 2019-06-20  265  		cfg->rxhs_settle = 0x0d;
f4c8116e294b12c Guido Günther 2019-06-20  266  	else if (dphy_opts->hs_clk_rate < MBPS(90))
f4c8116e294b12c Guido Günther 2019-06-20  267  		cfg->rxhs_settle = 0x0c;
f4c8116e294b12c Guido Günther 2019-06-20  268  	else if (dphy_opts->hs_clk_rate < MBPS(125))
f4c8116e294b12c Guido Günther 2019-06-20  269  		cfg->rxhs_settle = 0x0b;
f4c8116e294b12c Guido Günther 2019-06-20  270  	else if (dphy_opts->hs_clk_rate < MBPS(150))
f4c8116e294b12c Guido Günther 2019-06-20  271  		cfg->rxhs_settle = 0x0a;
f4c8116e294b12c Guido Günther 2019-06-20  272  	else if (dphy_opts->hs_clk_rate < MBPS(225))
f4c8116e294b12c Guido Günther 2019-06-20  273  		cfg->rxhs_settle = 0x09;
f4c8116e294b12c Guido Günther 2019-06-20  274  	else if (dphy_opts->hs_clk_rate < MBPS(500))
f4c8116e294b12c Guido Günther 2019-06-20  275  		cfg->rxhs_settle = 0x08;
f4c8116e294b12c Guido Günther 2019-06-20  276  	else
f4c8116e294b12c Guido Günther 2019-06-20  277  		cfg->rxhs_settle = 0x07;
f4c8116e294b12c Guido Günther 2019-06-20  278  
f4c8116e294b12c Guido Günther 2019-06-20  279  	dev_dbg(&phy->dev, "phy_config: %u %u %u %u %u %u %u\n",
f4c8116e294b12c Guido Günther 2019-06-20  280  		cfg->m_prg_hs_prepare, cfg->mc_prg_hs_prepare,
f4c8116e294b12c Guido Günther 2019-06-20  281  		cfg->m_prg_hs_zero, cfg->mc_prg_hs_zero,
f4c8116e294b12c Guido Günther 2019-06-20  282  		cfg->m_prg_hs_trail, cfg->mc_prg_hs_trail,
f4c8116e294b12c Guido Günther 2019-06-20  283  		cfg->rxhs_settle);
f4c8116e294b12c Guido Günther 2019-06-20  284  
f4c8116e294b12c Guido Günther 2019-06-20  285  	return 0;
f4c8116e294b12c Guido Günther 2019-06-20  286  }
f4c8116e294b12c Guido Günther 2019-06-20  287  
f4c8116e294b12c Guido Günther 2019-06-20  288  static void mixel_phy_set_hs_timings(struct phy *phy)
f4c8116e294b12c Guido Günther 2019-06-20  289  {
f4c8116e294b12c Guido Günther 2019-06-20  290  	struct mixel_dphy_priv *priv = phy_get_drvdata(phy);
f4c8116e294b12c Guido Günther 2019-06-20  291  
f4c8116e294b12c Guido Günther 2019-06-20  292  	phy_write(phy, priv->cfg.m_prg_hs_prepare, DPHY_M_PRG_HS_PREPARE);
f4c8116e294b12c Guido Günther 2019-06-20  293  	phy_write(phy, priv->cfg.mc_prg_hs_prepare, DPHY_MC_PRG_HS_PREPARE);
f4c8116e294b12c Guido Günther 2019-06-20  294  	phy_write(phy, priv->cfg.m_prg_hs_zero, DPHY_M_PRG_HS_ZERO);
f4c8116e294b12c Guido Günther 2019-06-20  295  	phy_write(phy, priv->cfg.mc_prg_hs_zero, DPHY_MC_PRG_HS_ZERO);
f4c8116e294b12c Guido Günther 2019-06-20  296  	phy_write(phy, priv->cfg.m_prg_hs_trail, DPHY_M_PRG_HS_TRAIL);
f4c8116e294b12c Guido Günther 2019-06-20  297  	phy_write(phy, priv->cfg.mc_prg_hs_trail, DPHY_MC_PRG_HS_TRAIL);
f4c8116e294b12c Guido Günther 2019-06-20  298  	phy_write(phy, priv->cfg.rxhs_settle, priv->devdata->reg_rxhs_settle);
f4c8116e294b12c Guido Günther 2019-06-20  299  }
f4c8116e294b12c Guido Günther 2019-06-20  300  
f4c8116e294b12c Guido Günther 2019-06-20  301  static int mixel_dphy_set_pll_params(struct phy *phy)
f4c8116e294b12c Guido Günther 2019-06-20  302  {
f4c8116e294b12c Guido Günther 2019-06-20  303  	struct mixel_dphy_priv *priv = dev_get_drvdata(phy->dev.parent);
f4c8116e294b12c Guido Günther 2019-06-20  304  
f4c8116e294b12c Guido Günther 2019-06-20  305  	if (priv->cfg.cm < 16 || priv->cfg.cm > 255 ||
f4c8116e294b12c Guido Günther 2019-06-20  306  	    priv->cfg.cn < 1 || priv->cfg.cn > 32 ||
f4c8116e294b12c Guido Günther 2019-06-20  307  	    priv->cfg.co < 1 || priv->cfg.co > 8) {
f4c8116e294b12c Guido Günther 2019-06-20  308  		dev_err(&phy->dev, "Invalid CM/CN/CO values! (%u/%u/%u)\n",
f4c8116e294b12c Guido Günther 2019-06-20  309  			priv->cfg.cm, priv->cfg.cn, priv->cfg.co);
f4c8116e294b12c Guido Günther 2019-06-20  310  		return -EINVAL;
f4c8116e294b12c Guido Günther 2019-06-20  311  	}
f4c8116e294b12c Guido Günther 2019-06-20  312  	dev_dbg(&phy->dev, "Using CM:%u CN:%u CO:%u\n",
f4c8116e294b12c Guido Günther 2019-06-20  313  		priv->cfg.cm, priv->cfg.cn, priv->cfg.co);
f4c8116e294b12c Guido Günther 2019-06-20  314  	phy_write(phy, CM(priv->cfg.cm), DPHY_CM);
f4c8116e294b12c Guido Günther 2019-06-20  315  	phy_write(phy, CN(priv->cfg.cn), DPHY_CN);
f4c8116e294b12c Guido Günther 2019-06-20  316  	phy_write(phy, CO(priv->cfg.co), DPHY_CO);
f4c8116e294b12c Guido Günther 2019-06-20  317  	return 0;
f4c8116e294b12c Guido Günther 2019-06-20  318  }
f4c8116e294b12c Guido Günther 2019-06-20  319  
f4c8116e294b12c Guido Günther 2019-06-20  320  static int mixel_dphy_configure(struct phy *phy, union phy_configure_opts *opts)
f4c8116e294b12c Guido Günther 2019-06-20  321  {
f4c8116e294b12c Guido Günther 2019-06-20  322  	struct mixel_dphy_priv *priv = phy_get_drvdata(phy);
f4c8116e294b12c Guido Günther 2019-06-20  323  	struct mixel_dphy_cfg cfg = { 0 };
f4c8116e294b12c Guido Günther 2019-06-20  324  	int ret;
f4c8116e294b12c Guido Günther 2019-06-20  325  
f4c8116e294b12c Guido Günther 2019-06-20 @326  	ret = mixel_dphy_config_from_opts(phy, &opts->mipi_dphy, &cfg);
f4c8116e294b12c Guido Günther 2019-06-20  327  	if (ret)
f4c8116e294b12c Guido Günther 2019-06-20  328  		return ret;
f4c8116e294b12c Guido Günther 2019-06-20  329  
f4c8116e294b12c Guido Günther 2019-06-20  330  	/* Update the configuration */
f4c8116e294b12c Guido Günther 2019-06-20  331  	memcpy(&priv->cfg, &cfg, sizeof(struct mixel_dphy_cfg));
f4c8116e294b12c Guido Günther 2019-06-20  332  
f4c8116e294b12c Guido Günther 2019-06-20  333  	phy_write(phy, 0x00, DPHY_LOCK_BYP);
f4c8116e294b12c Guido Günther 2019-06-20  334  	phy_write(phy, 0x01, priv->devdata->reg_tx_rcal);
f4c8116e294b12c Guido Günther 2019-06-20  335  	phy_write(phy, 0x00, priv->devdata->reg_auto_pd_en);
f4c8116e294b12c Guido Günther 2019-06-20  336  	phy_write(phy, 0x02, priv->devdata->reg_rxlprp);
f4c8116e294b12c Guido Günther 2019-06-20  337  	phy_write(phy, 0x02, priv->devdata->reg_rxcdrp);
f4c8116e294b12c Guido Günther 2019-06-20  338  	phy_write(phy, 0x25, DPHY_TST);
f4c8116e294b12c Guido Günther 2019-06-20  339  
f4c8116e294b12c Guido Günther 2019-06-20  340  	mixel_phy_set_hs_timings(phy);
f4c8116e294b12c Guido Günther 2019-06-20  341  	ret = mixel_dphy_set_pll_params(phy);
f4c8116e294b12c Guido Günther 2019-06-20  342  	if (ret < 0)
f4c8116e294b12c Guido Günther 2019-06-20  343  		return ret;
f4c8116e294b12c Guido Günther 2019-06-20  344  
f4c8116e294b12c Guido Günther 2019-06-20  345  	return 0;
f4c8116e294b12c Guido Günther 2019-06-20  346  }
f4c8116e294b12c Guido Günther 2019-06-20  347  
f4c8116e294b12c Guido Günther 2019-06-20  348  static int mixel_dphy_validate(struct phy *phy, enum phy_mode mode, int submode,
f4c8116e294b12c Guido Günther 2019-06-20  349  			       union phy_configure_opts *opts)
f4c8116e294b12c Guido Günther 2019-06-20  350  {
f4c8116e294b12c Guido Günther 2019-06-20  351  	struct mixel_dphy_cfg cfg = { 0 };
f4c8116e294b12c Guido Günther 2019-06-20  352  
f4c8116e294b12c Guido Günther 2019-06-20  353  	if (mode != PHY_MODE_MIPI_DPHY)
f4c8116e294b12c Guido Günther 2019-06-20  354  		return -EINVAL;
f4c8116e294b12c Guido Günther 2019-06-20  355  
f4c8116e294b12c Guido Günther 2019-06-20  356  	return mixel_dphy_config_from_opts(phy, &opts->mipi_dphy, &cfg);
f4c8116e294b12c Guido Günther 2019-06-20  357  }
f4c8116e294b12c Guido Günther 2019-06-20  358  
f4c8116e294b12c Guido Günther 2019-06-20  359  static int mixel_dphy_init(struct phy *phy)
f4c8116e294b12c Guido Günther 2019-06-20  360  {
f4c8116e294b12c Guido Günther 2019-06-20  361  	phy_write(phy, PWR_OFF, DPHY_PD_PLL);
f4c8116e294b12c Guido Günther 2019-06-20  362  	phy_write(phy, PWR_OFF, DPHY_PD_DPHY);
f4c8116e294b12c Guido Günther 2019-06-20  363  
f4c8116e294b12c Guido Günther 2019-06-20  364  	return 0;
f4c8116e294b12c Guido Günther 2019-06-20  365  }
f4c8116e294b12c Guido Günther 2019-06-20  366  
f4c8116e294b12c Guido Günther 2019-06-20  367  static int mixel_dphy_exit(struct phy *phy)
f4c8116e294b12c Guido Günther 2019-06-20  368  {
f4c8116e294b12c Guido Günther 2019-06-20  369  	phy_write(phy, 0, DPHY_CM);
f4c8116e294b12c Guido Günther 2019-06-20  370  	phy_write(phy, 0, DPHY_CN);
f4c8116e294b12c Guido Günther 2019-06-20  371  	phy_write(phy, 0, DPHY_CO);
f4c8116e294b12c Guido Günther 2019-06-20  372  
f4c8116e294b12c Guido Günther 2019-06-20  373  	return 0;
f4c8116e294b12c Guido Günther 2019-06-20  374  }
f4c8116e294b12c Guido Günther 2019-06-20  375  
f4c8116e294b12c Guido Günther 2019-06-20  376  static int mixel_dphy_power_on(struct phy *phy)
f4c8116e294b12c Guido Günther 2019-06-20  377  {
f4c8116e294b12c Guido Günther 2019-06-20  378  	struct mixel_dphy_priv *priv = phy_get_drvdata(phy);
f4c8116e294b12c Guido Günther 2019-06-20  379  	u32 locked;
f4c8116e294b12c Guido Günther 2019-06-20  380  	int ret;
f4c8116e294b12c Guido Günther 2019-06-20  381  
f4c8116e294b12c Guido Günther 2019-06-20  382  	ret = clk_prepare_enable(priv->phy_ref_clk);
f4c8116e294b12c Guido Günther 2019-06-20  383  	if (ret < 0)
f4c8116e294b12c Guido Günther 2019-06-20  384  		return ret;
f4c8116e294b12c Guido Günther 2019-06-20  385  
f4c8116e294b12c Guido Günther 2019-06-20  386  	phy_write(phy, PWR_ON, DPHY_PD_PLL);
f4c8116e294b12c Guido Günther 2019-06-20  387  	ret = regmap_read_poll_timeout(priv->regmap, DPHY_LOCK, locked,
f4c8116e294b12c Guido Günther 2019-06-20  388  				       locked, PLL_LOCK_SLEEP,
f4c8116e294b12c Guido Günther 2019-06-20  389  				       PLL_LOCK_TIMEOUT);
f4c8116e294b12c Guido Günther 2019-06-20  390  	if (ret < 0) {
f4c8116e294b12c Guido Günther 2019-06-20  391  		dev_err(&phy->dev, "Could not get DPHY lock (%d)!\n", ret);
f4c8116e294b12c Guido Günther 2019-06-20  392  		goto clock_disable;
f4c8116e294b12c Guido Günther 2019-06-20  393  	}
f4c8116e294b12c Guido Günther 2019-06-20  394  	phy_write(phy, PWR_ON, DPHY_PD_DPHY);
f4c8116e294b12c Guido Günther 2019-06-20  395  
f4c8116e294b12c Guido Günther 2019-06-20  396  	return 0;
f4c8116e294b12c Guido Günther 2019-06-20  397  clock_disable:
f4c8116e294b12c Guido Günther 2019-06-20  398  	clk_disable_unprepare(priv->phy_ref_clk);
f4c8116e294b12c Guido Günther 2019-06-20  399  	return ret;
f4c8116e294b12c Guido Günther 2019-06-20  400  }
f4c8116e294b12c Guido Günther 2019-06-20  401  
f4c8116e294b12c Guido Günther 2019-06-20  402  static int mixel_dphy_power_off(struct phy *phy)
f4c8116e294b12c Guido Günther 2019-06-20  403  {
f4c8116e294b12c Guido Günther 2019-06-20  404  	struct mixel_dphy_priv *priv = phy_get_drvdata(phy);
f4c8116e294b12c Guido Günther 2019-06-20  405  
f4c8116e294b12c Guido Günther 2019-06-20  406  	phy_write(phy, PWR_OFF, DPHY_PD_PLL);
f4c8116e294b12c Guido Günther 2019-06-20  407  	phy_write(phy, PWR_OFF, DPHY_PD_DPHY);
f4c8116e294b12c Guido Günther 2019-06-20  408  
f4c8116e294b12c Guido Günther 2019-06-20  409  	clk_disable_unprepare(priv->phy_ref_clk);
f4c8116e294b12c Guido Günther 2019-06-20  410  
f4c8116e294b12c Guido Günther 2019-06-20  411  	return 0;
f4c8116e294b12c Guido Günther 2019-06-20  412  }
f4c8116e294b12c Guido Günther 2019-06-20  413  
f4c8116e294b12c Guido Günther 2019-06-20  414  static const struct phy_ops mixel_dphy_phy_ops = {
f4c8116e294b12c Guido Günther 2019-06-20  415  	.init = mixel_dphy_init,
f4c8116e294b12c Guido Günther 2019-06-20  416  	.exit = mixel_dphy_exit,
f4c8116e294b12c Guido Günther 2019-06-20  417  	.power_on = mixel_dphy_power_on,
f4c8116e294b12c Guido Günther 2019-06-20  418  	.power_off = mixel_dphy_power_off,
f4c8116e294b12c Guido Günther 2019-06-20  419  	.configure = mixel_dphy_configure,
f4c8116e294b12c Guido Günther 2019-06-20  420  	.validate = mixel_dphy_validate,
f4c8116e294b12c Guido Günther 2019-06-20  421  	.owner = THIS_MODULE,
f4c8116e294b12c Guido Günther 2019-06-20  422  };
f4c8116e294b12c Guido Günther 2019-06-20  423  
f4c8116e294b12c Guido Günther 2019-06-20  424  static const struct of_device_id mixel_dphy_of_match[] = {
f4c8116e294b12c Guido Günther 2019-06-20  425  	{ .compatible = "fsl,imx8mq-mipi-dphy",
f4c8116e294b12c Guido Günther 2019-06-20  426  	  .data = &mixel_dphy_devdata[MIXEL_IMX8MQ] },
f4c8116e294b12c Guido Günther 2019-06-20  427  	{ /* sentinel */ },
f4c8116e294b12c Guido Günther 2019-06-20  428  };
f4c8116e294b12c Guido Günther 2019-06-20  429  MODULE_DEVICE_TABLE(of, mixel_dphy_of_match);
f4c8116e294b12c Guido Günther 2019-06-20  430  
f4c8116e294b12c Guido Günther 2019-06-20  431  static int mixel_dphy_probe(struct platform_device *pdev)
f4c8116e294b12c Guido Günther 2019-06-20  432  {
f4c8116e294b12c Guido Günther 2019-06-20  433  	struct device *dev = &pdev->dev;
f4c8116e294b12c Guido Günther 2019-06-20  434  	struct device_node *np = dev->of_node;
f4c8116e294b12c Guido Günther 2019-06-20  435  	struct phy_provider *phy_provider;
f4c8116e294b12c Guido Günther 2019-06-20  436  	struct mixel_dphy_priv *priv;
f4c8116e294b12c Guido Günther 2019-06-20  437  	struct resource *res;
f4c8116e294b12c Guido Günther 2019-06-20  438  	struct phy *phy;
f4c8116e294b12c Guido Günther 2019-06-20  439  	void __iomem *base;
f4c8116e294b12c Guido Günther 2019-06-20  440  
f4c8116e294b12c Guido Günther 2019-06-20  441  	if (!np)
f4c8116e294b12c Guido Günther 2019-06-20  442  		return -ENODEV;
f4c8116e294b12c Guido Günther 2019-06-20  443  
f4c8116e294b12c Guido Günther 2019-06-20  444  	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
f4c8116e294b12c Guido Günther 2019-06-20  445  	if (!priv)
f4c8116e294b12c Guido Günther 2019-06-20  446  		return -ENOMEM;
f4c8116e294b12c Guido Günther 2019-06-20  447  
f4c8116e294b12c Guido Günther 2019-06-20  448  	priv->devdata = of_device_get_match_data(&pdev->dev);
f4c8116e294b12c Guido Günther 2019-06-20  449  	if (!priv->devdata)
f4c8116e294b12c Guido Günther 2019-06-20  450  		return -EINVAL;
f4c8116e294b12c Guido Günther 2019-06-20  451  
f4c8116e294b12c Guido Günther 2019-06-20  452  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
f4c8116e294b12c Guido Günther 2019-06-20  453  	base = devm_ioremap_resource(dev, res);
f4c8116e294b12c Guido Günther 2019-06-20  454  	if (IS_ERR(base))
f4c8116e294b12c Guido Günther 2019-06-20  455  		return PTR_ERR(base);
f4c8116e294b12c Guido Günther 2019-06-20  456  
f4c8116e294b12c Guido Günther 2019-06-20  457  	priv->regmap = devm_regmap_init_mmio(&pdev->dev, base,
f4c8116e294b12c Guido Günther 2019-06-20  458  					     &mixel_dphy_regmap_config);
f4c8116e294b12c Guido Günther 2019-06-20  459  	if (IS_ERR(priv->regmap)) {
f4c8116e294b12c Guido Günther 2019-06-20  460  		dev_err(dev, "Couldn't create the DPHY regmap\n");
f4c8116e294b12c Guido Günther 2019-06-20  461  		return PTR_ERR(priv->regmap);
f4c8116e294b12c Guido Günther 2019-06-20  462  	}
f4c8116e294b12c Guido Günther 2019-06-20  463  
f4c8116e294b12c Guido Günther 2019-06-20  464  	priv->phy_ref_clk = devm_clk_get(&pdev->dev, "phy_ref");
f4c8116e294b12c Guido Günther 2019-06-20  465  	if (IS_ERR(priv->phy_ref_clk)) {
f4c8116e294b12c Guido Günther 2019-06-20  466  		dev_err(dev, "No phy_ref clock found\n");
f4c8116e294b12c Guido Günther 2019-06-20  467  		return PTR_ERR(priv->phy_ref_clk);
f4c8116e294b12c Guido Günther 2019-06-20  468  	}
f4c8116e294b12c Guido Günther 2019-06-20  469  	dev_dbg(dev, "phy_ref clock rate: %lu\n",
f4c8116e294b12c Guido Günther 2019-06-20  470  		clk_get_rate(priv->phy_ref_clk));
f4c8116e294b12c Guido Günther 2019-06-20  471  
f4c8116e294b12c Guido Günther 2019-06-20  472  	dev_set_drvdata(dev, priv);
f4c8116e294b12c Guido Günther 2019-06-20  473  
f4c8116e294b12c Guido Günther 2019-06-20  474  	phy = devm_phy_create(dev, np, &mixel_dphy_phy_ops);
f4c8116e294b12c Guido Günther 2019-06-20  475  	if (IS_ERR(phy)) {
f4c8116e294b12c Guido Günther 2019-06-20  476  		dev_err(dev, "Failed to create phy %ld\n", PTR_ERR(phy));
f4c8116e294b12c Guido Günther 2019-06-20  477  		return PTR_ERR(phy);
f4c8116e294b12c Guido Günther 2019-06-20  478  	}
f4c8116e294b12c Guido Günther 2019-06-20  479  	phy_set_drvdata(phy, priv);
f4c8116e294b12c Guido Günther 2019-06-20  480  
f4c8116e294b12c Guido Günther 2019-06-20  481  	phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
f4c8116e294b12c Guido Günther 2019-06-20  482  
f4c8116e294b12c Guido Günther 2019-06-20  483  	return PTR_ERR_OR_ZERO(phy_provider);
f4c8116e294b12c Guido Günther 2019-06-20  484  }
f4c8116e294b12c Guido Günther 2019-06-20  485  
f4c8116e294b12c Guido Günther 2019-06-20  486  static struct platform_driver mixel_dphy_driver = {
f4c8116e294b12c Guido Günther 2019-06-20  487  	.probe	= mixel_dphy_probe,
f4c8116e294b12c Guido Günther 2019-06-20  488  	.driver = {
f4c8116e294b12c Guido Günther 2019-06-20  489  		.name = "mixel-mipi-dphy",
f4c8116e294b12c Guido Günther 2019-06-20  490  		.of_match_table	= mixel_dphy_of_match,
f4c8116e294b12c Guido Günther 2019-06-20  491  	}
f4c8116e294b12c Guido Günther 2019-06-20  492  };
f4c8116e294b12c Guido Günther 2019-06-20  493  module_platform_driver(mixel_dphy_driver);
f4c8116e294b12c Guido Günther 2019-06-20  494  
f4c8116e294b12c Guido Günther 2019-06-20  495  MODULE_AUTHOR("NXP Semiconductor");
f4c8116e294b12c Guido Günther 2019-06-20  496  MODULE_DESCRIPTION("Mixel MIPI-DSI PHY driver");
f4c8116e294b12c Guido Günther 2019-06-20  497  MODULE_LICENSE("GPL");

:::::: The code at line 326 was first introduced by commit
:::::: f4c8116e294b12c360b724173f4b79f232573fb1 phy: Add driver for mixel mipi dphy found on NXP's i.MX8 SoCs

:::::: TO: Guido Günther <agx@sigxcpu.org>
:::::: CC: Kishon Vijay Abraham I <kishon@ti.com>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation


[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

RE: [PATCH] phy: Change the configuration interface param to void* to make it more general

[Zengtao (B)]

Hi Maxime:

Thanks for your reply.

>-----Original Message-----
>From: Maxime Ripard [mailto:maxime.ripard@bootlin.com]
>Sent: Thursday, July 11, 2019 7:21 PM
>To: Zengtao (B) <prime.zeng@hisilicon.com>
>Cc: kishon@ti.com; Chen-Yu Tsai <wens@csie.org>; Paul Kocialkowski
><paul.kocialkowski@bootlin.com>; Sakari Ailus <sakari.ailus@linux.intel.com>;
>linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org
>Subject: Re: [PATCH] phy: Change the configuration interface param to void*
>to make it more general
>
>* PGP Signed by an unknown key
>
>On Fri, Jul 12, 2019 at 02:04:08AM +0800, Zeng Tao wrote:
>> The phy framework now allows runtime configurations, but only limited
>> to mipi now, and it's not reasonable to introduce user specified
>> configurations into the union phy_configure_opts structure. An simple
>> way is to replace with a void *.
>
>I'm not sure why it's unreasonable?
>
The phy.h will need to include vendor specific phy headers, and the union phy_configure_opts
will become more complex. I don't think this is a good solution to include all vendor specific phy
configs into a single union structure. 

>> We have already got some phy drivers which introduce private phy API
>> for runtime configurations, and with this patch, they can switch to
>> the phy_configure as a replace.
>
>If you have a custom mode of operation, then you'll need a custom
>phy_mode as well, and surely you can have a custom set of parameters.
>
>Since those functions are meant to provide a two-way negotiation of the
>various parameters, you'll have to have that structure shared between the
>two either way, so the only thing required in addition to what you would have
>passing a void is one line to add that structure in the union.
>
>That's barely unreasonable.
>
>Maxime
>
>--
>Maxime Ripard, Bootlin
>Embedded Linux and Kernel engineering
>https://bootlin.com
>
>* Unknown Key
>* 0x671851C5
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] phy: Change the configuration interface param to void* to make it more general

[Maxime Ripard]

Hi,

On Wed, Jul 17, 2019 at 06:36:09AM +0000, Zengtao (B) wrote:
> Hi Maxime:
>
> Thanks for your reply.
>
> >-----Original Message-----
> >From: Maxime Ripard [mailto:maxime.ripard@bootlin.com]
> >Sent: Thursday, July 11, 2019 7:21 PM
> >To: Zengtao (B) <prime.zeng@hisilicon.com>
> >Cc: kishon@ti.com; Chen-Yu Tsai <wens@csie.org>; Paul Kocialkowski
> ><paul.kocialkowski@bootlin.com>; Sakari Ailus <sakari.ailus@linux.intel.com>;
> >linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org
> >Subject: Re: [PATCH] phy: Change the configuration interface param to void*
> >to make it more general
> >
> >* PGP Signed by an unknown key
> >
> >On Fri, Jul 12, 2019 at 02:04:08AM +0800, Zeng Tao wrote:
> >> The phy framework now allows runtime configurations, but only limited
> >> to mipi now, and it's not reasonable to introduce user specified
> >> configurations into the union phy_configure_opts structure. An simple
> >> way is to replace with a void *.
> >
> >I'm not sure why it's unreasonable?
> >
>
> The phy.h will need to include vendor specific phy headers

I'm not sure this is an issue.

> and the union phy_configure_opts will become more complex.

And this was the plan all along.

> I don't think this is a good solution to include all vendor specific
> phy configs into a single union structure.

The thing is, as Sakari have stated, this interface was meant as a
generic way to negotiate a configuration between a PHY consumer and a
PHY provider (ie, whatever sends data to the phy and the phy itself).

If you remove the explicit type check, then you need to have prior
knowledge (and agreement) on both sides, which breaks the initial
intent.

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

RE: [PATCH] phy: Change the configuration interface param to void* to make it more general

[Zengtao (B)]

Hi maxime: 

>-----Original Message-----
>From: Maxime Ripard [mailto:maxime.ripard@bootlin.com]
>Sent: Thursday, July 18, 2019 12:38 AM
>To: Zengtao (B) <prime.zeng@hisilicon.com>
>Cc: kishon@ti.com; Chen-Yu Tsai <wens@csie.org>; Paul Kocialkowski
><paul.kocialkowski@bootlin.com>; Sakari Ailus <sakari.ailus@linux.intel.com>;
>linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org
>Subject: Re: [PATCH] phy: Change the configuration interface param to void*
>to make it more general
>
>Hi,
>
>On Wed, Jul 17, 2019 at 06:36:09AM +0000, Zengtao (B) wrote:
>> Hi Maxime:
>>
>> Thanks for your reply.
>>
>> >-----Original Message-----
>> >From: Maxime Ripard [mailto:maxime.ripard@bootlin.com]
>> >Sent: Thursday, July 11, 2019 7:21 PM
>> >To: Zengtao (B) <prime.zeng@hisilicon.com>
>> >Cc: kishon@ti.com; Chen-Yu Tsai <wens@csie.org>; Paul Kocialkowski
>> ><paul.kocialkowski@bootlin.com>; Sakari Ailus
>> ><sakari.ailus@linux.intel.com>; linux-kernel@vger.kernel.org;
>> >linux-arm-kernel@lists.infradead.org
>> >Subject: Re: [PATCH] phy: Change the configuration interface param to
>> >void* to make it more general
>> >
>> >* PGP Signed by an unknown key
>> >
>> >On Fri, Jul 12, 2019 at 02:04:08AM +0800, Zeng Tao wrote:
>> >> The phy framework now allows runtime configurations, but only
>> >> limited to mipi now, and it's not reasonable to introduce user
>> >> specified configurations into the union phy_configure_opts
>> >> structure. An simple way is to replace with a void *.
>> >
>> >I'm not sure why it's unreasonable?
>> >
>>
>> The phy.h will need to include vendor specific phy headers
>
>I'm not sure this is an issue.
>
>> and the union phy_configure_opts will become more complex.
>
>And this was the plan all along.
>
>> I don't think this is a good solution to include all vendor specific
>> phy configs into a single union structure.
>
>The thing is, as Sakari have stated, this interface was meant as a generic way
>to negotiate a configuration between a PHY consumer and a PHY provider (ie,
>whatever sends data to the phy and the phy itself).
>
>If you remove the explicit type check, then you need to have prior knowledge
>(and agreement) on both sides, which breaks the initial intent.

I get your point, so if we follow your design, it will look this:

union phy_configure_opts {
	struct xxxx1_phy phy1_opts;
	struct xxxx1_phy phy2_opts;
	struct xxxx1_phy phy3_opts;
	.....
}

And the general phy framework needn't to know about the type but just pass through, 
the caller and the phy driver definitely need to know what they are doing.
So I suggest a more general type void *, otherwise the general phy will need to see a lot 
of details which is not that general. 

Zengtao 

>
>Maxime
>
>--
>Maxime Ripard, Bootlin
>Embedded Linux and Kernel engineering
>https://bootlin.com
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] phy: Change the configuration interface param to void* to make it more general

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 3795 bytes --]

Hi,

On Sat, Jul 20, 2019 at 03:03:20AM +0000, Zengtao (B) wrote:
> >-----Original Message-----
> >From: Maxime Ripard [mailto:maxime.ripard@bootlin.com]
> >Sent: Thursday, July 18, 2019 12:38 AM
> >To: Zengtao (B) <prime.zeng@hisilicon.com>
> >Cc: kishon@ti.com; Chen-Yu Tsai <wens@csie.org>; Paul Kocialkowski
> ><paul.kocialkowski@bootlin.com>; Sakari Ailus <sakari.ailus@linux.intel.com>;
> >linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org
> >Subject: Re: [PATCH] phy: Change the configuration interface param to void*
> >to make it more general
> >
> >Hi,
> >
> >On Wed, Jul 17, 2019 at 06:36:09AM +0000, Zengtao (B) wrote:
> >> Hi Maxime:
> >>
> >> Thanks for your reply.
> >>
> >> >-----Original Message-----
> >> >From: Maxime Ripard [mailto:maxime.ripard@bootlin.com]
> >> >Sent: Thursday, July 11, 2019 7:21 PM
> >> >To: Zengtao (B) <prime.zeng@hisilicon.com>
> >> >Cc: kishon@ti.com; Chen-Yu Tsai <wens@csie.org>; Paul Kocialkowski
> >> ><paul.kocialkowski@bootlin.com>; Sakari Ailus
> >> ><sakari.ailus@linux.intel.com>; linux-kernel@vger.kernel.org;
> >> >linux-arm-kernel@lists.infradead.org
> >> >Subject: Re: [PATCH] phy: Change the configuration interface param to
> >> >void* to make it more general
> >> >
> >> >* PGP Signed by an unknown key
> >> >
> >> >On Fri, Jul 12, 2019 at 02:04:08AM +0800, Zeng Tao wrote:
> >> >> The phy framework now allows runtime configurations, but only
> >> >> limited to mipi now, and it's not reasonable to introduce user
> >> >> specified configurations into the union phy_configure_opts
> >> >> structure. An simple way is to replace with a void *.
> >> >
> >> >I'm not sure why it's unreasonable?
> >> >
> >>
> >> The phy.h will need to include vendor specific phy headers
> >
> >I'm not sure this is an issue.
> >
> >> and the union phy_configure_opts will become more complex.
> >
> >And this was the plan all along.
> >
> >> I don't think this is a good solution to include all vendor specific
> >> phy configs into a single union structure.
> >
> >The thing is, as Sakari have stated, this interface was meant as a generic way
> >to negotiate a configuration between a PHY consumer and a PHY provider (ie,
> >whatever sends data to the phy and the phy itself).
> >
> >If you remove the explicit type check, then you need to have prior knowledge
> >(and agreement) on both sides, which breaks the initial intent.
>
> I get your point, so if we follow your design, it will look this:
>
> union phy_configure_opts {
> 	struct xxxx1_phy phy1_opts;
> 	struct xxxx1_phy phy2_opts;
> 	struct xxxx1_phy phy3_opts;
> 	.....
> }
>
> And the general phy framework needn't to know about the type but
> just pass through, the caller and the phy driver definitely need to
> know what they are doing.

I'm not quite sure what you mean here. The configuration only applies
to the current PHY mode. So the phy consumer will have changed the
mode, and the PHY will have accepted it.

That change is also doable from the framework.

Then, which part of the union is being used is easy to figure out for
both parties, since they agree on it already.

> So I suggest a more general type void *, otherwise the general phy
> will need to see a lot of details which is not that general.

Except that this effectively becomes a black box that the framework
has no control and / or knowledge about.

Which means that it cannot do any kind of checks on it anymore, and
again, that the consumer and driver need to have prior knowledge of
what is being passed, without any way to check whether it's actually
what needs to be passed.

To some extent, the union also allows that, but it's just less
explicit and in general worse, to just pass void* here.

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] phy: Change the configuration interface param to void* to make it more general

[kbuild test robot]

Hi Zeng,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[cannot apply to v5.3-rc3 next-20190808]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Zeng-Tao/phy-Change-the-configuration-interface-param-to-void-to-make-it-more-general/20190713-213420
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.1-rc1-7-g2b96cd8-dirty
        make ARCH=x86_64 allmodconfig
        make C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>


sparse warnings: (new ones prefixed by >>)

   include/linux/sched.h:609:43: sparse: sparse: bad integer constant expression
   include/linux/sched.h:609:73: sparse: sparse: invalid named zero-width bitfield `value'
   include/linux/sched.h:610:43: sparse: sparse: bad integer constant expression
   include/linux/sched.h:610:67: sparse: sparse: invalid named zero-width bitfield `bucket_id'
>> drivers/gpu/drm/bridge/cdns-dsi.c:609:73: sparse: sparse: using member 'mipi_dphy' in incomplete union phy_configure_opts
   drivers/gpu/drm/bridge/cdns-dsi.c:784:73: sparse: sparse: using member 'mipi_dphy' in incomplete union phy_configure_opts
--
   include/linux/sched.h:609:43: sparse: sparse: bad integer constant expression
   include/linux/sched.h:609:73: sparse: sparse: invalid named zero-width bitfield `value'
   include/linux/sched.h:610:43: sparse: sparse: bad integer constant expression
   include/linux/sched.h:610:67: sparse: sparse: invalid named zero-width bitfield `bucket_id'
   drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c:165:22: sparse: sparse: using member 'hs_clk_rate' in incomplete struct phy_configure_opts_mipi_dphy
   drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c:169:30: sparse: sparse: using member 'hs_clk_rate' in incomplete struct phy_configure_opts_mipi_dphy
   drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c:173:17: sparse: sparse: using member 'hs_clk_rate' in incomplete struct phy_configure_opts_mipi_dphy
   drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c:200:17: sparse: sparse: using member 'hs_clk_rate' in incomplete struct phy_configure_opts_mipi_dphy
   drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c:206:9: sparse: sparse: using member 'hs_clk_rate' in incomplete struct phy_configure_opts_mipi_dphy
   drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c:211:9: sparse: sparse: using member 'lp_clk_rate' in incomplete struct phy_configure_opts_mipi_dphy
   drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c:216:9: sparse: sparse: using member 'lp_clk_rate' in incomplete struct phy_configure_opts_mipi_dphy
   drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c:220:26: sparse: sparse: using member 'hs_prepare' in incomplete struct phy_configure_opts_mipi_dphy
   drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c:221:17: sparse: sparse: using member 'hs_prepare' in incomplete struct phy_configure_opts_mipi_dphy
   drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c:227:22: sparse: sparse: using member 'hs_prepare' in incomplete struct phy_configure_opts_mipi_dphy
   drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c:230:37: sparse: sparse: using member 'hs_prepare' in incomplete struct phy_configure_opts_mipi_dphy
   drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c:237:26: sparse: sparse: using member 'clk_prepare' in incomplete struct phy_configure_opts_mipi_dphy
   drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c:238:17: sparse: sparse: using member 'clk_prepare' in incomplete struct phy_configure_opts_mipi_dphy
   drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c:244:43: sparse: sparse: using member 'clk_prepare' in incomplete struct phy_configure_opts_mipi_dphy
   drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c:247:30: sparse: sparse: using member 'hs_clk_rate' in incomplete struct phy_configure_opts_mipi_dphy
   drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c:251:29: sparse: sparse: using member 'hs_clk_rate' in incomplete struct phy_configure_opts_mipi_dphy
   drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c:255:30: sparse: sparse: using member 'hs_clk_rate' in incomplete struct phy_configure_opts_mipi_dphy
   drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c:264:22: sparse: sparse: using member 'hs_clk_rate' in incomplete struct phy_configure_opts_mipi_dphy
   drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c:266:27: sparse: sparse: using member 'hs_clk_rate' in incomplete struct phy_configure_opts_mipi_dphy
   drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c:268:27: sparse: sparse: using member 'hs_clk_rate' in incomplete struct phy_configure_opts_mipi_dphy
   drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c:270:27: sparse: sparse: using member 'hs_clk_rate' in incomplete struct phy_configure_opts_mipi_dphy
   drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c:272:27: sparse: sparse: using member 'hs_clk_rate' in incomplete struct phy_configure_opts_mipi_dphy
   drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c:274:27: sparse: sparse: using member 'hs_clk_rate' in incomplete struct phy_configure_opts_mipi_dphy
>> drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c:326:53: sparse: sparse: using member 'mipi_dphy' in incomplete union phy_configure_opts
   drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c:356:54: sparse: sparse: using member 'mipi_dphy' in incomplete union phy_configure_opts

vim +/mipi_dphy +609 drivers/gpu/drm/bridge/cdns-dsi.c

4dad3e7f12f702 Maxime Ripard   2019-01-21  602  
4dad3e7f12f702 Maxime Ripard   2019-01-21  603  static int cdns_dsi_check_conf(struct cdns_dsi *dsi,
4dad3e7f12f702 Maxime Ripard   2019-01-21  604  			       const struct drm_display_mode *mode,
4dad3e7f12f702 Maxime Ripard   2019-01-21  605  			       struct cdns_dsi_cfg *dsi_cfg,
4dad3e7f12f702 Maxime Ripard   2019-01-21  606  			       bool mode_valid_check)
4dad3e7f12f702 Maxime Ripard   2019-01-21  607  {
4dad3e7f12f702 Maxime Ripard   2019-01-21  608  	struct cdns_dsi_output *output = &dsi->output;
fced5a364dee9d Maxime Ripard   2019-01-21 @609  	struct phy_configure_opts_mipi_dphy *phy_cfg = &output->phy_opts.mipi_dphy;
4dad3e7f12f702 Maxime Ripard   2019-01-21  610  	unsigned long dsi_hss_hsa_hse_hbp;
4dad3e7f12f702 Maxime Ripard   2019-01-21  611  	unsigned int nlanes = output->dev->lanes;
4dad3e7f12f702 Maxime Ripard   2019-01-21  612  	int ret;
4dad3e7f12f702 Maxime Ripard   2019-01-21  613  
4dad3e7f12f702 Maxime Ripard   2019-01-21  614  	ret = cdns_dsi_mode2cfg(dsi, mode, dsi_cfg, mode_valid_check);
4dad3e7f12f702 Maxime Ripard   2019-01-21  615  	if (ret)
4dad3e7f12f702 Maxime Ripard   2019-01-21  616  		return ret;
4dad3e7f12f702 Maxime Ripard   2019-01-21  617  
fced5a364dee9d Maxime Ripard   2019-01-21  618  	phy_mipi_dphy_get_default_config(mode->crtc_clock * 1000,
fced5a364dee9d Maxime Ripard   2019-01-21  619  					 mipi_dsi_pixel_format_to_bpp(output->dev->format),
fced5a364dee9d Maxime Ripard   2019-01-21  620  					 nlanes, phy_cfg);
fced5a364dee9d Maxime Ripard   2019-01-21  621  
fced5a364dee9d Maxime Ripard   2019-01-21  622  	ret = cdns_dsi_adjust_phy_config(dsi, dsi_cfg, phy_cfg, mode, mode_valid_check);
fced5a364dee9d Maxime Ripard   2019-01-21  623  	if (ret)
fced5a364dee9d Maxime Ripard   2019-01-21  624  		return ret;
fced5a364dee9d Maxime Ripard   2019-01-21  625  
fced5a364dee9d Maxime Ripard   2019-01-21  626  	ret = phy_validate(dsi->dphy, PHY_MODE_MIPI_DPHY, 0, &output->phy_opts);
4dad3e7f12f702 Maxime Ripard   2019-01-21  627  	if (ret)
4dad3e7f12f702 Maxime Ripard   2019-01-21  628  		return ret;
4dad3e7f12f702 Maxime Ripard   2019-01-21  629  
4dad3e7f12f702 Maxime Ripard   2019-01-21  630  	dsi_hss_hsa_hse_hbp = dsi_cfg->hbp + DSI_HBP_FRAME_OVERHEAD;
4dad3e7f12f702 Maxime Ripard   2019-01-21  631  	if (output->dev->mode_flags & MIPI_DSI_MODE_VIDEO_SYNC_PULSE)
4dad3e7f12f702 Maxime Ripard   2019-01-21  632  		dsi_hss_hsa_hse_hbp += dsi_cfg->hsa + DSI_HSA_FRAME_OVERHEAD;
e19233955d9e9a Boris Brezillon 2018-04-21  633  
e19233955d9e9a Boris Brezillon 2018-04-21  634  	/*
e19233955d9e9a Boris Brezillon 2018-04-21  635  	 * Make sure DPI(HFP) > DSI(HSS+HSA+HSE+HBP) to guarantee that the FIFO
e19233955d9e9a Boris Brezillon 2018-04-21  636  	 * is empty before we start a receiving a new line on the DPI
e19233955d9e9a Boris Brezillon 2018-04-21  637  	 * interface.
e19233955d9e9a Boris Brezillon 2018-04-21  638  	 */
fced5a364dee9d Maxime Ripard   2019-01-21  639  	if ((u64)phy_cfg->hs_clk_rate *
4dad3e7f12f702 Maxime Ripard   2019-01-21  640  	    mode_to_dpi_hfp(mode, mode_valid_check) * nlanes <
e19233955d9e9a Boris Brezillon 2018-04-21  641  	    (u64)dsi_hss_hsa_hse_hbp *
e19233955d9e9a Boris Brezillon 2018-04-21  642  	    (mode_valid_check ? mode->clock : mode->crtc_clock) * 1000)
e19233955d9e9a Boris Brezillon 2018-04-21  643  		return -EINVAL;
e19233955d9e9a Boris Brezillon 2018-04-21  644  
e19233955d9e9a Boris Brezillon 2018-04-21  645  	return 0;
e19233955d9e9a Boris Brezillon 2018-04-21  646  }
e19233955d9e9a Boris Brezillon 2018-04-21  647  

:::::: The code at line 609 was first introduced by commit
:::::: fced5a364dee9d3a3ed1e3290ea3b83984b78007 drm/bridge: cdns: Convert to phy framework

:::::: TO: Maxime Ripard <maxime.ripard@bootlin.com>
:::::: CC: Maxime Ripard <maxime.ripard@bootlin.com>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] phy: Change the configuration interface param to void* to make it more general

[Sakari Ailus]

On Fri, Jul 12, 2019 at 09:21:45AM +0200, Maxime Ripard wrote:
> On Fri, Jul 12, 2019 at 05:26:04PM +0800, Zeng Tao wrote:
> > The phy framework now allows runtime configurations, but only limited
> > to mipi now, and it's not reasonable to introduce user specified
> > configurations into the union phy_configure_opts structure. An simple
> > way is to replace with a void *.
> >
> > We have already got some phy drivers which introduce private phy API
> > for runtime configurations, and with this patch, they can switch to
> > the phy_configure as a replace.
> >
> > Signed-off-by: Zeng Tao <prime.zeng@hisilicon.com>
> 
> I still don't believe this is the right approach, for the reasons
> exposed in my first review of that patch.

I agree.

The very reason for having PHY type specific structs is to allow configuring
the PHY independently of the PHY device. This patch breaks that.

-- 
Regards,

Sakari Ailus
sakari.ailus@linux.intel.com

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH] phy: Change the configuration interface param to void* to make it more general

[Zeng Tao]

The phy framework now allows runtime configurations, but only limited
to mipi now, and it's not reasonable to introduce user specified
configurations into the union phy_configure_opts structure. An simple
way is to replace with a void *.

We have already got some phy drivers which introduce private phy API
for runtime configurations, and with this patch, they can switch to
the phy_configure as a replace.

Signed-off-by: Zeng Tao <prime.zeng@hisilicon.com>
---
 drivers/phy/allwinner/phy-sun6i-mipi-dphy.c |  4 ++--
 drivers/phy/cadence/cdns-dphy.c             |  8 ++++----
 drivers/phy/phy-core.c                      |  4 ++--
 include/linux/phy/phy.h                     | 24 ++++++------------------
 4 files changed, 14 insertions(+), 26 deletions(-)

diff --git a/drivers/phy/allwinner/phy-sun6i-mipi-dphy.c b/drivers/phy/allwinner/phy-sun6i-mipi-dphy.c
index 79c8af5..6a60473 100644
--- a/drivers/phy/allwinner/phy-sun6i-mipi-dphy.c
+++ b/drivers/phy/allwinner/phy-sun6i-mipi-dphy.c
@@ -105,12 +105,12 @@ static int sun6i_dphy_init(struct phy *phy)
 	return 0;
 }
 
-static int sun6i_dphy_configure(struct phy *phy, union phy_configure_opts *opts)
+static int sun6i_dphy_configure(struct phy *phy, void *opts)
 {
 	struct sun6i_dphy *dphy = phy_get_drvdata(phy);
 	int ret;
 
-	ret = phy_mipi_dphy_config_validate(&opts->mipi_dphy);
+	ret = phy_mipi_dphy_config_validate(opts);
 	if (ret)
 		return ret;
 
diff --git a/drivers/phy/cadence/cdns-dphy.c b/drivers/phy/cadence/cdns-dphy.c
index 90c4e9b..0ec5013 100644
--- a/drivers/phy/cadence/cdns-dphy.c
+++ b/drivers/phy/cadence/cdns-dphy.c
@@ -233,23 +233,23 @@ static int cdns_dphy_config_from_opts(struct phy *phy,
 }
 
 static int cdns_dphy_validate(struct phy *phy, enum phy_mode mode, int submode,
-			      union phy_configure_opts *opts)
+			      void *opts)
 {
 	struct cdns_dphy_cfg cfg = { 0 };
 
 	if (mode != PHY_MODE_MIPI_DPHY)
 		return -EINVAL;
 
-	return cdns_dphy_config_from_opts(phy, &opts->mipi_dphy, &cfg);
+	return cdns_dphy_config_from_opts(phy, opts, &cfg);
 }
 
-static int cdns_dphy_configure(struct phy *phy, union phy_configure_opts *opts)
+static int cdns_dphy_configure(struct phy *phy, void *opts)
 {
 	struct cdns_dphy *dphy = phy_get_drvdata(phy);
 	struct cdns_dphy_cfg cfg = { 0 };
 	int ret;
 
-	ret = cdns_dphy_config_from_opts(phy, &opts->mipi_dphy, &cfg);
+	ret = cdns_dphy_config_from_opts(phy, opts, &cfg);
 	if (ret)
 		return ret;
 
diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
index e3880c4a1..048d4d6 100644
--- a/drivers/phy/phy-core.c
+++ b/drivers/phy/phy-core.c
@@ -420,7 +420,7 @@ EXPORT_SYMBOL_GPL(phy_calibrate);
  *
  * Returns: 0 if successful, an negative error code otherwise
  */
-int phy_configure(struct phy *phy, union phy_configure_opts *opts)
+int phy_configure(struct phy *phy, void *opts)
 {
 	int ret;
 
@@ -455,7 +455,7 @@ EXPORT_SYMBOL_GPL(phy_configure);
  * Returns: 0 if successful, an negative error code otherwise
  */
 int phy_validate(struct phy *phy, enum phy_mode mode, int submode,
-		 union phy_configure_opts *opts)
+		 void *opts)
 {
 	int ret;
 
diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index 15032f14..8948f94 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -16,8 +16,6 @@
 #include <linux/pm_runtime.h>
 #include <linux/regulator/consumer.h>
 
-#include <linux/phy/phy-mipi-dphy.h>
-
 struct phy;
 
 enum phy_mode {
@@ -41,15 +39,6 @@ enum phy_mode {
 	PHY_MODE_SATA
 };
 
-/**
- * union phy_configure_opts - Opaque generic phy configuration
- *
- * @mipi_dphy:	Configuration set applicable for phys supporting
- *		the MIPI_DPHY phy mode.
- */
-union phy_configure_opts {
-	struct phy_configure_opts_mipi_dphy	mipi_dphy;
-};
 
 /**
  * struct phy_ops - set of function pointers for performing phy operations
@@ -80,7 +69,7 @@ struct phy_ops {
 	 *
 	 * Returns: 0 if successful, an negative error code otherwise
 	 */
-	int	(*configure)(struct phy *phy, union phy_configure_opts *opts);
+	int	(*configure)(struct phy *phy, void *opts);
 
 	/**
 	 * @validate:
@@ -99,7 +88,7 @@ struct phy_ops {
 	 * error code otherwise
 	 */
 	int	(*validate)(struct phy *phy, enum phy_mode mode, int submode,
-			    union phy_configure_opts *opts);
+			    void *opts);
 	int	(*reset)(struct phy *phy);
 	int	(*calibrate)(struct phy *phy);
 	void	(*release)(struct phy *phy);
@@ -207,9 +196,9 @@ int phy_power_off(struct phy *phy);
 int phy_set_mode_ext(struct phy *phy, enum phy_mode mode, int submode);
 #define phy_set_mode(phy, mode) \
 	phy_set_mode_ext(phy, mode, 0)
-int phy_configure(struct phy *phy, union phy_configure_opts *opts);
+int phy_configure(struct phy *phy, void *opts);
 int phy_validate(struct phy *phy, enum phy_mode mode, int submode,
-		 union phy_configure_opts *opts);
+		 void *opts);
 
 static inline enum phy_mode phy_get_mode(struct phy *phy)
 {
@@ -354,8 +343,7 @@ static inline int phy_calibrate(struct phy *phy)
 	return -ENOSYS;
 }
 
-static inline int phy_configure(struct phy *phy,
-				union phy_configure_opts *opts)
+static inline int phy_configure(struct phy *phy, void *opts)
 {
 	if (!phy)
 		return 0;
@@ -364,7 +352,7 @@ static inline int phy_configure(struct phy *phy,
 }
 
 static inline int phy_validate(struct phy *phy, enum phy_mode mode, int submode,
-			       union phy_configure_opts *opts)
+			       void *opts)
 {
 	if (!phy)
 		return 0;
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] phy: Change the configuration interface param to void* to make it more general

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 738 bytes --]

On Fri, Jul 12, 2019 at 05:26:04PM +0800, Zeng Tao wrote:
> The phy framework now allows runtime configurations, but only limited
> to mipi now, and it's not reasonable to introduce user specified
> configurations into the union phy_configure_opts structure. An simple
> way is to replace with a void *.
>
> We have already got some phy drivers which introduce private phy API
> for runtime configurations, and with this patch, they can switch to
> the phy_configure as a replace.
>
> Signed-off-by: Zeng Tao <prime.zeng@hisilicon.com>

I still don't believe this is the right approach, for the reasons
exposed in my first review of that patch.

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel