* [PATCH 1/3] watchdog: imx2: Only i.MX35 and later have a WMCR register
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-16 13: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
` (4 subsequent siblings)
5 siblings, 2 replies; 16+ 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] 16+ messages in thread* [PATCH 1/3] watchdog: imx2: Only i.MX35 and later have a WMCR register
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-16 13:34 ` Fabio Estevam
2017-01-16 18:34 ` Fabio Estevam
1 sibling, 0 replies; 16+ 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] 16+ messages in thread* [PATCH 1/3] watchdog: imx2: Only i.MX35 and later have a WMCR register
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-16 13:34 ` Fabio Estevam
@ 2017-01-16 18:34 ` Fabio Estevam
1 sibling, 0 replies; 16+ 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] 16+ messages in thread
* [PATCH 2/3] dts: teach newer i.MX machines to have the i.MX35 type watchdog
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 ` [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 18:35 ` Fabio Estevam
2017-01-09 9:50 ` [PATCH 3/3] watchdog: imx2: add compatibility for new " Uwe Kleine-König
` (3 subsequent siblings)
5 siblings, 1 reply; 16+ 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] 16+ messages in thread* [PATCH 2/3] dts: teach newer i.MX machines to have the i.MX35 type watchdog
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-16 18:35 ` Fabio Estevam
2017-01-16 20:43 ` Uwe Kleine-König
0 siblings, 1 reply; 16+ 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] 16+ 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
@ 2017-01-16 20:43 ` Uwe Kleine-König
2017-01-16 21:48 ` Vladimir Zapolskiy
0 siblings, 1 reply; 16+ 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] 16+ 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
@ 2017-01-16 21:48 ` Vladimir Zapolskiy
2017-01-17 8:10 ` Uwe Kleine-König
0 siblings, 1 reply; 16+ 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] 16+ 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
@ 2017-01-17 8:10 ` Uwe Kleine-König
0 siblings, 0 replies; 16+ 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] 16+ messages in thread
* [PATCH 3/3] watchdog: imx2: add compatibility for new i.MX35 type watchdog
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 ` [PATCH 1/3] watchdog: imx2: Only i.MX35 and later have a WMCR register Uwe Kleine-König
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: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
` (2 subsequent siblings)
5 siblings, 1 reply; 16+ 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] 16+ messages in thread* [PATCH 3/3] watchdog: imx2: add compatibility for new i.MX35 type watchdog
2017-01-09 9:50 ` [PATCH 3/3] watchdog: imx2: add compatibility for new " Uwe Kleine-König
@ 2017-01-16 18:36 ` Fabio Estevam
0 siblings, 0 replies; 16+ 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] 16+ messages in thread
* [PATCH 0/3] watchdog: imx2: handle WMCR only being available on i.MX35 and later
2017-01-09 9:50 [PATCH 0/3] watchdog: imx2: handle WMCR only being available on i.MX35 and later Uwe Kleine-König
` (2 preceding siblings ...)
2017-01-09 9:50 ` [PATCH 3/3] watchdog: imx2: add compatibility for new " Uwe Kleine-König
@ 2017-01-09 10:55 ` Vladimir Zapolskiy
2017-01-09 11:04 ` Uwe Kleine-König
2017-01-09 17:19 ` Guenter Roeck
2017-01-16 17:27 ` Magnus Lilja
5 siblings, 1 reply; 16+ 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] 16+ messages in thread* [PATCH 0/3] watchdog: imx2: handle WMCR only being available on i.MX35 and later
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 11:04 ` Uwe Kleine-König
0 siblings, 0 replies; 16+ 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] 16+ messages in thread
* [PATCH 0/3] watchdog: imx2: handle WMCR only being available on i.MX35 and later
2017-01-09 9:50 [PATCH 0/3] watchdog: imx2: handle WMCR only being available on i.MX35 and later Uwe Kleine-König
` (3 preceding siblings ...)
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 17:19 ` Guenter Roeck
2017-01-09 20:06 ` Uwe Kleine-König
2017-01-16 17:27 ` Magnus Lilja
5 siblings, 1 reply; 16+ 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] 16+ 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
@ 2017-01-09 20:06 ` Uwe Kleine-König
0 siblings, 0 replies; 16+ 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] 16+ messages in thread
* [PATCH 0/3] watchdog: imx2: handle WMCR only being available on i.MX35 and later
2017-01-09 9:50 [PATCH 0/3] watchdog: imx2: handle WMCR only being available on i.MX35 and later Uwe Kleine-König
` (4 preceding siblings ...)
2017-01-09 17:19 ` Guenter Roeck
@ 2017-01-16 17:27 ` Magnus Lilja
5 siblings, 0 replies; 16+ 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] 16+ messages in thread