On 10. 04. 19 10:52, Stefan Roese wrote: > On 10.04.19 10:46, Michal Simek wrote: >> On 10. 04. 19 10:40, Stefan Roese wrote: >>> On 10.04.19 09:44, Michal Simek wrote: >>>> On 09. 04. 19 16:34, Stefan Roese wrote: >>>>> On 09.04.19 16:27, Michal Simek wrote: >>>>>> On 09. 04. 19 16:22, Stefan Roese wrote: >>>>>>> On 09.04.19 12:45, Michal Simek wrote: >>>>>>>> On 08. 04. 19 11:28, Stefan Roese wrote: >>>>>>>>> This patch tries to implement a generic watchdog_reset() function >>>>>>>>> that >>>>>>>>> can be used by all boards that want to service the watchdog >>>>>>>>> device in >>>>>>>>> U-Boot. This watchdog servicing is enabled via CONFIG_WATCHDOG. >>>>>>>>> >>>>>>>>> Without this approach, new boards or platforms needed to >>>>>>>>> implement a >>>>>>>>> board specific version of this functionality, mostly copy'ing the >>>>>>>>> same >>>>>>>>> code over and over again into their board or platforms code base. >>>>>>>>> >>>>>>>>> With this new generic function, the scattered other functions are >>>>>>>>> now >>>>>>>>> removed to be replaced by the generic one. The new version also >>>>>>>>> enables >>>>>>>>> the configuration of the watchdog timeout via the DT "timeout-sec" >>>>>>>>> property (if enabled via CONFIG_OF_CONTROL). >>>>>>>>> >>>>>>>>> This patch also adds a new flag to the GD flags, to flag that the >>>>>>>>> watchdog is ready to use and adds the pointer to the watchdog >>>>>>>>> device >>>>>>>>> to the GD. This enables us to remove the global "watchdog_dev" >>>>>>>>> variable, which was prone to cause problems because of its >>>>>>>>> potentially >>>>>>>>> very early use in watchdog_reset(), even before the BSS is >>>>>>>>> cleared. >>>>>>>>> >>>>>>>>> Signed-off-by: Stefan Roese <[email protected]> >>>>>>>>> Cc: Heiko Schocher <[email protected]> >>>>>>>>> Cc: Tom Rini <[email protected]> >>>>>>>>> Cc: Michal Simek <[email protected]> >>>>>>>>> Cc: "Marek Behún" <[email protected]> >>>>>>>>> Cc: Daniel Schwierzeck <[email protected]> >>>>>>>>> --- >>>>>>>>> This patch depends on the following patches: >>>>>>>>> >>>>>>>>> [1] watchdog: Move watchdog_dev to data section (BSS may not be >>>>>>>>> cleared) >>>>>>>>> https://patchwork.ozlabs.org/patch/1075500/ >>>>>>>>> >>>>>>>>> [2] watchdog: Handle SPL build with watchdog disabled >>>>>>>>> https://patchwork.ozlabs.org/patch/1074098/ >>>>>>>>> >>>>>>>>> I would like to see [1] applied in this release, since its a real >>>>>>>>> fix on >>>>>>>>> some of the platforms with minimal chances of breakage. >>>>>>>>> >>>>>>>>> This patch now is a bigger rework and is intended for the next >>>>>>>>> merge >>>>>>>>> window. >>>>>>>>> >>>>>>>>> Please note that I didn't remove the "timeout-sec" handling >>>>>>>>> from the >>>>>>>>> driver already reading it from the DT (cdns & at91) in this patch. >>>>>>>>> The reason for this is, that in the cdns case, the removal also >>>>>>>>> brings >>>>>>>>> a functional change, which I wanted to do in a separate patch. And >>>>>>>>> in the at91 case its because there are updates to this driver >>>>>>>>> already >>>>>>>>> queued in the at91 next git branch which would most likely cause >>>>>>>>> merge >>>>>>>>> conflict. I'll send a cleanup patch for this driver later after >>>>>>>>> these >>>>>>>>> patches have been applied. >>>>>>>>> >>>>>>>>> Thanks, >>>>>>>>> Stefan >>>>>>>>> >>>>>>>>> v2: >>>>>>>>> - Rename watchdog_start() to initr_watchdog() and move it to >>>>>>>>> board_r.c. >>>>>>>>> This way its only called once, after the DM subsystem has >>>>>>>>> bound >>>>>>>>> all >>>>>>>>> watchdog drivers. This enables the use of the currently >>>>>>>>> implemented >>>>>>>>> logic of using the correct watchdog in U-Boot (via alias >>>>>>>>> etc). >>>>>>>>> >>>>>>>>> arch/mips/mach-mt7620/cpu.c | 36 >>>>>>>>> ----------------- >>>>>>>>> board/CZ.NIC/turris_mox/turris_mox.c | 30 >>>>>>>>> -------------- >>>>>>>>> board/CZ.NIC/turris_omnia/turris_omnia.c | 35 >>>>>>>>> ---------------- >>>>>>>>> .../microblaze-generic/microblaze-generic.c | 40 >>>>>>>>> ------------------- >>>>>>>>> board/xilinx/zynq/board.c | 39 >>>>>>>>> ------------------ >>>>>>>>> board/xilinx/zynqmp/zynqmp.c | 39 >>>>>>>>> ------------------ >>>>>>>>> common/board_r.c | 40 >>>>>>>>> +++++++++++++++++++ >>>>>>>>> drivers/watchdog/wdt-uclass.c | 26 >>>>>>>>> ++++++++++++ >>>>>>>>> include/asm-generic/global_data.h | 4 ++ >>>>>>>>> 9 files changed, 70 insertions(+), 219 deletions(-) >>>>>>> >>>>>>> <snip> >>>>>>> >>>>>>>>> int board_early_init_r(void) >>>>>>>>> { >>>>>>>>> u32 val; >>>>>>>>> diff --git a/common/board_r.c b/common/board_r.c >>>>>>>>> index 472987d5d5..d80f16c3ed 100644 >>>>>>>>> --- a/common/board_r.c >>>>>>>>> +++ b/common/board_r.c >>>>>>>>> @@ -48,6 +48,7 @@ >>>>>>>>> #include <linux/compiler.h> >>>>>>>>> #include <linux/err.h> >>>>>>>>> #include <efi_loader.h> >>>>>>>>> +#include <wdt.h> >>>>>>>>> DECLARE_GLOBAL_DATA_PTR; >>>>>>>>> @@ -621,6 +622,42 @@ static int initr_bedbug(void) >>>>>>>>> } >>>>>>>>> #endif >>>>>>>>> +#if defined(CONFIG_WATCHDOG) && defined(CONFIG_WDT) >>>>>>>> >>>>>>>> This is not correct. >>>>>>>> Here should be just CONFIG_WDT. >>>>>>>> >>>>>>>> Because there are cases where you want just start watchdog in >>>>>>>> u-boot >>>>>>>> but >>>>>>>> never service it by u-boot. >>>>>>> >>>>>>> AFAIK, this would change the current behavior. Currently, only when >>>>>>> CONFIG_WATCHDOG is enabled the watchdog driver is started >>>>>>> automatically >>>>>>> in U-Boot (and serviced). If I make the change you suggest above, >>>>>>> all >>>>>>> boards defining CONFIG_WDT (DM watchdog support) will automatically >>>>>>> enable the watchdog. This might make sense, but AFAICT this changes >>>>>>> the current behavior. >>>>>> >>>>>> The question is what default is. Because when I was adding support >>>>>> for >>>>>> Zynq/ZynqMP/Microblaze this logic is used there. >>>>>> I think that WDT should be there and if you think there are boards >>>>>> which >>>>>> want to have both we can cover that by Kconfig. >>>>> >>>>> As mentioned above, I agree that it makes sense to start the watchdog >>>>> automatically, if its enabled / selected via CONFIG_WDT. I suggest >>>>> that I move forward with your suggested change, but it would be fair >>>>> to Cc the maintainers of boards that have CONFIG_WDT set but >>>>> CONFIG_WATCHDOG unset. Do you (or anyone else) know of a tool / script >>>>> to detect such board configurations in U-Boot (one option set and >>>>> another unset)? >>>> >>>> good. >>>> Tom? >>> >>> I've run a Travis job on all boards with an error added for this >>> configuration (CONFIG_WDT set but CONFIG_WATCHDOG unset). Here the >>> list of these boards with their maintainers: >>> >>> taurus: Heiko Schocher <[email protected]> >>> smartweb: Heiko Schocher <[email protected]> >>> ast2500: Maxim Sloyko <[email protected]> >>> picosam9g45: Erik van Luijk <[email protected]> >>> mt7623n_bpir2: Ryder Lee <[email protected]> Weijie Gao >>> <[email protected]> >>> mt7629_rfb: Ryder Lee <[email protected]> Weijie Gao >>> <[email protected]> >>> bitmain_antminer_s9: Michal Simek <[email protected]> >> >> On this board disabling watchdog servicing was done by purpose. > > And it will stay disabled even with the imply, as the defconfig > currently disables CONFIG_WATCHDOG. So no change here.
yup > > BTW: bitmain_antminer_s9_defconfig seems to be the only board that > disables CONFIG_WATCHDOG. Possible and that's the board I used for tuning this feature. M _______________________________________________ U-Boot mailing list [email protected] https://lists.denx.de/listinfo/u-boot
