RE: bare pci configuration access functions ?

RE: bare pci configuration access functions ?

[Grover, Andrew]

> From: Greg KH [mailto:greg@kroah.com] 
> Nice, thanks for pointing that out.  But what about the fact that I
> think we can now start optimizing certain parts of the 
> "generic" code to
> play nicer with Linux?

It is much much more important that ACPI be *correct* than fast or small. To
that end, it is better to not fork ACPI-Linux from ACPI-everyone else. Linux
benefits from core bugs found by other OSes (FBSD is not the only one - for
some reason I'm not allowed to mention who else is using ACPI CA but they
*do* send bug reports) and vice versa.

> Now I don't mean this to be an ACPI rant, I know why they did 
> their code
> this way, and without it, there probably would not be any ACPI Linux
> code.  I just don't think it's the best way (from an engineering
> standpoint) to do things.  And again, we are getting way off 
> topic from
> the original problem, sorry.

I'm used to ACPI ranting from all quarters, you know that ;-) but let me
just say this:

- ACPI is not performance-critical
- ACPI will never be simple and elegant, even if you made it Linux-specific
- Portability enhances correctness and maximizes developer productivity
- Read my lips, no new taxes!

(dunno where that last one came from ;-)

Regards -- Andy

Re: bare pci configuration access functions ?

[Greg KH]

On Thu, Oct 31, 2002 at 06:07:31PM -0800, Grover, Andrew wrote:
> > From: Greg KH [mailto:greg@kroah.com] 
> > Nice, thanks for pointing that out.  But what about the fact that I
> > think we can now start optimizing certain parts of the "generic"
> > code to play nicer with Linux?
> 
> It is much much more important that ACPI be *correct* than fast or small.

I agree.  I was just thinking that ACPI was correct already, and we
could move on toward making it fast and small.  Sorry for thinking that
:)

> I'm used to ACPI ranting from all quarters, you know that ;-) but let me
> just say this:
> 
> - ACPI is not performance-critical
> - ACPI will never be simple and elegant, even if you made it Linux-specific
> - Portability enhances correctness and maximizes developer productivity
> - Read my lips, no new taxes!
> 
> (dunno where that last one came from ;-)

You already said 3 other lies, so a fourth one rounded them all out?  :)

(sorry, couldn't help myself.  For the readers in the peanut gallery,
 I consider Andy a friend, this was not a personal attack, just a chance
 to make a joke.)

To address the above:

> - ACPI is not performance-critical

But it can't hurt in both stack size, and execution speed to fix obvious
things that cause it to slow down.  And if ACPI is too slow, booting
takes longer, and getting ACPI events to other places start taking
unacceptable amounts of times.  Not that this is happening right now :)

> - ACPI will never be simple and elegant, even if you made it Linux-specific

Heh, you said it, I didn't.

> - Portability enhances correctness and maximizes developer productivity

Only if the developers are being forced to work on multiple platforms.
For the majority of Linux kernel developers, luckily we do not have to
do this.  For your group, I understand the constraints, and am willing
to live with it, in order to get a working ACPI implementation.  Beggars
can't be choosy :)

thanks,

greg k-h

RE: bare pci configuration access functions ?

[Grover, Andrew]

> From: Greg KH [mailto:greg@kroah.com] 
> In 2.5 we now have a 
> pci_bus_read_*
> and pci_bus_write_* functions, which the pci hotplug drivers use, as
> they at least know the bus on which the devices they are 
> looking for are
> on.  I also had to convert over some ACPI code that was using the
> pci_read_config functions to get everything to work properly, but I
> don't seem to be able to find that code in the latest 2.5 tree, so I
> guess you don't need to do that anymore?

It's still there in drivers/acpi/osl.c. We use the pci_root_ops directly,
instead of bus->ops (which is usually set to pci_root_ops anyways) but I
just mention this as a sidenote.

Regards -- Andy

Re: bare pci configuration access functions ?

[Greg KH]

On Thu, Oct 31, 2002 at 02:00:27PM -0800, Grover, Andrew wrote:
> > From: Greg KH [mailto:greg@kroah.com] 
> > In 2.5 we now have a 
> > pci_bus_read_*
> > and pci_bus_write_* functions, which the pci hotplug drivers use, as
> > they at least know the bus on which the devices they are 
> > looking for are
> > on.  I also had to convert over some ACPI code that was using the
> > pci_read_config functions to get everything to work properly, but I
> > don't seem to be able to find that code in the latest 2.5 tree, so I
> > guess you don't need to do that anymore?
> 
> It's still there in drivers/acpi/osl.c. We use the pci_root_ops directly,
> instead of bus->ops (which is usually set to pci_root_ops anyways) but I
> just mention this as a sidenote.

Ah, thanks for pointing it out, I try to forget my forays into the ACPI
code :)

But even then, you are building up a few pci structures yourself to talk
to the pci device.  In looking at the few places you call this function,
is there any reason that acpi_ex_pci_config_space_handler() can't just
call pci_bus_* itself, instead of having to go through
acpi_os_read_pci_configuration()?  If so, the one other usage of the
acpi_os_read_pci_configuration() can cause that function to be
simplified a lot.

Anyway, this is a nice diversion from the real problem here, for 2.4,
should I just backport the pci_ops changes which will allow pci
hotplugging to work again on ia64, or do we want to do something else?

thanks,

greg k-h

Re: bare pci configuration access functions ?

[Scott Murray]

On Thu, 31 Oct 2002, Greg KH wrote:
[snip]
> Anyway, this is a nice diversion from the real problem here, for 2.4,
> should I just backport the pci_ops changes which will allow pci
> hotplugging to work again on ia64, or do we want to do something else?

It would be nice from a hotplug driver maintenance point of view if the
2.4 and 2.5 interfaces were the same IMO.  How about submitting the change
in 2.4.21-pre?

Scott


-- 
Scott Murray
SOMA Networks, Inc.
Toronto, Ontario
e-mail: scottm@somanetworks.com

Re: bare pci configuration access functions ?

[Greg KH]

On Thu, Oct 31, 2002 at 05:50:06PM -0500, Scott Murray wrote:
> On Thu, 31 Oct 2002, Greg KH wrote:
> [snip]
> > Anyway, this is a nice diversion from the real problem here, for 2.4,
> > should I just backport the pci_ops changes which will allow pci
> > hotplugging to work again on ia64, or do we want to do something else?
> 
> It would be nice from a hotplug driver maintenance point of view if the
> 2.4 and 2.5 interfaces were the same IMO.

Yes it would be, but it's not a necessary thing :)

> How about submitting the change in 2.4.21-pre?

It is a _very_ big change.  It hits every architecture.  It was the
right thing to do in 2.5, I'm just questioning if it's the right thing
to do in 2.4 because of the magnitude of it.

So, if people say it's ok, I'll do it.  But I would like to hear from
the PPC64 group first, as I know I caused them a lot of grief and rework
because of it.

thanks,

greg k-h

Re: bare pci configuration access functions ?

[Scott Murray]

On Thu, 31 Oct 2002, Greg KH wrote:

> On Thu, Oct 31, 2002 at 05:50:06PM -0500, Scott Murray wrote:
> > On Thu, 31 Oct 2002, Greg KH wrote:
> > [snip]
> > > Anyway, this is a nice diversion from the real problem here, for 2.4,
> > > should I just backport the pci_ops changes which will allow pci
> > > hotplugging to work again on ia64, or do we want to do something else?
> >
> > It would be nice from a hotplug driver maintenance point of view if the
> > 2.4 and 2.5 interfaces were the same IMO.
>
> Yes it would be, but it's not a necessary thing :)

Yeah, yeah, I can deal. ;)

> > How about submitting the change in 2.4.21-pre?
>
> It is a _very_ big change.  It hits every architecture.  It was the
> right thing to do in 2.5, I'm just questioning if it's the right thing
> to do in 2.4 because of the magnitude of it.
>
> So, if people say it's ok, I'll do it.  But I would like to hear from
> the PPC64 group first, as I know I caused them a lot of grief and rework
> because of it.

I hadn't realized the magnitude of the changes from the previous
discussion here on the list, my apologies.

Scott


-- 
Scott Murray
SOMA Networks, Inc.
Toronto, Ontario
e-mail: scottm@somanetworks.com

Re: bare pci configuration access functions ?

[KOCHI, Takayoshi]

On Thu, 31 Oct 2002 14:11:36 -0800
Greg KH <greg@kroah.com> wrote:

> But even then, you are building up a few pci structures yourself to talk
> to the pci device.  In looking at the few places you call this function,
> is there any reason that acpi_ex_pci_config_space_handler() can't just
> call pci_bus_* itself, instead of having to go through
> acpi_os_read_pci_configuration()?  If so, the one other usage of the
> acpi_os_read_pci_configuration() can cause that function to be
> simplified a lot.

That's because of Linux port of ACPI CA structure.
ACPI CA divides the acpi driver into OS independent part and os dependent
part.  acpi_ex_pci_config_space_handler exists in OS-independent
part and acpi_os_read_pci_configuration exists in OS-dependent
part.  The OS independent part is shared with other OSes, while
OS dependent part (acpi_os_xxx functions) are Linux specific.

That's the way ACPI driver designers took and Linux can benefit
from other OS's feedback in OS-independent part.

> Anyway, this is a nice diversion from the real problem here, for 2.4,
> should I just backport the pci_ops changes which will allow pci
> hotplugging to work again on ia64, or do we want to do something else?

It would be great if we had the same 2.5 functions in 2.4.

Thanks,
-- 
KOCHI, Takayoshi <t-kouchi@cq.jp.nec.com/t-kouchi@mvf.biglobe.ne.jp>

Re: bare pci configuration access functions ?

[Greg KH]

On Thu, Oct 31, 2002 at 03:37:16PM -0800, KOCHI, Takayoshi wrote:
> 
> On Thu, 31 Oct 2002 14:11:36 -0800
> Greg KH <greg@kroah.com> wrote:
> 
> > But even then, you are building up a few pci structures yourself to talk
> > to the pci device.  In looking at the few places you call this function,
> > is there any reason that acpi_ex_pci_config_space_handler() can't just
> > call pci_bus_* itself, instead of having to go through
> > acpi_os_read_pci_configuration()?  If so, the one other usage of the
> > acpi_os_read_pci_configuration() can cause that function to be
> > simplified a lot.
> 
> That's because of Linux port of ACPI CA structure.
> ACPI CA divides the acpi driver into OS independent part and os dependent
> part.  acpi_ex_pci_config_space_handler exists in OS-independent
> part and acpi_os_read_pci_configuration exists in OS-dependent
> part.  The OS independent part is shared with other OSes, while
> OS dependent part (acpi_os_xxx functions) are Linux specific.

Ok, if you want to drag that baggage around for forever, fine.  But
don't force me to change anything in the pci core to enable that baggage
to stay :)

> That's the way ACPI driver designers took and Linux can benefit
> from other OS's feedback in OS-independent part.

Can I ask if any of the development for other OSs has actually helped
Linux development?  I'm just curious.

And isn't the ACPI core development basically finished, so that now we
can start to do these OS dependent optimizations to allow the ACPI code
base to start to shrink and move towards looking more like the rest of
the kernel code, instead of looking like "something that got run through
a Linux kernelizer style generator"?

> > Anyway, this is a nice diversion from the real problem here, for 2.4,
> > should I just backport the pci_ops changes which will allow pci
> > hotplugging to work again on ia64, or do we want to do something else?
> 
> It would be great if we had the same 2.5 functions in 2.4.

It would be, but as I just said in a different response, it's a _big_
change.  I can't just go breaking different arch's with impunity as I've
been doing in 2.5 :)

thanks,

greg k-h

Re: bare pci configuration access functions ?

[KOCHI, Takayoshi]

On Thu, 31 Oct 2002 15:54:57 -0800
Greg KH <greg@kroah.com> wrote:

> > That's the way ACPI driver designers took and Linux can benefit
> > from other OS's feedback in OS-independent part.
> 
> Can I ask if any of the development for other OSs has actually helped
> Linux development?  I'm just curious.

FreeBSD's acpi project is a good example.
http://www.jp.freebsd.org/acpi/index.html
(though this page doesn't seem to reflect recent status)

They share the same code base (OS-independent part) as Linux
and troubles FreeBSD had are troubles Linux will have or vice versa.

Its mailing-list is based in Japan but most discussions
are in English and some intel developers are also in the list.


AFAIK the aic7xxx driver has similar structure.

Thanks,
-- 
KOCHI, Takayoshi <t-kouchi@cq.jp.nec.com/t-kouchi@mvf.biglobe.ne.jp>

Re: bare pci configuration access functions ?

[Greg KH]

On Thu, Oct 31, 2002 at 04:23:56PM -0800, KOCHI, Takayoshi wrote:
> 
> On Thu, 31 Oct 2002 15:54:57 -0800
> Greg KH <greg@kroah.com> wrote:
> 
> > > That's the way ACPI driver designers took and Linux can benefit
> > > from other OS's feedback in OS-independent part.
> > 
> > Can I ask if any of the development for other OSs has actually helped
> > Linux development?  I'm just curious.
> 
> FreeBSD's acpi project is a good example.
> http://www.jp.freebsd.org/acpi/index.html
> (though this page doesn't seem to reflect recent status)
> 
> They share the same code base (OS-independent part) as Linux
> and troubles FreeBSD had are troubles Linux will have or vice versa.
> 
> Its mailing-list is based in Japan but most discussions
> are in English and some intel developers are also in the list.

Nice, thanks for pointing that out.  But what about the fact that I
think we can now start optimizing certain parts of the "generic" code to
play nicer with Linux?

> AFAIK the aic7xxx driver has similar structure.

Some other SCSI drivers have this kind of structure.  And in the end,
it's a big pain.  If it makes the aic7xxx driver author's life easier,
all the better, as he's doing that work.  I just hope the upcoming 2.5
scsi core changes will not mess with him too much.

And that's the biggest problem with portions of code that try to be "os
independent", it usually causes bloat, and keeps other developers from
trying to help out with the project (any change I would make would most
likely be a Linux specific change, and be refused by the maintainer.)
It requires yet another "shim" layer between the driver's core logic,
and the OS, and the kernel sure doesn't need anymore of those...

OS independent drivers have been proven to not work, you don't see UDI
in production use now do you?

Now I don't mean this to be an ACPI rant, I know why they did their code
this way, and without it, there probably would not be any ACPI Linux
code.  I just don't think it's the best way (from an engineering
standpoint) to do things.  And again, we are getting way off topic from
the original problem, sorry.

thanks,

greg k-h

RE: bare pci configuration access functions ?

[Grover, Andrew]

> From: Lee, Jung-Ik [mailto:jung-ik.lee@intel.com] 
> 	Some kernel drivers/components such as hotplug 
> pci/io-node drivers,
> ACPI driver, some console drivers, etc **need bare pci 
> configuration space
> access** before either pci driver is initialized or struct pci_dev is
> constructed.
> 
> ACPI needs this for ACPI/PCI population, hotplug pci driver 
> for populating
> hot-added pci hierarchy. As more drivers are cross ported 
> over to wider
> architectures, this would become wider need. Help me if 
> others need this
> too.

When the PCI Config stuff got revamped a few months ago, Greg KH, myself,
and some other people discussed this, and the conclusion seemed to be that
it was less ugly to make the code that needs bare PCI config access use fake
structs, than to have the bare functions exposed. Greg, am I remembering
correctly?

Regards -- Andy

Re: bare pci configuration access functions ?

[Greg KH]

On Thu, Oct 31, 2002 at 01:07:08PM -0800, Grover, Andrew wrote:
> > From: Lee, Jung-Ik [mailto:jung-ik.lee@intel.com] 
> > 	Some kernel drivers/components such as hotplug 
> > pci/io-node drivers,
> > ACPI driver, some console drivers, etc **need bare pci 
> > configuration space
> > access** before either pci driver is initialized or struct pci_dev is
> > constructed.
> > 
> > ACPI needs this for ACPI/PCI population, hotplug pci driver 
> > for populating
> > hot-added pci hierarchy. As more drivers are cross ported 
> > over to wider
> > architectures, this would become wider need. Help me if 
> > others need this
> > too.
> 
> When the PCI Config stuff got revamped a few months ago, Greg KH, myself,
> and some other people discussed this, and the conclusion seemed to be that
> it was less ugly to make the code that needs bare PCI config access use fake
> structs, than to have the bare functions exposed. Greg, am I remembering
> correctly?

No.  Well, I don't think so anyway.  In 2.5 we now have a pci_bus_read_*
and pci_bus_write_* functions, which the pci hotplug drivers use, as
they at least know the bus on which the devices they are looking for are
on.  I also had to convert over some ACPI code that was using the
pci_read_config functions to get everything to work properly, but I
don't seem to be able to find that code in the latest 2.5 tree, so I
guess you don't need to do that anymore?

(For the LKML readers, this is a spill-over from the pci hotplug and
ia64 mailing lists, where on 2.4 we now have a problem with pci hotplug
drivers as ia64 uses a pci "segment" and the existing pci_*_nodev
functions in the pci hotplug core don't properly set up this field.  See
the archives for either of those lists for more info.)

thanks,

greg k-h

RE: bare pci configuration access functions ?

[Kai Germaschewski]

On Thu, 31 Oct 2002, Grover, Andrew wrote:

> > From: Lee, Jung-Ik [mailto:jung-ik.lee@intel.com] 
> > 	Some kernel drivers/components such as hotplug 
> > pci/io-node drivers,
> > ACPI driver, some console drivers, etc **need bare pci 
> > configuration space
> > access** before either pci driver is initialized or struct pci_dev is
> > constructed.
> > 
> > ACPI needs this for ACPI/PCI population, hotplug pci driver 
> > for populating
> > hot-added pci hierarchy. As more drivers are cross ported 
> > over to wider
> > architectures, this would become wider need. Help me if 
> > others need this
> > too.
> 
> When the PCI Config stuff got revamped a few months ago, Greg KH, myself,
> and some other people discussed this, and the conclusion seemed to be that
> it was less ugly to make the code that needs bare PCI config access use fake
> structs, than to have the bare functions exposed. Greg, am I remembering
> correctly?

As one of the other people involved, I think the solution which was agreed
on was to have the lower level functions provided with (struct pci_bus *,
u8 dev, u8 fn) since that is what's really needed to implement the config
accesses. The standard PCI config functions, taking struct pci_dev * are
merely a wrapper around these. Greg did a patch to implement it, and it
looks like it got merged.

pci_bus_{read,write}_config_{byte,word,dword}()

--Kai