[PATCH 02/17] omap4: pm: Add SAR RAM support

-----Original Message-----
From: Kevin Hilman [mailto:khilman at ti.com]
Sent: Thursday, March 03, 2011 3:27 AM
To: Santosh Shilimkar
Cc: linux-omap at vger.kernel.org; linux-arm-kernel at lists.infradead.org
Subject: Re: [PATCH 02/17] omap4: pm: Add SAR RAM support

[..]

quoted

+
+	/* Static mapping, never released */
+	sar_ram_base = ioremap(OMAP44XX_SAR_RAM_BASE, SZ_8K);
+	BUG_ON(!sar_ram_base);

Again, a BUG is not approprate here.

Instead, other code needs to properly handle when sar_ram_base ==
NULL

Fixed.

quoted

+	return 0;
+}
+early_initcall(omap4_sar_ram_init);

diff --git a/arch/arm/mach-omap2/omap4-sar-layout.h

b/arch/arm/mach-omap2/omap4-sar-layout.h

quoted

new file mode 100644
index 0000000..bb66816

--- /dev/null
+++ b/arch/arm/mach-omap2/omap4-sar-layout.h

@@ -0,0 +1,24 @@

[....]

quoted

+
+extern void __iomem *sar_ram_base;

This patch creates this as global, but has no global users.

Also, personally, I don't like these 'base address as global
pointer'
that are appearing throughout the OMAP4 code.  This one is
continuing
the pattern of some others (gic_dist_base_addr, gic_cpu_base) etc.,
but
I'm not crazy about them.  BTW, the gic* ones also suffer from the
BUG
problem and do not properly handle error conditions.

It would be much cleaner to keep this base address static (and
local)
and just create some sar_read/write helpers that can be used from
other code.

I have fixed all of these in one patch and added helper functions
to get the address. Also removed BUG_ON() from gic_*() functions
as well.

Hmm, I see the assembly code uses this base address to.  For that, a
helper function to get the base address could be created.

Regards,
Santosh