[RFC PATCH 1/2] watchdog: imx2: fix hang-up on boot for i.MX21, i.MX27 and i.MX31 SoCs

[Magnus Lilja]

Hi,

On 26 September 2016 at 15:02, Guenter Roeck <linux at roeck-us.net> wrote:
> On 09/25/2016 05:39 PM, Vladimir Zapolskiy wrote:
>>
>> Power down counter enable/disable bit switch is located in WMCR
>> register, but watchdog controllers found on legacy i.MX21, i.MX27 and
>> i.MX31 SoCs don't have this register. As a result of writing data to
>> the non-existing register on driver probe the SoC hangs up, to fix the
>> problem add more OF compatible strings and on this basis get
>> information about availability of the WMCR register.
>>
>> Fixes: 5fe65ce7ccbb ("watchdog: imx2_wdt: Disable power down counter on
>> boot")
>> Signed-off-by: Vladimir Zapolskiy <vz at mleia.com>
>> ---
>>  drivers/watchdog/imx2_wdt.c | 47
>> +++++++++++++++++++++++++++++++++++++++++++--
>>  1 file changed, 45 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/watchdog/imx2_wdt.c b/drivers/watchdog/imx2_wdt.c
>> index 62f346b..b6763e0 100644
>> --- a/drivers/watchdog/imx2_wdt.c
>> +++ b/drivers/watchdog/imx2_wdt.c
>> @@ -29,6 +29,7 @@
>>  #include <linux/module.h>
>>  #include <linux/moduleparam.h>
>>  #include <linux/of_address.h>
>> +#include <linux/of_device.h>
>>  #include <linux/platform_device.h>
>>  #include <linux/regmap.h>
>>  #include <linux/watchdog.h>
>> @@ -57,6 +58,10 @@
>>
>>  #define WDOG_SEC_TO_COUNT(s)   ((s * 2 - 1) << 8)
>>
>> +struct imx2_wdt_data {
>> +       bool has_pdc;
>> +};
>> +
>>  struct imx2_wdt_device {
>>         struct clk *clk;
>>         struct regmap *regmap;
>> @@ -64,6 +69,8 @@ struct imx2_wdt_device {
>>         bool ext_reset;
>>  };
>>
>> +static const struct of_device_id imx2_wdt_dt_ids[];
>> +
>>  static bool nowayout = WATCHDOG_NOWAYOUT;
>>  module_param(nowayout, bool, 0);
>>  MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started
>> (default="
>> @@ -200,10 +207,13 @@ static const struct regmap_config
>> imx2_wdt_regmap_config = {
>>
>>  static int __init imx2_wdt_probe(struct platform_device *pdev)
>>  {
>> +       const struct of_device_id *of_id;
>> +       const struct imx2_wdt_data *data;
>>         struct imx2_wdt_device *wdev;
>>         struct watchdog_device *wdog;
>>         struct resource *res;
>>         void __iomem *base;
>> +       bool has_pdc;
>>         int ret;
>>         u32 val;
>>
>> @@ -261,12 +271,24 @@ static int __init imx2_wdt_probe(struct
>> platform_device *pdev)
>>                 set_bit(WDOG_HW_RUNNING, &wdog->status);
>>         }
>>
>> +       if (pdev->dev.of_node) {
>> +               of_id = of_match_device(imx2_wdt_dt_ids, &pdev->dev);
>> +               if (!of_id)
>> +                       return -ENODEV;
>> +
>> +               data = of_id->data;
>> +               has_pdc = data->has_pdc;
>> +       } else {
>> +               has_pdc = false;
>> +       }
>> +
>>         /*
>>          * Disable the watchdog power down counter at boot. Otherwise the
>> power
>>          * down counter will pull down the #WDOG interrupt line for one
>> clock
>>          * cycle.
>>          */
>> -       regmap_write(wdev->regmap, IMX2_WDT_WMCR, 0);
>> +       if (has_pdc)
>> +               regmap_write(wdev->regmap, IMX2_WDT_WMCR, 0);
>>
>>         ret = watchdog_register_device(wdog);
>>         if (ret) {
>> @@ -363,8 +385,29 @@ static int imx2_wdt_resume(struct device *dev)
>>  static SIMPLE_DEV_PM_OPS(imx2_wdt_pm_ops, imx2_wdt_suspend,
>>                          imx2_wdt_resume);
>>
>> +static const struct imx2_wdt_data imx21_wdt_data = {
>> +       .has_pdc = false,
>> +};
>> +
>> +static const struct imx2_wdt_data imx25_wdt_data = {
>> +       .has_pdc = true,
>> +};
>> +
>>  static const struct of_device_id imx2_wdt_dt_ids[] = {
>> -       { .compatible = "fsl,imx21-wdt", },
>> +       { .compatible = "fsl,imx21-wdt",  .data = &imx21_wdt_data },
>
>
> Please just specify the flag directly, as in
>                                         .data = (void *) true
> or, if you prefer, use defines.
>                                         .data = (void *) IMX_SUPPORTS_PDC
>
> Then, in above code:
>                 has_pdc = (bool) of_id->data;
>
> Guenter

Has this patch (or an updated one) been merged in any tree? I've
tested it on a i.MX31 board with positive result.


Regards, Magnus