[PATCH v2 0/3] Switch to ioremap_wc in the SRAM allocator code

[PATCH v2 0/3] Switch to ioremap_wc in the SRAM allocator code

[Abhilash Kesavan]

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

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

[Abhilash Kesavan]

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 <a.kesavan@samsung.com>
---
 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

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

[Abhilash Kesavan]

Implement a resource managed writecombine ioremap function.

Signed-off-by: Abhilash Kesavan <a.kesavan@samsung.com>
---
 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

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

[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 <a.kesavan@samsung.com>
Tested-by: Tony Lindgren <tony@atomide.com>
Tested-by: Heiko Stuebner <heiko@sntech.de>
---
 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

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

[Catalin Marinas]

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 <catalin.marinas@arm.com>

Thanks.

-- 
Catalin

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

[Philipp Zabel]

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 <a.kesavan@samsung.com>
> Tested-by: Tony Lindgren <tony@atomide.com>
> Tested-by: Heiko Stuebner <heiko@sntech.de>

Acked-by: Philipp Zabel <p.zabel@pengutronix.de>

regards
Philipp

Re: [alsa-devel] [PATCH v2 0/3] Switch to ioremap_wc in the SRAM allocator code

[Abhilash Kesavan]

Hi Greg,

On Fri, Feb 6, 2015 at 9:14 PM, Catalin Marinas <catalin.marinas@arm.com> wrote:
> 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 <catalin.marinas@arm.com>

These patches still apply cleanly on linux-next and have been ack'ed
by Catalin and Philipp. Would you like me to collect the tags and
resend the series or can they be applied as is ?

Thanks,
Abhilash
>
> Thanks.
>
> --
> Catalin
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [alsa-devel] [PATCH v2 0/3] Switch to ioremap_wc in the SRAM allocator code

[gregkh]

On Sun, Mar 01, 2015 at 10:00:37PM +0530, Abhilash Kesavan wrote:
> Hi Greg,
> 
> On Fri, Feb 6, 2015 at 9:14 PM, Catalin Marinas <catalin.marinas@arm.com> wrote:
> > 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 <catalin.marinas@arm.com>
> 
> These patches still apply cleanly on linux-next and have been ack'ed
> by Catalin and Philipp. Would you like me to collect the tags and
> resend the series or can they be applied as is ?

I can take them as-is, thanks.

greg k-h