Netdev Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Steve deRosier <email@example.com>
To: Mauro Carvalho Chehab <firstname.lastname@example.org>
Cc: Kalle Valo <email@example.com>,
John Stultz <firstname.lastname@example.org>,
Manivannan Sadhasivam <email@example.com>,
"David S. Miller" <firstname.lastname@example.org>,
Jakub Kicinski <email@example.com>, Maital Hahn <firstname.lastname@example.org>,
"Gustavo A. R. Silva" <email@example.com>,
Raz Bouganim <firstname.lastname@example.org>,
Tony Lindgren <email@example.com>,
Dinghao Liu <firstname.lastname@example.org>,
Johannes Berg <email@example.com>,
Fuqian Huang <firstname.lastname@example.org>,
Network Development <email@example.com>,
Subject: Re: [PATCH] Revert "wlcore: Adding suppoprt for IGTK key in wlcore driver"
Date: Thu, 27 Aug 2020 13:36:28 -0700 [thread overview]
Message-ID: <CALLGbRLsQpdtrcV9ydz4KJ4A9uaj4P1EhbF0_yMxcdLvOmnY9Q@mail.gmail.com> (raw)
On Thu, Aug 27, 2020 at 10:42 AM Mauro Carvalho Chehab
> Em Thu, 27 Aug 2020 08:48:30 -0700
> Steve deRosier <firstname.lastname@example.org> escreveu:
> > On Tue, Aug 25, 2020 at 10:49 PM Mauro Carvalho Chehab
> > <email@example.com> wrote:
> > >
> > > This patch causes a regression betwen Kernel 5.7 and 5.8 at wlcore:
> > > with it applied, WiFi stops working, and the Kernel starts printing
> > > this message every second:
> > >
> > > wlcore: PHY firmware version: Rev 22.214.171.124.242
> > > wlcore: firmware booted (Rev 126.96.36.199.79)
> > > wlcore: ERROR command execute failure 14
> > Only if NO firmware for the device in question supports the `KEY_IGTK`
> > value, then this revert is appropriate. Otherwise, it likely isn't.
> Yeah, that's what I suspect too: some specific firmware is required
> for KEY_IGTK to work.
> > My suspicion is that the feature that `KEY_IGTK` is enabling is
> > specific to a newer firmware that Mauro hasn't upgraded to. What the
> > OP should do is find the updated firmware and give it a try.
> I didn't try checking if linux-firmware tree has a newer version on
> it. I'm using Debian Bullseye on this device. So, I suspect that
> it may have a relatively new firmware.
> Btw, that's also the version that came together with Fedora 32:
> $ strings /lib/firmware/ti-connectivity/wl18xx-fw-4.bin |grep FRev
> FRev 188.8.131.52.79
> FRev 184.108.40.206.242
> Looking at:
> It sounds that there's a newer version released this year:
> 2020-05-28 Updated to FW 220.127.116.11.81
> 2018-07-29 Updated to FW 18.104.22.168.79
> However, it doesn't reached linux-firmware upstream yet:
> $ git log --pretty=oneline ti-connectivity/wl18xx-fw-4.bin
> 3a5103fc3c29 wl18xx: update firmware file 22.214.171.124.79
> 65b1c68c63f9 wl18xx: update firmware file 126.96.36.199.76
> dbb85a5154a5 wl18xx: update firmware file
> 69a250dd556b wl18xx: update firmware file
> dbe3f134bb69 wl18xx: update firmware file, remove conf file
> dab4b79b3fbc wl18xx: add version 4 of the wl18xx firmware
> > AND - since there's some firmware the feature doesn't work with, the
> > driver should be fixed to detect the running firmware version and not
> > do things that the firmware doesn't support. AND the firmware writer
> > should also make it so the firmware doesn't barf on bad input and
> > instead rejects it politely.
> Agreed. The main issue here seems to be that the current patch
> assumes that this feature is available. A proper approach would
> be to check if this feature is available before trying to use it.
> Now, I dunno if version 188.8.131.52.81 has what's required for it to
> work - or if KEY_IGTK require some custom firmware version.
> If it works with such version, one way would be to add a check
> for this specific version, disabling KEY_IGTK otherwise.
> Also, someone from TI should be sending the newer version to
> be added at linux-firmware.
> I'll try to do a test maybe tomorrow.
I think we're totally agreed on all of the above points.
Fundamentally: the orig patch should've been coded defensively and
tested properly since clearly it causes certain firmwares to break.
Be nice if TI would both update the firmware and also update the
driver to detect the relevant version for features. I don't know
about this one, but I do know the QCA firmwares (and others) have a
set of feature flags that are detected by the drivers to determine
what is supported.
I look forward to hearing the results of your test. This whole thing
has gotten me interested. I'd be tempted to pull out the relevant dev
boards and play with them myself, but IIRC they got sent back to a
previous employer and I don't have access to them anymore.
> > But I will say I'm making an educated guess; while I have played with
> > the TI devices in the past, it was years ago and I won't claim to be
> > an expert. I also am unable to fix it myself at this time.
> > I'd just rather see it fixed properly instead of a knee-jerk reaction
> > of reverting it simply because the OP doesn't have current firmware.
> > And let's revisit the discussion of having a kernel splat because an
> > unrelated piece of code fails yet the driver does exactly what it is
> > supposed to do. We shouldn't be dumping registers and stack-trace when
> > the code that crashed has nothing to do with the registers and
> > stack-trace outputted. It is a false positive. A simple printk WARN
> > or ERROR should output notifying us that the chip firmware has crashed
> > and why. IMHO.
next prev parent reply other threads:[~2020-08-27 20:37 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-08-26 5:49 Mauro Carvalho Chehab
2020-08-27 8:05 ` Kalle Valo
2020-08-27 15:48 ` Steve deRosier
2020-08-27 17:42 ` Mauro Carvalho Chehab
2020-08-27 20:36 ` Steve deRosier [this message]
2020-08-28 7:44 ` Mauro Carvalho Chehab
2020-08-28 8:31 ` Mauro Carvalho Chehab
2020-08-28 7:41 ` Kalle Valo
2020-09-01 9:31 ` Kalle Valo
[not found] ` <20200901093129.8A0FAC433B1@smtp.codeaurora.org>
2020-09-01 10:59 ` [EXTERNAL] " Bouganim, Raz
2020-09-01 11:45 ` Kalle Valo
2020-09-07 6:37 ` [PATCH v2] net: wireless: wlcore: fix support for IGTK key Mauro Carvalho Chehab
2020-09-07 7:31 ` [EXTERNAL] " Bouganim, Raz
2020-09-07 8:38 ` [PATCH] Revert "wlcore: Adding suppoprt for IGTK key in wlcore driver" Kalle Valo
2020-09-07 8:40 ` Kalle Valo
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:
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--subject='Re: [PATCH] Revert "wlcore: Adding suppoprt for IGTK key in wlcore driver"' \
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
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).