All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.