linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] PCI: iproc: SOC specific fixes.
@ 2017-06-01  5:27 Oza Pawandeep
  2017-06-01  5:27 ` [PATCH 1/2] PCI: iproc: Retry request when CRS returned from EP Oza Pawandeep
  2017-06-01  5:27 ` [PATCH 2/2] PCI: iproc: add device shutdown for PCI RC Oza Pawandeep
  0 siblings, 2 replies; 6+ messages in thread
From: Oza Pawandeep @ 2017-06-01  5:27 UTC (permalink / raw)
  To: Bjorn Helgaas, Ray Jui, Scott Branden, Jon Mason,
	bcm-kernel-feedback-list, Oza Pawandeep, Andy Gospodarek,
	linux-pci, linux-kernel, Oza Pawandeep

PCI: iproc: Retry request when CRS returned from EP

Above patch adds support for CRS in PCI RC driver,
otherwise if not handled at lower level, the user
space PMD (poll mode drivers) can timeout.

PCI: iproc: add device shutdown for PCI RC

This fixes the issue where certian PCI endpoints are
not getting detected on Stingray SOC after reboot.

Oza Pawandeep (2):
  PCI: iproc: Retry request when CRS returned from EP
  PCI: iproc: add device shutdown for PCI RC

 drivers/pci/host/pcie-iproc-platform.c |   8 +++
 drivers/pci/host/pcie-iproc.c          | 125 ++++++++++++++++++++++++++++-----
 drivers/pci/host/pcie-iproc.h          |   1 +
 3 files changed, 115 insertions(+), 19 deletions(-)

-- 
1.9.1

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/2] PCI: iproc: Retry request when CRS returned from EP
  2017-06-01  5:27 [PATCH 0/2] PCI: iproc: SOC specific fixes Oza Pawandeep
@ 2017-06-01  5:27 ` Oza Pawandeep
  2017-06-01  5:27 ` [PATCH 2/2] PCI: iproc: add device shutdown for PCI RC Oza Pawandeep
  1 sibling, 0 replies; 6+ messages in thread
From: Oza Pawandeep @ 2017-06-01  5:27 UTC (permalink / raw)
  To: Bjorn Helgaas, Ray Jui, Scott Branden, Jon Mason,
	bcm-kernel-feedback-list, Oza Pawandeep, Andy Gospodarek,
	linux-pci, linux-kernel, Oza Pawandeep

For Configuration Requests only, following reset
it is possible for a device to terminate the request
but indicate that it is temporarily unable to process
the Request, but will be able to process the Request
in the future – in this case, the Configuration Request
Retry Status 10 (CRS) Completion Status is used

SPDK user space NVMe driver reinitializes NVMe which
causes reset, while doing this some configuration requests
get NAKed by Endpoint (NVMe).
Current iproc PCI driver is agnostic about it.
PAXB will forward the NAKed response in stipulated AXI code.

NVMe spec defines this timeout in 500 ms units, and this
only happens if controller has been in reset, or with new firmware,
or in abrupt shutdown case.
Meanwhile config access could result into retry.

This patch fixes the problem, and attempts to read again in case
of PAXB forwarding the NAK.

It implements iproc_pcie_config_read which gets called for Stingray.
Otherwise it falls back to PCI generic APIs.

Signed-off-by: Oza Pawandeep <oza.oza@broadcom.com>
Reviewed-by: Ray Jui <ray.jui@broadcom.com>
Reviewed-by: Scott Branden <scott.branden@broadcom.com>

diff --git a/drivers/pci/host/pcie-iproc.c b/drivers/pci/host/pcie-iproc.c
index 0f39bd2..05a3647 100644
--- a/drivers/pci/host/pcie-iproc.c
+++ b/drivers/pci/host/pcie-iproc.c
@@ -68,6 +68,9 @@
 #define APB_ERR_EN_SHIFT             0
 #define APB_ERR_EN                   BIT(APB_ERR_EN_SHIFT)
 
+#define CFG_RETRY_STATUS             0xffff0001
+#define CFG_RETRY_STATUS_TIMEOUT_US  500000 /* 500 milli-seconds. */
+
 /* derive the enum index of the outbound/inbound mapping registers */
 #define MAP_REG(base_reg, index)      ((base_reg) + (index) * 2)
 
@@ -448,6 +451,47 @@ static inline void iproc_pcie_apb_err_disable(struct pci_bus *bus,
 	}
 }
 
+static int iproc_pcie_cfg_retry(void __iomem *cfg_data_p)
+{
+	int timeout = CFG_RETRY_STATUS_TIMEOUT_US;
+	unsigned int ret;
+
+	do {
+		ret = readl(cfg_data_p);
+		if (ret == CFG_RETRY_STATUS)
+			udelay(1);
+		else
+			return PCIBIOS_SUCCESSFUL;
+	} while (timeout--);
+
+	return PCIBIOS_DEVICE_NOT_FOUND;
+}
+
+static void __iomem *iproc_pcie_map_ep_cfg_reg(struct iproc_pcie *pcie,
+					       unsigned int busno,
+					       unsigned int slot,
+					       unsigned int fn,
+					       int where)
+{
+	u16 offset;
+	u32 val;
+
+	/* EP device access */
+	val = (busno << CFG_ADDR_BUS_NUM_SHIFT) |
+		(slot << CFG_ADDR_DEV_NUM_SHIFT) |
+		(fn << CFG_ADDR_FUNC_NUM_SHIFT) |
+		(where & CFG_ADDR_REG_NUM_MASK) |
+		(1 & CFG_ADDR_CFG_TYPE_MASK);
+
+	iproc_pcie_write_reg(pcie, IPROC_PCIE_CFG_ADDR, val);
+	offset = iproc_pcie_reg_offset(pcie, IPROC_PCIE_CFG_DATA);
+
+	if (iproc_pcie_reg_is_invalid(offset))
+		return NULL;
+
+	return (pcie->base + offset);
+}
+
 /**
  * Note access to the configuration registers are protected at the higher layer
  * by 'pci_lock' in drivers/pci/access.c
@@ -499,13 +543,48 @@ static void __iomem *iproc_pcie_map_cfg_bus(struct pci_bus *bus,
 		return (pcie->base + offset);
 }
 
+static int iproc_pcie_config_read(struct pci_bus *bus, unsigned int devfn,
+				    int where, int size, u32 *val)
+{
+	struct iproc_pcie *pcie = iproc_data(bus);
+	unsigned int slot = PCI_SLOT(devfn);
+	unsigned int fn = PCI_FUNC(devfn);
+	unsigned int busno = bus->number;
+	void __iomem *cfg_data_p;
+	int ret;
+
+	/* root complex access. */
+	if (busno == 0)
+		return pci_generic_config_read32(bus, devfn, where, size, val);
+
+	cfg_data_p = iproc_pcie_map_ep_cfg_reg(pcie, busno, slot, fn, where);
+
+	if (!cfg_data_p)
+		return PCIBIOS_DEVICE_NOT_FOUND;
+
+	ret = iproc_pcie_cfg_retry(cfg_data_p);
+	if (ret)
+		return ret;
+
+	*val = readl(cfg_data_p);
+
+	if (size <= 2)
+		*val = (*val >> (8 * (where & 3))) & ((1 << (size * 8)) - 1);
+
+	return PCIBIOS_SUCCESSFUL;
+}
+
 static int iproc_pcie_config_read32(struct pci_bus *bus, unsigned int devfn,
 				    int where, int size, u32 *val)
 {
 	int ret;
+	struct iproc_pcie *pcie = iproc_data(bus);
 
 	iproc_pcie_apb_err_disable(bus, true);
-	ret = pci_generic_config_read32(bus, devfn, where, size, val);
+	if (pcie->type == IPROC_PCIE_PAXB_V2)
+		ret = iproc_pcie_config_read(bus, devfn, where, size, val);
+	else
+		ret = pci_generic_config_read32(bus, devfn, where, size, val);
 	iproc_pcie_apb_err_disable(bus, false);
 
 	return ret;
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/2] PCI: iproc: add device shutdown for PCI RC
  2017-06-01  5:27 [PATCH 0/2] PCI: iproc: SOC specific fixes Oza Pawandeep
  2017-06-01  5:27 ` [PATCH 1/2] PCI: iproc: Retry request when CRS returned from EP Oza Pawandeep
@ 2017-06-01  5:27 ` Oza Pawandeep
  2017-06-01 16:01   ` kbuild test robot
  2017-06-01 18:11   ` Ray Jui
  1 sibling, 2 replies; 6+ messages in thread
From: Oza Pawandeep @ 2017-06-01  5:27 UTC (permalink / raw)
  To: Bjorn Helgaas, Ray Jui, Scott Branden, Jon Mason,
	bcm-kernel-feedback-list, Oza Pawandeep, Andy Gospodarek,
	linux-pci, linux-kernel, Oza Pawandeep

PERST# must be asserted around ~500ms before
the reboot is applied.

During soft reset (e.g., "reboot" from Linux) on some iProc based SoCs
LCPLL clock and PERST both goes off simultaneously.
This will cause certain Endpoints Intel NVMe not get detected, upon
next boot sequence.

This happens because; Endpoint is expecting the clock for some amount of
time after PERST is asserted, which is not happening in our case
(Compare to Intel X86 boards, will have clocks running).
this cause NVMe to behave in undefined way.

Essentially clock will remain alive for 500ms with PERST# = 0
before reboot.

This patch adds platform shutdown where it should be
called in device_shutdown while reboot command is issued.

So in sequence first Endpoint Shutdown (e.g. nvme_shutdown)
followed by RC shutdown is called, which issues safe PERST
assertion.

Signed-off-by: Oza Pawandeep <oza.oza@broadcom.com>
Reviewed-by: Ray Jui <ray.jui@broadcom.com>
Reviewed-by: Scott Branden <scott.branden@broadcom.com>

diff --git a/drivers/pci/host/pcie-iproc-platform.c b/drivers/pci/host/pcie-iproc-platform.c
index 90d2bdd..9512960 100644
--- a/drivers/pci/host/pcie-iproc-platform.c
+++ b/drivers/pci/host/pcie-iproc-platform.c
@@ -131,6 +131,13 @@ static int iproc_pcie_pltfm_remove(struct platform_device *pdev)
 	return iproc_pcie_remove(pcie);
 }
 
+static void iproc_pcie_pltfm_shutdown(struct platform_device *pdev)
+{
+	struct iproc_pcie *pcie = platform_get_drvdata(pdev);
+
+	iproc_pcie_shutdown(pcie);
+}
+
 static struct platform_driver iproc_pcie_pltfm_driver = {
 	.driver = {
 		.name = "iproc-pcie",
@@ -138,6 +145,7 @@ static int iproc_pcie_pltfm_remove(struct platform_device *pdev)
 	},
 	.probe = iproc_pcie_pltfm_probe,
 	.remove = iproc_pcie_pltfm_remove,
+	.shutdown = iproc_pcie_pltfm_shutdown,
 };
 module_platform_driver(iproc_pcie_pltfm_driver);
 
diff --git a/drivers/pci/host/pcie-iproc.c b/drivers/pci/host/pcie-iproc.c
index 05a3647..e9afc63 100644
--- a/drivers/pci/host/pcie-iproc.c
+++ b/drivers/pci/host/pcie-iproc.c
@@ -608,31 +608,38 @@ static int iproc_pcie_config_write32(struct pci_bus *bus, unsigned int devfn,
 	.write = iproc_pcie_config_write32,
 };
 
-static void iproc_pcie_reset(struct iproc_pcie *pcie)
+static void iproc_pcie_perst_ctrl(struct iproc_pcie *pcie, bool assert)
 {
 	u32 val;
 
 	/*
-	 * PAXC and the internal emulated endpoint device downstream should not
-	 * be reset.  If firmware has been loaded on the endpoint device at an
-	 * earlier boot stage, reset here causes issues.
+	 * The internal emulated endpoints (such as PAXC) device downstream
+	 * should not be reset. If firmware has been loaded on the endpoint
+	 * device at an earlier boot stage, reset here causes issues.
 	 */
 	if (pcie->ep_is_internal)
 		return;
 
-	/*
-	 * Select perst_b signal as reset source. Put the device into reset,
-	 * and then bring it out of reset
-	 */
-	val = iproc_pcie_read_reg(pcie, IPROC_PCIE_CLK_CTRL);
-	val &= ~EP_PERST_SOURCE_SELECT & ~EP_MODE_SURVIVE_PERST &
-		~RC_PCIE_RST_OUTPUT;
-	iproc_pcie_write_reg(pcie, IPROC_PCIE_CLK_CTRL, val);
-	udelay(250);
-
-	val |= RC_PCIE_RST_OUTPUT;
-	iproc_pcie_write_reg(pcie, IPROC_PCIE_CLK_CTRL, val);
-	msleep(100);
+	if (assert) {
+		val = iproc_pcie_read_reg(pcie, IPROC_PCIE_CLK_CTRL);
+		val &= ~EP_PERST_SOURCE_SELECT & ~EP_MODE_SURVIVE_PERST &
+			~RC_PCIE_RST_OUTPUT;
+		iproc_pcie_write_reg(pcie, IPROC_PCIE_CLK_CTRL, val);
+		udelay(250);
+	} else {
+		val = iproc_pcie_read_reg(pcie, IPROC_PCIE_CLK_CTRL);
+		val |= RC_PCIE_RST_OUTPUT;
+		iproc_pcie_write_reg(pcie, IPROC_PCIE_CLK_CTRL, val);
+		msleep(100);
+	}
+}
+
+int iproc_pcie_shutdown(struct iproc_pcie *pcie)
+{
+	iproc_pcie_perst_ctrl(pcie, true);
+	msleep(500);
+
+	return 0;
 }
 
 static int iproc_pcie_check_link(struct iproc_pcie *pcie, struct pci_bus *bus)
@@ -1310,7 +1317,8 @@ int iproc_pcie_setup(struct iproc_pcie *pcie, struct list_head *res)
 		goto err_exit_phy;
 	}
 
-	iproc_pcie_reset(pcie);
+	iproc_pcie_perst_ctrl(pcie, true);
+	iproc_pcie_perst_ctrl(pcie, false);
 
 	if (pcie->need_ob_cfg) {
 		ret = iproc_pcie_map_ranges(pcie, res);
diff --git a/drivers/pci/host/pcie-iproc.h b/drivers/pci/host/pcie-iproc.h
index 0bbe2ea..a6b55ce 100644
--- a/drivers/pci/host/pcie-iproc.h
+++ b/drivers/pci/host/pcie-iproc.h
@@ -110,6 +110,7 @@ struct iproc_pcie {
 
 int iproc_pcie_setup(struct iproc_pcie *pcie, struct list_head *res);
 int iproc_pcie_remove(struct iproc_pcie *pcie);
+int iproc_pcie_shutdown(struct iproc_pcie *pcie);
 
 #ifdef CONFIG_PCIE_IPROC_MSI
 int iproc_msi_init(struct iproc_pcie *pcie, struct device_node *node);
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] PCI: iproc: add device shutdown for PCI RC
  2017-06-01  5:27 ` [PATCH 2/2] PCI: iproc: add device shutdown for PCI RC Oza Pawandeep
@ 2017-06-01 16:01   ` kbuild test robot
  2017-06-01 18:11   ` Ray Jui
  1 sibling, 0 replies; 6+ messages in thread
From: kbuild test robot @ 2017-06-01 16:01 UTC (permalink / raw)
  To: Oza Pawandeep
  Cc: kbuild-all, Bjorn Helgaas, Ray Jui, Scott Branden, Jon Mason,
	bcm-kernel-feedback-list, Oza Pawandeep, Andy Gospodarek,
	linux-pci, linux-kernel, Oza Pawandeep

[-- Attachment #1: Type: text/plain, Size: 1014 bytes --]

Hi Oza,

[auto build test ERROR on pci/next]
[also build test ERROR on v4.12-rc3 next-20170601]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Oza-Pawandeep/PCI-iproc-SOC-specific-fixes/20170601-144218
base:   https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git next
config: arm-allmodconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
        wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=arm 

All errors (new ones prefixed by >>):

>> ERROR: "iproc_pcie_shutdown" [drivers/pci/host/pcie-iproc-platform.ko] undefined!

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 62427 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] PCI: iproc: add device shutdown for PCI RC
  2017-06-01  5:27 ` [PATCH 2/2] PCI: iproc: add device shutdown for PCI RC Oza Pawandeep
  2017-06-01 16:01   ` kbuild test robot
@ 2017-06-01 18:11   ` Ray Jui
  2017-06-02  2:28     ` Oza Oza
  1 sibling, 1 reply; 6+ messages in thread
From: Ray Jui @ 2017-06-01 18:11 UTC (permalink / raw)
  To: Oza Pawandeep, Bjorn Helgaas, Ray Jui, Scott Branden, Jon Mason,
	bcm-kernel-feedback-list, Andy Gospodarek, linux-pci,
	linux-kernel, Oza Pawandeep

Hi Oza,

On 5/31/17 10:27 PM, Oza Pawandeep wrote:
> PERST# must be asserted around ~500ms before
> the reboot is applied.
> 
> During soft reset (e.g., "reboot" from Linux) on some iProc based SoCs
> LCPLL clock and PERST both goes off simultaneously.
> This will cause certain Endpoints Intel NVMe not get detected, upon
> next boot sequence.
> 
> This happens because; Endpoint is expecting the clock for some amount of
> time after PERST is asserted, which is not happening in our case
> (Compare to Intel X86 boards, will have clocks running).
> this cause NVMe to behave in undefined way.
> 
> Essentially clock will remain alive for 500ms with PERST# = 0
> before reboot.
> 
> This patch adds platform shutdown where it should be
> called in device_shutdown while reboot command is issued.
> 
> So in sequence first Endpoint Shutdown (e.g. nvme_shutdown)
> followed by RC shutdown is called, which issues safe PERST
> assertion.
> 
> Signed-off-by: Oza Pawandeep <oza.oza@broadcom.com>
> Reviewed-by: Ray Jui <ray.jui@broadcom.com>
> Reviewed-by: Scott Branden <scott.branden@broadcom.com>
> 
> diff --git a/drivers/pci/host/pcie-iproc-platform.c b/drivers/pci/host/pcie-iproc-platform.c
> index 90d2bdd..9512960 100644
> --- a/drivers/pci/host/pcie-iproc-platform.c
> +++ b/drivers/pci/host/pcie-iproc-platform.c
> @@ -131,6 +131,13 @@ static int iproc_pcie_pltfm_remove(struct platform_device *pdev)
>  	return iproc_pcie_remove(pcie);
>  }
>  
> +static void iproc_pcie_pltfm_shutdown(struct platform_device *pdev)
> +{
> +	struct iproc_pcie *pcie = platform_get_drvdata(pdev);
> +
> +	iproc_pcie_shutdown(pcie);
> +}
> +
>  static struct platform_driver iproc_pcie_pltfm_driver = {
>  	.driver = {
>  		.name = "iproc-pcie",
> @@ -138,6 +145,7 @@ static int iproc_pcie_pltfm_remove(struct platform_device *pdev)
>  	},
>  	.probe = iproc_pcie_pltfm_probe,
>  	.remove = iproc_pcie_pltfm_remove,
> +	.shutdown = iproc_pcie_pltfm_shutdown,
>  };
>  module_platform_driver(iproc_pcie_pltfm_driver);
>  
> diff --git a/drivers/pci/host/pcie-iproc.c b/drivers/pci/host/pcie-iproc.c
> index 05a3647..e9afc63 100644
> --- a/drivers/pci/host/pcie-iproc.c
> +++ b/drivers/pci/host/pcie-iproc.c
> @@ -608,31 +608,38 @@ static int iproc_pcie_config_write32(struct pci_bus *bus, unsigned int devfn,
>  	.write = iproc_pcie_config_write32,
>  };
>  
> -static void iproc_pcie_reset(struct iproc_pcie *pcie)
> +static void iproc_pcie_perst_ctrl(struct iproc_pcie *pcie, bool assert)
>  {
>  	u32 val;
>  
>  	/*
> -	 * PAXC and the internal emulated endpoint device downstream should not
> -	 * be reset.  If firmware has been loaded on the endpoint device at an
> -	 * earlier boot stage, reset here causes issues.
> +	 * The internal emulated endpoints (such as PAXC) device downstream
> +	 * should not be reset. If firmware has been loaded on the endpoint
> +	 * device at an earlier boot stage, reset here causes issues.
>  	 */
>  	if (pcie->ep_is_internal)
>  		return;
>  
> -	/*
> -	 * Select perst_b signal as reset source. Put the device into reset,
> -	 * and then bring it out of reset
> -	 */
> -	val = iproc_pcie_read_reg(pcie, IPROC_PCIE_CLK_CTRL);
> -	val &= ~EP_PERST_SOURCE_SELECT & ~EP_MODE_SURVIVE_PERST &
> -		~RC_PCIE_RST_OUTPUT;
> -	iproc_pcie_write_reg(pcie, IPROC_PCIE_CLK_CTRL, val);
> -	udelay(250);
> -
> -	val |= RC_PCIE_RST_OUTPUT;
> -	iproc_pcie_write_reg(pcie, IPROC_PCIE_CLK_CTRL, val);
> -	msleep(100);
> +	if (assert) {
> +		val = iproc_pcie_read_reg(pcie, IPROC_PCIE_CLK_CTRL);
> +		val &= ~EP_PERST_SOURCE_SELECT & ~EP_MODE_SURVIVE_PERST &
> +			~RC_PCIE_RST_OUTPUT;
> +		iproc_pcie_write_reg(pcie, IPROC_PCIE_CLK_CTRL, val);
> +		udelay(250);
> +	} else {
> +		val = iproc_pcie_read_reg(pcie, IPROC_PCIE_CLK_CTRL);
> +		val |= RC_PCIE_RST_OUTPUT;
> +		iproc_pcie_write_reg(pcie, IPROC_PCIE_CLK_CTRL, val);
> +		msleep(100);
> +	}
> +}
> +
> +int iproc_pcie_shutdown(struct iproc_pcie *pcie)
> +{
> +	iproc_pcie_perst_ctrl(pcie, true);
> +	msleep(500);
> +
> +	return 0;
>  }

Please export the symbol here to fix the build issue detected by kbuild
test when the iProc PCIe platform driver is compiled as a kernel module.

>  
>  static int iproc_pcie_check_link(struct iproc_pcie *pcie, struct pci_bus *bus)
> @@ -1310,7 +1317,8 @@ int iproc_pcie_setup(struct iproc_pcie *pcie, struct list_head *res)
>  		goto err_exit_phy;
>  	}
>  
> -	iproc_pcie_reset(pcie);
> +	iproc_pcie_perst_ctrl(pcie, true);
> +	iproc_pcie_perst_ctrl(pcie, false);
>  
>  	if (pcie->need_ob_cfg) {
>  		ret = iproc_pcie_map_ranges(pcie, res);
> diff --git a/drivers/pci/host/pcie-iproc.h b/drivers/pci/host/pcie-iproc.h
> index 0bbe2ea..a6b55ce 100644
> --- a/drivers/pci/host/pcie-iproc.h
> +++ b/drivers/pci/host/pcie-iproc.h
> @@ -110,6 +110,7 @@ struct iproc_pcie {
>  
>  int iproc_pcie_setup(struct iproc_pcie *pcie, struct list_head *res);
>  int iproc_pcie_remove(struct iproc_pcie *pcie);
> +int iproc_pcie_shutdown(struct iproc_pcie *pcie);
>  
>  #ifdef CONFIG_PCIE_IPROC_MSI
>  int iproc_msi_init(struct iproc_pcie *pcie, struct device_node *node);
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] PCI: iproc: add device shutdown for PCI RC
  2017-06-01 18:11   ` Ray Jui
@ 2017-06-02  2:28     ` Oza Oza
  0 siblings, 0 replies; 6+ messages in thread
From: Oza Oza @ 2017-06-02  2:28 UTC (permalink / raw)
  To: Ray Jui
  Cc: Bjorn Helgaas, Ray Jui, Scott Branden, Jon Mason,
	BCM Kernel Feedback, Andy Gospodarek, linux-pci, linux-kernel,
	Oza Pawandeep

On Thu, Jun 1, 2017 at 11:41 PM, Ray Jui <ray.jui@broadcom.com> wrote:
> Hi Oza,
>
> On 5/31/17 10:27 PM, Oza Pawandeep wrote:
>> PERST# must be asserted around ~500ms before
>> the reboot is applied.
>>
>> During soft reset (e.g., "reboot" from Linux) on some iProc based SoCs
>> LCPLL clock and PERST both goes off simultaneously.
>> This will cause certain Endpoints Intel NVMe not get detected, upon
>> next boot sequence.
>>
>> This happens because; Endpoint is expecting the clock for some amount of
>> time after PERST is asserted, which is not happening in our case
>> (Compare to Intel X86 boards, will have clocks running).
>> this cause NVMe to behave in undefined way.
>>
>> Essentially clock will remain alive for 500ms with PERST# = 0
>> before reboot.
>>
>> This patch adds platform shutdown where it should be
>> called in device_shutdown while reboot command is issued.
>>
>> So in sequence first Endpoint Shutdown (e.g. nvme_shutdown)
>> followed by RC shutdown is called, which issues safe PERST
>> assertion.
>>
>> Signed-off-by: Oza Pawandeep <oza.oza@broadcom.com>
>> Reviewed-by: Ray Jui <ray.jui@broadcom.com>
>> Reviewed-by: Scott Branden <scott.branden@broadcom.com>
>>
>> diff --git a/drivers/pci/host/pcie-iproc-platform.c b/drivers/pci/host/pcie-iproc-platform.c
>> index 90d2bdd..9512960 100644
>> --- a/drivers/pci/host/pcie-iproc-platform.c
>> +++ b/drivers/pci/host/pcie-iproc-platform.c
>> @@ -131,6 +131,13 @@ static int iproc_pcie_pltfm_remove(struct platform_device *pdev)
>>       return iproc_pcie_remove(pcie);
>>  }
>>
>> +static void iproc_pcie_pltfm_shutdown(struct platform_device *pdev)
>> +{
>> +     struct iproc_pcie *pcie = platform_get_drvdata(pdev);
>> +
>> +     iproc_pcie_shutdown(pcie);
>> +}
>> +
>>  static struct platform_driver iproc_pcie_pltfm_driver = {
>>       .driver = {
>>               .name = "iproc-pcie",
>> @@ -138,6 +145,7 @@ static int iproc_pcie_pltfm_remove(struct platform_device *pdev)
>>       },
>>       .probe = iproc_pcie_pltfm_probe,
>>       .remove = iproc_pcie_pltfm_remove,
>> +     .shutdown = iproc_pcie_pltfm_shutdown,
>>  };
>>  module_platform_driver(iproc_pcie_pltfm_driver);
>>
>> diff --git a/drivers/pci/host/pcie-iproc.c b/drivers/pci/host/pcie-iproc.c
>> index 05a3647..e9afc63 100644
>> --- a/drivers/pci/host/pcie-iproc.c
>> +++ b/drivers/pci/host/pcie-iproc.c
>> @@ -608,31 +608,38 @@ static int iproc_pcie_config_write32(struct pci_bus *bus, unsigned int devfn,
>>       .write = iproc_pcie_config_write32,
>>  };
>>
>> -static void iproc_pcie_reset(struct iproc_pcie *pcie)
>> +static void iproc_pcie_perst_ctrl(struct iproc_pcie *pcie, bool assert)
>>  {
>>       u32 val;
>>
>>       /*
>> -      * PAXC and the internal emulated endpoint device downstream should not
>> -      * be reset.  If firmware has been loaded on the endpoint device at an
>> -      * earlier boot stage, reset here causes issues.
>> +      * The internal emulated endpoints (such as PAXC) device downstream
>> +      * should not be reset. If firmware has been loaded on the endpoint
>> +      * device at an earlier boot stage, reset here causes issues.
>>        */
>>       if (pcie->ep_is_internal)
>>               return;
>>
>> -     /*
>> -      * Select perst_b signal as reset source. Put the device into reset,
>> -      * and then bring it out of reset
>> -      */
>> -     val = iproc_pcie_read_reg(pcie, IPROC_PCIE_CLK_CTRL);
>> -     val &= ~EP_PERST_SOURCE_SELECT & ~EP_MODE_SURVIVE_PERST &
>> -             ~RC_PCIE_RST_OUTPUT;
>> -     iproc_pcie_write_reg(pcie, IPROC_PCIE_CLK_CTRL, val);
>> -     udelay(250);
>> -
>> -     val |= RC_PCIE_RST_OUTPUT;
>> -     iproc_pcie_write_reg(pcie, IPROC_PCIE_CLK_CTRL, val);
>> -     msleep(100);
>> +     if (assert) {
>> +             val = iproc_pcie_read_reg(pcie, IPROC_PCIE_CLK_CTRL);
>> +             val &= ~EP_PERST_SOURCE_SELECT & ~EP_MODE_SURVIVE_PERST &
>> +                     ~RC_PCIE_RST_OUTPUT;
>> +             iproc_pcie_write_reg(pcie, IPROC_PCIE_CLK_CTRL, val);
>> +             udelay(250);
>> +     } else {
>> +             val = iproc_pcie_read_reg(pcie, IPROC_PCIE_CLK_CTRL);
>> +             val |= RC_PCIE_RST_OUTPUT;
>> +             iproc_pcie_write_reg(pcie, IPROC_PCIE_CLK_CTRL, val);
>> +             msleep(100);
>> +     }
>> +}
>> +
>> +int iproc_pcie_shutdown(struct iproc_pcie *pcie)
>> +{
>> +     iproc_pcie_perst_ctrl(pcie, true);
>> +     msleep(500);
>> +
>> +     return 0;
>>  }
>
> Please export the symbol here to fix the build issue detected by kbuild
> test when the iProc PCIe platform driver is compiled as a kernel module.
>

Will take care of this Ray, Thanks for pointing it out.

Regards,
Oza.

>>
>>  static int iproc_pcie_check_link(struct iproc_pcie *pcie, struct pci_bus *bus)
>> @@ -1310,7 +1317,8 @@ int iproc_pcie_setup(struct iproc_pcie *pcie, struct list_head *res)
>>               goto err_exit_phy;
>>       }
>>
>> -     iproc_pcie_reset(pcie);
>> +     iproc_pcie_perst_ctrl(pcie, true);
>> +     iproc_pcie_perst_ctrl(pcie, false);
>>
>>       if (pcie->need_ob_cfg) {
>>               ret = iproc_pcie_map_ranges(pcie, res);
>> diff --git a/drivers/pci/host/pcie-iproc.h b/drivers/pci/host/pcie-iproc.h
>> index 0bbe2ea..a6b55ce 100644
>> --- a/drivers/pci/host/pcie-iproc.h
>> +++ b/drivers/pci/host/pcie-iproc.h
>> @@ -110,6 +110,7 @@ struct iproc_pcie {
>>
>>  int iproc_pcie_setup(struct iproc_pcie *pcie, struct list_head *res);
>>  int iproc_pcie_remove(struct iproc_pcie *pcie);
>> +int iproc_pcie_shutdown(struct iproc_pcie *pcie);
>>
>>  #ifdef CONFIG_PCIE_IPROC_MSI
>>  int iproc_msi_init(struct iproc_pcie *pcie, struct device_node *node);
>>

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2017-06-02  2:29 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-01  5:27 [PATCH 0/2] PCI: iproc: SOC specific fixes Oza Pawandeep
2017-06-01  5:27 ` [PATCH 1/2] PCI: iproc: Retry request when CRS returned from EP Oza Pawandeep
2017-06-01  5:27 ` [PATCH 2/2] PCI: iproc: add device shutdown for PCI RC Oza Pawandeep
2017-06-01 16:01   ` kbuild test robot
2017-06-01 18:11   ` Ray Jui
2017-06-02  2:28     ` Oza Oza

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).