Re: [PATCH v4 08/18] watchdog: orion: Make RSTOUT register a separate resource
From: Thomas Petazzoni <hidden>
Date: 2014-01-26 08:17:53
Dear Ezequiel Garcia, On Wed, 22 Jan 2014 20:05:05 -0300, Ezequiel Garcia wrote:
+/* 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)
I'm not a big fan of hardcoding the internal register window size in a driver. I would have preferred to have used the offset between the watchdog registers and the rstout register, but since this one is *before* the watchdog registers, it would have to be a negative offset. Not sure how to handle this.
+/* + * 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() Thomas -- Thomas Petazzoni, CTO, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com