| Message ID | 20250305134523.40111-2-neeraj.sanjaykale@nxp.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v1,1/3] Bluetooth: btnxpuart: Add correct bootloader error codes | expand |
| Context | Check | Description |
|---|---|---|
| tedd_an/pre-ci_am | success | Success |
| tedd_an/SubjectPrefix | success | Gitlint PASS |
Hi Neeraj, kernel test robot noticed the following build warnings: [auto build test WARNING on bluetooth/master] [also build test WARNING on bluetooth-next/master linus/master v6.14-rc5 next-20250306] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Neeraj-Sanjay-Kale/Bluetooth-btnxpuart-Handle-bootloader-error-during-change-baudrate/20250305-214856 base: https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth.git master patch link: https://lore.kernel.org/r/20250305134523.40111-2-neeraj.sanjaykale%40nxp.com patch subject: [PATCH v1 2/3] Bluetooth: btnxpuart: Handle bootloader error during change baudrate config: i386-buildonly-randconfig-001-20250306 (https://download.01.org/0day-ci/archive/20250307/202503070049.8e6dNvjC-lkp@intel.com/config) compiler: clang version 19.1.7 (https://github.com/llvm/llvm-project cd708029e0b2869e80abe31ddb175f7c35361f90) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250307/202503070049.8e6dNvjC-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202503070049.8e6dNvjC-lkp@intel.com/ All warnings (new ones prefixed by >>): >> drivers/bluetooth/btnxpuart.c:1104:6: warning: variable 'len' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized] 1104 | if (!req->error) { | ^~~~~~~~~~~ drivers/bluetooth/btnxpuart.c:1168:28: note: uninitialized use occurs here 1168 | nxpdev->fw_v3_prev_sent = len; | ^~~ drivers/bluetooth/btnxpuart.c:1104:2: note: remove the 'if' if its condition is always true 1104 | if (!req->error) { | ^~~~~~~~~~~~~~~~ 1105 | nxp_send_ack(NXP_ACK_V3, hdev); 1106 | if (nxpdev->timeout_changed == cmd_sent) 1107 | nxpdev->timeout_changed = changed; 1108 | if (nxpdev->baudrate_changed == cmd_sent) 1109 | nxpdev->baudrate_changed = changed; 1110 | } else { | ~~~~~~ 1111 | nxp_handle_fw_download_error(hdev, req); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 1112 | if (nxpdev->timeout_changed == cmd_sent && | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 1113 | req->error == NXP_CRC_RX_ERROR) { | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 1114 | nxpdev->fw_v3_offset_correction -= nxpdev->fw_v3_prev_sent; | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 1115 | nxpdev->timeout_changed = not_changed; | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 1116 | } | ~ 1117 | /* After baudrate change, it is normal to get ACK Timeout error */ | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 1118 | if (nxpdev->baudrate_changed == cmd_sent && | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 1119 | req->error == NXP_CRC_RX_ERROR) { | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ >> drivers/bluetooth/btnxpuart.c:1101:6: warning: variable 'len' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized] 1101 | if (!req || !nxpdev->fw) | ^~~~~~~~~~~~~~~~~~~ drivers/bluetooth/btnxpuart.c:1168:28: note: uninitialized use occurs here 1168 | nxpdev->fw_v3_prev_sent = len; | ^~~ drivers/bluetooth/btnxpuart.c:1101:2: note: remove the 'if' if its condition is always false 1101 | if (!req || !nxpdev->fw) | ^~~~~~~~~~~~~~~~~~~~~~~~ 1102 | goto free_skb; | ~~~~~~~~~~~~~ >> drivers/bluetooth/btnxpuart.c:1101:6: warning: variable 'len' is used uninitialized whenever '||' condition is true [-Wsometimes-uninitialized] 1101 | if (!req || !nxpdev->fw) | ^~~~ drivers/bluetooth/btnxpuart.c:1168:28: note: uninitialized use occurs here 1168 | nxpdev->fw_v3_prev_sent = len; | ^~~ drivers/bluetooth/btnxpuart.c:1101:6: note: remove the '||' if its condition is always false 1101 | if (!req || !nxpdev->fw) | ^~~~~~~ drivers/bluetooth/btnxpuart.c:1097:6: warning: variable 'len' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized] 1097 | if (!process_boot_signature(nxpdev)) | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ drivers/bluetooth/btnxpuart.c:1168:28: note: uninitialized use occurs here 1168 | nxpdev->fw_v3_prev_sent = len; | ^~~ drivers/bluetooth/btnxpuart.c:1097:2: note: remove the 'if' if its condition is always false 1097 | if (!process_boot_signature(nxpdev)) | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 1098 | goto free_skb; | ~~~~~~~~~~~~~ drivers/bluetooth/btnxpuart.c:1094:11: note: initialize the variable 'len' to silence this warning 1094 | __u16 len; | ^ | = 0 4 warnings generated. vim +1104 drivers/bluetooth/btnxpuart.c 27489364299a2d Neeraj Sanjay Kale 2024-06-14 1089 689ca16e523278 Neeraj Sanjay Kale 2023-03-16 1090 static int nxp_recv_fw_req_v3(struct hci_dev *hdev, struct sk_buff *skb) 689ca16e523278 Neeraj Sanjay Kale 2023-03-16 1091 { 689ca16e523278 Neeraj Sanjay Kale 2023-03-16 1092 struct btnxpuart_dev *nxpdev = hci_get_drvdata(hdev); 689ca16e523278 Neeraj Sanjay Kale 2023-03-16 1093 struct v3_data_req *req; 9e080b53dafae5 Luiz Augusto von Dentz 2023-04-17 1094 __u16 len; 9e080b53dafae5 Luiz Augusto von Dentz 2023-04-17 1095 __u32 offset; 689ca16e523278 Neeraj Sanjay Kale 2023-03-16 1096 689ca16e523278 Neeraj Sanjay Kale 2023-03-16 1097 if (!process_boot_signature(nxpdev)) 689ca16e523278 Neeraj Sanjay Kale 2023-03-16 1098 goto free_skb; 689ca16e523278 Neeraj Sanjay Kale 2023-03-16 1099 9e080b53dafae5 Luiz Augusto von Dentz 2023-04-17 1100 req = skb_pull_data(skb, sizeof(*req)); 689ca16e523278 Neeraj Sanjay Kale 2023-03-16 @1101 if (!req || !nxpdev->fw) 689ca16e523278 Neeraj Sanjay Kale 2023-03-16 1102 goto free_skb; 689ca16e523278 Neeraj Sanjay Kale 2023-03-16 1103 27489364299a2d Neeraj Sanjay Kale 2024-06-14 @1104 if (!req->error) { 689ca16e523278 Neeraj Sanjay Kale 2023-03-16 1105 nxp_send_ack(NXP_ACK_V3, hdev); 5bf71799df5728 Neeraj Sanjay Kale 2025-03-05 1106 if (nxpdev->timeout_changed == cmd_sent) 5bf71799df5728 Neeraj Sanjay Kale 2025-03-05 1107 nxpdev->timeout_changed = changed; 5bf71799df5728 Neeraj Sanjay Kale 2025-03-05 1108 if (nxpdev->baudrate_changed == cmd_sent) 5bf71799df5728 Neeraj Sanjay Kale 2025-03-05 1109 nxpdev->baudrate_changed = changed; 27489364299a2d Neeraj Sanjay Kale 2024-06-14 1110 } else { 27489364299a2d Neeraj Sanjay Kale 2024-06-14 1111 nxp_handle_fw_download_error(hdev, req); 5bf71799df5728 Neeraj Sanjay Kale 2025-03-05 1112 if (nxpdev->timeout_changed == cmd_sent && 5bf71799df5728 Neeraj Sanjay Kale 2025-03-05 1113 req->error == NXP_CRC_RX_ERROR) { 5bf71799df5728 Neeraj Sanjay Kale 2025-03-05 1114 nxpdev->fw_v3_offset_correction -= nxpdev->fw_v3_prev_sent; 5bf71799df5728 Neeraj Sanjay Kale 2025-03-05 1115 nxpdev->timeout_changed = not_changed; 5bf71799df5728 Neeraj Sanjay Kale 2025-03-05 1116 } 5bf71799df5728 Neeraj Sanjay Kale 2025-03-05 1117 /* After baudrate change, it is normal to get ACK Timeout error */ 5bf71799df5728 Neeraj Sanjay Kale 2025-03-05 1118 if (nxpdev->baudrate_changed == cmd_sent && 5bf71799df5728 Neeraj Sanjay Kale 2025-03-05 1119 req->error == NXP_CRC_RX_ERROR) { 5bf71799df5728 Neeraj Sanjay Kale 2025-03-05 1120 nxpdev->fw_v3_offset_correction -= nxpdev->fw_v3_prev_sent; 5bf71799df5728 Neeraj Sanjay Kale 2025-03-05 1121 nxpdev->baudrate_changed = not_changed; 5bf71799df5728 Neeraj Sanjay Kale 2025-03-05 1122 } 27489364299a2d Neeraj Sanjay Kale 2024-06-14 1123 goto free_skb; 27489364299a2d Neeraj Sanjay Kale 2024-06-14 1124 } 689ca16e523278 Neeraj Sanjay Kale 2023-03-16 1125 9e080b53dafae5 Luiz Augusto von Dentz 2023-04-17 1126 len = __le16_to_cpu(req->len); 9e080b53dafae5 Luiz Augusto von Dentz 2023-04-17 1127 5bf71799df5728 Neeraj Sanjay Kale 2025-03-05 1128 if (nxpdev->timeout_changed != changed) { 5bf71799df5728 Neeraj Sanjay Kale 2025-03-05 1129 nxp_fw_change_timeout(hdev, len); 5bf71799df5728 Neeraj Sanjay Kale 2025-03-05 1130 nxpdev->timeout_changed = cmd_sent; 689ca16e523278 Neeraj Sanjay Kale 2023-03-16 1131 goto free_skb; 689ca16e523278 Neeraj Sanjay Kale 2023-03-16 1132 } 689ca16e523278 Neeraj Sanjay Kale 2023-03-16 1133 5bf71799df5728 Neeraj Sanjay Kale 2025-03-05 1134 if (nxpdev->baudrate_changed != changed) { 5bf71799df5728 Neeraj Sanjay Kale 2025-03-05 1135 if (nxp_fw_change_baudrate(hdev, len)) { 5bf71799df5728 Neeraj Sanjay Kale 2025-03-05 1136 nxpdev->baudrate_changed = cmd_sent; 689ca16e523278 Neeraj Sanjay Kale 2023-03-16 1137 serdev_device_set_baudrate(nxpdev->serdev, 689ca16e523278 Neeraj Sanjay Kale 2023-03-16 1138 HCI_NXP_SEC_BAUDRATE); b0310d6ed684b8 Neeraj Sanjay Kale 2023-04-19 1139 serdev_device_set_flow_control(nxpdev->serdev, true); 689ca16e523278 Neeraj Sanjay Kale 2023-03-16 1140 nxpdev->current_baudrate = HCI_NXP_SEC_BAUDRATE; 689ca16e523278 Neeraj Sanjay Kale 2023-03-16 1141 } 689ca16e523278 Neeraj Sanjay Kale 2023-03-16 1142 goto free_skb; 689ca16e523278 Neeraj Sanjay Kale 2023-03-16 1143 } 689ca16e523278 Neeraj Sanjay Kale 2023-03-16 1144 689ca16e523278 Neeraj Sanjay Kale 2023-03-16 1145 if (req->len == 0) { 2684dd614ccf08 Neeraj Sanjay Kale 2024-05-15 1146 bt_dev_info(hdev, "FW Download Complete: %zu bytes", 9e080b53dafae5 Luiz Augusto von Dentz 2023-04-17 1147 nxpdev->fw->size); 689ca16e523278 Neeraj Sanjay Kale 2023-03-16 1148 clear_bit(BTNXPUART_FW_DOWNLOADING, &nxpdev->tx_state); 689ca16e523278 Neeraj Sanjay Kale 2023-03-16 1149 wake_up_interruptible(&nxpdev->fw_dnld_done_wait_q); 689ca16e523278 Neeraj Sanjay Kale 2023-03-16 1150 goto free_skb; 689ca16e523278 Neeraj Sanjay Kale 2023-03-16 1151 } 689ca16e523278 Neeraj Sanjay Kale 2023-03-16 1152 9e080b53dafae5 Luiz Augusto von Dentz 2023-04-17 1153 offset = __le32_to_cpu(req->offset); 9e080b53dafae5 Luiz Augusto von Dentz 2023-04-17 1154 if (offset < nxpdev->fw_v3_offset_correction) { 689ca16e523278 Neeraj Sanjay Kale 2023-03-16 1155 /* This scenario should ideally never occur. But if it ever does, 689ca16e523278 Neeraj Sanjay Kale 2023-03-16 1156 * FW is out of sync and needs a power cycle. 689ca16e523278 Neeraj Sanjay Kale 2023-03-16 1157 */ 9e080b53dafae5 Luiz Augusto von Dentz 2023-04-17 1158 bt_dev_err(hdev, "Something went wrong during FW download"); 9e080b53dafae5 Luiz Augusto von Dentz 2023-04-17 1159 bt_dev_err(hdev, "Please power cycle and try again"); 689ca16e523278 Neeraj Sanjay Kale 2023-03-16 1160 goto free_skb; 689ca16e523278 Neeraj Sanjay Kale 2023-03-16 1161 } 689ca16e523278 Neeraj Sanjay Kale 2023-03-16 1162 e3c4891098c875 Neeraj Sanjay Kale 2024-05-15 1163 nxpdev->fw_dnld_v3_offset = offset - nxpdev->fw_v3_offset_correction; e3c4891098c875 Neeraj Sanjay Kale 2024-05-15 1164 serdev_device_write_buf(nxpdev->serdev, nxpdev->fw->data + e3c4891098c875 Neeraj Sanjay Kale 2024-05-15 1165 nxpdev->fw_dnld_v3_offset, len); 689ca16e523278 Neeraj Sanjay Kale 2023-03-16 1166 689ca16e523278 Neeraj Sanjay Kale 2023-03-16 1167 free_skb: 5bf71799df5728 Neeraj Sanjay Kale 2025-03-05 1168 nxpdev->fw_v3_prev_sent = len; 689ca16e523278 Neeraj Sanjay Kale 2023-03-16 1169 kfree_skb(skb); 689ca16e523278 Neeraj Sanjay Kale 2023-03-16 1170 return 0; 689ca16e523278 Neeraj Sanjay Kale 2023-03-16 1171 } 689ca16e523278 Neeraj Sanjay Kale 2023-03-16 1172
diff --git a/drivers/bluetooth/btnxpuart.c b/drivers/bluetooth/btnxpuart.c index b8a00bf062e2..4367d59b4653 100644 --- a/drivers/bluetooth/btnxpuart.c +++ b/drivers/bluetooth/btnxpuart.c @@ -162,6 +162,12 @@ struct btnxpuart_data { const char *fw_name_old; }; +enum bootloader_param_change { + not_changed, + cmd_sent, + changed +}; + struct btnxpuart_dev { struct hci_dev *hdev; struct serdev_device *serdev; @@ -177,6 +183,7 @@ struct btnxpuart_dev { u32 fw_v1_sent_bytes; u32 fw_dnld_v3_offset; u32 fw_v3_offset_correction; + u32 fw_v3_prev_sent; u32 fw_v1_expected_len; u32 boot_reg_offset; wait_queue_head_t fw_dnld_done_wait_q; @@ -185,8 +192,8 @@ struct btnxpuart_dev { u32 new_baudrate; u32 current_baudrate; u32 fw_init_baudrate; - bool timeout_changed; - bool baudrate_changed; + enum bootloader_param_change timeout_changed; + enum bootloader_param_change baudrate_changed; bool helper_downloaded; struct ps_data psdata; @@ -660,8 +667,8 @@ static int nxp_download_firmware(struct hci_dev *hdev) nxpdev->boot_reg_offset = 0; nxpdev->fw_dnld_v3_offset = 0; nxpdev->fw_v3_offset_correction = 0; - nxpdev->baudrate_changed = false; - nxpdev->timeout_changed = false; + nxpdev->baudrate_changed = not_changed; + nxpdev->timeout_changed = not_changed; nxpdev->helper_downloaded = false; serdev_device_set_baudrate(nxpdev->serdev, HCI_NXP_PRI_BAUDRATE); @@ -883,15 +890,14 @@ static int nxp_recv_fw_req_v1(struct hci_dev *hdev, struct sk_buff *skb) len = __le16_to_cpu(req->len); if (!nxp_data->helper_fw_name) { - if (!nxpdev->timeout_changed) { - nxpdev->timeout_changed = nxp_fw_change_timeout(hdev, - len); + if (nxpdev->timeout_changed != changed) { + nxp_fw_change_timeout(hdev, len); + nxpdev->timeout_changed = changed; goto free_skb; } - if (!nxpdev->baudrate_changed) { - nxpdev->baudrate_changed = nxp_fw_change_baudrate(hdev, - len); - if (nxpdev->baudrate_changed) { + if (nxpdev->baudrate_changed != changed) { + if (nxp_fw_change_baudrate(hdev, len)) { + nxpdev->baudrate_changed = changed; serdev_device_set_baudrate(nxpdev->serdev, HCI_NXP_SEC_BAUDRATE); serdev_device_set_flow_control(nxpdev->serdev, true); @@ -1109,21 +1115,37 @@ static int nxp_recv_fw_req_v3(struct hci_dev *hdev, struct sk_buff *skb) if (!req->error) { nxp_send_ack(NXP_ACK_V3, hdev); + if (nxpdev->timeout_changed == cmd_sent) + nxpdev->timeout_changed = changed; + if (nxpdev->baudrate_changed == cmd_sent) + nxpdev->baudrate_changed = changed; } else { nxp_handle_fw_download_error(hdev, req); + if (nxpdev->timeout_changed == cmd_sent && + req->error == NXP_CRC_RX_ERROR) { + nxpdev->fw_v3_offset_correction -= nxpdev->fw_v3_prev_sent; + nxpdev->timeout_changed = not_changed; + } + /* After baudrate change, it is normal to get ACK Timeout error */ + if (nxpdev->baudrate_changed == cmd_sent && + req->error == NXP_CRC_RX_ERROR) { + nxpdev->fw_v3_offset_correction -= nxpdev->fw_v3_prev_sent; + nxpdev->baudrate_changed = not_changed; + } goto free_skb; } len = __le16_to_cpu(req->len); - if (!nxpdev->timeout_changed) { - nxpdev->timeout_changed = nxp_fw_change_timeout(hdev, len); + if (nxpdev->timeout_changed != changed) { + nxp_fw_change_timeout(hdev, len); + nxpdev->timeout_changed = cmd_sent; goto free_skb; } - if (!nxpdev->baudrate_changed) { - nxpdev->baudrate_changed = nxp_fw_change_baudrate(hdev, len); - if (nxpdev->baudrate_changed) { + if (nxpdev->baudrate_changed != changed) { + if (nxp_fw_change_baudrate(hdev, len)) { + nxpdev->baudrate_changed = cmd_sent; serdev_device_set_baudrate(nxpdev->serdev, HCI_NXP_SEC_BAUDRATE); serdev_device_set_flow_control(nxpdev->serdev, true); @@ -1155,6 +1177,7 @@ static int nxp_recv_fw_req_v3(struct hci_dev *hdev, struct sk_buff *skb) nxpdev->fw_dnld_v3_offset, len); free_skb: + nxpdev->fw_v3_prev_sent = len; kfree_skb(skb); return 0; }
This handles the scenario where the driver receives an error code after sending cmd5 or cmd7 in the bootloader signature during FW download. The bootloader error code is handled by the driver and FW offset is corrected accordingly, and the cmd5 or cmd7 is re-sent to the controller. Fixes: 689ca16e5232 ("Bluetooth: NXP: Add protocol support for NXP Bluetooth chipsets") Signed-off-by: Neeraj Sanjay Kale <neeraj.sanjaykale@nxp.com> --- drivers/bluetooth/btnxpuart.c | 55 +++++++++++++++++++++++++---------- 1 file changed, 39 insertions(+), 16 deletions(-)