cdns_wdt: who enables the clock

cdns_wdt: who enables the clock

[Jorge Ramirez-Ortiz, Foundries]

Hi Michal

The *linux* cadence_wdt driver for the watchdog enables the clock that
controls its  behaviour. However this is not done in the U-boot
driver; I dont think it is safe to assume that the clock will be active.
do you know why is the driver making that assumption? and how to
enable the clock?

Also, I am not sure it is either safe to assume that the WDT IP will
be enabled; IMO during probe we should do something like this
(otherwise the AXI bus will complain and fault the boot)

 static int cdns_wdt_probe(struct udevice *dev)
 {
-   debug("%s: Probing wdt%u\n", __func__, dev_seq(dev));
+ u32 rst = 0;
+
+ /* release WDT from reset */
+ rst = readl(0xFD1A0100) & ~BIT(15);
+ writel(rst, 0xFD1A0100);
+
+ debug("%s: Probing wdt%u)\n", __func__, dev_seq(dev));
 
        return 0;
 }

thanks
Jorge

Re: cdns_wdt: who enables the clock

[Michal Simek]

Hi,

On 9/8/21 5:39 PM, Jorge Ramirez-Ortiz, Foundries wrote:
> Hi Michal
> 
> The *linux* cadence_wdt driver for the watchdog enables the clock that
> controls its  behaviour. However this is not done in the U-boot
> driver; I dont think it is safe to assume that the clock will be active.
> do you know why is the driver making that assumption? and how to
> enable the clock?
> 
> Also, I am not sure it is either safe to assume that the WDT IP will
> be enabled; IMO during probe we should do something like this
> (otherwise the AXI bus will complain and fault the boot)
> 
>  static int cdns_wdt_probe(struct udevice *dev)
>  {
> -   debug("%s: Probing wdt%u\n", __func__, dev_seq(dev));
> + u32 rst = 0;
> +
> + /* release WDT from reset */
> + rst = readl(0xFD1A0100) & ~BIT(15);
> + writel(rst, 0xFD1A0100);
> +
> + debug("%s: Probing wdt%u)\n", __func__, dev_seq(dev));
>  
>         return 0;
>  }

If reset should be perform it should be done via
drivers/reset/reset-zynqmp.c

It means when you define it in DT you will get reset.

I am ok to wire all resets for all IPs to make sure that they are in
proper state but this has to be tested first.

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Xilinx Microblaze
Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs
U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP/Versal SoCs

Re: cdns_wdt: who enables the clock

[Jorge Ramirez-Ortiz, Foundries]

On 10/09/21, Michal Simek wrote:
> Hi,
> 
> On 9/8/21 5:39 PM, Jorge Ramirez-Ortiz, Foundries wrote:
> > Hi Michal
> > 
> > The *linux* cadence_wdt driver for the watchdog enables the clock that
> > controls its  behaviour. However this is not done in the U-boot
> > driver; I dont think it is safe to assume that the clock will be active.
> > do you know why is the driver making that assumption? and how to
> > enable the clock?
> > 
> > Also, I am not sure it is either safe to assume that the WDT IP will
> > be enabled; IMO during probe we should do something like this
> > (otherwise the AXI bus will complain and fault the boot)
> > 
> >  static int cdns_wdt_probe(struct udevice *dev)
> >  {
> > -   debug("%s: Probing wdt%u\n", __func__, dev_seq(dev));
> > + u32 rst = 0;
> > +
> > + /* release WDT from reset */
> > + rst = readl(0xFD1A0100) & ~BIT(15);
> > + writel(rst, 0xFD1A0100);
> > +
> > + debug("%s: Probing wdt%u)\n", __func__, dev_seq(dev));
> >  
> >         return 0;
> >  }
> 
> If reset should be perform it should be done via
> drivers/reset/reset-zynqmp.c

ok

> 
> It means when you define it in DT you will get reset.

makes sene

> 
> I am ok to wire all resets for all IPs to make sure that they are in
> proper state but this has to be tested first.

all right. but yes this particular case was tested.

> 
> Thanks,
> Michal
> 
> -- 
> Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
> w: www.monstr.eu p: +42-0-721842854
> Maintainer of Linux kernel - Xilinx Microblaze
> Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs
> U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP/Versal SoCs
>