[PATCH 1/2] arm64: a37xx: pci: Make PCIe Reset GPIO DT compatible with Linux kernel DT

[PATCH 1/2] arm64: a37xx: pci: Make PCIe Reset GPIO DT compatible with Linux kernel DT

[Pali Rohár]

Change active-high to active-low and change DT property name from
reset-gpio to reset-gpios. This format of gpio reset is used by
pci-aardvark driver in Linux kernel.

Signed-off-by: Pali Roh?r <pali@kernel.org>
---
 arch/arm/dts/armada-3720-db.dts          | 2 +-
 arch/arm/dts/armada-3720-espressobin.dts | 2 +-
 arch/arm/dts/armada-3720-turris-mox.dts  | 2 +-
 drivers/pci/pci-aardvark.c               | 6 +++---
 4 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/arm/dts/armada-3720-db.dts b/arch/arm/dts/armada-3720-db.dts
index 1b219c423b..1b1b66b94d 100644
--- a/arch/arm/dts/armada-3720-db.dts
+++ b/arch/arm/dts/armada-3720-db.dts
@@ -159,6 +159,6 @@
 &pcie0 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pcie_pins>;
-	reset-gpio = <&gpiosb 3 GPIO_ACTIVE_HIGH>;
+	reset-gpios = <&gpiosb 3 GPIO_ACTIVE_LOW>;
 	status = "okay";
 };
diff --git a/arch/arm/dts/armada-3720-espressobin.dts b/arch/arm/dts/armada-3720-espressobin.dts
index 84e2c2adba..f10a953ec5 100644
--- a/arch/arm/dts/armada-3720-espressobin.dts
+++ b/arch/arm/dts/armada-3720-espressobin.dts
@@ -145,6 +145,6 @@
 &pcie0 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pcie_pins>;
-	reset-gpio = <&gpiosb 3 GPIO_ACTIVE_HIGH>;
+	reset-gpios = <&gpiosb 3 GPIO_ACTIVE_LOW>;
 	status = "okay";
 };
diff --git a/arch/arm/dts/armada-3720-turris-mox.dts b/arch/arm/dts/armada-3720-turris-mox.dts
index 0f0a6ce65d..974270cc8c 100644
--- a/arch/arm/dts/armada-3720-turris-mox.dts
+++ b/arch/arm/dts/armada-3720-turris-mox.dts
@@ -172,6 +172,6 @@
 &pcie0 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pcie_pins>;
-	reset-gpio = <&gpiosb 3 GPIO_ACTIVE_HIGH>;
+	reset-gpios = <&gpiosb 3 GPIO_ACTIVE_LOW>;
 	status = "disabled";
 };
diff --git a/drivers/pci/pci-aardvark.c b/drivers/pci/pci-aardvark.c
index 711b930d0f..5b3f23c184 100644
--- a/drivers/pci/pci-aardvark.c
+++ b/drivers/pci/pci-aardvark.c
@@ -616,7 +616,7 @@ static int pcie_advk_probe(struct udevice *dev)
 #if CONFIG_IS_ENABLED(DM_GPIO)
 	struct gpio_desc reset_gpio;
 
-	gpio_request_by_name(dev, "reset-gpio", 0, &reset_gpio,
+	gpio_request_by_name(dev, "reset-gpios", 0, &reset_gpio,
 			     GPIOD_IS_OUT);
 	/*
 	 * Issue reset to add-in card through the dedicated GPIO.
@@ -633,9 +633,9 @@ static int pcie_advk_probe(struct udevice *dev)
 	 */
 	if (dm_gpio_is_valid(&reset_gpio)) {
 		dev_dbg(pcie->dev, "Toggle PCIE Reset GPIO ...\n");
-		dm_gpio_set_value(&reset_gpio, 0);
-		mdelay(200);
 		dm_gpio_set_value(&reset_gpio, 1);
+		mdelay(200);
+		dm_gpio_set_value(&reset_gpio, 0);
 	}
 #else
 	dev_dbg(pcie->dev, "PCIE Reset on GPIO support is missing\n");
-- 
2.20.1

[PATCH 2/2] arm64: a37xx: pci: Assert PERST# signal when unloading driver

[Pali Rohár]

This change ensures that PCIe card is put into reset state when U-Boot
stops using it.

DM_FLAG_OS_PREPARE ensures that U-Boot executes driver's remove callback
prior booting Linux kernel.

Linux kernel pci-aardvark driver needs to reset PCIe card via PERST# signal
prior initializing it. If it does not issue reset then some PCIe cards
(specially Compex WiFi cards) are not detected at all.

Putting PCIe card into reset state prior booting Linux kernel would ensure
that card would be properly reset at time when Linux kernel starts
initializing pci-aardvark driver.

Signed-off-by: Pali Roh?r <pali@kernel.org>
---
 drivers/pci/pci-aardvark.c | 27 +++++++++++++++++++++------
 1 file changed, 21 insertions(+), 6 deletions(-)

diff --git a/drivers/pci/pci-aardvark.c b/drivers/pci/pci-aardvark.c
index 5b3f23c184..8996be5309 100644
--- a/drivers/pci/pci-aardvark.c
+++ b/drivers/pci/pci-aardvark.c
@@ -148,6 +148,9 @@ struct pcie_advk {
 	void           *base;
 	int            first_busno;
 	struct udevice *dev;
+#if CONFIG_IS_ENABLED(DM_GPIO)
+	struct gpio_desc reset_gpio;
+#endif
 };
 
 static inline void advk_writel(struct pcie_advk *pcie, uint val, uint reg)
@@ -614,9 +617,7 @@ static int pcie_advk_probe(struct udevice *dev)
 	struct pcie_advk *pcie = dev_get_priv(dev);
 
 #if CONFIG_IS_ENABLED(DM_GPIO)
-	struct gpio_desc reset_gpio;
-
-	gpio_request_by_name(dev, "reset-gpios", 0, &reset_gpio,
+	gpio_request_by_name(dev, "reset-gpios", 0, &pcie->reset_gpio,
 			     GPIOD_IS_OUT);
 	/*
 	 * Issue reset to add-in card through the dedicated GPIO.
@@ -631,11 +632,11 @@ static int pcie_advk_probe(struct udevice *dev)
 	 *     possible before PCIe PHY initialization. Moreover, the PCIe
 	 *     clock should be gated as well.
 	 */
-	if (dm_gpio_is_valid(&reset_gpio)) {
+	if (dm_gpio_is_valid(&pcie->reset_gpio)) {
 		dev_dbg(pcie->dev, "Toggle PCIE Reset GPIO ...\n");
-		dm_gpio_set_value(&reset_gpio, 1);
+		dm_gpio_set_value(&pcie->reset_gpio, 1);
 		mdelay(200);
-		dm_gpio_set_value(&reset_gpio, 0);
+		dm_gpio_set_value(&pcie->reset_gpio, 0);
 	}
 #else
 	dev_dbg(pcie->dev, "PCIE Reset on GPIO support is missing\n");
@@ -647,6 +648,18 @@ static int pcie_advk_probe(struct udevice *dev)
 	return pcie_advk_setup_hw(pcie);
 }
 
+static int pcie_advk_remove(struct udevice *dev)
+{
+#if CONFIG_IS_ENABLED(DM_GPIO)
+	struct pcie_advk *pcie = dev_get_priv(dev);
+
+	if (dm_gpio_is_valid(&pcie->reset_gpio))
+		dm_gpio_set_value(&pcie->reset_gpio, 1);
+#endif /* DM_GPIO */
+
+	return 0;
+}
+
 /**
  * pcie_advk_ofdata_to_platdata() - Translate from DT to device state
  *
@@ -687,5 +700,7 @@ U_BOOT_DRIVER(pcie_advk) = {
 	.ops			= &pcie_advk_ops,
 	.ofdata_to_platdata	= pcie_advk_ofdata_to_platdata,
 	.probe			= pcie_advk_probe,
+	.remove			= pcie_advk_remove,
+	.flags			= DM_FLAG_OS_PREPARE,
 	.priv_auto_alloc_size	= sizeof(struct pcie_advk),
 };
-- 
2.20.1

[EXT] [PATCH 1/2] arm64: a37xx: pci: Make PCIe Reset GPIO DT compatible with Linux kernel DT

[Kostya Porotchkin]

________________________________________
From: Pali Roh?r <pali@kernel.org>
Sent: Wednesday, August 19, 2020 16:57
To: Stefan Roese; Kostya Porotchkin
Cc: u-boot at lists.denx.de
Subject: [EXT] [PATCH 1/2] arm64: a37xx: pci: Make PCIe Reset GPIO DT compatible with Linux kernel DT

External Email

----------------------------------------------------------------------
Change active-high to active-low and change DT property name from
reset-gpio to reset-gpios. This format of gpio reset is used by
pci-aardvark driver in Linux kernel.

Signed-off-by: Pali Roh?r <pali@kernel.org>
Reviewed-by: Konstantin Porotchkin <kostap@marvell.com>
---
 arch/arm/dts/armada-3720-db.dts          | 2 +-
 arch/arm/dts/armada-3720-espressobin.dts | 2 +-
 arch/arm/dts/armada-3720-turris-mox.dts  | 2 +-
 drivers/pci/pci-aardvark.c               | 6 +++---
 4 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/arm/dts/armada-3720-db.dts b/arch/arm/dts/armada-3720-db.dts
index 1b219c423b..1b1b66b94d 100644
--- a/arch/arm/dts/armada-3720-db.dts
+++ b/arch/arm/dts/armada-3720-db.dts
@@ -159,6 +159,6 @@
 &pcie0 {
        pinctrl-names = "default";
        pinctrl-0 = <&pcie_pins>;
-       reset-gpio = <&gpiosb 3 GPIO_ACTIVE_HIGH>;
+       reset-gpios = <&gpiosb 3 GPIO_ACTIVE_LOW>;
        status = "okay";
 };
diff --git a/arch/arm/dts/armada-3720-espressobin.dts b/arch/arm/dts/armada-3720-espressobin.dts
index 84e2c2adba..f10a953ec5 100644
--- a/arch/arm/dts/armada-3720-espressobin.dts
+++ b/arch/arm/dts/armada-3720-espressobin.dts
@@ -145,6 +145,6 @@
 &pcie0 {
        pinctrl-names = "default";
        pinctrl-0 = <&pcie_pins>;
-       reset-gpio = <&gpiosb 3 GPIO_ACTIVE_HIGH>;
+       reset-gpios = <&gpiosb 3 GPIO_ACTIVE_LOW>;
        status = "okay";
 };
diff --git a/arch/arm/dts/armada-3720-turris-mox.dts b/arch/arm/dts/armada-3720-turris-mox.dts
index 0f0a6ce65d..974270cc8c 100644
--- a/arch/arm/dts/armada-3720-turris-mox.dts
+++ b/arch/arm/dts/armada-3720-turris-mox.dts
@@ -172,6 +172,6 @@
 &pcie0 {
        pinctrl-names = "default";
        pinctrl-0 = <&pcie_pins>;
-       reset-gpio = <&gpiosb 3 GPIO_ACTIVE_HIGH>;
+       reset-gpios = <&gpiosb 3 GPIO_ACTIVE_LOW>;
        status = "disabled";
 };
diff --git a/drivers/pci/pci-aardvark.c b/drivers/pci/pci-aardvark.c
index 711b930d0f..5b3f23c184 100644
--- a/drivers/pci/pci-aardvark.c
+++ b/drivers/pci/pci-aardvark.c
@@ -616,7 +616,7 @@ static int pcie_advk_probe(struct udevice *dev)
 #if CONFIG_IS_ENABLED(DM_GPIO)
        struct gpio_desc reset_gpio;

-       gpio_request_by_name(dev, "reset-gpio", 0, &reset_gpio,
+       gpio_request_by_name(dev, "reset-gpios", 0, &reset_gpio,
                             GPIOD_IS_OUT);
        /*
         * Issue reset to add-in card through the dedicated GPIO.
@@ -633,9 +633,9 @@ static int pcie_advk_probe(struct udevice *dev)
         */
        if (dm_gpio_is_valid(&reset_gpio)) {
                dev_dbg(pcie->dev, "Toggle PCIE Reset GPIO ...\n");
-               dm_gpio_set_value(&reset_gpio, 0);
-               mdelay(200);
                dm_gpio_set_value(&reset_gpio, 1);
+               mdelay(200);
+               dm_gpio_set_value(&reset_gpio, 0);
        }
 #else
        dev_dbg(pcie->dev, "PCIE Reset on GPIO support is missing\n");
--
2.20.1

[EXT] [PATCH 2/2] arm64: a37xx: pci: Assert PERST# signal when unloading driver

[Kostya Porotchkin]

________________________________________
From: Pali Roh?r <pali@kernel.org>
Sent: Wednesday, August 19, 2020 16:57
To: Stefan Roese; Kostya Porotchkin
Cc: u-boot at lists.denx.de
Subject: [EXT] [PATCH 2/2] arm64: a37xx: pci: Assert PERST# signal when unloading driver

External Email

----------------------------------------------------------------------
This change ensures that PCIe card is put into reset state when U-Boot
stops using it.

DM_FLAG_OS_PREPARE ensures that U-Boot executes driver's remove callback
prior booting Linux kernel.

Linux kernel pci-aardvark driver needs to reset PCIe card via PERST# signal
prior initializing it. If it does not issue reset then some PCIe cards
(specially Compex WiFi cards) are not detected at all.

Putting PCIe card into reset state prior booting Linux kernel would ensure
that card would be properly reset at time when Linux kernel starts
initializing pci-aardvark driver.

Signed-off-by: Pali Roh?r <pali@kernel.org>
Reviewed-by: Konstantin Porotchkin <kostap@marvell.com>
---
 drivers/pci/pci-aardvark.c | 27 +++++++++++++++++++++------
 1 file changed, 21 insertions(+), 6 deletions(-)

diff --git a/drivers/pci/pci-aardvark.c b/drivers/pci/pci-aardvark.c
index 5b3f23c184..8996be5309 100644
--- a/drivers/pci/pci-aardvark.c
+++ b/drivers/pci/pci-aardvark.c
@@ -148,6 +148,9 @@ struct pcie_advk {
        void           *base;
        int            first_busno;
        struct udevice *dev;
+#if CONFIG_IS_ENABLED(DM_GPIO)
+       struct gpio_desc reset_gpio;
+#endif
 };

 static inline void advk_writel(struct pcie_advk *pcie, uint val, uint reg)
@@ -614,9 +617,7 @@ static int pcie_advk_probe(struct udevice *dev)
        struct pcie_advk *pcie = dev_get_priv(dev);

 #if CONFIG_IS_ENABLED(DM_GPIO)
-       struct gpio_desc reset_gpio;
-
-       gpio_request_by_name(dev, "reset-gpios", 0, &reset_gpio,
+       gpio_request_by_name(dev, "reset-gpios", 0, &pcie->reset_gpio,
                             GPIOD_IS_OUT);
        /*
         * Issue reset to add-in card through the dedicated GPIO.
@@ -631,11 +632,11 @@ static int pcie_advk_probe(struct udevice *dev)
         *     possible before PCIe PHY initialization. Moreover, the PCIe
         *     clock should be gated as well.
         */
-       if (dm_gpio_is_valid(&reset_gpio)) {
+       if (dm_gpio_is_valid(&pcie->reset_gpio)) {
                dev_dbg(pcie->dev, "Toggle PCIE Reset GPIO ...\n");
-               dm_gpio_set_value(&reset_gpio, 1);
+               dm_gpio_set_value(&pcie->reset_gpio, 1);
                mdelay(200);
-               dm_gpio_set_value(&reset_gpio, 0);
+               dm_gpio_set_value(&pcie->reset_gpio, 0);
        }
 #else
        dev_dbg(pcie->dev, "PCIE Reset on GPIO support is missing\n");
@@ -647,6 +648,18 @@ static int pcie_advk_probe(struct udevice *dev)
        return pcie_advk_setup_hw(pcie);
 }

+static int pcie_advk_remove(struct udevice *dev)
+{
+#if CONFIG_IS_ENABLED(DM_GPIO)
+       struct pcie_advk *pcie = dev_get_priv(dev);
+
+       if (dm_gpio_is_valid(&pcie->reset_gpio))
+               dm_gpio_set_value(&pcie->reset_gpio, 1);
+#endif /* DM_GPIO */
+
+       return 0;
+}
+
 /**
  * pcie_advk_ofdata_to_platdata() - Translate from DT to device state
  *
@@ -687,5 +700,7 @@ U_BOOT_DRIVER(pcie_advk) = {
        .ops                    = &pcie_advk_ops,
        .ofdata_to_platdata     = pcie_advk_ofdata_to_platdata,
        .probe                  = pcie_advk_probe,
+       .remove                 = pcie_advk_remove,
+       .flags                  = DM_FLAG_OS_PREPARE,
        .priv_auto_alloc_size   = sizeof(struct pcie_advk),
 };
--
2.20.1

[PATCH 1/2] arm64: a37xx: pci: Make PCIe Reset GPIO DT compatible with Linux kernel DT

[Stefan Roese]

On 19.08.20 15:57, Pali Roh?r wrote:
> Change active-high to active-low and change DT property name from
> reset-gpio to reset-gpios. This format of gpio reset is used by
> pci-aardvark driver in Linux kernel.
> 
> Signed-off-by: Pali Roh?r <pali@kernel.org>

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

> ---
>   arch/arm/dts/armada-3720-db.dts          | 2 +-
>   arch/arm/dts/armada-3720-espressobin.dts | 2 +-
>   arch/arm/dts/armada-3720-turris-mox.dts  | 2 +-
>   drivers/pci/pci-aardvark.c               | 6 +++---
>   4 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm/dts/armada-3720-db.dts b/arch/arm/dts/armada-3720-db.dts
> index 1b219c423b..1b1b66b94d 100644
> --- a/arch/arm/dts/armada-3720-db.dts
> +++ b/arch/arm/dts/armada-3720-db.dts
> @@ -159,6 +159,6 @@
>   &pcie0 {
>   	pinctrl-names = "default";
>   	pinctrl-0 = <&pcie_pins>;
> -	reset-gpio = <&gpiosb 3 GPIO_ACTIVE_HIGH>;
> +	reset-gpios = <&gpiosb 3 GPIO_ACTIVE_LOW>;
>   	status = "okay";
>   };
> diff --git a/arch/arm/dts/armada-3720-espressobin.dts b/arch/arm/dts/armada-3720-espressobin.dts
> index 84e2c2adba..f10a953ec5 100644
> --- a/arch/arm/dts/armada-3720-espressobin.dts
> +++ b/arch/arm/dts/armada-3720-espressobin.dts
> @@ -145,6 +145,6 @@
>   &pcie0 {
>   	pinctrl-names = "default";
>   	pinctrl-0 = <&pcie_pins>;
> -	reset-gpio = <&gpiosb 3 GPIO_ACTIVE_HIGH>;
> +	reset-gpios = <&gpiosb 3 GPIO_ACTIVE_LOW>;
>   	status = "okay";
>   };
> diff --git a/arch/arm/dts/armada-3720-turris-mox.dts b/arch/arm/dts/armada-3720-turris-mox.dts
> index 0f0a6ce65d..974270cc8c 100644
> --- a/arch/arm/dts/armada-3720-turris-mox.dts
> +++ b/arch/arm/dts/armada-3720-turris-mox.dts
> @@ -172,6 +172,6 @@
>   &pcie0 {
>   	pinctrl-names = "default";
>   	pinctrl-0 = <&pcie_pins>;
> -	reset-gpio = <&gpiosb 3 GPIO_ACTIVE_HIGH>;
> +	reset-gpios = <&gpiosb 3 GPIO_ACTIVE_LOW>;
>   	status = "disabled";
>   };
> diff --git a/drivers/pci/pci-aardvark.c b/drivers/pci/pci-aardvark.c
> index 711b930d0f..5b3f23c184 100644
> --- a/drivers/pci/pci-aardvark.c
> +++ b/drivers/pci/pci-aardvark.c
> @@ -616,7 +616,7 @@ static int pcie_advk_probe(struct udevice *dev)
>   #if CONFIG_IS_ENABLED(DM_GPIO)
>   	struct gpio_desc reset_gpio;
>   
> -	gpio_request_by_name(dev, "reset-gpio", 0, &reset_gpio,
> +	gpio_request_by_name(dev, "reset-gpios", 0, &reset_gpio,
>   			     GPIOD_IS_OUT);
>   	/*
>   	 * Issue reset to add-in card through the dedicated GPIO.
> @@ -633,9 +633,9 @@ static int pcie_advk_probe(struct udevice *dev)
>   	 */
>   	if (dm_gpio_is_valid(&reset_gpio)) {
>   		dev_dbg(pcie->dev, "Toggle PCIE Reset GPIO ...\n");
> -		dm_gpio_set_value(&reset_gpio, 0);
> -		mdelay(200);
>   		dm_gpio_set_value(&reset_gpio, 1);
> +		mdelay(200);
> +		dm_gpio_set_value(&reset_gpio, 0);
>   	}
>   #else
>   	dev_dbg(pcie->dev, "PCIE Reset on GPIO support is missing\n");
> 


Viele Gr??e,
Stefan

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de

[PATCH 2/2] arm64: a37xx: pci: Assert PERST# signal when unloading driver

[Stefan Roese]

On 19.08.20 15:57, Pali Roh?r wrote:
> This change ensures that PCIe card is put into reset state when U-Boot
> stops using it.
> 
> DM_FLAG_OS_PREPARE ensures that U-Boot executes driver's remove callback
> prior booting Linux kernel.
> 
> Linux kernel pci-aardvark driver needs to reset PCIe card via PERST# signal
> prior initializing it. If it does not issue reset then some PCIe cards
> (specially Compex WiFi cards) are not detected at all.
> 
> Putting PCIe card into reset state prior booting Linux kernel would ensure
> that card would be properly reset at time when Linux kernel starts
> initializing pci-aardvark driver.
> 
> Signed-off-by: Pali Roh?r <pali@kernel.org>
> ---
>   drivers/pci/pci-aardvark.c | 27 +++++++++++++++++++++------
>   1 file changed, 21 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/pci/pci-aardvark.c b/drivers/pci/pci-aardvark.c
> index 5b3f23c184..8996be5309 100644
> --- a/drivers/pci/pci-aardvark.c
> +++ b/drivers/pci/pci-aardvark.c
> @@ -148,6 +148,9 @@ struct pcie_advk {
>   	void           *base;
>   	int            first_busno;
>   	struct udevice *dev;
> +#if CONFIG_IS_ENABLED(DM_GPIO)
> +	struct gpio_desc reset_gpio;
> +#endif
>   };

Adding more #ifdef's is not recommended. Can't you "depend" this driver
on DM_GPIO in Kconfig instead? Are there any used that don't support
DM_GPIO right now?

>   static inline void advk_writel(struct pcie_advk *pcie, uint val, uint reg)
> @@ -614,9 +617,7 @@ static int pcie_advk_probe(struct udevice *dev)
>   	struct pcie_advk *pcie = dev_get_priv(dev);
>   
>   #if CONFIG_IS_ENABLED(DM_GPIO)

If you change the driver to rely on DM_GPIO, then please also remove all
other #ifdef's with DM_GPIO.

Thanks,
Stefan

> -	struct gpio_desc reset_gpio;
> -
> -	gpio_request_by_name(dev, "reset-gpios", 0, &reset_gpio,
> +	gpio_request_by_name(dev, "reset-gpios", 0, &pcie->reset_gpio,
>   			     GPIOD_IS_OUT);
>   	/*
>   	 * Issue reset to add-in card through the dedicated GPIO.
> @@ -631,11 +632,11 @@ static int pcie_advk_probe(struct udevice *dev)
>   	 *     possible before PCIe PHY initialization. Moreover, the PCIe
>   	 *     clock should be gated as well.
>   	 */
> -	if (dm_gpio_is_valid(&reset_gpio)) {
> +	if (dm_gpio_is_valid(&pcie->reset_gpio)) {
>   		dev_dbg(pcie->dev, "Toggle PCIE Reset GPIO ...\n");
> -		dm_gpio_set_value(&reset_gpio, 1);
> +		dm_gpio_set_value(&pcie->reset_gpio, 1);
>   		mdelay(200);
> -		dm_gpio_set_value(&reset_gpio, 0);
> +		dm_gpio_set_value(&pcie->reset_gpio, 0);
>   	}
>   #else
>   	dev_dbg(pcie->dev, "PCIE Reset on GPIO support is missing\n");
> @@ -647,6 +648,18 @@ static int pcie_advk_probe(struct udevice *dev)
>   	return pcie_advk_setup_hw(pcie);
>   }
>   
> +static int pcie_advk_remove(struct udevice *dev)
> +{
> +#if CONFIG_IS_ENABLED(DM_GPIO)
> +	struct pcie_advk *pcie = dev_get_priv(dev);
> +
> +	if (dm_gpio_is_valid(&pcie->reset_gpio))
> +		dm_gpio_set_value(&pcie->reset_gpio, 1);
> +#endif /* DM_GPIO */
> +
> +	return 0;
> +}
> +
>   /**
>    * pcie_advk_ofdata_to_platdata() - Translate from DT to device state
>    *
> @@ -687,5 +700,7 @@ U_BOOT_DRIVER(pcie_advk) = {
>   	.ops			= &pcie_advk_ops,
>   	.ofdata_to_platdata	= pcie_advk_ofdata_to_platdata,
>   	.probe			= pcie_advk_probe,
> +	.remove			= pcie_advk_remove,
> +	.flags			= DM_FLAG_OS_PREPARE,
>   	.priv_auto_alloc_size	= sizeof(struct pcie_advk),
>   };
> 


Viele Gr??e,
Stefan

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de

[EXT] [PATCH 1/2] arm64: a37xx: pci: Make PCIe Reset GPIO DT compatible with Linux kernel DT

[Stefan Roese]

Hi Kosta,

AFAICT, this mail does not contain any info from you. I can only
assume, that you wanted to send a Reviewed-by tag. If yes, then
please double-check which mails did not work and resend them with
the correct tags. As they are automatically collected by patchwork.

Thanks,
Stefan

On 19.08.20 19:28, Kostya Porotchkin wrote:
> 
> 
> ________________________________________
> From: Pali Roh?r <pali@kernel.org>
> Sent: Wednesday, August 19, 2020 16:57
> To: Stefan Roese; Kostya Porotchkin
> Cc: u-boot at lists.denx.de
> Subject: [EXT] [PATCH 1/2] arm64: a37xx: pci: Make PCIe Reset GPIO DT compatible with Linux kernel DT
> 
> External Email
> 
> ----------------------------------------------------------------------
> Change active-high to active-low and change DT property name from
> reset-gpio to reset-gpios. This format of gpio reset is used by
> pci-aardvark driver in Linux kernel.
> 
> Signed-off-by: Pali Roh?r <pali@kernel.org>
> Reviewed-by: Konstantin Porotchkin <kostap@marvell.com>
> ---
>   arch/arm/dts/armada-3720-db.dts          | 2 +-
>   arch/arm/dts/armada-3720-espressobin.dts | 2 +-
>   arch/arm/dts/armada-3720-turris-mox.dts  | 2 +-
>   drivers/pci/pci-aardvark.c               | 6 +++---
>   4 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm/dts/armada-3720-db.dts b/arch/arm/dts/armada-3720-db.dts
> index 1b219c423b..1b1b66b94d 100644
> --- a/arch/arm/dts/armada-3720-db.dts
> +++ b/arch/arm/dts/armada-3720-db.dts
> @@ -159,6 +159,6 @@
>   &pcie0 {
>          pinctrl-names = "default";
>          pinctrl-0 = <&pcie_pins>;
> -       reset-gpio = <&gpiosb 3 GPIO_ACTIVE_HIGH>;
> +       reset-gpios = <&gpiosb 3 GPIO_ACTIVE_LOW>;
>          status = "okay";
>   };
> diff --git a/arch/arm/dts/armada-3720-espressobin.dts b/arch/arm/dts/armada-3720-espressobin.dts
> index 84e2c2adba..f10a953ec5 100644
> --- a/arch/arm/dts/armada-3720-espressobin.dts
> +++ b/arch/arm/dts/armada-3720-espressobin.dts
> @@ -145,6 +145,6 @@
>   &pcie0 {
>          pinctrl-names = "default";
>          pinctrl-0 = <&pcie_pins>;
> -       reset-gpio = <&gpiosb 3 GPIO_ACTIVE_HIGH>;
> +       reset-gpios = <&gpiosb 3 GPIO_ACTIVE_LOW>;
>          status = "okay";
>   };
> diff --git a/arch/arm/dts/armada-3720-turris-mox.dts b/arch/arm/dts/armada-3720-turris-mox.dts
> index 0f0a6ce65d..974270cc8c 100644
> --- a/arch/arm/dts/armada-3720-turris-mox.dts
> +++ b/arch/arm/dts/armada-3720-turris-mox.dts
> @@ -172,6 +172,6 @@
>   &pcie0 {
>          pinctrl-names = "default";
>          pinctrl-0 = <&pcie_pins>;
> -       reset-gpio = <&gpiosb 3 GPIO_ACTIVE_HIGH>;
> +       reset-gpios = <&gpiosb 3 GPIO_ACTIVE_LOW>;
>          status = "disabled";
>   };
> diff --git a/drivers/pci/pci-aardvark.c b/drivers/pci/pci-aardvark.c
> index 711b930d0f..5b3f23c184 100644
> --- a/drivers/pci/pci-aardvark.c
> +++ b/drivers/pci/pci-aardvark.c
> @@ -616,7 +616,7 @@ static int pcie_advk_probe(struct udevice *dev)
>   #if CONFIG_IS_ENABLED(DM_GPIO)
>          struct gpio_desc reset_gpio;
> 
> -       gpio_request_by_name(dev, "reset-gpio", 0, &reset_gpio,
> +       gpio_request_by_name(dev, "reset-gpios", 0, &reset_gpio,
>                               GPIOD_IS_OUT);
>          /*
>           * Issue reset to add-in card through the dedicated GPIO.
> @@ -633,9 +633,9 @@ static int pcie_advk_probe(struct udevice *dev)
>           */
>          if (dm_gpio_is_valid(&reset_gpio)) {
>                  dev_dbg(pcie->dev, "Toggle PCIE Reset GPIO ...\n");
> -               dm_gpio_set_value(&reset_gpio, 0);
> -               mdelay(200);
>                  dm_gpio_set_value(&reset_gpio, 1);
> +               mdelay(200);
> +               dm_gpio_set_value(&reset_gpio, 0);
>          }
>   #else
>          dev_dbg(pcie->dev, "PCIE Reset on GPIO support is missing\n");
> --
> 2.20.1
> 


Viele Gr??e,
Stefan

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de

[EXT] [PATCH 1/2] arm64: a37xx: pci: Make PCIe Reset GPIO DT compatible with Linux kernel DT

[Kostya Porotchkin]

> -----Original Message-----
> From: Pali Roh?r <pali@kernel.org>
> Sent: Wednesday, August 19, 2020 16:57
> To: Stefan Roese <sr@denx.de>; Kostya Porotchkin <kostap@marvell.com>
> Cc: u-boot at lists.denx.de
> Subject: [EXT] [PATCH 1/2] arm64: a37xx: pci: Make PCIe Reset GPIO DT
> compatible with Linux kernel DT
> 
> External Email
> 
> ----------------------------------------------------------------------
> Change active-high to active-low and change DT property name from reset-
> gpio to reset-gpios. This format of gpio reset is used by pci-aardvark driver in
> Linux kernel.
> 
> Signed-off-by: Pali Roh?r <pali@kernel.org>
Reviewed-by: Konstantin Porotchkin <kostap@marvell.com>

> ---
>  arch/arm/dts/armada-3720-db.dts          | 2 +-
>  arch/arm/dts/armada-3720-espressobin.dts | 2 +-  arch/arm/dts/armada-
> 3720-turris-mox.dts  | 2 +-
>  drivers/pci/pci-aardvark.c               | 6 +++---
>  4 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm/dts/armada-3720-db.dts b/arch/arm/dts/armada-3720-
> db.dts index 1b219c423b..1b1b66b94d 100644
> --- a/arch/arm/dts/armada-3720-db.dts
> +++ b/arch/arm/dts/armada-3720-db.dts
> @@ -159,6 +159,6 @@
>  &pcie0 {
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&pcie_pins>;
> -	reset-gpio = <&gpiosb 3 GPIO_ACTIVE_HIGH>;
> +	reset-gpios = <&gpiosb 3 GPIO_ACTIVE_LOW>;
>  	status = "okay";
>  };
> diff --git a/arch/arm/dts/armada-3720-espressobin.dts
> b/arch/arm/dts/armada-3720-espressobin.dts
> index 84e2c2adba..f10a953ec5 100644
> --- a/arch/arm/dts/armada-3720-espressobin.dts
> +++ b/arch/arm/dts/armada-3720-espressobin.dts
> @@ -145,6 +145,6 @@
>  &pcie0 {
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&pcie_pins>;
> -	reset-gpio = <&gpiosb 3 GPIO_ACTIVE_HIGH>;
> +	reset-gpios = <&gpiosb 3 GPIO_ACTIVE_LOW>;
>  	status = "okay";
>  };
> diff --git a/arch/arm/dts/armada-3720-turris-mox.dts
> b/arch/arm/dts/armada-3720-turris-mox.dts
> index 0f0a6ce65d..974270cc8c 100644
> --- a/arch/arm/dts/armada-3720-turris-mox.dts
> +++ b/arch/arm/dts/armada-3720-turris-mox.dts
> @@ -172,6 +172,6 @@
>  &pcie0 {
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&pcie_pins>;
> -	reset-gpio = <&gpiosb 3 GPIO_ACTIVE_HIGH>;
> +	reset-gpios = <&gpiosb 3 GPIO_ACTIVE_LOW>;
>  	status = "disabled";
>  };
> diff --git a/drivers/pci/pci-aardvark.c b/drivers/pci/pci-aardvark.c index
> 711b930d0f..5b3f23c184 100644
> --- a/drivers/pci/pci-aardvark.c
> +++ b/drivers/pci/pci-aardvark.c
> @@ -616,7 +616,7 @@ static int pcie_advk_probe(struct udevice *dev)  #if
> CONFIG_IS_ENABLED(DM_GPIO)
>  	struct gpio_desc reset_gpio;
> 
> -	gpio_request_by_name(dev, "reset-gpio", 0, &reset_gpio,
> +	gpio_request_by_name(dev, "reset-gpios", 0, &reset_gpio,
>  			     GPIOD_IS_OUT);
>  	/*
>  	 * Issue reset to add-in card through the dedicated GPIO.
> @@ -633,9 +633,9 @@ static int pcie_advk_probe(struct udevice *dev)
>  	 */
>  	if (dm_gpio_is_valid(&reset_gpio)) {
>  		dev_dbg(pcie->dev, "Toggle PCIE Reset GPIO ...\n");
> -		dm_gpio_set_value(&reset_gpio, 0);
> -		mdelay(200);
>  		dm_gpio_set_value(&reset_gpio, 1);
> +		mdelay(200);
> +		dm_gpio_set_value(&reset_gpio, 0);
>  	}
>  #else
>  	dev_dbg(pcie->dev, "PCIE Reset on GPIO support is missing\n");
> --
> 2.20.1

[PATCH 2/2] arm64: a37xx: pci: Assert PERST# signal when unloading driver

[Pali Rohár]

On Thursday 20 August 2020 07:05:58 Stefan Roese wrote:
> On 19.08.20 15:57, Pali Roh?r wrote:
> > This change ensures that PCIe card is put into reset state when U-Boot
> > stops using it.
> > 
> > DM_FLAG_OS_PREPARE ensures that U-Boot executes driver's remove callback
> > prior booting Linux kernel.
> > 
> > Linux kernel pci-aardvark driver needs to reset PCIe card via PERST# signal
> > prior initializing it. If it does not issue reset then some PCIe cards
> > (specially Compex WiFi cards) are not detected at all.
> > 
> > Putting PCIe card into reset state prior booting Linux kernel would ensure
> > that card would be properly reset at time when Linux kernel starts
> > initializing pci-aardvark driver.
> > 
> > Signed-off-by: Pali Roh?r <pali@kernel.org>
> > ---
> >   drivers/pci/pci-aardvark.c | 27 +++++++++++++++++++++------
> >   1 file changed, 21 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/pci/pci-aardvark.c b/drivers/pci/pci-aardvark.c
> > index 5b3f23c184..8996be5309 100644
> > --- a/drivers/pci/pci-aardvark.c
> > +++ b/drivers/pci/pci-aardvark.c
> > @@ -148,6 +148,9 @@ struct pcie_advk {
> >   	void           *base;
> >   	int            first_busno;
> >   	struct udevice *dev;
> > +#if CONFIG_IS_ENABLED(DM_GPIO)
> > +	struct gpio_desc reset_gpio;
> > +#endif
> >   };
> 
> Adding more #ifdef's is not recommended. Can't you "depend" this driver
> on DM_GPIO in Kconfig instead? Are there any used that don't support
> DM_GPIO right now?

I'm not sure if this dependency is what people want. CCing Simon.

In past Simon created commit bcee8d6764f9215f16b393a35581000178633254
where described that want to build SPL without GPIO uclass support.

> >   static inline void advk_writel(struct pcie_advk *pcie, uint val, uint reg)
> > @@ -614,9 +617,7 @@ static int pcie_advk_probe(struct udevice *dev)
> >   	struct pcie_advk *pcie = dev_get_priv(dev);
> >   #if CONFIG_IS_ENABLED(DM_GPIO)
> 
> If you change the driver to rely on DM_GPIO, then please also remove all
> other #ifdef's with DM_GPIO.
> 
> Thanks,
> Stefan
> 
> > -	struct gpio_desc reset_gpio;
> > -
> > -	gpio_request_by_name(dev, "reset-gpios", 0, &reset_gpio,
> > +	gpio_request_by_name(dev, "reset-gpios", 0, &pcie->reset_gpio,
> >   			     GPIOD_IS_OUT);
> >   	/*
> >   	 * Issue reset to add-in card through the dedicated GPIO.
> > @@ -631,11 +632,11 @@ static int pcie_advk_probe(struct udevice *dev)
> >   	 *     possible before PCIe PHY initialization. Moreover, the PCIe
> >   	 *     clock should be gated as well.
> >   	 */
> > -	if (dm_gpio_is_valid(&reset_gpio)) {
> > +	if (dm_gpio_is_valid(&pcie->reset_gpio)) {
> >   		dev_dbg(pcie->dev, "Toggle PCIE Reset GPIO ...\n");
> > -		dm_gpio_set_value(&reset_gpio, 1);
> > +		dm_gpio_set_value(&pcie->reset_gpio, 1);
> >   		mdelay(200);
> > -		dm_gpio_set_value(&reset_gpio, 0);
> > +		dm_gpio_set_value(&pcie->reset_gpio, 0);
> >   	}
> >   #else
> >   	dev_dbg(pcie->dev, "PCIE Reset on GPIO support is missing\n");
> > @@ -647,6 +648,18 @@ static int pcie_advk_probe(struct udevice *dev)
> >   	return pcie_advk_setup_hw(pcie);
> >   }
> > +static int pcie_advk_remove(struct udevice *dev)
> > +{
> > +#if CONFIG_IS_ENABLED(DM_GPIO)
> > +	struct pcie_advk *pcie = dev_get_priv(dev);
> > +
> > +	if (dm_gpio_is_valid(&pcie->reset_gpio))
> > +		dm_gpio_set_value(&pcie->reset_gpio, 1);
> > +#endif /* DM_GPIO */
> > +
> > +	return 0;
> > +}
> > +
> >   /**
> >    * pcie_advk_ofdata_to_platdata() - Translate from DT to device state
> >    *
> > @@ -687,5 +700,7 @@ U_BOOT_DRIVER(pcie_advk) = {
> >   	.ops			= &pcie_advk_ops,
> >   	.ofdata_to_platdata	= pcie_advk_ofdata_to_platdata,
> >   	.probe			= pcie_advk_probe,
> > +	.remove			= pcie_advk_remove,
> > +	.flags			= DM_FLAG_OS_PREPARE,
> >   	.priv_auto_alloc_size	= sizeof(struct pcie_advk),
> >   };
> > 
> 
> 
> Viele Gr??e,
> Stefan
> 
> -- 
> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de

[PATCH 2/2] arm64: a37xx: pci: Assert PERST# signal when unloading driver

[Stefan Roese]

On 20.08.20 09:43, Pali Roh?r wrote:
> On Thursday 20 August 2020 07:05:58 Stefan Roese wrote:
>> On 19.08.20 15:57, Pali Roh?r wrote:
>>> This change ensures that PCIe card is put into reset state when U-Boot
>>> stops using it.
>>>
>>> DM_FLAG_OS_PREPARE ensures that U-Boot executes driver's remove callback
>>> prior booting Linux kernel.
>>>
>>> Linux kernel pci-aardvark driver needs to reset PCIe card via PERST# signal
>>> prior initializing it. If it does not issue reset then some PCIe cards
>>> (specially Compex WiFi cards) are not detected at all.
>>>
>>> Putting PCIe card into reset state prior booting Linux kernel would ensure
>>> that card would be properly reset at time when Linux kernel starts
>>> initializing pci-aardvark driver.
>>>
>>> Signed-off-by: Pali Roh?r <pali@kernel.org>
>>> ---
>>>    drivers/pci/pci-aardvark.c | 27 +++++++++++++++++++++------
>>>    1 file changed, 21 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/pci/pci-aardvark.c b/drivers/pci/pci-aardvark.c
>>> index 5b3f23c184..8996be5309 100644
>>> --- a/drivers/pci/pci-aardvark.c
>>> +++ b/drivers/pci/pci-aardvark.c
>>> @@ -148,6 +148,9 @@ struct pcie_advk {
>>>    	void           *base;
>>>    	int            first_busno;
>>>    	struct udevice *dev;
>>> +#if CONFIG_IS_ENABLED(DM_GPIO)
>>> +	struct gpio_desc reset_gpio;
>>> +#endif
>>>    };
>>
>> Adding more #ifdef's is not recommended. Can't you "depend" this driver
>> on DM_GPIO in Kconfig instead? Are there any used that don't support
>> DM_GPIO right now?
> 
> I'm not sure if this dependency is what people want. CCing Simon.
> 
> In past Simon created commit bcee8d6764f9215f16b393a35581000178633254
> where described that want to build SPL without GPIO uclass support.

Is this PCI driver ever built into SPL?

Thanks,
Stefan

[PATCH 2/2] arm64: a37xx: pci: Assert PERST# signal when unloading driver

[Pali Rohár]

On Thursday 20 August 2020 09:48:17 Stefan Roese wrote:
> On 20.08.20 09:43, Pali Roh?r wrote:
> > On Thursday 20 August 2020 07:05:58 Stefan Roese wrote:
> > > On 19.08.20 15:57, Pali Roh?r wrote:
> > > > This change ensures that PCIe card is put into reset state when U-Boot
> > > > stops using it.
> > > > 
> > > > DM_FLAG_OS_PREPARE ensures that U-Boot executes driver's remove callback
> > > > prior booting Linux kernel.
> > > > 
> > > > Linux kernel pci-aardvark driver needs to reset PCIe card via PERST# signal
> > > > prior initializing it. If it does not issue reset then some PCIe cards
> > > > (specially Compex WiFi cards) are not detected at all.
> > > > 
> > > > Putting PCIe card into reset state prior booting Linux kernel would ensure
> > > > that card would be properly reset at time when Linux kernel starts
> > > > initializing pci-aardvark driver.
> > > > 
> > > > Signed-off-by: Pali Roh?r <pali@kernel.org>
> > > > ---
> > > >    drivers/pci/pci-aardvark.c | 27 +++++++++++++++++++++------
> > > >    1 file changed, 21 insertions(+), 6 deletions(-)
> > > > 
> > > > diff --git a/drivers/pci/pci-aardvark.c b/drivers/pci/pci-aardvark.c
> > > > index 5b3f23c184..8996be5309 100644
> > > > --- a/drivers/pci/pci-aardvark.c
> > > > +++ b/drivers/pci/pci-aardvark.c
> > > > @@ -148,6 +148,9 @@ struct pcie_advk {
> > > >    	void           *base;
> > > >    	int            first_busno;
> > > >    	struct udevice *dev;
> > > > +#if CONFIG_IS_ENABLED(DM_GPIO)
> > > > +	struct gpio_desc reset_gpio;
> > > > +#endif
> > > >    };
> > > 
> > > Adding more #ifdef's is not recommended. Can't you "depend" this driver
> > > on DM_GPIO in Kconfig instead? Are there any used that don't support
> > > DM_GPIO right now?
> > 
> > I'm not sure if this dependency is what people want. CCing Simon.
> > 
> > In past Simon created commit bcee8d6764f9215f16b393a35581000178633254
> > where described that want to build SPL without GPIO uclass support.
> 
> Is this PCI driver ever built into SPL?

SPL is not used for Espressobin and Turris MOX. But I'm not sure for
other boards. Maybe Simon could provide more information about it.

Basically I added GPIO handling in aardvark driver as it was used on
other place.

[EXT] [PATCH 2/2] arm64: a37xx: pci: Assert PERST# signal when unloading driver

[Stefan Roese]

Hi Kosta,

this one as well please. ;)

Thanks,
Stefan

On 19.08.20 19:31, Kostya Porotchkin wrote:
> 
> 
> ________________________________________
> From: Pali Roh?r <pali@kernel.org>
> Sent: Wednesday, August 19, 2020 16:57
> To: Stefan Roese; Kostya Porotchkin
> Cc: u-boot at lists.denx.de
> Subject: [EXT] [PATCH 2/2] arm64: a37xx: pci: Assert PERST# signal when unloading driver
> 
> External Email
> 
> ----------------------------------------------------------------------
> This change ensures that PCIe card is put into reset state when U-Boot
> stops using it.
> 
> DM_FLAG_OS_PREPARE ensures that U-Boot executes driver's remove callback
> prior booting Linux kernel.
> 
> Linux kernel pci-aardvark driver needs to reset PCIe card via PERST# signal
> prior initializing it. If it does not issue reset then some PCIe cards
> (specially Compex WiFi cards) are not detected at all.
> 
> Putting PCIe card into reset state prior booting Linux kernel would ensure
> that card would be properly reset at time when Linux kernel starts
> initializing pci-aardvark driver.
> 
> Signed-off-by: Pali Roh?r <pali@kernel.org>
> Reviewed-by: Konstantin Porotchkin <kostap@marvell.com>
> ---
>   drivers/pci/pci-aardvark.c | 27 +++++++++++++++++++++------
>   1 file changed, 21 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/pci/pci-aardvark.c b/drivers/pci/pci-aardvark.c
> index 5b3f23c184..8996be5309 100644
> --- a/drivers/pci/pci-aardvark.c
> +++ b/drivers/pci/pci-aardvark.c
> @@ -148,6 +148,9 @@ struct pcie_advk {
>          void           *base;
>          int            first_busno;
>          struct udevice *dev;
> +#if CONFIG_IS_ENABLED(DM_GPIO)
> +       struct gpio_desc reset_gpio;
> +#endif
>   };
> 
>   static inline void advk_writel(struct pcie_advk *pcie, uint val, uint reg)
> @@ -614,9 +617,7 @@ static int pcie_advk_probe(struct udevice *dev)
>          struct pcie_advk *pcie = dev_get_priv(dev);
> 
>   #if CONFIG_IS_ENABLED(DM_GPIO)
> -       struct gpio_desc reset_gpio;
> -
> -       gpio_request_by_name(dev, "reset-gpios", 0, &reset_gpio,
> +       gpio_request_by_name(dev, "reset-gpios", 0, &pcie->reset_gpio,
>                               GPIOD_IS_OUT);
>          /*
>           * Issue reset to add-in card through the dedicated GPIO.
> @@ -631,11 +632,11 @@ static int pcie_advk_probe(struct udevice *dev)
>           *     possible before PCIe PHY initialization. Moreover, the PCIe
>           *     clock should be gated as well.
>           */
> -       if (dm_gpio_is_valid(&reset_gpio)) {
> +       if (dm_gpio_is_valid(&pcie->reset_gpio)) {
>                  dev_dbg(pcie->dev, "Toggle PCIE Reset GPIO ...\n");
> -               dm_gpio_set_value(&reset_gpio, 1);
> +               dm_gpio_set_value(&pcie->reset_gpio, 1);
>                  mdelay(200);
> -               dm_gpio_set_value(&reset_gpio, 0);
> +               dm_gpio_set_value(&pcie->reset_gpio, 0);
>          }
>   #else
>          dev_dbg(pcie->dev, "PCIE Reset on GPIO support is missing\n");
> @@ -647,6 +648,18 @@ static int pcie_advk_probe(struct udevice *dev)
>          return pcie_advk_setup_hw(pcie);
>   }
> 
> +static int pcie_advk_remove(struct udevice *dev)
> +{
> +#if CONFIG_IS_ENABLED(DM_GPIO)
> +       struct pcie_advk *pcie = dev_get_priv(dev);
> +
> +       if (dm_gpio_is_valid(&pcie->reset_gpio))
> +               dm_gpio_set_value(&pcie->reset_gpio, 1);
> +#endif /* DM_GPIO */
> +
> +       return 0;
> +}
> +
>   /**
>    * pcie_advk_ofdata_to_platdata() - Translate from DT to device state
>    *
> @@ -687,5 +700,7 @@ U_BOOT_DRIVER(pcie_advk) = {
>          .ops                    = &pcie_advk_ops,
>          .ofdata_to_platdata     = pcie_advk_ofdata_to_platdata,
>          .probe                  = pcie_advk_probe,
> +       .remove                 = pcie_advk_remove,
> +       .flags                  = DM_FLAG_OS_PREPARE,
>          .priv_auto_alloc_size   = sizeof(struct pcie_advk),
>   };
> --
> 2.20.1
> 


Viele Gr??e,
Stefan

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de

[PATCH 2/2] arm64: a37xx: pci: Assert PERST# signal when unloading driver

[Stefan Roese]

On 20.08.20 09:52, Pali Roh?r wrote:
> On Thursday 20 August 2020 09:48:17 Stefan Roese wrote:
>> On 20.08.20 09:43, Pali Roh?r wrote:
>>> On Thursday 20 August 2020 07:05:58 Stefan Roese wrote:
>>>> On 19.08.20 15:57, Pali Roh?r wrote:
>>>>> This change ensures that PCIe card is put into reset state when U-Boot
>>>>> stops using it.
>>>>>
>>>>> DM_FLAG_OS_PREPARE ensures that U-Boot executes driver's remove callback
>>>>> prior booting Linux kernel.
>>>>>
>>>>> Linux kernel pci-aardvark driver needs to reset PCIe card via PERST# signal
>>>>> prior initializing it. If it does not issue reset then some PCIe cards
>>>>> (specially Compex WiFi cards) are not detected at all.
>>>>>
>>>>> Putting PCIe card into reset state prior booting Linux kernel would ensure
>>>>> that card would be properly reset at time when Linux kernel starts
>>>>> initializing pci-aardvark driver.
>>>>>
>>>>> Signed-off-by: Pali Roh?r <pali@kernel.org>
>>>>> ---
>>>>>     drivers/pci/pci-aardvark.c | 27 +++++++++++++++++++++------
>>>>>     1 file changed, 21 insertions(+), 6 deletions(-)
>>>>>
>>>>> diff --git a/drivers/pci/pci-aardvark.c b/drivers/pci/pci-aardvark.c
>>>>> index 5b3f23c184..8996be5309 100644
>>>>> --- a/drivers/pci/pci-aardvark.c
>>>>> +++ b/drivers/pci/pci-aardvark.c
>>>>> @@ -148,6 +148,9 @@ struct pcie_advk {
>>>>>     	void           *base;
>>>>>     	int            first_busno;
>>>>>     	struct udevice *dev;
>>>>> +#if CONFIG_IS_ENABLED(DM_GPIO)
>>>>> +	struct gpio_desc reset_gpio;
>>>>> +#endif
>>>>>     };
>>>>
>>>> Adding more #ifdef's is not recommended. Can't you "depend" this driver
>>>> on DM_GPIO in Kconfig instead? Are there any used that don't support
>>>> DM_GPIO right now?
>>>
>>> I'm not sure if this dependency is what people want. CCing Simon.
>>>
>>> In past Simon created commit bcee8d6764f9215f16b393a35581000178633254
>>> where described that want to build SPL without GPIO uclass support.
>>
>> Is this PCI driver ever built into SPL?
> 
> SPL is not used for Espressobin and Turris MOX. But I'm not sure for
> other boards. Maybe Simon could provide more information about it.

AFAIK (I did not work on Armada for quite some time), only 32bit
Armada uses SPL. All 64bit Aramda 37xx / 7xxx / 8xxx do not use
SPL.

> Basically I added GPIO handling in aardvark driver as it was used on
> other place.

I understand.

I suggest to "depend" this driver on DM_GPIO and include the GPIO code
without any #ifdef's. And please run a world-build (buildman, Travis...)
to see, if nothing breaks.

Thanks,
Stefan

[EXT] [PATCH 2/2] arm64: a37xx: pci: Assert PERST# signal when unloading driver

[Kostya Porotchkin]

> -----Original Message-----
> From: Pali Roh?r <pali@kernel.org>
> Sent: Wednesday, August 19, 2020 16:57
> To: Stefan Roese <sr@denx.de>; Kostya Porotchkin <kostap@marvell.com>
> Cc: u-boot at lists.denx.de
> Subject: [EXT] [PATCH 2/2] arm64: a37xx: pci: Assert PERST# signal when
> unloading driver
> 
> External Email
> 
> ----------------------------------------------------------------------
> This change ensures that PCIe card is put into reset state when U-Boot stops
> using it.
> 
> DM_FLAG_OS_PREPARE ensures that U-Boot executes driver's remove
> callback prior booting Linux kernel.
> 
> Linux kernel pci-aardvark driver needs to reset PCIe card via PERST# signal
> prior initializing it. If it does not issue reset then some PCIe cards (specially
> Compex WiFi cards) are not detected at all.
> 
> Putting PCIe card into reset state prior booting Linux kernel would ensure
> that card would be properly reset at time when Linux kernel starts initializing
> pci-aardvark driver.
> 
> Signed-off-by: Pali Roh?r <pali@kernel.org>
Reviewed-by: Konstantin Porotchkin <kostap@marvell.com>

> ---
>  drivers/pci/pci-aardvark.c | 27 +++++++++++++++++++++------
>  1 file changed, 21 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/pci/pci-aardvark.c b/drivers/pci/pci-aardvark.c index
> 5b3f23c184..8996be5309 100644
> --- a/drivers/pci/pci-aardvark.c
> +++ b/drivers/pci/pci-aardvark.c
> @@ -148,6 +148,9 @@ struct pcie_advk {
>  	void           *base;
>  	int            first_busno;
>  	struct udevice *dev;
> +#if CONFIG_IS_ENABLED(DM_GPIO)
> +	struct gpio_desc reset_gpio;
> +#endif
>  };
> 
>  static inline void advk_writel(struct pcie_advk *pcie, uint val, uint reg) @@ -
> 614,9 +617,7 @@ static int pcie_advk_probe(struct udevice *dev)
>  	struct pcie_advk *pcie = dev_get_priv(dev);
> 
>  #if CONFIG_IS_ENABLED(DM_GPIO)
> -	struct gpio_desc reset_gpio;
> -
> -	gpio_request_by_name(dev, "reset-gpios", 0, &reset_gpio,
> +	gpio_request_by_name(dev, "reset-gpios", 0, &pcie->reset_gpio,
>  			     GPIOD_IS_OUT);
>  	/*
>  	 * Issue reset to add-in card through the dedicated GPIO.
> @@ -631,11 +632,11 @@ static int pcie_advk_probe(struct udevice *dev)
>  	 *     possible before PCIe PHY initialization. Moreover, the PCIe
>  	 *     clock should be gated as well.
>  	 */
> -	if (dm_gpio_is_valid(&reset_gpio)) {
> +	if (dm_gpio_is_valid(&pcie->reset_gpio)) {
>  		dev_dbg(pcie->dev, "Toggle PCIE Reset GPIO ...\n");
> -		dm_gpio_set_value(&reset_gpio, 1);
> +		dm_gpio_set_value(&pcie->reset_gpio, 1);
>  		mdelay(200);
> -		dm_gpio_set_value(&reset_gpio, 0);
> +		dm_gpio_set_value(&pcie->reset_gpio, 0);
>  	}
>  #else
>  	dev_dbg(pcie->dev, "PCIE Reset on GPIO support is missing\n"); @@
> -647,6 +648,18 @@ static int pcie_advk_probe(struct udevice *dev)
>  	return pcie_advk_setup_hw(pcie);
>  }
> 
> +static int pcie_advk_remove(struct udevice *dev) { #if
> +CONFIG_IS_ENABLED(DM_GPIO)
> +	struct pcie_advk *pcie = dev_get_priv(dev);
> +
> +	if (dm_gpio_is_valid(&pcie->reset_gpio))
> +		dm_gpio_set_value(&pcie->reset_gpio, 1); #endif /*
> DM_GPIO */
> +
> +	return 0;
> +}
> +
>  /**
>   * pcie_advk_ofdata_to_platdata() - Translate from DT to device state
>   *
> @@ -687,5 +700,7 @@ U_BOOT_DRIVER(pcie_advk) = {
>  	.ops			= &pcie_advk_ops,
>  	.ofdata_to_platdata	= pcie_advk_ofdata_to_platdata,
>  	.probe			= pcie_advk_probe,
> +	.remove			= pcie_advk_remove,
> +	.flags			= DM_FLAG_OS_PREPARE,
>  	.priv_auto_alloc_size	= sizeof(struct pcie_advk),
>  };
> --
> 2.20.1

[PATCH] arm64: a37xx: pci: Depends on DM_GPIO

[Pali Rohár]

For proper initialization of aardvark pci driver it is required to
de-assert reset GPIO. So depeneds on DM_GPIO option.

Signed-off-by: Pali Roh?r <pali@kernel.org>
---
 drivers/pci/Kconfig        |  1 +
 drivers/pci/pci-aardvark.c | 10 ++--------
 2 files changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
index 5e0a39396b..db70ccc288 100644
--- a/drivers/pci/Kconfig
+++ b/drivers/pci/Kconfig
@@ -30,6 +30,7 @@ config PCI_AARDVARK
 	bool "Enable Aardvark PCIe driver"
 	default n
 	depends on DM_PCI
+	depends on DM_GPIO
 	depends on ARMADA_3700
 	help
 	  Say Y here if you want to enable PCIe controller support on
diff --git a/drivers/pci/pci-aardvark.c b/drivers/pci/pci-aardvark.c
index 8996be5309..b2c417701f 100644
--- a/drivers/pci/pci-aardvark.c
+++ b/drivers/pci/pci-aardvark.c
@@ -148,9 +148,7 @@ struct pcie_advk {
 	void           *base;
 	int            first_busno;
 	struct udevice *dev;
-#if CONFIG_IS_ENABLED(DM_GPIO)
 	struct gpio_desc reset_gpio;
-#endif
 };
 
 static inline void advk_writel(struct pcie_advk *pcie, uint val, uint reg)
@@ -616,7 +614,6 @@ static int pcie_advk_probe(struct udevice *dev)
 {
 	struct pcie_advk *pcie = dev_get_priv(dev);
 
-#if CONFIG_IS_ENABLED(DM_GPIO)
 	gpio_request_by_name(dev, "reset-gpios", 0, &pcie->reset_gpio,
 			     GPIOD_IS_OUT);
 	/*
@@ -637,10 +634,9 @@ static int pcie_advk_probe(struct udevice *dev)
 		dm_gpio_set_value(&pcie->reset_gpio, 1);
 		mdelay(200);
 		dm_gpio_set_value(&pcie->reset_gpio, 0);
+	} else {
+		dev_warn(pcie->dev, "PCIE Reset on GPIO support is missing\n");
 	}
-#else
-	dev_dbg(pcie->dev, "PCIE Reset on GPIO support is missing\n");
-#endif /* DM_GPIO */
 
 	pcie->first_busno = dev->seq;
 	pcie->dev = pci_get_controller(dev);
@@ -650,12 +646,10 @@ static int pcie_advk_probe(struct udevice *dev)
 
 static int pcie_advk_remove(struct udevice *dev)
 {
-#if CONFIG_IS_ENABLED(DM_GPIO)
 	struct pcie_advk *pcie = dev_get_priv(dev);
 
 	if (dm_gpio_is_valid(&pcie->reset_gpio))
 		dm_gpio_set_value(&pcie->reset_gpio, 1);
-#endif /* DM_GPIO */
 
 	return 0;
 }
-- 
2.20.1

[PATCH 2/2] arm64: a37xx: pci: Assert PERST# signal when unloading driver

[Pali Rohár]

On Thursday 20 August 2020 10:00:52 Stefan Roese wrote:
> I suggest to "depend" this driver on DM_GPIO and include the GPIO code
> without any #ifdef's. And please run a world-build (buildman, Travis...)
> to see, if nothing breaks.

I sent third patch which removes those #ifdef's and run CI testing on
Github: https://github.com/u-boot/u-boot/pull/37

Everything passed.

[PATCH 2/2] arm64: a37xx: pci: Assert PERST# signal when unloading driver

[Stefan Roese]

On 25.08.20 10:46, Pali Roh?r wrote:
> On Thursday 20 August 2020 10:00:52 Stefan Roese wrote:
>> I suggest to "depend" this driver on DM_GPIO and include the GPIO code
>> without any #ifdef's. And please run a world-build (buildman, Travis...)
>> to see, if nothing breaks.
> 
> I sent third patch which removes those #ifdef's and run CI testing on
> Github: https://github.com/u-boot/u-boot/pull/37
> 
> Everything passed.

Perfect. ;)

Thanks,
Stefan

[PATCH] arm64: a37xx: pci: Depends on DM_GPIO

[Stefan Roese]

On 25.08.20 10:45, Pali Roh?r wrote:
> For proper initialization of aardvark pci driver it is required to
> de-assert reset GPIO. So depeneds on DM_GPIO option.
> 
> Signed-off-by: Pali Roh?r <pali@kernel.org>

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

[PATCH 1/2] arm64: a37xx: pci: Make PCIe Reset GPIO DT compatible with Linux kernel DT

[Andre Heider]

On 19/08/2020 15:57, Pali Roh?r wrote:
> Change active-high to active-low and change DT property name from
> reset-gpio to reset-gpios. This format of gpio reset is used by
> pci-aardvark driver in Linux kernel.
> 
> Signed-off-by: Pali Roh?r <pali@kernel.org>

Tested-by: Andre Heider <a.heider@gmail.com>

[PATCH 2/2] arm64: a37xx: pci: Assert PERST# signal when unloading driver

[Andre Heider]

On 19/08/2020 15:57, Pali Roh?r wrote:
> This change ensures that PCIe card is put into reset state when U-Boot
> stops using it.
> 
> DM_FLAG_OS_PREPARE ensures that U-Boot executes driver's remove callback
> prior booting Linux kernel.
> 
> Linux kernel pci-aardvark driver needs to reset PCIe card via PERST# signal
> prior initializing it. If it does not issue reset then some PCIe cards
> (specially Compex WiFi cards) are not detected at all.
> 
> Putting PCIe card into reset state prior booting Linux kernel would ensure
> that card would be properly reset at time when Linux kernel starts
> initializing pci-aardvark driver.
> 
> Signed-off-by: Pali Roh?r <pali@kernel.org>

Tested-by: Andre Heider <a.heider@gmail.com>

[PATCH] arm64: a37xx: pci: Depends on DM_GPIO

[Andre Heider]

On 25/08/2020 10:45, Pali Roh?r wrote:
> For proper initialization of aardvark pci driver it is required to
> de-assert reset GPIO. So depeneds on DM_GPIO option.
> 
> Signed-off-by: Pali Roh?r <pali@kernel.org>

Tested-by: Andre Heider <a.heider@gmail.com>

[PATCH 1/2] arm64: a37xx: pci: Make PCIe Reset GPIO DT compatible with Linux kernel DT

[Stefan Roese]

On 19.08.20 15:57, Pali Roh?r wrote:
> Change active-high to active-low and change DT property name from
> reset-gpio to reset-gpios. This format of gpio reset is used by
> pci-aardvark driver in Linux kernel.
> 
> Signed-off-by: Pali Roh?r <pali@kernel.org>

Applied to u-boot-marvell/master

Thanks,
Stefan

> ---
>   arch/arm/dts/armada-3720-db.dts          | 2 +-
>   arch/arm/dts/armada-3720-espressobin.dts | 2 +-
>   arch/arm/dts/armada-3720-turris-mox.dts  | 2 +-
>   drivers/pci/pci-aardvark.c               | 6 +++---
>   4 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm/dts/armada-3720-db.dts b/arch/arm/dts/armada-3720-db.dts
> index 1b219c423b..1b1b66b94d 100644
> --- a/arch/arm/dts/armada-3720-db.dts
> +++ b/arch/arm/dts/armada-3720-db.dts
> @@ -159,6 +159,6 @@
>   &pcie0 {
>   	pinctrl-names = "default";
>   	pinctrl-0 = <&pcie_pins>;
> -	reset-gpio = <&gpiosb 3 GPIO_ACTIVE_HIGH>;
> +	reset-gpios = <&gpiosb 3 GPIO_ACTIVE_LOW>;
>   	status = "okay";
>   };
> diff --git a/arch/arm/dts/armada-3720-espressobin.dts b/arch/arm/dts/armada-3720-espressobin.dts
> index 84e2c2adba..f10a953ec5 100644
> --- a/arch/arm/dts/armada-3720-espressobin.dts
> +++ b/arch/arm/dts/armada-3720-espressobin.dts
> @@ -145,6 +145,6 @@
>   &pcie0 {
>   	pinctrl-names = "default";
>   	pinctrl-0 = <&pcie_pins>;
> -	reset-gpio = <&gpiosb 3 GPIO_ACTIVE_HIGH>;
> +	reset-gpios = <&gpiosb 3 GPIO_ACTIVE_LOW>;
>   	status = "okay";
>   };
> diff --git a/arch/arm/dts/armada-3720-turris-mox.dts b/arch/arm/dts/armada-3720-turris-mox.dts
> index 0f0a6ce65d..974270cc8c 100644
> --- a/arch/arm/dts/armada-3720-turris-mox.dts
> +++ b/arch/arm/dts/armada-3720-turris-mox.dts
> @@ -172,6 +172,6 @@
>   &pcie0 {
>   	pinctrl-names = "default";
>   	pinctrl-0 = <&pcie_pins>;
> -	reset-gpio = <&gpiosb 3 GPIO_ACTIVE_HIGH>;
> +	reset-gpios = <&gpiosb 3 GPIO_ACTIVE_LOW>;
>   	status = "disabled";
>   };
> diff --git a/drivers/pci/pci-aardvark.c b/drivers/pci/pci-aardvark.c
> index 711b930d0f..5b3f23c184 100644
> --- a/drivers/pci/pci-aardvark.c
> +++ b/drivers/pci/pci-aardvark.c
> @@ -616,7 +616,7 @@ static int pcie_advk_probe(struct udevice *dev)
>   #if CONFIG_IS_ENABLED(DM_GPIO)
>   	struct gpio_desc reset_gpio;
>   
> -	gpio_request_by_name(dev, "reset-gpio", 0, &reset_gpio,
> +	gpio_request_by_name(dev, "reset-gpios", 0, &reset_gpio,
>   			     GPIOD_IS_OUT);
>   	/*
>   	 * Issue reset to add-in card through the dedicated GPIO.
> @@ -633,9 +633,9 @@ static int pcie_advk_probe(struct udevice *dev)
>   	 */
>   	if (dm_gpio_is_valid(&reset_gpio)) {
>   		dev_dbg(pcie->dev, "Toggle PCIE Reset GPIO ...\n");
> -		dm_gpio_set_value(&reset_gpio, 0);
> -		mdelay(200);
>   		dm_gpio_set_value(&reset_gpio, 1);
> +		mdelay(200);
> +		dm_gpio_set_value(&reset_gpio, 0);
>   	}
>   #else
>   	dev_dbg(pcie->dev, "PCIE Reset on GPIO support is missing\n");
> 


Viele Gr??e,
Stefan

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de

[PATCH 2/2] arm64: a37xx: pci: Assert PERST# signal when unloading driver

[Stefan Roese]

On 19.08.20 15:57, Pali Roh?r wrote:
> This change ensures that PCIe card is put into reset state when U-Boot
> stops using it.
> 
> DM_FLAG_OS_PREPARE ensures that U-Boot executes driver's remove callback
> prior booting Linux kernel.
> 
> Linux kernel pci-aardvark driver needs to reset PCIe card via PERST# signal
> prior initializing it. If it does not issue reset then some PCIe cards
> (specially Compex WiFi cards) are not detected at all.
> 
> Putting PCIe card into reset state prior booting Linux kernel would ensure
> that card would be properly reset at time when Linux kernel starts
> initializing pci-aardvark driver.
> 
> Signed-off-by: Pali Roh?r <pali@kernel.org>

Applied to u-boot-marvell/master

Thanks,
Stefan

> ---
>   drivers/pci/pci-aardvark.c | 27 +++++++++++++++++++++------
>   1 file changed, 21 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/pci/pci-aardvark.c b/drivers/pci/pci-aardvark.c
> index 5b3f23c184..8996be5309 100644
> --- a/drivers/pci/pci-aardvark.c
> +++ b/drivers/pci/pci-aardvark.c
> @@ -148,6 +148,9 @@ struct pcie_advk {
>   	void           *base;
>   	int            first_busno;
>   	struct udevice *dev;
> +#if CONFIG_IS_ENABLED(DM_GPIO)
> +	struct gpio_desc reset_gpio;
> +#endif
>   };
>   
>   static inline void advk_writel(struct pcie_advk *pcie, uint val, uint reg)
> @@ -614,9 +617,7 @@ static int pcie_advk_probe(struct udevice *dev)
>   	struct pcie_advk *pcie = dev_get_priv(dev);
>   
>   #if CONFIG_IS_ENABLED(DM_GPIO)
> -	struct gpio_desc reset_gpio;
> -
> -	gpio_request_by_name(dev, "reset-gpios", 0, &reset_gpio,
> +	gpio_request_by_name(dev, "reset-gpios", 0, &pcie->reset_gpio,
>   			     GPIOD_IS_OUT);
>   	/*
>   	 * Issue reset to add-in card through the dedicated GPIO.
> @@ -631,11 +632,11 @@ static int pcie_advk_probe(struct udevice *dev)
>   	 *     possible before PCIe PHY initialization. Moreover, the PCIe
>   	 *     clock should be gated as well.
>   	 */
> -	if (dm_gpio_is_valid(&reset_gpio)) {
> +	if (dm_gpio_is_valid(&pcie->reset_gpio)) {
>   		dev_dbg(pcie->dev, "Toggle PCIE Reset GPIO ...\n");
> -		dm_gpio_set_value(&reset_gpio, 1);
> +		dm_gpio_set_value(&pcie->reset_gpio, 1);
>   		mdelay(200);
> -		dm_gpio_set_value(&reset_gpio, 0);
> +		dm_gpio_set_value(&pcie->reset_gpio, 0);
>   	}
>   #else
>   	dev_dbg(pcie->dev, "PCIE Reset on GPIO support is missing\n");
> @@ -647,6 +648,18 @@ static int pcie_advk_probe(struct udevice *dev)
>   	return pcie_advk_setup_hw(pcie);
>   }
>   
> +static int pcie_advk_remove(struct udevice *dev)
> +{
> +#if CONFIG_IS_ENABLED(DM_GPIO)
> +	struct pcie_advk *pcie = dev_get_priv(dev);
> +
> +	if (dm_gpio_is_valid(&pcie->reset_gpio))
> +		dm_gpio_set_value(&pcie->reset_gpio, 1);
> +#endif /* DM_GPIO */
> +
> +	return 0;
> +}
> +
>   /**
>    * pcie_advk_ofdata_to_platdata() - Translate from DT to device state
>    *
> @@ -687,5 +700,7 @@ U_BOOT_DRIVER(pcie_advk) = {
>   	.ops			= &pcie_advk_ops,
>   	.ofdata_to_platdata	= pcie_advk_ofdata_to_platdata,
>   	.probe			= pcie_advk_probe,
> +	.remove			= pcie_advk_remove,
> +	.flags			= DM_FLAG_OS_PREPARE,
>   	.priv_auto_alloc_size	= sizeof(struct pcie_advk),
>   };
> 


Viele Gr??e,
Stefan

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de

[PATCH] arm64: a37xx: pci: Depends on DM_GPIO

[Stefan Roese]

On 25.08.20 10:45, Pali Roh?r wrote:
> For proper initialization of aardvark pci driver it is required to
> de-assert reset GPIO. So depeneds on DM_GPIO option.
> 
> Signed-off-by: Pali Roh?r <pali@kernel.org>

Applied to u-boot-marvell/master

Thanks,
Stefan

> ---
>   drivers/pci/Kconfig        |  1 +
>   drivers/pci/pci-aardvark.c | 10 ++--------
>   2 files changed, 3 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
> index 5e0a39396b..db70ccc288 100644
> --- a/drivers/pci/Kconfig
> +++ b/drivers/pci/Kconfig
> @@ -30,6 +30,7 @@ config PCI_AARDVARK
>   	bool "Enable Aardvark PCIe driver"
>   	default n
>   	depends on DM_PCI
> +	depends on DM_GPIO
>   	depends on ARMADA_3700
>   	help
>   	  Say Y here if you want to enable PCIe controller support on
> diff --git a/drivers/pci/pci-aardvark.c b/drivers/pci/pci-aardvark.c
> index 8996be5309..b2c417701f 100644
> --- a/drivers/pci/pci-aardvark.c
> +++ b/drivers/pci/pci-aardvark.c
> @@ -148,9 +148,7 @@ struct pcie_advk {
>   	void           *base;
>   	int            first_busno;
>   	struct udevice *dev;
> -#if CONFIG_IS_ENABLED(DM_GPIO)
>   	struct gpio_desc reset_gpio;
> -#endif
>   };
>   
>   static inline void advk_writel(struct pcie_advk *pcie, uint val, uint reg)
> @@ -616,7 +614,6 @@ static int pcie_advk_probe(struct udevice *dev)
>   {
>   	struct pcie_advk *pcie = dev_get_priv(dev);
>   
> -#if CONFIG_IS_ENABLED(DM_GPIO)
>   	gpio_request_by_name(dev, "reset-gpios", 0, &pcie->reset_gpio,
>   			     GPIOD_IS_OUT);
>   	/*
> @@ -637,10 +634,9 @@ static int pcie_advk_probe(struct udevice *dev)
>   		dm_gpio_set_value(&pcie->reset_gpio, 1);
>   		mdelay(200);
>   		dm_gpio_set_value(&pcie->reset_gpio, 0);
> +	} else {
> +		dev_warn(pcie->dev, "PCIE Reset on GPIO support is missing\n");
>   	}
> -#else
> -	dev_dbg(pcie->dev, "PCIE Reset on GPIO support is missing\n");
> -#endif /* DM_GPIO */
>   
>   	pcie->first_busno = dev->seq;
>   	pcie->dev = pci_get_controller(dev);
> @@ -650,12 +646,10 @@ static int pcie_advk_probe(struct udevice *dev)
>   
>   static int pcie_advk_remove(struct udevice *dev)
>   {
> -#if CONFIG_IS_ENABLED(DM_GPIO)
>   	struct pcie_advk *pcie = dev_get_priv(dev);
>   
>   	if (dm_gpio_is_valid(&pcie->reset_gpio))
>   		dm_gpio_set_value(&pcie->reset_gpio, 1);
> -#endif /* DM_GPIO */
>   
>   	return 0;
>   }
> 


Viele Gr??e,
Stefan

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de