Re: [LinuxBIOS] [PATCH] Add more PCI config data to device structure

[Uwe Hermann]

On Tue, Sep 11, 2007 at 11:29:38PM +0200, Jens Freimann wrote:
> this is a patch to add more data from the PCI configuration space to the
> device struct. It is needed for example to create a device tree in Open
> Firmware from the LB device tree.
>
> There is one patch vor v2 and another one for v3.
>
> Bye,
> Jens
> Add more information to the device structure, so an Open Firmware
> device tree can be created from data in struct device.
>
> Signed-off-by: Jens Freimann
> Index: src/devices/pci_device.c
>
> --- src/devices/pci_device.c	(revision 2770)
> +++ src/devices/pci_device.c	(working copy)
> @@ -972,7 +972,34 @@
>    /* Read the rest of the pci configuration information */
>    hdr_type = pci_read_config8(dev, PCI_HEADER_TYPE);
>    class = pci_read_config32(dev, PCI_CLASS_REVISION);
> u16 status = pci_read_config16(dev, PCI_STATUS);
> dev->status = status;
> u8 revision = pci_read_config8(dev, PCI_REVISION_ID);
> dev->revision = revision;
> u8 cache_line = pci_read_config8(dev, PCI_CACHE_LINE_SIZE);
> dev->cache_line = cache_line;
> u8 irq_line = pci_read_config8(dev, PCI_INTERRUPT_LINE);
> dev->irq_line = irq_line;
> u8 irq_pin = pci_read_config8(dev, PCI_INTERRUPT_PIN);
> dev->irq_pin = irq_pin;
> u8 min_gnt = pci_read_config8(dev, PCI_MIN_GNT);
> dev->min_gnt = min_gnt;
> u8 max_lat = pci_read_config8(dev, PCI_MAX_LAT);
> dev->max_lat = max_lat;
> u16 subsystem_vendor = pci_read_config16(dev, PCI_SUBSYSTEM_VENDOR_ID);
> dev->subsystem_vendor = subsystem_vendor;
> u16 subsystem_device = pci_read_config16(dev, PCI_SUBSYSTEM_ID);
> dev->subsystem_device = subsystem_device;

Is there a reaons why this wouldn't work?

dev->status = pci_read_config16(dev, PCI_STATUS);
dev->revision = pci_read_config8(dev, PCI_REVISION_ID);
dev->cache_line = pci_read_config8(dev, PCI_CACHE_LINE_SIZE);
dev->irq_line = pci_read_config8(dev, PCI_INTERRUPT_LINE);
dev->irq_pin = pci_read_config8(dev, PCI_INTERRUPT_PIN);
dev->min_gnt = pci_read_config8(dev, PCI_MIN_GNT);
dev->max_lat = pci_read_config8(dev, PCI_MAX_LAT);
dev->subsystem_vendor = pci_read_config16(dev, PCI_SUBSYSTEM_VENDOR_ID);
dev->subsystem_device = pci_read_config16(dev, PCI_SUBSYSTEM_ID);

(much shorter)
> /* Store the interesting information in the device structure */
>  dev->vendor = id & 0xffff;
>  dev->device = (id >> 16) & 0xffff;
> Index: src/include/device/device.h
>
> --- src/include/device/device.h	(revision 2770)
> +++ src/include/device/device.h	(working copy)
> @@ -68,9 +68,19 @@
>    device_t	sibling;	/* next device on this bus */
>    device_t	next;		/* chain of all devices */
> char dtsname[64];
> struct device_path path;
> unsigned 	vendor;
> unsigned 	device;
> u16 status;
> u8 revision;
> u8 cache_line;
> u8 irq_line;
> u8 irq_pin;
> u8 min_gnt;
> u8 max_lat;
> u16 subsystem_vendor;
> u16 subsystem_device;

Use TABs for indentation as per coding guidelines, please.

The patch looks good to me otherwise, but someone with more OFW
knowledge should probably look over it, too.

Uwe.