diff mbox

[v8,2/9] pci: Export find_pci_host_bridge() function.

Message ID 1404240214-9804-3-git-send-email-Liviu.Dudau@arm.com (mailing list archive)
State New, archived
Delegated to: Bjorn Helgaas
Headers show

Commit Message

Liviu Dudau July 1, 2014, 6:43 p.m. UTC 
This is a useful function and we should make it visible outside the
generic PCI code. Export it as a GPL symbol.

Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com>
Tested-by: Tanmay Inamdar <tinamdar@apm.com>
---
 drivers/pci/host-bridge.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Tanmay Inamdar July 2, 2014, 6:06 p.m. UTC | #1 
Hi,

On Tue, Jul 1, 2014 at 11:43 AM, Liviu Dudau <Liviu.Dudau@arm.com> wrote:
> This is a useful function and we should make it visible outside the
> generic PCI code. Export it as a GPL symbol.
>
> Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com>
> Tested-by: Tanmay Inamdar <tinamdar@apm.com>
> ---
>  drivers/pci/host-bridge.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pci/host-bridge.c b/drivers/pci/host-bridge.c
> index 0e5f3c9..36c669e 100644
> --- a/drivers/pci/host-bridge.c
> +++ b/drivers/pci/host-bridge.c
> @@ -16,12 +16,13 @@ static struct pci_bus *find_pci_root_bus(struct pci_bus *bus)
>         return bus;
>  }
>
> -static struct pci_host_bridge *find_pci_host_bridge(struct pci_bus *bus)
> +struct pci_host_bridge *find_pci_host_bridge(struct pci_bus *bus)
>  {
>         struct pci_bus *root_bus = find_pci_root_bus(bus);
>
>         return to_pci_host_bridge(root_bus->bridge);
>  }
> +EXPORT_SYMBOL_GPL(find_pci_host_bridge);

Is there any specific reason behind making this symbol GPL? I think
the other functions in this file are just EXPORT_SYMBOL. Ultimately
companies which have non gpl Linux modules (nvidia) will face issue
using this API.

The same applies to 'of_create_pci_host_bridge'.
>
>  void pci_set_host_bridge_release(struct pci_host_bridge *bridge,
>                                  void (*release_fn)(struct pci_host_bridge *),
> --
> 2.0.0
>
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Arnd Bergmann July 2, 2014, 7:12 p.m. UTC | #2 
On Wednesday 02 July 2014 11:06:38 Tanmay Inamdar wrote:
> On Tue, Jul 1, 2014 at 11:43 AM, Liviu Dudau <Liviu.Dudau@arm.com> wrote:
> > This is a useful function and we should make it visible outside the
> > generic PCI code. Export it as a GPL symbol.
> >
> > Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com>
> > Tested-by: Tanmay Inamdar <tinamdar@apm.com>
> > ---
> >  drivers/pci/host-bridge.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/pci/host-bridge.c b/drivers/pci/host-bridge.c
> > index 0e5f3c9..36c669e 100644
> > --- a/drivers/pci/host-bridge.c
> > +++ b/drivers/pci/host-bridge.c
> > @@ -16,12 +16,13 @@ static struct pci_bus *find_pci_root_bus(struct pci_bus *bus)
> >         return bus;
> >  }
> >
> > -static struct pci_host_bridge *find_pci_host_bridge(struct pci_bus *bus)
> > +struct pci_host_bridge *find_pci_host_bridge(struct pci_bus *bus)
> >  {
> >         struct pci_bus *root_bus = find_pci_root_bus(bus);
> >
> >         return to_pci_host_bridge(root_bus->bridge);
> >  }
> > +EXPORT_SYMBOL_GPL(find_pci_host_bridge);
> 
> Is there any specific reason behind making this symbol GPL? I think
> the other functions in this file are just EXPORT_SYMBOL. Ultimately
> companies which have non gpl Linux modules (nvidia) will face issue
> using this API.
> 
> The same applies to 'of_create_pci_host_bridge'.

I think EXPORT_SYMBOL_GPL() is better here. The new symbols are unlikely
to be used by a peripheral device driver, and PCI host controllers are
already restricted by EXPORT_SYMBOL_GPL.

nvidia will certainly not do a PCI host controller driver that is not
upstream or not GPL-compatible.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tanmay Inamdar July 2, 2014, 8:43 p.m. UTC | #3 
On Wed, Jul 2, 2014 at 12:12 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Wednesday 02 July 2014 11:06:38 Tanmay Inamdar wrote:
>> On Tue, Jul 1, 2014 at 11:43 AM, Liviu Dudau <Liviu.Dudau@arm.com> wrote:
>> > This is a useful function and we should make it visible outside the
>> > generic PCI code. Export it as a GPL symbol.
>> >
>> > Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com>
>> > Tested-by: Tanmay Inamdar <tinamdar@apm.com>
>> > ---
>> >  drivers/pci/host-bridge.c | 3 ++-
>> >  1 file changed, 2 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/drivers/pci/host-bridge.c b/drivers/pci/host-bridge.c
>> > index 0e5f3c9..36c669e 100644
>> > --- a/drivers/pci/host-bridge.c
>> > +++ b/drivers/pci/host-bridge.c
>> > @@ -16,12 +16,13 @@ static struct pci_bus *find_pci_root_bus(struct pci_bus *bus)
>> >         return bus;
>> >  }
>> >
>> > -static struct pci_host_bridge *find_pci_host_bridge(struct pci_bus *bus)
>> > +struct pci_host_bridge *find_pci_host_bridge(struct pci_bus *bus)
>> >  {
>> >         struct pci_bus *root_bus = find_pci_root_bus(bus);
>> >
>> >         return to_pci_host_bridge(root_bus->bridge);
>> >  }
>> > +EXPORT_SYMBOL_GPL(find_pci_host_bridge);
>>
>> Is there any specific reason behind making this symbol GPL? I think
>> the other functions in this file are just EXPORT_SYMBOL. Ultimately
>> companies which have non gpl Linux modules (nvidia) will face issue
>> using this API.
>>
>> The same applies to 'of_create_pci_host_bridge'.
>
> I think EXPORT_SYMBOL_GPL() is better here. The new symbols are unlikely
> to be used by a peripheral device driver, and PCI host controllers are
> already restricted by EXPORT_SYMBOL_GPL.
>

You are right as long as the functions are not used directly. But what
if GPL functions are called indirectly.  For example, 'pci_domain_nr'
implementation in Liviu's V7 series calls 'find_pci_host_bridge'.

> nvidia will certainly not do a PCI host controller driver that is not
> upstream or not GPL-compatible.
>
>         Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Liviu Dudau July 3, 2014, 9:53 a.m. UTC | #4 
On Wed, Jul 02, 2014 at 09:43:41PM +0100, Tanmay Inamdar wrote:
> On Wed, Jul 2, 2014 at 12:12 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Wednesday 02 July 2014 11:06:38 Tanmay Inamdar wrote:
> >> On Tue, Jul 1, 2014 at 11:43 AM, Liviu Dudau <Liviu.Dudau@arm.com> wrote:
> >> > This is a useful function and we should make it visible outside the
> >> > generic PCI code. Export it as a GPL symbol.
> >> >
> >> > Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com>
> >> > Tested-by: Tanmay Inamdar <tinamdar@apm.com>
> >> > ---
> >> >  drivers/pci/host-bridge.c | 3 ++-
> >> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >> >
> >> > diff --git a/drivers/pci/host-bridge.c b/drivers/pci/host-bridge.c
> >> > index 0e5f3c9..36c669e 100644
> >> > --- a/drivers/pci/host-bridge.c
> >> > +++ b/drivers/pci/host-bridge.c
> >> > @@ -16,12 +16,13 @@ static struct pci_bus *find_pci_root_bus(struct pci_bus *bus)
> >> >         return bus;
> >> >  }
> >> >
> >> > -static struct pci_host_bridge *find_pci_host_bridge(struct pci_bus *bus)
> >> > +struct pci_host_bridge *find_pci_host_bridge(struct pci_bus *bus)
> >> >  {
> >> >         struct pci_bus *root_bus = find_pci_root_bus(bus);
> >> >
> >> >         return to_pci_host_bridge(root_bus->bridge);
> >> >  }
> >> > +EXPORT_SYMBOL_GPL(find_pci_host_bridge);
> >>
> >> Is there any specific reason behind making this symbol GPL? I think
> >> the other functions in this file are just EXPORT_SYMBOL. Ultimately
> >> companies which have non gpl Linux modules (nvidia) will face issue
> >> using this API.
> >>
> >> The same applies to 'of_create_pci_host_bridge'.
> >
> > I think EXPORT_SYMBOL_GPL() is better here. The new symbols are unlikely
> > to be used by a peripheral device driver, and PCI host controllers are
> > already restricted by EXPORT_SYMBOL_GPL.
> >
> 
> You are right as long as the functions are not used directly. But what
> if GPL functions are called indirectly.  For example, 'pci_domain_nr'
> implementation in Liviu's V7 series calls 'find_pci_host_bridge'.

I will not be drawn into the discussion of EXPORT_SYMBOL vs EXPORT_SYMBOL_GPL()
other than to say that I don't understand what is so secret in implementing
a standard. I do not want to support host bridge drivers that are not open
source.

Best regards,
Liviu

> 
> > nvidia will certainly not do a PCI host controller driver that is not
> > upstream or not GPL-compatible.
> >
> >         Arnd
>
Arnd Bergmann July 3, 2014, 10:26 a.m. UTC | #5 
On Wednesday 02 July 2014 13:43:41 Tanmay Inamdar wrote:
> On Wed, Jul 2, 2014 at 12:12 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> > I think EXPORT_SYMBOL_GPL() is better here. The new symbols are unlikely
> > to be used by a peripheral device driver, and PCI host controllers are
> > already restricted by EXPORT_SYMBOL_GPL.
> >
> 
> You are right as long as the functions are not used directly. But what
> if GPL functions are called indirectly.  For example, 'pci_domain_nr'
> implementation in Liviu's V7 series calls 'find_pci_host_bridge'.

Good point. If pci_domain_nr() doesn't require access to an EXPORT_SYMBOL_GPL
symbol, it should not start doing that after this patch.

For of_create_pci_host_bridge() however, I can't think of any reason to use
a legacy EXPORT_SYMBOL.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Bjorn Helgaas July 7, 2014, 11:27 p.m. UTC | #6 
On Tue, Jul 01, 2014 at 07:43:27PM +0100, Liviu Dudau wrote:
> This is a useful function and we should make it visible outside the
> generic PCI code. Export it as a GPL symbol.
> 
> Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com>
> Tested-by: Tanmay Inamdar <tinamdar@apm.com>
> ---
>  drivers/pci/host-bridge.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/host-bridge.c b/drivers/pci/host-bridge.c
> index 0e5f3c9..36c669e 100644
> --- a/drivers/pci/host-bridge.c
> +++ b/drivers/pci/host-bridge.c
> @@ -16,12 +16,13 @@ static struct pci_bus *find_pci_root_bus(struct pci_bus *bus)
>  	return bus;
>  }
>  
> -static struct pci_host_bridge *find_pci_host_bridge(struct pci_bus *bus)
> +struct pci_host_bridge *find_pci_host_bridge(struct pci_bus *bus)
>  {
>  	struct pci_bus *root_bus = find_pci_root_bus(bus);
>  
>  	return to_pci_host_bridge(root_bus->bridge);
>  }
> +EXPORT_SYMBOL_GPL(find_pci_host_bridge);

There's nothing in this series that uses find_pci_host_bridge(), so 
how about we just wait until we have something that needs it?

Also, if/when we export this, I'd prefer a name that starts with "pci_"
as most of the other interfaces do.

>  void pci_set_host_bridge_release(struct pci_host_bridge *bridge,
>  				 void (*release_fn)(struct pci_host_bridge *),
> -- 
> 2.0.0
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Liviu Dudau July 8, 2014, 10:42 a.m. UTC | #7 
On Tue, Jul 08, 2014 at 12:27:49AM +0100, Bjorn Helgaas wrote:
> On Tue, Jul 01, 2014 at 07:43:27PM +0100, Liviu Dudau wrote:
> > This is a useful function and we should make it visible outside the
> > generic PCI code. Export it as a GPL symbol.
> > 
> > Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com>
> > Tested-by: Tanmay Inamdar <tinamdar@apm.com>
> > ---
> >  drivers/pci/host-bridge.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/pci/host-bridge.c b/drivers/pci/host-bridge.c
> > index 0e5f3c9..36c669e 100644
> > --- a/drivers/pci/host-bridge.c
> > +++ b/drivers/pci/host-bridge.c
> > @@ -16,12 +16,13 @@ static struct pci_bus *find_pci_root_bus(struct pci_bus *bus)
> >  	return bus;
> >  }
> >  
> > -static struct pci_host_bridge *find_pci_host_bridge(struct pci_bus *bus)
> > +struct pci_host_bridge *find_pci_host_bridge(struct pci_bus *bus)
> >  {
> >  	struct pci_bus *root_bus = find_pci_root_bus(bus);
> >  
> >  	return to_pci_host_bridge(root_bus->bridge);
> >  }
> > +EXPORT_SYMBOL_GPL(find_pci_host_bridge);
> 
> There's nothing in this series that uses find_pci_host_bridge(), so 
> how about we just wait until we have something that needs it?
> 
> Also, if/when we export this, I'd prefer a name that starts with "pci_"
> as most of the other interfaces do.

Understood.

I was using the function in the other patch series that adds PCIe support to arm64
in order to provide a pci_domain_nr() implementation. But I might not need it
in the end if I go ahead with separating pci_host_bridge creation from bus
creation.

Best regards,
Liviu

> 
> >  void pci_set_host_bridge_release(struct pci_host_bridge *bridge,
> >  				 void (*release_fn)(struct pci_host_bridge *),
> > -- 
> > 2.0.0
> > 
>
diff mbox

Patch

diff --git a/drivers/pci/host-bridge.c b/drivers/pci/host-bridge.c
index 0e5f3c9..36c669e 100644
--- a/drivers/pci/host-bridge.c
+++ b/drivers/pci/host-bridge.c
@@ -16,12 +16,13 @@  static struct pci_bus *find_pci_root_bus(struct pci_bus *bus)
 	return bus;
 }
 
-static struct pci_host_bridge *find_pci_host_bridge(struct pci_bus *bus)
+struct pci_host_bridge *find_pci_host_bridge(struct pci_bus *bus)
 {
 	struct pci_bus *root_bus = find_pci_root_bus(bus);
 
 	return to_pci_host_bridge(root_bus->bridge);
 }
+EXPORT_SYMBOL_GPL(find_pci_host_bridge);
 
 void pci_set_host_bridge_release(struct pci_host_bridge *bridge,
 				 void (*release_fn)(struct pci_host_bridge *),