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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).