On Fri, Jun 14, 2019 at 08:45:28PM +0000, Ken Sloat wrote: > > -----Original Message----- > > From: Alexandre Belloni <alexandre.belloni@xxxxxxxxxxx> > > Sent: Friday, June 14, 2019 4:33 PM > > To: Ken Sloat <KSloat@xxxxxxxxxxxxxx> > > Cc: Guenter Roeck <linux@xxxxxxxxxxxx>; nicolas.ferre@xxxxxxxxxxxxx; > > ludovic.desroches@xxxxxxxxxxxxx; wim@xxxxxxxxxxxxxxxxxx; linux-arm- > > kernel@xxxxxxxxxxxxxxxxxxx; linux-watchdog@xxxxxxxxxxxxxxx; linux- > > kernel@xxxxxxxxxxxxxxx > > Subject: Re: [PATCH v2 1/1] watchdog: atmel: atmel-sama5d4-wdt: Disable > > watchdog on system suspend > > > > [This is an EXTERNAL EMAIL] > > ________________________________ > > > > On 14/06/2019 18:43:22+0000, Ken Sloat wrote: > > > Well I'm a little confused still because there are two separate > > > comments in these statements. The first within resume implies that the > > > init should be called because we might have lost register values for > > > some reason unexplained. > > > > The sama5d2 has a suspend mode where power to the core is completely > > cut. Only a few IPs remain powered (in the backup power domain). > > Unfortunately, the watchdog is not in that domain and may lose its registers. > > > > > Then within the init it says that the bootloader might have modified > > > the registers so we should check them and then update it or otherwise > > > disable it. I'm not trying to pick apart the logic or anything, I'm > > > just readily assuming it is good as it was already reviewed before. > > > > > > > The bootloaders may have started the watchdog (this makes sense if you > > really care about reliability) and so we need to be careful to keep the proper > > parameters. > > Thanks for the explanation Alexandre I appreciate it. > > > > So without digging into that too much, if we don't know if any of the > > > runtime situations above might have occurred, then isn't it best to > > > leave my patch as is? Yes this has the side effect of resetting the > > > timer count, but if the init call is needed and we don't have any way > > > to know if any of the situations occurred, then we have no choice right? > > > > > > > Until we can differentiate between suspend modes, we have no other > > choice. > > Ok I will leave my patch as is for now then > If that is what those involved in this discussion argue for, they will need to confirm with Reviewed-by: or Acked-by: tags. Thanks, Guenter > > -- > > Alexandre Belloni, Bootlin > > Embedded Linux and Kernel engineering > > https://bootlin.com > > Thanks, > Ken Sloat