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

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

Posted by Felix Zhang 2 years ago

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

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

Posted by Johan Hovold 1 year, 12 months ago

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

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

Posted by Neal Gompa 1 year, 12 months ago

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>


-- 
真実はいつも一つ！/ Always, there's only one truth!

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

Posted by Sven Peter 1 year, 12 months ago

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