| Message ID | 1376333215-12885-5-git-send-email-sebastian.hesselbarth@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
On Aug 12, 2013, at 1:46 PM, Sebastian Hesselbarth wrote: > 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 <sebastian.hesselbarth@gmail.com> > --- > Cc: Russell King <linux@arm.linux.org.uk> > Cc: Jason Cooper <jason@lakedaemon.net> > Cc: Andrew Lunn <andrew@lunn.ch> > Cc: Bjorn Helgaas <bhelgaas@google.com> > Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> > Cc: devicetree@vger.kernel.org > Cc: linux-doc@vger.kernel.org > Cc: linux-kernel@vger.kernel.org > Cc: linux-arm-kernel@lists.infradead.org > Cc: linux-pci@vger.kernel.org > --- > .../devicetree/bindings/pci/mvebu-pci.txt | 2 ++ > drivers/pci/host/pci-mvebu.c | 33 +++++++++++++++++++- > 2 files changed, 34 insertions(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/pci/mvebu-pci.txt b/Documentation/devicetree/bindings/pci/mvebu-pci.txt > index 638673a..f2fa261 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-ms: delay in ms to wait after reset de-assertion Both of these should probably be marvell,reset... Does reset-delay-ms vary per board? I can't remember where the discussion got to about configuration info. > > Example: It's usually good to update example to show all optional properties. - k -- Employee of Qualcomm Innovation Center, Inc. Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation -- 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
On Mon, Aug 12, 2013 at 08:46:50PM +0200, Sebastian Hesselbarth wrote: > 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 <sebastian.hesselbarth@gmail.com> > --- > Cc: Russell King <linux@arm.linux.org.uk> > Cc: Jason Cooper <jason@lakedaemon.net> > Cc: Andrew Lunn <andrew@lunn.ch> > Cc: Bjorn Helgaas <bhelgaas@google.com> > Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> > Cc: devicetree@vger.kernel.org > Cc: linux-doc@vger.kernel.org > Cc: linux-kernel@vger.kernel.org > Cc: linux-arm-kernel@lists.infradead.org > Cc: linux-pci@vger.kernel.org > --- > .../devicetree/bindings/pci/mvebu-pci.txt | 2 ++ > drivers/pci/host/pci-mvebu.c | 33 +++++++++++++++++++- > 2 files changed, 34 insertions(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/pci/mvebu-pci.txt b/Documentation/devicetree/bindings/pci/mvebu-pci.txt > index 638673a..f2fa261 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-ms: delay in ms to wait after reset de-assertion I remember some recent discussion about this, and we now have this reset framework, so perhaps it makes more sense to use the reset binding for this? Cc'ing Stephen (as part of the device tree bindings maintainers team) who was involved in that recent reset bindings discussion. Thierry
Dear Thierry Reding, On Tue, 13 Aug 2013 10:09:56 +0200, Thierry Reding wrote: > > +- reset-gpios: optional gpio to PERST# > > +- reset-delay-ms: delay in ms to wait after reset de-assertion > > I remember some recent discussion about this, and we now have this reset > framework, so perhaps it makes more sense to use the reset binding for > this? Cc'ing Stephen (as part of the device tree bindings maintainers > team) who was involved in that recent reset bindings discussion. I also thought about this, but the reset framework seems to be designed for "reset controller" IPs, i.e special IPs that are controlling reset signals. Looking at Documentation/devicetree/bindings/reset/reset.txt, I'm not sure to see how this would apply to GPIO-controlled reset signals. Best regards, Thomas
[Added DT maintainers directly for a question below] On 08/13/13 02:56, Kumar Gala wrote: > On Aug 12, 2013, at 1:46 PM, Sebastian Hesselbarth wrote: > >> 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 <sebastian.hesselbarth@gmail.com> [...] >> diff --git a/Documentation/devicetree/bindings/pci/mvebu-pci.txt b/Documentation/devicetree/bindings/pci/mvebu-pci.txt >> index 638673a..f2fa261 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-ms: delay in ms to wait after reset de-assertion > > Both of these should probably be marvell,reset... Kumar, as Thomas already mentioned, non-prefixed "foo-gpios" are quite common. As reset gpios are likely to be common among SoC pci controller setups, I though I'd stick with the non-prefixed property. But I have no strong opinion about it - but let's see how DT jurisdiction will vote :) > Does reset-delay-ms vary per board? I can't remember where the discussion got to about configuration info. Hmm, for Dove SoC, I only have one board to test - but the driver currently is for 4 different SoCs. I just did some quick evaluation of the reset delay required to bring up the device on the D3Plug. Using 10ms was too short, 20ms worked. I guess there will be other SoC/boards requiring different delays but I am fine with hardcoded 20ms and re-introduce reset-delay property only if it is really required. The driver's default is 20ms anyway. >> >> Example: > > It's usually good to update example to show all optional properties. Ok, will add the properties to one of the example ports. Thanks, Sebastian -- 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
On Tue, Aug 13, 2013 at 10:30:30AM +0200, Thomas Petazzoni wrote: > Dear Thierry Reding, > > On Tue, 13 Aug 2013 10:09:56 +0200, Thierry Reding wrote: > > > > +- reset-gpios: optional gpio to PERST# > > > +- reset-delay-ms: delay in ms to wait after reset de-assertion > > > > I remember some recent discussion about this, and we now have this reset > > framework, so perhaps it makes more sense to use the reset binding for > > this? Cc'ing Stephen (as part of the device tree bindings maintainers > > team) who was involved in that recent reset bindings discussion. > > I also thought about this, but the reset framework seems to be designed > for "reset controller" IPs, i.e special IPs that are controlling reset > signals. Looking at Documentation/devicetree/bindings/reset/reset.txt, > I'm not sure to see how this would apply to GPIO-controlled reset > signals. Search for: [PATCH v10] reset: Add driver for gpio-controlled reset pins Which aims to be a solution for what you need here. Sascha
On Tue, Aug 13, 2013 at 10:30:30AM +0200, Thomas Petazzoni wrote: > Dear Thierry Reding, > > On Tue, 13 Aug 2013 10:09:56 +0200, Thierry Reding wrote: > > > > +- reset-gpios: optional gpio to PERST# > > > +- reset-delay-ms: delay in ms to wait after reset de-assertion > > > > I remember some recent discussion about this, and we now have this reset > > framework, so perhaps it makes more sense to use the reset binding for > > this? Cc'ing Stephen (as part of the device tree bindings maintainers > > team) who was involved in that recent reset bindings discussion. > > I also thought about this, but the reset framework seems to be designed > for "reset controller" IPs, i.e special IPs that are controlling reset > signals. Looking at Documentation/devicetree/bindings/reset/reset.txt, > I'm not sure to see how this would apply to GPIO-controlled reset > signals. See: http://www.mail-archive.com/devicetree-discuss@lists.ozlabs.org/msg36900.html which seems to have carried over to this at some point: http://www.spinics.net/lists/devicetree/msg00521.html Some of the messages in between I can't find in any archive, sorry. Thierry
On 08/13/13 12:03, Thierry Reding wrote: > On Tue, Aug 13, 2013 at 10:30:30AM +0200, Thomas Petazzoni wrote: >> Dear Thierry Reding, >> >> On Tue, 13 Aug 2013 10:09:56 +0200, Thierry Reding wrote: >> >>>> +- reset-gpios: optional gpio to PERST# >>>> +- reset-delay-ms: delay in ms to wait after reset de-assertion >>> >>> I remember some recent discussion about this, and we now have this reset >>> framework, so perhaps it makes more sense to use the reset binding for >>> this? Cc'ing Stephen (as part of the device tree bindings maintainers >>> team) who was involved in that recent reset bindings discussion. >> >> I also thought about this, but the reset framework seems to be designed >> for "reset controller" IPs, i.e special IPs that are controlling reset >> signals. Looking at Documentation/devicetree/bindings/reset/reset.txt, >> I'm not sure to see how this would apply to GPIO-controlled reset >> signals. > > See: > > http://www.mail-archive.com/devicetree-discuss@lists.ozlabs.org/msg36900.html > > which seems to have carried over to this at some point: > > http://www.spinics.net/lists/devicetree/msg00521.html > > Some of the messages in between I can't find in any archive, sorry. Thierry, Sascha, thanks for the input. Flipping through the above discussion, I guess using "reset-gpios" and "reset-delay-us" should be fine? I can also remove the delay property for now, as I cannot find a final conclusion about the configurable delay. In the driver, I will stick to bare gpiolib and wait for gpio-reset driver to become available. Currently, we don't have sophisticated reset handling in pci-mvebu anyway. Sebastian -- 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
Hi Sebastian, Am Dienstag, den 13.08.2013, 12:40 +0200 schrieb Sebastian Hesselbarth: > On 08/13/13 12:03, Thierry Reding wrote: > > On Tue, Aug 13, 2013 at 10:30:30AM +0200, Thomas Petazzoni wrote: > >> Dear Thierry Reding, > >> > >> On Tue, 13 Aug 2013 10:09:56 +0200, Thierry Reding wrote: > >> > >>>> +- reset-gpios: optional gpio to PERST# > >>>> +- reset-delay-ms: delay in ms to wait after reset de-assertion > >>> > >>> I remember some recent discussion about this, and we now have this reset > >>> framework, so perhaps it makes more sense to use the reset binding for > >>> this? Cc'ing Stephen (as part of the device tree bindings maintainers > >>> team) who was involved in that recent reset bindings discussion. > >> > >> I also thought about this, but the reset framework seems to be designed > >> for "reset controller" IPs, i.e special IPs that are controlling reset > >> signals. Looking at Documentation/devicetree/bindings/reset/reset.txt, > >> I'm not sure to see how this would apply to GPIO-controlled reset > >> signals. > > > > See: > > > > http://www.mail-archive.com/devicetree-discuss@lists.ozlabs.org/msg36900.html > > > > which seems to have carried over to this at some point: > > > > http://www.spinics.net/lists/devicetree/msg00521.html > > > > Some of the messages in between I can't find in any archive, sorry. > > Thierry, Sascha, > > thanks for the input. Flipping through the above discussion, I guess > using "reset-gpios" and "reset-delay-us" should be fine? > > I can also remove the delay property for now, as I cannot find a final > conclusion about the configurable delay. Yes, I'm in favor of using 'reset-gpios'. If we can agree on this binding, I'll add support to the reset framework core. > In the driver, I will stick to bare gpiolib and wait for gpio-reset > driver to become available. Currently, we don't have sophisticated > reset handling in pci-mvebu anyway. Sounds good to me. regards Philipp -- 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
diff --git a/Documentation/devicetree/bindings/pci/mvebu-pci.txt b/Documentation/devicetree/bindings/pci/mvebu-pci.txt index 638673a..f2fa261 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-ms: delay in ms to wait after reset de-assertion Example: diff --git a/drivers/pci/host/pci-mvebu.c b/drivers/pci/host/pci-mvebu.c index 2370b59..bdad4d4 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; @@ -856,6 +862,7 @@ static int mvebu_pcie_probe(struct platform_device *pdev) for_each_child_of_node(pdev->dev.of_node, child) { struct mvebu_pcie_port *port = &pcie->ports[pcie->nports]; + enum of_gpio_flags flags; if (!of_device_is_available(child)) continue; @@ -896,6 +903,25 @@ 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_mdelay = 20; + port->reset_active_low = flags & OF_GPIO_ACTIVE_LOW; + port->reset_name = kasprintf(GFP_KERNEL, + "pcie%d.%d-reset", port->port, port->lane); + ret = devm_gpio_request_one(&pdev->dev, + port->reset_gpio, GPIOF_DIR_OUT, port->reset_name); + if (ret) + return ret; + gpio_set_value(port->reset_gpio, + (port->reset_active_low) ? 1 : 0); + + of_property_read_u32(child, "reset-delay-ms", + &reset_mdelay); + mdelay(reset_mdelay); + } + 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", @@ -945,6 +971,11 @@ static int mvebu_pcie_remove(struct platform_device *pdev) int i; for (i = 0; i < pcie->nports; i++, port++) { + if (gpio_is_valid(port->reset_gpio)) { + gpio_set_value(port->reset_gpio, + (port->reset_active_low) ? 0 : 1); + kfree(port->reset_name); + } clk_disable_unprepare(port->clk); kfree(port->name); }
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 <sebastian.hesselbarth@gmail.com> --- Cc: Russell King <linux@arm.linux.org.uk> Cc: Jason Cooper <jason@lakedaemon.net> Cc: Andrew Lunn <andrew@lunn.ch> Cc: Bjorn Helgaas <bhelgaas@google.com> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> Cc: devicetree@vger.kernel.org Cc: linux-doc@vger.kernel.org Cc: linux-kernel@vger.kernel.org Cc: linux-arm-kernel@lists.infradead.org Cc: linux-pci@vger.kernel.org --- .../devicetree/bindings/pci/mvebu-pci.txt | 2 ++ drivers/pci/host/pci-mvebu.c | 33 +++++++++++++++++++- 2 files changed, 34 insertions(+), 1 deletion(-)