Hi Bin, Thanks a lot for your comments!
> -----Original Message----- > From: Bin Meng [mailto:bmeng...@gmail.com] > Sent: 2019年4月1日 11:37 > To: Z.q. Hou <zhiqiang....@nxp.com> > Cc: u-boot@lists.denx.de; albert.u.b...@aribaud.net; Priyanka Jain > <priyanka.j...@nxp.com>; York Sun <york....@nxp.com>; > sriram.d...@nxp.com; yamada.masah...@socionext.com; Prabhakar > Kushwaha <prabhakar.kushw...@nxp.com>; Mingkai Hu > <mingkai...@nxp.com>; M.h. Lian <minghuan.l...@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....@nxp.com> wrote: > > > > From: Hou Zhiqiang <zhiqiang....@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....@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 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot