[bug report] firmware: add Exynos ACPM protocol driver

[bug report] firmware: add Exynos ACPM protocol driver

[Dan Carpenter]

Hello Tudor Ambarus,

Commit a88927b534ba ("firmware: add Exynos ACPM protocol driver")
from Feb 13, 2025 (linux-next), leads to the following Smatch static
checker warning:

	drivers/input/misc/tps65219-pwrbutton.c:129 tps65219_pb_remove()
	warn: passing positive error code '1-255' to 'ERR_PTR'

drivers/input/misc/tps65219-pwrbutton.c
    120 static void tps65219_pb_remove(struct platform_device *pdev)
    121 {
    122         struct tps65219 *tps = dev_get_drvdata(pdev->dev.parent);
    123         int ret;
    124 
    125         /* Disable interrupt for the pushbutton */
    126         ret = regmap_set_bits(tps->regmap, TPS65219_REG_MASK_CONFIG,
    127                               TPS65219_REG_MASK_INT_FOR_PB_MASK);
    128         if (ret)
--> 129                 dev_warn(&pdev->dev, "Failed to disable irq (%pe)\n", ERR_PTR(ret));
    130 }

The problem is:

drivers/firmware/samsung/exynos-acpm-pmic.c
   208  int acpm_pmic_update_reg(const struct acpm_handle *handle,
   209                           unsigned int acpm_chan_id, u8 type, u8 reg, u8 chan,
   210                           u8 value, u8 mask)
   211  {
   212          struct acpm_xfer xfer;
   213          u32 cmd[4] = {0};
   214          int ret;
   215  
   216          acpm_pmic_init_update_cmd(cmd, type, reg, chan, value, mask);
   217          acpm_pmic_set_xfer(&xfer, cmd, sizeof(cmd), acpm_chan_id);
   218  
   219          ret = acpm_do_xfer(handle, &xfer);
   220          if (ret)
   221                  return ret;
   222  
   223          return FIELD_GET(ACPM_PMIC_RETURN, xfer.rxd[1]);
                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
This acpm_pmic_update_reg() is called by sec_pmic_acpm_bus_reg_update_bits()
via the (struct acpm_pmic_ops)->update_reg pointer.  This field get is
returning a u8 value but I'm pretty sure we should be returning either
zero or negative error codes.

   224  }

regards,
dan carpenter

Re: [bug report] firmware: add Exynos ACPM protocol driver

[Tudor Ambarus]

On 6/11/25 10:08 AM, Dan Carpenter wrote:
> Hello Tudor Ambarus,
> 
> Commit a88927b534ba ("firmware: add Exynos ACPM protocol driver")
> from Feb 13, 2025 (linux-next), leads to the following Smatch static
> checker warning:
> 
> 	drivers/input/misc/tps65219-pwrbutton.c:129 tps65219_pb_remove()
> 	warn: passing positive error code '1-255' to 'ERR_PTR'
> 
> drivers/input/misc/tps65219-pwrbutton.c
>     120 static void tps65219_pb_remove(struct platform_device *pdev)
>     121 {
>     122         struct tps65219 *tps = dev_get_drvdata(pdev->dev.parent);
>     123         int ret;
>     124 
>     125         /* Disable interrupt for the pushbutton */
>     126         ret = regmap_set_bits(tps->regmap, TPS65219_REG_MASK_CONFIG,
>     127                               TPS65219_REG_MASK_INT_FOR_PB_MASK);
>     128         if (ret)
> --> 129                 dev_warn(&pdev->dev, "Failed to disable irq (%pe)\n", ERR_PTR(ret));
>     130 }
> 
> The problem is:
> 
> drivers/firmware/samsung/exynos-acpm-pmic.c
>    208  int acpm_pmic_update_reg(const struct acpm_handle *handle,
>    209                           unsigned int acpm_chan_id, u8 type, u8 reg, u8 chan,
>    210                           u8 value, u8 mask)
>    211  {
>    212          struct acpm_xfer xfer;
>    213          u32 cmd[4] = {0};
>    214          int ret;
>    215  
>    216          acpm_pmic_init_update_cmd(cmd, type, reg, chan, value, mask);
>    217          acpm_pmic_set_xfer(&xfer, cmd, sizeof(cmd), acpm_chan_id);
>    218  
>    219          ret = acpm_do_xfer(handle, &xfer);
>    220          if (ret)
>    221                  return ret;
>    222  
>    223          return FIELD_GET(ACPM_PMIC_RETURN, xfer.rxd[1]);
>                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> This acpm_pmic_update_reg() is called by sec_pmic_acpm_bus_reg_update_bits()
> via the (struct acpm_pmic_ops)->update_reg pointer.  This field get is
> returning a u8 value but I'm pretty sure we should be returning either
> zero or negative error codes.
> 

oh, yes, thanks for the report. The concern is valid, I'm adding it to
my todo list.

Cheers,
ta