Message ID | 1390431915-5115-9-git-send-email-ezequiel.garcia@free-electrons.com |
---|---|
State | Superseded, archived |
Headers | show |
On Sun, Jan 26, 2014 at 09:17:53AM +0100, Thomas Petazzoni wrote: [..] > > +/* > > + * The original devicetree binding for this driver specified only > > + * one memory resource, so in order to keep DT backwards compatibility > > + * we try to fallback to a hardcoded register address, if the resource > > + * is missing from the devicetree. > > + */ > > +static void __iomem *try_rstout_ioremap(struct platform_device *pdev, > > + phys_addr_t internal_regs) > > Why is it called "try" ? It actually does the mapping. So I would > prefer the function to be named: > > orion_wdt_ioremap_rstout() > Ah, yes. This is a left over from the previous attempt.
On Sat, Jan 25, 2014 at 10:19:42AM -0800, Guenter Roeck wrote: > On 01/22/2014 03:05 PM, Ezequiel Garcia wrote: > > In order to support other SoC, it's required to distinguish > > the 'control' timer register, from the 'rstout' register > > that enables system reset on watchdog expiration. > > > > To prevent a compatibility break, this commit adds a fallback > > to a hardcoded RSTOUT address. > > > > Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com> > > --- > > .../devicetree/bindings/watchdog/marvel.txt | 6 ++- > > arch/arm/mach-dove/include/mach/bridge-regs.h | 1 + > > arch/arm/mach-kirkwood/include/mach/bridge-regs.h | 1 + > > arch/arm/mach-mv78xx0/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 | 44 +++++++++++++++++++++- > > 7 files changed, 56 insertions(+), 8 deletions(-) > > > > diff --git a/Documentation/devicetree/bindings/watchdog/marvel.txt b/Documentation/devicetree/bindings/watchdog/marvel.txt > > index 0731fbd..1544fe9 100644 > > --- a/Documentation/devicetree/bindings/watchdog/marvel.txt > > +++ b/Documentation/devicetree/bindings/watchdog/marvel.txt > > @@ -3,7 +3,9 @@ > > Required Properties: > > > > - Compatibility : "marvell,orion-wdt" > > -- reg : Address of the timer registers > > +- reg : Should contain two entries: first one with the > > + timer control address, second one with the > > + rstout enable address. > > > > Optional properties: > > > > @@ -14,7 +16,7 @@ Example: > > > > wdt@20300 { > > compatible = "marvell,orion-wdt"; > > - reg = <0x20300 0x28>; > > + reg = <0x20300 0x28>, <0x20108 0x4>; > > interrupts = <3>; > > timeout-sec = <10>; > > status = "okay"; > > diff --git a/arch/arm/mach-dove/include/mach/bridge-regs.h b/arch/arm/mach-dove/include/mach/bridge-regs.h > > index 5362df3..f4a5b34 100644 > > --- a/arch/arm/mach-dove/include/mach/bridge-regs.h > > +++ b/arch/arm/mach-dove/include/mach/bridge-regs.h > > @@ -21,6 +21,7 @@ > > #define CPU_CTRL_PCIE1_LINK 0x00000008 > > > > #define RSTOUTn_MASK (BRIDGE_VIRT_BASE + 0x0108) > > +#define RSTOUTn_MASK_PHYS (BRIDGE_PHYS_BASE + 0x0108) > > #define SOFT_RESET_OUT_EN 0x00000004 > > > > #define SYSTEM_SOFT_RESET (BRIDGE_VIRT_BASE + 0x010c) > > diff --git a/arch/arm/mach-kirkwood/include/mach/bridge-regs.h b/arch/arm/mach-kirkwood/include/mach/bridge-regs.h > > index 8b9d1c9..60f6421 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 RSTOUTn_MASK_PHYS (BRIDGE_PHYS_BASE + 0x0108) > > #define SOFT_RESET_OUT_EN 0x00000004 > > > > #define SYSTEM_SOFT_RESET (BRIDGE_VIRT_BASE + 0x010c) > > diff --git a/arch/arm/mach-mv78xx0/include/mach/bridge-regs.h b/arch/arm/mach-mv78xx0/include/mach/bridge-regs.h > > index 5f03484..e20d6da 100644 > > --- a/arch/arm/mach-mv78xx0/include/mach/bridge-regs.h > > +++ b/arch/arm/mach-mv78xx0/include/mach/bridge-regs.h > > @@ -15,6 +15,7 @@ > > #define L2_WRITETHROUGH 0x00020000 > > > > #define RSTOUTn_MASK (BRIDGE_VIRT_BASE + 0x0108) > > +#define RSTOUTn_MASK_PHYS (BRIDGE_PHYS_BASE + 0x0108) > > #define SOFT_RESET_OUT_EN 0x00000004 > > > > #define SYSTEM_SOFT_RESET (BRIDGE_VIRT_BASE + 0x010c) > > diff --git a/arch/arm/mach-orion5x/include/mach/bridge-regs.h b/arch/arm/mach-orion5x/include/mach/bridge-regs.h > > index f727d03..5766e3f 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 RSTOUTn_MASK_PHYS (ORION5X_BRIDGE_PHYS_BASE + 0x108) > > > > #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 c66d163..3375037 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(TIMER_PHYS_BASE, 0x28); > > +static struct resource orion_wdt_resource[] = { > > + DEFINE_RES_MEM(TIMER_PHYS_BASE, 0x04), > > + DEFINE_RES_MEM(RSTOUTn_MASK_PHYS, 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 f5e7b17..ba8eea9d 100644 > > --- a/drivers/watchdog/orion_wdt.c > > +++ b/drivers/watchdog/orion_wdt.c > > @@ -26,6 +26,12 @@ > > #include <linux/of.h> > > #include <mach/bridge-regs.h> > > > > +/* RSTOUT mask register physical address for Orion5x, Kirkwood and Dove */ > > +#define ORION_RSTOUT_MASK_OFFSET 0x20108 > > + > > +/* Internal registers can be configured at any 1 MiB aligned address */ > > +#define INTERNAL_REGS_MASK ~(SZ_1M - 1) > > + > > /* > > * Watchdog timer block registers. > > */ > > @@ -44,6 +50,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 *wdt_rstout; > > > > static int orion_wdt_ping(struct watchdog_device *wdt_dev) > > { > > @@ -64,14 +71,14 @@ static int orion_wdt_start(struct watchdog_device *wdt_dev) > > atomic_io_modify(wdt_reg + TIMER_CTRL, WDT_EN, WDT_EN); > > > > /* Enable reset on watchdog */ > > - atomic_io_modify(RSTOUTn_MASK, WDT_RESET_OUT_EN, WDT_RESET_OUT_EN); > > + atomic_io_modify(wdt_rstout, WDT_RESET_OUT_EN, WDT_RESET_OUT_EN); > > return 0; > > } > > > > static int orion_wdt_stop(struct watchdog_device *wdt_dev) > > { > > /* Disable reset on watchdog */ > > - atomic_io_modify(RSTOUTn_MASK, WDT_RESET_OUT_EN, 0); > > + atomic_io_modify(wdt_rstout, WDT_RESET_OUT_EN, 0); > > > > /* Disable watchdog timer */ > > atomic_io_modify(wdt_reg + TIMER_CTRL, WDT_EN, 0); > > @@ -116,6 +123,33 @@ static irqreturn_t orion_wdt_irq(int irq, void *devid) > > return IRQ_HANDLED; > > } > > > > +/* > > + * The original devicetree binding for this driver specified only > > + * one memory resource, so in order to keep DT backwards compatibility > > + * we try to fallback to a hardcoded register address, if the resource > > + * is missing from the devicetree. > > + */ > > +static void __iomem *try_rstout_ioremap(struct platform_device *pdev, > > + phys_addr_t internal_regs) > > +{ > > + struct resource *res; > > + phys_addr_t rstout; > > + > > + res = platform_get_resource(pdev, IORESOURCE_MEM, 1); > > + if (res) > > + return devm_ioremap(&pdev->dev, res->start, > > + resource_size(res)); > > + > > + /* This workaround works only for "orion-wdt", DT-enabled */ > > + if (!of_device_is_compatible(pdev->dev.of_node, "marvell,orion-wdt")) > > + return NULL; > > + > > + rstout = internal_regs + ORION_RSTOUT_MASK_OFFSET; > > + > > + WARN(1, FW_BUG "falling back to harcoded RSTOUT reg 0x%x\n", rstout); > > WARN seems to be a bit excessive here. Is that on purpose (sorry if that was discussed and I missed it) ? > > Assuming it is on purpose > > Reviewed-by: Guenter Roeck <linux@roeck-us.net> > Yes, it's on purpose. We want users to notice this and be aware they have a broken dtb (hence the sign firmware bug).
diff --git a/Documentation/devicetree/bindings/watchdog/marvel.txt b/Documentation/devicetree/bindings/watchdog/marvel.txt index 0731fbd..1544fe9 100644 --- a/Documentation/devicetree/bindings/watchdog/marvel.txt +++ b/Documentation/devicetree/bindings/watchdog/marvel.txt @@ -3,7 +3,9 @@ Required Properties: - Compatibility : "marvell,orion-wdt" -- reg : Address of the timer registers +- reg : Should contain two entries: first one with the + timer control address, second one with the + rstout enable address. Optional properties: @@ -14,7 +16,7 @@ Example: wdt@20300 { compatible = "marvell,orion-wdt"; - reg = <0x20300 0x28>; + reg = <0x20300 0x28>, <0x20108 0x4>; interrupts = <3>; timeout-sec = <10>; status = "okay"; diff --git a/arch/arm/mach-dove/include/mach/bridge-regs.h b/arch/arm/mach-dove/include/mach/bridge-regs.h index 5362df3..f4a5b34 100644 --- a/arch/arm/mach-dove/include/mach/bridge-regs.h +++ b/arch/arm/mach-dove/include/mach/bridge-regs.h @@ -21,6 +21,7 @@ #define CPU_CTRL_PCIE1_LINK 0x00000008 #define RSTOUTn_MASK (BRIDGE_VIRT_BASE + 0x0108) +#define RSTOUTn_MASK_PHYS (BRIDGE_PHYS_BASE + 0x0108) #define SOFT_RESET_OUT_EN 0x00000004 #define SYSTEM_SOFT_RESET (BRIDGE_VIRT_BASE + 0x010c) diff --git a/arch/arm/mach-kirkwood/include/mach/bridge-regs.h b/arch/arm/mach-kirkwood/include/mach/bridge-regs.h index 8b9d1c9..60f6421 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 RSTOUTn_MASK_PHYS (BRIDGE_PHYS_BASE + 0x0108) #define SOFT_RESET_OUT_EN 0x00000004 #define SYSTEM_SOFT_RESET (BRIDGE_VIRT_BASE + 0x010c) diff --git a/arch/arm/mach-mv78xx0/include/mach/bridge-regs.h b/arch/arm/mach-mv78xx0/include/mach/bridge-regs.h index 5f03484..e20d6da 100644 --- a/arch/arm/mach-mv78xx0/include/mach/bridge-regs.h +++ b/arch/arm/mach-mv78xx0/include/mach/bridge-regs.h @@ -15,6 +15,7 @@ #define L2_WRITETHROUGH 0x00020000 #define RSTOUTn_MASK (BRIDGE_VIRT_BASE + 0x0108) +#define RSTOUTn_MASK_PHYS (BRIDGE_PHYS_BASE + 0x0108) #define SOFT_RESET_OUT_EN 0x00000004 #define SYSTEM_SOFT_RESET (BRIDGE_VIRT_BASE + 0x010c) diff --git a/arch/arm/mach-orion5x/include/mach/bridge-regs.h b/arch/arm/mach-orion5x/include/mach/bridge-regs.h index f727d03..5766e3f 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 RSTOUTn_MASK_PHYS (ORION5X_BRIDGE_PHYS_BASE + 0x108) #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 c66d163..3375037 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(TIMER_PHYS_BASE, 0x28); +static struct resource orion_wdt_resource[] = { + DEFINE_RES_MEM(TIMER_PHYS_BASE, 0x04), + DEFINE_RES_MEM(RSTOUTn_MASK_PHYS, 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 f5e7b17..ba8eea9d 100644 --- a/drivers/watchdog/orion_wdt.c +++ b/drivers/watchdog/orion_wdt.c @@ -26,6 +26,12 @@ #include <linux/of.h> #include <mach/bridge-regs.h> +/* RSTOUT mask register physical address for Orion5x, Kirkwood and Dove */ +#define ORION_RSTOUT_MASK_OFFSET 0x20108 + +/* Internal registers can be configured at any 1 MiB aligned address */ +#define INTERNAL_REGS_MASK ~(SZ_1M - 1) + /* * Watchdog timer block registers. */ @@ -44,6 +50,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 *wdt_rstout; static int orion_wdt_ping(struct watchdog_device *wdt_dev) { @@ -64,14 +71,14 @@ static int orion_wdt_start(struct watchdog_device *wdt_dev) atomic_io_modify(wdt_reg + TIMER_CTRL, WDT_EN, WDT_EN); /* Enable reset on watchdog */ - atomic_io_modify(RSTOUTn_MASK, WDT_RESET_OUT_EN, WDT_RESET_OUT_EN); + atomic_io_modify(wdt_rstout, WDT_RESET_OUT_EN, WDT_RESET_OUT_EN); return 0; } static int orion_wdt_stop(struct watchdog_device *wdt_dev) { /* Disable reset on watchdog */ - atomic_io_modify(RSTOUTn_MASK, WDT_RESET_OUT_EN, 0); + atomic_io_modify(wdt_rstout, WDT_RESET_OUT_EN, 0); /* Disable watchdog timer */ atomic_io_modify(wdt_reg + TIMER_CTRL, WDT_EN, 0); @@ -116,6 +123,33 @@ static irqreturn_t orion_wdt_irq(int irq, void *devid) return IRQ_HANDLED; } +/* + * The original devicetree binding for this driver specified only + * one memory resource, so in order to keep DT backwards compatibility + * we try to fallback to a hardcoded register address, if the resource + * is missing from the devicetree. + */ +static void __iomem *try_rstout_ioremap(struct platform_device *pdev, + phys_addr_t internal_regs) +{ + struct resource *res; + phys_addr_t rstout; + + res = platform_get_resource(pdev, IORESOURCE_MEM, 1); + if (res) + return devm_ioremap(&pdev->dev, res->start, + resource_size(res)); + + /* This workaround works only for "orion-wdt", DT-enabled */ + if (!of_device_is_compatible(pdev->dev.of_node, "marvell,orion-wdt")) + return NULL; + + rstout = internal_regs + ORION_RSTOUT_MASK_OFFSET; + + WARN(1, FW_BUG "falling back to harcoded RSTOUT reg 0x%x\n", rstout); + return devm_ioremap(&pdev->dev, rstout, 0x4); +} + static int orion_wdt_probe(struct platform_device *pdev) { struct resource *res; @@ -143,6 +177,12 @@ static int orion_wdt_probe(struct platform_device *pdev) goto disable_clk; } + wdt_rstout = try_rstout_ioremap(pdev, res->start & INTERNAL_REGS_MASK); + if (!wdt_rstout) { + ret = -ENODEV; + goto disable_clk; + } + wdt_max_duration = WDT_MAX_CYCLE_COUNT / wdt_tclk; orion_wdt.timeout = wdt_max_duration;
In order to support other SoC, it's required to distinguish the 'control' timer register, from the 'rstout' register that enables system reset on watchdog expiration. To prevent a compatibility break, this commit adds a fallback to a hardcoded RSTOUT address. Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com> --- .../devicetree/bindings/watchdog/marvel.txt | 6 ++- arch/arm/mach-dove/include/mach/bridge-regs.h | 1 + arch/arm/mach-kirkwood/include/mach/bridge-regs.h | 1 + arch/arm/mach-mv78xx0/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 | 44 +++++++++++++++++++++- 7 files changed, 56 insertions(+), 8 deletions(-)