Netdev Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Steve deRosier <derosier@gmail.com>
To: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>,
	Kalle Valo <kvalo@codeaurora.org>
Cc: linuxarm@huawei.com, mauro.chehab@huawei.com,
	John Stultz <john.stultz@linaro.org>,
	Manivannan Sadhasivam <mani@kernel.org>,
	"David S. Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>, Maital Hahn <maitalm@ti.com>,
	"Gustavo A. R. Silva" <gustavo@embeddedor.com>,
	Raz Bouganim <r-bouganim@ti.com>,
	Tony Lindgren <tony@atomide.com>,
	Dinghao Liu <dinghao.liu@zju.edu.cn>,
	Johannes Berg <johannes.berg@intel.com>,
	Fuqian Huang <huangfq.daxian@gmail.com>,
	linux-wireless <linux-wireless@vger.kernel.org>,
	Network Development <netdev@vger.kernel.org>,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] Revert "wlcore: Adding suppoprt for IGTK key in wlcore driver"
Date: Thu, 27 Aug 2020 08:48:30 -0700	[thread overview]
Message-ID: <CALLGbRL+duiHFd3w7hcD=u47k+JM5rLpOkMrRpW0aQm=oTfUnA@mail.gmail.com> (raw)
In-Reply-To: <f0a2cb7ea606f1a284d4c23cbf983da2954ce9b6.1598420968.git.mchehab+huawei@kernel.org>

On Tue, Aug 25, 2020 at 10:49 PM Mauro Carvalho Chehab
<mchehab+huawei@kernel.org> 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 8.2.0.0.242
>    wlcore: firmware booted (Rev 8.9.0.0.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.
 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.

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.

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.

- Steve

  parent reply	other threads:[~2020-08-27 15:49 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 [this message]
2020-08-27 17:42   ` Mauro Carvalho Chehab
2020-08-27 20:36     ` Steve deRosier
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

Reply instructions:

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:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CALLGbRL+duiHFd3w7hcD=u47k+JM5rLpOkMrRpW0aQm=oTfUnA@mail.gmail.com' \
    --to=derosier@gmail.com \
    --cc=davem@davemloft.net \
    --cc=dinghao.liu@zju.edu.cn \
    --cc=gustavo@embeddedor.com \
    --cc=huangfq.daxian@gmail.com \
    --cc=johannes.berg@intel.com \
    --cc=john.stultz@linaro.org \
    --cc=kuba@kernel.org \
    --cc=kvalo@codeaurora.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=linuxarm@huawei.com \
    --cc=maitalm@ti.com \
    --cc=mani@kernel.org \
    --cc=mauro.chehab@huawei.com \
    --cc=mchehab+huawei@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=r-bouganim@ti.com \
    --cc=tony@atomide.com \
    --subject='Re: [PATCH] Revert "wlcore: Adding suppoprt for IGTK key in wlcore driver"' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* 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).