diff mbox series

[v1] Bluetooth: btintel: Fix boot address

Message ID 20210820115808.15895-1-kiran.k@intel.com
State New
Headers show
Series [v1] Bluetooth: btintel: Fix boot address | expand

Commit Message

K, Kiran Aug. 20, 2021, 11:58 a.m. UTC
Cache Boot address present in firmware file which
is later used Intel_Soft_Reset command to bring
controller from boot mode to operational mode.

Signed-off-by: Kiran K <kiran.k@intel.com>
---
 drivers/bluetooth/btintel.c | 31 +++++++++++++------------------
 1 file changed, 13 insertions(+), 18 deletions(-)

Comments

bluez.test.bot@gmail.com Aug. 20, 2021, 1:22 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=534859

---Test result---

Test Summary:
CheckPatch                    PASS      0.51 seconds
GitLint                       PASS      0.09 seconds
BuildKernel                   PASS      487.38 seconds
TestRunner: Setup             PASS      324.05 seconds
TestRunner: l2cap-tester      PASS      2.47 seconds
TestRunner: bnep-tester       PASS      1.78 seconds
TestRunner: mgmt-tester       PASS      29.13 seconds
TestRunner: rfcomm-tester     PASS      1.99 seconds
TestRunner: sco-tester        PASS      1.98 seconds
TestRunner: smp-tester        FAIL      2.02 seconds
TestRunner: userchan-tester   PASS      1.93 seconds

Details
##############################
Test: CheckPatch - PASS - 0.51 seconds
Run checkpatch.pl script with rule in .checkpatch.conf


##############################
Test: GitLint - PASS - 0.09 seconds
Run gitlint with rule in .gitlint


##############################
Test: BuildKernel - PASS - 487.38 seconds
Build Kernel with minimal configuration supports Bluetooth


##############################
Test: TestRunner: Setup - PASS - 324.05 seconds
Setup environment for running Test Runner


##############################
Test: TestRunner: l2cap-tester - PASS - 2.47 seconds
Run test-runner with l2cap-tester
Total: 40, Passed: 40 (100.0%), Failed: 0, Not Run: 0

##############################
Test: TestRunner: bnep-tester - PASS - 1.78 seconds
Run test-runner with bnep-tester
Total: 1, Passed: 1 (100.0%), Failed: 0, Not Run: 0

##############################
Test: TestRunner: mgmt-tester - PASS - 29.13 seconds
Run test-runner with mgmt-tester
Total: 448, Passed: 445 (99.3%), Failed: 0, Not Run: 3

##############################
Test: TestRunner: rfcomm-tester - PASS - 1.99 seconds
Run test-runner with rfcomm-tester
Total: 9, Passed: 9 (100.0%), Failed: 0, Not Run: 0

##############################
Test: TestRunner: sco-tester - PASS - 1.98 seconds
Run test-runner with sco-tester
Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0

##############################
Test: TestRunner: smp-tester - FAIL - 2.02 seconds
Run test-runner with smp-tester
Total: 8, Passed: 7 (87.5%), Failed: 1, Not Run: 0

Failed Test Cases
SMP Client - SC Request 2                            Failed       0.019 seconds

##############################
Test: TestRunner: userchan-tester - PASS - 1.93 seconds
Run test-runner with userchan-tester
Total: 3, Passed: 3 (100.0%), Failed: 0, Not Run: 0



---
Regards,
Linux Bluetooth
Marcel Holtmann Aug. 30, 2021, 3:07 p.m. UTC | #2
Hi Kiran,

> Cache Boot address present in firmware file which

> is later used Intel_Soft_Reset command to bring

> controller from boot mode to operational mode.

> 

> Signed-off-by: Kiran K <kiran.k@intel.com>

> ---

> drivers/bluetooth/btintel.c | 31 +++++++++++++------------------

> 1 file changed, 13 insertions(+), 18 deletions(-)

> 

> diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c

> index f1705b46fc88..80d6dd7ae51a 100644

> --- a/drivers/bluetooth/btintel.c

> +++ b/drivers/bluetooth/btintel.c

> @@ -1037,8 +1037,9 @@ static bool btintel_firmware_version(struct hci_dev *hdev,

> 

> 			params = (void *)(fw_ptr + sizeof(*cmd));

> 

> -			bt_dev_info(hdev, "Boot Address: 0x%x",

> -				    le32_to_cpu(params->boot_addr));

> +			*boot_addr = le32_to_cpu(params->boot_addr);

> +

> +			bt_dev_info(hdev, "Boot Address: 0x%x", *boot_addr);

> 


so this hunk looks good and is described in the commit message.

> 			bt_dev_info(hdev, "Firmware Version: %u-%u.%u",

> 				    params->fw_build_num, params->fw_build_ww,

> @@ -1071,9 +1072,6 @@ int btintel_download_firmware(struct hci_dev *hdev,

> 		/* Skip version checking */

> 		break;

> 	default:

> -		/* Skip reading firmware file version in bootloader mode */

> -		if (ver->fw_variant == 0x06)

> -			break;

> 

> 		/* Skip download if firmware has the same version */

> 		if (btintel_firmware_version(hdev, ver->fw_build_num,

> @@ -1114,19 +1112,16 @@ static int btintel_download_fw_tlv(struct hci_dev *hdev,

> 	int err;

> 	u32 css_header_ver;

> 

> -	/* Skip reading firmware file version in bootloader mode */

> -	if (ver->img_type != 0x01) {

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

> -		}

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

> 	}


This part however isn’t.

Regards

Marcel
K, Kiran Aug. 31, 2021, 12:57 p.m. UTC | #3
HI Marcel,

> -----Original Message-----

> From: Marcel Holtmann <marcel@holtmann.org>

> Sent: Monday, August 30, 2021 8:38 PM

> To: K, Kiran <kiran.k@intel.com>

> Cc: open list:BLUETOOTH SUBSYSTEM <linux-bluetooth@vger.kernel.org>;

> Srivatsa, Ravishankar <ravishankar.srivatsa@intel.com>; Tumkur Narayan,

> Chethan <chethan.tumkur.narayan@intel.com>; An, Tedd

> <tedd.an@intel.com>; Von Dentz, Luiz <luiz.von.dentz@intel.com>

> Subject: Re: [PATCH v1] Bluetooth: btintel: Fix boot address

> 

> Hi Kiran,

> 

> > Cache Boot address present in firmware file which is later used

> > Intel_Soft_Reset command to bring controller from boot mode to

> > operational mode.

> >

> > Signed-off-by: Kiran K <kiran.k@intel.com>

> > ---

> > drivers/bluetooth/btintel.c | 31 +++++++++++++------------------

> > 1 file changed, 13 insertions(+), 18 deletions(-)

> >

> > diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c

> > index f1705b46fc88..80d6dd7ae51a 100644

> > --- a/drivers/bluetooth/btintel.c

> > +++ b/drivers/bluetooth/btintel.c

> > @@ -1037,8 +1037,9 @@ static bool btintel_firmware_version(struct

> > hci_dev *hdev,

> >

> > 			params = (void *)(fw_ptr + sizeof(*cmd));

> >

> > -			bt_dev_info(hdev, "Boot Address: 0x%x",

> > -				    le32_to_cpu(params->boot_addr));

> > +			*boot_addr = le32_to_cpu(params->boot_addr);

> > +

> > +			bt_dev_info(hdev, "Boot Address: 0x%x",

> *boot_addr);

> >

> 

> so this hunk looks good and is described in the commit message.

> 

> > 			bt_dev_info(hdev, "Firmware Version: %u-%u.%u",

> > 				    params->fw_build_num, params-

> >fw_build_ww, @@ -1071,9 +1072,6

> > @@ int btintel_download_firmware(struct hci_dev *hdev,

> > 		/* Skip version checking */

> > 		break;

> > 	default:

> > -		/* Skip reading firmware file version in bootloader mode */

> > -		if (ver->fw_variant == 0x06)

> > -			break;

> >

> > 		/* Skip download if firmware has the same version */

> > 		if (btintel_firmware_version(hdev, ver->fw_build_num, @@

> -1114,19

> > +1112,16 @@ static int btintel_download_fw_tlv(struct hci_dev *hdev,

> > 	int err;

> > 	u32 css_header_ver;

> >

> > -	/* Skip reading firmware file version in bootloader mode */

> > -	if (ver->img_type != 0x01) {

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

> > -		}

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

> > 	}

> 

> This part however isn’t.


Ack. I have separated the patches with proper commit message and sent an updated version.
> 

> Regards

> 

> Marcel


Thanks,
Kiran
diff mbox series

Patch

diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
index f1705b46fc88..80d6dd7ae51a 100644
--- a/drivers/bluetooth/btintel.c
+++ b/drivers/bluetooth/btintel.c
@@ -1037,8 +1037,9 @@  static bool btintel_firmware_version(struct hci_dev *hdev,
 
 			params = (void *)(fw_ptr + sizeof(*cmd));
 
-			bt_dev_info(hdev, "Boot Address: 0x%x",
-				    le32_to_cpu(params->boot_addr));
+			*boot_addr = le32_to_cpu(params->boot_addr);
+
+			bt_dev_info(hdev, "Boot Address: 0x%x", *boot_addr);
 
 			bt_dev_info(hdev, "Firmware Version: %u-%u.%u",
 				    params->fw_build_num, params->fw_build_ww,
@@ -1071,9 +1072,6 @@  int btintel_download_firmware(struct hci_dev *hdev,
 		/* Skip version checking */
 		break;
 	default:
-		/* Skip reading firmware file version in bootloader mode */
-		if (ver->fw_variant == 0x06)
-			break;
 
 		/* Skip download if firmware has the same version */
 		if (btintel_firmware_version(hdev, ver->fw_build_num,
@@ -1114,19 +1112,16 @@  static int btintel_download_fw_tlv(struct hci_dev *hdev,
 	int err;
 	u32 css_header_ver;
 
-	/* Skip reading firmware file version in bootloader mode */
-	if (ver->img_type != 0x01) {
-		/* 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;
-		}
+	/* 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;
 	}
 
 	/* The firmware variant determines if the device is in bootloader