Re: [PATCH] altix: export sn_pcidev_info_get

Re: [PATCH] altix: export sn_pcidev_info_get

[Christoph Hellwig]

On Tue, Feb 14, 2006 at 10:23:37AM -0600, Mark Maule wrote:
> Export sn_pcidev_info_get.

Tony or Andrew please back this out again.  The only reason SGI wants this is
to support their illegal graphics driver, and no core code uses it.

And Mark, please stop submitting such patches.

Re: [PATCH] altix: export sn_pcidev_info_get

[Andrew Morton]

Christoph Hellwig <hch@infradead.org> wrote:
>
>  On Tue, Feb 14, 2006 at 10:23:37AM -0600, Mark Maule wrote:
>  > Export sn_pcidev_info_get.
> 
>  Tony or Andrew please back this out again.

Please send a patch.

Re: [PATCH] altix: export sn_pcidev_info_get

[Mark Maule]

On Mon, Feb 20, 2006 at 08:17:14PM +0000, Christoph Hellwig wrote:
> On Tue, Feb 14, 2006 at 10:23:37AM -0600, Mark Maule wrote:
> > Export sn_pcidev_info_get.
> 
> Tony or Andrew please back this out again.  The only reason SGI wants this is
> to support their illegal graphics driver, and no core code uses it.
> 
> And Mark, please stop submitting such patches.

All I'm doing by exporting sn_pcidev_info_get is allowing a module to use
the SGI SN_PCIDEV_BUSSOFT() macro which will tell a driver which piece of
altix PCI hw its device is sitting behind (e.g. PCIIO_ASIC_TYPE_TIOCP et. al).

While I acknowledge that the gfx driver folks requested this, I don't
understand what is "illegal" about this export, or the driver which wants
to use it.  What am I missing here?

Mark

Re: [PATCH] altix: export sn_pcidev_info_get

[Arjan van de Ven]

On Mon, 2006-02-20 at 20:47 -0600, Mark Maule wrote:
> On Mon, Feb 20, 2006 at 08:17:14PM +0000, Christoph Hellwig wrote:
> > On Tue, Feb 14, 2006 at 10:23:37AM -0600, Mark Maule wrote:
> > > Export sn_pcidev_info_get.
> > 
> > Tony or Andrew please back this out again.  The only reason SGI wants this is
> > to support their illegal graphics driver, and no core code uses it.
> > 
> > And Mark, please stop submitting such patches.
> 
> All I'm doing by exporting sn_pcidev_info_get is allowing a module to use
> the SGI SN_PCIDEV_BUSSOFT() macro which will tell a driver which piece of
> altix PCI hw its device is sitting behind (e.g. PCIIO_ASIC_TYPE_TIOCP et. al).
> 
> While I acknowledge that the gfx driver folks requested this, I don't
> understand what is "illegal" about this export, or the driver which wants
> to use it.  What am I missing here?

so you would have no objection to making this a _GPL export ?

Re: [PATCH] altix: export sn_pcidev_info_get

[Christian Hildner]

Arjan van de Ven schrieb:

>On Mon, 2006-02-20 at 20:47 -0600, Mark Maule wrote:
>  
>
>>On Mon, Feb 20, 2006 at 08:17:14PM +0000, Christoph Hellwig wrote:
>>    
>>
>>>On Tue, Feb 14, 2006 at 10:23:37AM -0600, Mark Maule wrote:
>>>      
>>>
>>>>Export sn_pcidev_info_get.
>>>>        
>>>>
>>>Tony or Andrew please back this out again.  The only reason SGI wants this is
>>>to support their illegal graphics driver, and no core code uses it.
>>>
>>>And Mark, please stop submitting such patches.
>>>      
>>>
>>All I'm doing by exporting sn_pcidev_info_get is allowing a module to use
>>the SGI SN_PCIDEV_BUSSOFT() macro which will tell a driver which piece of
>>altix PCI hw its device is sitting behind (e.g. PCIIO_ASIC_TYPE_TIOCP et. al).
>>
>>While I acknowledge that the gfx driver folks requested this, I don't
>>understand what is "illegal" about this export, or the driver which wants
>>to use it.  What am I missing here?
>>    
>>
>
>so you would have no objection to making this a _GPL export ?
>
Hi Folks,

sn_* symbols are "their" symbols so I can't see any legitimation for 
anyone else to not let them export their symbols as they want. They are 
doing a great job on linux to make it run scalable on the biggest 
machines on earth. Or don't you like the idea of running Linux on those 
supercomputers?

Christian

Re: [PATCH] altix: export sn_pcidev_info_get

[Christoph Hellwig]

On Mon, Feb 20, 2006 at 08:47:10PM -0600, Mark Maule wrote:
> On Mon, Feb 20, 2006 at 08:17:14PM +0000, Christoph Hellwig wrote:
> > On Tue, Feb 14, 2006 at 10:23:37AM -0600, Mark Maule wrote:
> > > Export sn_pcidev_info_get.
> > 
> > Tony or Andrew please back this out again.  The only reason SGI wants this is
> > to support their illegal graphics driver, and no core code uses it.
> > 
> > And Mark, please stop submitting such patches.
> 
> All I'm doing by exporting sn_pcidev_info_get is allowing a module to use
> the SGI SN_PCIDEV_BUSSOFT() macro which will tell a driver which piece of
> altix PCI hw its device is sitting behind (e.g. PCIIO_ASIC_TYPE_TIOCP et. al).

And that's absolutely not something a driver should care about.  If you
illegal binary driver uses it it's totally broken (but then we knew that
before anyway ;-))

Re: [PATCH] altix: export sn_pcidev_info_get

[Christoph Hellwig]

On Mon, Feb 20, 2006 at 01:39:56PM -0800, Andrew Morton wrote:
> Christoph Hellwig <hch@infradead.org> wrote:
> >
> >  On Tue, Feb 14, 2006 at 10:23:37AM -0600, Mark Maule wrote:
> >  > Export sn_pcidev_info_get.
> > 
> >  Tony or Andrew please back this out again.
> 
> Please send a patch.


--- a/arch/ia64/sn/kernel/io_init.c	20 Feb 2006 14:29:08 -0000	1.11
+++ b/arch/ia64/sn/kernel/io_init.c	21 Feb 2006 10:47:10 -0000
@@ -716,4 +716,3 @@ EXPORT_SYMBOL(sn_pci_unfixup_slot);
 EXPORT_SYMBOL(sn_pci_controller_fixup);
 EXPORT_SYMBOL(sn_bus_store_sysdata);
 EXPORT_SYMBOL(sn_bus_free_sysdata);
-EXPORT_SYMBOL(sn_pcidev_info_get);

Re: [PATCH] altix: export sn_pcidev_info_get

[Jes Sorensen]

>>>>> "Christoph" == Christoph Hellwig <hch@infradead.org> writes:

Christoph> On Mon, Feb 20, 2006 at 08:47:10PM -0600, Mark Maule wrote:
>> All I'm doing by exporting sn_pcidev_info_get is allowing a module
>> to use the SGI SN_PCIDEV_BUSSOFT() macro which will tell a driver
>> which piece of altix PCI hw its device is sitting behind
>> (e.g. PCIIO_ASIC_TYPE_TIOCP et. al).

Christoph> And that's absolutely not something a driver should care
Christoph> about.  If you illegal binary driver uses it it's totally
Christoph> broken (but then we knew that before anyway ;-))

Christoph,

Leaving the issues around the specific client aside here.

Any chance you could explain why some drivers in drivers/net use the
arch defines to set things like optimial burst sizes then?  Drivers
such as acenic.c, fealnx.c and starfire.c do this based on the arch
even though it really is a property of the combination of the PCI bus,
CPU and memory busses in the specific systems.

Were you planning on posting a patch to remove all of those instances
while you were at it?

Point is that there are cases where tuning requires you to know what
PCI bridge is below you in order to get the best performance out of a
card. One can keep a PCI ID blacklist to handle tuning of the PCI
bridge itself, but it can't handle things that needs to be tuned
by setting the PCI device's own registers.

Having a generic API to export this information would be a good thing
IMHO.

Cheers,
Jes

Re: [PATCH] altix: export sn_pcidev_info_get

[Christoph Hellwig]

On Tue, Feb 21, 2006 at 06:23:12AM -0500, Jes Sorensen wrote:
> Point is that there are cases where tuning requires you to know what
> PCI bridge is below you in order to get the best performance out of a
> card. One can keep a PCI ID blacklist to handle tuning of the PCI
> bridge itself, but it can't handle things that needs to be tuned
> by setting the PCI device's own registers.
> 
> Having a generic API to export this information would be a good thing
> IMHO.

So please submit a patch to add querying/tuning pci bridges.  And please
make it _GPL exports so people don't accidentally use it in their illegal
drivers.

Re: [PATCH] altix: export sn_pcidev_info_get

[Christoph Hellwig]

On Tue, Feb 21, 2006 at 10:48:41AM +0000, Christoph Hellwig wrote:
> On Mon, Feb 20, 2006 at 01:39:56PM -0800, Andrew Morton wrote:
> > Christoph Hellwig <hch@infradead.org> wrote:
> > >
> > >  On Tue, Feb 14, 2006 at 10:23:37AM -0600, Mark Maule wrote:
> > >  > Export sn_pcidev_info_get.
> > > 
> > >  Tony or Andrew please back this out again.
> > 
> > Please send a patch.

ping?

> --- a/arch/ia64/sn/kernel/io_init.c	20 Feb 2006 14:29:08 -0000	1.11
> +++ b/arch/ia64/sn/kernel/io_init.c	21 Feb 2006 10:47:10 -0000
> @@ -716,4 +716,3 @@ EXPORT_SYMBOL(sn_pci_unfixup_slot);
>  EXPORT_SYMBOL(sn_pci_controller_fixup);
>  EXPORT_SYMBOL(sn_bus_store_sysdata);
>  EXPORT_SYMBOL(sn_bus_free_sysdata);
> -EXPORT_SYMBOL(sn_pcidev_info_get);
---end quoted text---

Re: [PATCH] altix: export sn_pcidev_info_get

[Jes Sorensen]

>>>>> "Christoph" == Christoph Hellwig <hch@infradead.org> writes:

Christoph> On Tue, Feb 21, 2006 at 06:23:12AM -0500, Jes Sorensen
Christoph> wrote:
>> Point is that there are cases where tuning requires you to know
>> what PCI bridge is below you in order to get the best performance
>> out of a card. One can keep a PCI ID blacklist to handle tuning of
>> the PCI bridge itself, but it can't handle things that needs to be
>> tuned by setting the PCI device's own registers.
>> 
>> Having a generic API to export this information would be a good
>> thing IMHO.

Christoph> So please submit a patch to add querying/tuning pci
Christoph> bridges.  And please make it _GPL exports so people don't
Christoph> accidentally use it in their illegal drivers.

Mark checked with the gfx people and it seems that the requirement for
this information was a leftover from an older codebase and they don't
actually need it anymore. Anyway, I'm not sure what illegal drivers
you keep referring to.

Tony/Andrew, it should be ok to take the export out again.

I'll try to look into doing a more generic interface for querying
information about the low level PCI briges at some point. Since
there's no new users screaming for this right now and most current
drivers that will benefit from it are somewhat old, it's not going to
be my top priority item at this moment. Since this would be an
interface, it would obviously have to a regular symbol export.

Cheers,
Jes

RE: [PATCH] altix: export sn_pcidev_info_get

[Luck, Tony]

> Tony/Andrew, it should be ok to take the export out again.

I'm glad that this causes no distress ... I took the export out
yesterday, and Linus pulled, so it is already gone.

-Tony