Hi all,
I'm wondering whether the +1 in resource_size() is actually necessary.
To illustrate the problem imagine we have two subsequent 4k memory areas
which should be passed to ioremap(). When passing those address spaces
to the drivers we will use struct resource and define something like:
#define MEM_AREA1_BASE 0x480ab000
#define MEM_AREA2_BASE 0x480ac000
struct resource my_device_resources = {
{
.start = MEM_AREA1_BASE,
.end = MEM_AREA1_BASE + SZ_4K - 1,
.flags = IORESOURCE_MEM,
},
{
.start = MEM_AREA2_BASE,
.end = MEM_AREA2_BASE + SZ_4K - 1,
.flags = IORESOURCE_MEM,
},
};
that -1 would cause us to pass the correct end for each area,
respectively 0x480abfff and 0x480acffff, right ?
But resource_size() is defined as:
static inline resource_size_t resource_size(const struct resource *res)
{
return res->end - res->start + 1;
}
then we put +1 again and the end addresses would then become
respectively 0x480ac000 and 0x480ad0000. Wouldn't we then fail ioremap()
of the second area ? wouldn't it return -EBUSY ? Are we off-by-one
here ? Or is this all expected ?
I have to say, I didn't test this on real hw yet...
--
balbi
On Tue, 2009-12-29 at 16:12 -0800, Joe Perches wrote:
> On Wed, 2009-12-30 at 01:43 +0200, Felipe Balbi wrote:
> > I'm wondering whether the +1 in resource_size() is actually necessary.
> > resource_size() is defined as:
> []
> > static inline resource_size_t resource_size(const struct resource *res)
> > {
> > return res->end - res->start + 1;
> > }
> > Are we off-by-one
> > here ? Or is this all expected ?
>
> Imagine you have 1 byte sized resources.
>
> AREA1 = 0x40000000
> AREA2 = 0x40000001
>
> area1.start = 0x40000000
> area1.end = 0x40000000
>
> area2.start = 0x40000001
> area2.end = 0x40000001
(adding lkml back to the loop)
in that you wouldn't use any of the SZ_* macros and simply hardcode
start and end, right ? then you would define:
area1.start = 0x40000000
area1.end = 0x40000001
and ioremap 2 bytes due to +1 in resource_size().
--
balbi
On 30/12/2009, at 11:16 AM, Felipe Balbi wrote:
> On Tue, 2009-12-29 at 16:12 -0800, Joe Perches wrote:
>> On Wed, 2009-12-30 at 01:43 +0200, Felipe Balbi wrote:
>>> I'm wondering whether the +1 in resource_size() is actually necessary.
>>> resource_size() is defined as:
>> []
>>> static inline resource_size_t resource_size(const struct resource *res)
>>> {
>>> return res->end - res->start + 1;
>>> }
>>> Are we off-by-one
>>> here ? Or is this all expected ?
>>
>> Imagine you have 1 byte sized resources.
>>
>> AREA1 = 0x40000000
>> AREA2 = 0x40000001
>>
>> area1.start = 0x40000000
>> area1.end = 0x40000000
>>
>> area2.start = 0x40000001
>> area2.end = 0x40000001
>
> (adding lkml back to the loop)
>
> in that you wouldn't use any of the SZ_* macros and simply hardcode
> start and end, right ? then you would define:
>
> area1.start = 0x40000000
> area1.end = 0x40000001
No-o, you'd hardcode
area1.start=0x40000000
area1.end=0x40000000
As Joe wrote. Then the resource_size would return 1 and ioremap would map the 1 byte starting at 0x400000000, i.e. just 0x400000000. This is correct.
In your original example you had
.start = MEM_AREA1_BASE,
.end = MEM_AREA1_BASE + SZ_4K - 1
So resource_size would return SZ_4K and ioremap would map the SZ_4K bytes starting at MEM_AREA1_BASE, i.e. MEM_AREA1_BASE to MEM_AREA1_BASE - 1 /inclusive/. This is also correct.
--Ben.
On 30/12/2009, at 12:04 PM, Ben Nizette wrote:
>
>
> So resource_size would return SZ_4K and ioremap would map the SZ_4K bytes starting at MEM_AREA1_BASE, i.e. MEM_AREA1_BASE to MEM_AREA1_BASE - 1 /inclusive/. This is also correct.
>
IOW it's a matter of convention. Either we could define a single-byte range to be end=start and put the +1 in the resource_size or we could define a single-byte range to be end=start+1 and leave the +1 out of resource_size. The in-kernel convention is the former
> --Ben.
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
On Wed, 2009-12-30 at 12:08 +1100, Ben Nizette wrote:
> IOW it's a matter of convention. Either we could define a single-byte
> range to be end=start and put the +1 in the resource_size or we could
> define a single-byte range to be end=start+1 and leave the +1 out of
> resource_size. The in-kernel convention is the former
ok, thanks for the explanation Ben.
--
balbi