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 2/5] Bluetooth: btintel: Refactor firmware header download
 sequence
From:   Marcel Holtmann <marcel@holtmann.org>
In-Reply-To: <20200702120315.26768-3-kiran.k@intel.com>
Date:   Thu, 2 Jul 2020 15:25:57 +0200
Cc:     linux-bluetooth <linux-bluetooth@vger.kernel.org>,
        "Srivatsa, Ravishankar" <ravishankar.srivatsa@intel.com>,
        Chethan T N <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: <8040CBA1-AF50-43A1-8314-4E44E4F27173@holtmann.org>
References: <20200702120315.26768-1-kiran.k@intel.com>
 <20200702120315.26768-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 header 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>
> ---
> drivers/bluetooth/btintel.c | 21 +++++++++++++++++----
> 1 file changed, 17 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
> index dea96c585ecb..1c820c187421 100644
> --- a/drivers/bluetooth/btintel.c
> +++ b/drivers/bluetooth/btintel.c
> @@ -646,12 +646,10 @@ int btintel_read_boot_params(struct hci_dev *hdev,
> }
> EXPORT_SYMBOL_GPL(btintel_read_boot_params);
> 
> -int btintel_download_firmware(struct hci_dev *hdev, const struct firmware *fw,
> -			      u32 *boot_param)
> +static int btintel_sfi_rsa_header_secure_send(struct hci_dev *hdev,
> +					      const struct firmware *fw)
> {
> 	int err;
> -	const u8 *fw_ptr;
> -	u32 frag_len;
> 
> 	/* Start the firmware download transaction with the Init fragment
> 	 * represented by the 128 bytes of CSS header.
> @@ -679,6 +677,21 @@ int btintel_download_firmware(struct hci_dev *hdev, const struct firmware *fw,
> 		bt_dev_err(hdev, "Failed to send firmware signature (%d)", err);
> 		goto done;
> 	}

Here should be an extra empty line before the label.

> +done:
> +	return err;
> +}
> +
> +int btintel_download_firmware(struct hci_dev *hdev, const struct firmware *fw,
> +			      u32 *boot_param)
> +{
> +	int err;
> +	const u8 *fw_ptr;
> +	u32 frag_len;
> +
> +	err = btintel_sfi_rsa_header_secure_send(hdev, fw);
> +	if (err)
> +		goto done;
> +

Scrap the extra empty line here.

> 
> 	fw_ptr = fw->data + 644;
> 	frag_len = 0;

Regards

Marcel