* [PATCH 01/10] clocksource: orion: Add thread-safe API header
2013-07-15 23:32 [PATCH 00/10] Orion Watchdog fixes Ezequiel Garcia
@ 2013-07-15 23:32 ` Ezequiel Garcia
2013-07-15 23:32 ` [PATCH 02/10] watchdog: orion: Use thread-safe clocksource API Ezequiel Garcia
` (9 subsequent siblings)
10 siblings, 0 replies; 23+ messages in thread
From: Ezequiel Garcia @ 2013-07-15 23:32 UTC (permalink / raw)
To: linux-arm-kernel
Add a header declaration to allow drivers (such as watchdog)
to access this exported API.
Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
include/linux/time-orion.h | 7 +++++++
1 file changed, 7 insertions(+)
create mode 100644 include/linux/time-orion.h
diff --git a/include/linux/time-orion.h b/include/linux/time-orion.h
new file mode 100644
index 0000000..c7e6458
--- /dev/null
+++ b/include/linux/time-orion.h
@@ -0,0 +1,7 @@
+
+#ifndef __TIME_ORION_H
+#define __TIME_ORION_H
+
+void orion_timer_ctrl_clrset(u32 clr, u32 set);
+
+#endif
--
1.8.1.5
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH 02/10] watchdog: orion: Use thread-safe clocksource API
2013-07-15 23:32 [PATCH 00/10] Orion Watchdog fixes Ezequiel Garcia
2013-07-15 23:32 ` [PATCH 01/10] clocksource: orion: Add thread-safe API header Ezequiel Garcia
@ 2013-07-15 23:32 ` Ezequiel Garcia
2013-07-15 23:32 ` [PATCH 03/10] watchdog: orion: Rename device-tree binding documentation Ezequiel Garcia
` (8 subsequent siblings)
10 siblings, 0 replies; 23+ messages in thread
From: Ezequiel Garcia @ 2013-07-15 23:32 UTC (permalink / raw)
To: linux-arm-kernel
The TIMER_CTRL register allows to control timer and watchdog counters,
so it's a register shared between the clocksource and the watchdog
drivers. In order to prevent race-conditions the clocksource driver
exposed a thread-safe API. Use the API.
Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
drivers/watchdog/orion_wdt.c | 10 +++-------
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/drivers/watchdog/orion_wdt.c b/drivers/watchdog/orion_wdt.c
index da57798..c0597f4 100644
--- a/drivers/watchdog/orion_wdt.c
+++ b/drivers/watchdog/orion_wdt.c
@@ -25,12 +25,12 @@
#include <linux/clk.h>
#include <linux/err.h>
#include <linux/of.h>
+#include <linux/time-orion.h>
#include <mach/bridge-regs.h>
/*
* Watchdog timer block registers.
*/
-#define TIMER_CTRL 0x0000
#define WDT_EN 0x0010
#define WDT_VAL 0x0024
@@ -72,9 +72,7 @@ static int orion_wdt_start(struct watchdog_device *wdt_dev)
writel(reg, BRIDGE_CAUSE);
/* Enable watchdog timer */
- reg = readl(wdt_reg + TIMER_CTRL);
- reg |= WDT_EN;
- writel(reg, wdt_reg + TIMER_CTRL);
+ orion_timer_ctrl_clrset(0, WDT_EN);
/* Enable reset on watchdog */
reg = readl(RSTOUTn_MASK);
@@ -97,9 +95,7 @@ static int orion_wdt_stop(struct watchdog_device *wdt_dev)
writel(reg, RSTOUTn_MASK);
/* Disable watchdog timer */
- reg = readl(wdt_reg + TIMER_CTRL);
- reg &= ~WDT_EN;
- writel(reg, wdt_reg + TIMER_CTRL);
+ orion_timer_ctrl_clrset(WDT_EN, 0);
spin_unlock(&wdt_lock);
return 0;
--
1.8.1.5
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH 03/10] watchdog: orion: Rename device-tree binding documentation
2013-07-15 23:32 [PATCH 00/10] Orion Watchdog fixes Ezequiel Garcia
2013-07-15 23:32 ` [PATCH 01/10] clocksource: orion: Add thread-safe API header Ezequiel Garcia
2013-07-15 23:32 ` [PATCH 02/10] watchdog: orion: Use thread-safe clocksource API Ezequiel Garcia
@ 2013-07-15 23:32 ` Ezequiel Garcia
2013-07-15 23:32 ` [PATCH 04/10] watchdog: orion: Use the proper watchdog register Ezequiel Garcia
` (7 subsequent siblings)
10 siblings, 0 replies; 23+ messages in thread
From: Ezequiel Garcia @ 2013-07-15 23:32 UTC (permalink / raw)
To: linux-arm-kernel
Name this file to something a bit more judicious.
Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
Documentation/devicetree/bindings/watchdog/{marvel.txt => orion-wdt.txt} | 0
1 file changed, 0 insertions(+), 0 deletions(-)
rename Documentation/devicetree/bindings/watchdog/{marvel.txt => orion-wdt.txt} (100%)
diff --git a/Documentation/devicetree/bindings/watchdog/marvel.txt b/Documentation/devicetree/bindings/watchdog/orion-wdt.txt
similarity index 100%
rename from Documentation/devicetree/bindings/watchdog/marvel.txt
rename to Documentation/devicetree/bindings/watchdog/orion-wdt.txt
--
1.8.1.5
^ permalink raw reply [flat|nested] 23+ messages in thread* [PATCH 04/10] watchdog: orion: Use the proper watchdog register
2013-07-15 23:32 [PATCH 00/10] Orion Watchdog fixes Ezequiel Garcia
` (2 preceding siblings ...)
2013-07-15 23:32 ` [PATCH 03/10] watchdog: orion: Rename device-tree binding documentation Ezequiel Garcia
@ 2013-07-15 23:32 ` Ezequiel Garcia
2013-07-15 23:32 ` [PATCH 05/10] watchdog: orion: Add a memory resource for RSTOUT register Ezequiel Garcia
` (6 subsequent siblings)
10 siblings, 0 replies; 23+ messages in thread
From: Ezequiel Garcia @ 2013-07-15 23:32 UTC (permalink / raw)
To: linux-arm-kernel
Until now the watchdog driver was using the timer control register
to access the watchdog counter register. This is not appropriate,
given the timer control register should be controlled by the clocksource
driver alone.
Fix this by passing the correct register address to the driver and making
direct use of it. Note that this breaks the current device-tree binding
compatibility since it changes the meaning of the 'reg' property.
Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
arch/arm/mach-kirkwood/include/mach/bridge-regs.h | 1 +
arch/arm/mach-orion5x/include/mach/bridge-regs.h | 1 +
arch/arm/plat-orion/common.c | 2 +-
drivers/watchdog/orion_wdt.c | 7 +++----
4 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/arch/arm/mach-kirkwood/include/mach/bridge-regs.h b/arch/arm/mach-kirkwood/include/mach/bridge-regs.h
index d4cbe5e..c3f361d 100644
--- a/arch/arm/mach-kirkwood/include/mach/bridge-regs.h
+++ b/arch/arm/mach-kirkwood/include/mach/bridge-regs.h
@@ -40,6 +40,7 @@
#define TIMER_VIRT_BASE (BRIDGE_VIRT_BASE + 0x0300)
#define TIMER_PHYS_BASE (BRIDGE_PHYS_BASE + 0x0300)
+#define WDT_PHYS_BASE (BRIDGE_PHYS_BASE + 0x0324)
#define L2_CONFIG_REG (BRIDGE_VIRT_BASE + 0x0128)
#define L2_WRITETHROUGH 0x00000010
diff --git a/arch/arm/mach-orion5x/include/mach/bridge-regs.h b/arch/arm/mach-orion5x/include/mach/bridge-regs.h
index 461fd69..aa35de3 100644
--- a/arch/arm/mach-orion5x/include/mach/bridge-regs.h
+++ b/arch/arm/mach-orion5x/include/mach/bridge-regs.h
@@ -36,4 +36,5 @@
#define TIMER_VIRT_BASE (ORION5X_BRIDGE_VIRT_BASE + 0x300)
#define TIMER_PHYS_BASE (ORION5X_BRIDGE_PHYS_BASE + 0x300)
+#define WDT_PHYS_BASE (ORION5X_BRIDGE_PHYS_BASE + 0x324)
#endif
diff --git a/arch/arm/plat-orion/common.c b/arch/arm/plat-orion/common.c
index c019b7a..77fac6c 100644
--- a/arch/arm/plat-orion/common.c
+++ b/arch/arm/plat-orion/common.c
@@ -595,7 +595,7 @@ void __init orion_spi_1_init(unsigned long mapbase)
* Watchdog
****************************************************************************/
static struct resource orion_wdt_resource =
- DEFINE_RES_MEM(TIMER_PHYS_BASE, 0x28);
+ DEFINE_RES_MEM(WDT_PHYS_BASE, 0x04);
static struct platform_device orion_wdt_device = {
.name = "orion_wdt",
diff --git a/drivers/watchdog/orion_wdt.c b/drivers/watchdog/orion_wdt.c
index c0597f4..01bcf53 100644
--- a/drivers/watchdog/orion_wdt.c
+++ b/drivers/watchdog/orion_wdt.c
@@ -32,7 +32,6 @@
* Watchdog timer block registers.
*/
#define WDT_EN 0x0010
-#define WDT_VAL 0x0024
#define WDT_MAX_CYCLE_COUNT 0xffffffff
#define WDT_IN_USE 0
@@ -51,7 +50,7 @@ static int orion_wdt_ping(struct watchdog_device *wdt_dev)
spin_lock(&wdt_lock);
/* Reload watchdog duration */
- writel(wdt_tclk * wdt_dev->timeout, wdt_reg + WDT_VAL);
+ writel(wdt_tclk * wdt_dev->timeout, wdt_reg);
spin_unlock(&wdt_lock);
return 0;
@@ -64,7 +63,7 @@ static int orion_wdt_start(struct watchdog_device *wdt_dev)
spin_lock(&wdt_lock);
/* Set watchdog duration */
- writel(wdt_tclk * wdt_dev->timeout, wdt_reg + WDT_VAL);
+ writel(wdt_tclk * wdt_dev->timeout, wdt_reg);
/* Clear watchdog timer interrupt */
reg = readl(BRIDGE_CAUSE);
@@ -106,7 +105,7 @@ static unsigned int orion_wdt_get_timeleft(struct watchdog_device *wdt_dev)
unsigned int time_left;
spin_lock(&wdt_lock);
- time_left = readl(wdt_reg + WDT_VAL) / wdt_tclk;
+ time_left = readl(wdt_reg) / wdt_tclk;
spin_unlock(&wdt_lock);
return time_left;
--
1.8.1.5
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH 05/10] watchdog: orion: Add a memory resource for RSTOUT register
2013-07-15 23:32 [PATCH 00/10] Orion Watchdog fixes Ezequiel Garcia
` (3 preceding siblings ...)
2013-07-15 23:32 ` [PATCH 04/10] watchdog: orion: Use the proper watchdog register Ezequiel Garcia
@ 2013-07-15 23:32 ` Ezequiel Garcia
2013-07-16 14:04 ` Andrew Lunn
2013-07-15 23:32 ` [PATCH 06/10] watchdog: orion: Update device-tree binding documentation Ezequiel Garcia
` (5 subsequent siblings)
10 siblings, 1 reply; 23+ messages in thread
From: Ezequiel Garcia @ 2013-07-15 23:32 UTC (permalink / raw)
To: linux-arm-kernel
Instead of accessing the RSTOUT register directly, this commit
adds a platform memory resource to map this register into the driver.
Note that by adding a required 2nd-cell for the reg property,
this change breaks the device-tree binding compatibility.
Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
arch/arm/mach-kirkwood/include/mach/bridge-regs.h | 1 +
arch/arm/mach-orion5x/include/mach/bridge-regs.h | 1 +
arch/arm/plat-orion/common.c | 10 ++++++----
drivers/watchdog/orion_wdt.c | 18 ++++++++++--------
4 files changed, 18 insertions(+), 12 deletions(-)
diff --git a/arch/arm/mach-kirkwood/include/mach/bridge-regs.h b/arch/arm/mach-kirkwood/include/mach/bridge-regs.h
index c3f361d..dead75a 100644
--- a/arch/arm/mach-kirkwood/include/mach/bridge-regs.h
+++ b/arch/arm/mach-kirkwood/include/mach/bridge-regs.h
@@ -21,6 +21,7 @@
#define CPU_RESET 0x00000002
#define RSTOUTn_MASK (BRIDGE_VIRT_BASE + 0x0108)
+#define RSTOUT_PHYS_BASE (BRIDGE_PHYS_BASE + 0x0108)
#define WDT_RESET_OUT_EN 0x00000002
#define SOFT_RESET_OUT_EN 0x00000004
diff --git a/arch/arm/mach-orion5x/include/mach/bridge-regs.h b/arch/arm/mach-orion5x/include/mach/bridge-regs.h
index aa35de3..ccd91c9 100644
--- a/arch/arm/mach-orion5x/include/mach/bridge-regs.h
+++ b/arch/arm/mach-orion5x/include/mach/bridge-regs.h
@@ -18,6 +18,7 @@
#define CPU_CTRL (ORION5X_BRIDGE_VIRT_BASE + 0x104)
#define RSTOUTn_MASK (ORION5X_BRIDGE_VIRT_BASE + 0x108)
+#define RSTOUT_PHYS_BASE (ORION5X_BRIDGE_PHYS_BASE + 0x108)
#define WDT_RESET_OUT_EN 0x0002
#define CPU_SOFT_RESET (ORION5X_BRIDGE_VIRT_BASE + 0x10c)
diff --git a/arch/arm/plat-orion/common.c b/arch/arm/plat-orion/common.c
index 77fac6c..37924f8 100644
--- a/arch/arm/plat-orion/common.c
+++ b/arch/arm/plat-orion/common.c
@@ -594,14 +594,16 @@ void __init orion_spi_1_init(unsigned long mapbase)
/*****************************************************************************
* Watchdog
****************************************************************************/
-static struct resource orion_wdt_resource =
- DEFINE_RES_MEM(WDT_PHYS_BASE, 0x04);
+static struct resource orion_wdt_resource[] = {
+ DEFINE_RES_MEM(WDT_PHYS_BASE, 0x04),
+ DEFINE_RES_MEM(RSTOUT_PHYS_BASE, 0x04)
+};
static struct platform_device orion_wdt_device = {
.name = "orion_wdt",
.id = -1,
- .num_resources = 1,
- .resource = &orion_wdt_resource,
+ .num_resources = ARRAY_SIZE(orion_wdt_resource),
+ .resource = orion_wdt_resource,
};
void __init orion_wdt_init(void)
diff --git a/drivers/watchdog/orion_wdt.c b/drivers/watchdog/orion_wdt.c
index 01bcf53..3c9b3d2 100644
--- a/drivers/watchdog/orion_wdt.c
+++ b/drivers/watchdog/orion_wdt.c
@@ -43,6 +43,7 @@ static unsigned int wdt_max_duration; /* (seconds) */
static struct clk *clk;
static unsigned int wdt_tclk;
static void __iomem *wdt_reg;
+static void __iomem *rstout_reg;
static DEFINE_SPINLOCK(wdt_lock);
static int orion_wdt_ping(struct watchdog_device *wdt_dev)
@@ -74,9 +75,7 @@ static int orion_wdt_start(struct watchdog_device *wdt_dev)
orion_timer_ctrl_clrset(0, WDT_EN);
/* Enable reset on watchdog */
- reg = readl(RSTOUTn_MASK);
- reg |= WDT_RESET_OUT_EN;
- writel(reg, RSTOUTn_MASK);
+ writel(readl(rstout_reg) | WDT_RESET_OUT_EN, rstout_reg);
spin_unlock(&wdt_lock);
return 0;
@@ -84,14 +83,10 @@ static int orion_wdt_start(struct watchdog_device *wdt_dev)
static int orion_wdt_stop(struct watchdog_device *wdt_dev)
{
- u32 reg;
-
spin_lock(&wdt_lock);
/* Disable reset on watchdog */
- reg = readl(RSTOUTn_MASK);
- reg &= ~WDT_RESET_OUT_EN;
- writel(reg, RSTOUTn_MASK);
+ writel(readl(rstout_reg) & ~WDT_RESET_OUT_EN, rstout_reg);
/* Disable watchdog timer */
orion_timer_ctrl_clrset(WDT_EN, 0);
@@ -158,6 +153,13 @@ static int orion_wdt_probe(struct platform_device *pdev)
if (!wdt_reg)
return -ENOMEM;
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+ if (!res)
+ return -ENODEV;
+ rstout_reg = devm_ioremap(&pdev->dev, res->start, resource_size(res));
+ if (!rstout_reg)
+ return -ENOMEM;
+
wdt_max_duration = WDT_MAX_CYCLE_COUNT / wdt_tclk;
orion_wdt.timeout = wdt_max_duration;
--
1.8.1.5
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH 05/10] watchdog: orion: Add a memory resource for RSTOUT register
2013-07-15 23:32 ` [PATCH 05/10] watchdog: orion: Add a memory resource for RSTOUT register Ezequiel Garcia
@ 2013-07-16 14:04 ` Andrew Lunn
2013-07-16 14:18 ` Ezequiel Garcia
0 siblings, 1 reply; 23+ messages in thread
From: Andrew Lunn @ 2013-07-16 14:04 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Jul 15, 2013 at 08:32:38PM -0300, Ezequiel Garcia wrote:
> Instead of accessing the RSTOUT register directly, this commit
> adds a platform memory resource to map this register into the driver.
Hi Ezequiel
Have you looked at:
arch/arm/mach-mvebu/system-controller.c
It is also using this register. Are we going to have a similar problem
as the TIMER_CTRL register, which you refactered in an earlier patch?
"marvell,orion-system-controller" is not actually used yet, but once
kirkwood moves into mach-mvebu, it will start using it.
Andrew
>
> Note that by adding a required 2nd-cell for the reg property,
> this change breaks the device-tree binding compatibility.
>
> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
> ---
> arch/arm/mach-kirkwood/include/mach/bridge-regs.h | 1 +
> arch/arm/mach-orion5x/include/mach/bridge-regs.h | 1 +
> arch/arm/plat-orion/common.c | 10 ++++++----
> drivers/watchdog/orion_wdt.c | 18 ++++++++++--------
> 4 files changed, 18 insertions(+), 12 deletions(-)
>
> diff --git a/arch/arm/mach-kirkwood/include/mach/bridge-regs.h b/arch/arm/mach-kirkwood/include/mach/bridge-regs.h
> index c3f361d..dead75a 100644
> --- a/arch/arm/mach-kirkwood/include/mach/bridge-regs.h
> +++ b/arch/arm/mach-kirkwood/include/mach/bridge-regs.h
> @@ -21,6 +21,7 @@
> #define CPU_RESET 0x00000002
>
> #define RSTOUTn_MASK (BRIDGE_VIRT_BASE + 0x0108)
> +#define RSTOUT_PHYS_BASE (BRIDGE_PHYS_BASE + 0x0108)
> #define WDT_RESET_OUT_EN 0x00000002
> #define SOFT_RESET_OUT_EN 0x00000004
>
> diff --git a/arch/arm/mach-orion5x/include/mach/bridge-regs.h b/arch/arm/mach-orion5x/include/mach/bridge-regs.h
> index aa35de3..ccd91c9 100644
> --- a/arch/arm/mach-orion5x/include/mach/bridge-regs.h
> +++ b/arch/arm/mach-orion5x/include/mach/bridge-regs.h
> @@ -18,6 +18,7 @@
> #define CPU_CTRL (ORION5X_BRIDGE_VIRT_BASE + 0x104)
>
> #define RSTOUTn_MASK (ORION5X_BRIDGE_VIRT_BASE + 0x108)
> +#define RSTOUT_PHYS_BASE (ORION5X_BRIDGE_PHYS_BASE + 0x108)
> #define WDT_RESET_OUT_EN 0x0002
>
> #define CPU_SOFT_RESET (ORION5X_BRIDGE_VIRT_BASE + 0x10c)
> diff --git a/arch/arm/plat-orion/common.c b/arch/arm/plat-orion/common.c
> index 77fac6c..37924f8 100644
> --- a/arch/arm/plat-orion/common.c
> +++ b/arch/arm/plat-orion/common.c
> @@ -594,14 +594,16 @@ void __init orion_spi_1_init(unsigned long mapbase)
> /*****************************************************************************
> * Watchdog
> ****************************************************************************/
> -static struct resource orion_wdt_resource =
> - DEFINE_RES_MEM(WDT_PHYS_BASE, 0x04);
> +static struct resource orion_wdt_resource[] = {
> + DEFINE_RES_MEM(WDT_PHYS_BASE, 0x04),
> + DEFINE_RES_MEM(RSTOUT_PHYS_BASE, 0x04)
> +};
>
> static struct platform_device orion_wdt_device = {
> .name = "orion_wdt",
> .id = -1,
> - .num_resources = 1,
> - .resource = &orion_wdt_resource,
> + .num_resources = ARRAY_SIZE(orion_wdt_resource),
> + .resource = orion_wdt_resource,
> };
>
> void __init orion_wdt_init(void)
> diff --git a/drivers/watchdog/orion_wdt.c b/drivers/watchdog/orion_wdt.c
> index 01bcf53..3c9b3d2 100644
> --- a/drivers/watchdog/orion_wdt.c
> +++ b/drivers/watchdog/orion_wdt.c
> @@ -43,6 +43,7 @@ static unsigned int wdt_max_duration; /* (seconds) */
> static struct clk *clk;
> static unsigned int wdt_tclk;
> static void __iomem *wdt_reg;
> +static void __iomem *rstout_reg;
> static DEFINE_SPINLOCK(wdt_lock);
>
> static int orion_wdt_ping(struct watchdog_device *wdt_dev)
> @@ -74,9 +75,7 @@ static int orion_wdt_start(struct watchdog_device *wdt_dev)
> orion_timer_ctrl_clrset(0, WDT_EN);
>
> /* Enable reset on watchdog */
> - reg = readl(RSTOUTn_MASK);
> - reg |= WDT_RESET_OUT_EN;
> - writel(reg, RSTOUTn_MASK);
> + writel(readl(rstout_reg) | WDT_RESET_OUT_EN, rstout_reg);
>
> spin_unlock(&wdt_lock);
> return 0;
> @@ -84,14 +83,10 @@ static int orion_wdt_start(struct watchdog_device *wdt_dev)
>
> static int orion_wdt_stop(struct watchdog_device *wdt_dev)
> {
> - u32 reg;
> -
> spin_lock(&wdt_lock);
>
> /* Disable reset on watchdog */
> - reg = readl(RSTOUTn_MASK);
> - reg &= ~WDT_RESET_OUT_EN;
> - writel(reg, RSTOUTn_MASK);
> + writel(readl(rstout_reg) & ~WDT_RESET_OUT_EN, rstout_reg);
>
> /* Disable watchdog timer */
> orion_timer_ctrl_clrset(WDT_EN, 0);
> @@ -158,6 +153,13 @@ static int orion_wdt_probe(struct platform_device *pdev)
> if (!wdt_reg)
> return -ENOMEM;
>
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> + if (!res)
> + return -ENODEV;
> + rstout_reg = devm_ioremap(&pdev->dev, res->start, resource_size(res));
> + if (!rstout_reg)
> + return -ENOMEM;
> +
> wdt_max_duration = WDT_MAX_CYCLE_COUNT / wdt_tclk;
>
> orion_wdt.timeout = wdt_max_duration;
> --
> 1.8.1.5
>
^ permalink raw reply [flat|nested] 23+ messages in thread* [PATCH 05/10] watchdog: orion: Add a memory resource for RSTOUT register
2013-07-16 14:04 ` Andrew Lunn
@ 2013-07-16 14:18 ` Ezequiel Garcia
0 siblings, 0 replies; 23+ messages in thread
From: Ezequiel Garcia @ 2013-07-16 14:18 UTC (permalink / raw)
To: linux-arm-kernel
Andrew,
On Tue, Jul 16, 2013 at 04:04:15PM +0200, Andrew Lunn wrote:
> On Mon, Jul 15, 2013 at 08:32:38PM -0300, Ezequiel Garcia wrote:
> > Instead of accessing the RSTOUT register directly, this commit
> > adds a platform memory resource to map this register into the driver.
>
>
> Have you looked at:
>
> arch/arm/mach-mvebu/system-controller.c
>
Mmm... I saw the use of the RSTOUT register in kirkwood_restart()
but wasn't sure who should be the real 'owner' of this register.
> It is also using this register. Are we going to have a similar problem
> as the TIMER_CTRL register, which you refactered in an earlier patch?
>
Probably.
> "marvell,orion-system-controller" is not actually used yet, but once
> kirkwood moves into mach-mvebu, it will start using it.
>
I guess so. We should take that into account *now*. Let me think about
it and see if I can have something sane for v2.
Thanks!
--
Ezequiel Garc?a, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 06/10] watchdog: orion: Update device-tree binding documentation
2013-07-15 23:32 [PATCH 00/10] Orion Watchdog fixes Ezequiel Garcia
` (4 preceding siblings ...)
2013-07-15 23:32 ` [PATCH 05/10] watchdog: orion: Add a memory resource for RSTOUT register Ezequiel Garcia
@ 2013-07-15 23:32 ` Ezequiel Garcia
2013-07-16 13:24 ` Jason Cooper
2013-07-15 23:32 ` [PATCH 07/10] watchdog: orion: Remove unneeded BRIDGE_CAUSE clear Ezequiel Garcia
` (4 subsequent siblings)
10 siblings, 1 reply; 23+ messages in thread
From: Ezequiel Garcia @ 2013-07-15 23:32 UTC (permalink / raw)
To: linux-arm-kernel
Now that the 'reg' property meaning has been changed,
this commit updates the deivce-tree binding documentation.
Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
Documentation/devicetree/bindings/watchdog/orion-wdt.txt | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/watchdog/orion-wdt.txt b/Documentation/devicetree/bindings/watchdog/orion-wdt.txt
index 5dc8d30..48a71fc 100644
--- a/Documentation/devicetree/bindings/watchdog/orion-wdt.txt
+++ b/Documentation/devicetree/bindings/watchdog/orion-wdt.txt
@@ -3,7 +3,8 @@
Required Properties:
- Compatibility : "marvell,orion-wdt"
-- reg : Address of the timer registers
+- reg : First cell contains the register for the watchdog counter.
+ Second cell contains the register for the RSTOUT mask.
Optional properties:
@@ -11,9 +12,10 @@ Optional properties:
Example:
- wdt at 20300 {
+ wdt at 20324 {
compatible = "marvell,orion-wdt";
- reg = <0x20300 0x28>;
+ reg = <0x20324 0x4
+ 0x20108 0x4>;
timeout-sec = <10>;
status = "okay";
};
--
1.8.1.5
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH 06/10] watchdog: orion: Update device-tree binding documentation
2013-07-15 23:32 ` [PATCH 06/10] watchdog: orion: Update device-tree binding documentation Ezequiel Garcia
@ 2013-07-16 13:24 ` Jason Cooper
0 siblings, 0 replies; 23+ messages in thread
From: Jason Cooper @ 2013-07-16 13:24 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Jul 15, 2013 at 08:32:39PM -0300, Ezequiel Garcia wrote:
> Now that the 'reg' property meaning has been changed,
> this commit updates the deivce-tree binding documentation.
nit. s/deivce/device/
>
> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
> ---
> Documentation/devicetree/bindings/watchdog/orion-wdt.txt | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/watchdog/orion-wdt.txt b/Documentation/devicetree/bindings/watchdog/orion-wdt.txt
> index 5dc8d30..48a71fc 100644
> --- a/Documentation/devicetree/bindings/watchdog/orion-wdt.txt
> +++ b/Documentation/devicetree/bindings/watchdog/orion-wdt.txt
> @@ -3,7 +3,8 @@
> Required Properties:
>
> - Compatibility : "marvell,orion-wdt"
> -- reg : Address of the timer registers
> +- reg : First cell contains the register for the watchdog counter.
> + Second cell contains the register for the RSTOUT mask.
>
> Optional properties:
>
> @@ -11,9 +12,10 @@ Optional properties:
>
> Example:
>
> - wdt at 20300 {
> + wdt at 20324 {
> compatible = "marvell,orion-wdt";
> - reg = <0x20300 0x28>;
> + reg = <0x20324 0x4
> + 0x20108 0x4>;
> timeout-sec = <10>;
> status = "okay";
> };
> --
> 1.8.1.5
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 07/10] watchdog: orion: Remove unneeded BRIDGE_CAUSE clear
2013-07-15 23:32 [PATCH 00/10] Orion Watchdog fixes Ezequiel Garcia
` (5 preceding siblings ...)
2013-07-15 23:32 ` [PATCH 06/10] watchdog: orion: Update device-tree binding documentation Ezequiel Garcia
@ 2013-07-15 23:32 ` Ezequiel Garcia
2013-07-15 23:32 ` [PATCH 08/10] watchdog: orion: Remove mach-specific unneeded header Ezequiel Garcia
` (3 subsequent siblings)
10 siblings, 0 replies; 23+ messages in thread
From: Ezequiel Garcia @ 2013-07-15 23:32 UTC (permalink / raw)
To: linux-arm-kernel
With the introduction of the orion irqchip driver, now the BRIDGE_CAUSE
bit is cleared by it. There's no longer a need to do it in the watchdog
driver, so we can simply remove it.
Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
drivers/watchdog/orion_wdt.c | 7 -------
1 file changed, 7 deletions(-)
diff --git a/drivers/watchdog/orion_wdt.c b/drivers/watchdog/orion_wdt.c
index 3c9b3d2..4a0ab47 100644
--- a/drivers/watchdog/orion_wdt.c
+++ b/drivers/watchdog/orion_wdt.c
@@ -59,18 +59,11 @@ static int orion_wdt_ping(struct watchdog_device *wdt_dev)
static int orion_wdt_start(struct watchdog_device *wdt_dev)
{
- u32 reg;
-
spin_lock(&wdt_lock);
/* Set watchdog duration */
writel(wdt_tclk * wdt_dev->timeout, wdt_reg);
- /* Clear watchdog timer interrupt */
- reg = readl(BRIDGE_CAUSE);
- reg &= ~WDT_INT_REQ;
- writel(reg, BRIDGE_CAUSE);
-
/* Enable watchdog timer */
orion_timer_ctrl_clrset(0, WDT_EN);
--
1.8.1.5
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH 08/10] watchdog: orion: Remove mach-specific unneeded header
2013-07-15 23:32 [PATCH 00/10] Orion Watchdog fixes Ezequiel Garcia
` (6 preceding siblings ...)
2013-07-15 23:32 ` [PATCH 07/10] watchdog: orion: Remove unneeded BRIDGE_CAUSE clear Ezequiel Garcia
@ 2013-07-15 23:32 ` Ezequiel Garcia
2013-07-15 23:32 ` [PATCH 09/10] watchdog: orion: Use BIT() Ezequiel Garcia
` (2 subsequent siblings)
10 siblings, 0 replies; 23+ messages in thread
From: Ezequiel Garcia @ 2013-07-15 23:32 UTC (permalink / raw)
To: linux-arm-kernel
The mach/bridge-regs.h header is not needed anymore, so we can remove it.
This commit allows to use this driver on multiplatforms builds.
Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
drivers/watchdog/orion_wdt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/watchdog/orion_wdt.c b/drivers/watchdog/orion_wdt.c
index 4a0ab47..4ed7a74 100644
--- a/drivers/watchdog/orion_wdt.c
+++ b/drivers/watchdog/orion_wdt.c
@@ -26,12 +26,12 @@
#include <linux/err.h>
#include <linux/of.h>
#include <linux/time-orion.h>
-#include <mach/bridge-regs.h>
/*
* Watchdog timer block registers.
*/
#define WDT_EN 0x0010
+#define WDT_RESET_OUT_EN 0x0002
#define WDT_MAX_CYCLE_COUNT 0xffffffff
#define WDT_IN_USE 0
--
1.8.1.5
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH 09/10] watchdog: orion: Use BIT()
2013-07-15 23:32 [PATCH 00/10] Orion Watchdog fixes Ezequiel Garcia
` (7 preceding siblings ...)
2013-07-15 23:32 ` [PATCH 08/10] watchdog: orion: Remove mach-specific unneeded header Ezequiel Garcia
@ 2013-07-15 23:32 ` Ezequiel Garcia
2013-07-15 23:32 ` [PATCH 10/10] ARM: kirkwood: Fix the device-tree watchdog's node reg property Ezequiel Garcia
2013-07-16 6:59 ` [PATCH 00/10] Orion Watchdog fixes Andrew Lunn
10 siblings, 0 replies; 23+ messages in thread
From: Ezequiel Garcia @ 2013-07-15 23:32 UTC (permalink / raw)
To: linux-arm-kernel
This is a purely cosmetic commit: we replace hardcoded values that
representing bits by BIT(), which is slightly more readable.
Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
drivers/watchdog/orion_wdt.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/watchdog/orion_wdt.c b/drivers/watchdog/orion_wdt.c
index 4ed7a74..ae64cec 100644
--- a/drivers/watchdog/orion_wdt.c
+++ b/drivers/watchdog/orion_wdt.c
@@ -30,8 +30,8 @@
/*
* Watchdog timer block registers.
*/
-#define WDT_EN 0x0010
-#define WDT_RESET_OUT_EN 0x0002
+#define WDT_EN BIT(4)
+#define WDT_RESET_OUT_EN BIT(1)
#define WDT_MAX_CYCLE_COUNT 0xffffffff
#define WDT_IN_USE 0
--
1.8.1.5
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH 10/10] ARM: kirkwood: Fix the device-tree watchdog's node reg property
2013-07-15 23:32 [PATCH 00/10] Orion Watchdog fixes Ezequiel Garcia
` (8 preceding siblings ...)
2013-07-15 23:32 ` [PATCH 09/10] watchdog: orion: Use BIT() Ezequiel Garcia
@ 2013-07-15 23:32 ` Ezequiel Garcia
2013-07-16 6:59 ` [PATCH 00/10] Orion Watchdog fixes Andrew Lunn
10 siblings, 0 replies; 23+ messages in thread
From: Ezequiel Garcia @ 2013-07-15 23:32 UTC (permalink / raw)
To: linux-arm-kernel
The watchdog driver now needs two 'reg' property cells. The first one
is for the register containing the watchdog counter, while the second
one is for the RSTOUT register.
Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
arch/arm/boot/dts/kirkwood.dtsi | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/kirkwood.dtsi b/arch/arm/boot/dts/kirkwood.dtsi
index ca296c3..0d74ee9 100644
--- a/arch/arm/boot/dts/kirkwood.dtsi
+++ b/arch/arm/boot/dts/kirkwood.dtsi
@@ -116,7 +116,8 @@
wdt: watchdog-timer at 20300 {
compatible = "marvell,orion-wdt";
- reg = <0x20300 0x28>;
+ reg = <0x20324 0x04
+ 0x20108 0x04>;
interrupt-parent = <&bridge_intc>;
interrupts = <3>;
clocks = <&gate_clk 7>;
--
1.8.1.5
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH 00/10] Orion Watchdog fixes
2013-07-15 23:32 [PATCH 00/10] Orion Watchdog fixes Ezequiel Garcia
` (9 preceding siblings ...)
2013-07-15 23:32 ` [PATCH 10/10] ARM: kirkwood: Fix the device-tree watchdog's node reg property Ezequiel Garcia
@ 2013-07-16 6:59 ` Andrew Lunn
2013-07-16 7:20 ` Thomas Petazzoni
10 siblings, 1 reply; 23+ messages in thread
From: Andrew Lunn @ 2013-07-16 6:59 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Jul 15, 2013 at 08:32:33PM -0300, Ezequiel Garcia wrote:
> This patchset introduces a bunch of fixes that removes the direct use of
> the shared timer control register, and also removes the need to include
> a mach-specific header.
>
> With this changes the driver can be included in multiplatforms builds,
> and in particular be used for the Armada 370/XP SoC.
Hi Ezequiel
Maybe i'm missing something here. You are making use of
orion_timer_ctrl_clrset() from time-orion.c. How will this work on
370/XP which has a different clocksource driver?
Thanks
Andrew
^ permalink raw reply [flat|nested] 23+ messages in thread* [PATCH 00/10] Orion Watchdog fixes
2013-07-16 6:59 ` [PATCH 00/10] Orion Watchdog fixes Andrew Lunn
@ 2013-07-16 7:20 ` Thomas Petazzoni
2013-07-16 7:31 ` Andrew Lunn
0 siblings, 1 reply; 23+ messages in thread
From: Thomas Petazzoni @ 2013-07-16 7:20 UTC (permalink / raw)
To: linux-arm-kernel
Dear Andrew Lunn,
On Tue, 16 Jul 2013 08:59:52 +0200, Andrew Lunn wrote:
> Maybe i'm missing something here. You are making use of
> orion_timer_ctrl_clrset() from time-orion.c. How will this work on
> 370/XP which has a different clocksource driver?
I *think* the idea is that the Armada 370/XP driver will expose the
same function, so from the point of view of the watchdog driver, it
will just work. This set of patches is just some preparation patches on
the Orion watchdog driver, to later make it usable on Armada 370/XP.
The patches enabling its usage on Armada 370/XP will follow.
But Ezequiel will confirm all that, we had a discussion together about
this yesterday, so I guess what I said should be the plan, but it's
better if Ezequiel confirms, obviously :)
Thanks!
Thomas
--
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 00/10] Orion Watchdog fixes
2013-07-16 7:20 ` Thomas Petazzoni
@ 2013-07-16 7:31 ` Andrew Lunn
2013-07-16 7:48 ` Sebastian Hesselbarth
2013-07-16 12:14 ` Ezequiel Garcia
0 siblings, 2 replies; 23+ messages in thread
From: Andrew Lunn @ 2013-07-16 7:31 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Jul 16, 2013 at 09:20:59AM +0200, Thomas Petazzoni wrote:
> Dear Andrew Lunn,
>
> On Tue, 16 Jul 2013 08:59:52 +0200, Andrew Lunn wrote:
>
> > Maybe i'm missing something here. You are making use of
> > orion_timer_ctrl_clrset() from time-orion.c. How will this work on
> > 370/XP which has a different clocksource driver?
>
> I *think* the idea is that the Armada 370/XP driver will expose the
> same function, so from the point of view of the watchdog driver, it
> will just work.
Hi Thomas
That was what i was thinking would happen. And then i started to
wonder how well the kernel linker deals with multiple definitions of
the same symbol. Dove and 370/XP can end up in the same kernel. So we
need to have both orion-timer and the 370/XP timer in the same kernel,
so we end up with the same symbol in the kernel twice...
Lets wait for Ezequiel to answer.
Andrew
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 00/10] Orion Watchdog fixes
2013-07-16 7:31 ` Andrew Lunn
@ 2013-07-16 7:48 ` Sebastian Hesselbarth
2013-07-16 12:17 ` Ezequiel Garcia
2013-07-16 12:14 ` Ezequiel Garcia
1 sibling, 1 reply; 23+ messages in thread
From: Sebastian Hesselbarth @ 2013-07-16 7:48 UTC (permalink / raw)
To: linux-arm-kernel
Andrew, Thomas,
In the discussion about orion clocksource Russell was proposing a generic
thread-safe write. That puts a single lock around all those writes. Of
course, it will also blocked by totally unrelated thread-safe register
accesses but should prevent us from having dozens of locks and solves the
symbol issue.
Sebastian
On July 16, 2013 9:31:01 AM Andrew Lunn <andrew@lunn.ch> wrote:
> On Tue, Jul 16, 2013 at 09:20:59AM +0200, Thomas Petazzoni wrote:
> > Dear Andrew Lunn,
> > On Tue, 16 Jul 2013 08:59:52 +0200, Andrew Lunn wrote:
> > > Maybe i'm missing something here. You are making use of
> > > orion_timer_ctrl_clrset() from time-orion.c. How will this work on
> > > 370/XP which has a different clocksource driver?
> > I *think* the idea is that the Armada 370/XP driver will expose the
> > same function, so from the point of view of the watchdog driver, it
> > will just work.
>
> Hi Thomas
>
> That was what i was thinking would happen. And then i started to
> wonder how well the kernel linker deals with multiple definitions of
> the same symbol. Dove and 370/XP can end up in the same kernel. So we
> need to have both orion-timer and the 370/XP timer in the same kernel,
> so we end up with the same symbol in the kernel twice...
>
> Lets wait for Ezequiel to answer.
>
> Andrew
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 00/10] Orion Watchdog fixes
2013-07-16 7:48 ` Sebastian Hesselbarth
@ 2013-07-16 12:17 ` Ezequiel Garcia
0 siblings, 0 replies; 23+ messages in thread
From: Ezequiel Garcia @ 2013-07-16 12:17 UTC (permalink / raw)
To: linux-arm-kernel
Hi Sebastian,
On Tue, Jul 16, 2013 at 09:48:56AM +0200, Sebastian Hesselbarth wrote:
>
> In the discussion about orion clocksource Russell was proposing a generic
> thread-safe write. That puts a single lock around all those writes. Of
> course, it will also blocked by totally unrelated thread-safe register
> accesses but should prevent us from having dozens of locks and solves the
> symbol issue.
>
Thanks for the insight! I'll try to dig this suggestion in the clocksource
discussion. Do you have any plans for posting the clocksource soon?
This way I can base this series on something that's in Jason's trees...
Thanks!
--
Ezequiel Garc?a, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 00/10] Orion Watchdog fixes
2013-07-16 7:31 ` Andrew Lunn
2013-07-16 7:48 ` Sebastian Hesselbarth
@ 2013-07-16 12:14 ` Ezequiel Garcia
2013-07-16 13:44 ` Jason Cooper
1 sibling, 1 reply; 23+ messages in thread
From: Ezequiel Garcia @ 2013-07-16 12:14 UTC (permalink / raw)
To: linux-arm-kernel
Hi Thomas, Andrew:
Thanks for looking at this!
On Tue, Jul 16, 2013 at 09:31:01AM +0200, Andrew Lunn wrote:
> On Tue, Jul 16, 2013 at 09:20:59AM +0200, Thomas Petazzoni wrote:
> >
> > On Tue, 16 Jul 2013 08:59:52 +0200, Andrew Lunn wrote:
> >
> > > Maybe i'm missing something here. You are making use of
> > > orion_timer_ctrl_clrset() from time-orion.c. How will this work on
> > > 370/XP which has a different clocksource driver?
> >
> > I *think* the idea is that the Armada 370/XP driver will expose the
> > same function, so from the point of view of the watchdog driver, it
> > will just work.
Indeed that was one of the ideas. As Thomas said, this was just
preparation work.
>
> That was what i was thinking would happen. And then i started to
> wonder how well the kernel linker deals with multiple definitions of
> the same symbol. Dove and 370/XP can end up in the same kernel. So we
> need to have both orion-timer and the 370/XP timer in the same kernel,
> so we end up with the same symbol in the kernel twice...
>
Yeah, well... I wasn't sure about using the same name, so another approach
would be adding a new compatible to the driver and then make it use the
appropriate function in the 370/XP clocksource driver (with a different name).
And, yet another approach, is what Sebastian just said, although I'm
not sure I understood it :). In any case, we have already several solutions,
which is why I'm not too worried about this particular issue.
On the other side, I'm much interested in knowing if you are OK with
breaking the watchdog DT compatibility. If you NACK this, then I'll
start preparing a different watchdog driver for 370/XP, since I don't
want to extend a driver that is a bit dirty.
--
Ezequiel Garc?a, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 00/10] Orion Watchdog fixes
2013-07-16 12:14 ` Ezequiel Garcia
@ 2013-07-16 13:44 ` Jason Cooper
2013-07-16 14:04 ` Ezequiel Garcia
0 siblings, 1 reply; 23+ messages in thread
From: Jason Cooper @ 2013-07-16 13:44 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Jul 16, 2013 at 09:14:33AM -0300, Ezequiel Garcia wrote:
> On the other side, I'm much interested in knowing if you are OK with
> breaking the watchdog DT compatibility. If you NACK this, then I'll
> start preparing a different watchdog driver for 370/XP, since I don't
> want to extend a driver that is a bit dirty.
Apparently there is some agreement that the bindings are still in flux
and that they need to be for a bit longer in order to hammer out
problems such as this.
Arnd and Olof both mentioned that something (a doc, and email?) is
forthcoming about marking some bindings as stable. Whatever form that
takes, this one wouldn't get the stable marking yet. ;-)
Oh, and one more nit. The work 'fix' triggers a whole bunch of "get on
this right away, does it need to go to stable? Has anyone confirmed it?
Which commit caused the regression? etc." Although I hate the word, I
think 'refactoring' is much more appropriate description for this series.
thx,
Jason.
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 00/10] Orion Watchdog fixes
2013-07-16 13:44 ` Jason Cooper
@ 2013-07-16 14:04 ` Ezequiel Garcia
2013-07-16 14:29 ` Jason Cooper
0 siblings, 1 reply; 23+ messages in thread
From: Ezequiel Garcia @ 2013-07-16 14:04 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Jul 16, 2013 at 09:44:22AM -0400, Jason Cooper wrote:
> On Tue, Jul 16, 2013 at 09:14:33AM -0300, Ezequiel Garcia wrote:
> > On the other side, I'm much interested in knowing if you are OK with
> > breaking the watchdog DT compatibility. If you NACK this, then I'll
> > start preparing a different watchdog driver for 370/XP, since I don't
> > want to extend a driver that is a bit dirty.
>
> Apparently there is some agreement that the bindings are still in flux
> and that they need to be for a bit longer in order to hammer out
> problems such as this.
>
> Arnd and Olof both mentioned that something (a doc, and email?) is
> forthcoming about marking some bindings as stable. Whatever form that
> takes, this one wouldn't get the stable marking yet. ;-)
>
Yup, that's my understanding as well. But on the other side, I don't
want to break possible users out there.
So, just to check, you say it's early enough to safely do such change?
In that case, I'll extend this patchset to include Armada 370/XP support
and post it as soon as Sebastian's clocksource stuff gets in.
> Oh, and one more nit. The work 'fix' triggers a whole bunch of "get on
> this right away, does it need to go to stable? Has anyone confirmed it?
> Which commit caused the regression? etc." Although I hate the word, I
> think 'refactoring' is much more appropriate description for this series.
>
Oh, good observation. I wrote the cover letter at 8 PM, after ten long
hours (*) of hacking and smashing this into something easy to review,
and that's the best title I could come up with. I'll change it on v2.
Thanks,
(*) yes, I have another pair of eyes, in case these wear out.
--
Ezequiel Garc?a, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 00/10] Orion Watchdog fixes
2013-07-16 14:04 ` Ezequiel Garcia
@ 2013-07-16 14:29 ` Jason Cooper
0 siblings, 0 replies; 23+ messages in thread
From: Jason Cooper @ 2013-07-16 14:29 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Jul 16, 2013 at 11:04:36AM -0300, Ezequiel Garcia wrote:
> On Tue, Jul 16, 2013 at 09:44:22AM -0400, Jason Cooper wrote:
> > On Tue, Jul 16, 2013 at 09:14:33AM -0300, Ezequiel Garcia wrote:
> > > On the other side, I'm much interested in knowing if you are OK with
> > > breaking the watchdog DT compatibility. If you NACK this, then I'll
> > > start preparing a different watchdog driver for 370/XP, since I don't
> > > want to extend a driver that is a bit dirty.
> >
> > Apparently there is some agreement that the bindings are still in flux
> > and that they need to be for a bit longer in order to hammer out
> > problems such as this.
> >
> > Arnd and Olof both mentioned that something (a doc, and email?) is
> > forthcoming about marking some bindings as stable. Whatever form that
> > takes, this one wouldn't get the stable marking yet. ;-)
> >
>
> Yup, that's my understanding as well. But on the other side, I don't
> want to break possible users out there.
I agree, but living with a sub-standard binding is worse imho.
> So, just to check, you say it's early enough to safely do such change?
Well, it's not *safe*. But most consumers on mainline kernels are
currently developer/hackers/distro maintainers. eg, folks who can
handle it. It's better to do it now.
> In that case, I'll extend this patchset to include Armada 370/XP support
> and post it as soon as Sebastian's clocksource stuff gets in.
Cool.
thx,
Jason.
^ permalink raw reply [flat|nested] 23+ messages in thread