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

[Pavan Kondeti <pavan.kondeti@oss.qualcomm.com>]

On Tue, Oct 28, 2025 at 05:53:23PM +0100, Krzysztof Kozlowski wrote:
> On 28/10/2025 17:51, Dmitry Baryshkov wrote:
> > On Tue, Oct 28, 2025 at 05:40:33PM +0100, Krzysztof Kozlowski wrote:
> >> On 28/10/2025 17:33, Pavan Kondeti wrote:
> >>> On Tue, Oct 28, 2025 at 05:17:44PM +0100, Krzysztof Kozlowski wrote:
> >>>> On 28/10/2025 13:27, Pavan Kondeti wrote:
> >>>>> On Tue, Oct 28, 2025 at 12:07:40PM +0100, Krzysztof Kozlowski wrote:
> >>>>>> On 28/10/2025 12:04, Krzysztof Kozlowski wrote:
> >>>>>>> On 28/10/2025 11:58, Hrishabh Rajput wrote:
> >>>>>>>>
> >>>>>>>> On 10/28/2025 3:10 PM, Krzysztof Kozlowski wrote:
> >>>>>>>>> On 28/10/2025 10:35, Hrishabh Rajput via B4 Relay wrote:
> >>>>>>>>>> +
> >>>>>>>>>> +static int __init gunyah_wdt_init(void)
> >>>>>>>>>> +{
> >>>>>>>>>> +	struct arm_smccc_res res;
> >>>>>>>>>> +	struct device_node *np;
> >>>>>>>>>> +	int ret;
> >>>>>>>>>> +
> >>>>>>>>>> +	/* Check if we're running on a Qualcomm device */
> >>>>>>>>>> +	np = of_find_compatible_node(NULL, NULL, "qcom,smem");
> >>>>>>>>> I don't think you implemented my feedback. This again is executed on
> >>>>>>>>> every platform, e.g. on Samsung, pointlessly.
> >>>>>>>>>
> >>>>>>>>> Implement previous feedback.
> >>>>>>>>
> >>>>>>>> Do you want us to add platform device from another driver which is 
> >>>>>>>> probed only on Qualcomm devices (like socinfo from previous discussion) 
> >>>>>>>> and get rid of the module init function entirely? As keeping anything in 
> >>>>>>>> the module init will get it executed on all platforms.
> >>>>>>>
> >>>>>>> Instead of asking the same can you read previous discussion? What is
> >>>>>>> unclear here:
> >>>>>>> https://lore.kernel.org/all/3b901f9d-dbfa-4f93-a8d2-3e89bd9783c9@kernel.org/
> >>>>>>> ?
> >>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> With this patch version, we have tried to reduce the code execution on 
> >>>>>>>> non-Qualcomm devices (also tried the alternative as mentioned in the 
> >>>>>>>> cover letter). Adding platform device from another driver as described 
> >>>>>>>> above would eliminate it entirely, please let us know if you want us to 
> >>>>>>>> do that.
> >>>>>>>
> >>>>>>> Why do I need to repeat the same as last time?
> >>>>>>
> >>>>>>
> >>>>>> Now I see that you completely ignored previous discussion and sent THE
> >>>>>> SAME approach.
> >>>>>
> >>>>> Our intention is not to waste reviewers time at all. It is just a
> >>>>> misunderstanding on what your comment is about. Let me elaborate further
> >>>>> not to defend our approach here but to get a clarity so that we don't
> >>>>> end up in the same situation when v4 is posted.
> >>>>>
> >>>>> https://lore.kernel.org/all/b94d8ca3-af58-4a78-9a5a-12e3db0bf75f@kernel.org/ 
> >>>>>
> >>>>> You mentioned here
> >>>>>
> >>>>> ```
> >>>>> To me socinfo feels even better. That way only, really only qcom devices
> >>>>> will execute this SMC.
> >>>>> ```
> >>>>>
> >>>>> We interpreted this comment as `avoid executing this SMC on non qcom
> >>>>> devices`. That is exactly what we have done in the current patch. since
> >>>>
> >>>>
> >>>> So where did you use socinfo? Point me to the code.
> >>>>
> >>>
> >>> Okay, lets go a bit deep into the socinfo part. we have used
> >>> `soc_device_match()` API to detect if the device is qcom (`family =
> >>> Snapdragon`). It works. However, when we built both `socinfo` and
> >>
> >> socinfo driver. Read my first feedback:
> >>
> >>
> >> "No, your hypervisor driver (which you have) should start the module via
> >> adding platform/aux/something devices."
> >>
> >> And then I agreed if you start it from the socinfo driver.
> > 
> > I'd rather not tie this to socinfo. The socinfo is an optional driver,
> > which is mainly used to provide debugfs entries. Watchdog is much more
> > important. It should not be tied to debugfs-only entry.
> > 
> 
> No problem. Choose whatever driver it is. The problem is that they did
> not even implement that. They claimed they followed review but it is
> 100% ignored. Nothing got implemented and they send the same.
> 

We will register the wdt platform device from smem driver something like
below. This makes this gunyah-wdt driver to register for a platform
driver which gets probed only on qcom devices.

diff --git a/drivers/soc/qcom/smem.c b/drivers/soc/qcom/smem.c
index c4c45f15dca4..67bf5f075cb8 100644
--- a/drivers/soc/qcom/smem.c
+++ b/drivers/soc/qcom/smem.c
@@ -1236,6 +1236,8 @@ static int qcom_smem_probe(struct platform_device *pdev)
 	if (IS_ERR(smem->socinfo))
 		dev_dbg(&pdev->dev, "failed to register socinfo device\n");
 
+	register_hyp_wdt_after_other_dt_checks();
+
 	return 0;
 }
 
Thanks,
Pavan