Hi Bin, Thanks a lot for your comments!
> -----Original Message----- > From: Bin Meng [mailto:bmeng...@gmail.com] > Sent: 2019年4月1日 15:32 > 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 9/9] dm: pci: add APIs for MPS and MRRS > accessors > > On Mon, Apr 1, 2019 at 2:00 PM Bin Meng <bmeng...@gmail.com> wrote: > > > > 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 APIs for getting and updating the MPS and MRRS > > > fields of Device capability Device control register. > > > > > > Signed-off-by: Hou Zhiqiang <zhiqiang....@nxp.com> > > > --- > > > V4: > > > - New patch > > > > > > drivers/pci/pci-uclass.c | 92 > ++++++++++++++++++++++++++++++++++++++++ > > > include/pci.h | 13 ++++++ > > > 2 files changed, 105 insertions(+) > > > > > > diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c > > > index 4bb30f5d2b..b2d295435a 100644 > > > --- a/drivers/pci/pci-uclass.c > > > +++ b/drivers/pci/pci-uclass.c > > > @@ -7,6 +7,7 @@ > > > #include <common.h> > > > #include <dm.h> > > > #include <errno.h> > > > +#include <linux/log2.h> > > > #include <pci.h> > > > #include <asm/io.h> > > > #include <dm/device-internal.h> > > > @@ -1596,6 +1597,97 @@ int > dm_pci_capability_clear_and_set_dword(struct udevice *dev, int cap, > > > set, > PCI_SIZE_32); } > > > > > > +/** > > > + * dm_pci_get_readrq - get PCI Express read request size > > > + * @dev: PCI device to query > > > + * > > > + * Returns maximum memory read request in bytes > > > + * or appropriate error value. > > > + */ > > > > Please move the comment block to pci.h > > > > > +int dm_pci_get_readrq(struct udevice *dev) { > > > + u16 ctl; > > > + int ret; > > > + > > > + ret = dm_pci_capability_read_word(dev, PCI_CAP_ID_EXP, > > > + PCI_EXP_DEVCTL, > &ctl); > > > + if (ret) > > > + return ret; > > > + > > > + return 128 << ((ctl & PCI_EXP_DEVCTL_READRQ) >> 12); } > > > + > > > +/** > > > + * dm_pci_set_readrq - set PCI Express maximum memory read request > > > + * @dev: PCI device to query > > > + * @rq: maximum memory read count in bytes > > > + * valid values are 128, 256, 512, 1024, 2048, 4096 > > > + */ > > > +int dm_pci_set_readrq(struct udevice *dev, int rq) { > > > + u16 val; > > > + > > > + if (rq < 128 || rq > 4096 || !is_power_of_2(rq)) > > > + return -EINVAL; > > > + > > > + val = (ffs(rq) - 8) << 12; > > > + > > > + return dm_pci_capability_clear_and_set_word(dev, > PCI_CAP_ID_EXP, > > > + > PCI_EXP_DEVCTL, > > > + > PCI_EXP_DEVCTL_READRQ, > > > + val); } > > > + > > > +/** > > > + * dm_pci_get_mps - get PCI Express maximum payload size > > > + * @dev: PCI device to query > > > + * > > > + * Returns maximum payload size in bytes */ int > > > +dm_pci_get_mps(struct udevice *dev) { > > > + u16 ctl; > > > + int ret; > > > + > > > + ret = dm_pci_capability_read_word(dev, PCI_CAP_ID_EXP, > > > + PCI_EXP_DEVCTL, > &ctl); > > > + if (ret) > > > + return ret; > > > + > > > + return 128 << ((ctl & PCI_EXP_DEVCTL_PAYLOAD) >> 5); } > > > + > > > +/** > > > + * dm_pci_set_mps - set PCI Express maximum payload size > > > + * @dev: PCI device to query > > > + * @mps: maximum payload size in bytes > > > + * valid values are 128, 256, 512, 1024, 2048, 4096 > > > + */ > > > +int dm_pci_set_mps(struct udevice *dev, int mps) { > > > + u16 val, cap; > > > + int ret; > > > + > > > + if (mps < 128 || mps > 4096 || !is_power_of_2(mps)) > > > + return -EINVAL; > > > + > > > + ret = dm_pci_capability_read_word(dev, PCI_CAP_ID_EXP, > > > + PCI_EXP_DEVCAP, > &cap); > > > + if (ret) > > > + return ret; > > > + > > > + val = ffs(mps) - 8; > > > + if (val > (cap & PCI_EXP_DEVCAP_PAYLOAD)) > > > + return -EINVAL; > > > + > > > + val <<= 5; > > > + > > > + return dm_pci_capability_clear_and_set_word(dev, > PCI_CAP_ID_EXP, > > > + > PCI_EXP_DEVCTL, > > > + > PCI_EXP_DEVCTL_PAYLOAD, > > > + val); } > > > + > > > UCLASS_DRIVER(pci) = { > > > .id = UCLASS_PCI, > > > .name = "pci", > > > diff --git a/include/pci.h b/include/pci.h index > > > d7b6d9f4ff..b48df8a363 100644 > > > --- a/include/pci.h > > > +++ b/include/pci.h > > > @@ -414,6 +414,14 @@ > > > #define PCI_MAX_PCI_DEVICES 32 > > > #define PCI_MAX_PCI_FUNCTIONS 8 > > > > > > +/* PCI Express capability registers */ > > > +#define PCI_EXP_DEVCAP 4 /* Device > capabilities */ > > > +#define PCI_EXP_DEVCAP_PAYLOAD 0x0007 /* > Max_Payload_Size */ > > > + > > > +#define PCI_EXP_DEVCTL 8 /* Device > Control */ > > > +#define PCI_EXP_DEVCTL_PAYLOAD 0x00e0 /* > Max_Payload_Size */ > > > +#define PCI_EXP_DEVCTL_READRQ 0x7000 /* > Max_Read_Request_Size */ > > > + > > > #define PCI_FIND_CAP_TTL 0x48 > > > #define CAP_START_POS 0x40 > > > > > > @@ -1425,6 +1433,11 @@ int > > > dm_pci_capability_clear_and_set_word(struct udevice *dev, int cap, int > dm_pci_capability_clear_and_set_dword(struct udevice *dev, int cap, > > > int pos, u32 clear, > u32 > > > set); > > > > > > +int dm_pci_get_readrq(struct udevice *dev); int > > > +dm_pci_set_readrq(struct udevice *dev, int rq); int > > > +dm_pci_get_mps(struct udevice *dev); int dm_pci_set_mps(struct > > > +udevice *dev, int mps); > > > + > > > #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 add test cases test/dm/pci.c::dm_test_pci_cap() > > Zhiqiang, when you do new version patches, please split patch [8,9] to a > separate series, since it is not tightly coupled with other 7 patches, and > can go > separately. Ok, make sense. > > Regards, > Bin Thanks, Zhiqiang _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
