Hi Luiz,
> 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 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;
> - }
> -
why are you removing this? I put this in so that really only supported platforms are tried. We want to fail if you run on hardware that isn’t officially tested with this driver.
Regards
Marcel
