linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jonathan McDowell <noodles@earth.li>
To: Luca Coelho <luca@coelho.fi>
Cc: kvalo@codeaurora.org, linux-wireless@vger.kernel.org, lenb@kernel.org
Subject: Re: [PATCH v5.6] iwlwifi: don't send GEO_TX_POWER_LIMIT if no wgds table
Date: Wed, 18 Mar 2020 21:34:40 +0000	[thread overview]
Message-ID: <20200318213440.GO311@earth.li> (raw)
In-Reply-To: <iwlwifi.20200318081237.46db40617cc6.Id5cf852ec8c5dbf20ba86bad7b165a0c828f8b2e@changeid>

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.

  reply	other threads:[~2020-03-18 21:34 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 [this message]
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

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=20200318213440.GO311@earth.li \
    --to=noodles@earth.li \
    --cc=kvalo@codeaurora.org \
    --cc=lenb@kernel.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=luca@coelho.fi \
    /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).