Hi Sumit, On 01/09/2026 21:57, Sumit Singh Dhami wrote: > Configure the PMIC PON PBS reset type before deasserting MPM PS_HOLD. > > The PON PBS reset sequence requires PS_HOLD_RESET_CTL2 to be cleared, > the requested reset type to be written to PS_HOLD_SW_CTL, and > PS_HOLD_RESET_CTL2 to be enabled again before deasserting MPM_PS_HOLD.
why? what does this fix? what do these registers encode? what platforms need this? Please include this context in v2 and in your cover letter. I shouldn't feel the need to look at PMIC register maps to understand what this is for. > > Signed-off-by: Sumit Singh Dhami <[email protected]> > --- > drivers/sysreset/sysreset_qcom-pshold.c | 137 ++++++++++++++++++++++-- > 1 file changed, 130 insertions(+), 7 deletions(-) > > diff --git a/drivers/sysreset/sysreset_qcom-pshold.c > b/drivers/sysreset/sysreset_qcom-pshold.c > index 45290478536..de7c0b388aa 100644 > --- a/drivers/sysreset/sysreset_qcom-pshold.c > +++ b/drivers/sysreset/sysreset_qcom-pshold.c > @@ -13,15 +13,94 @@ > #include <sysreset.h> > #include <asm/io.h> > #include <linux/delay.h> > +#include <power/pmic.h> > + > +#define PON_PBS_RESET_TYPE_WARM 0x01 > +#define PON_PBS_RESET_TYPE_HARD 0x07 > + > +/* > + * Reset State of PON_PBS_PS_HOLD_RESET_CTL2 register > + */ > +#define PON_PBS_PS_HOLD_RESET_CTL2_S2_RESET_EN_BMSK 0x80 > > struct qcom_pshold_priv { > phys_addr_t base; > + > + struct udevice *pmic; > + u32 pbs_base; > + bool have_pon; > + > + u32 pon_pbs_pshold_sw_ctl_offset; > + u32 pon_pbs_pshold_reset_ctl2_offset; > }; > > +static int qcom_pshold_pon_cfg(struct qcom_pshold_priv *priv, u8 reset_type) > +{ > + int ret; > + > + ret = pmic_reg_write(priv->pmic, > + priv->pbs_base + > + priv->pon_pbs_pshold_reset_ctl2_offset, > + 0x00); > + if (ret) > + return ret; > + > + udelay(100); > + > + ret = pmic_reg_write(priv->pmic, > + priv->pbs_base + > priv->pon_pbs_pshold_sw_ctl_offset, > + reset_type); > + if (ret) > + return ret; > + > + udelay(300); > + > + ret = pmic_reg_write(priv->pmic, > + priv->pbs_base + > priv->pon_pbs_pshold_reset_ctl2_offset, > + PON_PBS_PS_HOLD_RESET_CTL2_S2_RESET_EN_BMSK); > + if (ret) > + return ret; > + > + udelay(100); > + > + return 0; > +} > + > +static int qcom_pshold_read_pon_offset(struct udevice *dev, struct > qcom_pshold_priv *priv) > +{ > + int ret; > + > + ret = dev_read_u32(dev, > + "qcom,pbs-pshold-sw-ctl", > + &priv->pon_pbs_pshold_sw_ctl_offset); > + if (ret) > + return ret; > + > + ret = dev_read_u32(dev, > + "qcom,pbs-pshold-reset-ctl2", > + &priv->pon_pbs_pshold_reset_ctl2_offset); Where are the bindings for this? why are this offsets read from DT anyway? Do they really vary so much? Can't we just match the pmics ID registers and pick the correct offset per-pmic? > + if (ret) > + return ret; > + > + return 0; > +} > + > static int qcom_pshold_request(struct udevice *dev, enum sysreset_t type) > { > struct qcom_pshold_priv *priv = dev_get_priv(dev); > > + if (priv->have_pon) { > + u8 reset_type = (type == SYSRESET_WARM) ? > + PON_PBS_RESET_TYPE_WARM : > + PON_PBS_RESET_TYPE_HARD; > + int ret; > + > + ret = qcom_pshold_pon_cfg(priv, reset_type); > + if (ret) > + debug("%s: failed to configure PON reset type: %d\n", > + dev->name, ret); > + } > + > writel(0, priv->base); > mdelay(10000); > > @@ -35,9 +114,53 @@ static struct sysreset_ops qcom_pshold_ops = { > static int qcom_pshold_probe(struct udevice *dev) > { > struct qcom_pshold_priv *priv = dev_get_priv(dev); > + struct ofnode_phandle_args args; > + ofnode pmic_node; > + int index, ret; > + > + priv->have_pon = false; > > priv->base = dev_read_addr(dev); > - return priv->base == FDT_ADDR_T_NONE ? -EINVAL : 0; > + if (priv->base == FDT_ADDR_T_NONE) > + return -EINVAL; > + > + ret = dev_read_phandle_with_args(dev, "qcom,pon", NULL, 0, 0, &args); > + if (ret) > + return 0; > + > + index = ofnode_stringlist_search(args.node, "reg-names", "pbs"); > + if (index < 0) { > + debug("%s: referenced PON node has no 'pbs' reg\n", dev->name); > + return 0; > + } > + > + ret = ofnode_read_u32_index(args.node, "reg", index, &priv->pbs_base); > + if (ret) { > + debug("%s: failed to read PON 'pbs' reg: %d\n", dev->name, ret); > + return 0; > + } > + > + ret = qcom_pshold_read_pon_offset(dev, priv); > + if (ret) { > + debug("%s: failed to read PON offset configs: %d\n", dev->name, > ret); > + return 0; > + } > + > + pmic_node = ofnode_get_parent(args.node); > + if (!ofnode_valid(pmic_node)) { > + debug("%s: failed to get PON's parent PMIC node\n", dev->name); > + return 0; > + } > + > + ret = uclass_get_device_by_ofnode(UCLASS_PMIC, pmic_node, &priv->pmic); > + if (ret) { > + debug("%s: failed to get PMIC device: %d\n", dev->name, ret); > + return 0; > + } > + > + priv->have_pon = true; > + > + return 0; > } > > static const struct udevice_id qcom_pshold_ids[] = { > @@ -46,10 +169,10 @@ static const struct udevice_id qcom_pshold_ids[] = { > }; > > U_BOOT_DRIVER(qcom_pshold) = { > - .name = "qcom_pshold", > - .id = UCLASS_SYSRESET, > - .of_match = qcom_pshold_ids, > - .probe = qcom_pshold_probe, > - .priv_auto = sizeof(struct qcom_pshold_priv), > - .ops = &qcom_pshold_ops, > + .name = "qcom_pshold", > + .id = UCLASS_SYSRESET, > + .of_match = qcom_pshold_ids, > + .probe = qcom_pshold_probe, > + .priv_auto = sizeof(struct qcom_pshold_priv), > + .ops = &qcom_pshold_ops, Please don't spuriously reformat blocks like this. > }; -- // Casey (she/her)
