Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Kalle Valo <kvalo@kernel.org>
To:     Jakub Kicinski <kuba@kernel.org>
Cc:     Ping-Ke Shih <pkshih@realtek.com>,
        "netdev\@vger.kernel.org" <netdev@vger.kernel.org>,
        "linux-wireless\@vger.kernel.org" <linux-wireless@vger.kernel.org>
Subject: Re: pull-request: wireless-next-2023-04-21
References: <20230421104726.800BCC433D2@smtp.kernel.org>
        <20230421075404.63c04bca@kernel.org>
        <e31dae6daa6640859d12bf4c4fc41599@realtek.com>
        <87leigr06u.fsf@kernel.org> <20230425071848.6156c0a0@kernel.org>
Date:   Fri, 28 Apr 2023 13:43:16 +0300
In-Reply-To: <20230425071848.6156c0a0@kernel.org> (Jakub Kicinski's message of
        "Tue, 25 Apr 2023 07:18:48 -0700")
Message-ID: <87cz3os2wr.fsf@kernel.org>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain
Precedence: bulk

Jakub Kicinski <kuba@kernel.org> writes:

> On Tue, 25 Apr 2023 08:38:17 +0300 Kalle Valo wrote:
>> IIRC we discussed this back in initial rtw88 or rtw89 driver review (not
>> sure which one). At the time I pushed for the current solution to have
>> the initvals in static variables just to avoid any backwards
>> compatibility issues. I agree that the initvals in .c files are ugly but
>> is it worth all the extra effort and complexity to move them outside the
>> kernel? I'm starting to lean towards it's not worth all the extra work.
>
> I don't think it's that much extra work, the driver requires FW
> according to modinfo, anyway, so /lib/firmware is already required.
> And on smaller systems with few hundred MB of RAM it'd be nice to not
> hold all the stuff in kernel memory, I'd think.

Later in this thread Ping explained pretty well the challenges here,
that sums exactly what I'm worried about.

> We have a rule against putting FW as a static table in the driver
> source, right? Or did we abandon that? Isn't this fundamentally similar?

My understanding is that these are just initialisation values for
hardware, not executable code. (Ping, please correct me if I
misunderstood.) So that's why I thought these are ok to have in kernel.
So I took practicality over elegance here.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

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