[PATCH v2 16/28] ARM: remove bunch of now unused mach/io.h files

[PATCH v2 16/28] ARM: remove bunch of now unused mach/io.h files

[H Hartley Sweeten]

On Wednesday, February 29, 2012 1:26 PM, Rob Herring wrote:
>
> Now that many platforms don't need mach/io.h, remove the usused ones.
>
> Signed-off-by: Rob Herring <rob.herring@calxeda.com>
> Acked-by: Shawn Guo <shawn.guo@linaro.org>
> Acked-by: Linus Walleij <linus.walleij@linaro.org>
> ---

<...>

arch/arm/mach-ep93xx/include/mach/io.h    |   13 --------

<...>

 delete mode 100644 arch/arm/mach-ep93xx/include/mach/io.h

<...>

> diff --git a/arch/arm/mach-ep93xx/include/mach/io.h b/arch/arm/mach-ep93xx/include/mach/io.h
> deleted file mode 100644
> index 17e76ef..0000000
> --- a/arch/arm/mach-ep93xx/include/mach/io.h
> +++ /dev/null
> @@ -1,13 +0,0 @@
> -/*
> - * arch/arm/mach-ep93xx/include/mach/io.h
> - */
> -
> -#ifndef __ASM_MACH_IO_H
> -#define __ASM_MACH_IO_H
> -
> -#define IO_SPACE_LIMIT		0xffffffff
> -
> -#define __io(p)			__typesafe_io(p)
> -#define __mem_pci(p)		(p)
> -
> -#endif /* __ASM_MACH_IO_H */

For ep93xx:

Acked-by: H Hartley Sweeten <hsweeten@visionengravers.com>

[PATCH v2 15/28] ARM: make mach/io.h include optional

[H Hartley Sweeten]

On Wednesday, February 29, 2012 1:26 PM, Rob Herring wrote:
>
> Add a kconfig option NEED_MACH_IO_H to conditionally include mach/io.h.
>
> Basing this on CONFIG_PCI and CONFIG_ISA doesn't quite work. Most ISA
> platforms don't need mach/io.h, but ebsa110 does. Most PCI platforms need
> mach/io.h for now, but ks8695 doesn't which means it's broken? omap has a
> lot of other stuff in its io.h, so it also needs io.h until it is cleaned
> up.
>
> Signed-off-by: Rob Herring <rob.herring@calxeda.com>
> ---
>  arch/arm/Kconfig          |   19 +++++++++++++++++++
>  arch/arm/include/asm/io.h |    5 +++++
>  2 files changed, 24 insertions(+), 0 deletions(-)

For ep93xx:

Acked-by: H Hartley Sweeten <hsweeten@visionengravers.com>

[PATCH v2 07/28] ARM: iop13xx: move io.h externs to io.c

[Rob Herring]

I dropped some CC's as I'm getting bounced.

On 02/29/2012 03:47 PM, Arnd Bergmann wrote:
> On Wednesday 29 February 2012, Rob Herring wrote:
>>  arch/arm/mach-iop13xx/include/mach/io.h |    4 ----
>>  arch/arm/mach-iop13xx/io.c              |    5 +++++
>>  2 files changed, 5 insertions(+), 4 deletions(-)
> 
> Moving them out of io.h is good, but moving declarations into a .c file
> is rather bad style. It should remain in a header file that is visible to
> both io.c and pci.c. mach/pci.h would be an obvious choice.

Yes I know, but it was only in 1 place. I'll add a mach-iop13xx/pci.h.

> I'm also not sure if the iop13xx magic ioremap stuff is really needed
> and worth keeping. You mentioned in the description for patch 6 that
> it's used for 64 bit address mapping, but I can't see that in the code.
> Where did you find that information?
> 

Look at the ULL defines:

#define IOP13XX_PCIX_MEM_PHYS_OFFSET  0x100000000ULL
#define IOP13XX_PCIX_MEM_WINDOW_SIZE  0x3a000000UL
#define IOP13XX_PCIX_LOWER_MEM_BA     (PHYS_OFFSET + IOP13XX_PCI_OFFSET)
#define IOP13XX_PCIX_LOWER_MEM_PA     (IOP13XX_PCIX_MEM_PHYS_OFFSET +\
				       IOP13XX_PCIX_LOWER_MEM_BA)

There's also this discussion (search the page for iop13xx):
http://comments.gmane.org/gmane.linux.ports.arm.kernel/30905

Rob

[PATCH v2 07/28] ARM: iop13xx: move io.h externs to io.c

[Arnd Bergmann]

On Wednesday 29 February 2012, Rob Herring wrote:
> Look at the ULL defines:
> 
> #define IOP13XX_PCIX_MEM_PHYS_OFFSET  0x100000000ULL
> #define IOP13XX_PCIX_MEM_WINDOW_SIZE  0x3a000000UL
> #define IOP13XX_PCIX_LOWER_MEM_BA     (PHYS_OFFSET + IOP13XX_PCI_OFFSET)
> #define IOP13XX_PCIX_LOWER_MEM_PA     (IOP13XX_PCIX_MEM_PHYS_OFFSET +\
>                                        IOP13XX_PCIX_LOWER_MEM_BA)
> 
> There's also this discussion (search the page for iop13xx):
> http://comments.gmane.org/gmane.linux.ports.arm.kernel/30905

Ok, got it. In that case I don't see a better alternative to your
patch anyway, especially since IOP13xx apparently predates
CONFIG_ARCH_PHYS_ADDR_T_64BIT and implemented the ioremap hack
in order to get around the same limitation. Changing this now for
a legacy platform is something I wouldn't want to try.

Thanks,

	Arnd

[PATCH v2 19/28] ARM: Add fixed PCI i/o mapping

[Rob Herring]

On 02/29/2012 03:53 PM, Arnd Bergmann wrote:
> On Wednesday 29 February 2012, Rob Herring wrote:
> 
>> +static struct map_desc pci_io_desc __initdata = {
>> +       .virtual        = PCI_IO_VIRT_BASE,
>> +       .pfn            = 0,
>> +       .length         = SZ_1M,
>> +       .type           = MT_DEVICE,
>> +};
>> +
>> +void __init pci_map_io(unsigned long paddr[], int nr)
>> +{
>> +       int i;
>> +
>> +       if (nr > 1)
>> +               pci_io_desc.length = SZ_64K;
>> +
>> +       for (i = 0; i < nr; i++) {
>> +               pci_io_desc.pfn = __phys_to_pfn(paddr[i]);
>> +               iotable_init(&pci_io_desc, 1);
>> +
>> +               pci_io_desc.virtual += SZ_64K;
>> +       }
>> +}
>> +
>> +void __init pci_map_io_single(unsigned long paddr)
>> +{
>> +       pci_map_io(&paddr, 1);
>> +}
> 
> Very clever interface, I like that!
> 
>> diff --git a/arch/arm/include/asm/io.h b/arch/arm/include/asm/io.h
>> index d7861b9..f0af548 100644
>> --- a/arch/arm/include/asm/io.h
>> +++ b/arch/arm/include/asm/io.h
>> @@ -111,6 +111,10 @@ static inline void __iomem *__typesafe_io(unsigned long addr)
>>   */
>>  #ifdef CONFIG_NEED_MACH_IO_H
>>  #include <mach/io.h>
>> +#elif defined(CONFIG_PCI)
>> +#define PCI_IO_VIRT_BASE 0xfef00000
>> +#define IO_SPACE_LIMIT ((resource_size_t)0xfffff)
>> +#define __io(a)                __typesafe_io(PCI_IO_VIRT_BASE + ((a) & IO_SPACE_LIMIT))
>>  #else
>>  #define __io(a)                ({ (void)(a); __typesafe_io(0); })
>>  #endif
> 
> Should we shrink the vmalloc area now so it stays clear of the io port range?
> 

No, then the mapping will fail. I do need to double check all the
mappings and make sure there is no overlap but we only care on the PCI
platforms I converted.

Also, it should be noted this shrinks the i/o space on some platforms.
dove and kirkwood had 1MB x 2 buses and now have 64KB per bus. ixp2000 I
think has 32MB with a note saying they need "a lot". Is there really a
use for lots of i/o space?

Rob

[PATCH v2 19/28] ARM: Add fixed PCI i/o mapping

[Arnd Bergmann]

On Wednesday 29 February 2012, Rob Herring wrote:
> No, then the mapping will fail. I do need to double check all the
> mappings and make sure there is no overlap but we only care on the PCI
> platforms I converted.
> 
> Also, it should be noted this shrinks the i/o space on some platforms.
> dove and kirkwood had 1MB x 2 buses and now have 64KB per bus. ixp2000 I
> think has 32MB with a note saying they need "a lot". Is there really a
> use for lots of i/o space?

Given that each PCI bus only wires up the lower 64k, I'd say no ;-)

My guess is that they used 1MB ranges in order to benefit from section
mapping. In case of ixp2000, I only see 64k in the resource:

static struct resource ixp2000_pci_io_space = {
        .start  = 0x00010000,
        .end    = 0x0001ffff,
        .flags  = IORESOURCE_IO,
        .name   = "PCI I/O Space"
};

The part that I don't understand here is why the resource starts at
64k and is another 64k in size. I think we need to double-check this
in order to be sure whether we have to put the pci io space into the
first or the second 64k chunk of the new mapping area.

Hmm, I guess you meant ixp23xx, not ixp2000, which indeed has

static struct resource ixp23xx_pci_io_space = {
        .start  = 0x00000100,
        .end    = 0x01ffffff,
        .flags  = IORESOURCE_IO,
        .name   = "PCI I/O Space"
};

This seems to be done just for simplicity in the implementation,
to keep all parts of the PCI controller 32MB aligned, I can't
see any real technical reason why it would be useful othewise.

	Arnd

[PATCH v2 19/28] ARM: Add fixed PCI i/o mapping

[Russell King - ARM Linux]

On Wed, Feb 29, 2012 at 10:43:13PM +0000, Arnd Bergmann wrote:
> On Wednesday 29 February 2012, Rob Herring wrote:
> > No, then the mapping will fail. I do need to double check all the
> > mappings and make sure there is no overlap but we only care on the PCI
> > platforms I converted.
> > 
> > Also, it should be noted this shrinks the i/o space on some platforms.
> > dove and kirkwood had 1MB x 2 buses and now have 64KB per bus. ixp2000 I
> > think has 32MB with a note saying they need "a lot". Is there really a
> > use for lots of i/o space?
> 
> Given that each PCI bus only wires up the lower 64k, I'd say no ;-)

That's incorrect.  There's no "wires up" option there.  Addresses and
data are transferred over a common set of 32 wires on 32-bit PCI, and
the bus has separate 'address' and 'data' phases.  If you only wire up
16 of those lines, then things aren't going to work (because you won't
be able to access bytes 2,3 of each word.)

However, there's a separate issue here:
(1) do peripherals decode the upper 16 bits programmed into their IO BARs?
    Some do.
(2) are host PCI controllers capable of generating IO accesses in the
    lower 64k bus address region?

I think while I was fiddling with an IOP platform years ago that it did
not work with PCI setup to only use the lower 64k bus address region
for IO accesses.  So I'd suggest a certain amount of care is required
here.

> My guess is that they used 1MB ranges in order to benefit from section
> mapping. In case of ixp2000, I only see 64k in the resource:
> 
> static struct resource ixp2000_pci_io_space = {
>         .start  = 0x00010000,
>         .end    = 0x0001ffff,
>         .flags  = IORESOURCE_IO,
>         .name   = "PCI I/O Space"
> };
> 
> The part that I don't understand here is why the resource starts at
> 64k and is another 64k in size. I think we need to double-check this
> in order to be sure whether we have to put the pci io space into the
> first or the second 64k chunk of the new mapping area.
> 
> Hmm, I guess you meant ixp23xx, not ixp2000, which indeed has
> 
> static struct resource ixp23xx_pci_io_space = {
>         .start  = 0x00000100,
>         .end    = 0x01ffffff,
>         .flags  = IORESOURCE_IO,
>         .name   = "PCI I/O Space"
> };
> 
> This seems to be done just for simplicity in the implementation,
> to keep all parts of the PCI controller 32MB aligned, I can't
> see any real technical reason why it would be useful othewise.

It's certainly a weird way to avoid ISA addresses.  Note that these
will control the set of bus addresses which get assigned within PCI.

[PATCH v2 19/28] ARM: Add fixed PCI i/o mapping

[Arnd Bergmann]

On Wednesday 29 February 2012, Russell King - ARM Linux wrote:
> On Wed, Feb 29, 2012 at 10:43:13PM +0000, Arnd Bergmann wrote:
> > On Wednesday 29 February 2012, Rob Herring wrote:
> > > No, then the mapping will fail. I do need to double check all the
> > > mappings and make sure there is no overlap but we only care on the PCI
> > > platforms I converted.
> > > 
> > > Also, it should be noted this shrinks the i/o space on some platforms.
> > > dove and kirkwood had 1MB x 2 buses and now have 64KB per bus. ixp2000 I
> > > think has 32MB with a note saying they need "a lot". Is there really a
> > > use for lots of i/o space?
> > 
> > Given that each PCI bus only wires up the lower 64k, I'd say no ;-)
> 
> That's incorrect.  There's no "wires up" option there.  Addresses and
> data are transferred over a common set of 32 wires on 32-bit PCI, and
> the bus has separate 'address' and 'data' phases.  If you only wire up
> 16 of those lines, then things aren't going to work (because you won't
> be able to access bytes 2,3 of each word.)
> 
> However, there's a separate issue here:
> (1) do peripherals decode the upper 16 bits programmed into their IO BARs?
>     Some do.
> (2) are host PCI controllers capable of generating IO accesses in the
>     lower 64k bus address region?

Very interesting, I had no idea and when I looked up the PCI spec, I
found this PCI-3.0, section 6.2.5.1:

"The upper 16 bits of the I/O Base Address register may be hardwired to
 zero for devices intended for 16-bit I/O systems, such as PC compatibles.
 However, a full 32-bit decode of I/O addresses must still be done."

So it is indeed valid to have a larger I/O space, although in practice
I would expect this not to get used because in an x86 PC it is impossible
to issue an instruction that accesses an I/O port beyond 0xffff.

> I think while I was fiddling with an IOP platform years ago that it did
> not work with PCI setup to only use the lower 64k bus address region
> for IO accesses.  So I'd suggest a certain amount of care is required
> here.

Ok. I've looked up the manual for IOP134x and it's very clear there that
it has only two 64k I/O windows but no more (developer manual, sections
2.2.2.1 and 3.3.2.1). Limiting the virtual space to 64k would result in
seeing only one of the buses, but 1 MB is enough if we map both buses
in there.

Similarly, the IOP333 defines one outbound I/O space window of a fixed
length of 64kb (processor developer's manual section 3.10.33) and won't
need more than that.

Any idea which IOP you might have seen this on?

> > My guess is that they used 1MB ranges in order to benefit from section
> > mapping. In case of ixp2000, I only see 64k in the resource:
> > 
> > static struct resource ixp2000_pci_io_space = {
> >         .start  = 0x00010000,
> >         .end    = 0x0001ffff,
> >         .flags  = IORESOURCE_IO,
> >         .name   = "PCI I/O Space"
> > };
> > 
> > The part that I don't understand here is why the resource starts at
> > 64k and is another 64k in size. I think we need to double-check this
> > in order to be sure whether we have to put the pci io space into the
> > first or the second 64k chunk of the new mapping area.
> > 
> > Hmm, I guess you meant ixp23xx, not ixp2000, which indeed has
> > 
> > static struct resource ixp23xx_pci_io_space = {
> >         .start  = 0x00000100,
> >         .end    = 0x01ffffff,
> >         .flags  = IORESOURCE_IO,
> >         .name   = "PCI I/O Space"
> > };
> > 
> > This seems to be done just for simplicity in the implementation,
> > to keep all parts of the PCI controller 32MB aligned, I can't
> > see any real technical reason why it would be useful othewise.
> 
> It's certainly a weird way to avoid ISA addresses.  Note that these
> will control the set of bus addresses which get assigned within PCI.

Are you referring to ipx2000 or the ixp23xx here?

	Arnd

[PATCH v2 19/28] ARM: Add fixed PCI i/o mapping

[Russell King - ARM Linux]

On Thu, Mar 01, 2012 at 01:52:35PM +0000, Arnd Bergmann wrote:
> Ok. I've looked up the manual for IOP134x and it's very clear there that
> it has only two 64k I/O windows but no more (developer manual, sections
> 2.2.2.1 and 3.3.2.1). Limiting the virtual space to 64k would result in
> seeing only one of the buses, but 1 MB is enough if we map both buses
> in there.

I think it was the 80312, and the manual does limit the window to 64k.

I did try shifting the PCI bus IO base for outbound transactions to
zero, and although the PCI BAR setup and bridge setup was correct, the
peripherals would not respond.  (That's because I've always hated the
way these buses are setup, mapping their PCI spaces at the physical
address location.)

I came to the conclusion that there was an undocumented problem which
meant that it doesn't work.

[PATCH v2 19/28] ARM: Add fixed PCI i/o mapping

[Arnd Bergmann]

On Thursday 01 March 2012, Russell King - ARM Linux wrote:
> On Thu, Mar 01, 2012 at 01:52:35PM +0000, Arnd Bergmann wrote:
> > Ok. I've looked up the manual for IOP134x and it's very clear there that
> > it has only two 64k I/O windows but no more (developer manual, sections
> > 2.2.2.1 and 3.3.2.1). Limiting the virtual space to 64k would result in
> > seeing only one of the buses, but 1 MB is enough if we map both buses
> > in there.
> 
> I think it was the 80312, and the manual does limit the window to 64k.

I can't see where we suport that one in Linux, was it removed at some
point? I also can't find any documentation for it on the web.

This makes it hard to figure out now what exactly the problem was,
but at least we don't have to worry about breaking that machine now.

The iop variants in the kernel today only map 64kb of I/O window
today, so I can't see how we would regress.
That leaves us still with Kirkwood/Dove and ixp2000 to figure out.

I can imagine that ixp2000 has a similar problem to the one you describe
because of the way it uses the second 64kb in the resource instead of
the first 64kb. I would suggest that for ixp2000 we just make the first
1MB of of the 32MB window, which covers the 64kb resource and more.
I've checked that none of the ixp2000 specific drivers use PC-style
I/O anywhere outside of PCI and that the only IO resource that is
registered is the one for the PCI bus, so I'm rather optimistic about this
one.

For Kirkwood and Dove, an easy way out would be to extend the I/O
space window to 2MB right away and just map both as before, but we
can also try just using the two lower 64k windows in each and see what
breaks, as those seem to be widely available for testing still.

	Arnd

[PATCH v2 19/28] ARM: Add fixed PCI i/o mapping

[Andrew Lunn]

> That leaves us still with Kirkwood/Dove and ixp2000 to figure out.
> 
...
> 
> For Kirkwood and Dove, an easy way out would be to extend the I/O
> space window to 2MB right away and just map both as before, but we
> can also try just using the two lower 64k windows in each and see what
> breaks, as those seem to be widely available for testing still.

I have two different kirkwood devices i can test with. Unfortunately,
only one has a pcie devices, an Atheros wireless card. I might be able
to test the machine without any pcie devices tomorrow, but the
interesting device will have to wait until next week.

	    Andrew

[PATCH v2 19/28] ARM: Add fixed PCI i/o mapping

[Nicolas Pitre]

On Thu, 1 Mar 2012, Arnd Bergmann wrote:

> For Kirkwood and Dove, an easy way out would be to extend the I/O
> space window to 2MB right away and just map both as before, but we
> can also try just using the two lower 64k windows in each and see what
> breaks, as those seem to be widely available for testing still.

The Kirkwood/Dove hardware I have may accommodate 3V PCI cards only, and 
none of the the 3V PCI cards I have make use of the I/O space.  this is 
therefore a pretty much underused feature anyway.  So I think we should 
just go ahead with the cleanup in this case.


Nicolas

[PATCH v2 19/28] ARM: Add fixed PCI i/o mapping

[Rob Herring]

On 02/29/2012 04:13 PM, Russell King - ARM Linux wrote:
> On Wed, Feb 29, 2012 at 02:25:38PM -0600, Rob Herring wrote:
>> diff --git a/arch/arm/common/pci.c b/arch/arm/common/pci.c
>> new file mode 100644
>> index 0000000..d255e85
>> --- /dev/null
>> +++ b/arch/arm/common/pci.c
>> @@ -0,0 +1,46 @@
>> +/*
>> + * Copyright 2012 Calxeda, Inc.
>> + *
>> + * This program is free software; you can redistribute it and/or modify it
>> + * under the terms and conditions of the GNU General Public License,
>> + * version 2, as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope it will be useful, but WITHOUT
>> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
>> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
>> + * more details.
>> + *
>> + * You should have received a copy of the GNU General Public License along with
>> + * this program.  If not, see <http://www.gnu.org/licenses/>.
>> + */
>> +#include <linux/init.h>
>> +#include <linux/io.h>
>> +#include <asm/sizes.h>
>> +#include <asm/mach/map.h>
> 
> I assume you've not tried building this with sparse, because it probably
> complains that these functions aren't static and are missing extern
> declarations (you're missing an include file.)
> 

No, fixed now. BTW, you can add this particular check with newer
versions gcc.

>> +
>> +static struct map_desc pci_io_desc __initdata = {
>> +	.virtual	= PCI_IO_VIRT_BASE,
>> +	.pfn		= 0,
>> +	.length		= SZ_1M,
>> +	.type		= MT_DEVICE,
>> +};
>> +
>> +void __init pci_map_io(unsigned long paddr[], int nr)
>> +{
>> +	int i;
>> +
>> +	if (nr > 1)
>> +		pci_io_desc.length = SZ_64K;
>> +
>> +	for (i = 0; i < nr; i++) {
>> +		pci_io_desc.pfn = __phys_to_pfn(paddr[i]);
>> +		iotable_init(&pci_io_desc, 1);
>> +
>> +		pci_io_desc.virtual += SZ_64K;
>> +	}
>> +}
>> +
>> +void __init pci_map_io_single(unsigned long paddr)
>> +{
>> +	pci_map_io(&paddr, 1);
>> +}
> 
> Is there a reason this can't live in arch/arm/kernel/bios32.c or
> arch/arm/kernel/isa.c ?
> 

No, I've moved it to bios32.c.

>> diff --git a/arch/arm/include/asm/io.h b/arch/arm/include/asm/io.h
>> index d7861b9..f0af548 100644
>> --- a/arch/arm/include/asm/io.h
>> +++ b/arch/arm/include/asm/io.h
>> @@ -111,6 +111,10 @@ static inline void __iomem *__typesafe_io(unsigned long addr)
>>   */
>>  #ifdef CONFIG_NEED_MACH_IO_H
>>  #include <mach/io.h>
>> +#elif defined(CONFIG_PCI)
>> +#define PCI_IO_VIRT_BASE 0xfef00000
>> +#define IO_SPACE_LIMIT ((resource_size_t)0xfffff)
> 
> So what's wrong with the standard definition of IO_SPACE_LIMIT below
> this place?
>

It's a 1MB region rather than 64K to support multiple buses. I can
increase the existing one instead.

>> +#define __io(a)		__typesafe_io(PCI_IO_VIRT_BASE + ((a) & IO_SPACE_LIMIT))
> 
> Why do you want ISA/PCI io port addresses to wrap?
> 
>>  #else
>>  #define __io(a)		({ (void)(a); __typesafe_io(0); })
>>  #endif
>> diff --git a/arch/arm/include/asm/mach/pci.h b/arch/arm/include/asm/mach/pci.h
>> index d943b7d..b6d6f8c 100644
>> --- a/arch/arm/include/asm/mach/pci.h
>> +++ b/arch/arm/include/asm/mach/pci.h
>> @@ -60,6 +60,19 @@ struct pci_sys_data {
>>  void pci_common_init(struct hw_pci *);
>>  
>>  /*
>> + * Setup fixed I/O mapping. Must be called from .map_io function.
>> + */
>> +#ifdef CONFIG_PCI
>> +extern void pci_map_io(unsigned long paddr[], int nr);
>> +#else
>> +static inline void pci_map_io(unsigned long paddr[], int nr) {}
>> +#endif
>> +static inline void __init pci_map_io_single(unsigned long paddr)
>> +{
>> +	pci_map_io(&paddr, 1);
>> +}
> 
> What's the reason for having this as an inline function, and as a
> separate global function in pci.c ?

Because I forgot to remove the non-inline version...

I've changed this some to use pfn directly rather than physaddr as
iop13xx has 64-bit phys addresses.

Rob

[PATCH v2 03/28] ARM: imx: convert to common runtime ioremap hook

[Rob Herring]

On 02/29/2012 03:34 PM, Arnd Bergmann wrote:
> On Wednesday 29 February 2012, Rob Herring wrote:
>> From: Rob Herring <rob.herring@calxeda.com>
>>
>> Convert i.MX platforms to use the common run-time ioremap hook instead of
>> the imx specific hook.
>>
>> Also, move addr_in_module out of io.h.
>>
>> Signed-off-by: Rob Herring <rob.herring@calxeda.com>
> 
> I wonder if we can't just get rid of imx3_ioremap if we put all the
> i.mx3 mappings below 0x80000000 into the static mapping area so that
> the new generic ioremap will just point to that.
> 

Sure, but it's not my call what the i.MX guys do. I'm just generalizing
what i.MX already has for other platforms to use. Since we have no way
to create static mappings from devicetrees I don't think we want to
expand static mappings. Also, your past guidance has been to get rid of
static mappings.

Even if we got rid of it here and on msm, there are still other
platforms that need custom ioremap where static mappings won't help.
ebsa110 and ixp4xx also need ioremap but I didn't convert them to
runtime from compile time since they still need io.h for other reasons
at this point.

Rob

[PATCH v2 03/28] ARM: imx: convert to common runtime ioremap hook

[Arnd Bergmann]

On Wednesday 29 February 2012, Rob Herring wrote:
> Sure, but it's not my call what the i.MX guys do. I'm just generalizing
> what i.MX already has for other platforms to use. Since we have no way
> to create static mappings from devicetrees I don't think we want to
> expand static mappings. Also, your past guidance has been to get rid of
> static mappings.

The problems with static mappings have been largely resolved with the
new way we automatically handle ioremap redirecting to the static
mapping area, so now it's no longer necessary to hardcode the virtual
address in a header file and the static mapping basically becomes
a performance optimization by using section mapping where possible.

> Even if we got rid of it here and on msm, there are still other
> platforms that need custom ioremap where static mappings won't help.
> ebsa110 and ixp4xx also need ioremap but I didn't convert them to
> runtime from compile time since they still need io.h for other reasons
> at this point.

Right. When I replied on this patch first, I hoped that this one might
be the only one left.

One thing that could be done though is to remove the __arch_ioremap
macros after all platforms overriding this at compile time have been
changed to do so at boot time.

If we wanted to get fancy, we could also have a Kconfig symbol that
turns on the run-time selected ioremap only for platforms that require
it, and otherwise directly calls __arm_ioremap.

	Arnd