On 15 May 2018 19:44, Guenter Roeck wrote, > Subject: Re: [PATCH v2] watchdog: da9063: Fix setting/changing timeout > > On Wed, May 09, 2018 at 04:10:15PM +0200, Marco Felsch wrote: > > Hi Steve, > > > > On 05/09/2018 03:50 PM, Steve Twiss wrote: > > >On 09 May 2018 13:33, Marco Felsch wrote: > > > > > >> > > >>The DA9063 watchdog always resets the system when it changes the timeout > > >>value after the bootloader (e.g. Barebox) has it already set. > > >> > > >>To update the timeout value we have to disable the watchdog, clear the > > >>watchdog counter value and write the new timeout value to the watchdog. > > >>Clearing the counter value is a feature to be on the safe side, because the > > >>data sheet doesn't describe the behaviour of the watchdog counter value > > >>after a watchdog disabling-enable-sequence. > > >> > > >>The patch is based on Philipp Zabel's <p.zabel@xxxxxxxxxxxxxx>previous > > >>patch but doesn't wait 150us because the DA9063 doesn't need this delay. > > >>https://www.dialog-semiconductor.com/products/DA9063 > > > > > >Yes, according to the Dialog datasheet DA9063_2v1, there is no 150us minimum > > >assert time limit. But ... that doesn't seem correct to me, because the DA9062 > > >driver and DA9062 datasheet both show a minimum assertion time, like you > > >said. > > > > > >https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git/tree/drivers/watchdog/da9062_wdt.c?h=v4.17-rc3#n67 > > > > > >So let me check with the hardware engineers. > > > > That would be great looking forward to hear from you. I will prepare a v3 if it is. > > > Any updates ? > > Guenter Hi Guenter and Marco, Thank you for your patience. The best advice I can give at the moment is: Please follow what has been done in the DA9062 and DA9053 device drivers. https://elixir.bootlin.com/linux/latest/source/drivers/watchdog/da9062_wdt.c#L90 https://elixir.bootlin.com/linux/latest/source/drivers/watchdog/da9052_wdt.c#L76 In these cases: "setting TWDSCALE to zero for at least 150 us before writing the new value" There will be a longer answer, but that must wait until the formal datasheets have been clarified by the hardware engineers. Regards, Steve > > > >>+ > > >> static int _da9063_wdt_set_timeout(struct da9063 *da9063, unsigned int regval) > > >> { > > >>+ int ret; > > >>+ > > >>+ /* > > >>+ * The watchdog trigger a reboot if a timeout value is already > > >>+ * programmed. Because the timeout value combines two functions in > > >>+ * one: indicating the counter limit and starting the watchdog. To be > > >>+ * able to set the watchdog a second time (first time was done by the > > >>+ * bootloader) disable the watchdog clear the counter value manually and > > >>+ * set the new timeout value. > > >>+ */ > > >>+ ret = regmap_update_bits(da9063->regmap, DA9063_REG_CONTROL_D, > > >>+ DA9063_TWDSCALE_MASK, DA9063_TWDSCALE_DISABLE); > > >>+ if (ret) > > >>+ dev_warn(da9063->dev, > > >>+ "Failed to disable watchdog before setting new timeout\n"); > > >>+ > > >>+ ret = _da9063_wdt_reset_timer(da9063); > > >>+ if (ret) > > >>+ dev_warn(da9063->dev, "Failed to reset watchdog counter\n"); > > > > BTW. can you ask them if it is necessary to reset the counter value register > > manually or if this is done by disabling the watchdog. > > > > Regards, > > Marco -- To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
