[U-Boot] [PATCH 04/15] dm: pci: Provide friendly config access functions

[Bin Meng]

Hi Simon,

On Tue, Jul 28, 2015 at 5:47 AM, Simon Glass <sjg at 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 <sjg at 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);
}

> +       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