Hi Axel, Mark,
we should not play to much with generous functions for this pmics. With this
patch we loose the fix_io code path from da9052_reg_update.
include/linux/mfd/da9052.h
...
if (da9052->fix_io) {
ret = da9052->fix_io(da9052, reg);
if (ret < 0)
return ret;
...
This fix_io used in drivers/mfd/da9052-i2c.c
/*
* There is an issue with DA9052 and DA9053_AA/BA/BB PMIC where the PMIC
* gets lockup up or fails to respond following a system reset.
* This fix is to follow any read or write with a dummy read to a safe
* register.
*/
static int da9052_i2c_fix(struct da9052 *da9052, unsigned char reg)
{
int val;
switch (da9052->chip_id) {
case DA9052:
case DA9053_AA:
case DA9053_BA:
case DA9053_BB:
/* A dummy read to a safe register address. */
if (!i2c_safe_reg(reg))
return regmap_read(da9052->regmap,
DA9052_PARK_REGISTER,
&val);
break;
default:
/*
* For other chips parking of I2C register
* to a safe place is not required.
*/
break;
}
return 0;
}
I suggest that Mark queue the revert of this patch.
Thanks,
Michael
On Tue, Dec 18, 2012 at 09:32:52AM +0800, Axel Lin wrote:
> Signed-off-by: Axel Lin <[email protected]>
> ---
> drivers/regulator/da9052-regulator.c | 41 +++++-----------------------------
> 1 file changed, 6 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/regulator/da9052-regulator.c b/drivers/regulator/da9052-regulator.c
> index d096309..c6d8651 100644
> --- a/drivers/regulator/da9052-regulator.c
> +++ b/drivers/regulator/da9052-regulator.c
> @@ -70,7 +70,6 @@ struct da9052_regulator_info {
> int step_uV;
> int min_uV;
> int max_uV;
> - unsigned char activate_bit;
> };
>
> struct da9052_regulator {
> @@ -210,36 +209,6 @@ static int da9052_map_voltage(struct regulator_dev *rdev,
> return sel;
> }
>
> -static int da9052_regulator_set_voltage_sel(struct regulator_dev *rdev,
> - unsigned int selector)
> -{
> - struct da9052_regulator *regulator = rdev_get_drvdata(rdev);
> - struct da9052_regulator_info *info = regulator->info;
> - int id = rdev_get_id(rdev);
> - int ret;
> -
> - ret = da9052_reg_update(regulator->da9052, rdev->desc->vsel_reg,
> - rdev->desc->vsel_mask, selector);
> - if (ret < 0)
> - return ret;
> -
> - /* Some LDOs and DCDCs are DVC controlled which requires enabling of
> - * the activate bit to implment the changes on the output.
> - */
> - switch (id) {
> - case DA9052_ID_BUCK1:
> - case DA9052_ID_BUCK2:
> - case DA9052_ID_BUCK3:
> - case DA9052_ID_LDO2:
> - case DA9052_ID_LDO3:
> - ret = da9052_reg_update(regulator->da9052, DA9052_SUPPLY_REG,
> - info->activate_bit, info->activate_bit);
> - break;
> - }
> -
> - return ret;
> -}
> -
> static struct regulator_ops da9052_dcdc_ops = {
> .get_current_limit = da9052_dcdc_get_current_limit,
> .set_current_limit = da9052_dcdc_set_current_limit,
> @@ -247,7 +216,7 @@ static struct regulator_ops da9052_dcdc_ops = {
> .list_voltage = da9052_list_voltage,
> .map_voltage = da9052_map_voltage,
> .get_voltage_sel = regulator_get_voltage_sel_regmap,
> - .set_voltage_sel = da9052_regulator_set_voltage_sel,
> + .set_voltage_sel = regulator_set_voltage_sel_regmap,
> .is_enabled = regulator_is_enabled_regmap,
> .enable = regulator_enable_regmap,
> .disable = regulator_disable_regmap,
> @@ -257,7 +226,7 @@ static struct regulator_ops da9052_ldo_ops = {
> .list_voltage = da9052_list_voltage,
> .map_voltage = da9052_map_voltage,
> .get_voltage_sel = regulator_get_voltage_sel_regmap,
> - .set_voltage_sel = da9052_regulator_set_voltage_sel,
> + .set_voltage_sel = regulator_set_voltage_sel_regmap,
> .is_enabled = regulator_is_enabled_regmap,
> .enable = regulator_enable_regmap,
> .disable = regulator_disable_regmap,
> @@ -274,13 +243,14 @@ static struct regulator_ops da9052_ldo_ops = {
> .owner = THIS_MODULE,\
> .vsel_reg = DA9052_BUCKCORE_REG + DA9052_ID_##_id, \
> .vsel_mask = (1 << (sbits)) - 1,\
> + .apply_reg = DA9052_SUPPLY_REG, \
> + .apply_bit = (abits), \
> .enable_reg = DA9052_BUCKCORE_REG + DA9052_ID_##_id, \
> .enable_mask = 1 << (ebits),\
> },\
> .min_uV = (min) * 1000,\
> .max_uV = (max) * 1000,\
> .step_uV = (step) * 1000,\
> - .activate_bit = (abits),\
> }
>
> #define DA9052_DCDC(_id, step, min, max, sbits, ebits, abits) \
> @@ -294,13 +264,14 @@ static struct regulator_ops da9052_ldo_ops = {
> .owner = THIS_MODULE,\
> .vsel_reg = DA9052_BUCKCORE_REG + DA9052_ID_##_id, \
> .vsel_mask = (1 << (sbits)) - 1,\
> + .apply_reg = DA9052_SUPPLY_REG, \
> + .apply_bit = (abits), \
> .enable_reg = DA9052_BUCKCORE_REG + DA9052_ID_##_id, \
> .enable_mask = 1 << (ebits),\
> },\
> .min_uV = (min) * 1000,\
> .max_uV = (max) * 1000,\
> .step_uV = (step) * 1000,\
> - .activate_bit = (abits),\
> }
>
> static struct da9052_regulator_info da9052_regulator_info[] = {
> --
> 1.7.9.5
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
On Wed, Oct 02, 2013 at 12:15:00PM +0200, Michael Grzeschik wrote:
> we should not play to much with generous functions for this pmics. With this
> patch we loose the fix_io code path from da9052_reg_update.
> I suggest that Mark queue the revert of this patch.
This seems far too obscure to be robust - if we need to bodge all I2C
I/O (which is what that looks like) then that should be being done in
the regmap for the device rather than by having stuff open coded in the
function drivers like this.