Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20210210115637.276676-1-luca@coelho.fi> <iwlwifi.20210210135352.889862e6d393.I8b894c1b2b3fe0ad2fb39bf438273ea47eb5afa4@changeid>
In-Reply-To: <iwlwifi.20210210135352.889862e6d393.I8b894c1b2b3fe0ad2fb39bf438273ea47eb5afa4@changeid>
From:   Kai-Heng Feng <kai.heng.feng@canonical.com>
Date:   Tue, 7 Sep 2021 19:30:21 +0800
Message-ID: <CAAd53p5udakjRZeFLyrzwtfoq5Vr937CNRghS6_YnCet9yhB1Q@mail.gmail.com>
Subject: Re: [PATCH v2 05/12] iwlwifi: mvm: store PPAG enabled/disabled flag properly
To:     Luca Coelho <luca@coelho.fi>
Cc:     Kalle Valo <kvalo@codeaurora.org>,
        linux-wireless <linux-wireless@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hi Luca,

On Wed, Feb 10, 2021 at 8:00 PM Luca Coelho <luca@coelho.fi> wrote:
>
> From: Luca Coelho <luciano.coelho@intel.com>
>
> When reading the PPAG table from ACPI, we should store everything in
> our fwrt structure, so it can be accessed later.  But we had a local
> ppag_table variable in the function and were erroneously storing the
> enabled/disabled flag in it instead of storing it in the fwrt.  Fix
> this by removing the local variable and storing everything directly in
> fwrt.

This patch enables PPAG, but it breaks one of HP laptop with Intel 9560.

dmesg with iwlwifi.debug=0x80 attached in the bug report:
https://bugzilla.kernel.org/show_bug.cgi?id=214343

>
> Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
> Fixes: f2134f66f40e ("iwlwifi: acpi: support ppag table command v2")
> Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
> ---
>  drivers/net/wireless/intel/iwlwifi/mvm/fw.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/fw.c b/drivers/net/wireless/intel/iwlwifi/mvm/fw.c
> index 3bfb80dd17cf..57471ab2f5ef 100644
> --- a/drivers/net/wireless/intel/iwlwifi/mvm/fw.c
> +++ b/drivers/net/wireless/intel/iwlwifi/mvm/fw.c
> @@ -895,7 +895,6 @@ static int iwl_mvm_sar_geo_init(struct iwl_mvm *mvm)
>  static int iwl_mvm_get_ppag_table(struct iwl_mvm *mvm)
>  {
>         union acpi_object *wifi_pkg, *data, *enabled;
> -       union iwl_ppag_table_cmd ppag_table;
>         int i, j, ret, tbl_rev, num_sub_bands;
>         int idx = 2;
>         s8 *gain;
> @@ -949,8 +948,8 @@ static int iwl_mvm_get_ppag_table(struct iwl_mvm *mvm)
>                 goto out_free;
>         }
>
> -       ppag_table.v1.enabled = cpu_to_le32(enabled->integer.value);
> -       if (!ppag_table.v1.enabled) {
> +       mvm->fwrt.ppag_table.v1.enabled = cpu_to_le32(enabled->integer.value);
> +       if (!mvm->fwrt.ppag_table.v1.enabled) {
>                 ret = 0;
>                 goto out_free;
>         }
> @@ -978,7 +977,7 @@ static int iwl_mvm_get_ppag_table(struct iwl_mvm *mvm)
>                             (j != 0 &&
>                              (gain[i * num_sub_bands + j] > ACPI_PPAG_MAX_HB ||
>                               gain[i * num_sub_bands + j] < ACPI_PPAG_MIN_HB))) {
> -                               ppag_table.v1.enabled = cpu_to_le32(0);
> +                               mvm->fwrt.ppag_table.v1.enabled = cpu_to_le32(0);
>                                 ret = -EINVAL;
>                                 goto out_free;
>                         }
> --
> 2.30.0
>