From: Rob Herring <robh+dt@kernel.org> To: Simon Xue <xxm@rock-chips.com> Cc: Bjorn Helgaas <bhelgaas@google.com>, Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>, linux-pci@vger.kernel.org, linux-rockchip@lists.infradead.org, devicetree@vger.kernel.org, Johan Jonker <jbx6244@gmail.com>, Heiko Stuebner <heiko@sntech.de>, Peter Geis <pgwipeout@gmail.com>, Kever Yang <kever.yang@rock-chips.com>, kernel test robot <lkp@intel.com>, Shawn Lin <shawn.lin@rock-chips.com> Subject: Re: [PATCH v9 2/2] PCI: rockchip: Add Rockchip RK356X host controller driver Date: Wed, 23 Jun 2021 15:15:03 -0600 [thread overview] Message-ID: <CAL_Jsq+4SiNHFLTWU7005x46r=9cF83sL0S=bPwqxFSgPLgJ4A@mail.gmail.com> (raw) In-Reply-To: <20210506023544.169196-1-xxm@rock-chips.com> == On Wed, May 5, 2021 at 8:35 PM Simon Xue <xxm@rock-chips.com> wrote: > > Add a driver for the DesignWare-based PCIe controller found on > RK356X. The existing pcie-rockchip-host driver is only used for > the Rockchip-designed IP found on RK3399. > > Tested-by: Peter Geis <pgwipeout@gmail.com> > Reviewed-by: Kever Yang <kever.yang@rock-chips.com> > Reported-by: kernel test robot <lkp@intel.com> > Signed-off-by: Simon Xue <xxm@rock-chips.com> > Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com> > --- > drivers/pci/controller/dwc/Kconfig | 11 + > drivers/pci/controller/dwc/Makefile | 1 + > drivers/pci/controller/dwc/pcie-dw-rockchip.c | 277 ++++++++++++++++++ > 3 files changed, 289 insertions(+) > create mode 100644 drivers/pci/controller/dwc/pcie-dw-rockchip.c > > diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig > index 423d35872ce4..60d3dde9ca39 100644 > --- a/drivers/pci/controller/dwc/Kconfig > +++ b/drivers/pci/controller/dwc/Kconfig > @@ -214,6 +214,17 @@ config PCIE_ARTPEC6_EP > Enables support for the PCIe controller in the ARTPEC-6 SoC to work in > endpoint mode. This uses the DesignWare core. > > +config PCIE_ROCKCHIP_DW_HOST > + bool "Rockchip DesignWare PCIe controller" > + select PCIE_DW > + select PCIE_DW_HOST > + depends on PCI_MSI_IRQ_DOMAIN > + depends on ARCH_ROCKCHIP || COMPILE_TEST > + depends on OF > + help > + Enables support for the DesignWare PCIe controller in the > + Rockchip SoC except RK3399. > + > config PCIE_INTEL_GW > bool "Intel Gateway PCIe host controller support" > depends on OF && (X86 || COMPILE_TEST) > diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile > index eca805c1a023..689b06a7d334 100644 > --- a/drivers/pci/controller/dwc/Makefile > +++ b/drivers/pci/controller/dwc/Makefile > @@ -14,6 +14,7 @@ obj-$(CONFIG_PCI_LAYERSCAPE_EP) += pci-layerscape-ep.o > obj-$(CONFIG_PCIE_QCOM) += pcie-qcom.o > obj-$(CONFIG_PCIE_ARMADA_8K) += pcie-armada8k.o > obj-$(CONFIG_PCIE_ARTPEC6) += pcie-artpec6.o > +obj-$(CONFIG_PCIE_ROCKCHIP_DW_HOST) += pcie-dw-rockchip.o > obj-$(CONFIG_PCIE_INTEL_GW) += pcie-intel-gw.o > obj-$(CONFIG_PCIE_KIRIN) += pcie-kirin.o > obj-$(CONFIG_PCIE_HISI_STB) += pcie-histb.o > diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c > new file mode 100644 > index 000000000000..3f060144eeab > --- /dev/null > +++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c > @@ -0,0 +1,277 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * PCIe host controller driver for Rockchip SoCs. > + * > + * Copyright (C) 2021 Rockchip Electronics Co., Ltd. > + * http://www.rock-chips.com > + * > + * Author: Simon Xue <xxm@rock-chips.com> > + */ > + > +#include <linux/clk.h> > +#include <linux/gpio/consumer.h> > +#include <linux/mfd/syscon.h> > +#include <linux/module.h> > +#include <linux/of_device.h> > +#include <linux/phy/phy.h> > +#include <linux/platform_device.h> > +#include <linux/regmap.h> > +#include <linux/reset.h> > + > +#include "pcie-designware.h" > + > +/* > + * The upper 16 bits of PCIE_CLIENT_CONFIG are a write > + * mask for the lower 16 bits. > + */ > +#define HIWORD_UPDATE(mask, val) (((mask) << 16) | (val)) > +#define HIWORD_UPDATE_BIT(val) HIWORD_UPDATE(val, val) > + > +#define to_rockchip_pcie(x) dev_get_drvdata((x)->dev) > + > +#define PCIE_CLIENT_RC_MODE HIWORD_UPDATE_BIT(0x40) > +#define PCIE_CLIENT_ENABLE_LTSSM HIWORD_UPDATE_BIT(0xc) > +#define PCIE_SMLH_LINKUP BIT(16) > +#define PCIE_RDLH_LINKUP BIT(17) > +#define PCIE_LINKUP (PCIE_SMLH_LINKUP | PCIE_RDLH_LINKUP) > +#define PCIE_L0S_ENTRY 0x11 > +#define PCIE_CLIENT_GENERAL_CONTROL 0x0 > +#define PCIE_CLIENT_GENERAL_DEBUG 0x104 > +#define PCIE_CLIENT_HOT_RESET_CTRL 0x180 > +#define PCIE_CLIENT_LTSSM_STATUS 0x300 > +#define PCIE_LTSSM_ENABLE_ENHANCE BIT(4) > + > +struct rockchip_pcie { > + struct dw_pcie pci; > + void __iomem *apb_base; > + struct phy *phy; > + struct clk_bulk_data *clks; > + unsigned int clk_cnt; > + struct reset_control *rst; > + struct gpio_desc *rst_gpio; > + struct regulator *vpcie3v3; > +}; > + > +static int rockchip_pcie_readl_apb(struct rockchip_pcie *rockchip, > + u32 reg) > +{ > + return readl(rockchip->apb_base + reg); Use _relaxed variant. > +} > + > +static void rockchip_pcie_writel_apb(struct rockchip_pcie *rockchip, > + u32 val, u32 reg) > +{ > + writel(val, rockchip->apb_base + reg); Use _relaxed variant. > +} > + > +static void rockchip_pcie_enable_ltssm(struct rockchip_pcie *rockchip) > +{ > + rockchip_pcie_writel_apb(rockchip, PCIE_CLIENT_ENABLE_LTSSM, > + PCIE_CLIENT_GENERAL_CONTROL); > +} > + > +static int rockchip_pcie_link_up(struct dw_pcie *pci) > +{ > + struct rockchip_pcie *rockchip = to_rockchip_pcie(pci); > + u32 val = rockchip_pcie_readl_apb(rockchip, PCIE_CLIENT_LTSSM_STATUS); > + > + if ((val & (PCIE_RDLH_LINKUP | PCIE_SMLH_LINKUP)) == PCIE_LINKUP && ((val & PCIE_LINKUP) == PCIE_LINKUP) > + (val & GENMASK(5, 0)) == PCIE_L0S_ENTRY) Perhaps a define for GENMASK(5, 0) > + return 1; > + > + return 0; > +} > + > +static int rockchip_pcie_start_link(struct dw_pcie *pci) > +{ > + struct rockchip_pcie *rockchip = to_rockchip_pcie(pci); > + > + /* Reset device */ > + gpiod_set_value_cansleep(rockchip->rst_gpio, 0); With gpiod, 0 means inactive which is reset deasserted. Your DT should have an ACTIVE_LOW flag. > + > + rockchip_pcie_enable_ltssm(rockchip); > + > + /* > + * PCIe requires the refclk to be stable for 100µs prior to releasing > + * PERST. See table 2-4 in section 2.6.2 AC Specifications of the PCI > + * Express Card Electromechanical Specification, 1.1. However, we don't > + * know if the refclk is coming from RC's PHY or external OSC. If it's > + * from RC, so enabling LTSSM is the just right place to release #PERST. > + * We need more extra time as before, rather than setting just > + * 100us as we don't know how long should the device need to reset. > + */ > + msleep(100); > + gpiod_set_value_cansleep(rockchip->rst_gpio, 1); > + > + return 0; > +} > + > +static int rockchip_pcie_host_init(struct pcie_port *pp) > +{ > + struct dw_pcie *pci = to_dw_pcie_from_pp(pp); > + struct rockchip_pcie *rockchip = to_rockchip_pcie(pci); > + u32 val; > + > + /* LTSSM enable control mode */ > + val = rockchip_pcie_readl_apb(rockchip, PCIE_CLIENT_HOT_RESET_CTRL); > + val |= PCIE_LTSSM_ENABLE_ENHANCE | (PCIE_LTSSM_ENABLE_ENHANCE << 16); Since you have a write mask, why do you need to do a read-mod-write? And why not use HIWORD_UPDATE_BIT? > + rockchip_pcie_writel_apb(rockchip, val, PCIE_CLIENT_HOT_RESET_CTRL); > + > + rockchip_pcie_writel_apb(rockchip, PCIE_CLIENT_RC_MODE, > + PCIE_CLIENT_GENERAL_CONTROL); > + > + return 0; > +}
WARNING: multiple messages have this Message-ID (diff)
From: Rob Herring <robh+dt@kernel.org> To: Simon Xue <xxm@rock-chips.com> Cc: Bjorn Helgaas <bhelgaas@google.com>, Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>, linux-pci@vger.kernel.org, linux-rockchip@lists.infradead.org, devicetree@vger.kernel.org, Johan Jonker <jbx6244@gmail.com>, Heiko Stuebner <heiko@sntech.de>, Peter Geis <pgwipeout@gmail.com>, Kever Yang <kever.yang@rock-chips.com>, kernel test robot <lkp@intel.com>, Shawn Lin <shawn.lin@rock-chips.com> Subject: Re: [PATCH v9 2/2] PCI: rockchip: Add Rockchip RK356X host controller driver Date: Wed, 23 Jun 2021 15:15:03 -0600 [thread overview] Message-ID: <CAL_Jsq+4SiNHFLTWU7005x46r=9cF83sL0S=bPwqxFSgPLgJ4A@mail.gmail.com> (raw) In-Reply-To: <20210506023544.169196-1-xxm@rock-chips.com> == On Wed, May 5, 2021 at 8:35 PM Simon Xue <xxm@rock-chips.com> wrote: > > Add a driver for the DesignWare-based PCIe controller found on > RK356X. The existing pcie-rockchip-host driver is only used for > the Rockchip-designed IP found on RK3399. > > Tested-by: Peter Geis <pgwipeout@gmail.com> > Reviewed-by: Kever Yang <kever.yang@rock-chips.com> > Reported-by: kernel test robot <lkp@intel.com> > Signed-off-by: Simon Xue <xxm@rock-chips.com> > Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com> > --- > drivers/pci/controller/dwc/Kconfig | 11 + > drivers/pci/controller/dwc/Makefile | 1 + > drivers/pci/controller/dwc/pcie-dw-rockchip.c | 277 ++++++++++++++++++ > 3 files changed, 289 insertions(+) > create mode 100644 drivers/pci/controller/dwc/pcie-dw-rockchip.c > > diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig > index 423d35872ce4..60d3dde9ca39 100644 > --- a/drivers/pci/controller/dwc/Kconfig > +++ b/drivers/pci/controller/dwc/Kconfig > @@ -214,6 +214,17 @@ config PCIE_ARTPEC6_EP > Enables support for the PCIe controller in the ARTPEC-6 SoC to work in > endpoint mode. This uses the DesignWare core. > > +config PCIE_ROCKCHIP_DW_HOST > + bool "Rockchip DesignWare PCIe controller" > + select PCIE_DW > + select PCIE_DW_HOST > + depends on PCI_MSI_IRQ_DOMAIN > + depends on ARCH_ROCKCHIP || COMPILE_TEST > + depends on OF > + help > + Enables support for the DesignWare PCIe controller in the > + Rockchip SoC except RK3399. > + > config PCIE_INTEL_GW > bool "Intel Gateway PCIe host controller support" > depends on OF && (X86 || COMPILE_TEST) > diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile > index eca805c1a023..689b06a7d334 100644 > --- a/drivers/pci/controller/dwc/Makefile > +++ b/drivers/pci/controller/dwc/Makefile > @@ -14,6 +14,7 @@ obj-$(CONFIG_PCI_LAYERSCAPE_EP) += pci-layerscape-ep.o > obj-$(CONFIG_PCIE_QCOM) += pcie-qcom.o > obj-$(CONFIG_PCIE_ARMADA_8K) += pcie-armada8k.o > obj-$(CONFIG_PCIE_ARTPEC6) += pcie-artpec6.o > +obj-$(CONFIG_PCIE_ROCKCHIP_DW_HOST) += pcie-dw-rockchip.o > obj-$(CONFIG_PCIE_INTEL_GW) += pcie-intel-gw.o > obj-$(CONFIG_PCIE_KIRIN) += pcie-kirin.o > obj-$(CONFIG_PCIE_HISI_STB) += pcie-histb.o > diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c > new file mode 100644 > index 000000000000..3f060144eeab > --- /dev/null > +++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c > @@ -0,0 +1,277 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * PCIe host controller driver for Rockchip SoCs. > + * > + * Copyright (C) 2021 Rockchip Electronics Co., Ltd. > + * http://www.rock-chips.com > + * > + * Author: Simon Xue <xxm@rock-chips.com> > + */ > + > +#include <linux/clk.h> > +#include <linux/gpio/consumer.h> > +#include <linux/mfd/syscon.h> > +#include <linux/module.h> > +#include <linux/of_device.h> > +#include <linux/phy/phy.h> > +#include <linux/platform_device.h> > +#include <linux/regmap.h> > +#include <linux/reset.h> > + > +#include "pcie-designware.h" > + > +/* > + * The upper 16 bits of PCIE_CLIENT_CONFIG are a write > + * mask for the lower 16 bits. > + */ > +#define HIWORD_UPDATE(mask, val) (((mask) << 16) | (val)) > +#define HIWORD_UPDATE_BIT(val) HIWORD_UPDATE(val, val) > + > +#define to_rockchip_pcie(x) dev_get_drvdata((x)->dev) > + > +#define PCIE_CLIENT_RC_MODE HIWORD_UPDATE_BIT(0x40) > +#define PCIE_CLIENT_ENABLE_LTSSM HIWORD_UPDATE_BIT(0xc) > +#define PCIE_SMLH_LINKUP BIT(16) > +#define PCIE_RDLH_LINKUP BIT(17) > +#define PCIE_LINKUP (PCIE_SMLH_LINKUP | PCIE_RDLH_LINKUP) > +#define PCIE_L0S_ENTRY 0x11 > +#define PCIE_CLIENT_GENERAL_CONTROL 0x0 > +#define PCIE_CLIENT_GENERAL_DEBUG 0x104 > +#define PCIE_CLIENT_HOT_RESET_CTRL 0x180 > +#define PCIE_CLIENT_LTSSM_STATUS 0x300 > +#define PCIE_LTSSM_ENABLE_ENHANCE BIT(4) > + > +struct rockchip_pcie { > + struct dw_pcie pci; > + void __iomem *apb_base; > + struct phy *phy; > + struct clk_bulk_data *clks; > + unsigned int clk_cnt; > + struct reset_control *rst; > + struct gpio_desc *rst_gpio; > + struct regulator *vpcie3v3; > +}; > + > +static int rockchip_pcie_readl_apb(struct rockchip_pcie *rockchip, > + u32 reg) > +{ > + return readl(rockchip->apb_base + reg); Use _relaxed variant. > +} > + > +static void rockchip_pcie_writel_apb(struct rockchip_pcie *rockchip, > + u32 val, u32 reg) > +{ > + writel(val, rockchip->apb_base + reg); Use _relaxed variant. > +} > + > +static void rockchip_pcie_enable_ltssm(struct rockchip_pcie *rockchip) > +{ > + rockchip_pcie_writel_apb(rockchip, PCIE_CLIENT_ENABLE_LTSSM, > + PCIE_CLIENT_GENERAL_CONTROL); > +} > + > +static int rockchip_pcie_link_up(struct dw_pcie *pci) > +{ > + struct rockchip_pcie *rockchip = to_rockchip_pcie(pci); > + u32 val = rockchip_pcie_readl_apb(rockchip, PCIE_CLIENT_LTSSM_STATUS); > + > + if ((val & (PCIE_RDLH_LINKUP | PCIE_SMLH_LINKUP)) == PCIE_LINKUP && ((val & PCIE_LINKUP) == PCIE_LINKUP) > + (val & GENMASK(5, 0)) == PCIE_L0S_ENTRY) Perhaps a define for GENMASK(5, 0) > + return 1; > + > + return 0; > +} > + > +static int rockchip_pcie_start_link(struct dw_pcie *pci) > +{ > + struct rockchip_pcie *rockchip = to_rockchip_pcie(pci); > + > + /* Reset device */ > + gpiod_set_value_cansleep(rockchip->rst_gpio, 0); With gpiod, 0 means inactive which is reset deasserted. Your DT should have an ACTIVE_LOW flag. > + > + rockchip_pcie_enable_ltssm(rockchip); > + > + /* > + * PCIe requires the refclk to be stable for 100µs prior to releasing > + * PERST. See table 2-4 in section 2.6.2 AC Specifications of the PCI > + * Express Card Electromechanical Specification, 1.1. However, we don't > + * know if the refclk is coming from RC's PHY or external OSC. If it's > + * from RC, so enabling LTSSM is the just right place to release #PERST. > + * We need more extra time as before, rather than setting just > + * 100us as we don't know how long should the device need to reset. > + */ > + msleep(100); > + gpiod_set_value_cansleep(rockchip->rst_gpio, 1); > + > + return 0; > +} > + > +static int rockchip_pcie_host_init(struct pcie_port *pp) > +{ > + struct dw_pcie *pci = to_dw_pcie_from_pp(pp); > + struct rockchip_pcie *rockchip = to_rockchip_pcie(pci); > + u32 val; > + > + /* LTSSM enable control mode */ > + val = rockchip_pcie_readl_apb(rockchip, PCIE_CLIENT_HOT_RESET_CTRL); > + val |= PCIE_LTSSM_ENABLE_ENHANCE | (PCIE_LTSSM_ENABLE_ENHANCE << 16); Since you have a write mask, why do you need to do a read-mod-write? And why not use HIWORD_UPDATE_BIT? > + rockchip_pcie_writel_apb(rockchip, val, PCIE_CLIENT_HOT_RESET_CTRL); > + > + rockchip_pcie_writel_apb(rockchip, PCIE_CLIENT_RC_MODE, > + PCIE_CLIENT_GENERAL_CONTROL); > + > + return 0; > +} _______________________________________________ Linux-rockchip mailing list Linux-rockchip@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-rockchip
next prev parent reply other threads:[~2021-06-23 21:15 UTC|newest] Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-05-06 2:34 [PATCH v9 1/2] dt-bindings: rockchip: Add DesignWare based PCIe controller Simon Xue 2021-05-06 2:34 ` Simon Xue 2021-05-06 2:35 ` [PATCH v9 2/2] PCI: rockchip: Add Rockchip RK356X host controller driver Simon Xue 2021-05-06 2:35 ` Simon Xue 2021-06-23 14:33 ` Lorenzo Pieralisi 2021-06-23 14:33 ` Lorenzo Pieralisi 2021-06-24 2:08 ` xxm 2021-06-24 2:08 ` xxm 2021-06-24 8:23 ` Pali Rohár 2021-06-24 8:23 ` Pali Rohár 2021-06-24 8:58 ` xxm 2021-06-24 8:58 ` xxm 2021-06-23 21:15 ` Rob Herring [this message] 2021-06-23 21:15 ` Rob Herring 2021-10-27 15:41 ` [PATCH v9 1/2] dt-bindings: rockchip: Add DesignWare based PCIe controller Rob Herring 2021-10-27 15:41 ` Rob Herring
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to='CAL_Jsq+4SiNHFLTWU7005x46r=9cF83sL0S=bPwqxFSgPLgJ4A@mail.gmail.com' \ --to=robh+dt@kernel.org \ --cc=bhelgaas@google.com \ --cc=devicetree@vger.kernel.org \ --cc=heiko@sntech.de \ --cc=jbx6244@gmail.com \ --cc=kever.yang@rock-chips.com \ --cc=linux-pci@vger.kernel.org \ --cc=linux-rockchip@lists.infradead.org \ --cc=lkp@intel.com \ --cc=lorenzo.pieralisi@arm.com \ --cc=pgwipeout@gmail.com \ --cc=shawn.lin@rock-chips.com \ --cc=xxm@rock-chips.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.