[U-Boot] [RESEND PATCHv4 8/9] dm: pci: add APIs for capability accessors

Z.q. Hou zhiqiang.hou at nxp.com
Mon Apr 1 04:21:57 UTC 2019


Hi Bin,

Thanks a lot for your comments!

> -----Original Message-----
> From: Bin Meng [mailto:bmeng.cn at gmail.com]
> Sent: 2019年4月1日 11:37
> To: Z.q. Hou <zhiqiang.hou at nxp.com>
> Cc: u-boot at lists.denx.de; albert.u.boot at aribaud.net; Priyanka Jain
> <priyanka.jain at nxp.com>; York Sun <york.sun at nxp.com>;
> sriram.dash at nxp.com; yamada.masahiro at socionext.com; Prabhakar
> Kushwaha <prabhakar.kushwaha at nxp.com>; Mingkai Hu
> <mingkai.hu at nxp.com>; M.h. Lian <minghuan.lian at nxp.com>
> Subject: Re: [RESEND PATCHv4 8/9] dm: pci: add APIs for capability accessors
> 
> On Mon, Mar 25, 2019 at 10:24 AM Z.q. Hou <zhiqiang.hou at nxp.com> wrote:
> >
> > From: Hou Zhiqiang <Zhiqiang.Hou at nxp.com>
> >
> > This patch introduce a set of PCI/PCIe capability accessors, including
> > 16-bit and 32-bit read, write and clear_and_set
> 
> No 8-bit accessors?

Is the 8-bit accessors needed? I'm not sure, will check.

> 
> > operations.
> >
> > Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou at nxp.com>
> > ---
> > V4:
> >  - New patch
> >
> >  drivers/pci/pci-uclass.c | 153
> +++++++++++++++++++++++++++++++++++++++
> >  include/pci.h            |  20 +++++
> >  2 files changed, 173 insertions(+)
> >
> > diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c index
> > 824fa11907..4bb30f5d2b 100644
> > --- a/drivers/pci/pci-uclass.c
> > +++ b/drivers/pci/pci-uclass.c
> > @@ -1443,6 +1443,159 @@ int dm_pci_find_ext_capability(struct udevice
> *dev, int cap)
> >         return dm_pci_find_next_ext_capability(dev, 0, cap);  }
> >
> > +/**
> > + * dm_pci_capability_read() - PCI capability register read
> > + *
> > + * @dev:       PCI device to read
> > + * @cap:       capability code
> > + * @pos:       register offset
> > + * @val:       pointer to keep the read value
> > + * @size:      register width
> > + *
> > + * Returns 0 if OK or appropriate error value.
> > + */
> 
> Please move the comment block to pci.h

Yes, will fix in v5.

> 
> > +int dm_pci_capability_read(struct udevice *dev, int cap, int pos,
> > +                          ulong *val, enum pci_size_t size) {
> > +       u32 off;
> > +
> > +       switch (size) {
> > +       case PCI_SIZE_16:
> > +               if (pos & 1)
> > +                       return -EINVAL;
> > +               break;
> > +       case PCI_SIZE_32:
> > +               if (pos & 3)
> > +                       return -EINVAL;
> > +               break;
> > +       default:
> > +               return -EINVAL;
> > +       }
> > +
> > +       off = dm_pci_find_capability(dev, cap);
> > +       if (off)
> > +               return dm_pci_read_config(dev, off + pos, val, size);
> > +
> > +       return -EINVAL;
> > +}
> > +
> > +/**
> > + * dm_pci_capability_write() - PCI capability register write
> > + *
> > + * @dev:       PCI device to write
> > + * @cap:       capability code
> > + * @pos:       register offset
> > + * @val:       value to write
> > + * @size:      register width
> > + *
> > + * Returns 0 if OK or appropriate error value.
> > + */
> > +int dm_pci_capability_write(struct udevice *dev, int cap, int pos,
> > +                           ulong val, enum pci_size_t size) {
> > +       u32 off;
> > +
> > +       switch (size) {
> > +       case PCI_SIZE_16:
> > +               if (pos & 1)
> > +                       return -EINVAL;
> > +               break;
> > +       case PCI_SIZE_32:
> > +               if (pos & 3)
> > +                       return -EINVAL;
> > +               break;
> > +       default:
> > +               return -EINVAL;
> > +       }
> > +
> > +       off = dm_pci_find_capability(dev, cap);
> > +       if (off)
> > +               return dm_pci_write_config(dev, off + pos, val, size);
> > +
> > +       return -EINVAL;
> > +}
> > +
> > +int dm_pci_capability_read_word(struct udevice *dev, int cap, int
> > +pos, u16 *val) {
> > +       ulong value;
> > +       int ret;
> > +
> > +       ret = dm_pci_capability_read(dev, cap, pos, &value, PCI_SIZE_16);
> > +       if (ret)
> > +               return ret;
> > +       *val = value;
> > +
> > +       return 0;
> > +}
> > +
> > +int dm_pci_capability_write_word(struct udevice *dev, int cap, int
> > +pos, u16 val) {
> > +       return dm_pci_capability_write(dev, cap, pos, val,
> > +PCI_SIZE_16); }
> > +
> > +int dm_pci_capability_read_dword(struct udevice *dev, int cap,
> > +                                int pos, u32 *val) {
> > +       ulong value;
> > +       int ret;
> > +
> > +       return dm_pci_capability_read(dev, cap, pos, &value,
> PCI_SIZE_32);
> > +       if (ret)
> > +               return ret;
> > +       *val = value;
> > +
> > +       return 0;
> > +}
> > +
> > +int dm_pci_capability_write_dword(struct udevice *dev, int cap,
> > +                                 int pos, u32 val) {
> > +       return dm_pci_capability_write(dev, cap, pos, val,
> > +PCI_SIZE_32); }
> > +
> > +/**
> > + * dm_pci_capability_clear_and_set() - PCI capability register update
> > + *
> > + * @dev:       PCI device to update
> > + * @cap:       capability code
> > + * @pos:       register offset
> > + * @clear:     bits to clear
> > + * @set:       bits to set
> > + * @size:      register width
> > + *
> > + * Returns 0 if OK or appropriate error value.
> > + */
> > +int dm_pci_capability_clear_and_set(struct udevice *dev, int cap, int pos,
> > +                                   ulong clear, ulong set,
> > +                                   enum pci_size_t size) {
> > +       int ret;
> > +       ulong val;
> > +
> > +       ret = dm_pci_capability_read(dev, cap, pos, &val, size);
> > +       if (!ret) {
> > +               val &= ~clear;
> > +               val |= set;
> > +               ret = dm_pci_capability_write(dev, cap, pos, val, size);
> > +       }
> > +
> > +       return ret;
> > +}
> > +
> > +int dm_pci_capability_clear_and_set_word(struct udevice *dev, int cap,
> > +                                        int pos, u16 clear, u16
> set)
> > +{
> > +       return dm_pci_capability_clear_and_set(dev, cap, pos, clear,
> > +                                              set,
> PCI_SIZE_16); }
> > +
> > +int dm_pci_capability_clear_and_set_dword(struct udevice *dev, int cap,
> > +                                         int pos, u32 clear, u32
> set)
> > +{
> > +       return dm_pci_capability_clear_and_set(dev, cap, pos, clear,
> > +                                              set,
> PCI_SIZE_32); }
> > +
> >  UCLASS_DRIVER(pci) = {
> >         .id             = UCLASS_PCI,
> >         .name           = "pci",
> > diff --git a/include/pci.h b/include/pci.h index
> > 041f8e3747..d7b6d9f4ff 100644
> > --- a/include/pci.h
> > +++ b/include/pci.h
> > @@ -1405,6 +1405,26 @@ int dm_pci_find_next_ext_capability(struct
> udevice *dev, int start, int cap);
> >   */
> >  int dm_pci_find_ext_capability(struct udevice *dev, int cap);
> >
> > +int dm_pci_capability_read(struct udevice *dev, int cap, int pos,
> > +                          ulong *val, enum pci_size_t size); int
> > +dm_pci_capability_write(struct udevice *dev, int cap, int pos,
> > +                           ulong val, enum pci_size_t size); int
> > +dm_pci_capability_read_word(struct udevice *dev, int cap,
> > +                               int pos, u16 *val); int
> > +dm_pci_capability_write_word(struct udevice *dev, int cap,
> > +                                int pos, u16 val); int
> > +dm_pci_capability_read_dword(struct udevice *dev, int cap,
> > +                                int pos, u32 *val); int
> > +dm_pci_capability_write_dword(struct udevice *dev, int cap,
> > +                                 int pos, u32 val); int
> > +dm_pci_capability_clear_and_set(struct udevice *dev, int cap, int pos,
> > +                                   ulong clear, ulong set,
> > +                                   enum pci_size_t size); int
> > +dm_pci_capability_clear_and_set_word(struct udevice *dev, int cap,
> > +                                        int pos, u16 clear, u16
> set);
> > +int dm_pci_capability_clear_and_set_dword(struct udevice *dev, int cap,
> > +                                         int pos, u32 clear, u32
> > +set);
> > +
> >  #define dm_pci_virt_to_bus(dev, addr, flags) \
> >         dm_pci_phys_to_bus(dev, (virt_to_phys(addr)), (flags))
> > #define dm_pci_bus_to_virt(dev, addr, flags, len, map_flags) \
> > --
> 
> Please also add test cases test/dm/pci.c::dm_test_pci_cap()

Yes, will add.

> 
> Regards,
> Bin

Thanks,
Zhiqiang


More information about the U-Boot mailing list