[PATCH] power: regulator: Fix incorrect use of binary and

[PATCH] power: regulator: Fix incorrect use of binary and

[Andrew Goodbody]

In regulator_list_autoset there is a test for ret being non-zero and
error being zero but it uses the binary '&' instead of the logical '&&'
which could well lead to unexpected results. Correct this to use the
logical '&&' instead.

This issue found by Smatch.

Signed-off-by: Andrew Goodbody <andrew.goodbody@linaro.org>
---
 drivers/power/regulator/regulator-uclass.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/power/regulator/regulator-uclass.c b/drivers/power/regulator/regulator-uclass.c
index 09567eb9dbb..fbd1f69ac72 100644
--- a/drivers/power/regulator/regulator-uclass.c
+++ b/drivers/power/regulator/regulator-uclass.c
@@ -389,7 +389,7 @@ int regulator_list_autoset(const char *list_platname[],
 		ret = regulator_autoset_by_name(list_platname[i], &dev);
 		if (ret != -EMEDIUMTYPE && verbose)
 			regulator_show(dev, ret);
-		if (ret & !error)
+		if (ret && !error)
 			error = ret;
 
 		if (list_devp)

---
base-commit: 7027b445cc0bfb86204ecb1f1fe596f5895048d9
change-id: 20250703-regulator_uclass_fix-d9457eff7fe9

Best regards,
-- 
Andrew Goodbody <andrew.goodbody@linaro.org>

Re: [PATCH] power: regulator: Fix incorrect use of binary and

[Quentin Schulz]

Hi Andrew,

On 7/3/25 1:53 PM, Andrew Goodbody wrote:
> In regulator_list_autoset there is a test for ret being non-zero and
> error being zero but it uses the binary '&' instead of the logical '&&'
> which could well lead to unexpected results. Correct this to use the
> logical '&&' instead.
> 
> This issue found by Smatch.
> 
> Signed-off-by: Andrew Goodbody <andrew.goodbody@linaro.org>

That seems like what was intended according to the documentation found 
in include/power/regulator.h

"""
Return: 0 on successfully setup of all list entries, otherwise first error.
"""

Therefore:

Reviewed-by: Quentin Schulz <quentin.schulz@cherry.de>

Thanks!
Quentin

Re: [PATCH] power: regulator: Fix incorrect use of binary and

[Andrew Goodbody]

ping?

On 03/07/2025 12:53, Andrew Goodbody wrote:
> In regulator_list_autoset there is a test for ret being non-zero and
> error being zero but it uses the binary '&' instead of the logical '&&'
> which could well lead to unexpected results. Correct this to use the
> logical '&&' instead.
> 
> This issue found by Smatch.
> 
> Signed-off-by: Andrew Goodbody <andrew.goodbody@linaro.org>
> ---
>   drivers/power/regulator/regulator-uclass.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/power/regulator/regulator-uclass.c b/drivers/power/regulator/regulator-uclass.c
> index 09567eb9dbb..fbd1f69ac72 100644
> --- a/drivers/power/regulator/regulator-uclass.c
> +++ b/drivers/power/regulator/regulator-uclass.c
> @@ -389,7 +389,7 @@ int regulator_list_autoset(const char *list_platname[],
>   		ret = regulator_autoset_by_name(list_platname[i], &dev);
>   		if (ret != -EMEDIUMTYPE && verbose)
>   			regulator_show(dev, ret);
> -		if (ret & !error)
> +		if (ret && !error)
>   			error = ret;
>   
>   		if (list_devp)
> 
> ---
> base-commit: 7027b445cc0bfb86204ecb1f1fe596f5895048d9
> change-id: 20250703-regulator_uclass_fix-d9457eff7fe9
> 
> Best regards,

Re: [PATCH] power: regulator: Fix incorrect use of binary and

[Andrew Goodbody]

On 14/07/2025 13:31, Quentin Schulz wrote:
> Hi Andrew,
> 
> On 7/3/25 1:53 PM, Andrew Goodbody wrote:
>> In regulator_list_autoset there is a test for ret being non-zero and
>> error being zero but it uses the binary '&' instead of the logical '&&'
>> which could well lead to unexpected results. Correct this to use the
>> logical '&&' instead.
>>
>> This issue found by Smatch.
>>
>> Signed-off-by: Andrew Goodbody <andrew.goodbody@linaro.org>
> 
> That seems like what was intended according to the documentation found 
> in include/power/regulator.h
> 
> """
> Return: 0 on successfully setup of all list entries, otherwise first error.
> """
> 
> Therefore:
> 
> Reviewed-by: Quentin Schulz <quentin.schulz@cherry.de>
> 
> Thanks!
> Quentin

Is there anything else needed before this can be merged please?

Thanks,
Andrew

Re: [PATCH] power: regulator: Fix incorrect use of binary and

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1246 bytes --]

On Thu, Jul 03, 2025 at 12:53:42PM +0100, Andrew Goodbody wrote:

> In regulator_list_autoset there is a test for ret being non-zero and
> error being zero but it uses the binary '&' instead of the logical '&&'
> which could well lead to unexpected results. Correct this to use the
> logical '&&' instead.
> 
> This issue found by Smatch.
> 
> Signed-off-by: Andrew Goodbody <andrew.goodbody@linaro.org>
> ---
>  drivers/power/regulator/regulator-uclass.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/power/regulator/regulator-uclass.c b/drivers/power/regulator/regulator-uclass.c
> index 09567eb9dbb..fbd1f69ac72 100644
> --- a/drivers/power/regulator/regulator-uclass.c
> +++ b/drivers/power/regulator/regulator-uclass.c
> @@ -389,7 +389,7 @@ int regulator_list_autoset(const char *list_platname[],
>  		ret = regulator_autoset_by_name(list_platname[i], &dev);
>  		if (ret != -EMEDIUMTYPE && verbose)
>  			regulator_show(dev, ret);
> -		if (ret & !error)
> +		if (ret && !error)
>  			error = ret;
>  
>  		if (list_devp)
> 

This leads to the testcase failing:
https://source.denx.de/u-boot/u-boot/-/jobs/1234999#L272
which should be fixed in the same commit.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH] power: regulator: Fix incorrect use of binary and

[Andrew Goodbody]

On 30/08/2025 19:45, Tom Rini wrote:
> On Thu, Jul 03, 2025 at 12:53:42PM +0100, Andrew Goodbody wrote:
> 
>> In regulator_list_autoset there is a test for ret being non-zero and
>> error being zero but it uses the binary '&' instead of the logical '&&'
>> which could well lead to unexpected results. Correct this to use the
>> logical '&&' instead.
>>
>> This issue found by Smatch.
>>
>> Signed-off-by: Andrew Goodbody <andrew.goodbody@linaro.org>
>> ---
>>   drivers/power/regulator/regulator-uclass.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/power/regulator/regulator-uclass.c b/drivers/power/regulator/regulator-uclass.c
>> index 09567eb9dbb..fbd1f69ac72 100644
>> --- a/drivers/power/regulator/regulator-uclass.c
>> +++ b/drivers/power/regulator/regulator-uclass.c
>> @@ -389,7 +389,7 @@ int regulator_list_autoset(const char *list_platname[],
>>   		ret = regulator_autoset_by_name(list_platname[i], &dev);
>>   		if (ret != -EMEDIUMTYPE && verbose)
>>   			regulator_show(dev, ret);
>> -		if (ret & !error)
>> +		if (ret && !error)
>>   			error = ret;
>>   
>>   		if (list_devp)
>>
> 
> This leads to the testcase failing:
> https://source.denx.de/u-boot/u-boot/-/jobs/1234999#L272
> which should be fixed in the same commit.

Ah, sorry I did not pick that up. The old incorrect code would ignore 
all even error returns. A couple of the regulators in the list returned 
-EALREADY (-114) which was quietly ignored. The corrected code picked 
this up and returned it, which I think is wrong in this specific case.

V2 coming to ignore -EALREADY.

Andrew