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