[PATCH 0/4] PCI: designware/dra7xx: misc fixes and cleanups

[PATCH 0/4] PCI: designware/dra7xx: misc fixes and cleanups

[Kishon Vijay Abraham I]

This series was created on 4.8 Kernel after merging "pci/host-designware"
and "pci/host-dra7xx" of Bjorn's PCI tree.

The first patch fixes a compilation warning in designware code.

It is now possible to force the DRA7xx controller to work in GEN1 mode
using a dt property.

It includes a fix in dra7xx driver so that even if CONFIG_PCI_MSI is enabled,
dra7xx driver can service legacy interrupts (provided the device
support only legacy interrupts).

Kishon Vijay Abraham I (4):
  PCI: designware: Fix compiler warning
  PCI: dra7xx: simplify the probe code
  PCI: dra7xx: Add support to force RC to work in GEN1 mode
  PCI: dra7xx: Enable MSI and legacy interrupts simultaneously

 Documentation/devicetree/bindings/pci/ti-pci.txt |    1 +
 drivers/pci/host/pci-dra7xx.c                    |   80 +++++++++++++---------
 drivers/pci/host/pcie-designware.c               |   10 +--
 3 files changed, 49 insertions(+), 42 deletions(-)

-- 
1.7.9.5

[PATCH 1/4] PCI: designware: Fix compiler warning

[Kishon Vijay Abraham I]

Fix the following compilation warning in dw_pcie_readl_unroll()
and dw_pcie_writel_unroll() by just invoking dw_pcie_readl_rc() and
dw_pcie_writel_rc() respectively instead of adding redundant code.

pcie-designware.c: In function 'dw_pcie_readl_unroll':
pcie-designware.c:165:32: warning: passing argument 2 of
 'pp->ops->readl_rc' makes integer from pointer without a cast
 [-Wint-conversion]
   return pp->ops->readl_rc(pp, pp->dbi_base + offset + reg);
                                ^
pcie-designware.c:165:32: note: expected 'u32 {aka unsigned int}' but
 argument is of type 'void *'
pcie-designware.c: In function 'dw_pcie_writel_unroll':
pcie-designware.c:176:31: warning: passing argument 3 of
 'pp->ops->writel_rc' makes integer from pointer without a cast
 [-Wint-conversion]
   pp->ops->writel_rc(pp, val, pp->dbi_base + offset + reg);
                               ^
drivers/pci/host/pcie-designware.c:176:31: note: expected 'u32
 {aka unsigned int}' but argument is of type 'void *'

Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
---
 drivers/pci/host/pcie-designware.c |   10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
index 5ee8772..b8feea4 100644
--- a/drivers/pci/host/pcie-designware.c
+++ b/drivers/pci/host/pcie-designware.c
@@ -161,10 +161,7 @@ static inline u32 dw_pcie_readl_unroll(struct pcie_port *pp, u32 index, u32 reg)
 {
 	u32 offset = PCIE_GET_ATU_OUTB_UNR_REG_OFFSET(index);
 
-	if (pp->ops->readl_rc)
-		return pp->ops->readl_rc(pp, pp->dbi_base + offset + reg);
-
-	return readl(pp->dbi_base + offset + reg);
+	return dw_pcie_readl_rc(pp, offset + reg);
 }
 
 static inline void dw_pcie_writel_unroll(struct pcie_port *pp, u32 index,
@@ -172,10 +169,7 @@ static inline void dw_pcie_writel_unroll(struct pcie_port *pp, u32 index,
 {
 	u32 offset = PCIE_GET_ATU_OUTB_UNR_REG_OFFSET(index);
 
-	if (pp->ops->writel_rc)
-		pp->ops->writel_rc(pp, val, pp->dbi_base + offset + reg);
-	else
-		writel(val, pp->dbi_base + offset + reg);
+	dw_pcie_writel_rc(pp, offset + reg, val);
 }
 
 static int dw_pcie_rd_own_conf(struct pcie_port *pp, int where, int size,
-- 
1.7.9.5

Re: [PATCH 1/4] PCI: designware: Fix compiler warning

[Bjorn Helgaas]

[+cc Jingoo, Kukjin, Krzysztof, linux-samsung-soc]

On Tue, Oct 11, 2016 at 06:28:32PM +0530, Kishon Vijay Abraham I wrote:
> Fix the following compilation warning in dw_pcie_readl_unroll()
> and dw_pcie_writel_unroll() by just invoking dw_pcie_readl_rc() and
> dw_pcie_writel_rc() respectively instead of adding redundant code.
> 
> pcie-designware.c: In function 'dw_pcie_readl_unroll':
> pcie-designware.c:165:32: warning: passing argument 2 of
>  'pp->ops->readl_rc' makes integer from pointer without a cast
>  [-Wint-conversion]
>    return pp->ops->readl_rc(pp, pp->dbi_base + offset + reg);
>                                 ^
> pcie-designware.c:165:32: note: expected 'u32 {aka unsigned int}' but
>  argument is of type 'void *'
> pcie-designware.c: In function 'dw_pcie_writel_unroll':
> pcie-designware.c:176:31: warning: passing argument 3 of
>  'pp->ops->writel_rc' makes integer from pointer without a cast
>  [-Wint-conversion]
>    pp->ops->writel_rc(pp, val, pp->dbi_base + offset + reg);
>                                ^
> drivers/pci/host/pcie-designware.c:176:31: note: expected 'u32
>  {aka unsigned int}' but argument is of type 'void *'
> 
> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> ---
>  drivers/pci/host/pcie-designware.c |   10 ++--------
>  1 file changed, 2 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
> index 5ee8772..b8feea4 100644
> --- a/drivers/pci/host/pcie-designware.c
> +++ b/drivers/pci/host/pcie-designware.c
> @@ -161,10 +161,7 @@ static inline u32 dw_pcie_readl_unroll(struct pcie_port *pp, u32 index, u32 reg)
>  {
>  	u32 offset = PCIE_GET_ATU_OUTB_UNR_REG_OFFSET(index);
>  
> -	if (pp->ops->readl_rc)
> -		return pp->ops->readl_rc(pp, pp->dbi_base + offset + reg);

I think this was worse than just a compiler warning, wasn't it?  I
think I completely broke exynos (the only implementer of
pp->ops->readl_rc) because we passed "pp->dbi_base + offset + reg" to
exynos_pcie_readl_rc(), which then added pp->dbi_base again:

  val = readl(pp->dbi_base + reg);

I incorporated your patch to use dw_pcie_readl_rc() directly (thanks a
lot for noticing that!) into pci/host-designware.

Then I rebased all the branches that were based on pci/host-designware
and repushed everything:

  pci/host-designware
  pci/host-armada
  pci/host-artpec
  pci/host-dra7xx
  pci/host-exynos
  pci/host-hisi
  pci/host-imx6
  pci/host-keystone
  pci/host-layerscape
  pci/host-qcom
  pci/host-spear

> -	return readl(pp->dbi_base + offset + reg);
> +	return dw_pcie_readl_rc(pp, offset + reg);
>  }
>  
>  static inline void dw_pcie_writel_unroll(struct pcie_port *pp, u32 index,
> @@ -172,10 +169,7 @@ static inline void dw_pcie_writel_unroll(struct pcie_port *pp, u32 index,
>  {
>  	u32 offset = PCIE_GET_ATU_OUTB_UNR_REG_OFFSET(index);
>  
> -	if (pp->ops->writel_rc)
> -		pp->ops->writel_rc(pp, val, pp->dbi_base + offset + reg);
> -	else
> -		writel(val, pp->dbi_base + offset + reg);
> +	dw_pcie_writel_rc(pp, offset + reg, val);
>  }
>  
>  static int dw_pcie_rd_own_conf(struct pcie_port *pp, int where, int size,
> -- 
> 1.7.9.5
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/4] PCI: dra7xx: simplify the probe code

[Kishon Vijay Abraham I]

No functional change. Use the new devm_gpiod_get_optional() to
simplify the probe code.

Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
Signed-off-by: Sekhar Nori <nsekhar@ti.com>
---
 drivers/pci/host/pci-dra7xx.c |   21 +++++----------------
 1 file changed, 5 insertions(+), 16 deletions(-)

diff --git a/drivers/pci/host/pci-dra7xx.c b/drivers/pci/host/pci-dra7xx.c
index 45eb833..4ccba6d 100644
--- a/drivers/pci/host/pci-dra7xx.c
+++ b/drivers/pci/host/pci-dra7xx.c
@@ -325,9 +325,7 @@ static int __init dra7xx_pcie_probe(struct platform_device *pdev)
 	void __iomem *base;
 	struct resource *res;
 	char name[10];
-	int gpio_sel;
-	enum of_gpio_flags flags;
-	unsigned long gpio_flags;
+	struct gpio_desc *reset;
 
 	dra7xx_pcie = devm_kzalloc(dev, sizeof(*dra7xx_pcie), GFP_KERNEL);
 	if (!dra7xx_pcie)
@@ -393,19 +391,10 @@ static int __init dra7xx_pcie_probe(struct platform_device *pdev)
 		goto err_get_sync;
 	}
 
-	gpio_sel = of_get_gpio_flags(dev->of_node, 0, &flags);
-	if (gpio_is_valid(gpio_sel)) {
-		gpio_flags = (flags & OF_GPIO_ACTIVE_LOW) ?
-				GPIOF_OUT_INIT_LOW : GPIOF_OUT_INIT_HIGH;
-		ret = devm_gpio_request_one(dev, gpio_sel, gpio_flags,
-					    "pcie_reset");
-		if (ret) {
-			dev_err(dev, "gpio%d request failed, ret %d\n",
-				gpio_sel, ret);
-			goto err_gpio;
-		}
-	} else if (gpio_sel == -EPROBE_DEFER) {
-		ret = -EPROBE_DEFER;
+	reset = devm_gpiod_get_optional(dev, NULL, GPIOD_OUT_HIGH);
+	if (IS_ERR(reset)) {
+		ret = PTR_ERR(reset);
+		dev_err(&pdev->dev, "gpio request failed, ret %d\n", ret);
 		goto err_gpio;
 	}
 
-- 
1.7.9.5

[PATCH 3/4] PCI: dra7xx: Add support to force RC to work in GEN1 mode

[Kishon Vijay Abraham I]

PCIe in AM57x/DRA7x devices is by default
configured to work in GEN2 mode.  However there
may be situations when working in GEN1 mode is
desired. One example is limitation i925 (PCIe GEN2
mode not supported at junction temperatures < 0C).

Add support to force Root Complex to work in GEN1
mode if so desired, but don't force GEN1 mode on
any board just yet.

Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
Signed-off-by: Sekhar Nori <nsekhar@ti.com>
---
 Documentation/devicetree/bindings/pci/ti-pci.txt |    1 +
 drivers/pci/host/pci-dra7xx.c                    |   27 ++++++++++++++++++++++
 2 files changed, 28 insertions(+)

diff --git a/Documentation/devicetree/bindings/pci/ti-pci.txt b/Documentation/devicetree/bindings/pci/ti-pci.txt
index 60e2516..a3d6ca3 100644
--- a/Documentation/devicetree/bindings/pci/ti-pci.txt
+++ b/Documentation/devicetree/bindings/pci/ti-pci.txt
@@ -25,6 +25,7 @@ PCIe Designware Controller
 
 Optional Property:
  - gpios : Should be added if a gpio line is required to drive PERST# line
+ - ti,pcie-is-gen1 : Force the PCIe controller to work in GEN1 (2.5 GT/s).
 
 Example:
 axi {
diff --git a/drivers/pci/host/pci-dra7xx.c b/drivers/pci/host/pci-dra7xx.c
index 4ccba6d..2a669cb 100644
--- a/drivers/pci/host/pci-dra7xx.c
+++ b/drivers/pci/host/pci-dra7xx.c
@@ -63,11 +63,14 @@
 #define	LINK_UP						BIT(16)
 #define	DRA7XX_CPU_TO_BUS_ADDR				0x0FFFFFFF
 
+#define EXP_CAP_ID_OFFSET				0x70
+
 struct dra7xx_pcie {
 	struct pcie_port	pp;
 	void __iomem		*base;		/* DT ti_conf */
 	int			phy_count;	/* DT phy-names count */
 	struct phy		**phy;
+	bool			is_gen1;
 };
 
 #define to_dra7xx_pcie(x)	container_of((x), struct dra7xx_pcie, pp)
@@ -96,12 +99,33 @@ static int dra7xx_pcie_establish_link(struct dra7xx_pcie *dra7xx_pcie)
 	struct pcie_port *pp = &dra7xx_pcie->pp;
 	struct device *dev = pp->dev;
 	u32 reg;
+	u32 exp_cap_off = EXP_CAP_ID_OFFSET;
 
 	if (dw_pcie_link_up(pp)) {
 		dev_err(dev, "link is already up\n");
 		return 0;
 	}
 
+	if (dra7xx_pcie->is_gen1) {
+		dw_pcie_cfg_read(pp->dbi_base + exp_cap_off + PCI_EXP_LNKCAP,
+				 4, &reg);
+		if ((reg & PCI_EXP_LNKCAP_SLS) != PCI_EXP_LNKCAP_SLS_2_5GB) {
+			reg &= ~((u32)PCI_EXP_LNKCAP_SLS);
+			reg |= PCI_EXP_LNKCAP_SLS_2_5GB;
+			dw_pcie_cfg_write(pp->dbi_base + exp_cap_off +
+					  PCI_EXP_LNKCAP, 4, reg);
+		}
+
+		dw_pcie_cfg_read(pp->dbi_base + exp_cap_off + PCI_EXP_LNKCTL2,
+				 2, &reg);
+		if ((reg & PCI_EXP_LNKCAP_SLS) != PCI_EXP_LNKCAP_SLS_2_5GB) {
+			reg &= ~((u32)PCI_EXP_LNKCAP_SLS);
+			reg |= PCI_EXP_LNKCAP_SLS_2_5GB;
+			dw_pcie_cfg_write(pp->dbi_base + exp_cap_off +
+					  PCI_EXP_LNKCTL2, 2, reg);
+		}
+	}
+
 	reg = dra7xx_pcie_readl(dra7xx_pcie, PCIECTRL_DRA7XX_CONF_DEVICE_CMD);
 	reg |= LTSSM_EN;
 	dra7xx_pcie_writel(dra7xx_pcie, PCIECTRL_DRA7XX_CONF_DEVICE_CMD, reg);
@@ -402,6 +426,9 @@ static int __init dra7xx_pcie_probe(struct platform_device *pdev)
 	reg &= ~LTSSM_EN;
 	dra7xx_pcie_writel(dra7xx_pcie, PCIECTRL_DRA7XX_CONF_DEVICE_CMD, reg);
 
+	if (of_property_read_bool(np, "ti,pcie-is-gen1"))
+		dra7xx_pcie->is_gen1 = true;
+
 	ret = dra7xx_add_pcie_port(dra7xx_pcie, pdev);
 	if (ret < 0)
 		goto err_gpio;
-- 
1.7.9.5

Re: [PATCH 3/4] PCI: dra7xx: Add support to force RC to work in GEN1 mode

[Sekhar Nori]

On Tuesday 11 October 2016 06:28 PM, Kishon Vijay Abraham I wrote:
> PCIe in AM57x/DRA7x devices is by default
> configured to work in GEN2 mode.  However there
> may be situations when working in GEN1 mode is
> desired. One example is limitation i925 (PCIe GEN2
> mode not supported at junction temperatures < 0C).
> 
> Add support to force Root Complex to work in GEN1
> mode if so desired, but don't force GEN1 mode on
> any board just yet.
> 
> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> Signed-off-by: Sekhar Nori <nsekhar@ti.com>
> ---
>  Documentation/devicetree/bindings/pci/ti-pci.txt |    1 +
>  drivers/pci/host/pci-dra7xx.c                    |   27 ++++++++++++++++++++++
>  2 files changed, 28 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/pci/ti-pci.txt b/Documentation/devicetree/bindings/pci/ti-pci.txt
> index 60e2516..a3d6ca3 100644
> --- a/Documentation/devicetree/bindings/pci/ti-pci.txt
> +++ b/Documentation/devicetree/bindings/pci/ti-pci.txt
> @@ -25,6 +25,7 @@ PCIe Designware Controller
>  
>  Optional Property:
>   - gpios : Should be added if a gpio line is required to drive PERST# line
> + - ti,pcie-is-gen1 : Force the PCIe controller to work in GEN1 (2.5 GT/s).
>  
>  Example:
>  axi {
> diff --git a/drivers/pci/host/pci-dra7xx.c b/drivers/pci/host/pci-dra7xx.c
> index 4ccba6d..2a669cb 100644
> --- a/drivers/pci/host/pci-dra7xx.c
> +++ b/drivers/pci/host/pci-dra7xx.c
> @@ -63,11 +63,14 @@
>  #define	LINK_UP						BIT(16)
>  #define	DRA7XX_CPU_TO_BUS_ADDR				0x0FFFFFFF
>  
> +#define EXP_CAP_ID_OFFSET				0x70
> +
>  struct dra7xx_pcie {
>  	struct pcie_port	pp;
>  	void __iomem		*base;		/* DT ti_conf */
>  	int			phy_count;	/* DT phy-names count */
>  	struct phy		**phy;
> +	bool			is_gen1;

I think it will be nice to reference the errata number as comments in
the code along with references to TI public documentation that explains
the said errata.

Or may be this can be added to the binding documentation. Somewhere 'git
grep' can find.

BTW, per Documentation/devicetree/bindings/submitting-patches.txt, the
binding documentation should be a separate patch.

Thanks,
Sekhar

Re: [PATCH 3/4] PCI: dra7xx: Add support to force RC to work in GEN1 mode

[Shawn Lin]

On 2016/10/11 20:58, Kishon Vijay Abraham I wrote:
> PCIe in AM57x/DRA7x devices is by default
> configured to work in GEN2 mode.  However there
> may be situations when working in GEN1 mode is
> desired. One example is limitation i925 (PCIe GEN2
> mode not supported at junction temperatures < 0C).
>

Just a drive-by comment.

Seems there are already much more requirments for host
drivers to know the limitation of link speed, so
I pushed patches recently to consolidate it for host
drivers[0] suggested by Rob. So maybe you could use that
instead. :)

[0]
https://patchwork.kernel.org/patch/9371931/
https://patchwork.kernel.org/patch/9371929/
> Add support to force Root Complex to work in GEN1
> mode if so desired, but don't force GEN1 mode on
> any board just yet.
>
> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> Signed-off-by: Sekhar Nori <nsekhar@ti.com>
> ---
>  Documentation/devicetree/bindings/pci/ti-pci.txt |    1 +
>  drivers/pci/host/pci-dra7xx.c                    |   27 ++++++++++++++++++++++
>  2 files changed, 28 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/pci/ti-pci.txt b/Documentation/devicetree/bindings/pci/ti-pci.txt
> index 60e2516..a3d6ca3 100644
> --- a/Documentation/devicetree/bindings/pci/ti-pci.txt
> +++ b/Documentation/devicetree/bindings/pci/ti-pci.txt
> @@ -25,6 +25,7 @@ PCIe Designware Controller
>
>  Optional Property:
>   - gpios : Should be added if a gpio line is required to drive PERST# line
> + - ti,pcie-is-gen1 : Force the PCIe controller to work in GEN1 (2.5 GT/s).
>
>  Example:
>  axi {
> diff --git a/drivers/pci/host/pci-dra7xx.c b/drivers/pci/host/pci-dra7xx.c
> index 4ccba6d..2a669cb 100644
> --- a/drivers/pci/host/pci-dra7xx.c
> +++ b/drivers/pci/host/pci-dra7xx.c
> @@ -63,11 +63,14 @@
>  #define	LINK_UP						BIT(16)
>  #define	DRA7XX_CPU_TO_BUS_ADDR				0x0FFFFFFF
>
> +#define EXP_CAP_ID_OFFSET				0x70
> +
>  struct dra7xx_pcie {
>  	struct pcie_port	pp;
>  	void __iomem		*base;		/* DT ti_conf */
>  	int			phy_count;	/* DT phy-names count */
>  	struct phy		**phy;
> +	bool			is_gen1;
>  };
>
>  #define to_dra7xx_pcie(x)	container_of((x), struct dra7xx_pcie, pp)
> @@ -96,12 +99,33 @@ static int dra7xx_pcie_establish_link(struct dra7xx_pcie *dra7xx_pcie)
>  	struct pcie_port *pp = &dra7xx_pcie->pp;
>  	struct device *dev = pp->dev;
>  	u32 reg;
> +	u32 exp_cap_off = EXP_CAP_ID_OFFSET;
>
>  	if (dw_pcie_link_up(pp)) {
>  		dev_err(dev, "link is already up\n");
>  		return 0;
>  	}
>
> +	if (dra7xx_pcie->is_gen1) {
> +		dw_pcie_cfg_read(pp->dbi_base + exp_cap_off + PCI_EXP_LNKCAP,
> +				 4, &reg);
> +		if ((reg & PCI_EXP_LNKCAP_SLS) != PCI_EXP_LNKCAP_SLS_2_5GB) {
> +			reg &= ~((u32)PCI_EXP_LNKCAP_SLS);
> +			reg |= PCI_EXP_LNKCAP_SLS_2_5GB;
> +			dw_pcie_cfg_write(pp->dbi_base + exp_cap_off +
> +					  PCI_EXP_LNKCAP, 4, reg);
> +		}
> +
> +		dw_pcie_cfg_read(pp->dbi_base + exp_cap_off + PCI_EXP_LNKCTL2,
> +				 2, &reg);
> +		if ((reg & PCI_EXP_LNKCAP_SLS) != PCI_EXP_LNKCAP_SLS_2_5GB) {
> +			reg &= ~((u32)PCI_EXP_LNKCAP_SLS);
> +			reg |= PCI_EXP_LNKCAP_SLS_2_5GB;
> +			dw_pcie_cfg_write(pp->dbi_base + exp_cap_off +
> +					  PCI_EXP_LNKCTL2, 2, reg);
> +		}
> +	}
> +
>  	reg = dra7xx_pcie_readl(dra7xx_pcie, PCIECTRL_DRA7XX_CONF_DEVICE_CMD);
>  	reg |= LTSSM_EN;
>  	dra7xx_pcie_writel(dra7xx_pcie, PCIECTRL_DRA7XX_CONF_DEVICE_CMD, reg);
> @@ -402,6 +426,9 @@ static int __init dra7xx_pcie_probe(struct platform_device *pdev)
>  	reg &= ~LTSSM_EN;
>  	dra7xx_pcie_writel(dra7xx_pcie, PCIECTRL_DRA7XX_CONF_DEVICE_CMD, reg);
>
> +	if (of_property_read_bool(np, "ti,pcie-is-gen1"))
> +		dra7xx_pcie->is_gen1 = true;
> +
>  	ret = dra7xx_add_pcie_port(dra7xx_pcie, pdev);
>  	if (ret < 0)
>  		goto err_gpio;
>


-- 
Best Regards
Shawn Lin

Re: [PATCH 3/4] PCI: dra7xx: Add support to force RC to work in GEN1 mode

[Bjorn Helgaas]

Hi Kishon,

On Tue, Oct 11, 2016 at 06:28:34PM +0530, Kishon Vijay Abraham I wrote:
> PCIe in AM57x/DRA7x devices is by default
> configured to work in GEN2 mode.  However there
> may be situations when working in GEN1 mode is
> desired. One example is limitation i925 (PCIe GEN2
> mode not supported at junction temperatures < 0C).
> 
> Add support to force Root Complex to work in GEN1
> mode if so desired, but don't force GEN1 mode on
> any board just yet.
> 
> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> Signed-off-by: Sekhar Nori <nsekhar@ti.com>
> ---
>  Documentation/devicetree/bindings/pci/ti-pci.txt |    1 +
>  drivers/pci/host/pci-dra7xx.c                    |   27 ++++++++++++++++++++++
>  2 files changed, 28 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/pci/ti-pci.txt b/Documentation/devicetree/bindings/pci/ti-pci.txt
> index 60e2516..a3d6ca3 100644
> --- a/Documentation/devicetree/bindings/pci/ti-pci.txt
> +++ b/Documentation/devicetree/bindings/pci/ti-pci.txt
> @@ -25,6 +25,7 @@ PCIe Designware Controller
>  
>  Optional Property:
>   - gpios : Should be added if a gpio line is required to drive PERST# line
> + - ti,pcie-is-gen1 : Force the PCIe controller to work in GEN1 (2.5 GT/s).

Can we use "max-link-speed" so it's similar to imx6?

>  Example:
>  axi {
> diff --git a/drivers/pci/host/pci-dra7xx.c b/drivers/pci/host/pci-dra7xx.c
> index 4ccba6d..2a669cb 100644
> --- a/drivers/pci/host/pci-dra7xx.c
> +++ b/drivers/pci/host/pci-dra7xx.c
> @@ -63,11 +63,14 @@
>  #define	LINK_UP						BIT(16)
>  #define	DRA7XX_CPU_TO_BUS_ADDR				0x0FFFFFFF
>  
> +#define EXP_CAP_ID_OFFSET				0x70
> +
>  struct dra7xx_pcie {
>  	struct pcie_port	pp;
>  	void __iomem		*base;		/* DT ti_conf */
>  	int			phy_count;	/* DT phy-names count */
>  	struct phy		**phy;
> +	bool			is_gen1;
>  };
>  
>  #define to_dra7xx_pcie(x)	container_of((x), struct dra7xx_pcie, pp)
> @@ -96,12 +99,33 @@ static int dra7xx_pcie_establish_link(struct dra7xx_pcie *dra7xx_pcie)
>  	struct pcie_port *pp = &dra7xx_pcie->pp;
>  	struct device *dev = pp->dev;
>  	u32 reg;
> +	u32 exp_cap_off = EXP_CAP_ID_OFFSET;
>  
>  	if (dw_pcie_link_up(pp)) {
>  		dev_err(dev, "link is already up\n");
>  		return 0;
>  	}
>  
> +	if (dra7xx_pcie->is_gen1) {
> +		dw_pcie_cfg_read(pp->dbi_base + exp_cap_off + PCI_EXP_LNKCAP,
> +				 4, &reg);
> +		if ((reg & PCI_EXP_LNKCAP_SLS) != PCI_EXP_LNKCAP_SLS_2_5GB) {
> +			reg &= ~((u32)PCI_EXP_LNKCAP_SLS);
> +			reg |= PCI_EXP_LNKCAP_SLS_2_5GB;
> +			dw_pcie_cfg_write(pp->dbi_base + exp_cap_off +
> +					  PCI_EXP_LNKCAP, 4, reg);
> +		}
> +
> +		dw_pcie_cfg_read(pp->dbi_base + exp_cap_off + PCI_EXP_LNKCTL2,
> +				 2, &reg);
> +		if ((reg & PCI_EXP_LNKCAP_SLS) != PCI_EXP_LNKCAP_SLS_2_5GB) {
> +			reg &= ~((u32)PCI_EXP_LNKCAP_SLS);
> +			reg |= PCI_EXP_LNKCAP_SLS_2_5GB;
> +			dw_pcie_cfg_write(pp->dbi_base + exp_cap_off +
> +					  PCI_EXP_LNKCTL2, 2, reg);
> +		}
> +	}
> +
>  	reg = dra7xx_pcie_readl(dra7xx_pcie, PCIECTRL_DRA7XX_CONF_DEVICE_CMD);
>  	reg |= LTSSM_EN;
>  	dra7xx_pcie_writel(dra7xx_pcie, PCIECTRL_DRA7XX_CONF_DEVICE_CMD, reg);
> @@ -402,6 +426,9 @@ static int __init dra7xx_pcie_probe(struct platform_device *pdev)
>  	reg &= ~LTSSM_EN;
>  	dra7xx_pcie_writel(dra7xx_pcie, PCIECTRL_DRA7XX_CONF_DEVICE_CMD, reg);
>  
> +	if (of_property_read_bool(np, "ti,pcie-is-gen1"))
> +		dra7xx_pcie->is_gen1 = true;
> +
>  	ret = dra7xx_add_pcie_port(dra7xx_pcie, pdev);
>  	if (ret < 0)
>  		goto err_gpio;
> -- 
> 1.7.9.5
> 

Re: [PATCH 3/4] PCI: dra7xx: Add support to force RC to work in GEN1 mode

[Kishon Vijay Abraham I]

Hi,

On Saturday 12 November 2016 02:45 AM, Bjorn Helgaas wrote:
> Hi Kishon,
> 
> On Tue, Oct 11, 2016 at 06:28:34PM +0530, Kishon Vijay Abraham I wrote:
>> PCIe in AM57x/DRA7x devices is by default
>> configured to work in GEN2 mode.  However there
>> may be situations when working in GEN1 mode is
>> desired. One example is limitation i925 (PCIe GEN2
>> mode not supported at junction temperatures < 0C).
>>
>> Add support to force Root Complex to work in GEN1
>> mode if so desired, but don't force GEN1 mode on
>> any board just yet.
>>
>> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
>> Signed-off-by: Sekhar Nori <nsekhar@ti.com>
>> ---
>>  Documentation/devicetree/bindings/pci/ti-pci.txt |    1 +
>>  drivers/pci/host/pci-dra7xx.c                    |   27 ++++++++++++++++++++++
>>  2 files changed, 28 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/pci/ti-pci.txt b/Documentation/devicetree/bindings/pci/ti-pci.txt
>> index 60e2516..a3d6ca3 100644
>> --- a/Documentation/devicetree/bindings/pci/ti-pci.txt
>> +++ b/Documentation/devicetree/bindings/pci/ti-pci.txt
>> @@ -25,6 +25,7 @@ PCIe Designware Controller
>>  
>>  Optional Property:
>>   - gpios : Should be added if a gpio line is required to drive PERST# line
>> + - ti,pcie-is-gen1 : Force the PCIe controller to work in GEN1 (2.5 GT/s).
> 
> Can we use "max-link-speed" so it's similar to imx6?

yeah, maybe we should make it a generic PCI property?

Thanks
Kishon

Re: [PATCH 3/4] PCI: dra7xx: Add support to force RC to work in GEN1 mode

[Bjorn Helgaas]

[+cc Shawn]

On Sat, Nov 12, 2016 at 12:40:10PM +0530, Kishon Vijay Abraham I wrote:
> Hi,
> 
> On Saturday 12 November 2016 02:45 AM, Bjorn Helgaas wrote:
> > Hi Kishon,
> > 
> > On Tue, Oct 11, 2016 at 06:28:34PM +0530, Kishon Vijay Abraham I wrote:
> >> PCIe in AM57x/DRA7x devices is by default
> >> configured to work in GEN2 mode.  However there
> >> may be situations when working in GEN1 mode is
> >> desired. One example is limitation i925 (PCIe GEN2
> >> mode not supported at junction temperatures < 0C).
> >>
> >> Add support to force Root Complex to work in GEN1
> >> mode if so desired, but don't force GEN1 mode on
> >> any board just yet.
> >>
> >> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> >> Signed-off-by: Sekhar Nori <nsekhar@ti.com>
> >> ---
> >>  Documentation/devicetree/bindings/pci/ti-pci.txt |    1 +
> >>  drivers/pci/host/pci-dra7xx.c                    |   27 ++++++++++++++++++++++
> >>  2 files changed, 28 insertions(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/pci/ti-pci.txt b/Documentation/devicetree/bindings/pci/ti-pci.txt
> >> index 60e2516..a3d6ca3 100644
> >> --- a/Documentation/devicetree/bindings/pci/ti-pci.txt
> >> +++ b/Documentation/devicetree/bindings/pci/ti-pci.txt
> >> @@ -25,6 +25,7 @@ PCIe Designware Controller
> >>  
> >>  Optional Property:
> >>   - gpios : Should be added if a gpio line is required to drive PERST# line
> >> + - ti,pcie-is-gen1 : Force the PCIe controller to work in GEN1 (2.5 GT/s).
> > 
> > Can we use "max-link-speed" so it's similar to imx6?
> 
> yeah, maybe we should make it a generic PCI property?

I forgot that Shawn has already done this!  I had already merged those
patches on pci/host-rockchip, but I moved them to pci/host since
they're not Rockchip-specific.  Can you take a look at that and see if
you can do what you need based on that pci/host branch?

Bjorn

Re: [PATCH 3/4] PCI: dra7xx: Add support to force RC to work in GEN1 mode

[Kishon Vijay Abraham I]

Hi Bjorn,

On Tuesday 15 November 2016 02:54 AM, Bjorn Helgaas wrote:
> [+cc Shawn]
> 
> On Sat, Nov 12, 2016 at 12:40:10PM +0530, Kishon Vijay Abraham I wrote:
>> Hi,
>>
>> On Saturday 12 November 2016 02:45 AM, Bjorn Helgaas wrote:
>>> Hi Kishon,
>>>
>>> On Tue, Oct 11, 2016 at 06:28:34PM +0530, Kishon Vijay Abraham I wrote:
>>>> PCIe in AM57x/DRA7x devices is by default
>>>> configured to work in GEN2 mode.  However there
>>>> may be situations when working in GEN1 mode is
>>>> desired. One example is limitation i925 (PCIe GEN2
>>>> mode not supported at junction temperatures < 0C).
>>>>
>>>> Add support to force Root Complex to work in GEN1
>>>> mode if so desired, but don't force GEN1 mode on
>>>> any board just yet.
>>>>
>>>> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
>>>> Signed-off-by: Sekhar Nori <nsekhar@ti.com>
>>>> ---
>>>>  Documentation/devicetree/bindings/pci/ti-pci.txt |    1 +
>>>>  drivers/pci/host/pci-dra7xx.c                    |   27 ++++++++++++++++++++++
>>>>  2 files changed, 28 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/pci/ti-pci.txt b/Documentation/devicetree/bindings/pci/ti-pci.txt
>>>> index 60e2516..a3d6ca3 100644
>>>> --- a/Documentation/devicetree/bindings/pci/ti-pci.txt
>>>> +++ b/Documentation/devicetree/bindings/pci/ti-pci.txt
>>>> @@ -25,6 +25,7 @@ PCIe Designware Controller
>>>>  
>>>>  Optional Property:
>>>>   - gpios : Should be added if a gpio line is required to drive PERST# line
>>>> + - ti,pcie-is-gen1 : Force the PCIe controller to work in GEN1 (2.5 GT/s).
>>>
>>> Can we use "max-link-speed" so it's similar to imx6?
>>
>> yeah, maybe we should make it a generic PCI property?
> 
> I forgot that Shawn has already done this!  I had already merged those
> patches on pci/host-rockchip, but I moved them to pci/host since
> they're not Rockchip-specific.  Can you take a look at that and see if
> you can do what you need based on that pci/host branch?

Sure, I'll look at that on Monday when I'm back from vacation.

Cheers
Kishon

[PATCH 4/4] PCI: dra7xx: Enable MSI and legacy interrupts simultaneously

[Kishon Vijay Abraham I]

dra7xx driver had a bug in that if CONFIG_PCI_MSI config is enabled,
it doesn't support legacy interrupt. Fix it here so that both MSI and
legacy interrupts can be enabled simultaneously and one of them will
be used based on the connected device.

Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
---
 drivers/pci/host/pci-dra7xx.c |   32 ++++++++++++++------------------
 1 file changed, 14 insertions(+), 18 deletions(-)

diff --git a/drivers/pci/host/pci-dra7xx.c b/drivers/pci/host/pci-dra7xx.c
index 2a669cb..1a68759 100644
--- a/drivers/pci/host/pci-dra7xx.c
+++ b/drivers/pci/host/pci-dra7xx.c
@@ -71,6 +71,7 @@ struct dra7xx_pcie {
 	int			phy_count;	/* DT phy-names count */
 	struct phy		**phy;
 	bool			is_gen1;
+	struct irq_domain	*irq_domain;
 };
 
 #define to_dra7xx_pcie(x)	container_of((x), struct dra7xx_pcie, pp)
@@ -142,13 +143,9 @@ static void dra7xx_pcie_enable_interrupts(struct dra7xx_pcie *dra7xx_pcie)
 	dra7xx_pcie_writel(dra7xx_pcie, PCIECTRL_DRA7XX_CONF_IRQSTATUS_MSI,
 			   ~LEG_EP_INTERRUPTS & ~MSI);
 
-	if (IS_ENABLED(CONFIG_PCI_MSI))
-		dra7xx_pcie_writel(dra7xx_pcie,
-				   PCIECTRL_DRA7XX_CONF_IRQENABLE_SET_MSI, MSI);
-	else
-		dra7xx_pcie_writel(dra7xx_pcie,
-				   PCIECTRL_DRA7XX_CONF_IRQENABLE_SET_MSI,
-				   LEG_EP_INTERRUPTS);
+	dra7xx_pcie_writel(dra7xx_pcie,
+			   PCIECTRL_DRA7XX_CONF_IRQENABLE_SET_MSI,
+			   MSI | LEG_EP_INTERRUPTS);
 }
 
 static void dra7xx_pcie_host_init(struct pcie_port *pp)
@@ -163,8 +160,7 @@ static void dra7xx_pcie_host_init(struct pcie_port *pp)
 	dw_pcie_setup_rc(pp);
 
 	dra7xx_pcie_establish_link(dra7xx_pcie);
-	if (IS_ENABLED(CONFIG_PCI_MSI))
-		dw_pcie_msi_init(pp);
+	dw_pcie_msi_init(pp);
 	dra7xx_pcie_enable_interrupts(dra7xx_pcie);
 }
 
@@ -189,6 +185,7 @@ static int dra7xx_pcie_intx_map(struct irq_domain *domain, unsigned int irq,
 static int dra7xx_pcie_init_irq_domain(struct pcie_port *pp)
 {
 	struct device *dev = pp->dev;
+	struct dra7xx_pcie *dra7xx_pcie = to_dra7xx_pcie(pp);
 	struct device_node *node = dev->of_node;
 	struct device_node *pcie_intc_node =  of_get_next_child(node, NULL);
 
@@ -197,9 +194,9 @@ static int dra7xx_pcie_init_irq_domain(struct pcie_port *pp)
 		return -ENODEV;
 	}
 
-	pp->irq_domain = irq_domain_add_linear(pcie_intc_node, 4,
-					       &intx_domain_ops, pp);
-	if (!pp->irq_domain) {
+	dra7xx_pcie->irq_domain = irq_domain_add_linear(pcie_intc_node, 4,
+							&intx_domain_ops, pp);
+	if (!dra7xx_pcie->irq_domain) {
 		dev_err(dev, "Failed to get a INTx IRQ domain\n");
 		return -ENODEV;
 	}
@@ -224,7 +221,8 @@ static irqreturn_t dra7xx_pcie_msi_irq_handler(int irq, void *arg)
 	case INTB:
 	case INTC:
 	case INTD:
-		generic_handle_irq(irq_find_mapping(pp->irq_domain, ffs(reg)));
+		generic_handle_irq(irq_find_mapping(dra7xx_pcie->irq_domain,
+				   ffs(reg)));
 		break;
 	}
 
@@ -314,11 +312,9 @@ static int __init dra7xx_add_pcie_port(struct dra7xx_pcie *dra7xx_pcie,
 		return ret;
 	}
 
-	if (!IS_ENABLED(CONFIG_PCI_MSI)) {
-		ret = dra7xx_pcie_init_irq_domain(pp);
-		if (ret < 0)
-			return ret;
-	}
+	ret = dra7xx_pcie_init_irq_domain(pp);
+	if (ret < 0)
+		return ret;
 
 	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "rc_dbics");
 	pp->dbi_base = devm_ioremap(dev, res->start, resource_size(res));
-- 
1.7.9.5