This patch set adds support for the PCIe controllers found on Marvell
Dove SoCs. It depends on mvebu-pci patches sent by Thomas Petazzoni.
The ARM Dove related patches have already been taken by Jason Cooper
and have been removed from v2 of this patch set. Changelog is added
to the individual patch emails.
Patches 1 and 2 fix some minor issues with pci-mvebu by moving
clk_prepare_enable before accessing any controller registers and
counting sucessfully registered ports only.
Patch 3 converts pci-mvebu from subsys_initcall registration to
normal platform driver registration to allow it to fail with
EPROBE_DEFER later.
Patch 4 adds DT parsing for reset (PERST#) GPIO pins and delay to
wait for PCIe devices after reset de-assertion.
Patch 5 finally adds a compatible to pci-mvebu for Dove SoCs.
[Patch 6-9 have already been taken by Jason Cooper]
Sebastian Hesselbarth (5):
PCI: mvebu: move clock enable before register access
PCI: mvebu: increment nports only for registered ports
PCI: mvebu: remove subsys_initcall
PCI: mvebu: add support for reset on GPIO
PCI: mvebu: add support for Marvell Dove SoCs
.../devicetree/bindings/pci/mvebu-pci.txt | 7 ++
drivers/pci/host/Kconfig | 2 +-
drivers/pci/host/pci-mvebu.c | 99 +++++++++++++-------
3 files changed, 71 insertions(+), 37 deletions(-)
---
Cc: Russell King <[email protected]>
Cc: Jason Cooper <[email protected]>
Cc: Andrew Lunn <[email protected]>
Cc: Bjorn Helgaas <[email protected]>
Cc: Thomas Petazzoni <[email protected]>
Cc: Thierry Reding <[email protected]>
Cc: Stephen Warren <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
--
1.7.10.4
This removes the subsys_initcall from the driver and converts it to
a normal platform_driver. Also, drvdata is set and a remove functions
is added to disable the clock and free resources. As pci driver removal
currently is not supported, set .suppress_bind_attrs to permit unbinding.
Signed-off-by: Sebastian Hesselbarth <[email protected]>
---
Changelog:
v1->v2:
- do not add .remove and set .suppress_bind_attrs to permit currently
unsupported driver unloading/unbinding (Reported by Thierry Reding and
Thomas Petazzoni)
- Note: I left the platform_set_drvdata(), as we will require that later
for device removal.
Cc: Russell King <[email protected]>
Cc: Jason Cooper <[email protected]>
Cc: Andrew Lunn <[email protected]>
Cc: Bjorn Helgaas <[email protected]>
Cc: Thomas Petazzoni <[email protected]>
Cc: Thierry Reding <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
drivers/pci/host/pci-mvebu.c | 33 ++++++++++++++-------------------
1 file changed, 14 insertions(+), 19 deletions(-)
diff --git a/drivers/pci/host/pci-mvebu.c b/drivers/pci/host/pci-mvebu.c
index d2e9ae7..bd7092a 100644
--- a/drivers/pci/host/pci-mvebu.c
+++ b/drivers/pci/host/pci-mvebu.c
@@ -165,7 +165,7 @@ static void mvebu_pcie_set_local_dev_nr(struct mvebu_pcie_port *port, int nr)
* BAR[0,2] -> disabled, BAR[1] -> covers all DRAM banks
* WIN[0-3] -> DRAM bank[0-3]
*/
-static void __init mvebu_pcie_setup_wins(struct mvebu_pcie_port *port)
+static void mvebu_pcie_setup_wins(struct mvebu_pcie_port *port)
{
const struct mbus_dram_target_info *dram;
u32 size;
@@ -217,7 +217,7 @@ static void __init mvebu_pcie_setup_wins(struct mvebu_pcie_port *port)
port->base + PCIE_BAR_CTRL_OFF(1));
}
-static void __init mvebu_pcie_setup_hw(struct mvebu_pcie_port *port)
+static void mvebu_pcie_setup_hw(struct mvebu_pcie_port *port)
{
u16 cmd;
u32 mask;
@@ -628,7 +628,7 @@ static struct pci_ops mvebu_pcie_ops = {
.write = mvebu_pcie_wr_conf,
};
-static int __init mvebu_pcie_setup(int nr, struct pci_sys_data *sys)
+static int mvebu_pcie_setup(int nr, struct pci_sys_data *sys)
{
struct mvebu_pcie *pcie = sys_to_pcie(sys);
int i;
@@ -647,7 +647,7 @@ static int __init mvebu_pcie_setup(int nr, struct pci_sys_data *sys)
return 1;
}
-static int __init mvebu_pcie_map_irq(const struct pci_dev *dev, u8 slot, u8 pin)
+static int mvebu_pcie_map_irq(const struct pci_dev *dev, u8 slot, u8 pin)
{
struct of_irq oirq;
int ret;
@@ -704,7 +704,7 @@ resource_size_t mvebu_pcie_align_resource(struct pci_dev *dev,
return start;
}
-static void __init mvebu_pcie_enable(struct mvebu_pcie *pcie)
+static void mvebu_pcie_enable(struct mvebu_pcie *pcie)
{
struct hw_pci hw;
@@ -727,10 +727,8 @@ static void __init mvebu_pcie_enable(struct mvebu_pcie *pcie)
* <...> property for one that matches the given port/lane. Once
* found, maps it.
*/
-static void __iomem * __init
-mvebu_pcie_map_registers(struct platform_device *pdev,
- struct device_node *np,
- struct mvebu_pcie_port *port)
+static void __iomem *mvebu_pcie_map_registers(struct platform_device *pdev,
+ struct device_node *np, struct mvebu_pcie_port *port)
{
struct resource regs;
int ret = 0;
@@ -786,7 +784,7 @@ static int mvebu_get_tgt_attr(struct device_node *np, int devfn,
return -ENOENT;
}
-static void __init mvebu_pcie_msi_enable(struct mvebu_pcie *pcie)
+static void mvebu_pcie_msi_enable(struct mvebu_pcie *pcie)
{
struct device_node *msi_node;
@@ -801,7 +799,7 @@ static void __init mvebu_pcie_msi_enable(struct mvebu_pcie *pcie)
pcie->msi->dev = &pcie->pdev->dev;
}
-static int __init mvebu_pcie_probe(struct platform_device *pdev)
+static int mvebu_pcie_probe(struct platform_device *pdev)
{
struct mvebu_pcie *pcie;
struct device_node *np = pdev->dev.of_node;
@@ -814,6 +812,7 @@ static int __init mvebu_pcie_probe(struct platform_device *pdev)
return -ENOMEM;
pcie->pdev = pdev;
+ platform_set_drvdata(pdev, pcie);
/* Get the PCIe memory and I/O aperture */
mvebu_mbus_get_pcie_mem_aperture(&pcie->mem);
@@ -956,16 +955,12 @@ static struct platform_driver mvebu_pcie_driver = {
.name = "mvebu-pcie",
.of_match_table =
of_match_ptr(mvebu_pcie_of_match_table),
+ /* driver unloading/unbinding currently not supported */
+ .suppress_bind_attrs = true,
},
+ .probe = mvebu_pcie_probe,
};
-
-static int __init mvebu_pcie_init(void)
-{
- return platform_driver_probe(&mvebu_pcie_driver,
- mvebu_pcie_probe);
-}
-
-subsys_initcall(mvebu_pcie_init);
+module_platform_driver(mvebu_pcie_driver);
MODULE_AUTHOR("Thomas Petazzoni <[email protected]>");
MODULE_DESCRIPTION("Marvell EBU PCIe driver");
--
1.7.10.4
The number of ports is probed by counting the number of available child nodes.
Later on, the registration of a port can fail and cause a mismatch between
the ->nports counter and registered ports. This patch modifies the counting
strategy, to make ->nports represent the number of registered ports instead
of the number of available childs.
Signed-off-by: Sebastian Hesselbarth <[email protected]>
---
Changelog:
v1->v2:
- also use 'i' as iterator during port parsing and assign at the end of
probing (Reported by Thomas Pettazoni)
Cc: Russell King <[email protected]>
Cc: Jason Cooper <[email protected]>
Cc: Andrew Lunn <[email protected]>
Cc: Bjorn Helgaas <[email protected]>
Cc: Thomas Petazzoni <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
drivers/pci/host/pci-mvebu.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/pci/host/pci-mvebu.c b/drivers/pci/host/pci-mvebu.c
index 1533fda..d2e9ae7 100644
--- a/drivers/pci/host/pci-mvebu.c
+++ b/drivers/pci/host/pci-mvebu.c
@@ -842,13 +842,14 @@ static int __init mvebu_pcie_probe(struct platform_device *pdev)
return ret;
}
+ i = 0;
for_each_child_of_node(pdev->dev.of_node, child) {
if (!of_device_is_available(child))
continue;
- pcie->nports++;
+ i++;
}
- pcie->ports = devm_kzalloc(&pdev->dev, pcie->nports *
+ pcie->ports = devm_kzalloc(&pdev->dev, i *
sizeof(struct mvebu_pcie_port),
GFP_KERNEL);
if (!pcie->ports)
@@ -934,8 +935,8 @@ static int __init mvebu_pcie_probe(struct platform_device *pdev)
i++;
}
+ pcie->nports = i;
mvebu_pcie_msi_enable(pcie);
-
mvebu_pcie_enable(pcie);
return 0;
--
1.7.10.4
This patch adds a compatible for the PCIe controller found on Marvell
Dove SoCs. Binding documentation and Kconfig entry are also updated.
Signed-off-by: Sebastian Hesselbarth <[email protected]>
---
Changelog:
v1->v2:
- nothing changed
Cc: Russell King <[email protected]>
Cc: Jason Cooper <[email protected]>
Cc: Andrew Lunn <[email protected]>
Cc: Bjorn Helgaas <[email protected]>
Cc: Thomas Petazzoni <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
Documentation/devicetree/bindings/pci/mvebu-pci.txt | 1 +
drivers/pci/host/Kconfig | 2 +-
drivers/pci/host/pci-mvebu.c | 1 +
3 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/pci/mvebu-pci.txt b/Documentation/devicetree/bindings/pci/mvebu-pci.txt
index 8bb3245..08c716b 100644
--- a/Documentation/devicetree/bindings/pci/mvebu-pci.txt
+++ b/Documentation/devicetree/bindings/pci/mvebu-pci.txt
@@ -5,6 +5,7 @@ Mandatory properties:
- compatible: one of the following values:
marvell,armada-370-pcie
marvell,armada-xp-pcie
+ marvell,dove-pcie
marvell,kirkwood-pcie
- #address-cells, set to <3>
- #size-cells, set to <2>
diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
index 1184ff6..492eec9 100644
--- a/drivers/pci/host/Kconfig
+++ b/drivers/pci/host/Kconfig
@@ -3,7 +3,7 @@ menu "PCI host controller drivers"
config PCI_MVEBU
bool "Marvell EBU PCIe controller"
- depends on ARCH_MVEBU || ARCH_KIRKWOOD
+ depends on ARCH_MVEBU || ARCH_DOVE || ARCH_KIRKWOOD
config PCIE_DW
bool
diff --git a/drivers/pci/host/pci-mvebu.c b/drivers/pci/host/pci-mvebu.c
index a9ad4b3..88c6790 100644
--- a/drivers/pci/host/pci-mvebu.c
+++ b/drivers/pci/host/pci-mvebu.c
@@ -975,6 +975,7 @@ static int mvebu_pcie_probe(struct platform_device *pdev)
static const struct of_device_id mvebu_pcie_of_match_table[] = {
{ .compatible = "marvell,armada-xp-pcie", },
{ .compatible = "marvell,armada-370-pcie", },
+ { .compatible = "marvell,dove-pcie", },
{ .compatible = "marvell,kirkwood-pcie", },
{},
};
--
1.7.10.4
The clock passed to PCI controller found on MVEBU SoCs may come from a
clock gate. This requires the clock to be enabled before any registers
are accessed. Therefore, move the clock enable before register iomap to
ensure it is enabled.
Signed-off-by: Sebastian Hesselbarth <[email protected]>
---
Changelog:
v1->v2:
- remove iounmap, add clk_disable_unprepare on failure
(Reported by Thomas Pettazoni)
Cc: Russell King <[email protected]>
Cc: Jason Cooper <[email protected]>
Cc: Andrew Lunn <[email protected]>
Cc: Bjorn Helgaas <[email protected]>
Cc: Thomas Petazzoni <[email protected]>
Cc: Thierry Reding <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
drivers/pci/host/pci-mvebu.c | 25 ++++++++++++-------------
1 file changed, 12 insertions(+), 13 deletions(-)
diff --git a/drivers/pci/host/pci-mvebu.c b/drivers/pci/host/pci-mvebu.c
index 6aa0daf..1533fda 100644
--- a/drivers/pci/host/pci-mvebu.c
+++ b/drivers/pci/host/pci-mvebu.c
@@ -897,10 +897,22 @@ static int __init mvebu_pcie_probe(struct platform_device *pdev)
continue;
}
+ port->clk = of_clk_get_by_name(child, NULL);
+ if (IS_ERR(port->clk)) {
+ dev_err(&pdev->dev, "PCIe%d.%d: cannot get clock\n",
+ port->port, port->lane);
+ continue;
+ }
+
+ ret = clk_prepare_enable(port->clk);
+ if (ret)
+ continue;
+
port->base = mvebu_pcie_map_registers(pdev, child, port);
if (!port->base) {
dev_err(&pdev->dev, "PCIe%d.%d: cannot map registers\n",
port->port, port->lane);
+ clk_disable_unprepare(port->clk);
continue;
}
@@ -916,22 +928,9 @@ static int __init mvebu_pcie_probe(struct platform_device *pdev)
port->port, port->lane);
}
- port->clk = of_clk_get_by_name(child, NULL);
- if (IS_ERR(port->clk)) {
- dev_err(&pdev->dev, "PCIe%d.%d: cannot get clock\n",
- port->port, port->lane);
- iounmap(port->base);
- port->haslink = 0;
- continue;
- }
-
port->dn = child;
-
- clk_prepare_enable(port->clk);
spin_lock_init(&port->conf_lock);
-
mvebu_sw_pci_bridge_init(port);
-
i++;
}
--
1.7.10.4
This patch adds a check for DT passed reset-gpios property and deasserts/
asserts reset pin on probe/remove with configurable delay. Corresponding
binding documentation is also updated.
Signed-off-by: Sebastian Hesselbarth <[email protected]>
---
Changelog:
v1->v2:
- use reset API compatible bindings (Reported by Kumar Gala and Thierry
Reding)
- add gpio properties to DT examples (Reported by Kumar Gala)
- only fail (return) on EPROBE_DEFER and skip port parsing otherwise
- use us delay instead of ms delay
Cc: Russell King <[email protected]>
Cc: Jason Cooper <[email protected]>
Cc: Andrew Lunn <[email protected]>
Cc: Bjorn Helgaas <[email protected]>
Cc: Thomas Petazzoni <[email protected]>
Cc: Thierry Reding <[email protected]>
Cc: Stephen Warren <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
.../devicetree/bindings/pci/mvebu-pci.txt | 6 ++++
drivers/pci/host/pci-mvebu.c | 33 +++++++++++++++++++-
2 files changed, 38 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/pci/mvebu-pci.txt b/Documentation/devicetree/bindings/pci/mvebu-pci.txt
index 638673a..8bb3245 100644
--- a/Documentation/devicetree/bindings/pci/mvebu-pci.txt
+++ b/Documentation/devicetree/bindings/pci/mvebu-pci.txt
@@ -76,6 +76,8 @@ and the following optional properties:
- marvell,pcie-lane: the physical PCIe lane number, for ports having
multiple lanes. If this property is not found, we assume that the
value is 0.
+- reset-gpios: optional gpio to PERST#
+- reset-delay-us: delay in us to wait after reset de-assertion
Example:
@@ -138,6 +140,10 @@ pcie-controller {
interrupt-map = <0 0 0 0 &mpic 58>;
marvell,pcie-port = <0>;
marvell,pcie-lane = <0>;
+ /* low-active PERST# reset on GPIO 25 */
+ reset-gpios = <&gpio0 25 1>;
+ /* wait 20ms for device settle after reset deassertion */
+ reset-delay-us = <20000>;
clocks = <&gateclk 5>;
status = "disabled";
};
diff --git a/drivers/pci/host/pci-mvebu.c b/drivers/pci/host/pci-mvebu.c
index bd7092a..a9ad4b3 100644
--- a/drivers/pci/host/pci-mvebu.c
+++ b/drivers/pci/host/pci-mvebu.c
@@ -9,14 +9,17 @@
#include <linux/kernel.h>
#include <linux/pci.h>
#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/gpio.h>
#include <linux/module.h>
#include <linux/mbus.h>
#include <linux/msi.h>
#include <linux/slab.h>
#include <linux/platform_device.h>
#include <linux/of_address.h>
-#include <linux/of_pci.h>
#include <linux/of_irq.h>
+#include <linux/of_gpio.h>
+#include <linux/of_pci.h>
#include <linux/of_platform.h>
/*
@@ -126,6 +129,9 @@ struct mvebu_pcie_port {
unsigned int io_target;
unsigned int io_attr;
struct clk *clk;
+ int reset_gpio;
+ int reset_active_low;
+ char *reset_name;
struct mvebu_sw_pci_bridge bridge;
struct device_node *dn;
struct mvebu_pcie *pcie;
@@ -857,6 +863,7 @@ static int mvebu_pcie_probe(struct platform_device *pdev)
i = 0;
for_each_child_of_node(pdev->dev.of_node, child) {
struct mvebu_pcie_port *port = &pcie->ports[i];
+ enum of_gpio_flags flags;
if (!of_device_is_available(child))
continue;
@@ -897,6 +904,30 @@ static int mvebu_pcie_probe(struct platform_device *pdev)
continue;
}
+ port->reset_gpio = of_get_named_gpio_flags(child,
+ "reset-gpios", 0, &flags);
+ if (gpio_is_valid(port->reset_gpio)) {
+ u32 reset_udelay = 20000;
+
+ port->reset_active_low = flags & OF_GPIO_ACTIVE_LOW;
+ port->reset_name = kasprintf(GFP_KERNEL,
+ "pcie%d.%d-reset", port->port, port->lane);
+ of_property_read_u32(child, "reset-delay-us",
+ &reset_udelay);
+
+ ret = devm_gpio_request_one(&pdev->dev,
+ port->reset_gpio, GPIOF_DIR_OUT, port->reset_name);
+ if (ret) {
+ if (ret == -EPROBE_DEFER)
+ return ret;
+ continue;
+ }
+
+ gpio_set_value(port->reset_gpio,
+ (port->reset_active_low) ? 1 : 0);
+ udelay(reset_udelay);
+ }
+
port->clk = of_clk_get_by_name(child, NULL);
if (IS_ERR(port->clk)) {
dev_err(&pdev->dev, "PCIe%d.%d: cannot get clock\n",
--
1.7.10.4
Bjorn,
On Tue, Aug 13, 2013 at 02:25:19PM +0200, Sebastian Hesselbarth wrote:
> This patch set adds support for the PCIe controllers found on Marvell
> Dove SoCs. It depends on mvebu-pci patches sent by Thomas Petazzoni.
Well, when it rains, it pours. If this series looks good to you, I'll
go ahead and add it on top of Thomas' patches.
> The ARM Dove related patches have already been taken by Jason Cooper
> and have been removed from v2 of this patch set. Changelog is added
> to the individual patch emails.
>
> Patches 1 and 2 fix some minor issues with pci-mvebu by moving
> clk_prepare_enable before accessing any controller registers and
> counting sucessfully registered ports only.
>
> Patch 3 converts pci-mvebu from subsys_initcall registration to
> normal platform driver registration to allow it to fail with
> EPROBE_DEFER later.
>
> Patch 4 adds DT parsing for reset (PERST#) GPIO pins and delay to
> wait for PCIe devices after reset de-assertion.
>
> Patch 5 finally adds a compatible to pci-mvebu for Dove SoCs.
>
> [Patch 6-9 have already been taken by Jason Cooper]
These were the dts files.
> Sebastian Hesselbarth (5):
> PCI: mvebu: move clock enable before register access
> PCI: mvebu: increment nports only for registered ports
> PCI: mvebu: remove subsys_initcall
> PCI: mvebu: add support for reset on GPIO
> PCI: mvebu: add support for Marvell Dove SoCs
>
> .../devicetree/bindings/pci/mvebu-pci.txt | 7 ++
> drivers/pci/host/Kconfig | 2 +-
> drivers/pci/host/pci-mvebu.c | 99 +++++++++++++-------
> 3 files changed, 71 insertions(+), 37 deletions(-)
Should be pretty isolated.
thx,
Jason.
On Tue, Aug 13, 2013 at 10:01:30AM -0400, Jason Cooper wrote:
> Bjorn,
>
> On Tue, Aug 13, 2013 at 02:25:19PM +0200, Sebastian Hesselbarth wrote:
> > This patch set adds support for the PCIe controllers found on Marvell
> > Dove SoCs. It depends on mvebu-pci patches sent by Thomas Petazzoni.
>
> Well, when it rains, it pours. If this series looks good to you, I'll
> go ahead and add it on top of Thomas' patches.
And I can confirm, this series definitely depends on mvebu/boards,
mvebu/mbus_dt, and mvebu/msi_mvebu (which, in turn, depends on
mvebu/msi_irq and mvebu/msi_pci).
thx,
Jason.
On Tue, Aug 13, 2013 at 8:01 AM, Jason Cooper <[email protected]> wrote:
> Bjorn,
>
> On Tue, Aug 13, 2013 at 02:25:19PM +0200, Sebastian Hesselbarth wrote:
>> This patch set adds support for the PCIe controllers found on Marvell
>> Dove SoCs. It depends on mvebu-pci patches sent by Thomas Petazzoni.
>
> Well, when it rains, it pours. If this series looks good to you, I'll
> go ahead and add it on top of Thomas' patches.
Yep, that's fine with me. I consider the stuff in drivers/pci/host/
as basically arch code that is collected as a convenience for code
sharing. I can't test it, and I can only evaluate it as far as
looking for the most obvious things related to use of core PCI
interfaces.
Bjorn
On Tue, Aug 13, 2013 at 02:25:19PM +0200, Sebastian Hesselbarth wrote:
> This patch set adds support for the PCIe controllers found on Marvell
> Dove SoCs. It depends on mvebu-pci patches sent by Thomas Petazzoni.
> The ARM Dove related patches have already been taken by Jason Cooper
> and have been removed from v2 of this patch set. Changelog is added
> to the individual patch emails.
>
> Patches 1 and 2 fix some minor issues with pci-mvebu by moving
> clk_prepare_enable before accessing any controller registers and
> counting sucessfully registered ports only.
>
> Patch 3 converts pci-mvebu from subsys_initcall registration to
> normal platform driver registration to allow it to fail with
> EPROBE_DEFER later.
>
> Patch 4 adds DT parsing for reset (PERST#) GPIO pins and delay to
> wait for PCIe devices after reset de-assertion.
>
> Patch 5 finally adds a compatible to pci-mvebu for Dove SoCs.
>
> [Patch 6-9 have already been taken by Jason Cooper]
>
> Sebastian Hesselbarth (5):
> PCI: mvebu: move clock enable before register access
> PCI: mvebu: increment nports only for registered ports
> PCI: mvebu: remove subsys_initcall
> PCI: mvebu: add support for reset on GPIO
> PCI: mvebu: add support for Marvell Dove SoCs
>
> .../devicetree/bindings/pci/mvebu-pci.txt | 7 ++
> drivers/pci/host/Kconfig | 2 +-
> drivers/pci/host/pci-mvebu.c | 99 +++++++++++++-------
> 3 files changed, 71 insertions(+), 37 deletions(-)
Ok, whole series applied to mvebu/dove_pcie-mbus-dt, which depends on
mvebu/boards, mvebu/mbus_dt, and mvebu/msi_mvebu.
sigh... if this branch makes it in, I'll be amazed*. And someone will
owe me a stiff drink. Well, I'll take a drink in either case. :-P
thx,
Jason.
* because of the intricate dependencies, the changes are fine.
Dear Sebastian Hesselbarth,
On Tue, 13 Aug 2013 14:25:23 +0200, Sebastian Hesselbarth wrote:
> + port->reset_gpio = of_get_named_gpio_flags(child,
> + "reset-gpios", 0, &flags);
> + if (gpio_is_valid(port->reset_gpio)) {
> + u32 reset_udelay = 20000;
> +
> + port->reset_active_low = flags & OF_GPIO_ACTIVE_LOW;
> + port->reset_name = kasprintf(GFP_KERNEL,
> + "pcie%d.%d-reset", port->port, port->lane);
> + of_property_read_u32(child, "reset-delay-us",
> + &reset_udelay);
> +
> + ret = devm_gpio_request_one(&pdev->dev,
> + port->reset_gpio, GPIOF_DIR_OUT, port->reset_name);
> + if (ret) {
> + if (ret == -EPROBE_DEFER)
> + return ret;
> + continue;
> + }
> +
> + gpio_set_value(port->reset_gpio,
> + (port->reset_active_low) ? 1 : 0);
> + udelay(reset_udelay);
> + }
Sorry for raising this only now, but I think I would have preferred to
see this reset-gpios handling be moved into a separate sub-function.
The loop initializing each PCIe interface is already quite large, and I
believe moving this reset-gpios thing to a sub-function would have made
sense.
But well, the patches have been applied, and we can always adjust this
with a followup patch.
Jason, have you re-created your for-next branch with all those patches?
I'd like to give them a test if possible.
Thanks!
Thomas
--
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
On Tue, Aug 13, 2013 at 10:29:43PM +0200, Thomas Petazzoni wrote:
> Dear Sebastian Hesselbarth,
>
> On Tue, 13 Aug 2013 14:25:23 +0200, Sebastian Hesselbarth wrote:
>
> > + port->reset_gpio = of_get_named_gpio_flags(child,
> > + "reset-gpios", 0, &flags);
> > + if (gpio_is_valid(port->reset_gpio)) {
> > + u32 reset_udelay = 20000;
> > +
> > + port->reset_active_low = flags & OF_GPIO_ACTIVE_LOW;
> > + port->reset_name = kasprintf(GFP_KERNEL,
> > + "pcie%d.%d-reset", port->port, port->lane);
> > + of_property_read_u32(child, "reset-delay-us",
> > + &reset_udelay);
> > +
> > + ret = devm_gpio_request_one(&pdev->dev,
> > + port->reset_gpio, GPIOF_DIR_OUT, port->reset_name);
> > + if (ret) {
> > + if (ret == -EPROBE_DEFER)
> > + return ret;
> > + continue;
> > + }
> > +
> > + gpio_set_value(port->reset_gpio,
> > + (port->reset_active_low) ? 1 : 0);
> > + udelay(reset_udelay);
> > + }
>
> Sorry for raising this only now, but I think I would have preferred to
> see this reset-gpios handling be moved into a separate sub-function.
> The loop initializing each PCIe interface is already quite large, and I
> believe moving this reset-gpios thing to a sub-function would have made
> sense.
>
> But well, the patches have been applied, and we can always adjust this
> with a followup patch.
The branch this is in will be the last PR, so if the patch is reworked
by tomorrow, I'll just replace it.
> Jason, have you re-created your for-next branch with all those patches?
> I'd like to give them a test if possible.
Yep. Give it a go.
thx,
Jason.
On Tue, Aug 13, 2013 at 02:25:23PM +0200, Sebastian Hesselbarth wrote:
[...]
> diff --git a/drivers/pci/host/pci-mvebu.c b/drivers/pci/host/pci-mvebu.c
[...]
> @@ -897,6 +904,30 @@ static int mvebu_pcie_probe(struct platform_device *pdev)
[...]
> + u32 reset_udelay = 20000;
[...]
> + udelay(reset_udelay);
udelay(20000) is probably not a good idea. You should probably use
something like usleep_range() or msleep() here. Also see:
Documentation/timers/timers-howto.txt
Thierry
On 08/14/2013 11:07 AM, Thierry Reding wrote:
> On Tue, Aug 13, 2013 at 02:25:23PM +0200, Sebastian Hesselbarth wrote:
> [...]
>> diff --git a/drivers/pci/host/pci-mvebu.c b/drivers/pci/host/pci-mvebu.c
> [...]
>> @@ -897,6 +904,30 @@ static int mvebu_pcie_probe(struct platform_device *pdev)
> [...]
>> + u32 reset_udelay = 20000;
> [...]
>> + udelay(reset_udelay);
>
> udelay(20000) is probably not a good idea. You should probably use
> something like usleep_range() or msleep() here. Also see:
>
> Documentation/timers/timers-howto.txt
Thierry,
thanks for mentioning this. I clearly got distracted from ms to us
switch due to the different reset API convention. As the delay will
be most likely 10ms+, we should change the above to
msleep(reset_udelay/1000) instead.
Sebastian
On Wed, Aug 14, 2013 at 11:25:02AM +0200, Sebastian Hesselbarth wrote:
> On 08/14/2013 11:07 AM, Thierry Reding wrote:
> >On Tue, Aug 13, 2013 at 02:25:23PM +0200, Sebastian Hesselbarth wrote:
> >[...]
> >>diff --git a/drivers/pci/host/pci-mvebu.c b/drivers/pci/host/pci-mvebu.c
> >[...]
> >>@@ -897,6 +904,30 @@ static int mvebu_pcie_probe(struct platform_device *pdev)
> >[...]
> >>+ u32 reset_udelay = 20000;
> >[...]
> >>+ udelay(reset_udelay);
> >
> >udelay(20000) is probably not a good idea. You should probably use
> >something like usleep_range() or msleep() here. Also see:
> >
> > Documentation/timers/timers-howto.txt
>
> Thierry,
>
> thanks for mentioning this. I clearly got distracted from ms to us
> switch due to the different reset API convention. As the delay will
> be most likely 10ms+, we should change the above to
> msleep(reset_udelay/1000) instead.
I'll just go ahead and fix this up locally. No need to resend.
thx,
Jason.
Dear Sebastian Hesselbarth,
On Tue, 13 Aug 2013 14:25:19 +0200, Sebastian Hesselbarth wrote:
> This patch set adds support for the PCIe controllers found on Marvell
> Dove SoCs. It depends on mvebu-pci patches sent by Thomas Petazzoni.
> The ARM Dove related patches have already been taken by Jason Cooper
> and have been removed from v2 of this patch set. Changelog is added
> to the individual patch emails.
>
> Patches 1 and 2 fix some minor issues with pci-mvebu by moving
> clk_prepare_enable before accessing any controller registers and
> counting sucessfully registered ports only.
>
> Patch 3 converts pci-mvebu from subsys_initcall registration to
> normal platform driver registration to allow it to fail with
> EPROBE_DEFER later.
>
> Patch 4 adds DT parsing for reset (PERST#) GPIO pins and delay to
> wait for PCIe devices after reset de-assertion.
>
> Patch 5 finally adds a compatible to pci-mvebu for Dove SoCs.
>
> [Patch 6-9 have already been taken by Jason Cooper]
>
> Sebastian Hesselbarth (5):
> PCI: mvebu: move clock enable before register access
> PCI: mvebu: increment nports only for registered ports
> PCI: mvebu: remove subsys_initcall
> PCI: mvebu: add support for reset on GPIO
> PCI: mvebu: add support for Marvell Dove SoCs
I've just seen that Jason Cooper has already sent the PR for this code,
but anyway, I just tested mvebu/for-next on my Armada XP GP board, and
the PCIe + MSI continues to work nicely, even with your code
integrated. So:
Tested-by: Thomas Petazzoni <[email protected]>
Thanks,
Thomas
--
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com