Hi Bin, On 28 July 2015 at 01:47, Bin Meng <bmeng...@gmail.com> wrote: > Hi Simon, > > On Tue, Jul 28, 2015 at 5:47 AM, Simon Glass <s...@chromium.org> wrote: >> At present there are no PCI functions which allow access to PCI >> configuration using a struct udevice. This is a sad situation for driver >> model as it makes use of PCI harder. Add these functions. >> >> Signed-off-by: Simon Glass <s...@chromium.org> >> --- >> >> drivers/pci/pci-uclass.c | 76 >> ++++++++++++++++++++++++++++++++++++++++++++++++ >> include/pci.h | 17 +++++++++++ >> 2 files changed, 93 insertions(+) >> >> diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c >> index c7d93f9..8d3536c 100644 >> --- a/drivers/pci/pci-uclass.c >> +++ b/drivers/pci/pci-uclass.c >> @@ -210,6 +210,17 @@ int pci_write_config(pci_dev_t bdf, int offset, >> unsigned long value, >> return pci_bus_write_config(bus, bdf, offset, value, size); >> } >> >> +int dm_pci_write_config(struct udevice *dev, int offset, unsigned long >> value, >> + enum pci_size_t size) >> +{ >> + struct udevice *bus; >> + >> + for (bus = dev; device_get_uclass_id(bus->parent) == UCLASS_PCI;) >> + bus = bus->parent; > > I think we can simplify this by just doing: > > int dm_pci_write_config(struct udevice *dev, int offset, unsigned long value, > enum pci_size_t size) > { > return pci_write_config(pci_get_bdf(dev), offset, value, size); > }
Yes but actually I want to remove those old functions. We should support multiple PCI controllers and reference everything from a device, I think. > >> + return pci_bus_write_config(bus, pci_get_bdf(dev), offset, value, >> size); >> +} >> + >> + >> int pci_write_config32(pci_dev_t bdf, int offset, u32 value) >> { >> return pci_write_config(bdf, offset, value, PCI_SIZE_32); >> @@ -225,6 +236,21 @@ int pci_write_config8(pci_dev_t bdf, int offset, u8 >> value) >> return pci_write_config(bdf, offset, value, PCI_SIZE_8); >> } >> >> +int dm_pci_write_config8(struct udevice *dev, int offset, u8 value) >> +{ >> + return dm_pci_write_config(dev, offset, value, PCI_SIZE_8); >> +} >> + >> +int dm_pci_write_config16(struct udevice *dev, int offset, u16 value) >> +{ >> + return dm_pci_write_config(dev, offset, value, PCI_SIZE_16); >> +} >> + >> +int dm_pci_write_config32(struct udevice *dev, int offset, u32 value) >> +{ >> + return dm_pci_write_config(dev, offset, value, PCI_SIZE_32); >> +} >> + >> int pci_bus_read_config(struct udevice *bus, pci_dev_t bdf, int offset, >> unsigned long *valuep, enum pci_size_t size) >> { >> @@ -249,6 +275,17 @@ int pci_read_config(pci_dev_t bdf, int offset, unsigned >> long *valuep, >> return pci_bus_read_config(bus, bdf, offset, valuep, size); >> } >> >> +int dm_pci_read_config(struct udevice *dev, int offset, unsigned long >> *valuep, >> + enum pci_size_t size) >> +{ >> + struct udevice *bus; >> + >> + for (bus = dev; device_get_uclass_id(bus->parent) == UCLASS_PCI;) >> + bus = bus->parent; > > Ditto. > >> + return pci_bus_read_config(bus, pci_get_bdf(dev), offset, valuep, >> + size); >> +} >> + >> int pci_read_config32(pci_dev_t bdf, int offset, u32 *valuep) >> { >> unsigned long value; >> @@ -288,6 +325,45 @@ int pci_read_config8(pci_dev_t bdf, int offset, u8 >> *valuep) >> return 0; >> } >> >> +int dm_pci_read_config8(struct udevice *dev, int offset, u8 *valuep) >> +{ >> + unsigned long value; >> + int ret; >> + >> + ret = dm_pci_read_config(dev, offset, &value, PCI_SIZE_8); >> + if (ret) >> + return ret; >> + *valuep = value; >> + >> + return 0; >> +} >> + >> +int dm_pci_read_config16(struct udevice *dev, int offset, u16 *valuep) >> +{ >> + unsigned long value; >> + int ret; >> + >> + ret = dm_pci_read_config(dev, offset, &value, PCI_SIZE_16); >> + if (ret) >> + return ret; >> + *valuep = value; >> + >> + return 0; >> +} >> + >> +int dm_pci_read_config32(struct udevice *dev, int offset, u32 *valuep) >> +{ >> + unsigned long value; >> + int ret; >> + >> + ret = dm_pci_read_config(dev, offset, &value, PCI_SIZE_32); >> + if (ret) >> + return ret; >> + *valuep = value; >> + >> + return 0; >> +} >> + >> int pci_auto_config_devices(struct udevice *bus) >> { >> struct pci_controller *hose = bus->uclass_priv; >> diff --git a/include/pci.h b/include/pci.h >> index 26cd80b..0bb3090 100644 >> --- a/include/pci.h >> +++ b/include/pci.h >> @@ -947,6 +947,23 @@ int pci_bus_read_config(struct udevice *bus, pci_dev_t >> bdf, int offset, >> int pci_bus_write_config(struct udevice *bus, pci_dev_t bdf, int offset, >> unsigned long value, enum pci_size_t size); >> >> +/** >> + * Driver mode PCI config access functions. Use these in preference to >> others > > Driver model > >> + * when you have a valid device >> + */ >> +int dm_pci_read_config(struct udevice *dev, int offset, unsigned long >> *valuep, >> + enum pci_size_t size); >> +int dm_pci_read_config8(struct udevice *dev, int offset, u8 *valuep); >> +int dm_pci_read_config16(struct udevice *dev, int offset, u16 *valuep); >> +int dm_pci_read_config32(struct udevice *dev, int offset, u32 *valuep); >> + >> +int dm_pci_write_config(struct udevice *dev, int offset, unsigned long >> value, >> + enum pci_size_t size); >> + > > Please either remove blank line here, or add a blank line between > dm_pci_read_config() and dm_pci_read_config8() above. > >> +int dm_pci_write_config8(struct udevice *dev, int offset, u8 value); >> +int dm_pci_write_config16(struct udevice *dev, int offset, u16 value); >> +int dm_pci_write_config32(struct udevice *dev, int offset, u32 value); >> + >> /* >> * The following functions provide access to the above without needing the >> * size parameter. We are trying to encourage the use of the 8/16/32-style >> -- > > Regards, > Bin Regards, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot