-
Notifications
You must be signed in to change notification settings - Fork 620
Improve download speed, and adaption for new pm command about BK7239N #7099
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
base: BK7239N_Support
Are you sure you want to change the base?
Conversation
ziliguo
commented
Dec 26, 2025
- HW checksum for T-Put further optimization;
- Improve the download speed;
- adaption for "power start -l"
- Support of image encryption.
|
@ewoodev @hk-gwak @jylee9613 Could you review this? |
|
@ziliguo Could you let us know details of |
Hi @sunghan-chang ,
For the noise issue with high-speed you have mentioned, we use flow below to keep stability:
|
| required = ["kernel", "bootparam"] | ||
| optional = ["app1", "app2", "userfs"] | ||
| if self.config.get("CONFIG_BINARY_SIGNING", "n") != "y": | ||
| if self.config.get("CONFIG_TFM", "n") != "y": |
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.
@ziliguo : Please explain what is effect of this change ?
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.
Hi @amandeep-samsung ,
Before this modification was added, the bootloader was packaged within all-app.bin. After this modification, the bootloader can be separated from all-app.bin. This allows all-app.bin to be downloaded via the bootloader, and the bootloader includes optimizations for download speed.
@ziliguo Latency Timer (msec) value is not changed in Realtek case, and we use same settings as 16 msec for it.
|
apps/examples/power/power_main.c
Outdated
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.
Thank you for find this issue.
We'll fix this issue for master branch.
For now use power start -l -t.
And when master branch is updated, you can cherry-pick that code.
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.
Hi @seokhun-eom24, got it. Once you update to the master branch, we will sync the PM changes to this BK7239N_Support branch. We will revert this commit for now. Thank you.
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.
It's merged now. you can revert this commit.
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.
okay. It has already been reverted now.
| return status; | ||
| } | ||
|
|
||
| int verify_firmware_signature(uint32_t address) |
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.
verify_firmware_signature seems verify only app binary signature.
How can we verify kernel binary? Would you please add up_verify_usersignature too?
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.
Hello @jylee9613, during each startup, the bootloader verifies the signature of the kernel image, and only if the verification passes will it execute the kernel code; however, in the application code, the interface up_verify_kernelsignature has not been properly adapted yet. We will provide this interface in the future. Thank you.
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.
It seems that verify_firmware_signature is used only in internal. How about change this API to static?
We only use up_verify_usersignature/up_verify_kernelsignature.
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.
@jylee9613 Yes, of course. It has already been updated to static now.
Hi @amandeep-samsung , thank you for your valuable suggestion. The MP Tools team will consider this request and conduct a feasibility assessment. |
- Move WiFi APIs to RAM, thus to improve more transfer performance; - Allocate more flash partition size for kernel
- Before this modification was added, the bootloader was packaged within all-app.bin. - After this modification, the bootloader can be separated from all-app.bin. - This allows all-app.bin to be downloaded via the bootloader, and the bootloader includes optimizations for download speed.
…ature - Adapt for pm test cmd - Update the calculate_sha256 api about signature - Fix the verify_firmware_signature api static issue
- Added BLE CoC SDK interface - Supports dynamic parameter changes for multi-advertisement of BLE - Update tfm and wifi libs
- Add hardware checksum port interface for lwIP. - Introduce hardware checksum registration mechanism to allow vendors to register hardware checksum functions for improved performance. The system will use software checksum automatically when no hardware function is registered. - Usage: 1. Enable CONFIG_NET_LWIP_HW_CHKSUM_PORT 2. Implement vendor-specific hardware checksum function with the following format: uint16_t vendor_chksum_func(const void *dataptr, int len) 3. Register the vendor function during initialization: lwip_register_hw_chksum(vendor_chksum_func)
| case TRBLE_ADV_TYPE_IND: | ||
| hal_ble_adv_env.array[hal_adv_index].adv_param.adv_prop = ADV_PROP_CONNECTABLE_BIT | ADV_PROP_SCANNABLE_BIT; | ||
| break; | ||
|
|
||
| case TRBLE_ADV_TYPE_DIRECT: | ||
| LOGE("can't set direct adv because hal api doesn't input peer addr !!!"); | ||
| return TRBLE_FAIL; | ||
| hal_ble_adv_env.array[hal_adv_index].adv_param.adv_prop = ADV_PROP_CONNECTABLE_BIT | ADV_PROP_DIRECTED_BIT; | ||
| break; | ||
|
|
||
| case TRBLE_ADV_TYPE_SCAN_IND: | ||
| hal_ble_adv_env.array[hal_adv_index].adv_param.adv_prop = ADV_PROP_SCANNABLE_BIT; | ||
| break; | ||
|
|
||
| case TRBLE_ADV_TYPE_NONCONN_IND: | ||
| hal_ble_adv_env.array[hal_adv_index].adv_param.adv_prop = 0; | ||
| break; |
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.
It appears that a different value is being used compared to the type implemented in the bk_tr_ble_advertiser_create_multi_adv function. Since the create_multi_adv function is already in use on other boards, it would be better to unify the enum values with create_multi_adv.
TizenRT/os/board/bk7239n/src/bsp_adapter/src/ble/ble_tizenrt_advertiser.c
Lines 657 to 681 in 5d4b91c
| case BK_EXT_ADV_LEGACY_ADV_CONN_SCAN_UNDIRECTED: | |
| hal_ble_adv_env.array[tmp_hal_adv_index].adv_param.adv_prop = ADV_PROP_CONNECTABLE_BIT | ADV_PROP_SCANNABLE_BIT; | |
| break; | |
| case BK_EXT_ADV_LEGACY_ADV_CONN_LOW_DUTY_DIRECTED: | |
| LOGE("can't set direct adv because hal api doesn't input peer addr !!!"); | |
| ret = TRBLE_FAIL; | |
| goto end; | |
| hal_ble_adv_env.array[tmp_hal_adv_index].adv_param.adv_prop = ADV_PROP_CONNECTABLE_BIT | ADV_PROP_DIRECTED_BIT; | |
| break; | |
| case BK_EXT_ADV_LEGACY_ADV_CONN_HIGH_DUTY_DIRECTED: | |
| LOGE("can't set direct adv because hal api doesn't input peer addr !!!"); | |
| ret = TRBLE_FAIL; | |
| goto end; | |
| hal_ble_adv_env.array[tmp_hal_adv_index].adv_param.adv_prop = ADV_PROP_CONNECTABLE_BIT | ADV_PROP_HDC_BIT; | |
| break; | |
| case BK_EXT_ADV_LEGACY_ADV_SCAN_UNDIRECTED: | |
| hal_ble_adv_env.array[tmp_hal_adv_index].adv_param.adv_prop = ADV_PROP_SCANNABLE_BIT; | |
| break; | |
| case BK_EXT_ADV_LEGACY_ADV_NON_SCAN_NON_CONN_UNDIRECTED: | |
| hal_ble_adv_env.array[tmp_hal_adv_index].adv_param.adv_prop = 0; | |
| break; |
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.
The enumeration type of the two functions is different. In the function trble_netmgr_set_multi_adv_type, trble_adv_type_e adv_type is used, while in the function trble_netmgr_create_multi_adv, uint8_t adv_event_prop is used. The two functions have different enumeration types, so type conversion is required.
The enumeration parameters provided by the layered interfaces on TizenRT are different, which requires conversion at the underlying level.
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.
The trble_adv_type_e enum value can currently be considered deprecated. It was used in the old single advertiser API (ble_server_set_adv_type). For the multi advertiser API, the parameter should be changed to uint8_t adv_event_prop, similar to what was done in the create function.
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.
Since set_multi_adv_type still uses the enumeration type trble_adv_type_e.
We'd like to re-adapt it after Samsung reimplements the set_multi_adv_type function.
Is that alright?
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.
I understand that the set_multi_adv_type function is a newly created API that did not exist before. (It's possible I missed something since I started managing this project midway.)
In the commit 5d4b91c, it appears that the functions set_multi_adv_type and set_multi_adv_interval have been newly defined in bk_ble_tizenrt.h. The reason we requested the implementation of these functions was to improve usability. Currently, changing the advertising type and interval requires deleting the advertiser and recreating it, so we requested additional APIs to simplify this process.
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.
To summarize, the request was to change the parameter used in set_multi_adv_type from trble_adv_type_e to adv_event_prop.
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.
I understand your requirements and will include them in our subsequent PR.
| case EVENT_WIFI_REGDOMAIN_CHANGED: | ||
| /* fixme: handle regulatory domain changed event */ | ||
| break; | ||
|
|
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.
Can you explain what this is? Or please explain after the functions here confirmed.
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.
Hello @hk-gwak ,
PR7075 incorporates features related to the regulatory domain(country code). Upon system power-up, the default country code is 00. After connecting to a router, it updates to CN, triggering the EVENT_WIFI_REGDOMAIN_CHANGED event. TizenRT does not support this event, so it is filtered out in the adaptation layer. If not filtered, it would be incorrectly handled as LWNL_EVT_UNKNOWN.
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.
So can we set country code dynamically (on runtime)?
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.
Yes, Beken SDK supports adaptive country code configuration,customer can also set country code dynamically.
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.
EVENT_WIFI_REGDOMAIN_CHANGED is related to the way of setting country code dynamically? I didn't understand this sentence. "TizenRT does not support this event,"
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.
@hk-gwak
1.EVENT_WIFI_REGDOMAIN_CHANGED is a notification from bk SDK actively informing tizen that the Wi-Fi module has detected a change in the country code. However, currently, the TizenRT lwnl_cb_wifi enumeration does not have a corresponding value for country code changes, which is why I said, "TizenRT does not support this event."
2. Regarding the country code, my understanding is that TizenRT does not directly configure the country code to the Wi-Fi module(i mean use bk apis to set country code). Instead, it configures the channel plan through api trwifi_set_channel_plan. Is my understanding correct?
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.
Yes, correct. what I said is channel plan! Thank you for your explanation.
| config NET_LWIP_HW_CHKSUM_PORT | ||
| bool "Enable hardware checksum port layer" | ||
| default n | ||
| ---help--- | ||
| Enable hardware checksum port layer support. This allows vendors | ||
| to register hardware checksum functions that will be used instead | ||
| of software implementation when available. |
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.
Looks like this option is for optimization of throughput.
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.
@hk-gwak Yes, it is.