[v1] firmware: exynos-acpm: fix PMIC returned errno

[PATCH] firmware: exynos-acpm: fix PMIC returned errno

Posted by Tudor Ambarus 1 month, 1 week ago

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@linaro.org>
Closes: https://lore.kernel.org/linux-input/aElHlTApXj-W_o1r@stanley.mountain/
Fixes: a88927b534ba ("firmware: add Exynos ACPM protocol driver")
Cc: stable@vger.kernel.org
Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.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 {
+	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] = {
+	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)
+{
+	if (errno >= ACPM_PMIC_SUCCESS && errno < ACPM_PMIC_ERR_MAX)
+		return acpm_pmic_linux_errmap[errno];
+	return -EIO;
+}
+
 static inline u32 acpm_pmic_set_bulk(u32 data, unsigned int i)
 {
 	return (data & ACPM_PMIC_BULK_MASK) << (ACPM_PMIC_BULK_SHIFT * i);
@@ -79,7 +100,8 @@ int acpm_pmic_read_reg(const struct acpm_handle *handle,
 
 	*buf = FIELD_GET(ACPM_PMIC_VALUE, xfer.rxd[1]);
 
-	return FIELD_GET(ACPM_PMIC_RETURN, xfer.rxd[1]);
+	return acpm_pmic_to_linux_errno(FIELD_GET(ACPM_PMIC_RETURN,
+						  xfer.rxd[1]));
 }
 
 static void acpm_pmic_init_bulk_read_cmd(u32 cmd[4], u8 type, u8 reg, u8 chan,
@@ -110,7 +132,8 @@ int acpm_pmic_bulk_read(const struct acpm_handle *handle,
 	if (ret)
 		return ret;
 
-	ret = FIELD_GET(ACPM_PMIC_RETURN, xfer.rxd[1]);
+	ret = acpm_pmic_to_linux_errno(FIELD_GET(ACPM_PMIC_RETURN,
+						 xfer.rxd[1]));
 	if (ret)
 		return ret;
 
@@ -150,7 +173,8 @@ int acpm_pmic_write_reg(const struct acpm_handle *handle,
 	if (ret)
 		return ret;
 
-	return FIELD_GET(ACPM_PMIC_RETURN, xfer.rxd[1]);
+	return acpm_pmic_to_linux_errno(FIELD_GET(ACPM_PMIC_RETURN,
+						  xfer.rxd[1]));
 }
 
 static void acpm_pmic_init_bulk_write_cmd(u32 cmd[4], u8 type, u8 reg, u8 chan,
@@ -190,7 +214,8 @@ int acpm_pmic_bulk_write(const struct acpm_handle *handle,
 	if (ret)
 		return ret;
 
-	return FIELD_GET(ACPM_PMIC_RETURN, xfer.rxd[1]);
+	return acpm_pmic_to_linux_errno(FIELD_GET(ACPM_PMIC_RETURN,
+						  xfer.rxd[1]));
 }
 
 static void acpm_pmic_init_update_cmd(u32 cmd[4], u8 type, u8 reg, u8 chan,
@@ -220,5 +245,6 @@ int acpm_pmic_update_reg(const struct acpm_handle *handle,
 	if (ret)
 		return ret;
 
-	return FIELD_GET(ACPM_PMIC_RETURN, xfer.rxd[1]);
+	return acpm_pmic_to_linux_errno(FIELD_GET(ACPM_PMIC_RETURN,
+						  xfer.rxd[1]));
 }

---
base-commit: c17b750b3ad9f45f2b6f7e6f7f4679844244f0b9
change-id: 20250820-acpm-pmix-fix-errno-92b3e6e17d1b

Best regards,
-- 
Tudor Ambarus <tudor.ambarus@linaro.org>

Re: [PATCH] firmware: exynos-acpm: fix PMIC returned errno

Posted by Krzysztof Kozlowski 1 month, 1 week ago

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@linaro.org>
> Closes: https://lore.kernel.org/linux-input/aElHlTApXj-W_o1r@stanley.mountain/
> Fixes: a88927b534ba ("firmware: add Exynos ACPM protocol driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.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.


> +{
> +	if (errno >= ACPM_PMIC_SUCCESS && errno < ACPM_PMIC_ERR_MAX)
> +		return acpm_pmic_linux_errmap[errno];
> +	return -EIO;
> +}



Best regards,
Krzysztof

Re: [PATCH] firmware: exynos-acpm: fix PMIC returned errno

Posted by Tudor Ambarus 1 month, 1 week ago


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@linaro.org>
>> Closes: https://lore.kernel.org/linux-input/aElHlTApXj-W_o1r@stanley.mountain/
>> Fixes: a88927b534ba ("firmware: add Exynos ACPM protocol driver")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.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