All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] watchdog: imx2: handle WMCR only being available on i.MX35 and later
@ 2017-01-09  9:50 ` Uwe Kleine-König
  0 siblings, 0 replies; 32+ messages in thread
From: Uwe Kleine-König @ 2017-01-09  9:50 UTC (permalink / raw)
  To: Vladimir Zapolskiy, Shawn Guo, Wim Van Sebroeck, Guenter Roeck,
	Fabio Estevam
  Cc: kernel, Magnus Lilja, linux-watchdog, linux-arm-kernel

Hello,

this is my approach to fix the issue reported by Vladimir Zapolskiy.

IMHO we don't need the third commit because I'm convinced most machines
just don't do anything when the WDOG signal becomes active. An affected
machine powers off 16 seconds after startup. This would be noticed
during development of the bootloader and so I assume all affected
machines having bootloaders that make the compat code unimportant.

If you want to know if your machine is affected, do:

	mw -w $(watchdog_base_addr) 0x10

The machines I tested this on (among a few customer boards a
Phycore i.MX35 and a Freescale i.MX53 Quick Start Board) don't power off.

I don't have manuals handy for ls1021a, ls1043a and ls1046a but assume
they use the i.MX35 type, too. This needs to be fixed before this series
is applied.

Compared to Vladimir's patch machines still using board files are fixed,
too.

On Thu, Dec 29, 2016 at 09:40:00PM -0200, Fabio Estevam wrote:
> On Thu, Dec 29, 2016 at 8:04 PM, Uwe Kleine-König
> <u.kleine-koenig@pengutronix.de> wrote:
> 
> > Ack. If you teach the driver now about i.MX35 and only make use of the WMCR
> > register on this one, the possible breakage is that this register isn't
> > written to on the i.MX35 type until the dtb is updated.
> 
> So basically you are saying that you don't care about old dtb compatibility.

No, I weight old dtb compatibility which IMHO doesn't buy us anything in
this case against maintenance overhead.

> As far as I can see this is the reason we have not made any progress
> with this patch since September.

I want to disagree with this. I replied the first time with my concerns
about the compat code on December 11, so I'm not responsible for the
time before that date. Also on December 23 I said that if Vladimir
really wants that compat code he should add it. The delay between
December 11 and December 23 isn't mine either. In this thread I am a
reviewer and so it's unfair to point at me when wailing about delays. I
mentioned my doubts and if you don't agree or understand the patch
submitter is free to discuss or ask the maintainer to overrule my
opinion.

Best regards
Uwe

Uwe Kleine-König (3):
  watchdog: imx2: Only i.MX35 and later have a WMCR register
  dts: teach newer i.MX machines to have the i.MX35 type watchdog
  watchdog: imx2: add compatibility for new i.MX35 type watchdog

 .../devicetree/bindings/watchdog/fsl-imx-wdt.txt   |  2 +-
 arch/arm/boot/dts/imx25.dtsi                       |  2 +-
 arch/arm/boot/dts/imx35.dtsi                       |  2 +-
 arch/arm/boot/dts/imx50.dtsi                       |  2 +-
 arch/arm/boot/dts/imx51.dtsi                       |  2 +-
 arch/arm/boot/dts/imx53.dtsi                       |  2 +-
 arch/arm/boot/dts/imx6qdl.dtsi                     |  2 +-
 arch/arm/boot/dts/imx6sl.dtsi                      |  4 +-
 arch/arm/boot/dts/imx6sx.dtsi                      |  6 +-
 arch/arm/boot/dts/imx6ul.dtsi                      |  4 +-
 arch/arm/boot/dts/imx7s.dtsi                       |  8 +-
 arch/arm/boot/dts/vfxxx.dtsi                       |  2 +-
 arch/arm/mach-imx/devices/devices-common.h         |  1 +
 arch/arm/mach-imx/devices/platform-imx2-wdt.c      | 13 +--
 drivers/watchdog/imx2_wdt.c                        | 97 ++++++++++++++++++++--
 15 files changed, 116 insertions(+), 33 deletions(-)

-- 
2.11.0

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH 0/3] watchdog: imx2: handle WMCR only being available on i.MX35 and later
@ 2017-01-09  9:50 ` Uwe Kleine-König
  0 siblings, 0 replies; 32+ messages in thread
From: Uwe Kleine-König @ 2017-01-09  9:50 UTC (permalink / raw)
  To: linux-arm-kernel

Hello,

this is my approach to fix the issue reported by Vladimir Zapolskiy.

IMHO we don't need the third commit because I'm convinced most machines
just don't do anything when the WDOG signal becomes active. An affected
machine powers off 16 seconds after startup. This would be noticed
during development of the bootloader and so I assume all affected
machines having bootloaders that make the compat code unimportant.

If you want to know if your machine is affected, do:

	mw -w $(watchdog_base_addr) 0x10

The machines I tested this on (among a few customer boards a
Phycore i.MX35 and a Freescale i.MX53 Quick Start Board) don't power off.

I don't have manuals handy for ls1021a, ls1043a and ls1046a but assume
they use the i.MX35 type, too. This needs to be fixed before this series
is applied.

Compared to Vladimir's patch machines still using board files are fixed,
too.

On Thu, Dec 29, 2016 at 09:40:00PM -0200, Fabio Estevam wrote:
> On Thu, Dec 29, 2016 at 8:04 PM, Uwe Kleine-K?nig
> <u.kleine-koenig@pengutronix.de> wrote:
> 
> > Ack. If you teach the driver now about i.MX35 and only make use of the WMCR
> > register on this one, the possible breakage is that this register isn't
> > written to on the i.MX35 type until the dtb is updated.
> 
> So basically you are saying that you don't care about old dtb compatibility.

No, I weight old dtb compatibility which IMHO doesn't buy us anything in
this case against maintenance overhead.

> As far as I can see this is the reason we have not made any progress
> with this patch since September.

I want to disagree with this. I replied the first time with my concerns
about the compat code on December 11, so I'm not responsible for the
time before that date. Also on December 23 I said that if Vladimir
really wants that compat code he should add it. The delay between
December 11 and December 23 isn't mine either. In this thread I am a
reviewer and so it's unfair to point at me when wailing about delays. I
mentioned my doubts and if you don't agree or understand the patch
submitter is free to discuss or ask the maintainer to overrule my
opinion.

Best regards
Uwe

Uwe Kleine-K?nig (3):
  watchdog: imx2: Only i.MX35 and later have a WMCR register
  dts: teach newer i.MX machines to have the i.MX35 type watchdog
  watchdog: imx2: add compatibility for new i.MX35 type watchdog

 .../devicetree/bindings/watchdog/fsl-imx-wdt.txt   |  2 +-
 arch/arm/boot/dts/imx25.dtsi                       |  2 +-
 arch/arm/boot/dts/imx35.dtsi                       |  2 +-
 arch/arm/boot/dts/imx50.dtsi                       |  2 +-
 arch/arm/boot/dts/imx51.dtsi                       |  2 +-
 arch/arm/boot/dts/imx53.dtsi                       |  2 +-
 arch/arm/boot/dts/imx6qdl.dtsi                     |  2 +-
 arch/arm/boot/dts/imx6sl.dtsi                      |  4 +-
 arch/arm/boot/dts/imx6sx.dtsi                      |  6 +-
 arch/arm/boot/dts/imx6ul.dtsi                      |  4 +-
 arch/arm/boot/dts/imx7s.dtsi                       |  8 +-
 arch/arm/boot/dts/vfxxx.dtsi                       |  2 +-
 arch/arm/mach-imx/devices/devices-common.h         |  1 +
 arch/arm/mach-imx/devices/platform-imx2-wdt.c      | 13 +--
 drivers/watchdog/imx2_wdt.c                        | 97 ++++++++++++++++++++--
 15 files changed, 116 insertions(+), 33 deletions(-)

-- 
2.11.0

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH 1/3] watchdog: imx2: Only i.MX35 and later have a WMCR register
  2017-01-09  9:50 ` Uwe Kleine-König
@ 2017-01-09  9:50   ` Uwe Kleine-König
  -1 siblings, 0 replies; 32+ messages in thread
From: Uwe Kleine-König @ 2017-01-09  9:50 UTC (permalink / raw)
  To: Vladimir Zapolskiy, Shawn Guo, Wim Van Sebroeck, Guenter Roeck,
	Fabio Estevam
  Cc: kernel, Magnus Lilja, linux-watchdog, linux-arm-kernel

Commit 5fe65ce7ccbb ("watchdog: imx2_wdt: Disable power down counter on
boot") introduced a write to the WMCR register that doesn't exist on
i.MX21, i.MX27 and i.MX31 and so makes the SoC hang during probe.

So teach the driver to differentiate between these two types. Note that
this effectively undoes commit 5fe65ce7ccbb for machines using dt until
their dtb is updated accordingly. This is critical iff the bootloader
doesn't disable the power down counter and the #WDOG signal actually
does something to the machine.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 arch/arm/mach-imx/devices/devices-common.h    |  1 +
 arch/arm/mach-imx/devices/platform-imx2-wdt.c | 13 +++---
 drivers/watchdog/imx2_wdt.c                   | 59 +++++++++++++++++++++++----
 3 files changed, 59 insertions(+), 14 deletions(-)

diff --git a/arch/arm/mach-imx/devices/devices-common.h b/arch/arm/mach-imx/devices/devices-common.h
index 6920e356f4e5..946cca627018 100644
--- a/arch/arm/mach-imx/devices/devices-common.h
+++ b/arch/arm/mach-imx/devices/devices-common.h
@@ -92,6 +92,7 @@ struct platform_device *__init imx_add_imx27_coda(
 
 struct imx_imx2_wdt_data {
 	int id;
+	const char *devname;
 	resource_size_t iobase;
 	resource_size_t iosize;
 };
diff --git a/arch/arm/mach-imx/devices/platform-imx2-wdt.c b/arch/arm/mach-imx/devices/platform-imx2-wdt.c
index 8c134c8d7500..d8d3dd58fcfe 100644
--- a/arch/arm/mach-imx/devices/platform-imx2-wdt.c
+++ b/arch/arm/mach-imx/devices/platform-imx2-wdt.c
@@ -11,9 +11,10 @@
 #include "../hardware.h"
 #include "devices-common.h"
 
-#define imx_imx2_wdt_data_entry_single(soc, _id, _hwid, _size)		\
+#define imx_imx2_wdt_data_entry_single(soc, _id, _devname, _hwid, _size)\
 	{								\
 		.id = _id,						\
+		.devname = _devname,					\
 		.iobase = soc ## _WDOG ## _hwid ## _BASE_ADDR,		\
 		.iosize = _size,					\
 	}
@@ -22,22 +23,22 @@
 
 #ifdef CONFIG_SOC_IMX21
 const struct imx_imx2_wdt_data imx21_imx2_wdt_data __initconst =
-	imx_imx2_wdt_data_entry_single(MX21, 0, , SZ_4K);
+	imx_imx2_wdt_data_entry_single(MX21, 0, "imx21-wdt", , SZ_4K);
 #endif /* ifdef CONFIG_SOC_IMX21 */
 
 #ifdef CONFIG_SOC_IMX27
 const struct imx_imx2_wdt_data imx27_imx2_wdt_data __initconst =
-	imx_imx2_wdt_data_entry_single(MX27, 0, , SZ_4K);
+	imx_imx2_wdt_data_entry_single(MX27, 0, "imx21-wdt", , SZ_4K);
 #endif /* ifdef CONFIG_SOC_IMX27 */
 
 #ifdef CONFIG_SOC_IMX31
 const struct imx_imx2_wdt_data imx31_imx2_wdt_data __initconst =
-	imx_imx2_wdt_data_entry_single(MX31, 0, , SZ_16K);
+	imx_imx2_wdt_data_entry_single(MX31, 0, "imx21-wdt", , SZ_16K);
 #endif /* ifdef CONFIG_SOC_IMX31 */
 
 #ifdef CONFIG_SOC_IMX35
 const struct imx_imx2_wdt_data imx35_imx2_wdt_data __initconst =
-	imx_imx2_wdt_data_entry_single(MX35, 0, , SZ_16K);
+	imx_imx2_wdt_data_entry_single(MX35, 0, "imx35-wdt", , SZ_16K);
 #endif /* ifdef CONFIG_SOC_IMX35 */
 
 struct platform_device *__init imx_add_imx2_wdt(
@@ -50,6 +51,6 @@ struct platform_device *__init imx_add_imx2_wdt(
 			.flags = IORESOURCE_MEM,
 		},
 	};
-	return imx_add_platform_device("imx2-wdt", data->id,
+	return imx_add_platform_device(data->devname, data->id,
 			res, ARRAY_SIZE(res), NULL, 0);
 }
diff --git a/drivers/watchdog/imx2_wdt.c b/drivers/watchdog/imx2_wdt.c
index 4874b0f18650..509b2fedb112 100644
--- a/drivers/watchdog/imx2_wdt.c
+++ b/drivers/watchdog/imx2_wdt.c
@@ -30,6 +30,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>
@@ -63,6 +64,18 @@
 
 #define WDOG_SEC_TO_COUNT(s)	((s * 2 - 1) << 8)
 
+struct imx2_wdt_type {
+	bool has_WMCR;
+};
+
+static const struct imx2_wdt_type imx2_wdt_type_imx21 = {
+	.has_WMCR = false,
+};
+
+static const struct imx2_wdt_type imx2_wdt_type_imx35 = {
+	.has_WMCR = true,
+};
+
 struct imx2_wdt_device {
 	struct clk *clk;
 	struct regmap *regmap;
@@ -248,6 +261,7 @@ static int __init imx2_wdt_probe(struct platform_device *pdev)
 	void __iomem *base;
 	int ret;
 	u32 val;
+	const struct imx2_wdt_type *type;
 
 	wdev = devm_kzalloc(&pdev->dev, sizeof(*wdev), GFP_KERNEL);
 	if (!wdev)
@@ -309,12 +323,21 @@ static int __init imx2_wdt_probe(struct platform_device *pdev)
 		set_bit(WDOG_HW_RUNNING, &wdog->status);
 	}
 
-	/*
-	 * 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);
+	type = of_device_get_match_data(&pdev->dev);
+	if (!type)
+		type = (void *)pdev->id_entry->driver_data;
+
+	if (type) {
+		if (type->has_WMCR)
+			/*
+			 * 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);
+	} else {
+		dev_warn(&pdev->dev, "Cannot determine if WMCR is present\n");
+	}
 
 	ret = watchdog_register_device(wdog);
 	if (ret) {
@@ -411,9 +434,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 platform_device_id imx2_wdt_devtype[] = {
+	{
+		.name = "imx21-wdt",
+		.driver_data = (kernel_ulong_t) &imx2_wdt_type_imx21,
+	}, {
+		.name = "imx35-wdt",
+		.driver_data = (kernel_ulong_t) &imx2_wdt_type_imx35,
+	}, {
+		/* sentinel */
+	}
+};
+MODULE_DEVICE_TABLE(platform, imx2_wdt_devtype);
+
 static const struct of_device_id imx2_wdt_dt_ids[] = {
-	{ .compatible = "fsl,imx21-wdt", },
-	{ /* sentinel */ }
+	{
+		.compatible = "fsl,imx21-wdt",
+		.data = &imx2_wdt_type_imx21,
+	}, {
+		.compatible = "fsl,imx35-wdt",
+		.data = &imx2_wdt_type_imx35,
+	}, {
+		/* sentinel */
+	}
 };
 MODULE_DEVICE_TABLE(of, imx2_wdt_dt_ids);
 
-- 
2.11.0


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 1/3] watchdog: imx2: Only i.MX35 and later have a WMCR register
@ 2017-01-09  9:50   ` Uwe Kleine-König
  0 siblings, 0 replies; 32+ messages in thread
From: Uwe Kleine-König @ 2017-01-09  9:50 UTC (permalink / raw)
  To: linux-arm-kernel

Commit 5fe65ce7ccbb ("watchdog: imx2_wdt: Disable power down counter on
boot") introduced a write to the WMCR register that doesn't exist on
i.MX21, i.MX27 and i.MX31 and so makes the SoC hang during probe.

So teach the driver to differentiate between these two types. Note that
this effectively undoes commit 5fe65ce7ccbb for machines using dt until
their dtb is updated accordingly. This is critical iff the bootloader
doesn't disable the power down counter and the #WDOG signal actually
does something to the machine.

Signed-off-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
---
 arch/arm/mach-imx/devices/devices-common.h    |  1 +
 arch/arm/mach-imx/devices/platform-imx2-wdt.c | 13 +++---
 drivers/watchdog/imx2_wdt.c                   | 59 +++++++++++++++++++++++----
 3 files changed, 59 insertions(+), 14 deletions(-)

diff --git a/arch/arm/mach-imx/devices/devices-common.h b/arch/arm/mach-imx/devices/devices-common.h
index 6920e356f4e5..946cca627018 100644
--- a/arch/arm/mach-imx/devices/devices-common.h
+++ b/arch/arm/mach-imx/devices/devices-common.h
@@ -92,6 +92,7 @@ struct platform_device *__init imx_add_imx27_coda(
 
 struct imx_imx2_wdt_data {
 	int id;
+	const char *devname;
 	resource_size_t iobase;
 	resource_size_t iosize;
 };
diff --git a/arch/arm/mach-imx/devices/platform-imx2-wdt.c b/arch/arm/mach-imx/devices/platform-imx2-wdt.c
index 8c134c8d7500..d8d3dd58fcfe 100644
--- a/arch/arm/mach-imx/devices/platform-imx2-wdt.c
+++ b/arch/arm/mach-imx/devices/platform-imx2-wdt.c
@@ -11,9 +11,10 @@
 #include "../hardware.h"
 #include "devices-common.h"
 
-#define imx_imx2_wdt_data_entry_single(soc, _id, _hwid, _size)		\
+#define imx_imx2_wdt_data_entry_single(soc, _id, _devname, _hwid, _size)\
 	{								\
 		.id = _id,						\
+		.devname = _devname,					\
 		.iobase = soc ## _WDOG ## _hwid ## _BASE_ADDR,		\
 		.iosize = _size,					\
 	}
@@ -22,22 +23,22 @@
 
 #ifdef CONFIG_SOC_IMX21
 const struct imx_imx2_wdt_data imx21_imx2_wdt_data __initconst =
-	imx_imx2_wdt_data_entry_single(MX21, 0, , SZ_4K);
+	imx_imx2_wdt_data_entry_single(MX21, 0, "imx21-wdt", , SZ_4K);
 #endif /* ifdef CONFIG_SOC_IMX21 */
 
 #ifdef CONFIG_SOC_IMX27
 const struct imx_imx2_wdt_data imx27_imx2_wdt_data __initconst =
-	imx_imx2_wdt_data_entry_single(MX27, 0, , SZ_4K);
+	imx_imx2_wdt_data_entry_single(MX27, 0, "imx21-wdt", , SZ_4K);
 #endif /* ifdef CONFIG_SOC_IMX27 */
 
 #ifdef CONFIG_SOC_IMX31
 const struct imx_imx2_wdt_data imx31_imx2_wdt_data __initconst =
-	imx_imx2_wdt_data_entry_single(MX31, 0, , SZ_16K);
+	imx_imx2_wdt_data_entry_single(MX31, 0, "imx21-wdt", , SZ_16K);
 #endif /* ifdef CONFIG_SOC_IMX31 */
 
 #ifdef CONFIG_SOC_IMX35
 const struct imx_imx2_wdt_data imx35_imx2_wdt_data __initconst =
-	imx_imx2_wdt_data_entry_single(MX35, 0, , SZ_16K);
+	imx_imx2_wdt_data_entry_single(MX35, 0, "imx35-wdt", , SZ_16K);
 #endif /* ifdef CONFIG_SOC_IMX35 */
 
 struct platform_device *__init imx_add_imx2_wdt(
@@ -50,6 +51,6 @@ struct platform_device *__init imx_add_imx2_wdt(
 			.flags = IORESOURCE_MEM,
 		},
 	};
-	return imx_add_platform_device("imx2-wdt", data->id,
+	return imx_add_platform_device(data->devname, data->id,
 			res, ARRAY_SIZE(res), NULL, 0);
 }
diff --git a/drivers/watchdog/imx2_wdt.c b/drivers/watchdog/imx2_wdt.c
index 4874b0f18650..509b2fedb112 100644
--- a/drivers/watchdog/imx2_wdt.c
+++ b/drivers/watchdog/imx2_wdt.c
@@ -30,6 +30,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>
@@ -63,6 +64,18 @@
 
 #define WDOG_SEC_TO_COUNT(s)	((s * 2 - 1) << 8)
 
+struct imx2_wdt_type {
+	bool has_WMCR;
+};
+
+static const struct imx2_wdt_type imx2_wdt_type_imx21 = {
+	.has_WMCR = false,
+};
+
+static const struct imx2_wdt_type imx2_wdt_type_imx35 = {
+	.has_WMCR = true,
+};
+
 struct imx2_wdt_device {
 	struct clk *clk;
 	struct regmap *regmap;
@@ -248,6 +261,7 @@ static int __init imx2_wdt_probe(struct platform_device *pdev)
 	void __iomem *base;
 	int ret;
 	u32 val;
+	const struct imx2_wdt_type *type;
 
 	wdev = devm_kzalloc(&pdev->dev, sizeof(*wdev), GFP_KERNEL);
 	if (!wdev)
@@ -309,12 +323,21 @@ static int __init imx2_wdt_probe(struct platform_device *pdev)
 		set_bit(WDOG_HW_RUNNING, &wdog->status);
 	}
 
-	/*
-	 * 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);
+	type = of_device_get_match_data(&pdev->dev);
+	if (!type)
+		type = (void *)pdev->id_entry->driver_data;
+
+	if (type) {
+		if (type->has_WMCR)
+			/*
+			 * 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);
+	} else {
+		dev_warn(&pdev->dev, "Cannot determine if WMCR is present\n");
+	}
 
 	ret = watchdog_register_device(wdog);
 	if (ret) {
@@ -411,9 +434,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 platform_device_id imx2_wdt_devtype[] = {
+	{
+		.name = "imx21-wdt",
+		.driver_data = (kernel_ulong_t) &imx2_wdt_type_imx21,
+	}, {
+		.name = "imx35-wdt",
+		.driver_data = (kernel_ulong_t) &imx2_wdt_type_imx35,
+	}, {
+		/* sentinel */
+	}
+};
+MODULE_DEVICE_TABLE(platform, imx2_wdt_devtype);
+
 static const struct of_device_id imx2_wdt_dt_ids[] = {
-	{ .compatible = "fsl,imx21-wdt", },
-	{ /* sentinel */ }
+	{
+		.compatible = "fsl,imx21-wdt",
+		.data = &imx2_wdt_type_imx21,
+	}, {
+		.compatible = "fsl,imx35-wdt",
+		.data = &imx2_wdt_type_imx35,
+	}, {
+		/* sentinel */
+	}
 };
 MODULE_DEVICE_TABLE(of, imx2_wdt_dt_ids);
 
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 2/3] dts: teach newer i.MX machines to have the i.MX35 type watchdog
  2017-01-09  9:50 ` Uwe Kleine-König
@ 2017-01-09  9:50   ` Uwe Kleine-König
  -1 siblings, 0 replies; 32+ messages in thread
From: Uwe Kleine-König @ 2017-01-09  9:50 UTC (permalink / raw)
  To: Vladimir Zapolskiy, Shawn Guo, Wim Van Sebroeck, Guenter Roeck,
	Fabio Estevam
  Cc: kernel, Magnus Lilja, linux-watchdog, linux-arm-kernel

Only i.MX35 and newer feature a WMCR register that should be written to. Older
SoCs hang when this address is written.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 Documentation/devicetree/bindings/watchdog/fsl-imx-wdt.txt | 2 +-
 arch/arm/boot/dts/imx25.dtsi                               | 2 +-
 arch/arm/boot/dts/imx35.dtsi                               | 2 +-
 arch/arm/boot/dts/imx50.dtsi                               | 2 +-
 arch/arm/boot/dts/imx51.dtsi                               | 2 +-
 arch/arm/boot/dts/imx53.dtsi                               | 2 +-
 arch/arm/boot/dts/imx6qdl.dtsi                             | 2 +-
 arch/arm/boot/dts/imx6sl.dtsi                              | 4 ++--
 arch/arm/boot/dts/imx6sx.dtsi                              | 6 +++---
 arch/arm/boot/dts/imx6ul.dtsi                              | 4 ++--
 arch/arm/boot/dts/imx7s.dtsi                               | 8 ++++----
 arch/arm/boot/dts/vfxxx.dtsi                               | 2 +-
 12 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/Documentation/devicetree/bindings/watchdog/fsl-imx-wdt.txt b/Documentation/devicetree/bindings/watchdog/fsl-imx-wdt.txt
index 107280ef0025..607b010a607a 100644
--- a/Documentation/devicetree/bindings/watchdog/fsl-imx-wdt.txt
+++ b/Documentation/devicetree/bindings/watchdog/fsl-imx-wdt.txt
@@ -15,7 +15,7 @@ Optional properties:
 Examples:
 
 wdt@73f98000 {
-	compatible = "fsl,imx51-wdt", "fsl,imx21-wdt";
+	compatible = "fsl,imx51-wdt", "fsl,imx35-wdt";
 	reg = <0x73f98000 0x4000>;
 	interrupts = <58>;
 	big-endian;
diff --git a/arch/arm/boot/dts/imx25.dtsi b/arch/arm/boot/dts/imx25.dtsi
index 831d09a28155..d9ef338e8af7 100644
--- a/arch/arm/boot/dts/imx25.dtsi
+++ b/arch/arm/boot/dts/imx25.dtsi
@@ -504,7 +504,7 @@
 			};
 
 			wdog@53fdc000 {
-				compatible = "fsl,imx25-wdt", "fsl,imx21-wdt";
+				compatible = "fsl,imx25-wdt", "fsl,imx35-wdt";
 				reg = <0x53fdc000 0x4000>;
 				clocks = <&clks 126>;
 				clock-names = "";
diff --git a/arch/arm/boot/dts/imx35.dtsi b/arch/arm/boot/dts/imx35.dtsi
index 9f40e6229189..d9a4e77b74d8 100644
--- a/arch/arm/boot/dts/imx35.dtsi
+++ b/arch/arm/boot/dts/imx35.dtsi
@@ -286,7 +286,7 @@
 			};
 
 			wdog: wdog@53fdc000 {
-				compatible = "fsl,imx35-wdt", "fsl,imx21-wdt";
+				compatible = "fsl,imx35-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 fe0221e4cbf7..cf90e3d44f1c 100644
--- a/arch/arm/boot/dts/imx50.dtsi
+++ b/arch/arm/boot/dts/imx50.dtsi
@@ -270,7 +270,7 @@
 			};
 
 			wdog1: wdog@53f98000 {
-				compatible = "fsl,imx50-wdt", "fsl,imx21-wdt";
+				compatible = "fsl,imx50-wdt", "fsl,imx35-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 33526cade735..998bf2ffd90d 100644
--- a/arch/arm/boot/dts/imx51.dtsi
+++ b/arch/arm/boot/dts/imx51.dtsi
@@ -347,7 +347,7 @@
 			};
 
 			wdog1: wdog@73f98000 {
-				compatible = "fsl,imx51-wdt", "fsl,imx21-wdt";
+				compatible = "fsl,imx51-wdt", "fsl,imx35-wdt";
 				reg = <0x73f98000 0x4000>;
 				interrupts = <58>;
 				clocks = <&clks IMX5_CLK_DUMMY>;
diff --git a/arch/arm/boot/dts/imx53.dtsi b/arch/arm/boot/dts/imx53.dtsi
index ca51dc03e327..22becf17529a 100644
--- a/arch/arm/boot/dts/imx53.dtsi
+++ b/arch/arm/boot/dts/imx53.dtsi
@@ -402,7 +402,7 @@
 			};
 
 			wdog1: wdog@53f98000 {
-				compatible = "fsl,imx53-wdt", "fsl,imx21-wdt";
+				compatible = "fsl,imx53-wdt", "fsl,imx35-wdt";
 				reg = <0x53f98000 0x4000>;
 				interrupts = <58>;
 				clocks = <&clks IMX5_CLK_DUMMY>;
diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
index 89b834f3fa17..5ca0ce926ccf 100644
--- a/arch/arm/boot/dts/imx6qdl.dtsi
+++ b/arch/arm/boot/dts/imx6qdl.dtsi
@@ -594,7 +594,7 @@
 			};
 
 			wdog1: wdog@020bc000 {
-				compatible = "fsl,imx6q-wdt", "fsl,imx21-wdt";
+				compatible = "fsl,imx6q-wdt", "fsl,imx35-wdt";
 				reg = <0x020bc000 0x4000>;
 				interrupts = <0 80 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 19cbd879c448..43b10ff725e0 100644
--- a/arch/arm/boot/dts/imx6sl.dtsi
+++ b/arch/arm/boot/dts/imx6sl.dtsi
@@ -479,14 +479,14 @@
 			};
 
 			wdog1: wdog@020bc000 {
-				compatible = "fsl,imx6sl-wdt", "fsl,imx21-wdt";
+				compatible = "fsl,imx6sl-wdt", "fsl,imx35-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,imx35-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 10f333016197..94d75c55bbef 100644
--- a/arch/arm/boot/dts/imx6sx.dtsi
+++ b/arch/arm/boot/dts/imx6sx.dtsi
@@ -534,14 +534,14 @@
 			};
 
 			wdog1: wdog@020bc000 {
-				compatible = "fsl,imx6sx-wdt", "fsl,imx21-wdt";
+				compatible = "fsl,imx6sx-wdt", "fsl,imx35-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,imx35-wdt";
 				reg = <0x020c0000 0x4000>;
 				interrupts = <GIC_SPI 81 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clks IMX6SX_CLK_DUMMY>;
@@ -1201,7 +1201,7 @@
 			};
 
 			wdog3: wdog@02288000 {
-				compatible = "fsl,imx6sx-wdt", "fsl,imx21-wdt";
+				compatible = "fsl,imx6sx-wdt", "fsl,imx35-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 39845a7e0463..e3816e808cfb 100644
--- a/arch/arm/boot/dts/imx6ul.dtsi
+++ b/arch/arm/boot/dts/imx6ul.dtsi
@@ -491,14 +491,14 @@
 			};
 
 			wdog1: wdog@020bc000 {
-				compatible = "fsl,imx6ul-wdt", "fsl,imx21-wdt";
+				compatible = "fsl,imx6ul-wdt", "fsl,imx35-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,imx35-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 8ff2cbdd8f0d..1a95803485d5 100644
--- a/arch/arm/boot/dts/imx7s.dtsi
+++ b/arch/arm/boot/dts/imx7s.dtsi
@@ -399,14 +399,14 @@
 			};
 
 			wdog1: wdog@30280000 {
-				compatible = "fsl,imx7d-wdt", "fsl,imx21-wdt";
+				compatible = "fsl,imx7d-wdt", "fsl,imx35-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,imx35-wdt";
 				reg = <0x30290000 0x10000>;
 				interrupts = <GIC_SPI 79 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clks IMX7D_WDOG2_ROOT_CLK>;
@@ -414,7 +414,7 @@
 			};
 
 			wdog3: wdog@302a0000 {
-				compatible = "fsl,imx7d-wdt", "fsl,imx21-wdt";
+				compatible = "fsl,imx7d-wdt", "fsl,imx35-wdt";
 				reg = <0x302a0000 0x10000>;
 				interrupts = <GIC_SPI 10 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clks IMX7D_WDOG3_ROOT_CLK>;
@@ -422,7 +422,7 @@
 			};
 
 			wdog4: wdog@302b0000 {
-				compatible = "fsl,imx7d-wdt", "fsl,imx21-wdt";
+				compatible = "fsl,imx7d-wdt", "fsl,imx35-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/vfxxx.dtsi b/arch/arm/boot/dts/vfxxx.dtsi
index e9d28474c26a..4ce240790a3d 100644
--- a/arch/arm/boot/dts/vfxxx.dtsi
+++ b/arch/arm/boot/dts/vfxxx.dtsi
@@ -326,7 +326,7 @@
 			};
 
 			wdoga5: wdog@4003e000 {
-				compatible = "fsl,vf610-wdt", "fsl,imx21-wdt";
+				compatible = "fsl,vf610-wdt", "fsl,imx35-wdt";
 				reg = <0x4003e000 0x1000>;
 				interrupts = <20 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clks VF610_CLK_WDT>;
-- 
2.11.0


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 2/3] dts: teach newer i.MX machines to have the i.MX35 type watchdog
@ 2017-01-09  9:50   ` Uwe Kleine-König
  0 siblings, 0 replies; 32+ messages in thread
From: Uwe Kleine-König @ 2017-01-09  9:50 UTC (permalink / raw)
  To: linux-arm-kernel

Only i.MX35 and newer feature a WMCR register that should be written to. Older
SoCs hang when this address is written.

Signed-off-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
---
 Documentation/devicetree/bindings/watchdog/fsl-imx-wdt.txt | 2 +-
 arch/arm/boot/dts/imx25.dtsi                               | 2 +-
 arch/arm/boot/dts/imx35.dtsi                               | 2 +-
 arch/arm/boot/dts/imx50.dtsi                               | 2 +-
 arch/arm/boot/dts/imx51.dtsi                               | 2 +-
 arch/arm/boot/dts/imx53.dtsi                               | 2 +-
 arch/arm/boot/dts/imx6qdl.dtsi                             | 2 +-
 arch/arm/boot/dts/imx6sl.dtsi                              | 4 ++--
 arch/arm/boot/dts/imx6sx.dtsi                              | 6 +++---
 arch/arm/boot/dts/imx6ul.dtsi                              | 4 ++--
 arch/arm/boot/dts/imx7s.dtsi                               | 8 ++++----
 arch/arm/boot/dts/vfxxx.dtsi                               | 2 +-
 12 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/Documentation/devicetree/bindings/watchdog/fsl-imx-wdt.txt b/Documentation/devicetree/bindings/watchdog/fsl-imx-wdt.txt
index 107280ef0025..607b010a607a 100644
--- a/Documentation/devicetree/bindings/watchdog/fsl-imx-wdt.txt
+++ b/Documentation/devicetree/bindings/watchdog/fsl-imx-wdt.txt
@@ -15,7 +15,7 @@ Optional properties:
 Examples:
 
 wdt at 73f98000 {
-	compatible = "fsl,imx51-wdt", "fsl,imx21-wdt";
+	compatible = "fsl,imx51-wdt", "fsl,imx35-wdt";
 	reg = <0x73f98000 0x4000>;
 	interrupts = <58>;
 	big-endian;
diff --git a/arch/arm/boot/dts/imx25.dtsi b/arch/arm/boot/dts/imx25.dtsi
index 831d09a28155..d9ef338e8af7 100644
--- a/arch/arm/boot/dts/imx25.dtsi
+++ b/arch/arm/boot/dts/imx25.dtsi
@@ -504,7 +504,7 @@
 			};
 
 			wdog at 53fdc000 {
-				compatible = "fsl,imx25-wdt", "fsl,imx21-wdt";
+				compatible = "fsl,imx25-wdt", "fsl,imx35-wdt";
 				reg = <0x53fdc000 0x4000>;
 				clocks = <&clks 126>;
 				clock-names = "";
diff --git a/arch/arm/boot/dts/imx35.dtsi b/arch/arm/boot/dts/imx35.dtsi
index 9f40e6229189..d9a4e77b74d8 100644
--- a/arch/arm/boot/dts/imx35.dtsi
+++ b/arch/arm/boot/dts/imx35.dtsi
@@ -286,7 +286,7 @@
 			};
 
 			wdog: wdog at 53fdc000 {
-				compatible = "fsl,imx35-wdt", "fsl,imx21-wdt";
+				compatible = "fsl,imx35-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 fe0221e4cbf7..cf90e3d44f1c 100644
--- a/arch/arm/boot/dts/imx50.dtsi
+++ b/arch/arm/boot/dts/imx50.dtsi
@@ -270,7 +270,7 @@
 			};
 
 			wdog1: wdog at 53f98000 {
-				compatible = "fsl,imx50-wdt", "fsl,imx21-wdt";
+				compatible = "fsl,imx50-wdt", "fsl,imx35-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 33526cade735..998bf2ffd90d 100644
--- a/arch/arm/boot/dts/imx51.dtsi
+++ b/arch/arm/boot/dts/imx51.dtsi
@@ -347,7 +347,7 @@
 			};
 
 			wdog1: wdog at 73f98000 {
-				compatible = "fsl,imx51-wdt", "fsl,imx21-wdt";
+				compatible = "fsl,imx51-wdt", "fsl,imx35-wdt";
 				reg = <0x73f98000 0x4000>;
 				interrupts = <58>;
 				clocks = <&clks IMX5_CLK_DUMMY>;
diff --git a/arch/arm/boot/dts/imx53.dtsi b/arch/arm/boot/dts/imx53.dtsi
index ca51dc03e327..22becf17529a 100644
--- a/arch/arm/boot/dts/imx53.dtsi
+++ b/arch/arm/boot/dts/imx53.dtsi
@@ -402,7 +402,7 @@
 			};
 
 			wdog1: wdog at 53f98000 {
-				compatible = "fsl,imx53-wdt", "fsl,imx21-wdt";
+				compatible = "fsl,imx53-wdt", "fsl,imx35-wdt";
 				reg = <0x53f98000 0x4000>;
 				interrupts = <58>;
 				clocks = <&clks IMX5_CLK_DUMMY>;
diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
index 89b834f3fa17..5ca0ce926ccf 100644
--- a/arch/arm/boot/dts/imx6qdl.dtsi
+++ b/arch/arm/boot/dts/imx6qdl.dtsi
@@ -594,7 +594,7 @@
 			};
 
 			wdog1: wdog at 020bc000 {
-				compatible = "fsl,imx6q-wdt", "fsl,imx21-wdt";
+				compatible = "fsl,imx6q-wdt", "fsl,imx35-wdt";
 				reg = <0x020bc000 0x4000>;
 				interrupts = <0 80 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 19cbd879c448..43b10ff725e0 100644
--- a/arch/arm/boot/dts/imx6sl.dtsi
+++ b/arch/arm/boot/dts/imx6sl.dtsi
@@ -479,14 +479,14 @@
 			};
 
 			wdog1: wdog at 020bc000 {
-				compatible = "fsl,imx6sl-wdt", "fsl,imx21-wdt";
+				compatible = "fsl,imx6sl-wdt", "fsl,imx35-wdt";
 				reg = <0x020bc000 0x4000>;
 				interrupts = <0 80 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clks IMX6SL_CLK_DUMMY>;
 			};
 
 			wdog2: wdog at 020c0000 {
-				compatible = "fsl,imx6sl-wdt", "fsl,imx21-wdt";
+				compatible = "fsl,imx6sl-wdt", "fsl,imx35-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 10f333016197..94d75c55bbef 100644
--- a/arch/arm/boot/dts/imx6sx.dtsi
+++ b/arch/arm/boot/dts/imx6sx.dtsi
@@ -534,14 +534,14 @@
 			};
 
 			wdog1: wdog at 020bc000 {
-				compatible = "fsl,imx6sx-wdt", "fsl,imx21-wdt";
+				compatible = "fsl,imx6sx-wdt", "fsl,imx35-wdt";
 				reg = <0x020bc000 0x4000>;
 				interrupts = <GIC_SPI 80 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clks IMX6SX_CLK_DUMMY>;
 			};
 
 			wdog2: wdog at 020c0000 {
-				compatible = "fsl,imx6sx-wdt", "fsl,imx21-wdt";
+				compatible = "fsl,imx6sx-wdt", "fsl,imx35-wdt";
 				reg = <0x020c0000 0x4000>;
 				interrupts = <GIC_SPI 81 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clks IMX6SX_CLK_DUMMY>;
@@ -1201,7 +1201,7 @@
 			};
 
 			wdog3: wdog at 02288000 {
-				compatible = "fsl,imx6sx-wdt", "fsl,imx21-wdt";
+				compatible = "fsl,imx6sx-wdt", "fsl,imx35-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 39845a7e0463..e3816e808cfb 100644
--- a/arch/arm/boot/dts/imx6ul.dtsi
+++ b/arch/arm/boot/dts/imx6ul.dtsi
@@ -491,14 +491,14 @@
 			};
 
 			wdog1: wdog at 020bc000 {
-				compatible = "fsl,imx6ul-wdt", "fsl,imx21-wdt";
+				compatible = "fsl,imx6ul-wdt", "fsl,imx35-wdt";
 				reg = <0x020bc000 0x4000>;
 				interrupts = <GIC_SPI 80 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clks IMX6UL_CLK_WDOG1>;
 			};
 
 			wdog2: wdog at 020c0000 {
-				compatible = "fsl,imx6ul-wdt", "fsl,imx21-wdt";
+				compatible = "fsl,imx6ul-wdt", "fsl,imx35-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 8ff2cbdd8f0d..1a95803485d5 100644
--- a/arch/arm/boot/dts/imx7s.dtsi
+++ b/arch/arm/boot/dts/imx7s.dtsi
@@ -399,14 +399,14 @@
 			};
 
 			wdog1: wdog at 30280000 {
-				compatible = "fsl,imx7d-wdt", "fsl,imx21-wdt";
+				compatible = "fsl,imx7d-wdt", "fsl,imx35-wdt";
 				reg = <0x30280000 0x10000>;
 				interrupts = <GIC_SPI 78 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clks IMX7D_WDOG1_ROOT_CLK>;
 			};
 
 			wdog2: wdog at 30290000 {
-				compatible = "fsl,imx7d-wdt", "fsl,imx21-wdt";
+				compatible = "fsl,imx7d-wdt", "fsl,imx35-wdt";
 				reg = <0x30290000 0x10000>;
 				interrupts = <GIC_SPI 79 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clks IMX7D_WDOG2_ROOT_CLK>;
@@ -414,7 +414,7 @@
 			};
 
 			wdog3: wdog at 302a0000 {
-				compatible = "fsl,imx7d-wdt", "fsl,imx21-wdt";
+				compatible = "fsl,imx7d-wdt", "fsl,imx35-wdt";
 				reg = <0x302a0000 0x10000>;
 				interrupts = <GIC_SPI 10 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clks IMX7D_WDOG3_ROOT_CLK>;
@@ -422,7 +422,7 @@
 			};
 
 			wdog4: wdog at 302b0000 {
-				compatible = "fsl,imx7d-wdt", "fsl,imx21-wdt";
+				compatible = "fsl,imx7d-wdt", "fsl,imx35-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/vfxxx.dtsi b/arch/arm/boot/dts/vfxxx.dtsi
index e9d28474c26a..4ce240790a3d 100644
--- a/arch/arm/boot/dts/vfxxx.dtsi
+++ b/arch/arm/boot/dts/vfxxx.dtsi
@@ -326,7 +326,7 @@
 			};
 
 			wdoga5: wdog at 4003e000 {
-				compatible = "fsl,vf610-wdt", "fsl,imx21-wdt";
+				compatible = "fsl,vf610-wdt", "fsl,imx35-wdt";
 				reg = <0x4003e000 0x1000>;
 				interrupts = <20 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clks VF610_CLK_WDT>;
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 3/3] watchdog: imx2: add compatibility for new i.MX35 type watchdog
  2017-01-09  9:50 ` Uwe Kleine-König
@ 2017-01-09  9:50   ` Uwe Kleine-König
  -1 siblings, 0 replies; 32+ messages in thread
From: Uwe Kleine-König @ 2017-01-09  9:50 UTC (permalink / raw)
  To: Vladimir Zapolskiy, Shawn Guo, Wim Van Sebroeck, Guenter Roeck,
	Fabio Estevam
  Cc: kernel, Magnus Lilja, linux-watchdog, linux-arm-kernel

When the watchdog driver learned about the i.MX35 type watchdog a new
compatibility string was introduced. Older dtb continue to use the
old fsl,imx21-wdt string and so stop writing the WMCR register which is
critical iff the machine powers off when WDOG becomes active and the
bootloader doesn't stop the power down counter.

It's unknown which boards are affected and I guess most of them are not.
Also note this is only an issue if a new kernel is operated with an old
dtb as the SoC dtsi files are fixed accordingly.

XXX: the XXX introduced needs to be changed to HEAD~2 when got a stable commit id.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/watchdog/imx2_wdt.c | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/drivers/watchdog/imx2_wdt.c b/drivers/watchdog/imx2_wdt.c
index 509b2fedb112..f5616e2c2c58 100644
--- a/drivers/watchdog/imx2_wdt.c
+++ b/drivers/watchdog/imx2_wdt.c
@@ -454,6 +454,44 @@ static const struct of_device_id imx2_wdt_dt_ids[] = {
 	}, {
 		.compatible = "fsl,imx35-wdt",
 		.data = &imx2_wdt_type_imx35,
+	},
+	/*
+	 * The following ids are only there to make commit XXX compatible with
+	 * older device trees. The result is that iff the machine makes use of
+	 * the WDG signal (most don't, only WDG_RST is used) and the bootloader
+	 * doesn't stop the power down counter it powers off the board after 16
+	 * seconds.
+	 */
+	{
+		.compatible = "fsl,imx25-wdt",
+		.data = &imx2_wdt_type_imx35,
+	}, {
+		.compatible = "fsl,imx50-wdt",
+		.data = &imx2_wdt_type_imx35,
+	}, {
+		.compatible = "fsl,imx51-wdt",
+		.data = &imx2_wdt_type_imx35,
+	}, {
+		.compatible = "fsl,imx53-wdt",
+		.data = &imx2_wdt_type_imx35,
+	}, {
+		.compatible = "fsl,imx6q-wdt",
+		.data = &imx2_wdt_type_imx35,
+	}, {
+		.compatible = "fsl,imx6sl-wdt",
+		.data = &imx2_wdt_type_imx35,
+	}, {
+		.compatible = "fsl,imx6sx-wdt",
+		.data = &imx2_wdt_type_imx35,
+	}, {
+		.compatible = "fsl,imx6ul-wdt",
+		.data = &imx2_wdt_type_imx35,
+	}, {
+		.compatible = "fsl,imx7d-wdt",
+		.data = &imx2_wdt_type_imx35,
+	}, {
+		.compatible = "fsl,vf610-wdt",
+		.data = &imx2_wdt_type_imx35,
 	}, {
 		/* sentinel */
 	}
-- 
2.11.0


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 3/3] watchdog: imx2: add compatibility for new i.MX35 type watchdog
@ 2017-01-09  9:50   ` Uwe Kleine-König
  0 siblings, 0 replies; 32+ messages in thread
From: Uwe Kleine-König @ 2017-01-09  9:50 UTC (permalink / raw)
  To: linux-arm-kernel

When the watchdog driver learned about the i.MX35 type watchdog a new
compatibility string was introduced. Older dtb continue to use the
old fsl,imx21-wdt string and so stop writing the WMCR register which is
critical iff the machine powers off when WDOG becomes active and the
bootloader doesn't stop the power down counter.

It's unknown which boards are affected and I guess most of them are not.
Also note this is only an issue if a new kernel is operated with an old
dtb as the SoC dtsi files are fixed accordingly.

XXX: the XXX introduced needs to be changed to HEAD~2 when got a stable commit id.

Signed-off-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
---
 drivers/watchdog/imx2_wdt.c | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/drivers/watchdog/imx2_wdt.c b/drivers/watchdog/imx2_wdt.c
index 509b2fedb112..f5616e2c2c58 100644
--- a/drivers/watchdog/imx2_wdt.c
+++ b/drivers/watchdog/imx2_wdt.c
@@ -454,6 +454,44 @@ static const struct of_device_id imx2_wdt_dt_ids[] = {
 	}, {
 		.compatible = "fsl,imx35-wdt",
 		.data = &imx2_wdt_type_imx35,
+	},
+	/*
+	 * The following ids are only there to make commit XXX compatible with
+	 * older device trees. The result is that iff the machine makes use of
+	 * the WDG signal (most don't, only WDG_RST is used) and the bootloader
+	 * doesn't stop the power down counter it powers off the board after 16
+	 * seconds.
+	 */
+	{
+		.compatible = "fsl,imx25-wdt",
+		.data = &imx2_wdt_type_imx35,
+	}, {
+		.compatible = "fsl,imx50-wdt",
+		.data = &imx2_wdt_type_imx35,
+	}, {
+		.compatible = "fsl,imx51-wdt",
+		.data = &imx2_wdt_type_imx35,
+	}, {
+		.compatible = "fsl,imx53-wdt",
+		.data = &imx2_wdt_type_imx35,
+	}, {
+		.compatible = "fsl,imx6q-wdt",
+		.data = &imx2_wdt_type_imx35,
+	}, {
+		.compatible = "fsl,imx6sl-wdt",
+		.data = &imx2_wdt_type_imx35,
+	}, {
+		.compatible = "fsl,imx6sx-wdt",
+		.data = &imx2_wdt_type_imx35,
+	}, {
+		.compatible = "fsl,imx6ul-wdt",
+		.data = &imx2_wdt_type_imx35,
+	}, {
+		.compatible = "fsl,imx7d-wdt",
+		.data = &imx2_wdt_type_imx35,
+	}, {
+		.compatible = "fsl,vf610-wdt",
+		.data = &imx2_wdt_type_imx35,
 	}, {
 		/* sentinel */
 	}
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* Re: [PATCH 0/3] watchdog: imx2: handle WMCR only being available on i.MX35 and later
  2017-01-09  9:50 ` Uwe Kleine-König
@ 2017-01-09 10:55   ` Vladimir Zapolskiy
  -1 siblings, 0 replies; 32+ messages in thread
From: Vladimir Zapolskiy @ 2017-01-09 10:55 UTC (permalink / raw)
  To: Uwe Kleine-König, Shawn Guo, Wim Van Sebroeck,
	Guenter Roeck, Fabio Estevam
  Cc: kernel, Magnus Lilja, linux-watchdog, linux-arm-kernel

Hello Uwe,

On 01/09/2017 11:50 AM, Uwe Kleine-König wrote:
> Hello,
> 
> this is my approach to fix the issue reported by Vladimir Zapolskiy.
> 
> IMHO we don't need the third commit because I'm convinced most machines
> just don't do anything when the WDOG signal becomes active. An affected
> machine powers off 16 seconds after startup. This would be noticed
> during development of the bootloader and so I assume all affected
> machines having bootloaders that make the compat code unimportant.
> 
> If you want to know if your machine is affected, do:
> 
> 	mw -w $(watchdog_base_addr) 0x10

is it a command in barebox shell?

> The machines I tested this on (among a few customer boards a
> Phycore i.MX35 and a Freescale i.MX53 Quick Start Board) don't power off.
> 
> I don't have manuals handy for ls1021a, ls1043a and ls1046a but assume
> they use the i.MX35 type, too. This needs to be fixed before this series
> is applied.
> 
> Compared to Vladimir's patch machines still using board files are fixed,
> too.
> 

This addition can be done on top of my changes, in my version the driver
ignores WMCR on all machines with board files.

So now we have two competing series unfortunately...

--
With best wishes,
Vladimir

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH 0/3] watchdog: imx2: handle WMCR only being available on i.MX35 and later
@ 2017-01-09 10:55   ` Vladimir Zapolskiy
  0 siblings, 0 replies; 32+ messages in thread
From: Vladimir Zapolskiy @ 2017-01-09 10:55 UTC (permalink / raw)
  To: linux-arm-kernel

Hello Uwe,

On 01/09/2017 11:50 AM, Uwe Kleine-K?nig wrote:
> Hello,
> 
> this is my approach to fix the issue reported by Vladimir Zapolskiy.
> 
> IMHO we don't need the third commit because I'm convinced most machines
> just don't do anything when the WDOG signal becomes active. An affected
> machine powers off 16 seconds after startup. This would be noticed
> during development of the bootloader and so I assume all affected
> machines having bootloaders that make the compat code unimportant.
> 
> If you want to know if your machine is affected, do:
> 
> 	mw -w $(watchdog_base_addr) 0x10

is it a command in barebox shell?

> The machines I tested this on (among a few customer boards a
> Phycore i.MX35 and a Freescale i.MX53 Quick Start Board) don't power off.
> 
> I don't have manuals handy for ls1021a, ls1043a and ls1046a but assume
> they use the i.MX35 type, too. This needs to be fixed before this series
> is applied.
> 
> Compared to Vladimir's patch machines still using board files are fixed,
> too.
> 

This addition can be done on top of my changes, in my version the driver
ignores WMCR on all machines with board files.

So now we have two competing series unfortunately...

--
With best wishes,
Vladimir

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 0/3] watchdog: imx2: handle WMCR only being available on i.MX35 and later
  2017-01-09 10:55   ` Vladimir Zapolskiy
@ 2017-01-09 11:04     ` Uwe Kleine-König
  -1 siblings, 0 replies; 32+ messages in thread
From: Uwe Kleine-König @ 2017-01-09 11:04 UTC (permalink / raw)
  To: Vladimir Zapolskiy
  Cc: Shawn Guo, Wim Van Sebroeck, Guenter Roeck, Fabio Estevam,
	kernel, Magnus Lilja, linux-watchdog, linux-arm-kernel

Hello Vladimir,

On Mon, Jan 09, 2017 at 12:55:28PM +0200, Vladimir Zapolskiy wrote:
> On 01/09/2017 11:50 AM, Uwe Kleine-König wrote:
> > Hello,
> > 
> > this is my approach to fix the issue reported by Vladimir Zapolskiy.
> > 
> > IMHO we don't need the third commit because I'm convinced most machines
> > just don't do anything when the WDOG signal becomes active. An affected
> > machine powers off 16 seconds after startup. This would be noticed
> > during development of the bootloader and so I assume all affected
> > machines having bootloaders that make the compat code unimportant.
> > 
> > If you want to know if your machine is affected, do:
> > 
> > 	mw -w $(watchdog_base_addr) 0x10
> 
> is it a command in barebox shell?

barebox cannot do $(watchdog_base_addr), but (on i.MX25) I can do:

	mw -w 0x53fdc000 0x10

. I think the corresponding U-Boot command is:

	mw.w 0x53fdc000 0x10

> > The machines I tested this on (among a few customer boards a
> > Phycore i.MX35 and a Freescale i.MX53 Quick Start Board) don't power off.
> > 
> > I don't have manuals handy for ls1021a, ls1043a and ls1046a but assume
> > they use the i.MX35 type, too. This needs to be fixed before this series
> > is applied.
> > 
> > Compared to Vladimir's patch machines still using board files are fixed,
> > too.
> > 
> 
> This addition can be done on top of my changes, in my version the driver
> ignores WMCR on all machines with board files.
> 
> So now we have two competing series unfortunately...

With mine being the better one, right? :-) 

You didn't update your series for my concerns, so I did it myself
instead of retrying to iterate about the changes I'd like to see on your
version. Feel free to criticise my variant.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH 0/3] watchdog: imx2: handle WMCR only being available on i.MX35 and later
@ 2017-01-09 11:04     ` Uwe Kleine-König
  0 siblings, 0 replies; 32+ messages in thread
From: Uwe Kleine-König @ 2017-01-09 11:04 UTC (permalink / raw)
  To: linux-arm-kernel

Hello Vladimir,

On Mon, Jan 09, 2017 at 12:55:28PM +0200, Vladimir Zapolskiy wrote:
> On 01/09/2017 11:50 AM, Uwe Kleine-K?nig wrote:
> > Hello,
> > 
> > this is my approach to fix the issue reported by Vladimir Zapolskiy.
> > 
> > IMHO we don't need the third commit because I'm convinced most machines
> > just don't do anything when the WDOG signal becomes active. An affected
> > machine powers off 16 seconds after startup. This would be noticed
> > during development of the bootloader and so I assume all affected
> > machines having bootloaders that make the compat code unimportant.
> > 
> > If you want to know if your machine is affected, do:
> > 
> > 	mw -w $(watchdog_base_addr) 0x10
> 
> is it a command in barebox shell?

barebox cannot do $(watchdog_base_addr), but (on i.MX25) I can do:

	mw -w 0x53fdc000 0x10

. I think the corresponding U-Boot command is:

	mw.w 0x53fdc000 0x10

> > The machines I tested this on (among a few customer boards a
> > Phycore i.MX35 and a Freescale i.MX53 Quick Start Board) don't power off.
> > 
> > I don't have manuals handy for ls1021a, ls1043a and ls1046a but assume
> > they use the i.MX35 type, too. This needs to be fixed before this series
> > is applied.
> > 
> > Compared to Vladimir's patch machines still using board files are fixed,
> > too.
> > 
> 
> This addition can be done on top of my changes, in my version the driver
> ignores WMCR on all machines with board files.
> 
> So now we have two competing series unfortunately...

With mine being the better one, right? :-) 

You didn't update your series for my concerns, so I did it myself
instead of retrying to iterate about the changes I'd like to see on your
version. Feel free to criticise my variant.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 0/3] watchdog: imx2: handle WMCR only being available on i.MX35 and later
  2017-01-09  9:50 ` Uwe Kleine-König
@ 2017-01-09 17:19   ` Guenter Roeck
  -1 siblings, 0 replies; 32+ messages in thread
From: Guenter Roeck @ 2017-01-09 17:19 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Vladimir Zapolskiy, Shawn Guo, Wim Van Sebroeck, Fabio Estevam,
	kernel, Magnus Lilja, linux-watchdog, linux-arm-kernel

On Mon, Jan 09, 2017 at 10:50:36AM +0100, Uwe Kleine-König wrote:
> Hello,
> 
> this is my approach to fix the issue reported by Vladimir Zapolskiy.
> 
So ...

Unfortunately, I don't see any convergence on this issue. My take is that,
given the lack of convergence, it can't be that severe since no one feels
that it is important enough to get it fixed one way or the other to budge
from your positions.

I had the other patch in my watchdog-next branch. I'll drop it, and wait for
the two of you to agree to something. You can either agree to that something,
or try to convince Wim to accept one or the other patch set. For my part, I'll
only accept a patch or patch series which has a Signed-off: and/or Reviewed-by:
from both of you.

Maybe you can get together into a room and agree not to leave it until white
smoke comes out of the chimney. Just an idea.

Thanks,
Guenter

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH 0/3] watchdog: imx2: handle WMCR only being available on i.MX35 and later
@ 2017-01-09 17:19   ` Guenter Roeck
  0 siblings, 0 replies; 32+ messages in thread
From: Guenter Roeck @ 2017-01-09 17:19 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Jan 09, 2017 at 10:50:36AM +0100, Uwe Kleine-K?nig wrote:
> Hello,
> 
> this is my approach to fix the issue reported by Vladimir Zapolskiy.
> 
So ...

Unfortunately, I don't see any convergence on this issue. My take is that,
given the lack of convergence, it can't be that severe since no one feels
that it is important enough to get it fixed one way or the other to budge
from your positions.

I had the other patch in my watchdog-next branch. I'll drop it, and wait for
the two of you to agree to something. You can either agree to that something,
or try to convince Wim to accept one or the other patch set. For my part, I'll
only accept a patch or patch series which has a Signed-off: and/or Reviewed-by:
from both of you.

Maybe you can get together into a room and agree not to leave it until white
smoke comes out of the chimney. Just an idea.

Thanks,
Guenter

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 0/3] watchdog: imx2: handle WMCR only being available on i.MX35 and later
  2017-01-09 17:19   ` Guenter Roeck
@ 2017-01-09 20:06     ` Uwe Kleine-König
  -1 siblings, 0 replies; 32+ messages in thread
From: Uwe Kleine-König @ 2017-01-09 20:06 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Vladimir Zapolskiy, Shawn Guo, Wim Van Sebroeck, Fabio Estevam,
	kernel, Magnus Lilja, linux-watchdog, linux-arm-kernel

On Mon, Jan 09, 2017 at 09:19:59AM -0800, Guenter Roeck wrote:
> On Mon, Jan 09, 2017 at 10:50:36AM +0100, Uwe Kleine-König wrote:
> > this is my approach to fix the issue reported by Vladimir Zapolskiy.
> > 
> So ...
> 
> Unfortunately, I don't see any convergence on this issue. My take is that,
> given the lack of convergence, it can't be that severe since no one feels
> that it is important enough to get it fixed one way or the other to budge
> from your positions.

I didn't get any critic for my patch set yet apart from being a second
one to address the same problem. So until proven differently I assume
that mine is fine.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH 0/3] watchdog: imx2: handle WMCR only being available on i.MX35 and later
@ 2017-01-09 20:06     ` Uwe Kleine-König
  0 siblings, 0 replies; 32+ messages in thread
From: Uwe Kleine-König @ 2017-01-09 20:06 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Jan 09, 2017 at 09:19:59AM -0800, Guenter Roeck wrote:
> On Mon, Jan 09, 2017 at 10:50:36AM +0100, Uwe Kleine-K?nig wrote:
> > this is my approach to fix the issue reported by Vladimir Zapolskiy.
> > 
> So ...
> 
> Unfortunately, I don't see any convergence on this issue. My take is that,
> given the lack of convergence, it can't be that severe since no one feels
> that it is important enough to get it fixed one way or the other to budge
> from your positions.

I didn't get any critic for my patch set yet apart from being a second
one to address the same problem. So until proven differently I assume
that mine is fine.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 1/3] watchdog: imx2: Only i.MX35 and later have a WMCR register
  2017-01-09  9:50   ` Uwe Kleine-König
@ 2017-01-16 13:34     ` Fabio Estevam
  -1 siblings, 0 replies; 32+ messages in thread
From: Fabio Estevam @ 2017-01-16 13:34 UTC (permalink / raw)
  To: Uwe Kleine-König, Magnus Lilja
  Cc: Vladimir Zapolskiy, Shawn Guo, Wim Van Sebroeck, Guenter Roeck,
	Fabio Estevam, linux-arm-kernel, linux-watchdog, Sascha Hauer

Hi Magnus,

On Mon, Jan 9, 2017 at 7:50 AM, Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> wrote:
> Commit 5fe65ce7ccbb ("watchdog: imx2_wdt: Disable power down counter on
> boot") introduced a write to the WMCR register that doesn't exist on
> i.MX21, i.MX27 and i.MX31 and so makes the SoC hang during probe.
>
> So teach the driver to differentiate between these two types. Note that
> this effectively undoes commit 5fe65ce7ccbb for machines using dt until
> their dtb is updated accordingly. This is critical iff the bootloader
> doesn't disable the power down counter and the #WDOG signal actually
> does something to the machine.
>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Is it possible for you to try this series and let us know if it fixes
the mx31 watchdog issue?

Thanks

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH 1/3] watchdog: imx2: Only i.MX35 and later have a WMCR register
@ 2017-01-16 13:34     ` Fabio Estevam
  0 siblings, 0 replies; 32+ messages in thread
From: Fabio Estevam @ 2017-01-16 13:34 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Magnus,

On Mon, Jan 9, 2017 at 7:50 AM, Uwe Kleine-K?nig
<u.kleine-koenig@pengutronix.de> wrote:
> Commit 5fe65ce7ccbb ("watchdog: imx2_wdt: Disable power down counter on
> boot") introduced a write to the WMCR register that doesn't exist on
> i.MX21, i.MX27 and i.MX31 and so makes the SoC hang during probe.
>
> So teach the driver to differentiate between these two types. Note that
> this effectively undoes commit 5fe65ce7ccbb for machines using dt until
> their dtb is updated accordingly. This is critical iff the bootloader
> doesn't disable the power down counter and the #WDOG signal actually
> does something to the machine.
>
> Signed-off-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>

Is it possible for you to try this series and let us know if it fixes
the mx31 watchdog issue?

Thanks

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 0/3] watchdog: imx2: handle WMCR only being available on i.MX35 and later
  2017-01-09  9:50 ` Uwe Kleine-König
@ 2017-01-16 17:27   ` Magnus Lilja
  -1 siblings, 0 replies; 32+ messages in thread
From: Magnus Lilja @ 2017-01-16 17:27 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Vladimir Zapolskiy, Shawn Guo, Wim Van Sebroeck, Guenter Roeck,
	Fabio Estevam, Sascha Hauer, linux-watchdog, linux-arm-kernel

Hi,

On 9 January 2017 at 10:50, Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> wrote:
> Hello,
>
> this is my approach to fix the issue reported by Vladimir Zapolskiy.
>

...

> Best regards
> Uwe
>
> Uwe Kleine-König (3):
>   watchdog: imx2: Only i.MX35 and later have a WMCR register
>   dts: teach newer i.MX machines to have the i.MX35 type watchdog
>   watchdog: imx2: add compatibility for new i.MX35 type watchdog
>
>  .../devicetree/bindings/watchdog/fsl-imx-wdt.txt   |  2 +-
>  arch/arm/boot/dts/imx25.dtsi                       |  2 +-
>  arch/arm/boot/dts/imx35.dtsi                       |  2 +-
>  arch/arm/boot/dts/imx50.dtsi                       |  2 +-
>  arch/arm/boot/dts/imx51.dtsi                       |  2 +-
>  arch/arm/boot/dts/imx53.dtsi                       |  2 +-
>  arch/arm/boot/dts/imx6qdl.dtsi                     |  2 +-
>  arch/arm/boot/dts/imx6sl.dtsi                      |  4 +-
>  arch/arm/boot/dts/imx6sx.dtsi                      |  6 +-
>  arch/arm/boot/dts/imx6ul.dtsi                      |  4 +-
>  arch/arm/boot/dts/imx7s.dtsi                       |  8 +-
>  arch/arm/boot/dts/vfxxx.dtsi                       |  2 +-
>  arch/arm/mach-imx/devices/devices-common.h         |  1 +
>  arch/arm/mach-imx/devices/platform-imx2-wdt.c      | 13 +--
>  drivers/watchdog/imx2_wdt.c                        | 97 ++++++++++++++++++++--
>  15 files changed, 116 insertions(+), 33 deletions(-)

Tested on i.MX31 PDK board.

Tested-by: Magnus Lilja <lilja.magnus@gmail.com>

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH 0/3] watchdog: imx2: handle WMCR only being available on i.MX35 and later
@ 2017-01-16 17:27   ` Magnus Lilja
  0 siblings, 0 replies; 32+ messages in thread
From: Magnus Lilja @ 2017-01-16 17:27 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

On 9 January 2017 at 10:50, Uwe Kleine-K?nig
<u.kleine-koenig@pengutronix.de> wrote:
> Hello,
>
> this is my approach to fix the issue reported by Vladimir Zapolskiy.
>

...

> Best regards
> Uwe
>
> Uwe Kleine-K?nig (3):
>   watchdog: imx2: Only i.MX35 and later have a WMCR register
>   dts: teach newer i.MX machines to have the i.MX35 type watchdog
>   watchdog: imx2: add compatibility for new i.MX35 type watchdog
>
>  .../devicetree/bindings/watchdog/fsl-imx-wdt.txt   |  2 +-
>  arch/arm/boot/dts/imx25.dtsi                       |  2 +-
>  arch/arm/boot/dts/imx35.dtsi                       |  2 +-
>  arch/arm/boot/dts/imx50.dtsi                       |  2 +-
>  arch/arm/boot/dts/imx51.dtsi                       |  2 +-
>  arch/arm/boot/dts/imx53.dtsi                       |  2 +-
>  arch/arm/boot/dts/imx6qdl.dtsi                     |  2 +-
>  arch/arm/boot/dts/imx6sl.dtsi                      |  4 +-
>  arch/arm/boot/dts/imx6sx.dtsi                      |  6 +-
>  arch/arm/boot/dts/imx6ul.dtsi                      |  4 +-
>  arch/arm/boot/dts/imx7s.dtsi                       |  8 +-
>  arch/arm/boot/dts/vfxxx.dtsi                       |  2 +-
>  arch/arm/mach-imx/devices/devices-common.h         |  1 +
>  arch/arm/mach-imx/devices/platform-imx2-wdt.c      | 13 +--
>  drivers/watchdog/imx2_wdt.c                        | 97 ++++++++++++++++++++--
>  15 files changed, 116 insertions(+), 33 deletions(-)

Tested on i.MX31 PDK board.

Tested-by: Magnus Lilja <lilja.magnus@gmail.com>

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 1/3] watchdog: imx2: Only i.MX35 and later have a WMCR register
  2017-01-09  9:50   ` Uwe Kleine-König
@ 2017-01-16 18:34     ` Fabio Estevam
  -1 siblings, 0 replies; 32+ messages in thread
From: Fabio Estevam @ 2017-01-16 18:34 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Vladimir Zapolskiy, Shawn Guo, Wim Van Sebroeck, Guenter Roeck,
	Fabio Estevam, linux-arm-kernel, Magnus Lilja, linux-watchdog,
	Sascha Hauer

On Mon, Jan 9, 2017 at 7:50 AM, Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> wrote:
> Commit 5fe65ce7ccbb ("watchdog: imx2_wdt: Disable power down counter on
> boot") introduced a write to the WMCR register that doesn't exist on
> i.MX21, i.MX27 and i.MX31 and so makes the SoC hang during probe.
>
> So teach the driver to differentiate between these two types. Note that
> this effectively undoes commit 5fe65ce7ccbb for machines using dt until
> their dtb is updated accordingly. This is critical iff the bootloader
> doesn't disable the power down counter and the #WDOG signal actually
> does something to the machine.
>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH 1/3] watchdog: imx2: Only i.MX35 and later have a WMCR register
@ 2017-01-16 18:34     ` Fabio Estevam
  0 siblings, 0 replies; 32+ messages in thread
From: Fabio Estevam @ 2017-01-16 18:34 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Jan 9, 2017 at 7:50 AM, Uwe Kleine-K?nig
<u.kleine-koenig@pengutronix.de> wrote:
> Commit 5fe65ce7ccbb ("watchdog: imx2_wdt: Disable power down counter on
> boot") introduced a write to the WMCR register that doesn't exist on
> i.MX21, i.MX27 and i.MX31 and so makes the SoC hang during probe.
>
> So teach the driver to differentiate between these two types. Note that
> this effectively undoes commit 5fe65ce7ccbb for machines using dt until
> their dtb is updated accordingly. This is critical iff the bootloader
> doesn't disable the power down counter and the #WDOG signal actually
> does something to the machine.
>
> Signed-off-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>

Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 2/3] dts: teach newer i.MX machines to have the i.MX35 type watchdog
  2017-01-09  9:50   ` Uwe Kleine-König
@ 2017-01-16 18:35     ` Fabio Estevam
  -1 siblings, 0 replies; 32+ messages in thread
From: Fabio Estevam @ 2017-01-16 18:35 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Vladimir Zapolskiy, Shawn Guo, Wim Van Sebroeck, Guenter Roeck,
	Fabio Estevam, linux-arm-kernel, Magnus Lilja, linux-watchdog,
	Sascha Hauer

On Mon, Jan 9, 2017 at 7:50 AM, Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> wrote:
> Only i.MX35 and newer feature a WMCR register that should be written to. Older
> SoCs hang when this address is written.
>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>

Only a minor nit: Subject could: ARM: dts: imx: teach...

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH 2/3] dts: teach newer i.MX machines to have the i.MX35 type watchdog
@ 2017-01-16 18:35     ` Fabio Estevam
  0 siblings, 0 replies; 32+ messages in thread
From: Fabio Estevam @ 2017-01-16 18:35 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Jan 9, 2017 at 7:50 AM, Uwe Kleine-K?nig
<u.kleine-koenig@pengutronix.de> wrote:
> Only i.MX35 and newer feature a WMCR register that should be written to. Older
> SoCs hang when this address is written.
>
> Signed-off-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>

Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>

Only a minor nit: Subject could: ARM: dts: imx: teach...

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 3/3] watchdog: imx2: add compatibility for new i.MX35 type watchdog
  2017-01-09  9:50   ` Uwe Kleine-König
@ 2017-01-16 18:36     ` Fabio Estevam
  -1 siblings, 0 replies; 32+ messages in thread
From: Fabio Estevam @ 2017-01-16 18:36 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Vladimir Zapolskiy, Shawn Guo, Wim Van Sebroeck, Guenter Roeck,
	Fabio Estevam, linux-arm-kernel, Magnus Lilja, linux-watchdog,
	Sascha Hauer

On Mon, Jan 9, 2017 at 7:50 AM, Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> wrote:
> When the watchdog driver learned about the i.MX35 type watchdog a new
> compatibility string was introduced. Older dtb continue to use the
> old fsl,imx21-wdt string and so stop writing the WMCR register which is
> critical iff the machine powers off when WDOG becomes active and the
> bootloader doesn't stop the power down counter.
>
> It's unknown which boards are affected and I guess most of them are not.

At least imx31 pdk is affected.

> Also note this is only an issue if a new kernel is operated with an old
> dtb as the SoC dtsi files are fixed accordingly.
>
> XXX: the XXX introduced needs to be changed to HEAD~2 when got a stable commit id.

This comment should not be here.


Other than that:

Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH 3/3] watchdog: imx2: add compatibility for new i.MX35 type watchdog
@ 2017-01-16 18:36     ` Fabio Estevam
  0 siblings, 0 replies; 32+ messages in thread
From: Fabio Estevam @ 2017-01-16 18:36 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Jan 9, 2017 at 7:50 AM, Uwe Kleine-K?nig
<u.kleine-koenig@pengutronix.de> wrote:
> When the watchdog driver learned about the i.MX35 type watchdog a new
> compatibility string was introduced. Older dtb continue to use the
> old fsl,imx21-wdt string and so stop writing the WMCR register which is
> critical iff the machine powers off when WDOG becomes active and the
> bootloader doesn't stop the power down counter.
>
> It's unknown which boards are affected and I guess most of them are not.

At least imx31 pdk is affected.

> Also note this is only an issue if a new kernel is operated with an old
> dtb as the SoC dtsi files are fixed accordingly.
>
> XXX: the XXX introduced needs to be changed to HEAD~2 when got a stable commit id.

This comment should not be here.


Other than that:

Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 2/3] dts: teach newer i.MX machines to have the i.MX35 type watchdog
  2017-01-16 18:35     ` Fabio Estevam
@ 2017-01-16 20:43       ` Uwe Kleine-König
  -1 siblings, 0 replies; 32+ messages in thread
From: Uwe Kleine-König @ 2017-01-16 20:43 UTC (permalink / raw)
  To: Fabio Estevam, Shawn Guo
  Cc: linux-watchdog, Magnus Lilja, Vladimir Zapolskiy,
	Wim Van Sebroeck, Guenter Roeck, kernel, Fabio Estevam,
	linux-arm-kernel

On Mon, Jan 16, 2017 at 04:35:26PM -0200, Fabio Estevam wrote:
> On Mon, Jan 9, 2017 at 7:50 AM, Uwe Kleine-König
> <u.kleine-koenig@pengutronix.de> wrote:
> > Only i.MX35 and newer feature a WMCR register that should be written to. Older
> > SoCs hang when this address is written.
> >
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> 
> Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>
> 
> Only a minor nit: Subject could: ARM: dts: imx: teach...

Ack. To reference the commit id of the first patch in the third I can
prepare a branch to pull. There I'd fix the Subject. Shawn, is this OK?

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH 2/3] dts: teach newer i.MX machines to have the i.MX35 type watchdog
@ 2017-01-16 20:43       ` Uwe Kleine-König
  0 siblings, 0 replies; 32+ messages in thread
From: Uwe Kleine-König @ 2017-01-16 20:43 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Jan 16, 2017 at 04:35:26PM -0200, Fabio Estevam wrote:
> On Mon, Jan 9, 2017 at 7:50 AM, Uwe Kleine-K?nig
> <u.kleine-koenig@pengutronix.de> wrote:
> > Only i.MX35 and newer feature a WMCR register that should be written to. Older
> > SoCs hang when this address is written.
> >
> > Signed-off-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
> 
> Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>
> 
> Only a minor nit: Subject could: ARM: dts: imx: teach...

Ack. To reference the commit id of the first patch in the third I can
prepare a branch to pull. There I'd fix the Subject. Shawn, is this OK?

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 2/3] dts: teach newer i.MX machines to have the i.MX35 type watchdog
  2017-01-16 20:43       ` Uwe Kleine-König
@ 2017-01-16 21:48         ` Vladimir Zapolskiy
  -1 siblings, 0 replies; 32+ messages in thread
From: Vladimir Zapolskiy @ 2017-01-16 21:48 UTC (permalink / raw)
  To: Uwe Kleine-König, Fabio Estevam, Shawn Guo
  Cc: linux-watchdog, Magnus Lilja, Wim Van Sebroeck, Guenter Roeck,
	kernel, Fabio Estevam, linux-arm-kernel

On 01/16/2017 10:43 PM, Uwe Kleine-König wrote:
> On Mon, Jan 16, 2017 at 04:35:26PM -0200, Fabio Estevam wrote:
>> On Mon, Jan 9, 2017 at 7:50 AM, Uwe Kleine-König
>> <u.kleine-koenig@pengutronix.de> wrote:
>>> Only i.MX35 and newer feature a WMCR register that should be written to. Older
>>> SoCs hang when this address is written.
>>>
>>> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
>>
>> Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>
>>
>> Only a minor nit: Subject could: ARM: dts: imx: teach...
> 
> Ack. To reference the commit id of the first patch in the third I can
> prepare a branch to pull. There I'd fix the Subject. Shawn, is this OK?
> 

This is too early for changes, which are not reviewed by Linux watchdog masters.

As I emphasized multiple times in our discussions your series is organized
improperly, it has interdependencies between DTS and driver, it lacks
atomicity feature of a proper fix, in the middle it breaks backward compatibility
with old DTB, all that makes it impossible to send an atomic fix for linux-stable,
some of the changes are captured from mine ones without preserving the authorship
or even a reported-by credit.

--
With best wishes,
Vladimir

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH 2/3] dts: teach newer i.MX machines to have the i.MX35 type watchdog
@ 2017-01-16 21:48         ` Vladimir Zapolskiy
  0 siblings, 0 replies; 32+ messages in thread
From: Vladimir Zapolskiy @ 2017-01-16 21:48 UTC (permalink / raw)
  To: linux-arm-kernel

On 01/16/2017 10:43 PM, Uwe Kleine-K?nig wrote:
> On Mon, Jan 16, 2017 at 04:35:26PM -0200, Fabio Estevam wrote:
>> On Mon, Jan 9, 2017 at 7:50 AM, Uwe Kleine-K?nig
>> <u.kleine-koenig@pengutronix.de> wrote:
>>> Only i.MX35 and newer feature a WMCR register that should be written to. Older
>>> SoCs hang when this address is written.
>>>
>>> Signed-off-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
>>
>> Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>
>>
>> Only a minor nit: Subject could: ARM: dts: imx: teach...
> 
> Ack. To reference the commit id of the first patch in the third I can
> prepare a branch to pull. There I'd fix the Subject. Shawn, is this OK?
> 

This is too early for changes, which are not reviewed by Linux watchdog masters.

As I emphasized multiple times in our discussions your series is organized
improperly, it has interdependencies between DTS and driver, it lacks
atomicity feature of a proper fix, in the middle it breaks backward compatibility
with old DTB, all that makes it impossible to send an atomic fix for linux-stable,
some of the changes are captured from mine ones without preserving the authorship
or even a reported-by credit.

--
With best wishes,
Vladimir

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 2/3] dts: teach newer i.MX machines to have the i.MX35 type watchdog
  2017-01-16 21:48         ` Vladimir Zapolskiy
@ 2017-01-17  8:10           ` Uwe Kleine-König
  -1 siblings, 0 replies; 32+ messages in thread
From: Uwe Kleine-König @ 2017-01-17  8:10 UTC (permalink / raw)
  To: Vladimir Zapolskiy
  Cc: Fabio Estevam, Shawn Guo, linux-watchdog, Wim Van Sebroeck,
	Magnus Lilja, linux-arm-kernel, kernel, Fabio Estevam,
	Guenter Roeck

Hello Vladimir,

On Mon, Jan 16, 2017 at 11:48:40PM +0200, Vladimir Zapolskiy wrote:
> On 01/16/2017 10:43 PM, Uwe Kleine-König wrote:
> > On Mon, Jan 16, 2017 at 04:35:26PM -0200, Fabio Estevam wrote:
> >> On Mon, Jan 9, 2017 at 7:50 AM, Uwe Kleine-König
> >> <u.kleine-koenig@pengutronix.de> wrote:
> >>> Only i.MX35 and newer feature a WMCR register that should be written to. Older
> >>> SoCs hang when this address is written.
> >>>
> >>> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> >>
> >> Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>
> >>
> >> Only a minor nit: Subject could: ARM: dts: imx: teach...
> > 
> > Ack. To reference the commit id of the first patch in the third I can
> > prepare a branch to pull. There I'd fix the Subject. Shawn, is this OK?
> > 
> 
> This is too early for changes, which are not reviewed by Linux watchdog masters.
> 
> As I emphasized multiple times in our discussions your series is organized
> improperly, it has interdependencies between DTS and driver, it lacks
> atomicity feature of a proper fix, in the middle it breaks backward compatibility
> with old DTB, all that makes it impossible to send an atomic fix for linux-stable,

Oh, I must have missed that. In my inbox there are two replys to my
series, the one I reply to being the first to mention these critics.

Regarding your critics:

 - organized improperly: that alone is too abstract to understand what
   you mean.
 - interdependencies between DTS and driver, lacks atomicity, impossible
   to send an atomic fix for linux-stable: I wonder how you want to get
   rid of that. Yes there are interdependencies, that's because both
   driver and dts need adaption. Well, I could squash together some of
   the changes, then there is a single patch that atomically fixes
   everything. Personally I prefer to have three commits, each one
   changing a single thing and considering all three together as a fix
   for stable.
 - in the middle it breaks backward compatibility: There are three
   patches:

   	1) watchdog: imx2: Only i.MX35 and later have a WMCR register
	2) dts: teach newer i.MX machines to have the i.MX35 type watchdog
	3) watchdog: imx2: add compatibility for new i.MX35 type watchdog

   Before the first patch we have:

	- newer i.mx dtsi wrongly claims to have i.mx21 type watchdog
	- watchdog handles i.MX21 as if it were i.MX35

   After the first patch we have:

	- newer i.mx dtsi wrongly claims to have i.mx21 type watchdog

   And after the second everything is fine (assuming a dtb not older
   than the kernel). So each patch fixes a single thing.

   Yes, after the first patch machines having an i.MX35 type watchdog
   might have a problem. That's because there are two things to fix (so
   there are two patches): The driver must know about the new type and
   the dts must make use of it. And fixing the first makes the second
   visible, but doesn't introduce it (after all the i.MX35 dtsi is wrong
   claiming to have the i.MX21 type).

Now that there are machines known that suffer from the problem under
discussion it's obvious that we want the changes from the third patch.
Personally I don't care much if it stays separate or is squashed into
the first one.

> some of the changes are captured from mine ones without preserving the authorship
> or even a reported-by credit.

Compared to your v1 the similarities are AFAICT:

 - added .data to dt_ids array
 - added an if to only conditionally execute the breaking command
 - added a new compatible to several dtsi files

All these are necessary to fix the problem discussed here. The way I
implemented the first two ones are different to your way. So IMHO it's
at least questionable if you or I should be the author of the patches I
sent.

I'm ok to add a Reported-by for you. I usually add such a tag only after
a request for it. I missed to ask for that, please excuse this.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH 2/3] dts: teach newer i.MX machines to have the i.MX35 type watchdog
@ 2017-01-17  8:10           ` Uwe Kleine-König
  0 siblings, 0 replies; 32+ messages in thread
From: Uwe Kleine-König @ 2017-01-17  8:10 UTC (permalink / raw)
  To: linux-arm-kernel

Hello Vladimir,

On Mon, Jan 16, 2017 at 11:48:40PM +0200, Vladimir Zapolskiy wrote:
> On 01/16/2017 10:43 PM, Uwe Kleine-K?nig wrote:
> > On Mon, Jan 16, 2017 at 04:35:26PM -0200, Fabio Estevam wrote:
> >> On Mon, Jan 9, 2017 at 7:50 AM, Uwe Kleine-K?nig
> >> <u.kleine-koenig@pengutronix.de> wrote:
> >>> Only i.MX35 and newer feature a WMCR register that should be written to. Older
> >>> SoCs hang when this address is written.
> >>>
> >>> Signed-off-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
> >>
> >> Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>
> >>
> >> Only a minor nit: Subject could: ARM: dts: imx: teach...
> > 
> > Ack. To reference the commit id of the first patch in the third I can
> > prepare a branch to pull. There I'd fix the Subject. Shawn, is this OK?
> > 
> 
> This is too early for changes, which are not reviewed by Linux watchdog masters.
> 
> As I emphasized multiple times in our discussions your series is organized
> improperly, it has interdependencies between DTS and driver, it lacks
> atomicity feature of a proper fix, in the middle it breaks backward compatibility
> with old DTB, all that makes it impossible to send an atomic fix for linux-stable,

Oh, I must have missed that. In my inbox there are two replys to my
series, the one I reply to being the first to mention these critics.

Regarding your critics:

 - organized improperly: that alone is too abstract to understand what
   you mean.
 - interdependencies between DTS and driver, lacks atomicity, impossible
   to send an atomic fix for linux-stable: I wonder how you want to get
   rid of that. Yes there are interdependencies, that's because both
   driver and dts need adaption. Well, I could squash together some of
   the changes, then there is a single patch that atomically fixes
   everything. Personally I prefer to have three commits, each one
   changing a single thing and considering all three together as a fix
   for stable.
 - in the middle it breaks backward compatibility: There are three
   patches:

   	1) watchdog: imx2: Only i.MX35 and later have a WMCR register
	2) dts: teach newer i.MX machines to have the i.MX35 type watchdog
	3) watchdog: imx2: add compatibility for new i.MX35 type watchdog

   Before the first patch we have:

	- newer i.mx dtsi wrongly claims to have i.mx21 type watchdog
	- watchdog handles i.MX21 as if it were i.MX35

   After the first patch we have:

	- newer i.mx dtsi wrongly claims to have i.mx21 type watchdog

   And after the second everything is fine (assuming a dtb not older
   than the kernel). So each patch fixes a single thing.

   Yes, after the first patch machines having an i.MX35 type watchdog
   might have a problem. That's because there are two things to fix (so
   there are two patches): The driver must know about the new type and
   the dts must make use of it. And fixing the first makes the second
   visible, but doesn't introduce it (after all the i.MX35 dtsi is wrong
   claiming to have the i.MX21 type).

Now that there are machines known that suffer from the problem under
discussion it's obvious that we want the changes from the third patch.
Personally I don't care much if it stays separate or is squashed into
the first one.

> some of the changes are captured from mine ones without preserving the authorship
> or even a reported-by credit.

Compared to your v1 the similarities are AFAICT:

 - added .data to dt_ids array
 - added an if to only conditionally execute the breaking command
 - added a new compatible to several dtsi files

All these are necessary to fix the problem discussed here. The way I
implemented the first two ones are different to your way. So IMHO it's
at least questionable if you or I should be the author of the patches I
sent.

I'm ok to add a Reported-by for you. I usually add such a tag only after
a request for it. I missed to ask for that, please excuse this.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

^ permalink raw reply	[flat|nested] 32+ messages in thread

end of thread, other threads:[~2017-01-17  8:10 UTC | newest]

Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-09  9:50 [PATCH 0/3] watchdog: imx2: handle WMCR only being available on i.MX35 and later Uwe Kleine-König
2017-01-09  9:50 ` Uwe Kleine-König
2017-01-09  9:50 ` [PATCH 1/3] watchdog: imx2: Only i.MX35 and later have a WMCR register Uwe Kleine-König
2017-01-09  9:50   ` Uwe Kleine-König
2017-01-16 13:34   ` Fabio Estevam
2017-01-16 13:34     ` Fabio Estevam
2017-01-16 18:34   ` Fabio Estevam
2017-01-16 18:34     ` Fabio Estevam
2017-01-09  9:50 ` [PATCH 2/3] dts: teach newer i.MX machines to have the i.MX35 type watchdog Uwe Kleine-König
2017-01-09  9:50   ` Uwe Kleine-König
2017-01-16 18:35   ` Fabio Estevam
2017-01-16 18:35     ` Fabio Estevam
2017-01-16 20:43     ` Uwe Kleine-König
2017-01-16 20:43       ` Uwe Kleine-König
2017-01-16 21:48       ` Vladimir Zapolskiy
2017-01-16 21:48         ` Vladimir Zapolskiy
2017-01-17  8:10         ` Uwe Kleine-König
2017-01-17  8:10           ` Uwe Kleine-König
2017-01-09  9:50 ` [PATCH 3/3] watchdog: imx2: add compatibility for new " Uwe Kleine-König
2017-01-09  9:50   ` Uwe Kleine-König
2017-01-16 18:36   ` Fabio Estevam
2017-01-16 18:36     ` Fabio Estevam
2017-01-09 10:55 ` [PATCH 0/3] watchdog: imx2: handle WMCR only being available on i.MX35 and later Vladimir Zapolskiy
2017-01-09 10:55   ` Vladimir Zapolskiy
2017-01-09 11:04   ` Uwe Kleine-König
2017-01-09 11:04     ` Uwe Kleine-König
2017-01-09 17:19 ` Guenter Roeck
2017-01-09 17:19   ` Guenter Roeck
2017-01-09 20:06   ` Uwe Kleine-König
2017-01-09 20:06     ` Uwe Kleine-König
2017-01-16 17:27 ` Magnus Lilja
2017-01-16 17:27   ` Magnus Lilja

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.