From: Sergio Paracuellos <sergio.paracuellos@gmail.com>
To: Greg Ungerer <gerg@kernel.org>
Cc: ryder.lee@mediatek.com, Greg KH <gregkh@linuxfoundation.org>,
driverdev-devel@linuxdriverproject.org, weijie.gao@mediatek.com,
NeilBrown <neil@brown.name>
Subject: Re: [PATCH v4 0/6] staging: mt7621-pci: re-do reset boot process
Date: Thu, 19 Mar 2020 14:43:44 +0100 [thread overview]
Message-ID: <CAMhs-H-GEAA79HdzvPJnJ-hBOKDUh15GiQ0qWKw2tKCpbVvCHw@mail.gmail.com> (raw)
In-Reply-To: <0e15884b-de8d-a7a0-7560-3221a2c8a978@kernel.org>
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:
>
> 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.
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
>
>
>
> > 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
next prev parent reply other threads:[~2020-03-19 13:44 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-13 20:09 [PATCH v4 0/6] staging: mt7621-pci: re-do reset boot process Sergio Paracuellos
2020-03-13 20:09 ` [PATCH v4 1/6] staging: mt7621-pci: use gpios for properly reset Sergio Paracuellos
2020-03-13 20:09 ` [PATCH v4 2/6] staging: mt7621-pci: change value for 'PERST_DELAY_MS' Sergio Paracuellos
2020-03-13 20:09 ` [PATCH v4 3/6] staging: mt7621-dts: make use of 'reset-gpios' property for pci Sergio Paracuellos
2020-03-13 20:09 ` [PATCH v4 4/6] staging: mt7621-pci: bindings: update doc accordly to last changes Sergio Paracuellos
2020-03-13 20:09 ` [PATCH v4 5/6] staging: mt7621-pci: release gpios after pci initialization Sergio Paracuellos
2020-03-20 14:59 ` Chuanhong Guo
2020-03-20 15:28 ` Sergio Paracuellos
2020-03-20 15:39 ` Chuanhong Guo
2020-03-20 16:32 ` Sergio Paracuellos
2020-03-20 16:34 ` Sergio Paracuellos
2020-03-21 6:36 ` Chuanhong Guo
2020-03-21 7:28 ` Sergio Paracuellos
2020-03-13 20:09 ` [PATCH v4 6/6] staging: mt7621-pci: delete no more needed 'mt7621_reset_port' Sergio Paracuellos
2020-03-14 10:42 ` [PATCH v4 0/6] staging: mt7621-pci: re-do reset boot process Sergio Paracuellos
2020-03-15 12:35 ` Sergio Paracuellos
2020-03-19 12:42 ` Greg Ungerer
2020-03-19 13:43 ` Sergio Paracuellos [this message]
2020-03-19 13:47 ` Sergio Paracuellos
2020-03-19 14:28 ` Greg Ungerer
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAMhs-H-GEAA79HdzvPJnJ-hBOKDUh15GiQ0qWKw2tKCpbVvCHw@mail.gmail.com \
--to=sergio.paracuellos@gmail.com \
--cc=driverdev-devel@linuxdriverproject.org \
--cc=gerg@kernel.org \
--cc=gregkh@linuxfoundation.org \
--cc=neil@brown.name \
--cc=ryder.lee@mediatek.com \
--cc=weijie.gao@mediatek.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).