On Mon, Aug 26, 2013 at 10:08:43AM -0400, Jason Cooper wrote: > On Fri, Aug 23, 2013 at 07:12:20PM -0300, Ezequiel Garcia wrote: > > Change the 'reg' property meaning, by defining two required cells. > > It's important to note this commit breaks DT-compatibility for this > > device. > > > > Cc: devicetree@xxxxxxxxxxxxxxx > > Signed-off-by: Ezequiel Garcia <ezequiel.garcia@xxxxxxxxxxxxxxxxxx> > > --- > > Documentation/devicetree/bindings/watchdog/orion-wdt.txt | 7 +++++-- > > 1 file changed, 5 insertions(+), 2 deletions(-) > > > > diff --git a/Documentation/devicetree/bindings/watchdog/orion-wdt.txt b/Documentation/devicetree/bindings/watchdog/orion-wdt.txt > > index 5dc8d30..a74c9c8 100644 > > --- a/Documentation/devicetree/bindings/watchdog/orion-wdt.txt > > +++ b/Documentation/devicetree/bindings/watchdog/orion-wdt.txt > > @@ -3,7 +3,9 @@ > > Required Properties: > > > > - Compatibility : "marvell,orion-wdt" > > -- reg : Address of the timer registers > > +- reg : Two cells are required: > > + First cell contains the global timer control register. > > + Second cell contains the watchdog counter register. > > > > Optional properties: > > > > @@ -13,7 +15,8 @@ Example: > > > > wdt@20300 { > > compatible = "marvell,orion-wdt"; > > - reg = <0x20300 0x28>; > > + reg = <0x20300 0x4 > > > > + 0x20324 0x4>; > > Is there a reason we're going this route over adding a new compatible > string? Well, it seemed to me that this register splitting was more device-treeish: it prevents you from fixing your driver, adding a new compatible-string, and rebuilding a kernel each time a new SoC appears with a different offset between registers. Instead, and trying to follow the DT-preachers, we would just change the "reg" values and -bang!- have forward-compatibility :-) That said... > iow, why can't we keep this knowledge in the driver and have a > "marvell,armada-wdt" or similar compat string that the orion-wdt driver > also serves? > ... we still need new compatible strings for armada-xp-wdt and armada-370-wdt, for they have differences between each other and with the orion-wdt: * The clock input is obtained in a different way in each case * The watchdog enable bit inside the timer control register is at a different location. So, thinking about this again, perhaps we should simply let alone the "reg" property and add the watchdog counter offset as yet another field in the compatible-data? What do you think? -- Ezequiel García, Free Electrons Embedded Linux, Kernel and Android Engineering http://free-electrons.com -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html