Hi Sam,

On 06/06/2026 10:47, Sam Day via B4 Relay wrote:
> From: Sam Day <[email protected]>
> 
> This is to be used in conjunction with PSHOLD sysreset driver.
> 
> This code manages the RST_CTL register block, which PM8916 uses to
> decide what action to take when PS_HOLD is de-asserted.
> 
> Signed-off-by: Sam Day <[email protected]>
> ---
>  drivers/misc/pm8916_pon.c | 82 
> +++++++++++++++++++++++++++++++++++++++++++++++
>  include/pm8916_pon.h      | 18 +++++++++++
>  2 files changed, 100 insertions(+)
> 
> diff --git a/drivers/misc/pm8916_pon.c b/drivers/misc/pm8916_pon.c
> index d65a7cab459..4cd8921a08a 100644
> --- a/drivers/misc/pm8916_pon.c
> +++ b/drivers/misc/pm8916_pon.c
> @@ -10,16 +10,98 @@
>  #include <dm/lists.h>
>  #include <dm/device_compat.h>
>  #include <power/pmic.h>
> +#include <linux/delay.h>
> +#include <sysreset.h>
>  #include "button/qcom-pmic.h"
> +#include "pm8916_pon.h"
>  
>  #define PON_REV2 0x01
>  
> +#define PON_PS_HOLD_RST_CTL 0x5a
> +#define PON_PS_HOLD_RST_CTL2 0x5b
> +
> +#define PON_PS_HOLD_ENABLE BIT(7)
> +
> +#define PON_PS_HOLD_TYPE_MASK GENMASK(3, 0)
> +#define PON_PS_HOLD_TYPE_WARM_RESET 1
> +#define PON_PS_HOLD_TYPE_SHUTDOWN 4
> +#define PON_PS_HOLD_TYPE_HARD_RESET 7
> +
>  struct pm8916_pon_priv {
>       struct udevice *pmic;
>       phys_addr_t base;
>       u32 revision;
>  };
>  
> +int pm8916_pon_set_reboot_type(enum sysreset_t reset_type)
> +{
> +     struct udevice *dev;
> +     struct pm8916_pon_priv *priv;
> +     int ret;
> +     uint pmic_reset_type;
> +     uint enable_reg;
> +
> +     ret = uclass_get_device_by_driver(UCLASS_MISC,
> +                                       DM_DRIVER_GET(pm8916_pon), &dev);
> +     if (ret) {
> +             pr_warn("couldn't find pm8916_pon device: %d\n", ret);

As mentioned, if ret is -ENODEV then we ought to return 0 here (maybe
with a debug print) since that's an expected scenario.

> +             return ret;
> +     }
> +     if (!dev) {
> +             dev_warn(dev, "couldn't find pm8916_pon device\n");

Don't call dev_warn() with a NULL dev.

Is there a case where ret = 0 and dev is null? Or can this be dropped

Thanks and regards,
> +             return -ENODEV;
> +     }
> +
> +     priv = dev_get_priv(dev);
> +
> +     /* PMICs with revision 0 have the enable bit in same register as ctrl */
> +     if (priv->revision == 0)
> +             enable_reg = PON_PS_HOLD_RST_CTL;
> +     else
> +             enable_reg = PON_PS_HOLD_RST_CTL2;
> +
> +     switch (reset_type) {
> +     case SYSRESET_POWER_OFF:
> +             pmic_reset_type = PON_PS_HOLD_TYPE_SHUTDOWN;
> +             break;
> +     case SYSRESET_COLD:
> +     case SYSRESET_POWER:
> +             pmic_reset_type = PON_PS_HOLD_TYPE_HARD_RESET;
> +             break;
> +     default:
> +             pmic_reset_type = PON_PS_HOLD_TYPE_WARM_RESET;
> +     }
> +
> +     dev_dbg(dev, "Setting PON reboot type: %x\n", pmic_reset_type);
> +
> +     ret = pmic_clrsetbits(priv->pmic, priv->base + enable_reg,
> +                           PON_PS_HOLD_ENABLE, 0);
> +     if (ret) {
> +             dev_warn(dev, "clear PON_PS_HOLD_ENABLE failed: %d\n", ret);
> +             return ret;
> +     }
> +
> +     /* Delay needed for disable to kick in. */
> +     /* Kernel uses usleep_range(100, 1000), lk2nd waits 300usec */
> +     udelay(300);
> +
> +     ret = pmic_clrsetbits(priv->pmic, priv->base + PON_PS_HOLD_RST_CTL,
> +                           PON_PS_HOLD_TYPE_MASK, pmic_reset_type);
> +     if (ret) {
> +             dev_warn(dev, "RST_CTL set failed: %d\n", ret);
> +             return ret;
> +     }
> +
> +     ret = pmic_clrsetbits(priv->pmic, priv->base + enable_reg, 0,
> +                           PON_PS_HOLD_ENABLE);
> +     if (ret) {
> +             dev_warn(dev, "PON_PS_HOLD_ENABLE failed: %d\n", ret);
> +             return ret;
> +     }
> +
> +     return 0;
> +}
> +
>  static int pm8916_pon_probe(struct udevice *dev)
>  {
>       int ret;
> diff --git a/include/pm8916_pon.h b/include/pm8916_pon.h
> new file mode 100644
> index 00000000000..cc553d43713
> --- /dev/null
> +++ b/include/pm8916_pon.h
> @@ -0,0 +1,18 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +
> +#ifndef __PM8916_PON_H__
> +#define __PM8916_PON_H__
> +
> +#include <linux/errno.h>
> +#include <sysreset.h>
> +
> +#if CONFIG_IS_ENABLED(PM8916_PON)
> +int pm8916_pon_set_reboot_type(enum sysreset_t reset_type);
> +#else
> +static inline int pm8916_pon_set_reboot_type(enum sysreset_t reset_type)
> +{
> +     return -ENOSYS;
> +}
> +#endif
> +
> +#endif /* __PM8916_PON_H__ */
> 

-- 
// Casey (she/her)

Reply via email to