From: Krzysztof Kozlowski <k.kozlowski@samsung.com>
To: Amit Daniel Kachhap <amit.daniel@samsung.com>,
Sangbeom Kim <sbkim73@samsung.com>,
Liam Girdwood <lgirdwood@gmail.com>,
Mark Brown <broonie@kernel.org>
Cc: linux-samsung-soc@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/3] regulator: s2mpa01: Optimize the regulator description macro
Date: Mon, 14 Jul 2014 10:49:56 +0200 [thread overview]
Message-ID: <53C399B4.4040207@samsung.com> (raw)
In-Reply-To: <1404822480-31525-2-git-send-email-amit.daniel@samsung.com>
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)
>
next prev parent reply other threads:[~2014-07-14 8:50 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-07-08 12:27 [PATCH 1/3] regulator: s2mpxxx: Move regulator min/step voltages in common place Amit Daniel Kachhap
2014-07-08 12:27 ` [PATCH 2/3] regulator: s2mpa01: Optimize the regulator description macro Amit Daniel Kachhap
2014-07-10 3:24 ` amit daniel kachhap
2014-07-10 7:35 ` Lee Jones
2014-07-10 10:10 ` amit daniel kachhap
2014-07-14 8:49 ` Krzysztof Kozlowski [this message]
2014-07-15 9:51 ` amit daniel kachhap
2014-07-08 12:28 ` [PATCH 3/3] regulator: s2mps11: " Amit Daniel Kachhap
2014-07-10 3:25 ` amit daniel kachhap
2014-07-09 9:25 ` [PATCH 1/3] regulator: s2mpxxx: Move regulator min/step voltages in common place Mark Brown
2014-07-10 3:22 ` amit daniel kachhap
2014-07-14 8:53 ` Krzysztof Kozlowski
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=53C399B4.4040207@samsung.com \
--to=k.kozlowski@samsung.com \
--cc=amit.daniel@samsung.com \
--cc=broonie@kernel.org \
--cc=lgirdwood@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-samsung-soc@vger.kernel.org \
--cc=sbkim73@samsung.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.