public inbox for linux-arm-kernel@lists.infradead.org
 help / color / mirror / Atom feed
From: jdzheng@broadcom.com (Jiandong Zheng)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] arm: fix build failure in code for mach-bcmring/dma.c
Date: Mon, 23 Jan 2012 10:53:27 -0800	[thread overview]
Message-ID: <4F1DACA7.1030501@broadcom.com> (raw)
In-Reply-To: <20120122211723.GB12326@n2100.arm.linux.org.uk>

On 1/22/2012 1:17 PM, Russell King - ARM Linux wrote:
> On Sun, Jan 22, 2012 at 04:09:39PM -0500, Paul Gortmaker wrote:
>> Upstream commit 99d1717dd7fecf2b10195b0d864323b952b4eba0
>>
>>         "ARM: Add init_consistent_dma_size()"
>>
>> essentially did this:
>>
>> -#define CONSISTENT_BASE                (CONSISTENT_END - CONSISTENT_DMA_SIZE)
>> +unsigned long consistent_base = CONSISTENT_END - DEFAULT_CONSISTENT_DMA_SIZE;
>>
>> but the bcmring code was still using the old CONSISTENT_BASE
>> macro.  Update it to now use the dynamic variable that reflects
>> the ability to resize early at boot.  To do so involves putting
>> the variable alongside of init_consistent_dma_size in dma-mapping.h
>
> Oh god, what are the bcmring people doing with this variable?
>
> DMA_MemType_t dma_mem_type(void *addr)
> {
>          unsigned long addrVal = (unsigned long)addr;
>
>          if (addrVal>= CONSISTENT_BASE) {
>                  /* NOTE: DMA virtual memory space starts at 0xFFxxxxxx */
>
>                  /* dma_alloc_xxx pages are physically and virtually contiguous */
>
>                  return DMA_MEM_TYPE_DMA;
>          }
>
Originally VMALLOC_END was used and it was changes to CONSISTENT_BASE in 
6a40b33270564d706396f1b514a988d3c by Nicolas and merged to linux-next, 
at a time it caused build error because CONSISTENT_BASE was no longer 
available in linux-next.

Can someone knows the context comment on these changes?

> int dma_mem_supports_dma(void *addr)
> {
>          DMA_MemType_t memType = dma_mem_type(addr);
>
>          return (memType == DMA_MEM_TYPE_DMA)
> #if ALLOW_MAP_OF_KMALLOC_MEMORY
>              || (memType == DMA_MEM_TYPE_KMALLOC)
> #endif
>              || (memType == DMA_MEM_TYPE_USER);
> }
>
> ...
>          case DMA_MEM_TYPE_DMA:
>                  {
>                          /* dma_alloc_xxx pages are physically contiguous */
>
>                          atomic_inc(&gDmaStatMemTypeCoherent);
>
>                          physAddr = (vmalloc_to_pfn(mem)<<  PAGE_SHIFT) + offset;
>
>                          dma_sync_single_for_cpu(NULL, physAddr, numBytes,
>                                                  memMap->dir);
>                          rc = dma_map_add_segment(memMap, region, mem, physAddr,
>                                                   numBytes);
>                          break;
>
> That's invalid for one thing.  Calling dma_sync_single_xxx with this
> made-up stuff isn't guaranteed to work - and certainly won't have the
> desired effect on ARM.  Not only that, but it's completely unnecessary
> for DMA coherent memory.
>
> So really, this code is broken.  It needs to be fixed, rather than fixing
> the core ARM code to allow this brokenness to persist.
>
I will take a look. As this code has been there for a while, is there 
something changed in, for example, DMA API I should be aware of?

  parent reply	other threads:[~2012-01-23 18:53 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-01-22 21:09 [PATCH] arm: fix build failure in code for mach-bcmring/dma.c Paul Gortmaker
2012-01-22 21:17 ` Russell King - ARM Linux
2012-01-22 23:32   ` Paul Gortmaker
2012-01-23 18:53   ` Jiandong Zheng [this message]
2012-01-23 20:18     ` Russell King - ARM Linux
2012-01-23 20:37       ` Jiandong Zheng

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4F1DACA7.1030501@broadcom.com \
    --to=jdzheng@broadcom.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox