Hi Luiz,
On Tue, 2021-02-09 at 15:34 -0800, Luiz Augusto von Dentz wrote:
> From: Luiz Augusto von Dentz <[email protected]>
>
> 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 <[email protected]>
> ---
> 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 153989bd8d5f..ccab05f67df9 100644
> --- a/drivers/bluetooth/btintel.c
> +++ b/drivers/bluetooth/btintel.c
> @@ -948,6 +948,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;
> @@ -974,6 +985,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 e896c6702d60..113ff780232f 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;
> }
>
> 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
> @@ -2693,35 +2709,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
> @@ -2740,16 +2727,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
> @@ -2777,6 +2756,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.
> @@ -2800,7 +2780,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) {
There is one more place to change the handling of return code for this function.
err = btusb_setup_intel_new_get_fw_name(&ver, ¶ms, ddcname,
sizeof(ddcname), "ddc");
if (!err) {
bt_dev_err(hdev, "Unsupported Intel firmware naming");
} else {
> + 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;
> }