Re: [PATCH v4 1/2] soc: qcom: smem: Register gunyah watchdog device

[Hrishabh Rajput]

On 11/2/2025 12:15 AM, Bjorn Andersson wrote:
> On Fri, Oct 31, 2025 at 10:18:13AM +0000, Hrishabh Rajput via B4 Relay wrote:
> > From: Hrishabh Rajput <hrishabh.rajput@xxxxxxxxxxxxxxxx>
> >
> > To restrict gunyah watchdog initialization to Qualcomm platforms,
> > register the watchdog device in the SMEM driver.
> >
> > When Gunyah is not present or Gunyah emulates MMIO-based
> > watchdog, we expect Qualcomm watchdog or ARM SBSA watchdog device to be
> > present in the devicetree. If none of these device nodes are detected,
> > we register the SMC-based Gunyah watchdog device.
> >
> > Signed-off-by: Hrishabh Rajput <hrishabh.rajput@xxxxxxxxxxxxxxxx>
> > ---
> >   drivers/soc/qcom/smem.c | 37 +++++++++++++++++++++++++++++++++++++
> >   1 file changed, 37 insertions(+)
> >
> > diff --git a/drivers/soc/qcom/smem.c b/drivers/soc/qcom/smem.c
> > index cf425930539e..40e4749fab02 100644
> > --- a/drivers/soc/qcom/smem.c
> > +++ b/drivers/soc/qcom/smem.c
> > @@ -1118,6 +1118,34 @@ static int qcom_smem_resolve_mem(struct qcom_smem *smem, const char *name,
> >   	return 0;
> >   }
> >   
> > +static int register_gunyah_wdt_device(void)
> > +{
> > +	struct platform_device *gunyah_wdt_dev;
> > +	struct device_node *np;
> > +
> > +	/*
> > +	 * When Gunyah is not present or Gunyah is emulating a memory-mapped
> > +	 * watchdog, either of Qualcomm watchdog or ARM SBSA watchdog will be
> > +	 * present. Skip initialization of SMC-based Gunyah watchdog if that is
> > +	 * the case.
> E.g. qcom-apq8064.dtsi doesn't define either qcom,kpss-wdt, nor
> arm,sbsa-gwdt, does that imply that it implements the Gunyah watchdog?


It doesn't implement Gunyah watchdog. For platforms like these we've 
kept a STATUS SMC call in the gunyah_wdt_probe().

The SMC Call is expected to fail on platforms which do not have support 
for SMC based Gunyah watchdog, which in turn will fail the probe.

Let us know if there's a better way to handle this.

> > +	 */
> > +	np = of_find_compatible_node(NULL, NULL, "qcom,kpss-wdt");
> > +	if (np) {
> > +		of_node_put(np);
> > +		return 0;
> > +	}
> > +
> > +	np = of_find_compatible_node(NULL, NULL, "arm,sbsa-gwdt");
> > +	if (np) {
> > +		of_node_put(np);
> > +		return 0;
> > +	}
> > +
> > +	gunyah_wdt_dev = platform_device_register_simple("gunyah-wdt", -1,
> > +							 NULL, 0);
> > +	return PTR_ERR_OR_ZERO(gunyah_wdt_dev);
> > +}
> > +
> >   static int qcom_smem_probe(struct platform_device *pdev)
> >   {
> >   	struct smem_header *header;
> > @@ -1236,11 +1264,20 @@ static int qcom_smem_probe(struct platform_device *pdev)
> >   	if (IS_ERR(smem->socinfo))
> >   		dev_dbg(&pdev->dev, "failed to register socinfo device\n");
> >   
> > +	ret = register_gunyah_wdt_device();
> > +	if (ret)
> > +		dev_dbg(&pdev->dev, "failed to register watchdog device\n");
> > +
> >   	return 0;
> >   }
> >   
> >   static void qcom_smem_remove(struct platform_device *pdev)
> >   {
> > +	/*
> > +	 * Gunyah watchdog is intended to be a persistent module. Hence, the
> > +	 * watchdog device is not unregistered.
> > +	 */
> Why? I don't see why the code needs to encode such policy, please
> explain.


You're right, there is no such need. We're at wrong here. We had an 
incorrect understanding of watchdog drivers being persistent. We will be 
implementing the module_exit() for the Gunyah watchdog making it not 
persistent.


Thanks,

Hrishabh