On 7/10/24 23:17, Rasmus Villemoes wrote:

This was added in commit 45a6d231b2f (bcm2835_wdt: support for the BCM2835/2836 watchdog), which did do 'select HW_WATCHDOG'. That incarnation of the watchdog driver later got removed in c7adc0b5f98 (watchdog: bcm2835_wdt: Remove unused BCM283x watchdog driver and its references), but this block was left behind.

Another rpi watchdog driver has since been added, but that does not select HW_WATCHDOG, so this remains dead and unused. Remove it.

Signed-off-by: Rasmus Villemoes rasmus.villemoes@prevas.dk

Reviewed-by: Stefan Roese sr@denx.de

Thanks, Stefan

---

board/raspberrypi/rpi/rpi.c | 4 ---- 1 file changed, 4 deletions(-)

diff --git a/board/raspberrypi/rpi/rpi.c b/board/raspberrypi/rpi/rpi.c index d996eb0cf69..9a83cf2d6a7 100644 --- a/board/raspberrypi/rpi/rpi.c +++ b/board/raspberrypi/rpi/rpi.c @@ -493,10 +493,6 @@ static void get_board_revision(void)

int board_init(void) { -#ifdef CONFIG_HW_WATCHDOG

• hw_watchdog_init();

-#endif

• get_board_revision();

  gd->bd->bi_boot_params = 0x100;

Viele Grüße, Stefan Roese

On 7/11/24 09:54, Peter Robinson wrote:

On Wed, 10 Jul 2024 at 22:17, Rasmus Villemoes rasmus.villemoes@prevas.dk wrote:

...

This empty stub was originally added as one branch of an #ifdef in commit 45a6d231b2f (bcm2835_wdt: support for the BCM2835/2836 watchdog). That incarnation of the rpi watchdog driver was later removed in c7adc0b5f98 (watchdog: bcm2835_wdt: Remove unused BCM283x watchdog driver and its references), but this now-pointless stub was

You don't need a - in "now-pointless" and I think "now unused" is a better term, please update the subject too.

...

left behind. The later (re-)added rpi watchdog driver does not define a hw_watchdog_disable() function, as that is properly integrated in the watchdog framework (i.e. not of the hw_* kind).

Since it wasn't declared static, the compiler cannot even elide it (though it does get inlined into the caller here, and the out-of-line definition does get gc'ed by the linker).

All the things in () don't add value to this, please remove them.

I am a big friend of extensive commit messages, so IMHO all this should stay in. If a developer is not interested in such details he can skip these infos. Still if somebody feels strong here to drop these infos, then I won't object.

Reviewed-by: Stefan Roese sr@denx.de

Thanks, Stefan

...

Signed-off-by: Rasmus Villemoes rasmus.villemoes@prevas.dk

arch/arm/mach-bcm283x/reset.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/arch/arm/mach-bcm283x/reset.c b/arch/arm/mach-bcm283x/reset.c index 9199234917f..1dc7ce50d1d 100644 --- a/arch/arm/mach-bcm283x/reset.c +++ b/arch/arm/mach-bcm283x/reset.c @@ -25,8 +25,6 @@ /* max ticks timeout */ #define BCM2835_WDOG_MAX_TIMEOUT 0x000fffff

-void hw_watchdog_disable(void) {}

• __efi_runtime_data struct bcm2835_wdog_regs *wdog_regs;

  static void __efi_runtime

@@ -34,10 +32,9 @@ __reset_cpu(struct bcm2835_wdog_regs *wdog_regs, ulong ticks) { uint32_t rstc, timeout;

•   if (ticks == 0) {
  

•           hw_watchdog_disable();
  

•   if (ticks == 0)
             timeout = RESET_TIMEOUT;
  

•   } else
  

•   else
             timeout = ticks & BCM2835_WDOG_MAX_TIMEOUT;
  
     rstc = readl(&wdog_regs->rstc);
  

-- 2.45.2

Viele Grüße, Stefan Roese