On 7/5/2013 1:59 PM, Jingoo Han wrote:
Exynos PCIe IP consists of Synopsys specific part and Exynos specific part. Only core block is a Synopsys designware part; other parts are Exynos specific. Also, the Synopsys designware part can be shared with other platforms; thus, it can be split two parts such as Synopsys designware part and Exynos specific part.
A quick and nice job :) Just few minor comments.
Signed-off-by: Jingoo Han <jg1.han@xxxxxxxxxxx> Cc: Pratyush Anand <pratyush.anand@xxxxxx> Cc: Mohit KUMAR <Mohit.KUMAR@xxxxxx> --- drivers/pci/host/Makefile | 1 + drivers/pci/host/pcie-designware.c | 907 +++++++----------------------------- drivers/pci/host/pcie-designware.h | 72 +++ drivers/pci/host/pcie-exynos.c | 619 ++++++++++++++++++++++++ 4 files changed, 862 insertions(+), 737 deletions(-) create mode 100644 drivers/pci/host/pcie-designware.h create mode 100644 drivers/pci/host/pcie-exynos.c
[...]
- -struct pcie_port { - struct device *dev; - u8 controller; - u8 root_bus_nr; - void __iomem *dbi_base; - void __iomem *elbi_base; - void __iomem *phy_base; - void __iomem *purple_base;
Just for knowledge, what is the purple_base. It might not be needed by all vendors. Can we explain the name in comment or can give some generic name?
- u64 cfg0_base; - void __iomem *va_cfg0_base; - u64 cfg1_base; - void __iomem *va_cfg1_base; - u64 io_base; - u64 mem_base; - spinlock_t conf_lock; - struct resource cfg; - struct resource io; - struct resource mem; - struct pcie_port_info config; - struct clk *clk; - struct clk *bus_clk; - int irq; - int reset_gpio; -}; -
[...]
- -static inline void readl_rc(struct pcie_port *pp, void *dbi_base, u32 *val) +static inline void dw_pcie_readl_rc(struct pcie_port *pp, void *dbi_base, + u32 *val)
dbi_base is part of pp. So why to pass 3 args?
{ - exynos_pcie_sideband_dbi_r_mode(pp, true); - *val = readl(dbi_base); - exynos_pcie_sideband_dbi_r_mode(pp, false); - return; + if (pp->ops->readl_rc) + pp->ops->readl_rc(pp, dbi_base, val);
ditto
+ else + *val = readl(dbi_base); } -static inline void writel_rc(struct pcie_port *pp, u32 val, void *dbi_base) +static inline void dw_pcie_writel_rc(struct pcie_port *pp, u32 val, + void *dbi_base)
ditto
{ - exynos_pcie_sideband_dbi_w_mode(pp, true); - writel(val, dbi_base); - exynos_pcie_sideband_dbi_w_mode(pp, false); - return; + if (pp->ops->writel_rc) + pp->ops->writel_rc(pp, val, dbi_base);
ditto
+ else + writel(val, dbi_base); }
Regards Pratyush -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html