[PATCH] PCI / OF linkage in sysfs

[PATCH] PCI / OF linkage in sysfs

[Benjamin Herrenschmidt]

Hi !

This patch adds a "devspec" property to all PCI entries in sysfs
that provides the full "Open Firmware" path to each device on
PPC and PPC64 platforms that have Open Firmware support.

For various reasons, the OF path is and will still be different than
the sysfs path, and userland needs the OF path for various things,
ranging from bootloader setup to XFree needing to access some OF
properties provided by the graphic card F-Code driver, etc...

The "devspec" name is what we already use for "macio" type devices,
it doesn't clash with anything else.

If you are happy with it, please apply (independently of the rest
of the PowerMac patch), I need that to fix various things in XFree
(among others), so it would be nice to have it in by 2.6.3 final

Regards,
Ben.

diff -urN linux-2.5/drivers/pci/pci-sysfs.c linuxppc-2.5-benh/drivers/pci/pci-sysfs.c
--- linux-2.5/drivers/pci/pci-sysfs.c	2004-02-02 13:09:08.000000000 +1100
+++ linuxppc-2.5-benh/drivers/pci/pci-sysfs.c	2004-02-04 17:57:05.000000000 +1100
@@ -20,6 +20,24 @@
 
 #include "pci.h"
 
+#ifdef CONFIG_PPC_OF
+#include <asm/prom.h>
+#include <asm/pci-bridge.h>
+
+static ssize_t pci_show_devspec(struct device *dev, char *buf)
+{
+	struct pci_dev *pdev;
+	struct device_node *np;
+
+	pdev = to_pci_dev (dev);
+	np = pci_device_to_OF_node(pdev);
+	if (np == NULL || np->full_name == NULL)
+		return 0;
+	return sprintf(buf, "%s", np->full_name);
+}
+static DEVICE_ATTR(devspec, S_IRUGO, pci_show_devspec, NULL);
+#endif /* CONFIG_PPC_OF */
+
 /* show configuration fields */
 #define pci_config_attr(field, format_string)				\
 static ssize_t								\
@@ -179,5 +197,8 @@
 	device_create_file (dev, &dev_attr_class);
 	device_create_file (dev, &dev_attr_irq);
 	device_create_file (dev, &dev_attr_resource);
+#ifdef CONFIG_PPC_OF
+	device_create_file (dev, &dev_attr_devspec);
+#endif
 	sysfs_create_bin_file(&dev->kobj, &pci_config_attr);
 }

Re: [PATCH] PCI / OF linkage in sysfs

[Linus Torvalds]

On Wed, 4 Feb 2004, Benjamin Herrenschmidt wrote:
> 
> This patch adds a "devspec" property to all PCI entries in sysfs
> that provides the full "Open Firmware" path to each device on
> PPC and PPC64 platforms that have Open Firmware support.

Wouldn't it make more sense to go the other way? Ie have the PCI devices 
be pointed to from the OF paths?

I'd prefer to avoid having OF-specific files in a PCI directory. That just 
leads to inherently unportable user mode stuff. In contrast, having the OF 
directory entry that points to the hardware (PCI) entry makes perfect 
sense.

umm?

		Linus

Re: [PATCH] PCI / OF linkage in sysfs

[Greg KH]

On Wed, Feb 04, 2004 at 02:08:58PM -0800, Linus Torvalds wrote:
> 
> 
> On Wed, 4 Feb 2004, Benjamin Herrenschmidt wrote:
> > 
> > This patch adds a "devspec" property to all PCI entries in sysfs
> > that provides the full "Open Firmware" path to each device on
> > PPC and PPC64 platforms that have Open Firmware support.
> 
> Wouldn't it make more sense to go the other way? Ie have the PCI devices 
> be pointed to from the OF paths?

Or, if you really want to be able to get the OF info from the pci device
in sysfs, why not create a symlink in the pci device directory pointing
to your OF path in sysfs?  That would seem like the best option.

thanks,

greg k-h

Re: [PATCH] PCI / OF linkage in sysfs

[Benjamin Herrenschmidt]

> Or, if you really want to be able to get the OF info from the pci device
> in sysfs, why not create a symlink in the pci device directory pointing
> to your OF path in sysfs?  That would seem like the best option.

The OF device-tree isn't in sysfs, it's in /proc/device-tree, we never
"ported" that code to sysfs for various reasons. I could start doing
so but I'm afraid I won't have time to do that before 2.6.3 and I
really want something in asap so I can finally fix the bootloader setup
and XFree once for all...

Now, if you consider a symlink to /proc/device-tree acceptable... :)

Ben.

Re: [PATCH] PCI / OF linkage in sysfs

[Greg KH]

On Thu, Feb 05, 2004 at 10:28:20AM +1100, Benjamin Herrenschmidt wrote:
> 
> > Or, if you really want to be able to get the OF info from the pci device
> > in sysfs, why not create a symlink in the pci device directory pointing
> > to your OF path in sysfs?  That would seem like the best option.
> 
> The OF device-tree isn't in sysfs, it's in /proc/device-tree, we never
> "ported" that code to sysfs for various reasons.

Reasons pertaining to the sysfs interface, or other non-technical
reasons?

Even ACPI shows up in sysfs these days :)

greg k-h

Re: [PATCH] PCI / OF linkage in sysfs

[Linus Torvalds]

On Wed, 4 Feb 2004, Greg KH wrote:
> 
> Or, if you really want to be able to get the OF info from the pci device
> in sysfs, why not create a symlink in the pci device directory pointing
> to your OF path in sysfs?  That would seem like the best option.

Where does this stop? Do we start doing the same for all different kinds 
of buses, and all kinds of firmware? 

In other words, instead of having <n> different buses all know about <m>
different kinds of firmware information that they really have nothing else
to do with, it's much better to just have the <m> different kinds of 
firmware information export their own information.

It just sounds _wrong_ to have the PCI layer have knowledge of OF. It has 
nothing to do with OF. For OF information, you should go to the /sys/of 
tree, which has the information that OF knows about (which may, of course, 
then include the information about PCI devices).

		Linus

Re: [PATCH] PCI / OF linkage in sysfs

[Greg KH]

On Wed, Feb 04, 2004 at 03:28:21PM -0800, Linus Torvalds wrote:
> 
> On Wed, 4 Feb 2004, Greg KH wrote:
> > 
> > Or, if you really want to be able to get the OF info from the pci device
> > in sysfs, why not create a symlink in the pci device directory pointing
> > to your OF path in sysfs?  That would seem like the best option.
> 
> Where does this stop? Do we start doing the same for all different kinds 
> of buses, and all kinds of firmware? 

Ok, good point.  You're right in that way we would eventually end up
with a huge mess...

Sorry Ben.

greg k-h

Re: [PATCH] PCI / OF linkage in sysfs

[Benjamin Herrenschmidt]

> Where does this stop? Do we start doing the same for all different kinds 
> of buses, and all kinds of firmware? 
> 
> In other words, instead of having <n> different buses all know about <m>
> different kinds of firmware information that they really have nothing else
> to do with, it's much better to just have the <m> different kinds of 
> firmware information export their own information.
> 
> It just sounds _wrong_ to have the PCI layer have knowledge of OF. It has 
> nothing to do with OF. For OF information, you should go to the /sys/of 
> tree, which has the information that OF knows about (which may, of course, 
> then include the information about PCI devices).

I don't quite agree... There are cases for example (USB, Firewire) where
we could construct an OF path to be used by the bootloader setup without
having the OF information in the first place (for devices that weren't
plugged during boot typically). I do no intend to go that way for 2.6
though.

In both cases, we don't "have" the information.

OF doesn't have informations about the linux PCI layout (bus numbering can
be different between OF and linux for example) and PCI doesn't have information
about OF (except that on ppc64, pci_dev->arch_data points to the OF node).

However, the arch code provides a routine that can provide that mapping
PCI -> OF (and in _that_ direction, there is one to go the other way around,
but I hate it, it's not very reliable at the moment, though I could rewrite
it..., and on ppc64, this is the most efficient way too).

It's just about providing a pointer to OF node, not actual informations out
of the device-tree...

Ben.


 

Re: [PATCH] PCI / OF linkage in sysfs

[Linus Torvalds]

On Thu, 5 Feb 2004, Benjamin Herrenschmidt wrote:
> 
> I don't quite agree... There are cases for example (USB, Firewire) where
> we could construct an OF path to be used by the bootloader setup without
> having the OF information in the first place (for devices that weren't
> plugged during boot typically). I do no intend to go that way for 2.6
> though.

Ok. Fair enough.

I think that I personally would be a lot happier with the situation if it 
wasn't that PCI had magic knowledge about OF in particular.  In other 
words, you'd likely be able to sell me on an idea where the PCI layer just 
knows about "let the firmware install a few files here", but is totally 
firmware-agnostic per se.

In other words, you migth just rename the "OF" functionality as "platform" 
functionality, and add dummy (empty) platform handlers for the other 
platforms (eg BIOS/EFI whatever). Maybe some day EFI will want to have a 
similar pointer..

So while I'd hate to have the PCI layer start having to learn details of 
all the platforms out there, I don't think it's necessarily wrong that the 
PCI layer knows about the _concept_ of a platform, as long as it doesn't 
get too specific.

Would that suit your needs?

		Linus

Re: [PATCH] PCI / OF linkage in sysfs

[Benjamin Herrenschmidt]

On Thu, 2004-02-05 at 11:04, Linus Torvalds wrote:

> I think that I personally would be a lot happier with the situation if it 
> wasn't that PCI had magic knowledge about OF in particular.  In other 
> words, you'd likely be able to sell me on an idea where the PCI layer just 
> knows about "let the firmware install a few files here", but is totally 
> firmware-agnostic per se.
> 
> In other words, you migth just rename the "OF" functionality as "platform" 
> functionality, and add dummy (empty) platform handlers for the other 
> platforms (eg BIOS/EFI whatever). Maybe some day EFI will want to have a 
> similar pointer..
> 
> So while I'd hate to have the PCI layer start having to learn details of 
> all the platforms out there, I don't think it's necessarily wrong that the 
> PCI layer knows about the _concept_ of a platform, as long as it doesn't 
> get too specific.
> 
> Would that suit your needs?

What about adding a pcibios_add_platform_entries(device) called by
pci_sysfs then ? By default an empty inline on asm/* and on PPC,
I can add my devspec without having OF-aware code in drivers/pci

Also, if you prefer a different name for "devspec", speak up now ;)

I can still change the name in the macio devices too, nothing uses
them right now and I expect things that will be fixed to use them
to rely on 2.6.3 minimum.

Ben.

Re: [PATCH] PCI / OF linkage in sysfs

[Linus Torvalds]

On Thu, 5 Feb 2004, Benjamin Herrenschmidt wrote:

> > Would that suit your needs?
> 
> What about adding a pcibios_add_platform_entries(device) called by
> pci_sysfs then ? By default an empty inline on asm/* and on PPC,
> I can add my devspec without having OF-aware code in drivers/pci

Yes.

> Also, if you prefer a different name for "devspec", speak up now ;)

I have to admit that "devspec" doesn't seem to do much for me, but I don't 
think we should call it "firmware", since that would (to me) be more about 
the firmware of the _device_ rather than the platform.

Maybe just "platform-data" or something. But if "devspec" has magic 
meaning on a Mac, and since this would be inherently platform-specific 
_anyway_, I don't actually see any reason to not use "devspec".

On some platforms, we might have multiple different entries (eg on a PC we 
might have pointers to ACPI data, to PnP data and to EFI data, all at the 
same time. I hope we never will, but maybe there would be reason for it). 
That would argue _against_ a "generic" name like "platform", and for 
something that is actually very much specific to the kind of data it 
points to (eg "of-data" rather than "platform-data").

End result: I don't think we much care about the name. Whatever makes you
happy. As long as the source code is clean and something like
"pcibios_add_platform_entries()" at least makes that come true.

		Linus

Re: [PATCH] PCI / OF linkage in sysfs

[Benjamin Herrenschmidt]

> Maybe just "platform-data" or something. But if "devspec" has magic 
> meaning on a Mac, and since this would be inherently platform-specific 
> _anyway_, I don't actually see any reason to not use "devspec".

No, no magic. Could have been "OF_path" but uppercase are ugly :)

> On some platforms, we might have multiple different entries (eg on a PC we 
> might have pointers to ACPI data, to PnP data and to EFI data, all at the 
> same time. I hope we never will, but maybe there would be reason for it). 
> That would argue _against_ a "generic" name like "platform", and for 
> something that is actually very much specific to the kind of data it 
> points to (eg "of-data" rather than "platform-data").

Maybe anything prefixed by "platform" ? like "platform-fwpath" for OF
spec, etc... 

> End result: I don't think we much care about the name. Whatever makes you
> happy. As long as the source code is clean and something like
> "pcibios_add_platform_entries()" at least makes that come true.

Ok. I dislike #ifdef's too indeed. I was probably too lazy to add the
empty inline to all archs :) I'll do a new patch.

Ben.

Re: [PATCH] PCI / OF linkage in sysfs

[Eric W. Biederman]

Linus Torvalds <torvalds@osdl.org> writes:

> On some platforms, we might have multiple different entries (eg on a PC we 
> might have pointers to ACPI data, to PnP data and to EFI data, all at the 
> same time. I hope we never will, but maybe there would be reason for it). 
> That would argue _against_ a "generic" name like "platform", and for 
> something that is actually very much specific to the kind of data it 
> points to (eg "of-data" rather than "platform-data").

And there is the possibility in the LinuxBIOS case that you can have
ACPI, EFI, OF and the native LinuxBIOS interfaces all on one box :) 

The EFI side is the only piece we are in wait and see mode for at
the moment.

So it is seriously possible you get everything simultaneously.

Eric

Re: [PATCH] PCI / OF linkage in sysfs

[Benjamin Herrenschmidt]

On Thu, 2004-02-05 at 09:08, Linus Torvalds wrote:
> On Wed, 4 Feb 2004, Benjamin Herrenschmidt wrote:
> > 
> > This patch adds a "devspec" property to all PCI entries in sysfs
> > that provides the full "Open Firmware" path to each device on
> > PPC and PPC64 platforms that have Open Firmware support.
> 
> Wouldn't it make more sense to go the other way? Ie have the PCI devices 
> be pointed to from the OF paths?

Well... There are a few problems with that. First, what people/drivers
actually want is the PCI -> OF mapping, rarely the other way around.
So having an OF -> PCI linkage will mean users will have to walk the
whole tree each time to locate the device.

Then, adding properties to the OF tree is a bit nasty as we try to
keep it as "intact" as possible and the current /proc interface to
it is rather static, this is not on our plan to change that in the
2.6 timeframe... 

Finally, the "macio" and "of platform" type devices already have
that "devspec" entry, so it would be more consistent to do the
same for PCI.

> I'd prefer to avoid having OF-specific files in a PCI directory. That just 
> leads to inherently unportable user mode stuff. In contrast, having the OF 
> directory entry that points to the hardware (PCI) entry makes perfect 
> sense.

Well, I don't see a portability problem. Usermode stuff that need to
use & access the OF information are inherently unportable already,
wether it does the lookup via /sys/devices or via /proc/device-tree
(or whatever we call it if we ever move it out of /proc).

The 2 usage of that I see (maybe more with IBM diagnostic & VPD stuff)
right are:

 - setting up the bootloader (inherently non-portable, currently uses
a rather broken shell script that sometimes work by mere luck and wild
guesses)

 - Xserver/XFree. On most platform, the drivers in there (typically ATI
ones) will use tables in the x86 BIOS for things like PLL parameters,
on OF machines, though, those infos are available via properties in the
OF device-tree set by the f-code firmware of the cards. We need a way
to access these (along with some connector information). Currently, lots
of configs work only with UseFBDev because of that.


Ben.