This looks good to me.
Acked-by: Graeme Gregory <gg@xxxxxxxxxxxxxxx>
On 18/04/13 14:02, Laxman Dewangan wrote:
> The Palma device like TPS65913 have the mode mask which is also
> used for enable/disable the rails. The mode bits are defined as
> 00: OFF
> 01: AUTO
> 10: ECO
> 11: Forced PWM
>
> and modes are set accordingly as
> REGULATOR_MODE_NORMAL: AUTO
> REGULATOR_MODE_IDLE: ECO
> REGULATOR_MODE_FAST: PWM
>
> Two issue observed:
> 1. If client calls following sequence:
> regulator_enable(),
> regulator_set_mode(FAST),
> regulator_disable()
>
> and again the regulator_enable() then the mode is reset
> to NORMAL inplace of keeping the mode as FAST.
>
> Fixing this by storing the current mode configured by client
> and restoring modes when enable() is called after disable().
>
> 2. In following sequence, the regulator get enabled:
> regulator_disable()
> regulator_set_mode(FAST),
>
> Fixing this by updating new mode in register only if it is
> enabled.
>
> Signed-off-by: Laxman Dewangan <ldewangan@xxxxxxxxxx>
> ---
> drivers/regulator/palmas-regulator.c | 30 +++++++++++++++++++++++-------
> include/linux/mfd/palmas.h | 1 +
> 2 files changed, 24 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/regulator/palmas-regulator.c b/drivers/regulator/palmas-regulator.c
> index f5612c3..2948d21 100644
> --- a/drivers/regulator/palmas-regulator.c
> +++ b/drivers/regulator/palmas-regulator.c
> @@ -275,7 +275,10 @@ static int palmas_enable_smps(struct regulator_dev *dev)
> palmas_smps_read(pmic->palmas, palmas_regs_info[id].ctrl_addr, ®);
>
> reg &= ~PALMAS_SMPS12_CTRL_MODE_ACTIVE_MASK;
> - reg |= SMPS_CTRL_MODE_ON;
> + if (pmic->current_reg_mode[id])
> + reg |= pmic->current_reg_mode[id];
> + else
> + reg |= SMPS_CTRL_MODE_ON;
>
> palmas_smps_write(pmic->palmas, palmas_regs_info[id].ctrl_addr, reg);
>
> @@ -297,16 +300,19 @@ static int palmas_disable_smps(struct regulator_dev *dev)
> return 0;
> }
>
> -
> static int palmas_set_mode_smps(struct regulator_dev *dev, unsigned int mode)
> {
> struct palmas_pmic *pmic = rdev_get_drvdata(dev);
> int id = rdev_get_id(dev);
> unsigned int reg;
> + bool rail_enable = true;
>
> palmas_smps_read(pmic->palmas, palmas_regs_info[id].ctrl_addr, ®);
> reg &= ~PALMAS_SMPS12_CTRL_MODE_ACTIVE_MASK;
>
> + if (reg == SMPS_CTRL_MODE_OFF)
> + rail_enable = false;
> +
> switch (mode) {
> case REGULATOR_MODE_NORMAL:
> reg |= SMPS_CTRL_MODE_ON;
> @@ -320,8 +326,11 @@ static int palmas_set_mode_smps(struct regulator_dev *dev, unsigned int mode)
> default:
> return -EINVAL;
> }
> - palmas_smps_write(pmic->palmas, palmas_regs_info[id].ctrl_addr, reg);
>
> + pmic->current_reg_mode[id] = reg & PALMAS_SMPS12_CTRL_MODE_ACTIVE_MASK;
> + if (rail_enable)
> + palmas_smps_write(pmic->palmas,
> + palmas_regs_info[id].ctrl_addr, reg);
> return 0;
> }
>
> @@ -331,9 +340,7 @@ static unsigned int palmas_get_mode_smps(struct regulator_dev *dev)
> int id = rdev_get_id(dev);
> unsigned int reg;
>
> - palmas_smps_read(pmic->palmas, palmas_regs_info[id].ctrl_addr, ®);
> - reg &= PALMAS_SMPS12_CTRL_STATUS_MASK;
> - reg >>= PALMAS_SMPS12_CTRL_STATUS_SHIFT;
> + reg = pmic->current_reg_mode[id] & PALMAS_SMPS12_CTRL_MODE_ACTIVE_MASK;
>
> switch (reg) {
> casThis looks good to me.
>
>
>
> Acked-by: Graeme Gregory <gg@xxxxxxxxxxxxxxx>e SMPS_CTRL_MODE_ON:
> @@ -872,7 +879,8 @@ static int palmas_regulators_probe(struct platform_device *pdev)
> /*
> * Read and store the RANGE bit for later use
> * This must be done before regulator is probed,
> - * otherwise we error in probe with unsupportable ranges.
> + * otherwise we error in probe with unsupportable
> + * ranges. Read the current smps mode for later use.
> */
> addr = palmas_regs_info[id].vsel_addr;
>
> @@ -889,6 +897,14 @@ static int palmas_regulators_probe(struct platform_device *pdev)
> palmas_regs_info[id].vsel_addr);
> pmic->desc[id].vsel_mask =
> PALMAS_SMPS12_VOLTAGE_VSEL_MASK;
> +This looks good to me.
>
>
>
> Acked-by: Graeme Gregory <gg@xxxxxxxxxxxxxxx>
> + /* Read the smps mode for later use. */
> + addr = palmas_regs_info[id].ctrl_addr;
> + ret = palmas_smps_read(pmic->palmas, addr, ®);
> + if (ret)
> + goto err_unregister_regulator;
> + pmic->current_reg_mode[id] = reg &
> + PALMAS_SMPS12_CTRL_MODE_ACTIVE_MASK;
> }
>
> pmic->desc[id].type = REGULATOR_VOLTAGE;
> diff --git a/include/linux/mfd/palmas.h b/include/linux/mfd/palmas.h
> index 91ef60c..8f21daf 100644
> --- a/include/linux/mfd/palmas.h
> +++ b/include/linux/mfd/palmas.h
> @@ -338,6 +338,7 @@ struct palmas_pmic {
>
> int range[PALMAS_REG_SMPS10];
> unsigned int ramp_delay[PALMAS_REG_SMPS10];
> + unsigned int current_reg_mode[PALMAS_REG_SMPS10];
> };
>
> struct palmas_resource {
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
