On Wed, 2021-02-10 at 08:59 -0800, Luiz Augusto von Dentz wrote: > From: Luiz Augusto von Dentz <luiz.von.dentz@xxxxxxxxx> > > In order to allow new firmware to be loaded it first needs to check if > the firmware version on file matches the one loaded if it doesn't then > it needs to revert to boorloader mode in order to load the new firmware. > > Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@xxxxxxxxx> > --- > drivers/bluetooth/btintel.c | 22 +++++++++++ > drivers/bluetooth/btusb.c | 74 +++++++++++++++---------------------- > 2 files changed, 52 insertions(+), 44 deletions(-) > > diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c > index 89f85d54ca64..0d0f643f972a 100644 > --- a/drivers/bluetooth/btintel.c > +++ b/drivers/bluetooth/btintel.c > @@ -949,6 +949,17 @@ int btintel_download_firmware(struct hci_dev *hdev, > return -EALREADY; > } > > + /* The firmware variant determines if the device is in bootloader > + * mode or is running operational firmware. The value 0x06 identifies > + * the bootloader and the value 0x23 identifies the operational > + * firmware. > + * > + * If the firmware version has changed that means it needs to be reset > + * to bootloader when operational so the new firmware can be loaded. > + */ > + if (ver->fw_variant == 0x23) > + return -EINVAL; > + > err = btintel_sfi_rsa_header_secure_send(hdev, fw); > if (err) > return err; > @@ -976,6 +987,17 @@ int btintel_download_firmware_newgen(struct hci_dev *hdev, > return -EALREADY; > } > > + /* The firmware variant determines if the device is in bootloader > + * mode or is running operational firmware. The value 0x03 identifies > + * the bootloader and the value 0x23 identifies the operational > + * firmware. > + * > + * If the firmware version has changed that means it needs to be reset > + * to bootloader when operational so the new firmware can be loaded. > + */ > + if (ver->img_type == 0x03) > + return -EINVAL; > + > /* iBT hardware variants 0x0b, 0x0c, 0x11, 0x12, 0x13, 0x14 support > * only RSA secure boot engine. Hence, the corresponding sfi file will > * have RSA header of 644 bytes followed by Command Buffer. > diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c > index c92060e7472c..a44f3cf25790 100644 > --- a/drivers/bluetooth/btusb.c > +++ b/drivers/bluetooth/btusb.c > @@ -2469,14 +2469,30 @@ static int btusb_send_frame_intel(struct hci_dev *hdev, struct sk_buff *skb) > return -EILSEQ; > } > > -static bool btusb_setup_intel_new_get_fw_name(struct intel_version *ver, > +static int btusb_setup_intel_new_get_fw_name(struct intel_version *ver, > struct intel_boot_params *params, > char *fw_name, size_t len, > const char *suffix) > { > + /* The hardware platform number has a fixed value of 0x37 and > + * for now only accept this single value. > + */ > + if (ver->hw_platform != 0x37) > + return -EINVAL; > + > switch (ver->hw_variant) { > case 0x0b: /* SfP */ > case 0x0c: /* WsP */ > + /* The firmware variant determines if the device is in > + * bootloader mode or is running operational firmware. > + * > + * Version checking cannot be performed in these models since > + * the firmware versioning depends on the firmware being in > + * bootloader mode. > + */ > + if (ver->fw_variant == 0x23) > + return -EALREADY; > + > snprintf(fw_name, len, "intel/ibt-%u-%u.%s", > le16_to_cpu(ver->hw_variant), > le16_to_cpu(params->dev_revid), > @@ -2493,9 +2509,10 @@ static bool btusb_setup_intel_new_get_fw_name(struct intel_version *ver, > suffix); > break; > default: > - return false; > + return -EINVAL; > } > - return true; > + > + return 0; There is one more place in btusb_setup_intel_new()@btusb.c to update the handling of return value of this funcion, which is related to loading the DDC. Code like this... if (!err) { bt_dev_err(hdev, "Unsupported Intel firmware naming"); } else { > } > > static void btusb_setup_intel_newgen_get_fw_name(const struct intel_version_tlv *ver_tlv, > @@ -2550,7 +2567,6 @@ static int btusb_intel_download_firmware_newgen(struct hci_dev *hdev, > if (ver->img_type == 0x03) { > clear_bit(BTUSB_BOOTLOADER, &data->flags); > btintel_check_bdaddr(hdev); > - return 0; > } > > /* Check for supported iBT hardware variants of this firmware > @@ -2694,35 +2710,6 @@ static int btusb_intel_download_firmware(struct hci_dev *hdev, > if (!ver || !params) > return -EINVAL; > > - /* The hardware platform number has a fixed value of 0x37 and > - * for now only accept this single value. > - */ > - if (ver->hw_platform != 0x37) { > - bt_dev_err(hdev, "Unsupported Intel hardware platform (%u)", > - ver->hw_platform); > - return -EINVAL; > - } > - > - /* Check for supported iBT hardware variants of this firmware > - * loading method. > - * > - * This check has been put in place to ensure correct forward > - * compatibility options when newer hardware variants come along. > - */ > - switch (ver->hw_variant) { > - case 0x0b: /* SfP */ > - case 0x0c: /* WsP */ > - case 0x11: /* JfP */ > - case 0x12: /* ThP */ > - case 0x13: /* HrP */ > - case 0x14: /* CcP */ > - break; > - default: > - bt_dev_err(hdev, "Unsupported Intel hardware variant (%u)", > - ver->hw_variant); > - return -EINVAL; > - } > - > btintel_version_info(hdev, ver); > > /* The firmware variant determines if the device is in bootloader > @@ -2741,16 +2728,8 @@ static int btusb_intel_download_firmware(struct hci_dev *hdev, > if (ver->fw_variant == 0x23) { > clear_bit(BTUSB_BOOTLOADER, &data->flags); > btintel_check_bdaddr(hdev); > - return 0; > - } > - > - /* If the device is not in bootloader mode, then the only possible > - * choice is to return an error and abort the device initialization. > - */ > - if (ver->fw_variant != 0x06) { > - bt_dev_err(hdev, "Unsupported Intel firmware variant (%u)", > - ver->fw_variant); > - return -ENODEV; > + /* Proceed to download to check if the version matches */ > + goto download; > } > > /* Read the secure boot parameters to identify the operating > @@ -2778,6 +2757,7 @@ static int btusb_intel_download_firmware(struct hci_dev *hdev, > set_bit(HCI_QUIRK_INVALID_BDADDR, &hdev->quirks); > } > > +download: > /* With this Intel bootloader only the hardware variant and device > * revision information are used to select the right firmware for SfP > * and WsP. > @@ -2801,7 +2781,13 @@ static int btusb_intel_download_firmware(struct hci_dev *hdev, > */ > err = btusb_setup_intel_new_get_fw_name(ver, params, fwname, > sizeof(fwname), "sfi"); > - if (!err) { > + if (err < 0) { > + if (err == -EALREADY) { > + /* Firmware has already been loaded */ > + set_bit(BTUSB_FIRMWARE_LOADED, &data->flags); > + goto done; > + } > + > bt_dev_err(hdev, "Unsupported Intel firmware naming"); > return -EINVAL; > }