[PATCH 1/5] ARM: vexpress: Get rid of MMIO_P2V

On Fri, Nov 18, 2011 at 12:20:48PM +0000, Pawel Moll wrote:

On Thu, 2011-11-17 at 15:43 +0000, Russell King - ARM Linux wrote:

quoted

quoted

+/* Tile's peripherals static mappings should start here */
+#define V2T_PERIPH 0xf8200000
+#define V2T_PERIPH_P2V(offset) ((void __iomem *)(V2T_PERIPH | (offset)))
+

Please get rid of these blank lines at the end of files.

Patch splitting leftover. Will fix.

quoted

quoted

+static void __iomem *v2m_sysreg_base;
+
+
+

More useless blank lines.

Ok, will change that. I just like the way that the data are visually
separated from the code like here:

ceade897 (Russell King 2010-02-11 21:44:53 +0000  68)         .init   = v2m_timer_init,
ceade897 (Russell King 2010-02-11 21:44:53 +0000  69) };
ceade897 (Russell King 2010-02-11 21:44:53 +0000  70) 
ceade897 (Russell King 2010-02-11 21:44:53 +0000  71) 
ceade897 (Russell King 2010-02-11 21:44:53 +0000  72) static DEFINE_SPINLOCK(v2m_cfg_lock);

or here:

ceade897 (Russell King 2010-02-11 21:44:53 +0000 289)         &rtc_device,
ceade897 (Russell King 2010-02-11 21:44:53 +0000 290) };
ceade897 (Russell King 2010-02-11 21:44:53 +0000 291) 
ceade897 (Russell King 2010-02-11 21:44:53 +0000 292) 
ceade897 (Russell King 2010-02-11 21:44:53 +0000 293) static long v2m_osc_round(struct clk *clk, unsigned long rate)

Note the difference in the number.  Two is acceptable to split sections
of code.  Three is getting excessive.

quoted

quoted

 static void __init v2m_timer_init(void)
 {
+	void *sysctl_base;
+	void *timer01_base;

Do you not use sparse?  __iomem.

Ok.

quoted

quoted

+	unsigned int timer01_irq;
 	u32 scctrl;
 
+		sysctl_base = ioremap(V2M_SYSCTL, SZ_4K);
+		BUG_ON(!sysctl_base);
+		timer01_base = ioremap(V2M_TIMER01, SZ_4K);
+		BUG_ON(!timer01_base);
+		timer01_irq = IRQ_V2M_TIMER0;

What's going on with the indentation here?

Patch-splitting artefact again, will fix.

quoted

quoted

@@ -413,6 +431,10 @@ static void __init v2m_populate_ct_desc(void)
 static void __init v2m_map_io(void)
 {
 	iotable_init(v2m_io_desc, ARRAY_SIZE(v2m_io_desc));
+
+	/* Will become an ioremap() when possible */
+	v2m_sysreg_base = V2M_PERIPH_P2V(V2M_SYSREGS);

It won't if it stays here.

Excuse my inferior English language capabilities, but what do you
suggest here?

What I'm saying is your comment is wrong.  ioremap() will never be
possible in the map_io function.