linux-kernel - Re: [PATCH] regulator: qcom_spmi: Improve readability for setting up enable/mode pin control

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 31 Jul 2020 23:12:27 -0700
From:   Bjorn Andersson <bjorn.andersson@...aro.org>
To:     Axel Lin <axel.lin@...ics.com>
Cc:     Mark Brown <broonie@...nel.org>, Andy Gross <agross@...nel.org>,
        "Angelo G . Del Regno" <kholk11@...il.com>,
        Liam Girdwood <lgirdwood@...il.com>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] regulator: qcom_spmi: Improve readability for setting up
 enable/mode pin control

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@...aro.org>

Regards,
Bjorn

> Signed-off-by: Axel Lin <axel.lin@...ics.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
>