[PATCH 0/4] watchdog: meson: Add meson8b watchdog

[PATCH 0/4] watchdog: meson: Add meson8b watchdog

[Carlo Caione]

From: Carlo Caione <carlo@endlessm.com>

Patchset to add support for the watchdog found on the Amlogic meson8b SoCs.
DTS and documentation are also updated.

Carlo Caione (4):
  watchdog: meson: Enable meson SoC specific data
  watchdog: meson: Add meson8b SoC specific data
  Documentation: watchdog: Add new bindings for meson8b
  ARM: dts: meson8b: Add watchdog node

 .../devicetree/bindings/watchdog/meson-wdt.txt     | 13 +++++
 .../devicetree/bindings/watchdog/meson6-wdt.txt    | 13 -----
 arch/arm/boot/dts/meson8b.dtsi                     |  6 +++
 drivers/watchdog/meson_wdt.c                       | 62 ++++++++++++++++------
 4 files changed, 64 insertions(+), 30 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/watchdog/meson-wdt.txt
 delete mode 100644 Documentation/devicetree/bindings/watchdog/meson6-wdt.txt

-- 
1.9.1

[PATCH 1/4] watchdog: meson: Enable meson SoC specific data

[Carlo Caione]

From: Carlo Caione <carlo@endlessm.com>

With this patch we refactor the driver code to enable watchdog support
for all platforms based on Amlogic meson SoCs.

Signed-off-by: Carlo Caione <carlo@endlessm.com>
---
 drivers/watchdog/meson_wdt.c | 55 ++++++++++++++++++++++++++++++--------------
 1 file changed, 38 insertions(+), 17 deletions(-)

diff --git a/drivers/watchdog/meson_wdt.c b/drivers/watchdog/meson_wdt.c
index 1f4155e..89944ed 100644
--- a/drivers/watchdog/meson_wdt.c
+++ b/drivers/watchdog/meson_wdt.c
@@ -19,6 +19,7 @@
 #include <linux/moduleparam.h>
 #include <linux/notifier.h>
 #include <linux/of.h>
+#include <linux/of_device.h>
 #include <linux/platform_device.h>
 #include <linux/reboot.h>
 #include <linux/types.h>
@@ -27,34 +28,45 @@
 #define DRV_NAME		"meson_wdt"
 
 #define MESON_WDT_TC		0x00
-#define MESON_WDT_TC_EN		BIT(22)
-#define MESON_WDT_TC_TM_MASK	0x3fffff
 #define MESON_WDT_DC_RESET	(3 << 24)
 
 #define MESON_WDT_RESET		0x04
 
-#define MESON_WDT_TIMEOUT	30
+#define MESON_WDT_TIMEOUT	5
 #define MESON_WDT_MIN_TIMEOUT	1
-#define MESON_WDT_MAX_TIMEOUT	(MESON_WDT_TC_TM_MASK / 100000)
 
-#define MESON_SEC_TO_TC(s)	((s) * 100000)
+#define MESON_SEC_TO_TC(s, c)	((s) * (c))
 
 static bool nowayout = WATCHDOG_NOWAYOUT;
 static unsigned int timeout = MESON_WDT_TIMEOUT;
 
+struct meson_wdt_data {
+	unsigned int shift_enable;
+	unsigned int terminal_count_mask;
+	unsigned int count_unit;
+};
+
+static struct meson_wdt_data meson6_wdt_data = {
+	.shift_enable		= 22,
+	.terminal_count_mask	= 0x3fffff,
+	.count_unit		= 100000, /* 10 us */
+};
+
 struct meson_wdt_dev {
 	struct watchdog_device wdt_dev;
 	void __iomem *wdt_base;
 	struct notifier_block restart_handler;
+	struct meson_wdt_data *data;
 };
 
 static int meson_restart_handle(struct notifier_block *this, unsigned long mode,
 				void *cmd)
 {
-	u32 tc_reboot = MESON_WDT_DC_RESET | MESON_WDT_TC_EN;
+	u32 tc_reboot = MESON_WDT_DC_RESET;
 	struct meson_wdt_dev *meson_wdt = container_of(this,
 						       struct meson_wdt_dev,
 						       restart_handler);
+	tc_reboot |= BIT(meson_wdt->data->shift_enable);
 
 	while (1) {
 		writel(tc_reboot, meson_wdt->wdt_base + MESON_WDT_TC);
@@ -80,8 +92,8 @@ static void meson_wdt_change_timeout(struct watchdog_device *wdt_dev,
 	u32 reg;
 
 	reg = readl(meson_wdt->wdt_base + MESON_WDT_TC);
-	reg &= ~MESON_WDT_TC_TM_MASK;
-	reg |= MESON_SEC_TO_TC(timeout);
+	reg &= ~(meson_wdt->data->terminal_count_mask);
+	reg |= MESON_SEC_TO_TC(timeout, meson_wdt->data->count_unit);
 	writel(reg, meson_wdt->wdt_base + MESON_WDT_TC);
 }
 
@@ -102,7 +114,7 @@ static int meson_wdt_stop(struct watchdog_device *wdt_dev)
 	u32 reg;
 
 	reg = readl(meson_wdt->wdt_base + MESON_WDT_TC);
-	reg &= ~MESON_WDT_TC_EN;
+	reg &= ~BIT(meson_wdt->data->shift_enable);
 	writel(reg, meson_wdt->wdt_base + MESON_WDT_TC);
 
 	return 0;
@@ -117,7 +129,7 @@ static int meson_wdt_start(struct watchdog_device *wdt_dev)
 	meson_wdt_ping(wdt_dev);
 
 	reg = readl(meson_wdt->wdt_base + MESON_WDT_TC);
-	reg |= MESON_WDT_TC_EN;
+	reg |= BIT(meson_wdt->data->shift_enable);
 	writel(reg, meson_wdt->wdt_base + MESON_WDT_TC);
 
 	return 0;
@@ -138,10 +150,17 @@ static const struct watchdog_ops meson_wdt_ops = {
 	.set_timeout	= meson_wdt_set_timeout,
 };
 
+static const struct of_device_id meson_wdt_dt_ids[] = {
+	{ .compatible = "amlogic,meson6-wdt", .data = &meson6_wdt_data },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, meson_wdt_dt_ids);
+
 static int meson_wdt_probe(struct platform_device *pdev)
 {
 	struct resource *res;
 	struct meson_wdt_dev *meson_wdt;
+	const struct of_device_id *of_id;
 	int err;
 
 	meson_wdt = devm_kzalloc(&pdev->dev, sizeof(*meson_wdt), GFP_KERNEL);
@@ -153,11 +172,19 @@ static int meson_wdt_probe(struct platform_device *pdev)
 	if (IS_ERR(meson_wdt->wdt_base))
 		return PTR_ERR(meson_wdt->wdt_base);
 
+	of_id = of_match_device(meson_wdt_dt_ids, &pdev->dev);
+	if (!of_id) {
+		dev_err(&pdev->dev, "Unable to setup WDT data\n");
+		return -ENODEV;
+	}
+	meson_wdt->data = (struct meson_wdt_data *) of_id->data;
+
 	meson_wdt->wdt_dev.parent = &pdev->dev;
 	meson_wdt->wdt_dev.info = &meson_wdt_info;
 	meson_wdt->wdt_dev.ops = &meson_wdt_ops;
 	meson_wdt->wdt_dev.timeout = MESON_WDT_TIMEOUT;
-	meson_wdt->wdt_dev.max_timeout = MESON_WDT_MAX_TIMEOUT;
+	meson_wdt->wdt_dev.max_timeout =
+		meson_wdt->data->terminal_count_mask / meson_wdt->data->count_unit;
 	meson_wdt->wdt_dev.min_timeout = MESON_WDT_MIN_TIMEOUT;
 
 	watchdog_set_drvdata(&meson_wdt->wdt_dev, meson_wdt);
@@ -204,12 +231,6 @@ static void meson_wdt_shutdown(struct platform_device *pdev)
 	meson_wdt_stop(&meson_wdt->wdt_dev);
 }
 
-static const struct of_device_id meson_wdt_dt_ids[] = {
-	{ .compatible = "amlogic,meson6-wdt" },
-	{ /* sentinel */ }
-};
-MODULE_DEVICE_TABLE(of, meson_wdt_dt_ids);
-
 static struct platform_driver meson_wdt_driver = {
 	.probe		= meson_wdt_probe,
 	.remove		= meson_wdt_remove,
-- 
1.9.1

[PATCH 2/4] watchdog: meson: Add meson8b SoC specific data

[Carlo Caione]

From: Carlo Caione <carlo@endlessm.com>

Add SoC specific data in the watchdog driver for the meson8b SoC.

Signed-off-by: Carlo Caione <carlo@endlessm.com>
---
 drivers/watchdog/meson_wdt.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/watchdog/meson_wdt.c b/drivers/watchdog/meson_wdt.c
index 89944ed..75c84f5 100644
--- a/drivers/watchdog/meson_wdt.c
+++ b/drivers/watchdog/meson_wdt.c
@@ -52,6 +52,12 @@ static struct meson_wdt_data meson6_wdt_data = {
 	.count_unit		= 100000, /* 10 us */
 };
 
+static struct meson_wdt_data meson8b_wdt_data = {
+	.shift_enable		= 19,
+	.terminal_count_mask	= 0xffff,
+	.count_unit		= 7812, /* 128 us */
+};
+
 struct meson_wdt_dev {
 	struct watchdog_device wdt_dev;
 	void __iomem *wdt_base;
@@ -152,6 +158,7 @@ static const struct watchdog_ops meson_wdt_ops = {
 
 static const struct of_device_id meson_wdt_dt_ids[] = {
 	{ .compatible = "amlogic,meson6-wdt", .data = &meson6_wdt_data },
+	{ .compatible = "amlogic,meson8b-wdt", .data = &meson8b_wdt_data },
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, meson_wdt_dt_ids);
-- 
1.9.1

[PATCH 3/4] Documentation: watchdog: Add new bindings for meson8b

[Carlo Caione]

From: Carlo Caione <carlo@endlessm.com>

We make the documentation not meson6 specific anymore and add the new
binding for meson8b SoC.

Signed-off-by: Carlo Caione <carlo@endlessm.com>
---
 Documentation/devicetree/bindings/watchdog/meson-wdt.txt  | 13 +++++++++++++
 Documentation/devicetree/bindings/watchdog/meson6-wdt.txt | 13 -------------
 2 files changed, 13 insertions(+), 13 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/watchdog/meson-wdt.txt
 delete mode 100644 Documentation/devicetree/bindings/watchdog/meson6-wdt.txt

diff --git a/Documentation/devicetree/bindings/watchdog/meson-wdt.txt b/Documentation/devicetree/bindings/watchdog/meson-wdt.txt
new file mode 100644
index 0000000..ae70185
--- /dev/null
+++ b/Documentation/devicetree/bindings/watchdog/meson-wdt.txt
@@ -0,0 +1,13 @@
+Meson SoCs Watchdog timer
+
+Required properties:
+
+- compatible : should be "amlogic,meson6-wdt" or "amlogic,meson8b-wdt"
+- reg : Specifies base physical address and size of the registers.
+
+Example:
+
+wdt: watchdog at c1109900 {
+	compatible = "amlogic,meson6-wdt";
+	reg = <0xc1109900 0x8>;
+};
diff --git a/Documentation/devicetree/bindings/watchdog/meson6-wdt.txt b/Documentation/devicetree/bindings/watchdog/meson6-wdt.txt
deleted file mode 100644
index 9200fc2..0000000
--- a/Documentation/devicetree/bindings/watchdog/meson6-wdt.txt
+++ /dev/null
@@ -1,13 +0,0 @@
-Meson SoCs Watchdog timer
-
-Required properties:
-
-- compatible : should be "amlogic,meson6-wdt"
-- reg : Specifies base physical address and size of the registers.
-
-Example:
-
-wdt: watchdog at c1109900 {
-	compatible = "amlogic,meson6-wdt";
-	reg = <0xc1109900 0x8>;
-};
-- 
1.9.1

[PATCH 4/4] ARM: dts: meson8b: Add watchdog node

[Carlo Caione]

From: Carlo Caione <carlo@endlessm.com>

With this patch we add the watchdog node in the meson8b DTS file.

Signed-off-by: Carlo Caione <carlo@endlessm.com>
---
 arch/arm/boot/dts/meson8b.dtsi | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm/boot/dts/meson8b.dtsi b/arch/arm/boot/dts/meson8b.dtsi
index ee352bf..8bad557 100644
--- a/arch/arm/boot/dts/meson8b.dtsi
+++ b/arch/arm/boot/dts/meson8b.dtsi
@@ -105,6 +105,12 @@
 			#interrupt-cells = <3>;
 		};
 
+		wdt: watchdog at c1109900 {
+			compatible = "amlogic,meson8b-wdt";
+			reg = <0xc1109900 0x8>;
+			interrupts = <0 0 1>;
+		};
+
 		timer at c1109940 {
 			compatible = "amlogic,meson6-timer";
 			reg = <0xc1109940 0x18>;
-- 
1.9.1

[PATCH 1/4] watchdog: meson: Enable meson SoC specific data

[Guenter Roeck]

On Fri, Nov 06, 2015 at 10:55:03PM +0100, Carlo Caione wrote:
> From: Carlo Caione <carlo@endlessm.com>
> 
> With this patch we refactor the driver code to enable watchdog support
> for all platforms based on Amlogic meson SoCs.
> 
> Signed-off-by: Carlo Caione <carlo@endlessm.com>
> ---
>  drivers/watchdog/meson_wdt.c | 55 ++++++++++++++++++++++++++++++--------------
>  1 file changed, 38 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/watchdog/meson_wdt.c b/drivers/watchdog/meson_wdt.c
> index 1f4155e..89944ed 100644
> --- a/drivers/watchdog/meson_wdt.c
> +++ b/drivers/watchdog/meson_wdt.c
> @@ -19,6 +19,7 @@
>  #include <linux/moduleparam.h>
>  #include <linux/notifier.h>
>  #include <linux/of.h>
> +#include <linux/of_device.h>
>  #include <linux/platform_device.h>
>  #include <linux/reboot.h>
>  #include <linux/types.h>
> @@ -27,34 +28,45 @@
>  #define DRV_NAME		"meson_wdt"
>  
>  #define MESON_WDT_TC		0x00
> -#define MESON_WDT_TC_EN		BIT(22)
> -#define MESON_WDT_TC_TM_MASK	0x3fffff
>  #define MESON_WDT_DC_RESET	(3 << 24)
>  
>  #define MESON_WDT_RESET		0x04
>  
> -#define MESON_WDT_TIMEOUT	30
> +#define MESON_WDT_TIMEOUT	5
>  #define MESON_WDT_MIN_TIMEOUT	1
> -#define MESON_WDT_MAX_TIMEOUT	(MESON_WDT_TC_TM_MASK / 100000)
>  
> -#define MESON_SEC_TO_TC(s)	((s) * 100000)
> +#define MESON_SEC_TO_TC(s, c)	((s) * (c))
>  
>  static bool nowayout = WATCHDOG_NOWAYOUT;
>  static unsigned int timeout = MESON_WDT_TIMEOUT;
>  
> +struct meson_wdt_data {
> +	unsigned int shift_enable;
> +	unsigned int terminal_count_mask;
> +	unsigned int count_unit;
> +};
> +
> +static struct meson_wdt_data meson6_wdt_data = {
> +	.shift_enable		= 22,

I have to admit that I completely fail to understand why it would be
better to move the bit calculation from a constant to runtime.

Can you explain ?

I am also not really a friend of removing definitions, even if they are
only used once. But I understand that this is a matter of opinion.

> +	.terminal_count_mask	= 0x3fffff,
> +	.count_unit		= 100000, /* 10 us */
> +};
> +
>  struct meson_wdt_dev {
>  	struct watchdog_device wdt_dev;
>  	void __iomem *wdt_base;
>  	struct notifier_block restart_handler;
> +	struct meson_wdt_data *data;
>  };
>  
>  static int meson_restart_handle(struct notifier_block *this, unsigned long mode,
>  				void *cmd)
>  {
> -	u32 tc_reboot = MESON_WDT_DC_RESET | MESON_WDT_TC_EN;
> +	u32 tc_reboot = MESON_WDT_DC_RESET;
>  	struct meson_wdt_dev *meson_wdt = container_of(this,
>  						       struct meson_wdt_dev,
>  						       restart_handler);
> +	tc_reboot |= BIT(meson_wdt->data->shift_enable);
>  

I am quite sure that this results in a checkpatch warning.
Did you run your patch through checkpatch ?

>  	while (1) {
>  		writel(tc_reboot, meson_wdt->wdt_base + MESON_WDT_TC);
> @@ -80,8 +92,8 @@ static void meson_wdt_change_timeout(struct watchdog_device *wdt_dev,
>  	u32 reg;
>  
>  	reg = readl(meson_wdt->wdt_base + MESON_WDT_TC);
> -	reg &= ~MESON_WDT_TC_TM_MASK;
> -	reg |= MESON_SEC_TO_TC(timeout);
> +	reg &= ~(meson_wdt->data->terminal_count_mask);

Unnecessary ( )

> +	reg |= MESON_SEC_TO_TC(timeout, meson_wdt->data->count_unit);
>  	writel(reg, meson_wdt->wdt_base + MESON_WDT_TC);
>  }
>  
> @@ -102,7 +114,7 @@ static int meson_wdt_stop(struct watchdog_device *wdt_dev)
>  	u32 reg;
>  
>  	reg = readl(meson_wdt->wdt_base + MESON_WDT_TC);
> -	reg &= ~MESON_WDT_TC_EN;
> +	reg &= ~BIT(meson_wdt->data->shift_enable);
>  	writel(reg, meson_wdt->wdt_base + MESON_WDT_TC);
>  
>  	return 0;
> @@ -117,7 +129,7 @@ static int meson_wdt_start(struct watchdog_device *wdt_dev)
>  	meson_wdt_ping(wdt_dev);
>  
>  	reg = readl(meson_wdt->wdt_base + MESON_WDT_TC);
> -	reg |= MESON_WDT_TC_EN;
> +	reg |= BIT(meson_wdt->data->shift_enable);
>  	writel(reg, meson_wdt->wdt_base + MESON_WDT_TC);
>  
>  	return 0;
> @@ -138,10 +150,17 @@ static const struct watchdog_ops meson_wdt_ops = {
>  	.set_timeout	= meson_wdt_set_timeout,
>  };
>  
> +static const struct of_device_id meson_wdt_dt_ids[] = {
> +	{ .compatible = "amlogic,meson6-wdt", .data = &meson6_wdt_data },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, meson_wdt_dt_ids);
> +
>  static int meson_wdt_probe(struct platform_device *pdev)
>  {
>  	struct resource *res;
>  	struct meson_wdt_dev *meson_wdt;
> +	const struct of_device_id *of_id;
>  	int err;
>  
>  	meson_wdt = devm_kzalloc(&pdev->dev, sizeof(*meson_wdt), GFP_KERNEL);
> @@ -153,11 +172,19 @@ static int meson_wdt_probe(struct platform_device *pdev)
>  	if (IS_ERR(meson_wdt->wdt_base))
>  		return PTR_ERR(meson_wdt->wdt_base);
>  
> +	of_id = of_match_device(meson_wdt_dt_ids, &pdev->dev);
> +	if (!of_id) {
> +		dev_err(&pdev->dev, "Unable to setup WDT data\n");

"set up" or maybe better initialize.

> +		return -ENODEV;
> +	}
> +	meson_wdt->data = (struct meson_wdt_data *) of_id->data;

Is this typecase necessary ?

> +
>  	meson_wdt->wdt_dev.parent = &pdev->dev;
>  	meson_wdt->wdt_dev.info = &meson_wdt_info;
>  	meson_wdt->wdt_dev.ops = &meson_wdt_ops;
>  	meson_wdt->wdt_dev.timeout = MESON_WDT_TIMEOUT;
> -	meson_wdt->wdt_dev.max_timeout = MESON_WDT_MAX_TIMEOUT;
> +	meson_wdt->wdt_dev.max_timeout =
> +		meson_wdt->data->terminal_count_mask / meson_wdt->data->count_unit;
>  	meson_wdt->wdt_dev.min_timeout = MESON_WDT_MIN_TIMEOUT;
>  
>  	watchdog_set_drvdata(&meson_wdt->wdt_dev, meson_wdt);
> @@ -204,12 +231,6 @@ static void meson_wdt_shutdown(struct platform_device *pdev)
>  	meson_wdt_stop(&meson_wdt->wdt_dev);
>  }
>  
> -static const struct of_device_id meson_wdt_dt_ids[] = {
> -	{ .compatible = "amlogic,meson6-wdt" },
> -	{ /* sentinel */ }
> -};
> -MODULE_DEVICE_TABLE(of, meson_wdt_dt_ids);
> -
>  static struct platform_driver meson_wdt_driver = {
>  	.probe		= meson_wdt_probe,
>  	.remove		= meson_wdt_remove,
> -- 
> 1.9.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/4] watchdog: meson: Enable meson SoC specific data

[Carlo Caione]

On Fri, Nov 6, 2015 at 11:15 PM, Guenter Roeck <linux@roeck-us.net> wrote:

>> +struct meson_wdt_data {
>> +     unsigned int shift_enable;
>> +     unsigned int terminal_count_mask;
>> +     unsigned int count_unit;
>> +};
>> +
>> +static struct meson_wdt_data meson6_wdt_data = {
>> +     .shift_enable           = 22,
>
> I have to admit that I completely fail to understand why it would be
> better to move the bit calculation from a constant to runtime.
>
> Can you explain ?

No real reasons and you are right. I'll change it back.

> I am also not really a friend of removing definitions, even if they are
> only used once. But I understand that this is a matter of opinion.
>
>> +     .terminal_count_mask    = 0x3fffff,
>> +     .count_unit             = 100000, /* 10 us */
>> +};
>> +
>>  struct meson_wdt_dev {
>>       struct watchdog_device wdt_dev;
>>       void __iomem *wdt_base;
>>       struct notifier_block restart_handler;
>> +     struct meson_wdt_data *data;
>>  };
>>
>>  static int meson_restart_handle(struct notifier_block *this, unsigned long mode,
>>                               void *cmd)
>>  {
>> -     u32 tc_reboot = MESON_WDT_DC_RESET | MESON_WDT_TC_EN;
>> +     u32 tc_reboot = MESON_WDT_DC_RESET;
>>       struct meson_wdt_dev *meson_wdt = container_of(this,
>>                                                      struct meson_wdt_dev,
>>                                                      restart_handler);
>> +     tc_reboot |= BIT(meson_wdt->data->shift_enable);
>>
>
> I am quite sure that this results in a checkpatch warning.
> Did you run your patch through checkpatch ?

Yeah, no error/warnings on that line.

>>       while (1) {
>>               writel(tc_reboot, meson_wdt->wdt_base + MESON_WDT_TC);
>> @@ -80,8 +92,8 @@ static void meson_wdt_change_timeout(struct watchdog_device *wdt_dev,
>>       u32 reg;
>>
>>       reg = readl(meson_wdt->wdt_base + MESON_WDT_TC);
>> -     reg &= ~MESON_WDT_TC_TM_MASK;
>> -     reg |= MESON_SEC_TO_TC(timeout);
>> +     reg &= ~(meson_wdt->data->terminal_count_mask);
>
> Unnecessary ( )

Ok

>>       meson_wdt = devm_kzalloc(&pdev->dev, sizeof(*meson_wdt), GFP_KERNEL);
>> @@ -153,11 +172,19 @@ static int meson_wdt_probe(struct platform_device *pdev)
>>       if (IS_ERR(meson_wdt->wdt_base))
>>               return PTR_ERR(meson_wdt->wdt_base);
>>
>> +     of_id = of_match_device(meson_wdt_dt_ids, &pdev->dev);
>> +     if (!of_id) {
>> +             dev_err(&pdev->dev, "Unable to setup WDT data\n");
>
> "set up" or maybe better initialize.

Ok

>> +             return -ENODEV;
>> +     }
>> +     meson_wdt->data = (struct meson_wdt_data *) of_id->data;
>
> Is this typecase necessary ?

Just for clarity. Not needed.

I'll fix everything in v2. Thanks.

-- 
Carlo Caione

[PATCH 1/4] watchdog: meson: Enable meson SoC specific data

[Guenter Roeck]

On 11/06/2015 02:47 PM, Carlo Caione wrote:
> On Fri, Nov 6, 2015 at 11:15 PM, Guenter Roeck <linux@roeck-us.net> wrote:
>
[ ... ]

>>>
>>>   static int meson_restart_handle(struct notifier_block *this, unsigned long mode,
>>>                                void *cmd)
>>>   {
>>> -     u32 tc_reboot = MESON_WDT_DC_RESET | MESON_WDT_TC_EN;
>>> +     u32 tc_reboot = MESON_WDT_DC_RESET;
>>>        struct meson_wdt_dev *meson_wdt = container_of(this,
>>>                                                       struct meson_wdt_dev,
>>>                                                       restart_handler);
>>> +     tc_reboot |= BIT(meson_wdt->data->shift_enable);
>>>
>>
>> I am quite sure that this results in a checkpatch warning.
>> Did you run your patch through checkpatch ?
>
> Yeah, no error/warnings on that line.
>

There should be a warning since there is no empty line after the declarations.
Maybe checkpatch doesn't catch it because of the continuation line above.

Still, the extra line would be appreciated.

Thanks,
Guenter