[RFC PATCH 0/2] watchdog: imx2+: fix hangup during watchdog initialization

[RFC PATCH 0/2] watchdog: imx2+: fix hangup during watchdog initialization

[Vladimir Zapolskiy]

It was discovered that the current version of imx2+ watchdog driver
does not work properly on i.MX31 boards and due to the same root
cause the kernel should not work on i.MX21 and i.MX27 powered boards,
if watchdog device driver is probed.

The root cause is that the watchdog controller on all modern SoCs from
i.MX series has two more registers in comparison to the watchdog on
i.MX21, i.MX27 and i.MX31, thus write access to the non-existing
register by the driver probe function causes a core hang.

The commit which caused a regression is 5fe65ce7ccbb ("watchdog:
imx2_wdt: Disable power down counter on boot"), for me it's hard
to say if this change is utterly needed or not (e.g. if it is done
in a bootloader), but it seems to be valid for modern i.MX SoCs.

The solution introduces a runtime difference between i.MX21 and i.MX25
compatible watchdog controllers, also this is reflected in the
updated DTS files.

The change is an RFC, because
* it is based on v4.8-rc1 and it will be rebased after getting review
  comments,
* the selection of compatible watchdog controllers is partially done
  on RMs, but I don't have access to all of them,
* the change fixes legacy boards (prior to DT support), but the fix is
  excessive in sense that WMCR register won't be set on them, to cover
  this case I feel reluctant to add a new header with platform data
  structure declaration for legacy boards, but comments are welcome
* I don't know if a complete list of watchdog compatibles (which is
  expected to be growing with time) is wanted in the driver code,
* if the DT change from the series is added, then only a check for
  fsl,imx25-wdt compatible migth be good enough, but this will
  nullify "disable power down counter" change for boards with
  not updated DTBs.
* note that ls1021a.dtsi does not have its named watchdog compatible,
  I know that Rob insists on adding SoC-named compatibles for
  controllers, this may help in situations like this one, other
  opinions or discussion or fix for LS1021A is welcome.

Vladimir Zapolskiy (2):
  watchdog: imx2: fix hang-up on boot for i.MX21, i.MX27 and i.MX31 SoCs
  ARM: i.MX: dts: add fsl,imx25-wdt compatible to all relevant watchdog nodes

 arch/arm/boot/dts/imx35.dtsi   |  3 ++-
 arch/arm/boot/dts/imx50.dtsi   |  3 ++-
 arch/arm/boot/dts/imx51.dtsi   |  6 ++++--
 arch/arm/boot/dts/imx53.dtsi   |  6 ++++--
 arch/arm/boot/dts/imx6qdl.dtsi |  6 ++++--
 arch/arm/boot/dts/imx6sl.dtsi  |  6 ++++--
 arch/arm/boot/dts/imx6sx.dtsi  |  9 +++++---
 arch/arm/boot/dts/imx6ul.dtsi  |  6 ++++--
 arch/arm/boot/dts/imx7s.dtsi   | 12 +++++++----
 arch/arm/boot/dts/ls1021a.dtsi |  2 +-
 arch/arm/boot/dts/vfxxx.dtsi   |  3 ++-
 drivers/watchdog/imx2_wdt.c    | 47 ++++++++++++++++++++++++++++++++++++++++--
 12 files changed, 86 insertions(+), 23 deletions(-)

-- 
2.8.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Vladimir Zapolskiy]

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-ChpfBGZJDbMAvxtiuMwx3w@public.gmane.org>
---
 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 },
+	{ .compatible = "fsl,imx25-wdt",  .data = &imx25_wdt_data },
+	{ .compatible = "fsl,imx27-wdt",  .data = &imx21_wdt_data },
+	{ .compatible = "fsl,imx31-wdt",  .data = &imx21_wdt_data },
+	{ .compatible = "fsl,imx35-wdt",  .data = &imx25_wdt_data },
+	{ .compatible = "fsl,imx50-wdt",  .data = &imx25_wdt_data },
+	{ .compatible = "fsl,imx51-wdt",  .data = &imx25_wdt_data },
+	{ .compatible = "fsl,imx53-wdt",  .data = &imx25_wdt_data },
+	{ .compatible = "fsl,imx6q-wdt",  .data = &imx25_wdt_data },
+	{ .compatible = "fsl,imx6sl-wdt", .data = &imx25_wdt_data },
+	{ .compatible = "fsl,imx6sx-wdt", .data = &imx25_wdt_data },
+	{ .compatible = "fsl,imx6ul-wdt", .data = &imx25_wdt_data },
+	{ .compatible = "fsl,imx7d-wdt",  .data = &imx25_wdt_data },
+	{ .compatible = "fsl,vf610-wdt",  .data = &imx25_wdt_data },
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, imx2_wdt_dt_ids);
-- 
2.8.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Guenter Roeck]

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-ChpfBGZJDbMAvxtiuMwx3w@public.gmane.org>
> ---
>  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

> +	{ .compatible = "fsl,imx25-wdt",  .data = &imx25_wdt_data },
> +	{ .compatible = "fsl,imx27-wdt",  .data = &imx21_wdt_data },
> +	{ .compatible = "fsl,imx31-wdt",  .data = &imx21_wdt_data },
> +	{ .compatible = "fsl,imx35-wdt",  .data = &imx25_wdt_data },
> +	{ .compatible = "fsl,imx50-wdt",  .data = &imx25_wdt_data },
> +	{ .compatible = "fsl,imx51-wdt",  .data = &imx25_wdt_data },
> +	{ .compatible = "fsl,imx53-wdt",  .data = &imx25_wdt_data },
> +	{ .compatible = "fsl,imx6q-wdt",  .data = &imx25_wdt_data },
> +	{ .compatible = "fsl,imx6sl-wdt", .data = &imx25_wdt_data },
> +	{ .compatible = "fsl,imx6sx-wdt", .data = &imx25_wdt_data },
> +	{ .compatible = "fsl,imx6ul-wdt", .data = &imx25_wdt_data },
> +	{ .compatible = "fsl,imx7d-wdt",  .data = &imx25_wdt_data },
> +	{ .compatible = "fsl,vf610-wdt",  .data = &imx25_wdt_data },
>  	{ /* sentinel */ }
>  };
>  MODULE_DEVICE_TABLE(of, imx2_wdt_dt_ids);
>

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [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-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org> 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-ChpfBGZJDbMAvxtiuMwx3w@public.gmane.org>
>> ---
>>  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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Guenter Roeck]

On 12/10/2016 11:28 AM, Magnus Lilja wrote:
> Hi,
>
> On 26 September 2016 at 15:02, Guenter Roeck <linux-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org> 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-ChpfBGZJDbMAvxtiuMwx3w@public.gmane.org>
>>> ---
>>>  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.
>

I had asked for a change, and I don't recall seeing v2. So, at least as far
as I know, the answer is no.

Guenter

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Vladimir Zapolskiy]

Hi Magnus,

On 12/10/2016 09:28 PM, Magnus Lilja wrote:
> Hi,
>
> On 26 September 2016 at 15:02, Guenter Roeck <linux-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org> 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-ChpfBGZJDbMAvxtiuMwx3w@public.gmane.org>
>>> ---

[snip]

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

no, it's not in the linux-watchdog repository at the moment, I'm going
to fix Guenter's review comments and send the change today or tomorrow
in hope that it enters v4.10.

Thank you for testing, for your information I'm on the way to send more
i.MX31 changes to get better i.MX31 device tree support.

--
With best wishes,
Vladimir
--
To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC PATCH 2/2] ARM: i.MX: dts: add fsl,imx25-wdt compatible to all relevant watchdog nodes

[Vladimir Zapolskiy]

Watchdog device controller found on all modern SoCs from i.MX series
and firstly introduced in i.MX25 is not one in one compatible with the
watchdog controllers on i.MX21, i.MX27 and i.MX31, the latter
controlles don't have WICR (and pretimeout notification support) and
WMCR registers. To get benefit from the more advanced watchdog device
and to avoid operations over non-existing registers on legacy SoCs add
fsl,imx25-wdt compatible to descriptions of all i.MX25 compatible
watchdog controllers.

Signed-off-by: Vladimir Zapolskiy <vz-ChpfBGZJDbMAvxtiuMwx3w@public.gmane.org>
---
 arch/arm/boot/dts/imx35.dtsi   |  3 ++-
 arch/arm/boot/dts/imx50.dtsi   |  3 ++-
 arch/arm/boot/dts/imx51.dtsi   |  6 ++++--
 arch/arm/boot/dts/imx53.dtsi   |  6 ++++--
 arch/arm/boot/dts/imx6qdl.dtsi |  6 ++++--
 arch/arm/boot/dts/imx6sl.dtsi  |  6 ++++--
 arch/arm/boot/dts/imx6sx.dtsi  |  9 ++++++---
 arch/arm/boot/dts/imx6ul.dtsi  |  6 ++++--
 arch/arm/boot/dts/imx7s.dtsi   | 12 ++++++++----
 arch/arm/boot/dts/ls1021a.dtsi |  2 +-
 arch/arm/boot/dts/vfxxx.dtsi   |  3 ++-
 11 files changed, 41 insertions(+), 21 deletions(-)

diff --git a/arch/arm/boot/dts/imx35.dtsi b/arch/arm/boot/dts/imx35.dtsi
index 490b7b4..8fd4482 100644
--- a/arch/arm/boot/dts/imx35.dtsi
+++ b/arch/arm/boot/dts/imx35.dtsi
@@ -284,7 +284,8 @@
 			};
 
 			wdog: wdog@53fdc000 {
-				compatible = "fsl,imx35-wdt", "fsl,imx21-wdt";
+				compatible = "fsl,imx35-wdt", "fsl,imx25-wdt",
+					     "fsl,imx21-wdt";
 				reg = <0x53fdc000 0x4000>;
 				clocks = <&clks 74>;
 				clock-names = "";
diff --git a/arch/arm/boot/dts/imx50.dtsi b/arch/arm/boot/dts/imx50.dtsi
index e245713..5ba6d5a 100644
--- a/arch/arm/boot/dts/imx50.dtsi
+++ b/arch/arm/boot/dts/imx50.dtsi
@@ -260,7 +260,8 @@
 			};
 
 			wdog1: wdog@53f98000 {
-				compatible = "fsl,imx50-wdt", "fsl,imx21-wdt";
+				compatible = "fsl,imx50-wdt", "fsl,imx25-wdt",
+					     "fsl,imx21-wdt";
 				reg = <0x53f98000 0x4000>;
 				interrupts = <58>;
 				clocks = <&clks IMX5_CLK_DUMMY>;
diff --git a/arch/arm/boot/dts/imx51.dtsi b/arch/arm/boot/dts/imx51.dtsi
index f46fe9b..d91f713 100644
--- a/arch/arm/boot/dts/imx51.dtsi
+++ b/arch/arm/boot/dts/imx51.dtsi
@@ -345,14 +345,16 @@
 			};
 
 			wdog1: wdog@73f98000 {
-				compatible = "fsl,imx51-wdt", "fsl,imx21-wdt";
+				compatible = "fsl,imx51-wdt", "fsl,imx25-wdt",
+					     "fsl,imx21-wdt";
 				reg = <0x73f98000 0x4000>;
 				interrupts = <58>;
 				clocks = <&clks IMX5_CLK_DUMMY>;
 			};
 
 			wdog2: wdog@73f9c000 {
-				compatible = "fsl,imx51-wdt", "fsl,imx21-wdt";
+				compatible = "fsl,imx51-wdt", "fsl,imx25-wdt",
+					     "fsl,imx21-wdt";
 				reg = <0x73f9c000 0x4000>;
 				interrupts = <59>;
 				clocks = <&clks IMX5_CLK_DUMMY>;
diff --git a/arch/arm/boot/dts/imx53.dtsi b/arch/arm/boot/dts/imx53.dtsi
index cd17037..c9edac2 100644
--- a/arch/arm/boot/dts/imx53.dtsi
+++ b/arch/arm/boot/dts/imx53.dtsi
@@ -390,14 +390,16 @@
 			};
 
 			wdog1: wdog@53f98000 {
-				compatible = "fsl,imx53-wdt", "fsl,imx21-wdt";
+				compatible = "fsl,imx53-wdt", "fsl,imx25-wdt",
+					     "fsl,imx21-wdt";
 				reg = <0x53f98000 0x4000>;
 				interrupts = <58>;
 				clocks = <&clks IMX5_CLK_DUMMY>;
 			};
 
 			wdog2: wdog@53f9c000 {
-				compatible = "fsl,imx53-wdt", "fsl,imx21-wdt";
+				compatible = "fsl,imx53-wdt", "fsl,imx25-wdt",
+					     "fsl,imx21-wdt";
 				reg = <0x53f9c000 0x4000>;
 				interrupts = <59>;
 				clocks = <&clks IMX5_CLK_DUMMY>;
diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
index b620ac8..d73edd7 100644
--- a/arch/arm/boot/dts/imx6qdl.dtsi
+++ b/arch/arm/boot/dts/imx6qdl.dtsi
@@ -593,14 +593,16 @@
 			};
 
 			wdog1: wdog@020bc000 {
-				compatible = "fsl,imx6q-wdt", "fsl,imx21-wdt";
+				compatible = "fsl,imx6q-wdt", "fsl,imx25-wdt",
+					     "fsl,imx21-wdt";
 				reg = <0x020bc000 0x4000>;
 				interrupts = <0 80 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clks IMX6QDL_CLK_DUMMY>;
 			};
 
 			wdog2: wdog@020c0000 {
-				compatible = "fsl,imx6q-wdt", "fsl,imx21-wdt";
+				compatible = "fsl,imx6q-wdt", "fsl,imx25-wdt",
+					     "fsl,imx21-wdt";
 				reg = <0x020c0000 0x4000>;
 				interrupts = <0 81 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clks IMX6QDL_CLK_DUMMY>;
diff --git a/arch/arm/boot/dts/imx6sl.dtsi b/arch/arm/boot/dts/imx6sl.dtsi
index 5425150..b8c71bd 100644
--- a/arch/arm/boot/dts/imx6sl.dtsi
+++ b/arch/arm/boot/dts/imx6sl.dtsi
@@ -430,14 +430,16 @@
 			};
 
 			wdog1: wdog@020bc000 {
-				compatible = "fsl,imx6sl-wdt", "fsl,imx21-wdt";
+				compatible = "fsl,imx6sl-wdt", "fsl,imx25-wdt",
+					     "fsl,imx21-wdt";
 				reg = <0x020bc000 0x4000>;
 				interrupts = <0 80 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clks IMX6SL_CLK_DUMMY>;
 			};
 
 			wdog2: wdog@020c0000 {
-				compatible = "fsl,imx6sl-wdt", "fsl,imx21-wdt";
+				compatible = "fsl,imx6sl-wdt", "fsl,imx25-wdt",
+					     "fsl,imx21-wdt";
 				reg = <0x020c0000 0x4000>;
 				interrupts = <0 81 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clks IMX6SL_CLK_DUMMY>;
diff --git a/arch/arm/boot/dts/imx6sx.dtsi b/arch/arm/boot/dts/imx6sx.dtsi
index 2863c52..2753c71 100644
--- a/arch/arm/boot/dts/imx6sx.dtsi
+++ b/arch/arm/boot/dts/imx6sx.dtsi
@@ -515,14 +515,16 @@
 			};
 
 			wdog1: wdog@020bc000 {
-				compatible = "fsl,imx6sx-wdt", "fsl,imx21-wdt";
+				compatible = "fsl,imx6sx-wdt", "fsl,imx25-wdt",
+					     "fsl,imx21-wdt";
 				reg = <0x020bc000 0x4000>;
 				interrupts = <GIC_SPI 80 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clks IMX6SX_CLK_DUMMY>;
 			};
 
 			wdog2: wdog@020c0000 {
-				compatible = "fsl,imx6sx-wdt", "fsl,imx21-wdt";
+				compatible = "fsl,imx6sx-wdt", "fsl,imx25-wdt",
+					     "fsl,imx21-wdt";
 				reg = <0x020c0000 0x4000>;
 				interrupts = <GIC_SPI 81 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clks IMX6SX_CLK_DUMMY>;
@@ -1178,7 +1180,8 @@
                         };
 
 			wdog3: wdog@02288000 {
-				compatible = "fsl,imx6sx-wdt", "fsl,imx21-wdt";
+				compatible = "fsl,imx6sx-wdt", "fsl,imx25-wdt",
+					     "fsl,imx21-wdt";
 				reg = <0x02288000 0x4000>;
 				interrupts = <GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clks IMX6SX_CLK_DUMMY>;
diff --git a/arch/arm/boot/dts/imx6ul.dtsi b/arch/arm/boot/dts/imx6ul.dtsi
index 33b95d7..fb0373f 100644
--- a/arch/arm/boot/dts/imx6ul.dtsi
+++ b/arch/arm/boot/dts/imx6ul.dtsi
@@ -483,14 +483,16 @@
 			};
 
 			wdog1: wdog@020bc000 {
-				compatible = "fsl,imx6ul-wdt", "fsl,imx21-wdt";
+				compatible = "fsl,imx6ul-wdt", "fsl,imx25-wdt",
+					     "fsl,imx21-wdt";
 				reg = <0x020bc000 0x4000>;
 				interrupts = <GIC_SPI 80 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clks IMX6UL_CLK_WDOG1>;
 			};
 
 			wdog2: wdog@020c0000 {
-				compatible = "fsl,imx6ul-wdt", "fsl,imx21-wdt";
+				compatible = "fsl,imx6ul-wdt", "fsl,imx25-wdt",
+					     "fsl,imx21-wdt";
 				reg = <0x020c0000 0x4000>;
 				interrupts = <GIC_SPI 81 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clks IMX6UL_CLK_WDOG2>;
diff --git a/arch/arm/boot/dts/imx7s.dtsi b/arch/arm/boot/dts/imx7s.dtsi
index 1e90bdb..e7c047e 100644
--- a/arch/arm/boot/dts/imx7s.dtsi
+++ b/arch/arm/boot/dts/imx7s.dtsi
@@ -394,14 +394,16 @@
 			};
 
 			wdog1: wdog@30280000 {
-				compatible = "fsl,imx7d-wdt", "fsl,imx21-wdt";
+				compatible = "fsl,imx7d-wdt", "fsl,imx25-wdt",
+					     "fsl,imx21-wdt";
 				reg = <0x30280000 0x10000>;
 				interrupts = <GIC_SPI 78 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clks IMX7D_WDOG1_ROOT_CLK>;
 			};
 
 			wdog2: wdog@30290000 {
-				compatible = "fsl,imx7d-wdt", "fsl,imx21-wdt";
+				compatible = "fsl,imx7d-wdt", "fsl,imx25-wdt",
+					     "fsl,imx21-wdt";
 				reg = <0x30290000 0x10000>;
 				interrupts = <GIC_SPI 79 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clks IMX7D_WDOG2_ROOT_CLK>;
@@ -409,7 +411,8 @@
 			};
 
 			wdog3: wdog@302a0000 {
-				compatible = "fsl,imx7d-wdt", "fsl,imx21-wdt";
+				compatible = "fsl,imx7d-wdt", "fsl,imx25-wdt",
+					     "fsl,imx21-wdt";
 				reg = <0x302a0000 0x10000>;
 				interrupts = <GIC_SPI 10 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clks IMX7D_WDOG3_ROOT_CLK>;
@@ -417,7 +420,8 @@
 			};
 
 			wdog4: wdog@302b0000 {
-				compatible = "fsl,imx7d-wdt", "fsl,imx21-wdt";
+				compatible = "fsl,imx7d-wdt", "fsl,imx25-wdt",
+					     "fsl,imx21-wdt";
 				reg = <0x302b0000 0x10000>;
 				interrupts = <GIC_SPI 109 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clks IMX7D_WDOG4_ROOT_CLK>;
diff --git a/arch/arm/boot/dts/ls1021a.dtsi b/arch/arm/boot/dts/ls1021a.dtsi
index 368e219..7e36fd8 100644
--- a/arch/arm/boot/dts/ls1021a.dtsi
+++ b/arch/arm/boot/dts/ls1021a.dtsi
@@ -441,7 +441,7 @@
 		};
 
 		wdog0: watchdog@2ad0000 {
-			compatible = "fsl,imx21-wdt";
+			compatible = "fsl,imx25-wdt", "fsl,imx21-wdt";
 			reg = <0x0 0x2ad0000 0x0 0x10000>;
 			interrupts = <GIC_SPI 115 IRQ_TYPE_LEVEL_HIGH>;
 			clocks = <&platform_clk 1>;
diff --git a/arch/arm/boot/dts/vfxxx.dtsi b/arch/arm/boot/dts/vfxxx.dtsi
index 2c13ec6..35f32ed 100644
--- a/arch/arm/boot/dts/vfxxx.dtsi
+++ b/arch/arm/boot/dts/vfxxx.dtsi
@@ -320,7 +320,8 @@
 			};
 
 			wdoga5: wdog@4003e000 {
-				compatible = "fsl,vf610-wdt", "fsl,imx21-wdt";
+				compatible = "fsl,vf610-wdt", "fsl,imx25-wdt",
+					     "fsl,imx21-wdt";
 				reg = <0x4003e000 0x1000>;
 				interrupts = <20 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clks VF610_CLK_WDT>;
-- 
2.8.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 2/2] ARM: i.MX: dts: add fsl, imx25-wdt compatible to all relevant watchdog nodes

[Baruch Siach]

Hi Vladimir,

On Mon, Sep 26, 2016 at 03:39:21AM +0300, Vladimir Zapolskiy wrote:
> Watchdog device controller found on all modern SoCs from i.MX series
> and firstly introduced in i.MX25 is not one in one compatible with the
> watchdog controllers on i.MX21, i.MX27 and i.MX31, the latter
> controlles don't have WICR (and pretimeout notification support) and
> WMCR registers. To get benefit from the more advanced watchdog device
> and to avoid operations over non-existing registers on legacy SoCs add
> fsl,imx25-wdt compatible to descriptions of all i.MX25 compatible
> watchdog controllers.

Maybe also update Documentation/devicetree/bindings/watchdog/fsl-imx-wdt.txt? 
In a separate patch.

baruch

-- 
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch-NswTu9S1W3P6gbPvEgmw2w@public.gmane.org - tel: +972.52.368.4656, http://www.tkos.co.il -
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 2/2] ARM: i.MX: dts: add fsl, imx25-wdt compatible to all relevant watchdog nodes

[Uwe Kleine-König]

Hello Vladimir,

On Mon, Sep 26, 2016 at 03:39:21AM +0300, Vladimir Zapolskiy wrote:
> Watchdog device controller found on all modern SoCs from i.MX series
> and firstly introduced in i.MX25 is not one in one compatible with the
> watchdog controllers on i.MX21, i.MX27 and i.MX31, the latter
> controlles don't have WICR (and pretimeout notification support) and
> WMCR registers. To get benefit from the more advanced watchdog device
> and to avoid operations over non-existing registers on legacy SoCs add
> fsl,imx25-wdt compatible to descriptions of all i.MX25 compatible
> watchdog controllers.
> 
> Signed-off-by: Vladimir Zapolskiy <vz-ChpfBGZJDbMAvxtiuMwx3w@public.gmane.org>
> ---
>  arch/arm/boot/dts/imx35.dtsi   |  3 ++-
>  arch/arm/boot/dts/imx50.dtsi   |  3 ++-
>  arch/arm/boot/dts/imx51.dtsi   |  6 ++++--
>  arch/arm/boot/dts/imx53.dtsi   |  6 ++++--
>  arch/arm/boot/dts/imx6qdl.dtsi |  6 ++++--
>  arch/arm/boot/dts/imx6sl.dtsi  |  6 ++++--
>  arch/arm/boot/dts/imx6sx.dtsi  |  9 ++++++---
>  arch/arm/boot/dts/imx6ul.dtsi  |  6 ++++--
>  arch/arm/boot/dts/imx7s.dtsi   | 12 ++++++++----
>  arch/arm/boot/dts/ls1021a.dtsi |  2 +-
>  arch/arm/boot/dts/vfxxx.dtsi   |  3 ++-
>  11 files changed, 41 insertions(+), 21 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/imx35.dtsi b/arch/arm/boot/dts/imx35.dtsi
> index 490b7b4..8fd4482 100644
> --- a/arch/arm/boot/dts/imx35.dtsi
> +++ b/arch/arm/boot/dts/imx35.dtsi
> @@ -284,7 +284,8 @@
>  			};
>  
>  			wdog: wdog@53fdc000 {
> -				compatible = "fsl,imx35-wdt", "fsl,imx21-wdt";
> +				compatible = "fsl,imx35-wdt", "fsl,imx25-wdt",
> +					     "fsl,imx21-wdt";

When this is used on an old kernel that doesn't know about fsl,imx25-wdt
this picks up the imx21 driver logic. As this is wrong I think you
should drop imx21-wdt here. Can one of the dt-people comfirm?

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 2/2] ARM: i.MX: dts: add fsl, imx25-wdt compatible to all relevant watchdog nodes

[Uwe Kleine-König]

Hello,

On Mon, Sep 26, 2016 at 08:27:59AM +0200, Uwe Kleine-König wrote:
> On Mon, Sep 26, 2016 at 03:39:21AM +0300, Vladimir Zapolskiy wrote:
> > Watchdog device controller found on all modern SoCs from i.MX series
> > and firstly introduced in i.MX25 is not one in one compatible with the
> > watchdog controllers on i.MX21, i.MX27 and i.MX31, the latter
> > controlles don't have WICR (and pretimeout notification support) and
> > WMCR registers. To get benefit from the more advanced watchdog device
> > and to avoid operations over non-existing registers on legacy SoCs add
> > fsl,imx25-wdt compatible to descriptions of all i.MX25 compatible
> > watchdog controllers.
> > 
> > Signed-off-by: Vladimir Zapolskiy <vz-ChpfBGZJDbMAvxtiuMwx3w@public.gmane.org>
> > ---
> >  arch/arm/boot/dts/imx35.dtsi   |  3 ++-
> >  arch/arm/boot/dts/imx50.dtsi   |  3 ++-
> >  arch/arm/boot/dts/imx51.dtsi   |  6 ++++--
> >  arch/arm/boot/dts/imx53.dtsi   |  6 ++++--
> >  arch/arm/boot/dts/imx6qdl.dtsi |  6 ++++--
> >  arch/arm/boot/dts/imx6sl.dtsi  |  6 ++++--
> >  arch/arm/boot/dts/imx6sx.dtsi  |  9 ++++++---
> >  arch/arm/boot/dts/imx6ul.dtsi  |  6 ++++--
> >  arch/arm/boot/dts/imx7s.dtsi   | 12 ++++++++----
> >  arch/arm/boot/dts/ls1021a.dtsi |  2 +-
> >  arch/arm/boot/dts/vfxxx.dtsi   |  3 ++-
> >  11 files changed, 41 insertions(+), 21 deletions(-)
> > 
> > diff --git a/arch/arm/boot/dts/imx35.dtsi b/arch/arm/boot/dts/imx35.dtsi
> > index 490b7b4..8fd4482 100644
> > --- a/arch/arm/boot/dts/imx35.dtsi
> > +++ b/arch/arm/boot/dts/imx35.dtsi
> > @@ -284,7 +284,8 @@
> >  			};
> >  
> >  			wdog: wdog@53fdc000 {
> > -				compatible = "fsl,imx35-wdt", "fsl,imx21-wdt";
> > +				compatible = "fsl,imx35-wdt", "fsl,imx25-wdt",
> > +					     "fsl,imx21-wdt";
> 
> When this is used on an old kernel that doesn't know about fsl,imx25-wdt
> this picks up the imx21 driver logic. As this is wrong I think you
> should drop imx21-wdt here. Can one of the dt-people comfirm?

I forgot in the mail at the other end of this thread that the dti were
already addressed. I (implicitly) wrote there that fsl,imx35-wdt should
be the new compatible describing the wdt with misc register. Picking
imx25 (as you did) works, too, but e.g. the CSPI device has

	compatible = "fsl,imx25-cspi", "fsl,imx35-cspi";

on the i.MX25 and

	compatible = "fsl,imx35-cspi";

on the i.MX35. So I think we should pick i.MX35 here, too, as the one
giving its name.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html