> > On 4. Jan 2024, at 08:47, Aditya Garg <gargaditya08@xxxxxxxx> wrote: > > > >> On 28-Dec-2023, at 5:41 PM, Johan Hovold <johan@xxxxxxxxxx> wrote: >> >> On Thu, Dec 28, 2023 at 10:46:57AM +0100, Sven Peter wrote: >> >>>>>> On Dec 27, 2023, at 11:30, Johan Hovold <johan@xxxxxxxxxx> 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/20231227101003.10534-1-johan+linaro@xxxxxxxxxx/ >>>> >>>> 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
