diff mbox series

[v2] Bluetooth:btintel: Do no pass vendor events to stack

Message ID 20241017102236.729685-1-kiran.k@intel.com (mailing list archive)
State Accepted
Commit 4d67eb2b5444e1c872db4f04e7921ae8a95251fe
Headers show
Series [v2] Bluetooth:btintel: Do no pass vendor events to stack | expand

Checks

Context Check Description
tedd_an/pre-ci_am success Success
tedd_an/CheckPatch success CheckPatch PASS
tedd_an/GitLint success Gitlint PASS
tedd_an/SubjectPrefix fail "Bluetooth: " prefix is not specified in the subject
tedd_an/BuildKernel success BuildKernel PASS
tedd_an/CheckAllWarning success CheckAllWarning PASS
tedd_an/CheckSparse success CheckSparse PASS

Commit Message

K, Kiran Oct. 17, 2024, 10:22 a.m. UTC
During firmware download, vendor specific events like boot up and
secure send result are generated. These events can be safely processed at
the driver level. Passing on these events to stack prints unnecessary
below warning log.

--
Bluetooth: hci0: Malformed MSFT vendor event: 0x02
--
Fixes: 3368aa357f3b ("Bluetooth: msft: Handle MSFT Monitor Device Event")
Signed-off-by: Kiran K <kiran.k@intel.com>
---
 drivers/bluetooth/btintel.c      | 6 ++++--
 drivers/bluetooth/btintel_pcie.c | 9 ++++++---
 2 files changed, 10 insertions(+), 5 deletions(-)

Comments

Paul Menzel Oct. 17, 2024, 11:09 a.m. UTC | #1
Dear Kiran,


Thank you for your patch. Please note, in the summary/title a space is 
missing before *btintel*.

Am 17.10.24 um 12:22 schrieb Kiran K:
> During firmware download, vendor specific events like boot up and
> secure send result are generated. These events can be safely processed at
> the driver level. Passing on these events to stack prints unnecessary
> below warning log.
> 
> --
> Bluetooth: hci0: Malformed MSFT vendor event: 0x02
> --

I’d remove the `--` and just indent the message by four characters to 
follow Markdown markup style.

On what device did you test this? Are there open reports about it?

> Fixes: 3368aa357f3b ("Bluetooth: msft: Handle MSFT Monitor Device Event")
> Signed-off-by: Kiran K <kiran.k@intel.com>
> ---
>   drivers/bluetooth/btintel.c      | 6 ++++--
>   drivers/bluetooth/btintel_pcie.c | 9 ++++++---
>   2 files changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
> index 2be6d48a2a65..e122dff855ba 100644
> --- a/drivers/bluetooth/btintel.c
> +++ b/drivers/bluetooth/btintel.c
> @@ -3395,7 +3395,8 @@ int btintel_recv_event(struct hci_dev *hdev, struct sk_buff *skb)
>   				 * indicating that the bootup completed.
>   				 */
>   				btintel_bootup(hdev, ptr, len);
> -				break;
> +				kfree_skb(skb);
> +				return 0;
>   			case 0x06:
>   				/* When the firmware loading completes the
>   				 * device sends out a vendor specific event
> @@ -3403,7 +3404,8 @@ int btintel_recv_event(struct hci_dev *hdev, struct sk_buff *skb)
>   				 * loading.
>   				 */
>   				btintel_secure_send_result(hdev, ptr, len);
> -				break;
> +				kfree_skb(skb);
> +				return 0;
>   			}
>   		}
>   
> diff --git a/drivers/bluetooth/btintel_pcie.c b/drivers/bluetooth/btintel_pcie.c
> index e4ae8c898dfd..deed8052b482 100644
> --- a/drivers/bluetooth/btintel_pcie.c
> +++ b/drivers/bluetooth/btintel_pcie.c
> @@ -551,7 +551,8 @@ static int btintel_pcie_recv_event(struct hci_dev *hdev, struct sk_buff *skb)
>   				if (btintel_pcie_in_op(data)) {
>   					btintel_pcie_wr_sleep_cntrl(data, BTINTEL_PCIE_STATE_D0);
>   					data->alive_intr_ctxt = BTINTEL_PCIE_INTEL_HCI_RESET2;
> -					break;
> +					kfree_skb(skb);
> +					return 0;
>   				}
>   
>   				if (btintel_pcie_in_iml(data)) {
> @@ -568,7 +569,8 @@ static int btintel_pcie_recv_event(struct hci_dev *hdev, struct sk_buff *skb)
>   						btintel_wake_up_flag(data->hdev,
>   								     INTEL_WAIT_FOR_D0);
>   				}
> -				break;
> +				kfree_skb(skb);
> +				return 0;
>   			case 0x06:
>   				/* When the firmware loading completes the
>   				 * device sends out a vendor specific event
> @@ -576,7 +578,8 @@ static int btintel_pcie_recv_event(struct hci_dev *hdev, struct sk_buff *skb)
>   				 * loading.
>   				 */
>   				btintel_secure_send_result(hdev, ptr, len);
> -				break;
> +				kfree_skb(skb);
> +				return 0;
>   			}
>   		}


Kind regards,

Paul
K, Kiran Oct. 17, 2024, 11:34 a.m. UTC | #2
Hi Paul,

Thanks for your comments. I will incorporate the  changes in v3 version of the patch.

>Subject: Re: [PATCH v2] Bluetooth:btintel: Do no pass vendor events to stack
>
>Dear Kiran,
>
>
>Thank you for your patch. Please note, in the summary/title a space is missing
>before *btintel*.
>
>Am 17.10.24 um 12:22 schrieb Kiran K:
>> During firmware download, vendor specific events like boot up and
>> secure send result are generated. These events can be safely processed
>> at the driver level. Passing on these events to stack prints
>> unnecessary below warning log.
>>
>> --
>> Bluetooth: hci0: Malformed MSFT vendor event: 0x02
>> --
>
>I’d remove the `--` and just indent the message by four characters to follow
>Markdown markup style.
>
>On what device did you test this? Are there open reports about it?
>
>> Fixes: 3368aa357f3b ("Bluetooth: msft: Handle MSFT Monitor Device
>> Event")
>> Signed-off-by: Kiran K <kiran.k@intel.com>
>> ---
>>   drivers/bluetooth/btintel.c      | 6 ++++--
>>   drivers/bluetooth/btintel_pcie.c | 9 ++++++---
>>   2 files changed, 10 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
>> index 2be6d48a2a65..e122dff855ba 100644
>> --- a/drivers/bluetooth/btintel.c
>> +++ b/drivers/bluetooth/btintel.c
>> @@ -3395,7 +3395,8 @@ int btintel_recv_event(struct hci_dev *hdev, struct
>sk_buff *skb)
>>   				 * indicating that the bootup completed.
>>   				 */
>>   				btintel_bootup(hdev, ptr, len);
>> -				break;
>> +				kfree_skb(skb);
>> +				return 0;
>>   			case 0x06:
>>   				/* When the firmware loading completes the
>>   				 * device sends out a vendor specific event
>@@ -3403,7 +3404,8
>> @@ int btintel_recv_event(struct hci_dev *hdev, struct sk_buff *skb)
>>   				 * loading.
>>   				 */
>>   				btintel_secure_send_result(hdev, ptr, len);
>> -				break;
>> +				kfree_skb(skb);
>> +				return 0;
>>   			}
>>   		}
>>
>> diff --git a/drivers/bluetooth/btintel_pcie.c
>> b/drivers/bluetooth/btintel_pcie.c
>> index e4ae8c898dfd..deed8052b482 100644
>> --- a/drivers/bluetooth/btintel_pcie.c
>> +++ b/drivers/bluetooth/btintel_pcie.c
>> @@ -551,7 +551,8 @@ static int btintel_pcie_recv_event(struct hci_dev
>*hdev, struct sk_buff *skb)
>>   				if (btintel_pcie_in_op(data)) {
>>   					btintel_pcie_wr_sleep_cntrl(data,
>BTINTEL_PCIE_STATE_D0);
>>   					data->alive_intr_ctxt =
>BTINTEL_PCIE_INTEL_HCI_RESET2;
>> -					break;
>> +					kfree_skb(skb);
>> +					return 0;
>>   				}
>>
>>   				if (btintel_pcie_in_iml(data)) { @@ -568,7
>+569,8 @@ static int
>> btintel_pcie_recv_event(struct hci_dev *hdev, struct sk_buff *skb)
>>   						btintel_wake_up_flag(data-
>>hdev,
>>
>INTEL_WAIT_FOR_D0);
>>   				}
>> -				break;
>> +				kfree_skb(skb);
>> +				return 0;
>>   			case 0x06:
>>   				/* When the firmware loading completes the
>>   				 * device sends out a vendor specific event
>@@ -576,7 +578,8 @@
>> static int btintel_pcie_recv_event(struct hci_dev *hdev, struct sk_buff *skb)
>>   				 * loading.
>>   				 */
>>   				btintel_secure_send_result(hdev, ptr, len);
>> -				break;
>> +				kfree_skb(skb);
>> +				return 0;
>>   			}
>>   		}
>
>
>Kind regards,
>
>Paul

Thanks,
Kiran
patchwork-bot+bluetooth@kernel.org Oct. 17, 2024, 5:20 p.m. UTC | #3
Hello:

This patch was applied to bluetooth/bluetooth-next.git (master)
by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>:

On Thu, 17 Oct 2024 15:52:36 +0530 you wrote:
> During firmware download, vendor specific events like boot up and
> secure send result are generated. These events can be safely processed at
> the driver level. Passing on these events to stack prints unnecessary
> below warning log.
> 
> --
> Bluetooth: hci0: Malformed MSFT vendor event: 0x02
> --
> Fixes: 3368aa357f3b ("Bluetooth: msft: Handle MSFT Monitor Device Event")
> Signed-off-by: Kiran K <kiran.k@intel.com>
> 
> [...]

Here is the summary with links:
  - [v2] Bluetooth:btintel: Do no pass vendor events to stack
    https://git.kernel.org/bluetooth/bluetooth-next/c/4d67eb2b5444

You are awesome, thank you!
diff mbox series

Patch

diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
index 2be6d48a2a65..e122dff855ba 100644
--- a/drivers/bluetooth/btintel.c
+++ b/drivers/bluetooth/btintel.c
@@ -3395,7 +3395,8 @@  int btintel_recv_event(struct hci_dev *hdev, struct sk_buff *skb)
 				 * indicating that the bootup completed.
 				 */
 				btintel_bootup(hdev, ptr, len);
-				break;
+				kfree_skb(skb);
+				return 0;
 			case 0x06:
 				/* When the firmware loading completes the
 				 * device sends out a vendor specific event
@@ -3403,7 +3404,8 @@  int btintel_recv_event(struct hci_dev *hdev, struct sk_buff *skb)
 				 * loading.
 				 */
 				btintel_secure_send_result(hdev, ptr, len);
-				break;
+				kfree_skb(skb);
+				return 0;
 			}
 		}
 
diff --git a/drivers/bluetooth/btintel_pcie.c b/drivers/bluetooth/btintel_pcie.c
index e4ae8c898dfd..deed8052b482 100644
--- a/drivers/bluetooth/btintel_pcie.c
+++ b/drivers/bluetooth/btintel_pcie.c
@@ -551,7 +551,8 @@  static int btintel_pcie_recv_event(struct hci_dev *hdev, struct sk_buff *skb)
 				if (btintel_pcie_in_op(data)) {
 					btintel_pcie_wr_sleep_cntrl(data, BTINTEL_PCIE_STATE_D0);
 					data->alive_intr_ctxt = BTINTEL_PCIE_INTEL_HCI_RESET2;
-					break;
+					kfree_skb(skb);
+					return 0;
 				}
 
 				if (btintel_pcie_in_iml(data)) {
@@ -568,7 +569,8 @@  static int btintel_pcie_recv_event(struct hci_dev *hdev, struct sk_buff *skb)
 						btintel_wake_up_flag(data->hdev,
 								     INTEL_WAIT_FOR_D0);
 				}
-				break;
+				kfree_skb(skb);
+				return 0;
 			case 0x06:
 				/* When the firmware loading completes the
 				 * device sends out a vendor specific event
@@ -576,7 +578,8 @@  static int btintel_pcie_recv_event(struct hci_dev *hdev, struct sk_buff *skb)
 				 * loading.
 				 */
 				btintel_secure_send_result(hdev, ptr, len);
-				break;
+				kfree_skb(skb);
+				return 0;
 			}
 		}