Bug in physmap driver?

Bug in physmap driver?

[David Howells]

In physmap_flash_probe(), isn't the '+ 1' in:

		info->map[i].size = dev->resource[i].end - dev->resource[i].start + 1;

incorrect?  It results in an artificially inflated size:

(gdb) p info->map[0]
$7 = {name = 0xc08aa520 "physmap-flash.0", size = 0x200001, phys = 0xff000000, virt = 0x0, cached = 0x0, bankwidth = 0x4, read = 0, copy_from = 0, write = 0, copy_to = 0, inval_cache = 0, set_vpp = 0, pfow_base = 0x0, map_priv_1 = 0x0, map_priv_2 = 0x0, fldrv_priv = 0x0, fldrv = 0x0}

which is then handed to devm_ioremap().

David

Re: Bug in physmap driver?

[Lennert Buytenhek]

On Thu, Apr 16, 2009 at 04:00:19PM +0100, David Howells wrote:

> In physmap_flash_probe(), isn't the '+ 1' in:
> 
> 		info->map[i].size = dev->resource[i].end - dev->resource[i].start + 1;
> 
> incorrect?  It results in an artificially inflated size:
> 
> (gdb) p info->map[0]
> $7 = {name = 0xc08aa520 "physmap-flash.0", size = 0x200001,

That would suggest that dev->resource[i].end is off-by-one?

Re: Bug in physmap driver?

[David Howells]

Lennert Buytenhek <buytenh@wantstofly.org> wrote:

> That would suggest that dev->resource[i].end is off-by-one?

Actually, that's a good question: should end be one after the last useful
addres, or should it be that last useful address?

David

Re: Bug in physmap driver?

[Lennert Buytenhek]

On Thu, Apr 16, 2009 at 04:09:41PM +0100, David Howells wrote:

> > That would suggest that dev->resource[i].end is off-by-one?
> 
> Actually, that's a good question: should end be one after the last useful
> addres, or should it be that last useful address?

The last useful address.  That's what the resource code expects, as
e.g. two regions with start=00100000,end=00200000 and
start=00200000,end=00300000 will conflict (registration of the second
will fail), whereas if you change the end addresses to 001fffff and
002fffff, they will no longer conflict.

And e.g. /proc/iomem on my x86 box has:

	c4000000-c40fffff : PCI Bus #01
	dff00000-efefffff : PCI Bus #05
	ff500000-ff5fffff : PCI Bus #01
	ff600000-ff6fffff : PCI Bus #02
	ff700000-ff7fffff : PCI Bus #03
	ff800000-ff8fffff : PCI Bus #04
	ff900000-ff9fffff : PCI Bus #05

Re: Bug in physmap driver?

[David Howells]

Lennert Buytenhek <buytenh@wantstofly.org> wrote:

> The last useful address.  That's what the resource code expects, as
> e.g. two regions with start=00100000,end=00200000 and
> start=00200000,end=00300000 will conflict (registration of the second
> will fail), whereas if you change the end addresses to 001fffff and
> 002fffff, they will no longer conflict.

Okay, yes; you're right.  It no longer locks up.

Do you by any chance know what the 'width' field of struct physmap_flash_data
means?  Is it the total width of the data bus going to the flash chips?  Or is
it the width of each flash chip?

David

Re: Bug in physmap driver?

[Nicolas Pitre]

On Thu, 16 Apr 2009, David Howells wrote:

> Do you by any chance know what the 'width' field of struct physmap_flash_data
> means?  Is it the total width of the data bus going to the flash chips?  Or is
> it the width of each flash chip?

It's the total bus width.


Nicolas