Re: [PATCH V9 2/3] watchdog: s3c2410_wdt: use syscon regmap interface to configure pmu register
Hi Doug,
Thanks for reviewing the patch series.
On Tue, Nov 26, 2013 at 4:14 AM, Doug Anderson diand...@chromium.org wrote:
Hi Leela Krishna,
On Mon, Nov 18, 2013 at 1:49 AM, Leela Krishna Amudala
l.kris...@samsung.com wrote:
Add device tree support for exynos5250 and 5420 SoCs and use syscon regmap
interface
to configure AUTOMATIC_WDT_RESET_DISABLE and MASK_WDT_RESET_REQUEST
registers of PMU
to mask/unmask enable/disable of watchdog in probe and s2r scenarios.
Signed-off-by: Leela Krishna Amudala l.kris...@samsung.com
---
.../devicetree/bindings/watchdog/samsung-wdt.txt | 21 ++-
drivers/watchdog/Kconfig |1 +
drivers/watchdog/s3c2410_wdt.c | 145
++--
3 files changed, 157 insertions(+), 10 deletions(-)
...
+struct s3c2410_wdt_variant {
+ int disable_reg;
+ int mask_reset_reg;
+ int mask_bit;
+ u32 quirks;
+};
Ideally you could add descriptions. I added them in a patch based on
yours
https://chromium-review.googlesource.com/#/c/177935/1/drivers/watchdog/s3c2410_wdt.c,
but since yours hasn't landed perhaps you can do it directly?
/**
* struct s3c2410_wdt_variant - Per-variant config data
*
* @disable_reg: Offset in pmureg for the register that disables the watchdog
* timer reset functionality.
* @mask_reset_reg: Offset in pmureg for the register that masks the watchdog
* timer reset functionality.
* @mask_bit: Bit number for the watchdog timer in the disable register and
the
* mask reset register.
* @quirks: A bitfield of quirks.
*/
Okay, Added the above descriptions
+
struct s3c2410_wdt {
struct device *dev;
struct clk *clock;
@@ -94,7 +107,49 @@ struct s3c2410_wdt {
unsigned long wtdat_save;
struct watchdog_device wdt_device;
struct notifier_block freq_transition;
+ struct s3c2410_wdt_variant *drv_data;
+ struct regmap *pmureg;
Total nit, but everything else in this structure is tab aligned and
your new elements are not.
Me and Tomasz had a discussion about it and he replied for this comment
static int s3c2410wdt_probe(struct platform_device *pdev)
{
struct device *dev;
@@ -354,6 +443,16 @@ static int s3c2410wdt_probe(struct platform_device
*pdev)
spin_lock_init(wdt-lock);
wdt-wdt_device = s3c2410_wdd;
+ wdt-drv_data = get_wdt_drv_data(pdev);
+ if (wdt-drv_data-quirks QUIRK_NEEDS_PMU_CONFIG) {
+ wdt-pmureg = syscon_regmap_lookup_by_phandle(dev-of_node,
+
samsung,syscon-phandle);
+ if (IS_ERR(wdt-pmureg)) {
+ dev_err(dev, syscon regmap lookup failed.\n);
+ return PTR_ERR(wdt-pmureg);
nit: this function appears to never call return directly. You'll
match other error handling better if you do:
ret = PTR_ERR(wdt-pmureg);
goto err;
(if someone else has already said they don't like that, just ignore me).
Will consider Tomasz suggestion for this
+ }
+ }
+
wdt_irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
if (wdt_irq == NULL) {
dev_err(dev, no irq resource specified\n);
@@ -444,6 +543,14 @@ static int s3c2410wdt_probe(struct platform_device
*pdev)
(wtcon S3C2410_WTCON_RSTEN) ? en : dis,
(wtcon S3C2410_WTCON_INTEN) ? en : dis);
+ if (wdt-drv_data-quirks QUIRK_NEEDS_PMU_CONFIG) {
+ ret = s3c2410wdt_mask_and_disable_reset(wdt, false);
+ if (ret 0) {
+ dev_err(wdt-dev, failed to update pmu register);
+ goto err_cpufreq;
+ }
+ }
There are a few minor problems here:
1. You're putting a potential failure condition _after_ printing that
the watchdog is enabled. You should put your code above the
dev_info.
Yes moved it like below
static int s3c2410wdt_probe(struct platform_device *pdev)
{
[snip]
if (tmr_atboot started == 0) {
dev_info(dev, starting watchdog timer\n);
s3c2410wdt_start(wdt-wdt_device);
}
[snip]
2. Error handling doesn't seem quite right. If you fail here you'll
be returning an error but you might have started the watchdog already.
Perhaps you should be moving the mask_and_disable up a little and
then you an undo it if needed?
Above mentioned comment will take care of this
3. I think it would be fine to put the dev_err directly in the
s3c2410wdt_mask_and_disable_reset() as per Guenter, but still return
the error code. You'll still use the error code here, right?
Yes, used dev_err and moved it into s3c2410wdt_mask_and_disable_reset()
and removed the dev_err message in the if (ret 0) check
+
return 0;
Re: [PATCH V9 2/3] watchdog: s3c2410_wdt: use syscon regmap interface to configure pmu register
Hi Leela Krishna,
On Mon, Nov 18, 2013 at 1:49 AM, Leela Krishna Amudala
l.kris...@samsung.com wrote:
Add device tree support for exynos5250 and 5420 SoCs and use syscon regmap
interface
to configure AUTOMATIC_WDT_RESET_DISABLE and MASK_WDT_RESET_REQUEST registers
of PMU
to mask/unmask enable/disable of watchdog in probe and s2r scenarios.
Signed-off-by: Leela Krishna Amudala l.kris...@samsung.com
---
.../devicetree/bindings/watchdog/samsung-wdt.txt | 21 ++-
drivers/watchdog/Kconfig |1 +
drivers/watchdog/s3c2410_wdt.c | 145
++--
3 files changed, 157 insertions(+), 10 deletions(-)
...
+struct s3c2410_wdt_variant {
+ int disable_reg;
+ int mask_reset_reg;
+ int mask_bit;
+ u32 quirks;
+};
Ideally you could add descriptions. I added them in a patch based on
yours
https://chromium-review.googlesource.com/#/c/177935/1/drivers/watchdog/s3c2410_wdt.c,
but since yours hasn't landed perhaps you can do it directly?
/**
* struct s3c2410_wdt_variant - Per-variant config data
*
* @disable_reg: Offset in pmureg for the register that disables the watchdog
* timer reset functionality.
* @mask_reset_reg: Offset in pmureg for the register that masks the watchdog
* timer reset functionality.
* @mask_bit: Bit number for the watchdog timer in the disable register and the
* mask reset register.
* @quirks: A bitfield of quirks.
*/
+
struct s3c2410_wdt {
struct device *dev;
struct clk *clock;
@@ -94,7 +107,49 @@ struct s3c2410_wdt {
unsigned long wtdat_save;
struct watchdog_device wdt_device;
struct notifier_block freq_transition;
+ struct s3c2410_wdt_variant *drv_data;
+ struct regmap *pmureg;
Total nit, but everything else in this structure is tab aligned and
your new elements are not.
static int s3c2410wdt_probe(struct platform_device *pdev)
{
struct device *dev;
@@ -354,6 +443,16 @@ static int s3c2410wdt_probe(struct platform_device *pdev)
spin_lock_init(wdt-lock);
wdt-wdt_device = s3c2410_wdd;
+ wdt-drv_data = get_wdt_drv_data(pdev);
+ if (wdt-drv_data-quirks QUIRK_NEEDS_PMU_CONFIG) {
+ wdt-pmureg = syscon_regmap_lookup_by_phandle(dev-of_node,
+
samsung,syscon-phandle);
+ if (IS_ERR(wdt-pmureg)) {
+ dev_err(dev, syscon regmap lookup failed.\n);
+ return PTR_ERR(wdt-pmureg);
nit: this function appears to never call return directly. You'll
match other error handling better if you do:
ret = PTR_ERR(wdt-pmureg);
goto err;
(if someone else has already said they don't like that, just ignore me).
+ }
+ }
+
wdt_irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
if (wdt_irq == NULL) {
dev_err(dev, no irq resource specified\n);
@@ -444,6 +543,14 @@ static int s3c2410wdt_probe(struct platform_device *pdev)
(wtcon S3C2410_WTCON_RSTEN) ? en : dis,
(wtcon S3C2410_WTCON_INTEN) ? en : dis);
+ if (wdt-drv_data-quirks QUIRK_NEEDS_PMU_CONFIG) {
+ ret = s3c2410wdt_mask_and_disable_reset(wdt, false);
+ if (ret 0) {
+ dev_err(wdt-dev, failed to update pmu register);
+ goto err_cpufreq;
+ }
+ }
There are a few minor problems here:
1. You're putting a potential failure condition _after_ printing that
the watchdog is enabled. You should put your code above the
dev_info.
2. Error handling doesn't seem quite right. If you fail here you'll
be returning an error but you might have started the watchdog already.
Perhaps you should be moving the mask_and_disable up a little and
then you an undo it if needed?
3. I think it would be fine to put the dev_err directly in the
s3c2410wdt_mask_and_disable_reset() as per Guenter, but still return
the error code. You'll still use the error code here, right?
+
return 0;
err_cpufreq:
@@ -459,8 +566,15 @@ static int s3c2410wdt_probe(struct platform_device *pdev)
static int s3c2410wdt_remove(struct platform_device *dev)
{
+ int ret;
struct s3c2410_wdt *wdt = platform_get_drvdata(dev);
+ if (wdt-drv_data-quirks QUIRK_NEEDS_PMU_CONFIG) {
+ ret = s3c2410wdt_mask_and_disable_reset(wdt, true);
This function does return an error. Why not pass the error on
through? The system wouldn't be in such a stellar state, but if
regmap is failing it's probably pretty bad anyway...
Nothing here is terrible and I wouldn't be upset if you landed these
as-is, but since it seems that Guenter is requesting a spin. Perhaps
you could address my comments, too?
-Doug
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
Re: [PATCH V9 2/3] watchdog: s3c2410_wdt: use syscon regmap interface to configure pmu register
On Monday 25 of November 2013 14:44:01 Doug Anderson wrote:
+
struct s3c2410_wdt {
struct device *dev;
struct clk *clock;
@@ -94,7 +107,49 @@ struct s3c2410_wdt {
unsigned long wtdat_save;
struct watchdog_device wdt_device;
struct notifier_block freq_transition;
+ struct s3c2410_wdt_variant *drv_data;
+ struct regmap *pmureg;
Total nit, but everything else in this structure is tab aligned and
your new elements are not.
That would mean adding extra tabs in lines above to align them with the
longest drv_data field.
AFAIK we're observing a trend of moving away from such field alignment,
so IMHO it would be better to keep this as is in this version and just
send a separate patch removing the alignment of remaining fields.
static int s3c2410wdt_probe(struct platform_device *pdev)
{
struct device *dev;
@@ -354,6 +443,16 @@ static int s3c2410wdt_probe(struct platform_device
*pdev)
spin_lock_init(wdt-lock);
wdt-wdt_device = s3c2410_wdd;
+ wdt-drv_data = get_wdt_drv_data(pdev);
+ if (wdt-drv_data-quirks QUIRK_NEEDS_PMU_CONFIG) {
+ wdt-pmureg = syscon_regmap_lookup_by_phandle(dev-of_node,
+
samsung,syscon-phandle);
+ if (IS_ERR(wdt-pmureg)) {
+ dev_err(dev, syscon regmap lookup failed.\n);
+ return PTR_ERR(wdt-pmureg);
nit: this function appears to never call return directly. You'll
match other error handling better if you do:
ret = PTR_ERR(wdt-pmureg);
goto err;
Jumping away just to a single return statement isn't really a good idea.
If there is nothing to do, returning right away seems less confusing IMHO
(and saves you one line of code per each such error case as a side
effect).
A separate patch could be possibly made to clean-up remaining error cases
and remove the err label.
As for all the remaining points, I agree with Doug.
Best regards,
Tomasz
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH V9 2/3] watchdog: s3c2410_wdt: use syscon regmap interface to configure pmu register
Add device tree support for exynos5250 and 5420 SoCs and use syscon regmap
interface
to configure AUTOMATIC_WDT_RESET_DISABLE and MASK_WDT_RESET_REQUEST registers
of PMU
to mask/unmask enable/disable of watchdog in probe and s2r scenarios.
Signed-off-by: Leela Krishna Amudala l.kris...@samsung.com
---
.../devicetree/bindings/watchdog/samsung-wdt.txt | 21 ++-
drivers/watchdog/Kconfig |1 +
drivers/watchdog/s3c2410_wdt.c | 145 ++--
3 files changed, 157 insertions(+), 10 deletions(-)
diff --git a/Documentation/devicetree/bindings/watchdog/samsung-wdt.txt
b/Documentation/devicetree/bindings/watchdog/samsung-wdt.txt
index 2aa486c..5dea363 100644
--- a/Documentation/devicetree/bindings/watchdog/samsung-wdt.txt
+++ b/Documentation/devicetree/bindings/watchdog/samsung-wdt.txt
@@ -5,10 +5,29 @@ after a preset amount of time during which the WDT reset
event has not
occurred.
Required properties:
-- compatible : should be samsung,s3c2410-wdt
+- compatible : should be one among the following
+ (a) samsung,s3c2410-wdt for Exynos4 and previous SoCs
+ (b) samsung,exynos5250-wdt for Exynos5250
+ (c) samsung,exynos5420-wdt for Exynos5420
+
- reg : base physical address of the controller and length of memory mapped
region.
- interrupts : interrupt number to the cpu.
+- samsung,syscon-phandle : reference to syscon node (This property required
only
+ in case of compatible being samsung,exynos5250-wdt or
samsung,exynos5420-wdt.
+ In case of Exynos5250 and 5420 this property points to syscon node
holding the PMU
+ base address)
Optional properties:
- timeout-sec : contains the watchdog timeout in seconds.
+
+Example:
+
+watchdog@101D {
+ compatible = samsung,exynos5250-wdt;
+ reg = 0x101D 0x100;
+ interrupts = 0 42 0;
+ clocks = clock 336;
+ clock-names = watchdog;
+ samsung,syscon-phandle = pmu_sys_reg;
+};
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index d1d53f3..0d272ae 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -188,6 +188,7 @@ config S3C2410_WATCHDOG
tristate S3C2410 Watchdog
depends on HAVE_S3C2410_WATCHDOG
select WATCHDOG_CORE
+ select MFD_SYSCON if ARCH_EXYNOS5
help
Watchdog timer block in the Samsung SoCs. This will reboot
the system when the timer expires with the watchdog enabled.
diff --git a/drivers/watchdog/s3c2410_wdt.c b/drivers/watchdog/s3c2410_wdt.c
index 23aad7c..42e0fd3 100644
--- a/drivers/watchdog/s3c2410_wdt.c
+++ b/drivers/watchdog/s3c2410_wdt.c
@@ -41,6 +41,8 @@
#include linux/slab.h
#include linux/err.h
#include linux/of.h
+#include linux/mfd/syscon.h
+#include linux/regmap.h
#define S3C2410_WTCON 0x00
#define S3C2410_WTDAT 0x04
@@ -61,6 +63,10 @@
#define CONFIG_S3C2410_WATCHDOG_ATBOOT (0)
#define CONFIG_S3C2410_WATCHDOG_DEFAULT_TIME (15)
+#define WDT_DISABLE_REG_OFFSET 0x0408
+#define WDT_MASK_RESET_REG_OFFSET 0x040c
+#define QUIRK_NEEDS_PMU_CONFIG (1 0)
+
static bool nowayout = WATCHDOG_NOWAYOUT;
static int tmr_margin;
static int tmr_atboot = CONFIG_S3C2410_WATCHDOG_ATBOOT;
@@ -84,6 +90,13 @@ MODULE_PARM_DESC(soft_noboot, Watchdog action, set to 1 to
ignore reboots,
0 to reboot (default 0));
MODULE_PARM_DESC(debug, Watchdog debug, set to 1 for debug (default 0));
+struct s3c2410_wdt_variant {
+ int disable_reg;
+ int mask_reset_reg;
+ int mask_bit;
+ u32 quirks;
+};
+
struct s3c2410_wdt {
struct device *dev;
struct clk *clock;
@@ -94,7 +107,49 @@ struct s3c2410_wdt {
unsigned long wtdat_save;
struct watchdog_device wdt_device;
struct notifier_block freq_transition;
+ struct s3c2410_wdt_variant *drv_data;
+ struct regmap *pmureg;
+};
+
+static const struct s3c2410_wdt_variant drv_data_s3c2410 = {
+ .quirks = 0
+};
+
+#ifdef CONFIG_OF
+static const struct s3c2410_wdt_variant drv_data_exynos5250 = {
+ .disable_reg = WDT_DISABLE_REG_OFFSET,
+ .mask_reset_reg = WDT_MASK_RESET_REG_OFFSET,
+ .mask_bit = 20,
+ .quirks = QUIRK_NEEDS_PMU_CONFIG
+};
+
+static const struct s3c2410_wdt_variant drv_data_exynos5420 = {
+ .disable_reg = WDT_DISABLE_REG_OFFSET,
+ .mask_reset_reg = WDT_MASK_RESET_REG_OFFSET,
+ .mask_bit = 0,
+ .quirks = QUIRK_NEEDS_PMU_CONFIG
+};
+
+static const struct of_device_id s3c2410_wdt_match[] = {
+ { .compatible = samsung,s3c2410-wdt,
+ .data = drv_data_s3c2410 },
+ { .compatible = samsung,exynos5250-wdt,
+ .data = drv_data_exynos5250 },
+ { .compatible = samsung,exynos5420-wdt,
+ .data = drv_data_exynos5420 },
+ {},
};
+MODULE_DEVICE_TABLE(of, s3c2410_wdt_match);
+#endif
+
+static const struct
Re: [PATCH V9 2/3] watchdog: s3c2410_wdt: use syscon regmap interface to configure pmu register
On Mon, Nov 18, 2013 at 03:19:48PM +0530, Leela Krishna Amudala wrote:
Add device tree support for exynos5250 and 5420 SoCs and use syscon regmap
interface
to configure AUTOMATIC_WDT_RESET_DISABLE and MASK_WDT_RESET_REQUEST registers
of PMU
to mask/unmask enable/disable of watchdog in probe and s2r scenarios.
Signed-off-by: Leela Krishna Amudala l.kris...@samsung.com
---
.../devicetree/bindings/watchdog/samsung-wdt.txt | 21 ++-
drivers/watchdog/Kconfig |1 +
drivers/watchdog/s3c2410_wdt.c | 145
++--
3 files changed, 157 insertions(+), 10 deletions(-)
diff --git a/Documentation/devicetree/bindings/watchdog/samsung-wdt.txt
b/Documentation/devicetree/bindings/watchdog/samsung-wdt.txt
index 2aa486c..5dea363 100644
--- a/Documentation/devicetree/bindings/watchdog/samsung-wdt.txt
+++ b/Documentation/devicetree/bindings/watchdog/samsung-wdt.txt
@@ -5,10 +5,29 @@ after a preset amount of time during which the WDT reset
event has not
occurred.
Required properties:
-- compatible : should be samsung,s3c2410-wdt
+- compatible : should be one among the following
+ (a) samsung,s3c2410-wdt for Exynos4 and previous SoCs
+ (b) samsung,exynos5250-wdt for Exynos5250
+ (c) samsung,exynos5420-wdt for Exynos5420
+
- reg : base physical address of the controller and length of memory mapped
region.
- interrupts : interrupt number to the cpu.
+- samsung,syscon-phandle : reference to syscon node (This property required
only
+ in case of compatible being samsung,exynos5250-wdt or
samsung,exynos5420-wdt.
+ In case of Exynos5250 and 5420 this property points to syscon node
holding the PMU
+ base address)
Optional properties:
- timeout-sec : contains the watchdog timeout in seconds.
+
+Example:
+
+watchdog@101D {
+ compatible = samsung,exynos5250-wdt;
+ reg = 0x101D 0x100;
+ interrupts = 0 42 0;
+ clocks = clock 336;
+ clock-names = watchdog;
+ samsung,syscon-phandle = pmu_sys_reg;
+};
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index d1d53f3..0d272ae 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -188,6 +188,7 @@ config S3C2410_WATCHDOG
tristate S3C2410 Watchdog
depends on HAVE_S3C2410_WATCHDOG
select WATCHDOG_CORE
+ select MFD_SYSCON if ARCH_EXYNOS5
help
Watchdog timer block in the Samsung SoCs. This will reboot
the system when the timer expires with the watchdog enabled.
diff --git a/drivers/watchdog/s3c2410_wdt.c b/drivers/watchdog/s3c2410_wdt.c
index 23aad7c..42e0fd3 100644
--- a/drivers/watchdog/s3c2410_wdt.c
+++ b/drivers/watchdog/s3c2410_wdt.c
@@ -41,6 +41,8 @@
#include linux/slab.h
#include linux/err.h
#include linux/of.h
+#include linux/mfd/syscon.h
+#include linux/regmap.h
#define S3C2410_WTCON0x00
#define S3C2410_WTDAT0x04
@@ -61,6 +63,10 @@
#define CONFIG_S3C2410_WATCHDOG_ATBOOT (0)
#define CONFIG_S3C2410_WATCHDOG_DEFAULT_TIME (15)
+#define WDT_DISABLE_REG_OFFSET 0x0408
+#define WDT_MASK_RESET_REG_OFFSET0x040c
+#define QUIRK_NEEDS_PMU_CONFIG (1 0)
+
static bool nowayout = WATCHDOG_NOWAYOUT;
static int tmr_margin;
static int tmr_atboot= CONFIG_S3C2410_WATCHDOG_ATBOOT;
@@ -84,6 +90,13 @@ MODULE_PARM_DESC(soft_noboot, Watchdog action, set to 1
to ignore reboots,
0 to reboot (default 0));
MODULE_PARM_DESC(debug, Watchdog debug, set to 1 for debug (default 0));
+struct s3c2410_wdt_variant {
+ int disable_reg;
+ int mask_reset_reg;
+ int mask_bit;
+ u32 quirks;
+};
+
struct s3c2410_wdt {
struct device *dev;
struct clk *clock;
@@ -94,7 +107,49 @@ struct s3c2410_wdt {
unsigned long wtdat_save;
struct watchdog_device wdt_device;
struct notifier_block freq_transition;
+ struct s3c2410_wdt_variant *drv_data;
+ struct regmap *pmureg;
+};
+
+static const struct s3c2410_wdt_variant drv_data_s3c2410 = {
+ .quirks = 0
+};
+
+#ifdef CONFIG_OF
+static const struct s3c2410_wdt_variant drv_data_exynos5250 = {
+ .disable_reg = WDT_DISABLE_REG_OFFSET,
+ .mask_reset_reg = WDT_MASK_RESET_REG_OFFSET,
+ .mask_bit = 20,
+ .quirks = QUIRK_NEEDS_PMU_CONFIG
+};
+
+static const struct s3c2410_wdt_variant drv_data_exynos5420 = {
+ .disable_reg = WDT_DISABLE_REG_OFFSET,
+ .mask_reset_reg = WDT_MASK_RESET_REG_OFFSET,
+ .mask_bit = 0,
+ .quirks = QUIRK_NEEDS_PMU_CONFIG
+};
+
+static const struct of_device_id s3c2410_wdt_match[] = {
+ { .compatible = samsung,s3c2410-wdt,
+ .data = drv_data_s3c2410 },
+ { .compatible = samsung,exynos5250-wdt,
+ .data = drv_data_exynos5250 },
+ { .compatible =
Re: [PATCH V9 2/3] watchdog: s3c2410_wdt: use syscon regmap interface to configure pmu register
Hi Guenter Roeck,
Thanks for reviewing the patch.
On Mon, Nov 18, 2013 at 10:12 PM, Guenter Roeck li...@roeck-us.net wrote:
On Mon, Nov 18, 2013 at 03:19:48PM +0530, Leela Krishna Amudala wrote:
Add device tree support for exynos5250 and 5420 SoCs and use syscon regmap
interface
to configure AUTOMATIC_WDT_RESET_DISABLE and MASK_WDT_RESET_REQUEST
registers of PMU
to mask/unmask enable/disable of watchdog in probe and s2r scenarios.
Signed-off-by: Leela Krishna Amudala l.kris...@samsung.com
---
.../devicetree/bindings/watchdog/samsung-wdt.txt | 21 ++-
drivers/watchdog/Kconfig |1 +
drivers/watchdog/s3c2410_wdt.c | 145
++--
3 files changed, 157 insertions(+), 10 deletions(-)
diff --git a/Documentation/devicetree/bindings/watchdog/samsung-wdt.txt
b/Documentation/devicetree/bindings/watchdog/samsung-wdt.txt
index 2aa486c..5dea363 100644
--- a/Documentation/devicetree/bindings/watchdog/samsung-wdt.txt
+++ b/Documentation/devicetree/bindings/watchdog/samsung-wdt.txt
@@ -5,10 +5,29 @@ after a preset amount of time during which the WDT reset
event has not
occurred.
Required properties:
-- compatible : should be samsung,s3c2410-wdt
+- compatible : should be one among the following
+ (a) samsung,s3c2410-wdt for Exynos4 and previous SoCs
+ (b) samsung,exynos5250-wdt for Exynos5250
+ (c) samsung,exynos5420-wdt for Exynos5420
+
- reg : base physical address of the controller and length of memory mapped
region.
- interrupts : interrupt number to the cpu.
+- samsung,syscon-phandle : reference to syscon node (This property required
only
+ in case of compatible being samsung,exynos5250-wdt or
samsung,exynos5420-wdt.
+ In case of Exynos5250 and 5420 this property points to syscon node
holding the PMU
+ base address)
Optional properties:
- timeout-sec : contains the watchdog timeout in seconds.
+
+Example:
+
+watchdog@101D {
+ compatible = samsung,exynos5250-wdt;
+ reg = 0x101D 0x100;
+ interrupts = 0 42 0;
+ clocks = clock 336;
+ clock-names = watchdog;
+ samsung,syscon-phandle = pmu_sys_reg;
+};
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index d1d53f3..0d272ae 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -188,6 +188,7 @@ config S3C2410_WATCHDOG
tristate S3C2410 Watchdog
depends on HAVE_S3C2410_WATCHDOG
select WATCHDOG_CORE
+ select MFD_SYSCON if ARCH_EXYNOS5
help
Watchdog timer block in the Samsung SoCs. This will reboot
the system when the timer expires with the watchdog enabled.
diff --git a/drivers/watchdog/s3c2410_wdt.c b/drivers/watchdog/s3c2410_wdt.c
index 23aad7c..42e0fd3 100644
--- a/drivers/watchdog/s3c2410_wdt.c
+++ b/drivers/watchdog/s3c2410_wdt.c
@@ -41,6 +41,8 @@
#include linux/slab.h
#include linux/err.h
#include linux/of.h
+#include linux/mfd/syscon.h
+#include linux/regmap.h
#define S3C2410_WTCON0x00
#define S3C2410_WTDAT0x04
@@ -61,6 +63,10 @@
#define CONFIG_S3C2410_WATCHDOG_ATBOOT (0)
#define CONFIG_S3C2410_WATCHDOG_DEFAULT_TIME (15)
+#define WDT_DISABLE_REG_OFFSET 0x0408
+#define WDT_MASK_RESET_REG_OFFSET0x040c
+#define QUIRK_NEEDS_PMU_CONFIG (1 0)
+
static bool nowayout = WATCHDOG_NOWAYOUT;
static int tmr_margin;
static int tmr_atboot= CONFIG_S3C2410_WATCHDOG_ATBOOT;
@@ -84,6 +90,13 @@ MODULE_PARM_DESC(soft_noboot, Watchdog action, set to 1
to ignore reboots,
0 to reboot (default 0));
MODULE_PARM_DESC(debug, Watchdog debug, set to 1 for debug (default 0));
+struct s3c2410_wdt_variant {
+ int disable_reg;
+ int mask_reset_reg;
+ int mask_bit;
+ u32 quirks;
+};
+
struct s3c2410_wdt {
struct device *dev;
struct clk *clock;
@@ -94,7 +107,49 @@ struct s3c2410_wdt {
unsigned long wtdat_save;
struct watchdog_device wdt_device;
struct notifier_block freq_transition;
+ struct s3c2410_wdt_variant *drv_data;
+ struct regmap *pmureg;
+};
+
+static const struct s3c2410_wdt_variant drv_data_s3c2410 = {
+ .quirks = 0
+};
+
+#ifdef CONFIG_OF
+static const struct s3c2410_wdt_variant drv_data_exynos5250 = {
+ .disable_reg = WDT_DISABLE_REG_OFFSET,
+ .mask_reset_reg = WDT_MASK_RESET_REG_OFFSET,
+ .mask_bit = 20,
+ .quirks = QUIRK_NEEDS_PMU_CONFIG
+};
+
+static const struct s3c2410_wdt_variant drv_data_exynos5420 = {
+ .disable_reg = WDT_DISABLE_REG_OFFSET,
+ .mask_reset_reg = WDT_MASK_RESET_REG_OFFSET,
+ .mask_bit = 0,
+ .quirks = QUIRK_NEEDS_PMU_CONFIG
+};
+
+static const struct of_device_id s3c2410_wdt_match[] = {
+ { .compatible = samsung,s3c2410-wdt,
+ .data = drv_data_s3c2410 },
+ {
Re: [PATCH V9 2/3] watchdog: s3c2410_wdt: use syscon regmap interface to configure pmu register
On 11/18/2013 08:36 PM, Leela Krishna Amudala wrote:
Hi Guenter Roeck,
Thanks for reviewing the patch.
On Mon, Nov 18, 2013 at 10:12 PM, Guenter Roeck li...@roeck-us.net wrote:
On Mon, Nov 18, 2013 at 03:19:48PM +0530, Leela Krishna Amudala wrote:
Add device tree support for exynos5250 and 5420 SoCs and use syscon regmap
interface
to configure AUTOMATIC_WDT_RESET_DISABLE and MASK_WDT_RESET_REQUEST registers
of PMU
to mask/unmask enable/disable of watchdog in probe and s2r scenarios.
Signed-off-by: Leela Krishna Amudala l.kris...@samsung.com
---
.../devicetree/bindings/watchdog/samsung-wdt.txt | 21 ++-
drivers/watchdog/Kconfig |1 +
drivers/watchdog/s3c2410_wdt.c | 145 ++--
3 files changed, 157 insertions(+), 10 deletions(-)
diff --git a/Documentation/devicetree/bindings/watchdog/samsung-wdt.txt
b/Documentation/devicetree/bindings/watchdog/samsung-wdt.txt
index 2aa486c..5dea363 100644
--- a/Documentation/devicetree/bindings/watchdog/samsung-wdt.txt
+++ b/Documentation/devicetree/bindings/watchdog/samsung-wdt.txt
@@ -5,10 +5,29 @@ after a preset amount of time during which the WDT reset
event has not
occurred.
Required properties:
-- compatible : should be samsung,s3c2410-wdt
+- compatible : should be one among the following
+ (a) samsung,s3c2410-wdt for Exynos4 and previous SoCs
+ (b) samsung,exynos5250-wdt for Exynos5250
+ (c) samsung,exynos5420-wdt for Exynos5420
+
- reg : base physical address of the controller and length of memory mapped
region.
- interrupts : interrupt number to the cpu.
+- samsung,syscon-phandle : reference to syscon node (This property required
only
+ in case of compatible being samsung,exynos5250-wdt or
samsung,exynos5420-wdt.
+ In case of Exynos5250 and 5420 this property points to syscon node
holding the PMU
+ base address)
Optional properties:
- timeout-sec : contains the watchdog timeout in seconds.
+
+Example:
+
+watchdog@101D {
+ compatible = samsung,exynos5250-wdt;
+ reg = 0x101D 0x100;
+ interrupts = 0 42 0;
+ clocks = clock 336;
+ clock-names = watchdog;
+ samsung,syscon-phandle = pmu_sys_reg;
+};
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index d1d53f3..0d272ae 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -188,6 +188,7 @@ config S3C2410_WATCHDOG
tristate S3C2410 Watchdog
depends on HAVE_S3C2410_WATCHDOG
select WATCHDOG_CORE
+ select MFD_SYSCON if ARCH_EXYNOS5
help
Watchdog timer block in the Samsung SoCs. This will reboot
the system when the timer expires with the watchdog enabled.
diff --git a/drivers/watchdog/s3c2410_wdt.c b/drivers/watchdog/s3c2410_wdt.c
index 23aad7c..42e0fd3 100644
--- a/drivers/watchdog/s3c2410_wdt.c
+++ b/drivers/watchdog/s3c2410_wdt.c
@@ -41,6 +41,8 @@
#include linux/slab.h
#include linux/err.h
#include linux/of.h
+#include linux/mfd/syscon.h
+#include linux/regmap.h
#define S3C2410_WTCON0x00
#define S3C2410_WTDAT0x04
@@ -61,6 +63,10 @@
#define CONFIG_S3C2410_WATCHDOG_ATBOOT (0)
#define CONFIG_S3C2410_WATCHDOG_DEFAULT_TIME (15)
+#define WDT_DISABLE_REG_OFFSET 0x0408
+#define WDT_MASK_RESET_REG_OFFSET0x040c
+#define QUIRK_NEEDS_PMU_CONFIG (1 0)
+
static bool nowayout = WATCHDOG_NOWAYOUT;
static int tmr_margin;
static int tmr_atboot= CONFIG_S3C2410_WATCHDOG_ATBOOT;
@@ -84,6 +90,13 @@ MODULE_PARM_DESC(soft_noboot, Watchdog action, set to 1 to
ignore reboots,
0 to reboot (default 0));
MODULE_PARM_DESC(debug, Watchdog debug, set to 1 for debug (default 0));
+struct s3c2410_wdt_variant {
+ int disable_reg;
+ int mask_reset_reg;
+ int mask_bit;
+ u32 quirks;
+};
+
struct s3c2410_wdt {
struct device *dev;
struct clk *clock;
@@ -94,7 +107,49 @@ struct s3c2410_wdt {
unsigned long wtdat_save;
struct watchdog_device wdt_device;
struct notifier_block freq_transition;
+ struct s3c2410_wdt_variant *drv_data;
+ struct regmap *pmureg;
+};
+
+static const struct s3c2410_wdt_variant drv_data_s3c2410 = {
+ .quirks = 0
+};
+
+#ifdef CONFIG_OF
+static const struct s3c2410_wdt_variant drv_data_exynos5250 = {
+ .disable_reg = WDT_DISABLE_REG_OFFSET,
+ .mask_reset_reg = WDT_MASK_RESET_REG_OFFSET,
+ .mask_bit = 20,
+ .quirks = QUIRK_NEEDS_PMU_CONFIG
+};
+
+static const struct s3c2410_wdt_variant drv_data_exynos5420 = {
+ .disable_reg = WDT_DISABLE_REG_OFFSET,
+ .mask_reset_reg = WDT_MASK_RESET_REG_OFFSET,
+ .mask_bit = 0,
+ .quirks = QUIRK_NEEDS_PMU_CONFIG
+};
+
+static const struct of_device_id s3c2410_wdt_match[] = {
+ { .compatible = samsung,s3c2410-wdt,
+ .data = drv_data_s3c2410 },
+ { .compatible = samsung,exynos5250-wdt,
Re: [PATCH V9 2/3] watchdog: s3c2410_wdt: use syscon regmap interface to configure pmu register
Hi Guenter Roeck,
On Tue, Nov 19, 2013 at 10:30 AM, Guenter Roeck li...@roeck-us.net wrote:
On 11/18/2013 08:36 PM, Leela Krishna Amudala wrote:
Hi Guenter Roeck,
Thanks for reviewing the patch.
On Mon, Nov 18, 2013 at 10:12 PM, Guenter Roeck li...@roeck-us.net
wrote:
On Mon, Nov 18, 2013 at 03:19:48PM +0530, Leela Krishna Amudala wrote:
Add device tree support for exynos5250 and 5420 SoCs and use syscon
regmap interface
to configure AUTOMATIC_WDT_RESET_DISABLE and MASK_WDT_RESET_REQUEST
registers of PMU
to mask/unmask enable/disable of watchdog in probe and s2r scenarios.
Signed-off-by: Leela Krishna Amudala l.kris...@samsung.com
---
.../devicetree/bindings/watchdog/samsung-wdt.txt | 21 ++-
drivers/watchdog/Kconfig |1 +
drivers/watchdog/s3c2410_wdt.c | 145
++--
3 files changed, 157 insertions(+), 10 deletions(-)
diff --git a/Documentation/devicetree/bindings/watchdog/samsung-wdt.txt
b/Documentation/devicetree/bindings/watchdog/samsung-wdt.txt
index 2aa486c..5dea363 100644
--- a/Documentation/devicetree/bindings/watchdog/samsung-wdt.txt
+++ b/Documentation/devicetree/bindings/watchdog/samsung-wdt.txt
@@ -5,10 +5,29 @@ after a preset amount of time during which the WDT
reset event has not
occurred.
Required properties:
-- compatible : should be samsung,s3c2410-wdt
+- compatible : should be one among the following
+ (a) samsung,s3c2410-wdt for Exynos4 and previous SoCs
+ (b) samsung,exynos5250-wdt for Exynos5250
+ (c) samsung,exynos5420-wdt for Exynos5420
+
- reg : base physical address of the controller and length of memory
mapped
region.
- interrupts : interrupt number to the cpu.
+- samsung,syscon-phandle : reference to syscon node (This property
required only
+ in case of compatible being samsung,exynos5250-wdt or
samsung,exynos5420-wdt.
+ In case of Exynos5250 and 5420 this property points to syscon node
holding the PMU
+ base address)
Optional properties:
- timeout-sec : contains the watchdog timeout in seconds.
+
+Example:
+
+watchdog@101D {
+ compatible = samsung,exynos5250-wdt;
+ reg = 0x101D 0x100;
+ interrupts = 0 42 0;
+ clocks = clock 336;
+ clock-names = watchdog;
+ samsung,syscon-phandle = pmu_sys_reg;
+};
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index d1d53f3..0d272ae 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -188,6 +188,7 @@ config S3C2410_WATCHDOG
tristate S3C2410 Watchdog
depends on HAVE_S3C2410_WATCHDOG
select WATCHDOG_CORE
+ select MFD_SYSCON if ARCH_EXYNOS5
help
Watchdog timer block in the Samsung SoCs. This will reboot
the system when the timer expires with the watchdog enabled.
diff --git a/drivers/watchdog/s3c2410_wdt.c
b/drivers/watchdog/s3c2410_wdt.c
index 23aad7c..42e0fd3 100644
--- a/drivers/watchdog/s3c2410_wdt.c
+++ b/drivers/watchdog/s3c2410_wdt.c
@@ -41,6 +41,8 @@
#include linux/slab.h
#include linux/err.h
#include linux/of.h
+#include linux/mfd/syscon.h
+#include linux/regmap.h
#define S3C2410_WTCON0x00
#define S3C2410_WTDAT0x04
@@ -61,6 +63,10 @@
#define CONFIG_S3C2410_WATCHDOG_ATBOOT (0)
#define CONFIG_S3C2410_WATCHDOG_DEFAULT_TIME (15)
+#define WDT_DISABLE_REG_OFFSET 0x0408
+#define WDT_MASK_RESET_REG_OFFSET0x040c
+#define QUIRK_NEEDS_PMU_CONFIG (1 0)
+
static bool nowayout = WATCHDOG_NOWAYOUT;
static int tmr_margin;
static int tmr_atboot= CONFIG_S3C2410_WATCHDOG_ATBOOT;
@@ -84,6 +90,13 @@ MODULE_PARM_DESC(soft_noboot, Watchdog action, set
to 1 to ignore reboots,
0 to reboot (default 0));
MODULE_PARM_DESC(debug, Watchdog debug, set to 1 for debug (default
0));
+struct s3c2410_wdt_variant {
+ int disable_reg;
+ int mask_reset_reg;
+ int mask_bit;
+ u32 quirks;
+};
+
struct s3c2410_wdt {
struct device *dev;
struct clk *clock;
@@ -94,7 +107,49 @@ struct s3c2410_wdt {
unsigned long wtdat_save;
struct watchdog_device wdt_device;
struct notifier_block freq_transition;
+ struct s3c2410_wdt_variant *drv_data;
+ struct regmap *pmureg;
+};
+
+static const struct s3c2410_wdt_variant drv_data_s3c2410 = {
+ .quirks = 0
+};
+
+#ifdef CONFIG_OF
+static const struct s3c2410_wdt_variant drv_data_exynos5250 = {
+ .disable_reg = WDT_DISABLE_REG_OFFSET,
+ .mask_reset_reg = WDT_MASK_RESET_REG_OFFSET,
+ .mask_bit = 20,
+ .quirks = QUIRK_NEEDS_PMU_CONFIG
+};
+
+static const struct s3c2410_wdt_variant drv_data_exynos5420 = {
+ .disable_reg = WDT_DISABLE_REG_OFFSET,
+ .mask_reset_reg = WDT_MASK_RESET_REG_OFFSET,
+ .mask_bit = 0,
+
Re: [PATCH V9 2/3] watchdog: s3c2410_wdt: use syscon regmap interface to configure pmu register
On 11/18/2013 09:26 PM, Leela Krishna Amudala wrote:
Hi Guenter Roeck,
On Tue, Nov 19, 2013 at 10:30 AM, Guenter Roeck li...@roeck-us.net wrote:
On 11/18/2013 08:36 PM, Leela Krishna Amudala wrote:
Hi Guenter Roeck,
Thanks for reviewing the patch.
On Mon, Nov 18, 2013 at 10:12 PM, Guenter Roeck li...@roeck-us.net
wrote:
On Mon, Nov 18, 2013 at 03:19:48PM +0530, Leela Krishna Amudala wrote:
Add device tree support for exynos5250 and 5420 SoCs and use syscon
regmap interface
to configure AUTOMATIC_WDT_RESET_DISABLE and MASK_WDT_RESET_REQUEST
registers of PMU
to mask/unmask enable/disable of watchdog in probe and s2r scenarios.
Signed-off-by: Leela Krishna Amudala l.kris...@samsung.com
---
.../devicetree/bindings/watchdog/samsung-wdt.txt | 21 ++-
drivers/watchdog/Kconfig |1 +
drivers/watchdog/s3c2410_wdt.c | 145
++--
3 files changed, 157 insertions(+), 10 deletions(-)
diff --git a/Documentation/devicetree/bindings/watchdog/samsung-wdt.txt
b/Documentation/devicetree/bindings/watchdog/samsung-wdt.txt
index 2aa486c..5dea363 100644
--- a/Documentation/devicetree/bindings/watchdog/samsung-wdt.txt
+++ b/Documentation/devicetree/bindings/watchdog/samsung-wdt.txt
@@ -5,10 +5,29 @@ after a preset amount of time during which the WDT
reset event has not
occurred.
Required properties:
-- compatible : should be samsung,s3c2410-wdt
+- compatible : should be one among the following
+ (a) samsung,s3c2410-wdt for Exynos4 and previous SoCs
+ (b) samsung,exynos5250-wdt for Exynos5250
+ (c) samsung,exynos5420-wdt for Exynos5420
+
- reg : base physical address of the controller and length of memory
mapped
region.
- interrupts : interrupt number to the cpu.
+- samsung,syscon-phandle : reference to syscon node (This property
required only
+ in case of compatible being samsung,exynos5250-wdt or
samsung,exynos5420-wdt.
+ In case of Exynos5250 and 5420 this property points to syscon node
holding the PMU
+ base address)
Optional properties:
- timeout-sec : contains the watchdog timeout in seconds.
+
+Example:
+
+watchdog@101D {
+ compatible = samsung,exynos5250-wdt;
+ reg = 0x101D 0x100;
+ interrupts = 0 42 0;
+ clocks = clock 336;
+ clock-names = watchdog;
+ samsung,syscon-phandle = pmu_sys_reg;
+};
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index d1d53f3..0d272ae 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -188,6 +188,7 @@ config S3C2410_WATCHDOG
tristate S3C2410 Watchdog
depends on HAVE_S3C2410_WATCHDOG
select WATCHDOG_CORE
+ select MFD_SYSCON if ARCH_EXYNOS5
help
Watchdog timer block in the Samsung SoCs. This will reboot
the system when the timer expires with the watchdog enabled.
diff --git a/drivers/watchdog/s3c2410_wdt.c
b/drivers/watchdog/s3c2410_wdt.c
index 23aad7c..42e0fd3 100644
--- a/drivers/watchdog/s3c2410_wdt.c
+++ b/drivers/watchdog/s3c2410_wdt.c
@@ -41,6 +41,8 @@
#include linux/slab.h
#include linux/err.h
#include linux/of.h
+#include linux/mfd/syscon.h
+#include linux/regmap.h
#define S3C2410_WTCON0x00
#define S3C2410_WTDAT0x04
@@ -61,6 +63,10 @@
#define CONFIG_S3C2410_WATCHDOG_ATBOOT (0)
#define CONFIG_S3C2410_WATCHDOG_DEFAULT_TIME (15)
+#define WDT_DISABLE_REG_OFFSET 0x0408
+#define WDT_MASK_RESET_REG_OFFSET0x040c
+#define QUIRK_NEEDS_PMU_CONFIG (1 0)
+
static bool nowayout = WATCHDOG_NOWAYOUT;
static int tmr_margin;
static int tmr_atboot= CONFIG_S3C2410_WATCHDOG_ATBOOT;
@@ -84,6 +90,13 @@ MODULE_PARM_DESC(soft_noboot, Watchdog action, set
to 1 to ignore reboots,
0 to reboot (default 0));
MODULE_PARM_DESC(debug, Watchdog debug, set to 1 for debug (default
0));
+struct s3c2410_wdt_variant {
+ int disable_reg;
+ int mask_reset_reg;
+ int mask_bit;
+ u32 quirks;
+};
+
struct s3c2410_wdt {
struct device *dev;
struct clk *clock;
@@ -94,7 +107,49 @@ struct s3c2410_wdt {
unsigned long wtdat_save;
struct watchdog_device wdt_device;
struct notifier_block freq_transition;
+ struct s3c2410_wdt_variant *drv_data;
+ struct regmap *pmureg;
+};
+
+static const struct s3c2410_wdt_variant drv_data_s3c2410 = {
+ .quirks = 0
+};
+
+#ifdef CONFIG_OF
+static const struct s3c2410_wdt_variant drv_data_exynos5250 = {
+ .disable_reg = WDT_DISABLE_REG_OFFSET,
+ .mask_reset_reg = WDT_MASK_RESET_REG_OFFSET,
+ .mask_bit = 20,
+ .quirks = QUIRK_NEEDS_PMU_CONFIG
+};
+
+static const struct s3c2410_wdt_variant drv_data_exynos5420 = {
+ .disable_reg = WDT_DISABLE_REG_OFFSET,
+ .mask_reset_reg = WDT_MASK_RESET_REG_OFFSET,
+ .mask_bit = 0,
+ .quirks = QUIRK_NEEDS_PMU_CONFIG
+};
+
