diff mbox

drm/msm/dsi: Fix regulator API abuse

Message ID 5703A0CA.8020802@codeaurora.org (mailing list archive)
State New, archived
Headers show

Commit Message

Archit Taneja April 5, 2016, 11:26 a.m. UTC 
Hi,

On 03/31/2016 12:23 AM, Mark Brown wrote:
> The voltage changing code in this driver is broken and should be
> removed.  The driver sets a single, exact voltage on probe.  Unless
> there is a very good reason for this (which should be documented in
> comments) constraints like this need to be set via the machine
> constraints, voltage setting in a driver is expected to be used in cases
> where the voltage varies at runtime.
>
> In addition client drivers should almost never be calling
> regulator_can_set_voltage(), if the device needs to set a voltage it
> needs to set the voltage and the regulator core will handle the case
> where the regulator is fixed voltage.  If the driver simply skips
> setting the voltage if it doesn't have permission then it shouild just

s/shouild/should

Could you also pull in the diff I've added below? It removes another
set_voltage call from the hdmi driver and the struct members used to
manage the voltage range.

> not bother in the first place.
>

+John, you'll need to update your regulator nodes with the min/max
voltage values in your N7 dtsi whenever you plan to rebase next.

Thanks,
Archit

> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
>   drivers/gpu/drm/msm/dsi/dsi_host.c | 12 ------------
>   1 file changed, 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
> index 4282ec6bbaaf..a3e47ad83eb3 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
> @@ -325,18 +325,6 @@ static int dsi_regulator_init(struct msm_dsi_host *msm_host)
>   		return ret;
>   	}
>
> -	for (i = 0; i < num; i++) {
> -		if (regulator_can_change_voltage(s[i].consumer)) {
> -			ret = regulator_set_voltage(s[i].consumer,
> -				regs[i].min_voltage, regs[i].max_voltage);
> -			if (ret < 0) {
> -				pr_err("regulator %d set voltage failed, %d\n",
> -					i, ret);
> -				return ret;
> -			}
> -		}
> -	}
> -
>   	return 0;
>   }
>
>

---8<---


--->8---

Comments

Mark Brown April 5, 2016, 4:17 p.m. UTC | #1 
On Tue, Apr 05, 2016 at 04:56:02PM +0530, Archit Taneja wrote:

> Could you also pull in the diff I've added below? It removes another
> set_voltage call from the hdmi driver and the struct members used to
> manage the voltage range.

Please update and submit this directly, I'm not adding anything by
sending it on.
diff mbox

Patch

diff --git a/drivers/gpu/drm/msm/dsi/dsi.h b/drivers/gpu/drm/msm/dsi/dsi.h
index 749fbb2..03f115f 100644
--- a/drivers/gpu/drm/msm/dsi/dsi.h
+++ b/drivers/gpu/drm/msm/dsi/dsi.h
@@ -41,8 +41,6 @@  enum msm_dsi_phy_type {
  /* Regulators for DSI devices */
  struct dsi_reg_entry {
  	char name[32];
-	int min_voltage;
-	int max_voltage;
  	int enable_load;
  	int disable_load;
  };
diff --git a/drivers/gpu/drm/msm/dsi/dsi_cfg.c 
b/drivers/gpu/drm/msm/dsi/dsi_cfg.c
index e58e9b9..cc802bb 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_cfg.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_cfg.c
@@ -22,9 +22,9 @@  static const struct msm_dsi_config apq8064_dsi_cfg = {
  	.reg_cfg = {
  		.num = 3,
  		.regs = {
-			{"vdda", 1200000, 1200000, 100000, 100},
-			{"avdd", 3000000, 3000000, 110000, 100},
-			{"vddio", 1800000, 1800000, 100000, 100},
+			{"vdda", 100000, 100},
+			{"avdd", 10000, 100},
+			{"vddio", 100000, 100},
  		},
  	},
  	.bus_clk_names = dsi_v2_bus_clk_names,
@@ -40,10 +40,10 @@  static const struct msm_dsi_config 
msm8974_apq8084_dsi_cfg = {
  	.reg_cfg = {
  		.num = 4,
  		.regs = {
-			{"gdsc", -1, -1, -1, -1},
-			{"vdd", 3000000, 3000000, 150000, 100},
-			{"vdda", 1200000, 1200000, 100000, 100},
-			{"vddio", 1800000, 1800000, 100000, 100},
+			{"gdsc", -1, -1},
+			{"vdd", 150000, 100},
+			{"vdda", 100000, 100},
+			{"vddio", 100000, 100},
  		},
  	},
  	.bus_clk_names = dsi_6g_bus_clk_names,
@@ -59,9 +59,9 @@  static const struct msm_dsi_config msm8916_dsi_cfg = {
  	.reg_cfg = {
  		.num = 3,
  		.regs = {
-			{"gdsc", -1, -1, -1, -1},
-			{"vdda", 1200000, 1200000, 100000, 100},
-			{"vddio", 1800000, 1800000, 100000, 100},
+			{"gdsc", -1, -1},
+			{"vdda", 100000, 100},
+			{"vddio", 100000, 100},
  		},
  	},
  	.bus_clk_names = dsi_8916_bus_clk_names,
@@ -73,13 +73,13 @@  static const struct msm_dsi_config msm8994_dsi_cfg = {
  	.reg_cfg = {
  		.num = 7,
  		.regs = {
-			{"gdsc", -1, -1, -1, -1},
-			{"vdda", 1250000, 1250000, 100000, 100},
-			{"vddio", 1800000, 1800000, 100000, 100},
-			{"vcca", 1000000, 1000000, 10000, 100},
-			{"vdd", 1800000, 1800000, 100000, 100},
-			{"lab_reg", -1, -1, -1, -1},
-			{"ibb_reg", -1, -1, -1, -1},
+			{"gdsc", -1, -1},
+			{"vdda", 100000, 100},
+			{"vddio", 100000, 100},
+			{"vcca", 10000, 100},
+			{"vdd", 100000, 100},
+			{"lab_reg", -1, -1},
+			{"ibb_reg", -1, -1},
  		},
  	},
  	.bus_clk_names = dsi_6g_bus_clk_names,
diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c 
b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
index 91a95fb..e2f42d8 100644
--- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
+++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
@@ -177,19 +177,6 @@  static int dsi_phy_regulator_init(struct 
msm_dsi_phy *phy)
  		return ret;
  	}

-	for (i = 0; i < num; i++) {
-		if (regulator_can_change_voltage(s[i].consumer)) {
-			ret = regulator_set_voltage(s[i].consumer,
-				regs[i].min_voltage, regs[i].max_voltage);
-			if (ret < 0) {
-				dev_err(dev,
-					"regulator %d set voltage failed, %d\n",
-					i, ret);
-				return ret;
-			}
-		}
-	}
-
  	return 0;
  }

diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_20nm.c 
b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_20nm.c
index 2e9ba11..3c93cfc 100644
--- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_20nm.c
+++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_20nm.c
@@ -138,8 +138,8 @@  const struct msm_dsi_phy_cfg dsi_phy_20nm_cfgs = {
  	.reg_cfg = {
  		.num = 2,
  		.regs = {
-			{"vddio", 1800000, 1800000, 100000, 100},
-			{"vcca", 1000000, 1000000, 10000, 100},
+			{"vddio", 100000, 100},
+			{"vcca", 10000, 100},
  		},
  	},
  	.ops = {
diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm.c 
b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm.c
index edf7411..397f09a 100644
--- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm.c
+++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm.c
@@ -138,7 +138,7 @@  const struct msm_dsi_phy_cfg dsi_phy_28nm_hpm_cfgs = {
  	.reg_cfg = {
  		.num = 1,
  		.regs = {
-			{"vddio", 1800000, 1800000, 100000, 100},
+			{"vddio", 100000, 100},
  		},
  	},
  	.ops = {
@@ -153,7 +153,7 @@  const struct msm_dsi_phy_cfg dsi_phy_28nm_lp_cfgs = {
  	.reg_cfg = {
  		.num = 1,
  		.regs = {
-			{"vddio", 1800000, 1800000, 100000, 100},
+			{"vddio", 100000, 100},
  		},
  	},
  	.ops = {
diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c 
b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c
index 197b039..4ed7b57 100644
--- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c
+++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c
@@ -185,7 +185,7 @@  const struct msm_dsi_phy_cfg dsi_phy_28nm_8960_cfgs = {
  	.reg_cfg = {
  		.num = 1,
  		.regs = {
-			{"vddio", 1800000, 1800000, 100000, 100},
+			{"vddio", 100000, 100},
  		},
  	},
  	.ops = {