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

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

[Thomas Pedersen]

From: Matthew McClintock <mmcclint@codeaurora.org>

Update the compatible string to align with driver and also
add SoC specific string to DTS.

CC: linux-watchdog@vger.kernel.org
Signed-off-by: Matthew McClintock <mmcclint@codeaurora.org>
Signed-off-by: Thomas Pedersen <twp@codeaurora.org>

---
v2:
Keep SoC specific compatible and add it to DTS.
---
 Documentation/devicetree/bindings/watchdog/qcom-wdt.txt | 2 ++
 arch/arm/boot/dts/qcom-apq8064.dtsi                     | 3 ++-
 arch/arm/boot/dts/qcom-ipq8064.dtsi                     | 3 ++-
 arch/arm/boot/dts/qcom-msm8960.dtsi                     | 3 ++-
 4 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/watchdog/qcom-wdt.txt b/Documentation/devicetree/bindings/watchdog/qcom-wdt.txt
index 4726924..6b7b5ce 100644
--- a/Documentation/devicetree/bindings/watchdog/qcom-wdt.txt
+++ b/Documentation/devicetree/bindings/watchdog/qcom-wdt.txt
@@ -7,6 +7,8 @@ Required properties :
 			"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
diff --git a/arch/arm/boot/dts/qcom-apq8064.dtsi b/arch/arm/boot/dts/qcom-apq8064.dtsi
index df96ccd..e318d04 100644
--- a/arch/arm/boot/dts/qcom-apq8064.dtsi
+++ b/arch/arm/boot/dts/qcom-apq8064.dtsi
@@ -247,7 +247,8 @@
 		};
 
 		timer@200a000 {
-			compatible = "qcom,kpss-timer", "qcom,msm-timer";
+			compatible = "qcom,kpss-timer",
+				     "qcom,kpss-wdt-apq8064", "qcom,msm-timer";
 			interrupts = <1 1 0x301>,
 				     <1 2 0x301>,
 				     <1 3 0x301>;
diff --git a/arch/arm/boot/dts/qcom-ipq8064.dtsi b/arch/arm/boot/dts/qcom-ipq8064.dtsi
index 2601a90..2e37557 100644
--- a/arch/arm/boot/dts/qcom-ipq8064.dtsi
+++ b/arch/arm/boot/dts/qcom-ipq8064.dtsi
@@ -122,7 +122,8 @@
 		};
 
 		timer@200a000 {
-			compatible = "qcom,kpss-timer", "qcom,msm-timer";
+			compatible = "qcom,kpss-timer",
+				     "qcom,kpss-wdt-ipq8064", "qcom,msm-timer";
 			interrupts = <1 1 0x301>,
 				     <1 2 0x301>,
 				     <1 3 0x301>,
diff --git a/arch/arm/boot/dts/qcom-msm8960.dtsi b/arch/arm/boot/dts/qcom-msm8960.dtsi
index da05e28..288f56e 100644
--- a/arch/arm/boot/dts/qcom-msm8960.dtsi
+++ b/arch/arm/boot/dts/qcom-msm8960.dtsi
@@ -87,7 +87,8 @@
 		};
 
 		timer@200a000 {
-			compatible = "qcom,kpss-timer", "qcom,msm-timer";
+			compatible = "qcom,kpss-timer",
+				     "qcom,kpss-wdt-msm8960", "qcom,msm-timer";
 			interrupts = <1 1 0x301>,
 				     <1 2 0x301>,
 				     <1 3 0x301>;
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v2 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>
Signed-off-by: Thomas Pedersen <twp@codeaurora.org>

---
v2:
use of_device_get_match_data()
---
 drivers/watchdog/qcom-wdt.c | 64 +++++++++++++++++++++++++++++++++------------
 1 file changed, 48 insertions(+), 16 deletions(-)

diff --git a/drivers/watchdog/qcom-wdt.c b/drivers/watchdog/qcom-wdt.c
index a043fa4..19a6cf0 100644
--- a/drivers/watchdog/qcom-wdt.c
+++ b/drivers/watchdog/qcom-wdt.c
@@ -18,19 +18,42 @@
 #include <linux/of.h>
 #include <linux/platform_device.h>
 #include <linux/watchdog.h>
+#include <linux/of_device.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 +64,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 +75,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 +83,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 +106,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.
@@ -119,9 +142,16 @@ 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 u32 *regs;
 	u32 percpu_offset;
 	int ret;
 
+	regs = of_device_get_match_data(&pdev->dev);
+	if (!regs) {
+		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 +202,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 = regs;
 
 	if (readl(wdt->base + WDT_STS) & 1)
 		wdt->wdd.bootstatus = WDIOF_CARDRESET;
@@ -208,8 +239,9 @@ static int qcom_wdt_remove(struct platform_device *pdev)
 }
 
 static const struct of_device_id qcom_wdt_of_table[] = {
-	{ .compatible = "qcom,kpss-timer" },
-	{ .compatible = "qcom,scss-timer" },
+	{ .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);
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v2 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>
Reviewed-by: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Thomas Pedersen <twp@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 19a6cf0..c9ae432 100644
--- a/drivers/watchdog/qcom-wdt.c
+++ b/drivers/watchdog/qcom-wdt.c
@@ -24,6 +24,7 @@ enum wdt_reg {
 	WDT_RST,
 	WDT_EN,
 	WDT_STS,
+	WDT_BARK_TIME,
 	WDT_BITE_TIME,
 };
 
@@ -31,6 +32,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,
 };
 
@@ -38,6 +40,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,
 };
 
@@ -66,6 +69,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;
@@ -108,6 +112,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 v2 2/3] watchdog: qcom: add option for standalone watchdog not in timer block

[Guenter Roeck]

On Tue, Jun 28, 2016 at 11:35:20AM -0700, 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>
> Signed-off-by: Thomas Pedersen <twp@codeaurora.org>
> 
> ---
> v2:
> use of_device_get_match_data()
> ---
>  drivers/watchdog/qcom-wdt.c | 64 +++++++++++++++++++++++++++++++++------------
>  1 file changed, 48 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/watchdog/qcom-wdt.c b/drivers/watchdog/qcom-wdt.c
> index a043fa4..19a6cf0 100644
> --- a/drivers/watchdog/qcom-wdt.c
> +++ b/drivers/watchdog/qcom-wdt.c
> @@ -18,19 +18,42 @@
>  #include <linux/of.h>
>  #include <linux/platform_device.h>
>  #include <linux/watchdog.h>
> +#include <linux/of_device.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 +64,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 +75,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 +83,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 +106,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.
> @@ -119,9 +142,16 @@ 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 u32 *regs;
>  	u32 percpu_offset;
>  	int ret;
>  
> +	regs = of_device_get_match_data(&pdev->dev);
> +	if (!regs) {
> +		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 +202,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 = regs;
>  
>  	if (readl(wdt->base + WDT_STS) & 1)
>  		wdt->wdd.bootstatus = WDIOF_CARDRESET;
> @@ -208,8 +239,9 @@ static int qcom_wdt_remove(struct platform_device *pdev)
>  }
>  
>  static const struct of_device_id qcom_wdt_of_table[] = {
> -	{ .compatible = "qcom,kpss-timer" },
> -	{ .compatible = "qcom,scss-timer" },
> +	{ .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},

Unnecessary '&'. Also, I'll have to look back at the earlier comments; it
seems to me that 'qcom,kpss-standalone' may be a less than optimal name
for this binding. Not sure if 'qcom,kpss-core' would be better (if that is
what it is). Either case, whatever we end up with, it will need to be
documented in the bindings.

Sorry, should have looked into it more deeply earlier.

Thanks,
Guenter

>  	{ },
>  };
>  MODULE_DEVICE_TABLE(of, qcom_wdt_of_table);
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
> 

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

[Thomas Pedersen]

On 2016-06-28 12:47, Guenter Roeck wrote:
> On Tue, Jun 28, 2016 at 11:35:20AM -0700, 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>
>> Signed-off-by: Thomas Pedersen <twp@codeaurora.org>
>> 
>> ---
>> v2:
>> use of_device_get_match_data()
>> ---
>>  drivers/watchdog/qcom-wdt.c | 64 
>> +++++++++++++++++++++++++++++++++------------
>>  1 file changed, 48 insertions(+), 16 deletions(-)
>> 
>> diff --git a/drivers/watchdog/qcom-wdt.c b/drivers/watchdog/qcom-wdt.c
>> index a043fa4..19a6cf0 100644
>> --- a/drivers/watchdog/qcom-wdt.c
>> +++ b/drivers/watchdog/qcom-wdt.c
>> @@ -18,19 +18,42 @@
>>  #include <linux/of.h>
>>  #include <linux/platform_device.h>
>>  #include <linux/watchdog.h>
>> +#include <linux/of_device.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 +64,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 +75,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 +83,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 +106,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.
>> @@ -119,9 +142,16 @@ 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 u32 *regs;
>>  	u32 percpu_offset;
>>  	int ret;
>> 
>> +	regs = of_device_get_match_data(&pdev->dev);
>> +	if (!regs) {
>> +		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 +202,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 = regs;
>> 
>>  	if (readl(wdt->base + WDT_STS) & 1)
>>  		wdt->wdd.bootstatus = WDIOF_CARDRESET;
>> @@ -208,8 +239,9 @@ static int qcom_wdt_remove(struct platform_device 
>> *pdev)
>>  }
>> 
>>  static const struct of_device_id qcom_wdt_of_table[] = {
>> -	{ .compatible = "qcom,kpss-timer" },
>> -	{ .compatible = "qcom,scss-timer" },
>> +	{ .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},
> 
> Unnecessary '&'. Also, I'll have to look back at the earlier comments; 
> it
> seems to me that 'qcom,kpss-standalone' may be a less than optimal name
> for this binding. Not sure if 'qcom,kpss-core' would be better (if that 
> is
> what it is). Either case, whatever we end up with, it will need to be
> documented in the bindings.

I'll rectify the missing documentation, and am fine with changing the 
name to
kpss-core.

-- 
thomas

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

[Matthew McClintock]

> On Jun 28, 2016, at 3:45 PM, Thomas Pedersen <twp@codeaurora.org> wrote:
> 
> I'll rectify the missing documentation, and am fine with changing the name to
> kpss-core.

I think the previous conclusion was to rename it to something with ipq4019 in the compat, but I’d suggest checking the comments in patchworks on the previous patches.

-M

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

[Guenter Roeck]

On 06/28/2016 02:24 PM, Matthew McClintock wrote:
>
>> On Jun 28, 2016, at 3:45 PM, Thomas Pedersen <twp@codeaurora.org> wrote:
>>
>> I'll rectify the missing documentation, and am fine with changing the name to
>> kpss-core.
>
> I think the previous conclusion was to rename it to something with ipq4019 in the compat, but I’d suggest checking the comments in patchworks on the previous patches.
>

I looked up my notes. Here is what I had suggested earlier:

"The property name should probably be something like 'qcom,kpss-wdt'
(or 'qcom,kpss-watchdog' ?), possibly in addition to 'qcom,kpss-ipq4019-wdt'
and 'qcom,kpss-msm8916-wdt'."

Guenter