Re: [PATCH v4 0/6] staging: mt7621-pci: re-do reset boot process

[Sergio Paracuellos <sergio.paracuellos@gmail.com>]

Sorry the mail was sent while I was still writing :)

On Thu, Mar 19, 2020 at 2:43 PM Sergio Paracuellos
<sergio.paracuellos@gmail.com> wrote:
>
> Hi Greg,
>
> On Thu, Mar 19, 2020 at 1:42 PM Greg Ungerer <gerg@kernel.org> 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@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel