Hi Matt,
I'm trying to clean up all the usages of struct class_device to use
struct device, and I ran into the pci_bus code. Right now you create a
symlink called "bridge" under the /sys/class/pci_bus/XXXX:XX/ directory
to the pci device that is the bridge.
This is messy to try to convert to struct device, but I have hack of a
patch below. I had some questions for you:
- is there userspace tools that use the 'bridge' symlink?
- do we really need a separate device here? If I just create a tree
of symlinks to the pci devices that the bridges are, and add the
sysfs attributes that are currently in the class_device, to the
pci_device location, will that be acceptable?
Any thoughts you have about this would be appreciated.
thanks,
greg k-h
---
drivers/pci/bus.c | 10 ++++-----
drivers/pci/pci.h | 2 -
drivers/pci/probe.c | 54 +++++++++++++++++++++++++--------------------------
drivers/pci/remove.c | 7 ++----
include/linux/pci.h | 4 +--
5 files changed, 38 insertions(+), 39 deletions(-)
--- gregkh-2.6.orig/drivers/pci/bus.c
+++ gregkh-2.6/drivers/pci/bus.c
@@ -138,11 +138,11 @@ void __devinit pci_bus_add_devices(struc
up_write(&pci_bus_sem);
}
pci_bus_add_devices(dev->subordinate);
- retval = sysfs_create_link(&dev->subordinate->class_dev.kobj,
- &dev->dev.kobj, "bridge");
- if (retval)
- dev_err(&dev->dev, "Error creating sysfs "
- "bridge symlink, continuing...\n");
+// retval = sysfs_create_link(&dev->subordinate->class_dev.kobj,
+// &dev->dev.kobj, "bridge");
+// if (retval)
+// dev_err(&dev->dev, "Error creating sysfs "
+// "bridge symlink, continuing...\n");
}
}
}
--- gregkh-2.6.orig/drivers/pci/pci.h
+++ gregkh-2.6/drivers/pci/pci.h
@@ -78,7 +78,7 @@ static inline int pci_no_d1d2(struct pci
}
extern int pcie_mch_quirk;
extern struct device_attribute pci_dev_attrs[];
-extern struct class_device_attribute class_device_attr_cpuaffinity;
+extern struct device_attribute dev_attr_cpuaffinity;
/**
* pci_match_one_device - Tell if a PCI device structure has a matching
--- gregkh-2.6.orig/drivers/pci/probe.c
+++ gregkh-2.6/drivers/pci/probe.c
@@ -42,7 +42,7 @@ static void pci_create_legacy_files(stru
b->legacy_io->attr.owner = THIS_MODULE;
b->legacy_io->read = pci_read_legacy_io;
b->legacy_io->write = pci_write_legacy_io;
- class_device_create_bin_file(&b->class_dev, b->legacy_io);
+ device_create_bin_file(&b->dev, b->legacy_io);
/* Allocated above after the legacy_io struct */
b->legacy_mem = b->legacy_io + 1;
@@ -51,15 +51,15 @@ static void pci_create_legacy_files(stru
b->legacy_mem->attr.mode = S_IRUSR | S_IWUSR;
b->legacy_mem->attr.owner = THIS_MODULE;
b->legacy_mem->mmap = pci_mmap_legacy_mem;
- class_device_create_bin_file(&b->class_dev, b->legacy_mem);
+ device_create_bin_file(&b->dev, b->legacy_mem);
}
}
void pci_remove_legacy_files(struct pci_bus *b)
{
if (b->legacy_io) {
- class_device_remove_bin_file(&b->class_dev, b->legacy_io);
- class_device_remove_bin_file(&b->class_dev, b->legacy_mem);
+ device_remove_bin_file(&b->dev, b->legacy_io);
+ device_remove_bin_file(&b->dev, b->legacy_mem);
kfree(b->legacy_io); /* both are allocated here */
}
}
@@ -71,26 +71,27 @@ void pci_remove_legacy_files(struct pci_
/*
* PCI Bus Class Devices
*/
-static ssize_t pci_bus_show_cpuaffinity(struct class_device *class_dev,
+static ssize_t pci_bus_show_cpuaffinity(struct device *dev,
+ struct device_attribute *attr,
char *buf)
{
int ret;
cpumask_t cpumask;
- cpumask = pcibus_to_cpumask(to_pci_bus(class_dev));
+ cpumask = pcibus_to_cpumask(to_pci_bus(dev));
ret = cpumask_scnprintf(buf, PAGE_SIZE, cpumask);
if (ret < PAGE_SIZE)
buf[ret++] = '\n';
return ret;
}
-CLASS_DEVICE_ATTR(cpuaffinity, S_IRUGO, pci_bus_show_cpuaffinity, NULL);
+DEVICE_ATTR(cpuaffinity, S_IRUGO, pci_bus_show_cpuaffinity, NULL);
/*
* PCI Bus Class
*/
-static void release_pcibus_dev(struct class_device *class_dev)
+static void release_pcibus_dev(struct device *dev)
{
- struct pci_bus *pci_bus = to_pci_bus(class_dev);
+ struct pci_bus *pci_bus = to_pci_bus(dev);
if (pci_bus->bridge)
put_device(pci_bus->bridge);
@@ -99,7 +100,7 @@ static void release_pcibus_dev(struct cl
static struct class pcibus_class = {
.name = "pci_bus",
- .release = &release_pcibus_dev,
+ .dev_release = &release_pcibus_dev,
};
static int __init pcibus_class_init(void)
@@ -398,13 +399,12 @@ pci_alloc_child_bus(struct pci_bus *pare
child->bus_flags = parent->bus_flags;
child->bridge = get_device(&bridge->dev);
- child->class_dev.class = &pcibus_class;
- sprintf(child->class_dev.class_id, "%04x:%02x", pci_domain_nr(child), busnr);
- retval = class_device_register(&child->class_dev);
+ child->dev.class = &pcibus_class;
+ sprintf(child->dev.bus_id, "%04x:%02x", pci_domain_nr(child), busnr);
+ retval = device_register(&child->dev);
if (retval)
goto error_register;
- retval = class_device_create_file(&child->class_dev,
- &class_device_attr_cpuaffinity);
+ retval = device_create_file(&child->dev, &dev_attr_cpuaffinity);
if (retval)
goto error_file_create;
@@ -426,7 +426,7 @@ pci_alloc_child_bus(struct pci_bus *pare
return child;
error_file_create:
- class_device_unregister(&child->class_dev);
+ device_unregister(&child->dev);
error_register:
kfree(child);
return NULL;
@@ -1081,21 +1081,21 @@ struct pci_bus * __devinit pci_create_bu
goto dev_reg_err;
b->bridge = get_device(dev);
- b->class_dev.class = &pcibus_class;
- sprintf(b->class_dev.class_id, "%04x:%02x", pci_domain_nr(b), bus);
- error = class_device_register(&b->class_dev);
+ b->dev.class = &pcibus_class;
+ sprintf(b->dev.bus_id, "%04x:%02x", pci_domain_nr(b), bus);
+ error = device_register(&b->dev);
if (error)
goto class_dev_reg_err;
- error = class_device_create_file(&b->class_dev, &class_device_attr_cpuaffinity);
+ error = device_create_file(&b->dev, &dev_attr_cpuaffinity);
if (error)
- goto class_dev_create_file_err;
+ goto dev_create_file_err;
/* Create legacy_io and legacy_mem files for this bus */
pci_create_legacy_files(b);
- error = sysfs_create_link(&b->class_dev.kobj, &b->bridge->kobj, "bridge");
- if (error)
- goto sys_create_link_err;
+// error = sysfs_create_link(&b->dev.kobj, &b->bridge->kobj, "bridge");
+// if (error)
+// goto sys_create_link_err;
b->number = b->secondary = bus;
b->resource[0] = &ioport_resource;
@@ -1104,9 +1104,9 @@ struct pci_bus * __devinit pci_create_bu
return b;
sys_create_link_err:
- class_device_remove_file(&b->class_dev, &class_device_attr_cpuaffinity);
-class_dev_create_file_err:
- class_device_unregister(&b->class_dev);
+ device_remove_file(&b->dev, &dev_attr_cpuaffinity);
+dev_create_file_err:
+ device_unregister(&b->dev);
class_dev_reg_err:
device_unregister(dev);
dev_reg_err:
--- gregkh-2.6.orig/drivers/pci/remove.c
+++ gregkh-2.6/drivers/pci/remove.c
@@ -74,10 +74,9 @@ void pci_remove_bus(struct pci_bus *pci_
list_del(&pci_bus->node);
up_write(&pci_bus_sem);
pci_remove_legacy_files(pci_bus);
- class_device_remove_file(&pci_bus->class_dev,
- &class_device_attr_cpuaffinity);
- sysfs_remove_link(&pci_bus->class_dev.kobj, "bridge");
- class_device_unregister(&pci_bus->class_dev);
+ device_remove_file(&pci_bus->dev, &dev_attr_cpuaffinity);
+// sysfs_remove_link(&pci_bus->class_dev.kobj, "bridge");
+ device_unregister(&pci_bus->dev);
}
EXPORT_SYMBOL(pci_remove_bus);
--- gregkh-2.6.orig/include/linux/pci.h
+++ gregkh-2.6/include/linux/pci.h
@@ -251,13 +251,13 @@ struct pci_bus {
unsigned short bridge_ctl; /* manage NO_ISA/FBB/et al behaviors */
pci_bus_flags_t bus_flags; /* Inherited by child busses */
struct device *bridge;
- struct class_device class_dev;
+ struct device dev;
struct bin_attribute *legacy_io; /* legacy I/O for this bus */
struct bin_attribute *legacy_mem; /* legacy mem */
};
#define pci_bus_b(n) list_entry(n, struct pci_bus, node)
-#define to_pci_bus(n) container_of(n, struct pci_bus, class_dev)
+#define to_pci_bus(n) container_of(n, struct pci_bus, dev)
/*
* Error values that may be returned by PCI functions.
On Wed, Jan 17, 2007 at 04:53:45PM -0800, Greg KH wrote:
> I'm trying to clean up all the usages of struct class_device to use
> struct device, and I ran into the pci_bus code. Right now you create a
> symlink called "bridge" under the /sys/class/pci_bus/XXXX:XX/ directory
> to the pci device that is the bridge.
I recommend we just delete the pci_bus class. I don't think it serves
any useful purpose. The bridge can be inferred frmo the sysfs hierarchy
(not to mention lspci will tell you). The cpuaffinity file should be
moved from the bus to the device -- it really doesn't make any sense to
talk about which cpu a bus is affine to, only a device.
On Wed, Jan 17, 2007 at 07:23:52PM -0700, Matthew Wilcox wrote:
> On Wed, Jan 17, 2007 at 04:53:45PM -0800, Greg KH wrote:
> > I'm trying to clean up all the usages of struct class_device to use
> > struct device, and I ran into the pci_bus code. Right now you create a
> > symlink called "bridge" under the /sys/class/pci_bus/XXXX:XX/ directory
> > to the pci device that is the bridge.
>
> I recommend we just delete the pci_bus class. I don't think it serves
> any useful purpose. The bridge can be inferred frmo the sysfs hierarchy
> (not to mention lspci will tell you). The cpuaffinity file should be
> moved from the bus to the device -- it really doesn't make any sense to
> talk about which cpu a bus is affine to, only a device.
I would like to do that, but I want to make sure that no userspace tools
are using this information.
But, as Matt Dobson is now gone off somewhere in Europe, not doing Linux
stuff anymore, he's not going to answer this. So I'll just make up a
removal patch and let it sit in -mm for a while to see if anything
breaks.
I really only think the big NUMA boxes care about that information, if
anything.
thanks,
greg k-h
Hello!
> I recommend we just delete the pci_bus class. I don't think it serves
> any useful purpose. The bridge can be inferred frmo the sysfs hierarchy
> (not to mention lspci will tell you). The cpuaffinity file should be
> moved from the bus to the device -- it really doesn't make any sense to
> talk about which cpu a bus is affine to, only a device.
It doesn't seem to serve any useful purpose other than the affinity now,
but I still think that it conceptually belongs there, because it makes
sense to have per-bus attributes. For example, in the future we could
show data width and signalling speed.
Have a nice fortnight
--
Martin `MJ' Mares <[email protected]> http://mj.ucw.cz/
Faculty of Math and Physics, Charles University, Prague, Czech Rep., Earth
"I invented the term Object-Oriented, and I can tell you I did not have C++ in mind." -- Alan Kay
On Thu, Jan 18, 2007 at 09:14:06AM +0100, Martin Mares wrote:
> Hello!
>
> > I recommend we just delete the pci_bus class. I don't think it serves
> > any useful purpose. The bridge can be inferred frmo the sysfs hierarchy
> > (not to mention lspci will tell you). The cpuaffinity file should be
> > moved from the bus to the device -- it really doesn't make any sense to
> > talk about which cpu a bus is affine to, only a device.
>
> It doesn't seem to serve any useful purpose other than the affinity now,
> but I still think that it conceptually belongs there, because it makes
> sense to have per-bus attributes. For example, in the future we could
> show data width and signalling speed.
So, if it were to stay, where in the tree should it be? Hanging off of
the pci device that is the bridge? Or just placing these files within
the pci device directory itself, as it is the bridge.
There are also some "legacy io" binary sysfs files in these directories
for those platforms that support it (#ifdef HAVE_PCI_LEGACY), and I'm
guessing that there is some user for them out there, otherwise they
would not have been added...
Hm, only ia64 enables that option. Matthew, do you care about those
files?
thanks,
greg k-h
On Thursday, January 18, 2007 1:00 am, Greg KH wrote:
> On Thu, Jan 18, 2007 at 09:14:06AM +0100, Martin Mares wrote:
> > Hello!
> >
> > > I recommend we just delete the pci_bus class. I don't think it
> > > serves any useful purpose. The bridge can be inferred frmo the
> > > sysfs hierarchy (not to mention lspci will tell you). The
> > > cpuaffinity file should be moved from the bus to the device -- it
> > > really doesn't make any sense to talk about which cpu a bus is
> > > affine to, only a device.
> >
> > It doesn't seem to serve any useful purpose other than the affinity
> > now, but I still think that it conceptually belongs there, because
> > it makes sense to have per-bus attributes. For example, in the
> > future we could show data width and signalling speed.
>
> So, if it were to stay, where in the tree should it be? Hanging off
> of the pci device that is the bridge? Or just placing these files
> within the pci device directory itself, as it is the bridge.
>
> There are also some "legacy io" binary sysfs files in these
> directories for those platforms that support it (#ifdef
> HAVE_PCI_LEGACY), and I'm guessing that there is some user for them
> out there, otherwise they would not have been added...
>
> Hm, only ia64 enables that option. Matthew, do you care about those
> files?
Those interfaces could be supported on other platforms (iirc benh has
been planning to add legacy_* support to ppc for awhile). It would be
great if they were supported everywhere to provide userspace with a
fairly sane way to get at this stuff on all Linux systems...
Should be trivial on i386 I think.
Jesse
Hello!
> So, if it were to stay, where in the tree should it be? Hanging off of
> the pci device that is the bridge? Or just placing these files within
> the pci device directory itself, as it is the bridge.
I originally didn't realize that we already represent devices on the
subordinate bus as subdirectories of the bridge device's directory,
without any extra level of abstraction. So it probably makes sense
to put the bus-specific files there or, in case of a root bus, to
/sys/devices/pciXXXX:YY/.
Have a nice fortnight
--
Martin `MJ' Mares <[email protected]> http://mj.ucw.cz/
Faculty of Math and Physics, Charles University, Prague, Czech Rep., Earth
"It is only with the heart that one can see rightly; What is essential is invisible to the eye." -- The Little Prince
On Thu, Jan 18, 2007 at 09:14:06AM +0100, Martin Mares wrote:
> Hello!
>
> > I recommend we just delete the pci_bus class. I don't think it serves
> > any useful purpose. The bridge can be inferred frmo the sysfs hierarchy
> > (not to mention lspci will tell you). The cpuaffinity file should be
> > moved from the bus to the device -- it really doesn't make any sense to
> > talk about which cpu a bus is affine to, only a device.
>
> It doesn't seem to serve any useful purpose other than the affinity now,
> but I still think that it conceptually belongs there, because it makes
> sense to have per-bus attributes. For example, in the future we could
> show data width and signalling speed.
Other per bus attributes might be address routing, VGA routing enabled,
Fast-back-to-back enabled. PCI-X bridges and PCI-e bridges might also
advertise data related to MMRBC and similar onboard buffer mgt behaviors.
ISTR, IBM PCI-X bridge works better with 512 "block" (data xfer size)
than larger sizes becuase it internally allocates buffer space
in 512B chunks. It would be useful to know along with downstream
device MMRBC. Not sure this all has to come from /sys though.
grant
On 1/18/07, Greg KH <[email protected]> wrote:
> Hm, only ia64 enables that option. Matthew, do you care about those
> files?
Given the ia64 nature, unless benh was truly wanting to do something
or ppc64, IBM's big NUMA boxes are pretty unlikely to care.
Tim
On Tue, 2007-01-23 at 16:22 -0800, Tim Pepper wrote:
> On 1/18/07, Greg KH <[email protected]> wrote:
>
> > Hm, only ia64 enables that option. Matthew, do you care about those
> > files?
>
> Given the ia64 nature, unless benh was truly wanting to do something
> or ppc64, IBM's big NUMA boxes are pretty unlikely to care.
I'd still like to do something at one point for ppc and ppc64... or
others. It's a generic problem with non-x86 machines.
In addition, there is the problem of legacy VGA arbitration
Part of the problem with legacy VGA accesses is also that it needs some
level of arbitration which we currently don't do in the kernel at all. X
does some stuff, but it doesn't necessarily arbitrate well with other
things for obvious reasons :-)
I wrote some code for that a little while ago but never quite finished,
and X would have had to be ported over. I might give it a go again one
of these days though. Now that X is slowly moving over some sane PCI
access library, there is a good opportunity to slap in support for a
nice kernel based VGA access arbitration interface.
Ben.
On Thu, Jan 18, 2007 at 11:58:05PM -0700, Grant Grundler wrote:
> Other per bus attributes might be address routing, VGA routing enabled,
> Fast-back-to-back enabled. PCI-X bridges and PCI-e bridges might also
> advertise data related to MMRBC and similar onboard buffer mgt behaviors.
>
> ISTR, IBM PCI-X bridge works better with 512 "block" (data xfer size)
> than larger sizes becuase it internally allocates buffer space
> in 512B chunks. It would be useful to know along with downstream
> device MMRBC. Not sure this all has to come from /sys though.
Most of this comes best from lspci / setpci.
On Thu, Jan 18, 2007 at 01:00:44AM -0800, Greg KH wrote:
> On Thu, Jan 18, 2007 at 09:14:06AM +0100, Martin Mares wrote:
> > Hello!
> >
> > > I recommend we just delete the pci_bus class. I don't think it serves
> > > any useful purpose. The bridge can be inferred frmo the sysfs hierarchy
> > > (not to mention lspci will tell you). The cpuaffinity file should be
> > > moved from the bus to the device -- it really doesn't make any sense to
> > > talk about which cpu a bus is affine to, only a device.
> >
> > It doesn't seem to serve any useful purpose other than the affinity now,
> > but I still think that it conceptually belongs there, because it makes
> > sense to have per-bus attributes. For example, in the future we could
> > show data width and signalling speed.
Data width is kind of hard to figure out since it's negotiated per
transaction. One could conceive of a device which only does 32-bit
transactions to some addresses, and 64-bit transactions to others.
What I've done in recent patches is make these kinds of attributes
available per slot.
> So, if it were to stay, where in the tree should it be? Hanging off of
> the pci device that is the bridge? Or just placing these files within
> the pci device directory itself, as it is the bridge.
/sys/bus/pci/busses ?
> There are also some "legacy io" binary sysfs files in these directories
> for those platforms that support it (#ifdef HAVE_PCI_LEGACY), and I'm
> guessing that there is some user for them out there, otherwise they
> would not have been added...
>
> Hm, only ia64 enables that option. Matthew, do you care about those
> files?
I think they were added for Altix ... not sure who uses them. Maybe X?
On Saturday, January 27, 2007 8:19 am, Matthew Wilcox wrote:
> > There are also some "legacy io" binary sysfs files in these
> > directories for those platforms that support it (#ifdef
> > HAVE_PCI_LEGACY), and I'm guessing that there is some user for them
> > out there, otherwise they would not have been added...
> >
> > Hm, only ia64 enables that option. Matthew, do you care about
> > those files?
>
> I think they were added for Altix ... not sure who uses them. Maybe
> X?
Yes. Though like I said, those interfaces make sense on other platforms
too, with weird ISA I/O and memory space mapping requirements.
Jesse