2020-07-03 07:12:04

by Kiran K

Subject: [PATCH v2 3/5] Bluetooth: btintel: Refactor firmware payload download code

Move firmware payload download code to a separate function to
enhance readability and reusability

Signed-off-by: Kiran K <[email protected]>
Signed-off-by: Amit K Bag <[email protected]>
Signed-off-by: Raghuram Hegde <[email protected]>
Reviewed-by: Chethan T N <[email protected]>
Reviewed-by: Sathish Narasimman <[email protected]>
Reviewed-by: Srivatsa Ravishankar <[email protected]>
---

Changes in v2:
- Avoid exporting new public function for firmware download
- Move firmware payload download code to a new function
Changes in v1:
- Move firmware payload download code to a new function


drivers/bluetooth/btintel.c | 22 +++++++++++++++++++---
1 file changed, 19 insertions(+), 3 deletions(-)

diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
index ffc888c4e9e3..2cb55a97598c 100644
--- a/drivers/bluetooth/btintel.c
+++ b/drivers/bluetooth/btintel.c
@@ -19,6 +19,7 @@
#define VERSION "0.1"

#define BDADDR_INTEL (&(bdaddr_t) {{0x00, 0x8b, 0x9e, 0x19, 0x03, 0x00}})
+#define RSA_HEADER_LEN 644

int btintel_check_bdaddr(struct hci_dev *hdev)
{
@@ -682,8 +683,9 @@ static int btintel_sfi_rsa_header_secure_send(struct hci_dev *hdev,
return err;
}

-int btintel_download_firmware(struct hci_dev *hdev, const struct firmware *fw,
- u32 *boot_param)
+static int btintel_download_firmware_payload(struct hci_dev *hdev,
+ const struct firmware *fw,
+ u32 *boot_param, size_t offset)
{
int err;
const u8 *fw_ptr;
@@ -693,8 +695,9 @@ int btintel_download_firmware(struct hci_dev *hdev, const struct firmware *fw,
if (err)
goto done;

- fw_ptr = fw->data + 644;
+ fw_ptr = fw->data + offset;
frag_len = 0;
+ err = -EINVAL;

while (fw_ptr - fw->data < fw->size) {
struct hci_command_hdr *cmd = (void *)(fw_ptr + frag_len);
@@ -740,6 +743,19 @@ int btintel_download_firmware(struct hci_dev *hdev, const struct firmware *fw,
done:
return err;
}
+
+int btintel_download_firmware(struct hci_dev *hdev,
+ const struct firmware *fw,
+ u32 *boot_param)
+{
+ int err;
+
+ 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);
+}
EXPORT_SYMBOL_GPL(btintel_download_firmware);

void btintel_reset_to_bootloader(struct hci_dev *hdev)
--
2.17.1

2020-07-13 19:19:09

by Marcel Holtmann

Subject: Re: [PATCH v2 3/5] Bluetooth: btintel: Refactor firmware payload download code

Hi Kiran,

> Move firmware payload download code to a separate function to
> enhance readability and reusability
>
> Signed-off-by: Kiran K <[email protected]>
> Signed-off-by: Amit K Bag <[email protected]>
> Signed-off-by: Raghuram Hegde <[email protected]>
> Reviewed-by: Chethan T N <[email protected]>
> Reviewed-by: Sathish Narasimman <[email protected]>
> Reviewed-by: Srivatsa Ravishankar <[email protected]>
> ---
>
> Changes in v2:
> - Avoid exporting new public function for firmware download
> - Move firmware payload download code to a new function
> Changes in v1:
> - Move firmware payload download code to a new function
>
>
> drivers/bluetooth/btintel.c | 22 +++++++++++++++++++---
> 1 file changed, 19 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
> index ffc888c4e9e3..2cb55a97598c 100644
> --- a/drivers/bluetooth/btintel.c
> +++ b/drivers/bluetooth/btintel.c
> @@ -19,6 +19,7 @@
> #define VERSION "0.1"
>
> #define BDADDR_INTEL (&(bdaddr_t) {{0x00, 0x8b, 0x9e, 0x19, 0x03, 0x00}})
> +#define RSA_HEADER_LEN 644
>
> int btintel_check_bdaddr(struct hci_dev *hdev)
> {
> @@ -682,8 +683,9 @@ static int btintel_sfi_rsa_header_secure_send(struct hci_dev *hdev,
> return err;
> }
>
> -int btintel_download_firmware(struct hci_dev *hdev, const struct firmware *fw,
> - u32 *boot_param)
> +static int btintel_download_firmware_payload(struct hci_dev *hdev,
> + const struct firmware *fw,
> + u32 *boot_param, size_t offset)
> {
> int err;
> const u8 *fw_ptr;
> @@ -693,8 +695,9 @@ int btintel_download_firmware(struct hci_dev *hdev, const struct firmware *fw,
> if (err)
> goto done;
>
> - fw_ptr = fw->data + 644;
> + fw_ptr = fw->data + offset;
> frag_len = 0;
> + err = -EINVAL;
>
> while (fw_ptr - fw->data < fw->size) {
> struct hci_command_hdr *cmd = (void *)(fw_ptr + frag_len);
> @@ -740,6 +743,19 @@ int btintel_download_firmware(struct hci_dev *hdev, const struct firmware *fw,
> done:
> return err;
> }
> +
> +int btintel_download_firmware(struct hci_dev *hdev,
> + const struct firmware *fw,
> + u32 *boot_param)
> +{
> + int err;
> +
> + 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);
> +}
> EXPORT_SYMBOL_GPL(btintel_download_firmware);
>
> void btintel_reset_to_bootloader(struct hci_dev *hdev)

can we apply patch 1 and 2 independently from general TLV support? If so, then I would just try to get these two applied right away.

Regards

Marcel