linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5.6] iwlwifi: don't send GEO_TX_POWER_LIMIT if no wgds table
@ 2020-03-18  6:12 Luca Coelho
  2020-03-18 21:34 ` Jonathan McDowell
  2020-03-23 16:38 ` Kalle Valo
  0 siblings, 2 replies; 8+ messages in thread
From: Luca Coelho @ 2020-03-18  6:12 UTC (permalink / raw)
  To: kvalo; +Cc: linux-wireless, lenb, noodles

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>
---
 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


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v5.6] iwlwifi: don't send GEO_TX_POWER_LIMIT if no wgds table
  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
  2020-03-23 16:38 ` Kalle Valo
  1 sibling, 1 reply; 8+ messages in thread
From: Jonathan McDowell @ 2020-03-18 21:34 UTC (permalink / raw)
  To: Luca Coelho; +Cc: kvalo, linux-wireless, lenb

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.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v5.6] iwlwifi: don't send GEO_TX_POWER_LIMIT if no wgds table
  2020-03-18 21:34 ` Jonathan McDowell
@ 2020-03-19 17:32   ` Len Brown
  2020-03-19 19:50     ` Len Brown
  0 siblings, 1 reply; 8+ messages in thread
From: Len Brown @ 2020-03-19 17:32 UTC (permalink / raw)
  To: Jonathan McDowell; +Cc: Luca Coelho, kvalo, linux-wireless

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v5.6] iwlwifi: don't send GEO_TX_POWER_LIMIT if no wgds table
  2020-03-19 17:32   ` Len Brown
@ 2020-03-19 19:50     ` Len Brown
  0 siblings, 0 replies; 8+ messages in thread
From: Len Brown @ 2020-03-19 19:50 UTC (permalink / raw)
  To: Jonathan McDowell; +Cc: Luca Coelho, kvalo, linux-wireless

BTW. also verified that this patch (with a small offset) applied to
5.5 and also fixes the problem there.

5.3 and 5.4 always worked fine not requiring any patching, as the
regression happened in 5.5-rc1.

thanks,
Len Brown, Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v5.6] iwlwifi: don't send GEO_TX_POWER_LIMIT if no wgds table
  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-23 16:38 ` Kalle Valo
  2020-03-28 11:02   ` Felipe Contreras
  1 sibling, 1 reply; 8+ messages in thread
From: Kalle Valo @ 2020-03-23 16:38 UTC (permalink / raw)
  To: Luca Coelho; +Cc: linux-wireless, lenb, noodles

Luca Coelho <luca@coelho.fi> 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>
> Tested-By: Jonathan McDowell <noodles@earth.li>
> Tested-by: Len Brown <len.brown@intel.com>

Patch applied to wireless-drivers.git, thanks.

0433ae556ec8 iwlwifi: don't send GEO_TX_POWER_LIMIT if no wgds table

-- 
https://patchwork.kernel.org/patch/11444587/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v5.6] iwlwifi: don't send GEO_TX_POWER_LIMIT if no wgds table
  2020-03-23 16:38 ` Kalle Valo
@ 2020-03-28 11:02   ` Felipe Contreras
  2020-03-28 13:10     ` Kalle Valo
  0 siblings, 1 reply; 8+ messages in thread
From: Felipe Contreras @ 2020-03-28 11:02 UTC (permalink / raw)
  To: Kalle Valo; +Cc: Luca Coelho, linux-wireless Mailing List, Len Brown, noodles

On Mon, Mar 23, 2020 at 10:42 AM Kalle Valo <kvalo@codeaurora.org> wrote:
>
> Luca Coelho <luca@coelho.fi> 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>
> > Tested-By: Jonathan McDowell <noodles@earth.li>
> > Tested-by: Len Brown <len.brown@intel.com>
>
> Patch applied to wireless-drivers.git, thanks.
>
> 0433ae556ec8 iwlwifi: don't send GEO_TX_POWER_LIMIT if no wgds table

This should be applied to the stable tree, shouldn't it? 5.5 stable
releases have this issue.

-- 
Felipe Contreras

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v5.6] iwlwifi: don't send GEO_TX_POWER_LIMIT if no wgds table
  2020-03-28 11:02   ` Felipe Contreras
@ 2020-03-28 13:10     ` Kalle Valo
  2020-03-31 20:29       ` Felipe Contreras
  0 siblings, 1 reply; 8+ messages in thread
From: Kalle Valo @ 2020-03-28 13:10 UTC (permalink / raw)
  To: Felipe Contreras
  Cc: Luca Coelho, linux-wireless Mailing List, Len Brown, noodles

Felipe Contreras <felipe.contreras@gmail.com> writes:

> On Mon, Mar 23, 2020 at 10:42 AM Kalle Valo <kvalo@codeaurora.org> wrote:
>>
>> Luca Coelho <luca@coelho.fi> 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>
>> > Tested-By: Jonathan McDowell <noodles@earth.li>
>> > Tested-by: Len Brown <len.brown@intel.com>
>>
>> Patch applied to wireless-drivers.git, thanks.
>>
>> 0433ae556ec8 iwlwifi: don't send GEO_TX_POWER_LIMIT if no wgds table
>
> This should be applied to the stable tree, shouldn't it? 5.5 stable
> releases have this issue.

It's now in Linus' tree so I think you can just send a mail to the
stable team and they will queue it.

-- 
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v5.6] iwlwifi: don't send GEO_TX_POWER_LIMIT if no wgds table
  2020-03-28 13:10     ` Kalle Valo
@ 2020-03-31 20:29       ` Felipe Contreras
  0 siblings, 0 replies; 8+ messages in thread
From: Felipe Contreras @ 2020-03-31 20:29 UTC (permalink / raw)
  To: Kalle Valo
  Cc: Luca Coelho, linux-wireless Mailing List, Len Brown, Jonathan McDowell

On Sat, Mar 28, 2020 at 7:10 AM Kalle Valo <kvalo@codeaurora.org> wrote:
>
> Felipe Contreras <felipe.contreras@gmail.com> writes:

> > This should be applied to the stable tree, shouldn't it? 5.5 stable
> > releases have this issue.
>
> It's now in Linus' tree so I think you can just send a mail to the
> stable team and they will queue it.

Ok. Done.

-- 
Felipe Contreras

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2020-03-31 20:29 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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
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

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