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

[Hrishabh Rajput <hrishabh.rajput@oss.qualcomm.com>]

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@oss.qualcomm.com>
>>
>> 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@oss.qualcomm.com>
>> ---
>>   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