zephyr
zephyr copied to clipboard
zephyrproject-rtos
sensortile_box_pro board: add sample to upgrade BlueNRG SoC firmware
Add sample that implement the BlueNRG SoC firmware upgrade using the AN5471 protocol on uart2 channel.
It uses blobs that must be fetched with: west blobs fetch stm32
Use of blobs approved by TSC (https://github.com/zephyrproject-rtos/zephyr/issues/74326)
(The CI for the sample is skipped because it requires the presence of a f/w blob that is not fetched by CI itself, as mentioned in https://docs.zephyrproject.org/latest/contribute/bin_blobs.html#support-and-maintenance)
no way, we do not allow binary blobs in samples
I saw that infineon is doing something similar, but they are using the Bluetooth firmware blob in the driver itself (brcm_patchram_buf). I guess that using it in drivers would be ok, right?
no way, we do not allow binary blobs in samples
I saw that infineon is doing something similar, but they are using the Bluetooth firmware blob in the driver itself (brcm_patchram_buf). I guess that using it in drivers would be ok, right?
Errr, you what? Can you post a link?
I had a look and it doesn't have the blob in the driver, you need to get the blob using west blobs, and that was approved by the TSC. What is not acceptable is putting a blob into zepyhr itself, see https://docs.zephyrproject.org/latest/contribute/bin_blobs.html for process.
Though note samples are expected to build and pass in CI and there can be no blobs there, so I would say this sample is not acceptable for inclusion even if the blob is moved out as it can never be built in CI
no way, we do not allow binary blobs in samples
I saw that infineon is doing something similar, but they are using the Bluetooth firmware blob in the driver itself (brcm_patchram_buf). I guess that using it in drivers would be ok, right?
Errr, you what? Can you post a link?
I had a look and it doesn't have the blob in the driver, you need to get the blob using
west blobs, and that was approved by the TSC. What is not acceptable is putting a blob into zepyhr itself, see https://docs.zephyrproject.org/latest/contribute/bin_blobs.html for process
Not sure I got your point, my bad.
I'm actually using same method, i.e. getting the blob using west blobs fetch hal_st.
no way, we do not allow binary blobs in samples
I saw that infineon is doing something similar, but they are using the Bluetooth firmware blob in the driver itself (brcm_patchram_buf). I guess that using it in drivers would be ok, right?
Errr, you what? Can you post a link? I had a look and it doesn't have the blob in the driver, you need to get the blob using
west blobs, and that was approved by the TSC. What is not acceptable is putting a blob into zepyhr itself, see https://docs.zephyrproject.org/latest/contribute/bin_blobs.html for processNot sure I got your point, my bad. I'm actually using same method, i.e. getting the blob using
west blobs fetch hal_st.
Why do you have a .bin file in this commit?
Why do you have a .bin file in this commit?
Ops, I think it was left there by mistake. Thanks for catching it. :(
The following west manifest projects have been modified in this Pull Request:
| Name | Old Revision | New Revision | Diff |
|---|
Note: This message is automatically posted and updated by the Manifest GitHub Action.
no way, we do not allow binary blobs in samples
I saw that infineon is doing something similar, but they are using the Bluetooth firmware blob in the driver itself (brcm_patchram_buf). I guess that using it in drivers would be ok, right?
Errr, you what? Can you post a link?
I had a look and it doesn't have the blob in the driver, you need to get the blob using
west blobs, and that was approved by the TSC. What is not acceptable is putting a blob into zepyhr itself, see https://docs.zephyrproject.org/latest/contribute/bin_blobs.html for process.Though note samples are expected to build and pass in CI and there can be no blobs there, so I would say this sample is not acceptable for inclusion even if the blob is moved out as it can never be built in CI
@thedjnK the process took a while, but finally it was approved by TSC (see #74326). Can you please take a look again to this PR? Thanks.
@avisconti Moved target milestone to V4.0.0 as this doesn't fit the scope of what could be merged in v3.7.1 (fixes, ..).
@fabiobaltieri what do you think of the changes done? Any feedback related to blob handling?
@fabiobaltieri what do you think of the changes done? Any feedback related to blob handling?
Should be alright, went through TSC and approved.
no way, we do not allow binary blobs in samples
I saw that infineon is doing something similar, but they are using the Bluetooth firmware blob in the driver itself (brcm_patchram_buf). I guess that using it in drivers would be ok, right?
Errr, you what? Can you post a link? I had a look and it doesn't have the blob in the driver, you need to get the blob using
west blobs, and that was approved by the TSC. What is not acceptable is putting a blob into zepyhr itself, see https://docs.zephyrproject.org/latest/contribute/bin_blobs.html for process. Though note samples are expected to build and pass in CI and there can be no blobs there, so I would say this sample is not acceptable for inclusion even if the blob is moved out as it can never be built in CI@thedjnK the process took a while, but finally it was approved by TSC (see #74326). Can you please take a look again to this PR? Thanks.
The blob was approved, this PR was not, would like @nashif to comment on acceptability of this, this sample requires a blob to build and cannot be run on zephyr's CI at all, unlike other samples where the blob is optional for some boards, here it is a requirement
no way, we do not allow binary blobs in samples
I saw that infineon is doing something similar, but they are using the Bluetooth firmware blob in the driver itself (brcm_patchram_buf). I guess that using it in drivers would be ok, right?
Errr, you what? Can you post a link? I had a look and it doesn't have the blob in the driver, you need to get the blob using
west blobs, and that was approved by the TSC. What is not acceptable is putting a blob into zepyhr itself, see https://docs.zephyrproject.org/latest/contribute/bin_blobs.html for process. Though note samples are expected to build and pass in CI and there can be no blobs there, so I would say this sample is not acceptable for inclusion even if the blob is moved out as it can never be built in CI@thedjnK the process took a while, but finally it was approved by TSC (see #74326). Can you please take a look again to this PR? Thanks.
The blob was approved, this PR was not, would like @nashif to comment on acceptability of this, this sample requires a blob to build and cannot be run on zephyr's CI at all, unlike other samples where the blob is optional for some boards, here it is a requirement
I understand your point despite the fact that there are already other samples (very few to be honest) which skip the CI. But I understand your point. The problem is that in order to increase the usability of the sensortile_box_pro board, in this case the ble usage, a method to update the ble chip f/w is necessary. Maybe a sample is not the answer, maybe there are other ways.
So, instead of a "you cannot do that" approach, I would rather appreciate some suggestions on an alternative way to perform the upgrade. Please note that this issue is common to other ST boards as well, so the solution that will come up on this PR will be probably generally adopted.
Maybe @nashif could really give his opinion on that.
There was a conversation about this some time ago, was about a vendor specific radio protocol that required a blob to build. IIRC the sample got blocked on the basis on not being buildable in CI since CI does not fetch any blobs. I think it's a bit of a weak argument though because the same can be said for most bluetooth HCI drivers, most of the ones that live in https://github.com/zephyrproject-rtos/zephyr/tree/main/drivers/bluetooth/hci can't built without blobs and no CI target builds them.
There was a conversation about this some time ago, was about a vendor specific radio protocol that required a blob to build. IIRC the sample got blocked on the basis on not being buildable in CI since CI does not fetch any blobs. I think it's a bit of a weak argument though because the same can be said for most bluetooth HCI drivers, most of the ones that live in https://github.com/zephyrproject-rtos/zephyr/tree/main/drivers/bluetooth/hci can't built without blobs and no CI target builds them.
Yeah, exactly. For example at least the Infineon cyw208xx ble driver. This is the one I took a look at when I started to explore the way to do it. This driver should have the same CI issue I'm running into I guess...
There was a conversation about this some time ago, was about a vendor specific radio protocol that required a blob to build. IIRC the sample got blocked on the basis on not being buildable in CI since CI does not fetch any blobs. I think it's a bit of a weak argument though because the same can be said for most bluetooth HCI drivers, most of the ones that live in https://github.com/zephyrproject-rtos/zephyr/tree/main/drivers/bluetooth/hci can't built without blobs and no CI target builds them.
This is a sample, this is not a driver. Blobs are hardware enablement, not sample enablement. The corresponding sample is https://github.com/zephyrproject-rtos/zephyr/tree/main/samples/bluetooth/hci_ipc which can and is built without blobs in CI
So, instead of a "you cannot do that" approach, I would rather appreciate some suggestions on an alternative way to perform the upgrade. Please note that this issue is common to other ST boards as well, so the solution that will come up on this PR will be probably generally adopted.
From my point of view it should be done downstream, out of the zephyr repo
So, instead of a "you cannot do that" approach, I would rather appreciate some suggestions on an alternative way to perform the upgrade. Please note that this issue is common to other ST boards as well, so the solution that will come up on this PR will be probably generally adopted.
From my point of view it should be done downstream, out of the zephyr repo
ok, I'll try to investigate in that direction then.
From my point of view it should be done downstream, out of the zephyr repo
What if the loader code was implemented as a drivers/misc/ driver?
From my point of view it should be done downstream, out of the zephyr repo
What if the loader code was implemented as a
drivers/misc/driver?
But I guess it would have same blob/ci issue, isn't it?
But I guess it would have same blob/ci issue, isn't it?
Sure but no one seems to be concerned with that for ble hci drivers...
Or maybe you could have a sample Kconfig option enabled by the test run that replaces the blob with dummy data so that the sample can be compiled?
But I guess it would have same blob/ci issue, isn't it?
Sure but no one seems to be concerned with that for ble hci drivers...
Or maybe you could have a sample Kconfig option enabled by the test run that replaces the blob with dummy data so that the sample can be compiled?
Well Fabio, I'm actually exploring the possibility to prepare an external zephyr application to accomplish this task. It is basically the same code of this sample, but replacing the blobs part which is not needed. It will require a bit of extra explanations in order to be used, but seems to me the best solution.
Sure I just find it incoherent that we are so worried about blobs in sample code and not worried at all about blobs in subsystem code.
Or maybe you could have a sample Kconfig option enabled by the test run that replaces the blob with dummy data so that the sample can be compiled?
@fabiobaltieri I had an internal conversation with someone here in STM that was basically proposing similar approach. It should be very easy to do such modifications and it won't affect CI, but I'm not sure that it would be accepted.
Something like
const uint8_t ble_fw_img[] = {
#ifdef CONFIG_BLOB_ENABLED
#include <dtm.bin.inc>
#else
/* do nothing, just remove blob dependency */
#endif
};
And then user would build the sample with -DCONFIG_BLOB_ENABLED=y
Would that be approved? @thedjnK what's also ur opinion?
And then user would build the sample with -DCONFIG_BLOB_ENABLED=y
Well it should be the opposite IMO, the default behavior uses the blob (so the user can use it as is) but the configuration used in CI (i.e. what's in sample.yaml) enables the dummy blob array thing. But yeah apart from that I think it would be fine, certainly worth trying anyway.
Just to be sure: so this is really not something one may want to compile in as part of their application? Just a standalone sample and that's it?
And then user would build the sample with -DCONFIG_BLOB_ENABLED=y
Well it should be the opposite IMO, the default behavior uses the blob (so the user can use it as is) but the configuration used in CI (i.e. what's in sample.yaml) enables the dummy blob array thing. But yeah apart from that I think it would be fine, certainly worth trying anyway.
It makes sense, thx.
Just to be sure: so this is really not something one may want to compile in as part of their application? Just a standalone sample and that's it?
Yes, exactly.
My feeling is that it won't be accepted anyway, as the real issue seems not to be the CI, but the fact that sample concept should not be used to convey a functionality, but to make examples. Let's see.
This PR has not been accepted and is going to be closed down. A new PR (#78564) has been opened instead, with only the part concerning board ble chip documentation, including reference on how to upgrade the ble f/w (no more blobs required, https://github.com/STMicroelectronics/stsw-mkbox-bleco/blob/master/ble_fw_upg_app/README.rst)