diff mbox series

[v4,1/6] Bluetooth: btintel: Check firmware version before download

Message ID 20210210165916.2148856-1-luiz.dentz@gmail.com (mailing list archive)
State New, archived
Headers show
Series [v4,1/6] Bluetooth: btintel: Check firmware version before download | expand

Commit Message

Luiz Augusto von Dentz Feb. 10, 2021, 4:59 p.m. UTC
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

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 <luiz.von.dentz@intel.com>
---
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(-)

Comments

bluez.test.bot@gmail.com Feb. 10, 2021, 5:33 p.m. UTC | #1
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
Tedd Ho-Jeong An Feb. 10, 2021, 6:50 p.m. UTC | #2
Hi Luiz,

On Wed, 2021-02-10 at 08:59 -0800, Luiz Augusto von Dentz wrote:
> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> 
> 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 <luiz.von.dentz@intel.com>
> ---
> 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 */
Luiz Augusto von Dentz Feb. 10, 2021, 7:06 p.m. UTC | #3
Hi Tedd,

On Wed, Feb 10, 2021 at 10:50 AM Tedd Ho-Jeong An
<tedd.an@linux.intel.com> wrote:
>
> Hi Luiz,
>
> On Wed, 2021-02-10 at 08:59 -0800, Luiz Augusto von Dentz wrote:
> > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> >
> > 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 <luiz.von.dentz@intel.com>
> > ---
> > 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 */
>
diff mbox series

Patch

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 */