On Mon, Apr 09, 2018 at 04:06:15PM +0300, Laurent Pinchart wrote: > Hello, > > On Monday, 9 April 2018 14:11:22 EEST Robin Murphy wrote: > > On 09/04/18 08:25, jacopo mondi wrote: > > > Hi Robin, Laurent, > > > > > > a long time passed, sorry about this. > > > > > > On Wed, Nov 15, 2017 at 01:38:23PM +0000, Robin Murphy wrote: > > >> On 14/11/17 17:08, Jacopo Mondi wrote: > > >>> On SH4 architecture, with SPARSEMEM memory model, translating page to > > >>> pfn hangs the CPU. Post-pone translation to pfn after > > >>> dma_mmap_from_dev_coherent() function call as it succeeds and make page > > >>> translation not necessary. > > >>> > > >>> This patch was suggested by Laurent Pinchart and he's working to submit > > >>> a proper fix mainline. Not sending for inclusion at the moment. > > >> > > >> Y'know, I think this patch does have some merit by itself - until we know > > >> that cpu_addr *doesn't* represent some device-private memory which is not > > >> guaranteed to be backed by a struct page, calling virt_to_page() on it is > > >> arguably semantically incorrect, even if it might happen to be benign in > > >> most cases. > > > > > > I still need to carry this patch in my trees to have a working dma memory > > > mapping on SH4 platforms. My understanding from your comment is that > > > there may be a way forward for this patch, do you still think the same? > > > Have you got any suggestion on how to improve this eventually for > > > inclusion? > > > > As before, the change itself does seem reasonable; it might be worth > > rewording the commit message in more general terms rather than making it > > sound like an SH-specific workaround (which I really don't think it is), > > but otherwise I'd say just repost it as a non-RFC patch. > > I actually can't remember any better fix I would have in mind, so this looks > good to me :-) I agree with Robin, the commit message should be reworded. > Robin's explanation of why virt_to_page() should be postponed until we know > that cpu_addr represents memory that is guaranteed to be backed by a struct > page is a good starting point. You can mention SH4 as an example of an > architecture that will crash when calling virt_to_page() in such a case, but > the fix isn't specific to SH4. Just checking since I joined late -- is the consensus that this does not require any action/review specific to SH (in my role as maintainer way behind on maintenance)? Rich
