Re: [RESEND PATCH v3 2/2] PCI: meson: add the Amlogic Meson PCIe controller driver
On 2018/9/24 16:34, Gustavo Pimentel wrote: > Hi Hanjie, > > On 21/09/2018 07:03, Hanjie Lin wrote: >> From: Yue Wang >> >> The Amlogic Meson PCIe host controller is based on the Synopsys DesignWare >> PCI core. This patch adds the driver support for Meson PCIe controller. >> >> Signed-off-by: Yue Wang >> Signed-off-by: Hanjie Lin >> --- >> drivers/pci/controller/dwc/Kconfig | 12 + >> drivers/pci/controller/dwc/Makefile| 1 + >> drivers/pci/controller/dwc/pci-meson.c | 617 >> + >> 3 files changed, 630 insertions(+) >> create mode 100644 drivers/pci/controller/dwc/pci-meson.c >> >> diff --git a/drivers/pci/controller/dwc/Kconfig >> b/drivers/pci/controller/dwc/Kconfig >> index 91b0194..6cb36f6 100644 >> --- a/drivers/pci/controller/dwc/Kconfig >> +++ b/drivers/pci/controller/dwc/Kconfig >> @@ -193,4 +193,16 @@ config PCIE_HISI_STB >> help >>Say Y here if you want PCIe controller support on HiSilicon STB >> SoCs >> >> +config PCI_MESON >> +bool "MESON PCIe controller" >> +depends on PCI >> +depends on PCI_MSI_IRQ_DOMAIN > > I would suggest to compress the previous 2 line into in just one, like: > depends on PCI && PCI_MSI_IRQ_DOMAIN > > Regards, > Gustavo > Right, depends on PCI is unnecessary actually. I will fix it. Thanks, Hanjie >> +select PCIEPORTBUS >> +select PCIE_DW_HOST >> +help >> + Say Y here if you want to enable PCI controller support on Amlogic >> + SoCs. The PCI controller on Amlogic is based on DesignWare hardware >> + and therefore the driver re-uses the DesignWare core functions to >> + implement the driver. >> + >> endmenu >> diff --git a/drivers/pci/controller/dwc/Makefile >> b/drivers/pci/controller/dwc/Makefile >> index 5d2ce72..cf676bd 100644 >> --- a/drivers/pci/controller/dwc/Makefile >> +++ b/drivers/pci/controller/dwc/Makefile >> @@ -14,6 +14,7 @@ obj-$(CONFIG_PCIE_ARMADA_8K) += pcie-armada8k.o >> obj-$(CONFIG_PCIE_ARTPEC6) += pcie-artpec6.o >> obj-$(CONFIG_PCIE_KIRIN) += pcie-kirin.o >> obj-$(CONFIG_PCIE_HISI_STB) += pcie-histb.o >> +obj-$(CONFIG_PCI_MESON) += pci-meson.o >> >> # The following drivers are for devices that use the generic ACPI >> # pci_root.c driver but don't support standard ECAM config access. >> diff --git a/drivers/pci/controller/dwc/pci-meson.c >> b/drivers/pci/controller/dwc/pci-meson.c >> new file mode 100644 >> index 000..9c92a89 >> --- /dev/null >> +++ b/drivers/pci/controller/dwc/pci-meson.c >> @@ -0,0 +1,617 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +/* >> + * PCIe host controller driver for Amlogic MESON SoCs >> + * >> + * Copyright (c) 2018 Amlogic, inc. >> + * Author: Yue Wang >> + */ >> + >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> + >> +#include "pcie-designware.h" >> + >> +#define to_meson_pcie(x) dev_get_drvdata((x)->dev) >> + >> +/* External local bus interface registers */ >> +#define PLR_OFFSET 0x700 >> +#define PCIE_PORT_LINK_CTRL_OFF (PLR_OFFSET + 0x10) >> +#define FAST_LINK_MODE BIT(7) >> +#define LINK_CAPABLE_MASK GENMASK(21, 16) >> +#define LINK_CAPABLE_X1 BIT(16) >> + >> +#define PCIE_GEN2_CTRL_OFF (PLR_OFFSET + 0x10c) >> +#define NUM_OF_LANES_MASK GENMASK(12, 8) >> +#define NUM_OF_LANES_X1 BIT(8) >> +#define DIRECT_SPEED_CHANGE BIT(17) >> + >> +#define TYPE1_HDR_OFFSET0x0 >> +#define PCIE_STATUS_COMMAND (TYPE1_HDR_OFFSET + 0x04) >> +#define PCI_IO_EN BIT(0) >> +#define PCI_MEM_SPACE_ENBIT(1) >> +#define PCI_BUS_MASTER_EN BIT(2) >> + >> +#define PCIE_BASE_ADDR0 (TYPE1_HDR_OFFSET + 0x10) >> +#define PCIE_BASE_ADDR1 (TYPE1_HDR_OFFSET + 0x14) >> + >> +#define PCIE_CAP_OFFSET 0x70 >> +#define PCIE_DEV_CTRL_DEV_STUS (PCIE_CAP_OFFSET + 0x08) >> +#define PCIE_CAP_MAX_PAYLOAD_MASK GENMASK(7, 5) >> +#define PCIE_CAP_MAX_PAYLOAD_SIZE(x)((x) << 5) >> +#define PCIE_CAP_MAX_READ_REQ_MASK GENMASK(14, 12) >> +#define PCIE_CAP_MAX_READ_REQ_SIZE(x) ((x) << 12) >> + >> +#define PCI_CLASS_REVISION_MASK GENMASK(7, 0) >> + >> +/* PCIe specific config registers */ >> +#define PCIE_CFG0 0x0 >> +#define APP_LTSSM_ENABLEBIT(7) >> + >> +#define PCIE_CFG_STATUS12 0x30 >> +#define IS_SMLH_LINK_UP(x) ((x) & (1 << 6)) >> +#define IS_RDLH_LINK_UP(x) ((x) & (1 << 16)) >> +#define IS_LTSSM_UP(x) x) >> 10) & 0x1f) == 0x11) >> + >> +#define PCIE_CFG_STATUS17 0x44 >> +#define PM_CURRENT_STATE(x) (((x) >> 7) & 0x1) >> + >> +#define WAIT_LINKUP_TIMEOUT 2000 >> +#define PORT_CLK_RATE 1UL >> +#define MAX_PAYLOAD_SIZE256
Re: [RESEND PATCH v3 2/2] PCI: meson: add the Amlogic Meson PCIe controller driver
On 2018/9/21 22:47, Lorenzo Pieralisi wrote: > On Fri, Sep 21, 2018 at 02:03:40PM +0800, Hanjie Lin wrote: >> From: Yue Wang >> >> The Amlogic Meson PCIe host controller is based on the Synopsys DesignWare >> PCI core. This patch adds the driver support for Meson PCIe controller. > > Please read: > > https://marc.info/?l=linux-pci&m=150905742808166&w=2 > > update the $SUBJECT accordingly, I can do it but you will repost anyway > so please do it yourself while at it. > OK, I will follow the list and check the patch. >> Signed-off-by: Yue Wang >> Signed-off-by: Hanjie Lin >> --- >> drivers/pci/controller/dwc/Kconfig | 12 + >> drivers/pci/controller/dwc/Makefile| 1 + >> drivers/pci/controller/dwc/pci-meson.c | 617 >> + >> 3 files changed, 630 insertions(+) >> create mode 100644 drivers/pci/controller/dwc/pci-meson.c > > I would request a MAINTAINERS update too. > I will update MAINTAINERS in this patch of next-version. >> diff --git a/drivers/pci/controller/dwc/Kconfig >> b/drivers/pci/controller/dwc/Kconfig >> index 91b0194..6cb36f6 100644 >> --- a/drivers/pci/controller/dwc/Kconfig >> +++ b/drivers/pci/controller/dwc/Kconfig >> @@ -193,4 +193,16 @@ config PCIE_HISI_STB >> help >>Say Y here if you want PCIe controller support on HiSilicon STB >> SoCs >> >> +config PCI_MESON >> +bool "MESON PCIe controller" >> +depends on PCI > > This is redundant, remove it. > Yes, really redundant I will remove it. >> +depends on PCI_MSI_IRQ_DOMAIN >> +select PCIEPORTBUS > > There is no real dependency on it, this should be left to kernel > configuration, remove it. > Yes, I will remove it. >> +select PCIE_DW_HOST >> +help >> + Say Y here if you want to enable PCI controller support on Amlogic >> + SoCs. The PCI controller on Amlogic is based on DesignWare hardware >> + and therefore the driver re-uses the DesignWare core functions to >> + implement the driver. >> + >> endmenu >> diff --git a/drivers/pci/controller/dwc/Makefile >> b/drivers/pci/controller/dwc/Makefile >> index 5d2ce72..cf676bd 100644 >> --- a/drivers/pci/controller/dwc/Makefile >> +++ b/drivers/pci/controller/dwc/Makefile >> @@ -14,6 +14,7 @@ obj-$(CONFIG_PCIE_ARMADA_8K) += pcie-armada8k.o >> obj-$(CONFIG_PCIE_ARTPEC6) += pcie-artpec6.o >> obj-$(CONFIG_PCIE_KIRIN) += pcie-kirin.o >> obj-$(CONFIG_PCIE_HISI_STB) += pcie-histb.o >> +obj-$(CONFIG_PCI_MESON) += pci-meson.o >> >> # The following drivers are for devices that use the generic ACPI >> # pci_root.c driver but don't support standard ECAM config access. >> diff --git a/drivers/pci/controller/dwc/pci-meson.c >> b/drivers/pci/controller/dwc/pci-meson.c >> new file mode 100644 >> index 000..9c92a89 >> --- /dev/null >> +++ b/drivers/pci/controller/dwc/pci-meson.c >> @@ -0,0 +1,617 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +/* >> + * PCIe host controller driver for Amlogic MESON SoCs >> + * >> + * Copyright (c) 2018 Amlogic, inc. >> + * Author: Yue Wang >> + */ >> + >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> + >> +#include "pcie-designware.h" >> + >> +#define to_meson_pcie(x) dev_get_drvdata((x)->dev) > > Side note: this macro can be made dwc global since it is the same for all > dwc based host bridges. > Yes, actually this patch refers to the other drivers in dwc directory. >> +/* External local bus interface registers */ >> +#define PLR_OFFSET 0x700 >> +#define PCIE_PORT_LINK_CTRL_OFF (PLR_OFFSET + 0x10) >> +#define FAST_LINK_MODE BIT(7) >> +#define LINK_CAPABLE_MASK GENMASK(21, 16) >> +#define LINK_CAPABLE_X1 BIT(16) >> + >> +#define PCIE_GEN2_CTRL_OFF (PLR_OFFSET + 0x10c) >> +#define NUM_OF_LANES_MASK GENMASK(12, 8) >> +#define NUM_OF_LANES_X1 BIT(8) >> +#define DIRECT_SPEED_CHANGE BIT(17) >> + >> +#define TYPE1_HDR_OFFSET0x0 >> +#define PCIE_STATUS_COMMAND (TYPE1_HDR_OFFSET + 0x04) >> +#define PCI_IO_EN BIT(0) >> +#define PCI_MEM_SPACE_ENBIT(1) >> +#define PCI_BUS_MASTER_EN BIT(2) >> + >> +#define PCIE_BASE_ADDR0 (TYPE1_HDR_OFFSET + 0x10) >> +#define PCIE_BASE_ADDR1 (TYPE1_HDR_OFFSET + 0x14) >> + >> +#define PCIE_CAP_OFFSET 0x70 >> +#define PCIE_DEV_CTRL_DEV_STUS (PCIE_CAP_OFFSET + 0x08) >> +#define PCIE_CAP_MAX_PAYLOAD_MASK GENMASK(7, 5) >> +#define PCIE_CAP_MAX_PAYLOAD_SIZE(x)((x) << 5) >> +#define PCIE_CAP_MAX_READ_REQ_MASK GENMASK(14, 12) >> +#define PCIE_CAP_MAX_READ_REQ_SIZE(x) ((x) << 12) >> + >> +#define PCI_CLASS_REVISION_MASK GENMASK(7, 0) >> + >> +/* PCIe specific config registers */ >> +#define PCIE_CFG0 0x0 >> +#define APP_LTSSM_ENABLE
Re: [RESEND PATCH v3 2/2] PCI: meson: add the Amlogic Meson PCIe controller driver
Hi Hanjie, On 21/09/2018 07:03, Hanjie Lin wrote: > From: Yue Wang > > The Amlogic Meson PCIe host controller is based on the Synopsys DesignWare > PCI core. This patch adds the driver support for Meson PCIe controller. > > Signed-off-by: Yue Wang > Signed-off-by: Hanjie Lin > --- > drivers/pci/controller/dwc/Kconfig | 12 + > drivers/pci/controller/dwc/Makefile| 1 + > drivers/pci/controller/dwc/pci-meson.c | 617 > + > 3 files changed, 630 insertions(+) > create mode 100644 drivers/pci/controller/dwc/pci-meson.c > > diff --git a/drivers/pci/controller/dwc/Kconfig > b/drivers/pci/controller/dwc/Kconfig > index 91b0194..6cb36f6 100644 > --- a/drivers/pci/controller/dwc/Kconfig > +++ b/drivers/pci/controller/dwc/Kconfig > @@ -193,4 +193,16 @@ config PCIE_HISI_STB > help >Say Y here if you want PCIe controller support on HiSilicon STB > SoCs > > +config PCI_MESON > + bool "MESON PCIe controller" > + depends on PCI > + depends on PCI_MSI_IRQ_DOMAIN I would suggest to compress the previous 2 line into in just one, like: depends on PCI && PCI_MSI_IRQ_DOMAIN Regards, Gustavo > + select PCIEPORTBUS > + select PCIE_DW_HOST > + help > + Say Y here if you want to enable PCI controller support on Amlogic > + SoCs. The PCI controller on Amlogic is based on DesignWare hardware > + and therefore the driver re-uses the DesignWare core functions to > + implement the driver. > + > endmenu > diff --git a/drivers/pci/controller/dwc/Makefile > b/drivers/pci/controller/dwc/Makefile > index 5d2ce72..cf676bd 100644 > --- a/drivers/pci/controller/dwc/Makefile > +++ b/drivers/pci/controller/dwc/Makefile > @@ -14,6 +14,7 @@ obj-$(CONFIG_PCIE_ARMADA_8K) += pcie-armada8k.o > obj-$(CONFIG_PCIE_ARTPEC6) += pcie-artpec6.o > obj-$(CONFIG_PCIE_KIRIN) += pcie-kirin.o > obj-$(CONFIG_PCIE_HISI_STB) += pcie-histb.o > +obj-$(CONFIG_PCI_MESON) += pci-meson.o > > # The following drivers are for devices that use the generic ACPI > # pci_root.c driver but don't support standard ECAM config access. > diff --git a/drivers/pci/controller/dwc/pci-meson.c > b/drivers/pci/controller/dwc/pci-meson.c > new file mode 100644 > index 000..9c92a89 > --- /dev/null > +++ b/drivers/pci/controller/dwc/pci-meson.c > @@ -0,0 +1,617 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * PCIe host controller driver for Amlogic MESON SoCs > + * > + * Copyright (c) 2018 Amlogic, inc. > + * Author: Yue Wang > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include "pcie-designware.h" > + > +#define to_meson_pcie(x) dev_get_drvdata((x)->dev) > + > +/* External local bus interface registers */ > +#define PLR_OFFSET 0x700 > +#define PCIE_PORT_LINK_CTRL_OFF (PLR_OFFSET + 0x10) > +#define FAST_LINK_MODE BIT(7) > +#define LINK_CAPABLE_MASKGENMASK(21, 16) > +#define LINK_CAPABLE_X1 BIT(16) > + > +#define PCIE_GEN2_CTRL_OFF (PLR_OFFSET + 0x10c) > +#define NUM_OF_LANES_MASKGENMASK(12, 8) > +#define NUM_OF_LANES_X1 BIT(8) > +#define DIRECT_SPEED_CHANGE BIT(17) > + > +#define TYPE1_HDR_OFFSET 0x0 > +#define PCIE_STATUS_COMMAND (TYPE1_HDR_OFFSET + 0x04) > +#define PCI_IO_ENBIT(0) > +#define PCI_MEM_SPACE_EN BIT(1) > +#define PCI_BUS_MASTER_ENBIT(2) > + > +#define PCIE_BASE_ADDR0 (TYPE1_HDR_OFFSET + 0x10) > +#define PCIE_BASE_ADDR1 (TYPE1_HDR_OFFSET + 0x14) > + > +#define PCIE_CAP_OFFSET 0x70 > +#define PCIE_DEV_CTRL_DEV_STUS (PCIE_CAP_OFFSET + 0x08) > +#define PCIE_CAP_MAX_PAYLOAD_MASKGENMASK(7, 5) > +#define PCIE_CAP_MAX_PAYLOAD_SIZE(x) ((x) << 5) > +#define PCIE_CAP_MAX_READ_REQ_MASK GENMASK(14, 12) > +#define PCIE_CAP_MAX_READ_REQ_SIZE(x)((x) << 12) > + > +#define PCI_CLASS_REVISION_MASK GENMASK(7, 0) > + > +/* PCIe specific config registers */ > +#define PCIE_CFG00x0 > +#define APP_LTSSM_ENABLE BIT(7) > + > +#define PCIE_CFG_STATUS120x30 > +#define IS_SMLH_LINK_UP(x) ((x) & (1 << 6)) > +#define IS_RDLH_LINK_UP(x) ((x) & (1 << 16)) > +#define IS_LTSSM_UP(x) x) >> 10) & 0x1f) == 0x11) > + > +#define PCIE_CFG_STATUS170x44 > +#define PM_CURRENT_STATE(x) (((x) >> 7) & 0x1) > + > +#define WAIT_LINKUP_TIMEOUT 2000 > +#define PORT_CLK_RATE1UL > +#define MAX_PAYLOAD_SIZE 256 > +#define MAX_READ_REQ_SIZE256 > +#define MESON_PCIE_PHY_POWERUP 0x1c > +#define PCIE_RESET_DELAY 500 > +#define PCIE_SHARED_RESET1 > +#define PCIE_NORMAL_RESET0 > + > +
Re: [RESEND PATCH v3 2/2] PCI: meson: add the Amlogic Meson PCIe controller driver
On Fri, Sep 21, 2018 at 02:03:40PM +0800, Hanjie Lin wrote: > From: Yue Wang > > The Amlogic Meson PCIe host controller is based on the Synopsys DesignWare > PCI core. This patch adds the driver support for Meson PCIe controller. Please read: https://marc.info/?l=linux-pci&m=150905742808166&w=2 update the $SUBJECT accordingly, I can do it but you will repost anyway so please do it yourself while at it. > Signed-off-by: Yue Wang > Signed-off-by: Hanjie Lin > --- > drivers/pci/controller/dwc/Kconfig | 12 + > drivers/pci/controller/dwc/Makefile| 1 + > drivers/pci/controller/dwc/pci-meson.c | 617 > + > 3 files changed, 630 insertions(+) > create mode 100644 drivers/pci/controller/dwc/pci-meson.c I would request a MAINTAINERS update too. > diff --git a/drivers/pci/controller/dwc/Kconfig > b/drivers/pci/controller/dwc/Kconfig > index 91b0194..6cb36f6 100644 > --- a/drivers/pci/controller/dwc/Kconfig > +++ b/drivers/pci/controller/dwc/Kconfig > @@ -193,4 +193,16 @@ config PCIE_HISI_STB > help >Say Y here if you want PCIe controller support on HiSilicon STB > SoCs > > +config PCI_MESON > + bool "MESON PCIe controller" > + depends on PCI This is redundant, remove it. > + depends on PCI_MSI_IRQ_DOMAIN > + select PCIEPORTBUS There is no real dependency on it, this should be left to kernel configuration, remove it. > + select PCIE_DW_HOST > + help > + Say Y here if you want to enable PCI controller support on Amlogic > + SoCs. The PCI controller on Amlogic is based on DesignWare hardware > + and therefore the driver re-uses the DesignWare core functions to > + implement the driver. > + > endmenu > diff --git a/drivers/pci/controller/dwc/Makefile > b/drivers/pci/controller/dwc/Makefile > index 5d2ce72..cf676bd 100644 > --- a/drivers/pci/controller/dwc/Makefile > +++ b/drivers/pci/controller/dwc/Makefile > @@ -14,6 +14,7 @@ obj-$(CONFIG_PCIE_ARMADA_8K) += pcie-armada8k.o > obj-$(CONFIG_PCIE_ARTPEC6) += pcie-artpec6.o > obj-$(CONFIG_PCIE_KIRIN) += pcie-kirin.o > obj-$(CONFIG_PCIE_HISI_STB) += pcie-histb.o > +obj-$(CONFIG_PCI_MESON) += pci-meson.o > > # The following drivers are for devices that use the generic ACPI > # pci_root.c driver but don't support standard ECAM config access. > diff --git a/drivers/pci/controller/dwc/pci-meson.c > b/drivers/pci/controller/dwc/pci-meson.c > new file mode 100644 > index 000..9c92a89 > --- /dev/null > +++ b/drivers/pci/controller/dwc/pci-meson.c > @@ -0,0 +1,617 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * PCIe host controller driver for Amlogic MESON SoCs > + * > + * Copyright (c) 2018 Amlogic, inc. > + * Author: Yue Wang > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include "pcie-designware.h" > + > +#define to_meson_pcie(x) dev_get_drvdata((x)->dev) Side note: this macro can be made dwc global since it is the same for all dwc based host bridges. > +/* External local bus interface registers */ > +#define PLR_OFFSET 0x700 > +#define PCIE_PORT_LINK_CTRL_OFF (PLR_OFFSET + 0x10) > +#define FAST_LINK_MODE BIT(7) > +#define LINK_CAPABLE_MASKGENMASK(21, 16) > +#define LINK_CAPABLE_X1 BIT(16) > + > +#define PCIE_GEN2_CTRL_OFF (PLR_OFFSET + 0x10c) > +#define NUM_OF_LANES_MASKGENMASK(12, 8) > +#define NUM_OF_LANES_X1 BIT(8) > +#define DIRECT_SPEED_CHANGE BIT(17) > + > +#define TYPE1_HDR_OFFSET 0x0 > +#define PCIE_STATUS_COMMAND (TYPE1_HDR_OFFSET + 0x04) > +#define PCI_IO_ENBIT(0) > +#define PCI_MEM_SPACE_EN BIT(1) > +#define PCI_BUS_MASTER_ENBIT(2) > + > +#define PCIE_BASE_ADDR0 (TYPE1_HDR_OFFSET + 0x10) > +#define PCIE_BASE_ADDR1 (TYPE1_HDR_OFFSET + 0x14) > + > +#define PCIE_CAP_OFFSET 0x70 > +#define PCIE_DEV_CTRL_DEV_STUS (PCIE_CAP_OFFSET + 0x08) > +#define PCIE_CAP_MAX_PAYLOAD_MASKGENMASK(7, 5) > +#define PCIE_CAP_MAX_PAYLOAD_SIZE(x) ((x) << 5) > +#define PCIE_CAP_MAX_READ_REQ_MASK GENMASK(14, 12) > +#define PCIE_CAP_MAX_READ_REQ_SIZE(x)((x) << 12) > + > +#define PCI_CLASS_REVISION_MASK GENMASK(7, 0) > + > +/* PCIe specific config registers */ > +#define PCIE_CFG00x0 > +#define APP_LTSSM_ENABLE BIT(7) > + > +#define PCIE_CFG_STATUS120x30 > +#define IS_SMLH_LINK_UP(x) ((x) & (1 << 6)) > +#define IS_RDLH_LINK_UP(x) ((x) & (1 << 16)) > +#define IS_LTSSM_UP(x) x) >> 10) & 0x1f) == 0x11) > + > +#define PCIE_CFG_STATUS170x44 > +#define PM_CURRENT_STATE(x) (((x) >> 7) & 0x1) > + > +#define WAIT_LINKUP_TIMEOUT 2000 > +#define PORT_C