[PATCH 0/3] qcom watchdog fixes

[PATCH 0/3] qcom watchdog fixes

[Thomas Pedersen]

Some boards (eg. qcom IPQ8064 based) Need these changes to avoid a spurious
watchdog reset.

Matthew McClintock (3):
  watchdog: qcom: update device tree bindings
  watchdog: qcom: add option for standalone watchdog not in timer block
  watchdog: qcom: configure BARK time in addition to BITE time

 .../devicetree/bindings/watchdog/qcom-wdt.txt      |  5 +-
 drivers/watchdog/qcom-wdt.c                        | 78 ++++++++++++++++------
 2 files changed, 59 insertions(+), 24 deletions(-)

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH 1/3] watchdog: qcom: update device tree bindings

[Thomas Pedersen]

From: Matthew McClintock <mmcclint@codeaurora.org>

Update the compatible string to align with driver

CC: linux-watchdog@vger.kernel.org
Signed-off-by: Matthew McClintock <mmcclint@codeaurora.org>
---
 Documentation/devicetree/bindings/watchdog/qcom-wdt.txt | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/watchdog/qcom-wdt.txt b/Documentation/devicetree/bindings/watchdog/qcom-wdt.txt
index 4726924..60bb2f98 100644
--- a/Documentation/devicetree/bindings/watchdog/qcom-wdt.txt
+++ b/Documentation/devicetree/bindings/watchdog/qcom-wdt.txt
@@ -4,9 +4,8 @@ Qualcomm Krait Processor Sub-system (KPSS) Watchdog
 Required properties :
 - compatible : shall contain only one of the following:
 
-			"qcom,kpss-wdt-msm8960"
-			"qcom,kpss-wdt-apq8064"
-			"qcom,kpss-wdt-ipq8064"
+			"qcom,kpss-timer"
+			"qcom,scss-timer"
 
 - reg : shall contain base register location and length
 - clocks : shall contain the input clock
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH 2/3] watchdog: qcom: add option for standalone watchdog not in timer block

[Thomas Pedersen]

From: Matthew McClintock <mmcclint@codeaurora.org>

Commit 0dfd582e026a ("watchdog: qcom: use timer devicetree binding") moved
to use the watchdog as a subset timer register block. Some devices have the
watchdog completely standalone with slightly different register offsets as
well so let's account for the differences here.

Signed-off-by: Matthew McClintock <mmcclint@codeaurora.org>
---
 drivers/watchdog/qcom-wdt.c | 73 ++++++++++++++++++++++++++++++++-------------
 1 file changed, 52 insertions(+), 21 deletions(-)

diff --git a/drivers/watchdog/qcom-wdt.c b/drivers/watchdog/qcom-wdt.c
index a043fa4..70e42ed 100644
--- a/drivers/watchdog/qcom-wdt.c
+++ b/drivers/watchdog/qcom-wdt.c
@@ -19,18 +19,40 @@
 #include <linux/platform_device.h>
 #include <linux/watchdog.h>
 
-#define WDT_RST		0x38
-#define WDT_EN		0x40
-#define WDT_STS		0x44
-#define WDT_BITE_TIME	0x5C
+enum wdt_reg {
+	WDT_RST,
+	WDT_EN,
+	WDT_STS,
+	WDT_BITE_TIME,
+};
+
+static const u32 reg_offset_data_apcs_tmr[] = {
+	[WDT_RST] = 0x38,
+	[WDT_EN] = 0x40,
+	[WDT_STS] = 0x44,
+	[WDT_BITE_TIME] = 0x5C,
+};
+
+static const u32 reg_offset_data_kpss[] = {
+	[WDT_RST] = 0x4,
+	[WDT_EN] = 0x8,
+	[WDT_STS] = 0xC,
+	[WDT_BITE_TIME] = 0x14,
+};
 
 struct qcom_wdt {
 	struct watchdog_device	wdd;
 	struct clk		*clk;
 	unsigned long		rate;
 	void __iomem		*base;
+	const u32		*layout;
 };
 
+static void __iomem *wdt_addr(struct qcom_wdt *wdt, enum wdt_reg reg)
+{
+	return wdt->base + wdt->layout[reg];
+}
+
 static inline
 struct qcom_wdt *to_qcom_wdt(struct watchdog_device *wdd)
 {
@@ -41,10 +63,10 @@ static int qcom_wdt_start(struct watchdog_device *wdd)
 {
 	struct qcom_wdt *wdt = to_qcom_wdt(wdd);
 
-	writel(0, wdt->base + WDT_EN);
-	writel(1, wdt->base + WDT_RST);
-	writel(wdd->timeout * wdt->rate, wdt->base + WDT_BITE_TIME);
-	writel(1, wdt->base + WDT_EN);
+	writel(0, wdt_addr(wdt, WDT_EN));
+	writel(1, wdt_addr(wdt, WDT_RST));
+	writel(wdd->timeout * wdt->rate, wdt_addr(wdt, WDT_BITE_TIME));
+	writel(1, wdt_addr(wdt, WDT_EN));
 	return 0;
 }
 
@@ -52,7 +74,7 @@ static int qcom_wdt_stop(struct watchdog_device *wdd)
 {
 	struct qcom_wdt *wdt = to_qcom_wdt(wdd);
 
-	writel(0, wdt->base + WDT_EN);
+	writel(0, wdt_addr(wdt, WDT_EN));
 	return 0;
 }
 
@@ -60,7 +82,7 @@ static int qcom_wdt_ping(struct watchdog_device *wdd)
 {
 	struct qcom_wdt *wdt = to_qcom_wdt(wdd);
 
-	writel(1, wdt->base + WDT_RST);
+	writel(1, wdt_addr(wdt, WDT_RST));
 	return 0;
 }
 
@@ -83,10 +105,10 @@ static int qcom_wdt_restart(struct watchdog_device *wdd, unsigned long action,
 	 */
 	timeout = 128 * wdt->rate / 1000;
 
-	writel(0, wdt->base + WDT_EN);
-	writel(1, wdt->base + WDT_RST);
-	writel(timeout, wdt->base + WDT_BITE_TIME);
-	writel(1, wdt->base + WDT_EN);
+	writel(0, wdt_addr(wdt, WDT_EN));
+	writel(1, wdt_addr(wdt, WDT_RST));
+	writel(timeout, wdt_addr(wdt, WDT_BITE_TIME));
+	writel(1, wdt_addr(wdt, WDT_EN));
 
 	/*
 	 * Actually make sure the above sequence hits hardware before sleeping.
@@ -114,14 +136,29 @@ static const struct watchdog_info qcom_wdt_info = {
 	.identity	= KBUILD_MODNAME,
 };
 
+static const struct of_device_id qcom_wdt_of_table[] = {
+	{ .compatible = "qcom,kpss-timer", .data = reg_offset_data_apcs_tmr },
+	{ .compatible = "qcom,scss-timer", .data = reg_offset_data_apcs_tmr },
+	{ .compatible = "qcom,kpss-standalone", .data = &reg_offset_data_kpss},
+	{ },
+};
+MODULE_DEVICE_TABLE(of, qcom_wdt_of_table);
+
 static int qcom_wdt_probe(struct platform_device *pdev)
 {
 	struct qcom_wdt *wdt;
 	struct resource *res;
 	struct device_node *np = pdev->dev.of_node;
+	const struct of_device_id *match;
 	u32 percpu_offset;
 	int ret;
 
+	match = of_match_node(qcom_wdt_of_table, np);
+	if (!match) {
+		dev_err(&pdev->dev, "Unsupported QCOM WDT module\n");
+		return -ENODEV;
+	}
+
 	wdt = devm_kzalloc(&pdev->dev, sizeof(*wdt), GFP_KERNEL);
 	if (!wdt)
 		return -ENOMEM;
@@ -172,6 +209,7 @@ static int qcom_wdt_probe(struct platform_device *pdev)
 	wdt->wdd.min_timeout = 1;
 	wdt->wdd.max_timeout = 0x10000000U / wdt->rate;
 	wdt->wdd.parent = &pdev->dev;
+	wdt->layout = match->data;
 
 	if (readl(wdt->base + WDT_STS) & 1)
 		wdt->wdd.bootstatus = WDIOF_CARDRESET;
@@ -207,13 +245,6 @@ static int qcom_wdt_remove(struct platform_device *pdev)
 	return 0;
 }
 
-static const struct of_device_id qcom_wdt_of_table[] = {
-	{ .compatible = "qcom,kpss-timer" },
-	{ .compatible = "qcom,scss-timer" },
-	{ },
-};
-MODULE_DEVICE_TABLE(of, qcom_wdt_of_table);
-
 static struct platform_driver qcom_watchdog_driver = {
 	.probe	= qcom_wdt_probe,
 	.remove	= qcom_wdt_remove,
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH 3/3] watchdog: qcom: configure BARK time in addition to BITE time

[Thomas Pedersen]

From: Matthew McClintock <mmcclint@codeaurora.org>

For certain parts and some versions of TZ, TZ will reset the chip
when a BARK is triggered even though it was not configured here. So
by default let's configure this BARK time as well.

Signed-off-by: Matthew McClintock <mmcclint@codeaurora.org>
---
 drivers/watchdog/qcom-wdt.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/watchdog/qcom-wdt.c b/drivers/watchdog/qcom-wdt.c
index 70e42ed..e2ad6e1 100644
--- a/drivers/watchdog/qcom-wdt.c
+++ b/drivers/watchdog/qcom-wdt.c
@@ -23,6 +23,7 @@ enum wdt_reg {
 	WDT_RST,
 	WDT_EN,
 	WDT_STS,
+	WDT_BARK_TIME,
 	WDT_BITE_TIME,
 };
 
@@ -30,6 +31,7 @@ static const u32 reg_offset_data_apcs_tmr[] = {
 	[WDT_RST] = 0x38,
 	[WDT_EN] = 0x40,
 	[WDT_STS] = 0x44,
+	[WDT_BARK_TIME] = 0x4C,
 	[WDT_BITE_TIME] = 0x5C,
 };
 
@@ -37,6 +39,7 @@ static const u32 reg_offset_data_kpss[] = {
 	[WDT_RST] = 0x4,
 	[WDT_EN] = 0x8,
 	[WDT_STS] = 0xC,
+	[WDT_BARK_TIME] = 0x10,
 	[WDT_BITE_TIME] = 0x14,
 };
 
@@ -65,6 +68,7 @@ static int qcom_wdt_start(struct watchdog_device *wdd)
 
 	writel(0, wdt_addr(wdt, WDT_EN));
 	writel(1, wdt_addr(wdt, WDT_RST));
+	writel(wdd->timeout * wdt->rate, wdt_addr(wdt, WDT_BARK_TIME));
 	writel(wdd->timeout * wdt->rate, wdt_addr(wdt, WDT_BITE_TIME));
 	writel(1, wdt_addr(wdt, WDT_EN));
 	return 0;
@@ -107,6 +111,7 @@ static int qcom_wdt_restart(struct watchdog_device *wdd, unsigned long action,
 
 	writel(0, wdt_addr(wdt, WDT_EN));
 	writel(1, wdt_addr(wdt, WDT_RST));
+	writel(timeout, wdt_addr(wdt, WDT_BARK_TIME));
 	writel(timeout, wdt_addr(wdt, WDT_BITE_TIME));
 	writel(1, wdt_addr(wdt, WDT_EN));
 
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH 1/3] watchdog: qcom: update device tree bindings

[Guenter Roeck]

On 06/27/2016 04:26 PM, Thomas Pedersen wrote:
> From: Matthew McClintock <mmcclint@codeaurora.org>
>
> Update the compatible string to align with driver
>
> CC: linux-watchdog@vger.kernel.org
> Signed-off-by: Matthew McClintock <mmcclint@codeaurora.org>
> ---
>   Documentation/devicetree/bindings/watchdog/qcom-wdt.txt | 5 ++---
>   1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/watchdog/qcom-wdt.txt b/Documentation/devicetree/bindings/watchdog/qcom-wdt.txt
> index 4726924..60bb2f98 100644
> --- a/Documentation/devicetree/bindings/watchdog/qcom-wdt.txt
> +++ b/Documentation/devicetree/bindings/watchdog/qcom-wdt.txt
> @@ -4,9 +4,8 @@ Qualcomm Krait Processor Sub-system (KPSS) Watchdog
>   Required properties :
>   - compatible : shall contain only one of the following:
>
> -			"qcom,kpss-wdt-msm8960"
> -			"qcom,kpss-wdt-apq8064"
> -			"qcom,kpss-wdt-ipq8064"
> +			"qcom,kpss-timer"
> +			"qcom,scss-timer"
>
>   - reg : shall contain base register location and length
>   - clocks : shall contain the input clock
>

Rob's earlier feedback to the same patch you had submitted in March:

"Keep the SoC specific ones even if they are not used. The DTS should
have both strings."

Guenter

Re: [PATCH 2/3] watchdog: qcom: add option for standalone watchdog not in timer block

[Guenter Roeck]

On 06/27/2016 04:26 PM, Thomas Pedersen wrote:
> From: Matthew McClintock <mmcclint@codeaurora.org>
>
> Commit 0dfd582e026a ("watchdog: qcom: use timer devicetree binding") moved
> to use the watchdog as a subset timer register block. Some devices have the
> watchdog completely standalone with slightly different register offsets as
> well so let's account for the differences here.
>
> Signed-off-by: Matthew McClintock <mmcclint@codeaurora.org>
> ---
>   drivers/watchdog/qcom-wdt.c | 73 ++++++++++++++++++++++++++++++++-------------
>   1 file changed, 52 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/watchdog/qcom-wdt.c b/drivers/watchdog/qcom-wdt.c
> index a043fa4..70e42ed 100644
> --- a/drivers/watchdog/qcom-wdt.c
> +++ b/drivers/watchdog/qcom-wdt.c
> @@ -19,18 +19,40 @@
>   #include <linux/platform_device.h>
>   #include <linux/watchdog.h>
>
> -#define WDT_RST		0x38
> -#define WDT_EN		0x40
> -#define WDT_STS		0x44
> -#define WDT_BITE_TIME	0x5C
> +enum wdt_reg {
> +	WDT_RST,
> +	WDT_EN,
> +	WDT_STS,
> +	WDT_BITE_TIME,
> +};
> +
> +static const u32 reg_offset_data_apcs_tmr[] = {
> +	[WDT_RST] = 0x38,
> +	[WDT_EN] = 0x40,
> +	[WDT_STS] = 0x44,
> +	[WDT_BITE_TIME] = 0x5C,
> +};
> +
> +static const u32 reg_offset_data_kpss[] = {
> +	[WDT_RST] = 0x4,
> +	[WDT_EN] = 0x8,
> +	[WDT_STS] = 0xC,
> +	[WDT_BITE_TIME] = 0x14,
> +};
>
>   struct qcom_wdt {
>   	struct watchdog_device	wdd;
>   	struct clk		*clk;
>   	unsigned long		rate;
>   	void __iomem		*base;
> +	const u32		*layout;
>   };
>
> +static void __iomem *wdt_addr(struct qcom_wdt *wdt, enum wdt_reg reg)
> +{
> +	return wdt->base + wdt->layout[reg];
> +}
> +
>   static inline
>   struct qcom_wdt *to_qcom_wdt(struct watchdog_device *wdd)
>   {
> @@ -41,10 +63,10 @@ static int qcom_wdt_start(struct watchdog_device *wdd)
>   {
>   	struct qcom_wdt *wdt = to_qcom_wdt(wdd);
>
> -	writel(0, wdt->base + WDT_EN);
> -	writel(1, wdt->base + WDT_RST);
> -	writel(wdd->timeout * wdt->rate, wdt->base + WDT_BITE_TIME);
> -	writel(1, wdt->base + WDT_EN);
> +	writel(0, wdt_addr(wdt, WDT_EN));
> +	writel(1, wdt_addr(wdt, WDT_RST));
> +	writel(wdd->timeout * wdt->rate, wdt_addr(wdt, WDT_BITE_TIME));
> +	writel(1, wdt_addr(wdt, WDT_EN));
>   	return 0;
>   }
>
> @@ -52,7 +74,7 @@ static int qcom_wdt_stop(struct watchdog_device *wdd)
>   {
>   	struct qcom_wdt *wdt = to_qcom_wdt(wdd);
>
> -	writel(0, wdt->base + WDT_EN);
> +	writel(0, wdt_addr(wdt, WDT_EN));
>   	return 0;
>   }
>
> @@ -60,7 +82,7 @@ static int qcom_wdt_ping(struct watchdog_device *wdd)
>   {
>   	struct qcom_wdt *wdt = to_qcom_wdt(wdd);
>
> -	writel(1, wdt->base + WDT_RST);
> +	writel(1, wdt_addr(wdt, WDT_RST));
>   	return 0;
>   }
>
> @@ -83,10 +105,10 @@ static int qcom_wdt_restart(struct watchdog_device *wdd, unsigned long action,
>   	 */
>   	timeout = 128 * wdt->rate / 1000;
>
> -	writel(0, wdt->base + WDT_EN);
> -	writel(1, wdt->base + WDT_RST);
> -	writel(timeout, wdt->base + WDT_BITE_TIME);
> -	writel(1, wdt->base + WDT_EN);
> +	writel(0, wdt_addr(wdt, WDT_EN));
> +	writel(1, wdt_addr(wdt, WDT_RST));
> +	writel(timeout, wdt_addr(wdt, WDT_BITE_TIME));
> +	writel(1, wdt_addr(wdt, WDT_EN));
>
>   	/*
>   	 * Actually make sure the above sequence hits hardware before sleeping.
> @@ -114,14 +136,29 @@ static const struct watchdog_info qcom_wdt_info = {
>   	.identity	= KBUILD_MODNAME,
>   };
>
> +static const struct of_device_id qcom_wdt_of_table[] = {
> +	{ .compatible = "qcom,kpss-timer", .data = reg_offset_data_apcs_tmr },
> +	{ .compatible = "qcom,scss-timer", .data = reg_offset_data_apcs_tmr },
> +	{ .compatible = "qcom,kpss-standalone", .data = &reg_offset_data_kpss},
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(of, qcom_wdt_of_table);
> +
>   static int qcom_wdt_probe(struct platform_device *pdev)
>   {
>   	struct qcom_wdt *wdt;
>   	struct resource *res;
>   	struct device_node *np = pdev->dev.of_node;
> +	const struct of_device_id *match;
>   	u32 percpu_offset;
>   	int ret;
>
> +	match = of_match_node(qcom_wdt_of_table, np);
> +	if (!match) {
> +		dev_err(&pdev->dev, "Unsupported QCOM WDT module\n");
> +		return -ENODEV;
> +	}
> +
>   	wdt = devm_kzalloc(&pdev->dev, sizeof(*wdt), GFP_KERNEL);
>   	if (!wdt)
>   		return -ENOMEM;
> @@ -172,6 +209,7 @@ static int qcom_wdt_probe(struct platform_device *pdev)
>   	wdt->wdd.min_timeout = 1;
>   	wdt->wdd.max_timeout = 0x10000000U / wdt->rate;
>   	wdt->wdd.parent = &pdev->dev;
> +	wdt->layout = match->data;
>
>   	if (readl(wdt->base + WDT_STS) & 1)
>   		wdt->wdd.bootstatus = WDIOF_CARDRESET;
> @@ -207,13 +245,6 @@ static int qcom_wdt_remove(struct platform_device *pdev)
>   	return 0;
>   }
>
> -static const struct of_device_id qcom_wdt_of_table[] = {
> -	{ .compatible = "qcom,kpss-timer" },
> -	{ .compatible = "qcom,scss-timer" },
> -	{ },
> -};
> -MODULE_DEVICE_TABLE(of, qcom_wdt_of_table);
> -

Earlier comment from Stephen Boyd:

Leave this here and use of_device_get_match_data() in probe
instead.

>   static struct platform_driver qcom_watchdog_driver = {
>   	.probe	= qcom_wdt_probe,
>   	.remove	= qcom_wdt_remove,
>

Re: [PATCH 3/3] watchdog: qcom: configure BARK time in addition to BITE time

[Guenter Roeck]

On 06/27/2016 04:26 PM, Thomas Pedersen wrote:
> From: Matthew McClintock <mmcclint@codeaurora.org>
>
> For certain parts and some versions of TZ, TZ will reset the chip
> when a BARK is triggered even though it was not configured here. So
> by default let's configure this BARK time as well.
>
> Signed-off-by: Matthew McClintock <mmcclint@codeaurora.org>

Seems to be identical to the earlier version of the same patch, which
already had my

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
>   drivers/watchdog/qcom-wdt.c | 5 +++++
>   1 file changed, 5 insertions(+)
>
> diff --git a/drivers/watchdog/qcom-wdt.c b/drivers/watchdog/qcom-wdt.c
> index 70e42ed..e2ad6e1 100644
> --- a/drivers/watchdog/qcom-wdt.c
> +++ b/drivers/watchdog/qcom-wdt.c
> @@ -23,6 +23,7 @@ enum wdt_reg {
>   	WDT_RST,
>   	WDT_EN,
>   	WDT_STS,
> +	WDT_BARK_TIME,
>   	WDT_BITE_TIME,
>   };
>
> @@ -30,6 +31,7 @@ static const u32 reg_offset_data_apcs_tmr[] = {
>   	[WDT_RST] = 0x38,
>   	[WDT_EN] = 0x40,
>   	[WDT_STS] = 0x44,
> +	[WDT_BARK_TIME] = 0x4C,
>   	[WDT_BITE_TIME] = 0x5C,
>   };
>
> @@ -37,6 +39,7 @@ static const u32 reg_offset_data_kpss[] = {
>   	[WDT_RST] = 0x4,
>   	[WDT_EN] = 0x8,
>   	[WDT_STS] = 0xC,
> +	[WDT_BARK_TIME] = 0x10,
>   	[WDT_BITE_TIME] = 0x14,
>   };
>
> @@ -65,6 +68,7 @@ static int qcom_wdt_start(struct watchdog_device *wdd)
>
>   	writel(0, wdt_addr(wdt, WDT_EN));
>   	writel(1, wdt_addr(wdt, WDT_RST));
> +	writel(wdd->timeout * wdt->rate, wdt_addr(wdt, WDT_BARK_TIME));
>   	writel(wdd->timeout * wdt->rate, wdt_addr(wdt, WDT_BITE_TIME));
>   	writel(1, wdt_addr(wdt, WDT_EN));
>   	return 0;
> @@ -107,6 +111,7 @@ static int qcom_wdt_restart(struct watchdog_device *wdd, unsigned long action,
>
>   	writel(0, wdt_addr(wdt, WDT_EN));
>   	writel(1, wdt_addr(wdt, WDT_RST));
> +	writel(timeout, wdt_addr(wdt, WDT_BARK_TIME));
>   	writel(timeout, wdt_addr(wdt, WDT_BITE_TIME));
>   	writel(1, wdt_addr(wdt, WDT_EN));
>
>

Re: [PATCH 0/3] qcom watchdog fixes

[Guenter Roeck]

On 06/27/2016 04:26 PM, Thomas Pedersen wrote:
> Some boards (eg. qcom IPQ8064 based) Need these changes to avoid a spurious
> watchdog reset.
>
> Matthew McClintock (3):
>    watchdog: qcom: update device tree bindings
>    watchdog: qcom: add option for standalone watchdog not in timer block
>    watchdog: qcom: configure BARK time in addition to BITE time
>
>   .../devicetree/bindings/watchdog/qcom-wdt.txt      |  5 +-
>   drivers/watchdog/qcom-wdt.c                        | 78 ++++++++++++++++------
>   2 files changed, 59 insertions(+), 24 deletions(-)
>

General comment: If you don't plan to address the comments made to
the previous submission, please at least let the reviewers know
what you are doing and why. Also, a note indicating that this
is a resend would be helpful for those who did not see the original
series.

Thanks,
Guenter

Re: [PATCH 0/3] qcom watchdog fixes

[Thomas Pedersen]

Hi Guenter,


On 2016-06-27 22:31, Guenter Roeck wrote:
> On 06/27/2016 04:26 PM, Thomas Pedersen wrote:
>> Some boards (eg. qcom IPQ8064 based) Need these changes to avoid a 
>> spurious
>> watchdog reset.
>> 
>> Matthew McClintock (3):
>>    watchdog: qcom: update device tree bindings
>>    watchdog: qcom: add option for standalone watchdog not in timer 
>> block
>>    watchdog: qcom: configure BARK time in addition to BITE time
>> 
>>   .../devicetree/bindings/watchdog/qcom-wdt.txt      |  5 +-
>>   drivers/watchdog/qcom-wdt.c                        | 78 
>> ++++++++++++++++------
>>   2 files changed, 59 insertions(+), 24 deletions(-)
>> 
> 
> General comment: If you don't plan to address the comments made to
> the previous submission, please at least let the reviewers know
> what you are doing and why. Also, a note indicating that this
> is a resend would be helpful for those who did not see the original
> series.

Thanks, checking for earlier submissions slipped my mind. I'll address 
your
comments in a v2.

-- 
thomas

Re: [PATCH 1/3] watchdog: qcom: update device tree bindings

[Thomas Pedersen]

On 2016-06-27 22:20, Guenter Roeck wrote:
> On 06/27/2016 04:26 PM, Thomas Pedersen wrote:
>> From: Matthew McClintock <mmcclint@codeaurora.org>
>> 
>> Update the compatible string to align with driver
>> 
>> CC: linux-watchdog@vger.kernel.org
>> Signed-off-by: Matthew McClintock <mmcclint@codeaurora.org>
>> ---
>>   Documentation/devicetree/bindings/watchdog/qcom-wdt.txt | 5 ++---
>>   1 file changed, 2 insertions(+), 3 deletions(-)
>> 
>> diff --git a/Documentation/devicetree/bindings/watchdog/qcom-wdt.txt 
>> b/Documentation/devicetree/bindings/watchdog/qcom-wdt.txt
>> index 4726924..60bb2f98 100644
>> --- a/Documentation/devicetree/bindings/watchdog/qcom-wdt.txt
>> +++ b/Documentation/devicetree/bindings/watchdog/qcom-wdt.txt
>> @@ -4,9 +4,8 @@ Qualcomm Krait Processor Sub-system (KPSS) Watchdog
>>   Required properties :
>>   - compatible : shall contain only one of the following:
>> 
>> -			"qcom,kpss-wdt-msm8960"
>> -			"qcom,kpss-wdt-apq8064"
>> -			"qcom,kpss-wdt-ipq8064"
>> +			"qcom,kpss-timer"
>> +			"qcom,scss-timer"
>> 
>>   - reg : shall contain base register location and length
>>   - clocks : shall contain the input clock
>> 
> 
> Rob's earlier feedback to the same patch you had submitted in March:
> 
> "Keep the SoC specific ones even if they are not used. The DTS should
> have both strings."

Why would we add the SoC specific compatible strings to the DTS if 
they're not
even used?

-- 
thomas

Re: [PATCH 1/3] watchdog: qcom: update device tree bindings

[Mark Rutland]

On Tue, Jun 28, 2016 at 10:17:32AM -0700, Thomas Pedersen wrote:
> On 2016-06-27 22:20, Guenter Roeck wrote:
> >On 06/27/2016 04:26 PM, Thomas Pedersen wrote:
> >>From: Matthew McClintock <mmcclint@codeaurora.org>
> >>
> >>Update the compatible string to align with driver
> >>
> >>CC: linux-watchdog@vger.kernel.org
> >>Signed-off-by: Matthew McClintock <mmcclint@codeaurora.org>
> >>---
> >>  Documentation/devicetree/bindings/watchdog/qcom-wdt.txt | 5 ++---
> >>  1 file changed, 2 insertions(+), 3 deletions(-)
> >>
> >>diff --git
> >>a/Documentation/devicetree/bindings/watchdog/qcom-wdt.txt
> >>b/Documentation/devicetree/bindings/watchdog/qcom-wdt.txt
> >>index 4726924..60bb2f98 100644
> >>--- a/Documentation/devicetree/bindings/watchdog/qcom-wdt.txt
> >>+++ b/Documentation/devicetree/bindings/watchdog/qcom-wdt.txt
> >>@@ -4,9 +4,8 @@ Qualcomm Krait Processor Sub-system (KPSS) Watchdog
> >>  Required properties :
> >>  - compatible : shall contain only one of the following:
> >>
> >>-			"qcom,kpss-wdt-msm8960"
> >>-			"qcom,kpss-wdt-apq8064"
> >>-			"qcom,kpss-wdt-ipq8064"
> >>+			"qcom,kpss-timer"
> >>+			"qcom,scss-timer"
> >>
> >>  - reg : shall contain base register location and length
> >>  - clocks : shall contain the input clock
> >>
> >
> >Rob's earlier feedback to the same patch you had submitted in March:
> >
> >"Keep the SoC specific ones even if they are not used. The DTS should
> >have both strings."
> 
> Why would we add the SoC specific compatible strings to the DTS if
> they're not
> even used?

Because they might be in future, and having them present in existing
DTBs will make our lives easier. Otherwise, they cost very little.

Thanks,
Mark.

Re: [PATCH 1/3] watchdog: qcom: update device tree bindings

[Thomas Pedersen]

On 2016-06-28 10:24, Mark Rutland wrote:
> On Tue, Jun 28, 2016 at 10:17:32AM -0700, Thomas Pedersen wrote:
>> On 2016-06-27 22:20, Guenter Roeck wrote:
>> >On 06/27/2016 04:26 PM, Thomas Pedersen wrote:
>> >>From: Matthew McClintock <mmcclint@codeaurora.org>
>> >>
>> >>Update the compatible string to align with driver
>> >>
>> >>CC: linux-watchdog@vger.kernel.org
>> >>Signed-off-by: Matthew McClintock <mmcclint@codeaurora.org>
>> >>---
>> >>  Documentation/devicetree/bindings/watchdog/qcom-wdt.txt | 5 ++---
>> >>  1 file changed, 2 insertions(+), 3 deletions(-)
>> >>
>> >>diff --git
>> >>a/Documentation/devicetree/bindings/watchdog/qcom-wdt.txt
>> >>b/Documentation/devicetree/bindings/watchdog/qcom-wdt.txt
>> >>index 4726924..60bb2f98 100644
>> >>--- a/Documentation/devicetree/bindings/watchdog/qcom-wdt.txt
>> >>+++ b/Documentation/devicetree/bindings/watchdog/qcom-wdt.txt
>> >>@@ -4,9 +4,8 @@ Qualcomm Krait Processor Sub-system (KPSS) Watchdog
>> >>  Required properties :
>> >>  - compatible : shall contain only one of the following:
>> >>
>> >>-			"qcom,kpss-wdt-msm8960"
>> >>-			"qcom,kpss-wdt-apq8064"
>> >>-			"qcom,kpss-wdt-ipq8064"
>> >>+			"qcom,kpss-timer"
>> >>+			"qcom,scss-timer"
>> >>
>> >>  - reg : shall contain base register location and length
>> >>  - clocks : shall contain the input clock
>> >>
>> >
>> >Rob's earlier feedback to the same patch you had submitted in March:
>> >
>> >"Keep the SoC specific ones even if they are not used. The DTS should
>> >have both strings."
>> 
>> Why would we add the SoC specific compatible strings to the DTS if
>> they're not
>> even used?
> 
> Because they might be in future, and having them present in existing
> DTBs will make our lives easier. Otherwise, they cost very little.

Hm, ok.

-- 
thomas