On pią, 2014-05-30 at 09:25 +0900, Chanwoo Choi wrote:
> This patch add S2MPU02 regulator device to existing S2MPS11 device driver
> because of little difference between S2MPS1x and S2MPU02. The S2MPU02
> regulator device includes LDO[1-28] and BUCK[1-7].
>
> Signed-off-by: Chanwoo Choi <[email protected]>
> [Add missing linear_min_sel of S2MPU02 LDO regulators by Jonghwa Lee]
> Signed-off-by: Jonghwa Lee <[email protected]>
> ---
> drivers/mfd/sec-core.c | 26 +++
> drivers/regulator/s2mps11.c | 319
> +++++++++++++++++++++++++++++++++---
> include/linux/mfd/samsung/s2mpu02.h | 200 ++++++++++++++++++++++
> 3 files changed, 525 insertions(+), 20 deletions(-)
> create mode 100644 include/linux/mfd/samsung/s2mpu02.h
>
(...)
> diff --git a/drivers/regulator/s2mps11.c b/drivers/regulator/s2mps11.c
> index 02e2fb2..f7e5dab3d 100644
> --- a/drivers/regulator/s2mps11.c
> +++ b/drivers/regulator/s2mps11.c
> @@ -31,6 +31,7 @@
> #include <linux/mfd/samsung/core.h>
> #include <linux/mfd/samsung/s2mps11.h>
> #include <linux/mfd/samsung/s2mps14.h>
> +#include <linux/mfd/samsung/s2mpu02.h>
>
> struct s2mps11_info {
> unsigned int rdev_num;
> @@ -40,11 +41,15 @@ struct s2mps11_info {
> int ramp_delay16;
> int ramp_delay7810;
> int ramp_delay9;
> +
> + enum sec_device_type dev_type;
> +
> /*
> - * One bit for each S2MPS14 regulator whether the suspend mode
> + * One bit for each S2MPS14/S2MPU02 regulator whether the suspend mode
> * was enabled.
> */
> - unsigned int s2mps14_suspend_state:30;
> + unsigned long long s2mps14_suspend_state:35;
> +
> /* Array of size rdev_num with GPIO-s for external sleep control */
> int *ext_control_gpio;
> };
> @@ -415,12 +420,24 @@ static int s2mps14_regulator_enable(struct
> regulator_dev *rdev)
> struct s2mps11_info *s2mps11 = rdev_get_drvdata(rdev);
> unsigned int val;
>
> - if (s2mps11->s2mps14_suspend_state & (1 << rdev_get_id(rdev)))
> - val = S2MPS14_ENABLE_SUSPEND;
> - else if (gpio_is_valid(s2mps11->ext_control_gpio[rdev_get_id(rdev)]))
> - val = S2MPS14_ENABLE_EXT_CONTROL;
> - else
> - val = rdev->desc->enable_mask;
> + switch (s2mps11->dev_type) {
> + case S2MPS14X:
> + if (s2mps11->s2mps14_suspend_state & (1 << rdev_get_id(rdev)))
> + val = S2MPS14_ENABLE_SUSPEND;
> + else if
> (gpio_is_valid(s2mps11->ext_control_gpio[rdev_get_id(rdev)]))
> + val = S2MPS14_ENABLE_EXT_CONTROL;
> + else
> + val = rdev->desc->enable_mask;
> + break;
> + case S2MPU02:
> + if (s2mps11->s2mps14_suspend_state & (1 << rdev_get_id(rdev)))
> + val = S2MPU02_ENABLE_SUSPEND;
> + else
> + val = rdev->desc->enable_mask;
> + break;
> + default:
> + return -EINVAL;
> + };
>
> return regmap_update_bits(rdev->regmap, rdev->desc->enable_reg,
> rdev->desc->enable_mask, val);
> @@ -431,16 +448,42 @@ static int s2mps14_regulator_set_suspend_disable(struct
> regulator_dev *rdev)
> int ret;
> unsigned int val;
> struct s2mps11_info *s2mps11 = rdev_get_drvdata(rdev);
> + int rdev_id = rdev_get_id(rdev);
>
> - /* LDO3 should be always on and does not support suspend mode */
> - if (rdev_get_id(rdev) == S2MPS14_LDO3)
> - return 0;
> + /* Below LDO should be always on or does not support suspend mode. */
> + switch (s2mps11->dev_type) {
> + case S2MPS14X:
> + switch (rdev_id) {
> + case S2MPS14_LDO3:
> + return 0;
> + };
> + case S2MPU02:
> + switch (rdev_id) {
> + case S2MPU02_LDO13:
> + case S2MPU02_LDO14:
> + case S2MPU02_LDO15:
> + case S2MPU02_LDO16:
> + case S2MPU02_LDO17:
> + case S2MPU02_BUCK7:
> + return 0;
> + };
> + default:
> + return -EINVAL;
> + };
>
> ret = regmap_read(rdev->regmap, rdev->desc->enable_reg, &val);
> if (ret < 0)
> return ret;
>
> - s2mps11->s2mps14_suspend_state |= (1 << rdev_get_id(rdev));
> + switch (s2mps11->dev_type) {
> + case S2MPS14X:
> + case S2MPU02:
> + s2mps11->s2mps14_suspend_state |= (1 << rdev_get_id(rdev));
> + break;
> + default:
> + return -EINVAL;
> + };
> +
I think this change is not needed. You are actually returning EINVAL on
wrong devices in switch above so:
s2mps11->s2mps14_suspend_state |= (1 << rdev_get_id(rdev));
would be safe and sufficient.
Anyway:
Reviewed-by: Krzysztof Kozlowski <[email protected]>
Best regards,
Krzysztof
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
