lkml.org 
[lkml]   [2013]   [Jul]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRe: [PATCH] pci: exynos: split into two parts such as Synopsys part and Exynos part
    Date
    On Friday, July 05, 2013 7:44 PM, Pratyush Anand wrote:
    > 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.

    Thank you.
    Without your comment, it could be done. :)

    >
    > > Signed-off-by: Jingoo Han <jg1.han@samsung.com>
    > > Cc: Pratyush Anand <pratyush.anand@st.com>
    > > Cc: Mohit KUMAR <Mohit.KUMAR@st.com>
    > > ---
    > > 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?

    purple_base is a register to control PCIe block.

    For example, controlling various reset control signals,
    PLL lock indication, etc.

    Generic name would be 'block_base' or 'misc_base'?


    >
    > > - 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?

    OK, I see.
    I will use 2 args.

    Best regards,
    Jingoo Han





    \
     
     \ /
      Last update: 2013-07-10 23:03    [W:2.657 / U:0.172 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site