From: Bjorn Andersson <bjorn.andersson@linaro.org>
To: Axel Lin <axel.lin@ingics.com>
Cc: Mark Brown <broonie@kernel.org>, Andy Gross <agross@kernel.org>,
"Angelo G . Del Regno" <kholk11@gmail.com>,
Liam Girdwood <lgirdwood@gmail.com>,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH] regulator: qcom_spmi: Improve readability for setting up enable/mode pin control
Date: Fri, 31 Jul 2020 23:12:27 -0700 [thread overview]
Message-ID: <20200801061227.GG61202@yoga> (raw)
In-Reply-To: <20200801054820.134859-1-axel.lin@ingics.com>
On Fri 31 Jul 22:48 PDT 2020, Axel Lin wrote:
> By checking data->pin_ctrl_enable / data->pin_ctrl_hpm flags first, then
> use switch-case to improve readability.
>
Nice!
Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Regards,
Bjorn
> Signed-off-by: Axel Lin <axel.lin@ingics.com>
> ---
> drivers/regulator/qcom_spmi-regulator.c | 70 ++++++++++++-------------
> 1 file changed, 34 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/regulator/qcom_spmi-regulator.c b/drivers/regulator/qcom_spmi-regulator.c
> index 5ee7c5305d95..05080483fe1b 100644
> --- a/drivers/regulator/qcom_spmi-regulator.c
> +++ b/drivers/regulator/qcom_spmi-regulator.c
> @@ -1633,45 +1633,43 @@ static int spmi_regulator_init_registers(struct spmi_regulator *vreg,
> return ret;
>
> /* Set up enable pin control. */
> - if ((type == SPMI_REGULATOR_LOGICAL_TYPE_SMPS
> - || type == SPMI_REGULATOR_LOGICAL_TYPE_LDO
> - || type == SPMI_REGULATOR_LOGICAL_TYPE_VS)
> - && !(data->pin_ctrl_enable
> - & SPMI_REGULATOR_PIN_CTRL_ENABLE_HW_DEFAULT)) {
> - ctrl_reg[SPMI_COMMON_IDX_ENABLE] &=
> - ~SPMI_COMMON_ENABLE_FOLLOW_ALL_MASK;
> - ctrl_reg[SPMI_COMMON_IDX_ENABLE] |=
> - data->pin_ctrl_enable & SPMI_COMMON_ENABLE_FOLLOW_ALL_MASK;
> + if (!(data->pin_ctrl_enable & SPMI_REGULATOR_PIN_CTRL_ENABLE_HW_DEFAULT)) {
> + switch (type) {
> + case SPMI_REGULATOR_LOGICAL_TYPE_SMPS:
> + case SPMI_REGULATOR_LOGICAL_TYPE_LDO:
> + case SPMI_REGULATOR_LOGICAL_TYPE_VS:
> + ctrl_reg[SPMI_COMMON_IDX_ENABLE] &=
> + ~SPMI_COMMON_ENABLE_FOLLOW_ALL_MASK;
> + ctrl_reg[SPMI_COMMON_IDX_ENABLE] |=
> + data->pin_ctrl_enable & SPMI_COMMON_ENABLE_FOLLOW_ALL_MASK;
> + break;
> + default:
> + break;
> + }
> }
>
> /* Set up mode pin control. */
> - if ((type == SPMI_REGULATOR_LOGICAL_TYPE_SMPS
> - || type == SPMI_REGULATOR_LOGICAL_TYPE_LDO)
> - && !(data->pin_ctrl_hpm
> - & SPMI_REGULATOR_PIN_CTRL_HPM_HW_DEFAULT)) {
> - ctrl_reg[SPMI_COMMON_IDX_MODE] &=
> - ~SPMI_COMMON_MODE_FOLLOW_ALL_MASK;
> - ctrl_reg[SPMI_COMMON_IDX_MODE] |=
> - data->pin_ctrl_hpm & SPMI_COMMON_MODE_FOLLOW_ALL_MASK;
> - }
> -
> - if (type == SPMI_REGULATOR_LOGICAL_TYPE_VS
> - && !(data->pin_ctrl_hpm & SPMI_REGULATOR_PIN_CTRL_HPM_HW_DEFAULT)) {
> - ctrl_reg[SPMI_COMMON_IDX_MODE] &=
> - ~SPMI_COMMON_MODE_FOLLOW_AWAKE_MASK;
> - ctrl_reg[SPMI_COMMON_IDX_MODE] |=
> - data->pin_ctrl_hpm & SPMI_COMMON_MODE_FOLLOW_AWAKE_MASK;
> - }
> -
> - if ((type == SPMI_REGULATOR_LOGICAL_TYPE_ULT_LO_SMPS
> - || type == SPMI_REGULATOR_LOGICAL_TYPE_ULT_HO_SMPS
> - || type == SPMI_REGULATOR_LOGICAL_TYPE_ULT_LDO)
> - && !(data->pin_ctrl_hpm
> - & SPMI_REGULATOR_PIN_CTRL_HPM_HW_DEFAULT)) {
> - ctrl_reg[SPMI_COMMON_IDX_MODE] &=
> - ~SPMI_COMMON_MODE_FOLLOW_AWAKE_MASK;
> - ctrl_reg[SPMI_COMMON_IDX_MODE] |=
> - data->pin_ctrl_hpm & SPMI_COMMON_MODE_FOLLOW_AWAKE_MASK;
> + if (!(data->pin_ctrl_hpm & SPMI_REGULATOR_PIN_CTRL_HPM_HW_DEFAULT)) {
> + switch (type) {
> + case SPMI_REGULATOR_LOGICAL_TYPE_SMPS:
> + case SPMI_REGULATOR_LOGICAL_TYPE_LDO:
> + ctrl_reg[SPMI_COMMON_IDX_MODE] &=
> + ~SPMI_COMMON_MODE_FOLLOW_ALL_MASK;
> + ctrl_reg[SPMI_COMMON_IDX_MODE] |=
> + data->pin_ctrl_hpm & SPMI_COMMON_MODE_FOLLOW_ALL_MASK;
> + break;
> + case SPMI_REGULATOR_LOGICAL_TYPE_VS:
> + case SPMI_REGULATOR_LOGICAL_TYPE_ULT_LO_SMPS:
> + case SPMI_REGULATOR_LOGICAL_TYPE_ULT_HO_SMPS:
> + case SPMI_REGULATOR_LOGICAL_TYPE_ULT_LDO:
> + ctrl_reg[SPMI_COMMON_IDX_MODE] &=
> + ~SPMI_COMMON_MODE_FOLLOW_AWAKE_MASK;
> + ctrl_reg[SPMI_COMMON_IDX_MODE] |=
> + data->pin_ctrl_hpm & SPMI_COMMON_MODE_FOLLOW_AWAKE_MASK;
> + break;
> + default:
> + break;
> + }
> }
>
> /* Write back any control register values that were modified. */
> --
> 2.25.1
>
next prev parent reply other threads:[~2020-08-01 6:14 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-08-01 5:48 [PATCH] regulator: qcom_spmi: Improve readability for setting up enable/mode pin control Axel Lin
2020-08-01 6:12 ` Bjorn Andersson [this message]
2020-08-18 16:56 ` Mark Brown
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=20200801061227.GG61202@yoga \
--to=bjorn.andersson@linaro.org \
--cc=agross@kernel.org \
--cc=axel.lin@ingics.com \
--cc=broonie@kernel.org \
--cc=kholk11@gmail.com \
--cc=lgirdwood@gmail.com \
--cc=linux-kernel@vger.kernel.org \
/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