Re: [PATCH v3] watchdog: Add driver for Gunyah Watchdog

[Guenter Roeck <linux@roeck-us.net>]

On 10/28/25 09:40, Pavan Kondeti wrote:
> On Tue, Oct 28, 2025 at 09:06:12AM -0700, Guenter Roeck wrote:
>> On 10/28/25 02:35, Hrishabh Rajput via B4 Relay wrote:
>>> From: Hrishabh Rajput <hrishabh.rajput@oss.qualcomm.com>
>>>
>>> On Qualcomm SoCs running under the Gunyah hypervisor, access to watchdog
>>> through MMIO is not available on all platforms. Depending on the
>>> hypervisor configuration, the watchdog is either fully emulated or
>>> exposed via ARM's SMC Calling Conventions (SMCCC) through the Vendor
>>> Specific Hypervisor Service Calls space.
>>>
>>> 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 we detect either of the device nodes, we don't
>>> proceed ahead. Otherwise, we go ahead and invoke GUNYAH_WDT_STATUS SMC
>>> to initiate the discovery of the SMC-based watchdog.
>>>
>>> Add driver to support the SMC-based watchdog provided by the Gunyah
>>> Hypervisor. module_exit() is intentionally not implemented as this
>>> driver is intended to be a persistent module.
>>>
>>> Signed-off-by: Hrishabh Rajput <hrishabh.rajput@oss.qualcomm.com>
>>> ---

...
>>> +	gunyah_wdt_dev = platform_device_register_simple(GUNYAH_WDT_DRV_NAME,
>>> +							 -1, NULL, 0);
>>
>> I did not follow the discussion around this, so I may be missing something.
>> If so, apologies.
>>
>> This is a highly unusual approach. What is the point of not instantiating
>> the watchdog device through devicetree and doing it in the init function
>> instead ? There should be a devicetree node which instantiates the device;
>> it should never be instantiated from the init function unless there _is_
>> no devicetree, which is obviously not the case here.
>>
>> Every other driver which supports devicetree has an .of_match_table
>> which triggers device instantiation. If the Gunyah watchdog can for
>> some reason not use that approach, its devicetree description should
>> be fixed. Instantiating the device from its init function because its
>> devicetree description is bad or missing is just wrong. It is even more
>> wrong to try to contact the hardware or embedded controller to figure out
>> if the device is there. This can have all kinds of negative impact on other
>> hardware.
>>
> The Gunyah WDT node gets overlayed by bootloader. We see that this
> overlay is failing w/ upstream device tree since the overlay has
> references to downstream code. Please see [1]. Hence we are trying to
> register the platform device dynamically.
> 

This is just wrong. Whatever happens downstream is not an upstream concern.
If an overlay is broken, fix it.

NACK to the current approach.

Guenter