2016-10-11 13:00:12

by Kishon Vijay Abraham I

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

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


2016-10-11 13:00:11

by Kishon Vijay Abraham I

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

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 <[email protected]>
---
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

2016-10-11 13:03:55

by Kishon Vijay Abraham I

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

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

Signed-off-by: Kishon Vijay Abraham I <[email protected]>
Signed-off-by: Sekhar Nori <[email protected]>
---
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

2016-10-11 13:03:54

by Kishon Vijay Abraham I

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

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 <[email protected]>
---
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

2016-10-11 13:03:54

by Kishon Vijay Abraham I

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

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 <[email protected]>
Signed-off-by: Sekhar Nori <[email protected]>
---
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

2016-10-11 13:51:00

by Bjorn Helgaas

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

[+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 <[email protected]>
> ---
> 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2016-10-11 16:25:31

by Sekhar Nori

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

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 <[email protected]>
> Signed-off-by: Sekhar Nori <[email protected]>
> ---
> 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

2016-10-12 04:14:33

by Shawn Lin

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

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 <[email protected]>
> Signed-off-by: Sekhar Nori <[email protected]>
> ---
> 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

2016-11-11 21:15:35

by Bjorn Helgaas

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

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 <[email protected]>
> Signed-off-by: Sekhar Nori <[email protected]>
> ---
> 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
>

2016-11-13 20:17:29

by Kishon Vijay Abraham I

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

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 <[email protected]>
>> Signed-off-by: Sekhar Nori <[email protected]>
>> ---
>> 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

2016-11-14 21:24:07

by Bjorn Helgaas

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

[+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 <[email protected]>
> >> Signed-off-by: Sekhar Nori <[email protected]>
> >> ---
> >> 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

2016-11-16 16:44:25

by Kishon Vijay Abraham I

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

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 <[email protected]>
>>>> Signed-off-by: Sekhar Nori <[email protected]>
>>>> ---
>>>> 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