Re: [PATCH V6 3/3] watchdog: s3c2410_wdt: use syscon regmap interface to configure pmu register

2013-11-10 Thread Tomasz Figa
Hi Leela,

On Thursday 07 of November 2013 17:21:18 Leela Krishna Amudala wrote:
 The syscon regmap interface is used 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/s3c2410_wdt.c |  139 
 ++--
  2 files changed, 151 insertions(+), 9 deletions(-)
[snip]
 diff --git a/drivers/watchdog/s3c2410_wdt.c b/drivers/watchdog/s3c2410_wdt.c
 index 23aad7c..a151c20 100644
 --- a/drivers/watchdog/s3c2410_wdt.c
 +++ b/drivers/watchdog/s3c2410_wdt.c
[snip]
 +/* s3c2410_get_wdt_driver_data */
 +static inline struct s3c2410_wdt_variant *
 +get_wdt_drv_data(struct platform_device *pdev)
 +{
 +#ifdef CONFIG_OF
 + if (pdev-dev.of_node) {
 + const struct of_device_id *match;
 + match = of_match_node(s3c2410_wdt_match, pdev-dev.of_node);
 + return (struct s3c2410_wdt_variant *)match-data;
 + }
 +#endif
 + return (struct s3c2410_wdt_variant *)
 + platform_get_device_id(pdev)-driver_data;

This code calls platform_get_device_id(), but I don't see any array of
platform_device_id in this driver. This will always crash on a NULL
pointer dereference whenever booted on a platform using board files.

Also, as this driver is now using syscon API, its Kconfig entry should
select MFD_SYSCON.

Other than that, the patch looks fine.

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 V6 3/3] watchdog: s3c2410_wdt: use syscon regmap interface to configure pmu register

2013-11-07 Thread Leela Krishna Amudala
The syscon regmap interface is used 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/s3c2410_wdt.c |  139 ++--
 2 files changed, 151 insertions(+), 9 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/s3c2410_wdt.c b/drivers/watchdog/s3c2410_wdt.c
index 23aad7c..a151c20 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,40 @@ struct s3c2410_wdt {
unsigned long   wtdat_save;
struct watchdog_device  wdt_device;
struct notifier_block   freq_transition;
+   struct s3c2410_wdt_variant *pmu_config;
+   struct regmap *pmureg;
+};
+
+#ifdef CONFIG_OF
+static const struct s3c2410_wdt_variant pmu_config_s3c2410 = {
+   .quirks = 0
+};
+
+static const struct s3c2410_wdt_variant pmu_config_5250  = {
+   .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 pmu_config_5420 = {
+   .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 = pmu_config_s3c2410 },
+   { .compatible = samsung,exynos5250-wdt,
+ .data = pmu_config_5250 },
+   { .compatible = samsung,exynos5420-wdt,
+ .data = pmu_config_5420 },
+   {},
 };
+MODULE_DEVICE_TABLE(of, s3c2410_wdt_match);
+#endif
 
 /* watchdog control routines */
 
@@ -111,6 +157,30 @@ static inline struct s3c2410_wdt *freq_to_wdt(struct 
notifier_block *nb)
return container_of(nb, struct s3c2410_wdt, freq_transition);
 }
 
+static int s3c2410wdt_mask_and_disable_reset(struct s3c2410_wdt *wdt, bool 
mask)
+{
+   int ret;
+   u32 mask_val = 1  wdt-pmu_config-mask_bit;
+   u32 val = 0;
+
+   if (mask)
+   val = mask_val;
+
+   ret = regmap_update_bits(wdt-pmureg,
+   wdt-pmu_config-disable_reg,
+   mask_val, val);
+   if (ret  0)
+   return ret;
+
+   ret =