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.
Due to the bit HCI_QUIRK_USE_BDADDR_PROPERTY being always set in
drivers/bluetooth/hci_bcm4377.c (line 2371), the chip would be left
unconfigured on kernels compiled after commit 6945795bc81a
("Bluetooth:
fix use-bdaddr-property quirk") due to a change in its logic. On the
M1 Macs, the device would be configured in the devicetree. However,
that is not the case on T2 Macs. Because the bluetooth adapter is
left
unconfigured, it is not usable in the operating system. In order to
circumvent this issue, a flag is added to prevent the bit from being
set on the BCM4377, while setting it on the other devices.
Because I do not have an M1 device to test this patch on, I am not sure
whether the patch breaks anything for said devices. I would be very
grateful if anyone is willing to test this patch on their M1 device.
I would also like to thank Kerem Karabay <[email protected]> for
assisting me with this patch.
Fixes: 6945795bc81a ("Bluetooth: fix use-bdaddr-property quirk")
Signed-off-by: Felix Zhang <[email protected]>
---
v3:
* Adjust the format to pass the CI (again).
---
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,7 +2369,8 @@ 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)
set_bit(HCI_QUIRK_BROKEN_MWS_TRANSPORT_CONFIG, &hdev-
>quirks);
if (bcm4377->hw->broken_ext_scan)
@@ -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
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
Dear Felix,
Thank you very much for the patch. I am adding Johan to Cc field.
Am 25.12.23 um 21:01 schrieb Felix Zhang:
> 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
Somehow a blank line snug in above.
> bluetoothctl, and then try to run commands like scan on, show, list,
> it returns "No default controller available." I have tried reloading
> the
It’d be great if you reflowed for 75 characters per line (also below).
> 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.
>
> Due to the bit HCI_QUIRK_USE_BDADDR_PROPERTY being always set in
> drivers/bluetooth/hci_bcm4377.c (line 2371), the chip would be left
> unconfigured on kernels compiled after commit 6945795bc81a
> ("Bluetooth:
> fix use-bdaddr-property quirk") due to a change in its logic. On the
> M1 Macs, the device would be configured in the devicetree. However,
> that is not the case on T2 Macs. Because the bluetooth adapter is
> left
> unconfigured, it is not usable in the operating system. In order to
> circumvent this issue, a flag is added to prevent the bit from being
> set on the BCM4377, while setting it on the other devices.
>
> Because I do not have an M1 device to test this patch on, I am not sure
> whether the patch breaks anything for said devices. I would be very
> grateful if anyone is willing to test this patch on their M1 device.
>
> I would also like to thank Kerem Karabay <[email protected]> for
> assisting me with this patch.
>
> Fixes: 6945795bc81a ("Bluetooth: fix use-bdaddr-property quirk")
> Signed-off-by: Felix Zhang <[email protected]>
> ---
> v3:
> * Adjust the format to pass the CI (again).
> ---
> 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,7 +2369,8 @@ 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)
> set_bit(HCI_QUIRK_BROKEN_MWS_TRANSPORT_CONFIG, &hdev-
>> quirks);
> if (bcm4377->hw->broken_ext_scan)
> @@ -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,
> },
The diff looks good, and it works for you.
Kind regards,
Paul
On Mon, Dec 25, 2023 at 3:01 PM Felix Zhang <[email protected]> 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.
>
> Due to the bit HCI_QUIRK_USE_BDADDR_PROPERTY being always set in
> drivers/bluetooth/hci_bcm4377.c (line 2371), the chip would be left
> unconfigured on kernels compiled after commit 6945795bc81a
> ("Bluetooth:
> fix use-bdaddr-property quirk") due to a change in its logic. On the
> M1 Macs, the device would be configured in the devicetree. However,
> that is not the case on T2 Macs. Because the bluetooth adapter is
> left
> unconfigured, it is not usable in the operating system. In order to
> circumvent this issue, a flag is added to prevent the bit from being
> set on the BCM4377, while setting it on the other devices.
>
> Because I do not have an M1 device to test this patch on, I am not sure
> whether the patch breaks anything for said devices. I would be very
> grateful if anyone is willing to test this patch on their M1 device.
>
> I would also like to thank Kerem Karabay <[email protected]> for
> assisting me with this patch.
>
> Fixes: 6945795bc81a ("Bluetooth: fix use-bdaddr-property quirk")
> Signed-off-by: Felix Zhang <[email protected]>
> ---
> v3:
> * Adjust the format to pass the CI (again).
> ---
> 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,7 +2369,8 @@ 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)
> set_bit(HCI_QUIRK_BROKEN_MWS_TRANSPORT_CONFIG, &hdev-
> >quirks);
> if (bcm4377->hw->broken_ext_scan)
> @@ -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
>
This looks reasonable to me, thanks for fixing this!
Reviewed-by: Neal Gompa <[email protected]>
--
真実はいつも一つ！/ Always, there's only one truth!
On Mon, Dec 25, 2023 at 09:26:05PM +0100, Paul Menzel wrote:
> Thank you very much for the patch. I am adding Johan to Cc field.
Thanks for the report. Guess I could use a break from the proverbial
eggnog.
> Am 25.12.23 um 21:01 schrieb Felix Zhang:
> > 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
>
> Somehow a blank line snug in above.
>
> > bluetoothctl, and then try to run commands like scan on, show, list,
> > it returns "No default controller available." I have tried reloading
> > the
>
> It’d be great if you reflowed for 75 characters per line (also below).
>
> > 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.
> >
> > Due to the bit HCI_QUIRK_USE_BDADDR_PROPERTY being always set in
> > drivers/bluetooth/hci_bcm4377.c (line 2371), the chip would be left
> > unconfigured on kernels compiled after commit 6945795bc81a
> > ("Bluetooth:
> > fix use-bdaddr-property quirk") due to a change in its logic. On the
> > M1 Macs, the device would be configured in the devicetree. However,
> > that is not the case on T2 Macs. Because the bluetooth adapter is
> > left
> > unconfigured, it is not usable in the operating system. In order to
> > circumvent this issue, a flag is added to prevent the bit from being
> > set on the BCM4377, while setting it on the other devices.
The commit you tracked this down to restored the original semantics for
HCI_QUIRK_USE_BDADDR_PROPERTY, which means that it should only be set
for devices with an invalid address.
The Broadcom BCM4377 driver has so far been setting this flag
unconditionally which now potentially results in also valid addresses
being marked as invalid.
I've just sent a patch that makes sure to only mark invalid addresses as
invalid:
https://lore.kernel.org/lkml/[email protected]/
Note however that the flag still needs to be set in case your device
lacks storage for a unique device address so you cannot simply drop it
for some device classes as you do below (unless you are certain that
these devices will always have a valid address).
Devices without a valid address starts in an unconfigured state and
cannot be used until userspace has provided an address (e.g. using
btmgmt):
btmgmt --index 0 public-addr 00:11:22:33:44:55
> > Fixes: 6945795bc81a ("Bluetooth: fix use-bdaddr-property quirk")
> > Signed-off-by: Felix Zhang <[email protected]>
> > @@ -2368,7 +2369,8 @@ 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)
> > set_bit(HCI_QUIRK_BROKEN_MWS_TRANSPORT_CONFIG, &hdev-
> >> quirks);
> > if (bcm4377->hw->broken_ext_scan)
> > @@ -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,
> > },
Back to the eggnog.
Johan
Sending this again because Apple mail decided to default to HTML
mails since the last update apparently *sigh*
Hi,
> On Dec 27, 2023, at 11:30, Johan Hovold <[email protected]> wrote:
>
> On Mon, Dec 25, 2023 at 09:26:05PM +0100, Paul Menzel wrote:
>
>> Thank you very much for the patch. I am adding Johan to Cc field.
>
> Thanks for the report. Guess I could use a break from the proverbial
> eggnog.
>
>> Am 25.12.23 um 21:01 schrieb Felix Zhang:
>>> 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
>> Somehow a blank line snug in above.
>>> bluetoothctl, and then try to run commands like scan on, show, list,
>>> it returns "No default controller available." I have tried reloading
>>> the
>> It’d be great if you reflowed for 75 characters per line (also below).
>>> 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.
>>> Due to the bit HCI_QUIRK_USE_BDADDR_PROPERTY being always set in
>>> drivers/bluetooth/hci_bcm4377.c (line 2371), the chip would be left
>>> unconfigured on kernels compiled after commit 6945795bc81a
>>> ("Bluetooth:
>>> fix use-bdaddr-property quirk") due to a change in its logic. On the
>>> M1 Macs, the device would be configured in the devicetree. However,
>>> that is not the case on T2 Macs. Because the bluetooth adapter is
>>> left
>>> unconfigured, it is not usable in the operating system. In order to
>>> circumvent this issue, a flag is added to prevent the bit from being
>>> set on the BCM4377, while setting it on the other devices.
>
> The commit you tracked this down to restored the original semantics for
> HCI_QUIRK_USE_BDADDR_PROPERTY, which means that it should only be set
> for devices with an invalid address.
>
> The Broadcom BCM4377 driver has so far been setting this flag
> unconditionally which now potentially results in also valid addresses
> being marked as invalid.
>
> I've just sent a patch that makes sure to only mark invalid addresses as
> invalid:
>
> https://lore.kernel.org/lkml/[email protected]/
>
> Note however that the flag still needs to be set in case your device
> lacks storage for a unique device address so you cannot simply drop it
> for some device classes as you do below (unless you are certain that
> these devices will always have a valid address).
We do know that though.
BCM4377 is present on Apple’s x86 Macs and always has internal storage
for the address. If the board comes up without an address there’s nothing
much we can do because the address isn’t provided by ACPI or anything
else and setting the invalid address quirk for that situation seems appropriate.
BCM4378/4387 is present on Apple’s ARM Macs and never has internal storage.
The address is always provided by our bootloader in the device tree.
These should always unconditionally set HCI_QUIRK_USE_BDADDR_PROPERTY
just like this patch does.
Best,
Sven
On Thu, Dec 28, 2023 at 10:46:57AM +0100, Sven Peter wrote:
> > On Dec 27, 2023, at 11:30, Johan Hovold <[email protected]> wrote:
> > The commit you tracked this down to restored the original semantics for
> > HCI_QUIRK_USE_BDADDR_PROPERTY, which means that it should only be set
> > for devices with an invalid address.
> >
> > The Broadcom BCM4377 driver has so far been setting this flag
> > unconditionally which now potentially results in also valid addresses
> > being marked as invalid.
> >
> > I've just sent a patch that makes sure to only mark invalid addresses as
> > invalid:
> >
> > https://lore.kernel.org/lkml/[email protected]/
> >
> > Note however that the flag still needs to be set in case your device
> > lacks storage for a unique device address so you cannot simply drop it
> > for some device classes as you do below (unless you are certain that
> > these devices will always have a valid address).
> We do know that though.
>
> BCM4377 is present on Apple’s x86 Macs and always has internal storage
> for the address. If the board comes up without an address there’s nothing
> much we can do because the address isn’t provided by ACPI or anything
> else and setting the invalid address quirk for that situation seems appropriate.
>
> BCM4378/4387 is present on Apple’s ARM Macs and never has internal storage.
> The address is always provided by our bootloader in the device tree.
> These should always unconditionally set HCI_QUIRK_USE_BDADDR_PROPERTY
> just like this patch does.
Ok, good, then this patch and the one I posted are mostly equivalent
assuming that the BCM4378/4387 return an invalid address during setup.
This patch may be preferred as it does not need to rely on such
assumptions, though.
Johan
> On 28-Dec-2023, at 5:41 PM, Johan Hovold <[email protected]> wrote:
>
> On Thu, Dec 28, 2023 at 10:46:57AM +0100, Sven Peter wrote:
>
>>>> On Dec 27, 2023, at 11:30, Johan Hovold <[email protected]> wrote:
>>
>>>> The commit you tracked this down to restored the original semantics for
>>> HCI_QUIRK_USE_BDADDR_PROPERTY, which means that it should only be set
>>> for devices with an invalid address.
>>>
>>> The Broadcom BCM4377 driver has so far been setting this flag
>>> unconditionally which now potentially results in also valid addresses
>>> being marked as invalid.
>>>
>>> I've just sent a patch that makes sure to only mark invalid addresses as
>>> invalid:
>>>
>>> https://lore.kernel.org/lkml/[email protected]/
>>>
>>> Note however that the flag still needs to be set in case your device
>>> lacks storage for a unique device address so you cannot simply drop it
>>> for some device classes as you do below (unless you are certain that
>>> these devices will always have a valid address).
>
>> We do know that though.
>>
>> BCM4377 is present on Apple’s x86 Macs and always has internal storage
>> for the address. If the board comes up without an address there’s nothing
>> much we can do because the address isn’t provided by ACPI or anything
>> else and setting the invalid address quirk for that situation seems appropriate.
>>
>> BCM4378/4387 is present on Apple’s ARM Macs and never has internal storage.
>> The address is always provided by our bootloader in the device tree.
>> These should always unconditionally set HCI_QUIRK_USE_BDADDR_PROPERTY
>> just like this patch does.
>
> Ok, good, then this patch and the one I posted are mostly equivalent
> assuming that the BCM4378/4387 return an invalid address during setup.
>
> This patch may be preferred as it does not need to rely on such
> assumptions, though.
>
> Johan
So what's the final take on this? Which one is gonna be merged upstream?
>
> On 4. Jan 2024, at 08:47, Aditya Garg <[email protected]> wrote:
>
>
>
>> On 28-Dec-2023, at 5:41 PM, Johan Hovold <[email protected]> wrote:
>>
>> On Thu, Dec 28, 2023 at 10:46:57AM +0100, Sven Peter wrote:
>>
>>>>>> On Dec 27, 2023, at 11:30, Johan Hovold <[email protected]> wrote:
>>>
>>>>> The commit you tracked this down to restored the original semantics for
>>>> HCI_QUIRK_USE_BDADDR_PROPERTY, which means that it should only be set
>>>> for devices with an invalid address.
>>>>
>>>> The Broadcom BCM4377 driver has so far been setting this flag
>>>> unconditionally which now potentially results in also valid addresses
>>>> being marked as invalid.
>>>>
>>>> I've just sent a patch that makes sure to only mark invalid addresses as
>>>> invalid:
>>>>
>>>> https://lore.kernel.org/lkml/[email protected]/
>>>>
>>>> Note however that the flag still needs to be set in case your device
>>>> lacks storage for a unique device address so you cannot simply drop it
>>>> for some device classes as you do below (unless you are certain that
>>>> these devices will always have a valid address).
>>
>>> We do know that though.
>>>
>>> BCM4377 is present on Apple’s x86 Macs and always has internal storage
>>> for the address. If the board comes up without an address there’s nothing
>>> much we can do because the address isn’t provided by ACPI or anything
>>> else and setting the invalid address quirk for that situation seems appropriate.
>>>
>>> BCM4378/4387 is present on Apple’s ARM Macs and never has internal storage.
>>> The address is always provided by our bootloader in the device tree.
>>> These should always unconditionally set HCI_QUIRK_USE_BDADDR_PROPERTY
>>> just like this patch does.
>>
>> Ok, good, then this patch and the one I posted are mostly equivalent
>> assuming that the BCM4378/4387 return an invalid address during setup.
>>
>> This patch may be preferred as it does not need to rely on such
>> assumptions, though.
>>
>> Johan
>
> So what's the final take on this? Which one is gonna be merged upstream?
I would’ve preferred this one (possibly with a better commit message) since it’s more explicit and doesn’t rely on additional assumptions but it looks like Johan’s version was already merged.
Sven
On Thu, Jan 04, 2024 at 08:50:19AM +0100, Sven Peter wrote:
> > On 4. Jan 2024, at 08:47, Aditya Garg <[email protected]> wrote:
> >> On 28-Dec-2023, at 5:41 PM, Johan Hovold <[email protected]> wrote:
> >> Ok, good, then this patch and the one I posted are mostly equivalent
> >> assuming that the BCM4378/4387 return an invalid address during setup.
> >>
> >> This patch may be preferred as it does not need to rely on such
> >> assumptions, though.
> > So what's the final take on this? Which one is gonna be merged upstream?
>
> I would’ve preferred this one (possibly with a better commit message)
> since it’s more explicit and doesn’t rely on additional assumptions
> but it looks like Johan’s version was already merged.
Which addresses do BCM4378/4387 return before they are configured?
Should be easy enough to verify that the current check for invalid
addresses catches those or otherwise add them to the list.
Johan
>
> On 4. Jan 2024, at 09:21, Johan Hovold <[email protected]> wrote:
>
> On Thu, Jan 04, 2024 at 08:50:19AM +0100, Sven Peter wrote:
>>> On 4. Jan 2024, at 08:47, Aditya Garg <[email protected]> wrote:
>>>> On 28-Dec-2023, at 5:41 PM, Johan Hovold <[email protected]> wrote:
>
>>>> Ok, good, then this patch and the one I posted are mostly equivalent
>>>> assuming that the BCM4378/4387 return an invalid address during setup.
>>>>
>>>> This patch may be preferred as it does not need to rely on such
>>>> assumptions, though.
>
>>> So what's the final take on this? Which one is gonna be merged upstream?
>>
>> I would’ve preferred this one (possibly with a better commit message)
>> since it’s more explicit and doesn’t rely on additional assumptions
>> but it looks like Johan’s version was already merged.
>
> Which addresses do BCM4378/4387 return before they are configured?
> Should be easy enough to verify that the current check for invalid
> addresses catches those or otherwise add them to the list.
>
> Johan
I think the check used to work for BRCM4378 when I originally wrote the driver but I don’t have any BRCM4387 hardware so can’t test that myself.
Sven