Re: [Xen-devel] [PATCH v5 1/5] pci: add pci_iomap_wc() variants

["Luis R. Rodriguez" <mcgrof@xxxxxxxxxxxxxxxx>]

On Wed, May 20, 2015 at 1:39 PM, Bjorn Helgaas <bhelgaas@xxxxxxxxxx> 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@xxxxxxxxxx> wrote:
>> >
>> >> On Tue, May 19, 2015 at 4:02 PM, Bjorn Helgaas <bhelgaas@xxxxxxxxxx> 
>> >> wrote:
>> >> > [-cc Venkatesh (bouncing)
>> >> >
>> >> > On Tue, May 19, 2015 at 5:46 PM, Luis R. Rodriguez
>> >> > <mcgrof@xxxxxxxxxxxxxxxx> wrote:
>> >> >> On Tue, May 19, 2015 at 3:44 PM, Bjorn Helgaas <bhelgaas@xxxxxxxxxx>
>> >> >> wrote:
>> >> >>> Acked-by: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>
>> >> >>
>> >> >> 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

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel