Hi Bjorn, On 2016/9/2 23:53, Bjorn Helgaas wrote: > These are cleanups against 2098142ae87d, the current pci/host-rockchip > head in my tree. > Thanks so much for you to help clean up this driver, since I think it should be my duty to take over this. Hope not too late for me to help your cleanup. I think the v2 cannot compile gracefully without the appended patch. After fixing these compile errors, I backported this driver entirely to my downstream 4.4 tree and it worked fine without regression. Once again, thanks for doing this. :) > Changes from v1: > > - Rework HIWORD_UPDATE > - Remove duplicate CSR definitions > - Move CSR block offset from read/write caller to CSR definition > - Organize CSRs into logical blocks > - Fix some inconsistent CSR names > - Add names for registers at the base of CSR blocks > > I was disappointed to find how disorganized the v10 CSR definitions were. > It was quite a hodgepodge. I should have noticed that earlier, but as > penance, I tried to clean it up myself. > > These are in git as pci/host-rockchip-wip. Again, I intend to squash these > all into the single commit that adds the driver when I finally merge it. > > --- > > Bjorn Helgaas (15): > Remove unused symbols, unnecessary parens, other minor comments from > Rename pcie_read() and pcie_write() to rockchip_pcie_read() and > Always use "rockchip" as the pointer to per-device struct. > Rename struct rockchip_pcie_port to struct rockchip_pcie. > Use a local "dev" to avoid repetition of "rockchip->dev". > Add comment about why 32-bit read/modify/write isn't safe. > Simplify the confusing HIWORD_UPDATE scheme. > Remove duplicate CSR definition. > Move CSR bases into definition. > Group related CSR definitions together. > Rename PCIE_CORE_RC_CONF_SCC_SHIFT to match similar definitions. > Rename ROCKCHIP_PCIE_RPIFR1_INTR_MASK and ROCKCHIP_PCIE_RPIFR1_INTR_SHIFT > The register at PCIE_CLIENT_BASE presumably has a name of its own. Add a > Simplify testing of link status and speed testing. > Move msleeps to address Guenter's comments. > > > drivers/pci/host/pcie-rockchip.c | 842 ++++++++++++++++++-------------------- > 1 file changed, 391 insertions(+), 451 deletions(-) > > > -- Best Regards Shawn Lin -------------- next part -------------- diff --git a/drivers/pci/host/pcie-rockchip.c b/drivers/pci/host/pcie-rockchip.c index 754d24b..2bc1c35 100644 --- a/drivers/pci/host/pcie-rockchip.c +++ b/drivers/pci/host/pcie-rockchip.c @@ -931,7 +931,7 @@ static int rockchip_pcie_prog_ob_atu(struct rockchip_pcie *rockchip, u32 ob_addr_0; u32 ob_addr_1; u32 ob_desc_0; - void __iomem *aw_offset; + u32 aw_offset; if (region_no >= MAX_AXI_WRAPPER_REGION_NUM) return -EINVAL; @@ -955,13 +955,13 @@ static int rockchip_pcie_prog_ob_atu(struct rockchip_pcie *rockchip, ob_addr_1 = upper_addr; ob_desc_0 = (1 << 23 | type); - rockchip_pcie_writel(rockchip, ob_addr_0, + rockchip_pcie_write(rockchip, ob_addr_0, PCIE_CORE_OB_REGION_ADDR0 + aw_offset); - rockchip_pcie_writel(rockchip, ob_addr_1, + rockchip_pcie_write(rockchip, ob_addr_1, PCIE_CORE_OB_REGION_ADDR1 + aw_offset); - rockchip_pcie_writel(rockchip, ob_desc_0, + rockchip_pcie_write(rockchip, ob_desc_0, PCIE_CORE_OB_REGION_DESC0 + aw_offset); - rockchip_pcie_writel(rockchip, 0, + rockchip_pcie_write(rockchip, 0, PCIE_CORE_OB_REGION_DESC1 + aw_offset); return 0; @@ -973,7 +973,7 @@ static int rockchip_pcie_prog_ib_atu(struct rockchip_pcie *rockchip, { u32 ib_addr_0; u32 ib_addr_1; - void __iomem *aw_offset; + u32 aw_offset; if (region_no > MAX_AXI_IB_ROOTPORT_REGION_NUM) return -EINVAL; @@ -988,8 +988,8 @@ static int rockchip_pcie_prog_ib_atu(struct rockchip_pcie *rockchip, ib_addr_0 |= (lower_addr << 8) & PCIE_CORE_IB_REGION_ADDR0_LO_ADDR; ib_addr_1 = upper_addr; - rockchip_pcie_writel(rockchip, ib_addr_0, PCIE_RP_IB_ADDR0 + aw_offset); - rockchip_pcie_writel(rockchip, ib_addr_1, PCIE_RP_IB_ADDR1 + aw_offset); + rockchip_pcie_write(rockchip, ib_addr_0, PCIE_RP_IB_ADDR0 + aw_offset); + rockchip_pcie_write(rockchip, ib_addr_1, PCIE_RP_IB_ADDR1 + aw_offset); return 0; }