On Mon, Sep 12, 2011 at 09:41:47AM +0200, Uwe Kleine-K?nig wrote:

Hello Shawn,

On Mon, Sep 12, 2011 at 10:30:01AM +0800, Shawn Guo wrote:

quoted

On Wed, Sep 07, 2011 at 02:36:35PM +0200, Uwe Kleine-K?nig wrote:

quoted

On Wed, Sep 07, 2011 at 07:00:02PM +0800, Shawn Guo wrote:

quoted

On Tue, Sep 06, 2011 at 10:25:55PM +0200, Uwe Kleine-K?nig wrote:

quoted

On Tue, Sep 06, 2011 at 05:58:36PM +0800, Shawn Guo wrote:

[...]

quoted

quoted

quoted

quoted

diff --git a/arch/arm/plat-mxc/include/mach/mx6q.h b/arch/arm/plat-mxc/include/mach/mx6q.h
new file mode 100644
index 0000000..7432310
--- /dev/null
+++ b/arch/arm/plat-mxc/include/mach/mx6q.h

@@ -0,0 +1,29 @@
+/*
+ * Copyright 2011 Freescale Semiconductor, Inc. All Rights Reserved.
+ * Copyright 2011 Linaro Ltd.
+ *
+ * The code contained herein is licensed under the GNU General Public
+ * License. You may obtain a copy of the GNU General Public License
+ * Version 2 or later at the following locations:
+ *
+ * http://www.opensource.org/licenses/gpl-license.html
+ * http://www.gnu.org/copyleft/gpl.html
+ */
+
+#ifndef __MACH_MX6Q_H__
+#define __MACH_MX6Q_H__
+
+/* static mappings */
+#define IMX6Q_VA(x)		(0xf4000000 + (x))
+
+#define MX6Q_SCU_BASE_ADDR	0x00a00000
+#define MX6Q_CCM_BASE_ADDR	0x020c4000
+#define MX6Q_ANATOP_BASE_ADDR	0x020c8000
+#define MX6Q_UART4_BASE_ADDR	0x021f0000
+
+#define MX6Q_SCU_BASE_VADDR	IMX6Q_VA(MX6Q_SCU_BASE_ADDR)
+#define MX6Q_CCM_BASE_VADDR	IMX6Q_VA(MX6Q_CCM_BASE_ADDR)
+#define MX6Q_ANATOP_BASE_VADDR	IMX6Q_VA(MX6Q_ANATOP_BASE_ADDR)
+#define MX6Q_UART4_BASE_VADDR	IMX6Q_VA(MX6Q_UART4_BASE_ADDR)

Depending on the sizes of these memory regions you can use the existing
IMX_IO_P2V here. The conditions are:

	SCU_SIZE <= 0x200000

Actually this should be 0x100000 as this is the size that is mapped in a
continous chunk.

quoted

quoted

	CCM_SIZE <= 0x4000
	ANATOP_SIZE <= 0x28000
	UART4 <= 0x100000

This is exactly what I dislike about IMX_IO_P2V().  We have to verify
all these conditions manually.  Looking at the static mapping below

I did it semi-manual, I just put your constants for imx6q into my script
(all with size=0x4000 for now) and it told me:

...
 * mx6q:
 *	SCU	0x00a00000+0x004000	->	0xf4000000+0x004000
 *	CCM	0x020c4000+0x004000	->	0xf42c4000+0x004000
 *	ANATOP	0x020c8000+0x004000	->	0xf42c8000+0x004000
 *	UART4	0x021f0000+0x004000	->	0xf42f0000+0x004000
...

quoted

(copied from arch/arm/plat-mxc/include/mach/hardware.h), do you think
it's really scalable and easy to maintain for the long run?

At least it provides a way to verify the correctness of the mapping.
Before I introduced the mapping function there was a conflict IIRC.

quoted

/*
 * This is rather complicated for humans and ugly to verify, but for a machine
 * it's OK.  Still more as it is usually only applied to constants.  The upsides
 * on using this approach are:
 *
 *  - same mapping on all i.MX machines
 *  - works for assembler, too
 *  - no need to nurture #defines for virtual addresses
 *
 * The downside it, it's hard to verify (but I have a script for that).

If you want I can provide you that script.

Yes, please. I would use IMX_IO_P2V() in the next post.

I just found a bug in the conflict detection code, so I put it in a
repository to prevent the broken script to enter the archives. You can
find it on

	git://git.pengutronix.de/git/ukl/imx-iop2v.git

Thanks, Uwe.

quoted

[...]

quoted

quoted

#define IMX_IO_P2V(x)	(						\
			0xf4000000 +					\
			(((x) & 0x50000000) >> 6) +			\
			(((x) & 0x0b000000) >> 4) +			\
			(((x) & 0x000fffff)))

quoted

hmm, looking at patch 4 SCU_BASE is determined by a coprocessor
instruction?!

Ah, yes.  The SCU physical base address can be retrieved from
coprocessor.

Is there an advantage in autodetecting the base address?
 

Any disadvantage for doing so?

It makes

	#define MX6Q_SCU_BASE_ADDR       0x00a00000

superfluous.

The definition is actually not needed.  I put it there as a
documentation telling SCU is statically mapped.

And in case the real address is different it might make

	IMX_IO_P2V(real scu address) != scu_io_desc.virtual

This should never happen.

One thing I'm trying to do is to keep arch/arm/mach-imx/platsmp.c
not specific to imx6q.  The auto-detecting helps to keep
MX6Q_SCU_BASE_ADDR away from the file, so that one day when we have
another imx smp soc, we can simply reuse the file.

Regards,
Shawn

which makes the scriptable check for conflicts kind of useless and might
come as a surprise.