Re:[PATCH] PCI: add PCI Express Port Bus Driver subsystem

From: long
Date: Tue Jan 18 2005 - 19:57:08 EST

On Tue Jan 18 11:41:01 2005 Greg KH wrote:
>> >
>> >
>> > This puts all of the pcie "port" structures in /sys/devices/ Shouldn't
>> > you make the parent of the device you create point to the pci_dev
>> > structure that's passed into this function? That would make the sysfs
>> > tree a lot saner I think.
>>
>> The patch makes the parent of the device point to the pci_dev structure
>> that is passed into this function. If you think it is cleaner that the
>> patch should not, I will update the patch to reflect your input.
>
>That would be great, but it doesn't show up that way on my box. All of
>the portX devices are in /sys/devices/ which is what I don't think you
>want. I would love for them to have the parent of the pci_dev structure
>:)

Agree. Thanks for your inputs. The patch below include the changes based
on your previous post.

Signed-off-by: T. Long Nguyen <tom.l.nguyen@xxxxxxxxx>

---------------------------------------------------------------------
diff -urpN greg-patch/drivers/pci/pcie/portdrv_bus.c greg-patch-update/drivers/pci/pcie/portdrv_bus.c
--- greg-patch/drivers/pci/pcie/portdrv_bus.c 2005-01-18 15:59:38.000000000 -0500
+++ greg-patch-update/drivers/pci/pcie/portdrv_bus.c 2005-01-18 16:03:47.000000000 -0500
@@ -14,8 +14,6 @@

#include <linux/pcieport_if.h>

-static int generic_probe (struct device *dev) { return 0;}
-static int generic_remove (struct device *dev) { return 0;}
static int pcie_port_bus_match(struct device *dev, struct device_driver *drv);
static int pcie_port_bus_suspend(struct device *dev, u32 state);
static int pcie_port_bus_resume(struct device *dev);
@@ -27,23 +25,14 @@ struct bus_type pcie_port_bus_type = {
.resume = pcie_port_bus_resume,
};

-struct device_driver pcieport_generic_driver = {
- .name = "pcieport",
- .bus = &pcie_port_bus_type,
- .probe = generic_probe,
- .remove = generic_remove,
-};
-
static int pcie_port_bus_match(struct device *dev, struct device_driver *drv)
{
struct pcie_device *pciedev;
struct pcie_port_service_driver *driver;

- if ( drv->bus != &pcie_port_bus_type ||
- dev->bus != &pcie_port_bus_type ||
- drv == &pcieport_generic_driver) {
+ if (drv->bus != &pcie_port_bus_type || dev->bus != &pcie_port_bus_type)
return 0;
- }
+
pciedev = to_pcie_device(dev);
driver = to_service_driver(drv);
if ( (driver->id_table->vendor != PCI_ANY_ID &&
diff -urpN greg-patch/drivers/pci/pcie/portdrv_core.c greg-patch-update/drivers/pci/pcie/portdrv_core.c
--- greg-patch/drivers/pci/pcie/portdrv_core.c 2005-01-18 15:59:38.000000000 -0500
+++ greg-patch-update/drivers/pci/pcie/portdrv_core.c 2005-01-18 16:06:51.000000000 -0500
@@ -17,8 +17,6 @@

extern int pcie_mch_quirk; /* MSI-quirk Indicator */

-extern struct device_driver pcieport_generic_driver;
-
static int pcie_port_probe_service(struct device *dev)
{
struct pcie_device *pciedev;
@@ -103,6 +101,7 @@ static int pcie_port_resume_service(stru
*/
static void release_pcie_device(struct device *dev)
{
+ printk(KERN_DEBUG "Free Port Service[%s]\n", dev->bus_id);
kfree(to_pcie_device(dev));
}

@@ -217,18 +216,18 @@ static int get_port_device_capability(st
return services;
}

-static void pcie_device_init(struct pcie_device *parent,
- struct pcie_device *dev,
- int port_type, int service_type)
+static void pcie_device_init(struct pci_dev *parent, struct pcie_device *dev,
+ int port_type, int service_type, int irq, int irq_mode)
{
struct device *device;

- if (parent) {
- dev->id.vendor = parent->port->vendor;
- dev->id.device = parent->port->device;
- dev->id.port_type = port_type;
- dev->id.service_type = (1 << service_type);
- }
+ dev->port = parent;
+ dev->interrupt_mode = irq_mode;
+ dev->irq = irq;
+ dev->id.vendor = parent->vendor;
+ dev->id.device = parent->device;
+ dev->id.port_type = port_type;
+ dev->id.service_type = (1 << service_type);

/* Initialize generic device interface */
device = &dev->device;
@@ -240,35 +239,23 @@ static void pcie_device_init(struct pcie
device->driver = NULL;
device->driver_data = NULL;
device->release = release_pcie_device; /* callback to free pcie dev */
- sprintf(&device->bus_id[0], "%s.%02x", parent->device.bus_id,
- get_descriptor_id(port_type, service_type));
- device->parent = ((parent == NULL) ? NULL : &parent->device);
+ sprintf(&device->bus_id[0], "pcie%02x",
+ get_descriptor_id(port_type, service_type));
+ device->parent = &parent->dev;
}

-static struct pcie_device* alloc_pcie_device(
- struct pcie_device *parent, struct pci_dev *bridge,
+static struct pcie_device* alloc_pcie_device(struct pci_dev *parent,
int port_type, int service_type, int irq, int irq_mode)
{
struct pcie_device *device;
- static int NR_PORTS = 0;

device = kmalloc(sizeof(struct pcie_device), GFP_KERNEL);
if (!device)
return NULL;

memset(device, 0, sizeof(struct pcie_device));
- device->port = bridge;
- device->interrupt_mode = irq_mode;
- device->irq = irq;
- if (!parent) {
- pcie_device_init(NULL, device, port_type, service_type);
- NR_PORTS++;
- device->device.driver = &pcieport_generic_driver;
- sprintf(&device->device.bus_id[0], "port%d", NR_PORTS);
- } else {
- pcie_device_init(parent, device, port_type, service_type);
- }
- printk(KERN_DEBUG "Allocate Port Device[%s]\n", device->device.bus_id);
+ pcie_device_init(parent, device, port_type, service_type, irq,irq_mode);
+ printk(KERN_DEBUG "Allocate Port Service[%s]\n", device->device.bus_id);
return device;
}

@@ -291,7 +278,6 @@ int pcie_port_device_probe(struct pci_de

int pcie_port_device_register(struct pci_dev *dev)
{
- struct pcie_device *parent;
int status, type, capabilities, irq_mode, i;
int vectors[PCIE_PORT_DEVICE_MAXSERVICES];
u16 reg16;
@@ -306,27 +292,13 @@ int pcie_port_device_register(struct pci
capabilities = get_port_device_capability(dev);
irq_mode = assign_interrupt_mode(dev, vectors, capabilities);

- /* Allocate parent */
- parent = alloc_pcie_device(NULL, dev, type, 0, dev->irq, irq_mode);
- if (!parent)
- return -ENOMEM;
-
- status = device_register(&parent->device);
- if (status) {
- kfree(parent);
- return status;
- }
- get_device(&parent->device);
- pci_set_drvdata(dev, parent);
-
/* Allocate child services if any */
for (i = 0; i < PCIE_PORT_DEVICE_MAXSERVICES; i++) {
struct pcie_device *child;

if (capabilities & (1 << i)) {
child = alloc_pcie_device(
- parent, /* parent */
- dev, /* Root/Upstream/Downstream */
+ dev, /* parent */
type, /* port type */
i, /* service type */
vectors[i], /* irq */
@@ -345,17 +317,21 @@ int pcie_port_device_register(struct pci
}

#ifdef CONFIG_PM
-int pcie_port_device_suspend(struct pcie_device *dev, u32 state)
+int pcie_port_device_suspend(struct pci_dev *dev, u32 state)
{
- struct list_head *head;
+ struct list_head *head, *tmp;
struct device *parent, *child;
struct device_driver *driver;
struct pcie_port_service_driver *service_driver;

- parent = &dev->device;
+ parent = &dev->dev;
head = &parent->children;
- while (!list_empty(head)) {
- child = container_of(head->next, struct device, node);
+ tmp = head->next;
+ while (head != tmp) {
+ child = container_of(tmp, struct device, node);
+ tmp = tmp->next;
+ if (child->bus != &pcie_port_bus_type)
+ continue;
driver = child->driver;
if (!driver)
continue;
@@ -366,17 +342,21 @@ int pcie_port_device_suspend(struct pcie
return 0;
}

-int pcie_port_device_resume(struct pcie_device *dev)
+int pcie_port_device_resume(struct pci_dev *dev)
{
- struct list_head *head;
+ struct list_head *head, *tmp;
struct device *parent, *child;
struct device_driver *driver;
struct pcie_port_service_driver *service_driver;

- parent = &dev->device;
+ parent = &dev->dev;
head = &parent->children;
- while (!list_empty(head)) {
- child = container_of(head->next, struct device, node);
+ tmp = head->next;
+ while (head != tmp) {
+ child = container_of(tmp, struct device, node);
+ tmp = tmp->next;
+ if (child->bus != &pcie_port_bus_type)
+ continue;
driver = child->driver;
if (!driver)
continue;
@@ -389,45 +369,46 @@ int pcie_port_device_resume(struct pcie_
}
#endif

-void pcie_port_device_remove(struct pcie_device *dev)
+void pcie_port_device_remove(struct pci_dev *dev)
{
- struct list_head *head;
+ struct list_head *head, *tmp;
struct device *parent, *child;
struct device_driver *driver;
struct pcie_port_service_driver *service_driver;
+ int interrupt_mode = PCIE_PORT_INTx_MODE;

- parent = &dev->device;
+ parent = &dev->dev;
head = &parent->children;
- while (!list_empty(head)) {
- child = container_of(head->next, struct device, node);
+ tmp = head->next;
+ while (head != tmp) {
+ child = container_of(tmp, struct device, node);
+ tmp = tmp->next;
+ if (child->bus != &pcie_port_bus_type)
+ continue;
driver = child->driver;
if (driver) {
service_driver = to_service_driver(driver);
if (service_driver->remove)
service_driver->remove(to_pcie_device(child));
}
+ interrupt_mode = (to_pcie_device(child))->interrupt_mode;
put_device(child);
device_unregister(child);
}
-
/* Switch to INTx by default if MSI enabled */
- if (dev->interrupt_mode == PCIE_PORT_MSIX_MODE)
- pci_disable_msix(dev->port);
- else if (dev->interrupt_mode == PCIE_PORT_MSI_MODE)
- pci_disable_msi(dev->port);
- put_device(parent);
- device_unregister(parent);
+ if (interrupt_mode == PCIE_PORT_MSIX_MODE)
+ pci_disable_msix(dev);
+ else if (interrupt_mode == PCIE_PORT_MSI_MODE)
+ pci_disable_msi(dev);
}

void pcie_port_bus_register(void)
{
bus_register(&pcie_port_bus_type);
- driver_register(&pcieport_generic_driver);
}

void pcie_port_bus_unregister(void)
{
- driver_unregister(&pcieport_generic_driver);
bus_unregister(&pcie_port_bus_type);
}

diff -urpN greg-patch/drivers/pci/pcie/portdrv.h greg-patch-update/drivers/pci/pcie/portdrv.h
--- greg-patch/drivers/pci/pcie/portdrv.h 2005-01-18 15:59:38.000000000 -0500
+++ greg-patch-update/drivers/pci/pcie/portdrv.h 2005-01-18 16:08:33.000000000 -0500
@@ -28,14 +28,13 @@
#define get_descriptor_id(type, service) (((type - 4) << 4) | service)

extern struct bus_type pcie_port_bus_type;
-extern struct device_driver pcieport_generic_driver;
extern int pcie_port_device_probe(struct pci_dev *dev);
extern int pcie_port_device_register(struct pci_dev *dev);
#ifdef CONFIG_PM
-extern int pcie_port_device_suspend(struct pcie_device *dev, u32 state);
-extern int pcie_port_device_resume(struct pcie_device *dev);
+extern int pcie_port_device_suspend(struct pci_dev *dev, u32 state);
+extern int pcie_port_device_resume(struct pci_dev *dev);
#endif
-extern void pcie_port_device_remove(struct pcie_device *dev);
+extern void pcie_port_device_remove(struct pci_dev *dev);
extern void pcie_port_bus_register(void);
extern void pcie_port_bus_unregister(void);

diff -urpN greg-patch/drivers/pci/pcie/portdrv_pci.c greg-patch-update/drivers/pci/pcie/portdrv_pci.c
--- greg-patch/drivers/pci/pcie/portdrv_pci.c 2005-01-18 15:59:38.000000000 -0500
+++ greg-patch-update/drivers/pci/pcie/portdrv_pci.c 2005-01-18 16:39:07.000000000 -0500
@@ -63,34 +63,18 @@ static int __devinit pcie_portdrv_probe

static void pcie_portdrv_remove (struct pci_dev *dev)
{
- struct pcie_device *pciedev;
-
- pciedev = (struct pcie_device *)pci_get_drvdata(dev);
- if (pciedev) {
- pcie_port_device_remove(pciedev);
- pci_set_drvdata(dev, NULL);
- }
+ pcie_port_device_remove(dev);
}

#ifdef CONFIG_PM
static int pcie_portdrv_suspend (struct pci_dev *dev, u32 state)
{
- struct pcie_device *pciedev;
-
- pciedev = (struct pcie_device *)pci_get_drvdata(dev);
- if (pciedev)
- pcie_port_device_suspend(pciedev, state);
- return 0;
+ return pcie_port_device_suspend(dev, state);
}

static int pcie_portdrv_resume (struct pci_dev *dev)
{
- struct pcie_device *pciedev;
-
- pciedev = (struct pcie_device *)pci_get_drvdata(dev);
- if (pciedev)
- pcie_port_device_resume(pciedev);
- return 0;
+ return pcie_port_device_resume(dev);
}
#endif

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/