driverdev-devel.linuxdriverproject.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/5] staging: mt7621-pci: re-do reset boot process
@ 2020-03-12  9:00 Sergio Paracuellos
  2020-03-12  9:00 ` [PATCH v3 1/5] staging: mt7621-pci: use gpios for properly reset Sergio Paracuellos
                   ` (5 more replies)
  0 siblings, 6 replies; 11+ messages in thread
From: Sergio Paracuellos @ 2020-03-12  9:00 UTC (permalink / raw)
  To: gregkh; +Cc: ryder.lee, driverdev-devel, weijie.gao, gerg, neil

Some time ago Greg Ungerer reported some random hangs using
the staging mt7621-pci driver:

See:
* http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2019-June/134947.html 

Try to fix that is the main motivation of this patch series.

Also in openwrt there is a driver for mt7621-pci which seems was rewritten
from scratch (for kernel 4.14) by Ryder Lee and Weijie Gao from mediatek. 
There the approach for reset assert-deassert process is to set as 'gpio'
the function for all the 'pcie' group for the pinctrl driver and use those
gpio's as a reset for the end points. The driver I am talking about is still
using legacy pci and legacy gpio kernel interfaces. IMHO, the correct thing
to do is make this staging driver properly clean and functional and put it
in its correct place in the mainline.

See:
* https://gist.github.com/dengqf6/7a9e9b4032d99f1a91dd9256c8a65c36

Because of all of this this patch series tries to avoid random hangs of boot
trying to use the 'reset-gpios' approach.

Changes are being tested by openwrt people and seems to work.

Hope this helps.

Changes in v3:
    * Avoid to fail if gpio descriptor fails on get.
    * re-do PATCH 1 commit message.
    * Take into account gpio low polarity on request and assert and deassert.
    * Review error path of driver to properly release gpio's resources.

Changes in v2:
    * restore configuration for pers mode to GPIO.
    * Avoid to read FTS_NUM register in reset state.
    * Release gpio's patch added

Best regards,
    Sergio Paracuellos

Sergio Paracuellos (5):
  staging: mt7621-pci: use gpios for properly reset
  staging: mt7621-pci: change value for 'PERST_DELAY_US'
  staging: mt7621-dts: make use of 'reset-gpios' property for pci
  staging: mt7621-pci: bindings: update doc accordly to last changes
  staging: mt7621-pci: release gpios after pci initialization

 drivers/staging/mt7621-dts/mt7621.dtsi        |  11 +-
 .../mt7621-pci/mediatek,mt7621-pci.txt        |   7 +-
 drivers/staging/mt7621-pci/pci-mt7621.c       | 107 +++++++++++-------
 3 files changed, 80 insertions(+), 45 deletions(-)

-- 
2.25.1

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

* [PATCH v3 1/5] staging: mt7621-pci: use gpios for properly reset
  2020-03-12  9:00 [PATCH v3 0/5] staging: mt7621-pci: re-do reset boot process Sergio Paracuellos
@ 2020-03-12  9:00 ` Sergio Paracuellos
  2020-03-12  9:00 ` [PATCH v3 2/5] staging: mt7621-pci: change value for 'PERST_DELAY_US' Sergio Paracuellos
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 11+ messages in thread
From: Sergio Paracuellos @ 2020-03-12  9:00 UTC (permalink / raw)
  To: gregkh; +Cc: ryder.lee, driverdev-devel, weijie.gao, gerg, neil

Original driver code was using three gpio's for reset
asserts and deasserts the pcis. Instead of using that
a general reset control with a perst gpio was introduced
and it seems it is partially working but sometimes there
are some unexpected hangs on boot. This commit make use of
the three original gpios using 'reset-gpios' property of
the device tree and removes the reset line and perst gpio.
According to the mediatek application note v0.1 there are
three gpios used for pcie ports reset control: gpio#19,
gpio#8 and gpio#7 for slots 0, 1 and 2 respectively.
This schema can be used separately for mt7621A but in some
boards due to pin share issue, if the PCM and I2S function
are enable at the same time, there are no enough GPIO to
control per-port PCIe reset. In those cases gpio#19 is enought
for reset the three ports together. Because of this we just
try to get the three gpios but if some of them fail we are not
failing in boot process, just prints a kernel notice and take
after into account if the descriptor is or not valid in order
to use it. All of them are set as GPIO output low configuration.
The gpio descriptor's API takes device tree property into account
and invert value if the pin is configured as active low.
So we also have to properly request pins from device tree
and set values correct in assert and deassert functions.
After this changes the order to make all assert and
deassert in the 'probe' process makes more sense:
* Parse device tree.
* make assert of the RC's and EP's before doing anything else.
* make deassert of the RC's before initializing the phy.
* Init the phy.
* make deassert of the EP's before initialize pci ports.
* Normal PCI initialization.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 drivers/staging/mt7621-pci/pci-mt7621.c | 82 +++++++++++++++----------
 1 file changed, 49 insertions(+), 33 deletions(-)

diff --git a/drivers/staging/mt7621-pci/pci-mt7621.c b/drivers/staging/mt7621-pci/pci-mt7621.c
index 770dabd3a70d..34f6bcd53fbf 100644
--- a/drivers/staging/mt7621-pci/pci-mt7621.c
+++ b/drivers/staging/mt7621-pci/pci-mt7621.c
@@ -95,6 +95,7 @@
  * @pcie: pointer to PCIe host info
  * @phy: pointer to PHY control block
  * @pcie_rst: pointer to port reset control
+ * @gpio_rst: gpio reset
  * @slot: port slot
  * @enabled: indicates if port is enabled
  */
@@ -104,6 +105,7 @@ struct mt7621_pcie_port {
 	struct mt7621_pcie *pcie;
 	struct phy *phy;
 	struct reset_control *pcie_rst;
+	struct gpio_desc *gpio_rst;
 	u32 slot;
 	bool enabled;
 };
@@ -117,8 +119,6 @@ struct mt7621_pcie_port {
  * @offset: IO / Memory offset
  * @dev: Pointer to PCIe device
  * @ports: pointer to PCIe port information
- * @perst: gpio reset
- * @rst: pointer to pcie reset
  * @resets_inverted: depends on chip revision
  * reset lines are inverted.
  */
@@ -133,8 +133,6 @@ struct mt7621_pcie {
 		resource_size_t io;
 	} offset;
 	struct list_head ports;
-	struct gpio_desc *perst;
-	struct reset_control *rst;
 	bool resets_inverted;
 };
 
@@ -210,16 +208,16 @@ static void write_config(struct mt7621_pcie *pcie, unsigned int dev,
 	pcie_write(pcie, val, RALINK_PCI_CONFIG_DATA);
 }
 
-static inline void mt7621_perst_gpio_pcie_assert(struct mt7621_pcie *pcie)
+static inline void mt7621_rst_gpio_pcie_assert(struct mt7621_pcie_port *port)
 {
-	gpiod_set_value(pcie->perst, 0);
-	mdelay(PERST_DELAY_US);
+	if (port->gpio_rst)
+		gpiod_set_value(port->gpio_rst, 1);
 }
 
-static inline void mt7621_perst_gpio_pcie_deassert(struct mt7621_pcie *pcie)
+static inline void mt7621_rst_gpio_pcie_deassert(struct mt7621_pcie_port *port)
 {
-	gpiod_set_value(pcie->perst, 1);
-	mdelay(PERST_DELAY_US);
+	if (port->gpio_rst)
+		gpiod_set_value(port->gpio_rst, 0);
 }
 
 static inline bool mt7621_pcie_port_is_linkup(struct mt7621_pcie_port *port)
@@ -367,6 +365,11 @@ static int mt7621_pcie_parse_port(struct mt7621_pcie *pcie,
 	if (IS_ERR(port->phy))
 		return PTR_ERR(port->phy);
 
+	port->gpio_rst = devm_gpiod_get_index(dev, "reset", slot,
+					      GPIOD_OUT_LOW);
+	if (IS_ERR(port->gpio_rst))
+		dev_notice(dev, "Failed to get GPIO for PCIe%d\n", slot);
+
 	port->slot = slot;
 	port->pcie = pcie;
 
@@ -383,12 +386,6 @@ static int mt7621_pcie_parse_dt(struct mt7621_pcie *pcie)
 	struct resource regs;
 	int err;
 
-	pcie->perst = devm_gpiod_get(dev, "perst", GPIOD_OUT_HIGH);
-	if (IS_ERR(pcie->perst)) {
-		dev_err(dev, "failed to get gpio perst\n");
-		return PTR_ERR(pcie->perst);
-	}
-
 	err = of_address_to_resource(node, 0, &regs);
 	if (err) {
 		dev_err(dev, "missing \"reg\" property\n");
@@ -399,12 +396,6 @@ static int mt7621_pcie_parse_dt(struct mt7621_pcie *pcie)
 	if (IS_ERR(pcie->base))
 		return PTR_ERR(pcie->base);
 
-	pcie->rst = devm_reset_control_get_exclusive(dev, "pcie");
-	if (PTR_ERR(pcie->rst) == -EPROBE_DEFER) {
-		dev_err(dev, "failed to get pcie reset control\n");
-		return PTR_ERR(pcie->rst);
-	}
-
 	for_each_available_child_of_node(node, child) {
 		int slot;
 
@@ -458,16 +449,49 @@ static int mt7621_pcie_init_port(struct mt7621_pcie_port *port)
 	return 0;
 }
 
+static void mt7621_pcie_reset_assert(struct mt7621_pcie *pcie)
+{
+	struct mt7621_pcie_port *port;
+
+	list_for_each_entry(port, &pcie->ports, list) {
+		/* PCIe RC reset assert */
+		mt7621_control_assert(port);
+
+		/* PCIe EP reset assert */
+		mt7621_rst_gpio_pcie_assert(port);
+	}
+
+	mdelay(PERST_DELAY_US);
+}
+
+static void mt7621_pcie_reset_rc_deassert(struct mt7621_pcie *pcie)
+{
+	struct mt7621_pcie_port *port;
+
+	list_for_each_entry(port, &pcie->ports, list)
+		mt7621_control_deassert(port);
+}
+
+static void mt7621_pcie_reset_ep_deassert(struct mt7621_pcie *pcie)
+{
+	struct mt7621_pcie_port *port;
+
+	list_for_each_entry(port, &pcie->ports, list)
+		mt7621_rst_gpio_pcie_deassert(port);
+
+	mdelay(PERST_DELAY_US);
+}
+
 static void mt7621_pcie_init_ports(struct mt7621_pcie *pcie)
 {
 	struct device *dev = pcie->dev;
 	struct mt7621_pcie_port *port, *tmp;
-	u32 val = 0;
 	int err;
 
 	rt_sysc_m32(PERST_MODE_MASK, PERST_MODE_GPIO, MT7621_GPIO_MODE);
 
-	mt7621_perst_gpio_pcie_assert(pcie);
+	mt7621_pcie_reset_assert(pcie);
+	mt7621_pcie_reset_rc_deassert(pcie);
 
 	list_for_each_entry_safe(port, tmp, &pcie->ports, list) {
 		u32 slot = port->slot;
@@ -476,16 +500,10 @@ static void mt7621_pcie_init_ports(struct mt7621_pcie *pcie)
 		if (err) {
 			dev_err(dev, "Initiating port %d failed\n", slot);
 			list_del(&port->list);
-		} else {
-			val = read_config(pcie, slot, PCIE_FTS_NUM);
-			dev_info(dev, "Port %d N_FTS = %x\n", slot,
-				 (unsigned int)val);
 		}
 	}
 
-	reset_control_assert(pcie->rst);
-
-	mt7621_perst_gpio_pcie_deassert(pcie);
+	mt7621_pcie_reset_ep_deassert(pcie);
 
 	list_for_each_entry(port, &pcie->ports, list) {
 		u32 slot = port->slot;
@@ -499,8 +517,6 @@ static void mt7621_pcie_init_ports(struct mt7621_pcie *pcie)
 			port->enabled = false;
 		}
 	}
-
-	reset_control_deassert(pcie->rst);
 }
 
 static void mt7621_pcie_enable_port(struct mt7621_pcie_port *port)
-- 
2.25.1

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

* [PATCH v3 2/5] staging: mt7621-pci: change value for 'PERST_DELAY_US'
  2020-03-12  9:00 [PATCH v3 0/5] staging: mt7621-pci: re-do reset boot process Sergio Paracuellos
  2020-03-12  9:00 ` [PATCH v3 1/5] staging: mt7621-pci: use gpios for properly reset Sergio Paracuellos
@ 2020-03-12  9:00 ` Sergio Paracuellos
  2020-03-12  9:00 ` [PATCH v3 3/5] staging: mt7621-dts: make use of 'reset-gpios' property for pci Sergio Paracuellos
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 11+ messages in thread
From: Sergio Paracuellos @ 2020-03-12  9:00 UTC (permalink / raw)
  To: gregkh; +Cc: ryder.lee, driverdev-devel, weijie.gao, gerg, neil

Value of 'PERST_DELAY_US' is too high and it is ok just
to set up to 100 us.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 drivers/staging/mt7621-pci/pci-mt7621.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/mt7621-pci/pci-mt7621.c b/drivers/staging/mt7621-pci/pci-mt7621.c
index 34f6bcd53fbf..5306a0dd769f 100644
--- a/drivers/staging/mt7621-pci/pci-mt7621.c
+++ b/drivers/staging/mt7621-pci/pci-mt7621.c
@@ -86,7 +86,7 @@
 #define MEMORY_BASE			0x0
 #define PERST_MODE_MASK			GENMASK(11, 10)
 #define PERST_MODE_GPIO			BIT(10)
-#define PERST_DELAY_US			1000
+#define PERST_DELAY_US			100
 
 /**
  * struct mt7621_pcie_port - PCIe port information
-- 
2.25.1

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

* [PATCH v3 3/5] staging: mt7621-dts: make use of 'reset-gpios' property for pci
  2020-03-12  9:00 [PATCH v3 0/5] staging: mt7621-pci: re-do reset boot process Sergio Paracuellos
  2020-03-12  9:00 ` [PATCH v3 1/5] staging: mt7621-pci: use gpios for properly reset Sergio Paracuellos
  2020-03-12  9:00 ` [PATCH v3 2/5] staging: mt7621-pci: change value for 'PERST_DELAY_US' Sergio Paracuellos
@ 2020-03-12  9:00 ` Sergio Paracuellos
  2020-03-12  9:00 ` [PATCH v3 4/5] staging: mt7621-pci: bindings: update doc accordly to last changes Sergio Paracuellos
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 11+ messages in thread
From: Sergio Paracuellos @ 2020-03-12  9:00 UTC (permalink / raw)
  To: gregkh; +Cc: ryder.lee, driverdev-devel, weijie.gao, gerg, neil

Properly set pins for group pcie as 'gpio' function and declare
gpio's in the pci node to make reset stuff properly functional.
Delete no more needed general reset and previous pers gpio which
is now being used in 'reset-gpios' property.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 drivers/staging/mt7621-dts/mt7621.dtsi | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/mt7621-dts/mt7621.dtsi b/drivers/staging/mt7621-dts/mt7621.dtsi
index d89d68ffa7bc..488474153535 100644
--- a/drivers/staging/mt7621-dts/mt7621.dtsi
+++ b/drivers/staging/mt7621-dts/mt7621.dtsi
@@ -286,7 +286,7 @@ mdio0 {
 		pcie_pins: pcie0 {
 			pcie0 {
 				groups = "pcie";
-				function = "pcie rst";
+				function = "gpio";
 			};
 		};
 
@@ -512,7 +512,6 @@ pcie: pcie@1e140000 {
 		#address-cells = <3>;
 		#size-cells = <2>;
 
-		perst-gpio = <&gpio 19 GPIO_ACTIVE_HIGH>;
 		pinctrl-names = "default";
 		pinctrl-0 = <&pcie_pins>;
 
@@ -532,13 +531,17 @@ pcie: pcie@1e140000 {
 
 		status = "disabled";
 
-		resets = <&rstctrl 23 &rstctrl 24 &rstctrl 25 &rstctrl 26>;
-		reset-names = "pcie", "pcie0", "pcie1", "pcie2";
+		resets = <&rstctrl 24 &rstctrl 25 &rstctrl 26>;
+		reset-names = "pcie0", "pcie1", "pcie2";
 		clocks = <&clkctrl 24 &clkctrl 25 &clkctrl 26>;
 		clock-names = "pcie0", "pcie1", "pcie2";
 		phys = <&pcie0_phy 0>, <&pcie0_phy 1>, <&pcie1_phy 0>;
 		phy-names = "pcie-phy0", "pcie-phy1", "pcie-phy2";
 
+		reset-gpios = <&gpio 19 GPIO_ACTIVE_LOW>,
+				<&gpio 8 GPIO_ACTIVE_LOW>,
+				<&gpio 7 GPIO_ACTIVE_LOW>;
+
 		pcie@0,0 {
 			reg = <0x0000 0 0 0 0>;
 			#address-cells = <3>;
-- 
2.25.1

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

* [PATCH v3 4/5] staging: mt7621-pci: bindings: update doc accordly to last changes
  2020-03-12  9:00 [PATCH v3 0/5] staging: mt7621-pci: re-do reset boot process Sergio Paracuellos
                   ` (2 preceding siblings ...)
  2020-03-12  9:00 ` [PATCH v3 3/5] staging: mt7621-dts: make use of 'reset-gpios' property for pci Sergio Paracuellos
@ 2020-03-12  9:00 ` Sergio Paracuellos
  2020-03-12  9:00 ` [PATCH v3 5/5] staging: mt7621-pci: release gpios after pci initialization Sergio Paracuellos
  2020-03-13  3:51 ` [PATCH v3 0/5] staging: mt7621-pci: re-do reset boot process Greg Ungerer
  5 siblings, 0 replies; 11+ messages in thread
From: Sergio Paracuellos @ 2020-03-12  9:00 UTC (permalink / raw)
  To: gregkh; +Cc: ryder.lee, driverdev-devel, weijie.gao, gerg, neil

Properly update bindings documentation with added 'reset-gpios'
property. Delete also 'perst-gpio' which is not being used anymore.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 drivers/staging/mt7621-pci/mediatek,mt7621-pci.txt | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/mt7621-pci/mediatek,mt7621-pci.txt b/drivers/staging/mt7621-pci/mediatek,mt7621-pci.txt
index 604ec813bd45..327a68267309 100644
--- a/drivers/staging/mt7621-pci/mediatek,mt7621-pci.txt
+++ b/drivers/staging/mt7621-pci/mediatek,mt7621-pci.txt
@@ -6,7 +6,6 @@ Required properties:
 - reg: Base addresses and lengths of the PCIe subsys and root ports.
 - bus-range: Range of bus numbers associated with this controller.
 - #address-cells: Address representation for root ports (must be 3)
-- perst-gpio: PCIe reset signal line.
 - pinctrl-names : The pin control state names.
 - pinctrl-0: The "default" pinctrl state.
 - #size-cells: Size representation for root ports (must be 2)
@@ -24,6 +23,7 @@ Required properties:
   See ../clocks/clock-bindings.txt for details.
 - clock-names: Must be "pcie0", "pcie1", "pcieN"... based on the number of
   root ports.
+- reset-gpios: GPIO specs for the reset pins.
 
 In addition, the device tree node must have sub-nodes describing each PCIe port
 interface, having the following mandatory properties:
@@ -49,7 +49,6 @@ Example for MT7621:
 		#address-cells = <3>;
 		#size-cells = <2>;
 
-		perst-gpio = <&gpio 19 GPIO_ACTIVE_HIGH>;
 		pinctrl-names = "default";
 		pinctrl-0 = <&pcie_pins>;
 
@@ -74,6 +73,10 @@ Example for MT7621:
 		clocks = <&clkctrl 24 &clkctrl 25 &clkctrl 26>;
 		clock-names = "pcie0", "pcie1", "pcie2";
 
+		reset-gpios = <&gpio 19 GPIO_ACTIVE_LOW>,
+				<&gpio 8 GPIO_ACTIVE_LOW>,
+				<&gpio 7 GPIO_ACTIVE_LOW>;
+
 		pcie@0,0 {
 			reg = <0x0000 0 0 0 0>;
 			#address-cells = <3>;
-- 
2.25.1

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

* [PATCH v3 5/5] staging: mt7621-pci: release gpios after pci initialization
  2020-03-12  9:00 [PATCH v3 0/5] staging: mt7621-pci: re-do reset boot process Sergio Paracuellos
                   ` (3 preceding siblings ...)
  2020-03-12  9:00 ` [PATCH v3 4/5] staging: mt7621-pci: bindings: update doc accordly to last changes Sergio Paracuellos
@ 2020-03-12  9:00 ` Sergio Paracuellos
  2020-03-13  3:51 ` [PATCH v3 0/5] staging: mt7621-pci: re-do reset boot process Greg Ungerer
  5 siblings, 0 replies; 11+ messages in thread
From: Sergio Paracuellos @ 2020-03-12  9:00 UTC (permalink / raw)
  To: gregkh; +Cc: ryder.lee, driverdev-devel, weijie.gao, gerg, neil

R3G's LEDs fail to initialize because one of them uses GPIO8
Hence, release the GPIO resources after PCIe initialization
and properly release also in driver error path.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 drivers/staging/mt7621-pci/pci-mt7621.c | 23 ++++++++++++++++++-----
 1 file changed, 18 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/mt7621-pci/pci-mt7621.c b/drivers/staging/mt7621-pci/pci-mt7621.c
index 5306a0dd769f..6ea284942b6f 100644
--- a/drivers/staging/mt7621-pci/pci-mt7621.c
+++ b/drivers/staging/mt7621-pci/pci-mt7621.c
@@ -482,6 +482,15 @@ static void mt7621_pcie_reset_ep_deassert(struct mt7621_pcie *pcie)
 	mdelay(PERST_DELAY_US);
 }
 
+static void mt7621_pcie_release_gpios(struct mt7621_pcie *pcie)
+{
+	struct mt7621_pcie_port *port;
+
+	list_for_each_entry(port, &pcie->ports, list)
+		if (port->gpio_rst)
+			gpiod_put(port->gpio_rst);
+}
+
 static void mt7621_pcie_init_ports(struct mt7621_pcie *pcie)
 {
 	struct device *dev = pcie->dev;
@@ -681,7 +690,8 @@ static int mt7621_pci_probe(struct platform_device *pdev)
 	err = mt7621_pcie_init_virtual_bridges(pcie);
 	if (err) {
 		dev_err(dev, "Nothing is connected in virtual bridges. Exiting...");
-		return 0;
+		err = 0;
+		goto out_release_gpios;
 	}
 
 	mt7621_pcie_enable_ports(pcie);
@@ -689,7 +699,7 @@ static int mt7621_pci_probe(struct platform_device *pdev)
 	err = mt7621_pci_parse_request_of_pci_ranges(pcie);
 	if (err) {
 		dev_err(dev, "Error requesting pci resources from ranges");
-		return err;
+		goto out_release_gpios;
 	}
 
 	setup_cm_memory_region(pcie);
@@ -697,16 +707,19 @@ static int mt7621_pci_probe(struct platform_device *pdev)
 	err = mt7621_pcie_request_resources(pcie, &res);
 	if (err) {
 		dev_err(dev, "Error requesting resources\n");
-		return err;
+		goto out_release_gpios;
 	}
 
 	err = mt7621_pcie_register_host(bridge, &res);
 	if (err) {
 		dev_err(dev, "Error registering host\n");
-		return err;
+		goto out_release_gpios;
 	}
 
-	return 0;
+out_release_gpios:
+	mt7621_pcie_release_gpios(pcie);
+
+	return err;
 }
 
 static const struct of_device_id mt7621_pci_ids[] = {
-- 
2.25.1

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

* Re: [PATCH v3 0/5] staging: mt7621-pci: re-do reset boot process
  2020-03-12  9:00 [PATCH v3 0/5] staging: mt7621-pci: re-do reset boot process Sergio Paracuellos
                   ` (4 preceding siblings ...)
  2020-03-12  9:00 ` [PATCH v3 5/5] staging: mt7621-pci: release gpios after pci initialization Sergio Paracuellos
@ 2020-03-13  3:51 ` Greg Ungerer
  2020-03-13  5:45   ` Sergio Paracuellos
  5 siblings, 1 reply; 11+ messages in thread
From: Greg Ungerer @ 2020-03-13  3:51 UTC (permalink / raw)
  To: Sergio Paracuellos, gregkh; +Cc: neil, driverdev-devel, weijie.gao, ryder.lee

Hi Sergio,

On 12/3/20 7:00 pm, Sergio Paracuellos wrote:
> Some time ago Greg Ungerer reported some random hangs using
> the staging mt7621-pci driver:
> 
> See:
> * http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2019-June/134947.html
> 
> Try to fix that is the main motivation of this patch series.
> 
> Also in openwrt there is a driver for mt7621-pci which seems was rewritten
> from scratch (for kernel 4.14) by Ryder Lee and Weijie Gao from mediatek.
> There the approach for reset assert-deassert process is to set as 'gpio'
> the function for all the 'pcie' group for the pinctrl driver and use those
> gpio's as a reset for the end points. The driver I am talking about is still
> using legacy pci and legacy gpio kernel interfaces. IMHO, the correct thing
> to do is make this staging driver properly clean and functional and put it
> in its correct place in the mainline.
> 
> See:
> * https://gist.github.com/dengqf6/7a9e9b4032d99f1a91dd9256c8a65c36
> 
> Because of all of this this patch series tries to avoid random hangs of boot
> trying to use the 'reset-gpios' approach.
> 
> Changes are being tested by openwrt people and seems to work.

Thanks for the updated patch.

I applied the patches to the staging git tree for testing.
I have seen a couple of different problems on boot up:

...
rt2880-pinmux pinctrl: pcie is already enabled
mt7621-pci 1e140000.pcie: Error applying setting, reverse things back
mt7621-pci 1e140000.pcie: Failed to get GPIO for PCIe1
mt7621-pci 1e140000.pcie: Failed to get GPIO for PCIe2
gpiod_set_value: invalid GPIO (errorpointer)
gpiod_set_value: invalid GPIO (errorpointer)
mt7621-pci-phy 1e149000.pcie-phy: Xtal is 40MHz
mt7621-pci-phy 1e149000.pcie-phy: Xtal is 40MHz
mt7621-pci-phy 1e14a000.pcie-phy: Xtal is 40MHz
gpiod_set_value: invalid GPIO (errorpointer)
gpiod_set_value: invalid GPIO (errorpointer)
mt7621-pci 1e140000.pcie: pcie1 no card, disable it (RST & CLK)
mt7621-pci 1e140000.pcie: pcie2 no card, disable it (RST & CLK)
mt7621-pci 1e140000.pcie: PCIE0 enabled
mt7621-pci 1e140000.pcie: PCI coherence region base: 0x60000000, mask/settings: 0xf0000002
mt7621-pci 1e140000.pcie: PCI host bridge to bus 0000:00
pci_bus 0000:00: root bus resource [io  0xffffffff]
pci_bus 0000:00: root bus resource [mem 0x60000000-0x6fffffff]
pci_bus 0000:00: root bus resource [bus 00-ff]
pci 0000:00:00.0: [0e8d:0801] type 01 class 0x060400
pci 0000:00:00.0: reg 0x10: [mem 0x00000000-0x7fffffff]
pci 0000:00:00.0: reg 0x14: [mem 0x00000000-0x0000ffff]
pci 0000:00:00.0: supports D1
pci 0000:00:00.0: PME# supported from D0 D1 D3hot
pci 0000:00:00.0: bridge configuration invalid ([bus 00-00]), reconfiguring
pci 0000:01:00.0: [168c:003c] type 00 class 0x028000
pci 0000:01:00.0: reg 0x10: [mem 0x00000000-0x001fffff 64bit]
pci 0000:01:00.0: reg 0x30: [mem 0x00000000-0x0000ffff pref]
pci 0000:01:00.0: supports D1 D2
pci 0000:00:00.0: PCI bridge to [bus 01-ff]
pci 0000:00:00.0:   bridge window [io  0x0000-0x0fff]
pci 0000:00:00.0:   bridge window [mem 0x00000000-0x000fffff]
pci 0000:00:00.0:   bridge window [mem 0x00000000-0x000fffff pref]
pci_bus 0000:01: busn_res: [bus 01-ff] end is updated to 01
pci 0000:00:00.0: BAR 0: no space for [mem size 0x80000000]
pci 0000:00:00.0: BAR 0: failed to assign [mem size 0x80000000]
pci 0000:00:00.0: BAR 8: assigned [mem 0x60000000-0x601fffff]
pci 0000:00:00.0: BAR 9: assigned [mem 0x60200000-0x602fffff pref]
pci 0000:00:00.0: BAR 1: assigned [mem 0x60300000-0x6030ffff]
pci 0000:00:00.0: BAR 7: no space for [io  size 0x1000]
pci 0000:00:00.0: BAR 7: failed to assign [io  size 0x1000]
pci 0000:01:00.0: BAR 0: assigned [mem 0x60000000-0x601fffff 64bit]
pci 0000:01:00.0: BAR 6: assigned [mem 0x60200000-0x6020ffff pref]
pci 0000:00:00.0: PCI bridge to [bus 01]
pci 0000:00:00.0:   bridge window [mem 0x60000000-0x601fffff]
pci 0000:00:00.0:   bridge window [mem 0x60200000-0x602fffff pref]
pcieport 0000:00:00.0: of_irq_parse_pci: failed with rc=-22
pcieport 0000:00:00.0: enabling device (0004 -> 0006)
CPU 2 Unable to handle kernel paging request at virtual address fffffff0, epc == 8039c7b0, ra == 804fe128
Oops[#1]:
CPU: 2 PID: 107 Comm: kworker/2:1 Not tainted 5.6.0-rc3-00180-gc15e7f072288-dirty #2
Workqueue: events deferred_probe_work_func
$ 0   : 00000000 00000001 820d6984 00000001
$ 4   : fffffff0 00000001 00000000 803a0000
$ 8   : 00000024 ffffffff 00000001 302e3030
$12   : 00000000 8e26fbd8 ffffffff 00000020
$16   : 820d6980 820e0250 00000000 8e26fcd0
$20   : 820e01e0 809a0000 820e0228 8fd47e10
$24   : 00000000 00000020
$28   : 8e26e000 8e26fc70 8e26fcf8 804fe128
Hi    : 00000125
Lo    : 122f2000
epc   : 8039c7b0 gpiod_free+0x14/0x6c
ra    : 804fe128 mt7621_pci_probe+0x700/0xcd8
Status: 1100fc03	KERNEL EXL IE
Cause : 00800008 (ExcCode 02)
BadVA : fffffff0
PrId  : 0001992f (MIPS 1004Kc)
Modules linked in:
Process kworker/2:1 (pid: 107, threadinfo=(ptrval), task=(ptrval), tls=00000000)
Stack : 820e0250 809e2aa0 820d6380 820e0250 820d6980 804fe128 1e160000 00000001
         60000000 00000000 f0000002 00000000 00000000 809a0000 8fd47e10 808d63d4
         80930000 808d6260 808d0000 808d61fc 808c0000 8fd47e10 808d6208 808d0000
         8e26fcd0 8e26fcd0 809e2aa0 809db510 809db510 00000006 00000001 00000000
         00000000 00000000 01000000 1e1440ff 00000000 00000000 1e160000 00000000
         ...
Call Trace:
[<8039c7b0>] gpiod_free+0x14/0x6c
[<804fe128>] mt7621_pci_probe+0x700/0xcd8
[<80402ab8>] platform_drv_probe+0x40/0x94
[<80400a74>] really_probe+0x104/0x364
[<803feb74>] bus_for_each_drv+0x84/0xdc
[<80400924>] __device_attach+0xdc/0x120
[<803ffb5c>] bus_probe_device+0xa0/0xbc
[<80400124>] deferred_probe_work_func+0x7c/0xbc
[<800420e8>] process_one_work+0x230/0x450
[<80042638>] worker_thread+0x330/0x5fc
[<80048eb0>] kthread+0x12c/0x134
[<80007438>] ret_from_kernel_thread+0x14/0x1c
Code: 27bdffe8  afb00010  afbf0014 <8c830000> 10600005  00808025  0c0e6efb  00000000  14400007

---[ end trace d9831fec9efd296d ]---



And on another boot:

...
rt2880-pinmux pinctrl: pcie is already enabled
mt7621-pci 1e140000.pcie: Error applying setting, reverse things back
mt7621-pci 1e140000.pcie: Failed to get GPIO for PCIe1
mt7621-pci 1e140000.pcie: Failed to get GPIO for PCIe2
gpiod_set_value: invalid GPIO (errorpointer)
gpiod_set_value: invalid GPIO (errorpointer)
mt7621-pci-phy 1e149000.pcie-phy: Xtal is 40MHz
mt7621-pci-phy 1e149000.pcie-phy: Xtal is 40MHz
mt7621-pci-phy 1e14a000.pcie-phy: Xtal is 40MHz
gpiod_set_value: invalid GPIO (errorpointer)
gpiod_set_value: invalid GPIO (errorpointer)
mt7621-pci 1e140000.pcie: pcie0 no card, disable it (RST & CLK)
mt7621-pci 1e140000.pcie: pcie1 no card, disable it (RST & CLK)
mt7621-pci 1e140000.pcie: pcie2 no card, disable it (RST & CLK)
mt7621-pci 1e140000.pcie: Nothing is connected in virtual bridges. Exiting...
CPU 1 Unable to handle kernel paging request at virtual address fffffff0, epc == 8039c7b0, ra == 804fe128
Oops[#1]:
CPU: 1 PID: 103 Comm: kworker/1:1 Not tainted 5.6.0-rc3-00180-gc15e7f072288-dirty #2
Workqueue: events deferred_probe_work_func
$ 0   : 00000000 00000001 82051e84 00000001
$ 4   : fffffff0 00000001 00000000 803a0000
$ 8   : 2e000000 80940000 00000018 000000d6
$12   : 6976206e 00000000 00000000 61757472
$16   : 82051e80 820d4250 00000000 8fd6dcd0
$20   : 820d41e0 820d424c 00000000 809a0000
$24   : 00000008 803ef8f4
$28   : 8fd6c000 8fd6dc70 808d6280 804fe128
Hi    : 00000125
Lo    : 122f2000
epc   : 8039c7b0 gpiod_free+0x14/0x6c
ra    : 804fe128 mt7621_pci_probe+0x700/0xcd8
Status: 1100fc03	KERNEL EXL IE
Cause : 00800008 (ExcCode 02)
BadVA : fffffff0
PrId  : 0001992f (MIPS 1004Kc)
Modules linked in:
Process kworker/1:1 (pid: 103, threadinfo=(ptrval), task=(ptrval), tls=00000000)
Stack : 8fd6dc90 808d62a8 82051880 820d4250 82051e80 804fe128 809d0000 00000001
         00000001 00000001 00000000 00000001 00000000 8fd47e10 8fd47e10 808d63d4
         80930000 808d6260 808d0000 808d61fc 808c0000 8fd47e10 808d6208 808d0000
         8fd6dcd0 8fd6dcd0 1e140000 1e1400ff 809e2af8 00000200 00000000 00000000
         00000000 00000000 1e144000 1e1440ff 809e2af8 00000200 00000000 00000000
         ...
Call Trace:
[<8039c7b0>] gpiod_free+0x14/0x6c
[<804fe128>] mt7621_pci_probe+0x700/0xcd8
[<80402ab8>] platform_drv_probe+0x40/0x94
[<80400a74>] really_probe+0x104/0x364
[<803feb74>] bus_for_each_drv+0x84/0xdc
[<80400924>] __device_attach+0xdc/0x120
[<803ffb5c>] bus_probe_device+0xa0/0xbc
[<80400124>] deferred_probe_work_func+0x7c/0xbc
[<800420e8>] process_one_work+0x230/0x450
[<80042638>] worker_thread+0x330/0x5fc
[<80048eb0>] kthread+0x12c/0x134
[<80007438>] ret_from_kernel_thread+0x14/0x1c
Code: 27bdffe8  afb00010  afbf0014 <8c830000> 10600005  00808025  0c0e6efb  00000000  14400007

---[ end trace 5affc5903364c81f ]---



Regards
Greg


> Hope this helps.
> 
> Changes in v3:
>      * Avoid to fail if gpio descriptor fails on get.
>      * re-do PATCH 1 commit message.
>      * Take into account gpio low polarity on request and assert and deassert.
>      * Review error path of driver to properly release gpio's resources.
> 
> Changes in v2:
>      * restore configuration for pers mode to GPIO.
>      * Avoid to read FTS_NUM register in reset state.
>      * Release gpio's patch added
> 
> Best regards,
>      Sergio Paracuellos
> 
> Sergio Paracuellos (5):
>    staging: mt7621-pci: use gpios for properly reset
>    staging: mt7621-pci: change value for 'PERST_DELAY_US'
>    staging: mt7621-dts: make use of 'reset-gpios' property for pci
>    staging: mt7621-pci: bindings: update doc accordly to last changes
>    staging: mt7621-pci: release gpios after pci initialization
> 
>   drivers/staging/mt7621-dts/mt7621.dtsi        |  11 +-
>   .../mt7621-pci/mediatek,mt7621-pci.txt        |   7 +-
>   drivers/staging/mt7621-pci/pci-mt7621.c       | 107 +++++++++++-------
>   3 files changed, 80 insertions(+), 45 deletions(-)
> 
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

* Re: [PATCH v3 0/5] staging: mt7621-pci: re-do reset boot process
  2020-03-13  3:51 ` [PATCH v3 0/5] staging: mt7621-pci: re-do reset boot process Greg Ungerer
@ 2020-03-13  5:45   ` Sergio Paracuellos
  2020-03-13  6:31     ` Sergio Paracuellos
  2020-03-15 12:32     ` Greg Ungerer
  0 siblings, 2 replies; 11+ messages in thread
From: Sergio Paracuellos @ 2020-03-13  5:45 UTC (permalink / raw)
  To: Greg Ungerer; +Cc: ryder.lee, Greg KH, driverdev-devel, weijie.gao, NeilBrown

Hi Greg,

On Fri, Mar 13, 2020 at 4:51 AM Greg Ungerer <gerg@kernel.org> wrote:
[snip]
>
> I applied the patches to the staging git tree for testing.
> I have seen a couple of different problems on boot up:
>
> ...
> rt2880-pinmux pinctrl: pcie is already enabled
> mt7621-pci 1e140000.pcie: Error applying setting, reverse things back
> mt7621-pci 1e140000.pcie: Failed to get GPIO for PCIe1
> mt7621-pci 1e140000.pcie: Failed to get GPIO for PCIe2
> gpiod_set_value: invalid GPIO (errorpointer)
> gpiod_set_value: invalid GPIO (errorpointer)

So the not grabbed gpio's are error pointers instead of NULL. What
happen if you just
set them to NULL? Something like:

diff --git a/drivers/staging/mt7621-pci/pci-mt7621.c
b/drivers/staging/mt7621-pci/pci-mt7621.c
index 6ea284942b6f..03c1f057b29f 100644
--- a/drivers/staging/mt7621-pci/pci-mt7621.c
+++ b/drivers/staging/mt7621-pci/pci-mt7621.c
@@ -367,8 +367,10 @@ static int mt7621_pcie_parse_port(struct mt7621_pcie *pcie,

        port->gpio_rst = devm_gpiod_get_index(dev, "reset", slot,
                                              GPIOD_OUT_LOW);
-       if (IS_ERR(port->gpio_rst))
+       if (IS_ERR(port->gpio_rst)) {
+               port->gpio_rst = NULL;
                dev_notice(dev, "Failed to get GPIO for PCIe%d\n", slot);
+       }

Another possibility would be to use 'gpio_is_valid' getting int gpio
value from descriptor using 'gpio_to_desc' but I think we were mixing
legacy gpio APIS with the descriptor interface one which sound not
good.

> mt7621-pci-phy 1e149000.pcie-phy: Xtal is 40MHz
> mt7621-pci-phy 1e149000.pcie-phy: Xtal is 40MHz
> mt7621-pci-phy 1e14a000.pcie-phy: Xtal is 40MHz
> gpiod_set_value: invalid GPIO (errorpointer)
> gpiod_set_value: invalid GPIO (errorpointer)
> mt7621-pci 1e140000.pcie: pcie1 no card, disable it (RST & CLK)
> mt7621-pci 1e140000.pcie: pcie2 no card, disable it (RST & CLK)
> mt7621-pci 1e140000.pcie: PCIE0 enabled
> mt7621-pci 1e140000.pcie: PCI coherence region base: 0x60000000, mask/settings: 0xf0000002
> mt7621-pci 1e140000.pcie: PCI host bridge to bus 0000:00
> pci_bus 0000:00: root bus resource [io  0xffffffff]
> pci_bus 0000:00: root bus resource [mem 0x60000000-0x6fffffff]
> pci_bus 0000:00: root bus resource [bus 00-ff]
> pci 0000:00:00.0: [0e8d:0801] type 01 class 0x060400
> pci 0000:00:00.0: reg 0x10: [mem 0x00000000-0x7fffffff]
> pci 0000:00:00.0: reg 0x14: [mem 0x00000000-0x0000ffff]
> pci 0000:00:00.0: supports D1
> pci 0000:00:00.0: PME# supported from D0 D1 D3hot
> pci 0000:00:00.0: bridge configuration invalid ([bus 00-00]), reconfiguring
> pci 0000:01:00.0: [168c:003c] type 00 class 0x028000
> pci 0000:01:00.0: reg 0x10: [mem 0x00000000-0x001fffff 64bit]
> pci 0000:01:00.0: reg 0x30: [mem 0x00000000-0x0000ffff pref]
> pci 0000:01:00.0: supports D1 D2
> pci 0000:00:00.0: PCI bridge to [bus 01-ff]
> pci 0000:00:00.0:   bridge window [io  0x0000-0x0fff]
> pci 0000:00:00.0:   bridge window [mem 0x00000000-0x000fffff]
> pci 0000:00:00.0:   bridge window [mem 0x00000000-0x000fffff pref]
> pci_bus 0000:01: busn_res: [bus 01-ff] end is updated to 01
> pci 0000:00:00.0: BAR 0: no space for [mem size 0x80000000]
> pci 0000:00:00.0: BAR 0: failed to assign [mem size 0x80000000]
> pci 0000:00:00.0: BAR 8: assigned [mem 0x60000000-0x601fffff]
> pci 0000:00:00.0: BAR 9: assigned [mem 0x60200000-0x602fffff pref]
> pci 0000:00:00.0: BAR 1: assigned [mem 0x60300000-0x6030ffff]
> pci 0000:00:00.0: BAR 7: no space for [io  size 0x1000]
> pci 0000:00:00.0: BAR 7: failed to assign [io  size 0x1000]
> pci 0000:01:00.0: BAR 0: assigned [mem 0x60000000-0x601fffff 64bit]
> pci 0000:01:00.0: BAR 6: assigned [mem 0x60200000-0x6020ffff pref]
> pci 0000:00:00.0: PCI bridge to [bus 01]
> pci 0000:00:00.0:   bridge window [mem 0x60000000-0x601fffff]
> pci 0000:00:00.0:   bridge window [mem 0x60200000-0x602fffff pref]
> pcieport 0000:00:00.0: of_irq_parse_pci: failed with rc=-22
> pcieport 0000:00:00.0: enabling device (0004 -> 0006)
> CPU 2 Unable to handle kernel paging request at virtual address fffffff0, epc == 8039c7b0, ra == 804fe128
> Oops[#1]:
> CPU: 2 PID: 107 Comm: kworker/2:1 Not tainted 5.6.0-rc3-00180-gc15e7f072288-dirty #2
> Workqueue: events deferred_probe_work_func
> $ 0   : 00000000 00000001 820d6984 00000001
> $ 4   : fffffff0 00000001 00000000 803a0000
> $ 8   : 00000024 ffffffff 00000001 302e3030
> $12   : 00000000 8e26fbd8 ffffffff 00000020
> $16   : 820d6980 820e0250 00000000 8e26fcd0
> $20   : 820e01e0 809a0000 820e0228 8fd47e10
> $24   : 00000000 00000020
> $28   : 8e26e000 8e26fc70 8e26fcf8 804fe128
> Hi    : 00000125
> Lo    : 122f2000
> epc   : 8039c7b0 gpiod_free+0x14/0x6c
> ra    : 804fe128 mt7621_pci_probe+0x700/0xcd8
> Status: 1100fc03        KERNEL EXL IE
> Cause : 00800008 (ExcCode 02)
> BadVA : fffffff0
> PrId  : 0001992f (MIPS 1004Kc)
> Modules linked in:
> Process kworker/2:1 (pid: 107, threadinfo=(ptrval), task=(ptrval), tls=00000000)
> Stack : 820e0250 809e2aa0 820d6380 820e0250 820d6980 804fe128 1e160000 00000001
>          60000000 00000000 f0000002 00000000 00000000 809a0000 8fd47e10 808d63d4
>          80930000 808d6260 808d0000 808d61fc 808c0000 8fd47e10 808d6208 808d0000
>          8e26fcd0 8e26fcd0 809e2aa0 809db510 809db510 00000006 00000001 00000000
>          00000000 00000000 01000000 1e1440ff 00000000 00000000 1e160000 00000000
>          ...
> Call Trace:
> [<8039c7b0>] gpiod_free+0x14/0x6c
> [<804fe128>] mt7621_pci_probe+0x700/0xcd8
> [<80402ab8>] platform_drv_probe+0x40/0x94
> [<80400a74>] really_probe+0x104/0x364
> [<803feb74>] bus_for_each_drv+0x84/0xdc
> [<80400924>] __device_attach+0xdc/0x120
> [<803ffb5c>] bus_probe_device+0xa0/0xbc
> [<80400124>] deferred_probe_work_func+0x7c/0xbc
> [<800420e8>] process_one_work+0x230/0x450
> [<80042638>] worker_thread+0x330/0x5fc
> [<80048eb0>] kthread+0x12c/0x134
> [<80007438>] ret_from_kernel_thread+0x14/0x1c
> Code: 27bdffe8  afb00010  afbf0014 <8c830000> 10600005  00808025  0c0e6efb  00000000  14400007
>
> ---[ end trace d9831fec9efd296d ]---
>

This is to be because the 'gpio_dput' makes internally a gpiod_free in
gpio's and desc is an error pointer descriptor instead of NULL.
Expected, looking the previous traces.

>
>
> And on another boot:
>
> ...
> rt2880-pinmux pinctrl: pcie is already enabled
> mt7621-pci 1e140000.pcie: Error applying setting, reverse things back
> mt7621-pci 1e140000.pcie: Failed to get GPIO for PCIe1
> mt7621-pci 1e140000.pcie: Failed to get GPIO for PCIe2
> gpiod_set_value: invalid GPIO (errorpointer)
> gpiod_set_value: invalid GPIO (errorpointer)
> mt7621-pci-phy 1e149000.pcie-phy: Xtal is 40MHz
> mt7621-pci-phy 1e149000.pcie-phy: Xtal is 40MHz
> mt7621-pci-phy 1e14a000.pcie-phy: Xtal is 40MHz
> gpiod_set_value: invalid GPIO (errorpointer)
> gpiod_set_value: invalid GPIO (errorpointer)
> mt7621-pci 1e140000.pcie: pcie0 no card, disable it (RST & CLK)
> mt7621-pci 1e140000.pcie: pcie1 no card, disable it (RST & CLK)
> mt7621-pci 1e140000.pcie: pcie2 no card, disable it (RST & CLK)
> mt7621-pci 1e140000.pcie: Nothing is connected in virtual bridges. Exiting...

Maybe 100 us is not enough? We should focus first in avoiding the boot
hangs and the we can play with the

#define PERST_DELAY_US                  100

This series change it from 1000 to 100. Maybe you can try to put
another value if hangs dissapear...

> CPU 1 Unable to handle kernel paging request at virtual address fffffff0, epc == 8039c7b0, ra == 804fe128
> Oops[#1]:
> CPU: 1 PID: 103 Comm: kworker/1:1 Not tainted 5.6.0-rc3-00180-gc15e7f072288-dirty #2
> Workqueue: events deferred_probe_work_func
> $ 0   : 00000000 00000001 82051e84 00000001
> $ 4   : fffffff0 00000001 00000000 803a0000
> $ 8   : 2e000000 80940000 00000018 000000d6
> $12   : 6976206e 00000000 00000000 61757472
> $16   : 82051e80 820d4250 00000000 8fd6dcd0
> $20   : 820d41e0 820d424c 00000000 809a0000
> $24   : 00000008 803ef8f4
> $28   : 8fd6c000 8fd6dc70 808d6280 804fe128
> Hi    : 00000125
> Lo    : 122f2000
> epc   : 8039c7b0 gpiod_free+0x14/0x6c
> ra    : 804fe128 mt7621_pci_probe+0x700/0xcd8
> Status: 1100fc03        KERNEL EXL IE
> Cause : 00800008 (ExcCode 02)
> BadVA : fffffff0
> PrId  : 0001992f (MIPS 1004Kc)
> Modules linked in:
> Process kworker/1:1 (pid: 103, threadinfo=(ptrval), task=(ptrval), tls=00000000)
> Stack : 8fd6dc90 808d62a8 82051880 820d4250 82051e80 804fe128 809d0000 00000001
>          00000001 00000001 00000000 00000001 00000000 8fd47e10 8fd47e10 808d63d4
>          80930000 808d6260 808d0000 808d61fc 808c0000 8fd47e10 808d6208 808d0000
>          8fd6dcd0 8fd6dcd0 1e140000 1e1400ff 809e2af8 00000200 00000000 00000000
>          00000000 00000000 1e144000 1e1440ff 809e2af8 00000200 00000000 00000000
>          ...
> Call Trace:
> [<8039c7b0>] gpiod_free+0x14/0x6c
> [<804fe128>] mt7621_pci_probe+0x700/0xcd8
> [<80402ab8>] platform_drv_probe+0x40/0x94
> [<80400a74>] really_probe+0x104/0x364
> [<803feb74>] bus_for_each_drv+0x84/0xdc
> [<80400924>] __device_attach+0xdc/0x120
> [<803ffb5c>] bus_probe_device+0xa0/0xbc
> [<80400124>] deferred_probe_work_func+0x7c/0xbc
> [<800420e8>] process_one_work+0x230/0x450
> [<80042638>] worker_thread+0x330/0x5fc
> [<80048eb0>] kthread+0x12c/0x134
> [<80007438>] ret_from_kernel_thread+0x14/0x1c
> Code: 27bdffe8  afb00010  afbf0014 <8c830000> 10600005  00808025  0c0e6efb  00000000  14400007
>
> ---[ end trace 5affc5903364c81f ]---
>
Again, the 'gpio_dput' makes internally a gpiod_free in gpio's and
desc is an error pointer descriptor instead of NULL.

>
> Regards
> Greg

Thanks for testing this series and letting me know.

Best regards,
    Sergio Paracuellos
>
>
> > Hope this helps.
> >
> > Changes in v3:
> >      * Avoid to fail if gpio descriptor fails on get.
> >      * re-do PATCH 1 commit message.
> >      * Take into account gpio low polarity on request and assert and deassert.
> >      * Review error path of driver to properly release gpio's resources.
> >
> > Changes in v2:
> >      * restore configuration for pers mode to GPIO.
> >      * Avoid to read FTS_NUM register in reset state.
> >      * Release gpio's patch added
> >
> > Best regards,
> >      Sergio Paracuellos
> >
> > Sergio Paracuellos (5):
> >    staging: mt7621-pci: use gpios for properly reset
> >    staging: mt7621-pci: change value for 'PERST_DELAY_US'
> >    staging: mt7621-dts: make use of 'reset-gpios' property for pci
> >    staging: mt7621-pci: bindings: update doc accordly to last changes
> >    staging: mt7621-pci: release gpios after pci initialization
> >
> >   drivers/staging/mt7621-dts/mt7621.dtsi        |  11 +-
> >   .../mt7621-pci/mediatek,mt7621-pci.txt        |   7 +-
> >   drivers/staging/mt7621-pci/pci-mt7621.c       | 107 +++++++++++-------
> >   3 files changed, 80 insertions(+), 45 deletions(-)
> >
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

* Re: [PATCH v3 0/5] staging: mt7621-pci: re-do reset boot process
  2020-03-13  5:45   ` Sergio Paracuellos
@ 2020-03-13  6:31     ` Sergio Paracuellos
  2020-03-15 12:32     ` Greg Ungerer
  1 sibling, 0 replies; 11+ messages in thread
From: Sergio Paracuellos @ 2020-03-13  6:31 UTC (permalink / raw)
  To: Greg Ungerer; +Cc: ryder.lee, Greg KH, driverdev-devel, weijie.gao, NeilBrown

Hi again Greg,

On Fri, Mar 13, 2020 at 6:45 AM Sergio Paracuellos
<sergio.paracuellos@gmail.com> wrote:
>
> Hi Greg,
>
> On Fri, Mar 13, 2020 at 4:51 AM Greg Ungerer <gerg@kernel.org> wrote:
> [snip]
> >
> > I applied the patches to the staging git tree for testing.
> > I have seen a couple of different problems on boot up:
> >
> > ...
> > rt2880-pinmux pinctrl: pcie is already enabled
> > mt7621-pci 1e140000.pcie: Error applying setting, reverse things back
> > mt7621-pci 1e140000.pcie: Failed to get GPIO for PCIe1
> > mt7621-pci 1e140000.pcie: Failed to get GPIO for PCIe2
> > gpiod_set_value: invalid GPIO (errorpointer)
> > gpiod_set_value: invalid GPIO (errorpointer)
>
> So the not grabbed gpio's are error pointers instead of NULL. What
> happen if you just
> set them to NULL? Something like:
>
> diff --git a/drivers/staging/mt7621-pci/pci-mt7621.c
> b/drivers/staging/mt7621-pci/pci-mt7621.c
> index 6ea284942b6f..03c1f057b29f 100644
> --- a/drivers/staging/mt7621-pci/pci-mt7621.c
> +++ b/drivers/staging/mt7621-pci/pci-mt7621.c
> @@ -367,8 +367,10 @@ static int mt7621_pcie_parse_port(struct mt7621_pcie *pcie,
>
>         port->gpio_rst = devm_gpiod_get_index(dev, "reset", slot,
>                                               GPIOD_OUT_LOW);
> -       if (IS_ERR(port->gpio_rst))
> +       if (IS_ERR(port->gpio_rst)) {
> +               port->gpio_rst = NULL;
>                 dev_notice(dev, "Failed to get GPIO for PCIe%d\n", slot);
> +       }
>
> Another possibility would be to use 'gpio_is_valid' getting int gpio
> value from descriptor using 'gpio_to_desc' but I think we were mixing
> legacy gpio APIS with the descriptor interface one which sound not
> good.

Maybe just change devm_gpiod_get_index with
devm_gpiod_get_index_optional could be enough to get all of this
working as it is.

-       port->gpio_rst = devm_gpiod_get_index(dev, "reset", slot,
-                                             GPIOD_OUT_LOW);
+       port->gpio_rst = devm_gpiod_get_index_optional(dev, "reset", slot,
+                                                      GPIOD_OUT_LOW);
        if (IS_ERR(port->gpio_rst))

Regards,
    Sergio Paracuellos

>
> > mt7621-pci-phy 1e149000.pcie-phy: Xtal is 40MHz
> > mt7621-pci-phy 1e149000.pcie-phy: Xtal is 40MHz
> > mt7621-pci-phy 1e14a000.pcie-phy: Xtal is 40MHz
> > gpiod_set_value: invalid GPIO (errorpointer)
> > gpiod_set_value: invalid GPIO (errorpointer)
> > mt7621-pci 1e140000.pcie: pcie1 no card, disable it (RST & CLK)
> > mt7621-pci 1e140000.pcie: pcie2 no card, disable it (RST & CLK)
> > mt7621-pci 1e140000.pcie: PCIE0 enabled
> > mt7621-pci 1e140000.pcie: PCI coherence region base: 0x60000000, mask/settings: 0xf0000002
> > mt7621-pci 1e140000.pcie: PCI host bridge to bus 0000:00
> > pci_bus 0000:00: root bus resource [io  0xffffffff]
> > pci_bus 0000:00: root bus resource [mem 0x60000000-0x6fffffff]
> > pci_bus 0000:00: root bus resource [bus 00-ff]
> > pci 0000:00:00.0: [0e8d:0801] type 01 class 0x060400
> > pci 0000:00:00.0: reg 0x10: [mem 0x00000000-0x7fffffff]
> > pci 0000:00:00.0: reg 0x14: [mem 0x00000000-0x0000ffff]
> > pci 0000:00:00.0: supports D1
> > pci 0000:00:00.0: PME# supported from D0 D1 D3hot
> > pci 0000:00:00.0: bridge configuration invalid ([bus 00-00]), reconfiguring
> > pci 0000:01:00.0: [168c:003c] type 00 class 0x028000
> > pci 0000:01:00.0: reg 0x10: [mem 0x00000000-0x001fffff 64bit]
> > pci 0000:01:00.0: reg 0x30: [mem 0x00000000-0x0000ffff pref]
> > pci 0000:01:00.0: supports D1 D2
> > pci 0000:00:00.0: PCI bridge to [bus 01-ff]
> > pci 0000:00:00.0:   bridge window [io  0x0000-0x0fff]
> > pci 0000:00:00.0:   bridge window [mem 0x00000000-0x000fffff]
> > pci 0000:00:00.0:   bridge window [mem 0x00000000-0x000fffff pref]
> > pci_bus 0000:01: busn_res: [bus 01-ff] end is updated to 01
> > pci 0000:00:00.0: BAR 0: no space for [mem size 0x80000000]
> > pci 0000:00:00.0: BAR 0: failed to assign [mem size 0x80000000]
> > pci 0000:00:00.0: BAR 8: assigned [mem 0x60000000-0x601fffff]
> > pci 0000:00:00.0: BAR 9: assigned [mem 0x60200000-0x602fffff pref]
> > pci 0000:00:00.0: BAR 1: assigned [mem 0x60300000-0x6030ffff]
> > pci 0000:00:00.0: BAR 7: no space for [io  size 0x1000]
> > pci 0000:00:00.0: BAR 7: failed to assign [io  size 0x1000]
> > pci 0000:01:00.0: BAR 0: assigned [mem 0x60000000-0x601fffff 64bit]
> > pci 0000:01:00.0: BAR 6: assigned [mem 0x60200000-0x6020ffff pref]
> > pci 0000:00:00.0: PCI bridge to [bus 01]
> > pci 0000:00:00.0:   bridge window [mem 0x60000000-0x601fffff]
> > pci 0000:00:00.0:   bridge window [mem 0x60200000-0x602fffff pref]
> > pcieport 0000:00:00.0: of_irq_parse_pci: failed with rc=-22
> > pcieport 0000:00:00.0: enabling device (0004 -> 0006)
> > CPU 2 Unable to handle kernel paging request at virtual address fffffff0, epc == 8039c7b0, ra == 804fe128
> > Oops[#1]:
> > CPU: 2 PID: 107 Comm: kworker/2:1 Not tainted 5.6.0-rc3-00180-gc15e7f072288-dirty #2
> > Workqueue: events deferred_probe_work_func
> > $ 0   : 00000000 00000001 820d6984 00000001
> > $ 4   : fffffff0 00000001 00000000 803a0000
> > $ 8   : 00000024 ffffffff 00000001 302e3030
> > $12   : 00000000 8e26fbd8 ffffffff 00000020
> > $16   : 820d6980 820e0250 00000000 8e26fcd0
> > $20   : 820e01e0 809a0000 820e0228 8fd47e10
> > $24   : 00000000 00000020
> > $28   : 8e26e000 8e26fc70 8e26fcf8 804fe128
> > Hi    : 00000125
> > Lo    : 122f2000
> > epc   : 8039c7b0 gpiod_free+0x14/0x6c
> > ra    : 804fe128 mt7621_pci_probe+0x700/0xcd8
> > Status: 1100fc03        KERNEL EXL IE
> > Cause : 00800008 (ExcCode 02)
> > BadVA : fffffff0
> > PrId  : 0001992f (MIPS 1004Kc)
> > Modules linked in:
> > Process kworker/2:1 (pid: 107, threadinfo=(ptrval), task=(ptrval), tls=00000000)
> > Stack : 820e0250 809e2aa0 820d6380 820e0250 820d6980 804fe128 1e160000 00000001
> >          60000000 00000000 f0000002 00000000 00000000 809a0000 8fd47e10 808d63d4
> >          80930000 808d6260 808d0000 808d61fc 808c0000 8fd47e10 808d6208 808d0000
> >          8e26fcd0 8e26fcd0 809e2aa0 809db510 809db510 00000006 00000001 00000000
> >          00000000 00000000 01000000 1e1440ff 00000000 00000000 1e160000 00000000
> >          ...
> > Call Trace:
> > [<8039c7b0>] gpiod_free+0x14/0x6c
> > [<804fe128>] mt7621_pci_probe+0x700/0xcd8
> > [<80402ab8>] platform_drv_probe+0x40/0x94
> > [<80400a74>] really_probe+0x104/0x364
> > [<803feb74>] bus_for_each_drv+0x84/0xdc
> > [<80400924>] __device_attach+0xdc/0x120
> > [<803ffb5c>] bus_probe_device+0xa0/0xbc
> > [<80400124>] deferred_probe_work_func+0x7c/0xbc
> > [<800420e8>] process_one_work+0x230/0x450
> > [<80042638>] worker_thread+0x330/0x5fc
> > [<80048eb0>] kthread+0x12c/0x134
> > [<80007438>] ret_from_kernel_thread+0x14/0x1c
> > Code: 27bdffe8  afb00010  afbf0014 <8c830000> 10600005  00808025  0c0e6efb  00000000  14400007
> >
> > ---[ end trace d9831fec9efd296d ]---
> >
>
> This is to be because the 'gpio_dput' makes internally a gpiod_free in
> gpio's and desc is an error pointer descriptor instead of NULL.
> Expected, looking the previous traces.
>
> >
> >
> > And on another boot:
> >
> > ...
> > rt2880-pinmux pinctrl: pcie is already enabled
> > mt7621-pci 1e140000.pcie: Error applying setting, reverse things back
> > mt7621-pci 1e140000.pcie: Failed to get GPIO for PCIe1
> > mt7621-pci 1e140000.pcie: Failed to get GPIO for PCIe2
> > gpiod_set_value: invalid GPIO (errorpointer)
> > gpiod_set_value: invalid GPIO (errorpointer)
> > mt7621-pci-phy 1e149000.pcie-phy: Xtal is 40MHz
> > mt7621-pci-phy 1e149000.pcie-phy: Xtal is 40MHz
> > mt7621-pci-phy 1e14a000.pcie-phy: Xtal is 40MHz
> > gpiod_set_value: invalid GPIO (errorpointer)
> > gpiod_set_value: invalid GPIO (errorpointer)
> > mt7621-pci 1e140000.pcie: pcie0 no card, disable it (RST & CLK)
> > mt7621-pci 1e140000.pcie: pcie1 no card, disable it (RST & CLK)
> > mt7621-pci 1e140000.pcie: pcie2 no card, disable it (RST & CLK)
> > mt7621-pci 1e140000.pcie: Nothing is connected in virtual bridges. Exiting...
>
> Maybe 100 us is not enough? We should focus first in avoiding the boot
> hangs and the we can play with the
>
> #define PERST_DELAY_US                  100
>
> This series change it from 1000 to 100. Maybe you can try to put
> another value if hangs dissapear...
>
> > CPU 1 Unable to handle kernel paging request at virtual address fffffff0, epc == 8039c7b0, ra == 804fe128
> > Oops[#1]:
> > CPU: 1 PID: 103 Comm: kworker/1:1 Not tainted 5.6.0-rc3-00180-gc15e7f072288-dirty #2
> > Workqueue: events deferred_probe_work_func
> > $ 0   : 00000000 00000001 82051e84 00000001
> > $ 4   : fffffff0 00000001 00000000 803a0000
> > $ 8   : 2e000000 80940000 00000018 000000d6
> > $12   : 6976206e 00000000 00000000 61757472
> > $16   : 82051e80 820d4250 00000000 8fd6dcd0
> > $20   : 820d41e0 820d424c 00000000 809a0000
> > $24   : 00000008 803ef8f4
> > $28   : 8fd6c000 8fd6dc70 808d6280 804fe128
> > Hi    : 00000125
> > Lo    : 122f2000
> > epc   : 8039c7b0 gpiod_free+0x14/0x6c
> > ra    : 804fe128 mt7621_pci_probe+0x700/0xcd8
> > Status: 1100fc03        KERNEL EXL IE
> > Cause : 00800008 (ExcCode 02)
> > BadVA : fffffff0
> > PrId  : 0001992f (MIPS 1004Kc)
> > Modules linked in:
> > Process kworker/1:1 (pid: 103, threadinfo=(ptrval), task=(ptrval), tls=00000000)
> > Stack : 8fd6dc90 808d62a8 82051880 820d4250 82051e80 804fe128 809d0000 00000001
> >          00000001 00000001 00000000 00000001 00000000 8fd47e10 8fd47e10 808d63d4
> >          80930000 808d6260 808d0000 808d61fc 808c0000 8fd47e10 808d6208 808d0000
> >          8fd6dcd0 8fd6dcd0 1e140000 1e1400ff 809e2af8 00000200 00000000 00000000
> >          00000000 00000000 1e144000 1e1440ff 809e2af8 00000200 00000000 00000000
> >          ...
> > Call Trace:
> > [<8039c7b0>] gpiod_free+0x14/0x6c
> > [<804fe128>] mt7621_pci_probe+0x700/0xcd8
> > [<80402ab8>] platform_drv_probe+0x40/0x94
> > [<80400a74>] really_probe+0x104/0x364
> > [<803feb74>] bus_for_each_drv+0x84/0xdc
> > [<80400924>] __device_attach+0xdc/0x120
> > [<803ffb5c>] bus_probe_device+0xa0/0xbc
> > [<80400124>] deferred_probe_work_func+0x7c/0xbc
> > [<800420e8>] process_one_work+0x230/0x450
> > [<80042638>] worker_thread+0x330/0x5fc
> > [<80048eb0>] kthread+0x12c/0x134
> > [<80007438>] ret_from_kernel_thread+0x14/0x1c
> > Code: 27bdffe8  afb00010  afbf0014 <8c830000> 10600005  00808025  0c0e6efb  00000000  14400007
> >
> > ---[ end trace 5affc5903364c81f ]---
> >
> Again, the 'gpio_dput' makes internally a gpiod_free in gpio's and
> desc is an error pointer descriptor instead of NULL.
>
> >
> > Regards
> > Greg
>
> Thanks for testing this series and letting me know.
>
> Best regards,
>     Sergio Paracuellos
> >
> >
> > > Hope this helps.
> > >
> > > Changes in v3:
> > >      * Avoid to fail if gpio descriptor fails on get.
> > >      * re-do PATCH 1 commit message.
> > >      * Take into account gpio low polarity on request and assert and deassert.
> > >      * Review error path of driver to properly release gpio's resources.
> > >
> > > Changes in v2:
> > >      * restore configuration for pers mode to GPIO.
> > >      * Avoid to read FTS_NUM register in reset state.
> > >      * Release gpio's patch added
> > >
> > > Best regards,
> > >      Sergio Paracuellos
> > >
> > > Sergio Paracuellos (5):
> > >    staging: mt7621-pci: use gpios for properly reset
> > >    staging: mt7621-pci: change value for 'PERST_DELAY_US'
> > >    staging: mt7621-dts: make use of 'reset-gpios' property for pci
> > >    staging: mt7621-pci: bindings: update doc accordly to last changes
> > >    staging: mt7621-pci: release gpios after pci initialization
> > >
> > >   drivers/staging/mt7621-dts/mt7621.dtsi        |  11 +-
> > >   .../mt7621-pci/mediatek,mt7621-pci.txt        |   7 +-
> > >   drivers/staging/mt7621-pci/pci-mt7621.c       | 107 +++++++++++-------
> > >   3 files changed, 80 insertions(+), 45 deletions(-)
> > >
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

* Re: [PATCH v3 0/5] staging: mt7621-pci: re-do reset boot process
  2020-03-13  5:45   ` Sergio Paracuellos
  2020-03-13  6:31     ` Sergio Paracuellos
@ 2020-03-15 12:32     ` Greg Ungerer
  2020-03-15 12:39       ` Sergio Paracuellos
  1 sibling, 1 reply; 11+ messages in thread
From: Greg Ungerer @ 2020-03-15 12:32 UTC (permalink / raw)
  To: Sergio Paracuellos
  Cc: ryder.lee, Greg KH, driverdev-devel, weijie.gao, NeilBrown

Hi Sergio,

On 13/3/20 3:45 pm, Sergio Paracuellos wrote:
> On Fri, Mar 13, 2020 at 4:51 AM Greg Ungerer <gerg@kernel.org> wrote:
> [snip]
>>
>> I applied the patches to the staging git tree for testing.
>> I have seen a couple of different problems on boot up:
>>
>> ...
>> rt2880-pinmux pinctrl: pcie is already enabled
>> mt7621-pci 1e140000.pcie: Error applying setting, reverse things back
>> mt7621-pci 1e140000.pcie: Failed to get GPIO for PCIe1
>> mt7621-pci 1e140000.pcie: Failed to get GPIO for PCIe2
>> gpiod_set_value: invalid GPIO (errorpointer)
>> gpiod_set_value: invalid GPIO (errorpointer)
> 
> So the not grabbed gpio's are error pointers instead of NULL. What
> happen if you just
> set them to NULL? Something like:
> 
> diff --git a/drivers/staging/mt7621-pci/pci-mt7621.c
> b/drivers/staging/mt7621-pci/pci-mt7621.c
> index 6ea284942b6f..03c1f057b29f 100644
> --- a/drivers/staging/mt7621-pci/pci-mt7621.c
> +++ b/drivers/staging/mt7621-pci/pci-mt7621.c
> @@ -367,8 +367,10 @@ static int mt7621_pcie_parse_port(struct mt7621_pcie *pcie,
> 
>          port->gpio_rst = devm_gpiod_get_index(dev, "reset", slot,
>                                                GPIOD_OUT_LOW);
> -       if (IS_ERR(port->gpio_rst))
> +       if (IS_ERR(port->gpio_rst)) {
> +               port->gpio_rst = NULL;
>                  dev_notice(dev, "Failed to get GPIO for PCIe%d\n", slot);
> +       }
> 
> Another possibility would be to use 'gpio_is_valid' getting int gpio
> value from descriptor using 'gpio_to_desc' but I think we were mixing
> legacy gpio APIS with the descriptor interface one which sound not
> good.

I will try this and let you know. (It looks like in v4 you did this
differently though?).

FWIW, on my hardware platform GPIO 7 and 8 are not used for PCI resets.
They are wired out as serial port signals (DCD and DTR), and they are
defined in my device tree as that.

Regards
Greg



>> mt7621-pci-phy 1e149000.pcie-phy: Xtal is 40MHz
>> mt7621-pci-phy 1e149000.pcie-phy: Xtal is 40MHz
>> mt7621-pci-phy 1e14a000.pcie-phy: Xtal is 40MHz
>> gpiod_set_value: invalid GPIO (errorpointer)
>> gpiod_set_value: invalid GPIO (errorpointer)
>> mt7621-pci 1e140000.pcie: pcie1 no card, disable it (RST & CLK)
>> mt7621-pci 1e140000.pcie: pcie2 no card, disable it (RST & CLK)
>> mt7621-pci 1e140000.pcie: PCIE0 enabled
>> mt7621-pci 1e140000.pcie: PCI coherence region base: 0x60000000, mask/settings: 0xf0000002
>> mt7621-pci 1e140000.pcie: PCI host bridge to bus 0000:00
>> pci_bus 0000:00: root bus resource [io  0xffffffff]
>> pci_bus 0000:00: root bus resource [mem 0x60000000-0x6fffffff]
>> pci_bus 0000:00: root bus resource [bus 00-ff]
>> pci 0000:00:00.0: [0e8d:0801] type 01 class 0x060400
>> pci 0000:00:00.0: reg 0x10: [mem 0x00000000-0x7fffffff]
>> pci 0000:00:00.0: reg 0x14: [mem 0x00000000-0x0000ffff]
>> pci 0000:00:00.0: supports D1
>> pci 0000:00:00.0: PME# supported from D0 D1 D3hot
>> pci 0000:00:00.0: bridge configuration invalid ([bus 00-00]), reconfiguring
>> pci 0000:01:00.0: [168c:003c] type 00 class 0x028000
>> pci 0000:01:00.0: reg 0x10: [mem 0x00000000-0x001fffff 64bit]
>> pci 0000:01:00.0: reg 0x30: [mem 0x00000000-0x0000ffff pref]
>> pci 0000:01:00.0: supports D1 D2
>> pci 0000:00:00.0: PCI bridge to [bus 01-ff]
>> pci 0000:00:00.0:   bridge window [io  0x0000-0x0fff]
>> pci 0000:00:00.0:   bridge window [mem 0x00000000-0x000fffff]
>> pci 0000:00:00.0:   bridge window [mem 0x00000000-0x000fffff pref]
>> pci_bus 0000:01: busn_res: [bus 01-ff] end is updated to 01
>> pci 0000:00:00.0: BAR 0: no space for [mem size 0x80000000]
>> pci 0000:00:00.0: BAR 0: failed to assign [mem size 0x80000000]
>> pci 0000:00:00.0: BAR 8: assigned [mem 0x60000000-0x601fffff]
>> pci 0000:00:00.0: BAR 9: assigned [mem 0x60200000-0x602fffff pref]
>> pci 0000:00:00.0: BAR 1: assigned [mem 0x60300000-0x6030ffff]
>> pci 0000:00:00.0: BAR 7: no space for [io  size 0x1000]
>> pci 0000:00:00.0: BAR 7: failed to assign [io  size 0x1000]
>> pci 0000:01:00.0: BAR 0: assigned [mem 0x60000000-0x601fffff 64bit]
>> pci 0000:01:00.0: BAR 6: assigned [mem 0x60200000-0x6020ffff pref]
>> pci 0000:00:00.0: PCI bridge to [bus 01]
>> pci 0000:00:00.0:   bridge window [mem 0x60000000-0x601fffff]
>> pci 0000:00:00.0:   bridge window [mem 0x60200000-0x602fffff pref]
>> pcieport 0000:00:00.0: of_irq_parse_pci: failed with rc=-22
>> pcieport 0000:00:00.0: enabling device (0004 -> 0006)
>> CPU 2 Unable to handle kernel paging request at virtual address fffffff0, epc == 8039c7b0, ra == 804fe128
>> Oops[#1]:
>> CPU: 2 PID: 107 Comm: kworker/2:1 Not tainted 5.6.0-rc3-00180-gc15e7f072288-dirty #2
>> Workqueue: events deferred_probe_work_func
>> $ 0   : 00000000 00000001 820d6984 00000001
>> $ 4   : fffffff0 00000001 00000000 803a0000
>> $ 8   : 00000024 ffffffff 00000001 302e3030
>> $12   : 00000000 8e26fbd8 ffffffff 00000020
>> $16   : 820d6980 820e0250 00000000 8e26fcd0
>> $20   : 820e01e0 809a0000 820e0228 8fd47e10
>> $24   : 00000000 00000020
>> $28   : 8e26e000 8e26fc70 8e26fcf8 804fe128
>> Hi    : 00000125
>> Lo    : 122f2000
>> epc   : 8039c7b0 gpiod_free+0x14/0x6c
>> ra    : 804fe128 mt7621_pci_probe+0x700/0xcd8
>> Status: 1100fc03        KERNEL EXL IE
>> Cause : 00800008 (ExcCode 02)
>> BadVA : fffffff0
>> PrId  : 0001992f (MIPS 1004Kc)
>> Modules linked in:
>> Process kworker/2:1 (pid: 107, threadinfo=(ptrval), task=(ptrval), tls=00000000)
>> Stack : 820e0250 809e2aa0 820d6380 820e0250 820d6980 804fe128 1e160000 00000001
>>           60000000 00000000 f0000002 00000000 00000000 809a0000 8fd47e10 808d63d4
>>           80930000 808d6260 808d0000 808d61fc 808c0000 8fd47e10 808d6208 808d0000
>>           8e26fcd0 8e26fcd0 809e2aa0 809db510 809db510 00000006 00000001 00000000
>>           00000000 00000000 01000000 1e1440ff 00000000 00000000 1e160000 00000000
>>           ...
>> Call Trace:
>> [<8039c7b0>] gpiod_free+0x14/0x6c
>> [<804fe128>] mt7621_pci_probe+0x700/0xcd8
>> [<80402ab8>] platform_drv_probe+0x40/0x94
>> [<80400a74>] really_probe+0x104/0x364
>> [<803feb74>] bus_for_each_drv+0x84/0xdc
>> [<80400924>] __device_attach+0xdc/0x120
>> [<803ffb5c>] bus_probe_device+0xa0/0xbc
>> [<80400124>] deferred_probe_work_func+0x7c/0xbc
>> [<800420e8>] process_one_work+0x230/0x450
>> [<80042638>] worker_thread+0x330/0x5fc
>> [<80048eb0>] kthread+0x12c/0x134
>> [<80007438>] ret_from_kernel_thread+0x14/0x1c
>> Code: 27bdffe8  afb00010  afbf0014 <8c830000> 10600005  00808025  0c0e6efb  00000000  14400007
>>
>> ---[ end trace d9831fec9efd296d ]---
>>
> 
> This is to be because the 'gpio_dput' makes internally a gpiod_free in
> gpio's and desc is an error pointer descriptor instead of NULL.
> Expected, looking the previous traces.
> 
>>
>>
>> And on another boot:
>>
>> ...
>> rt2880-pinmux pinctrl: pcie is already enabled
>> mt7621-pci 1e140000.pcie: Error applying setting, reverse things back
>> mt7621-pci 1e140000.pcie: Failed to get GPIO for PCIe1
>> mt7621-pci 1e140000.pcie: Failed to get GPIO for PCIe2
>> gpiod_set_value: invalid GPIO (errorpointer)
>> gpiod_set_value: invalid GPIO (errorpointer)
>> mt7621-pci-phy 1e149000.pcie-phy: Xtal is 40MHz
>> mt7621-pci-phy 1e149000.pcie-phy: Xtal is 40MHz
>> mt7621-pci-phy 1e14a000.pcie-phy: Xtal is 40MHz
>> gpiod_set_value: invalid GPIO (errorpointer)
>> gpiod_set_value: invalid GPIO (errorpointer)
>> mt7621-pci 1e140000.pcie: pcie0 no card, disable it (RST & CLK)
>> mt7621-pci 1e140000.pcie: pcie1 no card, disable it (RST & CLK)
>> mt7621-pci 1e140000.pcie: pcie2 no card, disable it (RST & CLK)
>> mt7621-pci 1e140000.pcie: Nothing is connected in virtual bridges. Exiting...
> 
> Maybe 100 us is not enough? We should focus first in avoiding the boot
> hangs and the we can play with the
> 
> #define PERST_DELAY_US                  100
> 
> This series change it from 1000 to 100. Maybe you can try to put
> another value if hangs dissapear...
> 
>> CPU 1 Unable to handle kernel paging request at virtual address fffffff0, epc == 8039c7b0, ra == 804fe128
>> Oops[#1]:
>> CPU: 1 PID: 103 Comm: kworker/1:1 Not tainted 5.6.0-rc3-00180-gc15e7f072288-dirty #2
>> Workqueue: events deferred_probe_work_func
>> $ 0   : 00000000 00000001 82051e84 00000001
>> $ 4   : fffffff0 00000001 00000000 803a0000
>> $ 8   : 2e000000 80940000 00000018 000000d6
>> $12   : 6976206e 00000000 00000000 61757472
>> $16   : 82051e80 820d4250 00000000 8fd6dcd0
>> $20   : 820d41e0 820d424c 00000000 809a0000
>> $24   : 00000008 803ef8f4
>> $28   : 8fd6c000 8fd6dc70 808d6280 804fe128
>> Hi    : 00000125
>> Lo    : 122f2000
>> epc   : 8039c7b0 gpiod_free+0x14/0x6c
>> ra    : 804fe128 mt7621_pci_probe+0x700/0xcd8
>> Status: 1100fc03        KERNEL EXL IE
>> Cause : 00800008 (ExcCode 02)
>> BadVA : fffffff0
>> PrId  : 0001992f (MIPS 1004Kc)
>> Modules linked in:
>> Process kworker/1:1 (pid: 103, threadinfo=(ptrval), task=(ptrval), tls=00000000)
>> Stack : 8fd6dc90 808d62a8 82051880 820d4250 82051e80 804fe128 809d0000 00000001
>>           00000001 00000001 00000000 00000001 00000000 8fd47e10 8fd47e10 808d63d4
>>           80930000 808d6260 808d0000 808d61fc 808c0000 8fd47e10 808d6208 808d0000
>>           8fd6dcd0 8fd6dcd0 1e140000 1e1400ff 809e2af8 00000200 00000000 00000000
>>           00000000 00000000 1e144000 1e1440ff 809e2af8 00000200 00000000 00000000
>>           ...
>> Call Trace:
>> [<8039c7b0>] gpiod_free+0x14/0x6c
>> [<804fe128>] mt7621_pci_probe+0x700/0xcd8
>> [<80402ab8>] platform_drv_probe+0x40/0x94
>> [<80400a74>] really_probe+0x104/0x364
>> [<803feb74>] bus_for_each_drv+0x84/0xdc
>> [<80400924>] __device_attach+0xdc/0x120
>> [<803ffb5c>] bus_probe_device+0xa0/0xbc
>> [<80400124>] deferred_probe_work_func+0x7c/0xbc
>> [<800420e8>] process_one_work+0x230/0x450
>> [<80042638>] worker_thread+0x330/0x5fc
>> [<80048eb0>] kthread+0x12c/0x134
>> [<80007438>] ret_from_kernel_thread+0x14/0x1c
>> Code: 27bdffe8  afb00010  afbf0014 <8c830000> 10600005  00808025  0c0e6efb  00000000  14400007
>>
>> ---[ end trace 5affc5903364c81f ]---
>>
> Again, the 'gpio_dput' makes internally a gpiod_free in gpio's and
> desc is an error pointer descriptor instead of NULL.
> 
>>
>> Regards
>> Greg
> 
> Thanks for testing this series and letting me know.
> 
> Best regards,
>      Sergio Paracuellos
>>
>>
>>> Hope this helps.
>>>
>>> Changes in v3:
>>>       * Avoid to fail if gpio descriptor fails on get.
>>>       * re-do PATCH 1 commit message.
>>>       * Take into account gpio low polarity on request and assert and deassert.
>>>       * Review error path of driver to properly release gpio's resources.
>>>
>>> Changes in v2:
>>>       * restore configuration for pers mode to GPIO.
>>>       * Avoid to read FTS_NUM register in reset state.
>>>       * Release gpio's patch added
>>>
>>> Best regards,
>>>       Sergio Paracuellos
>>>
>>> Sergio Paracuellos (5):
>>>     staging: mt7621-pci: use gpios for properly reset
>>>     staging: mt7621-pci: change value for 'PERST_DELAY_US'
>>>     staging: mt7621-dts: make use of 'reset-gpios' property for pci
>>>     staging: mt7621-pci: bindings: update doc accordly to last changes
>>>     staging: mt7621-pci: release gpios after pci initialization
>>>
>>>    drivers/staging/mt7621-dts/mt7621.dtsi        |  11 +-
>>>    .../mt7621-pci/mediatek,mt7621-pci.txt        |   7 +-
>>>    drivers/staging/mt7621-pci/pci-mt7621.c       | 107 +++++++++++-------
>>>    3 files changed, 80 insertions(+), 45 deletions(-)
>>>
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

* Re: [PATCH v3 0/5] staging: mt7621-pci: re-do reset boot process
  2020-03-15 12:32     ` Greg Ungerer
@ 2020-03-15 12:39       ` Sergio Paracuellos
  0 siblings, 0 replies; 11+ messages in thread
From: Sergio Paracuellos @ 2020-03-15 12:39 UTC (permalink / raw)
  To: Greg Ungerer; +Cc: ryder.lee, Greg KH, driverdev-devel, weijie.gao, NeilBrown

Hi Greg,

On Sun, Mar 15, 2020 at 1:32 PM Greg Ungerer <gerg@kernel.org> wrote:
>
> Hi Sergio,
>
> On 13/3/20 3:45 pm, Sergio Paracuellos wrote:
> > On Fri, Mar 13, 2020 at 4:51 AM Greg Ungerer <gerg@kernel.org> wrote:
> > [snip]
> >>
> >> I applied the patches to the staging git tree for testing.
> >> I have seen a couple of different problems on boot up:
> >>
> >> ...
> >> rt2880-pinmux pinctrl: pcie is already enabled
> >> mt7621-pci 1e140000.pcie: Error applying setting, reverse things back
> >> mt7621-pci 1e140000.pcie: Failed to get GPIO for PCIe1
> >> mt7621-pci 1e140000.pcie: Failed to get GPIO for PCIe2
> >> gpiod_set_value: invalid GPIO (errorpointer)
> >> gpiod_set_value: invalid GPIO (errorpointer)
> >
> > So the not grabbed gpio's are error pointers instead of NULL. What
> > happen if you just
> > set them to NULL? Something like:
> >
> > diff --git a/drivers/staging/mt7621-pci/pci-mt7621.c
> > b/drivers/staging/mt7621-pci/pci-mt7621.c
> > index 6ea284942b6f..03c1f057b29f 100644
> > --- a/drivers/staging/mt7621-pci/pci-mt7621.c
> > +++ b/drivers/staging/mt7621-pci/pci-mt7621.c
> > @@ -367,8 +367,10 @@ static int mt7621_pcie_parse_port(struct mt7621_pcie *pcie,
> >
> >          port->gpio_rst = devm_gpiod_get_index(dev, "reset", slot,
> >                                                GPIOD_OUT_LOW);
> > -       if (IS_ERR(port->gpio_rst))
> > +       if (IS_ERR(port->gpio_rst)) {
> > +               port->gpio_rst = NULL;
> >                  dev_notice(dev, "Failed to get GPIO for PCIe%d\n", slot);
> > +       }
> >
> > Another possibility would be to use 'gpio_is_valid' getting int gpio
> > value from descriptor using 'gpio_to_desc' but I think we were mixing
> > legacy gpio APIS with the descriptor interface one which sound not
> > good.
>
> I will try this and let you know. (It looks like in v4 you did this
> differently though?).

v4 uses 'devm_gpiod_get_index_optional' instead of
'devm_gpiod_get_index' to avoid the error pointer and explicitely set
it to NULL. It should work for you also, AFAICT.

Regards,
    Sergio Paracuellos
>
> FWIW, on my hardware platform GPIO 7 and 8 are not used for PCI resets.
> They are wired out as serial port signals (DCD and DTR), and they are
> defined in my device tree as that.
>
> Regards
> Greg
>
>
>
> >> mt7621-pci-phy 1e149000.pcie-phy: Xtal is 40MHz
> >> mt7621-pci-phy 1e149000.pcie-phy: Xtal is 40MHz
> >> mt7621-pci-phy 1e14a000.pcie-phy: Xtal is 40MHz
> >> gpiod_set_value: invalid GPIO (errorpointer)
> >> gpiod_set_value: invalid GPIO (errorpointer)
> >> mt7621-pci 1e140000.pcie: pcie1 no card, disable it (RST & CLK)
> >> mt7621-pci 1e140000.pcie: pcie2 no card, disable it (RST & CLK)
> >> mt7621-pci 1e140000.pcie: PCIE0 enabled
> >> mt7621-pci 1e140000.pcie: PCI coherence region base: 0x60000000, mask/settings: 0xf0000002
> >> mt7621-pci 1e140000.pcie: PCI host bridge to bus 0000:00
> >> pci_bus 0000:00: root bus resource [io  0xffffffff]
> >> pci_bus 0000:00: root bus resource [mem 0x60000000-0x6fffffff]
> >> pci_bus 0000:00: root bus resource [bus 00-ff]
> >> pci 0000:00:00.0: [0e8d:0801] type 01 class 0x060400
> >> pci 0000:00:00.0: reg 0x10: [mem 0x00000000-0x7fffffff]
> >> pci 0000:00:00.0: reg 0x14: [mem 0x00000000-0x0000ffff]
> >> pci 0000:00:00.0: supports D1
> >> pci 0000:00:00.0: PME# supported from D0 D1 D3hot
> >> pci 0000:00:00.0: bridge configuration invalid ([bus 00-00]), reconfiguring
> >> pci 0000:01:00.0: [168c:003c] type 00 class 0x028000
> >> pci 0000:01:00.0: reg 0x10: [mem 0x00000000-0x001fffff 64bit]
> >> pci 0000:01:00.0: reg 0x30: [mem 0x00000000-0x0000ffff pref]
> >> pci 0000:01:00.0: supports D1 D2
> >> pci 0000:00:00.0: PCI bridge to [bus 01-ff]
> >> pci 0000:00:00.0:   bridge window [io  0x0000-0x0fff]
> >> pci 0000:00:00.0:   bridge window [mem 0x00000000-0x000fffff]
> >> pci 0000:00:00.0:   bridge window [mem 0x00000000-0x000fffff pref]
> >> pci_bus 0000:01: busn_res: [bus 01-ff] end is updated to 01
> >> pci 0000:00:00.0: BAR 0: no space for [mem size 0x80000000]
> >> pci 0000:00:00.0: BAR 0: failed to assign [mem size 0x80000000]
> >> pci 0000:00:00.0: BAR 8: assigned [mem 0x60000000-0x601fffff]
> >> pci 0000:00:00.0: BAR 9: assigned [mem 0x60200000-0x602fffff pref]
> >> pci 0000:00:00.0: BAR 1: assigned [mem 0x60300000-0x6030ffff]
> >> pci 0000:00:00.0: BAR 7: no space for [io  size 0x1000]
> >> pci 0000:00:00.0: BAR 7: failed to assign [io  size 0x1000]
> >> pci 0000:01:00.0: BAR 0: assigned [mem 0x60000000-0x601fffff 64bit]
> >> pci 0000:01:00.0: BAR 6: assigned [mem 0x60200000-0x6020ffff pref]
> >> pci 0000:00:00.0: PCI bridge to [bus 01]
> >> pci 0000:00:00.0:   bridge window [mem 0x60000000-0x601fffff]
> >> pci 0000:00:00.0:   bridge window [mem 0x60200000-0x602fffff pref]
> >> pcieport 0000:00:00.0: of_irq_parse_pci: failed with rc=-22
> >> pcieport 0000:00:00.0: enabling device (0004 -> 0006)
> >> CPU 2 Unable to handle kernel paging request at virtual address fffffff0, epc == 8039c7b0, ra == 804fe128
> >> Oops[#1]:
> >> CPU: 2 PID: 107 Comm: kworker/2:1 Not tainted 5.6.0-rc3-00180-gc15e7f072288-dirty #2
> >> Workqueue: events deferred_probe_work_func
> >> $ 0   : 00000000 00000001 820d6984 00000001
> >> $ 4   : fffffff0 00000001 00000000 803a0000
> >> $ 8   : 00000024 ffffffff 00000001 302e3030
> >> $12   : 00000000 8e26fbd8 ffffffff 00000020
> >> $16   : 820d6980 820e0250 00000000 8e26fcd0
> >> $20   : 820e01e0 809a0000 820e0228 8fd47e10
> >> $24   : 00000000 00000020
> >> $28   : 8e26e000 8e26fc70 8e26fcf8 804fe128
> >> Hi    : 00000125
> >> Lo    : 122f2000
> >> epc   : 8039c7b0 gpiod_free+0x14/0x6c
> >> ra    : 804fe128 mt7621_pci_probe+0x700/0xcd8
> >> Status: 1100fc03        KERNEL EXL IE
> >> Cause : 00800008 (ExcCode 02)
> >> BadVA : fffffff0
> >> PrId  : 0001992f (MIPS 1004Kc)
> >> Modules linked in:
> >> Process kworker/2:1 (pid: 107, threadinfo=(ptrval), task=(ptrval), tls=00000000)
> >> Stack : 820e0250 809e2aa0 820d6380 820e0250 820d6980 804fe128 1e160000 00000001
> >>           60000000 00000000 f0000002 00000000 00000000 809a0000 8fd47e10 808d63d4
> >>           80930000 808d6260 808d0000 808d61fc 808c0000 8fd47e10 808d6208 808d0000
> >>           8e26fcd0 8e26fcd0 809e2aa0 809db510 809db510 00000006 00000001 00000000
> >>           00000000 00000000 01000000 1e1440ff 00000000 00000000 1e160000 00000000
> >>           ...
> >> Call Trace:
> >> [<8039c7b0>] gpiod_free+0x14/0x6c
> >> [<804fe128>] mt7621_pci_probe+0x700/0xcd8
> >> [<80402ab8>] platform_drv_probe+0x40/0x94
> >> [<80400a74>] really_probe+0x104/0x364
> >> [<803feb74>] bus_for_each_drv+0x84/0xdc
> >> [<80400924>] __device_attach+0xdc/0x120
> >> [<803ffb5c>] bus_probe_device+0xa0/0xbc
> >> [<80400124>] deferred_probe_work_func+0x7c/0xbc
> >> [<800420e8>] process_one_work+0x230/0x450
> >> [<80042638>] worker_thread+0x330/0x5fc
> >> [<80048eb0>] kthread+0x12c/0x134
> >> [<80007438>] ret_from_kernel_thread+0x14/0x1c
> >> Code: 27bdffe8  afb00010  afbf0014 <8c830000> 10600005  00808025  0c0e6efb  00000000  14400007
> >>
> >> ---[ end trace d9831fec9efd296d ]---
> >>
> >
> > This is to be because the 'gpio_dput' makes internally a gpiod_free in
> > gpio's and desc is an error pointer descriptor instead of NULL.
> > Expected, looking the previous traces.
> >
> >>
> >>
> >> And on another boot:
> >>
> >> ...
> >> rt2880-pinmux pinctrl: pcie is already enabled
> >> mt7621-pci 1e140000.pcie: Error applying setting, reverse things back
> >> mt7621-pci 1e140000.pcie: Failed to get GPIO for PCIe1
> >> mt7621-pci 1e140000.pcie: Failed to get GPIO for PCIe2
> >> gpiod_set_value: invalid GPIO (errorpointer)
> >> gpiod_set_value: invalid GPIO (errorpointer)
> >> mt7621-pci-phy 1e149000.pcie-phy: Xtal is 40MHz
> >> mt7621-pci-phy 1e149000.pcie-phy: Xtal is 40MHz
> >> mt7621-pci-phy 1e14a000.pcie-phy: Xtal is 40MHz
> >> gpiod_set_value: invalid GPIO (errorpointer)
> >> gpiod_set_value: invalid GPIO (errorpointer)
> >> mt7621-pci 1e140000.pcie: pcie0 no card, disable it (RST & CLK)
> >> mt7621-pci 1e140000.pcie: pcie1 no card, disable it (RST & CLK)
> >> mt7621-pci 1e140000.pcie: pcie2 no card, disable it (RST & CLK)
> >> mt7621-pci 1e140000.pcie: Nothing is connected in virtual bridges. Exiting...
> >
> > Maybe 100 us is not enough? We should focus first in avoiding the boot
> > hangs and the we can play with the
> >
> > #define PERST_DELAY_US                  100
> >
> > This series change it from 1000 to 100. Maybe you can try to put
> > another value if hangs dissapear...
> >
> >> CPU 1 Unable to handle kernel paging request at virtual address fffffff0, epc == 8039c7b0, ra == 804fe128
> >> Oops[#1]:
> >> CPU: 1 PID: 103 Comm: kworker/1:1 Not tainted 5.6.0-rc3-00180-gc15e7f072288-dirty #2
> >> Workqueue: events deferred_probe_work_func
> >> $ 0   : 00000000 00000001 82051e84 00000001
> >> $ 4   : fffffff0 00000001 00000000 803a0000
> >> $ 8   : 2e000000 80940000 00000018 000000d6
> >> $12   : 6976206e 00000000 00000000 61757472
> >> $16   : 82051e80 820d4250 00000000 8fd6dcd0
> >> $20   : 820d41e0 820d424c 00000000 809a0000
> >> $24   : 00000008 803ef8f4
> >> $28   : 8fd6c000 8fd6dc70 808d6280 804fe128
> >> Hi    : 00000125
> >> Lo    : 122f2000
> >> epc   : 8039c7b0 gpiod_free+0x14/0x6c
> >> ra    : 804fe128 mt7621_pci_probe+0x700/0xcd8
> >> Status: 1100fc03        KERNEL EXL IE
> >> Cause : 00800008 (ExcCode 02)
> >> BadVA : fffffff0
> >> PrId  : 0001992f (MIPS 1004Kc)
> >> Modules linked in:
> >> Process kworker/1:1 (pid: 103, threadinfo=(ptrval), task=(ptrval), tls=00000000)
> >> Stack : 8fd6dc90 808d62a8 82051880 820d4250 82051e80 804fe128 809d0000 00000001
> >>           00000001 00000001 00000000 00000001 00000000 8fd47e10 8fd47e10 808d63d4
> >>           80930000 808d6260 808d0000 808d61fc 808c0000 8fd47e10 808d6208 808d0000
> >>           8fd6dcd0 8fd6dcd0 1e140000 1e1400ff 809e2af8 00000200 00000000 00000000
> >>           00000000 00000000 1e144000 1e1440ff 809e2af8 00000200 00000000 00000000
> >>           ...
> >> Call Trace:
> >> [<8039c7b0>] gpiod_free+0x14/0x6c
> >> [<804fe128>] mt7621_pci_probe+0x700/0xcd8
> >> [<80402ab8>] platform_drv_probe+0x40/0x94
> >> [<80400a74>] really_probe+0x104/0x364
> >> [<803feb74>] bus_for_each_drv+0x84/0xdc
> >> [<80400924>] __device_attach+0xdc/0x120
> >> [<803ffb5c>] bus_probe_device+0xa0/0xbc
> >> [<80400124>] deferred_probe_work_func+0x7c/0xbc
> >> [<800420e8>] process_one_work+0x230/0x450
> >> [<80042638>] worker_thread+0x330/0x5fc
> >> [<80048eb0>] kthread+0x12c/0x134
> >> [<80007438>] ret_from_kernel_thread+0x14/0x1c
> >> Code: 27bdffe8  afb00010  afbf0014 <8c830000> 10600005  00808025  0c0e6efb  00000000  14400007
> >>
> >> ---[ end trace 5affc5903364c81f ]---
> >>
> > Again, the 'gpio_dput' makes internally a gpiod_free in gpio's and
> > desc is an error pointer descriptor instead of NULL.
> >
> >>
> >> Regards
> >> Greg
> >
> > Thanks for testing this series and letting me know.
> >
> > Best regards,
> >      Sergio Paracuellos
> >>
> >>
> >>> Hope this helps.
> >>>
> >>> Changes in v3:
> >>>       * Avoid to fail if gpio descriptor fails on get.
> >>>       * re-do PATCH 1 commit message.
> >>>       * Take into account gpio low polarity on request and assert and deassert.
> >>>       * Review error path of driver to properly release gpio's resources.
> >>>
> >>> Changes in v2:
> >>>       * restore configuration for pers mode to GPIO.
> >>>       * Avoid to read FTS_NUM register in reset state.
> >>>       * Release gpio's patch added
> >>>
> >>> Best regards,
> >>>       Sergio Paracuellos
> >>>
> >>> Sergio Paracuellos (5):
> >>>     staging: mt7621-pci: use gpios for properly reset
> >>>     staging: mt7621-pci: change value for 'PERST_DELAY_US'
> >>>     staging: mt7621-dts: make use of 'reset-gpios' property for pci
> >>>     staging: mt7621-pci: bindings: update doc accordly to last changes
> >>>     staging: mt7621-pci: release gpios after pci initialization
> >>>
> >>>    drivers/staging/mt7621-dts/mt7621.dtsi        |  11 +-
> >>>    .../mt7621-pci/mediatek,mt7621-pci.txt        |   7 +-
> >>>    drivers/staging/mt7621-pci/pci-mt7621.c       | 107 +++++++++++-------
> >>>    3 files changed, 80 insertions(+), 45 deletions(-)
> >>>
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

end of thread, other threads:[~2020-03-15 12:41 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-12  9:00 [PATCH v3 0/5] staging: mt7621-pci: re-do reset boot process Sergio Paracuellos
2020-03-12  9:00 ` [PATCH v3 1/5] staging: mt7621-pci: use gpios for properly reset Sergio Paracuellos
2020-03-12  9:00 ` [PATCH v3 2/5] staging: mt7621-pci: change value for 'PERST_DELAY_US' Sergio Paracuellos
2020-03-12  9:00 ` [PATCH v3 3/5] staging: mt7621-dts: make use of 'reset-gpios' property for pci Sergio Paracuellos
2020-03-12  9:00 ` [PATCH v3 4/5] staging: mt7621-pci: bindings: update doc accordly to last changes Sergio Paracuellos
2020-03-12  9:00 ` [PATCH v3 5/5] staging: mt7621-pci: release gpios after pci initialization Sergio Paracuellos
2020-03-13  3:51 ` [PATCH v3 0/5] staging: mt7621-pci: re-do reset boot process Greg Ungerer
2020-03-13  5:45   ` Sergio Paracuellos
2020-03-13  6:31     ` Sergio Paracuellos
2020-03-15 12:32     ` Greg Ungerer
2020-03-15 12:39       ` Sergio Paracuellos

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).