[2/3] regulator: s2mpa01: Optimize the regulator description macro

Message ID 1404822480-31525-2-git-send-email-amit.daniel@samsung.com
State New
Headers show

Commit Message

Amit Daniel Kachhap July 8, 2014, 12:27 p.m.
This patch makes the regulator description macro take minimum and
steps voltage as parameter. In this way many repeated macros can be
removed. Now these macros are repeated only if the the LDO/BUCK ctrl
registers have non-linear positions. The good thing is these ctrl registers
are mostly linear so they are not passed as parameters.

This patch reduces the code size and also allow easy addition of more
s2mpxxx PMIC drivers which differs a lot in minimum/step voltages.

Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
---
 drivers/regulator/s2mpa01.c | 136 ++++++++++++--------------------------------
 1 file changed, 37 insertions(+), 99 deletions(-)

Comments

Amit Daniel Kachhap July 10, 2014, 3:24 a.m. | #1
Adding MFD maintainers.

On Tue, Jul 8, 2014 at 5:57 PM, Amit Daniel Kachhap
<amit.daniel@samsung.com> wrote:
> This patch makes the regulator description macro take minimum and
> steps voltage as parameter. In this way many repeated macros can be
> removed. Now these macros are repeated only if the the LDO/BUCK ctrl
> registers have non-linear positions. The good thing is these ctrl registers
> are mostly linear so they are not passed as parameters.
>
> This patch reduces the code size and also allow easy addition of more
> s2mpxxx PMIC drivers which differs a lot in minimum/step voltages.
>
> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
> ---
>  drivers/regulator/s2mpa01.c | 136 ++++++++++++--------------------------------
>  1 file changed, 37 insertions(+), 99 deletions(-)
>
> diff --git a/drivers/regulator/s2mpa01.c b/drivers/regulator/s2mpa01.c
> index 962c5f1..8073466 100644
> --- a/drivers/regulator/s2mpa01.c
> +++ b/drivers/regulator/s2mpa01.c
> @@ -235,28 +235,14 @@ static struct regulator_ops s2mpa01_buck_ops = {
>         .set_ramp_delay         = s2mpa01_set_ramp_delay,
>  };
>
> -#define regulator_desc_ldo1(num)       {               \
> +#define regulator_desc_ldo(num, min, step)     {       \
>         .name           = "LDO"#num,                    \
>         .id             = S2MPA01_LDO##num,             \
>         .ops            = &s2mpa01_ldo_ops,             \
>         .type           = REGULATOR_VOLTAGE,            \
>         .owner          = THIS_MODULE,                  \
> -       .min_uV         = MIN_800_MV,                   \
> -       .uV_step        = STEP_50_MV,                   \
> -       .n_voltages     = S2MPA01_LDO_N_VOLTAGES,       \
> -       .vsel_reg       = S2MPA01_REG_L1CTRL + num - 1, \
> -       .vsel_mask      = S2MPA01_LDO_VSEL_MASK,        \
> -       .enable_reg     = S2MPA01_REG_L1CTRL + num - 1, \
> -       .enable_mask    = S2MPA01_ENABLE_MASK           \
> -}
> -#define regulator_desc_ldo2(num)       {               \
> -       .name           = "LDO"#num,                    \
> -       .id             = S2MPA01_LDO##num,             \
> -       .ops            = &s2mpa01_ldo_ops,             \
> -       .type           = REGULATOR_VOLTAGE,            \
> -       .owner          = THIS_MODULE,                  \
> -       .min_uV         = MIN_800_MV,                   \
> -       .uV_step        = STEP_25_MV,                   \
> +       .min_uV         = min,                          \
> +       .uV_step        = step,                         \
>         .n_voltages     = S2MPA01_LDO_N_VOLTAGES,       \
>         .vsel_reg       = S2MPA01_REG_L1CTRL + num - 1, \
>         .vsel_mask      = S2MPA01_LDO_VSEL_MASK,        \
> @@ -296,14 +282,14 @@ static struct regulator_ops s2mpa01_buck_ops = {
>         .enable_mask    = S2MPA01_ENABLE_MASK                   \
>  }
>
> -#define regulator_desc_buck6_7(num)    {                       \
> +#define regulator_desc_buck6_10(num, min, step)        {               \
>         .name           = "BUCK"#num,                           \
>         .id             = S2MPA01_BUCK##num,                    \
>         .ops            = &s2mpa01_buck_ops,                    \
>         .type           = REGULATOR_VOLTAGE,                    \
>         .owner          = THIS_MODULE,                          \
> -       .min_uV         = MIN_600_MV,                           \
> -       .uV_step        = STEP_6_25_MV,                         \
> +       .min_uV         = min,                                  \
> +       .uV_step        = step,                                 \
>         .n_voltages     = S2MPA01_BUCK_N_VOLTAGES,              \
>         .ramp_delay     = S2MPA01_RAMP_DELAY,                   \
>         .vsel_reg       = S2MPA01_REG_B6CTRL2 + (num - 6) * 2,  \
> @@ -312,91 +298,43 @@ static struct regulator_ops s2mpa01_buck_ops = {
>         .enable_mask    = S2MPA01_ENABLE_MASK                   \
>  }
>
> -#define regulator_desc_buck8   {                               \
> -       .name           = "BUCK8",                              \
> -       .id             = S2MPA01_BUCK8,                        \
> -       .ops            = &s2mpa01_buck_ops,                    \
> -       .type           = REGULATOR_VOLTAGE,                    \
> -       .owner          = THIS_MODULE,                          \
> -       .min_uV         = MIN_800_MV,                           \
> -       .uV_step        = STEP_12_5_MV,                         \
> -       .n_voltages     = S2MPA01_BUCK_N_VOLTAGES,              \
> -       .ramp_delay     = S2MPA01_RAMP_DELAY,                   \
> -       .vsel_reg       = S2MPA01_REG_B8CTRL2,                  \
> -       .vsel_mask      = S2MPA01_BUCK_VSEL_MASK,               \
> -       .enable_reg     = S2MPA01_REG_B8CTRL1,                  \
> -       .enable_mask    = S2MPA01_ENABLE_MASK                   \
> -}
> -
> -#define regulator_desc_buck9   {                               \
> -       .name           = "BUCK9",                              \
> -       .id             = S2MPA01_BUCK9,                        \
> -       .ops            = &s2mpa01_buck_ops,                    \
> -       .type           = REGULATOR_VOLTAGE,                    \
> -       .owner          = THIS_MODULE,                          \
> -       .min_uV         = MIN_1500_MV,                          \
> -       .uV_step        = STEP_12_5_MV,                         \
> -       .n_voltages     = S2MPA01_BUCK_N_VOLTAGES,              \
> -       .ramp_delay     = S2MPA01_RAMP_DELAY,                   \
> -       .vsel_reg       = S2MPA01_REG_B9CTRL2,                  \
> -       .vsel_mask      = S2MPA01_BUCK_VSEL_MASK,               \
> -       .enable_reg     = S2MPA01_REG_B9CTRL1,                  \
> -       .enable_mask    = S2MPA01_ENABLE_MASK                   \
> -}
> -
> -#define regulator_desc_buck10  {                               \
> -       .name           = "BUCK10",                             \
> -       .id             = S2MPA01_BUCK10,                       \
> -       .ops            = &s2mpa01_buck_ops,                    \
> -       .type           = REGULATOR_VOLTAGE,                    \
> -       .owner          = THIS_MODULE,                          \
> -       .min_uV         = MIN_1000_MV,                          \
> -       .uV_step        = STEP_12_5_MV,                         \
> -       .n_voltages     = S2MPA01_BUCK_N_VOLTAGES,              \
> -       .ramp_delay     = S2MPA01_RAMP_DELAY,                   \
> -       .vsel_reg       = S2MPA01_REG_B10CTRL2,                 \
> -       .vsel_mask      = S2MPA01_BUCK_VSEL_MASK,               \
> -       .enable_reg     = S2MPA01_REG_B10CTRL1,                 \
> -       .enable_mask    = S2MPA01_ENABLE_MASK                   \
> -}
> -
>  static struct regulator_desc regulators[] = {
> -       regulator_desc_ldo2(1),
> -       regulator_desc_ldo1(2),
> -       regulator_desc_ldo1(3),
> -       regulator_desc_ldo1(4),
> -       regulator_desc_ldo1(5),
> -       regulator_desc_ldo2(6),
> -       regulator_desc_ldo1(7),
> -       regulator_desc_ldo1(8),
> -       regulator_desc_ldo1(9),
> -       regulator_desc_ldo1(10),
> -       regulator_desc_ldo2(11),
> -       regulator_desc_ldo1(12),
> -       regulator_desc_ldo1(13),
> -       regulator_desc_ldo1(14),
> -       regulator_desc_ldo1(15),
> -       regulator_desc_ldo1(16),
> -       regulator_desc_ldo1(17),
> -       regulator_desc_ldo1(18),
> -       regulator_desc_ldo1(19),
> -       regulator_desc_ldo1(20),
> -       regulator_desc_ldo1(21),
> -       regulator_desc_ldo2(22),
> -       regulator_desc_ldo2(23),
> -       regulator_desc_ldo1(24),
> -       regulator_desc_ldo1(25),
> -       regulator_desc_ldo1(26),
> +       regulator_desc_ldo(1, MIN_800_MV, STEP_25_MV),
> +       regulator_desc_ldo(2, MIN_800_MV, STEP_50_MV),
> +       regulator_desc_ldo(3, MIN_800_MV, STEP_50_MV),
> +       regulator_desc_ldo(4, MIN_800_MV, STEP_50_MV),
> +       regulator_desc_ldo(5, MIN_800_MV, STEP_50_MV),
> +       regulator_desc_ldo(6, MIN_800_MV, STEP_25_MV),
> +       regulator_desc_ldo(7, MIN_800_MV, STEP_50_MV),
> +       regulator_desc_ldo(8, MIN_800_MV, STEP_50_MV),
> +       regulator_desc_ldo(9, MIN_800_MV, STEP_50_MV),
> +       regulator_desc_ldo(10, MIN_800_MV, STEP_50_MV),
> +       regulator_desc_ldo(11, MIN_800_MV, STEP_25_MV),
> +       regulator_desc_ldo(12, MIN_800_MV, STEP_50_MV),
> +       regulator_desc_ldo(13, MIN_800_MV, STEP_50_MV),
> +       regulator_desc_ldo(14, MIN_800_MV, STEP_50_MV),
> +       regulator_desc_ldo(15, MIN_800_MV, STEP_50_MV),
> +       regulator_desc_ldo(16, MIN_800_MV, STEP_50_MV),
> +       regulator_desc_ldo(17, MIN_800_MV, STEP_50_MV),
> +       regulator_desc_ldo(18, MIN_800_MV, STEP_50_MV),
> +       regulator_desc_ldo(19, MIN_800_MV, STEP_50_MV),
> +       regulator_desc_ldo(20, MIN_800_MV, STEP_50_MV),
> +       regulator_desc_ldo(21, MIN_800_MV, STEP_50_MV),
> +       regulator_desc_ldo(22, MIN_800_MV, STEP_25_MV),
> +       regulator_desc_ldo(23, MIN_800_MV, STEP_25_MV),
> +       regulator_desc_ldo(24, MIN_800_MV, STEP_50_MV),
> +       regulator_desc_ldo(25, MIN_800_MV, STEP_50_MV),
> +       regulator_desc_ldo(26, MIN_800_MV, STEP_50_MV),
>         regulator_desc_buck1_4(1),
>         regulator_desc_buck1_4(2),
>         regulator_desc_buck1_4(3),
>         regulator_desc_buck1_4(4),
>         regulator_desc_buck5,
> -       regulator_desc_buck6_7(6),
> -       regulator_desc_buck6_7(7),
> -       regulator_desc_buck8,
> -       regulator_desc_buck9,
> -       regulator_desc_buck10,
> +       regulator_desc_buck6_10(6, MIN_600_MV, STEP_6_25_MV),
> +       regulator_desc_buck6_10(7, MIN_600_MV, STEP_6_25_MV),
> +       regulator_desc_buck6_10(8, MIN_800_MV, STEP_12_5_MV),
> +       regulator_desc_buck6_10(9, MIN_1500_MV, STEP_12_5_MV),
> +       regulator_desc_buck6_10(10, MIN_1000_MV, STEP_12_5_MV),
>  };
>
>  static int s2mpa01_pmic_probe(struct platform_device *pdev)
> --
> 1.9.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Lee Jones July 10, 2014, 7:35 a.m. | #2
On Thu, 10 Jul 2014, amit daniel kachhap wrote:
> Adding MFD maintainers.

Why, what for?

> On Tue, Jul 8, 2014 at 5:57 PM, Amit Daniel Kachhap
> <amit.daniel@samsung.com> wrote:
> > This patch makes the regulator description macro take minimum and
> > steps voltage as parameter. In this way many repeated macros can be
> > removed. Now these macros are repeated only if the the LDO/BUCK ctrl
> > registers have non-linear positions. The good thing is these ctrl registers
> > are mostly linear so they are not passed as parameters.
> >
> > This patch reduces the code size and also allow easy addition of more
> > s2mpxxx PMIC drivers which differs a lot in minimum/step voltages.
> >
> > Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
> > ---
> >  drivers/regulator/s2mpa01.c | 136 ++++++++++++--------------------------------
> >  1 file changed, 37 insertions(+), 99 deletions(-)
> >
> > diff --git a/drivers/regulator/s2mpa01.c b/drivers/regulator/s2mpa01.c
> > index 962c5f1..8073466 100644
> > --- a/drivers/regulator/s2mpa01.c
> > +++ b/drivers/regulator/s2mpa01.c
> > @@ -235,28 +235,14 @@ static struct regulator_ops s2mpa01_buck_ops = {
> >         .set_ramp_delay         = s2mpa01_set_ramp_delay,
> >  };
> >
> > -#define regulator_desc_ldo1(num)       {               \
> > +#define regulator_desc_ldo(num, min, step)     {       \
> >         .name           = "LDO"#num,                    \
> >         .id             = S2MPA01_LDO##num,             \
> >         .ops            = &s2mpa01_ldo_ops,             \
> >         .type           = REGULATOR_VOLTAGE,            \
> >         .owner          = THIS_MODULE,                  \
> > -       .min_uV         = MIN_800_MV,                   \
> > -       .uV_step        = STEP_50_MV,                   \
> > -       .n_voltages     = S2MPA01_LDO_N_VOLTAGES,       \
> > -       .vsel_reg       = S2MPA01_REG_L1CTRL + num - 1, \
> > -       .vsel_mask      = S2MPA01_LDO_VSEL_MASK,        \
> > -       .enable_reg     = S2MPA01_REG_L1CTRL + num - 1, \
> > -       .enable_mask    = S2MPA01_ENABLE_MASK           \
> > -}
> > -#define regulator_desc_ldo2(num)       {               \
> > -       .name           = "LDO"#num,                    \
> > -       .id             = S2MPA01_LDO##num,             \
> > -       .ops            = &s2mpa01_ldo_ops,             \
> > -       .type           = REGULATOR_VOLTAGE,            \
> > -       .owner          = THIS_MODULE,                  \
> > -       .min_uV         = MIN_800_MV,                   \
> > -       .uV_step        = STEP_25_MV,                   \
> > +       .min_uV         = min,                          \
> > +       .uV_step        = step,                         \
> >         .n_voltages     = S2MPA01_LDO_N_VOLTAGES,       \
> >         .vsel_reg       = S2MPA01_REG_L1CTRL + num - 1, \
> >         .vsel_mask      = S2MPA01_LDO_VSEL_MASK,        \
> > @@ -296,14 +282,14 @@ static struct regulator_ops s2mpa01_buck_ops = {
> >         .enable_mask    = S2MPA01_ENABLE_MASK                   \
> >  }
> >
> > -#define regulator_desc_buck6_7(num)    {                       \
> > +#define regulator_desc_buck6_10(num, min, step)        {               \
> >         .name           = "BUCK"#num,                           \
> >         .id             = S2MPA01_BUCK##num,                    \
> >         .ops            = &s2mpa01_buck_ops,                    \
> >         .type           = REGULATOR_VOLTAGE,                    \
> >         .owner          = THIS_MODULE,                          \
> > -       .min_uV         = MIN_600_MV,                           \
> > -       .uV_step        = STEP_6_25_MV,                         \
> > +       .min_uV         = min,                                  \
> > +       .uV_step        = step,                                 \
> >         .n_voltages     = S2MPA01_BUCK_N_VOLTAGES,              \
> >         .ramp_delay     = S2MPA01_RAMP_DELAY,                   \
> >         .vsel_reg       = S2MPA01_REG_B6CTRL2 + (num - 6) * 2,  \
> > @@ -312,91 +298,43 @@ static struct regulator_ops s2mpa01_buck_ops = {
> >         .enable_mask    = S2MPA01_ENABLE_MASK                   \
> >  }
> >
> > -#define regulator_desc_buck8   {                               \
> > -       .name           = "BUCK8",                              \
> > -       .id             = S2MPA01_BUCK8,                        \
> > -       .ops            = &s2mpa01_buck_ops,                    \
> > -       .type           = REGULATOR_VOLTAGE,                    \
> > -       .owner          = THIS_MODULE,                          \
> > -       .min_uV         = MIN_800_MV,                           \
> > -       .uV_step        = STEP_12_5_MV,                         \
> > -       .n_voltages     = S2MPA01_BUCK_N_VOLTAGES,              \
> > -       .ramp_delay     = S2MPA01_RAMP_DELAY,                   \
> > -       .vsel_reg       = S2MPA01_REG_B8CTRL2,                  \
> > -       .vsel_mask      = S2MPA01_BUCK_VSEL_MASK,               \
> > -       .enable_reg     = S2MPA01_REG_B8CTRL1,                  \
> > -       .enable_mask    = S2MPA01_ENABLE_MASK                   \
> > -}
> > -
> > -#define regulator_desc_buck9   {                               \
> > -       .name           = "BUCK9",                              \
> > -       .id             = S2MPA01_BUCK9,                        \
> > -       .ops            = &s2mpa01_buck_ops,                    \
> > -       .type           = REGULATOR_VOLTAGE,                    \
> > -       .owner          = THIS_MODULE,                          \
> > -       .min_uV         = MIN_1500_MV,                          \
> > -       .uV_step        = STEP_12_5_MV,                         \
> > -       .n_voltages     = S2MPA01_BUCK_N_VOLTAGES,              \
> > -       .ramp_delay     = S2MPA01_RAMP_DELAY,                   \
> > -       .vsel_reg       = S2MPA01_REG_B9CTRL2,                  \
> > -       .vsel_mask      = S2MPA01_BUCK_VSEL_MASK,               \
> > -       .enable_reg     = S2MPA01_REG_B9CTRL1,                  \
> > -       .enable_mask    = S2MPA01_ENABLE_MASK                   \
> > -}
> > -
> > -#define regulator_desc_buck10  {                               \
> > -       .name           = "BUCK10",                             \
> > -       .id             = S2MPA01_BUCK10,                       \
> > -       .ops            = &s2mpa01_buck_ops,                    \
> > -       .type           = REGULATOR_VOLTAGE,                    \
> > -       .owner          = THIS_MODULE,                          \
> > -       .min_uV         = MIN_1000_MV,                          \
> > -       .uV_step        = STEP_12_5_MV,                         \
> > -       .n_voltages     = S2MPA01_BUCK_N_VOLTAGES,              \
> > -       .ramp_delay     = S2MPA01_RAMP_DELAY,                   \
> > -       .vsel_reg       = S2MPA01_REG_B10CTRL2,                 \
> > -       .vsel_mask      = S2MPA01_BUCK_VSEL_MASK,               \
> > -       .enable_reg     = S2MPA01_REG_B10CTRL1,                 \
> > -       .enable_mask    = S2MPA01_ENABLE_MASK                   \
> > -}
> > -
> >  static struct regulator_desc regulators[] = {
> > -       regulator_desc_ldo2(1),
> > -       regulator_desc_ldo1(2),
> > -       regulator_desc_ldo1(3),
> > -       regulator_desc_ldo1(4),
> > -       regulator_desc_ldo1(5),
> > -       regulator_desc_ldo2(6),
> > -       regulator_desc_ldo1(7),
> > -       regulator_desc_ldo1(8),
> > -       regulator_desc_ldo1(9),
> > -       regulator_desc_ldo1(10),
> > -       regulator_desc_ldo2(11),
> > -       regulator_desc_ldo1(12),
> > -       regulator_desc_ldo1(13),
> > -       regulator_desc_ldo1(14),
> > -       regulator_desc_ldo1(15),
> > -       regulator_desc_ldo1(16),
> > -       regulator_desc_ldo1(17),
> > -       regulator_desc_ldo1(18),
> > -       regulator_desc_ldo1(19),
> > -       regulator_desc_ldo1(20),
> > -       regulator_desc_ldo1(21),
> > -       regulator_desc_ldo2(22),
> > -       regulator_desc_ldo2(23),
> > -       regulator_desc_ldo1(24),
> > -       regulator_desc_ldo1(25),
> > -       regulator_desc_ldo1(26),
> > +       regulator_desc_ldo(1, MIN_800_MV, STEP_25_MV),
> > +       regulator_desc_ldo(2, MIN_800_MV, STEP_50_MV),
> > +       regulator_desc_ldo(3, MIN_800_MV, STEP_50_MV),
> > +       regulator_desc_ldo(4, MIN_800_MV, STEP_50_MV),
> > +       regulator_desc_ldo(5, MIN_800_MV, STEP_50_MV),
> > +       regulator_desc_ldo(6, MIN_800_MV, STEP_25_MV),
> > +       regulator_desc_ldo(7, MIN_800_MV, STEP_50_MV),
> > +       regulator_desc_ldo(8, MIN_800_MV, STEP_50_MV),
> > +       regulator_desc_ldo(9, MIN_800_MV, STEP_50_MV),
> > +       regulator_desc_ldo(10, MIN_800_MV, STEP_50_MV),
> > +       regulator_desc_ldo(11, MIN_800_MV, STEP_25_MV),
> > +       regulator_desc_ldo(12, MIN_800_MV, STEP_50_MV),
> > +       regulator_desc_ldo(13, MIN_800_MV, STEP_50_MV),
> > +       regulator_desc_ldo(14, MIN_800_MV, STEP_50_MV),
> > +       regulator_desc_ldo(15, MIN_800_MV, STEP_50_MV),
> > +       regulator_desc_ldo(16, MIN_800_MV, STEP_50_MV),
> > +       regulator_desc_ldo(17, MIN_800_MV, STEP_50_MV),
> > +       regulator_desc_ldo(18, MIN_800_MV, STEP_50_MV),
> > +       regulator_desc_ldo(19, MIN_800_MV, STEP_50_MV),
> > +       regulator_desc_ldo(20, MIN_800_MV, STEP_50_MV),
> > +       regulator_desc_ldo(21, MIN_800_MV, STEP_50_MV),
> > +       regulator_desc_ldo(22, MIN_800_MV, STEP_25_MV),
> > +       regulator_desc_ldo(23, MIN_800_MV, STEP_25_MV),
> > +       regulator_desc_ldo(24, MIN_800_MV, STEP_50_MV),
> > +       regulator_desc_ldo(25, MIN_800_MV, STEP_50_MV),
> > +       regulator_desc_ldo(26, MIN_800_MV, STEP_50_MV),
> >         regulator_desc_buck1_4(1),
> >         regulator_desc_buck1_4(2),
> >         regulator_desc_buck1_4(3),
> >         regulator_desc_buck1_4(4),
> >         regulator_desc_buck5,
> > -       regulator_desc_buck6_7(6),
> > -       regulator_desc_buck6_7(7),
> > -       regulator_desc_buck8,
> > -       regulator_desc_buck9,
> > -       regulator_desc_buck10,
> > +       regulator_desc_buck6_10(6, MIN_600_MV, STEP_6_25_MV),
> > +       regulator_desc_buck6_10(7, MIN_600_MV, STEP_6_25_MV),
> > +       regulator_desc_buck6_10(8, MIN_800_MV, STEP_12_5_MV),
> > +       regulator_desc_buck6_10(9, MIN_1500_MV, STEP_12_5_MV),
> > +       regulator_desc_buck6_10(10, MIN_1000_MV, STEP_12_5_MV),
> >  };
> >
> >  static int s2mpa01_pmic_probe(struct platform_device *pdev)
> >
Amit Daniel Kachhap July 10, 2014, 10:10 a.m. | #3
On Thu, Jul 10, 2014 at 1:05 PM, Lee Jones <lee.jones@linaro.org> wrote:
> On Thu, 10 Jul 2014, amit daniel kachhap wrote:
>> Adding MFD maintainers.
>
> Why, what for?
Sorry not needed for this patch.
>
>> On Tue, Jul 8, 2014 at 5:57 PM, Amit Daniel Kachhap
>> <amit.daniel@samsung.com> wrote:
>> > This patch makes the regulator description macro take minimum and
>> > steps voltage as parameter. In this way many repeated macros can be
>> > removed. Now these macros are repeated only if the the LDO/BUCK ctrl
>> > registers have non-linear positions. The good thing is these ctrl registers
>> > are mostly linear so they are not passed as parameters.
>> >
>> > This patch reduces the code size and also allow easy addition of more
>> > s2mpxxx PMIC drivers which differs a lot in minimum/step voltages.
>> >
>> > Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
>> > ---
>> >  drivers/regulator/s2mpa01.c | 136 ++++++++++++--------------------------------
>> >  1 file changed, 37 insertions(+), 99 deletions(-)
>> >
>> > diff --git a/drivers/regulator/s2mpa01.c b/drivers/regulator/s2mpa01.c
>> > index 962c5f1..8073466 100644
>> > --- a/drivers/regulator/s2mpa01.c
>> > +++ b/drivers/regulator/s2mpa01.c
>> > @@ -235,28 +235,14 @@ static struct regulator_ops s2mpa01_buck_ops = {
>> >         .set_ramp_delay         = s2mpa01_set_ramp_delay,
>> >  };
>> >
>> > -#define regulator_desc_ldo1(num)       {               \
>> > +#define regulator_desc_ldo(num, min, step)     {       \
>> >         .name           = "LDO"#num,                    \
>> >         .id             = S2MPA01_LDO##num,             \
>> >         .ops            = &s2mpa01_ldo_ops,             \
>> >         .type           = REGULATOR_VOLTAGE,            \
>> >         .owner          = THIS_MODULE,                  \
>> > -       .min_uV         = MIN_800_MV,                   \
>> > -       .uV_step        = STEP_50_MV,                   \
>> > -       .n_voltages     = S2MPA01_LDO_N_VOLTAGES,       \
>> > -       .vsel_reg       = S2MPA01_REG_L1CTRL + num - 1, \
>> > -       .vsel_mask      = S2MPA01_LDO_VSEL_MASK,        \
>> > -       .enable_reg     = S2MPA01_REG_L1CTRL + num - 1, \
>> > -       .enable_mask    = S2MPA01_ENABLE_MASK           \
>> > -}
>> > -#define regulator_desc_ldo2(num)       {               \
>> > -       .name           = "LDO"#num,                    \
>> > -       .id             = S2MPA01_LDO##num,             \
>> > -       .ops            = &s2mpa01_ldo_ops,             \
>> > -       .type           = REGULATOR_VOLTAGE,            \
>> > -       .owner          = THIS_MODULE,                  \
>> > -       .min_uV         = MIN_800_MV,                   \
>> > -       .uV_step        = STEP_25_MV,                   \
>> > +       .min_uV         = min,                          \
>> > +       .uV_step        = step,                         \
>> >         .n_voltages     = S2MPA01_LDO_N_VOLTAGES,       \
>> >         .vsel_reg       = S2MPA01_REG_L1CTRL + num - 1, \
>> >         .vsel_mask      = S2MPA01_LDO_VSEL_MASK,        \
>> > @@ -296,14 +282,14 @@ static struct regulator_ops s2mpa01_buck_ops = {
>> >         .enable_mask    = S2MPA01_ENABLE_MASK                   \
>> >  }
>> >
>> > -#define regulator_desc_buck6_7(num)    {                       \
>> > +#define regulator_desc_buck6_10(num, min, step)        {               \
>> >         .name           = "BUCK"#num,                           \
>> >         .id             = S2MPA01_BUCK##num,                    \
>> >         .ops            = &s2mpa01_buck_ops,                    \
>> >         .type           = REGULATOR_VOLTAGE,                    \
>> >         .owner          = THIS_MODULE,                          \
>> > -       .min_uV         = MIN_600_MV,                           \
>> > -       .uV_step        = STEP_6_25_MV,                         \
>> > +       .min_uV         = min,                                  \
>> > +       .uV_step        = step,                                 \
>> >         .n_voltages     = S2MPA01_BUCK_N_VOLTAGES,              \
>> >         .ramp_delay     = S2MPA01_RAMP_DELAY,                   \
>> >         .vsel_reg       = S2MPA01_REG_B6CTRL2 + (num - 6) * 2,  \
>> > @@ -312,91 +298,43 @@ static struct regulator_ops s2mpa01_buck_ops = {
>> >         .enable_mask    = S2MPA01_ENABLE_MASK                   \
>> >  }
>> >
>> > -#define regulator_desc_buck8   {                               \
>> > -       .name           = "BUCK8",                              \
>> > -       .id             = S2MPA01_BUCK8,                        \
>> > -       .ops            = &s2mpa01_buck_ops,                    \
>> > -       .type           = REGULATOR_VOLTAGE,                    \
>> > -       .owner          = THIS_MODULE,                          \
>> > -       .min_uV         = MIN_800_MV,                           \
>> > -       .uV_step        = STEP_12_5_MV,                         \
>> > -       .n_voltages     = S2MPA01_BUCK_N_VOLTAGES,              \
>> > -       .ramp_delay     = S2MPA01_RAMP_DELAY,                   \
>> > -       .vsel_reg       = S2MPA01_REG_B8CTRL2,                  \
>> > -       .vsel_mask      = S2MPA01_BUCK_VSEL_MASK,               \
>> > -       .enable_reg     = S2MPA01_REG_B8CTRL1,                  \
>> > -       .enable_mask    = S2MPA01_ENABLE_MASK                   \
>> > -}
>> > -
>> > -#define regulator_desc_buck9   {                               \
>> > -       .name           = "BUCK9",                              \
>> > -       .id             = S2MPA01_BUCK9,                        \
>> > -       .ops            = &s2mpa01_buck_ops,                    \
>> > -       .type           = REGULATOR_VOLTAGE,                    \
>> > -       .owner          = THIS_MODULE,                          \
>> > -       .min_uV         = MIN_1500_MV,                          \
>> > -       .uV_step        = STEP_12_5_MV,                         \
>> > -       .n_voltages     = S2MPA01_BUCK_N_VOLTAGES,              \
>> > -       .ramp_delay     = S2MPA01_RAMP_DELAY,                   \
>> > -       .vsel_reg       = S2MPA01_REG_B9CTRL2,                  \
>> > -       .vsel_mask      = S2MPA01_BUCK_VSEL_MASK,               \
>> > -       .enable_reg     = S2MPA01_REG_B9CTRL1,                  \
>> > -       .enable_mask    = S2MPA01_ENABLE_MASK                   \
>> > -}
>> > -
>> > -#define regulator_desc_buck10  {                               \
>> > -       .name           = "BUCK10",                             \
>> > -       .id             = S2MPA01_BUCK10,                       \
>> > -       .ops            = &s2mpa01_buck_ops,                    \
>> > -       .type           = REGULATOR_VOLTAGE,                    \
>> > -       .owner          = THIS_MODULE,                          \
>> > -       .min_uV         = MIN_1000_MV,                          \
>> > -       .uV_step        = STEP_12_5_MV,                         \
>> > -       .n_voltages     = S2MPA01_BUCK_N_VOLTAGES,              \
>> > -       .ramp_delay     = S2MPA01_RAMP_DELAY,                   \
>> > -       .vsel_reg       = S2MPA01_REG_B10CTRL2,                 \
>> > -       .vsel_mask      = S2MPA01_BUCK_VSEL_MASK,               \
>> > -       .enable_reg     = S2MPA01_REG_B10CTRL1,                 \
>> > -       .enable_mask    = S2MPA01_ENABLE_MASK                   \
>> > -}
>> > -
>> >  static struct regulator_desc regulators[] = {
>> > -       regulator_desc_ldo2(1),
>> > -       regulator_desc_ldo1(2),
>> > -       regulator_desc_ldo1(3),
>> > -       regulator_desc_ldo1(4),
>> > -       regulator_desc_ldo1(5),
>> > -       regulator_desc_ldo2(6),
>> > -       regulator_desc_ldo1(7),
>> > -       regulator_desc_ldo1(8),
>> > -       regulator_desc_ldo1(9),
>> > -       regulator_desc_ldo1(10),
>> > -       regulator_desc_ldo2(11),
>> > -       regulator_desc_ldo1(12),
>> > -       regulator_desc_ldo1(13),
>> > -       regulator_desc_ldo1(14),
>> > -       regulator_desc_ldo1(15),
>> > -       regulator_desc_ldo1(16),
>> > -       regulator_desc_ldo1(17),
>> > -       regulator_desc_ldo1(18),
>> > -       regulator_desc_ldo1(19),
>> > -       regulator_desc_ldo1(20),
>> > -       regulator_desc_ldo1(21),
>> > -       regulator_desc_ldo2(22),
>> > -       regulator_desc_ldo2(23),
>> > -       regulator_desc_ldo1(24),
>> > -       regulator_desc_ldo1(25),
>> > -       regulator_desc_ldo1(26),
>> > +       regulator_desc_ldo(1, MIN_800_MV, STEP_25_MV),
>> > +       regulator_desc_ldo(2, MIN_800_MV, STEP_50_MV),
>> > +       regulator_desc_ldo(3, MIN_800_MV, STEP_50_MV),
>> > +       regulator_desc_ldo(4, MIN_800_MV, STEP_50_MV),
>> > +       regulator_desc_ldo(5, MIN_800_MV, STEP_50_MV),
>> > +       regulator_desc_ldo(6, MIN_800_MV, STEP_25_MV),
>> > +       regulator_desc_ldo(7, MIN_800_MV, STEP_50_MV),
>> > +       regulator_desc_ldo(8, MIN_800_MV, STEP_50_MV),
>> > +       regulator_desc_ldo(9, MIN_800_MV, STEP_50_MV),
>> > +       regulator_desc_ldo(10, MIN_800_MV, STEP_50_MV),
>> > +       regulator_desc_ldo(11, MIN_800_MV, STEP_25_MV),
>> > +       regulator_desc_ldo(12, MIN_800_MV, STEP_50_MV),
>> > +       regulator_desc_ldo(13, MIN_800_MV, STEP_50_MV),
>> > +       regulator_desc_ldo(14, MIN_800_MV, STEP_50_MV),
>> > +       regulator_desc_ldo(15, MIN_800_MV, STEP_50_MV),
>> > +       regulator_desc_ldo(16, MIN_800_MV, STEP_50_MV),
>> > +       regulator_desc_ldo(17, MIN_800_MV, STEP_50_MV),
>> > +       regulator_desc_ldo(18, MIN_800_MV, STEP_50_MV),
>> > +       regulator_desc_ldo(19, MIN_800_MV, STEP_50_MV),
>> > +       regulator_desc_ldo(20, MIN_800_MV, STEP_50_MV),
>> > +       regulator_desc_ldo(21, MIN_800_MV, STEP_50_MV),
>> > +       regulator_desc_ldo(22, MIN_800_MV, STEP_25_MV),
>> > +       regulator_desc_ldo(23, MIN_800_MV, STEP_25_MV),
>> > +       regulator_desc_ldo(24, MIN_800_MV, STEP_50_MV),
>> > +       regulator_desc_ldo(25, MIN_800_MV, STEP_50_MV),
>> > +       regulator_desc_ldo(26, MIN_800_MV, STEP_50_MV),
>> >         regulator_desc_buck1_4(1),
>> >         regulator_desc_buck1_4(2),
>> >         regulator_desc_buck1_4(3),
>> >         regulator_desc_buck1_4(4),
>> >         regulator_desc_buck5,
>> > -       regulator_desc_buck6_7(6),
>> > -       regulator_desc_buck6_7(7),
>> > -       regulator_desc_buck8,
>> > -       regulator_desc_buck9,
>> > -       regulator_desc_buck10,
>> > +       regulator_desc_buck6_10(6, MIN_600_MV, STEP_6_25_MV),
>> > +       regulator_desc_buck6_10(7, MIN_600_MV, STEP_6_25_MV),
>> > +       regulator_desc_buck6_10(8, MIN_800_MV, STEP_12_5_MV),
>> > +       regulator_desc_buck6_10(9, MIN_1500_MV, STEP_12_5_MV),
>> > +       regulator_desc_buck6_10(10, MIN_1000_MV, STEP_12_5_MV),
>> >  };
>> >
>> >  static int s2mpa01_pmic_probe(struct platform_device *pdev)
>> >
>
> --
> Lee Jones
> Linaro STMicroelectronics Landing Team Lead
> Linaro.org │ Open source software for ARM SoCs
> Follow Linaro: Facebook | Twitter | Blog
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Krzysztof Kozlowski July 14, 2014, 8:49 a.m. | #4
On 08.07.2014 14:27, Amit Daniel Kachhap wrote:
> This patch makes the regulator description macro take minimum and
> steps voltage as parameter. In this way many repeated macros can be
> removed. Now these macros are repeated only if the the LDO/BUCK ctrl
> registers have non-linear positions. The good thing is these ctrl registers
> are mostly linear so they are not passed as parameters.
>
> This patch reduces the code size and also allow easy addition of more
> s2mpxxx PMIC drivers which differs a lot in minimum/step voltages.
>
> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
> ---
>   drivers/regulator/s2mpa01.c | 136 ++++++++++++--------------------------------
>   1 file changed, 37 insertions(+), 99 deletions(-)
>
> diff --git a/drivers/regulator/s2mpa01.c b/drivers/regulator/s2mpa01.c
> index 962c5f1..8073466 100644
> --- a/drivers/regulator/s2mpa01.c
> +++ b/drivers/regulator/s2mpa01.c
> @@ -235,28 +235,14 @@ static struct regulator_ops s2mpa01_buck_ops = {
>   	.set_ramp_delay		= s2mpa01_set_ramp_delay,
>   };
>
> -#define regulator_desc_ldo1(num)	{		\
> +#define regulator_desc_ldo(num, min, step)	{	\

Why adding parameter for the 'min' value? It is always the same for LDOs 
- 800 mV.

The same applies for the s2mps11 regulator driver.

Best regards,
Krzysztof

>   	.name		= "LDO"#num,			\
>   	.id		= S2MPA01_LDO##num,		\
>   	.ops		= &s2mpa01_ldo_ops,		\
>   	.type		= REGULATOR_VOLTAGE,		\
>   	.owner		= THIS_MODULE,			\
> -	.min_uV		= MIN_800_MV,			\
> -	.uV_step	= STEP_50_MV,			\
> -	.n_voltages	= S2MPA01_LDO_N_VOLTAGES,	\
> -	.vsel_reg	= S2MPA01_REG_L1CTRL + num - 1,	\
> -	.vsel_mask	= S2MPA01_LDO_VSEL_MASK,	\
> -	.enable_reg	= S2MPA01_REG_L1CTRL + num - 1,	\
> -	.enable_mask	= S2MPA01_ENABLE_MASK		\
> -}
> -#define regulator_desc_ldo2(num)	{		\
> -	.name		= "LDO"#num,			\
> -	.id		= S2MPA01_LDO##num,		\
> -	.ops		= &s2mpa01_ldo_ops,		\
> -	.type		= REGULATOR_VOLTAGE,		\
> -	.owner		= THIS_MODULE,			\
> -	.min_uV		= MIN_800_MV,			\
> -	.uV_step	= STEP_25_MV,			\
> +	.min_uV		= min,				\
> +	.uV_step	= step,				\
>   	.n_voltages	= S2MPA01_LDO_N_VOLTAGES,	\
>   	.vsel_reg	= S2MPA01_REG_L1CTRL + num - 1,	\
>   	.vsel_mask	= S2MPA01_LDO_VSEL_MASK,	\
> @@ -296,14 +282,14 @@ static struct regulator_ops s2mpa01_buck_ops = {
>   	.enable_mask	= S2MPA01_ENABLE_MASK			\
>   }
>
> -#define regulator_desc_buck6_7(num)	{			\
> +#define regulator_desc_buck6_10(num, min, step)	{		\
>   	.name		= "BUCK"#num,				\
>   	.id		= S2MPA01_BUCK##num,			\
>   	.ops		= &s2mpa01_buck_ops,			\
>   	.type		= REGULATOR_VOLTAGE,			\
>   	.owner		= THIS_MODULE,				\
> -	.min_uV		= MIN_600_MV,				\
> -	.uV_step	= STEP_6_25_MV,				\
> +	.min_uV		= min,					\
> +	.uV_step	= step,					\
>   	.n_voltages	= S2MPA01_BUCK_N_VOLTAGES,		\
>   	.ramp_delay	= S2MPA01_RAMP_DELAY,			\
>   	.vsel_reg	= S2MPA01_REG_B6CTRL2 + (num - 6) * 2,	\
> @@ -312,91 +298,43 @@ static struct regulator_ops s2mpa01_buck_ops = {
>   	.enable_mask	= S2MPA01_ENABLE_MASK			\
>   }
>
> -#define regulator_desc_buck8	{				\
> -	.name		= "BUCK8",				\
> -	.id		= S2MPA01_BUCK8,			\
> -	.ops		= &s2mpa01_buck_ops,			\
> -	.type		= REGULATOR_VOLTAGE,			\
> -	.owner		= THIS_MODULE,				\
> -	.min_uV		= MIN_800_MV,				\
> -	.uV_step	= STEP_12_5_MV,				\
> -	.n_voltages	= S2MPA01_BUCK_N_VOLTAGES,		\
> -	.ramp_delay	= S2MPA01_RAMP_DELAY,			\
> -	.vsel_reg	= S2MPA01_REG_B8CTRL2,			\
> -	.vsel_mask	= S2MPA01_BUCK_VSEL_MASK,		\
> -	.enable_reg	= S2MPA01_REG_B8CTRL1,			\
> -	.enable_mask	= S2MPA01_ENABLE_MASK			\
> -}
> -
> -#define regulator_desc_buck9	{				\
> -	.name		= "BUCK9",				\
> -	.id		= S2MPA01_BUCK9,			\
> -	.ops		= &s2mpa01_buck_ops,			\
> -	.type		= REGULATOR_VOLTAGE,			\
> -	.owner		= THIS_MODULE,				\
> -	.min_uV		= MIN_1500_MV,				\
> -	.uV_step	= STEP_12_5_MV,				\
> -	.n_voltages	= S2MPA01_BUCK_N_VOLTAGES,		\
> -	.ramp_delay	= S2MPA01_RAMP_DELAY,			\
> -	.vsel_reg	= S2MPA01_REG_B9CTRL2,			\
> -	.vsel_mask	= S2MPA01_BUCK_VSEL_MASK,		\
> -	.enable_reg	= S2MPA01_REG_B9CTRL1,			\
> -	.enable_mask	= S2MPA01_ENABLE_MASK			\
> -}
> -
> -#define regulator_desc_buck10	{				\
> -	.name		= "BUCK10",				\
> -	.id		= S2MPA01_BUCK10,			\
> -	.ops		= &s2mpa01_buck_ops,			\
> -	.type		= REGULATOR_VOLTAGE,			\
> -	.owner		= THIS_MODULE,				\
> -	.min_uV		= MIN_1000_MV,				\
> -	.uV_step	= STEP_12_5_MV,				\
> -	.n_voltages	= S2MPA01_BUCK_N_VOLTAGES,		\
> -	.ramp_delay	= S2MPA01_RAMP_DELAY,			\
> -	.vsel_reg	= S2MPA01_REG_B10CTRL2,			\
> -	.vsel_mask	= S2MPA01_BUCK_VSEL_MASK,		\
> -	.enable_reg	= S2MPA01_REG_B10CTRL1,			\
> -	.enable_mask	= S2MPA01_ENABLE_MASK			\
> -}
> -
>   static struct regulator_desc regulators[] = {
> -	regulator_desc_ldo2(1),
> -	regulator_desc_ldo1(2),
> -	regulator_desc_ldo1(3),
> -	regulator_desc_ldo1(4),
> -	regulator_desc_ldo1(5),
> -	regulator_desc_ldo2(6),
> -	regulator_desc_ldo1(7),
> -	regulator_desc_ldo1(8),
> -	regulator_desc_ldo1(9),
> -	regulator_desc_ldo1(10),
> -	regulator_desc_ldo2(11),
> -	regulator_desc_ldo1(12),
> -	regulator_desc_ldo1(13),
> -	regulator_desc_ldo1(14),
> -	regulator_desc_ldo1(15),
> -	regulator_desc_ldo1(16),
> -	regulator_desc_ldo1(17),
> -	regulator_desc_ldo1(18),
> -	regulator_desc_ldo1(19),
> -	regulator_desc_ldo1(20),
> -	regulator_desc_ldo1(21),
> -	regulator_desc_ldo2(22),
> -	regulator_desc_ldo2(23),
> -	regulator_desc_ldo1(24),
> -	regulator_desc_ldo1(25),
> -	regulator_desc_ldo1(26),
> +	regulator_desc_ldo(1, MIN_800_MV, STEP_25_MV),
> +	regulator_desc_ldo(2, MIN_800_MV, STEP_50_MV),
> +	regulator_desc_ldo(3, MIN_800_MV, STEP_50_MV),
> +	regulator_desc_ldo(4, MIN_800_MV, STEP_50_MV),
> +	regulator_desc_ldo(5, MIN_800_MV, STEP_50_MV),
> +	regulator_desc_ldo(6, MIN_800_MV, STEP_25_MV),
> +	regulator_desc_ldo(7, MIN_800_MV, STEP_50_MV),
> +	regulator_desc_ldo(8, MIN_800_MV, STEP_50_MV),
> +	regulator_desc_ldo(9, MIN_800_MV, STEP_50_MV),
> +	regulator_desc_ldo(10, MIN_800_MV, STEP_50_MV),
> +	regulator_desc_ldo(11, MIN_800_MV, STEP_25_MV),
> +	regulator_desc_ldo(12, MIN_800_MV, STEP_50_MV),
> +	regulator_desc_ldo(13, MIN_800_MV, STEP_50_MV),
> +	regulator_desc_ldo(14, MIN_800_MV, STEP_50_MV),
> +	regulator_desc_ldo(15, MIN_800_MV, STEP_50_MV),
> +	regulator_desc_ldo(16, MIN_800_MV, STEP_50_MV),
> +	regulator_desc_ldo(17, MIN_800_MV, STEP_50_MV),
> +	regulator_desc_ldo(18, MIN_800_MV, STEP_50_MV),
> +	regulator_desc_ldo(19, MIN_800_MV, STEP_50_MV),
> +	regulator_desc_ldo(20, MIN_800_MV, STEP_50_MV),
> +	regulator_desc_ldo(21, MIN_800_MV, STEP_50_MV),
> +	regulator_desc_ldo(22, MIN_800_MV, STEP_25_MV),
> +	regulator_desc_ldo(23, MIN_800_MV, STEP_25_MV),
> +	regulator_desc_ldo(24, MIN_800_MV, STEP_50_MV),
> +	regulator_desc_ldo(25, MIN_800_MV, STEP_50_MV),
> +	regulator_desc_ldo(26, MIN_800_MV, STEP_50_MV),
>   	regulator_desc_buck1_4(1),
>   	regulator_desc_buck1_4(2),
>   	regulator_desc_buck1_4(3),
>   	regulator_desc_buck1_4(4),
>   	regulator_desc_buck5,
> -	regulator_desc_buck6_7(6),
> -	regulator_desc_buck6_7(7),
> -	regulator_desc_buck8,
> -	regulator_desc_buck9,
> -	regulator_desc_buck10,
> +	regulator_desc_buck6_10(6, MIN_600_MV, STEP_6_25_MV),
> +	regulator_desc_buck6_10(7, MIN_600_MV, STEP_6_25_MV),
> +	regulator_desc_buck6_10(8, MIN_800_MV, STEP_12_5_MV),
> +	regulator_desc_buck6_10(9, MIN_1500_MV, STEP_12_5_MV),
> +	regulator_desc_buck6_10(10, MIN_1000_MV, STEP_12_5_MV),
>   };
>
>   static int s2mpa01_pmic_probe(struct platform_device *pdev)
>

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Amit Daniel Kachhap July 15, 2014, 9:51 a.m. | #5
On Mon, Jul 14, 2014 at 2:19 PM, Krzysztof Kozlowski
<k.kozlowski@samsung.com> wrote:
> On 08.07.2014 14:27, Amit Daniel Kachhap wrote:
>>
>> This patch makes the regulator description macro take minimum and
>> steps voltage as parameter. In this way many repeated macros can be
>> removed. Now these macros are repeated only if the the LDO/BUCK ctrl
>> registers have non-linear positions. The good thing is these ctrl
>> registers
>> are mostly linear so they are not passed as parameters.
>>
>> This patch reduces the code size and also allow easy addition of more
>> s2mpxxx PMIC drivers which differs a lot in minimum/step voltages.
>>
>> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
>> ---
>>   drivers/regulator/s2mpa01.c | 136
>> ++++++++++++--------------------------------
>>   1 file changed, 37 insertions(+), 99 deletions(-)
>>
>> diff --git a/drivers/regulator/s2mpa01.c b/drivers/regulator/s2mpa01.c
>> index 962c5f1..8073466 100644
>> --- a/drivers/regulator/s2mpa01.c
>> +++ b/drivers/regulator/s2mpa01.c
>> @@ -235,28 +235,14 @@ static struct regulator_ops s2mpa01_buck_ops = {
>>         .set_ramp_delay         = s2mpa01_set_ramp_delay,
>>   };
>>
>> -#define regulator_desc_ldo1(num)       {               \
>> +#define regulator_desc_ldo(num, min, step)     {       \
>
>
> Why adding parameter for the 'min' value? It is always the same for LDOs -
> 800 mV.
>
> The same applies for the s2mps11 regulator driver.
Correct. min parameter can be removed.
Thanks for the review.
>
> Best regards,
> Krzysztof
>
>
>>         .name           = "LDO"#num,                    \
>>         .id             = S2MPA01_LDO##num,             \
>>         .ops            = &s2mpa01_ldo_ops,             \
>>         .type           = REGULATOR_VOLTAGE,            \
>>         .owner          = THIS_MODULE,                  \
>> -       .min_uV         = MIN_800_MV,                   \
>> -       .uV_step        = STEP_50_MV,                   \
>> -       .n_voltages     = S2MPA01_LDO_N_VOLTAGES,       \
>> -       .vsel_reg       = S2MPA01_REG_L1CTRL + num - 1, \
>> -       .vsel_mask      = S2MPA01_LDO_VSEL_MASK,        \
>> -       .enable_reg     = S2MPA01_REG_L1CTRL + num - 1, \
>> -       .enable_mask    = S2MPA01_ENABLE_MASK           \
>> -}
>> -#define regulator_desc_ldo2(num)       {               \
>> -       .name           = "LDO"#num,                    \
>> -       .id             = S2MPA01_LDO##num,             \
>> -       .ops            = &s2mpa01_ldo_ops,             \
>> -       .type           = REGULATOR_VOLTAGE,            \
>> -       .owner          = THIS_MODULE,                  \
>> -       .min_uV         = MIN_800_MV,                   \
>> -       .uV_step        = STEP_25_MV,                   \
>> +       .min_uV         = min,                          \
>> +       .uV_step        = step,                         \
>>         .n_voltages     = S2MPA01_LDO_N_VOLTAGES,       \
>>         .vsel_reg       = S2MPA01_REG_L1CTRL + num - 1, \
>>         .vsel_mask      = S2MPA01_LDO_VSEL_MASK,        \
>> @@ -296,14 +282,14 @@ static struct regulator_ops s2mpa01_buck_ops = {
>>         .enable_mask    = S2MPA01_ENABLE_MASK                   \
>>   }
>>
>> -#define regulator_desc_buck6_7(num)    {                       \
>> +#define regulator_desc_buck6_10(num, min, step)        {               \
>>         .name           = "BUCK"#num,                           \
>>         .id             = S2MPA01_BUCK##num,                    \
>>         .ops            = &s2mpa01_buck_ops,                    \
>>         .type           = REGULATOR_VOLTAGE,                    \
>>         .owner          = THIS_MODULE,                          \
>> -       .min_uV         = MIN_600_MV,                           \
>> -       .uV_step        = STEP_6_25_MV,                         \
>> +       .min_uV         = min,                                  \
>> +       .uV_step        = step,                                 \
>>         .n_voltages     = S2MPA01_BUCK_N_VOLTAGES,              \
>>         .ramp_delay     = S2MPA01_RAMP_DELAY,                   \
>>         .vsel_reg       = S2MPA01_REG_B6CTRL2 + (num - 6) * 2,  \
>> @@ -312,91 +298,43 @@ static struct regulator_ops s2mpa01_buck_ops = {
>>         .enable_mask    = S2MPA01_ENABLE_MASK                   \
>>   }
>>
>> -#define regulator_desc_buck8   {                               \
>> -       .name           = "BUCK8",                              \
>> -       .id             = S2MPA01_BUCK8,                        \
>> -       .ops            = &s2mpa01_buck_ops,                    \
>> -       .type           = REGULATOR_VOLTAGE,                    \
>> -       .owner          = THIS_MODULE,                          \
>> -       .min_uV         = MIN_800_MV,                           \
>> -       .uV_step        = STEP_12_5_MV,                         \
>> -       .n_voltages     = S2MPA01_BUCK_N_VOLTAGES,              \
>> -       .ramp_delay     = S2MPA01_RAMP_DELAY,                   \
>> -       .vsel_reg       = S2MPA01_REG_B8CTRL2,                  \
>> -       .vsel_mask      = S2MPA01_BUCK_VSEL_MASK,               \
>> -       .enable_reg     = S2MPA01_REG_B8CTRL1,                  \
>> -       .enable_mask    = S2MPA01_ENABLE_MASK                   \
>> -}
>> -
>> -#define regulator_desc_buck9   {                               \
>> -       .name           = "BUCK9",                              \
>> -       .id             = S2MPA01_BUCK9,                        \
>> -       .ops            = &s2mpa01_buck_ops,                    \
>> -       .type           = REGULATOR_VOLTAGE,                    \
>> -       .owner          = THIS_MODULE,                          \
>> -       .min_uV         = MIN_1500_MV,                          \
>> -       .uV_step        = STEP_12_5_MV,                         \
>> -       .n_voltages     = S2MPA01_BUCK_N_VOLTAGES,              \
>> -       .ramp_delay     = S2MPA01_RAMP_DELAY,                   \
>> -       .vsel_reg       = S2MPA01_REG_B9CTRL2,                  \
>> -       .vsel_mask      = S2MPA01_BUCK_VSEL_MASK,               \
>> -       .enable_reg     = S2MPA01_REG_B9CTRL1,                  \
>> -       .enable_mask    = S2MPA01_ENABLE_MASK                   \
>> -}
>> -
>> -#define regulator_desc_buck10  {                               \
>> -       .name           = "BUCK10",                             \
>> -       .id             = S2MPA01_BUCK10,                       \
>> -       .ops            = &s2mpa01_buck_ops,                    \
>> -       .type           = REGULATOR_VOLTAGE,                    \
>> -       .owner          = THIS_MODULE,                          \
>> -       .min_uV         = MIN_1000_MV,                          \
>> -       .uV_step        = STEP_12_5_MV,                         \
>> -       .n_voltages     = S2MPA01_BUCK_N_VOLTAGES,              \
>> -       .ramp_delay     = S2MPA01_RAMP_DELAY,                   \
>> -       .vsel_reg       = S2MPA01_REG_B10CTRL2,                 \
>> -       .vsel_mask      = S2MPA01_BUCK_VSEL_MASK,               \
>> -       .enable_reg     = S2MPA01_REG_B10CTRL1,                 \
>> -       .enable_mask    = S2MPA01_ENABLE_MASK                   \
>> -}
>> -
>>   static struct regulator_desc regulators[] = {
>> -       regulator_desc_ldo2(1),
>> -       regulator_desc_ldo1(2),
>> -       regulator_desc_ldo1(3),
>> -       regulator_desc_ldo1(4),
>> -       regulator_desc_ldo1(5),
>> -       regulator_desc_ldo2(6),
>> -       regulator_desc_ldo1(7),
>> -       regulator_desc_ldo1(8),
>> -       regulator_desc_ldo1(9),
>> -       regulator_desc_ldo1(10),
>> -       regulator_desc_ldo2(11),
>> -       regulator_desc_ldo1(12),
>> -       regulator_desc_ldo1(13),
>> -       regulator_desc_ldo1(14),
>> -       regulator_desc_ldo1(15),
>> -       regulator_desc_ldo1(16),
>> -       regulator_desc_ldo1(17),
>> -       regulator_desc_ldo1(18),
>> -       regulator_desc_ldo1(19),
>> -       regulator_desc_ldo1(20),
>> -       regulator_desc_ldo1(21),
>> -       regulator_desc_ldo2(22),
>> -       regulator_desc_ldo2(23),
>> -       regulator_desc_ldo1(24),
>> -       regulator_desc_ldo1(25),
>> -       regulator_desc_ldo1(26),
>> +       regulator_desc_ldo(1, MIN_800_MV, STEP_25_MV),
>> +       regulator_desc_ldo(2, MIN_800_MV, STEP_50_MV),
>> +       regulator_desc_ldo(3, MIN_800_MV, STEP_50_MV),
>> +       regulator_desc_ldo(4, MIN_800_MV, STEP_50_MV),
>> +       regulator_desc_ldo(5, MIN_800_MV, STEP_50_MV),
>> +       regulator_desc_ldo(6, MIN_800_MV, STEP_25_MV),
>> +       regulator_desc_ldo(7, MIN_800_MV, STEP_50_MV),
>> +       regulator_desc_ldo(8, MIN_800_MV, STEP_50_MV),
>> +       regulator_desc_ldo(9, MIN_800_MV, STEP_50_MV),
>> +       regulator_desc_ldo(10, MIN_800_MV, STEP_50_MV),
>> +       regulator_desc_ldo(11, MIN_800_MV, STEP_25_MV),
>> +       regulator_desc_ldo(12, MIN_800_MV, STEP_50_MV),
>> +       regulator_desc_ldo(13, MIN_800_MV, STEP_50_MV),
>> +       regulator_desc_ldo(14, MIN_800_MV, STEP_50_MV),
>> +       regulator_desc_ldo(15, MIN_800_MV, STEP_50_MV),
>> +       regulator_desc_ldo(16, MIN_800_MV, STEP_50_MV),
>> +       regulator_desc_ldo(17, MIN_800_MV, STEP_50_MV),
>> +       regulator_desc_ldo(18, MIN_800_MV, STEP_50_MV),
>> +       regulator_desc_ldo(19, MIN_800_MV, STEP_50_MV),
>> +       regulator_desc_ldo(20, MIN_800_MV, STEP_50_MV),
>> +       regulator_desc_ldo(21, MIN_800_MV, STEP_50_MV),
>> +       regulator_desc_ldo(22, MIN_800_MV, STEP_25_MV),
>> +       regulator_desc_ldo(23, MIN_800_MV, STEP_25_MV),
>> +       regulator_desc_ldo(24, MIN_800_MV, STEP_50_MV),
>> +       regulator_desc_ldo(25, MIN_800_MV, STEP_50_MV),
>> +       regulator_desc_ldo(26, MIN_800_MV, STEP_50_MV),
>>         regulator_desc_buck1_4(1),
>>         regulator_desc_buck1_4(2),
>>         regulator_desc_buck1_4(3),
>>         regulator_desc_buck1_4(4),
>>         regulator_desc_buck5,
>> -       regulator_desc_buck6_7(6),
>> -       regulator_desc_buck6_7(7),
>> -       regulator_desc_buck8,
>> -       regulator_desc_buck9,
>> -       regulator_desc_buck10,
>> +       regulator_desc_buck6_10(6, MIN_600_MV, STEP_6_25_MV),
>> +       regulator_desc_buck6_10(7, MIN_600_MV, STEP_6_25_MV),
>> +       regulator_desc_buck6_10(8, MIN_800_MV, STEP_12_5_MV),
>> +       regulator_desc_buck6_10(9, MIN_1500_MV, STEP_12_5_MV),
>> +       regulator_desc_buck6_10(10, MIN_1000_MV, STEP_12_5_MV),
>>   };
>>
>>   static int s2mpa01_pmic_probe(struct platform_device *pdev)
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/regulator/s2mpa01.c b/drivers/regulator/s2mpa01.c
index 962c5f1..8073466 100644
--- a/drivers/regulator/s2mpa01.c
+++ b/drivers/regulator/s2mpa01.c
@@ -235,28 +235,14 @@  static struct regulator_ops s2mpa01_buck_ops = {
 	.set_ramp_delay		= s2mpa01_set_ramp_delay,
 };
 
-#define regulator_desc_ldo1(num)	{		\
+#define regulator_desc_ldo(num, min, step)	{	\
 	.name		= "LDO"#num,			\
 	.id		= S2MPA01_LDO##num,		\
 	.ops		= &s2mpa01_ldo_ops,		\
 	.type		= REGULATOR_VOLTAGE,		\
 	.owner		= THIS_MODULE,			\
-	.min_uV		= MIN_800_MV,			\
-	.uV_step	= STEP_50_MV,			\
-	.n_voltages	= S2MPA01_LDO_N_VOLTAGES,	\
-	.vsel_reg	= S2MPA01_REG_L1CTRL + num - 1,	\
-	.vsel_mask	= S2MPA01_LDO_VSEL_MASK,	\
-	.enable_reg	= S2MPA01_REG_L1CTRL + num - 1,	\
-	.enable_mask	= S2MPA01_ENABLE_MASK		\
-}
-#define regulator_desc_ldo2(num)	{		\
-	.name		= "LDO"#num,			\
-	.id		= S2MPA01_LDO##num,		\
-	.ops		= &s2mpa01_ldo_ops,		\
-	.type		= REGULATOR_VOLTAGE,		\
-	.owner		= THIS_MODULE,			\
-	.min_uV		= MIN_800_MV,			\
-	.uV_step	= STEP_25_MV,			\
+	.min_uV		= min,				\
+	.uV_step	= step,				\
 	.n_voltages	= S2MPA01_LDO_N_VOLTAGES,	\
 	.vsel_reg	= S2MPA01_REG_L1CTRL + num - 1,	\
 	.vsel_mask	= S2MPA01_LDO_VSEL_MASK,	\
@@ -296,14 +282,14 @@  static struct regulator_ops s2mpa01_buck_ops = {
 	.enable_mask	= S2MPA01_ENABLE_MASK			\
 }
 
-#define regulator_desc_buck6_7(num)	{			\
+#define regulator_desc_buck6_10(num, min, step)	{		\
 	.name		= "BUCK"#num,				\
 	.id		= S2MPA01_BUCK##num,			\
 	.ops		= &s2mpa01_buck_ops,			\
 	.type		= REGULATOR_VOLTAGE,			\
 	.owner		= THIS_MODULE,				\
-	.min_uV		= MIN_600_MV,				\
-	.uV_step	= STEP_6_25_MV,				\
+	.min_uV		= min,					\
+	.uV_step	= step,					\
 	.n_voltages	= S2MPA01_BUCK_N_VOLTAGES,		\
 	.ramp_delay	= S2MPA01_RAMP_DELAY,			\
 	.vsel_reg	= S2MPA01_REG_B6CTRL2 + (num - 6) * 2,	\
@@ -312,91 +298,43 @@  static struct regulator_ops s2mpa01_buck_ops = {
 	.enable_mask	= S2MPA01_ENABLE_MASK			\
 }
 
-#define regulator_desc_buck8	{				\
-	.name		= "BUCK8",				\
-	.id		= S2MPA01_BUCK8,			\
-	.ops		= &s2mpa01_buck_ops,			\
-	.type		= REGULATOR_VOLTAGE,			\
-	.owner		= THIS_MODULE,				\
-	.min_uV		= MIN_800_MV,				\
-	.uV_step	= STEP_12_5_MV,				\
-	.n_voltages	= S2MPA01_BUCK_N_VOLTAGES,		\
-	.ramp_delay	= S2MPA01_RAMP_DELAY,			\
-	.vsel_reg	= S2MPA01_REG_B8CTRL2,			\
-	.vsel_mask	= S2MPA01_BUCK_VSEL_MASK,		\
-	.enable_reg	= S2MPA01_REG_B8CTRL1,			\
-	.enable_mask	= S2MPA01_ENABLE_MASK			\
-}
-
-#define regulator_desc_buck9	{				\
-	.name		= "BUCK9",				\
-	.id		= S2MPA01_BUCK9,			\
-	.ops		= &s2mpa01_buck_ops,			\
-	.type		= REGULATOR_VOLTAGE,			\
-	.owner		= THIS_MODULE,				\
-	.min_uV		= MIN_1500_MV,				\
-	.uV_step	= STEP_12_5_MV,				\
-	.n_voltages	= S2MPA01_BUCK_N_VOLTAGES,		\
-	.ramp_delay	= S2MPA01_RAMP_DELAY,			\
-	.vsel_reg	= S2MPA01_REG_B9CTRL2,			\
-	.vsel_mask	= S2MPA01_BUCK_VSEL_MASK,		\
-	.enable_reg	= S2MPA01_REG_B9CTRL1,			\
-	.enable_mask	= S2MPA01_ENABLE_MASK			\
-}
-
-#define regulator_desc_buck10	{				\
-	.name		= "BUCK10",				\
-	.id		= S2MPA01_BUCK10,			\
-	.ops		= &s2mpa01_buck_ops,			\
-	.type		= REGULATOR_VOLTAGE,			\
-	.owner		= THIS_MODULE,				\
-	.min_uV		= MIN_1000_MV,				\
-	.uV_step	= STEP_12_5_MV,				\
-	.n_voltages	= S2MPA01_BUCK_N_VOLTAGES,		\
-	.ramp_delay	= S2MPA01_RAMP_DELAY,			\
-	.vsel_reg	= S2MPA01_REG_B10CTRL2,			\
-	.vsel_mask	= S2MPA01_BUCK_VSEL_MASK,		\
-	.enable_reg	= S2MPA01_REG_B10CTRL1,			\
-	.enable_mask	= S2MPA01_ENABLE_MASK			\
-}
-
 static struct regulator_desc regulators[] = {
-	regulator_desc_ldo2(1),
-	regulator_desc_ldo1(2),
-	regulator_desc_ldo1(3),
-	regulator_desc_ldo1(4),
-	regulator_desc_ldo1(5),
-	regulator_desc_ldo2(6),
-	regulator_desc_ldo1(7),
-	regulator_desc_ldo1(8),
-	regulator_desc_ldo1(9),
-	regulator_desc_ldo1(10),
-	regulator_desc_ldo2(11),
-	regulator_desc_ldo1(12),
-	regulator_desc_ldo1(13),
-	regulator_desc_ldo1(14),
-	regulator_desc_ldo1(15),
-	regulator_desc_ldo1(16),
-	regulator_desc_ldo1(17),
-	regulator_desc_ldo1(18),
-	regulator_desc_ldo1(19),
-	regulator_desc_ldo1(20),
-	regulator_desc_ldo1(21),
-	regulator_desc_ldo2(22),
-	regulator_desc_ldo2(23),
-	regulator_desc_ldo1(24),
-	regulator_desc_ldo1(25),
-	regulator_desc_ldo1(26),
+	regulator_desc_ldo(1, MIN_800_MV, STEP_25_MV),
+	regulator_desc_ldo(2, MIN_800_MV, STEP_50_MV),
+	regulator_desc_ldo(3, MIN_800_MV, STEP_50_MV),
+	regulator_desc_ldo(4, MIN_800_MV, STEP_50_MV),
+	regulator_desc_ldo(5, MIN_800_MV, STEP_50_MV),
+	regulator_desc_ldo(6, MIN_800_MV, STEP_25_MV),
+	regulator_desc_ldo(7, MIN_800_MV, STEP_50_MV),
+	regulator_desc_ldo(8, MIN_800_MV, STEP_50_MV),
+	regulator_desc_ldo(9, MIN_800_MV, STEP_50_MV),
+	regulator_desc_ldo(10, MIN_800_MV, STEP_50_MV),
+	regulator_desc_ldo(11, MIN_800_MV, STEP_25_MV),
+	regulator_desc_ldo(12, MIN_800_MV, STEP_50_MV),
+	regulator_desc_ldo(13, MIN_800_MV, STEP_50_MV),
+	regulator_desc_ldo(14, MIN_800_MV, STEP_50_MV),
+	regulator_desc_ldo(15, MIN_800_MV, STEP_50_MV),
+	regulator_desc_ldo(16, MIN_800_MV, STEP_50_MV),
+	regulator_desc_ldo(17, MIN_800_MV, STEP_50_MV),
+	regulator_desc_ldo(18, MIN_800_MV, STEP_50_MV),
+	regulator_desc_ldo(19, MIN_800_MV, STEP_50_MV),
+	regulator_desc_ldo(20, MIN_800_MV, STEP_50_MV),
+	regulator_desc_ldo(21, MIN_800_MV, STEP_50_MV),
+	regulator_desc_ldo(22, MIN_800_MV, STEP_25_MV),
+	regulator_desc_ldo(23, MIN_800_MV, STEP_25_MV),
+	regulator_desc_ldo(24, MIN_800_MV, STEP_50_MV),
+	regulator_desc_ldo(25, MIN_800_MV, STEP_50_MV),
+	regulator_desc_ldo(26, MIN_800_MV, STEP_50_MV),
 	regulator_desc_buck1_4(1),
 	regulator_desc_buck1_4(2),
 	regulator_desc_buck1_4(3),
 	regulator_desc_buck1_4(4),
 	regulator_desc_buck5,
-	regulator_desc_buck6_7(6),
-	regulator_desc_buck6_7(7),
-	regulator_desc_buck8,
-	regulator_desc_buck9,
-	regulator_desc_buck10,
+	regulator_desc_buck6_10(6, MIN_600_MV, STEP_6_25_MV),
+	regulator_desc_buck6_10(7, MIN_600_MV, STEP_6_25_MV),
+	regulator_desc_buck6_10(8, MIN_800_MV, STEP_12_5_MV),
+	regulator_desc_buck6_10(9, MIN_1500_MV, STEP_12_5_MV),
+	regulator_desc_buck6_10(10, MIN_1000_MV, STEP_12_5_MV),
 };
 
 static int s2mpa01_pmic_probe(struct platform_device *pdev)