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: <20201106100708.4609-1-cjhuang@codeaurora.org> <20201106100708.4609-4-cjhuang@codeaurora.org>
In-Reply-To: <20201106100708.4609-4-cjhuang@codeaurora.org>
From:   Abhishek Kumar <kuabhs@chromium.org>
Date:   Thu, 19 Nov 2020 12:02:16 -0800
Message-ID: <CACTWRwtRfgaHXYMAttNwptHxCA7k6r+G_sC0=oyHZxjYb-GeWA@mail.gmail.com>
Subject: Re: [PATCH 3/3] ath10k: allow dynamic SAR power limits via common API
To:     Carl Huang <cjhuang@codeaurora.org>
Cc:     ath10k <ath10k@lists.infradead.org>,
        linux-wireless <linux-wireless@vger.kernel.org>,
        Brian Norris <briannorris@chromium.org>,
        Douglas Anderson <dianders@chromium.org>, kuabhs@google.com
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hi,
This patch looks good to me, there is one small nit, If the maintainer
can take care of it then probably we don't need a new rev.

> @@ -329,6 +336,7 @@ static const struct ath10k_hw_params ath10k_hw_params_list[] = {
>                 .fw_diag_ce_download = true,
>                 .tx_stats_over_pktlog = false,
>                 .supports_peer_stats_info = true,
> +               .dynamic_sar_support = true,
>         },
Orthogonal to this patch, other people might probably differ, I guess
putting dynamic sar support as a firmware feature capability should be
more scalable and then we don't need a structure table for each
firmware. This might hold for other firmware features as well.

> +       ath10k_dbg(ar, ATH10K_DBG_MAC, "mac txpower 2g:%d 5g:%d\n",
> +                  tx_power_2g, tx_power_5g);
just a nit: space after colon. This might throw a warning in checkpatch.pl

-Abhishek