| Message ID | 20190325022546.38427-9-Zhiqiang.Hou@nxp.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Prabhakar Kushwaha |
| Headers | show |
| Series | pci: Add PCIe Gen4 controller driver for NXP Layerscape SoCs | expand |
On Mon, Mar 25, 2019 at 10:24 AM Z.q. Hou <zhiqiang.hou@nxp.com> wrote: > > From: Hou Zhiqiang <Zhiqiang.Hou@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? > operations. > > Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@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 > +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() Regards, Bin
Hi Bin, Thanks a lot for your comments! > -----Original Message----- > From: Bin Meng [mailto:bmeng.cn@gmail.com] > Sent: 2019年4月1日 11:37 > To: Z.q. Hou <zhiqiang.hou@nxp.com> > Cc: u-boot@lists.denx.de; albert.u.boot@aribaud.net; Priyanka Jain > <priyanka.jain@nxp.com>; York Sun <york.sun@nxp.com>; > sriram.dash@nxp.com; yamada.masahiro@socionext.com; Prabhakar > Kushwaha <prabhakar.kushwaha@nxp.com>; Mingkai Hu > <mingkai.hu@nxp.com>; M.h. Lian <minghuan.lian@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@nxp.com> wrote: > > > > From: Hou Zhiqiang <Zhiqiang.Hou@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@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
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. + */ +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) \