Changeset View
Standalone View
sys/compat/linuxkpi/common/include/linux/pci.h
| Context not available. | |||||
| #define PCI_DEVICE_ID_MELLANOX_SINAI 0x6274 | #define PCI_DEVICE_ID_MELLANOX_SINAI 0x6274 | ||||
| #define PCI_SUBDEVICE_ID_QEMU 0x1100 | #define PCI_SUBDEVICE_ID_QEMU 0x1100 | ||||
| #define PCI_DEVFN(slot, func) ((((slot) & 0x1f) << 3) | ((func) & 0x07)) | #define PCI_DEVFN(slot, func) ((((slot) & 0x1f) << 8) | (func)) | ||||
hselasky: These definitions do no longer match what Linux defines them to be.
Are you sure this is OK? | |||||
hselaskyUnsubmitted Not Done Inline ActionsSee the mlx4 driver for example, how it assumes how the current macros work. static int extended_func_num(struct pci_dev *pdev)
{
return PCI_SLOT(pdev->devfn) * 8 + PCI_FUNC(pdev->devfn);
}hselasky: See the mlx4 driver for example, how it assumes how the current macros work.
```
static int… | |||||
manuAuthorUnsubmitted Done Inline ActionsIt doesn't matter as we have setup the devfn with the new macro. manu: It doesn't matter as we have setup the devfn with the new macro. | |||||
Not Done Inline ActionsIs there a reason we do lose the & 0x07 on the (func) ? bz: Is there a reason we do lose the & 0x07 on the (func) ? | |||||
Not Done Inline ActionsFor ARI devices. slot is zero and function takes all lower byte. So these definitions are compatible both with usual (legacy bleh) addressing and with ARI. kib: For ARI devices. slot is zero and function takes all lower byte. So these definitions are… | |||||
| #define PCI_SLOT(devfn) (((devfn) >> 3) & 0x1f) | #define PCI_SLOT(devfn) (((devfn) >> 8) & 0x1f) | ||||
kibUnsubmitted Done Inline ActionsThis does not work with non-ARI devices. kib: This does not work with non-ARI devices. | |||||
| #define PCI_FUNC(devfn) ((devfn) & 0x07) | #define PCI_FUNC(devfn) ((devfn) & 0xff) | ||||
| #define PCI_BUS_NUM(devfn) (((devfn) >> 8) & 0xff) | #define PCI_BUS_NUM(devfn) (((devfn) >> 16) & 0xff) | ||||
| #define PCI_VDEVICE(_vendor, _device) \ | #define PCI_VDEVICE(_vendor, _device) \ | ||||
| .vendor = PCI_VENDOR_ID_##_vendor, .device = (_device), \ | .vendor = PCI_VENDOR_ID_##_vendor, .device = (_device), \ | ||||
| Context not available. | |||||
| } | } | ||||
| } | } | ||||
| static inline void | |||||
| lkpi_pci_save_state(struct pci_dev *pdev) | |||||
| { | |||||
| pci_save_state(pdev->dev.bsddev); | |||||
| } | |||||
| static inline void | |||||
| lkpi_pci_restore_state(struct pci_dev *pdev) | |||||
| { | |||||
| pci_restore_state(pdev->dev.bsddev); | |||||
| } | |||||
| #define pci_save_state(dev) lkpi_pci_save_state(dev) | |||||
| #define pci_restore_state(dev) lkpi_pci_restore_state(dev) | |||||
Done Inline ActionsDo we need to have the defines for inline functions? I thought in that case we could just use the original names directly? bz: Do we need to have the defines for inline functions? I thought in that case we could just use… | |||||
Done Inline ActionsNope, this was a copy paste from drm-kmod but this should be directly using the linux name. Will change that. manu: Nope, this was a copy paste from drm-kmod but this should be directly using the linux name. | |||||
Done Inline ActionsWell no it's needed as we have the same functions defined in pcivar.h so I'll just rename them to linuxkpi_ manu: Well no it's needed as we have the same functions defined in pcivar.h so I'll just rename them… | |||||
Not Done Inline ActionsCan we make the macros arguments for both of these also pdev instead dev to avoid possible confusion? bz: Can we make the macros arguments for both of these also pdev instead dev to avoid possible… | |||||
| #define DEFINE_PCI_DEVICE_TABLE(_table) \ | #define DEFINE_PCI_DEVICE_TABLE(_table) \ | ||||
| const struct pci_device_id _table[] __devinitdata | const struct pci_device_id _table[] __devinitdata | ||||
| Context not available. | |||||
| return (0); | return (0); | ||||
| } | } | ||||
| static inline bool | |||||
| pci_is_root_bus(struct pci_bus *pbus) | |||||
| { | |||||
| return (pbus->self == NULL); | |||||
| } | |||||
| struct pci_dev *lkpi_pci_get_domain_bus_and_slot(int domain, | |||||
Done Inline Actionslong line. bz: long line. | |||||
| unsigned int bus, unsigned int devfn); | |||||
| #define pci_get_domain_bus_and_slot(domain, bus, devfn) \ | |||||
| lkpi_pci_get_domain_bus_and_slot(domain, bus, devfn) | |||||
| static inline int | |||||
Done Inline ActionsIs there are reason this is not an inline function? bz: Is there are reason this is not an inline function? | |||||
Done Inline ActionsGuess it could be. Will change. manu: Guess it could be. Will change. | |||||
| pci_domain_nr(struct pci_bus *pbus) | |||||
| { | |||||
| return (pci_get_domain(pbus->self->dev.bsddev)); | |||||
Not Done Inline Actionsreturn (pbus->domain); It is now initialised in lkpifill_pci_dev() so should be done for all paths, so we can simply return it. bz: ```return (pbus->domain);```
It is now initialised in lkpifill_pci_dev() so should be done for… | |||||
Done Inline ActionsTrue, will change. manu: True, will change. | |||||
Not Done Inline ActionsSo this is still not done ? kib: So this is still not done ? | |||||
| } | |||||
| static inline int | |||||
| pci_bus_read_config(struct pci_bus *bus, unsigned int devfn, | |||||
| int pos, uint32_t *val, int len) | |||||
Not Done Inline ActionsStyle: 4 spaces indent for continuation line. kib: Style: 4 spaces indent for continuation line. | |||||
| { | |||||
| device_t dev; | |||||
Not Done Inline ActionsI actually do not understand this function. What devfn argument is for ? kib: I actually do not understand this function. What devfn argument is for ? | |||||
| dev = pci_find_dbsf(pci_get_domain(bus->self->dev.bsddev), | |||||
| pci_get_bus(bus->self->dev.bsddev), | |||||
| PCI_SLOT(devfn), | |||||
| PCI_FUNC(devfn)); | |||||
Done Inline ActionsThis breaks for ARI devices. In particular, PCI_FUNC() is broken, it truncates the function address. Arguably it is more an issue with PCI_FUNC() macro. kib: This breaks for ARI devices. In particular, PCI_FUNC() is broken, it truncates the function… | |||||
Done Inline ActionsWhat's ARI devices ? manu: What's ARI devices ? | |||||
Done Inline ActionsARI is PCIe cap which indicates that instead of using the second byte of the device address for slot:func, whole second byte is dedicated to the function number. This way device might have 255 siblings instead of 31, which is useful/needed for things like SRIOV. kib: ARI is PCIe cap which indicates that instead of using the second byte of the device address for… | |||||
| *val = pci_read_config(dev, pos, len); | |||||
| return (0); | |||||
| } | |||||
| static inline int | |||||
| pci_bus_read_config_word(struct pci_bus *bus, unsigned int devfn, int pos, u16 *val) | |||||
| { | |||||
| uint32_t tmp; | |||||
Not Done Inline ActionsThis doesn't look right. I think you need a temporary uint32_t variable before storing into uint16_t. Ditto below. hselasky: This doesn't look right. I think you need a temporary uint32_t variable before storing into… | |||||
Done Inline ActionsYes this only work on little-endian. kib: Yes this only work on little-endian. | |||||
Done Inline ActionsRight ok, will fix. manu: Right ok, will fix.
This is the only part not tested as it's only used by the radeon and I… | |||||
| int ret; | |||||
| ret = pci_bus_read_config(bus, devfn, pos, &tmp, 2); | |||||
| *val = (u16)tmp; | |||||
| return (ret); | |||||
| } | |||||
| static inline int | |||||
| pci_bus_read_config_byte(struct pci_bus *bus, unsigned int devfn, int pos, u8 *val) | |||||
| { | |||||
| uint32_t tmp; | |||||
| int ret; | |||||
| ret = pci_bus_read_config(bus, devfn, pos, &tmp, 1); | |||||
| *val = (u8)tmp; | |||||
| return (ret); | |||||
| } | |||||
Done Inline ActionsSame. kib: Same. | |||||
| static inline int | |||||
| pci_bus_write_config(struct pci_bus *bus, unsigned int devfn, int pos, | |||||
| uint32_t val, int size) | |||||
| { | |||||
| device_t dev; | |||||
| dev = pci_find_dbsf(pci_get_domain(bus->self->dev.bsddev), | |||||
kibUnsubmitted Done Inline ActionsThis does not work with ARI. kib: This does not work with ARI. | |||||
manuAuthorUnsubmitted Done Inline ActionsOk, how so ? manu: Ok, how so ? | |||||
kibUnsubmitted Done Inline ActionsI see, you changed PCI_SLOT()/PCI_FUNC() definitions. But now PCI_SLOT() would break With this KPI, I believe you must use PCIB_ARI_ENABLED() to get it right. kib: I see, you changed PCI_SLOT()/PCI_FUNC() definitions. But now PCI_SLOT() would break
for non… | |||||
| pci_get_bus(bus->self->dev.bsddev), | |||||
| PCI_SLOT(devfn), | |||||
| PCI_FUNC(devfn)); | |||||
| pci_write_config(dev, pos, val, size); | |||||
| return (0); | |||||
| } | |||||
| static inline int | |||||
| pci_bus_write_config_byte(struct pci_bus *bus, unsigned int devfn, int pos, | |||||
| uint8_t val) | |||||
| { | |||||
| return (pci_bus_write_config(bus, devfn, pos, val, 1)); | |||||
| } | |||||
| static inline int | |||||
| pci_bus_write_config_word(struct pci_bus *bus, unsigned int devfn, int pos, | |||||
| uint16_t val) | |||||
| { | |||||
| return (pci_bus_write_config(bus, devfn, pos, val, 2)); | |||||
| } | |||||
| struct pci_dev *lkpi_pci_get_class(unsigned int class, struct pci_dev *from); | |||||
| #define pci_get_class(class, from) lkpi_pci_get_class(class, from) | |||||
| #endif /* _LINUX_PCI_H_ */ | #endif /* _LINUX_PCI_H_ */ | ||||
| Context not available. | |||||
These definitions do no longer match what Linux defines them to be.
Are you sure this is OK?