On Mon, 25 Mar 2019 at 21:23, Ben Greear <greearb@xxxxxxxxxxxxxxx> wrote: > > On 3/25/19 1:08 PM, Michał Kazior wrote: > > On Mon, 25 Mar 2019 at 16:55, Ben Greear <greearb@xxxxxxxxxxxxxxx> wrote: > >> On 3/25/19 5:14 AM, Michał Kazior wrote: > >>> On Sat, 23 Mar 2019 at 08:20, Arend Van Spriel > >>> <arend.vanspriel@xxxxxxxxxxxx> 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ł