From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.6 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_2 autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 6DD2DC433E1 for ; Thu, 27 Aug 2020 17:42:35 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 3E8342068E for ; Thu, 27 Aug 2020 17:42:35 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1598550155; bh=zyfUowNsWV1AH9t2Y/OhVrtMRfKaUwAivYAzLDicwBM=; h=Date:From:To:Cc:Subject:In-Reply-To:References:List-ID:From; b=K7RIFEqrGmmJO+N6Hn1DQSwHClmWt/AvZ2fB7QUFqpTRU7xWPrkfUoKzFChDm1BQc mAuN9vW79+U0y9f5D3hhzHT3znulR/X1XLNHlvcveGiyOsG/xmZnsY5gzWF33VK6oL Dz++O9UtBYR+aRAxJ7h7cZxpUdyIrzkKLSHz4FeI= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726867AbgH0Rme (ORCPT ); Thu, 27 Aug 2020 13:42:34 -0400 Received: from mail.kernel.org ([198.145.29.99]:37152 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726307AbgH0Rmd (ORCPT ); Thu, 27 Aug 2020 13:42:33 -0400 Received: from coco.lan (ip5f5ad5a8.dynamic.kabel-deutschland.de [95.90.213.168]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 2A2762087E; Thu, 27 Aug 2020 17:42:28 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1598550152; bh=zyfUowNsWV1AH9t2Y/OhVrtMRfKaUwAivYAzLDicwBM=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=07ZU3viQ9K4W/2j7rtUmnY13uTbszZo4JxebwRXpVTU0rbPDvX8crv4sulA5txXDE kYmlUOuNr9MdYmatsOfu9hXzg5jshHEN71HClrwf41F7Sbk0qcHLXN5PzjHJnVgRLn r0uWVOa7IlmgAj8HdF77sDzEoeitLX/MPi0J6RwU= Date: Thu, 27 Aug 2020 19:42:25 +0200 From: Mauro Carvalho Chehab To: Steve deRosier Cc: Kalle Valo , linuxarm@huawei.com, mauro.chehab@huawei.com, John Stultz , Manivannan Sadhasivam , "David S. Miller" , Jakub Kicinski , Maital Hahn , "Gustavo A. R. Silva" , Raz Bouganim , Tony Lindgren , Dinghao Liu , Johannes Berg , Fuqian Huang , linux-wireless , Network Development , LKML Subject: Re: [PATCH] Revert "wlcore: Adding suppoprt for IGTK key in wlcore driver" Message-ID: <20200827194225.281eb7dc@coco.lan> In-Reply-To: References: X-Mailer: Claws Mail 3.17.6 (GTK+ 2.24.32; x86_64-redhat-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Em Thu, 27 Aug 2020 08:48:30 -0700 Steve deRosier escreveu: > On Tue, Aug 25, 2020 at 10:49 PM Mauro Carvalho Chehab > 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. 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 8.9.0.0.79 FRev 8.2.0.0.242 Looking at: https://git.ti.com/cgit/wilink8-wlan/wl18xx_fw/ It sounds that there's a newer version released this year: 2020-05-28 Updated to FW 8.9.0.0.81 2018-07-29 Updated to FW 8.9.0.0.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 8.9.0.0.79 65b1c68c63f9 wl18xx: update firmware file 8.9.0.0.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 8.9.0.0.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. > 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. Thanks, Mauro