| Message ID | 20140909120328.GA20662@amd |
|---|---|
| State | Superseded |
| Delegated to: | Marek Vasut |
| Headers | show |
On Tuesday, September 09, 2014 at 02:03:28 PM, Pavel Machek wrote: > This adds watchdog disable. It is neccessary for running Linux kernel. Why do we not enable WDT in Linux instead ? Also, I recall there was a call to explicitly enable the L4 watchdog, so why do we not get rid of that instead ? > Signed-off-by: Pavel Machek <pavel@denx.de> > > diff --git a/arch/arm/cpu/armv7/socfpga/reset_manager.c > b/arch/arm/cpu/armv7/socfpga/reset_manager.c index e320c01..5503059 100644 > --- a/arch/arm/cpu/armv7/socfpga/reset_manager.c > +++ b/arch/arm/cpu/armv7/socfpga/reset_manager.c > @@ -14,6 +14,19 @@ DECLARE_GLOBAL_DATA_PTR; > static const struct socfpga_reset_manager *reset_manager_base = > (void *)SOCFPGA_RSTMGR_ADDRESS; > > +#define RSTMGR_PERMODRST_L4WD0_LSB 6 > + > +/* Disable the watchdog (toggle reset to watchdog) */ > +void watchdog_disable(void) > +{ > + /* assert reset for watchdog */ > + setbits_le32(&reset_manager_base->per_mod_reset, > + (1<<RSTMGR_PERMODRST_L4WD0_LSB)); > + /* deassert watchdog from reset (watchdog in not running state) */ > + clrbits_le32(&reset_manager_base->per_mod_reset, > + (1<<RSTMGR_PERMODRST_L4WD0_LSB)); > +} > + > /* > * Write the reset manager register to cause reset > */ > diff --git a/arch/arm/include/asm/arch-socfpga/reset_manager.h > b/arch/arm/include/asm/arch-socfpga/reset_manager.h index 3e95476..eb95c5b > 100644 > --- a/arch/arm/include/asm/arch-socfpga/reset_manager.h > +++ b/arch/arm/include/asm/arch-socfpga/reset_manager.h > @@ -10,6 +10,8 @@ > void reset_cpu(ulong addr); > void reset_deassert_peripherals_handoff(void); > > +void watchdog_disable(void); > + > struct socfpga_reset_manager { > u32 status; > u32 ctrl; Best regards, Marek Vasut
On Tue 2014-09-09 14:20:23, Marek Vasut wrote: > On Tuesday, September 09, 2014 at 02:03:28 PM, Pavel Machek wrote: > > This adds watchdog disable. It is neccessary for running Linux kernel. > > Why do we not enable WDT in Linux instead ? Also, I recall there was a call to > explicitly enable the L4 watchdog, so why do we not get rid of that > instead ? Booting linux kernel without watchdog support should be valid operation. Pavel
On Tuesday, September 09, 2014 at 02:30:01 PM, Pavel Machek wrote: > On Tue 2014-09-09 14:20:23, Marek Vasut wrote: > > On Tuesday, September 09, 2014 at 02:03:28 PM, Pavel Machek wrote: > > > This adds watchdog disable. It is neccessary for running Linux kernel. > > > > Why do we not enable WDT in Linux instead ? Also, I recall there was a > > call to explicitly enable the L4 watchdog, so why do we not get rid of > > that instead ? > > Booting linux kernel without watchdog support should be valid > operation. If you watchdog is enabled, then not so much. Also, why can we not just disable the WDT in U-Boot instead using a config option ? Best regards, Marek Vasut
On Tue 2014-09-09 14:31:46, Marek Vasut wrote: > On Tuesday, September 09, 2014 at 02:30:01 PM, Pavel Machek wrote: > > On Tue 2014-09-09 14:20:23, Marek Vasut wrote: > > > On Tuesday, September 09, 2014 at 02:03:28 PM, Pavel Machek wrote: > > > > This adds watchdog disable. It is neccessary for running Linux kernel. > > > > > > Why do we not enable WDT in Linux instead ? Also, I recall there was a > > > call to explicitly enable the L4 watchdog, so why do we not get rid of > > > that instead ? > > > > Booting linux kernel without watchdog support should be valid > > operation. > > If you watchdog is enabled, then not so much. Also, why can we not just disable > the WDT in U-Boot instead using a config option ? It seems that watchdog is enabled by default (by hardware or SPL), so yes, watchdog disable operation (and this patch) in u-boot is a good idea. Pavel
On Tuesday, September 09, 2014 at 03:09:48 PM, Pavel Machek wrote: > On Tue 2014-09-09 14:31:46, Marek Vasut wrote: > > On Tuesday, September 09, 2014 at 02:30:01 PM, Pavel Machek wrote: > > > On Tue 2014-09-09 14:20:23, Marek Vasut wrote: > > > > On Tuesday, September 09, 2014 at 02:03:28 PM, Pavel Machek wrote: > > > > > This adds watchdog disable. It is neccessary for running Linux > > > > > kernel. > > > > > > > > Why do we not enable WDT in Linux instead ? Also, I recall there was > > > > a call to explicitly enable the L4 watchdog, so why do we not get > > > > rid of that instead ? > > > > > > Booting linux kernel without watchdog support should be valid > > > operation. > > > > If you watchdog is enabled, then not so much. Also, why can we not just > > disable the WDT in U-Boot instead using a config option ? > > It seems that watchdog is enabled by default (by hardware or SPL), so > yes, watchdog disable operation (and this patch) in u-boot is a good > idea. OK. Best regards, Marek Vasut
Hi Pavel, On Tue, 2014-09-09 at 14:03 +0200, ZY - pavel wrote: > This adds watchdog disable. It is neccessary for running Linux kernel. > > Signed-off-by: Pavel Machek <pavel@denx.de> > > diff --git a/arch/arm/cpu/armv7/socfpga/reset_manager.c b/arch/arm/cpu/armv7/socfpga/reset_manager.c > index e320c01..5503059 100644 > --- a/arch/arm/cpu/armv7/socfpga/reset_manager.c > +++ b/arch/arm/cpu/armv7/socfpga/reset_manager.c > @@ -14,6 +14,19 @@ DECLARE_GLOBAL_DATA_PTR; > static const struct socfpga_reset_manager *reset_manager_base = > (void *)SOCFPGA_RSTMGR_ADDRESS; > > +#define RSTMGR_PERMODRST_L4WD0_LSB 6 Would suggest to put this into header file. Thanks Chin Liang > + > +/* Disable the watchdog (toggle reset to watchdog) */ > +void watchdog_disable(void) > +{ > + /* assert reset for watchdog */ > + setbits_le32(&reset_manager_base->per_mod_reset, > + (1<<RSTMGR_PERMODRST_L4WD0_LSB)); > + /* deassert watchdog from reset (watchdog in not running state) */ > + clrbits_le32(&reset_manager_base->per_mod_reset, > + (1<<RSTMGR_PERMODRST_L4WD0_LSB)); > +} > + > /* > * Write the reset manager register to cause reset > */ > diff --git a/arch/arm/include/asm/arch-socfpga/reset_manager.h b/arch/arm/include/asm/arch-socfpga/reset_manager.h > index 3e95476..eb95c5b 100644 > --- a/arch/arm/include/asm/arch-socfpga/reset_manager.h > +++ b/arch/arm/include/asm/arch-socfpga/reset_manager.h > @@ -10,6 +10,8 @@ > void reset_cpu(ulong addr); > void reset_deassert_peripherals_handoff(void); > > +void watchdog_disable(void); > + > struct socfpga_reset_manager { > u32 status; > u32 ctrl; >
Hi Pavel, On Tue, 2014-09-09 at 15:09 +0200, ZY - pavel wrote: > On Tue 2014-09-09 14:31:46, Marek Vasut wrote: > > On Tuesday, September 09, 2014 at 02:30:01 PM, Pavel Machek wrote: > > > On Tue 2014-09-09 14:20:23, Marek Vasut wrote: > > > > On Tuesday, September 09, 2014 at 02:03:28 PM, Pavel Machek wrote: > > > > > This adds watchdog disable. It is neccessary for running Linux kernel. > > > > > > > > Why do we not enable WDT in Linux instead ? Also, I recall there was a > > > > call to explicitly enable the L4 watchdog, so why do we not get rid of > > > > that instead ? > > > > > > Booting linux kernel without watchdog support should be valid > > > operation. > > > > If you watchdog is enabled, then not so much. Also, why can we not just disable > > the WDT in U-Boot instead using a config option ? > > It seems that watchdog is enabled by default (by hardware or SPL), so > yes, watchdog disable operation (and this patch) in u-boot is a good > idea. If refer to http://rocketboards.org/gitweb/?p=u-boot-socfpga.git;a=blob_plain;f=arch/arm/cpu/armv7/socfpga/s_init.c;hb=fe010493a1754e761f77ba19243ba93a9d038443, its current disable during U-Boot initialization, s_init. Thanks Chin Liang > > Pavel
On Friday, September 12, 2014 at 08:10:22 AM, Chin Liang See wrote: > Hi Pavel, > > On Tue, 2014-09-09 at 14:03 +0200, ZY - pavel wrote: > > This adds watchdog disable. It is neccessary for running Linux kernel. > > > > Signed-off-by: Pavel Machek <pavel@denx.de> > > > > diff --git a/arch/arm/cpu/armv7/socfpga/reset_manager.c > > b/arch/arm/cpu/armv7/socfpga/reset_manager.c index e320c01..5503059 > > 100644 > > --- a/arch/arm/cpu/armv7/socfpga/reset_manager.c > > +++ b/arch/arm/cpu/armv7/socfpga/reset_manager.c > > @@ -14,6 +14,19 @@ DECLARE_GLOBAL_DATA_PTR; > > > > static const struct socfpga_reset_manager *reset_manager_base = > > > > (void *)SOCFPGA_RSTMGR_ADDRESS; > > > > +#define RSTMGR_PERMODRST_L4WD0_LSB 6 > > Would suggest to put this into header file. > Thanks I agree. We're currently working on a proper cleanup, so please give us a few days to prepare a series. Best regards, Marek Vasut
diff --git a/arch/arm/cpu/armv7/socfpga/reset_manager.c b/arch/arm/cpu/armv7/socfpga/reset_manager.c index e320c01..5503059 100644 --- a/arch/arm/cpu/armv7/socfpga/reset_manager.c +++ b/arch/arm/cpu/armv7/socfpga/reset_manager.c @@ -14,6 +14,19 @@ DECLARE_GLOBAL_DATA_PTR; static const struct socfpga_reset_manager *reset_manager_base = (void *)SOCFPGA_RSTMGR_ADDRESS; +#define RSTMGR_PERMODRST_L4WD0_LSB 6 + +/* Disable the watchdog (toggle reset to watchdog) */ +void watchdog_disable(void) +{ + /* assert reset for watchdog */ + setbits_le32(&reset_manager_base->per_mod_reset, + (1<<RSTMGR_PERMODRST_L4WD0_LSB)); + /* deassert watchdog from reset (watchdog in not running state) */ + clrbits_le32(&reset_manager_base->per_mod_reset, + (1<<RSTMGR_PERMODRST_L4WD0_LSB)); +} + /* * Write the reset manager register to cause reset */ diff --git a/arch/arm/include/asm/arch-socfpga/reset_manager.h b/arch/arm/include/asm/arch-socfpga/reset_manager.h index 3e95476..eb95c5b 100644 --- a/arch/arm/include/asm/arch-socfpga/reset_manager.h +++ b/arch/arm/include/asm/arch-socfpga/reset_manager.h @@ -10,6 +10,8 @@ void reset_cpu(ulong addr); void reset_deassert_peripherals_handoff(void); +void watchdog_disable(void); + struct socfpga_reset_manager { u32 status; u32 ctrl;
This adds watchdog disable. It is neccessary for running Linux kernel. Signed-off-by: Pavel Machek <pavel@denx.de>