On Wed, 20 May 2015 14:21:40 +0800, Zhou Wang wrote: > > This patch adds PCIe host support for Hisilicon Soc Hip05. > > Signed-off-by: Zhou Wang <wangzhou1@xxxxxxxxxxxxx> Hi Zhou Wang, I added some minor comments. > --- > drivers/pci/host/Kconfig | 5 + > drivers/pci/host/Makefile | 1 + > drivers/pci/host/pcie-hisi.c | 252 +++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 258 insertions(+) > create mode 100644 drivers/pci/host/pcie-hisi.c > > diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig > index 1dfb567..486d822 100644 > --- a/drivers/pci/host/Kconfig > +++ b/drivers/pci/host/Kconfig > @@ -125,4 +125,9 @@ config PCIE_IPROC_PLATFORM > Say Y here if you want to use the Broadcom iProc PCIe controller > through the generic platform bus interface > > +config PCI_HISI > + depends on OF && ARM64 > + bool "Hisilicon Soc HIP05 PCIe controller" > + select PCIE_DW > + > endmenu > diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile > index f733b4e..562142e 100644 > --- a/drivers/pci/host/Makefile > +++ b/drivers/pci/host/Makefile > @@ -15,3 +15,4 @@ obj-$(CONFIG_PCI_LAYERSCAPE) += pci-layerscape.o > obj-$(CONFIG_PCI_VERSATILE) += pci-versatile.o > obj-$(CONFIG_PCIE_IPROC) += pcie-iproc.o > obj-$(CONFIG_PCIE_IPROC_PLATFORM) += pcie-iproc-platform.o > +obj-$(CONFIG_PCI_HISI) += pcie-hisi.o > diff --git a/drivers/pci/host/pcie-hisi.c b/drivers/pci/host/pcie-hisi.c > new file mode 100644 > index 0000000..3f8cb9a > --- /dev/null > +++ b/drivers/pci/host/pcie-hisi.c > @@ -0,0 +1,252 @@ > +/* > + * PCIe host controller driver for Hisilicon Hip05 SoCs > + * > + * Copyright (C) 2014 Hisilicon Co., Ltd. http://www.hisilicon.com s/2014/2015 > + * > + * Author: Zhou Wang <wangzhou1@xxxxxxxxxxxxx> > + * Dacai Zhu <zhudacai@xxxxxxxxxxxxx> [.....] > +#define PCIE_SUBCTRL_MODE_REG (0x2800) > +#define PCIE_SUBCTRL_SYS_STATE4_REG (0x6818) > +#define PCIE_SLV_DBI_MODE (0x0) > +#define PCIE_SLV_SYSCTRL_MODE (0x1) > +#define PCIE_SLV_CONTENT_MODE (0x2) > +#define PCIE_LTSSM_LINKUP_STATE (0x11) > +#define PCIE_LTSSM_STATE_MASK (0x3F) > +#define PCIE_MSI_CONTEXT_VALUE (0x1011000) > +#define PCIE_MSI_TRANS_ENABLE (0x1ff0) > +#define PCIE_MSI_LOW_ADDRESS (0x1b4) > +#define PCIE_MSI_HIGH_ADDRESS (0x1c4) Please remove unnecessary braces as below. +#define PCIE_SUBCTRL_MODE_REG 0x2800 +#define PCIE_SUBCTRL_SYS_STATE4_REG 0x6818 ..... [.....] > +/* Configure vmid/asid table in PCIe host */ > +static void hisi_pcie_config_context(struct hisi_pcie *pcie) > +{ > + int i; > + > + hisi_pcie_change_apb_mode(pcie, PCIE_SLV_CONTENT_MODE); > + > + for (i = 0; i < 0x400; i++) > + hisi_pcie_apb_writel(pcie, 0x0, i * 4); > + > + for (i = 0x400; i < 0x800; i++) > + hisi_pcie_apb_writel(pcie, 0x0, i * 4); How about the following? + for (i = 0; i < 0x800; i++) + hisi_pcie_apb_writel(pcie, 0x0, i * 4); > + > + hisi_pcie_change_apb_mode(pcie, PCIE_SLV_SYSCTRL_MODE); > + > + hisi_pcie_apb_writel(pcie, 0xb7010040, PCIE_MSI_LOW_ADDRESS); > + hisi_pcie_apb_writel(pcie, 0x0, PCIE_MSI_HIGH_ADDRESS); > + hisi_pcie_apb_writel(pcie, PCIE_MSI_CONTEXT_VALUE, 0x10); > + hisi_pcie_apb_writel(pcie, PCIE_MSI_TRANS_ENABLE, 0x1c8); Please don't use hardcoded numbers as possible. > + > + hisi_pcie_change_apb_mode(pcie, PCIE_SLV_DBI_MODE); > +} [.....] Best regards, Jingoo Han -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
