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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Sergio,

On 19/3/20 11:43 pm, Sergio Paracuellos wrote:
On Thu, Mar 19, 2020 at 1:42 PM Greg Ungerer <gerg@xxxxxxxxxx> wrote:
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?

Yes, as per table 2-2. GPIO pins 7 and 8 are used for other purposes
on my hardware - and my devicetree has those assigned for those
other purposes. They are not available for, or used for, PCI reset.

Regards
Greg



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@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel



[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux