RE: [PATCH RESEND V2 1/4] dt-bindings: fsl: scu: add watchdog binding

[Anson Huang]

Hi, Rob

Best Regards!
Anson Huang

> -----Original Message-----
> From: Rob Herring [mailto:robh@xxxxxxxxxx]
> Sent: 2019å3æ12æ 5:26
> To: Aisheng Dong <aisheng.dong@xxxxxxx>
> Cc: Anson Huang <anson.huang@xxxxxxx>; Guenter Roeck <linux@roeck-
> us.net>; mark.rutland@xxxxxxx; ulf.hansson@xxxxxxxxxx; heiko@xxxxxxxxx;
> catalin.marinas@xxxxxxx; will.deacon@xxxxxxx;
> bjorn.andersson@xxxxxxxxxx; festevam@xxxxxxxxx;
> jagan@xxxxxxxxxxxxxxxxxxxx; Andy Gross <andy.gross@xxxxxxxxxx>; dl-
> linux-imx <linux-imx@xxxxxxx>; devicetree@xxxxxxxxxxxxxxx; linux-
> watchdog@xxxxxxxxxxxxxxx; arnd@xxxxxxxx; marc.w.gonzalez@xxxxxxx;
> s.hauer@xxxxxxxxxxxxxx; enric.balletbo@xxxxxxxxxxxxx;
> horms+renesas@xxxxxxxxxxxx; wim@xxxxxxxxxxxxxxxxxx; Daniel Baluta
> <daniel.baluta@xxxxxxx>; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx; kernel@xxxxxxxxxxxxxx; olof@xxxxxxxxx;
> shawnguo@xxxxxxxxxx; Jens Wiklander <jens.wiklander@xxxxxxxxxx>
> Subject: Re: [PATCH RESEND V2 1/4] dt-bindings: fsl: scu: add watchdog
> binding
> 
> +Jens W
> 
> On Thu, Mar 7, 2019 at 6:22 AM Aisheng Dong <aisheng.dong@xxxxxxx>
> wrote:
> >
> > Hi Rob,
> >
> > > > > I think Rob suggested that the SCU parent driver should
> > > > > instantiate the watchdog without explicit watchdog node. That
> > > > > would be possible, but it currently uses
> > > > > devm_of_platform_populate() to do the instantiation, and
> > > > > changing that would be a mess. Besides, it does sem to me that
> > > > > your suggested node would describe the hardware, so I am not
> > > > > sure I understand the
> > > reasoning.
> > >
> > > It would just be a call to create a platform device instead. How is that a
> mess?
> > >
> > > It's describing firmware. We have DT for describing h/w we've failed
> > > to make discoverable. We should not repeat that and just describe
> firmware in DT.
> > > Make the firmware discoverable! Though there are cases like firmware
> > > provided clocks where we still need something in DT, but this is not
> > > one of them.
> > >
> >
> > The watchdog node here in question actually is not using SCU firmware call.
> > Due to security requirement by SCU, watchdog can only be accessed in
> > security mode, for IMX case, via ARM Trust Firmware. That means the
> > watchdog used in Linux actually is using ARM SMC call and does not
> > depend SCU driver. So It would be strange for SCU driver to instantiate it.
> >
> > For this situation, do you think we can move watchdog out of scu node?
> > Maybe rename the compatible string like "fsl,imx8qxp-sip-watchdog"
> > because it's actually a watchdog serviced by ATF firmware.
> 
> Yes, but that creates more questions. What exactly does ATF talk to for the
> watchdog? The SCU firmware?

Yes, ATF talks to SCU firmware directly, Linux kernel watchdog driver call SMC instructions
to send command to ATF, and ATF will call SCU firmware API to finish the operation requested
by Linux kernel watchdog driver.

> 
> Maybe ATF should define and provide a standard watchdog interface? It is
> still a question of making the firmware discoverable rather than trying to
> describe the firmware in DT.

The SMC call by Linux kernel watchdog already follow the SIP(silicon provider) standard, each
SoC can define its own protocol for SIP. ATF does NOT have a standard common watchdog interface
now, since it is more like a platform specific feature, most of platforms can control watchdog directly
from Linux kernel I think. 

So, do you have suggestion for this case? Either find a place in DT to put watchdog node, or make it
a build-in device inside SCU driver? Or you have other better ideas?

Thanks,
Anson.

> 
> Rob