-
Notifications
You must be signed in to change notification settings - Fork 8.1k
soc: silabs: siwx91x: Transform silabs network coprocessor (NWP) SoC files into a driver. #97554
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.
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
soc: silabs: siwx91x: Transform silabs network coprocessor (NWP) SoC files into a driver. #97554
Conversation
6d023f1
to
6064d22
Compare
goto out; | ||
} | ||
if (IS_ENABLED(CONFIG_WISECONNECT_NETWORK_STACK)) { | ||
if (IS_ENABLED(CONFIG_SILABS_SIWX91X_NWP)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe this could be done in a further PR, but I believe the idea is to move this code in the nwp driver in the PM_ACTIONS
.
Will be moved back to the SoC folder but still as a driver. I was not aware that we can have driver outside of the driver folder. |
6064d22
to
6117c80
Compare
6117c80
to
f3d8971
Compare
|
||
#define HCI_DEVICE_INIT(inst) \ | ||
static struct hci_config hci_config_##inst = { \ | ||
.nwp_dev = DEVICE_DT_GET(DT_INST_PARENT(0)) \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For correctness, shouldn't you use inst
instead of 0
here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
absolutely, that's a miss after some copy/paste.
f3d8971
to
bc8eef9
Compare
V2:
|
SL_SI91X_EXT_FEAT_XTAL_CLK, | ||
} | ||
}; | ||
}}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Original indentation was better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
if (memcmp(country_code, region_maps[i].codes[j], | ||
WIFI_COUNTRY_CODE_LEN) == 0) { | ||
if (memcmp(country_code, region_maps[i].codes[j], WIFI_COUNTRY_CODE_LEN) == | ||
0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Original indentation was better
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
"VI", "VN", "VU", "00" | ||
/* Map "00" (world domain) to US region, | ||
* as using the world domain is not recommended | ||
*/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm... maybe we should keep this comment.:
"SG", "MX", "SV", "TC", "TH", "TT", "US", "UY", "VE", "VI", "VN", "VU",
/* Map World Domain ("00") to US region */
"00"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
static const struct siwx91x_nwp_config siwx91x_nwp_config_##inst = { \ | ||
.config_irq = silabs_siwx91x_nwp_irq_configure_##inst, \ | ||
.power_profile = DT_ENUM_IDX(DT_DRV_INST(inst), power_profile), \ | ||
.stack_size = DT_INST_PROP(inst, stack_size)}; \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
New line before the closing brace.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
static void silabs_siwx91x_nwp_irq_configure_##inst(const struct device *dev) \ | ||
{ \ | ||
ARG_UNUSED(dev); \ | ||
SIWX91X_NWP_IRQ_CONNECT(inst); \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any reason to not inline the SIWX91X_NWP_IRQ_CONNECT
macro?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no i've done it.
&expected_version.build_num); | ||
&expected_version.rom_id, &expected_version.major, &expected_version.minor, | ||
&expected_version.security_version, &expected_version.patch_num, | ||
&expected_version.customer_id, &expected_version.build_num); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Original indentation was not so bad (aren't you the author of the original code BTW?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done (yeah but clang-format didn't like it)
The goal of this patch is to switch from the nwp.c and nwp.h soc files to the new nwp driver. During this transition, we also renamed CONFIG_WISECONNECT_NETWORK_STACK to CONFIG_SILABS_SIWX91X_NWP which are a better naming to let the user knows that the network coprocessor files will be added to the compilation. The switch from a soc file to a driver device introduce a notion of nwp device that allows us to check for good initialization and ressources allocation. Before this patch, it is not possible to know if the nwp have booted successfully or not. We can now check if the device driver is ready or not before trying to do operation related to the nwp. Signed-off-by: Martin Hoff <[email protected]>
The bluetooth hci on silabs siwx91x depends on the nwp (network coprocessor). This patch allows to check for the correct initialization of the nwp before using bt hci on siwx91x. Signed-off-by: Martin Hoff <[email protected]>
bc8eef9
to
508b8f2
Compare
|
The goal of this PR is to transform the SoC files for the NWP (Network coprocessor) into a driver.
The node for the NWP already exists, but declaring it as a driver allows us to have a device and check device health during initialization and when other components (like WiFi or Bluetooth) need it.
I also renamed the Kconfig option from "CONFIG_WISECONNECT_NETWORK_STACK" to "CONFIG_SILABS_SIWX91X_NWP" to make it more explicit.
Currently, we don't have "default=y" for the Kconfig option because we don't want to boot up the NWP if no component will use it. Components that need it must select this option.
The first commit is quite large, but I couldn't find a way to split it while maintaining successful compilation between individual commits. Any suggestions are welcome.
I will do some cleanup of the siwx91x SoC folder on an upcoming PR.