[PATCH v4 0/2] PCI: iproc: SOC specific fixes

[PATCH v4 0/2] PCI: iproc: SOC specific fixes

[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.


Changes Since v4:
Bjorn's comments addressed.

Changes Since v3:
[re-send]

Changes Since v2:
Fix compilation errors for pcie-iproc-platform.ko which was caught by
kbuild.

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          | 134 ++++++++++++++++++++++++++++-----
 drivers/pci/host/pcie-iproc.h          |   1 +
 3 files changed, 124 insertions(+), 19 deletions(-)

-- 
1.9.1

[PATCH v4 1/2] PCI: iproc: Retry request when CRS returned from EP

[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 100 (CRS)
Completion Status is used.

As per PCI spec, CRS Software Visibility only affects config read of the
Vendor ID, for config write or any other config read the Root must
automatically re-issue configuration request again as a new request.
Iproc based PCIe RC (hw) does not retry request on its own.

As a result of the fact, PCIe RC driver (sw) should take care of CRS.
This patch fixes the problem, and attempts to read config space again in
case of PCIe code forwarding the CRS back to CPU.
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..b0abcd7 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,55 @@ 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;
+
+	/*
+	 * As per PCI spec, CRS Software Visibility only
+	 * affects config read of the Vendor ID.
+	 * For config write or any other config read the Root must
+	 * automatically re-issue configuration request again as a
+	 * new request. Iproc based PCIe RC (hw) does not retry
+	 * request on its own, so handle it here.
+	 */
+	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 +551,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

[PATCH v4 2/2] PCI: iproc: add device shutdown for PCI RC

[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 is specifically happening with Intel P3700 400GB series.
Endpoint is expecting the clock for some amount of time after PERST is
asserted, which is not happening in Stingray (iproc based SOC).
This causes NVMe to behave in undefined way.

On the contrary Intel x86 boards, will have ref clock running, so we
do not see this behavior there.

Besides, PCI spec does not stipulate about such timings.
In-fact it does not tell us, whether PCIe device should consider
refclk to be available or not to be.

It is completely up to vendor to design their EP the way they want
with respect to ref clock availability.

500ms is just based on the observation and taken as safe margin.
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, 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 b0abcd7..d1bcdc9 100644
--- a/drivers/pci/host/pcie-iproc.c
+++ b/drivers/pci/host/pcie-iproc.c
@@ -616,32 +616,40 @@ 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;
 }
+EXPORT_SYMBOL(iproc_pcie_shutdown);
 
 static int iproc_pcie_check_link(struct iproc_pcie *pcie, struct pci_bus *bus)
 {
@@ -1318,7 +1326,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

Re: [PATCH v4 2/2] PCI: iproc: add device shutdown for PCI RC

[Ray Jui]

Hi Oza,

It looks like you missed my comments during the internal review. See my 
comments inline below.


On 7/4/2017 8:08 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 is specifically happening with Intel P3700 400GB series.
> Endpoint is expecting the clock for some amount of time after PERST is
> asserted, which is not happening in Stingray (iproc based SOC).
> This causes NVMe to behave in undefined way.
>
> On the contrary Intel x86 boards, will have ref clock running, so we
> do not see this behavior there.
>
> Besides, PCI spec does not stipulate about such timings.
> In-fact it does not tell us, whether PCIe device should consider
> refclk to be available or not to be.
>
> It is completely up to vendor to design their EP the way they want
> with respect to ref clock availability.
>
> 500ms is just based on the observation and taken as safe margin.
> 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, 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 b0abcd7..d1bcdc9 100644
> --- a/drivers/pci/host/pcie-iproc.c
> +++ b/drivers/pci/host/pcie-iproc.c
> @@ -616,32 +616,40 @@ 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.
>   	 */

Why is the above comment modified? Is it even related to this patch that
implements the shutdown routine for PAXB? In addition, the original
comment is more correct. The new comment by stating the "internal
emulated endpoints (such as PAXC)" is wrong. PAXC is not an endpoint.
PAXC is the root complex interface. The endpoint is Nitro.

Thanks,

Ray
>   	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;
>   }
> +EXPORT_SYMBOL(iproc_pcie_shutdown);
>   
>   static int iproc_pcie_check_link(struct iproc_pcie *pcie, struct pci_bus *bus)
>   {
> @@ -1318,7 +1326,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);

Re: [PATCH v4 2/2] PCI: iproc: add device shutdown for PCI RC

[Oza Oza]

On Wed, Jul 5, 2017 at 9:21 AM, Ray Jui <ray.jui@broadcom.com> wrote:
> Hi Oza,
>
> It looks like you missed my comments during the internal review. See my
> comments inline below.
>
>
>
> On 7/4/2017 8:08 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 is specifically happening with Intel P3700 400GB series.
>> Endpoint is expecting the clock for some amount of time after PERST is
>> asserted, which is not happening in Stingray (iproc based SOC).
>> This causes NVMe to behave in undefined way.
>>
>> On the contrary Intel x86 boards, will have ref clock running, so we
>> do not see this behavior there.
>>
>> Besides, PCI spec does not stipulate about such timings.
>> In-fact it does not tell us, whether PCIe device should consider
>> refclk to be available or not to be.
>>
>> It is completely up to vendor to design their EP the way they want
>> with respect to ref clock availability.
>>
>> 500ms is just based on the observation and taken as safe margin.
>> 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, 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 b0abcd7..d1bcdc9 100644
>> --- a/drivers/pci/host/pcie-iproc.c
>> +++ b/drivers/pci/host/pcie-iproc.c
>> @@ -616,32 +616,40 @@ 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.
>>          */
>
>
> Why is the above comment modified? Is it even related to this patch that
> implements the shutdown routine for PAXB? In addition, the original
> comment is more correct. The new comment by stating the "internal
> emulated endpoints (such as PAXC)" is wrong. PAXC is not an endpoint.
> PAXC is the root complex interface. The endpoint is Nitro.
>
> Thanks,
>
> Ray

Thanks Ray for pointing this. I did not intend to change this but for
some reason,
comment looked changed.

Regards,
Oza.

>
>>         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;
>>   }
>> +EXPORT_SYMBOL(iproc_pcie_shutdown);
>>     static int iproc_pcie_check_link(struct iproc_pcie *pcie, struct
>> pci_bus *bus)
>>   {
>> @@ -1318,7 +1326,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);
>
>