On Tue, May 26, 2020 at 06:41:20PM +0300, Serge Semin wrote: > In case if the DW Watchdog IP core is synthesised with > WDT_USE_FIX_TOP == false, the TOP interval indexes make the device > to load a custom periods to the counter. These periods are hardwired > at the IP synthesis stage and can be within [2^8, 2^(WDT_CNT_WIDTH - 1)]. > Alas their values can't be detected at runtime and must be somehow > supplied to the driver so one could properly determine the watchdog > timeout intervals. For this purpose we suggest to have a vendor- > specific dts property "snps,watchdog-tops" utilized, which would > provide an array of sixteen counter values. At device probe stage they > will be used to initialize the watchdog device timeouts determined > from the array values and current clocks source rate. > > In order to have custom TOP values supported the driver must be > altered in the following way. First of all the fixed-top values > ready-to-use array must be determined for compatibility with currently > supported devices, which were synthesised with WDT_USE_FIX_TOP == true. > It will be used if either fixed TOP feature is detected being enabled or > no custom TOPs are fetched from the device dt node. Secondly at the probe > stage we must initialize an array of the watchdog timeouts corresponding > to the detected TOPs list and the reference clock rate. For generality the > procedure of initialization is designed in a way to support the TOPs array > with no limitations on the items order or value. Finally the watchdog > period search methods should be altered to support the new timeouts data > structure. > > Signed-off-by: Serge Semin <sergey.se...@baikalelectronics.ru> > Cc: Alexey Malahov <alexey.mala...@baikalelectronics.ru> > Cc: Thomas Bogendoerfer <tsbog...@alpha.franken.de> > Cc: Arnd Bergmann <a...@arndb.de> > Cc: Rob Herring <robh...@kernel.org> > Cc: linux-m...@vger.kernel.org > Cc: devicet...@vger.kernel.org
Reviewed-by: Guenter Roeck <li...@roeck-us.net> > --- > > Changelog v2: > - Rearrange SoBs. > - Add "ms" suffix to the methods returning msec and convert the methods > with no "ms" suffix to return a timeout in sec. > - Make sure minimum timeout is at least 1 sec. > - Refactor the timeouts calculation procedure to retain the timeouts in > the ascending order. > - Make sure there is no integer overflow in milliseconds calculation. It > is saved in a dedicated uint field of the timeout structure. > --- > drivers/watchdog/dw_wdt.c | 191 +++++++++++++++++++++++++++++++++----- > 1 file changed, 167 insertions(+), 24 deletions(-) > > diff --git a/drivers/watchdog/dw_wdt.c b/drivers/watchdog/dw_wdt.c > index fba21de2bbad..693c0d1fd796 100644 > --- a/drivers/watchdog/dw_wdt.c > +++ b/drivers/watchdog/dw_wdt.c > @@ -13,6 +13,8 @@ > */ > > #include <linux/bitops.h> > +#include <linux/limits.h> > +#include <linux/kernel.h> > #include <linux/clk.h> > #include <linux/delay.h> > #include <linux/err.h> > @@ -34,21 +36,40 @@ > #define WDOG_CURRENT_COUNT_REG_OFFSET 0x08 > #define WDOG_COUNTER_RESTART_REG_OFFSET 0x0c > #define WDOG_COUNTER_RESTART_KICK_VALUE 0x76 > +#define WDOG_COMP_PARAMS_1_REG_OFFSET 0xf4 > +#define WDOG_COMP_PARAMS_1_USE_FIX_TOP BIT(6) > > -/* The maximum TOP (timeout period) value that can be set in the watchdog. */ > -#define DW_WDT_MAX_TOP 15 > +/* There are sixteen TOPs (timeout periods) that can be set in the watchdog. > */ > +#define DW_WDT_NUM_TOPS 16 > +#define DW_WDT_FIX_TOP(_idx) (1U << (16 + _idx)) > > #define DW_WDT_DEFAULT_SECONDS 30 > > +static const u32 dw_wdt_fix_tops[DW_WDT_NUM_TOPS] = { > + DW_WDT_FIX_TOP(0), DW_WDT_FIX_TOP(1), DW_WDT_FIX_TOP(2), > + DW_WDT_FIX_TOP(3), DW_WDT_FIX_TOP(4), DW_WDT_FIX_TOP(5), > + DW_WDT_FIX_TOP(6), DW_WDT_FIX_TOP(7), DW_WDT_FIX_TOP(8), > + DW_WDT_FIX_TOP(9), DW_WDT_FIX_TOP(10), DW_WDT_FIX_TOP(11), > + DW_WDT_FIX_TOP(12), DW_WDT_FIX_TOP(13), DW_WDT_FIX_TOP(14), > + DW_WDT_FIX_TOP(15) > +}; > + > static bool nowayout = WATCHDOG_NOWAYOUT; > module_param(nowayout, bool, 0); > MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started " > "(default=" __MODULE_STRING(WATCHDOG_NOWAYOUT) ")"); > > +struct dw_wdt_timeout { > + u32 top_val; > + unsigned int sec; > + unsigned int msec; > +}; > + > struct dw_wdt { > void __iomem *regs; > struct clk *clk; > unsigned long rate; > + struct dw_wdt_timeout timeouts[DW_WDT_NUM_TOPS]; > struct watchdog_device wdd; > struct reset_control *rst; > /* Save/restore */ > @@ -64,20 +85,66 @@ static inline int dw_wdt_is_enabled(struct dw_wdt *dw_wdt) > WDOG_CONTROL_REG_WDT_EN_MASK; > } > > -static inline int dw_wdt_top_in_seconds(struct dw_wdt *dw_wdt, unsigned top) > +static unsigned int dw_wdt_find_best_top(struct dw_wdt *dw_wdt, > + unsigned int timeout, u32 *top_val) > { > + int idx; > + > /* > - * There are 16 possible timeout values in 0..15 where the number of > - * cycles is 2 ^ (16 + i) and the watchdog counts down. > + * Find a TOP with timeout greater or equal to the requested number. > + * Note we'll select a TOP with maximum timeout if the requested > + * timeout couldn't be reached. > */ > - return (1U << (16 + top)) / dw_wdt->rate; > + for (idx = 0; idx < DW_WDT_NUM_TOPS; ++idx) { > + if (dw_wdt->timeouts[idx].sec >= timeout) > + break; > + } > + > + if (idx == DW_WDT_NUM_TOPS) > + --idx; > + > + *top_val = dw_wdt->timeouts[idx].top_val; > + > + return dw_wdt->timeouts[idx].sec; > } > > -static int dw_wdt_get_top(struct dw_wdt *dw_wdt) > +static unsigned int dw_wdt_get_min_timeout(struct dw_wdt *dw_wdt) > { > - int top = readl(dw_wdt->regs + WDOG_TIMEOUT_RANGE_REG_OFFSET) & 0xF; > + int idx; > + > + /* > + * We'll find a timeout greater or equal to one second anyway because > + * the driver probe would have failed if there was none. > + */ > + for (idx = 0; idx < DW_WDT_NUM_TOPS; ++idx) { > + if (dw_wdt->timeouts[idx].sec) > + break; > + } > > - return dw_wdt_top_in_seconds(dw_wdt, top); > + return dw_wdt->timeouts[idx].sec; > +} > + > +static unsigned int dw_wdt_get_max_timeout_ms(struct dw_wdt *dw_wdt) > +{ > + struct dw_wdt_timeout *timeout = &dw_wdt->timeouts[DW_WDT_NUM_TOPS - 1]; > + u64 msec; > + > + msec = (u64)timeout->sec * MSEC_PER_SEC + timeout->msec; > + > + return msec < UINT_MAX ? msec : UINT_MAX; > +} > + > +static unsigned int dw_wdt_get_timeout(struct dw_wdt *dw_wdt) > +{ > + int top_val = readl(dw_wdt->regs + WDOG_TIMEOUT_RANGE_REG_OFFSET) & 0xF; > + int idx; > + > + for (idx = 0; idx < DW_WDT_NUM_TOPS; ++idx) { > + if (dw_wdt->timeouts[idx].top_val == top_val) > + break; > + } > + > + return dw_wdt->timeouts[idx].sec; > } > > static int dw_wdt_ping(struct watchdog_device *wdd) > @@ -93,17 +160,10 @@ static int dw_wdt_ping(struct watchdog_device *wdd) > static int dw_wdt_set_timeout(struct watchdog_device *wdd, unsigned int > top_s) > { > struct dw_wdt *dw_wdt = to_dw_wdt(wdd); > - int i, top_val = DW_WDT_MAX_TOP; > + unsigned int timeout; > + u32 top_val; > > - /* > - * Iterate over the timeout values until we find the closest match. We > - * always look for >=. > - */ > - for (i = 0; i <= DW_WDT_MAX_TOP; ++i) > - if (dw_wdt_top_in_seconds(dw_wdt, i) >= top_s) { > - top_val = i; > - break; > - } > + timeout = dw_wdt_find_best_top(dw_wdt, top_s, &top_val); > > /* > * Set the new value in the watchdog. Some versions of dw_wdt > @@ -120,7 +180,7 @@ static int dw_wdt_set_timeout(struct watchdog_device > *wdd, unsigned int top_s) > * wdd->max_hw_heartbeat_ms > */ > if (top_s * 1000 <= wdd->max_hw_heartbeat_ms) > - wdd->timeout = dw_wdt_top_in_seconds(dw_wdt, top_val); > + wdd->timeout = timeout; > else > wdd->timeout = top_s; > > @@ -238,6 +298,86 @@ static int dw_wdt_resume(struct device *dev) > > static SIMPLE_DEV_PM_OPS(dw_wdt_pm_ops, dw_wdt_suspend, dw_wdt_resume); > > +/* > + * In case if DW WDT IP core is synthesized with fixed TOP feature disabled > the > + * TOPs array can be arbitrary ordered with nearly any sixteen uint numbers > + * depending on the system engineer imagination. The next method handles the > + * passed TOPs array to pre-calculate the effective timeouts and to sort the > + * TOP items out in the ascending order with respect to the timeouts. > + */ > + > +static void dw_wdt_handle_tops(struct dw_wdt *dw_wdt, const u32 *tops) > +{ > + struct dw_wdt_timeout tout, *dst; > + int val, tidx; > + u64 msec; > + > + /* > + * We walk over the passed TOPs array and calculate corresponding > + * timeouts in seconds and milliseconds. The milliseconds granularity > + * is needed to distinguish the TOPs with very close timeouts and to > + * set the watchdog max heartbeat setting further. > + */ > + for (val = 0; val < DW_WDT_NUM_TOPS; ++val) { > + tout.top_val = val; > + tout.sec = tops[val] / dw_wdt->rate; > + msec = (u64)tops[val] * MSEC_PER_SEC; > + do_div(msec, dw_wdt->rate); > + tout.msec = msec - ((u64)tout.sec * MSEC_PER_SEC); > + > + /* > + * Find a suitable place for the current TOP in the timeouts > + * array so that the list is remained in the ascending order. > + */ > + for (tidx = 0; tidx < val; ++tidx) { > + dst = &dw_wdt->timeouts[tidx]; > + if (tout.sec > dst->sec || (tout.sec == dst->sec && > + tout.msec >= dst->msec)) > + continue; > + else > + swap(*dst, tout); > + } > + > + dw_wdt->timeouts[val] = tout; > + } > +} > + > +static int dw_wdt_init_timeouts(struct dw_wdt *dw_wdt, struct device *dev) > +{ > + u32 data, of_tops[DW_WDT_NUM_TOPS]; > + const u32 *tops; > + int ret; > + > + /* > + * Retrieve custom or fixed counter values depending on the > + * WDT_USE_FIX_TOP flag found in the component specific parameters > + * #1 register. > + */ > + data = readl(dw_wdt->regs + WDOG_COMP_PARAMS_1_REG_OFFSET); > + if (data & WDOG_COMP_PARAMS_1_USE_FIX_TOP) { > + tops = dw_wdt_fix_tops; > + } else { > + ret = of_property_read_variable_u32_array(dev_of_node(dev), > + "snps,watchdog-tops", of_tops, DW_WDT_NUM_TOPS, > + DW_WDT_NUM_TOPS); > + if (ret < 0) { > + dev_warn(dev, "No valid TOPs array specified\n"); > + tops = dw_wdt_fix_tops; > + } else { > + tops = of_tops; > + } > + } > + > + /* Convert the specified TOPs into an array of watchdog timeouts. */ > + dw_wdt_handle_tops(dw_wdt, tops); > + if (!dw_wdt->timeouts[DW_WDT_NUM_TOPS - 1].sec) { > + dev_err(dev, "No any valid TOP detected\n"); > + return -EINVAL; > + } > + > + return 0; > +} > + > static int dw_wdt_drv_probe(struct platform_device *pdev) > { > struct device *dev = &pdev->dev; > @@ -275,12 +415,15 @@ static int dw_wdt_drv_probe(struct platform_device > *pdev) > > reset_control_deassert(dw_wdt->rst); > > + ret = dw_wdt_init_timeouts(dw_wdt, dev); > + if (ret) > + goto out_disable_clk; > + > wdd = &dw_wdt->wdd; > wdd->info = &dw_wdt_ident; > wdd->ops = &dw_wdt_ops; > - wdd->min_timeout = 1; > - wdd->max_hw_heartbeat_ms = > - dw_wdt_top_in_seconds(dw_wdt, DW_WDT_MAX_TOP) * 1000; > + wdd->min_timeout = dw_wdt_get_min_timeout(dw_wdt); > + wdd->max_hw_heartbeat_ms = dw_wdt_get_max_timeout_ms(dw_wdt); > wdd->parent = dev; > > watchdog_set_drvdata(wdd, dw_wdt); > @@ -293,7 +436,7 @@ static int dw_wdt_drv_probe(struct platform_device *pdev) > * devicetree. > */ > if (dw_wdt_is_enabled(dw_wdt)) { > - wdd->timeout = dw_wdt_get_top(dw_wdt); > + wdd->timeout = dw_wdt_get_timeout(dw_wdt); > set_bit(WDOG_HW_RUNNING, &wdd->status); > } else { > wdd->timeout = DW_WDT_DEFAULT_SECONDS;