[PATCH 2/6] ARM: add Highbank core platform support

On 08/25/2011 11:02 AM, Arnd Bergmann wrote:

On Saturday 20 August 2011, Rob Herring wrote:

quoted

quoted

quoted

+#ifndef _MACH_HIGHBANK__SYSREGS_H_
+#define _MACH_HIGHBANK__SYSREGS_H_
+
+extern void __iomem *sregs_base;
+
+#define HB_SREG_A9_PWR_REQ          0xf00
+#define HB_SREG_A9_BOOT_STAT                0xf04
+#define HB_SREG_A9_BOOT_DATA                0xf08
+
+#define HB_PWR_SUSPEND                      0
+#define HB_PWR_SOFT_RESET           1
+#define HB_PWR_HARD_RESET           2
+#define HB_PWR_SHUTDOWN                     3
+
+#endif

Do these really need to be global?

I think it's better to put the base address and register definitions into a
single file and export functions to be used from elsewhere.

Yes, sregs are a random collection of functions, so it's going to be a
mixture of various users. Just HB_PWR_* alone are in 2 or 3 different
places.

Sorry, I'm not following.

Do you mean 'yes, they need to be global' or 'yes, it's better to export
the functions'?

I meant the former and functions as in h/w functionality, not C functions.

This will mainly be clock control plus a few other things. Is it really
desired to add another layer here when these are all just single
register writes?

Rob