From: Luiz Augusto von Dentz <[email protected]>
This checks the firmware build number, week and year matches with
repective version loaded and then skip the download process.
Signed-off-by: Luiz Augusto von Dentz <[email protected]>
---
v2: Add patch that mover checks for operational mode after the version
checking.
v3: Fix not checking for operation mode before using btintel_read_boot_params
since some models depend on that to contruct the fw filename. Also attempt to
cleanup duplicated code.
v4: Fix forwarding -EALREADY when firmware has already been loaded.
drivers/bluetooth/btintel.c | 96 +++++++++++++++++++++++++++--------
drivers/bluetooth/btintel.h | 5 +-
drivers/bluetooth/btusb.c | 18 ++++++-
drivers/bluetooth/hci_intel.c | 7 ++-
4 files changed, 100 insertions(+), 26 deletions(-)
diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
index 88ce5f0ffc4b..89f85d54ca64 100644
--- a/drivers/bluetooth/btintel.c
+++ b/drivers/bluetooth/btintel.c
@@ -24,6 +24,14 @@
#define ECDSA_OFFSET 644
#define ECDSA_HEADER_LEN 320
+#define CMD_WRITE_BOOT_PARAMS 0xfc0e
+struct cmd_write_boot_params {
+ u32 boot_addr;
+ u8 fw_build_num;
+ u8 fw_build_ww;
+ u8 fw_build_yy;
+} __packed;
+
int btintel_check_bdaddr(struct hci_dev *hdev)
{
struct hci_rp_read_bd_addr *bda;
@@ -841,7 +849,7 @@ static int btintel_sfi_ecdsa_header_secure_send(struct hci_dev *hdev,
static int btintel_download_firmware_payload(struct hci_dev *hdev,
const struct firmware *fw,
- u32 *boot_param, size_t offset)
+ size_t offset)
{
int err;
const u8 *fw_ptr;
@@ -854,20 +862,6 @@ static int btintel_download_firmware_payload(struct hci_dev *hdev,
while (fw_ptr - fw->data < fw->size) {
struct hci_command_hdr *cmd = (void *)(fw_ptr + frag_len);
- /* Each SKU has a different reset parameter to use in the
- * HCI_Intel_Reset command and it is embedded in the firmware
- * data. So, instead of using static value per SKU, check
- * the firmware data and save it for later use.
- */
- if (le16_to_cpu(cmd->opcode) == 0xfc0e) {
- /* The boot parameter is the first 32-bit value
- * and rest of 3 octets are reserved.
- */
- *boot_param = get_unaligned_le32(fw_ptr + sizeof(*cmd));
-
- bt_dev_dbg(hdev, "boot_param=0x%x", *boot_param);
- }
-
frag_len += sizeof(*cmd) + cmd->plen;
/* The parameter length of the secure send command requires
@@ -896,28 +890,92 @@ static int btintel_download_firmware_payload(struct hci_dev *hdev,
return err;
}
+static bool btintel_firmware_version(struct hci_dev *hdev,
+ u8 num, u8 ww, u8 yy,
+ const struct firmware *fw,
+ u32 *boot_addr)
+{
+ const u8 *fw_ptr;
+ u32 frag_len;
+
+ fw_ptr = fw->data;
+ frag_len = 0;
+
+ while (fw_ptr - fw->data < fw->size) {
+ struct hci_command_hdr *cmd = (void *)(fw_ptr + frag_len);
+
+ /* Each SKU has a different reset parameter to use in the
+ * HCI_Intel_Reset command and it is embedded in the firmware
+ * data. So, instead of using static value per SKU, check
+ * the firmware data and save it for later use.
+ */
+ if (le16_to_cpu(cmd->opcode) == CMD_WRITE_BOOT_PARAMS) {
+ struct cmd_write_boot_params *params;
+
+ params = (void *)(fw_ptr + sizeof(*cmd));
+
+ bt_dev_info(hdev, "Boot Address: 0x%x",
+ le32_to_cpu(params->boot_addr));
+
+ bt_dev_info(hdev, "Firmware Version: %u-%u.%u",
+ params->fw_build_num, params->fw_build_ww,
+ params->fw_build_yy);
+
+ return (num == params->fw_build_num &&
+ ww == params->fw_build_ww &&
+ yy == params->fw_build_yy);
+ }
+
+ frag_len += sizeof(*cmd) + cmd->plen;
+ }
+
+ return false;
+}
+
int btintel_download_firmware(struct hci_dev *hdev,
+ struct intel_version *ver,
const struct firmware *fw,
u32 *boot_param)
{
int err;
+ /* Skip download if firmware has the same version */
+ if (btintel_firmware_version(hdev, ver->fw_build_num, ver->fw_build_ww,
+ ver->fw_build_yy, fw, boot_param)) {
+ bt_dev_info(hdev, "Firmware already loaded");
+ /* Return -EALREADY to indicate that the firmware has already
+ * been loaded.
+ */
+ return -EALREADY;
+ }
+
err = btintel_sfi_rsa_header_secure_send(hdev, fw);
if (err)
return err;
- return btintel_download_firmware_payload(hdev, fw, boot_param,
- RSA_HEADER_LEN);
+ return btintel_download_firmware_payload(hdev, fw, RSA_HEADER_LEN);
}
EXPORT_SYMBOL_GPL(btintel_download_firmware);
int btintel_download_firmware_newgen(struct hci_dev *hdev,
+ struct intel_version_tlv *ver,
const struct firmware *fw, u32 *boot_param,
u8 hw_variant, u8 sbe_type)
{
int err;
u32 css_header_ver;
+ /* Skip download if firmware has the same version */
+ if (btintel_firmware_version(hdev, ver->min_fw_build_nn,
+ ver->min_fw_build_cw, ver->min_fw_build_yy,
+ fw, boot_param)) {
+ bt_dev_info(hdev, "Firmware already loaded");
+ /* Return -EALREADY to indicate that firmware has already been
+ * loaded.
+ */
+ return -EALREADY;
+ }
+
/* 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.
@@ -947,7 +1005,7 @@ int btintel_download_firmware_newgen(struct hci_dev *hdev,
if (err)
return err;
- err = btintel_download_firmware_payload(hdev, fw, boot_param, RSA_HEADER_LEN);
+ err = btintel_download_firmware_payload(hdev, fw, RSA_HEADER_LEN);
if (err)
return err;
} else if (hw_variant >= 0x17) {
@@ -968,7 +1026,6 @@ int btintel_download_firmware_newgen(struct hci_dev *hdev,
return err;
err = btintel_download_firmware_payload(hdev, fw,
- boot_param,
RSA_HEADER_LEN + ECDSA_HEADER_LEN);
if (err)
return err;
@@ -978,7 +1035,6 @@ int btintel_download_firmware_newgen(struct hci_dev *hdev,
return err;
err = btintel_download_firmware_payload(hdev, fw,
- boot_param,
RSA_HEADER_LEN + ECDSA_HEADER_LEN);
if (err)
return err;
diff --git a/drivers/bluetooth/btintel.h b/drivers/bluetooth/btintel.h
index 6511b091caf5..51f1f2c883b4 100644
--- a/drivers/bluetooth/btintel.h
+++ b/drivers/bluetooth/btintel.h
@@ -163,9 +163,10 @@ struct regmap *btintel_regmap_init(struct hci_dev *hdev, u16 opcode_read,
int btintel_send_intel_reset(struct hci_dev *hdev, u32 boot_param);
int btintel_read_boot_params(struct hci_dev *hdev,
struct intel_boot_params *params);
-int btintel_download_firmware(struct hci_dev *dev, const struct firmware *fw,
- u32 *boot_param);
+int btintel_download_firmware(struct hci_dev *dev, struct intel_version *ver,
+ const struct firmware *fw, u32 *boot_param);
int btintel_download_firmware_newgen(struct hci_dev *hdev,
+ struct intel_version_tlv *ver,
const struct firmware *fw,
u32 *boot_param, u8 hw_variant,
u8 sbe_type);
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index 66ada8217797..c92060e7472c 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -2623,10 +2623,17 @@ static int btusb_intel_download_firmware_newgen(struct hci_dev *hdev,
set_bit(BTUSB_DOWNLOADING, &data->flags);
/* Start firmware downloading and get boot parameter */
- err = btintel_download_firmware_newgen(hdev, fw, boot_param,
+ err = btintel_download_firmware_newgen(hdev, ver, fw, boot_param,
INTEL_HW_VARIANT(ver->cnvi_bt),
ver->sbe_type);
if (err < 0) {
+ if (err == -EALREADY) {
+ /* Firmware has already been loaded */
+ set_bit(BTUSB_FIRMWARE_LOADED, &data->flags);
+ err = 0;
+ goto done;
+ }
+
/* When FW download fails, send Intel Reset to retry
* FW download.
*/
@@ -2817,8 +2824,15 @@ static int btusb_intel_download_firmware(struct hci_dev *hdev,
set_bit(BTUSB_DOWNLOADING, &data->flags);
/* Start firmware downloading and get boot parameter */
- err = btintel_download_firmware(hdev, fw, boot_param);
+ err = btintel_download_firmware(hdev, ver, fw, boot_param);
if (err < 0) {
+ if (err == -EALREADY) {
+ /* Firmware has already been loaded */
+ set_bit(BTUSB_FIRMWARE_LOADED, &data->flags);
+ err = 0;
+ goto done;
+ }
+
/* When FW download fails, send Intel Reset to retry
* FW download.
*/
diff --git a/drivers/bluetooth/hci_intel.c b/drivers/bluetooth/hci_intel.c
index b20a40fab83e..7249b91d9b91 100644
--- a/drivers/bluetooth/hci_intel.c
+++ b/drivers/bluetooth/hci_intel.c
@@ -735,7 +735,7 @@ static int intel_setup(struct hci_uart *hu)
set_bit(STATE_DOWNLOADING, &intel->flags);
/* Start firmware downloading and get boot parameter */
- err = btintel_download_firmware(hdev, fw, &boot_param);
+ err = btintel_download_firmware(hdev, &ver, fw, &boot_param);
if (err < 0)
goto done;
@@ -784,7 +784,10 @@ static int intel_setup(struct hci_uart *hu)
done:
release_firmware(fw);
- if (err < 0)
+ /* Check if there was an error and if is not -EALREADY which means the
+ * firmware has already been loaded.
+ */
+ if (err < 0 && err != -EALREADY)
return err;
/* We need to restore the default speed before Intel reset */
--
2.26.2
From: Luiz Augusto von Dentz <[email protected]>
This moves duplicated code for waiting firmware download to complete to
a function that can then be reused.
Signed-off-by: Luiz Augusto von Dentz <[email protected]>
---
drivers/bluetooth/btusb.c | 148 +++++++++++++++++---------------------
1 file changed, 66 insertions(+), 82 deletions(-)
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index c4d30525dafe..6ab955c9b309 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -2810,6 +2810,68 @@ static int btusb_intel_download_firmware(struct hci_dev *hdev,
return err;
}
+static int btusb_boot_wait(struct hci_dev *hdev, ktime_t calltime, int msec)
+{
+ struct btusb_data *data = hci_get_drvdata(hdev);
+ ktime_t delta, rettime;
+ unsigned long long duration;
+ int err;
+
+ bt_dev_info(hdev, "Waiting for device to boot");
+
+ err = wait_on_bit_timeout(&data->flags, BTUSB_BOOTING,
+ TASK_INTERRUPTIBLE,
+ msecs_to_jiffies(msec));
+ if (err == -EINTR) {
+ bt_dev_err(hdev, "Device boot interrupted");
+ return -EINTR;
+ }
+
+ if (err) {
+ bt_dev_err(hdev, "Device boot timeout");
+ return -ETIMEDOUT;
+ }
+
+ rettime = ktime_get();
+ delta = ktime_sub(rettime, calltime);
+ duration = (unsigned long long) ktime_to_ns(delta) >> 10;
+
+ bt_dev_info(hdev, "Device booted in %llu usecs", duration);
+
+ return 0;
+}
+
+static int btusb_intel_boot(struct hci_dev *hdev, u32 boot_addr)
+{
+ struct btusb_data *data = hci_get_drvdata(hdev);
+ ktime_t calltime;
+ int err;
+
+ calltime = ktime_get();
+
+ set_bit(BTUSB_BOOTING, &data->flags);
+
+ err = btintel_send_intel_reset(hdev, boot_addr);
+ if (err) {
+ bt_dev_err(hdev, "Intel Soft Reset failed (%d)", err);
+ btintel_reset_to_bootloader(hdev);
+ return err;
+ }
+
+ /* The bootloader will not indicate when the device is ready. This
+ * is done by the operational firmware sending bootup notification.
+ *
+ * Booting into operational firmware should not take longer than
+ * 1 second. However if that happens, then just fail the setup
+ * since something went wrong.
+ */
+ err = btusb_boot_wait(hdev, calltime, 1000);
+ if (err == -ETIMEDOUT)
+ btintel_reset_to_bootloader(hdev);
+
+ return err;
+}
+
static int btusb_setup_intel_new(struct hci_dev *hdev)
{
struct btusb_data *data = hci_get_drvdata(hdev);
@@ -2817,8 +2879,6 @@ static int btusb_setup_intel_new(struct hci_dev *hdev)
struct intel_boot_params params;
u32 boot_param;
char ddcname[64];
- ktime_t calltime, delta, rettime;
- unsigned long long duration;
int err;
struct intel_debug_features features;
@@ -2853,46 +2913,9 @@ static int btusb_setup_intel_new(struct hci_dev *hdev)
if (ver.fw_variant == 0x23)
goto finish;
- calltime = ktime_get();
-
- set_bit(BTUSB_BOOTING, &data->flags);
-
- err = btintel_send_intel_reset(hdev, boot_param);
- if (err) {
- bt_dev_err(hdev, "Intel Soft Reset failed (%d)", err);
- btintel_reset_to_bootloader(hdev);
+ err = btusb_intel_boot(hdev, boot_param);
+ if (err)
return err;
- }
-
- /* The bootloader will not indicate when the device is ready. This
- * is done by the operational firmware sending bootup notification.
- *
- * Booting into operational firmware should not take longer than
- * 1 second. However if that happens, then just fail the setup
- * since something went wrong.
- */
- bt_dev_info(hdev, "Waiting for device to boot");
-
- err = wait_on_bit_timeout(&data->flags, BTUSB_BOOTING,
- TASK_INTERRUPTIBLE,
- msecs_to_jiffies(1000));
-
- if (err == -EINTR) {
- bt_dev_err(hdev, "Device boot interrupted");
- return -EINTR;
- }
-
- if (err) {
- bt_dev_err(hdev, "Device boot timeout");
- btintel_reset_to_bootloader(hdev);
- return -ETIMEDOUT;
- }
-
- rettime = ktime_get();
- delta = ktime_sub(rettime, calltime);
- duration = (unsigned long long) ktime_to_ns(delta) >> 10;
-
- bt_dev_info(hdev, "Device booted in %llu usecs", duration);
clear_bit(BTUSB_BOOTLOADER, &data->flags);
@@ -2956,8 +2979,6 @@ static int btusb_setup_intel_newgen(struct hci_dev *hdev)
struct btusb_data *data = hci_get_drvdata(hdev);
u32 boot_param;
char ddcname[64];
- ktime_t calltime, delta, rettime;
- unsigned long long duration;
int err;
struct intel_debug_features features;
struct intel_version_tlv version;
@@ -2993,46 +3014,9 @@ static int btusb_setup_intel_newgen(struct hci_dev *hdev)
if (version.img_type == 0x03)
goto finish;
- calltime = ktime_get();
-
- set_bit(BTUSB_BOOTING, &data->flags);
-
- err = btintel_send_intel_reset(hdev, boot_param);
- if (err) {
- bt_dev_err(hdev, "Intel Soft Reset failed (%d)", err);
- btintel_reset_to_bootloader(hdev);
+ err = btusb_intel_boot(hdev, boot_param);
+ if (err)
return err;
- }
-
- /* The bootloader will not indicate when the device is ready. This
- * is done by the operational firmware sending bootup notification.
- *
- * Booting into operational firmware should not take longer than
- * 1 second. However if that happens, then just fail the setup
- * since something went wrong.
- */
- bt_dev_info(hdev, "Waiting for device to boot");
-
- err = wait_on_bit_timeout(&data->flags, BTUSB_BOOTING,
- TASK_INTERRUPTIBLE,
- msecs_to_jiffies(1000));
-
- if (err == -EINTR) {
- bt_dev_err(hdev, "Device boot interrupted");
- return -EINTR;
- }
-
- if (err) {
- bt_dev_err(hdev, "Device boot timeout");
- btintel_reset_to_bootloader(hdev);
- return -ETIMEDOUT;
- }
-
- rettime = ktime_get();
- delta = ktime_sub(rettime, calltime);
- duration = (unsigned long long)ktime_to_ns(delta) >> 10;
-
- bt_dev_info(hdev, "Device booted in %llu usecs", duration);
clear_bit(BTUSB_BOOTLOADER, &data->flags);
--
2.26.2
From: Luiz Augusto von Dentz <[email protected]>
This moves checks version checks of intel_version to
btintel_version_info.
Signed-off-by: Luiz Augusto von Dentz <[email protected]>
---
drivers/bluetooth/btintel.c | 36 ++++++++++++++++++++++++++++++++++--
drivers/bluetooth/btintel.h | 2 +-
drivers/bluetooth/btusb.c | 12 ++++--------
3 files changed, 39 insertions(+), 11 deletions(-)
diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
index 82fba6b9328f..490a7d9d42eb 100644
--- a/drivers/bluetooth/btintel.c
+++ b/drivers/bluetooth/btintel.c
@@ -216,10 +216,39 @@ void btintel_hw_error(struct hci_dev *hdev, u8 code)
}
EXPORT_SYMBOL_GPL(btintel_hw_error);
-void btintel_version_info(struct hci_dev *hdev, struct intel_version *ver)
+int btintel_version_info(struct hci_dev *hdev, struct intel_version *ver)
{
const char *variant;
+ /* 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;
+ }
+
switch (ver->fw_variant) {
case 0x06:
variant = "Bootloader";
@@ -228,13 +257,16 @@ void btintel_version_info(struct hci_dev *hdev, struct intel_version *ver)
variant = "Firmware";
break;
default:
- return;
+ bt_dev_err(hdev, "Unsupported firmware variant(%02x)", ver->fw_variant);
+ return -EINVAL;
}
bt_dev_info(hdev, "%s revision %u.%u build %u week %u %u",
variant, ver->fw_revision >> 4, ver->fw_revision & 0x0f,
ver->fw_build_num, ver->fw_build_ww,
2000 + ver->fw_build_yy);
+
+ return 0;
}
EXPORT_SYMBOL_GPL(btintel_version_info);
diff --git a/drivers/bluetooth/btintel.h b/drivers/bluetooth/btintel.h
index 94a63e898826..7163170410a8 100644
--- a/drivers/bluetooth/btintel.h
+++ b/drivers/bluetooth/btintel.h
@@ -148,7 +148,7 @@ int btintel_set_diag(struct hci_dev *hdev, bool enable);
int btintel_set_diag_mfg(struct hci_dev *hdev, bool enable);
void btintel_hw_error(struct hci_dev *hdev, u8 code);
-void btintel_version_info(struct hci_dev *hdev, struct intel_version *ver);
+int btintel_version_info(struct hci_dev *hdev, struct intel_version *ver);
int btintel_version_info_tlv(struct hci_dev *hdev, struct intel_version_tlv *version);
int btintel_secure_send(struct hci_dev *hdev, u8 fragment_type, u32 plen,
const void *param);
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index 634406058ccf..df0b1f8c7ed7 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -2474,12 +2474,6 @@ static int btusb_setup_intel_new_get_fw_name(struct intel_version *ver,
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 */
@@ -2658,8 +2652,6 @@ static int btusb_intel_download_firmware(struct hci_dev *hdev,
if (!ver || !params)
return -EINVAL;
- btintel_version_info(hdev, ver);
-
/* 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
@@ -2847,6 +2839,10 @@ static int btusb_setup_intel_new(struct hci_dev *hdev)
return err;
}
+ err = btintel_version_info(hdev, &ver);
+ if (err)
+ return err;
+
err = btusb_intel_download_firmware(hdev, &ver, ¶ms, &boot_param);
if (err)
return err;
--
2.26.2
This is automated email and please do not reply to this email!
Dear submitter,
Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=431641
---Test result---
##############################
Test: CheckPatch - PASS
##############################
Test: CheckGitLint - PASS
##############################
Test: CheckBuildK - PASS
##############################
Test: CheckTestRunner: Setup - PASS
##############################
Test: CheckTestRunner: l2cap-tester - PASS
Total: 40, Passed: 34 (85.0%), Failed: 0, Not Run: 6
##############################
Test: CheckTestRunner: bnep-tester - PASS
Total: 1, Passed: 1 (100.0%), Failed: 0, Not Run: 0
##############################
Test: CheckTestRunner: mgmt-tester - PASS
Total: 416, Passed: 402 (96.6%), Failed: 0, Not Run: 14
##############################
Test: CheckTestRunner: rfcomm-tester - PASS
Total: 9, Passed: 9 (100.0%), Failed: 0, Not Run: 0
##############################
Test: CheckTestRunner: sco-tester - PASS
Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0
##############################
Test: CheckTestRunner: smp-tester - PASS
Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0
##############################
Test: CheckTestRunner: userchan-tester - PASS
Total: 3, Passed: 3 (100.0%), Failed: 0, Not Run: 0
---
Regards,
Linux Bluetooth
Hi Luiz,
On Wed, 2021-02-10 at 08:59 -0800, Luiz Augusto von Dentz wrote:
> From: Luiz Augusto von Dentz <[email protected]>
>
> This checks the firmware build number, week and year matches with
> repective version loaded and then skip the download process.
>
> Signed-off-by: Luiz Augusto von Dentz <[email protected]>
> ---
> v2: Add patch that mover checks for operational mode after the version
> checking.
> v3: Fix not checking for operation mode before using btintel_read_boot_params
> since some models depend on that to contruct the fw filename. Also attempt to
> cleanup duplicated code.
> v4: Fix forwarding -EALREADY when firmware has already been loaded.
>
> drivers/bluetooth/btintel.c | 96 +++++++++++++++++++++++++++--------
> drivers/bluetooth/btintel.h | 5 +-
> drivers/bluetooth/btusb.c | 18 ++++++-
> drivers/bluetooth/hci_intel.c | 7 ++-
> 4 files changed, 100 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
> index 88ce5f0ffc4b..89f85d54ca64 100644
> --- a/drivers/bluetooth/btintel.c
> +++ b/drivers/bluetooth/btintel.c
> @@ -24,6 +24,14 @@
> #define ECDSA_OFFSET 644
> #define ECDSA_HEADER_LEN 320
>
> +#define CMD_WRITE_BOOT_PARAMS 0xfc0e
> +struct cmd_write_boot_params {
> + u32 boot_addr;
> + u8 fw_build_num;
> + u8 fw_build_ww;
> + u8 fw_build_yy;
> +} __packed;
> +
> int btintel_check_bdaddr(struct hci_dev *hdev)
> {
> struct hci_rp_read_bd_addr *bda;
> @@ -841,7 +849,7 @@ static int btintel_sfi_ecdsa_header_secure_send(struct hci_dev *hdev,
>
> static int btintel_download_firmware_payload(struct hci_dev *hdev,
> const struct firmware *fw,
> - u32 *boot_param, size_t offset)
> + size_t offset)
> {
> int err;
> const u8 *fw_ptr;
> @@ -854,20 +862,6 @@ static int btintel_download_firmware_payload(struct hci_dev *hdev,
> while (fw_ptr - fw->data < fw->size) {
> struct hci_command_hdr *cmd = (void *)(fw_ptr + frag_len);
>
> - /* Each SKU has a different reset parameter to use in the
> - * HCI_Intel_Reset command and it is embedded in the firmware
> - * data. So, instead of using static value per SKU, check
> - * the firmware data and save it for later use.
> - */
> - if (le16_to_cpu(cmd->opcode) == 0xfc0e) {
> - /* The boot parameter is the first 32-bit value
> - * and rest of 3 octets are reserved.
> - */
> - *boot_param = get_unaligned_le32(fw_ptr + sizeof(*cmd));
> -
> - bt_dev_dbg(hdev, "boot_param=0x%x", *boot_param);
> - }
> -
> frag_len += sizeof(*cmd) + cmd->plen;
>
> /* The parameter length of the secure send command requires
> @@ -896,28 +890,92 @@ static int btintel_download_firmware_payload(struct hci_dev *hdev,
> return err;
> }
>
> +static bool btintel_firmware_version(struct hci_dev *hdev,
> + u8 num, u8 ww, u8 yy,
> + const struct firmware *fw,
> + u32 *boot_addr)
> +{
> + const u8 *fw_ptr;
> + u32 frag_len;
> +
> + fw_ptr = fw->data;
> + frag_len = 0;
> +
> + while (fw_ptr - fw->data < fw->size) {
Looping with all constant value may causes an buffer overflow if the file
doesn't have "CMD_WRITE_BOOT_PARAMS" for some reason.
> + struct hci_command_hdr *cmd = (void *)(fw_ptr + frag_len);
> +
> + /* Each SKU has a different reset parameter to use in the
> + * HCI_Intel_Reset command and it is embedded in the firmware
> + * data. So, instead of using static value per SKU, check
> + * the firmware data and save it for later use.
> + */
> + if (le16_to_cpu(cmd->opcode) == CMD_WRITE_BOOT_PARAMS) {
> + struct cmd_write_boot_params *params;
> +
> + params = (void *)(fw_ptr + sizeof(*cmd));
The params doesn't point the right value since the fw_ptr never updates in the loop.
This might cause reloading the firmware even if fw version is same since it alwasy return false.
> +
> + bt_dev_info(hdev, "Boot Address: 0x%x",
> + le32_to_cpu(params->boot_addr));
> +
> + bt_dev_info(hdev, "Firmware Version: %u-%u.%u",
> + params->fw_build_num, params->fw_build_ww,
> + params->fw_build_yy);
> +
> + return (num == params->fw_build_num &&
> + ww == params->fw_build_ww &&
> + yy == params->fw_build_yy);
> + }
> +
> + frag_len += sizeof(*cmd) + cmd->plen;
> + }
> +
> + return false;
> +}
> +
> int btintel_download_firmware(struct hci_dev *hdev,
> + struct intel_version *ver,
> const struct firmware *fw,
> u32 *boot_param)
> {
> int err;
>
> + /* Skip download if firmware has the same version */
> + if (btintel_firmware_version(hdev, ver->fw_build_num, ver->fw_build_ww,
> + ver->fw_build_yy, fw, boot_param)) {
> + bt_dev_info(hdev, "Firmware already loaded");
> + /* Return -EALREADY to indicate that the firmware has already
> + * been loaded.
> + */
> + return -EALREADY;
> + }
> +
> err = btintel_sfi_rsa_header_secure_send(hdev, fw);
> if (err)
> return err;
>
> - return btintel_download_firmware_payload(hdev, fw, boot_param,
> - RSA_HEADER_LEN);
> + return btintel_download_firmware_payload(hdev, fw, RSA_HEADER_LEN);
> }
> EXPORT_SYMBOL_GPL(btintel_download_firmware);
>
> int btintel_download_firmware_newgen(struct hci_dev *hdev,
> + struct intel_version_tlv *ver,
> const struct firmware *fw, u32 *boot_param,
> u8 hw_variant, u8 sbe_type)
> {
> int err;
> u32 css_header_ver;
>
> + /* Skip download if firmware has the same version */
> + if (btintel_firmware_version(hdev, ver->min_fw_build_nn,
> + ver->min_fw_build_cw, ver->min_fw_build_yy,
> + fw, boot_param)) {
> + bt_dev_info(hdev, "Firmware already loaded");
> + /* Return -EALREADY to indicate that firmware has already been
> + * loaded.
> + */
> + return -EALREADY;
> + }
> +
> /* 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.
> @@ -947,7 +1005,7 @@ int btintel_download_firmware_newgen(struct hci_dev *hdev,
> if (err)
> return err;
>
> - err = btintel_download_firmware_payload(hdev, fw, boot_param, RSA_HEADER_LEN);
> + err = btintel_download_firmware_payload(hdev, fw, RSA_HEADER_LEN);
> if (err)
> return err;
> } else if (hw_variant >= 0x17) {
> @@ -968,7 +1026,6 @@ int btintel_download_firmware_newgen(struct hci_dev *hdev,
> return err;
>
> err = btintel_download_firmware_payload(hdev, fw,
> - boot_param,
> RSA_HEADER_LEN + ECDSA_HEADER_LEN);
> if (err)
> return err;
> @@ -978,7 +1035,6 @@ int btintel_download_firmware_newgen(struct hci_dev *hdev,
> return err;
>
> err = btintel_download_firmware_payload(hdev, fw,
> - boot_param,
> RSA_HEADER_LEN + ECDSA_HEADER_LEN);
> if (err)
> return err;
> diff --git a/drivers/bluetooth/btintel.h b/drivers/bluetooth/btintel.h
> index 6511b091caf5..51f1f2c883b4 100644
> --- a/drivers/bluetooth/btintel.h
> +++ b/drivers/bluetooth/btintel.h
> @@ -163,9 +163,10 @@ struct regmap *btintel_regmap_init(struct hci_dev *hdev, u16 opcode_read,
> int btintel_send_intel_reset(struct hci_dev *hdev, u32 boot_param);
> int btintel_read_boot_params(struct hci_dev *hdev,
> struct intel_boot_params *params);
> -int btintel_download_firmware(struct hci_dev *dev, const struct firmware *fw,
> - u32 *boot_param);
> +int btintel_download_firmware(struct hci_dev *dev, struct intel_version *ver,
> + const struct firmware *fw, u32 *boot_param);
> int btintel_download_firmware_newgen(struct hci_dev *hdev,
> + struct intel_version_tlv *ver,
> const struct firmware *fw,
> u32 *boot_param, u8 hw_variant,
> u8 sbe_type);
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> index 66ada8217797..c92060e7472c 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -2623,10 +2623,17 @@ static int btusb_intel_download_firmware_newgen(struct hci_dev *hdev,
> set_bit(BTUSB_DOWNLOADING, &data->flags);
>
> /* Start firmware downloading and get boot parameter */
> - err = btintel_download_firmware_newgen(hdev, fw, boot_param,
> + err = btintel_download_firmware_newgen(hdev, ver, fw, boot_param,
> INTEL_HW_VARIANT(ver->cnvi_bt),
> ver->sbe_type);
> if (err < 0) {
> + if (err == -EALREADY) {
> + /* Firmware has already been loaded */
> + set_bit(BTUSB_FIRMWARE_LOADED, &data->flags);
> + err = 0;
> + goto done;
> + }
> +
> /* When FW download fails, send Intel Reset to retry
> * FW download.
> */
> @@ -2817,8 +2824,15 @@ static int btusb_intel_download_firmware(struct hci_dev *hdev,
> set_bit(BTUSB_DOWNLOADING, &data->flags);
>
> /* Start firmware downloading and get boot parameter */
> - err = btintel_download_firmware(hdev, fw, boot_param);
> + err = btintel_download_firmware(hdev, ver, fw, boot_param);
> if (err < 0) {
> + if (err == -EALREADY) {
> + /* Firmware has already been loaded */
> + set_bit(BTUSB_FIRMWARE_LOADED, &data->flags);
> + err = 0;
> + goto done;
> + }
> +
> /* When FW download fails, send Intel Reset to retry
> * FW download.
> */
> diff --git a/drivers/bluetooth/hci_intel.c b/drivers/bluetooth/hci_intel.c
> index b20a40fab83e..7249b91d9b91 100644
> --- a/drivers/bluetooth/hci_intel.c
> +++ b/drivers/bluetooth/hci_intel.c
> @@ -735,7 +735,7 @@ static int intel_setup(struct hci_uart *hu)
> set_bit(STATE_DOWNLOADING, &intel->flags);
>
> /* Start firmware downloading and get boot parameter */
> - err = btintel_download_firmware(hdev, fw, &boot_param);
> + err = btintel_download_firmware(hdev, &ver, fw, &boot_param);
> if (err < 0)
> goto done;
>
> @@ -784,7 +784,10 @@ static int intel_setup(struct hci_uart *hu)
> done:
> release_firmware(fw);
>
> - if (err < 0)
> + /* Check if there was an error and if is not -EALREADY which means the
> + * firmware has already been loaded.
> + */
> + if (err < 0 && err != -EALREADY)
> return err;
>
> /* We need to restore the default speed before Intel reset */
Hi Tedd,
On Wed, Feb 10, 2021 at 10:50 AM Tedd Ho-Jeong An
<[email protected]> wrote:
>
> Hi Luiz,
>
> On Wed, 2021-02-10 at 08:59 -0800, Luiz Augusto von Dentz wrote:
> > From: Luiz Augusto von Dentz <[email protected]>
> >
> > This checks the firmware build number, week and year matches with
> > repective version loaded and then skip the download process.
> >
> > Signed-off-by: Luiz Augusto von Dentz <[email protected]>
> > ---
> > v2: Add patch that mover checks for operational mode after the version
> > checking.
> > v3: Fix not checking for operation mode before using btintel_read_boot_params
> > since some models depend on that to contruct the fw filename. Also attempt to
> > cleanup duplicated code.
> > v4: Fix forwarding -EALREADY when firmware has already been loaded.
> >
> > drivers/bluetooth/btintel.c | 96 +++++++++++++++++++++++++++--------
> > drivers/bluetooth/btintel.h | 5 +-
> > drivers/bluetooth/btusb.c | 18 ++++++-
> > drivers/bluetooth/hci_intel.c | 7 ++-
> > 4 files changed, 100 insertions(+), 26 deletions(-)
> >
> > diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
> > index 88ce5f0ffc4b..89f85d54ca64 100644
> > --- a/drivers/bluetooth/btintel.c
> > +++ b/drivers/bluetooth/btintel.c
> > @@ -24,6 +24,14 @@
> > #define ECDSA_OFFSET 644
> > #define ECDSA_HEADER_LEN 320
> >
> > +#define CMD_WRITE_BOOT_PARAMS 0xfc0e
> > +struct cmd_write_boot_params {
> > + u32 boot_addr;
> > + u8 fw_build_num;
> > + u8 fw_build_ww;
> > + u8 fw_build_yy;
> > +} __packed;
> > +
> > int btintel_check_bdaddr(struct hci_dev *hdev)
> > {
> > struct hci_rp_read_bd_addr *bda;
> > @@ -841,7 +849,7 @@ static int btintel_sfi_ecdsa_header_secure_send(struct hci_dev *hdev,
> >
> > static int btintel_download_firmware_payload(struct hci_dev *hdev,
> > const struct firmware *fw,
> > - u32 *boot_param, size_t offset)
> > + size_t offset)
> > {
> > int err;
> > const u8 *fw_ptr;
> > @@ -854,20 +862,6 @@ static int btintel_download_firmware_payload(struct hci_dev *hdev,
> > while (fw_ptr - fw->data < fw->size) {
> > struct hci_command_hdr *cmd = (void *)(fw_ptr + frag_len);
> >
> > - /* Each SKU has a different reset parameter to use in the
> > - * HCI_Intel_Reset command and it is embedded in the firmware
> > - * data. So, instead of using static value per SKU, check
> > - * the firmware data and save it for later use.
> > - */
> > - if (le16_to_cpu(cmd->opcode) == 0xfc0e) {
> > - /* The boot parameter is the first 32-bit value
> > - * and rest of 3 octets are reserved.
> > - */
> > - *boot_param = get_unaligned_le32(fw_ptr + sizeof(*cmd));
> > -
> > - bt_dev_dbg(hdev, "boot_param=0x%x", *boot_param);
> > - }
> > -
> > frag_len += sizeof(*cmd) + cmd->plen;
> >
> > /* The parameter length of the secure send command requires
> > @@ -896,28 +890,92 @@ static int btintel_download_firmware_payload(struct hci_dev *hdev,
> > return err;
> > }
> >
> > +static bool btintel_firmware_version(struct hci_dev *hdev,
> > + u8 num, u8 ww, u8 yy,
> > + const struct firmware *fw,
> > + u32 *boot_addr)
> > +{
> > + const u8 *fw_ptr;
> > + u32 frag_len;
> > +
> > + fw_ptr = fw->data;
> > + frag_len = 0;
> > +
> > + while (fw_ptr - fw->data < fw->size) {
>
> Looping with all constant value may causes an buffer overflow if the file
> doesn't have "CMD_WRITE_BOOT_PARAMS" for some reason.
>
> > + struct hci_command_hdr *cmd = (void *)(fw_ptr + frag_len);
> > +
> > + /* Each SKU has a different reset parameter to use in the
> > + * HCI_Intel_Reset command and it is embedded in the firmware
> > + * data. So, instead of using static value per SKU, check
> > + * the firmware data and save it for later use.
> > + */
> > + if (le16_to_cpu(cmd->opcode) == CMD_WRITE_BOOT_PARAMS) {
> > + struct cmd_write_boot_params *params;
> > +
> > + params = (void *)(fw_ptr + sizeof(*cmd));
>
> The params doesn't point the right value since the fw_ptr never updates in the loop.
> This might cause reloading the firmware even if fw version is same since it alwasy return false.
Yep, this will never gonna work if we don't advance fw_ptr.
> > +
> > + bt_dev_info(hdev, "Boot Address: 0x%x",
> > + le32_to_cpu(params->boot_addr));
> > +
> > + bt_dev_info(hdev, "Firmware Version: %u-%u.%u",
> > + params->fw_build_num, params->fw_build_ww,
> > + params->fw_build_yy);
> > +
> > + return (num == params->fw_build_num &&
> > + ww == params->fw_build_ww &&
> > + yy == params->fw_build_yy);
> > + }
> > +
> > + frag_len += sizeof(*cmd) + cmd->plen;
> > + }
> > +
> > + return false;
> > +}
> > +
> > int btintel_download_firmware(struct hci_dev *hdev,
> > + struct intel_version *ver,
> > const struct firmware *fw,
> > u32 *boot_param)
> > {
> > int err;
> >
> > + /* Skip download if firmware has the same version */
> > + if (btintel_firmware_version(hdev, ver->fw_build_num, ver->fw_build_ww,
> > + ver->fw_build_yy, fw, boot_param)) {
> > + bt_dev_info(hdev, "Firmware already loaded");
> > + /* Return -EALREADY to indicate that the firmware has already
> > + * been loaded.
> > + */
> > + return -EALREADY;
> > + }
> > +
> > err = btintel_sfi_rsa_header_secure_send(hdev, fw);
> > if (err)
> > return err;
> >
> > - return btintel_download_firmware_payload(hdev, fw, boot_param,
> > - RSA_HEADER_LEN);
> > + return btintel_download_firmware_payload(hdev, fw, RSA_HEADER_LEN);
> > }
> > EXPORT_SYMBOL_GPL(btintel_download_firmware);
> >
> > int btintel_download_firmware_newgen(struct hci_dev *hdev,
> > + struct intel_version_tlv *ver,
> > const struct firmware *fw, u32 *boot_param,
> > u8 hw_variant, u8 sbe_type)
> > {
> > int err;
> > u32 css_header_ver;
> >
> > + /* Skip download if firmware has the same version */
> > + if (btintel_firmware_version(hdev, ver->min_fw_build_nn,
> > + ver->min_fw_build_cw, ver->min_fw_build_yy,
> > + fw, boot_param)) {
> > + bt_dev_info(hdev, "Firmware already loaded");
> > + /* Return -EALREADY to indicate that firmware has already been
> > + * loaded.
> > + */
> > + return -EALREADY;
> > + }
> > +
> > /* 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.
> > @@ -947,7 +1005,7 @@ int btintel_download_firmware_newgen(struct hci_dev *hdev,
> > if (err)
> > return err;
> >
> > - err = btintel_download_firmware_payload(hdev, fw, boot_param, RSA_HEADER_LEN);
> > + err = btintel_download_firmware_payload(hdev, fw, RSA_HEADER_LEN);
> > if (err)
> > return err;
> > } else if (hw_variant >= 0x17) {
> > @@ -968,7 +1026,6 @@ int btintel_download_firmware_newgen(struct hci_dev *hdev,
> > return err;
> >
> > err = btintel_download_firmware_payload(hdev, fw,
> > - boot_param,
> > RSA_HEADER_LEN + ECDSA_HEADER_LEN);
> > if (err)
> > return err;
> > @@ -978,7 +1035,6 @@ int btintel_download_firmware_newgen(struct hci_dev *hdev,
> > return err;
> >
> > err = btintel_download_firmware_payload(hdev, fw,
> > - boot_param,
> > RSA_HEADER_LEN + ECDSA_HEADER_LEN);
> > if (err)
> > return err;
> > diff --git a/drivers/bluetooth/btintel.h b/drivers/bluetooth/btintel.h
> > index 6511b091caf5..51f1f2c883b4 100644
> > --- a/drivers/bluetooth/btintel.h
> > +++ b/drivers/bluetooth/btintel.h
> > @@ -163,9 +163,10 @@ struct regmap *btintel_regmap_init(struct hci_dev *hdev, u16 opcode_read,
> > int btintel_send_intel_reset(struct hci_dev *hdev, u32 boot_param);
> > int btintel_read_boot_params(struct hci_dev *hdev,
> > struct intel_boot_params *params);
> > -int btintel_download_firmware(struct hci_dev *dev, const struct firmware *fw,
> > - u32 *boot_param);
> > +int btintel_download_firmware(struct hci_dev *dev, struct intel_version *ver,
> > + const struct firmware *fw, u32 *boot_param);
> > int btintel_download_firmware_newgen(struct hci_dev *hdev,
> > + struct intel_version_tlv *ver,
> > const struct firmware *fw,
> > u32 *boot_param, u8 hw_variant,
> > u8 sbe_type);
> > diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> > index 66ada8217797..c92060e7472c 100644
> > --- a/drivers/bluetooth/btusb.c
> > +++ b/drivers/bluetooth/btusb.c
> > @@ -2623,10 +2623,17 @@ static int btusb_intel_download_firmware_newgen(struct hci_dev *hdev,
> > set_bit(BTUSB_DOWNLOADING, &data->flags);
> >
> > /* Start firmware downloading and get boot parameter */
> > - err = btintel_download_firmware_newgen(hdev, fw, boot_param,
> > + err = btintel_download_firmware_newgen(hdev, ver, fw, boot_param,
> > INTEL_HW_VARIANT(ver->cnvi_bt),
> > ver->sbe_type);
> > if (err < 0) {
> > + if (err == -EALREADY) {
> > + /* Firmware has already been loaded */
> > + set_bit(BTUSB_FIRMWARE_LOADED, &data->flags);
> > + err = 0;
> > + goto done;
> > + }
> > +
> > /* When FW download fails, send Intel Reset to retry
> > * FW download.
> > */
> > @@ -2817,8 +2824,15 @@ static int btusb_intel_download_firmware(struct hci_dev *hdev,
> > set_bit(BTUSB_DOWNLOADING, &data->flags);
> >
> > /* Start firmware downloading and get boot parameter */
> > - err = btintel_download_firmware(hdev, fw, boot_param);
> > + err = btintel_download_firmware(hdev, ver, fw, boot_param);
> > if (err < 0) {
> > + if (err == -EALREADY) {
> > + /* Firmware has already been loaded */
> > + set_bit(BTUSB_FIRMWARE_LOADED, &data->flags);
> > + err = 0;
> > + goto done;
> > + }
> > +
> > /* When FW download fails, send Intel Reset to retry
> > * FW download.
> > */
> > diff --git a/drivers/bluetooth/hci_intel.c b/drivers/bluetooth/hci_intel.c
> > index b20a40fab83e..7249b91d9b91 100644
> > --- a/drivers/bluetooth/hci_intel.c
> > +++ b/drivers/bluetooth/hci_intel.c
> > @@ -735,7 +735,7 @@ static int intel_setup(struct hci_uart *hu)
> > set_bit(STATE_DOWNLOADING, &intel->flags);
> >
> > /* Start firmware downloading and get boot parameter */
> > - err = btintel_download_firmware(hdev, fw, &boot_param);
> > + err = btintel_download_firmware(hdev, &ver, fw, &boot_param);
> > if (err < 0)
> > goto done;
> >
> > @@ -784,7 +784,10 @@ static int intel_setup(struct hci_uart *hu)
> > done:
> > release_firmware(fw);
> >
> > - if (err < 0)
> > + /* Check if there was an error and if is not -EALREADY which means the
> > + * firmware has already been loaded.
> > + */
> > + if (err < 0 && err != -EALREADY)
> > return err;
> >
> > /* We need to restore the default speed before Intel reset */
>
--
Luiz Augusto von Dentz