On Fri, Jun 10, 2011 at 11:51:02AM +0800, Ying-Chun Liu (PaulLiu) wrote:
> From: "Ying-Chun Liu (PaulLiu)" <paul....@linaro.org>
> 
> Add DA9052 regulator driver from Dialog.
> Modify Kconfig/Makefile for DA9052 regulator driver.

This has *many* of the serious shortcomings that were present in the
original driver submission from Dialog.  While they've still got issues
their current mainline code is much better than what they originally
tried to submit, please sync up with them.

> Signed-off-by: Zhou Jingyu <jingyu.z...@freescale.com>
> Acked-by: Lily Zhang <r58...@freescale.com>
> Signed-off-by: Ying-Chun Liu (PaulLiu) <paul....@linaro.org>

Given that this bears a remarkable resemblance 

> +++ b/drivers/regulator/Kconfig
> @@ -303,5 +303,12 @@ config REGULATOR_TPS65910
>       help
>         This driver supports TPS65910 voltage regulator chips.
>  
> +config REGULATOR_DA9052
> +     tristate "Dialog DA9052 regulators"
> +     depends on PMIC_DIALOG
> +     help

Keep Kconfig and Makefile sorted (or at least don't make the situation
worse).

> +#if defined(CONFIG_PMIC_DA9052)
> +     DA9052_LDO(DA9052_BUCK_PERI, DA9052_BUCK_PERI_VOLT_UPPER,
> +                     DA9052_BUCK_PERI_VOLT_LOWER,
> +                     DA9052_BUCK_PERI_STEP_BELOW_3000, DA9052_BUCKPERI_REG,
> +                     DA9052_BUCKPERI_VBPERI, DA9052_BUCKPERI_BPERIEN),
> +#elif defined(CONFIG_PMIC_DA9053AA) || (CONFIG_PMIC_DA9053Bx)
> +     DA9052_LDO(DA9052_BUCK_PERI, DA9052_BUCK_PERI_VOLT_UPPER,
> +                     DA9052_BUCK_PERI_VOLT_LOWER,
> +                     DA9052_BUCK_PERI_STEP, DA9052_BUCKPERI_REG,
> +                     DA9052_BUCKPERI_VBPERI, DA9052_BUCKPERI_BPERIEN),
> +#endif

You need to be able to support all three chips in a single kernel image.

> +int da9052_ldo_buck_enable(struct regulator_dev *rdev)
> +{
> +     struct da9052_regulator_priv *priv = rdev_get_drvdata(rdev);
> +     int id = rdev_get_id(rdev);
> +     int ret = 0;
> +     struct da9052_ssc_msg ssc_msg;
> +
> +     ssc_msg.addr = da9052_regulators[id].reg_add;
> +     ssc_msg.data = 0;
> +
> +     da9052_lock(priv->da9052);
> +     ret = priv->da9052->read(priv->da9052, &ssc_msg);

All the register access stuff hsa been totally redone to save having to
implement basic things like locking for register write operations in
every single caller (never mind driver...) using them.

> +
> +     return 0;
> +}
> +EXPORT_SYMBOL_GPL(da9052_ldo_buck_enable);
> +/* Code added to support additional attribure in sysfs - changestate */

This should be setting off warning flags...

I've stopped reviewing here.

_______________________________________________
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

Reply via email to