Ping-Ke Shih <pkshih@xxxxxxxxxxx> writes: >> -----Original Message----- >> From: Jakub Kicinski <kuba@xxxxxxxxxx> >> Sent: Friday, April 21, 2023 10:54 PM >> To: Kalle Valo <kvalo@xxxxxxxxxx> >> Cc: netdev@xxxxxxxxxxxxxxx; linux-wireless@xxxxxxxxxxxxxxx >> Subject: Re: pull-request: wireless-next-2023-04-21 >> >> On Fri, 21 Apr 2023 10:47:26 +0000 (UTC) Kalle Valo wrote: >> > .../net/wireless/realtek/rtw89/rtw8851b_table.c | 14824 +++++++++++++++++++ >> > .../net/wireless/realtek/rtw89/rtw8851b_table.h | 21 + >> >> We should load these like FW, see the proposal outlined in >> https://lore.kernel.org/all/20221116222339.54052a83@xxxxxxxxxx/ >> for example. Would that not work? >> > > That would work, and I think struct fields addr and val should be __le32. > And, I have some draft ideas to handle some situations we will face: > > 1. upgrading to newer driver without built-in tables will break user space > if people don't download table file from linux-firmware.git. > Maybe, we can keep the built-in tables and support loading from files > for couple years at least. > > 2. c code can do changes along with these tables, so driver should do some > compatibility things for register version. > > 3. The file contains not only simple registers tables but also TX power tables > and power tracking tables. These tables are multiple dimensions, and > dimensions can be changed due to more channels are supported, for example. > To be backward compatible, we need to add conversion function from > v1, v2 ... to current. > > I will think further to make this change smooth. 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. For me most important is that backwards compatibility is not broken, that would be bad for the users. So whatever we decide let's keep that in mind. -- https://patchwork.kernel.org/project/linux-wireless/list/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
