Skip to contentSkip to conversation

Bluetooth: btusb: fixed command length alignment for Intel 8260

PATCH Superseded Tedd Ho-Jeong An 2 participants 3 messages May 7, 2015 – May 12, 2015 Bluetooth drivers

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Tedd Ho-Jeong An

author
#0 lore raw

PATCH 1/1 Bluetooth: btusb: fixed command length alignment for Intel 8260

From: Tedd Ho-Jeong An <[email protected]>

This patch fixes the command length alignment issue for Intel Bluetooth 8260.

The length of parameters in the firmware downloading command must be multiplication of 4. If not, the command must append Intel_NOP command with extra parameters, zeros, at the end and the firmware file is already included Intel_NOP command for alignment.

This patch checks the length of the parameters and if it is not aligned, then read the next command which is Intel_NOP command and append it to the first read command and send to the device together.

For example, if the data from the firmware file is like this:
8E FC 03 11 22 33 02 FC 03 00 00 00

Previously, btusb sends two commands:
09 FC 06 8E FC 03 11 22 33
09 FC 06 02 FC 03 00 00 00

This will not work due to the length of parameters are 6 and the device failed to align the data.

This patch will append them together and send one command:
09 FC 0B 8E FC 03 11 22 33 02 FC 03 00 00 00

Signed-off-by Tedd Ho-Jeong An <[email protected]>
---
 drivers/bluetooth/btusb.c | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)
drivers/bluetooth/btusb.c +35 -0
index d21f3b4..57f4eac 100644
@@ -2541,6 +2541,41 @@ static int btusb_setup_intel_new(struct hci_dev *hdev)
 
 		cmd_len = sizeof(*cmd) + cmd->plen;
 
+		/* The parameter length of secure send command should be a
+		 * multiplication of 4. If it is not, it appends Intel_NOP
+		 * command with arbitrary number of parameters (zeros) to meet
+		 * the 4 byte alignment.
+		 * The FW file has already aligned and appended Intel_NOP
+		 * command. So it needs to read next Intel_NOP command and send
+		 * them together.
+		 */
+		if ((cmd_len % 4) != 0) {
+			/* Make sure the next command is Intel_NOP command.
+			 */
+			struct hci_command_hdr *nop_cmd =
+						(void *)(fw_ptr + cmd_len);
+			if (nop_cmd->opcode != 0xfc02) {
+				BT_ERR("%s: Intel_NOP command is missing",
+				       hdev->name);
+				err = -EBADF;
+				goto done;
+			}
+
+			/* Command data is updated to include NOP command
+			 */
+			cmd_len += sizeof(*nop_cmd) + nop_cmd->plen;
+
+			/* Make sure the total length is aligned and smaller
+			 * than maximum param length
+			 */
+			if ((cmd_len % 4) != 0 || cmd_len > 252)   {
+				BT_ERR("%s: 4 byte alignment failed",
+				       hdev->name);
+				err = -EBADF;
+				goto done;
+			}
+		}
+
 		/* Send each command from the firmware data buffer as
 		 * a single Data fragment.
 		 */
Signature · 1 line
2.1.0
author
#1 lore raw

Hi Marcel,

I just wonder if you had a chance to take a look at this patch.

Let me know if you have any comments.

Regards,
Tedd

Quoted 78 lines · On Thu, 7 May 2015 09:08:56 -0700 Tedd Ho-Jeong An <[email protected]> wrote:

From: Tedd Ho-Jeong An <[email protected]>

This patch fixes the command length alignment issue for Intel Bluetooth 8260.

The length of parameters in the firmware downloading command must be multiplication of 4. If not, the command must append Intel_NOP command with extra parameters, zeros, at the end and the firmware file is already included Intel_NOP command for alignment.

This patch checks the length of the parameters and if it is not aligned, then read the next command which is Intel_NOP command and append it to the first read command and send to the device together.

For example, if the data from the firmware file is like this:
8E FC 03 11 22 33 02 FC 03 00 00 00

Previously, btusb sends two commands:
09 FC 06 8E FC 03 11 22 33
09 FC 06 02 FC 03 00 00 00

This will not work due to the length of parameters are 6 and the device failed to align the data.

This patch will append them together and send one command:
09 FC 0B 8E FC 03 11 22 33 02 FC 03 00 00 00

Signed-off-by Tedd Ho-Jeong An <[email protected]>
---
 drivers/bluetooth/btusb.c | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)
drivers/bluetooth/btusb.c +35 -0
index d21f3b4..57f4eac 100644
@@ -2541,6 +2541,41 @@ static int btusb_setup_intel_new(struct hci_dev *hdev)
 
 		cmd_len = sizeof(*cmd) + cmd->plen;
 
+		/* The parameter length of secure send command should be a
+		 * multiplication of 4. If it is not, it appends Intel_NOP
+		 * command with arbitrary number of parameters (zeros) to meet
+		 * the 4 byte alignment.
+		 * The FW file has already aligned and appended Intel_NOP
+		 * command. So it needs to read next Intel_NOP command and send
+		 * them together.
+		 */
+		if ((cmd_len % 4) != 0) {
+			/* Make sure the next command is Intel_NOP command.
+			 */
+			struct hci_command_hdr *nop_cmd =
+						(void *)(fw_ptr + cmd_len);
+			if (nop_cmd->opcode != 0xfc02) {
+				BT_ERR("%s: Intel_NOP command is missing",
+				       hdev->name);
+				err = -EBADF;
+				goto done;
+			}
+
+			/* Command data is updated to include NOP command
+			 */
+			cmd_len += sizeof(*nop_cmd) + nop_cmd->plen;
+
+			/* Make sure the total length is aligned and smaller
+			 * than maximum param length
+			 */
+			if ((cmd_len % 4) != 0 || cmd_len > 252)   {
+				BT_ERR("%s: 4 byte alignment failed",
+				       hdev->name);
+				err = -EBADF;
+				goto done;
+			}
+		}
+
 		/* Send each command from the firmware data buffer as
 		 * a single Data fragment.
 		 */
maintainer
#2 lore raw
In reply to Tedd Ho-Jeong An

Hi Tedd,

Quoted 36 lines

This patch fixes the command length alignment issue for Intel Bluetooth 8260.

The length of parameters in the firmware downloading command must be multiplication of 4. If not, the command must append Intel_NOP command with extra parameters, zeros, at the end and the firmware file is already included Intel_NOP command for alignment.

This patch checks the length of the parameters and if it is not aligned, then read the next command which is Intel_NOP command and append it to the first read command and send to the device together.

For example, if the data from the firmware file is like this:
8E FC 03 11 22 33 02 FC 03 00 00 00

Previously, btusb sends two commands:
09 FC 06 8E FC 03 11 22 33
09 FC 06 02 FC 03 00 00 00

This will not work due to the length of parameters are 6 and the device failed to align the data.

This patch will append them together and send one command:
09 FC 0B 8E FC 03 11 22 33 02 FC 03 00 00 00

Signed-off-by Tedd Ho-Jeong An <[email protected]>

---
drivers/bluetooth/btusb.c | 35 +++++++++++++++++++++++++++++++++++

1 file changed, 35 insertions(+)
drivers/bluetooth/btusb.c +0 -0
index d21f3b4..57f4eac 100644
@@ -2541,6 +2541,41 @@ static int btusb_setup_intel_new(struct hci_dev *hdev)

drivers/bluetooth/btusb.c static int btusb_setup_intel_new(struct hci_dev *hdev) · line 2551

Quoted 10 lines
drivers/bluetooth/btusb.c +8 -0
 		cmd_len = sizeof(*cmd) + cmd->plen;
 
+		/* The parameter length of secure send command should be a
+		 * multiplication of 4. If it is not, it appends Intel_NOP
+		 * command with arbitrary number of parameters (zeros) to meet
+		 * the 4 byte alignment.
+		 * The FW file has already aligned and appended Intel_NOP
+		 * command. So it needs to read next Intel_NOP command and send
+		 * them together.
+		 */

I would assume we just rely on the firmware file to build this correctly. So instead of checking the alignment, I would prefer that we check if the next command is a NOP and then send both commands.

drivers/bluetooth/btusb.c static int btusb_setup_intel_new(struct hci_dev *hdev) · line 2557

Quoted 6 lines
drivers/bluetooth/btusb.c +6 -0
+		if ((cmd_len % 4) != 0) {
+			/* Make sure the next command is Intel_NOP command.
+			 */
+			struct hci_command_hdr *nop_cmd =
+						(void *)(fw_ptr + cmd_len);
+			if (nop_cmd->opcode != 0xfc02) {

This is not big-endian safe.

Quoted 4 lines
drivers/bluetooth/btusb.c +4 -0
+				BT_ERR("%s: Intel_NOP command is missing",
+				       hdev->name);
+				err = -EBADF;
+				goto done;

drivers/bluetooth/btusb.c static int btusb_setup_intel_new(struct hci_dev *hdev) · line 2571

Quoted 10 lines
drivers/bluetooth/btusb.c +10 -0
+			}
+
+			/* Command data is updated to include NOP command
+			 */
+			cmd_len += sizeof(*nop_cmd) + nop_cmd->plen;
+
+			/* Make sure the total length is aligned and smaller
+			 * than maximum param length
+			 */
+			if ((cmd_len % 4) != 0 || cmd_len > 252)   {

I wonder why we check again here? The fragmentation is done the secure_send helper.

drivers/bluetooth/btusb.c static int btusb_setup_intel_new(struct hci_dev *hdev) · line 2581

Quoted 10 lines
drivers/bluetooth/btusb.c +7 -0
+				BT_ERR("%s: 4 byte alignment failed",
+				       hdev->name);
+				err = -EBADF;
+				goto done;
+			}
+		}
+
 		/* Send each command from the firmware data buffer as
 		 * a single Data fragment.
 		 */

Regards

Marcel