I'd kind of like to get rid of pci_dev->slot_name. It's redundant with
pci_dev->dev.bus_id, but that's one hell of a search and replace job.
So let me propose pci_name(pci_dev) as a replacement. That has the
benefit of being shorter than either of the others and lets us do fun
& interesting things later (maybe construct it on the fly for systems
that want to save 20 bytes per device?). We can transition it in over
2.5/2.6/2.7 and kill pci_dev->slot_name for 2.8.
Oh, and without killing slot_name immediately, we can save 4 bytes on
32-bit platforms by turning it into a pointer to the dev.bus_id.
Comments?
--
"It's not Hollywood. War is real, war is primarily not about defeat or
victory, it is about death. I've seen thousands and thousands of dead bodies.
Do you think I want to have an academic debate on this subject?" -- Robert Fisk
Matthew Wilcox wrote:
> I'd kind of like to get rid of pci_dev->slot_name. It's redundant with
> pci_dev->dev.bus_id, but that's one hell of a search and replace job.
> So let me propose pci_name(pci_dev) as a replacement. That has the
> benefit of being shorter than either of the others and lets us do fun
> & interesting things later (maybe construct it on the fly for systems
and also it's nicely backwards compatible (source-wise), too.
Jeff
On Thu, Jun 26, 2003 at 12:35:25AM +0100, Matthew Wilcox wrote:
>
> I'd kind of like to get rid of pci_dev->slot_name. It's redundant with
> pci_dev->dev.bus_id, but that's one hell of a search and replace job.
> So let me propose pci_name(pci_dev) as a replacement. That has the
> benefit of being shorter than either of the others and lets us do fun
> & interesting things later (maybe construct it on the fly for systems
> that want to save 20 bytes per device?). We can transition it in over
> 2.5/2.6/2.7 and kill pci_dev->slot_name for 2.8.
That sounds reasonable. But do we really need to do this for 2.6?
Just trying to keep things sane...
thanks,
greg k-h
On Wed, Jun 25, 2003 at 05:36:20PM -0700, Greg KH wrote:
> On Thu, Jun 26, 2003 at 12:35:25AM +0100, Matthew Wilcox wrote:
> >
> > I'd kind of like to get rid of pci_dev->slot_name. It's redundant with
> > pci_dev->dev.bus_id, but that's one hell of a search and replace job.
> > So let me propose pci_name(pci_dev) as a replacement. That has the
>
> That sounds reasonable. But do we really need to do this for 2.6?
>
> Just trying to keep things sane...
I think we really do need to introduce pci_name() for 2.6 (and put it
in 2.4 too). We don't need to eliminate pci_dev->slot_name for 2.6,
but drivers that care need to be able to tell the user which card is
a message is referring to. With overlapping pci bus numbers, the 8
bytes of bus:device.func is no longer unique, so we need to report the
domain number too.
That information's already placed in bus_id, but as I said, I don't
want to start converting all the drivers. We could just make slot_name
larger (Anton posted a patch for this) but I don't want to make pci_dev
even bigger. Having a nice interface like pci_name() makes drivers more
portable between 2.4, 2.6 and 2.8 (as Jeff pointed out).
--
"It's not Hollywood. War is real, war is primarily not about defeat or
victory, it is about death. I've seen thousands and thousands of dead bodies.
Do you think I want to have an academic debate on this subject?" -- Robert Fisk
On Thu, Jun 26, 2003 at 01:53:15AM +0100, Matthew Wilcox wrote:
> On Wed, Jun 25, 2003 at 05:36:20PM -0700, Greg KH wrote:
> > On Thu, Jun 26, 2003 at 12:35:25AM +0100, Matthew Wilcox wrote:
> > >
> > > I'd kind of like to get rid of pci_dev->slot_name. It's redundant with
> > > pci_dev->dev.bus_id, but that's one hell of a search and replace job.
> > > So let me propose pci_name(pci_dev) as a replacement. That has the
> >
> > That sounds reasonable. But do we really need to do this for 2.6?
> >
> > Just trying to keep things sane...
>
> I think we really do need to introduce pci_name() for 2.6 (and put it
> in 2.4 too). We don't need to eliminate pci_dev->slot_name for 2.6,
> but drivers that care need to be able to tell the user which card is
> a message is referring to. With overlapping pci bus numbers, the 8
> bytes of bus:device.func is no longer unique, so we need to report the
> domain number too.
>
> That information's already placed in bus_id, but as I said, I don't
> want to start converting all the drivers. We could just make slot_name
> larger (Anton posted a patch for this) but I don't want to make pci_dev
> even bigger. Having a nice interface like pci_name() makes drivers more
> portable between 2.4, 2.6 and 2.8 (as Jeff pointed out).
Ok, I'll buy that, feel free to send the patch :)
thanks,
greg k-h
> I'd kind of like to get rid of pci_dev->slot_name. It's redundant with
> pci_dev->dev.bus_id, but that's one hell of a search and replace job.
> So let me propose pci_name(pci_dev) as a replacement. That has the
> benefit of being shorter than either of the others and lets us do fun
> & interesting things later (maybe construct it on the fly for systems
> that want to save 20 bytes per device?). We can transition it in over
> 2.5/2.6/2.7 and kill pci_dev->slot_name for 2.8.
>
> Oh, and without killing slot_name immediately, we can save 4 bytes on
> 32-bit platforms by turning it into a pointer to the dev.bus_id.
Works for me, I wanted to hijack pci_dev->slot_name (or the equivalent) so
we print both domain:bus:devfn as well as the system location on ppc64.
The system location makes it easier to work out which slot the card is in.
I dont necessarily need pci_name() since I could do what I want if
pci_dev->slot_name becomes a pointer.
Anton
On Wed, Jun 25, 2003 at 06:02:40PM -0700, Greg KH wrote:
> Ok, I'll buy that, feel free to send the patch :)
It's kind of late to be writing patches ... but this boots & works for me.
This patch introduces pci_name() and converts slot_name into a pointer to
dev.bus_id.
Index: drivers/pci/probe.c
===================================================================
RCS file: /var/cvs/linux-2.5/drivers/pci/probe.c,v
retrieving revision 1.16
diff -u -p -r1.16 probe.c
--- drivers/pci/probe.c 23 Jun 2003 03:30:26 -0000 1.16
+++ drivers/pci/probe.c 26 Jun 2003 02:45:53 -0000
@@ -106,7 +109,7 @@ static void pci_read_bases(struct pci_de
(((unsigned long) ~sz) << 32);
#else
if (l) {
- printk(KERN_ERR "PCI: Unable to handle 64-bit address for device %s\n", dev->slot_name);
+ printk(KERN_ERR "PCI: Unable to handle 64-bit address for device %s\n", pci_name(dev));
res->start = 0;
res->flags = 0;
continue;
@@ -301,7 +304,7 @@ int __devinit pci_scan_bridge(struct pci
pci_read_config_dword(dev, PCI_PRIMARY_BUS, &buses);
DBG("Scanning behind PCI bridge %s, config %06x, pass %d\n",
- dev->slot_name, buses & 0xffffff, pass);
+ pci_name(dev), buses & 0xffffff, pass);
if ((buses & 0xffff00) && !pcibios_assign_all_busses() && !is_cardbus) {
unsigned int cmax;
@@ -400,8 +403,9 @@ static int pci_setup_device(struct pci_d
{
u32 class;
- sprintf(dev->slot_name, "%02x:%02x.%d", dev->bus->number,
- PCI_SLOT(dev->devfn), PCI_FUNC(dev->devfn));
+ dev->slot_name = dev->dev.bus_id;
+ sprintf(pci_name(dev), "%04x:%02x:%02x.%d", pci_domain_nr(dev->bus),
+ dev->bus->number, PCI_SLOT(dev->devfn), PCI_FUNC(dev->devfn));
sprintf(dev->dev.name, "PCI device %04x:%04x",
dev->vendor, dev->device);
@@ -449,12 +453,12 @@ static int pci_setup_device(struct pci_d
default: /* unknown header */
printk(KERN_ERR "PCI: device %s has unknown header type %02x, ignoring.\n",
- dev->slot_name, dev->hdr_type);
+ pci_name(dev), dev->hdr_type);
return -1;
bad:
printk(KERN_ERR "PCI: %s: class %x doesn't match header type %02x. Ignoring class.\n",
- dev->slot_name, class, dev->hdr_type);
+ pci_name(dev), class, dev->hdr_type);
dev->class = PCI_CLASS_NOT_DEFINED;
}
@@ -528,9 +532,6 @@ pci_scan_device(struct pci_bus *bus, int
pci_name_device(dev);
- /* now put in global tree */
- sprintf(dev->dev.bus_id, "%04x:%s", pci_domain_nr(bus),
- dev->slot_name);
dev->dev.dma_mask = &dev->dma_mask;
return dev;
Index: include/linux/pci.h
===================================================================
RCS file: /var/cvs/linux-2.5/include/linux/pci.h,v
retrieving revision 1.18
diff -u -p -r1.18 pci.h
--- include/linux/pci.h 23 Jun 2003 03:30:53 -0000 1.18
+++ include/linux/pci.h 26 Jun 2003 02:45:54 -0000
@@ -414,7 +414,7 @@ struct pci_dev {
struct resource dma_resource[DEVICE_COUNT_DMA];
struct resource irq_resource[DEVICE_COUNT_IRQ];
- char slot_name[8]; /* slot name */
+ char * slot_name; /* pointer to dev.bus_id */
/* These fields are used by common fixups */
unsigned int transparent:1; /* Transparent PCI bridge */
@@ -802,6 +802,14 @@ static inline void *pci_get_drvdata (str
static inline void pci_set_drvdata (struct pci_dev *pdev, void *data)
{
dev_set_drvdata(&pdev->dev, data);
+}
+
+/* If you want to know what to call your pci_dev, ask this function.
+ * Again, it's a wrapper around the generic device.
+ */
+static inline char *pci_name(struct pci_dev *pdev)
+{
+ return pdev->dev.bus_id;
}
/*
--
"It's not Hollywood. War is real, war is primarily not about defeat or
victory, it is about death. I've seen thousands and thousands of dead bodies.
Do you think I want to have an academic debate on this subject?" -- Robert Fisk
On Thu, Jun 26, 2003 at 03:50:57AM +0100, Matthew Wilcox wrote:
> On Wed, Jun 25, 2003 at 06:02:40PM -0700, Greg KH wrote:
> > Ok, I'll buy that, feel free to send the patch :)
>
> It's kind of late to be writing patches ... but this boots & works for me.
Here's an example of converting a driver (tg3, just to annoy jeff ;-) to
pci_name().
Index: drivers/net/tg3.c
===================================================================
RCS file: /var/cvs/linux-2.5/drivers/net/tg3.c,v
retrieving revision 1.16
diff -u -p -r1.16 tg3.c
--- drivers/net/tg3.c 14 Jun 2003 22:15:21 -0000 1.16
+++ drivers/net/tg3.c 26 Jun 2003 02:45:54 -0000
@@ -5118,7 +5118,7 @@ static int tg3_ethtool_ioctl (struct net
strcpy (info.driver, DRV_MODULE_NAME);
strcpy (info.version, DRV_MODULE_VERSION);
memset(&info.fw_version, 0, sizeof(info.fw_version));
- strcpy (info.bus_info, pci_dev->slot_name);
+ strcpy (info.bus_info, pci_name(pci_dev));
info.eedump_len = 0;
info.regdump_len = TG3_REGDUMP_LEN;
if (copy_to_user (useraddr, &info, sizeof (info)))
@@ -6087,7 +6087,7 @@ static int __devinit tg3_get_invariants(
err = tg3_set_power_state(tp, 0);
if (err) {
printk(KERN_ERR PFX "(%s) transition to D0 failed\n",
- tp->pdev->slot_name);
+ pci_name(tp->pdev));
return err;
}
@@ -6198,7 +6198,7 @@ static int __devinit tg3_get_invariants(
err = tg3_phy_probe(tp);
if (err) {
printk(KERN_ERR PFX "(%s) phy probe failed, err %d\n",
- tp->pdev->slot_name, err);
+ pci_name(tp->pdev), err);
/* ... but do not return immediately ... */
}
@@ -6682,7 +6682,7 @@ static int __devinit tg3_init_one(struct
if (!pci_set_dma_mask(pdev, (u64) 0xffffffffffffffffULL)) {
pci_using_dac = 1;
if (pci_set_consistent_dma_mask(pdev,
- (u64) 0xffffffffffffffff)) {
+ (u64) 0xffffffffffffffffULL)) {
printk(KERN_ERR PFX "Unable to obtain 64 bit DMA "
"for consistent allocations\n");
goto err_out_free_res;
--
"It's not Hollywood. War is real, war is primarily not about defeat or
victory, it is about death. I've seen thousands and thousands of dead bodies.
Do you think I want to have an academic debate on this subject?" -- Robert Fisk
On Thu, Jun 26, 2003 at 03:50:57AM +0100, Matthew Wilcox wrote:
> This patch introduces pci_name() and converts slot_name into a pointer to
> dev.bus_id.
Here's the compatibility patch for 2.4:
--- linux/include/linux/pci.h 2003-05-21 05:21:29.000000000 -0400
+++ linux-acpi/include/linux/pci.h 2003-06-26 06:39:58.000000000 -0400
@@ -773,6 +773,11 @@
pdev->driver_data = data;
}
+static inline char *pci_name(struct pci_dev *pdev)
+{
+ return pdev->slot_name;
+}
+
/*
* The world is not perfect and supplies us with broken PCI devices.
* For at least a part of these bugs we need a work-around, so both
--
"It's not Hollywood. War is real, war is primarily not about defeat or
victory, it is about death. I've seen thousands and thousands of dead bodies.
Do you think I want to have an academic debate on this subject?" -- Robert Fisk