Message ID | 1319817030-23992-1-git-send-email-galak@kernel.crashing.org (mailing list archive) |
---|---|
State | Not Applicable |
Delegated to: | Kumar Gala |
Headers | show |
On Fri, Oct 28, 2011 at 10:50:29AM -0500, Kumar Gala wrote: > For some devices, the default behavior of pgprot_noncached() is not > appropriate for all of its mappable regions. This provides a means for > the kernel side of the UIO driver to override the flags without having > to implement its own full mmap callback. Thanks for also providing an example driver showing the use of this. You should also post this driver in a mainline-ready version, I'm a bit uncomfortable with adding a new function pointer without having any users. And since you change uio_driver.h you should also update documentation accordingly (Documentation/DocBook/uio-howto.tmpl). Otherwise, I have no general objections. Thanks, Hans > > Signed-off-by: Kumar Gala <galak@kernel.crashing.org> > Signed-off-by: Geoff Thorpe <geoff@geoffthorpe.net> > --- > drivers/uio/uio.c | 6 +++++- > include/linux/uio_driver.h | 3 +++ > 2 files changed, 8 insertions(+), 1 deletions(-) > > diff --git a/drivers/uio/uio.c b/drivers/uio/uio.c > index dc27d89..0aebe27 100644 > --- a/drivers/uio/uio.c > +++ b/drivers/uio/uio.c > @@ -655,7 +655,11 @@ static int uio_mmap_physical(struct vm_area_struct *vma) > > vma->vm_flags |= VM_IO | VM_RESERVED; > > - vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot); > + if (idev->info->set_pgprot) > + vma->vm_page_prot = idev->info->set_pgprot(idev->info, mi, > + vma->vm_page_prot); > + else > + vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot); > > return remap_pfn_range(vma, > vma->vm_start, > diff --git a/include/linux/uio_driver.h b/include/linux/uio_driver.h > index fd99ff9..edfe7c8 100644 > --- a/include/linux/uio_driver.h > +++ b/include/linux/uio_driver.h > @@ -80,6 +80,7 @@ struct uio_device; > * @open: open operation for this uio device > * @release: release operation for this uio device > * @irqcontrol: disable/enable irqs when 0/1 is written to /dev/uioX > + * @set_pgprot: allow driver to override default(noncached) pgprot > */ > struct uio_info { > struct uio_device *uio_dev; > @@ -95,6 +96,8 @@ struct uio_info { > int (*open)(struct uio_info *info, struct inode *inode); > int (*release)(struct uio_info *info, struct inode *inode); > int (*irqcontrol)(struct uio_info *info, s32 irq_on); > + pgprot_t (*set_pgprot)(struct uio_info *uio, unsigned int mem_idx, > + pgprot_t prot); > }; > > extern int __must_check > -- > 1.7.3.4 > >
On Fri, Oct 28, 2011 at 11:48:12PM +0200, Hans J. Koch wrote: > On Fri, Oct 28, 2011 at 10:50:29AM -0500, Kumar Gala wrote: > > For some devices, the default behavior of pgprot_noncached() is not > > appropriate for all of its mappable regions. This provides a means for > > the kernel side of the UIO driver to override the flags without having > > to implement its own full mmap callback. > > Thanks for also providing an example driver showing the use of this. > You should also post this driver in a mainline-ready version, I'm a bit > uncomfortable with adding a new function pointer without having any users. I'm more than "uncomfortable", I'll refuse to take any such patch unless there is a in-kernel user, otherwise it makes no sense to add the pointer at all. thanks, greg k-h
On Oct 29, 2011, at 1:38 AM, Greg KH wrote: > On Fri, Oct 28, 2011 at 11:48:12PM +0200, Hans J. Koch wrote: >> On Fri, Oct 28, 2011 at 10:50:29AM -0500, Kumar Gala wrote: >>> For some devices, the default behavior of pgprot_noncached() is not >>> appropriate for all of its mappable regions. This provides a means for >>> the kernel side of the UIO driver to override the flags without having >>> to implement its own full mmap callback. >> >> Thanks for also providing an example driver showing the use of this. >> You should also post this driver in a mainline-ready version, I'm a bit >> uncomfortable with adding a new function pointer without having any users. > > I'm more than "uncomfortable", I'll refuse to take any such patch unless > there is a in-kernel user, otherwise it makes no sense to add the > pointer at all. > > thanks, > > greg k-h I'm in agreement with this view. I wanted to post this to make sure the direction we took was ok so when the upstream driver is posted this patch / change isn't a concern. - k
diff --git a/drivers/uio/uio.c b/drivers/uio/uio.c index dc27d89..0aebe27 100644 --- a/drivers/uio/uio.c +++ b/drivers/uio/uio.c @@ -655,7 +655,11 @@ static int uio_mmap_physical(struct vm_area_struct *vma) vma->vm_flags |= VM_IO | VM_RESERVED; - vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot); + if (idev->info->set_pgprot) + vma->vm_page_prot = idev->info->set_pgprot(idev->info, mi, + vma->vm_page_prot); + else + vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot); return remap_pfn_range(vma, vma->vm_start, diff --git a/include/linux/uio_driver.h b/include/linux/uio_driver.h index fd99ff9..edfe7c8 100644 --- a/include/linux/uio_driver.h +++ b/include/linux/uio_driver.h @@ -80,6 +80,7 @@ struct uio_device; * @open: open operation for this uio device * @release: release operation for this uio device * @irqcontrol: disable/enable irqs when 0/1 is written to /dev/uioX + * @set_pgprot: allow driver to override default(noncached) pgprot */ struct uio_info { struct uio_device *uio_dev; @@ -95,6 +96,8 @@ struct uio_info { int (*open)(struct uio_info *info, struct inode *inode); int (*release)(struct uio_info *info, struct inode *inode); int (*irqcontrol)(struct uio_info *info, s32 irq_on); + pgprot_t (*set_pgprot)(struct uio_info *uio, unsigned int mem_idx, + pgprot_t prot); }; extern int __must_check