Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Siwx917 Wifi improvements #44

Merged
merged 3 commits into from
Sep 25, 2024
Merged

Siwx917 Wifi improvements #44

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5e45625
drivers: siwx917_wifi: Fix support for IPv6
jerome-pouiller Sep 19, 2024
ea195ea
drivers: siwx917_wifi: Fix scheduler issue
jerome-pouiller Sep 20, 2024
5757dd5
workflows: Fix execution of samples/net/wifi
jerome-pouiller Sep 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/build.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,4 +44,4 @@ jobs:
EXTRA_TWISTER_FLAGS="--short-build-path -O/tmp/twister-out"
fi
west twister --test sample.basic.helloworld -p siwx917_rb4338a -v --inline-logs $EXTRA_TWISTER_FLAGS
west twister --test sample.net.wifi -p siwx917_rb4338a -v --inline-logs $EXTRA_TWISTER_FLAGS
west twister --test sample.net.wifi -p siwx917_rb4338a -v --inline-logs -K $EXTRA_TWISTER_FLAGS
Comment on lines 46 to +47
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just as a note for the future, twister supports multiple --test arguments, so we should likely just merge these into a single west twister --test ... --test ... command. I've postponed doing it since we have several PRs open which would conflict with such a change.

Another improvement would be to take advantage of GitHub workflow "matrix", so that GitHub would run in parallel the same workflow with different input parameters for each instance. The parameter could e.g. the a specific platform to build against, i.e. this becomes more relevant as we grow the list of platforms covered by our CI.

https://docs.github.com/en/actions/writing-workflows/choosing-what-your-workflow-does/running-variations-of-jobs-in-a-workflow

2 changes: 1 addition & 1 deletion .github/workflows/upstream-build.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,4 +57,4 @@ jobs:
EXTRA_TWISTER_FLAGS="--short-build-path -O/tmp/twister-out"
fi
west twister -T ../zephyr/samples -s sample.basic.helloworld -p siwx917_rb4338a -v --inline-logs $EXTRA_TWISTER_FLAGS
west twister -T ../zephyr/samples -s sample.net.wifi -p siwx917_rb4338a -v --inline-logs $EXTRA_TWISTER_FLAGS
west twister -T ../zephyr/samples -s sample.net.wifi -p siwx917_rb4338a -v --inline-logs -K $EXTRA_TWISTER_FLAGS
1 change: 1 addition & 0 deletions boards/silabs/radio_boards/siwx917_rb4338a/siwx917_rb4338a.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,5 @@ toolchain:
supported:
- gpio
- i2c
- wifi
vendor: silabs
5 changes: 5 additions & 0 deletions drivers/wifi/siwx917/Kconfig.siwx917
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,11 @@ config WIFI_SIWX917

if WIFI_SIWX917

# WiseConnect create threads with realtime priority. Default (10kHz) clock tick
# prevent proper use of the system with these threads.
config SYS_CLOCK_TICKS_PER_SEC
default 1024
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think 1000 would make more sense (most new WiseConnect examples are also using 1000).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That seems strange, considering a 32768 Hz clock divided by 32 would give 1024. Are they not running on a low frequency timer (we are not yet, but were planning to)?


config NUM_PREEMPT_PRIORITIES
default 56

Expand Down
57 changes: 40 additions & 17 deletions drivers/wifi/siwx917/siwx917_wifi.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,33 @@
NET_BUF_POOL_FIXED_DEFINE(siwx917_tx_pool, 1, NET_ETH_MTU, 0, NULL);
NET_BUF_POOL_FIXED_DEFINE(siwx917_rx_pool, 10, NET_ETH_MTU, 0, NULL);

/* SiWx917 does not use the standard struct sockaddr (despite it uses the same
* name):
* - uses Little Endian for port number while Posix uses big endian
* - IPv6 addresses are bytes swapped
* Note: this function allows to have in == out.
*/
static void siwx917_sockaddr_swap_bytes(struct sockaddr *out,
const struct sockaddr *in, socklen_t in_len)
{

Check notice on line 52 in drivers/wifi/siwx917/siwx917_wifi.c

View workflow job for this annotation

GitHub Actions / build

You may want to run clang-format on this change 

drivers/wifi/siwx917/siwx917_wifi.c:52 
-static void siwx917_sockaddr_swap_bytes(struct sockaddr *out,
-					const struct sockaddr *in, socklen_t in_len)
+static void siwx917_sockaddr_swap_bytes(struct sockaddr *out, const struct sockaddr *in,
+					socklen_t in_len)
const struct sockaddr_in6 *in6 = (const struct sockaddr_in6 *)in;
struct sockaddr_in6 *out6 = (struct sockaddr_in6 *)out;
int i;

/* In Zephyr, size of sockaddr == size of sockaddr_storage
* (while in Posix sockaddr is smaller than sockaddr_storage).
*/
memcpy(out, in, in_len);
if (in->sa_family == AF_INET6) {
for (i = 0; i < ARRAY_SIZE(in6->sin6_addr.s6_addr32); i++) {
out6->sin6_addr.s6_addr32[i] = ntohl(in6->sin6_addr.s6_addr32[i]);
}
out6->sin6_port = ntohs(in6->sin6_port);
} else if (in->sa_family == AF_INET) {
out6->sin6_port = ntohs(in6->sin6_port);
}
}

static void siwx917_on_join_ipv4(struct siwx917_dev *sidev)
{
#ifdef CONFIG_NET_IPV4
Expand Down Expand Up @@ -71,12 +98,18 @@
.type = SL_IPV6,
};
struct in6_addr addr6 = { };
int ret;
int ret, i;

/* FIXME: support for static IP configuration */
ret = sl_si91x_configure_ip_address(&ip_config6, SL_SI91X_WIFI_CLIENT_VAP_ID);
if (!ret) {
memcpy(addr6.s6_addr, ip_config6.ip.v6.global_address.bytes, sizeof(addr6.s6_addr));
for (i = 0; i < ARRAY_SIZE(addr6.s6_addr32); i++) {
addr6.s6_addr32[i] = ntohl(ip_config6.ip.v6.global_address.value[i]);
}
/* SiWx917 already take care of DAD and sending ND is not
* supported anyway.
*/
net_if_flag_set(sidev->iface, NET_IF_IPV6_NO_ND);
/* FIXME: also report gateway and link local address */
net_if_ipv6_addr_add(sidev->iface, &addr6, NET_ADDR_AUTOCONF, 0);
} else {
Expand Down Expand Up @@ -287,17 +320,6 @@
return 0;
}

/* SiWx917 uses Little Endian for port number while Posix uses Big Endian */
static void siwx917_sock_swap_port_endian(struct sockaddr *out,
const struct sockaddr *in, socklen_t in_len)
{
/* In Zephyr, size of sockaddr == size of sockaddr_storage
* (while in Posix sockaddr is smaller than sockaddr_storage).
*/
memcpy(out, in, in_len);
((struct sockaddr_in *)out)->sin_port = ntohs(((struct sockaddr_in *)in)->sin_port);
}

static int siwx917_sock_recv_sync(struct net_context *context,
net_context_recv_cb_t cb, void *user_data)
{
Expand Down Expand Up @@ -403,7 +425,7 @@
!((struct sockaddr_in *)addr)->sin_port) {
return 0;
}
siwx917_sock_swap_port_endian(&addr_le, addr, addrlen);
siwx917_sockaddr_swap_bytes(&addr_le, addr, addrlen);
ret = sl_si91x_bind(sockfd, &addr_le, addrlen);
if (ret) {
return -errno;
Expand All @@ -426,8 +448,9 @@
struct sockaddr addr_le;
int ret;

printk("foo\n");
/* sl_si91x_connect() always return immediately, so we ignore timeout */
siwx917_sock_swap_port_endian(&addr_le, addr, addrlen);
siwx917_sockaddr_swap_bytes(&addr_le, addr, addrlen);
ret = sl_si91x_connect(sockfd, &addr_le, addrlen);
if (ret) {
ret = -errno;
Expand Down Expand Up @@ -486,7 +509,7 @@
}
newcontext->flags |= NET_CONTEXT_REMOTE_ADDR_SET;
newcontext->offload_context = (void *)ret;
siwx917_sock_swap_port_endian(&newcontext->remote, &addr_le, sizeof(addr_le));
siwx917_sockaddr_swap_bytes(&newcontext->remote, &addr_le, sizeof(addr_le));

SL_SI91X_FD_SET(ret, &sidev->fds_watch);
sl_si91x_select(NUMBER_OF_BSD_SOCKETS, &sidev->fds_watch, NULL, NULL, NULL,
Expand Down Expand Up @@ -528,7 +551,7 @@
net_buf_add(buf, net_pkt_get_len(pkt));

/* sl_si91x_sendto() always return immediately, so we ignore timeout */
siwx917_sock_swap_port_endian(&addr_le, addr, addrlen);
siwx917_sockaddr_swap_bytes(&addr_le, addr, addrlen);
ret = sl_si91x_sendto(sockfd, buf->data, net_pkt_get_len(pkt), 0, &addr_le, addrlen);
if (ret < 0) {
ret = -errno;
Expand Down
2 changes: 1 addition & 1 deletion west.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ manifest:
projects:
- name: hal_silabs
remote: silabs
revision: 3febda5e93f51324961a7e374321d10d6b2688f3
revision: c1fcd068ee40be8f13ebe33039cd13560e1af75f
path: modules/hal/silabs
- name: zephyr
remote: zephyrproject-rtos
Expand Down
Loading