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

[Michał Kazior]

On Mon, 25 Mar 2019 at 21:23, Ben Greear <greearb at candelatech.com> wrote:
>
> On 3/25/19 1:08 PM, Michał Kazior wrote:
> > On Mon, 25 Mar 2019 at 16:55, Ben Greear <greearb at candelatech.com> wrote:
> >> On 3/25/19 5:14 AM, Michał Kazior wrote:
> >>> 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.
> >>
> >> The crash is related to the v1 chip.  Is there a good way to detect
> >> that this is the chip in question and only apply this work-around
> >> for the problem chip?
> >
> > I don't know of any good way to do that.
> >
> > You could consider device-tree but that would be no different from
> > having a module blacklist in the C7v1 build recipe, or to not build
> > the module at all. That is unless you actually want to make v1 chip
> > work with ath10k at which point there's more fun to be had before it
> > can actually work.
>
> I remember v1, and I have no interest in trying to make it work :)
>
> If we could blacklist certain pci slots in the ath10k driver, I guess
> that would work?
>
> I think the goal is to not use the v1 chip, but allow users to add a
> v2 NIC to the platform, so driver still needs to load.

That makes sense, but I don't see how blacklisting pci slots would
help someone putting v2 nic into C7v1 mobo? Won't the slot be the same
regardless what nic is put?

The best thing I can come up with is something like this:

--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -3629,6 +3629,19 @@ static int ath10k_pci_probe(struct pci_dev *pdev,
                goto err_deinit_irq;
        }

+       if (hw_rev == ATH10K_HW_QCA988X) {
+               /* v1 can crash the system on chip_reset()
+                * so all we can do is keep our fingers
+                * crossed v2 never reports 0 without a
+                * chip_reset()
+                */
+               if (ath10k_pci_soc_read32(ar, SOC_CHIP_ID_ADDRESS) == 0) {
+                       ath10k_err(ar, "qca9880 v1 is chip not supported");
+                       ret = -ENOTSUP;
+                       goto err_free_irq;
+               }
+       }
+
        ret = ath10k_pci_chip_reset(ar);
        if (ret) {
                ath10k_err(ar, "failed to reset chip: %d\n", ret);

I didn't test it. Someone needs to compile and test and make sure v2
doesn't regress when fw hangs and cold & warm host cpu resets are
mixed in.


Michał

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