[PATCH v2 0/3] fix memremap on ARM
From: Dan Williams <hidden>
Date: 2016-03-03 17:55:58
On Thu, Mar 3, 2016 at 9:41 AM, Ard Biesheuvel [off-list ref] wrote:
On 3 March 2016 at 18:33, Dan Williams [off-list ref] wrote:quoted
On Thu, Mar 3, 2016 at 9:30 AM, Ard Biesheuvel [off-list ref] wrote:quoted
On 3 March 2016 at 18:27, Dan Williams [off-list ref] wrote:quoted
On Thu, Mar 3, 2016 at 9:16 AM, Ard Biesheuvel [off-list ref] wrote:quoted
On 3 March 2016 at 18:13, Russell King - ARM Linux [off-list ref] wrote:quoted
On Thu, Mar 03, 2016 at 05:24:41PM +0100, Ard Biesheuvel wrote:quoted
If there are no remaining objections, may I suggest that Dan takes patch #1 and #2, and that I put the third patch into Russell's patch system? The only strict ordering requirement is that #1 is merged before #2 and #3 both hit mainline, and that is solved by taking #1 and #2 in order.How does the dependency between 1 and 3 get satisfied? Your comment makes no sense to me.Patch #3 introduces a function arch_memremap_wb() on the ARM side which will never get called unless patch #2 is also merged. Once both #2 and #3 are in, the memremap() call that #1 reverts will use MT_MEMORY_RW attributes rather than MT_DEVICE_CACHED, so #1 needs to go first, but #2 and #3 can go in in either order. In other words, there are no build dependencies between the patches, but to prevent pxa2xx-flash from using MT_MEMORY_RW attributes, it needs to go in before #2Can we add a new MEMREMAP_ type for this case so that we don't need to keep carrying the confusing ioremap_cache() which means different things to different archs and silently falls back to uncached on some archs.Actually, I don't think so. Unifying ioremap_cache() between architectures was a mistake to begin with, since I/O mapping attributes vary so wildly between architectures, and there are very few drivers that depend on such specific behavior that actually need to be built for multiple architectures. So I think your memremap() is a fantastic idea, considering the widespread abuse of ioremap_cache(), also in common code. But removing it altogether to replace it with something that looks generic but never is, is just repeating the same mistake imo.I'm not suggesting it be a generic flag. I.e. something like MEMREMAP_ARM_ that explicitly documents that this driver has arch specific mapping dependencies. That request type will fail on any arch that does not implement MEMREMAP_ARM_ support.I don't see the point of adding this to a generic API. ioremap_cache() is used to perform I/O, even if it has memory semantics on read. Getting rid of the abuse, where ioremap_cache() is used to map system RAM to access ACPI tables etc is something that we should fix, yes, but arch specific I/O needs to stay out of the picture imo, simply because there is no problem to fix here.
Hmm, then what about Russell's suggestoin, if I'm not mischaracterizing, that ioremap_cache() move to its old ioremap_cached() name. The latter has less cross-arch confusion.