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 at 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<---

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 = {

--->8---

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora 
Forum, hosted by The Linux Foundation

Reply via email to