All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] PCI: amlogic: Fix reset assertion via gpio descriptor
@ 2019-09-01 13:39 ` Remi Pommarel
  0 siblings, 0 replies; 14+ messages in thread
From: Remi Pommarel @ 2019-09-01 13:39 UTC (permalink / raw)
  To: Yue Wang, Lorenzo Pieralisi, Bjorn Helgaas, Kevin Hilman
  Cc: linux-pci, linux-amlogic, Remi Pommarel

Normally asserting reset signal on gpio would be achieved with:
	gpiod_set_value_cansleep(reset_gpio, 1);

Meson PCI driver set reset value to '0' instead of '1' as it takes into
account the PERST# signal polarity. The polarity should be taken care
in the device tree instead.

This fixes the reset assertion meaning and moves out the polarity
configuration in DT (please note that there is no DT currently using
this driver).

Signed-off-by: Remi Pommarel <repk@triplefau.lt>
---
 drivers/pci/controller/dwc/pci-meson.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/controller/dwc/pci-meson.c b/drivers/pci/controller/dwc/pci-meson.c
index e35e9eaa50ee..541f37a6f6a5 100644
--- a/drivers/pci/controller/dwc/pci-meson.c
+++ b/drivers/pci/controller/dwc/pci-meson.c
@@ -287,9 +287,9 @@ static inline void meson_cfg_writel(struct meson_pcie *mp, u32 val, u32 reg)
 
 static void meson_pcie_assert_reset(struct meson_pcie *mp)
 {
-	gpiod_set_value_cansleep(mp->reset_gpio, 0);
-	udelay(500);
 	gpiod_set_value_cansleep(mp->reset_gpio, 1);
+	udelay(500);
+	gpiod_set_value_cansleep(mp->reset_gpio, 0);
 }
 
 static void meson_pcie_init_dw(struct meson_pcie *mp)
-- 
2.20.1


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

* [PATCH] PCI: amlogic: Fix reset assertion via gpio descriptor
@ 2019-09-01 13:39 ` Remi Pommarel
  0 siblings, 0 replies; 14+ messages in thread
From: Remi Pommarel @ 2019-09-01 13:39 UTC (permalink / raw)
  To: Yue Wang, Lorenzo Pieralisi, Bjorn Helgaas, Kevin Hilman
  Cc: linux-pci, Remi Pommarel, linux-amlogic

Normally asserting reset signal on gpio would be achieved with:
	gpiod_set_value_cansleep(reset_gpio, 1);

Meson PCI driver set reset value to '0' instead of '1' as it takes into
account the PERST# signal polarity. The polarity should be taken care
in the device tree instead.

This fixes the reset assertion meaning and moves out the polarity
configuration in DT (please note that there is no DT currently using
this driver).

Signed-off-by: Remi Pommarel <repk@triplefau.lt>
---
 drivers/pci/controller/dwc/pci-meson.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/controller/dwc/pci-meson.c b/drivers/pci/controller/dwc/pci-meson.c
index e35e9eaa50ee..541f37a6f6a5 100644
--- a/drivers/pci/controller/dwc/pci-meson.c
+++ b/drivers/pci/controller/dwc/pci-meson.c
@@ -287,9 +287,9 @@ static inline void meson_cfg_writel(struct meson_pcie *mp, u32 val, u32 reg)
 
 static void meson_pcie_assert_reset(struct meson_pcie *mp)
 {
-	gpiod_set_value_cansleep(mp->reset_gpio, 0);
-	udelay(500);
 	gpiod_set_value_cansleep(mp->reset_gpio, 1);
+	udelay(500);
+	gpiod_set_value_cansleep(mp->reset_gpio, 0);
 }
 
 static void meson_pcie_init_dw(struct meson_pcie *mp)
-- 
2.20.1


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

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

* Re: [PATCH] PCI: amlogic: Fix reset assertion via gpio descriptor
  2019-09-01 13:39 ` Remi Pommarel
@ 2019-09-01 21:46   ` Martin Blumenstingl
  -1 siblings, 0 replies; 14+ messages in thread
From: Martin Blumenstingl @ 2019-09-01 21:46 UTC (permalink / raw)
  To: Remi Pommarel
  Cc: Yue Wang, Lorenzo Pieralisi, Bjorn Helgaas, Kevin Hilman,
	linux-pci, linux-amlogic

On Sun, Sep 1, 2019 at 3:30 PM Remi Pommarel <repk@triplefau.lt> wrote:
>
> Normally asserting reset signal on gpio would be achieved with:
>         gpiod_set_value_cansleep(reset_gpio, 1);
>
> Meson PCI driver set reset value to '0' instead of '1' as it takes into
> account the PERST# signal polarity. The polarity should be taken care
> in the device tree instead.
>
> This fixes the reset assertion meaning and moves out the polarity
> configuration in DT (please note that there is no DT currently using
> this driver).
>

Fixes: 9c0ef6d34fdb ("PCI: amlogic: Add the Amlogic Meson PCIe
controller driver")
> Signed-off-by: Remi Pommarel <repk@triplefau.lt>
Reviewed-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>

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

* Re: [PATCH] PCI: amlogic: Fix reset assertion via gpio descriptor
@ 2019-09-01 21:46   ` Martin Blumenstingl
  0 siblings, 0 replies; 14+ messages in thread
From: Martin Blumenstingl @ 2019-09-01 21:46 UTC (permalink / raw)
  To: Remi Pommarel
  Cc: Lorenzo Pieralisi, Kevin Hilman, Yue Wang, linux-pci,
	Bjorn Helgaas, linux-amlogic

On Sun, Sep 1, 2019 at 3:30 PM Remi Pommarel <repk@triplefau.lt> wrote:
>
> Normally asserting reset signal on gpio would be achieved with:
>         gpiod_set_value_cansleep(reset_gpio, 1);
>
> Meson PCI driver set reset value to '0' instead of '1' as it takes into
> account the PERST# signal polarity. The polarity should be taken care
> in the device tree instead.
>
> This fixes the reset assertion meaning and moves out the polarity
> configuration in DT (please note that there is no DT currently using
> this driver).
>

Fixes: 9c0ef6d34fdb ("PCI: amlogic: Add the Amlogic Meson PCIe
controller driver")
> Signed-off-by: Remi Pommarel <repk@triplefau.lt>
Reviewed-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

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

* Re: [PATCH] PCI: amlogic: Fix reset assertion via gpio descriptor
  2019-09-01 13:39 ` Remi Pommarel
@ 2019-09-02  8:11   ` Neil Armstrong
  -1 siblings, 0 replies; 14+ messages in thread
From: Neil Armstrong @ 2019-09-02  8:11 UTC (permalink / raw)
  To: Remi Pommarel, Yue Wang, Lorenzo Pieralisi, Bjorn Helgaas, Kevin Hilman
  Cc: linux-pci, linux-amlogic

On 01/09/2019 15:39, Remi Pommarel wrote:
> Normally asserting reset signal on gpio would be achieved with:
> 	gpiod_set_value_cansleep(reset_gpio, 1);
> 
> Meson PCI driver set reset value to '0' instead of '1' as it takes into
> account the PERST# signal polarity. The polarity should be taken care
> in the device tree instead.
> 
> This fixes the reset assertion meaning and moves out the polarity
> configuration in DT (please note that there is no DT currently using
> this driver).
> 
> Signed-off-by: Remi Pommarel <repk@triplefau.lt>
> ---
>  drivers/pci/controller/dwc/pci-meson.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pci-meson.c b/drivers/pci/controller/dwc/pci-meson.c
> index e35e9eaa50ee..541f37a6f6a5 100644
> --- a/drivers/pci/controller/dwc/pci-meson.c
> +++ b/drivers/pci/controller/dwc/pci-meson.c
> @@ -287,9 +287,9 @@ static inline void meson_cfg_writel(struct meson_pcie *mp, u32 val, u32 reg)
>  
>  static void meson_pcie_assert_reset(struct meson_pcie *mp)
>  {
> -	gpiod_set_value_cansleep(mp->reset_gpio, 0);
> -	udelay(500);
>  	gpiod_set_value_cansleep(mp->reset_gpio, 1);
> +	udelay(500);
> +	gpiod_set_value_cansleep(mp->reset_gpio, 0);
>  }
>  
>  static void meson_pcie_init_dw(struct meson_pcie *mp)
> 

Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>

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

* Re: [PATCH] PCI: amlogic: Fix reset assertion via gpio descriptor
@ 2019-09-02  8:11   ` Neil Armstrong
  0 siblings, 0 replies; 14+ messages in thread
From: Neil Armstrong @ 2019-09-02  8:11 UTC (permalink / raw)
  To: Remi Pommarel, Yue Wang, Lorenzo Pieralisi, Bjorn Helgaas, Kevin Hilman
  Cc: linux-pci, linux-amlogic

On 01/09/2019 15:39, Remi Pommarel wrote:
> Normally asserting reset signal on gpio would be achieved with:
> 	gpiod_set_value_cansleep(reset_gpio, 1);
> 
> Meson PCI driver set reset value to '0' instead of '1' as it takes into
> account the PERST# signal polarity. The polarity should be taken care
> in the device tree instead.
> 
> This fixes the reset assertion meaning and moves out the polarity
> configuration in DT (please note that there is no DT currently using
> this driver).
> 
> Signed-off-by: Remi Pommarel <repk@triplefau.lt>
> ---
>  drivers/pci/controller/dwc/pci-meson.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pci-meson.c b/drivers/pci/controller/dwc/pci-meson.c
> index e35e9eaa50ee..541f37a6f6a5 100644
> --- a/drivers/pci/controller/dwc/pci-meson.c
> +++ b/drivers/pci/controller/dwc/pci-meson.c
> @@ -287,9 +287,9 @@ static inline void meson_cfg_writel(struct meson_pcie *mp, u32 val, u32 reg)
>  
>  static void meson_pcie_assert_reset(struct meson_pcie *mp)
>  {
> -	gpiod_set_value_cansleep(mp->reset_gpio, 0);
> -	udelay(500);
>  	gpiod_set_value_cansleep(mp->reset_gpio, 1);
> +	udelay(500);
> +	gpiod_set_value_cansleep(mp->reset_gpio, 0);
>  }
>  
>  static void meson_pcie_init_dw(struct meson_pcie *mp)
> 

Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

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

* Re: [PATCH] PCI: amlogic: Fix reset assertion via gpio descriptor
  2019-09-01 13:39 ` Remi Pommarel
@ 2019-09-02 10:55   ` Andrew Murray
  -1 siblings, 0 replies; 14+ messages in thread
From: Andrew Murray @ 2019-09-02 10:55 UTC (permalink / raw)
  To: Remi Pommarel
  Cc: Yue Wang, Lorenzo Pieralisi, Bjorn Helgaas, Kevin Hilman,
	linux-pci, linux-amlogic

On Sun, Sep 01, 2019 at 03:39:15PM +0200, Remi Pommarel wrote:
> Normally asserting reset signal on gpio would be achieved with:
> 	gpiod_set_value_cansleep(reset_gpio, 1);
> 
> Meson PCI driver set reset value to '0' instead of '1' as it takes into
> account the PERST# signal polarity. The polarity should be taken care
> in the device tree instead.
> 
> This fixes the reset assertion meaning and moves out the polarity
> configuration in DT (please note that there is no DT currently using
> this driver).

The device tree bindings for this give an example configuration:

        pcie: pcie@f9800000 {
                        compatible = "amlogic,axg-pcie", "snps,dw-pcie";
                        reg = <0x0 0xf9800000 0x0 0x400000
                                        0x0 0xff646000 0x0 0x2000
                                        0x0 0xff644000 0x0 0x2000
                                        0x0 0xf9f00000 0x0 0x100000>;
                        reg-names = "elbi", "cfg", "phy", "config";
                        reset-gpios = <&gpio GPIOX_19 GPIO_ACTIVE_HIGH>;

Is the 'reset-gpios' line still consistent with this change, or does
this need to be updated as well?

Thanks,

Andrew Murray

> 
> Signed-off-by: Remi Pommarel <repk@triplefau.lt>
> ---
>  drivers/pci/controller/dwc/pci-meson.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pci-meson.c b/drivers/pci/controller/dwc/pci-meson.c
> index e35e9eaa50ee..541f37a6f6a5 100644
> --- a/drivers/pci/controller/dwc/pci-meson.c
> +++ b/drivers/pci/controller/dwc/pci-meson.c
> @@ -287,9 +287,9 @@ static inline void meson_cfg_writel(struct meson_pcie *mp, u32 val, u32 reg)
>  
>  static void meson_pcie_assert_reset(struct meson_pcie *mp)
>  {
> -	gpiod_set_value_cansleep(mp->reset_gpio, 0);
> -	udelay(500);
>  	gpiod_set_value_cansleep(mp->reset_gpio, 1);
> +	udelay(500);
> +	gpiod_set_value_cansleep(mp->reset_gpio, 0);
>  }
>  
>  static void meson_pcie_init_dw(struct meson_pcie *mp)
> -- 
> 2.20.1
> 

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

* Re: [PATCH] PCI: amlogic: Fix reset assertion via gpio descriptor
@ 2019-09-02 10:55   ` Andrew Murray
  0 siblings, 0 replies; 14+ messages in thread
From: Andrew Murray @ 2019-09-02 10:55 UTC (permalink / raw)
  To: Remi Pommarel
  Cc: Lorenzo Pieralisi, Kevin Hilman, Yue Wang, linux-pci,
	Bjorn Helgaas, linux-amlogic

On Sun, Sep 01, 2019 at 03:39:15PM +0200, Remi Pommarel wrote:
> Normally asserting reset signal on gpio would be achieved with:
> 	gpiod_set_value_cansleep(reset_gpio, 1);
> 
> Meson PCI driver set reset value to '0' instead of '1' as it takes into
> account the PERST# signal polarity. The polarity should be taken care
> in the device tree instead.
> 
> This fixes the reset assertion meaning and moves out the polarity
> configuration in DT (please note that there is no DT currently using
> this driver).

The device tree bindings for this give an example configuration:

        pcie: pcie@f9800000 {
                        compatible = "amlogic,axg-pcie", "snps,dw-pcie";
                        reg = <0x0 0xf9800000 0x0 0x400000
                                        0x0 0xff646000 0x0 0x2000
                                        0x0 0xff644000 0x0 0x2000
                                        0x0 0xf9f00000 0x0 0x100000>;
                        reg-names = "elbi", "cfg", "phy", "config";
                        reset-gpios = <&gpio GPIOX_19 GPIO_ACTIVE_HIGH>;

Is the 'reset-gpios' line still consistent with this change, or does
this need to be updated as well?

Thanks,

Andrew Murray

> 
> Signed-off-by: Remi Pommarel <repk@triplefau.lt>
> ---
>  drivers/pci/controller/dwc/pci-meson.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pci-meson.c b/drivers/pci/controller/dwc/pci-meson.c
> index e35e9eaa50ee..541f37a6f6a5 100644
> --- a/drivers/pci/controller/dwc/pci-meson.c
> +++ b/drivers/pci/controller/dwc/pci-meson.c
> @@ -287,9 +287,9 @@ static inline void meson_cfg_writel(struct meson_pcie *mp, u32 val, u32 reg)
>  
>  static void meson_pcie_assert_reset(struct meson_pcie *mp)
>  {
> -	gpiod_set_value_cansleep(mp->reset_gpio, 0);
> -	udelay(500);
>  	gpiod_set_value_cansleep(mp->reset_gpio, 1);
> +	udelay(500);
> +	gpiod_set_value_cansleep(mp->reset_gpio, 0);
>  }
>  
>  static void meson_pcie_init_dw(struct meson_pcie *mp)
> -- 
> 2.20.1
> 

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

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

* Re: [PATCH] PCI: amlogic: Fix reset assertion via gpio descriptor
  2019-09-02 10:55   ` Andrew Murray
@ 2019-09-02 14:43     ` Remi Pommarel
  -1 siblings, 0 replies; 14+ messages in thread
From: Remi Pommarel @ 2019-09-02 14:43 UTC (permalink / raw)
  To: Andrew Murray
  Cc: Yue Wang, Lorenzo Pieralisi, Bjorn Helgaas, Kevin Hilman,
	linux-pci, linux-amlogic

On Mon, Sep 02, 2019 at 11:55:36AM +0100, Andrew Murray wrote:
> On Sun, Sep 01, 2019 at 03:39:15PM +0200, Remi Pommarel wrote:
> > Normally asserting reset signal on gpio would be achieved with:
> > 	gpiod_set_value_cansleep(reset_gpio, 1);
> > 
> > Meson PCI driver set reset value to '0' instead of '1' as it takes into
> > account the PERST# signal polarity. The polarity should be taken care
> > in the device tree instead.
> > 
> > This fixes the reset assertion meaning and moves out the polarity
> > configuration in DT (please note that there is no DT currently using
> > this driver).
> 
> The device tree bindings for this give an example configuration:
> 
>         pcie: pcie@f9800000 {
>                         compatible = "amlogic,axg-pcie", "snps,dw-pcie";
>                         reg = <0x0 0xf9800000 0x0 0x400000
>                                         0x0 0xff646000 0x0 0x2000
>                                         0x0 0xff644000 0x0 0x2000
>                                         0x0 0xf9f00000 0x0 0x100000>;
>                         reg-names = "elbi", "cfg", "phy", "config";
>                         reset-gpios = <&gpio GPIOX_19 GPIO_ACTIVE_HIGH>;
> 
> Is the 'reset-gpios' line still consistent with this change, or does
> this need to be updated as well?

Good catch, the polarity of the reset gpio will more likely be
GPIO_ACTIVE_LOW.

Do you want a separate patch for that or can I just include it in v2 ?

Thanks.

-- 
Remi

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

* Re: [PATCH] PCI: amlogic: Fix reset assertion via gpio descriptor
@ 2019-09-02 14:43     ` Remi Pommarel
  0 siblings, 0 replies; 14+ messages in thread
From: Remi Pommarel @ 2019-09-02 14:43 UTC (permalink / raw)
  To: Andrew Murray
  Cc: Lorenzo Pieralisi, Kevin Hilman, Yue Wang, linux-pci,
	Bjorn Helgaas, linux-amlogic

On Mon, Sep 02, 2019 at 11:55:36AM +0100, Andrew Murray wrote:
> On Sun, Sep 01, 2019 at 03:39:15PM +0200, Remi Pommarel wrote:
> > Normally asserting reset signal on gpio would be achieved with:
> > 	gpiod_set_value_cansleep(reset_gpio, 1);
> > 
> > Meson PCI driver set reset value to '0' instead of '1' as it takes into
> > account the PERST# signal polarity. The polarity should be taken care
> > in the device tree instead.
> > 
> > This fixes the reset assertion meaning and moves out the polarity
> > configuration in DT (please note that there is no DT currently using
> > this driver).
> 
> The device tree bindings for this give an example configuration:
> 
>         pcie: pcie@f9800000 {
>                         compatible = "amlogic,axg-pcie", "snps,dw-pcie";
>                         reg = <0x0 0xf9800000 0x0 0x400000
>                                         0x0 0xff646000 0x0 0x2000
>                                         0x0 0xff644000 0x0 0x2000
>                                         0x0 0xf9f00000 0x0 0x100000>;
>                         reg-names = "elbi", "cfg", "phy", "config";
>                         reset-gpios = <&gpio GPIOX_19 GPIO_ACTIVE_HIGH>;
> 
> Is the 'reset-gpios' line still consistent with this change, or does
> this need to be updated as well?

Good catch, the polarity of the reset gpio will more likely be
GPIO_ACTIVE_LOW.

Do you want a separate patch for that or can I just include it in v2 ?

Thanks.

-- 
Remi

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

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

* Re: [PATCH] PCI: amlogic: Fix reset assertion via gpio descriptor
  2019-09-02 10:55   ` Andrew Murray
@ 2019-09-02 22:34     ` Martin Blumenstingl
  -1 siblings, 0 replies; 14+ messages in thread
From: Martin Blumenstingl @ 2019-09-02 22:34 UTC (permalink / raw)
  To: Andrew Murray
  Cc: Remi Pommarel, Lorenzo Pieralisi, Kevin Hilman, Yue Wang,
	linux-pci, Bjorn Helgaas, linux-amlogic

Hi Andrew,

On Mon, Sep 2, 2019 at 12:55 PM Andrew Murray <andrew.murray@arm.com> wrote:
>
> On Sun, Sep 01, 2019 at 03:39:15PM +0200, Remi Pommarel wrote:
> > Normally asserting reset signal on gpio would be achieved with:
> >       gpiod_set_value_cansleep(reset_gpio, 1);
> >
> > Meson PCI driver set reset value to '0' instead of '1' as it takes into
> > account the PERST# signal polarity. The polarity should be taken care
> > in the device tree instead.
> >
> > This fixes the reset assertion meaning and moves out the polarity
> > configuration in DT (please note that there is no DT currently using
> > this driver).
>
> The device tree bindings for this give an example configuration:
>
>         pcie: pcie@f9800000 {
>                         compatible = "amlogic,axg-pcie", "snps,dw-pcie";
>                         reg = <0x0 0xf9800000 0x0 0x400000
>                                         0x0 0xff646000 0x0 0x2000
>                                         0x0 0xff644000 0x0 0x2000
>                                         0x0 0xf9f00000 0x0 0x100000>;
>                         reg-names = "elbi", "cfg", "phy", "config";
>                         reset-gpios = <&gpio GPIOX_19 GPIO_ACTIVE_HIGH>;
>
> Is the 'reset-gpios' line still consistent with this change, or does
> this need to be updated as well?
in my opinion the example is still valid
whether GPIO_ACTIVE_HIGH or GPIO_ACTIVE_LOW is correct depends on the
actual circuit on the board:
- if the GPIO signal is directly connected to the PERST# line of the
PCIe card then above example is correct
- if the GPIO signal is inverted, for example by using a transistor,
then GPIO_ACTIVE_LOW should be used instead

I haven't looked into the schematics of the boards using a G12A or
G12B SoC (I don't have any schematics of a board with an AXG SoC) so I
can't tell what "most boards" use (active LOW or HIGH).
if there's a pattern in those board schematics (which is likely since
most are derived from Amlogics reference design) then we can update
the example based that.


Martin

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

* Re: [PATCH] PCI: amlogic: Fix reset assertion via gpio descriptor
@ 2019-09-02 22:34     ` Martin Blumenstingl
  0 siblings, 0 replies; 14+ messages in thread
From: Martin Blumenstingl @ 2019-09-02 22:34 UTC (permalink / raw)
  To: Andrew Murray
  Cc: Lorenzo Pieralisi, linux-pci, Yue Wang, Remi Pommarel,
	Kevin Hilman, Bjorn Helgaas, linux-amlogic

Hi Andrew,

On Mon, Sep 2, 2019 at 12:55 PM Andrew Murray <andrew.murray@arm.com> wrote:
>
> On Sun, Sep 01, 2019 at 03:39:15PM +0200, Remi Pommarel wrote:
> > Normally asserting reset signal on gpio would be achieved with:
> >       gpiod_set_value_cansleep(reset_gpio, 1);
> >
> > Meson PCI driver set reset value to '0' instead of '1' as it takes into
> > account the PERST# signal polarity. The polarity should be taken care
> > in the device tree instead.
> >
> > This fixes the reset assertion meaning and moves out the polarity
> > configuration in DT (please note that there is no DT currently using
> > this driver).
>
> The device tree bindings for this give an example configuration:
>
>         pcie: pcie@f9800000 {
>                         compatible = "amlogic,axg-pcie", "snps,dw-pcie";
>                         reg = <0x0 0xf9800000 0x0 0x400000
>                                         0x0 0xff646000 0x0 0x2000
>                                         0x0 0xff644000 0x0 0x2000
>                                         0x0 0xf9f00000 0x0 0x100000>;
>                         reg-names = "elbi", "cfg", "phy", "config";
>                         reset-gpios = <&gpio GPIOX_19 GPIO_ACTIVE_HIGH>;
>
> Is the 'reset-gpios' line still consistent with this change, or does
> this need to be updated as well?
in my opinion the example is still valid
whether GPIO_ACTIVE_HIGH or GPIO_ACTIVE_LOW is correct depends on the
actual circuit on the board:
- if the GPIO signal is directly connected to the PERST# line of the
PCIe card then above example is correct
- if the GPIO signal is inverted, for example by using a transistor,
then GPIO_ACTIVE_LOW should be used instead

I haven't looked into the schematics of the boards using a G12A or
G12B SoC (I don't have any schematics of a board with an AXG SoC) so I
can't tell what "most boards" use (active LOW or HIGH).
if there's a pattern in those board schematics (which is likely since
most are derived from Amlogics reference design) then we can update
the example based that.


Martin

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

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

* Re: [PATCH] PCI: amlogic: Fix reset assertion via gpio descriptor
  2019-09-01 13:39 ` Remi Pommarel
@ 2019-10-15 14:02   ` Lorenzo Pieralisi
  -1 siblings, 0 replies; 14+ messages in thread
From: Lorenzo Pieralisi @ 2019-10-15 14:02 UTC (permalink / raw)
  To: Remi Pommarel
  Cc: Yue Wang, Bjorn Helgaas, Kevin Hilman, linux-pci, linux-amlogic

On Sun, Sep 01, 2019 at 03:39:15PM +0200, Remi Pommarel wrote:
> Normally asserting reset signal on gpio would be achieved with:
> 	gpiod_set_value_cansleep(reset_gpio, 1);
> 
> Meson PCI driver set reset value to '0' instead of '1' as it takes into
> account the PERST# signal polarity. The polarity should be taken care
> in the device tree instead.
> 
> This fixes the reset assertion meaning and moves out the polarity
> configuration in DT (please note that there is no DT currently using
> this driver).
> 
> Signed-off-by: Remi Pommarel <repk@triplefau.lt>
> ---
>  drivers/pci/controller/dwc/pci-meson.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Applied to pci/meson, thanks.

Lorenzo

> diff --git a/drivers/pci/controller/dwc/pci-meson.c b/drivers/pci/controller/dwc/pci-meson.c
> index e35e9eaa50ee..541f37a6f6a5 100644
> --- a/drivers/pci/controller/dwc/pci-meson.c
> +++ b/drivers/pci/controller/dwc/pci-meson.c
> @@ -287,9 +287,9 @@ static inline void meson_cfg_writel(struct meson_pcie *mp, u32 val, u32 reg)
>  
>  static void meson_pcie_assert_reset(struct meson_pcie *mp)
>  {
> -	gpiod_set_value_cansleep(mp->reset_gpio, 0);
> -	udelay(500);
>  	gpiod_set_value_cansleep(mp->reset_gpio, 1);
> +	udelay(500);
> +	gpiod_set_value_cansleep(mp->reset_gpio, 0);
>  }
>  
>  static void meson_pcie_init_dw(struct meson_pcie *mp)
> -- 
> 2.20.1
> 

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

* Re: [PATCH] PCI: amlogic: Fix reset assertion via gpio descriptor
@ 2019-10-15 14:02   ` Lorenzo Pieralisi
  0 siblings, 0 replies; 14+ messages in thread
From: Lorenzo Pieralisi @ 2019-10-15 14:02 UTC (permalink / raw)
  To: Remi Pommarel
  Cc: Bjorn Helgaas, Kevin Hilman, linux-amlogic, linux-pci, Yue Wang

On Sun, Sep 01, 2019 at 03:39:15PM +0200, Remi Pommarel wrote:
> Normally asserting reset signal on gpio would be achieved with:
> 	gpiod_set_value_cansleep(reset_gpio, 1);
> 
> Meson PCI driver set reset value to '0' instead of '1' as it takes into
> account the PERST# signal polarity. The polarity should be taken care
> in the device tree instead.
> 
> This fixes the reset assertion meaning and moves out the polarity
> configuration in DT (please note that there is no DT currently using
> this driver).
> 
> Signed-off-by: Remi Pommarel <repk@triplefau.lt>
> ---
>  drivers/pci/controller/dwc/pci-meson.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Applied to pci/meson, thanks.

Lorenzo

> diff --git a/drivers/pci/controller/dwc/pci-meson.c b/drivers/pci/controller/dwc/pci-meson.c
> index e35e9eaa50ee..541f37a6f6a5 100644
> --- a/drivers/pci/controller/dwc/pci-meson.c
> +++ b/drivers/pci/controller/dwc/pci-meson.c
> @@ -287,9 +287,9 @@ static inline void meson_cfg_writel(struct meson_pcie *mp, u32 val, u32 reg)
>  
>  static void meson_pcie_assert_reset(struct meson_pcie *mp)
>  {
> -	gpiod_set_value_cansleep(mp->reset_gpio, 0);
> -	udelay(500);
>  	gpiod_set_value_cansleep(mp->reset_gpio, 1);
> +	udelay(500);
> +	gpiod_set_value_cansleep(mp->reset_gpio, 0);
>  }
>  
>  static void meson_pcie_init_dw(struct meson_pcie *mp)
> -- 
> 2.20.1
> 

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

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

end of thread, other threads:[~2019-10-15 14:02 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-01 13:39 [PATCH] PCI: amlogic: Fix reset assertion via gpio descriptor Remi Pommarel
2019-09-01 13:39 ` Remi Pommarel
2019-09-01 21:46 ` Martin Blumenstingl
2019-09-01 21:46   ` Martin Blumenstingl
2019-09-02  8:11 ` Neil Armstrong
2019-09-02  8:11   ` Neil Armstrong
2019-09-02 10:55 ` Andrew Murray
2019-09-02 10:55   ` Andrew Murray
2019-09-02 14:43   ` Remi Pommarel
2019-09-02 14:43     ` Remi Pommarel
2019-09-02 22:34   ` Martin Blumenstingl
2019-09-02 22:34     ` Martin Blumenstingl
2019-10-15 14:02 ` Lorenzo Pieralisi
2019-10-15 14:02   ` 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.