diff mbox series

[v4] Bluetooth: Fix Bluetooth for BCM4377 on T2 Intel MacBooks

Message ID aaa107865f4cbd61f8f9006fd3e7ac43b5d1bdad.camel@mrman314.tech (mailing list archive)
State New, archived
Headers show
Series [v4] Bluetooth: Fix Bluetooth for BCM4377 on T2 Intel MacBooks | expand

Checks

Context Check Description
tedd_an/pre-ci_am fail error: patch fragment without header at line 10: @@ -2465,6 +2467,7 @@ static const struct bcm4377_hw hint: Use 'git am --show-current-patch' to see the failed patch

Commit Message

Felix Zhang Dec. 25, 2023, 8:21 p.m. UTC
Starting v6.5, Bluetooth does not work at all on my T2
MacBookAir9,1 with the BCM4377 chip.  When I boot up the computer,
go into bluetoothctl, and then try to run commands like scan on,
show, list, it returns "No default controller available."  I have
tried reloading the kernel module, in which the log outputs
"{Added,Removed} hci0 (unconfigured)."  With this patch, I
am able to use Bluetooth as normal without any errors regarding
hci0 being unconfigured.  However, an issue is still present
where sometimes hci_bcm4377 will have to be reloaded in order to
get bluetooth to work.  I believe this was still present before
the previously mentioned commit.

I would also like to thank Kerem Karabay <kekrby@gmail.com> for
assisting me with this patch.

Fixes: 6945795bc81a ("Bluetooth: fix use-bdaddr-property quirk")
Cc: <stable@vger.kernel.org>
Signed-off-by: Felix Zhang <mrman@mrman314.tech>
---
v4:
* Adjust the format to pass the CI (again).
* Shorten description
---
 drivers/bluetooth/hci_bcm4377.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

 	if (bcm4377->hw->broken_mws_transport_config)
@@ -2465,6 +2467,7 @@ static const struct bcm4377_hw
bcm4377_hw_variants[] = {
 		.has_bar0_core2_window2 = true,
 		.broken_mws_transport_config = true,
 		.broken_le_coded = true,
+		.use_bdaddr_property = true,
 		.send_calibration = bcm4378_send_calibration,
 		.send_ptb = bcm4378_send_ptb,
 	},
@@ -2479,6 +2482,7 @@ static const struct bcm4377_hw
bcm4377_hw_variants[] = {
 		.clear_pciecfg_subsystem_ctrl_bit19 = true,
 		.broken_mws_transport_config = true,
 		.broken_le_coded = true,
+		.use_bdaddr_property = true,
 		.send_calibration = bcm4387_send_calibration,
 		.send_ptb = bcm4378_send_ptb,
 	},

Comments

bluez.test.bot@gmail.com Dec. 25, 2023, 8:37 p.m. UTC | #1
This is an automated email and please do not reply to this email.

Dear Submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
While preparing the CI tests, the patches you submitted couldn't be applied to the current HEAD of the repository.

----- Output -----

error: patch fragment without header at line 10: @@ -2465,6 +2467,7 @@ static const struct bcm4377_hw
hint: Use 'git am --show-current-patch' to see the failed patch

Please resolve the issue and submit the patches again.


---
Regards,
Linux Bluetooth
Sven Peter Dec. 27, 2023, 10:11 a.m. UTC | #2
Hi,


> On 25. Dec 2023, at 21:21, Felix Zhang <mrman@mrman314.tech> wrote:
> Starting v6.5, Bluetooth does not work at all on my T2
> MacBookAir9,1 with the BCM4377 chip.  When I boot up the computer,
> go into bluetoothctl, and then try to run commands like scan on,
> show, list, it returns "No default controller available."  I have
> tried reloading the kernel module, in which the log outputs
> "{Added,Removed} hci0 (unconfigured)."  With this patch, I
> am able to use Bluetooth as normal without any errors regarding
> hci0 being unconfigured.  However, an issue is still present
> where sometimes hci_bcm4377 will have to be reloaded in order to
> get bluetooth to work.  I believe this was still present before
> the previously mentioned commit.
> I would also like to thank Kerem Karabay <kekrby@gmail.com> for
> assisting me with this patch.
> 
> Fixes: 6945795bc81a ("Bluetooth: fix use-bdaddr-property quirk")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Felix Zhang <mrman@mrman314.tech>
> ---

Thanks a lot for debugging and fixing this! The diff looks good to me.

Reviewed-by: Sven Peter <sven@svenpeter.dev>


Best,

Sven

> v4:
> * Adjust the format to pass the CI (again).
> * Shorten description
> ---
>  drivers/bluetooth/hci_bcm4377.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/bluetooth/hci_bcm4377.c
> b/drivers/bluetooth/hci_bcm4377.c
> index a61757835695..5c6fef1aa0f6 100644
> --- a/drivers/bluetooth/hci_bcm4377.c
> +++ b/drivers/bluetooth/hci_bcm4377.c
> @@ -513,6 +513,7 @@ struct bcm4377_hw {
>      unsigned long broken_ext_scan : 1;
>      unsigned long broken_mws_transport_config : 1;
>      unsigned long broken_le_coded : 1;
> +    unsigned long use_bdaddr_property : 1;
>  
>      int (*send_calibration)(struct bcm4377_data *bcm4377);
>      int (*send_ptb)(struct bcm4377_data *bcm4377,
> @@ -2368,5 +2369,6 @@ static int bcm4377_probe(struct pci_dev *pdev,
> const struct pci_device_id *id)
>      hdev->set_bdaddr = bcm4377_hci_set_bdaddr;
>      hdev->setup = bcm4377_hci_setup;
>  
> -    set_bit(HCI_QUIRK_USE_BDADDR_PROPERTY, &hdev->quirks);
> +    if (bcm4377->hw->use_bdaddr_property)
> +        set_bit(HCI_QUIRK_USE_BDADDR_PROPERTY, &hdev->quirks);
>      if (bcm4377->hw->broken_mws_transport_config)
> @@ -2465,6 +2467,7 @@ static const struct bcm4377_hw
> bcm4377_hw_variants[] = {
>          .has_bar0_core2_window2 = true,
>          .broken_mws_transport_config = true,
>          .broken_le_coded = true,
> +        .use_bdaddr_property = true,
>          .send_calibration = bcm4378_send_calibration,
>          .send_ptb = bcm4378_send_ptb,
>      },
> @@ -2479,6 +2482,7 @@ static const struct bcm4377_hw
> bcm4377_hw_variants[] = {
>          .clear_pciecfg_subsystem_ctrl_bit19 = true,
>          .broken_mws_transport_config = true,
>          .broken_le_coded = true,
> +        .use_bdaddr_property = true,
>          .send_calibration = bcm4387_send_calibration,
>          .send_ptb = bcm4378_send_ptb,
>      },
> -- 
> 2.43.0
Neal Gompa Dec. 27, 2023, 10:23 a.m. UTC | #3
On Mon, Dec 25, 2023 at 3:21 PM Felix Zhang <mrman@mrman314.tech> wrote:
>
> Starting v6.5, Bluetooth does not work at all on my T2
> MacBookAir9,1 with the BCM4377 chip.  When I boot up the computer,
> go into bluetoothctl, and then try to run commands like scan on,
> show, list, it returns "No default controller available."  I have
> tried reloading the kernel module, in which the log outputs
> "{Added,Removed} hci0 (unconfigured)."  With this patch, I
> am able to use Bluetooth as normal without any errors regarding
> hci0 being unconfigured.  However, an issue is still present
> where sometimes hci_bcm4377 will have to be reloaded in order to
> get bluetooth to work.  I believe this was still present before
> the previously mentioned commit.
>
> I would also like to thank Kerem Karabay <kekrby@gmail.com> for
> assisting me with this patch.
>
> Fixes: 6945795bc81a ("Bluetooth: fix use-bdaddr-property quirk")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Felix Zhang <mrman@mrman314.tech>
> ---
> v4:
> * Adjust the format to pass the CI (again).
> * Shorten description
> ---
>  drivers/bluetooth/hci_bcm4377.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/bluetooth/hci_bcm4377.c
> b/drivers/bluetooth/hci_bcm4377.c
> index a61757835695..5c6fef1aa0f6 100644
> --- a/drivers/bluetooth/hci_bcm4377.c
> +++ b/drivers/bluetooth/hci_bcm4377.c
> @@ -513,6 +513,7 @@ struct bcm4377_hw {
>         unsigned long broken_ext_scan : 1;
>         unsigned long broken_mws_transport_config : 1;
>         unsigned long broken_le_coded : 1;
> +       unsigned long use_bdaddr_property : 1;
>
>         int (*send_calibration)(struct bcm4377_data *bcm4377);
>         int (*send_ptb)(struct bcm4377_data *bcm4377,
> @@ -2368,5 +2369,6 @@ static int bcm4377_probe(struct pci_dev *pdev,
> const struct pci_device_id *id)
>         hdev->set_bdaddr = bcm4377_hci_set_bdaddr;
>         hdev->setup = bcm4377_hci_setup;
>
> -       set_bit(HCI_QUIRK_USE_BDADDR_PROPERTY, &hdev->quirks);
> +       if (bcm4377->hw->use_bdaddr_property)
> +               set_bit(HCI_QUIRK_USE_BDADDR_PROPERTY, &hdev->quirks);
>         if (bcm4377->hw->broken_mws_transport_config)
> @@ -2465,6 +2467,7 @@ static const struct bcm4377_hw
> bcm4377_hw_variants[] = {
>                 .has_bar0_core2_window2 = true,
>                 .broken_mws_transport_config = true,
>                 .broken_le_coded = true,
> +               .use_bdaddr_property = true,
>                 .send_calibration = bcm4378_send_calibration,
>                 .send_ptb = bcm4378_send_ptb,
>         },
> @@ -2479,6 +2482,7 @@ static const struct bcm4377_hw
> bcm4377_hw_variants[] = {
>                 .clear_pciecfg_subsystem_ctrl_bit19 = true,
>                 .broken_mws_transport_config = true,
>                 .broken_le_coded = true,
> +               .use_bdaddr_property = true,
>                 .send_calibration = bcm4387_send_calibration,
>                 .send_ptb = bcm4378_send_ptb,
>         },
> --
> 2.43.0
>
>

Sorry, excuse me for replying to the wrong message, I got confused by
Gmail on what to reply to.

This is fine and thanks for the fix!

Reviewed-by: Neal Gompa <neal@gompa.dev>
Johan Hovold Dec. 27, 2023, 10:35 a.m. UTC | #4
On Mon, Dec 25, 2023 at 03:21:04PM -0500, Felix Zhang wrote:
> Starting v6.5, Bluetooth does not work at all on my T2
> MacBookAir9,1 with the BCM4377 chip.  When I boot up the computer,
> go into bluetoothctl, and then try to run commands like scan on,
> show, list, it returns "No default controller available."  I have
> tried reloading the kernel module, in which the log outputs
> "{Added,Removed} hci0 (unconfigured)."  With this patch, I
> am able to use Bluetooth as normal without any errors regarding
> hci0 being unconfigured.  However, an issue is still present
> where sometimes hci_bcm4377 will have to be reloaded in order to
> get bluetooth to work.  I believe this was still present before
> the previously mentioned commit.
> 
> I would also like to thank Kerem Karabay <kekrby@gmail.com> for
> assisting me with this patch.
> 
> Fixes: 6945795bc81a ("Bluetooth: fix use-bdaddr-property quirk")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Felix Zhang <mrman@mrman314.tech>
> ---
> v4:
> * Adjust the format to pass the CI (again).
> * Shorten description

As explained here:

	https://lore.kernel.org/all/ZYv8tp3fMiAqK8OI@hovoldconsulting.com/

I don't this is necessarily the right fix. The BD_ADDR quirk property
should not be set unconditionally but it is still needed for devices
that lack storage for a unique device address.

So the following fix is needed either way and is probably all that is
needed here:

	https://lore.kernel.org/lkml/20231227101003.10534-1-johan+linaro@kernel.org/

Johan
Aditya Garg Dec. 28, 2023, 6:13 a.m. UTC | #5
I got it tested by a person and Johan's patch works.

> On 27-Dec-2023, at 4:05 PM, Johan Hovold <johan@kernel.org> wrote:
> 
> On Mon, Dec 25, 2023 at 03:21:04PM -0500, Felix Zhang wrote:
>> Starting v6.5, Bluetooth does not work at all on my T2
>> MacBookAir9,1 with the BCM4377 chip.  When I boot up the computer,
>> go into bluetoothctl, and then try to run commands like scan on,
>> show, list, it returns "No default controller available."  I have
>> tried reloading the kernel module, in which the log outputs
>> "{Added,Removed} hci0 (unconfigured)."  With this patch, I
>> am able to use Bluetooth as normal without any errors regarding
>> hci0 being unconfigured.  However, an issue is still present
>> where sometimes hci_bcm4377 will have to be reloaded in order to
>> get bluetooth to work.  I believe this was still present before
>> the previously mentioned commit.
>> 
>> I would also like to thank Kerem Karabay <kekrby@gmail.com> for
>> assisting me with this patch.
>> 
>> Fixes: 6945795bc81a ("Bluetooth: fix use-bdaddr-property quirk")
>> Cc: <stable@vger.kernel.org>
>> Signed-off-by: Felix Zhang <mrman@mrman314.tech>
>> ---
>> v4:
>> * Adjust the format to pass the CI (again).
>> * Shorten description
> 
> As explained here:
> 
>    https://lore.kernel.org/all/ZYv8tp3fMiAqK8OI@hovoldconsulting.com/
> 
> I don't this is necessarily the right fix. The BD_ADDR quirk property
> should not be set unconditionally but it is still needed for devices
> that lack storage for a unique device address.
> 
> So the following fix is needed either way and is probably all that is
> needed here:
> 
>    https://lore.kernel.org/lkml/20231227101003.10534-1-johan+linaro@kernel.org/
> 
> Johan
diff mbox series

Patch

diff --git a/drivers/bluetooth/hci_bcm4377.c
b/drivers/bluetooth/hci_bcm4377.c
index a61757835695..5c6fef1aa0f6 100644
--- a/drivers/bluetooth/hci_bcm4377.c
+++ b/drivers/bluetooth/hci_bcm4377.c
@@ -513,6 +513,7 @@  struct bcm4377_hw {
 	unsigned long broken_ext_scan : 1;
 	unsigned long broken_mws_transport_config : 1;
 	unsigned long broken_le_coded : 1;
+	unsigned long use_bdaddr_property : 1;
 
 	int (*send_calibration)(struct bcm4377_data *bcm4377);
 	int (*send_ptb)(struct bcm4377_data *bcm4377,
@@ -2368,5 +2369,6 @@  static int bcm4377_probe(struct pci_dev *pdev,
const struct pci_device_id *id)
 	hdev->set_bdaddr = bcm4377_hci_set_bdaddr;
 	hdev->setup = bcm4377_hci_setup;
 
-	set_bit(HCI_QUIRK_USE_BDADDR_PROPERTY, &hdev->quirks);
+	if (bcm4377->hw->use_bdaddr_property)
+		set_bit(HCI_QUIRK_USE_BDADDR_PROPERTY, &hdev->quirks);