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.

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

riscv: telink: add dual mode for b92 and buteo and CI. #355

Merged
merged 1 commit into from
Dec 30, 2024

Conversation

haiwentelink
Copy link
Collaborator

@haiwentelink haiwentelink commented Dec 17, 2024

dual mode switch commit , it will works on b92 and tl3218x

@haiwentelink haiwentelink marked this pull request as draft December 17, 2024 09:21
@haiwentelink haiwentelink removed the request for review from s07641069 December 17, 2024 09:21
config I2C_LED
bool "Control the i2 led slave use raw address"
default n if I2C
default n
Copy link
Collaborator

Choose a reason for hiding this comment

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

第二个default n看着多余?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

第二行应该是之前写的有问题,之前的想法是想用depend on I2C,我再梳理下。

default n

config DUAL_MODE_SWTICH
bool "Control the dual mode switch part"
Copy link
Collaborator

Choose a reason for hiding this comment

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

建议改成bool "Enable Dual-Mode Switching"

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

config的变量不能加Enable,否则CI会报错。

Copy link
Collaborator

Choose a reason for hiding this comment

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

我意思是把这个配置项的描述改成bool "Enable Dual-Mode Switching"

#if CONFIG_DUAL_MODE_SWTICH
if (sBoot_zb)
{
/* Switch from the touch link, need to restore previous values */
Copy link
Collaborator

@liucoldstar liucoldstar Dec 26, 2024

Choose a reason for hiding this comment

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

建议改成 /* Switching from TouchLink (Zigbee) to Matter. Restore previous states. */

sfixture_on = user_para.onoff;
sBrightness = user_para.lightness;
sAppTask.UpdateClusterState();
printk("Matter: Updated ZB On/Off state and brightness.\n");
Copy link
Collaborator

@liucoldstar liucoldstar Dec 26, 2024

Choose a reason for hiding this comment

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

可以改进为:
printk("Matter: Restored Zigbee On/Off and brightness states.\n");

{
return;
}
/*If initialization of Dnss takes longer than 60 seconds, the device will reboot and revert to Zigbee mode*/
Copy link
Collaborator

Choose a reason for hiding this comment

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

/*
 * If Dnss initialization takes longer than 60 seconds,
 * the device will reboot and revert to Zigbee mode.
 */
printk("Matter: DnssTimer expired. Rebooting...\n");

CHIP_ERROR AppTaskCommon::StartApp(void)
{

#if CONFIG_DUAL_MODE_SWTICH
/* Proc ota boot flag , and erase flag */
Copy link
Collaborator

Choose a reason for hiding this comment

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

/* Read OTA boot flag and erase flag from user parameters */

if (user_para.val == USER_ZB_SW_VAL)
{
sBoot_zb = 1;
/* Ensure lightness is at least 2 to avoid display error on HomePod Mini */
Copy link
Collaborator

Choose a reason for hiding this comment

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

/* Ensure brightness is at least 2 to avoid display issues on HomePod Mini */

{
user_para.lightness = 2;
}
/* Pass the value to the init part to avoid gaps in pwm_pool init */
Copy link
Collaborator

Choose a reason for hiding this comment

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

    /*
     * Pass the brightness value to the initialization code
     * to avoid gaps in PWM pool initialization.
     */

}
k_timer_init(&sDnssTimer, &AppTask::DnssTimerTimeoutCallback, nullptr);
k_timer_start(&sDnssTimer, K_MSEC(kDnssTimeout), K_NO_WAIT);
printk("Matter: start timer to protect Dnss initialized %x \r\n", *(int *) (&user_para));
Copy link
Collaborator

Choose a reason for hiding this comment

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

printk("Matter: Started DNS protection timer. user_para=%x\n", *(int *)(&user_para));

#if CONFIG_DUAL_MODE_SWTICH
/* Proc ota boot flag , and erase flag */
flash_read(flash_para_dev, USER_PARTITION_OFFSET, &user_para, sizeof(user_para));
/* Boot from Zigbee , need to clean the user parameters sector first and set a flag */
Copy link
Collaborator

Choose a reason for hiding this comment

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

/*
 * If the device boots from Zigbee, set a flag and adjust user parameters.
 * Then start a timer to ensure Dnss initialization completes within
 * a given timeframe.
 */

@@ -588,6 +650,10 @@ void AppTaskCommon::FactoryResetHandler(AppEvent * aEvent)
k_timer_stop(&sFactoryResetTimer);
sFactoryResetCntr = 0;

#if CONFIG_DUAL_MODE_SWTICH
printk("Factory reset triggered by button, resetting to Zigbee mode");
Copy link
Collaborator

Choose a reason for hiding this comment

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

printk("Factory reset triggered by button; reverting to Zigbee mode\n");

uint8_t val = USER_MATTER_PAIR_VAL;
flash_erase(flash_para_dev, USER_PARTITION_OFFSET, USER_PARTITION_SIZE);
flash_write(flash_para_dev, USER_PARTITION_OFFSET, &val, 1);
printk("Commissioning complete, set Matter commissionined flag");
Copy link
Collaborator

Choose a reason for hiding this comment

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

printk("Commissioning complete; Matter commissioned flag set.\n");

/* Reset to Zigbee mode if commissioning fails */
if (sBoot_zb)
{
printk("FailSafeTimer expired, Matter commissioning failed, rebooting to Zigbee mode.\r\n");
Copy link
Collaborator

Choose a reason for hiding this comment

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

printk("FailSafeTimer expired; Matter commissioning failed. Rebooting to Zigbee mode...\n");

}
else
{
printk("FailSafeTimer expired, Matter commissioning failed.\r\n");
Copy link
Collaborator

Choose a reason for hiding this comment

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

printk("FailSafeTimer expired; Matter commissioning failed.\n");

Copy link
Collaborator

@damien0x0023 damien0x0023 left a comment

Choose a reason for hiding this comment

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

@haiwentelink
I pulled all your working branches in matter/zephyr/hal_telink/mcuboot and built dual-mode firmware on tlsr9528a/tl3218a. Both of them work with standalone ZigBee firmware.
However, I have some concerns about wording and logging. Please check if we can refine them.


config I2C_LED
bool "Control the i2 led slave use raw address"
default n if I2C
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do we need two default NOs here? Can we set it to "default y if I2C".

@@ -34,7 +34,7 @@ choice LOG_MODE
endchoice

choice MATTER_LOG_LEVEL_CHOICE
default MATTER_LOG_LEVEL_WRN if SOC_RISCV_TELINK_TL321X
default MATTER_LOG_LEVEL_DBG if SOC_RISCV_TELINK_TL321X
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could we remove this line as they all use DBG level?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@damien0x0023 For tl321x which will not have enough flash size ,open just for debug , or will close.but for 4m , which we will have enough space , reserver option for different soc .

sfixture_on = user_para.onoff;
sBrightness = user_para.lightness;
sAppTask.UpdateClusterState();
printk("Matter: Updated ZB On/Off state and brightness.\n");
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could we use Matter's logging API (ChipLogProgress) or Zephyr's logging API (LOG_INF or LOG_DBG) to replace printk()?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

should not bound to the log info , we want this print take effect in all case , if we open the matter print , the log will lost .

@@ -159,6 +179,11 @@ class AppFabricTableDelegate : public FabricTable::Delegate
{
ChipLogError(DeviceLayer, "Storage clearance failed: %d", status);
}
#if CONFIG_DUAL_MODE_SWTICH
printk("Erasing user parameters and resetting to Zigbee mode");
Copy link
Collaborator

Choose a reason for hiding this comment

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

I have the same concern about logging API here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

should not bound to the log info , we want this print take effect in all case , if we open the matter print , the log will lost .

if (sBoot_zb)
{
k_timer_stop(&sDnssTimer);
printk("Dnss Timer stopped, Matter commissioning kDnssdInitialized.\r\n");
Copy link
Collaborator

Choose a reason for hiding this comment

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

printk("DnssTimer stopped; DNS-SD has been initialized.\n");

#if CONFIG_DUAL_MODE_SWTICH
const struct device * flash_para_dev = USER_PARTITION_DEVICE;
constexpr int kDnssTimeout = 60000;
k_timer sDnssTimer;
Copy link
Collaborator

Choose a reason for hiding this comment

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

建议加static,因为sDnsTimer只在这个文件用到了。

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ok

@@ -9,31 +9,55 @@

boot_partition: partition@0 {
label = "mcuboot";
reg = <0x00000000 0x13000>;
reg = <0x00000000 0x12000>;// bootloader should use the size optimize to low down to 72k,reserve 4k for user.
Copy link
Collaborator

Choose a reason for hiding this comment

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

boot_partition: partition@0 {
label = "mcuboot";
// The bootloader is optimized to fit within 72 KB (0x12000).
// 4 KB (0x1000) are reserved for user data or future expansion.
reg = <0x00000000 0x12000>;
};

label = "image-0";
reg = <0x13000 0xef000>;
reg = <0x12000 0x122000>;// 0x12000 is matter, and reserve 912k for matter , 0xF6000 is zb reserve 248k for zigbee
Copy link
Collaborator

Choose a reason for hiding this comment

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

slot0_partition: partition@12000 {
label = "image-0";
// Starting at 0x12000, this partition reserves 912 KB for Matter,
// and an additional 248 KB for Zigbee, totaling 0x122000 in size.
reg = <0x12000 0x122000>;
};

factory_rfu_partition: partition@102800 {
label = "factory-data-rfu";
reg = <0x102800 0x800>;
slot1_partition: partition@134000 {
Copy link
Collaborator

Choose a reason for hiding this comment

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

slot1_partition: partition@134000 {
label = "image-1";
// Uses LZMA compression to reduce binary size.
// The combined space (920 KB + 248 KB) is reduced to ~60% (~712 KB).
// Zigbee NVS data will also reside here.
reg = <0x134000 0xb2000>;
};

};
secure_partition: partition@1f8000 {
label = "secure";
reg = <0x1f8000 0x1000>; //secure info ,reserved for secure boot .if not use , can beused by other way .
Copy link
Collaborator

Choose a reason for hiding this comment

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

secure_partition: partition@1f8000 {
label = "secure";
// Secure area reserved for secure boot.
// If not in use, it may be repurposed.
reg = <0x1f8000 0x1000>;
};

};
factory_partition: partition@1f9000 {
label = "factory-data";
reg = <0x1f9000 0x1000>; // factory data info and dac info
Copy link
Collaborator

Choose a reason for hiding this comment

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

// Factory data

};
dac_keypair_partition: partition@1f9800 {
label = "dac-keypair";
reg = <0x1f9800 0x800>; //store dac and key pair.
Copy link
Collaborator

Choose a reason for hiding this comment

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

dac_keypair_partition: partition@1f9800 {
label = "dac-keypair";
// Stores DAC certificate and key pair (2 KB).
reg = <0x1f9800 0x800>;
};

};
user_rfu_partition: partition@1fa000 {
label = "user_rfu";
reg = <0x1fa000 0x4000>; //user_rfu: reserve 16k for user extend.
Copy link
Collaborator

Choose a reason for hiding this comment

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

// Reserves 16 KB for user-specific extensions.

vendor_partition: partition@1fe000 {
label = "vendor-data";
reg = <0x1fe000 0x2000>;
reg = <0x1fe000 0x2000>;// mac and adc info.
Copy link
Collaborator

Choose a reason for hiding this comment

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

// Holds MAC, ADC, or other vendor-specific data (8 KB).

@haiwentelink haiwentelink force-pushed the develop_dual_cp_ci branch 2 times, most recently from 37b84bb to cf6a8ba Compare December 27, 2024 05:15
- clean the boot flag in init part .
- add the kDnsstimer to protect from init to dnss .
- add special proc for ikea.
- add the user-para proc in matter.
- add 2m flash for buteo ,and 4m for b92.
- update CI for b92 and tl3218x .
- fix the switch logic part .

Signed-off-by: Haiwen Xia <[email protected]>
@haiwentelink haiwentelink marked this pull request as ready for review December 30, 2024 03:12
@haiwentelink haiwentelink merged commit e7b9e81 into develop_dual Dec 30, 2024
64 checks passed
@haiwentelink haiwentelink deleted the develop_dual_cp_ci branch December 30, 2024 05:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants