On 5 November 2014 20:39, Kumar Gala <ga...@codeaurora.org> wrote: > > On Nov 5, 2014, at 6:55 AM, Ankit Jindal <ankit.jin...@linaro.org> wrote: > >> Hi Kumar, >> >> On 31 October 2014 19:09, Kumar Gala <ga...@codeaurora.org> wrote: >>> >>> On Oct 31, 2014, at 4:30 AM, Ankit Jindal <ankit.jin...@linaro.org> wrote: >>> >>>> Hi Kumar, >>>> >>>> On 21 October 2014 12:08, Kumar Gala <ga...@codeaurora.org> wrote: >>>>> >>>>> On Oct 21, 2014, at 7:56 AM, Ankit Jindal <ankit.jin...@linaro.org> wrote: >>>>> >>>>>> Currently, three types of mem regions are supported: UIO_MEM_PHYS, >>>>>> UIO_MEM_LOGICAL and UIO_MEM_VIRTUAL. Among these UIO_MEM_PHYS helps >>>>>> UIO driver export physcial memory to user space as non-cacheable >>>>>> user memory. Typcially memory-mapped registers of a device are exported >>>>>> to user space as UIO_MEM_PHYS type mem region. The UIO_MEM_PHYS type >>>>>> is not efficient if dma-capable devices are capable of maintaining >>>>>> coherency >>>>>> with CPU caches. >>>>>> >>>>>> This patch adds new type UIO_MEM_PHYS_CACHE for mem regions to enable >>>>>> cacheable access to physical memory from user space. >>>>>> >>>>>> Signed-off-by: Ankit Jindal <ankit.jin...@linaro.org> >>>>>> Signed-off-by: Tushar Jagad <tushar.ja...@linaro.org> >>>>>> --- >>>>>> drivers/uio/uio.c | 11 ++++++++--- >>>>>> include/linux/uio_driver.h | 1 + >>>>>> 2 files changed, 9 insertions(+), 3 deletions(-) >>>>> >>>>> Rather than adding a new type, why not allow the driver to set the pgprot >>>>> value, this way one has full control and we don’t need to keep adding >>>>> types for various different cache attributions in the future. >>>> >>>> Do you mean to add a new field pgprot_t in the memtype structure and >>>> uio_mmap_physical will set vma->vm_page_prot to this value provided by >>>> driver ? If this is the case then we will need to change all the >>>> current uio based drivers which was the reason I preferred to have a >>>> new mem type. >>>> >>>> Please let me know if I have misunderstood anything. >>> >>> I’m suggeting in uio_mmap_physical to do something like: >>> >>> if (idev->info->set_pgprot) >>> idev->info->set_pgprot(vma->vm_page_prot) >>> else >>> vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot); >>> >>> And add a set_prprot callback to 'struct uio_info’. >>> >>> Here’s patch from several years ago: >>> >>> http://patchwork.ozlabs.org/patch/119224/ >> >> The suggested solution looks okey but not sure whether there is any >> available drivers using different combinations. Also, I looked at the >> available pgprot routines, looks like only pgprot_noncached and >> pgprot_writecombine are the available ones. So if we are not going to >> use these pgprot routines then driver might have architecture >> dependent switches, which we should avoid. > > There are cases that are arch/driver specific that do not fall into > pgprot_noncached or pgprot_writecombine. So I don’t see why we should limit > them. For example the Freescale networking guys need cacheable-noncoherent > for some of their UIO work. > > We can deal with arch specific issues during review of the UIO driver > themselves.
Ok. But, in our case we do not want to set any special attribute, instead just want to avoid setting non-cacheable attribute. So if we go by the approach as in your patch, then I need to register a dummy routine which does nothing. I think another approach could be to have new mem type as in this patch and add something like this to this patch: if (cacheable) { /* check if driver want to set any special cacheable attribute combination */ if (idev->info->set_pgprot) idev->info->set_pgprot(vma->vm_page_prot) } else { vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot); } Please let me know your view. > - k > > -- > Qualcomm Innovation Center, Inc. > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, > a Linux Foundation Collaborative Project > Thanks, Ankit -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/