[PATCH] ARM: imx: Modify IMX_IO_P2V macro

[PATCH] ARM: imx: Modify IMX_IO_P2V macro

[Robert Lee]

A change is needed in the IMX_IO_P2V macro to allow all imx5 platforms
to use common definitions when accessing registers of peripherals on
the AIPS2 bus.

This change was tested for mapping conflicts using the iop2v script
found at git://git.pengutronix.de/git/ukl/imx-iop2v.git and by
performing a bootup of a default build using imx_v6_v7_defconfig
on a imx51 babbage board and imx53 loco board.

Signed-off-by: Robert Lee <rob.lee@linaro.org>
---
 arch/arm/plat-mxc/include/mach/hardware.h |    1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/plat-mxc/include/mach/hardware.h b/arch/arm/plat-mxc/include/mach/hardware.h
index 0630513..065cc04 100644
--- a/arch/arm/plat-mxc/include/mach/hardware.h
+++ b/arch/arm/plat-mxc/include/mach/hardware.h
@@ -96,6 +96,7 @@
  */
 #define IMX_IO_P2V(x)	(						\
 			0xf4000000 +					\
+			(((x) & 0x80000000) >> 7) +			\
 			(((x) & 0x50000000) >> 6) +			\
 			(((x) & 0x0b000000) >> 4) +			\
 			(((x) & 0x000fffff)))
-- 
1.7.10

[PATCH] ARM: imx: Modify IMX_IO_P2V macro

[Uwe Kleine-König]

Hello Robert,

On Wed, May 09, 2012 at 07:24:17PM -0500, Robert Lee wrote:
> A change is needed in the IMX_IO_P2V macro to allow all imx5 platforms
> to use common definitions when accessing registers of peripherals on
> the AIPS2 bus.
> 
> This change was tested for mapping conflicts using the iop2v script
> found at git://git.pengutronix.de/git/ukl/imx-iop2v.git and by
> performing a bootup of a default build using imx_v6_v7_defconfig
> on a imx51 babbage board and imx53 loco board.
> 
> Signed-off-by: Robert Lee <rob.lee@linaro.org>
It took me some time to understand your goal. With your change you
achieve that

	IMX_IO_P2V(MX50_AIPS2_BASE_ADDR) ==
	IMX_IO_P2V(MX51_AIPS2_BASE_ADDR) ==
	IMX_IO_P2V(MX53_AIPS2_BASE_ADDR)

(which IMHO is more than "allowing ... to use common definitions [for
the] AIPS2 bus).

I can confirm that my imx-iop2v script doesn't find a conflict with your
change. Having said that I didn't check that the memory regions listed
in the script still match the regions that are statically mapped on imx.

And I wonder why do you need to have the aips2 bus mapped to the same
address on all imx5 machines. The future is that the base addresses of
the various devices are determined via dt and then the static mappings
go away. Do I miss something?

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[PATCH] ARM: imx: Modify IMX_IO_P2V macro

[Rob Lee]

Hello Uwe,

On Thu, May 10, 2012 at 1:56 AM, Uwe Kleine-K?nig
<u.kleine-koenig@pengutronix.de> wrote:
> Hello Robert,
>
> On Wed, May 09, 2012 at 07:24:17PM -0500, Robert Lee wrote:
>> A change is needed in the IMX_IO_P2V macro to allow all imx5 platforms
>> to use common definitions when accessing registers of peripherals on
>> the AIPS2 bus.
>>
>> This change was tested for mapping conflicts using the iop2v script
>> found at git://git.pengutronix.de/git/ukl/imx-iop2v.git and by
>> performing a bootup of a default build using imx_v6_v7_defconfig
>> on a imx51 babbage board and imx53 loco board.
>>
>> Signed-off-by: Robert Lee <rob.lee@linaro.org>
> It took me some time to understand your goal. With your change you
> achieve that
>
> ? ? ? ?IMX_IO_P2V(MX50_AIPS2_BASE_ADDR) ==
> ? ? ? ?IMX_IO_P2V(MX51_AIPS2_BASE_ADDR) ==
> ? ? ? ?IMX_IO_P2V(MX53_AIPS2_BASE_ADDR)
>
> (which IMHO is more than "allowing ... to use common definitions [for
> the] AIPS2 bus).

Sorry, I just have been more thorough in my description.  In the patch
I also forgot to change the comments and give the new virtual
addresses so I can add that in a v2 if this change ends up being
acceptable.

>
> I can confirm that my imx-iop2v script doesn't find a conflict with your
> change. Having said that I didn't check that the memory regions listed
> in the script still match the regions that are statically mapped on imx.
>
> And I wonder why do you need to have the aips2 bus mapped to the same
> address on all imx5 machines. The future is that the base addresses of
> the various devices are determined via dt and then the static mappings
> go away. Do I miss something?

For now, because both dt and non dt configurations are available for
imx5, this is needed so that the low power mx5_cpu_lp_set() function
in pm-imx5.c can be readily used by imx53 and imx50 for low power
idling.

If this is not acceptable, another option would be to only support
idle for dt builds that have common base addresses.  This may require
a temporary hack for cpuidle enablement to enable it for dt but not
for non-dt since they currently use some common init code.  Or, just
don't support low power idling at all on imx53 and imx50 until the
non-dt support is removed.

>
> Best regards
> Uwe
>
> --
> Pengutronix e.K. ? ? ? ? ? ? ? ? ? ? ? ? ? | Uwe Kleine-K?nig ? ? ? ? ? ?|
> Industrial Linux Solutions ? ? ? ? ? ? ? ? | http://www.pengutronix.de/ ?|

[PATCH] ARM: imx: Modify IMX_IO_P2V macro

[Rob Lee]

Hello Uwe and Sascha,

On Wed, May 9, 2012 at 7:24 PM, Robert Lee <rob.lee@linaro.org> wrote:
> A change is needed in the IMX_IO_P2V macro to allow all imx5 platforms
> to use common definitions when accessing registers of peripherals on
> the AIPS2 bus.
>
> This change was tested for mapping conflicts using the iop2v script
> found at git://git.pengutronix.de/git/ukl/imx-iop2v.git and by
> performing a bootup of a default build using imx_v6_v7_defconfig
> on a imx51 babbage board and imx53 loco board.
>
> Signed-off-by: Robert Lee <rob.lee@linaro.org>
> ---
> ?arch/arm/plat-mxc/include/mach/hardware.h | ? ?1 +
> ?1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm/plat-mxc/include/mach/hardware.h b/arch/arm/plat-mxc/include/mach/hardware.h
> index 0630513..065cc04 100644
> --- a/arch/arm/plat-mxc/include/mach/hardware.h
> +++ b/arch/arm/plat-mxc/include/mach/hardware.h
> @@ -96,6 +96,7 @@
> ?*/
> ?#define IMX_IO_P2V(x) ?( ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? \
> ? ? ? ? ? ? ? ? ? ? ? ?0xf4000000 + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
> + ? ? ? ? ? ? ? ? ? ? ? (((x) & 0x80000000) >> 7) + ? ? ? ? ? ? ? ? ? ? \

I doubled checked this today and this will result in some of the
platform addresses being in the 0xf6000000 boundary.  Instead, the '+'
can be made an '|' and the addresses that get generated all appear to
be acceptable without any conflicts.

I'll re-submit this patch with the above fix and commet changes as
part of a imx5 idle cleanup series unless I'm told that a change to
this macro is unacceptable.

Thanks,
Rob

> ? ? ? ? ? ? ? ? ? ? ? ?(((x) & 0x50000000) >> 6) + ? ? ? ? ? ? ? ? ? ? \
> ? ? ? ? ? ? ? ? ? ? ? ?(((x) & 0x0b000000) >> 4) + ? ? ? ? ? ? ? ? ? ? \
> ? ? ? ? ? ? ? ? ? ? ? ?(((x) & 0x000fffff)))
> --
> 1.7.10
>