Re: [PATCH 2/2] pci: Add Rockchip dwc based PCIe controller driver【请注意，邮件由s...@google.com代发】
Hi Shawn,
On Fri, 15 Jan 2021 at 02:15, Shawn Lin wrote:
>
> Hi Simon
>
> Thanks you for reviewing it.
>
> 在 2021/1/14 23:42, Simon Glass 写道:
> > Hi Shawn,
> >
> > On Thu, 14 Jan 2021 at 01:15, Shawn Lin wrote:
> >>
>
> 8<--
>
> >> +
> >> +static int rockchip_pcie_init_port(struct udevice *dev)
> >> +{
> >> + int ret;
> >> + u32 val;
> >> + struct rk_pcie *priv = dev_get_priv(dev);
> >> +
> >> + /* Set power and maybe external ref clk input */
> >> + if (priv->vpcie3v3) {
> >> + ret = regulator_set_value(priv->vpcie3v3, 330);
> >
> > Is there an autoset option for this so this info can go in the device
> > tree instead?
>
> I think we should control this by driver as other PCIe drivers did, as
> we can't guarantee all kernel dtbs doing the right thing and 3v3 power
> is very important for both of devices and PHY block.
That's OK. Note that U-Boot uses its own devicetree so you can make
sure it is correct in a separate patch if you like.
Regards,
Simon
Re: [PATCH 2/2] pci: Add Rockchip dwc based PCIe controller driver【请注意，邮件由s...@google.com代发】
Hi Simon
Thanks you for reviewing it.
在 2021/1/14 23:42, Simon Glass 写道:
Hi Shawn,
On Thu, 14 Jan 2021 at 01:15, Shawn Lin wrote:
8<--
+
+static int rockchip_pcie_init_port(struct udevice *dev)
+{
+ int ret;
+ u32 val;
+ struct rk_pcie *priv = dev_get_priv(dev);
+
+ /* Set power and maybe external ref clk input */
+ if (priv->vpcie3v3) {
+ ret = regulator_set_value(priv->vpcie3v3, 330);
Is there an autoset option for this so this info can go in the device
tree instead?
I think we should control this by driver as other PCIe drivers did, as
we can't guarantee all kernel dtbs doing the right thing and 3v3 power
is very important for both of devices and PHY block.
+ if (ret) {
+ dev_err(priv->dev, "failed to enable vpcie3v3
(ret=%d)\n",
+ ret);
+ return ret;
Regards,
Simon
Re: [PATCH 2/2] pci: Add Rockchip dwc based PCIe controller driver
Hi Shawn,
On Thu, 14 Jan 2021 at 01:15, Shawn Lin wrote:
>
> Add Rockchip dwc based PCIe controller driver for rk356x platform.
> Driver support Gen3 by operating as a Root complex.
>
> Signed-off-by: Shawn Lin
>
> ---
>
> drivers/pci/Kconfig| 9 +
> drivers/pci/Makefile | 1 +
> drivers/pci/pcie_dw_rockchip.c | 755 +
> 3 files changed, 765 insertions(+)
> create mode 100644 drivers/pci/pcie_dw_rockchip.c
>
> diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
> index 65498bce1d..b1de38f766 100644
> --- a/drivers/pci/Kconfig
> +++ b/drivers/pci/Kconfig
> @@ -281,6 +281,15 @@ config PCIE_ROCKCHIP
> Say Y here if you want to enable PCIe controller support on
> Rockchip SoCs.
>
> +config PCIE_DW_ROCKCHIP
> + bool "Rockchip DesignWare based PCIe controller"
> + depends on ARCH_ROCKCHIP
> + select DM_PCI
> + select PHY_ROCKCHIP_SNPS_PCIE3
> + help
> + Say Y here if you want to enable DW PCIe controller support on
> + Rockchip SoCs.
> +
> config PCI_BRCMSTB
> bool "Broadcom STB PCIe controller"
> depends on DM_PCI
> diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile
> index 8b4d49a590..5ed94bc95c 100644
> --- a/drivers/pci/Makefile
> +++ b/drivers/pci/Makefile
> @@ -48,5 +48,6 @@ obj-$(CONFIG_PCIE_INTEL_FPGA) += pcie_intel_fpga.o
> obj-$(CONFIG_PCI_KEYSTONE) += pcie_dw_ti.o
> obj-$(CONFIG_PCIE_MEDIATEK) += pcie_mediatek.o
> obj-$(CONFIG_PCIE_ROCKCHIP) += pcie_rockchip.o
> +obj-$(CONFIG_PCIE_DW_ROCKCHIP) += pcie_dw_rockchip.o
> obj-$(CONFIG_PCI_BRCMSTB) += pcie_brcmstb.o
> obj-$(CONFIG_PCI_OCTEONTX) += pci_octeontx.o
> diff --git a/drivers/pci/pcie_dw_rockchip.c b/drivers/pci/pcie_dw_rockchip.c
> new file mode 100644
> index 00..aa2cc15e15
> --- /dev/null
> +++ b/drivers/pci/pcie_dw_rockchip.c
> @@ -0,0 +1,755 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Rockchip DesignWare based PCIe host controller driver
> + *
> + * Copyright (c) 2021 Rockchip, Inc.
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
that one should go at the end
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +struct rk_pcie {
> + struct udevice *dev;
> + struct udevice *vpcie3v3;
> + void*dbi_base;
> + void*apb_base;
> + void*cfg_base;
> + fdt_size_t cfg_size;
> + struct phy phy;
> + struct clk_bulk clks;
> + int first_busno;
> + struct reset_ctl_bulk rsts;
> + struct gpio_descrst_gpio;
> + struct pci_region io;
> + struct pci_region mem;
these members need comments
> +};
> +
> +enum {
> + PCIBIOS_SUCCESSFUL = 0x,
> + PCIBIOS_UNSUPPORTED = -ENODEV,
Can you use a different error? This is used by drive model to know
when there is no device, but generally there is a device in a driver!
> + PCIBIOS_NODEV = -ENODEV,
> +};
> +
> +#define msleep(a) udelay((a) * 1000)
That is in linux/delay.h I think
> +
> +/* Parameters for the waiting for iATU enabled routine */
> +#define PCIE_CLIENT_GENERAL_DEBUG 0x104
> +#define PCIE_CLIENT_HOT_RESET_CTRL 0x180
> +#define PCIE_LTSSM_ENABLE_ENHANCE BIT(4)
> +#define PCIE_CLIENT_LTSSM_STATUS 0x300
> +#define SMLH_LINKUPBIT(16)
> +#define RDLH_LINKUPBIT(17)
> +#define PCIE_CLIENT_DBG_FIFO_MODE_CON 0x310
> +#define PCIE_CLIENT_DBG_FIFO_PTN_HIT_D0 0x320
> +#define PCIE_CLIENT_DBG_FIFO_PTN_HIT_D1 0x324
> +#define PCIE_CLIENT_DBG_FIFO_TRN_HIT_D0 0x328
> +#define PCIE_CLIENT_DBG_FIFO_TRN_HIT_D1 0x32c
> +#define PCIE_CLIENT_DBG_FIFO_STATUS0x350
> +#define PCIE_CLIENT_DBG_TRANSITION_DATA0x
> +#define PCIE_CLIENT_DBF_EN 0x0003
> +#define RK_PCIE_DBG0
Can you rely on DEBUG in this file instead?
> +
> +/* PCI DBICS registers */
> +#define PCIE_LINK_STATUS_REG 0x80
> +#define PCIE_LINK_STATUS_SPEED_OFF 16
> +#define PCIE_LINK_STATUS_SPEED_MASK(0xf << PCIE_LINK_STATUS_SPEED_OFF)
> +#define PCIE_LINK_STATUS_WIDTH_OFF 20
> +#define PCIE_LINK_STATUS_WIDTH_MASK(0xf << PCIE_LINK_STATUS_WIDTH_OFF)
> +
> +#define PCIE_LINK_CAPABILITY 0x7c
> +#define PCIE_LINK_CTL_20xa0
> +#define TARGET_LINK_SPEED_MASK 0xf
> +#define LINK_SPEED_GEN_1 0x1
> +#define LINK_SPEED_GEN_2 0x2
> +#define LINK_SPEED_GEN_3 0x3
> +
> +#define PCIE_MISC_CONTROL_1_OFF0x8bc
> +#define PCIE_DBI_RO_WR_EN BIT(0)
> +
> +#define PCIE_LINK_WIDTH_SPEED_CONTROL 0x80c
> +#define PORT_LOGIC_SPEED_CHANGEBIT(17)
> +
> +/*
> + * iATU Unroll-specific register definitions
> + * From 4.80 core version the
[PATCH 2/2] pci: Add Rockchip dwc based PCIe controller driver
Add Rockchip dwc based PCIe controller driver for rk356x platform.
Driver support Gen3 by operating as a Root complex.
Signed-off-by: Shawn Lin
---
drivers/pci/Kconfig| 9 +
drivers/pci/Makefile | 1 +
drivers/pci/pcie_dw_rockchip.c | 755 +
3 files changed, 765 insertions(+)
create mode 100644 drivers/pci/pcie_dw_rockchip.c
diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
index 65498bce1d..b1de38f766 100644
--- a/drivers/pci/Kconfig
+++ b/drivers/pci/Kconfig
@@ -281,6 +281,15 @@ config PCIE_ROCKCHIP
Say Y here if you want to enable PCIe controller support on
Rockchip SoCs.
+config PCIE_DW_ROCKCHIP
+ bool "Rockchip DesignWare based PCIe controller"
+ depends on ARCH_ROCKCHIP
+ select DM_PCI
+ select PHY_ROCKCHIP_SNPS_PCIE3
+ help
+ Say Y here if you want to enable DW PCIe controller support on
+ Rockchip SoCs.
+
config PCI_BRCMSTB
bool "Broadcom STB PCIe controller"
depends on DM_PCI
diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile
index 8b4d49a590..5ed94bc95c 100644
--- a/drivers/pci/Makefile
+++ b/drivers/pci/Makefile
@@ -48,5 +48,6 @@ obj-$(CONFIG_PCIE_INTEL_FPGA) += pcie_intel_fpga.o
obj-$(CONFIG_PCI_KEYSTONE) += pcie_dw_ti.o
obj-$(CONFIG_PCIE_MEDIATEK) += pcie_mediatek.o
obj-$(CONFIG_PCIE_ROCKCHIP) += pcie_rockchip.o
+obj-$(CONFIG_PCIE_DW_ROCKCHIP) += pcie_dw_rockchip.o
obj-$(CONFIG_PCI_BRCMSTB) += pcie_brcmstb.o
obj-$(CONFIG_PCI_OCTEONTX) += pci_octeontx.o
diff --git a/drivers/pci/pcie_dw_rockchip.c b/drivers/pci/pcie_dw_rockchip.c
new file mode 100644
index 00..aa2cc15e15
--- /dev/null
+++ b/drivers/pci/pcie_dw_rockchip.c
@@ -0,0 +1,755 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Rockchip DesignWare based PCIe host controller driver
+ *
+ * Copyright (c) 2021 Rockchip, Inc.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+DECLARE_GLOBAL_DATA_PTR;
+
+struct rk_pcie {
+ struct udevice *dev;
+ struct udevice *vpcie3v3;
+ void*dbi_base;
+ void*apb_base;
+ void*cfg_base;
+ fdt_size_t cfg_size;
+ struct phy phy;
+ struct clk_bulk clks;
+ int first_busno;
+ struct reset_ctl_bulk rsts;
+ struct gpio_descrst_gpio;
+ struct pci_region io;
+ struct pci_region mem;
+};
+
+enum {
+ PCIBIOS_SUCCESSFUL = 0x,
+ PCIBIOS_UNSUPPORTED = -ENODEV,
+ PCIBIOS_NODEV = -ENODEV,
+};
+
+#define msleep(a) udelay((a) * 1000)
+
+/* Parameters for the waiting for iATU enabled routine */
+#define PCIE_CLIENT_GENERAL_DEBUG 0x104
+#define PCIE_CLIENT_HOT_RESET_CTRL 0x180
+#define PCIE_LTSSM_ENABLE_ENHANCE BIT(4)
+#define PCIE_CLIENT_LTSSM_STATUS 0x300
+#define SMLH_LINKUPBIT(16)
+#define RDLH_LINKUPBIT(17)
+#define PCIE_CLIENT_DBG_FIFO_MODE_CON 0x310
+#define PCIE_CLIENT_DBG_FIFO_PTN_HIT_D0 0x320
+#define PCIE_CLIENT_DBG_FIFO_PTN_HIT_D1 0x324
+#define PCIE_CLIENT_DBG_FIFO_TRN_HIT_D0 0x328
+#define PCIE_CLIENT_DBG_FIFO_TRN_HIT_D1 0x32c
+#define PCIE_CLIENT_DBG_FIFO_STATUS0x350
+#define PCIE_CLIENT_DBG_TRANSITION_DATA0x
+#define PCIE_CLIENT_DBF_EN 0x0003
+#define RK_PCIE_DBG0
+
+/* PCI DBICS registers */
+#define PCIE_LINK_STATUS_REG 0x80
+#define PCIE_LINK_STATUS_SPEED_OFF 16
+#define PCIE_LINK_STATUS_SPEED_MASK(0xf << PCIE_LINK_STATUS_SPEED_OFF)
+#define PCIE_LINK_STATUS_WIDTH_OFF 20
+#define PCIE_LINK_STATUS_WIDTH_MASK(0xf << PCIE_LINK_STATUS_WIDTH_OFF)
+
+#define PCIE_LINK_CAPABILITY 0x7c
+#define PCIE_LINK_CTL_20xa0
+#define TARGET_LINK_SPEED_MASK 0xf
+#define LINK_SPEED_GEN_1 0x1
+#define LINK_SPEED_GEN_2 0x2
+#define LINK_SPEED_GEN_3 0x3
+
+#define PCIE_MISC_CONTROL_1_OFF0x8bc
+#define PCIE_DBI_RO_WR_EN BIT(0)
+
+#define PCIE_LINK_WIDTH_SPEED_CONTROL 0x80c
+#define PORT_LOGIC_SPEED_CHANGEBIT(17)
+
+/*
+ * iATU Unroll-specific register definitions
+ * From 4.80 core version the address translation will be made by unroll.
+ * The registers are offset from atu_base
+ */
+#define PCIE_ATU_UNR_REGION_CTRL1 0x00
+#define PCIE_ATU_UNR_REGION_CTRL2 0x04
+#define PCIE_ATU_UNR_LOWER_BASE0x08
+#define PCIE_ATU_UNR_UPPER_BASE0x0c
+#define PCIE_ATU_UNR_LIMIT 0x10
+#define PCIE_ATU_UNR_LOWER_TARGET 0x14
+#define PCIE_ATU_UNR_UPPER_TARGET 0x18
+
+#define PCIE_ATU_REGION_INDEX1 (0x1 << 0)
+#define PCIE_ATU_REGION_INDEX0 (0x0 << 0)
+#define PCIE_ATU_TYPE_MEM (0x0 << 0)
+#define PCIE_ATU_TYPE_IO
