2022-05-24 12:31:51

by Viktor Barna

[permalink] [raw]
Subject: [RFC v2 03/96] cl8k: add Kconfig

From: Viktor Barna <[email protected]>

(Part of the split. Please, take a look at the cover letter for more
details).

Signed-off-by: Viktor Barna <[email protected]>
---
drivers/net/wireless/celeno/cl8k/Kconfig | 41 ++++++++++++++++++++++++
1 file changed, 41 insertions(+)
create mode 100644 drivers/net/wireless/celeno/cl8k/Kconfig

diff --git a/drivers/net/wireless/celeno/cl8k/Kconfig b/drivers/net/wireless/celeno/cl8k/Kconfig
new file mode 100644
index 000000000000..7af6dfafa6af
--- /dev/null
+++ b/drivers/net/wireless/celeno/cl8k/Kconfig
@@ -0,0 +1,41 @@
+# SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause
+config CL8K
+ tristate "Celeno CL8K WLAN support"
+ depends on m
+ depends on MAC80211
+ help
+ This option enables support for Celeno CL8K WLAN.
+ Select M (recommended), if you have a wireless module.
+
+config CL8K_VERSION
+ string "Version"
+ depends on CL8K
+ default "8.1.x"
+ help
+ Sets module version, which may be important for FW compatibility
+ analysis and syncing upstream codebase with the internal codebase.
+
+config CL8K_EEPROM_STM24256
+ bool "EEPROM STM24256 support"
+ depends on CL8K
+ default n
+ help
+ Enables EEPROM STM24256 (specific for some of the platforms).
+
+config CL8K_DYN_BCAST_RATE
+ bool "Enable dynamic broadcast rate selection"
+ depends on CL8K
+ default n
+ help
+ Enables dynamic broadcast rate selection,
+ that allows to tune rate of broadcast frames taking into account
+ capabilities of all associated stations.
+
+config CL8K_DYN_MCAST_RATE
+ bool "Enable dynamic multicast rate selection"
+ depends on CL8K
+ default n
+ help
+ Enables dynamic multicast rate selection,
+ that allows to tune rate of multicast frames taking into account
+ capabilities of all associated stations.
--
2.36.1



2022-05-26 20:44:56

by Johannes Berg

[permalink] [raw]
Subject: Re: [RFC v2 03/96] cl8k: add Kconfig

On Tue, 2022-05-24 at 14:33 +0300, [email protected] wrote:
> +
> +config CL8K_VERSION
> + string "Version"
> + depends on CL8K
> + default "8.1.x"
> + help
> + Sets module version, which may be important for FW compatibility
> + analysis and syncing upstream codebase with the internal codebase.
>

This, along with the def.h stuff using it, and MODULE_VERSION(), is all
rather pointless, I think you should remove it.

johannes

2022-05-28 19:36:08

by Kalle Valo

[permalink] [raw]
Subject: Re: [RFC v2 03/96] cl8k: add Kconfig

Johannes Berg <[email protected]> writes:

> On Tue, 2022-05-24 at 14:33 +0300, [email protected] wrote:
>> +
>> +config CL8K_VERSION
>> + string "Version"
>> + depends on CL8K
>> + default "8.1.x"
>> + help
>> + Sets module version, which may be important for FW compatibility
>> + analysis and syncing upstream codebase with the internal codebase.
>>
>
> This, along with the def.h stuff using it, and MODULE_VERSION(), is all
> rather pointless, I think you should remove it.

s/should/need to/ :)

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

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

2022-07-11 23:08:44

by Viktor Barna

[permalink] [raw]
Subject: Re: [RFC v2 03/96] cl8k: add Kconfig

On Fri, 27 May 2022 09:09:19 +0300, [email protected] wrote:
>
>> On Thu, 26 May 2022 20:18:35 +0200, [email protected] wrote:
>>
>> This, along with the def.h stuff using it, and MODULE_VERSION(), is all
>> rather pointless, I think you should remove it.
>
> s/should/need to/ :)
>

That is an unexpected comment:) Can you please clarify what is wrong with that
place and why it is pointless (regarding the fact that it can be used to keep
track of versions of the code, that were upstreamed)? I can see multiple
drivers with that macro and related defines.

Best regards,
Viktor Barna

2022-07-13 07:48:31

by Kalle Valo

[permalink] [raw]
Subject: Re: [RFC v2 03/96] cl8k: add Kconfig

[email protected] writes:

> From: Viktor Barna <[email protected]>
>
> (Part of the split. Please, take a look at the cover letter for more
> details).
>
> Signed-off-by: Viktor Barna <[email protected]>

I haven't looked at rest of the driver yet, will do that in a later
revision. Just looking at build scripts in this round.

> --- /dev/null
> +++ b/drivers/net/wireless/celeno/cl8k/Kconfig
> @@ -0,0 +1,41 @@
> +# SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause
> +config CL8K
> + tristate "Celeno CL8K WLAN support"
> + depends on m
> + depends on MAC80211
> + help
> + This option enables support for Celeno CL8K WLAN.
> + Select M (recommended), if you have a wireless module.

Why depending on m? Drivers should be able to link to the kernel.

> +config CL8K_VERSION
> + string "Version"
> + depends on CL8K
> + default "8.1.x"
> + help
> + Sets module version, which may be important for FW compatibility
> + analysis and syncing upstream codebase with the internal codebase.

Johannes already commented on this.

> +config CL8K_EEPROM_STM24256
> + bool "EEPROM STM24256 support"
> + depends on CL8K
> + default n
> + help
> + Enables EEPROM STM24256 (specific for some of the platforms).

Kconfig should not be used for device configuration, ie. the same kernel
binary should work on all devices. Is there a better way to detect this
runtime? If not, I suggest to drop this in initial submission and submit
after the driver is accepted.

> +config CL8K_DYN_BCAST_RATE
> + bool "Enable dynamic broadcast rate selection"
> + depends on CL8K
> + default n
> + help
> + Enables dynamic broadcast rate selection,
> + that allows to tune rate of broadcast frames taking into account
> + capabilities of all associated stations.

I don't think features like this should be in Kconfig. Why not always enable this?

> +config CL8K_DYN_MCAST_RATE
> + bool "Enable dynamic multicast rate selection"
> + depends on CL8K
> + default n
> + help
> + Enables dynamic multicast rate selection,
> + that allows to tune rate of multicast frames taking into account
> + capabilities of all associated stations.

Same with this one.

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

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