Messages in this thread | | | Subject | Re: [PATCH V7 2/3] PCI: dwc: Cleanup DBI,ATU read and write APIs | From | Vidya Sagar <> | Date | Sun, 23 Jun 2019 20:13:14 +0530 |
| |
On 6/23/2019 1:17 PM, Jingoo Han wrote: > On 6/23/19, 1:52 AM, Vidya Sagar wrote: >> >> Cleanup DBI read and write APIs by removing "__" (underscore) from their >> names as there are no no-underscore versions and the underscore versions >> are already doing what no-underscore versions typically do. It also removes >> passing dbi/dbi2 base address as one of the arguments as the same can be >> derived with in read and write APIs. Since dw_pcie_{readl/writel}_dbi() >> APIs can't be used for ATU read/write as ATU base address could be >> different from DBI base address, this patch attempts to implement >> ATU read/write APIs using ATU base address without using >> dw_pcie_{readl/writel}_dbi() APIs. >> >> Signed-off-by: Vidya Sagar <vidyas@nvidia.com> >> --- >> Changes from v6: >> * Modified ATU read/write APIs to use implementation specific DBI read/write >> APIs if present. >> >> Changes from v5: >> * Removed passing base address as one of the arguments as the same can be derived within >> the API itself. >> * Modified ATU read/write APIs to call dw_pcie_{write/read}() API >> >> Changes from v4: >> * This is a new patch in this series >> >> drivers/pci/controller/dwc/pcie-designware.c | 28 +++++------ >> drivers/pci/controller/dwc/pcie-designware.h | 51 +++++++++++++------- >> 2 files changed, 45 insertions(+), 34 deletions(-) > > ..... > >> static inline void dw_pcie_writel_atu(struct dw_pcie *pci, u32 reg, u32 val) >> { >> - __dw_pcie_write_dbi(pci, pci->atu_base, reg, 0x4, val); >> + int ret; >> + >> + if (pci->ops->write_dbi) { >> + pci->ops->write_dbi(pci, pci->atu_base, reg, 0x4, val); >> + return; >> + } >> + >> + ret = dw_pcie_write(pci->atu_base + reg, 0x4, val); >> + if (ret) >> + dev_err(pci->dev, "Write ATU address failed\n"); >> } >> >> static inline u32 dw_pcie_readl_atu(struct dw_pcie *pci, u32 reg) >> { >> - return __dw_pcie_read_dbi(pci, pci->atu_base, reg, 0x4); >> + int ret; >> + u32 val; >> + >> + if (pci->ops->read_dbi) >> + return pci->ops->read_dbi(pci, pci->atu_base, reg, 0x4); >> + >> + ret = dw_pcie_read(pci->atu_base + reg, 0x4, &val); >> + if (ret) >> + dev_err(pci->dev, "Read ATU address failed\n"); >> + >> + return val; >> } > > Hmm. In cases of dbi and dbi2, readb/readw/readl and writeb/writew/writel are > located in pcie-designware.h. These functions just call read/write which are located > in pcie-designware.c. For readability, would you write the code as below? > > 1. For drivers/pci/controller/dwc/pcie-designware.h, > Just call dw_pcie_{write/read}_atu(), instead of implementing functions as below. > > static inline void dw_pcie_writel_atu(struct dw_pcie *pci, u32 reg, u32 val) > { > return dw_pcie_write_atu(pci, reg, 0x4, val); > } > > static inline u32 dw_pcie_readl_atu(struct dw_pcie *pci, u32 reg) > { > return dw_pcie_read_atu(pci, reg, 0x4); > } > > 2. For drivers/pci/controller/dwc/pcie-designware.c, > Please add new dw_pcie_{write/read}_atu() as below. > > void dw_pcie_write_atu(struct dw_pcie *pci, u32 reg, size_t size, u32 val) > { > int ret; > > if (pci->ops->write_dbi) { > pci->ops->write_dbi(pci, pci->atu_base, reg, size, val); > return; > } > > ret = dw_pcie_write(pci->atu_base + reg, size, val); > if (ret) > dev_err(pci->dev, "Write ATU address failed\n"); > } > > u32 dw_pcie_read_atu(struct dw_pcie *pci, u32 reg, size_t size) > { > int ret; > u32 val; > > if (pci->ops->read_dbi) > return pci->ops->read_dbi(pci, pci->atu_base, reg, size); > > ret = dw_pcie_read(pci->atu_base + reg, size, &val); > if (ret) > dev_err(pci->dev, "Read ATU address failed\n"); > > return val; > } > > Thank you. > > Best regards, > Jingoo Han Ok. I'll take care of it in next patch.
> >> >> static inline void dw_pcie_dbi_ro_wr_en(struct dw_pcie *pci) >> -- >> 2.17.1 >
| |