[OpenWrt-Devel] [PATCH] ath10k: reset chip after supported check

[Michał Kazior]

On Sat, 23 Mar 2019 at 08:20, Arend Van Spriel
<arend.vanspriel at broadcom.com> wrote:
>
> * resending with corrected email address from Kalle
> --------------------------------------------------------------------
> + Michał

Thanks!


> On 3/22/2019 8:25 PM, Christian Lamparter wrote:
>  > On Friday, March 22, 2019 7:58:40 PM CET Tomislav Požega wrote:
>  >> When chip reset is done before the chip is checked if supported
>  >> there will be crash. Previous behaviour caused bootloops on
>  >> Archer C7 v1 units, this patch allows clean device boot without
>  >> excluding ath10k driver.

Can you elaborate more a bit? What kind of crashes are you seeing?
What does the bootloop look like? Do you have uart connected to
diagnose?

Didn't C7 v1 have the old QCA9880 hw v1 which isn't really supported
by ath10k? I recall the v1 chip was really buggy and required
hammering registers sometimes to get things working.


[...]
> Looking at the commit subject makes me suspicious whether this is a
> proper fix.
[...]
> It seems to me the chip reset was done explicitly *before* reading the
> chipid for a reason.
>
> """
> ath10k: reset chip before reading chip_id in probe
> There are some very rare cases with some hardware
> configuration that the device doesn't init quickly
> enough in which case reading chip_id yielded 0.
> This caused driver to subsequently fail to setup
> the device.
>
> Signed-off-by: Michal Kazior <michal.kazior at tieto.com>
> Signed-off-by: Kalle Valo <kvalo at qca.qualcomm.com>
> """

Good catch, Arend!


> Might be the ath10k_pci_chip_reset() function needs to be modified to
> work properly for Archer C7 v1 units.

When driver boots up hardware is in an unknown state. It could've been
power cycled or it could've remained powered via pcie aux while host
cpu went through a warm reset (eg. watchdog). For what it's worth it
could still be beaconing if that is what it was doing before host cpu
reset. Therefore the device can be in a messy state and it should be
reset before talking to it or else you risk reading garbage.

I don't recall details about this particular commit but given the
timeline of commits at the time this change suggests this was related
to supporting qca61x4. In other words your change risks breaking some
qca61x4 chips.


Michał

_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel