Received-SPF: pass (google.com: best guess record for domain of linux-wireless-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
DMARC-Filter: OpenDMARC Filter v1.3.2 smtp.codeaurora.org 511CA6034E
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: Re: [PATCH 3/6] rtw88: use a module parameter to control LPS enter
From:   Kalle Valo <kvalo@codeaurora.org>
In-Reply-To: <20191025093345.22643-4-yhchuang@realtek.com>
References: <20191025093345.22643-4-yhchuang@realtek.com>
To:     <yhchuang@realtek.com>
Cc:     <linux-wireless@vger.kernel.org>, <briannorris@chromium.org>,
        <g.schlmm@googlemail.com>
User-Agent: pwcli/0.0.0-git (https://github.com/kvalo/pwcli/) Python/2.7.12
Message-Id: <20191031075911.24E1D60930@smtp.codeaurora.org>
Date:   Thu, 31 Oct 2019 07:59:11 +0000 (UTC)
Sender: linux-wireless-owner@vger.kernel.org
Precedence: bulk

<yhchuang@realtek.com> wrote:

> From: Yan-Hsuan Chuang <yhchuang@realtek.com>
> 
> If the number of packets is less than the LPS threshold, driver
> can then enter LPS mode.
> And driver used to take RTW_LPS_THRESHOLD as the threshold. As
> the macro can not be changed after compiled, use a parameter
> instead.
> 
> The larger of the threshold, the more traffic required to leave
> power save mode, responsive time could be longer, but also the
> power consumption could be lower.
> 
> Signed-off-by: Yan-Hsuan Chuang <yhchuang@realtek.com>
> Reviewed-by: Chris Chiu <chiu@endlessm.com>

I don't think a module parameter should be used to control power save
level, instead there should be a generic interface for that. Also the commit
log does not give any explanation why this needs to be a module parameter.

Tony, there's a high barrier for adding new module parameters. It's a common
phrase for me to say "module parameters are not windows .ini files". And to make it
easier for everyone always submit controversial patches separately, do not hide
within a bigger patchset.

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

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