Received-SPF: pass (google.com: domain of linux-bluetooth-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Content-Type: text/plain;
        charset=us-ascii
Mime-Version: 1.0 (Mac OS X Mail 13.4 \(3608.80.23.2.2\))
Subject: Re: [PATCH v2 3/5] Bluetooth: btintel: Refactor firmware payload
 download code
From:   Marcel Holtmann <marcel@holtmann.org>
In-Reply-To: <20200703071212.17046-3-kiran.k@intel.com>
Date:   Mon, 13 Jul 2020 21:16:10 +0200
Cc:     Bluetooth Kernel Mailing List <linux-bluetooth@vger.kernel.org>,
        ravishankar.srivatsa@intel.com, chethan.tumkur.narayan@intel.com,
        kiraank@gmail.com, Amit K Bag <amit.k.bag@intel.com>,
        Raghuram Hegde <raghuram.hegde@intel.com>
Content-Transfer-Encoding: 8BIT
Message-Id: <87AEDF27-8DE1-4A95-A669-F33BEED91880@holtmann.org>
References: <20200703071212.17046-1-kiran.k@intel.com>
 <20200703071212.17046-3-kiran.k@intel.com>
To:     Kiran K <kiran.k@intel.com>
Sender: linux-bluetooth-owner@vger.kernel.org
Precedence: bulk

Hi Kiran,

> Move firmware payload download code to a separate function to
> enhance readability and reusability
> 
> Signed-off-by: Kiran K <kiran.k@intel.com>
> Signed-off-by: Amit K Bag <amit.k.bag@intel.com>
> Signed-off-by: Raghuram Hegde <raghuram.hegde@intel.com>
> Reviewed-by: Chethan T N <chethan.tumkur.narayan@intel.com>
> Reviewed-by: Sathish Narasimman <Sathish.Narasimman@intel.com>
> Reviewed-by: Srivatsa Ravishankar <ravishankar.srivatsa@intel.com>
> ---
> 
> 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