[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <b84dbc22-62d6-4a63-9b53-80e939c36e38@linaro.org>
Date: Mon, 25 Aug 2025 13:33:33 +0100
From: Tudor Ambarus <tudor.ambarus@...aro.org>
To: Krzysztof Kozlowski <krzk@...nel.org>,
Alim Akhtar <alim.akhtar@...sung.com>
Cc: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
linux-kernel@...r.kernel.org, linux-samsung-soc@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, peter.griffin@...aro.org,
andre.draszik@...aro.org, willmcvicker@...gle.com, kernel-team@...roid.com,
Dan Carpenter <dan.carpenter@...aro.org>, stable@...r.kernel.org
Subject: Re: [PATCH] firmware: exynos-acpm: fix PMIC returned errno
On 8/24/25 5:50 PM, Krzysztof Kozlowski wrote:
> On 21/08/2025 15:28, Tudor Ambarus wrote:
>> ACPM PMIC command handlers returned a u8 value when they should
>> have returned either zero or negative error codes.
>> Translate the APM PMIC errno to linux errno.
>>
>> Reported-by: Dan Carpenter <dan.carpenter@...aro.org>
>> Closes: https://lore.kernel.org/linux-input/aElHlTApXj-W_o1r@stanley.mountain/
>> Fixes: a88927b534ba ("firmware: add Exynos ACPM protocol driver")
>> Cc: stable@...r.kernel.org
>> Signed-off-by: Tudor Ambarus <tudor.ambarus@...aro.org>
>> ---
>> drivers/firmware/samsung/exynos-acpm-pmic.c | 36 +++++++++++++++++++++++++----
>> 1 file changed, 31 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/firmware/samsung/exynos-acpm-pmic.c b/drivers/firmware/samsung/exynos-acpm-pmic.c
>> index 39b33a356ebd240506b6390163229a70a2d1fe68..a355ee194027c09431f275f0fd296f45652af536 100644
>> --- a/drivers/firmware/samsung/exynos-acpm-pmic.c
>> +++ b/drivers/firmware/samsung/exynos-acpm-pmic.c
>> @@ -5,6 +5,7 @@
>> * Copyright 2024 Linaro Ltd.
>> */
>> #include <linux/bitfield.h>
>> +#include <linux/errno.h>
>> #include <linux/firmware/samsung/exynos-acpm-protocol.h>
>> #include <linux/ktime.h>
>> #include <linux/types.h>
>> @@ -33,6 +34,26 @@ enum exynos_acpm_pmic_func {
>> ACPM_PMIC_BULK_WRITE,
>> };
>>
>> +enum acpm_pmic_error_codes {
>
> This enum is not used. Size is not needed and you can just use
> designated initializers in the array.
>
>> + ACPM_PMIC_SUCCESS = 0,
>> + ACPM_PMIC_ERR_READ = 1,
>> + ACPM_PMIC_ERR_WRITE = 2,
>> + ACPM_PMIC_ERR_MAX
>> +};
>> +
>> +static int acpm_pmic_linux_errmap[ACPM_PMIC_ERR_MAX] = {
>
> const
>
>> + 0, /* ACPM_PMIC_SUCCESS */
>> + -EACCES, /* Read register can't be accessed or issues to access it. */
>> + -EACCES, /* Write register can't be accessed or issues to access it. */
>> +};
>> +
>> +static inline int acpm_pmic_to_linux_errno(int errno)
>
> Drop inline
>
> s/int errno/int err/
> (or code?)
>
> errno is just too similar to Linux errno.
>
I agree with all the comments, will address them. Thanks for the review!
ta
Powered by blists - more mailing lists