RE: [PATCH] drivers: cma: fix addressing on PAE machines

[Michal Nazarewicz <mina86@mina86.com>]

[-- Attachment #1: Type: text/plain, Size: 2849 bytes --]

>> On Fri, Nov 30 2012, Vitaly Andrianov <vitalya@ti.com> wrote:
>> > This patch fixes a couple of bugs that otherwise impair CMA
>> > functionality on PAE machines:
>> >
>> >   - alignment must be a 64-bit type when running on systems with 64-
>> bit
>> >     physical addresses.  If this is not the case, the limit
>> calculation thunks
>> >     allocations down to an address range < 4G.
>> >
>> >   - The allocated range check is removed. On 32bit ARM kernel with
>> LPAE
>> >     enabled the base may be allocated outside the fist 4GB of
>> physical
>> >     memory (keystone SoC for example).
>> >
>> > Signed-off-by: Vitaly Andrianov <vitalya@ti.com>
>> > Signed-off-by: Cyril Chemparathy <cyril@ti.com>
>> 
>> To be consistent, you should also change the type in:
>> 
>> static long size_cmdline = -1;
>> static const unsigned long size_bytes = CMA_SIZE_MBYTES * SZ_1M; static
>> unsigned long __init __maybe_unused cma_early_percent_memory(void)
>> 
>> and in
>> 
>> void __init dma_contiguous_reserve(phys_addr_t limit)

On Fri, Nov 30 2012, Andrianov, Vitaly wrote:
> Can you elaborate why we need to change type for size? 

Like I've wrote, for consistency.  The size should use the same type as
alignment.  This most likely won't affect the way you use the code, but
it's the right thing to do.  It should be done that way from the start,
so it's really my mistake, but since you are touching the code, it'd
best to have the changes contained in a single commit.  Alternatively
I can just send my own patch with all those fixes (including yours).

>> > diff --git a/drivers/base/dma-contiguous.c
>> > b/drivers/base/dma-contiguous.c index 9a14694..7936b2e 100644
>> > --- a/drivers/base/dma-contiguous.c
>> > +++ b/drivers/base/dma-contiguous.c
>> > @@ -234,7 +234,7 @@ int __init dma_declare_contiguous(struct device
>> *dev, unsigned long size,
>> >  				  phys_addr_t base, phys_addr_t limit)  {
>> >  	struct cma_reserved *r = &cma_reserved[cma_reserved_count];
>> > -	unsigned long alignment;
>> > +	phys_addr_t alignment;
>> >
>> >  	pr_debug("%s(size %lx, base %08lx, limit %08lx)\n", __func__,
>> >  		 (unsigned long)size, (unsigned long)base, @@ -271,10 +271,6
>> @@ int
>> > __init dma_declare_contiguous(struct device *dev, unsigned long size,
>> >  		if (!addr) {
>> >  			base = -ENOMEM;
>> >  			goto err;
>> > -		} else if (addr + size > ~(unsigned long)0) {
>> > -			memblock_free(addr, size);
>> > -			base = -EINVAL;
>> > -			goto err;
>> >  		} else {
>> >  			base = addr;
>> >  		}

-- 
Best regards,                                         _     _
.o. | Liege of Serenely Enlightened Majesty of      o' \,=./ `o
..o | Computer Science,  Michał “mina86” Nazarewicz    (o o)
ooo +----<email/xmpp: mpn@google.com>--------------ooO--(_)--Ooo--

[-- Attachment #2.1: Type: text/plain, Size: 0 bytes --]



[-- Attachment #2.2: Type: application/pgp-signature, Size: 835 bytes --]