| Message ID | 1430415364-19679-1-git-send-email-mcgrof@do-not-panic.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Apr 30, 2015 at 10:36:04AM -0700, Luis R. Rodriguez wrote: > From: "Luis R. Rodriguez" <mcgrof@suse.com> <-- snip --> > --- > > This v5 makes the code return NULL for IORESOURCE_IO and fixes the commit > log to clarify the conclusions reached for MTRR and our review of > IORESOURCE_PREFETCH. > > include/asm-generic/pci_iomap.h | 14 ++++++++++ > lib/pci_iomap.c | 61 +++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 75 insertions(+) Hey Bjorn, any feedack on this series? Thanks. Luis -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Apr 30, 2015 at 10:36:04AM -0700, Luis R. Rodriguez wrote: > From: "Luis R. Rodriguez" <mcgrof@suse.com> > > This allows drivers to take advantage of write-combining > when possible. The PCI specification does not allow for us > to automatically identify a memory region which needs > write-combining so drivers have to identify these areas > on their own. There is IORESOURCE_PREFETCH but as clarified > by Michael and confirmed later by Bjorn, PCI prefetch bit > merely means bridges can combine writes and prefetch reads. > Prefetch does not affect ordering rules and does not allow > writes to be collapsed [0]. WC is stronger, it allows collapsing > and changes ordering rules. WC can also hurt latency as small > writes are buffered. Because of all this drivers needs to > know what they are doing, we can't set a write-combining > preference flag in the pci core automatically for drivers. > > Lastly although there is also arch_phys_wc_add() this makes > use of architecture specific write-combining *hacks* and > the only one currently defined and used is MTRR for x86. > MTRRs are legacy, limited in number, have restrictive size > constraints, and are known to interact pooly with the BIOS. > MTRRs should only really be considered on old video framebuffer > drivers. If we made ioremap_wc() and similar calls start > automatically adding MTRRs, then performance will vary wildly > with the order of driver loading because we'll run out of MTRRs > part-way through bootup. > > There are a few motivations for phasing out of MTRR and > helping driver change over to use write-combining with PAT: > > a) Take advantage of PAT when available > > b) Help bury MTRR code away, MTRR is architecture specific and on > x86 its replaced by PAT > > c) Help with the goal of eventually using _PAGE_CACHE_UC over > _PAGE_CACHE_UC_MINUS on x86 on ioremap_nocache() (see commit > de33c442e titled "x86 PAT: fix performance drop for glx, > use UC minus for ioremap(), ioremap_nocache() and > pci_mmap_page_range()") > > [0] https://lkml.org/lkml/2015/4/21/714 > > Cc: Toshi Kani <toshi.kani@hp.com> > Cc: Andy Lutomirski <luto@amacapital.net> > Cc: Suresh Siddha <sbsiddha@gmail.com> > Cc: Ingo Molnar <mingo@elte.hu> > Cc: Thomas Gleixner <tglx@linutronix.de> > Cc: Juergen Gross <jgross@suse.com> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > Cc: Dave Airlie <airlied@redhat.com> > Cc: Bjorn Helgaas <bhelgaas@google.com> > Cc: Antonino Daplas <adaplas@gmail.com> > Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com> > Cc: Tomi Valkeinen <tomi.valkeinen@ti.com> > Cc: Dave Hansen <dave.hansen@linux.intel.com> > Cc: Arnd Bergmann <arnd@arndb.de> > Cc: Michael S. Tsirkin <mst@redhat.com> > Cc: venkatesh.pallipadi@intel.com > Cc: Stefan Bader <stefan.bader@canonical.com> > Cc: Ville Syrjälä <syrjala@sci.fi> > Cc: Mel Gorman <mgorman@suse.de> > Cc: Vlastimil Babka <vbabka@suse.cz> > Cc: Borislav Petkov <bp@suse.de> > Cc: Davidlohr Bueso <dbueso@suse.de> > Cc: konrad.wilk@oracle.com > Cc: ville.syrjala@linux.intel.com > Cc: david.vrabel@citrix.com > Cc: jbeulich@suse.com > Cc: Roger Pau Monné <roger.pau@citrix.com> > Cc: linux-fbdev@vger.kernel.org > Cc: linux-kernel@vger.kernel.org > Cc: xen-devel@lists.xensource.com > Cc: linux-pci@vger.kernel.org > Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com> Acked-by: Bjorn Helgaas <bhelgaas@google.com> > --- > > This v5 makes the code return NULL for IORESOURCE_IO and fixes the commit > log to clarify the conclusions reached for MTRR and our review of > IORESOURCE_PREFETCH. > > include/asm-generic/pci_iomap.h | 14 ++++++++++ > lib/pci_iomap.c | 61 +++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 75 insertions(+) > > diff --git a/include/asm-generic/pci_iomap.h b/include/asm-generic/pci_iomap.h > index 7389c87..b1e17fc 100644 > --- a/include/asm-generic/pci_iomap.h > +++ b/include/asm-generic/pci_iomap.h > @@ -15,9 +15,13 @@ struct pci_dev; > #ifdef CONFIG_PCI > /* Create a virtual mapping cookie for a PCI BAR (memory or IO) */ > extern void __iomem *pci_iomap(struct pci_dev *dev, int bar, unsigned long max); > +extern void __iomem *pci_iomap_wc(struct pci_dev *dev, int bar, unsigned long max); > extern void __iomem *pci_iomap_range(struct pci_dev *dev, int bar, > unsigned long offset, > unsigned long maxlen); > +extern void __iomem *pci_iomap_wc_range(struct pci_dev *dev, int bar, > + unsigned long offset, > + unsigned long maxlen); > /* Create a virtual mapping cookie for a port on a given PCI device. > * Do not call this directly, it exists to make it easier for architectures > * to override */ > @@ -34,12 +38,22 @@ static inline void __iomem *pci_iomap(struct pci_dev *dev, int bar, unsigned lon > return NULL; > } > > +static inline void __iomem *pci_iomap_wc(struct pci_dev *dev, int bar, unsigned long max) > +{ > + return NULL; > +} > static inline void __iomem *pci_iomap_range(struct pci_dev *dev, int bar, > unsigned long offset, > unsigned long maxlen) > { > return NULL; > } > +static inline void __iomem *pci_iomap_wc_range(struct pci_dev *dev, int bar, > + unsigned long offset, > + unsigned long maxlen) > +{ > + return NULL; > +} > #endif > > #endif /* __ASM_GENERIC_IO_H */ > diff --git a/lib/pci_iomap.c b/lib/pci_iomap.c > index bcce5f1..9604dcb 100644 > --- a/lib/pci_iomap.c > +++ b/lib/pci_iomap.c > @@ -52,6 +52,46 @@ void __iomem *pci_iomap_range(struct pci_dev *dev, > EXPORT_SYMBOL(pci_iomap_range); > > /** > + * pci_iomap_wc_range - create a virtual WC mapping cookie for a PCI BAR > + * @dev: PCI device that owns the BAR > + * @bar: BAR number > + * @offset: map memory at the given offset in BAR > + * @maxlen: max length of the memory to map > + * > + * Using this function you will get a __iomem address to your device BAR. > + * You can access it using ioread*() and iowrite*(). These functions hide > + * the details if this is a MMIO or PIO address space and will just do what > + * you expect from them in the correct way. When possible write combining > + * is used. > + * > + * @maxlen specifies the maximum length to map. If you want to get access to > + * the complete BAR from offset to the end, pass %0 here. > + * */ > +void __iomem *pci_iomap_wc_range(struct pci_dev *dev, > + int bar, > + unsigned long offset, > + unsigned long maxlen) > +{ > + resource_size_t start = pci_resource_start(dev, bar); > + resource_size_t len = pci_resource_len(dev, bar); > + unsigned long flags = pci_resource_flags(dev, bar); > + > + if (len <= offset || !start) > + return NULL; > + len -= offset; > + start += offset; > + if (maxlen && len > maxlen) > + len = maxlen; > + if (flags & IORESOURCE_IO) > + return NULL; > + if (flags & IORESOURCE_MEM) > + return ioremap_wc(start, len); > + /* What? */ > + return NULL; > +} > +EXPORT_SYMBOL_GPL(pci_iomap_wc_range); > + > +/** > * pci_iomap - create a virtual mapping cookie for a PCI BAR > * @dev: PCI device that owns the BAR > * @bar: BAR number > @@ -70,4 +110,25 @@ void __iomem *pci_iomap(struct pci_dev *dev, int bar, unsigned long maxlen) > return pci_iomap_range(dev, bar, 0, maxlen); > } > EXPORT_SYMBOL(pci_iomap); > + > +/** > + * pci_iomap_wc - create a virtual WC mapping cookie for a PCI BAR > + * @dev: PCI device that owns the BAR > + * @bar: BAR number > + * @maxlen: length of the memory to map > + * > + * Using this function you will get a __iomem address to your device BAR. > + * You can access it using ioread*() and iowrite*(). These functions hide > + * the details if this is a MMIO or PIO address space and will just do what > + * you expect from them in the correct way. When possible write combining > + * is used. > + * > + * @maxlen specifies the maximum length to map. If you want to get access to > + * the complete BAR without checking for its length first, pass %0 here. > + * */ > +void __iomem *pci_iomap_wc(struct pci_dev *dev, int bar, unsigned long maxlen) > +{ > + return pci_iomap_wc_range(dev, bar, 0, maxlen); > +} > +EXPORT_SYMBOL_GPL(pci_iomap_wc); > #endif /* CONFIG_PCI */ > -- > 2.3.2.209.gd67f9d5.dirty > -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, May 19, 2015 at 3:44 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> Acked-by: Bjorn Helgaas <bhelgaas@google.com>
Thanks! Who's tree should this go through?
Luis
--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[-cc Venkatesh (bouncing) On Tue, May 19, 2015 at 5:46 PM, Luis R. Rodriguez <mcgrof@do-not-panic.com> wrote: > On Tue, May 19, 2015 at 3:44 PM, Bjorn Helgaas <bhelgaas@google.com> wrote: >> Acked-by: Bjorn Helgaas <bhelgaas@google.com> > > Thanks! Who's tree should this go through? I don't know. This is the only patch that went to linux-pci, so I haven't seen the rest. Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, May 19, 2015 at 4:02 PM, Bjorn Helgaas <bhelgaas@google.com> wrote: > [-cc Venkatesh (bouncing) > > On Tue, May 19, 2015 at 5:46 PM, Luis R. Rodriguez > <mcgrof@do-not-panic.com> wrote: >> On Tue, May 19, 2015 at 3:44 PM, Bjorn Helgaas <bhelgaas@google.com> wrote: >>> Acked-by: Bjorn Helgaas <bhelgaas@google.com> >> >> Thanks! Who's tree should this go through? > > I don't know. This is the only patch that went to linux-pci, so I > haven't seen the rest. Oh I only rev'd a v5 for 1/5 as that's the only one that had feedback asking for changes. Patch v4 2/5 was for "lib: devres: add pcim_iomap_wc() variants", you had questions about EXPORT_SYMBOL_GPL() and the fact that this is not yet used. I replied. This patch can then be ignored but again, I'd hate for folks to go in and try to add a non EXPORT_SYMBOL_GPL() symbol of this. Patches v4 3-5 remain intact, I had addressed it to you, but failed to Cc linux-pci, I'll go ahead and bounce those now. Just today Dave Arlie provided a Reviewed-by to some simple framebuffer device driver changes. I wonder if these changes should go through the framebuffer tree provided you already gave the Acked-by for the PCI parts, or if the PCI parts should go in first and only later (I guess we'd have to wait) then intake the driver changes that use the symbol. What we decide should likely also apply to the series that adds pci_ioremap_wc_bar() and makes use of it on drivers. Dave, Tomi, any preference? Luis -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> On Tue, May 19, 2015 at 4:02 PM, Bjorn Helgaas <bhelgaas@google.com> wrote: > > [-cc Venkatesh (bouncing) > > > > On Tue, May 19, 2015 at 5:46 PM, Luis R. Rodriguez > > <mcgrof@do-not-panic.com> wrote: > >> On Tue, May 19, 2015 at 3:44 PM, Bjorn Helgaas <bhelgaas@google.com> > >> wrote: > >>> Acked-by: Bjorn Helgaas <bhelgaas@google.com> > >> > >> Thanks! Who's tree should this go through? > > > > I don't know. This is the only patch that went to linux-pci, so I > > haven't seen the rest. > > Oh I only rev'd a v5 for 1/5 as that's the only one that had feedback > asking for changes. > > Patch v4 2/5 was for "lib: devres: add pcim_iomap_wc() variants", you > had questions about EXPORT_SYMBOL_GPL() and the fact that this is not > yet used. I replied. This patch can then be ignored but again, I'd > hate for folks to go in and try to add a non EXPORT_SYMBOL_GPL() > symbol of this. > > Patches v4 3-5 remain intact, I had addressed it to you, but failed to > Cc linux-pci, I'll go ahead and bounce those now. > > Just today Dave Arlie provided a Reviewed-by to some simple > framebuffer device driver changes. I wonder if these changes should go > through the framebuffer tree provided you already gave the Acked-by > for the PCI parts, or if the PCI parts should go in first and only > later (I guess we'd have to wait) then intake the driver changes that > use the symbol. > > What we decide should likely also apply to the series that adds > pci_ioremap_wc_bar() and makes use of it on drivers. > > Dave, Tomi, any preference? > Maybe send Bjorn a pull request with a tree with the pci changes, and the fb changes reviewed-by me and acked by Tomi. Seems like it could be the simplest path forward. Dave. -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, May 19, 2015 at 4:29 PM, David Airlie <airlied@redhat.com> wrote: > >> On Tue, May 19, 2015 at 4:02 PM, Bjorn Helgaas <bhelgaas@google.com> wrote: >> > [-cc Venkatesh (bouncing) >> > >> > On Tue, May 19, 2015 at 5:46 PM, Luis R. Rodriguez >> > <mcgrof@do-not-panic.com> wrote: >> >> On Tue, May 19, 2015 at 3:44 PM, Bjorn Helgaas <bhelgaas@google.com> >> >> wrote: >> >>> Acked-by: Bjorn Helgaas <bhelgaas@google.com> >> >> >> >> Thanks! Who's tree should this go through? >> > >> > I don't know. This is the only patch that went to linux-pci, so I >> > haven't seen the rest. >> >> Oh I only rev'd a v5 for 1/5 as that's the only one that had feedback >> asking for changes. >> >> Patch v4 2/5 was for "lib: devres: add pcim_iomap_wc() variants", you >> had questions about EXPORT_SYMBOL_GPL() and the fact that this is not >> yet used. I replied. This patch can then be ignored but again, I'd >> hate for folks to go in and try to add a non EXPORT_SYMBOL_GPL() >> symbol of this. >> >> Patches v4 3-5 remain intact, I had addressed it to you, but failed to >> Cc linux-pci, I'll go ahead and bounce those now. >> >> Just today Dave Arlie provided a Reviewed-by to some simple >> framebuffer device driver changes. I wonder if these changes should go >> through the framebuffer tree provided you already gave the Acked-by >> for the PCI parts, or if the PCI parts should go in first and only >> later (I guess we'd have to wait) then intake the driver changes that >> use the symbol. >> >> What we decide should likely also apply to the series that adds >> pci_ioremap_wc_bar() and makes use of it on drivers. >> >> Dave, Tomi, any preference? >> > > Maybe send Bjorn a pull request with a tree with the pci changes, and the fb changes reviewed-by me and acked by Tomi. > > Seems like it could be the simplest path forward. Works with me, Bjorn, are you OK with that? Dave, Tomi, I still need your Reviewed-by and Acked-by's Tomi on these two PCI series. Please let me know if I should resend those first, but I think I had Cc'd you folks already on them. Luis -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 20/05/15 02:45, Luis R. Rodriguez wrote: > On Tue, May 19, 2015 at 4:29 PM, David Airlie <airlied@redhat.com> wrote: >> >>> On Tue, May 19, 2015 at 4:02 PM, Bjorn Helgaas <bhelgaas@google.com> wrote: >>>> [-cc Venkatesh (bouncing) >>>> >>>> On Tue, May 19, 2015 at 5:46 PM, Luis R. Rodriguez >>>> <mcgrof@do-not-panic.com> wrote: >>>>> On Tue, May 19, 2015 at 3:44 PM, Bjorn Helgaas <bhelgaas@google.com> >>>>> wrote: >>>>>> Acked-by: Bjorn Helgaas <bhelgaas@google.com> >>>>> >>>>> Thanks! Who's tree should this go through? >>>> >>>> I don't know. This is the only patch that went to linux-pci, so I >>>> haven't seen the rest. >>> >>> Oh I only rev'd a v5 for 1/5 as that's the only one that had feedback >>> asking for changes. >>> >>> Patch v4 2/5 was for "lib: devres: add pcim_iomap_wc() variants", you >>> had questions about EXPORT_SYMBOL_GPL() and the fact that this is not >>> yet used. I replied. This patch can then be ignored but again, I'd >>> hate for folks to go in and try to add a non EXPORT_SYMBOL_GPL() >>> symbol of this. >>> >>> Patches v4 3-5 remain intact, I had addressed it to you, but failed to >>> Cc linux-pci, I'll go ahead and bounce those now. >>> >>> Just today Dave Arlie provided a Reviewed-by to some simple >>> framebuffer device driver changes. I wonder if these changes should go >>> through the framebuffer tree provided you already gave the Acked-by >>> for the PCI parts, or if the PCI parts should go in first and only >>> later (I guess we'd have to wait) then intake the driver changes that >>> use the symbol. >>> >>> What we decide should likely also apply to the series that adds >>> pci_ioremap_wc_bar() and makes use of it on drivers. >>> >>> Dave, Tomi, any preference? >>> >> >> Maybe send Bjorn a pull request with a tree with the pci changes, and the fb changes reviewed-by me and acked by Tomi. >> >> Seems like it could be the simplest path forward. > > Works with me, Bjorn, are you OK with that? > > Dave, Tomi, I still need your Reviewed-by and Acked-by's Tomi on these > two PCI series. Please let me know if I should resend those first, but > I think I had Cc'd you folks already on them. For the fb parts: Acked-by: Tomi Valkeinen <tomi.valkeinen@ti.com> I don't have anything in my fbdev-next branch touching the fbdev drivers changed in this series, so merging via some other tree might go fine without conflicts. Tomi
On Tue, May 19, 2015 at 04:45:30PM -0700, Luis R. Rodriguez wrote: > On Tue, May 19, 2015 at 4:29 PM, David Airlie <airlied@redhat.com> wrote: > > > >> On Tue, May 19, 2015 at 4:02 PM, Bjorn Helgaas <bhelgaas@google.com> wrote: > >> > [-cc Venkatesh (bouncing) > >> > > >> > On Tue, May 19, 2015 at 5:46 PM, Luis R. Rodriguez > >> > <mcgrof@do-not-panic.com> wrote: > >> >> On Tue, May 19, 2015 at 3:44 PM, Bjorn Helgaas <bhelgaas@google.com> > >> >> wrote: > >> >>> Acked-by: Bjorn Helgaas <bhelgaas@google.com> > >> >> > >> >> Thanks! Who's tree should this go through? > >> > > >> > I don't know. This is the only patch that went to linux-pci, so I > >> > haven't seen the rest. > >> > >> Oh I only rev'd a v5 for 1/5 as that's the only one that had feedback > >> asking for changes. > >> > >> Patch v4 2/5 was for "lib: devres: add pcim_iomap_wc() variants", you > >> had questions about EXPORT_SYMBOL_GPL() and the fact that this is not > >> yet used. I replied. This patch can then be ignored but again, I'd > >> hate for folks to go in and try to add a non EXPORT_SYMBOL_GPL() > >> symbol of this. I'm not really a fan of adding code before it's needed. But even when we have a user and that objection is resolved, I'd like to have a little more guidance on the difference between EXPORT_SYMBOL() and EXPORT_SYMBOL_GPL(), e.g., a patch to clarify what's in Documentation/DocBook/kernel-hacking.tmpl. I can't evaluate that issue based on "current trends and reality"; I need something a little more formal. If we want to allow non-GPL modules and we want to give them a consistent kernel interface, even though it might be lacking some implementation-specific things, I can follow that guideline. That's how I read the current kernel-hacking.tmpl text ("[EXPORT_SYMBOL_GPL()] implies that the function is considered an internal implementation issue, and not really an interface"), and that would lead me to suggest EXPORT_SYMBOL() in this case. If we don't care about consistency, and EXPORT_SYMBOL_GPL() should be used at the developer's preference, I can follow that guideline instead, but it would be easier if it were made more explicit. > >> Patches v4 3-5 remain intact, I had addressed it to you, but failed to > >> Cc linux-pci, I'll go ahead and bounce those now. > >> > >> Just today Dave Arlie provided a Reviewed-by to some simple > >> framebuffer device driver changes. I wonder if these changes should go > >> through the framebuffer tree provided you already gave the Acked-by > >> for the PCI parts, or if the PCI parts should go in first and only > >> later (I guess we'd have to wait) then intake the driver changes that > >> use the symbol. > >> > >> What we decide should likely also apply to the series that adds > >> pci_ioremap_wc_bar() and makes use of it on drivers. > >> > >> Dave, Tomi, any preference? > >> > > > > Maybe send Bjorn a pull request with a tree with the pci changes, and the fb changes reviewed-by me and acked by Tomi. > > > > Seems like it could be the simplest path forward. > > Works with me, Bjorn, are you OK with that? Can you incorporate the acks and just post a complete v6? I don't like trying to assemble patches from different versions of a series; it's too much administrative hassle and too easy for me to screw up. Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, May 20, 2015 at 1:39 PM, Bjorn Helgaas <bhelgaas@google.com> wrote: > On Tue, May 19, 2015 at 04:45:30PM -0700, Luis R. Rodriguez wrote: >> On Tue, May 19, 2015 at 4:29 PM, David Airlie <airlied@redhat.com> wrote: >> > >> >> On Tue, May 19, 2015 at 4:02 PM, Bjorn Helgaas <bhelgaas@google.com> wrote: >> >> > [-cc Venkatesh (bouncing) >> >> > >> >> > On Tue, May 19, 2015 at 5:46 PM, Luis R. Rodriguez >> >> > <mcgrof@do-not-panic.com> wrote: >> >> >> On Tue, May 19, 2015 at 3:44 PM, Bjorn Helgaas <bhelgaas@google.com> >> >> >> wrote: >> >> >>> Acked-by: Bjorn Helgaas <bhelgaas@google.com> >> >> >> >> >> >> Thanks! Who's tree should this go through? >> >> > >> >> > I don't know. This is the only patch that went to linux-pci, so I >> >> > haven't seen the rest. >> >> >> >> Oh I only rev'd a v5 for 1/5 as that's the only one that had feedback >> >> asking for changes. >> >> >> >> Patch v4 2/5 was for "lib: devres: add pcim_iomap_wc() variants", you >> >> had questions about EXPORT_SYMBOL_GPL() and the fact that this is not >> >> yet used. I replied. This patch can then be ignored but again, I'd >> >> hate for folks to go in and try to add a non EXPORT_SYMBOL_GPL() >> >> symbol of this. > > I'm not really a fan of adding code before it's needed. OK I'll skip this patch. > But even when we have a user and that objection is resolved, I'd > like to have a little more guidance on the difference between > EXPORT_SYMBOL() and EXPORT_SYMBOL_GPL(), e.g., a patch to clarify > what's in Documentation/DocBook/kernel-hacking.tmpl. I can't > evaluate that issue based on "current trends and reality"; I need > something a little more formal. > > If we want to allow non-GPL modules and we want to give them a > consistent kernel interface, even though it might be lacking some > implementation-specific things, I can follow that guideline. > > That's how I read the current kernel-hacking.tmpl text > ("[EXPORT_SYMBOL_GPL()] implies that the function is considered > an internal implementation issue, and not really an interface"), > and that would lead me to suggest EXPORT_SYMBOL() in this case. > > If we don't care about consistency, and EXPORT_SYMBOL_GPL() > should be used at the developer's preference, I can follow that > guideline instead, but it would be easier if it were made more > explicit. I think that's fair for a maintainer to ask, provided that some maintainers may not be aware of some history, I'll send a patch to update the documentation to reflect reality. Note that this is not just about allowing or not proprietary drivers, some folks also take code from the kernel and use it or sell it on proprietary systems. The rabbit hole is pretty big. >> >> Patches v4 3-5 remain intact, I had addressed it to you, but failed to >> >> Cc linux-pci, I'll go ahead and bounce those now. >> >> >> >> Just today Dave Arlie provided a Reviewed-by to some simple >> >> framebuffer device driver changes. I wonder if these changes should go >> >> through the framebuffer tree provided you already gave the Acked-by >> >> for the PCI parts, or if the PCI parts should go in first and only >> >> later (I guess we'd have to wait) then intake the driver changes that >> >> use the symbol. >> >> >> >> What we decide should likely also apply to the series that adds >> >> pci_ioremap_wc_bar() and makes use of it on drivers. >> >> >> >> Dave, Tomi, any preference? >> >> >> > >> > Maybe send Bjorn a pull request with a tree with the pci changes, and the fb changes reviewed-by me and acked by Tomi. >> > >> > Seems like it could be the simplest path forward. >> >> Works with me, Bjorn, are you OK with that? > > Can you incorporate the acks and just post a complete v6? I don't like > trying to assemble patches from different versions of a series; it's too > much administrative hassle and too easy for me to screw up. Yeah sure, will do. While at it, can you also review the patch that adds pci_ioremap_wc_bar() as well? I'll bundle that series up too. Luis -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/include/asm-generic/pci_iomap.h b/include/asm-generic/pci_iomap.h index 7389c87..b1e17fc 100644 --- a/include/asm-generic/pci_iomap.h +++ b/include/asm-generic/pci_iomap.h @@ -15,9 +15,13 @@ struct pci_dev; #ifdef CONFIG_PCI /* Create a virtual mapping cookie for a PCI BAR (memory or IO) */ extern void __iomem *pci_iomap(struct pci_dev *dev, int bar, unsigned long max); +extern void __iomem *pci_iomap_wc(struct pci_dev *dev, int bar, unsigned long max); extern void __iomem *pci_iomap_range(struct pci_dev *dev, int bar, unsigned long offset, unsigned long maxlen); +extern void __iomem *pci_iomap_wc_range(struct pci_dev *dev, int bar, + unsigned long offset, + unsigned long maxlen); /* Create a virtual mapping cookie for a port on a given PCI device. * Do not call this directly, it exists to make it easier for architectures * to override */ @@ -34,12 +38,22 @@ static inline void __iomem *pci_iomap(struct pci_dev *dev, int bar, unsigned lon return NULL; } +static inline void __iomem *pci_iomap_wc(struct pci_dev *dev, int bar, unsigned long max) +{ + return NULL; +} static inline void __iomem *pci_iomap_range(struct pci_dev *dev, int bar, unsigned long offset, unsigned long maxlen) { return NULL; } +static inline void __iomem *pci_iomap_wc_range(struct pci_dev *dev, int bar, + unsigned long offset, + unsigned long maxlen) +{ + return NULL; +} #endif #endif /* __ASM_GENERIC_IO_H */ diff --git a/lib/pci_iomap.c b/lib/pci_iomap.c index bcce5f1..9604dcb 100644 --- a/lib/pci_iomap.c +++ b/lib/pci_iomap.c @@ -52,6 +52,46 @@ void __iomem *pci_iomap_range(struct pci_dev *dev, EXPORT_SYMBOL(pci_iomap_range); /** + * pci_iomap_wc_range - create a virtual WC mapping cookie for a PCI BAR + * @dev: PCI device that owns the BAR + * @bar: BAR number + * @offset: map memory at the given offset in BAR + * @maxlen: max length of the memory to map + * + * Using this function you will get a __iomem address to your device BAR. + * You can access it using ioread*() and iowrite*(). These functions hide + * the details if this is a MMIO or PIO address space and will just do what + * you expect from them in the correct way. When possible write combining + * is used. + * + * @maxlen specifies the maximum length to map. If you want to get access to + * the complete BAR from offset to the end, pass %0 here. + * */ +void __iomem *pci_iomap_wc_range(struct pci_dev *dev, + int bar, + unsigned long offset, + unsigned long maxlen) +{ + resource_size_t start = pci_resource_start(dev, bar); + resource_size_t len = pci_resource_len(dev, bar); + unsigned long flags = pci_resource_flags(dev, bar); + + if (len <= offset || !start) + return NULL; + len -= offset; + start += offset; + if (maxlen && len > maxlen) + len = maxlen; + if (flags & IORESOURCE_IO) + return NULL; + if (flags & IORESOURCE_MEM) + return ioremap_wc(start, len); + /* What? */ + return NULL; +} +EXPORT_SYMBOL_GPL(pci_iomap_wc_range); + +/** * pci_iomap - create a virtual mapping cookie for a PCI BAR * @dev: PCI device that owns the BAR * @bar: BAR number @@ -70,4 +110,25 @@ void __iomem *pci_iomap(struct pci_dev *dev, int bar, unsigned long maxlen) return pci_iomap_range(dev, bar, 0, maxlen); } EXPORT_SYMBOL(pci_iomap); + +/** + * pci_iomap_wc - create a virtual WC mapping cookie for a PCI BAR + * @dev: PCI device that owns the BAR + * @bar: BAR number + * @maxlen: length of the memory to map + * + * Using this function you will get a __iomem address to your device BAR. + * You can access it using ioread*() and iowrite*(). These functions hide + * the details if this is a MMIO or PIO address space and will just do what + * you expect from them in the correct way. When possible write combining + * is used. + * + * @maxlen specifies the maximum length to map. If you want to get access to + * the complete BAR without checking for its length first, pass %0 here. + * */ +void __iomem *pci_iomap_wc(struct pci_dev *dev, int bar, unsigned long maxlen) +{ + return pci_iomap_wc_range(dev, bar, 0, maxlen); +} +EXPORT_SYMBOL_GPL(pci_iomap_wc); #endif /* CONFIG_PCI */