driver: bluetooth: Added Infineon AIROC CYW43xxx BT driver #55014

npal-cy · 2023-02-20T22:18:23Z

Added initial version of BT driver ( H4 HCI extension) for support Infineon AIROC CYW43xx devices.

dependency:

cc: @ifyall

drivers/bluetooth/hci/vendor/infineon/CMakeLists.txt

drivers/bluetooth/hci/vendor/infineon/Kconfig

drivers/bluetooth/hci/Kconfig

jhedberg

Generally looks pretty good! Just a few mainly style-related comments.

jhedberg · 2023-02-22T10:38:15Z

drivers/bluetooth/hci/Kconfig.infineon

+	bool "MURATA_1YN"
+endchoice
+
+config CYW43XX_CUSTOM_FIRMWARE_HCD_BLOB


Where and how is this option used? I don't see any references to it in this PR.

i will push one more commit with zephyr\modules\hal_infineon\btstack-integration\CMakeLists.txt after #44211.

Zephyr\modules\hal_infineon\btstack-integration\CMakeLists.txt will use this.
So user in prj.conf should provide path to HCD blob file:

it can be relative path from Application folder:

CONFIG_CYW43XX_CUSTOM_FIRMWARE_HCD_BLOB="BCM4343A1_001.002.009.0153.0000_Generic.hcd"

or absolute path:

CONFIG_CYW43XX_CUSTOM_FIRMWARE_HCD_BLOB="c:/cyw43xxx_fw/BCM4343A1_001.002.009.0153.0000_Generic.hcd"

zephyr\modules\hal_infineon\btstack-integration\CMakeLists.txt will check CONFIG_CYW43XX_CUSTOM_FIRMWARE_HCD_BLOB and generate c-array. See fragment of cmake below, full version i attached in post #55014 (comment):

.... .... # use user provided FIRMWARE HCD file (path must be defined in CONFIG_CYW43XX_CUSTOM_FIRMWARE_HCD_BLOB) if(CONFIG_CYW43XX_CUSTOM_FIRMWARE_HCD_BLOB) # Allowed to pass absolute path to HCD blob file, or relative path from Application folder. if(IS_ABSOLUTE ${CONFIG_CYW43XX_CUSTOM_FIRMWARE_HCD_BLOB}) set(blob_hcd_file ${CONFIG_CYW43XX_CUSTOM_FIRMWARE_HCD_BLOB}) else() set(blob_hcd_file ${APPLICATION_SOURCE_DIR}/${CONFIG_CYW43XX_CUSTOM_FIRMWARE_HCD_BLOB}) endif() endif() # generate Bluetooth include blob from HCD binary if(EXISTS ${blob_hcd_file}) message(INFO " generate include of blob Bluetooth file: ${blob_hcd_file}") generate_inc_file_for_target(app ${blob_hcd_file} ${blob_gen_inc_file}) zephyr_library_sources(${CMAKE_CURRENT_SOURCE_DIR}/w_bt_firmware_controller.c) endif()

added commits 2424db1 with Cmake where this option is used.

jhedberg · 2023-02-22T10:40:39Z

drivers/bluetooth/hci/cyw43xxx.c

+		printk("Error %d: failed to configure bt_reg_on %s pin %d\n",
+		       err, bt_reg_on.port->name, bt_reg_on.pin);


LOG_ERR()

jhedberg · 2023-02-22T10:40:57Z

drivers/bluetooth/hci/cyw43xxx.c

+		printk("Error: failed to configure bt_reg_on %s pin %d\n",
+		       bt_reg_on.port->name, bt_reg_on.pin);


LOG_ERR()

jhedberg · 2023-02-22T10:41:15Z

drivers/bluetooth/hci/cyw43xxx.c

+		/* Allocate buffer for hci_write_ram/hci_launch_ram command. */
+		buf = bt_hci_cmd_create(op_code, data_length);
+		if (buf == NULL) {
+			printk("Unable to allocate command buffer\n");


LOG_ERR()

jhedberg · 2023-02-22T10:43:03Z

drivers/bluetooth/hci/cyw43xxx.c

+	buf = bt_hci_cmd_create(BT_HCI_VND_OP_UPDATE_BAUDRATE,
+				HCI_VSC_UPDATE_BAUD_RATE_LENGTH);
+	if (buf == NULL) {
+		return -1;


It's a bad idea to mix -1 with other negative error values that actually mean something specific (POSIX error numbers), like you do when passing the error back up e.g. from bt_hci_cmd_send_sync(). Probably something like return -ENOMEM; would make sense here.

updated

It doesn't look like you did. Did you forget to commit the change?

jhedberg · 2023-02-22T10:43:45Z

drivers/bluetooth/hci/cyw43xxx.c

+static int bt_hci_uart_set_baudrate(const struct device *bt_uart_dev, uint32_t baudrate)
+{
+	struct uart_config uart_cfg;
+	int err = 0;


Unnecessary initialisation upon declaration.

dts/bindings/bluetooth/infineon,cyw43xxx-bt-hci.yaml

drivers/bluetooth/hci/Kconfig

drivers/bluetooth/hci/Kconfig.infineon

drivers/bluetooth/hci/cyw43xxx.c

fabiobaltieri · 2023-02-22T19:27:16Z

drivers/bluetooth/hci/cyw43xxx.c

+	 * ensure the integrity of the firmware image sent to the bluetooth chip.
+	 */
+	while (remaining_length) {
+		size_t data_length = data[2]; /* data length from firmware image block */


I'd suggest adding a debug log line in the cycle, this hangs after the first command if the firmware is incorrect, may come in handy. Up to you.

drivers/bluetooth/hci/cyw43xxx.c

dts/bindings/bluetooth/infineon,cyw43xxx-bt-hci.yaml

jhedberg · 2023-03-01T17:35:17Z

drivers/bluetooth/hci/cyw43xxx.c

+	buf = bt_hci_cmd_create(BT_HCI_VND_OP_UPDATE_BAUDRATE,
+				HCI_VSC_UPDATE_BAUD_RATE_LENGTH);
+	if (buf == NULL) {
+		return -1;


updated

It doesn't look like you did. Did you forget to commit the change?

jhedberg · 2023-03-01T17:35:24Z

drivers/bluetooth/hci/cyw43xxx.c

+	buf = bt_hci_cmd_create(BT_HCI_VND_OP_UPDATE_BAUDRATE,
+				HCI_VSC_UPDATE_BAUD_RATE_LENGTH);
+	if (buf == NULL) {
+		printk("Unable to allocate command buffer\n");


LOG_ERR()

jhedberg · 2023-03-01T17:36:15Z

drivers/bluetooth/hci/cyw43xxx.c

+		/* Allocate buffer for hci_write_ram/hci_launch_ram command. */
+		buf = bt_hci_cmd_create(op_code, data_length);
+		if (buf == NULL) {
+			LOG_ERR("Unable to allocate command buffer\n");


The LOG_*() macros automatically take care of line terminators, so you should remove the \n here

jhedberg · 2023-03-01T17:39:27Z

drivers/bluetooth/hci/cyw43xxx.c

+			return -ENOMEM;
+		}
+		net_buf_unref(rsp);
+		net_buf_unref(buf);


This looks like a bug. The bt_hci_cmd_send*() functions take ownership of the buffer passed to it. If you enabled net_buf asserts you'd probably trigger them when testing this code.

jhedberg · 2023-03-01T17:40:11Z

drivers/bluetooth/hci/cyw43xxx.c

+	/* Configure bt_reg_on as output  */
+	err = gpio_pin_configure_dt(&bt_reg_on, GPIO_OUTPUT | GPIO_PULL_UP);
+	if (err) {
+		LOG_ERR("Error %d: failed to configure bt_reg_on %s pin %d\n",


Remove the \n

fabiobaltieri · 2023-03-22T10:03:41Z

west.yml

@@ -73,7 +73,7 @@ manifest:
      groups:
        - hal
    - name: hal_infineon
-      revision: 8485083ad91d3e2cc5d706da3464716718a6a42e
+      revision: pull/7/head


zephyrproject-rtos/hal_infineon#7 has been merged, can you update this?

done with minor documentation updates in:

dts/bindings/bluetooth/infineon,cyw43xxx-bt-hci.yaml (added note that CYW43xxx requires fetch binary files of BT controller)

drivers/bluetooth/hci/Kconfig.infineon (update/simplified description for modules)

- Added Cmakefiles for modules\hal_infineon\btstack-integration\, where handle generation of Bluetooth include blob from HCD binary. - Supported following device/modules: -- CYW43012/MURATA-1LV module -- CYW4343W/MURATA-1DX module -- CYW43439/MURATA-1YN module -- CYW4373/STERLING-LWB5plus module - Added possibility to use user provided BT Firmware HCD file via kconfig (path must be defined in CONFIG_CYW43XX_CUSTOM_FIRMWARE_HCD_BLOB) - Updated top makefile Signed-off-by: Nazar Palamar <nazar.palamar@infineon.com>

jhedberg · 2023-03-22T13:13:39Z

drivers/bluetooth/hci/cyw43xxx.c

+		void *data_buf = net_buf_add(buf, data_length);
+
+		(void)memcpy(data_buf, (void *)&data[3], data_length);


net_buf_add_mem(buf, &data[3], data_length);

updated in two places: https://github.com/zephyrproject-rtos/zephyr/compare/13a27679822aeafb6d35aa9c869542ef50f826ae..68282841e8cbc6ea418a4c5ef8125ea7546d5f77

Added initial version of Infineon cyw43xxx BT ( H4 HCI extension drivers/bluetooth/hci/CMakeLists.txt) Add initial version of binding file for Infineon CYW43xx BT HCI extension driver. Signed-off-by: Nazar Palamar <nazar.palamar@infineon.com>

Enable BLE features for CY8CPROTO-062-4343W board. Signed-off-by: Nazar Palamar <nazar.palamar@infineon.com>

npal-cy requested review from galak, alwa-nordic, jhedberg and Vudentz as code owners February 20, 2023 22:18

zephyrbot added area: Devicetree Binding PR modifies or adds a Device Tree binding area: Bluetooth labels Feb 20, 2023

zephyrbot requested review from asbjornsabo, hermabe, jori-nordic, sjanc, Thalley and theob-pro February 20, 2023 22:18

zephyrbot assigned jhedberg Feb 20, 2023

npal-cy mentioned this pull request Feb 20, 2023

Introduce blobs for AIROC CYW43xx Bluetooth/Wi-Fi Connectivity SoCs #54936

Closed

2 tasks

npal-cy changed the title ~~driver: bluetooth: Added Infineon cyw43xxx BT driver~~ driver: bluetooth: Added Infineon AIROC CYW43xxx BT driver Feb 20, 2023

jori-nordic requested changes Feb 21, 2023

View reviewed changes

npal-cy force-pushed the topic/add_bt_cyw43xxx_driver branch from 8996183 to b43af6a Compare February 22, 2023 09:08

jori-nordic previously approved these changes Feb 22, 2023

View reviewed changes

jhedberg requested changes Feb 22, 2023

View reviewed changes

fabiobaltieri reviewed Feb 22, 2023

View reviewed changes

petejohanson mentioned this pull request Feb 24, 2023

rp2040 (raspberry pi pico) support tracking issue zmkfirmware/zmk#1085

Open

mniestroj reviewed Mar 1, 2023

View reviewed changes

drivers/bluetooth/hci/cyw43xxx.c Show resolved Hide resolved

dts/bindings/bluetooth/infineon,cyw43xxx-bt-hci.yaml Outdated Show resolved Hide resolved

npal-cy dismissed jori-nordic’s stale review via 0571c58 March 1, 2023 14:44

npal-cy force-pushed the topic/add_bt_cyw43xxx_driver branch from b43af6a to 0571c58 Compare March 1, 2023 14:44

npal-cy requested a review from nashif as a code owner March 1, 2023 14:44

zephyrbot added the manifest-hal_infineon label Mar 1, 2023

zephyrbot requested a review from ifyall March 1, 2023 14:45

ifyall previously approved these changes Mar 1, 2023

View reviewed changes

jhedberg requested changes Mar 1, 2023

View reviewed changes

npal-cy dismissed ifyall’s stale review via c994f51 March 1, 2023 17:57

fabiobaltieri previously approved these changes Mar 10, 2023

View reviewed changes

fabiobaltieri reviewed Mar 22, 2023

View reviewed changes

npal-cy dismissed stale reviews from fabiobaltieri, jori-nordic, and mniestroj via 5a2826f March 22, 2023 11:11

npal-cy force-pushed the topic/add_bt_cyw43xxx_driver branch from 0fd68f2 to 5a2826f Compare March 22, 2023 11:11

zephyrbot added platform: Infineon Infineon Technologies AG and removed DNM This PR should not be merged (Do Not Merge) labels Mar 22, 2023

npal-cy force-pushed the topic/add_bt_cyw43xxx_driver branch from 5a2826f to 13a2767 Compare March 22, 2023 11:31

carlescufi requested a review from jhedberg March 22, 2023 12:54

jhedberg reviewed Mar 22, 2023

View reviewed changes

npal-cy added 2 commits March 22, 2023 15:38

boards: arm: cy8cproto-062-4343w: enable BLE features

6828284

Enable BLE features for CY8CPROTO-062-4343W board. Signed-off-by: Nazar Palamar <nazar.palamar@infineon.com>

npal-cy force-pushed the topic/add_bt_cyw43xxx_driver branch from 13a2767 to 6828284 Compare March 22, 2023 13:39

npal-cy requested review from jhedberg and removed request for nashif, Vudentz, Thalley, sjanc, galak, hermabe, asbjornsabo, alwa-nordic and theob-pro March 22, 2023 14:01

jhedberg approved these changes Mar 22, 2023

View reviewed changes

ifyall self-requested a review March 22, 2023 14:38

ifyall approved these changes Mar 22, 2023

View reviewed changes

fabiobaltieri approved these changes Mar 22, 2023

View reviewed changes

carlescufi merged commit 8fb4a18 into zephyrproject-rtos:main Mar 22, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

driver: bluetooth: Added Infineon AIROC CYW43xxx BT driver #55014

driver: bluetooth: Added Infineon AIROC CYW43xxx BT driver #55014

npal-cy commented Feb 20, 2023 •

edited

Loading

jhedberg left a comment

jhedberg Feb 22, 2023

npal-cy Feb 22, 2023

npal-cy Mar 1, 2023

jhedberg Feb 22, 2023

npal-cy Mar 1, 2023

jhedberg Feb 22, 2023

npal-cy Mar 1, 2023

jhedberg Feb 22, 2023

npal-cy Mar 1, 2023

jhedberg Feb 22, 2023

npal-cy Mar 1, 2023

jhedberg Mar 1, 2023

jhedberg Feb 22, 2023

npal-cy Mar 1, 2023

fabiobaltieri Feb 22, 2023

jhedberg Mar 1, 2023

jhedberg Mar 1, 2023

npal-cy Mar 2, 2023

jhedberg Mar 1, 2023

jhedberg Mar 1, 2023

npal-cy Mar 2, 2023

jhedberg Mar 1, 2023

fabiobaltieri Mar 22, 2023

npal-cy Mar 22, 2023

jhedberg Mar 22, 2023

npal-cy Mar 22, 2023

		printk("Error %d: failed to configure bt_reg_on %s pin %d\n",
		err, bt_reg_on.port->name, bt_reg_on.pin);

		printk("Error: failed to configure bt_reg_on %s pin %d\n",
		bt_reg_on.port->name, bt_reg_on.pin);

		void *data_buf = net_buf_add(buf, data_length);

		(void)memcpy(data_buf, (void *)&data[3], data_length);

driver: bluetooth: Added Infineon AIROC CYW43xxx BT driver #55014

driver: bluetooth: Added Infineon AIROC CYW43xxx BT driver #55014

Conversation

npal-cy commented Feb 20, 2023 • edited Loading

jhedberg left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

npal-cy commented Feb 20, 2023 •

edited

Loading