From: Len Brown <lenb@kernel.org>
To: Jonathan McDowell <noodles@earth.li>
Cc: Luca Coelho <luca@coelho.fi>,
kvalo@codeaurora.org, linux-wireless@vger.kernel.org
Subject: Re: [PATCH v5.6] iwlwifi: don't send GEO_TX_POWER_LIMIT if no wgds table
Date: Thu, 19 Mar 2020 13:32:10 -0400 [thread overview]
Message-ID: <CAJvTdKmrKJQee-WrG4UCo0FYrr++Er=mftrdLKjcCuy23N+HPw@mail.gmail.com> (raw)
In-Reply-To: <20200318213440.GO311@earth.li>
Tested-by: Len Brown <len.brown@intel.com>
on Linux-5.6-rc6 WITH this patch:
[ 2.923632] iwlwifi 0000:3a:00.0: enabling device (0000 -> 0002)
[ 2.933367] iwlwifi 0000:3a:00.0: loaded firmware version
17.3216344376.0 3160-17.ucode op_mode iwlmvm
[ 3.407034] iwlwifi 0000:3a:00.0: Detected Intel(R) Dual Band
Wireless AC 3160, REV=0x164
[ 3.428304] iwlwifi 0000:3a:00.0: base HW address: e4:02:9b:c0:4d:f1
[ 3.546341] iwlwifi 0000:3a:00.0 wlp58s0: renamed from wlan0
Linux 5.6-rc6 WITHOUT this patch:
[ 2.773544] iwlwifi 0000:3a:00.0: enabling device (0000 -> 0002)
[ 2.829310] iwlwifi 0000:3a:00.0: loaded firmware version
17.3216344376.0 3160-17.ucode op_mode iwlmvm
[ 3.348050] iwlwifi 0000:3a:00.0: Detected Intel(R) Dual Band
Wireless AC 3160, REV=0x164
[ 3.721261] iwlwifi 0000:3a:00.0: base HW address: e4:02:9b:c0:4d:f1
[ 4.431592] iwlwifi 0000:3a:00.0 wlp58s0: renamed from wlan0
[ 5.581857] iwlwifi 0000:3a:00.0: Microcode SW error detected.
Restarting 0x2000000.
[ 5.583340] iwlwifi 0000:3a:00.0: Start IWL Error Log Dump:
[ 5.584693] iwlwifi 0000:3a:00.0: Status: 0x00000040, count: 6
[ 5.586150] iwlwifi 0000:3a:00.0: Loaded firmware version:
17.3216344376.0 3160-17.ucode
[ 5.587194] iwlwifi 0000:3a:00.0: 0x00000038 | BAD_COMMAND
...
and wifi is permanently dead...
thanks!
-Len
On Wed, Mar 18, 2020 at 5:57 PM Jonathan McDowell <noodles@earth.li> wrote:
>
> On Wed, Mar 18, 2020 at 08:12:54AM +0200, Luca Coelho wrote:
> > From: Golan Ben Ami <golan.ben.ami@intel.com>
> >
> > The GEO_TX_POWER_LIMIT command was sent although
> > there is no wgds table, so the fw got wrong SAR values
> > from the driver.
> >
> > Fix this by avoiding sending the command if no wgds
> > tables are available.
> >
> > Signed-off-by: Golan Ben Ami <golan.ben.ami@intel.com>
> > Fixes: 39c1a9728f93 ("iwlwifi: refactor the SAR tables from mvm to acpi")
> > Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
>
> Thanks Luca.
>
> Works for me on an AC 3168 with firmware 29.1044073957.0
>
> Tested-By: Jonathan McDowell <noodles@earth.li>
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=206395
> Cc: stable@vger.kernel.org # 5.5.10+
>
> > ---
> > drivers/net/wireless/intel/iwlwifi/fw/acpi.c | 14 ++++++++------
> > drivers/net/wireless/intel/iwlwifi/fw/acpi.h | 14 ++++++++------
> > drivers/net/wireless/intel/iwlwifi/mvm/fw.c | 9 ++++++++-
> > 3 files changed, 24 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/net/wireless/intel/iwlwifi/fw/acpi.c b/drivers/net/wireless/intel/iwlwifi/fw/acpi.c
> > index 48d375a86d86..ba2aff3af0fe 100644
> > --- a/drivers/net/wireless/intel/iwlwifi/fw/acpi.c
> > +++ b/drivers/net/wireless/intel/iwlwifi/fw/acpi.c
> > @@ -6,7 +6,7 @@
> > * GPL LICENSE SUMMARY
> > *
> > * Copyright(c) 2017 Intel Deutschland GmbH
> > - * Copyright (C) 2019 Intel Corporation
> > + * Copyright (C) 2019 - 2020 Intel Corporation
> > *
> > * This program is free software; you can redistribute it and/or modify
> > * it under the terms of version 2 of the GNU General Public License as
> > @@ -27,7 +27,7 @@
> > * BSD LICENSE
> > *
> > * Copyright(c) 2017 Intel Deutschland GmbH
> > - * Copyright (C) 2019 Intel Corporation
> > + * Copyright (C) 2019 - 2020 Intel Corporation
> > * All rights reserved.
> > *
> > * Redistribution and use in source and binary forms, with or without
> > @@ -491,13 +491,13 @@ int iwl_validate_sar_geo_profile(struct iwl_fw_runtime *fwrt,
> > }
> > IWL_EXPORT_SYMBOL(iwl_validate_sar_geo_profile);
> >
> > -void iwl_sar_geo_init(struct iwl_fw_runtime *fwrt,
> > - struct iwl_per_chain_offset_group *table)
> > +int iwl_sar_geo_init(struct iwl_fw_runtime *fwrt,
> > + struct iwl_per_chain_offset_group *table)
> > {
> > int ret, i, j;
> >
> > if (!iwl_sar_geo_support(fwrt))
> > - return;
> > + return -EOPNOTSUPP;
> >
> > ret = iwl_sar_get_wgds_table(fwrt);
> > if (ret < 0) {
> > @@ -505,7 +505,7 @@ void iwl_sar_geo_init(struct iwl_fw_runtime *fwrt,
> > "Geo SAR BIOS table invalid or unavailable. (%d)\n",
> > ret);
> > /* we don't fail if the table is not available */
> > - return;
> > + return -ENOENT;
> > }
> >
> > BUILD_BUG_ON(ACPI_NUM_GEO_PROFILES * ACPI_WGDS_NUM_BANDS *
> > @@ -530,5 +530,7 @@ void iwl_sar_geo_init(struct iwl_fw_runtime *fwrt,
> > i, j, value[1], value[2], value[0]);
> > }
> > }
> > +
> > + return 0;
> > }
> > IWL_EXPORT_SYMBOL(iwl_sar_geo_init);
> > diff --git a/drivers/net/wireless/intel/iwlwifi/fw/acpi.h b/drivers/net/wireless/intel/iwlwifi/fw/acpi.h
> > index 4a6e8262974b..5590e5cc8fbb 100644
> > --- a/drivers/net/wireless/intel/iwlwifi/fw/acpi.h
> > +++ b/drivers/net/wireless/intel/iwlwifi/fw/acpi.h
> > @@ -6,7 +6,7 @@
> > * GPL LICENSE SUMMARY
> > *
> > * Copyright(c) 2017 Intel Deutschland GmbH
> > - * Copyright(c) 2018 - 2019 Intel Corporation
> > + * Copyright(c) 2018 - 2020 Intel Corporation
> > *
> > * This program is free software; you can redistribute it and/or modify
> > * it under the terms of version 2 of the GNU General Public License as
> > @@ -27,7 +27,7 @@
> > * BSD LICENSE
> > *
> > * Copyright(c) 2017 Intel Deutschland GmbH
> > - * Copyright(c) 2018 - 2019 Intel Corporation
> > + * Copyright(c) 2018 - 2020 Intel Corporation
> > * All rights reserved.
> > *
> > * Redistribution and use in source and binary forms, with or without
> > @@ -171,8 +171,9 @@ bool iwl_sar_geo_support(struct iwl_fw_runtime *fwrt);
> > int iwl_validate_sar_geo_profile(struct iwl_fw_runtime *fwrt,
> > struct iwl_host_cmd *cmd);
> >
> > -void iwl_sar_geo_init(struct iwl_fw_runtime *fwrt,
> > - struct iwl_per_chain_offset_group *table);
> > +int iwl_sar_geo_init(struct iwl_fw_runtime *fwrt,
> > + struct iwl_per_chain_offset_group *table);
> > +
> > #else /* CONFIG_ACPI */
> >
> > static inline void *iwl_acpi_get_object(struct device *dev, acpi_string method)
> > @@ -243,9 +244,10 @@ static inline int iwl_validate_sar_geo_profile(struct iwl_fw_runtime *fwrt,
> > return -ENOENT;
> > }
> >
> > -static inline void iwl_sar_geo_init(struct iwl_fw_runtime *fwrt,
> > - struct iwl_per_chain_offset_group *table)
> > +static inline int iwl_sar_geo_init(struct iwl_fw_runtime *fwrt,
> > + struct iwl_per_chain_offset_group *table)
> > {
> > + return -ENOENT;
> > }
> >
> > #endif /* CONFIG_ACPI */
> > diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/fw.c b/drivers/net/wireless/intel/iwlwifi/mvm/fw.c
> > index 54c094e88474..98263cd37944 100644
> > --- a/drivers/net/wireless/intel/iwlwifi/mvm/fw.c
> > +++ b/drivers/net/wireless/intel/iwlwifi/mvm/fw.c
> > @@ -762,10 +762,17 @@ static int iwl_mvm_sar_geo_init(struct iwl_mvm *mvm)
> > u16 cmd_wide_id = WIDE_ID(PHY_OPS_GROUP, GEO_TX_POWER_LIMIT);
> > union geo_tx_power_profiles_cmd cmd;
> > u16 len;
> > + int ret;
> >
> > cmd.geo_cmd.ops = cpu_to_le32(IWL_PER_CHAIN_OFFSET_SET_TABLES);
> >
> > - iwl_sar_geo_init(&mvm->fwrt, cmd.geo_cmd.table);
> > + ret = iwl_sar_geo_init(&mvm->fwrt, cmd.geo_cmd.table);
> > + /*
> > + * It is a valid scenario to not support SAR, or miss wgds table,
> > + * but in that case there is no need to send the command.
> > + */
> > + if (ret)
> > + return 0;
> >
> > cmd.geo_cmd.table_revision = cpu_to_le32(mvm->fwrt.geo_rev);
> >
> > --
> > 2.25.1
> >
>
> J.
>
> --
> I don't know. I'm a dog. | .''`. Debian GNU/Linux Developer
> | : :' : Happy to accept PGP signed
> | `. `' or encrypted mail - RSA
> | `- key on the keyservers.
--
Len Brown, Intel Open Source Technology Center
next prev parent reply other threads:[~2020-03-19 17:32 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-18 6:12 [PATCH v5.6] iwlwifi: don't send GEO_TX_POWER_LIMIT if no wgds table Luca Coelho
2020-03-18 21:34 ` Jonathan McDowell
2020-03-19 17:32 ` Len Brown [this message]
2020-03-19 19:50 ` Len Brown
2020-03-23 16:38 ` Kalle Valo
2020-03-28 11:02 ` Felipe Contreras
2020-03-28 13:10 ` Kalle Valo
2020-03-31 20:29 ` Felipe Contreras
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='CAJvTdKmrKJQee-WrG4UCo0FYrr++Er=mftrdLKjcCuy23N+HPw@mail.gmail.com' \
--to=lenb@kernel.org \
--cc=kvalo@codeaurora.org \
--cc=linux-wireless@vger.kernel.org \
--cc=luca@coelho.fi \
--cc=noodles@earth.li \
/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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).