| Message ID | 20250306180931.57705-2-neeraj.sanjaykale@nxp.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2,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 linus/master v6.14-rc5 next-20250307] [cannot apply to bluetooth-next/master] [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-cmd5-and-cmd7/20250307-021228 base: https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth.git master patch link: https://lore.kernel.org/r/20250306180931.57705-2-neeraj.sanjaykale%40nxp.com patch subject: [PATCH v2 2/3] Bluetooth: btnxpuart: Handle bootloader error during cmd5 and cmd7 config: microblaze-randconfig-r123-20250309 (https://download.01.org/0day-ci/archive/20250309/202503091936.x9Evtskg-lkp@intel.com/config) compiler: microblaze-linux-gcc (GCC) 14.2.0 reproduce: (https://download.01.org/0day-ci/archive/20250309/202503091936.x9Evtskg-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/202503091936.x9Evtskg-lkp@intel.com/ sparse warnings: (new ones prefixed by >>) >> drivers/bluetooth/btnxpuart.c:1113:24: sparse: sparse: restricted __le16 degrades to integer drivers/bluetooth/btnxpuart.c:1119:24: sparse: sparse: restricted __le16 degrades to integer vim +1113 drivers/bluetooth/btnxpuart.c 1089 1090 static int nxp_recv_fw_req_v3(struct hci_dev *hdev, struct sk_buff *skb) 1091 { 1092 struct btnxpuart_dev *nxpdev = hci_get_drvdata(hdev); 1093 struct v3_data_req *req; 1094 __u16 len = 0; 1095 __u32 offset; 1096 1097 if (!process_boot_signature(nxpdev)) 1098 goto free_skb; 1099 1100 req = skb_pull_data(skb, sizeof(*req)); 1101 if (!req || !nxpdev->fw) 1102 goto free_skb; 1103 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) { 1120 nxpdev->fw_v3_offset_correction -= nxpdev->fw_v3_prev_sent; 1121 nxpdev->baudrate_changed = not_changed; 1122 } 1123 goto free_skb; 1124 } 1125 1126 len = __le16_to_cpu(req->len); 1127 1128 if (nxpdev->timeout_changed != changed) { 1129 nxp_fw_change_timeout(hdev, len); 1130 nxpdev->timeout_changed = cmd_sent; 1131 goto free_skb; 1132 } 1133 1134 if (nxpdev->baudrate_changed != changed) { 1135 if (nxp_fw_change_baudrate(hdev, len)) { 1136 nxpdev->baudrate_changed = cmd_sent; 1137 serdev_device_set_baudrate(nxpdev->serdev, 1138 HCI_NXP_SEC_BAUDRATE); 1139 serdev_device_set_flow_control(nxpdev->serdev, true); 1140 nxpdev->current_baudrate = HCI_NXP_SEC_BAUDRATE; 1141 } 1142 goto free_skb; 1143 } 1144 1145 if (req->len == 0) { 1146 bt_dev_info(hdev, "FW Download Complete: %zu bytes", 1147 nxpdev->fw->size); 1148 clear_bit(BTNXPUART_FW_DOWNLOADING, &nxpdev->tx_state); 1149 wake_up_interruptible(&nxpdev->fw_dnld_done_wait_q); 1150 goto free_skb; 1151 } 1152 1153 offset = __le32_to_cpu(req->offset); 1154 if (offset < nxpdev->fw_v3_offset_correction) { 1155 /* This scenario should ideally never occur. But if it ever does, 1156 * FW is out of sync and needs a power cycle. 1157 */ 1158 bt_dev_err(hdev, "Something went wrong during FW download"); 1159 bt_dev_err(hdev, "Please power cycle and try again"); 1160 goto free_skb; 1161 } 1162 1163 nxpdev->fw_dnld_v3_offset = offset - nxpdev->fw_v3_offset_correction; 1164 serdev_device_write_buf(nxpdev->serdev, nxpdev->fw->data + 1165 nxpdev->fw_dnld_v3_offset, len); 1166 1167 free_skb: 1168 nxpdev->fw_v3_prev_sent = len; 1169 kfree_skb(skb); 1170 return 0; 1171 } 1172
diff --git a/drivers/bluetooth/btnxpuart.c b/drivers/bluetooth/btnxpuart.c index b8a00bf062e2..a47181298e81 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); @@ -1097,7 +1103,7 @@ static int nxp_recv_fw_req_v3(struct hci_dev *hdev, struct sk_buff *skb) { struct btnxpuart_dev *nxpdev = hci_get_drvdata(hdev); struct v3_data_req *req; - __u16 len; + __u16 len = 0; __u32 offset; if (!process_boot_signature(nxpdev)) @@ -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 in case of CRC error. Fixes: 689ca16e5232 ("Bluetooth: NXP: Add protocol support for NXP Bluetooth chipsets") Signed-off-by: Neeraj Sanjay Kale <neeraj.sanjaykale@nxp.com> --- v2: Fix unused variable warning. (kernel test robot <lkp@intel.com>) --- drivers/bluetooth/btnxpuart.c | 57 ++++++++++++++++++++++++----------- 1 file changed, 40 insertions(+), 17 deletions(-)