All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] PCI: armada8k: add support for gpio controlled reset signal
@ 2018-10-03 12:49 ` Baruch Siach
  0 siblings, 0 replies; 18+ messages in thread
From: Baruch Siach @ 2018-10-03 12:49 UTC (permalink / raw)
  To: Thomas Petazzoni
  Cc: linux-pci, linux-arm-kernel, Lorenzo Pieralisi, Bjorn Helgaas,
	Baruch Siach

This commit adds support for the gpio reset signal binding as described
in the designware-pcie.txt DT binding document. Both the documented
'reset-gpio' property name, and the more standard 'reset-gpios' name are
supported.

Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
 drivers/pci/controller/dwc/pcie-armada8k.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-armada8k.c b/drivers/pci/controller/dwc/pcie-armada8k.c
index 0c389a30ef5d..b171b6bc15c8 100644
--- a/drivers/pci/controller/dwc/pcie-armada8k.c
+++ b/drivers/pci/controller/dwc/pcie-armada8k.c
@@ -22,6 +22,7 @@
 #include <linux/resource.h>
 #include <linux/of_pci.h>
 #include <linux/of_irq.h>
+#include <linux/gpio/consumer.h>
 
 #include "pcie-designware.h"
 
@@ -29,6 +30,7 @@ struct armada8k_pcie {
 	struct dw_pcie *pci;
 	struct clk *clk;
 	struct clk *clk_reg;
+	struct gpio_desc *reset_gpio;
 };
 
 #define PCIE_VENDOR_REGS_OFFSET		0x8000
@@ -137,6 +139,12 @@ static int armada8k_pcie_host_init(struct pcie_port *pp)
 	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
 	struct armada8k_pcie *pcie = to_armada8k_pcie(pci);
 
+	if (pcie->reset_gpio) {
+		/* assert and then deassert the reset signal */
+		gpiod_set_value_cansleep(pcie->reset_gpio, 1);
+		msleep(100);
+		gpiod_set_value_cansleep(pcie->reset_gpio, 0);
+	}
 	dw_pcie_setup_rc(pp);
 	armada8k_pcie_establish_link(pcie);
 
@@ -249,6 +257,14 @@ static int armada8k_pcie_probe(struct platform_device *pdev)
 		goto fail_clkreg;
 	}
 
+	/* Get reset gpio signal and hold asserted (logically high) */
+	pcie->reset_gpio = devm_gpiod_get_optional(dev, "reset",
+						   GPIOD_OUT_HIGH);
+	if (IS_ERR(pcie->reset_gpio)) {
+		ret = PTR_ERR(pcie->reset_gpio);
+		goto fail_clkreg;
+	}
+
 	platform_set_drvdata(pdev, pcie);
 
 	ret = armada8k_add_pcie_port(pcie, pdev);
-- 
2.19.0


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH] PCI: armada8k: add support for gpio controlled reset signal
@ 2018-10-03 12:49 ` Baruch Siach
  0 siblings, 0 replies; 18+ messages in thread
From: Baruch Siach @ 2018-10-03 12:49 UTC (permalink / raw)
  To: linux-arm-kernel

This commit adds support for the gpio reset signal binding as described
in the designware-pcie.txt DT binding document. Both the documented
'reset-gpio' property name, and the more standard 'reset-gpios' name are
supported.

Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
 drivers/pci/controller/dwc/pcie-armada8k.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-armada8k.c b/drivers/pci/controller/dwc/pcie-armada8k.c
index 0c389a30ef5d..b171b6bc15c8 100644
--- a/drivers/pci/controller/dwc/pcie-armada8k.c
+++ b/drivers/pci/controller/dwc/pcie-armada8k.c
@@ -22,6 +22,7 @@
 #include <linux/resource.h>
 #include <linux/of_pci.h>
 #include <linux/of_irq.h>
+#include <linux/gpio/consumer.h>
 
 #include "pcie-designware.h"
 
@@ -29,6 +30,7 @@ struct armada8k_pcie {
 	struct dw_pcie *pci;
 	struct clk *clk;
 	struct clk *clk_reg;
+	struct gpio_desc *reset_gpio;
 };
 
 #define PCIE_VENDOR_REGS_OFFSET		0x8000
@@ -137,6 +139,12 @@ static int armada8k_pcie_host_init(struct pcie_port *pp)
 	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
 	struct armada8k_pcie *pcie = to_armada8k_pcie(pci);
 
+	if (pcie->reset_gpio) {
+		/* assert and then deassert the reset signal */
+		gpiod_set_value_cansleep(pcie->reset_gpio, 1);
+		msleep(100);
+		gpiod_set_value_cansleep(pcie->reset_gpio, 0);
+	}
 	dw_pcie_setup_rc(pp);
 	armada8k_pcie_establish_link(pcie);
 
@@ -249,6 +257,14 @@ static int armada8k_pcie_probe(struct platform_device *pdev)
 		goto fail_clkreg;
 	}
 
+	/* Get reset gpio signal and hold asserted (logically high) */
+	pcie->reset_gpio = devm_gpiod_get_optional(dev, "reset",
+						   GPIOD_OUT_HIGH);
+	if (IS_ERR(pcie->reset_gpio)) {
+		ret = PTR_ERR(pcie->reset_gpio);
+		goto fail_clkreg;
+	}
+
 	platform_set_drvdata(pdev, pcie);
 
 	ret = armada8k_add_pcie_port(pcie, pdev);
-- 
2.19.0

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH] PCI: armada8k: add support for gpio controlled reset signal
  2018-10-03 12:49 ` Baruch Siach
@ 2018-10-03 13:28   ` Andrew Lunn
  -1 siblings, 0 replies; 18+ messages in thread
From: Andrew Lunn @ 2018-10-03 13:28 UTC (permalink / raw)
  To: Baruch Siach
  Cc: Thomas Petazzoni, Bjorn Helgaas, linux-pci, Lorenzo Pieralisi,
	linux-arm-kernel

On Wed, Oct 03, 2018 at 03:49:43PM +0300, Baruch Siach wrote:
> This commit adds support for the gpio reset signal binding as described
> in the designware-pcie.txt DT binding document. Both the documented
> 'reset-gpio' property name, and the more standard 'reset-gpios' name are
> supported.

Hi Baruch

I don't know this code at all, so maybe a dumb question. Why support
the old none-standard binding of reset-gpio on new hardware? I'm
assuming reset-gpio is marked a deprecated and reset-gpios is
recommended?

    Andrew

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH] PCI: armada8k: add support for gpio controlled reset signal
@ 2018-10-03 13:28   ` Andrew Lunn
  0 siblings, 0 replies; 18+ messages in thread
From: Andrew Lunn @ 2018-10-03 13:28 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Oct 03, 2018 at 03:49:43PM +0300, Baruch Siach wrote:
> This commit adds support for the gpio reset signal binding as described
> in the designware-pcie.txt DT binding document. Both the documented
> 'reset-gpio' property name, and the more standard 'reset-gpios' name are
> supported.

Hi Baruch

I don't know this code at all, so maybe a dumb question. Why support
the old none-standard binding of reset-gpio on new hardware? I'm
assuming reset-gpio is marked a deprecated and reset-gpios is
recommended?

    Andrew

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] PCI: armada8k: add support for gpio controlled reset signal
  2018-10-03 13:28   ` Andrew Lunn
@ 2018-10-03 13:35     ` Baruch Siach
  -1 siblings, 0 replies; 18+ messages in thread
From: Baruch Siach @ 2018-10-03 13:35 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Thomas Petazzoni, Bjorn Helgaas, linux-pci, Lorenzo Pieralisi,
	linux-arm-kernel

Hi Andrew,

On Wed, Oct 03, 2018 at 03:28:11PM +0200, Andrew Lunn wrote:
> On Wed, Oct 03, 2018 at 03:49:43PM +0300, Baruch Siach wrote:
> > This commit adds support for the gpio reset signal binding as described
> > in the designware-pcie.txt DT binding document. Both the documented
> > 'reset-gpio' property name, and the more standard 'reset-gpios' name are
> > supported.
> 
> I don't know this code at all, so maybe a dumb question. Why support
> the old none-standard binding of reset-gpio on new hardware? I'm
> assuming reset-gpio is marked a deprecated and reset-gpios is
> recommended?

This is all hidden behind the devm_gpiod_get_optional() call that only sees 
the "reset" string. The generic code supports both the new property name and 
also the older one for backward compatibility. This patch changes nothing in 
this regard.

The designware-pcie.txt document mentions the older 'reset-gpio' name. So I 
mentioned that name as well in the commit log.

baruch

-- 
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch@tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH] PCI: armada8k: add support for gpio controlled reset signal
@ 2018-10-03 13:35     ` Baruch Siach
  0 siblings, 0 replies; 18+ messages in thread
From: Baruch Siach @ 2018-10-03 13:35 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Andrew,

On Wed, Oct 03, 2018 at 03:28:11PM +0200, Andrew Lunn wrote:
> On Wed, Oct 03, 2018 at 03:49:43PM +0300, Baruch Siach wrote:
> > This commit adds support for the gpio reset signal binding as described
> > in the designware-pcie.txt DT binding document. Both the documented
> > 'reset-gpio' property name, and the more standard 'reset-gpios' name are
> > supported.
> 
> I don't know this code at all, so maybe a dumb question. Why support
> the old none-standard binding of reset-gpio on new hardware? I'm
> assuming reset-gpio is marked a deprecated and reset-gpios is
> recommended?

This is all hidden behind the devm_gpiod_get_optional() call that only sees 
the "reset" string. The generic code supports both the new property name and 
also the older one for backward compatibility. This patch changes nothing in 
this regard.

The designware-pcie.txt document mentions the older 'reset-gpio' name. So I 
mentioned that name as well in the commit log.

baruch

-- 
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] PCI: armada8k: add support for gpio controlled reset signal
  2018-10-03 13:35     ` Baruch Siach
@ 2018-11-16 12:10       ` Lorenzo Pieralisi
  -1 siblings, 0 replies; 18+ messages in thread
From: Lorenzo Pieralisi @ 2018-11-16 12:10 UTC (permalink / raw)
  To: Baruch Siach, Thomas Petazzoni
  Cc: Andrew Lunn, Thomas Petazzoni, Bjorn Helgaas, linux-pci,
	linux-arm-kernel

On Wed, Oct 03, 2018 at 04:35:43PM +0300, Baruch Siach wrote:
> Hi Andrew,
> 
> On Wed, Oct 03, 2018 at 03:28:11PM +0200, Andrew Lunn wrote:
> > On Wed, Oct 03, 2018 at 03:49:43PM +0300, Baruch Siach wrote:
> > > This commit adds support for the gpio reset signal binding as described
> > > in the designware-pcie.txt DT binding document. Both the documented
> > > 'reset-gpio' property name, and the more standard 'reset-gpios' name are
> > > supported.
> > 
> > I don't know this code at all, so maybe a dumb question. Why support
> > the old none-standard binding of reset-gpio on new hardware? I'm
> > assuming reset-gpio is marked a deprecated and reset-gpios is
> > recommended?
> 
> This is all hidden behind the devm_gpiod_get_optional() call that only sees 
> the "reset" string. The generic code supports both the new property name and 
> also the older one for backward compatibility. This patch changes nothing in 
> this regard.
> 
> The designware-pcie.txt document mentions the older 'reset-gpio' name. So I 
> mentioned that name as well in the commit log.

I need Thomas' ACK to proceed with this patch.

Thanks,
Lorenzo

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH] PCI: armada8k: add support for gpio controlled reset signal
@ 2018-11-16 12:10       ` Lorenzo Pieralisi
  0 siblings, 0 replies; 18+ messages in thread
From: Lorenzo Pieralisi @ 2018-11-16 12:10 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Oct 03, 2018 at 04:35:43PM +0300, Baruch Siach wrote:
> Hi Andrew,
> 
> On Wed, Oct 03, 2018 at 03:28:11PM +0200, Andrew Lunn wrote:
> > On Wed, Oct 03, 2018 at 03:49:43PM +0300, Baruch Siach wrote:
> > > This commit adds support for the gpio reset signal binding as described
> > > in the designware-pcie.txt DT binding document. Both the documented
> > > 'reset-gpio' property name, and the more standard 'reset-gpios' name are
> > > supported.
> > 
> > I don't know this code at all, so maybe a dumb question. Why support
> > the old none-standard binding of reset-gpio on new hardware? I'm
> > assuming reset-gpio is marked a deprecated and reset-gpios is
> > recommended?
> 
> This is all hidden behind the devm_gpiod_get_optional() call that only sees 
> the "reset" string. The generic code supports both the new property name and 
> also the older one for backward compatibility. This patch changes nothing in 
> this regard.
> 
> The designware-pcie.txt document mentions the older 'reset-gpio' name. So I 
> mentioned that name as well in the commit log.

I need Thomas' ACK to proceed with this patch.

Thanks,
Lorenzo

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] PCI: armada8k: add support for gpio controlled reset signal
  2018-11-16 12:10       ` Lorenzo Pieralisi
@ 2018-11-21  7:09         ` Baruch Siach
  -1 siblings, 0 replies; 18+ messages in thread
From: Baruch Siach @ 2018-11-21  7:09 UTC (permalink / raw)
  To: Thomas Petazzoni
  Cc: Lorenzo Pieralisi, Andrew Lunn, Bjorn Helgaas, linux-pci,
	linux-arm-kernel

Hi Thomas,

As the listed maintainer of this driver, can you review this patch? This
patch makes the PCIe slot reset sequence independent from the
bootloader.

Thanks,
baruch

Lorenzo Pieralisi writes:
> On Wed, Oct 03, 2018 at 04:35:43PM +0300, Baruch Siach wrote:
>> On Wed, Oct 03, 2018 at 03:28:11PM +0200, Andrew Lunn wrote:
>> > On Wed, Oct 03, 2018 at 03:49:43PM +0300, Baruch Siach wrote:
>> > > This commit adds support for the gpio reset signal binding as described
>> > > in the designware-pcie.txt DT binding document. Both the documented
>> > > 'reset-gpio' property name, and the more standard 'reset-gpios' name are
>> > > supported.
>> >
>> > I don't know this code at all, so maybe a dumb question. Why support
>> > the old none-standard binding of reset-gpio on new hardware? I'm
>> > assuming reset-gpio is marked a deprecated and reset-gpios is
>> > recommended?
>>
>> This is all hidden behind the devm_gpiod_get_optional() call that only sees
>> the "reset" string. The generic code supports both the new property name and
>> also the older one for backward compatibility. This patch changes nothing in
>> this regard.
>>
>> The designware-pcie.txt document mentions the older 'reset-gpio' name. So I
>> mentioned that name as well in the commit log.
>
> I need Thomas' ACK to proceed with this patch.
>
> Thanks,
> Lorenzo

--
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch@tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH] PCI: armada8k: add support for gpio controlled reset signal
@ 2018-11-21  7:09         ` Baruch Siach
  0 siblings, 0 replies; 18+ messages in thread
From: Baruch Siach @ 2018-11-21  7:09 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Thomas,

As the listed maintainer of this driver, can you review this patch? This
patch makes the PCIe slot reset sequence independent from the
bootloader.

Thanks,
baruch

Lorenzo Pieralisi writes:
> On Wed, Oct 03, 2018 at 04:35:43PM +0300, Baruch Siach wrote:
>> On Wed, Oct 03, 2018 at 03:28:11PM +0200, Andrew Lunn wrote:
>> > On Wed, Oct 03, 2018 at 03:49:43PM +0300, Baruch Siach wrote:
>> > > This commit adds support for the gpio reset signal binding as described
>> > > in the designware-pcie.txt DT binding document. Both the documented
>> > > 'reset-gpio' property name, and the more standard 'reset-gpios' name are
>> > > supported.
>> >
>> > I don't know this code at all, so maybe a dumb question. Why support
>> > the old none-standard binding of reset-gpio on new hardware? I'm
>> > assuming reset-gpio is marked a deprecated and reset-gpios is
>> > recommended?
>>
>> This is all hidden behind the devm_gpiod_get_optional() call that only sees
>> the "reset" string. The generic code supports both the new property name and
>> also the older one for backward compatibility. This patch changes nothing in
>> this regard.
>>
>> The designware-pcie.txt document mentions the older 'reset-gpio' name. So I
>> mentioned that name as well in the commit log.
>
> I need Thomas' ACK to proceed with this patch.
>
> Thanks,
> Lorenzo

--
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] PCI: armada8k: add support for gpio controlled reset signal
  2018-11-21  7:09         ` Baruch Siach
@ 2018-11-21  7:47           ` Thomas Petazzoni
  -1 siblings, 0 replies; 18+ messages in thread
From: Thomas Petazzoni @ 2018-11-21  7:47 UTC (permalink / raw)
  To: Baruch Siach
  Cc: Lorenzo Pieralisi, Andrew Lunn, Bjorn Helgaas, linux-pci,
	linux-arm-kernel

Hello,

On Wed, 21 Nov 2018 09:09:46 +0200, Baruch Siach wrote:

> As the listed maintainer of this driver, can you review this patch? This
> patch makes the PCIe slot reset sequence independent from the
> bootloader.

Yes, I'll have a look. Sorry for the delay.

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH] PCI: armada8k: add support for gpio controlled reset signal
@ 2018-11-21  7:47           ` Thomas Petazzoni
  0 siblings, 0 replies; 18+ messages in thread
From: Thomas Petazzoni @ 2018-11-21  7:47 UTC (permalink / raw)
  To: linux-arm-kernel

Hello,

On Wed, 21 Nov 2018 09:09:46 +0200, Baruch Siach wrote:

> As the listed maintainer of this driver, can you review this patch? This
> patch makes the PCIe slot reset sequence independent from the
> bootloader.

Yes, I'll have a look. Sorry for the delay.

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] PCI: armada8k: add support for gpio controlled reset signal
  2018-10-03 12:49 ` Baruch Siach
@ 2018-11-22 14:45   ` Thomas Petazzoni
  -1 siblings, 0 replies; 18+ messages in thread
From: Thomas Petazzoni @ 2018-11-22 14:45 UTC (permalink / raw)
  To: Baruch Siach
  Cc: linux-pci, linux-arm-kernel, Lorenzo Pieralisi, Bjorn Helgaas

Hello Baruch,

Sorry for the delayed review.

On Wed,  3 Oct 2018 15:49:43 +0300, Baruch Siach wrote:

>  #define PCIE_VENDOR_REGS_OFFSET		0x8000
> @@ -137,6 +139,12 @@ static int armada8k_pcie_host_init(struct pcie_port *pp)
>  	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
>  	struct armada8k_pcie *pcie = to_armada8k_pcie(pci);
>  
> +	if (pcie->reset_gpio) {

This should be:

	if (!IS_ERR(pcie->reset_gpio))

Indeed, in the case of an error, pcie->reset_gpio will be non-NULL,
with the error encoded as a ERR_PTR().

> +		/* assert and then deassert the reset signal */
> +		gpiod_set_value_cansleep(pcie->reset_gpio, 1);
> +		msleep(100);
> +		gpiod_set_value_cansleep(pcie->reset_gpio, 0);
> +	}
>	dw_pcie_setup_rc(pp);

An empty new line here would be good before the dw_pcie_setup_rc() call.

If you send a new iteration with those two issues fixed, you can
directly add my

  Acked-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH] PCI: armada8k: add support for gpio controlled reset signal
@ 2018-11-22 14:45   ` Thomas Petazzoni
  0 siblings, 0 replies; 18+ messages in thread
From: Thomas Petazzoni @ 2018-11-22 14:45 UTC (permalink / raw)
  To: linux-arm-kernel

Hello Baruch,

Sorry for the delayed review.

On Wed,  3 Oct 2018 15:49:43 +0300, Baruch Siach wrote:

>  #define PCIE_VENDOR_REGS_OFFSET		0x8000
> @@ -137,6 +139,12 @@ static int armada8k_pcie_host_init(struct pcie_port *pp)
>  	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
>  	struct armada8k_pcie *pcie = to_armada8k_pcie(pci);
>  
> +	if (pcie->reset_gpio) {

This should be:

	if (!IS_ERR(pcie->reset_gpio))

Indeed, in the case of an error, pcie->reset_gpio will be non-NULL,
with the error encoded as a ERR_PTR().

> +		/* assert and then deassert the reset signal */
> +		gpiod_set_value_cansleep(pcie->reset_gpio, 1);
> +		msleep(100);
> +		gpiod_set_value_cansleep(pcie->reset_gpio, 0);
> +	}
>	dw_pcie_setup_rc(pp);

An empty new line here would be good before the dw_pcie_setup_rc() call.

If you send a new iteration with those two issues fixed, you can
directly add my

  Acked-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] PCI: armada8k: add support for gpio controlled reset signal
  2018-11-22 14:45   ` Thomas Petazzoni
@ 2018-11-22 14:46     ` Thomas Petazzoni
  -1 siblings, 0 replies; 18+ messages in thread
From: Thomas Petazzoni @ 2018-11-22 14:46 UTC (permalink / raw)
  To: Baruch Siach
  Cc: linux-pci, linux-arm-kernel, Lorenzo Pieralisi, Bjorn Helgaas

Hello,

On Thu, 22 Nov 2018 15:45:23 +0100, Thomas Petazzoni wrote:

> This should be:
> 
> 	if (!IS_ERR(pcie->reset_gpio))
> 
> Indeed, in the case of an error, pcie->reset_gpio will be non-NULL,
> with the error encoded as a ERR_PTR().

Meh, scrap that. If pcie->reset_gpio was an error, probe() has failed.
So by the time we are in armada8k_pcie_host_init(), pcie->reset_gpio is
either NULL or a valid GPIO. So my comment was stupid.

Since the newline thing is way too minor to require a new iteration:

Acked-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>

Thanks, and sorry for the noise.

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH] PCI: armada8k: add support for gpio controlled reset signal
@ 2018-11-22 14:46     ` Thomas Petazzoni
  0 siblings, 0 replies; 18+ messages in thread
From: Thomas Petazzoni @ 2018-11-22 14:46 UTC (permalink / raw)
  To: linux-arm-kernel

Hello,

On Thu, 22 Nov 2018 15:45:23 +0100, Thomas Petazzoni wrote:

> This should be:
> 
> 	if (!IS_ERR(pcie->reset_gpio))
> 
> Indeed, in the case of an error, pcie->reset_gpio will be non-NULL,
> with the error encoded as a ERR_PTR().

Meh, scrap that. If pcie->reset_gpio was an error, probe() has failed.
So by the time we are in armada8k_pcie_host_init(), pcie->reset_gpio is
either NULL or a valid GPIO. So my comment was stupid.

Since the newline thing is way too minor to require a new iteration:

Acked-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>

Thanks, and sorry for the noise.

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] PCI: armada8k: add support for gpio controlled reset signal
  2018-10-03 12:49 ` Baruch Siach
@ 2018-11-22 15:23   ` Lorenzo Pieralisi
  -1 siblings, 0 replies; 18+ messages in thread
From: Lorenzo Pieralisi @ 2018-11-22 15:23 UTC (permalink / raw)
  To: Baruch Siach; +Cc: Thomas Petazzoni, linux-pci, linux-arm-kernel, Bjorn Helgaas

On Wed, Oct 03, 2018 at 03:49:43PM +0300, Baruch Siach wrote:
> This commit adds support for the gpio reset signal binding as described
> in the designware-pcie.txt DT binding document. Both the documented
> 'reset-gpio' property name, and the more standard 'reset-gpios' name are
> supported.
> 
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> ---
>  drivers/pci/controller/dwc/pcie-armada8k.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)

Applied to pci/dwc for v4.21, thanks.

Lorenzo

> diff --git a/drivers/pci/controller/dwc/pcie-armada8k.c b/drivers/pci/controller/dwc/pcie-armada8k.c
> index 0c389a30ef5d..b171b6bc15c8 100644
> --- a/drivers/pci/controller/dwc/pcie-armada8k.c
> +++ b/drivers/pci/controller/dwc/pcie-armada8k.c
> @@ -22,6 +22,7 @@
>  #include <linux/resource.h>
>  #include <linux/of_pci.h>
>  #include <linux/of_irq.h>
> +#include <linux/gpio/consumer.h>
>  
>  #include "pcie-designware.h"
>  
> @@ -29,6 +30,7 @@ struct armada8k_pcie {
>  	struct dw_pcie *pci;
>  	struct clk *clk;
>  	struct clk *clk_reg;
> +	struct gpio_desc *reset_gpio;
>  };
>  
>  #define PCIE_VENDOR_REGS_OFFSET		0x8000
> @@ -137,6 +139,12 @@ static int armada8k_pcie_host_init(struct pcie_port *pp)
>  	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
>  	struct armada8k_pcie *pcie = to_armada8k_pcie(pci);
>  
> +	if (pcie->reset_gpio) {
> +		/* assert and then deassert the reset signal */
> +		gpiod_set_value_cansleep(pcie->reset_gpio, 1);
> +		msleep(100);
> +		gpiod_set_value_cansleep(pcie->reset_gpio, 0);
> +	}
>  	dw_pcie_setup_rc(pp);
>  	armada8k_pcie_establish_link(pcie);
>  
> @@ -249,6 +257,14 @@ static int armada8k_pcie_probe(struct platform_device *pdev)
>  		goto fail_clkreg;
>  	}
>  
> +	/* Get reset gpio signal and hold asserted (logically high) */
> +	pcie->reset_gpio = devm_gpiod_get_optional(dev, "reset",
> +						   GPIOD_OUT_HIGH);
> +	if (IS_ERR(pcie->reset_gpio)) {
> +		ret = PTR_ERR(pcie->reset_gpio);
> +		goto fail_clkreg;
> +	}
> +
>  	platform_set_drvdata(pdev, pcie);
>  
>  	ret = armada8k_add_pcie_port(pcie, pdev);
> -- 
> 2.19.0
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH] PCI: armada8k: add support for gpio controlled reset signal
@ 2018-11-22 15:23   ` Lorenzo Pieralisi
  0 siblings, 0 replies; 18+ messages in thread
From: Lorenzo Pieralisi @ 2018-11-22 15:23 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Oct 03, 2018 at 03:49:43PM +0300, Baruch Siach wrote:
> This commit adds support for the gpio reset signal binding as described
> in the designware-pcie.txt DT binding document. Both the documented
> 'reset-gpio' property name, and the more standard 'reset-gpios' name are
> supported.
> 
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> ---
>  drivers/pci/controller/dwc/pcie-armada8k.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)

Applied to pci/dwc for v4.21, thanks.

Lorenzo

> diff --git a/drivers/pci/controller/dwc/pcie-armada8k.c b/drivers/pci/controller/dwc/pcie-armada8k.c
> index 0c389a30ef5d..b171b6bc15c8 100644
> --- a/drivers/pci/controller/dwc/pcie-armada8k.c
> +++ b/drivers/pci/controller/dwc/pcie-armada8k.c
> @@ -22,6 +22,7 @@
>  #include <linux/resource.h>
>  #include <linux/of_pci.h>
>  #include <linux/of_irq.h>
> +#include <linux/gpio/consumer.h>
>  
>  #include "pcie-designware.h"
>  
> @@ -29,6 +30,7 @@ struct armada8k_pcie {
>  	struct dw_pcie *pci;
>  	struct clk *clk;
>  	struct clk *clk_reg;
> +	struct gpio_desc *reset_gpio;
>  };
>  
>  #define PCIE_VENDOR_REGS_OFFSET		0x8000
> @@ -137,6 +139,12 @@ static int armada8k_pcie_host_init(struct pcie_port *pp)
>  	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
>  	struct armada8k_pcie *pcie = to_armada8k_pcie(pci);
>  
> +	if (pcie->reset_gpio) {
> +		/* assert and then deassert the reset signal */
> +		gpiod_set_value_cansleep(pcie->reset_gpio, 1);
> +		msleep(100);
> +		gpiod_set_value_cansleep(pcie->reset_gpio, 0);
> +	}
>  	dw_pcie_setup_rc(pp);
>  	armada8k_pcie_establish_link(pcie);
>  
> @@ -249,6 +257,14 @@ static int armada8k_pcie_probe(struct platform_device *pdev)
>  		goto fail_clkreg;
>  	}
>  
> +	/* Get reset gpio signal and hold asserted (logically high) */
> +	pcie->reset_gpio = devm_gpiod_get_optional(dev, "reset",
> +						   GPIOD_OUT_HIGH);
> +	if (IS_ERR(pcie->reset_gpio)) {
> +		ret = PTR_ERR(pcie->reset_gpio);
> +		goto fail_clkreg;
> +	}
> +
>  	platform_set_drvdata(pdev, pcie);
>  
>  	ret = armada8k_add_pcie_port(pcie, pdev);
> -- 
> 2.19.0
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2018-11-22 15:23 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-03 12:49 [PATCH] PCI: armada8k: add support for gpio controlled reset signal Baruch Siach
2018-10-03 12:49 ` Baruch Siach
2018-10-03 13:28 ` Andrew Lunn
2018-10-03 13:28   ` Andrew Lunn
2018-10-03 13:35   ` Baruch Siach
2018-10-03 13:35     ` Baruch Siach
2018-11-16 12:10     ` Lorenzo Pieralisi
2018-11-16 12:10       ` Lorenzo Pieralisi
2018-11-21  7:09       ` Baruch Siach
2018-11-21  7:09         ` Baruch Siach
2018-11-21  7:47         ` Thomas Petazzoni
2018-11-21  7:47           ` Thomas Petazzoni
2018-11-22 14:45 ` Thomas Petazzoni
2018-11-22 14:45   ` Thomas Petazzoni
2018-11-22 14:46   ` Thomas Petazzoni
2018-11-22 14:46     ` Thomas Petazzoni
2018-11-22 15:23 ` Lorenzo Pieralisi
2018-11-22 15:23   ` Lorenzo Pieralisi

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.