2015-02-06 13:45:43

by Abhilash Kesavan

[permalink] [raw]
Subject: [PATCH v2 0/3] Switch to ioremap_wc in the SRAM allocator code

Fix alignment faults seen during play-back of files with specific
sampling rates such as 44.1K. This is based on the discussion here:
http://www.spinics.net/lists/arm-kernel/msg384647.html

Patch 1 is needed as m32r arch does not define ioremap_wc. Patch 2 adds a
resource managed helper function for ioremap_wc which is used in patch 3.

Changes since v1:
- Fix the compilation error with m32r due to missing ioremap_wc
definition.
- Fixed minor indentation issues.

Note: Other architectures either have ioremap_wc defined or are including
generic "io.h"/"iomap.h". c6x arch is the only one I am not sure of.

Abhilash Kesavan (3):
m32r: add definition of ioremap_wc to io.h
lib: devres: add a helper function for ioremap_wc
misc: sram: switch to ioremap_wc from ioremap

Documentation/driver-model/devres.txt | 1 +
arch/m32r/include/asm/io.h | 1 +
drivers/misc/sram.c | 17 ++++++++++++++---
include/linux/io.h | 2 ++
lib/devres.c | 28 ++++++++++++++++++++++++++++
5 files changed, 46 insertions(+), 3 deletions(-)

--
1.7.9.5


2015-02-06 13:45:50

by Abhilash Kesavan

[permalink] [raw]
Subject: [PATCH v2 1/3] m32r: add definition of ioremap_wc to io.h

Before adding a resource managed ioremap_wc function, we need
to have ioremap_wc defined for m32r to prevent build errors.

Signed-off-by: Abhilash Kesavan <[email protected]>
---
arch/m32r/include/asm/io.h | 1 +
1 file changed, 1 insertion(+)

diff --git a/arch/m32r/include/asm/io.h b/arch/m32r/include/asm/io.h
index 6e7787f..9cc00db 100644
--- a/arch/m32r/include/asm/io.h
+++ b/arch/m32r/include/asm/io.h
@@ -67,6 +67,7 @@ static inline void __iomem *ioremap(unsigned long offset, unsigned long size)

extern void iounmap(volatile void __iomem *addr);
#define ioremap_nocache(off,size) ioremap(off,size)
+#define ioremap_wc ioremap_nocache

/*
* IO bus memory addresses are also 1:1 with the physical address
--
1.7.9.5

2015-02-06 13:45:54

by Abhilash Kesavan

[permalink] [raw]
Subject: [PATCH v2 2/3] lib: devres: add a helper function for ioremap_wc

Implement a resource managed writecombine ioremap function.

Signed-off-by: Abhilash Kesavan <[email protected]>
---
Documentation/driver-model/devres.txt | 1 +
include/linux/io.h | 2 ++
lib/devres.c | 28 ++++++++++++++++++++++++++++
3 files changed, 31 insertions(+)

diff --git a/Documentation/driver-model/devres.txt b/Documentation/driver-model/devres.txt
index 6d1e8ee..7fe7fd2 100644
--- a/Documentation/driver-model/devres.txt
+++ b/Documentation/driver-model/devres.txt
@@ -276,6 +276,7 @@ IOMAP
devm_ioport_unmap()
devm_ioremap()
devm_ioremap_nocache()
+ devm_ioremap_wc()
devm_ioremap_resource() : checks resource, requests memory region, ioremaps
devm_iounmap()
pcim_iomap()
diff --git a/include/linux/io.h b/include/linux/io.h
index fa02e55..42b33f0 100644
--- a/include/linux/io.h
+++ b/include/linux/io.h
@@ -64,6 +64,8 @@ void __iomem *devm_ioremap(struct device *dev, resource_size_t offset,
resource_size_t size);
void __iomem *devm_ioremap_nocache(struct device *dev, resource_size_t offset,
resource_size_t size);
+void __iomem *devm_ioremap_wc(struct device *dev, resource_size_t offset,
+ resource_size_t size);
void devm_iounmap(struct device *dev, void __iomem *addr);
int check_signature(const volatile void __iomem *io_addr,
const unsigned char *signature, int length);
diff --git a/lib/devres.c b/lib/devres.c
index 0f1dd2e..fbe2aac 100644
--- a/lib/devres.c
+++ b/lib/devres.c
@@ -72,6 +72,34 @@ void __iomem *devm_ioremap_nocache(struct device *dev, resource_size_t offset,
EXPORT_SYMBOL(devm_ioremap_nocache);

/**
+ * devm_ioremap_wc - Managed ioremap_wc()
+ * @dev: Generic device to remap IO address for
+ * @offset: BUS offset to map
+ * @size: Size of map
+ *
+ * Managed ioremap_wc(). Map is automatically unmapped on driver detach.
+ */
+void __iomem *devm_ioremap_wc(struct device *dev, resource_size_t offset,
+ resource_size_t size)
+{
+ void __iomem **ptr, *addr;
+
+ ptr = devres_alloc(devm_ioremap_release, sizeof(*ptr), GFP_KERNEL);
+ if (!ptr)
+ return NULL;
+
+ addr = ioremap_wc(offset, size);
+ if (addr) {
+ *ptr = addr;
+ devres_add(dev, ptr);
+ } else
+ devres_free(ptr);
+
+ return addr;
+}
+EXPORT_SYMBOL(devm_ioremap_wc);
+
+/**
* devm_iounmap - Managed iounmap()
* @dev: Generic device to unmap for
* @addr: Address to unmap
--
1.7.9.5

2015-02-06 13:46:01

by Abhilash Kesavan

[permalink] [raw]
Subject: [PATCH v2 3/3] misc: sram: switch to ioremap_wc from ioremap

Currently, the SRAM allocator returns device memory via ioremap.
This causes issues on ARM64 when the internal SoC SRAM allocated by
the generic sram driver is used for audio playback. The destination
buffer address (which is ioremapped SRAM) is not 64-bit aligned for
certain streams (e.g. 44.1k sampling rate). In such cases we get
unhandled alignment faults. Use ioremap_wc in place of ioremap which
gives us normal non-cacheable memory instead of device memory.

Signed-off-by: Abhilash Kesavan <[email protected]>
Tested-by: Tony Lindgren <[email protected]>
Tested-by: Heiko Stuebner <[email protected]>
---
drivers/misc/sram.c | 17 ++++++++++++++---
1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/misc/sram.c b/drivers/misc/sram.c
index 21181fa..15b4d4e 100644
--- a/drivers/misc/sram.c
+++ b/drivers/misc/sram.c
@@ -69,12 +69,23 @@ static int sram_probe(struct platform_device *pdev)
INIT_LIST_HEAD(&reserve_list);

res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- virt_base = devm_ioremap_resource(&pdev->dev, res);
- if (IS_ERR(virt_base))
- return PTR_ERR(virt_base);
+ if (!res) {
+ dev_err(&pdev->dev, "found no memory resource\n");
+ return -EINVAL;
+ }

size = resource_size(res);

+ if (!devm_request_mem_region(&pdev->dev,
+ res->start, size, pdev->name)) {
+ dev_err(&pdev->dev, "could not request region for resource\n");
+ return -EBUSY;
+ }
+
+ virt_base = devm_ioremap_wc(&pdev->dev, res->start, size);
+ if (IS_ERR(virt_base))
+ return PTR_ERR(virt_base);
+
sram = devm_kzalloc(&pdev->dev, sizeof(*sram), GFP_KERNEL);
if (!sram)
return -ENOMEM;
--
1.7.9.5

2015-02-06 15:44:35

by Catalin Marinas

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] Switch to ioremap_wc in the SRAM allocator code

On Fri, Feb 06, 2015 at 01:45:25PM +0000, Abhilash Kesavan wrote:
> Fix alignment faults seen during play-back of files with specific
> sampling rates such as 44.1K. This is based on the discussion here:
> http://www.spinics.net/lists/arm-kernel/msg384647.html
>
> Patch 1 is needed as m32r arch does not define ioremap_wc. Patch 2 adds a
> resource managed helper function for ioremap_wc which is used in patch 3.
>
> Changes since v1:
> - Fix the compilation error with m32r due to missing ioremap_wc
> definition.
> - Fixed minor indentation issues.
>
> Note: Other architectures either have ioremap_wc defined or are including
> generic "io.h"/"iomap.h". c6x arch is the only one I am not sure of.
>
> Abhilash Kesavan (3):
> m32r: add definition of ioremap_wc to io.h
> lib: devres: add a helper function for ioremap_wc
> misc: sram: switch to ioremap_wc from ioremap
>
> Documentation/driver-model/devres.txt | 1 +
> arch/m32r/include/asm/io.h | 1 +
> drivers/misc/sram.c | 17 ++++++++++++++---
> include/linux/io.h | 2 ++
> lib/devres.c | 28 ++++++++++++++++++++++++++++
> 5 files changed, 46 insertions(+), 3 deletions(-)

For the series:

Acked-by: Catalin Marinas <[email protected]>

Thanks.

--
Catalin

2015-02-06 16:10:16

by Philipp Zabel

[permalink] [raw]
Subject: Re: [PATCH v2 3/3] misc: sram: switch to ioremap_wc from ioremap

Am Freitag, den 06.02.2015, 19:15 +0530 schrieb Abhilash Kesavan:
> Currently, the SRAM allocator returns device memory via ioremap.
> This causes issues on ARM64 when the internal SoC SRAM allocated by
> the generic sram driver is used for audio playback. The destination
> buffer address (which is ioremapped SRAM) is not 64-bit aligned for
> certain streams (e.g. 44.1k sampling rate). In such cases we get
> unhandled alignment faults. Use ioremap_wc in place of ioremap which
> gives us normal non-cacheable memory instead of device memory.
>
> Signed-off-by: Abhilash Kesavan <[email protected]>
> Tested-by: Tony Lindgren <[email protected]>
> Tested-by: Heiko Stuebner <[email protected]>

Acked-by: Philipp Zabel <[email protected]>

regards
Philipp