On 04/01/2014 04:20 AM, Michal Simek wrote:
Hi Guenter,+/** + * struct cdns_wdt - Watchdog device structure + * @regs: baseaddress of device + * @rst: reset flag + * @clk: struct clk * of a clock source + * @prescaler: for saving prescaler value + * @ctrl_clksel: counter clock prescaler selection + * @io_lock: spinlock for IO register access + * @cdns_wdt_device: watchdog device structure + * @cdns_wdt_notifier: notifier structure + * + * Structure containing parameters specific to cadence watchdog. + */ +struct cdns_wdt { + void __iomem *regs; + u32 rst; + struct clk *clk; + u32 prescaler; + u32 ctrl_clksel; + spinlock_t io_lock; + struct watchdog_device cdns_wdt_device; + struct notifier_block cdns_wdt_notifier; +}; + +/* Write access to Registers */ +static inline void cdns_wdt_writereg(void __iomem *offset, u32 val) +{ + writel_relaxed(val, offset); +} +Not really sure if this function provides any value.I can't see any problem to use this helper IO function but maybe we could do it a little bit differently. Currently implementation is just passing values to writel_relaxed() What about to do it like this? static inline void cdns_wdt_writereg(struct cdns_wdt *wdt, u32 offset, u32 val) { writel_relaxed(val, wdt->regs + offset); }
Yes, that would make more sense. Guenter
This solution was suggested by Mark here too. https://lkml.org/lkml/2014/3/17/234 The reason for having one IO helper function is 1. Simper debugging when you add printks to one location and you get full list for accesses 2. This driver can be also used by BE Linux on Microblaze in PL that's why there could be a need to autodetect endianess directly on the IP itself. Then using helper function is necessary. 3. We have met with secure monitor implementation where all these io accesses should be done via smc calls and having IO helper function is easier for change. Thanks, Michal
-- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
