Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1329398 > unrolled thread

[RESEND2 PATCH 1/3] memremap: add MEMREMAP_WC flag

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [RESEND2 PATCH 1/3] memremap: add MEMREMAP_WC flag Brian Starkey <brian.starkey@arm.com> - 2016-02-08 18:40 +0100
    Re: [RESEND2 PATCH 1/3] memremap: add MEMREMAP_WC flag Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-02-08 19:40 +0100
      Re: [RESEND2 PATCH 1/3] memremap: add MEMREMAP_WC flag Brian Starkey <brian.starkey@arm.com> - 2016-02-09 10:20 +0100
    Re: [RESEND2 PATCH 1/3] memremap: add MEMREMAP_WC flag Andrew Morton <akpm@linux-foundation.org> - 2016-02-08 21:10 +0100

#1329398 — [RESEND2 PATCH 1/3] memremap: add MEMREMAP_WC flag

Add a flag to memremap() for writecombine mappings. Mappings satisfied
by this flag will not be cached, however writes may be delayed or
combined into more efficient bursts. This is most suitable for
buffers written sequentially by the CPU for use by other DMA devices.

Signed-off-by: Brian Starkey <brian.starkey@arm.com>
Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
---
 include/linux/io.h |    1 +
 kernel/memremap.c  |   15 +++++++++++++--
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/include/linux/io.h b/include/linux/io.h
index 32403b5..e2c8419 100644
--- a/include/linux/io.h
+++ b/include/linux/io.h
@@ -135,6 +135,7 @@ enum {
 	/* See memremap() kernel-doc for usage description... */
 	MEMREMAP_WB = 1 << 0,
 	MEMREMAP_WT = 1 << 1,
+	MEMREMAP_WC = 1 << 2,
 };
 
 void *memremap(resource_size_t offset, size_t size, unsigned long flags);
diff --git a/kernel/memremap.c b/kernel/memremap.c
index e517a16..3849987 100644
--- a/kernel/memremap.c
+++ b/kernel/memremap.c
@@ -41,11 +41,13 @@ static void *try_ram_remap(resource_size_t offset, size_t size)
  * memremap() - remap an iomem_resource as cacheable memory
  * @offset: iomem resource start address
  * @size: size of remap
- * @flags: either MEMREMAP_WB or MEMREMAP_WT
+ * @flags: any of MEMREMAP_WB, MEMREMAP_WT and MEMREMAP_WC
  *
  * memremap() is "ioremap" for cases where it is known that the resource
  * being mapped does not have i/o side effects and the __iomem
- * annotation is not applicable.
+ * annotation is not applicable. In the case of multiple flags, the different
+ * mapping types will be attempted in the order listed below until one of
+ * them succeeds.
  *
  * MEMREMAP_WB - matches the default mapping for "System RAM" on
  * the architecture.  This is usually a read-allocate write-back cache.
@@ -57,6 +59,10 @@ static void *try_ram_remap(resource_size_t offset, size_t size)
  * cache or are written through to memory and never exist in a
  * cache-dirty state with respect to program visibility.  Attempts to
  * map "System RAM" with this mapping type will fail.
+ *
+ * MEMREMAP_WC - establish a writecombine mapping, whereby writes may
+ * be coalesced together (e.g. in the CPU's write buffers), but is otherwise
+ * uncached. Attempts to map "System RAM" with this mapping type will fail.
  */
 void *memremap(resource_size_t offset, size_t size, unsigned long flags)
 {
@@ -101,6 +107,11 @@ void *memremap(resource_size_t offset, size_t size, unsigned long flags)
 		addr = ioremap_wt(offset, size);
 	}
 
+	if (!addr && (flags & MEMREMAP_WC)) {
+		flags &= ~MEMREMAP_WC;
+		addr = ioremap_wc(offset, size);
+	}
+
 	return addr;
 }
 EXPORT_SYMBOL(memremap);
-- 
1.7.9.5

[toc] | [next] | [standalone]

#1329464

On Mon, Feb 08, 2016 at 05:30:50PM +0000, Brian Starkey wrote:
> Add a flag to memremap() for writecombine mappings. Mappings satisfied
> by this flag will not be cached, however writes may be delayed or
> combined into more efficient bursts. This is most suitable for
> buffers written sequentially by the CPU for use by other DMA devices.
> 
> Signed-off-by: Brian Starkey <brian.starkey@arm.com>
> Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
> ---
>  include/linux/io.h |    1 +
>  kernel/memremap.c  |   15 +++++++++++++--
>  2 files changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/io.h b/include/linux/io.h
> index 32403b5..e2c8419 100644
> --- a/include/linux/io.h
> +++ b/include/linux/io.h
> @@ -135,6 +135,7 @@ enum {
>  	/* See memremap() kernel-doc for usage description... */
>  	MEMREMAP_WB = 1 << 0,
>  	MEMREMAP_WT = 1 << 1,
> +	MEMREMAP_WC = 1 << 2,

You didn't update the documentation :(

[toc] | [prev] | [next] | [standalone]

#1329982

Hi Greg,

On Mon, Feb 08, 2016 at 10:30:06AM -0800, Greg KH wrote:
>On Mon, Feb 08, 2016 at 05:30:50PM +0000, Brian Starkey wrote:
>> diff --git a/include/linux/io.h b/include/linux/io.h
>> index 32403b5..e2c8419 100644
>> --- a/include/linux/io.h
>> +++ b/include/linux/io.h
>> @@ -135,6 +135,7 @@ enum {
>>  	/* See memremap() kernel-doc for usage description... */
>>  	MEMREMAP_WB = 1 << 0,
>>  	MEMREMAP_WT = 1 << 1,
>> +	MEMREMAP_WC = 1 << 2,
>
>You didn't update the documentation :(
>

Maybe I missed something, but I don't think there's anything to update
here? Like the comment says, the flags are documented in the memremap()
kernel-doc (which I did update - see the next two hunks of this patch).

Thanks,

Brian

[toc] | [prev] | [next] | [standalone]

#1329515

On Mon,  8 Feb 2016 17:30:50 +0000 Brian Starkey <brian.starkey@arm.com> wrote:

> Add a flag to memremap() for writecombine mappings. Mappings satisfied
> by this flag will not be cached, however writes may be delayed or
> combined into more efficient bursts. This is most suitable for
> buffers written sequentially by the CPU for use by other DMA devices.
> 
> ...

The patch generally looks OK to me.  It generates rejects against
linux-next because of some janitorial changes in memremap.c.


> @@ -101,6 +107,11 @@ void *memremap(resource_size_t offset, size_t size, unsigned long flags)
>  		addr = ioremap_wt(offset, size);
>  	}
>  
> +	if (!addr && (flags & MEMREMAP_WC)) {
> +		flags &= ~MEMREMAP_WC;
> +		addr = ioremap_wc(offset, size);
> +	}
> +
>  	return addr;
>  }

The modifications of `flags' is unneeded (and the compiler will remove
it).  And generally the modification of incoming args is a bit nasty
IMO - I find it's better to treat them as const - part of the calling
environment which can be relied upon to be unaltered as the code
evolves.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web