Hi Sam, On 06/06/2026 10:47, Sam Day via B4 Relay wrote: > From: Sam Day <[email protected]> > > Before de-asserting PS_HOLD, PM8916 PMIC can be informed of what kind of > reset should take place. > > This might seem a little hacky/weird, but it's a byproduct of how the DT > nodes are structured, and the way u-boot handles sysreset.
ahh yeah this is annoying that there's no DT link between these, this is probably the best approach. The way this is implemented right now means a board that doesn't have a pm8916 will get at least a warning and an error on reset due to the pmic device being missing. Could you demote the error here to a dev_dbg() at least for the expected -ENODEV case, and do the same in the previous patch. > > The qcom,pshold node does not belong to the PMIC hierarchy. If U-Boot's > sysreset infra was a little more flexible, we might be able to model the > qcom,pm8916-pon driver as UCLASS_SYSRESET in order to catch the sysreset > type and set the appropriate bits in the PMIC, before chaining to the > pshold driver to de-assert PS_HOLD (which actually triggers the reset > type that was written in the previous step). I think an approach that could work better here would be to have the pm8916-pon driver instead get a reference to the qcom,pshold in its probe function and then have an exported qcom_pshold_register_pmic() function. This way we keep the reset path simple and avoid the unnecessary device lookup on platforms that don't have pm8916. That said, I'm happy to defer something like that to a future optimisation and get this merged more or less as-is. Thanks and kind regards, > > Signed-off-by: Sam Day <[email protected]> > --- > drivers/sysreset/sysreset_qcom-pshold.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/drivers/sysreset/sysreset_qcom-pshold.c > b/drivers/sysreset/sysreset_qcom-pshold.c > index 45290478536..20ddb4ae4ec 100644 > --- a/drivers/sysreset/sysreset_qcom-pshold.c > +++ b/drivers/sysreset/sysreset_qcom-pshold.c > @@ -13,6 +13,8 @@ > #include <sysreset.h> > #include <asm/io.h> > #include <linux/delay.h> > +#include <dm/device_compat.h> > +#include "pm8916_pon.h" > > struct qcom_pshold_priv { > phys_addr_t base; > @@ -22,6 +24,13 @@ static int qcom_pshold_request(struct udevice *dev, enum > sysreset_t type) > { > struct qcom_pshold_priv *priv = dev_get_priv(dev); > > + if (CONFIG_IS_ENABLED(PM8916_PON)) { > + int ret = pm8916_pon_set_reboot_type(type); > + > + if (ret) > + dev_err(dev, "Failed to set reboot type: %d\n", ret); > + } > + > writel(0, priv->base); > mdelay(10000); > > -- // Casey (she/her)
