Sorry the mail was sent while I was still writing :) On Thu, Mar 19, 2020 at 2:43 PM Sergio Paracuellos <sergio.paracuellos@xxxxxxxxx> wrote: > > Hi Greg, > > On Thu, Mar 19, 2020 at 1:42 PM Greg Ungerer <gerg@xxxxxxxxxx> wrote: > > > > Hi Sergio, > > > > On 14/3/20 6:09 am, 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. > > > > > > Hope this helps. > > > > > > Changes in v4: > > > * Make use of 'devm_gpiod_get_index_optional' instead of 'devm_gpiod_get_index'. > > > * Use 'dev_err' instead of 'dev_notice' and return ERR_PTR if > > > 'devm_gpiod_get_index_optional' fails. > > > * Rename pers dealy macro to PERST_DELAY_MS. > > > * Add new patch 6 which removes function 'mt7621_reset_port' not needed anymore. > > > > Testing this v4 series always fails during boot with: > > > > ... > > NET: Registered protocol family 17 > > NET: Registered protocol family 15 > > 8021q: 802.1Q VLAN Support v1.8 > > Loading compiled-in X.509 certificates > > AppArmor: AppArmor sha1 policy hashing enabled > > > > 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: Parsing DT failed > > mt7621-pci: probe of 1e140000.pcie failed with error -16 > > Looks like the gpio is valid but has been assigned to anything else. > It looks like a device-tree issue for me. > Does your hardware follows the indications of the mediatek application note? > > https://github.com/openwrt/openwrt/files/4317436/an-mt7621-pcie-application-note-v0.1.pdf > > To be able to test this you can just change the device tree and set > reset gpios to only perst-reset pin > > reset-gpios = <&gpio 19 GPIO_ACTIVE_LOW>; > > or avoid the "return PTR_ERR(port->gpio_rst);" after the call to > 'devm_gpiod_get_index_optional'. > > Or just make an exception if the pin is busy, which seems to be the > problem here: diff --git a/drivers/staging/mt7621-pci/pci-mt7621.c b/drivers/staging/mt7621-pci/pci-mt7621.c index 13b272597442..767b10fce18f 100644 --- a/drivers/staging/mt7621-pci/pci-mt7621.c +++ b/drivers/staging/mt7621-pci/pci-mt7621.c @@ -369,7 +369,8 @@ static int mt7621_pcie_parse_port(struct mt7621_pcie *pcie, GPIOD_OUT_LOW); if (IS_ERR(port->gpio_rst)) { dev_err(dev, "Failed to get GPIO for PCIe%d\n", slot); - return PTR_ERR(port->gpio_rst); + if (PTR_ERR(port->gpio_rst) != -EBUSY) + return PTR_ERR(port->gpio_rst); } port->slot = slot; > > > > > UBI error: cannot open mtd 3, error -19 > > hctosys: unable to open rtc device (rtc0) > > cfg80211: Loading compiled-in X.509 certificates for regulatory database > > cfg80211: Loaded X.509 cert 'sforshee: 00b28ddf47aef9cea7' > > ... > > > > It never hangs, always boots up all the way. But always the same failure > > with PCIe. If it hangs it should hang on the pci initilization process... > > This series has been applied to the staging tree and are properly > running for me in gnubee pc1. > > You should test using all confirmed changes in staging-next branch and > this patch which fix a wrong register usage issue: > > http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2020-March/142472.html > > > > > > Regards > > Greg Hope this helps. Best regards, Sergio Paracuellos > > > > > > > > > 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 (6): > > > staging: mt7621-pci: use gpios for properly reset > > > staging: mt7621-pci: change value for 'PERST_DELAY_MS' > > > 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 > > > staging: mt7621-pci: delete no more needed 'mt7621_reset_port' > > > > > > drivers/staging/mt7621-dts/mt7621.dtsi | 11 +- > > > .../mt7621-pci/mediatek,mt7621-pci.txt | 7 +- > > > drivers/staging/mt7621-pci/pci-mt7621.c | 122 ++++++++++-------- > > > 3 files changed, 82 insertions(+), 58 deletions(-) > > > _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
