drivers: wifi: siwx917: Add support for advanced scan options #86

ArunmaniAlagarsamy2710 · 2025-01-29T12:35:29Z

This commit adds support for the following scan functionalities

Single and multi-channel scan
Direct SSID scan
Dwell time configuration for both active and passive scans
Maximum BSS count configuration for the 2.4 GHz band

Since the device operates solely on the 2.4 GHz frequency band, this enhancement enables it to send probe requests and listen for beacon frames on specified channels, improving scan efficiency and customization.

ArunmaniAlagarsamy2710 · 2025-01-30T06:38:27Z

@jhedberg @jerome-pouiller Please help to review.

jerome-pouiller · 2025-01-30T10:58:09Z

drivers/wifi/siwx917/siwx917_wifi.c

+		LOG_INF("No APs detected");
+		return;
+	}
+


Please do not declare variables after statements.

BTW, I don't think we need to log anything if scan_count == 0.

BTW, calling this function with index >= scan_count is a bug.

jerome-pouiller · 2025-01-30T11:03:13Z

drivers/wifi/siwx917/siwx917_wifi.c

+		.band = (result->scan_info[item].rf_channel > 0 &&
+				result->scan_info[item].rf_channel <= 14) ?
+							WIFI_FREQ_BAND_2_4_GHZ
+							: WIFI_FREQ_BAND_UNKNOWN,


Don't abuse of the trinary operator. I suggest:

.band = WIFI_FREQ_BAND_2_4_GHZ, [...] if (result->scan_info[item].rf_channel <= 0 || result->scan_info[item].rf_channel > 14)) { LOG_WARN("Unexpected scan result"); tmp.band = WIFI_FREQ_BAND_UNKNOWN; }

jerome-pouiller · 2025-01-30T11:04:14Z

drivers/wifi/siwx917/siwx917_wifi.c

+
+	if (sidev->max_bss_cnt) {
+		result->scan_count = (sidev->max_bss_cnt > result->scan_count) ?
+			result->scan_count : sidev->max_bss_cnt;


MIN(result->scan_count, sidev->max_bss_cnt);

BTW, I suggest to avoid change the input argument. Rather use a local variable to store this value.

jerome-pouiller · 2025-01-30T20:11:13Z

drivers/wifi/siwx917/siwx917_wifi.c

@@ -185,6 +194,17 @@ static unsigned int siwx917_on_scan(sl_wifi_event_t event, sl_wifi_scan_result_t
 	if (!sidev->scan_res_cb) {
 		return -EFAULT;
 	}
+
+	if (SL_WIFI_CHECK_IF_EVENT_FAILED(event)) {


SL_WIFI_CHECK_IF_EVENT_FAILED() looks useless. event & SL_WIFI_EVENT_FAIL_INDICATION is simpler and clearer for anyone.

jerome-pouiller · 2025-01-30T20:15:35Z

drivers/wifi/siwx917/siwx917_wifi.c

+
+	if (SL_WIFI_CHECK_IF_EVENT_FAILED(event)) {
+		memset(result, 0, sizeof(*result));
+		siwx917_report_scan_res(sidev, result, 0);


What is purpose of this call?

jerome-pouiller · 2025-01-30T20:37:37Z

drivers/wifi/siwx917/siwx917_wifi.c

+#ifdef CONFIG_WIFI_MGMT_SCAN_CHAN_MAX_MANUAL
+	for (int i = 0; i < WIFI_MGMT_SCAN_CHAN_MAX_MANUAL; i++) {
+		sl_scan_config.channel_bitmap_2g4 |= (1 <<
+						(z_scan_config->band_chan[i].channel - 1));


Use BIT()

jerome-pouiller · 2025-01-30T20:38:39Z

drivers/wifi/siwx917/siwx917_wifi.c

+		strncpy(ssid.value, z_scan_config->ssids[0], WIFI_SSID_MAX_LEN);
+		ssid.length = strlen(z_scan_config->ssids[0]);
+	}
+#endif


Rather use IF_ENABLED() when possible.

jerome-pouiller · 2025-01-30T20:47:20Z

drivers/wifi/siwx917/siwx917_wifi.h

@@ -17,6 +17,7 @@
 struct siwx917_dev {
 	struct net_if *iface;
 	sl_mac_address_t macaddr;
+	uint16_t max_bss_cnt;


The name of this variable does not allow to understand what it does. Rename to scan_max_bss_cnt and place it beside scan_res_cb.

jerome-pouiller · 2025-01-30T20:48:19Z

drivers/wifi/siwx917/siwx917_wifi.c

 	if (ret != SL_STATUS_IN_PROGRESS) {
+		LOG_ERR("Failed to start scan: %d", ret);


This error will appear in the user interface even if the case is properly handled by the upper layers. I suggest tot drop it.

jerome-pouiller · 2025-01-30T20:49:24Z

drivers/wifi/siwx917/siwx917_wifi.c

 	sidev->scan_res_cb = cb;
-	ret = sl_wifi_start_scan(SL_WIFI_CLIENT_INTERFACE, NULL, &sl_scan_config);
+	ret = sl_wifi_start_scan(SL_WIFI_CLIENT_2_4GHZ_INTERFACE,
+			(ssid.length > 0) ? &ssid : NULL, &sl_scan_config);


Align with opening parenthese.

jerome-pouiller · 2025-01-30T20:52:33Z

It would have been easier to review if the features would been introduced in separated commits.

jerome-pouiller · 2025-01-31T16:17:09Z

drivers/wifi/siwx917/siwx917_wifi.c

@@ -211,9 +212,16 @@ static int siwx917_scan(const struct device *dev, struct wifi_scan_params *z_sca

 	sl_scan_config.channel_bitmap_2g4 = 0xFFFF;
 	memset(sl_scan_config.channel_bitmap_5g, 0xFF, sizeof(sl_scan_config.channel_bitmap_5g));
+	IF_ENABLED(CONFIG_WIFI_MGMT_SCAN_SSID_FILT_MAX, (


My bad, I meant IS_ENABLED():

if (IS_ENABLED(CONFIG_WIFI_MGMT_SCAN_SSID_FILT_MAX)) { [....]

jerome-pouiller · 2025-01-31T16:23:03Z

drivers/wifi/siwx917/siwx917_wifi.c

+		} else {
+			ret = sl_si91x_configure_timeout(SL_SI91X_CHANNEL_ACTIVE_SCAN_TIMEOUT,
+							 SL_WIFI_DEFAULT_ACTIVE_CHANNEL_SCAN_TIME);
+		}


hmm... It seems the condition is useless. You can unconditionally run:

ret = sl_si91x_configure_timeout(SL_SI91X_CHANNEL_ACTIVE_SCAN_TIMEOUT, z_scan_config>dwell_time_active);

I have realized you probably wanted to write:

if (!z_scan_config>dwell_time_active) { ret = sl_si91x_configure_timeout(SL_SI91X_CHANNEL_ACTIVE_SCAN_TIMEOUT, SL_WIFI_DEFAULT_ACTIVE_CHANNEL_SCAN_TIME); } else { ret = sl_si91x_configure_timeout(SL_SI91X_CHANNEL_ACTIVE_SCAN_TIMEOUT, z_scan_config>dwell_time_active); }

jerome-pouiller · 2025-01-31T16:23:13Z

drivers/wifi/siwx917/siwx917_wifi.c

+		} else {
+			ret = sl_si91x_configure_timeout(SL_SI91X_CHANNEL_PASSIVE_SCAN_TIMEOUT,
+							 SL_WIFI_DEFAULT_PASSIVE_CHANNEL_SCAN_TIME);
+		}


This commit introduces support for direct SSID scans The feature allows the device to send the probe requests and listen the beacon frame on the specified SSID. Signed-off-by: Arunmani Alagarsamy <arunmani.a@silabs.com>

This commit introduces support for limiting the number of BSS scan results returned by the siwx917 Wi-Fi driver. If scan_max_bss_cnt is specified, the driver will return up to the specified count of BSS results. If not specified, the driver defaults to returning all available results. Signed-off-by: Arunmani Alagarsamy <arunmani.a@silabs.com>

This commit introduces support for single and multi-channel Wi-Fi scans. The device can now scan one specific channel or multiple specified channels. Signed-off-by: Arunmani Alagarsamy <arunmani.a@silabs.com>

ArunmaniAlagarsamy2710 · 2025-02-03T11:56:40Z

@jerome-pouiller Changes have been made, please revisit

This commit adds support for configuring dwell times for both active and passive Wi-Fi scans. The dwell time specifies the duration the device spends on each channel during a scan. Signed-off-by: Arunmani Alagarsamy <arunmani.a@silabs.com>

ArunmaniAlagarsamy2710 · 2025-02-05T07:43:36Z

@jerome-pouiller @jhedberg Gentle ping!

ArunmaniAlagarsamy2710 requested a review from a team as a code owner January 29, 2025 12:35

jerome-pouiller requested changes Jan 30, 2025

View reviewed changes

ArunmaniAlagarsamy2710 force-pushed the dev/scan_param branch 2 times, most recently from ee819b1 to 0b2f3b3 Compare January 31, 2025 14:47

jerome-pouiller reviewed Jan 31, 2025

View reviewed changes

ArunmaniAlagarsamy2710 added 3 commits February 3, 2025 12:37

driver: wifi: siwx917: Add direct ssid scan support

bbab7fc

This commit introduces support for direct SSID scans The feature allows the device to send the probe requests and listen the beacon frame on the specified SSID. Signed-off-by: Arunmani Alagarsamy <arunmani.a@silabs.com>

driver: wifi: siwx917: Add single and multi-channel scan support

9fd6a20

This commit introduces support for single and multi-channel Wi-Fi scans. The device can now scan one specific channel or multiple specified channels. Signed-off-by: Arunmani Alagarsamy <arunmani.a@silabs.com>

ArunmaniAlagarsamy2710 force-pushed the dev/scan_param branch from 0b2f3b3 to 90b614b Compare February 3, 2025 07:59

ArunmaniAlagarsamy2710 force-pushed the dev/scan_param branch from 90b614b to fcf438d Compare February 4, 2025 14:44

jhedberg approved these changes Feb 5, 2025

View reviewed changes

jerome-pouiller approved these changes Feb 5, 2025

View reviewed changes

jerome-pouiller merged commit d61c3f3 into SiliconLabsSoftware:main Feb 5, 2025
3 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

drivers: wifi: siwx917: Add support for advanced scan options #86

drivers: wifi: siwx917: Add support for advanced scan options #86

ArunmaniAlagarsamy2710 commented Jan 29, 2025

ArunmaniAlagarsamy2710 commented Jan 30, 2025

jerome-pouiller Jan 30, 2025

jerome-pouiller Jan 30, 2025

jerome-pouiller Jan 30, 2025 •

edited

Loading

jerome-pouiller Jan 30, 2025

jerome-pouiller Jan 30, 2025

jerome-pouiller Jan 30, 2025

jerome-pouiller Jan 30, 2025

jerome-pouiller Jan 30, 2025

jerome-pouiller Jan 30, 2025

jerome-pouiller Jan 30, 2025

jerome-pouiller Jan 30, 2025

jerome-pouiller commented Jan 30, 2025

jerome-pouiller Jan 31, 2025 •

edited

Loading

jerome-pouiller Jan 31, 2025

jerome-pouiller Feb 3, 2025

jerome-pouiller Jan 31, 2025

ArunmaniAlagarsamy2710 commented Feb 3, 2025

ArunmaniAlagarsamy2710 commented Feb 5, 2025

		if (ret != SL_STATUS_IN_PROGRESS) {
		LOG_ERR("Failed to start scan: %d", ret);

drivers: wifi: siwx917: Add support for advanced scan options #86

drivers: wifi: siwx917: Add support for advanced scan options #86

Conversation

ArunmaniAlagarsamy2710 commented Jan 29, 2025

ArunmaniAlagarsamy2710 commented Jan 30, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jerome-pouiller Jan 30, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jerome-pouiller commented Jan 30, 2025

jerome-pouiller Jan 31, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ArunmaniAlagarsamy2710 commented Feb 3, 2025

ArunmaniAlagarsamy2710 commented Feb 5, 2025

jerome-pouiller Jan 30, 2025 •

edited

Loading

jerome-pouiller Jan 31, 2025 •

edited

Loading