bluekitchen / btstack

Dual-mode Bluetooth stack, with small memory footprint.
http://bluekitchen-gmbh.com
Other
1.74k stars 614 forks source link

Workaround for Infineon CYW55573 initialization #643

Closed py-ir0nf1st closed 1 week ago

py-ir0nf1st commented 1 week ago

Infineon CYW55573 doesn't respond to READ_LOCAL_NAME or BAUD_CHANGE after HCI_RESET. This is to skip READ_LOCAL_NAME and BADU_CHANGE during HCI_STATE_INITIALIZING. After received response to READ_LOCAL_VERSION_INFORMATION, the state machine goes to HCI_INIT_CUSTOM_INIT to enable mini driver or patch ram downloading.

mringwal commented 1 week ago

Hi. Thanks for this PR. I kind of like the idea of skipping the read local name command when an newer AIROC Controller is detected. I don't want to list this and further Controllers in hci.c. Maybe this could be moved into main.c or the chipset driver.

Another problem with the "newer" CYW555xx is that they need to RESET with CTS set to low. Please try out port/posix-h4-bcm which asks the user to press the RESET button after the UART has been opened.

py-ir0nf1st commented 1 week ago

Hi,

I actually tried out the posix-h4-bcm port. The problem is that the port is broken with most of the existing examples if not all. When executing the btstack_main function of those examples, it calls hci_power_control(HCI_POWER_ON). After that hci.c sends HCI_RESET and CYW55573 will reset and later won't respond to READ_LOCAL_NAME, then the hci stack will stuck in HCI_STATE_INITIALIZING.

I spent a few days trying to figure out an elegant way to workaround it. One option is that, like you said, put the code in application main or chip driver. But if you read your code carefully you will find out that the application main or chip driver has no way to influence(modify or halt) hci stack state or substate. Please correct me if wrong. The other option is to add timeout mechanism to READ_LOCAL_NAME or BAUD_CHANGE until the hci stack can skip to HCI_INIT_CUSTOM_INIT. But it's a waste to spend a few hundreds of milliseconds waiting for nothing when the stack has enough information from READ_LOCAL_VERSION_INFORMATION. The third option is use event callback together with macro HAVE_HOST_CONTROLLER_API. The application main or chip driver listens on READ_LOCAL_VERSION_INFORMATION complete event and do its customized initialization. But the application code has to halt hci stack state transition for a while, a possible way to archive this is to manipulate h4 transport and make the hci stack halts on sending. I feel this option is too hacky and too complicated then the way I proposed.

Another thing is that you probably have a misunderstanding on "CYW555xx is that they need to RESET with CTS set to low". From what I observed, the "CTS" word mentioned in Infineon document or code stands for an input signal to the host. HAL_GPIO_WritePin(BT_REG_ON_GPIO_Port, BT_REG_ON_Pin, GPIO_PIN_RESET); HAL_Delay(100); HAL_GPIO_WritePin(BT_REG_ON_GPIO_Port, BT_REG_ON_Pin, GPIO_PIN_SET); uint32_t tickStart = HAL_GetTick(); while (1) { if (GPIO_PIN_RESET == HAL_GPIO_ReadPin(GPIOD, GPIO_PIN_3)) { printf("HCI CTS asserted after %u ticks\r\n", HAL_GetTick() - tickStart); break; } } The above code is what I used for resetting the chip and got to know if the chip is out of reset. When BT_REG_ON(high effective) is asserted, the chip starts to reset and host CTS is pulled high by the chip. Once the host CTS is pulled low by the chip, the chip is out of reset and the host is Clear To Send. At this moment the host is able to send HCI_RESET with an arbitrary baud rate(not really arbitrary but can be up to 1.5M from my observation) and the chip automatically detects the baudrate from the HCI_RESET pattern.

py-ir0nf1st commented 1 week ago

I actually tried out the posix-h4-bcm port. The problem is that the port is broken with most of the existing examples if not all. When executing the btstack_main function of those examples, it calls hci_power_control(HCI_POWER_ON). After that hci.c sends HCI_RESET and CYW55573 will reset and later won't respond to READ_LOCAL_NAME, then the hci stack will stuck in HCI_STATE_INITIALIZING. The posix-h4-bcm port is not broken. The issue is caused by my own code so to close this PR.

mringwal commented 1 week ago

Hi there. I was about to comment and confirm the posix-h4-port is working - I did a quick test with the CYW55513 to start evaluating its LE Audio features yesterday.

Thanks for thinking about the best way to integrate the 'new' AIROC firmware download mechanism and sharing these. The current one with the additional download code isn't particularly elegant. As there are other Controllers which firmware mechanism are not related to HCI at all but also use the UART and we're trying to keep the code base portable e.g. the Renesas/Dialog Controllers, the current version was created using the same mechanism.

A minor comment on timeout option: I agree that using a timeout there to detect the download mode wastes time, my understanding is that it's not possible to send another HCI command after that (at least the HCI flow control logic does not allow for that. The behaviour is incorrect as the Controller should sent a HCI Command Complete with error 'unknown HCI command' or similar).

Anyway, I like that your PR allows to use the existing custom init mechanism without a special "pre-loader". As mentioned, I don't want to list all future AIRO Controllers in hci.c by LMP subversion and map it to a PatchRAM file. While it might be possible to use the HCI + LMP subversions somehow to assume all versions newer than x require the new mechanism, it causes additional complexity. I think I have a simpler (to maintain) idea: we could add a hci_init_skip_read_local_name() function which tells the stack to skip the HCI Read Local Name during initialization.

For older Broadcom/Cypress/Infineon(AIROC) Controllers, the name indicates the PatchRAM file to load, the stack doesn't need it otherwise.

The new AIROC Controllers require the special reset mechanism anyway and if that is implemented by the platform port of BTstack, it can just call this function after hci_init().