Subject: Issue with ioremap

Hi,

We are using the pl353 smc controller for interfacing the nand in our zynq SOC.
The driver for this controller is currently under mainline review.
Recently we are moved to 4.4 kernel and observing issues with the driver.
while debug, found that the issue is with the virtual address returned from
the ioremap is not aligned to the physical address and causing nand
access failures.
the nand controller physical address starts at 0xE1000000 and the size is 16MB.
the ioremap function in 4.3 kernel returns the virtual address that is
aligned to the size
but not the case in 4.4 kernel.

this controller uses the bits [31:24] as base address and use rest all bits for
configuring adders cycles, chip select information. so it expects the
virtual address also
aligned to 0xFF000000 otherwise the nand commands issued will fail.


with >= 4.4 kernel
0xf0200000-0xf1201000 16781312 devm_ioremap+0x3c/0x70 phys=e1000000 ioremap

with <= 4.3 kernel
0xf1000000-0xf2001000 16781312 devm_ioremap+0x38/0x68 phys=e1000000 ioremap

the below hack fixes the issue. but its not a proper fix and it just pointing
me the clue for this issue. so, any pointers and help to over come this issue ?
is there a way to do static mapping for the above requirement?


diff --git a/mm/vmalloc.c b/mm/vmalloc.c index 8e3c9c5..fda58d6 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -1340,9 +1340,13 @@ static struct vm_struct
*__get_vm_area_node(unsigned long size,
PAGE_SHIFT, IOREMAP_MAX_ORDER);

size = PAGE_ALIGN(size);
+ if (size == 0x1000000)
+ align = 0x1000000;
if (unlikely(!size))
return NULL;

+ printk(" size %x align %x\n", size, align);
+
area = kzalloc_node(sizeof(*area), gfp_mask & GFP_RECLAIM_MASK, node);
if (unlikely(!area))
return NULL;

Thanks,
Punnaiah


2016-03-31 20:01:50

by Josh Cartwright

[permalink] [raw]
Subject: Re: Issue with ioremap

On Fri, Apr 01, 2016 at 01:13:06AM +0530, punnaiah choudary kalluri wrote:
> Hi,
>
> We are using the pl353 smc controller for interfacing the nand in our zynq SOC.
> The driver for this controller is currently under mainline review.
> Recently we are moved to 4.4 kernel and observing issues with the driver.
> while debug, found that the issue is with the virtual address returned from
> the ioremap is not aligned to the physical address and causing nand
> access failures.
> the nand controller physical address starts at 0xE1000000 and the size is 16MB.
> the ioremap function in 4.3 kernel returns the virtual address that is
> aligned to the size
> but not the case in 4.4 kernel.

:(. I had actually ran into this, too, as I was evaluating the use of
the upstream-targetted pl353 stuff; sorry I didn't say anything.

> this controller uses the bits [31:24] as base address and use rest all
> bits for configuring adders cycles, chip select information. so it
> expects the virtual address also aligned to 0xFF000000 otherwise the
> nand commands issued will fail.

The driver _currently_ expects the virtual address to be 16M aligned,
but is that a hard requirement? It seems possible that the driver could
be written without this assumption, correct?

This would mean that the driver would need to maintain the cs/cycles
configuration state outside of the mapped virtual address, and then
calculate + add the calculated offset to the base. Would that work?
I had been meaning to give it a try, but haven't gotten around to it.

Josh

2016-03-31 22:53:33

by Laura Abbott

[permalink] [raw]
Subject: Re: Issue with ioremap

(cc linux-arm)

On 03/31/2016 01:01 PM, Josh Cartwright wrote:
> On Fri, Apr 01, 2016 at 01:13:06AM +0530, punnaiah choudary kalluri wrote:
>> Hi,
>>
>> We are using the pl353 smc controller for interfacing the nand in our zynq SOC.
>> The driver for this controller is currently under mainline review.
>> Recently we are moved to 4.4 kernel and observing issues with the driver.
>> while debug, found that the issue is with the virtual address returned from
>> the ioremap is not aligned to the physical address and causing nand
>> access failures.
>> the nand controller physical address starts at 0xE1000000 and the size is 16MB.
>> the ioremap function in 4.3 kernel returns the virtual address that is
>> aligned to the size
>> but not the case in 4.4 kernel.
>
> :(. I had actually ran into this, too, as I was evaluating the use of
> the upstream-targetted pl353 stuff; sorry I didn't say anything.
>
>> this controller uses the bits [31:24] as base address and use rest all
>> bits for configuring adders cycles, chip select information. so it
>> expects the virtual address also aligned to 0xFF000000 otherwise the
>> nand commands issued will fail.
>
> The driver _currently_ expects the virtual address to be 16M aligned,
> but is that a hard requirement? It seems possible that the driver could
> be written without this assumption, correct?
>
> This would mean that the driver would need to maintain the cs/cycles
> configuration state outside of the mapped virtual address, and then
> calculate + add the calculated offset to the base. Would that work?
> I had been meaning to give it a try, but haven't gotten around to it.
>
> Josh
>

I was curious so I took a look and this seems to be caused by

commit 803e3dbcb4cf80c898faccf01875f6ff6e5e76fd
Author: Sergey Dyasly <[email protected]>
Date: Wed Sep 9 16:27:18 2015 +0100

ARM: 8430/1: use default ioremap alignment for SMP or LPAE

16MB alignment for ioremap mappings was added by commit a069c896d0d6 ("[ARM]
3705/1: add supersection support to ioremap()") in order to support supersection
mappings. But __arm_ioremap_pfn_caller uses section and supersection mappings
only in !SMP && !LPAE case. There is no need for such big alignment if either
SMP or LPAE is enabled.

After this change, ioremap will use default maximum alignment of 128 pages.

Link: https://lkml.kernel.org/g/[email protected]

Cc: Guan Xuetao <[email protected]>
Cc: Nicolas Pitre <[email protected]>
Cc: James Bottomley <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: Catalin Marinas <[email protected]>
Cc: Andrew Morton <[email protected]>
Cc: Dmitry Safonov <[email protected]>
Signed-off-by: Sergey Dyasly <[email protected]>
Signed-off-by: Russell King <[email protected]>

The thread assumed the higher alignment behavior was only needed for super
section mappings. Apparently not.

Thanks,
Laura

2016-03-31 23:09:03

by Russell King - ARM Linux

[permalink] [raw]
Subject: Re: Issue with ioremap

On Thu, Mar 31, 2016 at 03:53:26PM -0700, Laura Abbott wrote:
> (cc linux-arm)
>
> On 03/31/2016 01:01 PM, Josh Cartwright wrote:
> >The driver _currently_ expects the virtual address to be 16M aligned,
> >but is that a hard requirement? It seems possible that the driver could
> >be written without this assumption, correct?
> >
> >This would mean that the driver would need to maintain the cs/cycles
> >configuration state outside of the mapped virtual address, and then
> >calculate + add the calculated offset to the base. Would that work?
> >I had been meaning to give it a try, but haven't gotten around to it.
>
> I was curious so I took a look and this seems to be caused by

The driver is most likely buggy in the way Josh has identified. The
peripheral device has no clue what virtual address is used to access
it, all it sees is the address on the bus.

--
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.