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)