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

[Hrishabh Rajput]

On 10/31/2025 8:54 PM, Guenter Roeck wrote:
> On 10/31/25 03:18, 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.
>
> There should also be an explanation why there is no "qcom,gunyah-wdt"
> devicetree node, both here and in the file.

Ok sure, we'll include an explanation about this in the commit 
description and in the file.

> > 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.
> > +     */
> > +    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.
> > +     */
> > +
>
> Odd explanation. I would assume that the smem device is supposed to be
> persistent as well. Since  that is not the case, what happens if _this_
> device is unregistered and registered again ?

Thanks for pointing this out. qcom_smem_probe() will try to register the 
watchdog device again while it is already registered. As per the 
discussion in the other thread, we'll be implementing the module_exit() 
for Gunyah watchdog driver so it will not be a persistent module 
anymore. This problem will not exist then.


Thanks,

Hrishabh