Tomasz, On Thu, Jul 25, 2013 at 3:27 AM, Tomasz Figa <t.figa@xxxxxxxxxxx> wrote: > Hi Leela, > > On Wednesday 24 of July 2013 15:08:17 Leela Krishna Amudala wrote: >> This patch parses the watchdog node to read pmu wdt sys registers >> addresses and do mask/unmask enable/disable of WDT in probe and s2r >> scenarios. >> >> Reviewed-by: Doug Anderson <dianders@xxxxxxxxxxxx> >> Signed-off-by: Leela Krishna Amudala <l.krishna@xxxxxxxxxxx> >> --- >> .../devicetree/bindings/watchdog/samsung-wdt.txt | 14 ++++- >> drivers/watchdog/s3c2410_wdt.c | 56 >> ++++++++++++++++++++ 2 files changed, 69 insertions(+), 1 deletion(-) >> >> diff --git a/Documentation/devicetree/bindings/watchdog/samsung-wdt.txt >> b/Documentation/devicetree/bindings/watchdog/samsung-wdt.txt index >> 2aa486c..4c798e3 100644 >> --- a/Documentation/devicetree/bindings/watchdog/samsung-wdt.txt >> +++ b/Documentation/devicetree/bindings/watchdog/samsung-wdt.txt >> @@ -7,8 +7,20 @@ occurred. >> Required properties: >> - compatible : should be "samsung,s3c2410-wdt" >> - reg : base physical address of the controller and length of memory >> mapped - region. >> + region and the optional (addresses and length of memory mapped regions >> + of) PMU registers for masking/unmasking WDT. >> - interrupts : interrupt number to the cpu. > > I don't think PMU registers should be passed like this, even if USB PHY > driver currently does it - it needs to be fixed. Every time you are mapping > a single 4-byte register, you are creating new mapping for the whole page > (4K) containing it. In addition, this makes the WDT driver map IO memory > that does not belong to the IP block it handles, which is not nice. > > I would suggest looking into regmap helpers to implement a driver for the > PMU that can share PMU registers with interested drivers. We've done this in some other drivers as well. I remember someone suggested that this was fine when I posted the change to the ADC driver for something similar. Do you really think it's worth adding a level of abstraction here? The argument I remember in the past was that it was fine as long as the register itself was used only by this driver. If we want to do as you say, we'll have to figure out whether you want to create a new generic interface for this or reuse an existing one. You could kinda think of these bits as enabling power, so you could use the regulator framework. ...but that framework often gets abused. Olof: I think I got your advice when I did the ADC work. Do you have any advice here? Thanks! -Doug -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
 |