Re: [PATCH] regulator: tps65917: Fix SMPS enable/disable/is_enable
From: Nishanth Menon
Date: Fri Jun 27 2014 - 13:15:15 EST
On 14:48-20140627, Keerthy wrote:
> Hello Nishanth,
>
> On Friday 27 June 2014 12:01 AM, Nishanth Menon wrote:
> >We use regmap regulator ops to enable/disable and check if regulator
> >is enabled for various SMPS. However, these depend on valid
> >enable_reg, enable_mask and enable_value in regulator descriptor.
> >
> >So, similar to fix we did in commit 318dbb02b50c
> >("regulator: palmas: Fix SMPS enable/disable/is_enabled"), populate the
> >same for TPS65917 SMPS registration. LDO definitions are already in place.
> >
> >Fixes: d6f83370ed97 ("regulator: palmas: Add tps65917 PMIC support")
> >Signed-off-by: Nishanth Menon <nm@xxxxxx>
> >---
> >
> >Applies on:
> > git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git
> >branch: topic/palmas (4c0c9ca Merge remote-tracking branch 'regulator/fix/palmas' into regulator-palmas)
> >
> >Note: Ignored the minor style check from checkpatch --strict as fixing
> >it would create an 80 char warning
> >
> > drivers/regulator/palmas-regulator.c | 8 ++++++++
> > 1 file changed, 8 insertions(+)
> >
> >diff --git a/drivers/regulator/palmas-regulator.c b/drivers/regulator/palmas-regulator.c
> >index 7c8b441..c7aa1b1 100644
> >--- a/drivers/regulator/palmas-regulator.c
> >+++ b/drivers/regulator/palmas-regulator.c
> >@@ -1333,6 +1333,14 @@ static int tps65917_smps_registration(struct palmas_pmic *pmic,
> > pmic->current_reg_mode[id] = reg &
> > PALMAS_SMPS12_CTRL_MODE_ACTIVE_MASK;
> >+ pmic->desc[id].enable_reg =
> >+ PALMAS_BASE_TO_REG(PALMAS_SMPS_BASE,
> >+ palmas_regs_info[id].ctrl_addr);
>
> This is wrong. Please change palmas_regs_info[id].ctrl_addr to
> ddata->palmas_regs_info[id].ctrl_addr. The palmas_regs_info
> should come from the driver data for specific instances as the regmap
> is different for the different PMICs we support.
>
> Once you make the above changes please feel free to add
Uggh.. just realized... since I was testing CPUFREQ with SMPS1 never
realized it. and searched elsewhere on topic/palmas branch as well..
Looks like we should make the following changes? since the register data
is now a parameter and it so happened that palmas_reg_info was mapping
back to a valid variable and the error is easily missed - so rename the
variable as well - I can break this up as proper series of patches for
topic/palmas - let me know what you think..
diff --git a/drivers/regulator/palmas-regulator.c b/drivers/regulator/palmas-regulator.c
index 7c8b441..86fefdfb 100644
--- a/drivers/regulator/palmas-regulator.c
+++ b/drivers/regulator/palmas-regulator.c
@@ -41,7 +41,7 @@ static const struct regulator_linear_range smps_high_ranges[] = {
REGULATOR_LINEAR_RANGE(3300000, 0x7A, 0x7f, 0),
};
-static struct regs_info palmas_regs_info[] = {
+static struct regs_info palmas_generic_regs_info[] = {
{
.name = "SMPS12",
.sname = "smps1-in",
@@ -1165,18 +1165,18 @@ static int palmas_smps_registration(struct palmas_pmic *pmic,
pmic->desc[id].ops = &palmas_ops_smps10;
pmic->desc[id].vsel_reg =
PALMAS_BASE_TO_REG(PALMAS_SMPS_BASE,
- PALMAS_SMPS10_CTRL);
+ ddata->palmas_regs_info[id].ctrl_addr);
pmic->desc[id].vsel_mask = SMPS10_VSEL;
pmic->desc[id].enable_reg =
PALMAS_BASE_TO_REG(PALMAS_SMPS_BASE,
- PALMAS_SMPS10_CTRL);
+ ddata->palmas_regs_info[id].ctrl_addr);
if (id == PALMAS_REG_SMPS10_OUT1)
pmic->desc[id].enable_mask = SMPS10_SWITCH_EN;
else
pmic->desc[id].enable_mask = SMPS10_BOOST_EN;
pmic->desc[id].bypass_reg =
PALMAS_BASE_TO_REG(PALMAS_SMPS_BASE,
- PALMAS_SMPS10_CTRL);
+ ddata->palmas_regs_info[id].ctrl_addr);
pmic->desc[id].bypass_mask = SMPS10_BYPASS_EN;
pmic->desc[id].min_uV = 3750000;
pmic->desc[id].uV_step = 1250000;
@@ -1188,7 +1188,7 @@ static int palmas_smps_registration(struct palmas_pmic *pmic,
* otherwise we error in probe with unsupportable
* ranges. Read the current smps mode for later use.
*/
- addr = palmas_regs_info[id].vsel_addr;
+ addr = ddata->palmas_regs_info[id].vsel_addr;
pmic->desc[id].n_linear_ranges = 3;
ret = palmas_smps_read(pmic->palmas, addr, ®);
@@ -1209,12 +1209,12 @@ static int palmas_smps_registration(struct palmas_pmic *pmic,
pmic->desc[id].n_voltages = PALMAS_SMPS_NUM_VOLTAGES;
pmic->desc[id].vsel_reg =
PALMAS_BASE_TO_REG(PALMAS_SMPS_BASE,
- palmas_regs_info[id].vsel_addr);
+ ddata->palmas_regs_info[id].vsel_addr);
pmic->desc[id].vsel_mask =
PALMAS_SMPS12_VOLTAGE_VSEL_MASK;
/* Read the smps mode for later use. */
- addr = palmas_regs_info[id].ctrl_addr;
+ addr = ddata->palmas_regs_info[id].ctrl_addr;
ret = palmas_smps_read(pmic->palmas, addr, ®);
if (ret)
return ret;
@@ -1223,7 +1223,7 @@ static int palmas_smps_registration(struct palmas_pmic *pmic,
pmic->desc[id].enable_reg =
PALMAS_BASE_TO_REG(PALMAS_SMPS_BASE,
- palmas_regs_info[id].ctrl_addr);
+ ddata->palmas_regs_info[id].ctrl_addr);
pmic->desc[id].enable_mask =
PALMAS_SMPS12_CTRL_MODE_ACTIVE_MASK;
/* set_mode overrides this value */
@@ -1319,7 +1319,7 @@ static int tps65917_smps_registration(struct palmas_pmic *pmic,
pmic->desc[id].n_voltages = PALMAS_SMPS_NUM_VOLTAGES;
pmic->desc[id].vsel_reg =
PALMAS_BASE_TO_REG(PALMAS_SMPS_BASE,
- tps65917_regs_info[id].vsel_addr);
+ ddata->palmas_regs_info[id].vsel_addr);
pmic->desc[id].vsel_mask =
PALMAS_SMPS12_VOLTAGE_VSEL_MASK;
@@ -1414,7 +1414,7 @@ struct palmas_pmic_driver_data palmas_ddata = {
.ldo_begin = PALMAS_REG_LDO1,
.ldo_end = PALMAS_REG_LDOUSB,
.max_reg = PALMAS_NUM_REGS,
- .palmas_regs_info = palmas_regs_info,
+ .palmas_regs_info = palmas_generic_regs_info,
.palmas_matches = palmas_matches,
.sleep_req_info = palma_sleep_req_info,
.smps_register = palmas_smps_registration,
--
Regards,
Nishanth Menon
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/