On Sat, Nov 06, 2021 at 09:41:28AM +0000, Aditya Garg wrote:
>
>
> > On 06-Nov-2021, at 3:17 AM, Luiz Augusto von Dentz <[email protected]> wrote:
> >
> > Hi Orlando,
> >
> > On Fri, Oct 1, 2021 at 1:56 AM Orlando Chamberlain
> > <[email protected]> wrote:
> >>
> >> The LE Read Transmit Power command is Advertised on some Broadcom
> >> controlers, but not supported. Using this command breaks Bluetooth
> >> on the MacBookPro16,1 and iMac20,1. Added a quirk disabling LE Read
> >> Transmit Power for these devices, based off their common chip id 150.
> >>
> >> Link: https://lore.kernel.org/r/[email protected]
> >> Signed-off-by: Orlando Chamberlain <[email protected]>
> >> ---
> >> v1->v2: Clarified quirk description
> >>
> >> drivers/bluetooth/btbcm.c | 4 ++++
> >> include/net/bluetooth/hci.h | 11 +++++++++++
> >> net/bluetooth/hci_core.c | 3 ++-
> >> 3 files changed, 17 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/bluetooth/btbcm.c b/drivers/bluetooth/btbcm.c
> >> index e4182acee488..4ecc50d93107 100644
> >> --- a/drivers/bluetooth/btbcm.c
> >> +++ b/drivers/bluetooth/btbcm.c
> >> @@ -353,6 +353,10 @@ static int btbcm_read_info(struct hci_dev *hdev)
> >> return PTR_ERR(skb);
> >>
> >> bt_dev_info(hdev, "BCM: chip id %u", skb->data[1]);
> >> +
> >> + if (skb->data[1] == 150)
> >> + set_bit(HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER, &hdev->quirks);
> >> +
> >> kfree_skb(skb);
> >>
> >> /* Read Controller Features */
> >> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> >> index b80415011dcd..6da9bd6b7259 100644
> >> --- a/include/net/bluetooth/hci.h
> >> +++ b/include/net/bluetooth/hci.h
> >> @@ -246,6 +246,17 @@ enum {
> >> * HCI after resume.
> >> */
> >> HCI_QUIRK_NO_SUSPEND_NOTIFIER,
> >> +
> >> + /* When this quirk is set, LE Read Transmit Power is disabled.
> >> + * This is mainly due to the fact that the HCI LE Read Transmit
> >> + * Power command is advertised, but not supported; these
> >> + * controllers often reply with unknown command and need a hard
> >> + * reset.
> >> + *
> >> + * This quirk can be set before hci_register_dev is called or
> >> + * during the hdev->setup vendor callback.
> >> + */
> >> + HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER,
> >> };
> >>
> >> /* HCI device flags */
> >> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> >> index 8a47a3017d61..9a23fe7c8d67 100644
> >> --- a/net/bluetooth/hci_core.c
> >> +++ b/net/bluetooth/hci_core.c
> >> @@ -742,7 +742,8 @@ static int hci_init3_req(struct hci_request *req, unsigned long opt)
> >> hci_req_add(req, HCI_OP_LE_READ_ADV_TX_POWER, 0, NULL);
> >> }
> >>
> >> - if (hdev->commands[38] & 0x80) {
> >> + if (hdev->commands[38] & 0x80 &&
> >> + !test_bit(HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER, &hdev->quirks)) {
> >> /* Read LE Min/Max Tx Power*/
> >> hci_req_add(req, HCI_OP_LE_READ_TRANSMIT_POWER,
> >> 0, NULL);
> >> --
> >> 2.33.0
> >
> > Nowadays it is possible to treat errors such like this on a per
> > command basis (assuming it is not essential for the init sequence):
> >
> > diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> > index 979da5179ff4..f244f42cc609 100644
> > --- a/include/net/bluetooth/hci.h
> > +++ b/include/net/bluetooth/hci.h
> > @@ -551,6 +551,7 @@ enum {
> > #define HCI_LK_AUTH_COMBINATION_P256 0x08
> >
> > /* ---- HCI Error Codes ---- */
> > +#define HCI_ERROR_UNKNOWN_CMD 0x01
> > #define HCI_ERROR_UNKNOWN_CONN_ID 0x02
> > #define HCI_ERROR_AUTH_FAILURE 0x05
> > #define HCI_ERROR_PIN_OR_KEY_MISSING 0x06
> > diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
> This diff cannot be applied to stable 5.15. Could you provide a patch capable of being applied to stable.
That is not needed until a patch is in Linus's tree. Why do you need it
earlier?
thanks,
greg k-h
> On 06-Nov-2021, at 5:19 PM, Greg KH <[email protected]> wrote:
>
> On Sat, Nov 06, 2021 at 09:41:28AM +0000, Aditya Garg wrote:
>>
>>
>>> On 06-Nov-2021, at 3:17 AM, Luiz Augusto von Dentz <[email protected]> wrote:
>>>
>>> Hi Orlando,
>>>
>>> On Fri, Oct 1, 2021 at 1:56 AM Orlando Chamberlain
>>> <[email protected]> wrote:
>>>>
>>>> The LE Read Transmit Power command is Advertised on some Broadcom
>>>> controlers, but not supported. Using this command breaks Bluetooth
>>>> on the MacBookPro16,1 and iMac20,1. Added a quirk disabling LE Read
>>>> Transmit Power for these devices, based off their common chip id 150.
>>>>
>>>> Link: https://lore.kernel.org/r/[email protected]
>>>> Signed-off-by: Orlando Chamberlain <[email protected]>
>>>> ---
>>>> v1->v2: Clarified quirk description
>>>>
>>>> drivers/bluetooth/btbcm.c | 4 ++++
>>>> include/net/bluetooth/hci.h | 11 +++++++++++
>>>> net/bluetooth/hci_core.c | 3 ++-
>>>> 3 files changed, 17 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/bluetooth/btbcm.c b/drivers/bluetooth/btbcm.c
>>>> index e4182acee488..4ecc50d93107 100644
>>>> --- a/drivers/bluetooth/btbcm.c
>>>> +++ b/drivers/bluetooth/btbcm.c
>>>> @@ -353,6 +353,10 @@ static int btbcm_read_info(struct hci_dev *hdev)
>>>> return PTR_ERR(skb);
>>>>
>>>> bt_dev_info(hdev, "BCM: chip id %u", skb->data[1]);
>>>> +
>>>> + if (skb->data[1] == 150)
>>>> + set_bit(HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER, &hdev->quirks);
>>>> +
>>>> kfree_skb(skb);
>>>>
>>>> /* Read Controller Features */
>>>> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
>>>> index b80415011dcd..6da9bd6b7259 100644
>>>> --- a/include/net/bluetooth/hci.h
>>>> +++ b/include/net/bluetooth/hci.h
>>>> @@ -246,6 +246,17 @@ enum {
>>>> * HCI after resume.
>>>> */
>>>> HCI_QUIRK_NO_SUSPEND_NOTIFIER,
>>>> +
>>>> + /* When this quirk is set, LE Read Transmit Power is disabled.
>>>> + * This is mainly due to the fact that the HCI LE Read Transmit
>>>> + * Power command is advertised, but not supported; these
>>>> + * controllers often reply with unknown command and need a hard
>>>> + * reset.
>>>> + *
>>>> + * This quirk can be set before hci_register_dev is called or
>>>> + * during the hdev->setup vendor callback.
>>>> + */
>>>> + HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER,
>>>> };
>>>>
>>>> /* HCI device flags */
>>>> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
>>>> index 8a47a3017d61..9a23fe7c8d67 100644
>>>> --- a/net/bluetooth/hci_core.c
>>>> +++ b/net/bluetooth/hci_core.c
>>>> @@ -742,7 +742,8 @@ static int hci_init3_req(struct hci_request *req, unsigned long opt)
>>>> hci_req_add(req, HCI_OP_LE_READ_ADV_TX_POWER, 0, NULL);
>>>> }
>>>>
>>>> - if (hdev->commands[38] & 0x80) {
>>>> + if (hdev->commands[38] & 0x80 &&
>>>> + !test_bit(HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER, &hdev->quirks)) {
>>>> /* Read LE Min/Max Tx Power*/
>>>> hci_req_add(req, HCI_OP_LE_READ_TRANSMIT_POWER,
>>>> 0, NULL);
>>>> --
>>>> 2.33.0
>>>
>>> Nowadays it is possible to treat errors such like this on a per
>>> command basis (assuming it is not essential for the init sequence):
>>>
>>> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
>>> index 979da5179ff4..f244f42cc609 100644
>>> --- a/include/net/bluetooth/hci.h
>>> +++ b/include/net/bluetooth/hci.h
>>> @@ -551,6 +551,7 @@ enum {
>>> #define HCI_LK_AUTH_COMBINATION_P256 0x08
>>>
>>> /* ---- HCI Error Codes ---- */
>>> +#define HCI_ERROR_UNKNOWN_CMD 0x01
>>> #define HCI_ERROR_UNKNOWN_CONN_ID 0x02
>>> #define HCI_ERROR_AUTH_FAILURE 0x05
>>> #define HCI_ERROR_PIN_OR_KEY_MISSING 0x06
>>> diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
>> This diff cannot be applied to stable 5.15. Could you provide a patch capable of being applied to stable.
>
> That is not needed until a patch is in Linus's tree. Why do you need it
> earlier?
>
Well not an emergency, but the issue of Bluetooth not working on some Apple Macs has been there for a long time. BTW, will this patch be there in Linus’s tree in the coming days?
> thanks,
>
> greg k-h
On Sat, Nov 06, 2021 at 05:27:27PM +0000, Aditya Garg wrote:
>
>
> > On 06-Nov-2021, at 5:19 PM, Greg KH <[email protected]> wrote:
> >
> > On Sat, Nov 06, 2021 at 09:41:28AM +0000, Aditya Garg wrote:
> >>
> >>
> >>> On 06-Nov-2021, at 3:17 AM, Luiz Augusto von Dentz <[email protected]> wrote:
> >>>
> >>> Hi Orlando,
> >>>
> >>> On Fri, Oct 1, 2021 at 1:56 AM Orlando Chamberlain
> >>> <[email protected]> wrote:
> >>>>
> >>>> The LE Read Transmit Power command is Advertised on some Broadcom
> >>>> controlers, but not supported. Using this command breaks Bluetooth
> >>>> on the MacBookPro16,1 and iMac20,1. Added a quirk disabling LE Read
> >>>> Transmit Power for these devices, based off their common chip id 150.
> >>>>
> >>>> Link: https://lore.kernel.org/r/[email protected]
> >>>> Signed-off-by: Orlando Chamberlain <[email protected]>
> >>>> ---
> >>>> v1->v2: Clarified quirk description
> >>>>
> >>>> drivers/bluetooth/btbcm.c | 4 ++++
> >>>> include/net/bluetooth/hci.h | 11 +++++++++++
> >>>> net/bluetooth/hci_core.c | 3 ++-
> >>>> 3 files changed, 17 insertions(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/drivers/bluetooth/btbcm.c b/drivers/bluetooth/btbcm.c
> >>>> index e4182acee488..4ecc50d93107 100644
> >>>> --- a/drivers/bluetooth/btbcm.c
> >>>> +++ b/drivers/bluetooth/btbcm.c
> >>>> @@ -353,6 +353,10 @@ static int btbcm_read_info(struct hci_dev *hdev)
> >>>> return PTR_ERR(skb);
> >>>>
> >>>> bt_dev_info(hdev, "BCM: chip id %u", skb->data[1]);
> >>>> +
> >>>> + if (skb->data[1] == 150)
> >>>> + set_bit(HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER, &hdev->quirks);
> >>>> +
> >>>> kfree_skb(skb);
> >>>>
> >>>> /* Read Controller Features */
> >>>> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> >>>> index b80415011dcd..6da9bd6b7259 100644
> >>>> --- a/include/net/bluetooth/hci.h
> >>>> +++ b/include/net/bluetooth/hci.h
> >>>> @@ -246,6 +246,17 @@ enum {
> >>>> * HCI after resume.
> >>>> */
> >>>> HCI_QUIRK_NO_SUSPEND_NOTIFIER,
> >>>> +
> >>>> + /* When this quirk is set, LE Read Transmit Power is disabled.
> >>>> + * This is mainly due to the fact that the HCI LE Read Transmit
> >>>> + * Power command is advertised, but not supported; these
> >>>> + * controllers often reply with unknown command and need a hard
> >>>> + * reset.
> >>>> + *
> >>>> + * This quirk can be set before hci_register_dev is called or
> >>>> + * during the hdev->setup vendor callback.
> >>>> + */
> >>>> + HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER,
> >>>> };
> >>>>
> >>>> /* HCI device flags */
> >>>> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> >>>> index 8a47a3017d61..9a23fe7c8d67 100644
> >>>> --- a/net/bluetooth/hci_core.c
> >>>> +++ b/net/bluetooth/hci_core.c
> >>>> @@ -742,7 +742,8 @@ static int hci_init3_req(struct hci_request *req, unsigned long opt)
> >>>> hci_req_add(req, HCI_OP_LE_READ_ADV_TX_POWER, 0, NULL);
> >>>> }
> >>>>
> >>>> - if (hdev->commands[38] & 0x80) {
> >>>> + if (hdev->commands[38] & 0x80 &&
> >>>> + !test_bit(HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER, &hdev->quirks)) {
> >>>> /* Read LE Min/Max Tx Power*/
> >>>> hci_req_add(req, HCI_OP_LE_READ_TRANSMIT_POWER,
> >>>> 0, NULL);
> >>>> --
> >>>> 2.33.0
> >>>
> >>> Nowadays it is possible to treat errors such like this on a per
> >>> command basis (assuming it is not essential for the init sequence):
> >>>
> >>> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> >>> index 979da5179ff4..f244f42cc609 100644
> >>> --- a/include/net/bluetooth/hci.h
> >>> +++ b/include/net/bluetooth/hci.h
> >>> @@ -551,6 +551,7 @@ enum {
> >>> #define HCI_LK_AUTH_COMBINATION_P256 0x08
> >>>
> >>> /* ---- HCI Error Codes ---- */
> >>> +#define HCI_ERROR_UNKNOWN_CMD 0x01
> >>> #define HCI_ERROR_UNKNOWN_CONN_ID 0x02
> >>> #define HCI_ERROR_AUTH_FAILURE 0x05
> >>> #define HCI_ERROR_PIN_OR_KEY_MISSING 0x06
> >>> diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
> >> This diff cannot be applied to stable 5.15. Could you provide a patch capable of being applied to stable.
> >
> > That is not needed until a patch is in Linus's tree. Why do you need it
> > earlier?
> >
> Well not an emergency, but the issue of Bluetooth not working on some Apple Macs has been there for a long time. BTW, will this patch be there in Linus’s tree in the coming days?
That is up to the bluetooth maintainers, they have to accept it first.
thanks,
greg k-h
Hi, this is your Linux kernel regression tracker speaking. To make this
easy and quick to read for everyone I for once rely on top-posting:
Bluetooth maintainers, what's the status here? The proposed patch is
fixing a regression. It's not a recent one (it afaics was introduced in
v5.11-rc1). Nevertheless it would be good to get this finally resolved.
But this thread seems inactive for more than a week now. Or was progress
made, but is only visible somewhere else?
Ciao, Thorsten (carrying his Linux kernel regression tracker hat)
P.S.: As a Linux kernel regression tracker I'm getting a lot of reports
on my table. I can only look briefly into most of them. Therefore I
unfortunately will get things wrong or miss something important. I hope
that's not the case here; if you think it is, don't hesitate to tell me
about it in a public reply. That's in everyone's interest, as what I
wrote above might be misleading to everyone reading this, which is
something I'd like to avoid.
BTW, I have no personal interest in this issue, which is tracked using
regzbot, my Linux kernel regression tracking bot
(https://linux-regtracking.leemhuis.info/regzbot/). I'm only posting
this mail to hopefully get things rolling again and hence don't need to
be CC on all further activities wrt to this regression. Also feel free
to ignore the following lines, they are meant for regzbot:
#regzbot poke
On 07.11.21 09:35, Greg KH wrote:
> On Sat, Nov 06, 2021 at 05:27:27PM +0000, Aditya Garg wrote:
>>> On 06-Nov-2021, at 5:19 PM, Greg KH <[email protected]> wrote:
>>> On Sat, Nov 06, 2021 at 09:41:28AM +0000, Aditya Garg wrote:
>>>>> On 06-Nov-2021, at 3:17 AM, Luiz Augusto von Dentz <[email protected]> wrote:
>>>>> On Fri, Oct 1, 2021 at 1:56 AM Orlando Chamberlain
>>>>> <[email protected]> wrote:
>>>>>>
>>>>>> The LE Read Transmit Power command is Advertised on some Broadcom
>>>>>> controlers, but not supported. Using this command breaks Bluetooth
>>>>>> on the MacBookPro16,1 and iMac20,1. Added a quirk disabling LE Read
>>>>>> Transmit Power for these devices, based off their common chip id 150.
>>>>>>
>>>>>> Link: https://lore.kernel.org/r/[email protected]
>>>>>> Signed-off-by: Orlando Chamberlain <[email protected]>
>>>>>> ---
>>>>>> v1->v2: Clarified quirk description
>>>>>>
>>>>>> drivers/bluetooth/btbcm.c | 4 ++++
>>>>>> include/net/bluetooth/hci.h | 11 +++++++++++
>>>>>> net/bluetooth/hci_core.c | 3 ++-
>>>>>> 3 files changed, 17 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/drivers/bluetooth/btbcm.c b/drivers/bluetooth/btbcm.c
>>>>>> index e4182acee488..4ecc50d93107 100644
>>>>>> --- a/drivers/bluetooth/btbcm.c
>>>>>> +++ b/drivers/bluetooth/btbcm.c
>>>>>> @@ -353,6 +353,10 @@ static int btbcm_read_info(struct hci_dev *hdev)
>>>>>> return PTR_ERR(skb);
>>>>>>
>>>>>> bt_dev_info(hdev, "BCM: chip id %u", skb->data[1]);
>>>>>> +
>>>>>> + if (skb->data[1] == 150)
>>>>>> + set_bit(HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER, &hdev->quirks);
>>>>>> +
>>>>>> kfree_skb(skb);
>>>>>>
>>>>>> /* Read Controller Features */
>>>>>> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
>>>>>> index b80415011dcd..6da9bd6b7259 100644
>>>>>> --- a/include/net/bluetooth/hci.h
>>>>>> +++ b/include/net/bluetooth/hci.h
>>>>>> @@ -246,6 +246,17 @@ enum {
>>>>>> * HCI after resume.
>>>>>> */
>>>>>> HCI_QUIRK_NO_SUSPEND_NOTIFIER,
>>>>>> +
>>>>>> + /* When this quirk is set, LE Read Transmit Power is disabled.
>>>>>> + * This is mainly due to the fact that the HCI LE Read Transmit
>>>>>> + * Power command is advertised, but not supported; these
>>>>>> + * controllers often reply with unknown command and need a hard
>>>>>> + * reset.
>>>>>> + *
>>>>>> + * This quirk can be set before hci_register_dev is called or
>>>>>> + * during the hdev->setup vendor callback.
>>>>>> + */
>>>>>> + HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER,
>>>>>> };
>>>>>>
>>>>>> /* HCI device flags */
>>>>>> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
>>>>>> index 8a47a3017d61..9a23fe7c8d67 100644
>>>>>> --- a/net/bluetooth/hci_core.c
>>>>>> +++ b/net/bluetooth/hci_core.c
>>>>>> @@ -742,7 +742,8 @@ static int hci_init3_req(struct hci_request *req, unsigned long opt)
>>>>>> hci_req_add(req, HCI_OP_LE_READ_ADV_TX_POWER, 0, NULL);
>>>>>> }
>>>>>>
>>>>>> - if (hdev->commands[38] & 0x80) {
>>>>>> + if (hdev->commands[38] & 0x80 &&
>>>>>> + !test_bit(HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER, &hdev->quirks)) {
>>>>>> /* Read LE Min/Max Tx Power*/
>>>>>> hci_req_add(req, HCI_OP_LE_READ_TRANSMIT_POWER,
>>>>>> 0, NULL);
>>>>>> --
>>>>>> 2.33.0
>>>>>
>>>>> Nowadays it is possible to treat errors such like this on a per
>>>>> command basis (assuming it is not essential for the init sequence):
>>>>>
>>>>> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
>>>>> index 979da5179ff4..f244f42cc609 100644
>>>>> --- a/include/net/bluetooth/hci.h
>>>>> +++ b/include/net/bluetooth/hci.h
>>>>> @@ -551,6 +551,7 @@ enum {
>>>>> #define HCI_LK_AUTH_COMBINATION_P256 0x08
>>>>>
>>>>> /* ---- HCI Error Codes ---- */
>>>>> +#define HCI_ERROR_UNKNOWN_CMD 0x01
>>>>> #define HCI_ERROR_UNKNOWN_CONN_ID 0x02
>>>>> #define HCI_ERROR_AUTH_FAILURE 0x05
>>>>> #define HCI_ERROR_PIN_OR_KEY_MISSING 0x06
>>>>> diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
>>>> This diff cannot be applied to stable 5.15. Could you provide a patch capable of being applied to stable.
>>>
>>> That is not needed until a patch is in Linus's tree. Why do you need it
>>> earlier?
>>>
>> Well not an emergency, but the issue of Bluetooth not working on some Apple Macs has been there for a long time. BTW, will this patch be there in Linus’s tree in the coming days?
>
> That is up to the bluetooth maintainers, they have to accept it first.
>
> thanks,
>
> greg k-h
>
>
> Bluetooth maintainers, what's the status here? The proposed patch is
> fixing a regression. It's not a recent one (it afaics was introduced in
> v5.11-rc1). Nevertheless it would be good to get this finally resolved.
> But this thread seems inactive for more than a week now. Or was progress
> made, but is only visible somewhere else?
I think the best solution is getting broadcom to update their firmware,
I've just sent them a message through a form on their website, I couldn't
seem to get it to tell me "Your message has been sent", so it's possible
that it didn't submit (more likely I've sent the same message several times).
If I hear back from them I'll send something here.
On 16.11.21 10:02, Orlando Chamberlain wrote:
>> Bluetooth maintainers, what's the status here? The proposed patch is
>> fixing a regression. It's not a recent one (it afaics was introduced in
>> v5.11-rc1). Nevertheless it would be good to get this finally resolved.
>> But this thread seems inactive for more than a week now. Or was progress
>> made, but is only visible somewhere else?
>
> I think the best solution is getting broadcom to update their firmware,
> I've just sent them a message through a form on their website, I couldn't
> seem to get it to tell me "Your message has been sent", so it's possible
> that it didn't submit (more likely I've sent the same message several times).
>
> If I hear back from them I'll send something here.
Thx for that. But FWIW: from the point of the regression tracker that's
not the best solution, as according to your report this is a regression.
IOW: we deal with something that used to up to a certain kernel version
and was broken by a change to the kernel. That is something frown upon
in Linux kernel development, hence changes introducing regression are
often quickly reverted, if they can't get fixed by follow up change quickly.
That sentence has two "quickly", as we want to prevent more people
running into the issue, resulting in a loss of trust. But that's what
will happen if we wait for a firmware update to get developed, tested,
published, and rolled out. And even then we can't expect users to have
the latest firmware installed when they switch to a new kernel.
Hence the best solution *afaics* might be: fix this in the kernel
somehow now with a workaround; once the firmware update is out, change
the kernel again to only apply the workaround if the old firmware is in use.
At least that's how it looks to me from the outside. But as mentioned
earlier already: as a Linux kernel regression tracker I'm getting a lot
of reports on my table. I can only look briefly into most of them.
Therefore I unfortunately will get things wrong or miss something
important. I hope that's not the case here; if you think it is, don't
hesitate to tell me about it in a public reply. That's in everyone's
interest, as what I wrote above might be misleading to everyone reading
this, which is something I'd like to avoid.
Ciao, Thorsten (carrying his Linux kernel regression tracker hat)
> On 16-Nov-2021, at 2:56 PM, Thorsten Leemhuis <[email protected]> wrote:
>
> On 16.11.21 10:02, Orlando Chamberlain wrote:
>>> Bluetooth maintainers, what's the status here? The proposed patch is
>>> fixing a regression. It's not a recent one (it afaics was introduced in
>>> v5.11-rc1). Nevertheless it would be good to get this finally resolved.
>>> But this thread seems inactive for more than a week now. Or was progress
>>> made, but is only visible somewhere else?
>>
>> I think the best solution is getting broadcom to update their firmware,
>> I've just sent them a message through a form on their website, I couldn't
>> seem to get it to tell me "Your message has been sent", so it's possible
>> that it didn't submit (more likely I've sent the same message several times).
>>
>> If I hear back from them I'll send something here.
>
> Thx for that. But FWIW: from the point of the regression tracker that's
> not the best solution, as according to your report this is a regression.
> IOW: we deal with something that used to up to a certain kernel version
> and was broken by a change to the kernel. That is something frown upon
> in Linux kernel development, hence changes introducing regression are
> often quickly reverted, if they can't get fixed by follow up change quickly.
>
> That sentence has two "quickly", as we want to prevent more people
> running into the issue, resulting in a loss of trust. But that's what
> will happen if we wait for a firmware update to get developed, tested,
> published, and rolled out. And even then we can't expect users to have
> the latest firmware installed when they switch to a new kernel.
>
> Hence the best solution *afaics* might be: fix this in the kernel
> somehow now with a workaround; once the firmware update is out, change
> the kernel again to only apply the workaround if the old firmware is in use.
I have an idea. Can we make LE Read Transmit Power as a module parameter and users can turn it off if it is causing trouble. I have a patch for the same but haven't tested it yet.
>
> At least that's how it looks to me from the outside. But as mentioned
> earlier already: as a Linux kernel regression tracker I'm getting a lot
> of reports on my table. I can only look briefly into most of them.
> Therefore I unfortunately will get things wrong or miss something
> important. I hope that's not the case here; if you think it is, don't
> hesitate to tell me about it in a public reply. That's in everyone's
> interest, as what I wrote above might be misleading to everyone reading
> this, which is something I'd like to avoid.
>
> Ciao, Thorsten (carrying his Linux kernel regression tracker hat)
On Wed, Nov 17, 2021 at 03:28:29AM +0000, Aditya Garg wrote:
>
>
> > On 16-Nov-2021, at 2:56 PM, Thorsten Leemhuis <[email protected]> wrote:
> >
> > On 16.11.21 10:02, Orlando Chamberlain wrote:
> >>> Bluetooth maintainers, what's the status here? The proposed patch is
> >>> fixing a regression. It's not a recent one (it afaics was introduced in
> >>> v5.11-rc1). Nevertheless it would be good to get this finally resolved.
> >>> But this thread seems inactive for more than a week now. Or was progress
> >>> made, but is only visible somewhere else?
> >>
> >> I think the best solution is getting broadcom to update their firmware,
> >> I've just sent them a message through a form on their website, I couldn't
> >> seem to get it to tell me "Your message has been sent", so it's possible
> >> that it didn't submit (more likely I've sent the same message several times).
> >>
> >> If I hear back from them I'll send something here.
> >
> > Thx for that. But FWIW: from the point of the regression tracker that's
> > not the best solution, as according to your report this is a regression.
> > IOW: we deal with something that used to up to a certain kernel version
> > and was broken by a change to the kernel. That is something frown upon
> > in Linux kernel development, hence changes introducing regression are
> > often quickly reverted, if they can't get fixed by follow up change quickly.
> >
> > That sentence has two "quickly", as we want to prevent more people
> > running into the issue, resulting in a loss of trust. But that's what
> > will happen if we wait for a firmware update to get developed, tested,
> > published, and rolled out. And even then we can't expect users to have
> > the latest firmware installed when they switch to a new kernel.
> >
> > Hence the best solution *afaics* might be: fix this in the kernel
> > somehow now with a workaround; once the firmware update is out, change
> > the kernel again to only apply the workaround if the old firmware is in use.
> I have an idea. Can we make LE Read Transmit Power as a module parameter and users can turn it off if it is causing trouble. I have a patch for the same but haven't tested it yet.
Module parameters are for the 1990's, please never add new ones as they
modify code, not data, and you want to do something like this on a
per-device basis, not on "all devices in the system", right?
thanks,
greg k-h
> On 17-Nov-2021, at 12:55 PM, Greg KH <[email protected]> wrote:
>
> On Wed, Nov 17, 2021 at 03:28:29AM +0000, Aditya Garg wrote:
>>
>>
>>> On 16-Nov-2021, at 2:56 PM, Thorsten Leemhuis <[email protected]> wrote:
>>>
>>> On 16.11.21 10:02, Orlando Chamberlain wrote:
>>>>> Bluetooth maintainers, what's the status here? The proposed patch is
>>>>> fixing a regression. It's not a recent one (it afaics was introduced in
>>>>> v5.11-rc1). Nevertheless it would be good to get this finally resolved.
>>>>> But this thread seems inactive for more than a week now. Or was progress
>>>>> made, but is only visible somewhere else?
>>>>
>>>> I think the best solution is getting broadcom to update their firmware,
>>>> I've just sent them a message through a form on their website, I couldn't
>>>> seem to get it to tell me "Your message has been sent", so it's possible
>>>> that it didn't submit (more likely I've sent the same message several times).
>>>>
>>>> If I hear back from them I'll send something here.
>>>
>>> Thx for that. But FWIW: from the point of the regression tracker that's
>>> not the best solution, as according to your report this is a regression.
>>> IOW: we deal with something that used to up to a certain kernel version
>>> and was broken by a change to the kernel. That is something frown upon
>>> in Linux kernel development, hence changes introducing regression are
>>> often quickly reverted, if they can't get fixed by follow up change quickly.
>>>
>>> That sentence has two "quickly", as we want to prevent more people
>>> running into the issue, resulting in a loss of trust. But that's what
>>> will happen if we wait for a firmware update to get developed, tested,
>>> published, and rolled out. And even then we can't expect users to have
>>> the latest firmware installed when they switch to a new kernel.
>>>
>>> Hence the best solution *afaics* might be: fix this in the kernel
>>> somehow now with a workaround; once the firmware update is out, change
>>> the kernel again to only apply the workaround if the old firmware is in use.
>> I have an idea. Can we make LE Read Transmit Power as a module parameter and users can turn it off if it is causing trouble. I have a patch for the same but haven't tested it yet.
>
> Module parameters are for the 1990's, please never add new ones as they
> modify code, not data, and you want to do something like this on a
> per-device basis, not on "all devices in the system", right?
Exactly. Since the issue affects only a few Macs and not all devices. In fact I have spotted just 2 Macs yet affected with this issue.
>
> thanks,
>
> greg k-h
On 17.11.21 10:26, Aditya Garg wrote:
>> On 17-Nov-2021, at 12:55 PM, Greg KH <[email protected]> wrote:
>> On Wed, Nov 17, 2021 at 03:28:29AM +0000, Aditya Garg wrote:
>>>> On 16-Nov-2021, at 2:56 PM, Thorsten Leemhuis <[email protected]> wrote:
>>>> On 16.11.21 10:02, Orlando Chamberlain wrote:
>>>>>> Bluetooth maintainers, what's the status here? The proposed patch is
>>>>>> fixing a regression. It's not a recent one (it afaics was introduced in
>>>>>> v5.11-rc1). Nevertheless it would be good to get this finally resolved.
>>>>>> But this thread seems inactive for more than a week now. Or was progress
>>>>>> made, but is only visible somewhere else?
>>>>>
>>>>> I think the best solution is getting broadcom to update their firmware,
>>>>> I've just sent them a message through a form on their website, I couldn't
>>>>> seem to get it to tell me "Your message has been sent", so it's possible
>>>>> that it didn't submit (more likely I've sent the same message several times).
>>>>>
>>>>> If I hear back from them I'll send something here.
>>>>
>>>> Thx for that. But FWIW: from the point of the regression tracker that's
>>>> not the best solution, as according to your report this is a regression.
>>>> IOW: we deal with something that used to up to a certain kernel version
>>>> and was broken by a change to the kernel. That is something frown upon
>>>> in Linux kernel development, hence changes introducing regression are
>>>> often quickly reverted, if they can't get fixed by follow up change quickly.
>>>>
>>>> That sentence has two "quickly", as we want to prevent more people
>>>> running into the issue, resulting in a loss of trust. But that's what
>>>> will happen if we wait for a firmware update to get developed, tested,
>>>> published, and rolled out. And even then we can't expect users to have
>>>> the latest firmware installed when they switch to a new kernel.
>>>>
>>>> Hence the best solution *afaics* might be: fix this in the kernel
>>>> somehow now with a workaround; once the firmware update is out, change
>>>> the kernel again to only apply the workaround if the old firmware is in use.
>>> I have an idea. Can we make LE Read Transmit Power as a module parameter and users can turn it off if it is causing trouble. I have a patch for the same but haven't tested it yet.
>>
>> Module parameters are for the 1990's, please never add new ones as they
>> modify code, not data, and you want to do something like this on a
>> per-device basis, not on "all devices in the system", right?
>
> Exactly. Since the issue affects only a few Macs and not all devices.
> In fact I have spotted just 2 Macs yet affected with this issue.
When Greg said "per-device basis", he afaics meant: per-device in a
system, as a module parameter would also affect a second bluetooth
controller if there was one (say one connected via USB) -- and that
shouldn't happen.
And FWIW: it's still a regression if something that used to work
suddenly requires a module parameter to get working.
So if this just affects two macs, why can't the fix be realized as a
quirk that is only enabled on those two systems? Or are they impossible
to detect clearly via DMI data or something like that?
Ciao, Thorsten
> On 17-Nov-2021, at 3:12 PM, Thorsten Leemhuis <[email protected]> wrote:
>
> On 17.11.21 10:26, Aditya Garg wrote:
>>> On 17-Nov-2021, at 12:55 PM, Greg KH <[email protected]> wrote:
>>> On Wed, Nov 17, 2021 at 03:28:29AM +0000, Aditya Garg wrote:
>>>>> On 16-Nov-2021, at 2:56 PM, Thorsten Leemhuis <[email protected]> wrote:
>>>>> On 16.11.21 10:02, Orlando Chamberlain wrote:
>>>>>>> Bluetooth maintainers, what's the status here? The proposed patch is
>>>>>>> fixing a regression. It's not a recent one (it afaics was introduced in
>>>>>>> v5.11-rc1). Nevertheless it would be good to get this finally resolved.
>>>>>>> But this thread seems inactive for more than a week now. Or was progress
>>>>>>> made, but is only visible somewhere else?
>>>>>>
>>>>>> I think the best solution is getting broadcom to update their firmware,
>>>>>> I've just sent them a message through a form on their website, I couldn't
>>>>>> seem to get it to tell me "Your message has been sent", so it's possible
>>>>>> that it didn't submit (more likely I've sent the same message several times).
>>>>>>
>>>>>> If I hear back from them I'll send something here.
>>>>>
>>>>> Thx for that. But FWIW: from the point of the regression tracker that's
>>>>> not the best solution, as according to your report this is a regression.
>>>>> IOW: we deal with something that used to up to a certain kernel version
>>>>> and was broken by a change to the kernel. That is something frown upon
>>>>> in Linux kernel development, hence changes introducing regression are
>>>>> often quickly reverted, if they can't get fixed by follow up change quickly.
>>>>>
>>>>> That sentence has two "quickly", as we want to prevent more people
>>>>> running into the issue, resulting in a loss of trust. But that's what
>>>>> will happen if we wait for a firmware update to get developed, tested,
>>>>> published, and rolled out. And even then we can't expect users to have
>>>>> the latest firmware installed when they switch to a new kernel.
>>>>>
>>>>> Hence the best solution *afaics* might be: fix this in the kernel
>>>>> somehow now with a workaround; once the firmware update is out, change
>>>>> the kernel again to only apply the workaround if the old firmware is in use.
>>>> I have an idea. Can we make LE Read Transmit Power as a module parameter and users can turn it off if it is causing trouble. I have a patch for the same but haven't tested it yet.
>>>
>>> Module parameters are for the 1990's, please never add new ones as they
>>> modify code, not data, and you want to do something like this on a
>>> per-device basis, not on "all devices in the system", right?
>>
>> Exactly. Since the issue affects only a few Macs and not all devices.
>> In fact I have spotted just 2 Macs yet affected with this issue.
> When Greg said "per-device basis", he afaics meant: per-device in a
> system, as a module parameter would also affect a second bluetooth
> controller if there was one (say one connected via USB) -- and that
> shouldn't happen.
>
> And FWIW: it's still a regression if something that used to work
> suddenly requires a module parameter to get working.
>
> So if this just affects two macs, why can't the fix be realized as a
> quirk that is only enabled on those two systems? Or are they impossible
> to detect clearly via DMI data or something like that?
<RESENDING AS PLAIN TEXT>
A part of the output of dmidecode is below :-
Handle 0x0005, DMI type 1, 27 bytes
System Information
Manufacturer: Apple Inc.
Product Name: MacBookPro16,1
Version: 1.0
Serial Number: <Removed for Privacy>
UUID: <Removed for Privacy>
Wake-up Type: Power Switch
SKU Number:
Family: MacBook Pro
Handle 0x0006, DMI type 2, 17 bytes
Base Board Information
Manufacturer: Apple Inc.
Product Name: Mac-E1008331FDC96864
Version: MacBookPro16,1
Serial Number: <Removed for Privacy>
Asset Tag:
Features:
Board is a hosting board
Location In Chassis:
Chassis Handle: 0x0007
Type: Motherboard
Contained Object Handles: 0
The product name, MacBookPro16,1 in my case, is unique for each model. If possible a quirk to disable LE Read Transmit Power can be made on this basis.
>
> Ciao, Thorsten
> So if this just affects two macs, why can't the fix be realized as a
> quirk that is only enabled on those two systems? Or are they impossible
> to detect clearly via DMI data or something like that?
I think we should be able to quirk based off the acpi _CID "apple-uart-blth"
or _HID "BCM2E7C". Marcel suggested quirking based of the acpi table here
https://lore.kernel.org/linux-bluetooth/[email protected]/
This would catch some unaffected Macs, but they don't support the LE Read
Transmit Power command anyway (the affected macs were released after it
was added to the Bluetooth spec, while the unaffected Macs were released
before it was added to the spec, and thus don't support it).
I'm not sure how to go about applying a quirk based off this, there are
quirks in drivers/bluetooth/hci_bcm.c (no_early_set_baudrate and
drive_rts_on_open), but they don't seem to be based off acpi ids.
It might be simpler to make it ignore the Unknown Command error, like
in this patch https://lore.kernel.org/linux-bluetooth/CABBYNZLjSfcG_KqTEbL6NOSvHhA5-b1t_S=3FQP4=GwW21kuzg@mail.gmail.com/
however that only applies on bluetooth-next and needed the status it
checks for to be -56, not 0x01.
--
Thanks,
Orlando
Hi Orlando,
>> So if this just affects two macs, why can't the fix be realized as a
>> quirk that is only enabled on those two systems? Or are they impossible
>> to detect clearly via DMI data or something like that?
>
> I think we should be able to quirk based off the acpi _CID "apple-uart-blth"
> or _HID "BCM2E7C". Marcel suggested quirking based of the acpi table here
> https://lore.kernel.org/linux-bluetooth/[email protected]/
>
> This would catch some unaffected Macs, but they don't support the LE Read
> Transmit Power command anyway (the affected macs were released after it
> was added to the Bluetooth spec, while the unaffected Macs were released
> before it was added to the spec, and thus don't support it).
>
> I'm not sure how to go about applying a quirk based off this, there are
> quirks in drivers/bluetooth/hci_bcm.c (no_early_set_baudrate and
> drive_rts_on_open), but they don't seem to be based off acpi ids.
>
> It might be simpler to make it ignore the Unknown Command error, like
> in this patch https://lore.kernel.org/linux-bluetooth/CABBYNZLjSfcG_KqTEbL6NOSvHhA5-b1t_S=3FQP4=GwW21kuzg@mail.gmail.com/
> however that only applies on bluetooth-next and needed the status it
> checks for to be -56, not 0x01.
so we abstain from try-and-error sending of commands. The Bluetooth spec
has a list of supported commands that a host can query for a reason. This
is really broken behavior of the controller and needs to be pointed out as
such.
The question is just how we quirk it.
Regards
Marcel
> On 18-Nov-2021, at 12:31 AM, Marcel Holtmann <[email protected]> wrote:
>
> Hi Orlando,
>
>>> So if this just affects two macs, why can't the fix be realized as a
>>> quirk that is only enabled on those two systems? Or are they impossible
>>> to detect clearly via DMI data or something like that?
>>
>> I think we should be able to quirk based off the acpi _CID "apple-uart-blth"
>> or _HID "BCM2E7C". Marcel suggested quirking based of the acpi table here
>> https://lore.kernel.org/linux-bluetooth/[email protected]/
>>
>> This would catch some unaffected Macs, but they don't support the LE Read
>> Transmit Power command anyway (the affected macs were released after it
>> was added to the Bluetooth spec, while the unaffected Macs were released
>> before it was added to the spec, and thus don't support it).
>>
>> I'm not sure how to go about applying a quirk based off this, there are
>> quirks in drivers/bluetooth/hci_bcm.c (no_early_set_baudrate and
>> drive_rts_on_open), but they don't seem to be based off acpi ids.
>>
>> It might be simpler to make it ignore the Unknown Command error, like
>> in this patch https://lore.kernel.org/linux-bluetooth/CABBYNZLjSfcG_KqTEbL6NOSvHhA5-b1t_S=3FQP4=GwW21kuzg@mail.gmail.com/
>> however that only applies on bluetooth-next and needed the status it
>> checks for to be -56, not 0x01.
>
> so we abstain from try-and-error sending of commands. The Bluetooth spec
> has a list of supported commands that a host can query for a reason. This
> is really broken behavior of the controller and needs to be pointed out as
> such.
Well all I can do is provide you any logs or information I can. But we do really wish to get this regression fixed soon.
>
> The question is just how we quirk it.
>
> Regards
>
> Marcel
>
Hi, this is your Linux kernel regression tracker speaking again.
On 19.11.21 17:59, Aditya Garg wrote:
>> On 18-Nov-2021, at 12:31 AM, Marcel Holtmann <[email protected]> wrote:
>>>> So if this just affects two macs, why can't the fix be realized as a
>>>> quirk that is only enabled on those two systems? Or are they impossible
>>>> to detect clearly via DMI data or something like that?
>>>
>>> I think we should be able to quirk based off the acpi _CID "apple-uart-blth"
>>> or _HID "BCM2E7C". Marcel suggested quirking based of the acpi table here
>>> https://lore.kernel.org/linux-bluetooth/[email protected]/
>>>
>>> This would catch some unaffected Macs, but they don't support the LE Read
>>> Transmit Power command anyway (the affected macs were released after it
>>> was added to the Bluetooth spec, while the unaffected Macs were released
>>> before it was added to the spec, and thus don't support it).
>>>
>>> I'm not sure how to go about applying a quirk based off this, there are
>>> quirks in drivers/bluetooth/hci_bcm.c (no_early_set_baudrate and
>>> drive_rts_on_open), but they don't seem to be based off acpi ids.
>>>
>>> It might be simpler to make it ignore the Unknown Command error, like
>>> in this patch https://lore.kernel.org/linux-bluetooth/CABBYNZLjSfcG_KqTEbL6NOSvHhA5-b1t_S=3FQP4=GwW21kuzg@mail.gmail.com/
>>> however that only applies on bluetooth-next and needed the status it
>>> checks for to be -56, not 0x01.
>>
>> so we abstain from try-and-error sending of commands. The Bluetooth spec
>> has a list of supported commands that a host can query for a reason. This
>> is really broken behavior of the controller and needs to be pointed out as
>> such.
> Well all I can do is provide you any logs or information I can. But we do really wish to get this regression fixed soon.
>>
>> The question is just how we quirk it.
This thread once again looks stalled and smells a lot like "everyone
agrees that his should be fixed, but afaics nobody submitted a fix or
committed to work on one". Please speak up if my impression is wrong, as
this is a regression and thus needs to be fixed, ideally quickly. Part
of my job is to make that happen and thus remind developers and
maintainers about this until we have a fix.
Ciao, Thorsten
#regzbot title bluetooth: "Query LE tx power on startup" broke Bluetooth
on MacBookPro16,1
#regzbot poke
> On 25-Nov-2021, at 5:56 PM, Thorsten Leemhuis <[email protected]> wrote:
>
> Hi, this is your Linux kernel regression tracker speaking again.
>
> On 19.11.21 17:59, Aditya Garg wrote:
>>> On 18-Nov-2021, at 12:31 AM, Marcel Holtmann <[email protected]> wrote:
>>>>> So if this just affects two macs, why can't the fix be realized as a
>>>>> quirk that is only enabled on those two systems? Or are they impossible
>>>>> to detect clearly via DMI data or something like that?
>>>>
>>>> I think we should be able to quirk based off the acpi _CID "apple-uart-blth"
>>>> or _HID "BCM2E7C". Marcel suggested quirking based of the acpi table here
>>>> https://lore.kernel.org/linux-bluetooth/[email protected]/
>>>>
>>>> This would catch some unaffected Macs, but they don't support the LE Read
>>>> Transmit Power command anyway (the affected macs were released after it
>>>> was added to the Bluetooth spec, while the unaffected Macs were released
>>>> before it was added to the spec, and thus don't support it).
>>>>
>>>> I'm not sure how to go about applying a quirk based off this, there are
>>>> quirks in drivers/bluetooth/hci_bcm.c (no_early_set_baudrate and
>>>> drive_rts_on_open), but they don't seem to be based off acpi ids.
>>>>
>>>> It might be simpler to make it ignore the Unknown Command error, like
>>>> in this patch https://lore.kernel.org/linux-bluetooth/CABBYNZLjSfcG_KqTEbL6NOSvHhA5-b1t_S=3FQP4=GwW21kuzg@mail.gmail.com/
>>>> however that only applies on bluetooth-next and needed the status it
>>>> checks for to be -56, not 0x01.
>>>
>>> so we abstain from try-and-error sending of commands. The Bluetooth spec
>>> has a list of supported commands that a host can query for a reason. This
>>> is really broken behavior of the controller and needs to be pointed out as
>>> such.
>> Well all I can do is provide you any logs or information I can. But we do really wish to get this regression fixed soon.
>>>
>>> The question is just how we quirk it.
>
> This thread once again looks stalled and smells a lot like "everyone
> agrees that his should be fixed, but afaics nobody submitted a fix or
> committed to work on one". Please speak up if my impression is wrong, as
> this is a regression and thus needs to be fixed, ideally quickly. Part
> of my job is to make that happen and thus remind developers and
> maintainers about this until we have a fix.
On the basis of DMI data, I have made this patch to disable read transmit power on 16,1. I have tested this on my 16,1 successfully. Still consider this as a draft as more models have to be added. I am sending this to get the approval of the maintainers whether this quirk is acceptable or not. If yes, I shall send the final patch.
From 3dab2e1e9e0b266574f5f010efc6680417fb0c61 Mon Sep 17 00:00:00 2001
From: Aditya Garg <[email protected]>
Date: Fri, 26 Nov 2021 18:28:46 +0530
Subject: [PATCH] Add quirk to disable read transmit power on MacBook Pro 16
inch, 2019
---
net/bluetooth/hci_core.c | 18 +++++++++++++++++-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 8d33aa64846b1c..d11064cb3666ef 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -32,6 +32,7 @@
#include <linux/property.h>
#include <linux/suspend.h>
#include <linux/wait.h>
+#include <linux/dmi.h>
#include <asm/unaligned.h>
#include <net/bluetooth/bluetooth.h>
@@ -461,9 +462,23 @@ static void hci_set_event_mask_page_2(struct hci_request *req)
sizeof(events), events);
}
+static const struct dmi_system_id fix_up_apple_bluetooth[] = {
+ {
+ /* Match for Apple MacBook Pro 16 inch, 2019 which needs
+ * read transmit power to be disabled
+ */
+ .matches = {
+ DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro16,1"),
+ },
+ },
+ { }
+};
+
static int hci_init3_req(struct hci_request *req, unsigned long opt)
{
struct hci_dev *hdev = req->hdev;
+ const struct dmi_system_id *dmi_id;
u8 p;
hci_setup_event_mask(req);
@@ -619,7 +634,8 @@ static int hci_init3_req(struct hci_request *req, unsigned long opt)
hci_req_add(req, HCI_OP_LE_READ_ADV_TX_POWER, 0, NULL);
}
- if (hdev->commands[38] & 0x80) {
+ dmi_id = dmi_first_match(fix_up_apple_bluetooth);
+ if (hdev->commands[38] & 0x80 && (!dmi_id)) {
/* Read LE Min/Max Tx Power*/
hci_req_add(req, HCI_OP_LE_READ_TRANSMIT_POWER,
0, NULL);
>
> Ciao, Thorsten
>
> #regzbot title bluetooth: "Query LE tx power on startup" broke Bluetooth
> on MacBookPro16,1
> #regzbot poke
So I have finally managed to add quirks in btbcm on the basis on DMI data. This shall be advantageous in the situation when the user shall be using a USB adapter so that the quirk gets ineffective for the adapter.
> On 26-Nov-2021, at 8:45 PM, Aditya Garg <[email protected]> wrote:
>
>
>
>> On 25-Nov-2021, at 5:56 PM, Thorsten Leemhuis <[email protected]> wrote:
>>
>> Hi, this is your Linux kernel regression tracker speaking again.
>>
>> On 19.11.21 17:59, Aditya Garg wrote:
>>>> On 18-Nov-2021, at 12:31 AM, Marcel Holtmann <[email protected]> wrote:
>>>>>> So if this just affects two macs, why can't the fix be realized as a
>>>>>> quirk that is only enabled on those two systems? Or are they impossible
>>>>>> to detect clearly via DMI data or something like that?
>>>>>
>>>>> I think we should be able to quirk based off the acpi _CID "apple-uart-blth"
>>>>> or _HID "BCM2E7C". Marcel suggested quirking based of the acpi table here
>>>>> https://lore.kernel.org/linux-bluetooth/[email protected]/
>>>>>
>>>>> This would catch some unaffected Macs, but they don't support the LE Read
>>>>> Transmit Power command anyway (the affected macs were released after it
>>>>> was added to the Bluetooth spec, while the unaffected Macs were released
>>>>> before it was added to the spec, and thus don't support it).
>>>>>
>>>>> I'm not sure how to go about applying a quirk based off this, there are
>>>>> quirks in drivers/bluetooth/hci_bcm.c (no_early_set_baudrate and
>>>>> drive_rts_on_open), but they don't seem to be based off acpi ids.
>>>>>
>>>>> It might be simpler to make it ignore the Unknown Command error, like
>>>>> in this patch https://lore.kernel.org/linux-bluetooth/CABBYNZLjSfcG_KqTEbL6NOSvHhA5-b1t_S=3FQP4=GwW21kuzg@mail.gmail.com/
>>>>> however that only applies on bluetooth-next and needed the status it
>>>>> checks for to be -56, not 0x01.
>>>>
>>>> so we abstain from try-and-error sending of commands. The Bluetooth spec
>>>> has a list of supported commands that a host can query for a reason. This
>>>> is really broken behavior of the controller and needs to be pointed out as
>>>> such.
>>> Well all I can do is provide you any logs or information I can. But we do really wish to get this regression fixed soon.
>>>>
>>>> The question is just how we quirk it.
>>
>> This thread once again looks stalled and smells a lot like "everyone
>> agrees that his should be fixed, but afaics nobody submitted a fix or
>> committed to work on one". Please speak up if my impression is wrong, as
>> this is a regression and thus needs to be fixed, ideally quickly. Part
>> of my job is to make that happen and thus remind developers and
>> maintainers about this until we have a fix.
> On the basis of DMI data, I have made this patch to disable read transmit power on 16,1. I have tested this on my 16,1 successfully. Still consider this as a draft as more models have to be added. I am sending this to get the approval of the maintainers whether this quirk is acceptable or not. If yes, I shall send the final patch.
>
> From 3dab2e1e9e0b266574f5f010efc6680417fb0c61 Mon Sep 17 00:00:00 2001
> From: Aditya Garg <[email protected]>
> Date: Fri, 26 Nov 2021 18:28:46 +0530
> Subject: [PATCH] Add quirk to disable read transmit power on MacBook Pro 16
> inch, 2019
>
> ---
> net/bluetooth/hci_core.c | 18 +++++++++++++++++-
> 1 file changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index 8d33aa64846b1c..d11064cb3666ef 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -32,6 +32,7 @@
> #include <linux/property.h>
> #include <linux/suspend.h>
> #include <linux/wait.h>
> +#include <linux/dmi.h>
> #include <asm/unaligned.h>
>
> #include <net/bluetooth/bluetooth.h>
> @@ -461,9 +462,23 @@ static void hci_set_event_mask_page_2(struct hci_request *req)
> sizeof(events), events);
> }
>
> +static const struct dmi_system_id fix_up_apple_bluetooth[] = {
> + {
> + /* Match for Apple MacBook Pro 16 inch, 2019 which needs
> + * read transmit power to be disabled
> + */
> + .matches = {
> + DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
> + DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro16,1"),
> + },
> + },
> + { }
> +};
> +
> static int hci_init3_req(struct hci_request *req, unsigned long opt)
> {
> struct hci_dev *hdev = req->hdev;
> + const struct dmi_system_id *dmi_id;
> u8 p;
>
> hci_setup_event_mask(req);
> @@ -619,7 +634,8 @@ static int hci_init3_req(struct hci_request *req, unsigned long opt)
> hci_req_add(req, HCI_OP_LE_READ_ADV_TX_POWER, 0, NULL);
> }
>
> - if (hdev->commands[38] & 0x80) {
> + dmi_id = dmi_first_match(fix_up_apple_bluetooth);
> + if (hdev->commands[38] & 0x80 && (!dmi_id)) {
> /* Read LE Min/Max Tx Power*/
> hci_req_add(req, HCI_OP_LE_READ_TRANSMIT_POWER,
> 0, NULL);
>>
>> Ciao, Thorsten
>>
>> #regzbot title bluetooth: "Query LE tx power on startup" broke Bluetooth
>> on MacBookPro16,1
>> #regzbot poke
From: Aditya Garg <[email protected]>
Some devices have a bug causing them to not work if they query LE tx power on startup. Thus we add a quirk in order to not query it and default min/max tx power values to HCI_TX_POWER_INVALID.
Signed-off-by: Aditya Garg <[email protected]>
Tested-by: Aditya Garg <[email protected]>
---
include/net/bluetooth/hci.h | 9 +++++++++
net/bluetooth/hci_core.c | 3 ++-
2 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 63065bc01b766c..383342efcdc464 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -246,6 +246,15 @@ enum {
* HCI after resume.
*/
HCI_QUIRK_NO_SUSPEND_NOTIFIER,
+
+ /*
+ * When this quirk is set, LE tx power is not queried on startup
+ * and the min/max tx power values default to HCI_TX_POWER_INVALID.
+ *
+ * This quirk can be set before hci_register_dev is called or
+ * during the hdev->setup vendor callback.
+ */
+ HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER,
};
/* HCI device flags */
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 8d33aa64846b1c..434c6878fe9640 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -619,7 +619,8 @@ static int hci_init3_req(struct hci_request *req, unsigned long opt)
hci_req_add(req, HCI_OP_LE_READ_ADV_TX_POWER, 0, NULL);
}
- if (hdev->commands[38] & 0x80) {
+ if (hdev->commands[38] & 0x80 &&
+ !test_bit(HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER, &hdev->quirks)) {
/* Read LE Min/Max Tx Power*/
hci_req_add(req, HCI_OP_LE_READ_TRANSMIT_POWER,
0, NULL);
From: Aditya Garg <[email protected]>
Bluetooth on Apple MacBook Pro 16,1 is unable to start due to LE Min/Max Tx Power being queried on startup. Add a DMI based quirk so that it is disabled.
Signed-off-by: Aditya Garg <[email protected]>
Tested-by: Aditya Garg <[email protected]>
---
drivers/bluetooth/btbcm.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/drivers/bluetooth/btbcm.c b/drivers/bluetooth/btbcm.c
index e4182acee488c5..c1b0ca63880a68 100644
--- a/drivers/bluetooth/btbcm.c
+++ b/drivers/bluetooth/btbcm.c
@@ -8,6 +8,7 @@
#include <linux/module.h>
#include <linux/firmware.h>
+#include <linux/dmi.h>
#include <asm/unaligned.h>
#include <net/bluetooth/bluetooth.h>
@@ -343,9 +344,23 @@ static struct sk_buff *btbcm_read_usb_product(struct hci_dev *hdev)
return skb;
}
+static const struct dmi_system_id disable_broken_read_transmit_power[] = {
+ {
+ /* Match for Apple MacBook Pro 16,1 which needs
+ * Read LE Min/Max Tx Power to be disabled.
+ */
+ .matches = {
+ DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro16,1"),
+ },
+ },
+ { }
+};
+
static int btbcm_read_info(struct hci_dev *hdev)
{
struct sk_buff *skb;
+ const struct dmi_system_id *dmi_id;
/* Read Verbose Config Version Info */
skb = btbcm_read_verbose_config(hdev);
@@ -362,6 +377,11 @@ static int btbcm_read_info(struct hci_dev *hdev)
bt_dev_info(hdev, "BCM: features 0x%2.2x", skb->data[1]);
kfree_skb(skb);
+
+ /* Read DMI and disable broken Read LE Min/Max Tx Power */
+ dmi_id = dmi_first_match(disable_broken_read_transmit_power);
+ if (dmi_id)
+ set_bit(HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER, &hdev->quirks);
return 0;
}
From: Aditya Garg <[email protected]>
Bluetooth on Apple MacBook Pro 16,2 is unable to start due to LE Min/Max Tx Power being queried on startup. Add a DMI based quirk so that it is disabled.
Signed-off-by: Aditya Garg <[email protected]>
---
drivers/bluetooth/btbcm.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/bluetooth/btbcm.c b/drivers/bluetooth/btbcm.c
index c1b0ca63880a6..ab7b754855d8a 100644
--- a/drivers/bluetooth/btbcm.c
+++ b/drivers/bluetooth/btbcm.c
@@ -354,6 +354,15 @@ static const struct dmi_system_id disable_broken_read_transmit_power[] = {
DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro16,1"),
},
},
+ {
+ /* Match for Apple MacBook Pro 16,2 which needs
+ * Read LE Min/Max Tx Power to be disabled.
+ */
+ .matches = {
+ DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro16,2"),
+ },
+ },
{ }
};
From: Aditya Garg <[email protected]>
Bluetooth on Apple MacBook Pro 16,4 is unable to start due to LE Min/Max Tx Power being queried on startup. Add a DMI based quirk so that it is disabled.
Signed-off-by: Aditya Garg <[email protected]>
---
drivers/bluetooth/btbcm.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/bluetooth/btbcm.c b/drivers/bluetooth/btbcm.c
index 348a4afa0774e..88214b453b0ce 100644
--- a/drivers/bluetooth/btbcm.c
+++ b/drivers/bluetooth/btbcm.c
@@ -363,6 +363,15 @@ static const struct dmi_system_id disable_broken_read_transmit_power[] = {
DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro16,2"),
},
},
+ {
+ /* Match for Apple MacBook Pro 16,4 which needs
+ * Read LE Min/Max Tx Power to be disabled.
+ */
+ .matches = {
+ DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro16,4"),
+ },
+ },
{ }
};
From: Aditya Garg <[email protected]>
Bluetooth on Apple iMac 20,1 is unable to start due to LE Min/Max Tx Power being queried on startup. Add a DMI based quirk so that it is disabled.
Signed-off-by: Aditya Garg <[email protected]>
---
drivers/bluetooth/btbcm.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/bluetooth/btbcm.c b/drivers/bluetooth/btbcm.c
index 88214b453b0ce..15c5be927c659 100644
--- a/drivers/bluetooth/btbcm.c
+++ b/drivers/bluetooth/btbcm.c
@@ -372,6 +372,15 @@ static const struct dmi_system_id disable_broken_read_transmit_power[] = {
DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro16,4"),
},
},
+ {
+ /* Match for Apple iMac 20,1 which needs
+ * Read LE Min/Max Tx Power to be disabled.
+ */
+ .matches = {
+ DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "iMac20,1"),
+ },
+ },
{ }
};
From: Aditya Garg <[email protected]>
Bluetooth on Apple iMac 20,2 is unable to start due to LE Min/Max Tx Power being queried on startup. Add a DMI based quirk so that it is disabled.
Signed-off-by: Aditya Garg <[email protected]>
---
drivers/bluetooth/btbcm.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/bluetooth/btbcm.c b/drivers/bluetooth/btbcm.c
index 15c5be927c659..601337b5a5130 100644
--- a/drivers/bluetooth/btbcm.c
+++ b/drivers/bluetooth/btbcm.c
@@ -381,6 +381,15 @@ static const struct dmi_system_id disable_broken_read_transmit_power[] = {
DMI_MATCH(DMI_PRODUCT_NAME, "iMac20,1"),
},
},
+ {
+ /* Match for Apple iMac 20,2 which needs
+ * Read LE Min/Max Tx Power to be disabled.
+ */
+ .matches = {
+ DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "iMac20,2"),
+ },
+ },
{ }
};
On Mon, Nov 29, 2021 at 07:22:27AM +0000, Aditya Garg wrote:
> From: Aditya Garg <[email protected]>
>
> Some devices have a bug causing them to not work if they query LE tx power on startup. Thus we add a quirk in order to not query it and default min/max tx power values to HCI_TX_POWER_INVALID.
Please wrap your changelog text at 72 columns, like your editor asked
you to :)
>
> Signed-off-by: Aditya Garg <[email protected]>
> Tested-by: Aditya Garg <[email protected]>
Tested-by: is implicit for patches you create yourself, so no need to
add it again :)
> ---
> include/net/bluetooth/hci.h | 9 +++++++++
> net/bluetooth/hci_core.c | 3 ++-
> 2 files changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> index 63065bc01b766c..383342efcdc464 100644
> --- a/include/net/bluetooth/hci.h
> +++ b/include/net/bluetooth/hci.h
> @@ -246,6 +246,15 @@ enum {
> * HCI after resume.
> */
> HCI_QUIRK_NO_SUSPEND_NOTIFIER,
> +
> + /*
> + * When this quirk is set, LE tx power is not queried on startup
> + * and the min/max tx power values default to HCI_TX_POWER_INVALID.
> + *
> + * This quirk can be set before hci_register_dev is called or
> + * during the hdev->setup vendor callback.
> + */
> + HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER,
> };
>
> /* HCI device flags */
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index 8d33aa64846b1c..434c6878fe9640 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -619,7 +619,8 @@ static int hci_init3_req(struct hci_request *req, unsigned long opt)
> hci_req_add(req, HCI_OP_LE_READ_ADV_TX_POWER, 0, NULL);
> }
>
> - if (hdev->commands[38] & 0x80) {
> + if (hdev->commands[38] & 0x80 &&
> + !test_bit(HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER, &hdev->quirks)) {
Did you run checkpatch on this patch? Please indent properly.
thanks,
greg k-h
From: Aditya Garg <[email protected]>
Some devices have a bug causing them to not work if they query LE tx power on startup. Thus we add a
quirk in order to not query it and default min/max tx power values to HCI_TX_POWER_INVALID.
v2: Wrap the changeling at 72 columns, correct email and remove tested by.
Signed-off-by: Aditya Garg <[email protected]>
---
include/net/bluetooth/hci.h | 9 +++++++++
net/bluetooth/hci_core.c | 3 ++-
2 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 63065bc01b766c..383342efcdc464 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -246,6 +246,15 @@ enum {
* HCI after resume.
*/
HCI_QUIRK_NO_SUSPEND_NOTIFIER,
+
+ /*
+ * When this quirk is set, LE tx power is not queried on startup
+ * and the min/max tx power values default to HCI_TX_POWER_INVALID.
+ *
+ * This quirk can be set before hci_register_dev is called or
+ * during the hdev->setup vendor callback.
+ */
+ HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER,
};
/* HCI device flags */
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 8d33aa64846b1c..434c6878fe9640 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -619,7 +619,8 @@ static int hci_init3_req(struct hci_request *req, unsigned long opt)
hci_req_add(req, HCI_OP_LE_READ_ADV_TX_POWER, 0, NULL);
}
- if (hdev->commands[38] & 0x80) {
+ if (hdev->commands[38] & 0x80 &&
+ !test_bit(HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER, &hdev->quirks)) {
/* Read LE Min/Max Tx Power*/
hci_req_add(req, HCI_OP_LE_READ_TRANSMIT_POWER,
0, NULL);
From: Aditya Garg <[email protected]>
Bluetooth on Apple MacBook Pro 16,1 is unable to start due to LE Min/Max Tx Power being queried on
startup. Add a DMI based quirk so that it is disabled.
v2: Wrap the changeling at 72 columns and remove tested by.
Signed-off-by: Aditya Garg <[email protected]>
---
drivers/bluetooth/btbcm.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/drivers/bluetooth/btbcm.c b/drivers/bluetooth/btbcm.c
index e4182acee488c5..c1b0ca63880a68 100644
--- a/drivers/bluetooth/btbcm.c
+++ b/drivers/bluetooth/btbcm.c
@@ -8,6 +8,7 @@
#include <linux/module.h>
#include <linux/firmware.h>
+#include <linux/dmi.h>
#include <asm/unaligned.h>
#include <net/bluetooth/bluetooth.h>
@@ -343,9 +344,23 @@ static struct sk_buff *btbcm_read_usb_product(struct hci_dev *hdev)
return skb;
}
+static const struct dmi_system_id disable_broken_read_transmit_power[] = {
+ {
+ /* Match for Apple MacBook Pro 16,1 which needs
+ * Read LE Min/Max Tx Power to be disabled.
+ */
+ .matches = {
+ DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro16,1"),
+ },
+ },
+ { }
+};
+
static int btbcm_read_info(struct hci_dev *hdev)
{
struct sk_buff *skb;
+ const struct dmi_system_id *dmi_id;
/* Read Verbose Config Version Info */
skb = btbcm_read_verbose_config(hdev);
@@ -362,6 +377,11 @@ static int btbcm_read_info(struct hci_dev *hdev)
bt_dev_info(hdev, "BCM: features 0x%2.2x", skb->data[1]);
kfree_skb(skb);
+
+ /* Read DMI and disable broken Read LE Min/Max Tx Power */
+ dmi_id = dmi_first_match(disable_broken_read_transmit_power);
+ if (dmi_id)
+ set_bit(HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER, &hdev->quirks);
return 0;
}
From: Aditya Garg <[email protected]>
Bluetooth on Apple MacBook Pro 16,2 is unable to start due to LE Min/Max Tx Power being queried on
startup. Add a DMI based quirk so that it is disabled.
v2: Wrap changelog in 72 columns
Signed-off-by: Aditya Garg <[email protected]>
---
drivers/bluetooth/btbcm.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/bluetooth/btbcm.c b/drivers/bluetooth/btbcm.c
index c1b0ca63880a6..ab7b754855d8a 100644
--- a/drivers/bluetooth/btbcm.c
+++ b/drivers/bluetooth/btbcm.c
@@ -354,6 +354,15 @@ static const struct dmi_system_id disable_broken_read_transmit_power[] = {
DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro16,1"),
},
},
+ {
+ /* Match for Apple MacBook Pro 16,2 which needs
+ * Read LE Min/Max Tx Power to be disabled.
+ */
+ .matches = {
+ DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro16,2"),
+ },
+ },
{ }
};
From: Aditya Garg <[email protected]>
Bluetooth on Apple MacBook Pro 16,4 is unable to start due to LE Min/Max Tx Power being queried on
startup. Add a DMI based quirk so that it is disabled.
v2: Wrap changelog in 72 columns.
Signed-off-by: Aditya Garg <[email protected]>
---
drivers/bluetooth/btbcm.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/bluetooth/btbcm.c b/drivers/bluetooth/btbcm.c
index 348a4afa0774e..88214b453b0ce 100644
--- a/drivers/bluetooth/btbcm.c
+++ b/drivers/bluetooth/btbcm.c
@@ -363,6 +363,15 @@ static const struct dmi_system_id disable_broken_read_transmit_power[] = {
DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro16,2"),
},
},
+ {
+ /* Match for Apple MacBook Pro 16,4 which needs
+ * Read LE Min/Max Tx Power to be disabled.
+ */
+ .matches = {
+ DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro16,4"),
+ },
+ },
{ }
};
On Mon, Nov 29, 2021 at 07:42:39AM +0000, Aditya Garg wrote:
> From: Aditya Garg <[email protected]>
>
> Some devices have a bug causing them to not work if they query LE tx power on startup. Thus we add a
> quirk in order to not query it and default min/max tx power values to HCI_TX_POWER_INVALID.
>
> v2: Wrap the changeling at 72 columns, correct email and remove tested by.
These lines are not wrapped at 72 columns :(
Also the changes line goes below the --- line, as documented in the
kernel documentation on how to submit a patch.
thanks,
greg k-h
On Mon, Nov 29, 2021 at 07:42:39AM +0000, Aditya Garg wrote:
> From: Aditya Garg <[email protected]>
>
> Some devices have a bug causing them to not work if they query LE tx power on startup. Thus we add a
> quirk in order to not query it and default min/max tx power values to HCI_TX_POWER_INVALID.
>
> v2: Wrap the changeling at 72 columns, correct email and remove tested by.
>
> Signed-off-by: Aditya Garg <[email protected]>
> ---
> include/net/bluetooth/hci.h | 9 +++++++++
> net/bluetooth/hci_core.c | 3 ++-
> 2 files changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> index 63065bc01b766c..383342efcdc464 100644
> --- a/include/net/bluetooth/hci.h
> +++ b/include/net/bluetooth/hci.h
> @@ -246,6 +246,15 @@ enum {
> * HCI after resume.
> */
> HCI_QUIRK_NO_SUSPEND_NOTIFIER,
> +
> + /*
> + * When this quirk is set, LE tx power is not queried on startup
> + * and the min/max tx power values default to HCI_TX_POWER_INVALID.
> + *
> + * This quirk can be set before hci_register_dev is called or
> + * during the hdev->setup vendor callback.
> + */
> + HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER,
> };
>
> /* HCI device flags */
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index 8d33aa64846b1c..434c6878fe9640 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -619,7 +619,8 @@ static int hci_init3_req(struct hci_request *req, unsigned long opt)
> hci_req_add(req, HCI_OP_LE_READ_ADV_TX_POWER, 0, NULL);
> }
>
> - if (hdev->commands[38] & 0x80) {
> + if (hdev->commands[38] & 0x80 &&
> + !test_bit(HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER, &hdev->quirks)) {
You did not fix this formatting? Why not?
thanks,
greg k-h
> On 29-Nov-2021, at 1:17 PM, Greg KH <[email protected]> wrote:
>
> On Mon, Nov 29, 2021 at 07:42:39AM +0000, Aditya Garg wrote:
>> From: Aditya Garg <[email protected]>
>>
>> Some devices have a bug causing them to not work if they query LE tx power on startup. Thus we add a
>> quirk in order to not query it and default min/max tx power values to HCI_TX_POWER_INVALID.
>>
>> v2: Wrap the changeling at 72 columns, correct email and remove tested by.
>
> These lines are not wrapped at 72 columns :(
If I am not wrong, you mean that there should be 72 characters in one line right?
>
> Also the changes line goes below the --- line, as documented in the
> kernel documentation on how to submit a patch.
>
> thanks,
>
> greg k-h
Hi Aditya,
> Some devices have a bug causing them to not work if they query LE tx power on startup. Thus we add a
> quirk in order to not query it and default min/max tx power values to HCI_TX_POWER_INVALID.
>
> v2: Wrap the changeling at 72 columns, correct email and remove tested by.
that part is for the reviewer and needs to go after ---. Otherwise please break
at 72 characters.
>
> Signed-off-by: Aditya Garg <[email protected]>
> ---
> include/net/bluetooth/hci.h | 9 +++++++++
> net/bluetooth/hci_core.c | 3 ++-
> 2 files changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> index 63065bc01b766c..383342efcdc464 100644
> --- a/include/net/bluetooth/hci.h
> +++ b/include/net/bluetooth/hci.h
> @@ -246,6 +246,15 @@ enum {
> * HCI after resume.
> */
> HCI_QUIRK_NO_SUSPEND_NOTIFIER,
> +
> + /*
> + * When this quirk is set, LE tx power is not queried on startup
> + * and the min/max tx power values default to HCI_TX_POWER_INVALID.
> + *
> + * This quirk can be set before hci_register_dev is called or
> + * during the hdev->setup vendor callback.
> + */
> + HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER,
> };
>
> /* HCI device flags */
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index 8d33aa64846b1c..434c6878fe9640 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -619,7 +619,8 @@ static int hci_init3_req(struct hci_request *req, unsigned long opt)
> hci_req_add(req, HCI_OP_LE_READ_ADV_TX_POWER, 0, NULL);
> }
>
> - if (hdev->commands[38] & 0x80) {
> + if (hdev->commands[38] & 0x80 &&
> + !test_bit(HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER, &hdev->quirks)) {
if ((hdev->commands[38] & 0x80) &&
!test_bit(HCI_QUIRK_.., &hdev->quirks)) {
> /* Read LE Min/Max Tx Power*/
> hci_req_add(req, HCI_OP_LE_READ_TRANSMIT_POWER,
> 0, NULL);
>
Regards
Marcel
Hi Aditya,
> Bluetooth on Apple MacBook Pro 16,1 is unable to start due to LE Min/Max Tx Power being queried on startup. Add a DMI based quirk so that it is disabled.
list all the MacBooks that you found problematic right now. We add the
initial as a large batch instead of all individual.
>
> Signed-off-by: Aditya Garg <[email protected]>
> Tested-by: Aditya Garg <[email protected]>
> ---
> drivers/bluetooth/btbcm.c | 20 ++++++++++++++++++++
> 1 file changed, 20 insertions(+)
>
> diff --git a/drivers/bluetooth/btbcm.c b/drivers/bluetooth/btbcm.c
> index e4182acee488c5..c1b0ca63880a68 100644
> --- a/drivers/bluetooth/btbcm.c
> +++ b/drivers/bluetooth/btbcm.c
> @@ -8,6 +8,7 @@
>
> #include <linux/module.h>
> #include <linux/firmware.h>
> +#include <linux/dmi.h>
> #include <asm/unaligned.h>
>
> #include <net/bluetooth/bluetooth.h>
> @@ -343,9 +344,23 @@ static struct sk_buff *btbcm_read_usb_product(struct hci_dev *hdev)
> return skb;
> }
>
> +static const struct dmi_system_id disable_broken_read_transmit_power[] = {
> + {
> + /* Match for Apple MacBook Pro 16,1 which needs
> + * Read LE Min/Max Tx Power to be disabled.
> + */
Actually leave the comment out. You are not adding any value that isn’t
already in the variable name or the DMI. It is just repeating the obvious.
> + .matches = {
> + DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
> + DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro16,1"),
> + },
> + },
> + { }
> +};
> +
> static int btbcm_read_info(struct hci_dev *hdev)
> {
> struct sk_buff *skb;
> + const struct dmi_system_id *dmi_id;
>
> /* Read Verbose Config Version Info */
> skb = btbcm_read_verbose_config(hdev);
> @@ -362,6 +377,11 @@ static int btbcm_read_info(struct hci_dev *hdev)
>
> bt_dev_info(hdev, "BCM: features 0x%2.2x", skb->data[1]);
> kfree_skb(skb);
> +
> + /* Read DMI and disable broken Read LE Min/Max Tx Power */
> + dmi_id = dmi_first_match(disable_broken_read_transmit_power);
> + if (dmi_id)
> + set_bit(HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER, &hdev->quirks);
if (dmi_first_match(..))
set_bit(.., &hdev->quirks);
There is really no need to have a variable for this.
Regards
Marcel
> On 29-Nov-2021, at 1:38 PM, Marcel Holtmann <[email protected]> wrote:
>
> Hi Aditya,
>
>> Bluetooth on Apple MacBook Pro 16,1 is unable to start due to LE Min/Max Tx Power being queried on startup. Add a DMI based quirk so that it is disabled.
>
> list all the MacBooks that you found problematic right now. We add the
> initial as a large batch instead of all individual.
>
>>
>> Signed-off-by: Aditya Garg <[email protected]>
>> Tested-by: Aditya Garg <[email protected]>
>> ---
>> drivers/bluetooth/btbcm.c | 20 ++++++++++++++++++++
>> 1 file changed, 20 insertions(+)
>>
>> diff --git a/drivers/bluetooth/btbcm.c b/drivers/bluetooth/btbcm.c
>> index e4182acee488c5..c1b0ca63880a68 100644
>> --- a/drivers/bluetooth/btbcm.c
>> +++ b/drivers/bluetooth/btbcm.c
>> @@ -8,6 +8,7 @@
>>
>> #include <linux/module.h>
>> #include <linux/firmware.h>
>> +#include <linux/dmi.h>
>> #include <asm/unaligned.h>
>>
>> #include <net/bluetooth/bluetooth.h>
>> @@ -343,9 +344,23 @@ static struct sk_buff *btbcm_read_usb_product(struct hci_dev *hdev)
>> return skb;
>> }
>>
>> +static const struct dmi_system_id disable_broken_read_transmit_power[] = {
>> + {
>> + /* Match for Apple MacBook Pro 16,1 which needs
>> + * Read LE Min/Max Tx Power to be disabled.
>> + */
>
> Actually leave the comment out. You are not adding any value that isn’t
> already in the variable name or the DMI. It is just repeating the obvious.
Alright, I prepare the patches into a single one
>
>> + .matches = {
>> + DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
>> + DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro16,1"),
>> + },
>> + },
>> + { }
>> +};
>> +
>> static int btbcm_read_info(struct hci_dev *hdev)
>> {
>> struct sk_buff *skb;
>> + const struct dmi_system_id *dmi_id;
>>
>> /* Read Verbose Config Version Info */
>> skb = btbcm_read_verbose_config(hdev);
>> @@ -362,6 +377,11 @@ static int btbcm_read_info(struct hci_dev *hdev)
>>
>> bt_dev_info(hdev, "BCM: features 0x%2.2x", skb->data[1]);
>> kfree_skb(skb);
>> +
>> + /* Read DMI and disable broken Read LE Min/Max Tx Power */
>> + dmi_id = dmi_first_match(disable_broken_read_transmit_power);
>> + if (dmi_id)
>> + set_bit(HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER, &hdev->quirks);
>
> if (dmi_first_match(..))
> set_bit(.., &hdev->quirks);
>
> There is really no need to have a variable for this.
Fine, Ill correct this
>
> Regards
>
> Marcel
>
Hi Aditya,
>>> Bluetooth on Apple MacBook Pro 16,1 is unable to start due to LE Min/Max Tx Power being queried on startup. Add a DMI based quirk so that it is disabled.
>>
>> list all the MacBooks that you found problematic right now. We add the
>> initial as a large batch instead of all individual.
>>
>>>
>>> Signed-off-by: Aditya Garg <[email protected]>
>>> Tested-by: Aditya Garg <[email protected]>
>>> ---
>>> drivers/bluetooth/btbcm.c | 20 ++++++++++++++++++++
>>> 1 file changed, 20 insertions(+)
>>>
>>> diff --git a/drivers/bluetooth/btbcm.c b/drivers/bluetooth/btbcm.c
>>> index e4182acee488c5..c1b0ca63880a68 100644
>>> --- a/drivers/bluetooth/btbcm.c
>>> +++ b/drivers/bluetooth/btbcm.c
>>> @@ -8,6 +8,7 @@
>>>
>>> #include <linux/module.h>
>>> #include <linux/firmware.h>
>>> +#include <linux/dmi.h>
>>> #include <asm/unaligned.h>
>>>
>>> #include <net/bluetooth/bluetooth.h>
>>> @@ -343,9 +344,23 @@ static struct sk_buff *btbcm_read_usb_product(struct hci_dev *hdev)
>>> return skb;
>>> }
>>>
>>> +static const struct dmi_system_id disable_broken_read_transmit_power[] = {
>>> + {
>>> + /* Match for Apple MacBook Pro 16,1 which needs
>>> + * Read LE Min/Max Tx Power to be disabled.
>>> + */
>>
>> Actually leave the comment out. You are not adding any value that isn’t
>> already in the variable name or the DMI. It is just repeating the obvious.
> Alright, I prepare the patches into a single one
two patches, one for adding the quirk to the core and one for adjusting the driver.
Regards
Marcel
From: Aditya Garg <[email protected]>
Some devices have a bug causing them to not work if they query
LE tx power on startup. Thus we add a quirk in order to not query it
and default min/max tx power values to HCI_TX_POWER_INVALID.
Signed-off-by: Aditya Garg <[email protected]>
---
include/net/bluetooth/hci.h | 9 +++++++++
net/bluetooth/hci_core.c | 3 ++-
2 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 63065bc01b766c..383342efcdc464 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -246,6 +246,15 @@ enum {
* HCI after resume.
*/
HCI_QUIRK_NO_SUSPEND_NOTIFIER,
+
+ /*
+ * When this quirk is set, LE tx power is not queried on startup
+ * and the min/max tx power values default to HCI_TX_POWER_INVALID.
+ *
+ * This quirk can be set before hci_register_dev is called or
+ * during the hdev->setup vendor callback.
+ */
+ HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER,
};
/* HCI device flags */
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 8d33aa64846b1c..434c6878fe9640 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -619,7 +619,8 @@ static int hci_init3_req(struct hci_request *req, unsigned long opt)
hci_req_add(req, HCI_OP_LE_READ_ADV_TX_POWER, 0, NULL);
}
- if (hdev->commands[38] & 0x80) {
+ if (hdev->commands[38] & 0x80 &&
+ !test_bit(HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER, &hdev->quirks)) {
/* Read LE Min/Max Tx Power*/
hci_req_add(req, HCI_OP_LE_READ_TRANSMIT_POWER,
0, NULL);
From: Aditya Garg <[email protected]>
Some Macs with the T2 security chip had Bluetooth not working.
To fix it we add DMI based quirks to disable querying of LE Tx power.
Signed-off-by: Aditya Garg <[email protected]>
---
drivers/bluetooth/btbcm.c | 40 +++++++++++++++++++++++++++++++++++++++
1 file changed, 40 insertions(+)
diff --git a/drivers/bluetooth/btbcm.c b/drivers/bluetooth/btbcm.c
index e4182acee488c5..40f7c9c5cf0a5a 100644
--- a/drivers/bluetooth/btbcm.c
+++ b/drivers/bluetooth/btbcm.c
@@ -8,6 +8,7 @@
#include <linux/module.h>
#include <linux/firmware.h>
+#include <linux/dmi.h>
#include <asm/unaligned.h>
#include <net/bluetooth/bluetooth.h>
@@ -343,9 +344,44 @@ static struct sk_buff *btbcm_read_usb_product(struct hci_dev *hdev)
return skb;
}
+static const struct dmi_system_id disable_broken_read_transmit_power[] = {
+ {
+ .matches = {
+ DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro16,1"),
+ },
+ },
+ {
+ .matches = {
+ DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro16,2"),
+ },
+ },
+ {
+ .matches = {
+ DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro16,4"),
+ },
+ },
+ {
+ .matches = {
+ DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "iMac20,1"),
+ },
+ },
+ {
+ .matches = {
+ DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "iMac20,2"),
+ },
+ },
+ { }
+};
+
static int btbcm_read_info(struct hci_dev *hdev)
{
struct sk_buff *skb;
+ const struct dmi_system_id *dmi_id;
/* Read Verbose Config Version Info */
skb = btbcm_read_verbose_config(hdev);
@@ -363,6 +399,10 @@ static int btbcm_read_info(struct hci_dev *hdev)
bt_dev_info(hdev, "BCM: features 0x%2.2x", skb->data[1]);
kfree_skb(skb);
+ /* Read DMI and disable broken Read LE Min/Max Tx Power */
+ if (dmi_first_match(disable_broken_read_transmit_power))
+ set_bit(HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER, &hdev->quirks);
+
return 0;
}
> On 29-Nov-2021, at 1:52 PM, Marcel Holtmann <[email protected]> wrote:
>
> Hi Aditya,
>
>>>> Bluetooth on Apple MacBook Pro 16,1 is unable to start due to LE Min/Max Tx Power being queried on startup. Add a DMI based quirk so that it is disabled.
>>>
>>> list all the MacBooks that you found problematic right now. We add the
>>> initial as a large batch instead of all individual.
>>>
>>>>
>>>> Signed-off-by: Aditya Garg <[email protected]>
>>>> Tested-by: Aditya Garg <[email protected]>
>>>> ---
>>>> drivers/bluetooth/btbcm.c | 20 ++++++++++++++++++++
>>>> 1 file changed, 20 insertions(+)
>>>>
>>>> diff --git a/drivers/bluetooth/btbcm.c b/drivers/bluetooth/btbcm.c
>>>> index e4182acee488c5..c1b0ca63880a68 100644
>>>> --- a/drivers/bluetooth/btbcm.c
>>>> +++ b/drivers/bluetooth/btbcm.c
>>>> @@ -8,6 +8,7 @@
>>>>
>>>> #include <linux/module.h>
>>>> #include <linux/firmware.h>
>>>> +#include <linux/dmi.h>
>>>> #include <asm/unaligned.h>
>>>>
>>>> #include <net/bluetooth/bluetooth.h>
>>>> @@ -343,9 +344,23 @@ static struct sk_buff *btbcm_read_usb_product(struct hci_dev *hdev)
>>>> return skb;
>>>> }
>>>>
>>>> +static const struct dmi_system_id disable_broken_read_transmit_power[] = {
>>>> + {
>>>> + /* Match for Apple MacBook Pro 16,1 which needs
>>>> + * Read LE Min/Max Tx Power to be disabled.
>>>> + */
>>>
>>> Actually leave the comment out. You are not adding any value that isn’t
>>> already in the variable name or the DMI. It is just repeating the obvious.
>> Alright, I prepare the patches into a single one
>
> two patches, one for adding the quirk to the core and one for adjusting the driver.
Sent
>
> Regards
>
> Marcel
>
From: Aditya Garg <[email protected]>
Some devices have a bug causing them to not work if they query
LE tx power on startup. Thus we add a quirk in order to not query it
and default min/max tx power values to HCI_TX_POWER_INVALID.
Signed-off-by: Aditya Garg <[email protected]>
---
include/net/bluetooth/hci.h | 9 +++++++++
net/bluetooth/hci_core.c | 3 ++-
2 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 63065bc01b766c..383342efcdc464 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -246,6 +246,15 @@ enum {
* HCI after resume.
*/
HCI_QUIRK_NO_SUSPEND_NOTIFIER,
+
+ /*
+ * When this quirk is set, LE tx power is not queried on startup
+ * and the min/max tx power values default to HCI_TX_POWER_INVALID.
+ *
+ * This quirk can be set before hci_register_dev is called or
+ * during the hdev->setup vendor callback.
+ */
+ HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER,
};
/* HCI device flags */
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 8d33aa64846b1c..434c6878fe9640 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -619,7 +619,8 @@ static int hci_init3_req(struct hci_request *req, unsigned long opt)
hci_req_add(req, HCI_OP_LE_READ_ADV_TX_POWER, 0, NULL);
}
- if (hdev->commands[38] & 0x80) {
+ if (hdev->commands[38] & 0x80 &&
+ !test_bit(HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER, &hdev->quirks)) {
/* Read LE Min/Max Tx Power*/
hci_req_add(req, HCI_OP_LE_READ_TRANSMIT_POWER,
0, NULL);
From: Aditya Garg <[email protected]>
Some Macs with the T2 security chip had Bluetooth not working.
To fix it we add DMI based quirks to disable querying of LE Tx power.
Signed-off-by: Aditya Garg <[email protected]>
---
drivers/bluetooth/btbcm.c | 40 +++++++++++++++++++++++++++++++++++++++
1 file changed, 40 insertions(+)
diff --git a/drivers/bluetooth/btbcm.c b/drivers/bluetooth/btbcm.c
index e4182acee488c5..40f7c9c5cf0a5a 100644
--- a/drivers/bluetooth/btbcm.c
+++ b/drivers/bluetooth/btbcm.c
@@ -8,6 +8,7 @@
#include <linux/module.h>
#include <linux/firmware.h>
+#include <linux/dmi.h>
#include <asm/unaligned.h>
#include <net/bluetooth/bluetooth.h>
@@ -343,9 +344,44 @@ static struct sk_buff *btbcm_read_usb_product(struct hci_dev *hdev)
return skb;
}
+static const struct dmi_system_id disable_broken_read_transmit_power[] = {
+ {
+ .matches = {
+ DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro16,1"),
+ },
+ },
+ {
+ .matches = {
+ DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro16,2"),
+ },
+ },
+ {
+ .matches = {
+ DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro16,4"),
+ },
+ },
+ {
+ .matches = {
+ DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "iMac20,1"),
+ },
+ },
+ {
+ .matches = {
+ DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "iMac20,2"),
+ },
+ },
+ { }
+};
+
static int btbcm_read_info(struct hci_dev *hdev)
{
struct sk_buff *skb;
+ const struct dmi_system_id *dmi_id;
/* Read Verbose Config Version Info */
skb = btbcm_read_verbose_config(hdev);
@@ -363,6 +399,10 @@ static int btbcm_read_info(struct hci_dev *hdev)
bt_dev_info(hdev, "BCM: features 0x%2.2x", skb->data[1]);
kfree_skb(skb);
+ /* Read DMI and disable broken Read LE Min/Max Tx Power */
+ if (dmi_first_match(disable_broken_read_transmit_power))
+ set_bit(HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER, &hdev->quirks);
+
return 0;
}
From: Aditya Garg <[email protected]>
Some devices have a bug causing them to not work if they query
LE tx power on startup. Thus we add a quirk in order to not query it
and default min/max tx power values to HCI_TX_POWER_INVALID.
Signed-off-by: Aditya Garg <[email protected]>
Reported-by: Orlando Chamberlain <[email protected]>
Link:
https://lore.kernel.org/r/[email protected]
Fixes: 7c395ea521e6 ("Bluetooth: Query LE tx power on startup")
---
include/net/bluetooth/hci.h | 9 +++++++++
net/bluetooth/hci_core.c | 3 ++-
2 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 63065bc01b766c..383342efcdc464 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -246,6 +246,15 @@ enum {
* HCI after resume.
*/
HCI_QUIRK_NO_SUSPEND_NOTIFIER,
+
+ /*
+ * When this quirk is set, LE tx power is not queried on startup
+ * and the min/max tx power values default to HCI_TX_POWER_INVALID.
+ *
+ * This quirk can be set before hci_register_dev is called or
+ * during the hdev->setup vendor callback.
+ */
+ HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER,
};
/* HCI device flags */
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 8d33aa64846b1c..434c6878fe9640 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -619,7 +619,8 @@ static int hci_init3_req(struct hci_request *req, unsigned long opt)
hci_req_add(req, HCI_OP_LE_READ_ADV_TX_POWER, 0, NULL);
}
- if (hdev->commands[38] & 0x80) {
+ if (hdev->commands[38] & 0x80 &&
+ !test_bit(HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER, &hdev->quirks)) {
/* Read LE Min/Max Tx Power*/
hci_req_add(req, HCI_OP_LE_READ_TRANSMIT_POWER,
0, NULL);
Hi! Great to see progress for this regression.
On 29.11.21 09:32, Aditya Garg wrote:
> From: Aditya Garg <[email protected]>
>
> Some devices have a bug causing them to not work if they query
> LE tx power on startup. Thus we add a quirk in order to not query it
> and default min/max tx power values to HCI_TX_POWER_INVALID.
>
> Signed-off-by: Aditya Garg <[email protected]>
> ---
FWIW: In case you need to send an improved patch, could you please add
this after your 'Signed-off-by:' (see at (¹) below for the reasoning):
Reported-by: Orlando Chamberlain <[email protected]>
Link:
https://lore.kernel.org/r/[email protected]
Fixes: 7c395ea521e6 ("Bluetooth: Query LE tx power on startup")
And if the patch is already good to go: could the subsystem maintainer
please add it when applying? See (¹) for the reasoning.
Ciao, Thorsten, your Linux kernel regression tracker.
(¹) Long story: The commit message would benefit from a link to the
regression report, for reasons explained in
Documentation/process/submitting-patches.rst. To quote:
```
If related discussions or any other background information behind the
change can be found on the web, add 'Link:' tags pointing to it. In case
your patch fixes a bug, for example, add a tag with a URL referencing
the report in the mailing list archives or a bug tracker;
```
This concept is old, but the text was reworked recently to make this use
case for the Link: tag clearer. For details see:
https://git.kernel.org/linus/1f57bd42b77c
Yes, that "Link:" is not really crucial; but it's good to have if
someone needs to look into the backstory of this change sometime in the
future. But I care for a different reason. I'm tracking this regression
(and others) with regzbot, my Linux kernel regression tracking bot. This
bot will notice if a patch with a Link: tag to a tracked regression gets
posted and record that, which allowed anyone looking into the regression
to quickly gasp the current status from regzbot's webui
(https://linux-regtracking.leemhuis.info/regzbot ) or its reports. The
bot will also notice if a commit with a Link: tag to a regression report
is applied by Linus and then automatically mark the regression as
resolved then.
IOW: this tag makes my life a regression tracker a lot easier, as I
otherwise have to tell regzbot manually when the fix lands. :-/
P.S.: As a Linux kernel regression tracker I'm getting a lot of reports
on my table. I can only look briefly into most of them. Unfortunately
therefore I sometimes will get things wrong or miss something important.
I hope that's not the case here; if you think it is, don't hesitate to
tell me about it in a public reply. That's in everyone's interest, as
what I wrote above might be misleading to everyone reading this; any
suggestion I gave they thus might sent someone reading this down the
wrong rabbit hole, which none of us wants.
BTW, I have no personal interest in this issue, which is tracked using
regzbot, my Linux kernel regression tracking bot
(https://linux-regtracking.leemhuis.info/regzbot/). I'm only posting
this mail to get things rolling again and hence don't need to be CC on
all further activities wrt to this regression.
From: Aditya Garg <[email protected]>
Some Macs with the T2 security chip had Bluetooth not working.
To fix it we add DMI based quirks to disable querying of LE Tx power.
Signed-off-by: Aditya Garg <[email protected]>
Reported-by: Orlando Chamberlain <[email protected]>
Link:
https://lore.kernel.org/r/[email protected]
Fixes: 7c395ea521e6 ("Bluetooth: Query LE tx power on startup")
---
drivers/bluetooth/btbcm.c | 40 +++++++++++++++++++++++++++++++++++++++
1 file changed, 40 insertions(+)
diff --git a/drivers/bluetooth/btbcm.c b/drivers/bluetooth/btbcm.c
index e4182acee488c5..40f7c9c5cf0a5a 100644
--- a/drivers/bluetooth/btbcm.c
+++ b/drivers/bluetooth/btbcm.c
@@ -8,6 +8,7 @@
#include <linux/module.h>
#include <linux/firmware.h>
+#include <linux/dmi.h>
#include <asm/unaligned.h>
#include <net/bluetooth/bluetooth.h>
@@ -343,9 +344,44 @@ static struct sk_buff *btbcm_read_usb_product(struct hci_dev *hdev)
return skb;
}
+static const struct dmi_system_id disable_broken_read_transmit_power[] = {
+ {
+ .matches = {
+ DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro16,1"),
+ },
+ },
+ {
+ .matches = {
+ DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro16,2"),
+ },
+ },
+ {
+ .matches = {
+ DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro16,4"),
+ },
+ },
+ {
+ .matches = {
+ DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "iMac20,1"),
+ },
+ },
+ {
+ .matches = {
+ DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "iMac20,2"),
+ },
+ },
+ { }
+};
+
static int btbcm_read_info(struct hci_dev *hdev)
{
struct sk_buff *skb;
+ const struct dmi_system_id *dmi_id;
/* Read Verbose Config Version Info */
skb = btbcm_read_verbose_config(hdev);
@@ -363,6 +399,10 @@ static int btbcm_read_info(struct hci_dev *hdev)
bt_dev_info(hdev, "BCM: features 0x%2.2x", skb->data[1]);
kfree_skb(skb);
+ /* Read DMI and disable broken Read LE Min/Max Tx Power */
+ if (dmi_first_match(disable_broken_read_transmit_power))
+ set_bit(HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER, &hdev->quirks);
+
return 0;
}
From: Aditya Garg <[email protected]>
Some devices have a bug causing them to not work if they query
LE tx power on startup. Thus we add a quirk in order to not query it
and default min/max tx power values to HCI_TX_POWER_INVALID.
Signed-off-by: Aditya Garg <[email protected]>
Reported-by: Orlando Chamberlain <[email protected]>
Link:
https://lore.kernel.org/r/[email protected]
Fixes: 7c395ea521e6 ("Bluetooth: Query LE tx power on startup")
---
include/net/bluetooth/hci.h | 9 +++++++++
net/bluetooth/hci_core.c | 3 ++-
2 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 63065bc01b766c..383342efcdc464 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -246,6 +246,15 @@ enum {
* HCI after resume.
*/
HCI_QUIRK_NO_SUSPEND_NOTIFIER,
+
+ /*
+ * When this quirk is set, LE tx power is not queried on startup
+ * and the min/max tx power values default to HCI_TX_POWER_INVALID.
+ *
+ * This quirk can be set before hci_register_dev is called or
+ * during the hdev->setup vendor callback.
+ */
+ HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER,
};
/* HCI device flags */
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 8d33aa64846b1c..434c6878fe9640 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -619,7 +619,8 @@ static int hci_init3_req(struct hci_request *req, unsigned long opt)
hci_req_add(req, HCI_OP_LE_READ_ADV_TX_POWER, 0, NULL);
}
- if (hdev->commands[38] & 0x80) {
+ if (hdev->commands[38] & 0x80 &&
+ !test_bit(HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER, &hdev->quirks)) {
/* Read LE Min/Max Tx Power*/
hci_req_add(req, HCI_OP_LE_READ_TRANSMIT_POWER,
0, NULL);
From: Aditya Garg <[email protected]>
Some Macs with the T2 security chip had Bluetooth not working.
To fix it we add DMI based quirks to disable querying of LE Tx power.
Signed-off-by: Aditya Garg <[email protected]>
Reported-by: Orlando Chamberlain <[email protected]>
Link:
https://lore.kernel.org/r/[email protected]
Fixes: 7c395ea521e6 ("Bluetooth: Query LE tx power on startup")
---
drivers/bluetooth/btbcm.c | 40 +++++++++++++++++++++++++++++++++++++++
1 file changed, 40 insertions(+)
diff --git a/drivers/bluetooth/btbcm.c b/drivers/bluetooth/btbcm.c
index e4182acee488c5..40f7c9c5cf0a5a 100644
--- a/drivers/bluetooth/btbcm.c
+++ b/drivers/bluetooth/btbcm.c
@@ -8,6 +8,7 @@
#include <linux/module.h>
#include <linux/firmware.h>
+#include <linux/dmi.h>
#include <asm/unaligned.h>
#include <net/bluetooth/bluetooth.h>
@@ -343,9 +344,44 @@ static struct sk_buff *btbcm_read_usb_product(struct hci_dev *hdev)
return skb;
}
+static const struct dmi_system_id disable_broken_read_transmit_power[] = {
+ {
+ .matches = {
+ DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro16,1"),
+ },
+ },
+ {
+ .matches = {
+ DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro16,2"),
+ },
+ },
+ {
+ .matches = {
+ DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro16,4"),
+ },
+ },
+ {
+ .matches = {
+ DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "iMac20,1"),
+ },
+ },
+ {
+ .matches = {
+ DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "iMac20,2"),
+ },
+ },
+ { }
+};
+
static int btbcm_read_info(struct hci_dev *hdev)
{
struct sk_buff *skb;
+ const struct dmi_system_id *dmi_id;
/* Read Verbose Config Version Info */
skb = btbcm_read_verbose_config(hdev);
@@ -363,6 +399,10 @@ static int btbcm_read_info(struct hci_dev *hdev)
bt_dev_info(hdev, "BCM: features 0x%2.2x", skb->data[1]);
kfree_skb(skb);
+ /* Read DMI and disable broken Read LE Min/Max Tx Power */
+ if (dmi_first_match(disable_broken_read_transmit_power))
+ set_bit(HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER, &hdev->quirks);
+
return 0;
}
Hi Aditya,
> Some devices have a bug causing them to not work if they query
> LE tx power on startup. Thus we add a quirk in order to not query it
> and default min/max tx power values to HCI_TX_POWER_INVALID.
>
> Signed-off-by: Aditya Garg <[email protected]>
> ---
> include/net/bluetooth/hci.h | 9 +++++++++
> net/bluetooth/hci_core.c | 3 ++-
> 2 files changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> index 63065bc01b766c..383342efcdc464 100644
> --- a/include/net/bluetooth/hci.h
> +++ b/include/net/bluetooth/hci.h
> @@ -246,6 +246,15 @@ enum {
> * HCI after resume.
> */
> HCI_QUIRK_NO_SUSPEND_NOTIFIER,
> +
> + /*
> + * When this quirk is set, LE tx power is not queried on startup
> + * and the min/max tx power values default to HCI_TX_POWER_INVALID.
> + *
> + * This quirk can be set before hci_register_dev is called or
> + * during the hdev->setup vendor callback.
> + */
> + HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER,
> };
>
> /* HCI device flags */
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index 8d33aa64846b1c..434c6878fe9640 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -619,7 +619,8 @@ static int hci_init3_req(struct hci_request *req, unsigned long opt)
> hci_req_add(req, HCI_OP_LE_READ_ADV_TX_POWER, 0, NULL);
> }
>
> - if (hdev->commands[38] & 0x80) {
> + if (hdev->commands[38] & 0x80 &&
> + !test_bit(HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER, &hdev->quirks)) {
if ((hdev->commands[38] && 0x80) &&
!test_bit(HCI_QUIRK_.., &hdev->quirks)) {
> /* Read LE Min/Max Tx Power*/
> hci_req_add(req, HCI_OP_LE_READ_TRANSMIT_POWER,
> 0, NULL);
>
Regards
Marcel
Hi Aditya,
> Some Macs with the T2 security chip had Bluetooth not working.
> To fix it we add DMI based quirks to disable querying of LE Tx power.
>
> Signed-off-by: Aditya Garg <[email protected]>
> ---
> drivers/bluetooth/btbcm.c | 40 +++++++++++++++++++++++++++++++++++++++
> 1 file changed, 40 insertions(+)
>
> diff --git a/drivers/bluetooth/btbcm.c b/drivers/bluetooth/btbcm.c
> index e4182acee488c5..40f7c9c5cf0a5a 100644
> --- a/drivers/bluetooth/btbcm.c
> +++ b/drivers/bluetooth/btbcm.c
> @@ -8,6 +8,7 @@
>
> #include <linux/module.h>
> #include <linux/firmware.h>
> +#include <linux/dmi.h>
> #include <asm/unaligned.h>
>
> #include <net/bluetooth/bluetooth.h>
> @@ -343,9 +344,44 @@ static struct sk_buff *btbcm_read_usb_product(struct hci_dev *hdev)
> return skb;
> }
>
> +static const struct dmi_system_id disable_broken_read_transmit_power[] = {
> + {
> + .matches = {
> + DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
> + DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro16,1"),
> + },
> + },
> + {
> + .matches = {
> + DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
> + DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro16,2"),
> + },
> + },
> + {
> + .matches = {
> + DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
> + DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro16,4"),
> + },
> + },
> + {
> + .matches = {
> + DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
> + DMI_MATCH(DMI_PRODUCT_NAME, "iMac20,1"),
> + },
> + },
> + {
> + .matches = {
> + DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
> + DMI_MATCH(DMI_PRODUCT_NAME, "iMac20,2"),
> + },
> + },
> + { }
> +};
> +
> static int btbcm_read_info(struct hci_dev *hdev)
> {
> struct sk_buff *skb;
> + const struct dmi_system_id *dmi_id;
this variable is not needed.
>
> /* Read Verbose Config Version Info */
> skb = btbcm_read_verbose_config(hdev);
> @@ -363,6 +399,10 @@ static int btbcm_read_info(struct hci_dev *hdev)
> bt_dev_info(hdev, "BCM: features 0x%2.2x", skb->data[1]);
> kfree_skb(skb);
>
> + /* Read DMI and disable broken Read LE Min/Max Tx Power */
> + if (dmi_first_match(disable_broken_read_transmit_power))
> + set_bit(HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER, &hdev->quirks);
> +
> return 0;
> }
Regards
Marcel
> On 29-Nov-2021, at 4:33 PM, Marcel Holtmann <[email protected]> wrote:
>
> Hi Aditya,
>
>> Some Macs with the T2 security chip had Bluetooth not working.
>> To fix it we add DMI based quirks to disable querying of LE Tx power.
>>
>> Signed-off-by: Aditya Garg <[email protected]>
>> ---
>> drivers/bluetooth/btbcm.c | 40 +++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 40 insertions(+)
>>
>> diff --git a/drivers/bluetooth/btbcm.c b/drivers/bluetooth/btbcm.c
>> index e4182acee488c5..40f7c9c5cf0a5a 100644
>> --- a/drivers/bluetooth/btbcm.c
>> +++ b/drivers/bluetooth/btbcm.c
>> @@ -8,6 +8,7 @@
>>
>> #include <linux/module.h>
>> #include <linux/firmware.h>
>> +#include <linux/dmi.h>
>> #include <asm/unaligned.h>
>>
>> #include <net/bluetooth/bluetooth.h>
>> @@ -343,9 +344,44 @@ static struct sk_buff *btbcm_read_usb_product(struct hci_dev *hdev)
>> return skb;
>> }
>>
>> +static const struct dmi_system_id disable_broken_read_transmit_power[] = {
>> + {
>> + .matches = {
>> + DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
>> + DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro16,1"),
>> + },
>> + },
>> + {
>> + .matches = {
>> + DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
>> + DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro16,2"),
>> + },
>> + },
>> + {
>> + .matches = {
>> + DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
>> + DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro16,4"),
>> + },
>> + },
>> + {
>> + .matches = {
>> + DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
>> + DMI_MATCH(DMI_PRODUCT_NAME, "iMac20,1"),
>> + },
>> + },
>> + {
>> + .matches = {
>> + DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
>> + DMI_MATCH(DMI_PRODUCT_NAME, "iMac20,2"),
>> + },
>> + },
>> + { }
>> +};
>> +
>> static int btbcm_read_info(struct hci_dev *hdev)
>> {
>> struct sk_buff *skb;
>> + const struct dmi_system_id *dmi_id;
>
> this variable is not needed.
You want me to replace const struct dmi_system_id *dmi_id; with const struct dmi_system_id or remove it altogether.
>>
>> /* Read Verbose Config Version Info */
>> skb = btbcm_read_verbose_config(hdev);
>> @@ -363,6 +399,10 @@ static int btbcm_read_info(struct hci_dev *hdev)
>> bt_dev_info(hdev, "BCM: features 0x%2.2x", skb->data[1]);
>> kfree_skb(skb);
>>
>> + /* Read DMI and disable broken Read LE Min/Max Tx Power */
>> + if (dmi_first_match(disable_broken_read_transmit_power))
>> + set_bit(HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER, &hdev->quirks);
>> +
>> return 0;
>> }
>
> Regards
>
> Marcel
>
From: Aditya Garg <[email protected]>
Some devices have a bug causing them to not work if they query
LE tx power on startup. Thus we add a quirk in order to not query it
and default min/max tx power values to HCI_TX_POWER_INVALID.
Signed-off-by: Aditya Garg <[email protected]>
Reported-by: Orlando Chamberlain <[email protected]>
Link:
https://lore.kernel.org/r/[email protected]
Fixes: 7c395ea521e6 ("Bluetooth: Query LE tx power on startup")
---
include/net/bluetooth/hci.h | 9 +++++++++
net/bluetooth/hci_core.c | 3 ++-
2 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 63065bc01b766c..383342efcdc464 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -246,6 +246,15 @@ enum {
* HCI after resume.
*/
HCI_QUIRK_NO_SUSPEND_NOTIFIER,
+
+ /*
+ * When this quirk is set, LE tx power is not queried on startup
+ * and the min/max tx power values default to HCI_TX_POWER_INVALID.
+ *
+ * This quirk can be set before hci_register_dev is called or
+ * during the hdev->setup vendor callback.
+ */
+ HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER,
};
/* HCI device flags */
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 8d33aa64846b1c..434c6878fe9640 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -619,7 +619,8 @@ static int hci_init3_req(struct hci_request *req, unsigned long opt)
hci_req_add(req, HCI_OP_LE_READ_ADV_TX_POWER, 0, NULL);
}
- if (hdev->commands[38] & 0x80) {
+ if ((hdev->commands[38] & 0x80) &&
+ !test_bit(HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER, &hdev->quirks)) {
/* Read LE Min/Max Tx Power*/
hci_req_add(req, HCI_OP_LE_READ_TRANSMIT_POWER,
0, NULL);
From: Aditya Garg <[email protected]>
Some Macs with the T2 security chip had Bluetooth not working.
To fix it we add DMI based quirks to disable querying of LE Tx power.
Signed-off-by: Aditya Garg <[email protected]>
Reported-by: Orlando Chamberlain <[email protected]>
Link:
https://lore.kernel.org/r/[email protected]
Fixes: 7c395ea521e6 ("Bluetooth: Query LE tx power on startup")
---
drivers/bluetooth/btbcm.c | 40 +++++++++++++++++++++++++++++++++++++++
1 file changed, 40 insertions(+)
diff --git a/drivers/bluetooth/btbcm.c b/drivers/bluetooth/btbcm.c
index e4182acee488c5..40f7c9c5cf0a5a 100644
--- a/drivers/bluetooth/btbcm.c
+++ b/drivers/bluetooth/btbcm.c
@@ -8,6 +8,7 @@
#include <linux/module.h>
#include <linux/firmware.h>
+#include <linux/dmi.h>
#include <asm/unaligned.h>
#include <net/bluetooth/bluetooth.h>
@@ -343,9 +344,44 @@ static struct sk_buff *btbcm_read_usb_product(struct hci_dev *hdev)
return skb;
}
+static const struct dmi_system_id disable_broken_read_transmit_power[] = {
+ {
+ .matches = {
+ DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro16,1"),
+ },
+ },
+ {
+ .matches = {
+ DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro16,2"),
+ },
+ },
+ {
+ .matches = {
+ DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro16,4"),
+ },
+ },
+ {
+ .matches = {
+ DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "iMac20,1"),
+ },
+ },
+ {
+ .matches = {
+ DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "iMac20,2"),
+ },
+ },
+ { }
+};
+
static int btbcm_read_info(struct hci_dev *hdev)
{
struct sk_buff *skb;
+ const struct dmi_system_id;
/* Read Verbose Config Version Info */
skb = btbcm_read_verbose_config(hdev);
@@ -363,6 +399,10 @@ static int btbcm_read_info(struct hci_dev *hdev)
bt_dev_info(hdev, "BCM: features 0x%2.2x", skb->data[1]);
kfree_skb(skb);
+ /* Read DMI and disable broken Read LE Min/Max Tx Power */
+ if (dmi_first_match(disable_broken_read_transmit_power))
+ set_bit(HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER, &hdev->quirks);
+
return 0;
}
> On 29-Nov-2021, at 7:30 PM, Aditya Garg <[email protected]> wrote:
>
> From: Aditya Garg <[email protected]>
>
> Some Macs with the T2 security chip had Bluetooth not working.
> To fix it we add DMI based quirks to disable querying of LE Tx power.
>
> Signed-off-by: Aditya Garg <[email protected]>
> Reported-by: Orlando Chamberlain <[email protected]>
> Link:
> https://lore.kernel.org/r/[email protected]
> Fixes: 7c395ea521e6 ("Bluetooth: Query LE tx power on startup")
> ---
> drivers/bluetooth/btbcm.c | 40 +++++++++++++++++++++++++++++++++++++++
> 1 file changed, 40 insertions(+)
>
> diff --git a/drivers/bluetooth/btbcm.c b/drivers/bluetooth/btbcm.c
> index e4182acee488c5..40f7c9c5cf0a5a 100644
> --- a/drivers/bluetooth/btbcm.c
> +++ b/drivers/bluetooth/btbcm.c
> @@ -8,6 +8,7 @@
>
> #include <linux/module.h>
> #include <linux/firmware.h>
> +#include <linux/dmi.h>
> #include <asm/unaligned.h>
>
> #include <net/bluetooth/bluetooth.h>
> @@ -343,9 +344,44 @@ static struct sk_buff *btbcm_read_usb_product(struct hci_dev *hdev)
> return skb;
> }
>
> +static const struct dmi_system_id disable_broken_read_transmit_power[] = {
> + {
> + .matches = {
> + DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
> + DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro16,1"),
> + },
> + },
> + {
> + .matches = {
> + DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
> + DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro16,2"),
> + },
> + },
> + {
> + .matches = {
> + DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
> + DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro16,4"),
> + },
> + },
> + {
> + .matches = {
> + DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
> + DMI_MATCH(DMI_PRODUCT_NAME, "iMac20,1"),
> + },
> + },
> + {
> + .matches = {
> + DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
> + DMI_MATCH(DMI_PRODUCT_NAME, "iMac20,2"),
> + },
> + },
> + { }
> +};
> +
> static int btbcm_read_info(struct hci_dev *hdev)
> {
> struct sk_buff *skb;
> + const struct dmi_system_id;
>
> /* Read Verbose Config Version Info */
> skb = btbcm_read_verbose_config(hdev);
> @@ -363,6 +399,10 @@ static int btbcm_read_info(struct hci_dev *hdev)
> bt_dev_info(hdev, "BCM: features 0x%2.2x", skb->data[1]);
> kfree_skb(skb);
>
> + /* Read DMI and disable broken Read LE Min/Max Tx Power */
> + if (dmi_first_match(disable_broken_read_transmit_power))
> + set_bit(HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER, &hdev->quirks);
> +
> return 0;
> }
>
May I know whether this is fine or not.
>
On Tue, Nov 30, 2021 at 08:45:44AM +0000, Aditya Garg wrote:
>
>
> > On 29-Nov-2021, at 7:30 PM, Aditya Garg <[email protected]> wrote:
> >
> > From: Aditya Garg <[email protected]>
> >
> > Some Macs with the T2 security chip had Bluetooth not working.
> > To fix it we add DMI based quirks to disable querying of LE Tx power.
> >
> > Signed-off-by: Aditya Garg <[email protected]>
> > Reported-by: Orlando Chamberlain <[email protected]>
> > Link:
> > https://lore.kernel.org/r/[email protected]
> > Fixes: 7c395ea521e6 ("Bluetooth: Query LE tx power on startup")
> > ---
> > drivers/bluetooth/btbcm.c | 40 +++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 40 insertions(+)
> >
> > diff --git a/drivers/bluetooth/btbcm.c b/drivers/bluetooth/btbcm.c
> > index e4182acee488c5..40f7c9c5cf0a5a 100644
> > --- a/drivers/bluetooth/btbcm.c
> > +++ b/drivers/bluetooth/btbcm.c
> > @@ -8,6 +8,7 @@
> >
> > #include <linux/module.h>
> > #include <linux/firmware.h>
> > +#include <linux/dmi.h>
> > #include <asm/unaligned.h>
> >
> > #include <net/bluetooth/bluetooth.h>
> > @@ -343,9 +344,44 @@ static struct sk_buff *btbcm_read_usb_product(struct hci_dev *hdev)
> > return skb;
> > }
> >
> > +static const struct dmi_system_id disable_broken_read_transmit_power[] = {
> > + {
> > + .matches = {
> > + DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
> > + DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro16,1"),
> > + },
> > + },
> > + {
> > + .matches = {
> > + DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
> > + DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro16,2"),
> > + },
> > + },
> > + {
> > + .matches = {
> > + DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
> > + DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro16,4"),
> > + },
> > + },
> > + {
> > + .matches = {
> > + DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
> > + DMI_MATCH(DMI_PRODUCT_NAME, "iMac20,1"),
> > + },
> > + },
> > + {
> > + .matches = {
> > + DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
> > + DMI_MATCH(DMI_PRODUCT_NAME, "iMac20,2"),
> > + },
> > + },
> > + { }
> > +};
> > +
> > static int btbcm_read_info(struct hci_dev *hdev)
> > {
> > struct sk_buff *skb;
> > + const struct dmi_system_id;
> >
> > /* Read Verbose Config Version Info */
> > skb = btbcm_read_verbose_config(hdev);
> > @@ -363,6 +399,10 @@ static int btbcm_read_info(struct hci_dev *hdev)
> > bt_dev_info(hdev, "BCM: features 0x%2.2x", skb->data[1]);
> > kfree_skb(skb);
> >
> > + /* Read DMI and disable broken Read LE Min/Max Tx Power */
> > + if (dmi_first_match(disable_broken_read_transmit_power))
> > + set_bit(HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER, &hdev->quirks);
> > +
> > return 0;
> > }
> >
> May I know whether this is fine or not.
Please realize that maintainers have lots and lots of patches to review.
If after 2 weeks of no response, it is fine to resend the patch again.
If you wish to help out with the maintainer's review load, please feel
free to review patches on the relevant mailing list to help lighten that
load such that your patch can be reviewed quicker.
thanks,
greg k-h
On Tue, 30 Nov 2021 01:00:43 +1100
"Aditya Garg" <[email protected]> wrote:
> From: Aditya Garg <[email protected]>
>
> Some Macs with the T2 security chip had Bluetooth not working.
> To fix it we add DMI based quirks to disable querying of LE Tx power.
>
> Signed-off-by: Aditya Garg <[email protected]>
> Reported-by: Orlando Chamberlain <[email protected]>
> Link:
> https://lore.kernel.org/r/[email protected]
> Fixes: 7c395ea521e6 ("Bluetooth: Query LE tx power on startup")
> ---
It's hard to tell what the differences between versions of this patch
are. This spot here after the "---" is often used for a change log
(e.g. "v5->v6: Made change X and change Y"), so it would be useful to
have that if you can add one in future patches. I think someone may have
mentioned this earlier.
> drivers/bluetooth/btbcm.c | 40
> +++++++++++++++++++++++++++++++++++++++ 1 file changed, 40
> insertions(+)
>
> diff --git a/drivers/bluetooth/btbcm.c b/drivers/bluetooth/btbcm.c
> index e4182acee488c5..40f7c9c5cf0a5a 100644
> --- a/drivers/bluetooth/btbcm.c
> +++ b/drivers/bluetooth/btbcm.c
> @@ -8,6 +8,7 @@
>
> #include <linux/module.h>
> #include <linux/firmware.h>
> +#include <linux/dmi.h>
> #include <asm/unaligned.h>
>
> #include <net/bluetooth/bluetooth.h>
> @@ -343,9 +344,44 @@ static struct sk_buff
> *btbcm_read_usb_product(struct hci_dev *hdev) return skb;
> }
>
> +static const struct dmi_system_id
> disable_broken_read_transmit_power[] = {
> + {
> + .matches = {
> + DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
> + DMI_MATCH(DMI_PRODUCT_NAME,
> "MacBookPro16,1"),
> + },
> + },
> + {
> + .matches = {
> + DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
> + DMI_MATCH(DMI_PRODUCT_NAME,
> "MacBookPro16,2"),
> + },
> + },
> + {
> + .matches = {
> + DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
> + DMI_MATCH(DMI_PRODUCT_NAME,
> "MacBookPro16,4"),
> + },
> + },
> + {
> + .matches = {
> + DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
> + DMI_MATCH(DMI_PRODUCT_NAME, "iMac20,1"),
> + },
> + },
> + {
> + .matches = {
> + DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
> + DMI_MATCH(DMI_PRODUCT_NAME, "iMac20,2"),
> + },
> + },
> + { }
> +};
> +
> static int btbcm_read_info(struct hci_dev *hdev)
> {
> struct sk_buff *skb;
> + const struct dmi_system_id;
This line seems to produce a compiler warning:
drivers/bluetooth/btbcm.c: In function ‘btbcm_read_info’:
drivers/bluetooth/btbcm.c:384:22: warning: empty declaration with type
qualifier does not redeclare tag
384 | const struct dmi_system_id;
| ^~~~~~~~~~~~~
I think Marcel mentioned this line could be removed.
The two patches make Bluetooth work on my MacBookPro16,1, with and without
that line.
Tested-by: Orlando Chamberlain <[email protected]>
>
> /* Read Verbose Config Version Info */
> skb = btbcm_read_verbose_config(hdev);
> @@ -363,6 +399,10 @@ static int btbcm_read_info(struct hci_dev *hdev)
> bt_dev_info(hdev, "BCM: features 0x%2.2x", skb->data[1]);
> kfree_skb(skb);
>
> + /* Read DMI and disable broken Read LE Min/Max Tx Power */
> + if (dmi_first_match(disable_broken_read_transmit_power))
> + set_bit(HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER,
> &hdev->quirks); +
> return 0;
> }
>
>
--
Thanks,
Orlando
From: Aditya Garg <[email protected]>
Some devices have a bug causing them to not work if they query
LE tx power on startup. Thus we add a quirk in order to not query it
and default min/max tx power values to HCI_TX_POWER_INVALID.
Signed-off-by: Aditya Garg <[email protected]>
Reported-by: Orlando Chamberlain <[email protected]>
Tested-by: Orlando Chamberlain <[email protected]>
Link:
https://lore.kernel.org/r/[email protected]
Fixes: 7c395ea521e6 ("Bluetooth: Query LE tx power on startup")
---
v7 :- Added Tested-by.
include/net/bluetooth/hci.h | 9 +++++++++
net/bluetooth/hci_core.c | 3 ++-
2 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 63065bc01b766c..383342efcdc464 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -246,6 +246,15 @@ enum {
* HCI after resume.
*/
HCI_QUIRK_NO_SUSPEND_NOTIFIER,
+
+ /*
+ * When this quirk is set, LE tx power is not queried on startup
+ * and the min/max tx power values default to HCI_TX_POWER_INVALID.
+ *
+ * This quirk can be set before hci_register_dev is called or
+ * during the hdev->setup vendor callback.
+ */
+ HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER,
};
/* HCI device flags */
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 8d33aa64846b1c..434c6878fe9640 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -619,7 +619,8 @@ static int hci_init3_req(struct hci_request *req, unsigned long opt)
hci_req_add(req, HCI_OP_LE_READ_ADV_TX_POWER, 0, NULL);
}
- if (hdev->commands[38] & 0x80) {
+ if ((hdev->commands[38] & 0x80) &&
+ !test_bit(HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER, &hdev->quirks)) {
/* Read LE Min/Max Tx Power*/
hci_req_add(req, HCI_OP_LE_READ_TRANSMIT_POWER,
0, NULL);
From: Aditya Garg <[email protected]>
Some Macs with the T2 security chip had Bluetooth not working.
To fix it we add DMI based quirks to disable querying of LE Tx power.
Signed-off-by: Aditya Garg <[email protected]>
Reported-by: Orlando Chamberlain <[email protected]>
Tested-by: Orlando Chamberlain <[email protected]>
Link:
https://lore.kernel.org/r/[email protected]
Fixes: 7c395ea521e6 ("Bluetooth: Query LE tx power on startup")
---
v7 :- Removed unused variable and added Tested-by.
drivers/bluetooth/btbcm.c | 39 +++++++++++++++++++++++++++++++++++++++
1 file changed, 39 insertions(+)
diff --git a/drivers/bluetooth/btbcm.c b/drivers/bluetooth/btbcm.c
index e4182acee488c5..07fabaa5aa2979 100644
--- a/drivers/bluetooth/btbcm.c
+++ b/drivers/bluetooth/btbcm.c
@@ -8,6 +8,7 @@
#include <linux/module.h>
#include <linux/firmware.h>
+#include <linux/dmi.h>
#include <asm/unaligned.h>
#include <net/bluetooth/bluetooth.h>
@@ -343,6 +344,40 @@ static struct sk_buff *btbcm_read_usb_product(struct hci_dev *hdev)
return skb;
}
+static const struct dmi_system_id disable_broken_read_transmit_power[] = {
+ {
+ .matches = {
+ DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro16,1"),
+ },
+ },
+ {
+ .matches = {
+ DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro16,2"),
+ },
+ },
+ {
+ .matches = {
+ DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro16,4"),
+ },
+ },
+ {
+ .matches = {
+ DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "iMac20,1"),
+ },
+ },
+ {
+ .matches = {
+ DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "iMac20,2"),
+ },
+ },
+ { }
+};
+
static int btbcm_read_info(struct hci_dev *hdev)
{
struct sk_buff *skb;
@@ -363,6 +398,10 @@ static int btbcm_read_info(struct hci_dev *hdev)
bt_dev_info(hdev, "BCM: features 0x%2.2x", skb->data[1]);
kfree_skb(skb);
+ /* Read DMI and disable broken Read LE Min/Max Tx Power */
+ if (dmi_first_match(disable_broken_read_transmit_power))
+ set_bit(HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER, &hdev->quirks);
+
return 0;
}
On Tue, Nov 30, 2021 at 11:38:58AM +0000, Aditya Garg wrote:
> From: Aditya Garg <[email protected]>
>
> Some devices have a bug causing them to not work if they query
> LE tx power on startup. Thus we add a quirk in order to not query it
> and default min/max tx power values to HCI_TX_POWER_INVALID.
>
> Signed-off-by: Aditya Garg <[email protected]>
> Reported-by: Orlando Chamberlain <[email protected]>
> Tested-by: Orlando Chamberlain <[email protected]>
> Link:
> https://lore.kernel.org/r/[email protected]
> Fixes: 7c395ea521e6 ("Bluetooth: Query LE tx power on startup")
> ---
> v7 :- Added Tested-by.
> include/net/bluetooth/hci.h | 9 +++++++++
> net/bluetooth/hci_core.c | 3 ++-
> 2 files changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> index 63065bc01b766c..383342efcdc464 100644
> --- a/include/net/bluetooth/hci.h
> +++ b/include/net/bluetooth/hci.h
> @@ -246,6 +246,15 @@ enum {
> * HCI after resume.
> */
> HCI_QUIRK_NO_SUSPEND_NOTIFIER,
> +
> + /*
> + * When this quirk is set, LE tx power is not queried on startup
> + * and the min/max tx power values default to HCI_TX_POWER_INVALID.
> + *
> + * This quirk can be set before hci_register_dev is called or
> + * during the hdev->setup vendor callback.
> + */
> + HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER,
> };
>
> /* HCI device flags */
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index 8d33aa64846b1c..434c6878fe9640 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -619,7 +619,8 @@ static int hci_init3_req(struct hci_request *req, unsigned long opt)
> hci_req_add(req, HCI_OP_LE_READ_ADV_TX_POWER, 0, NULL);
> }
>
> - if (hdev->commands[38] & 0x80) {
> + if ((hdev->commands[38] & 0x80) &&
> + !test_bit(HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER, &hdev->quirks)) {
This line is still not properly formatted.
Please always use scripts/checkpatch.pl to find issues like this.
thanks,
greg k-h
From: Aditya Garg <[email protected]>
Some devices have a bug causing them to not work if they query
LE tx power on startup. Thus we add a quirk in order to not query it
and default min/max tx power values to HCI_TX_POWER_INVALID.
Signed-off-by: Aditya Garg <[email protected]>
Reported-by: Orlando Chamberlain <[email protected]>
Tested-by: Orlando Chamberlain <[email protected]>
Link:
https://lore.kernel.org/r/[email protected]
Fixes: 7c395ea521e6 ("Bluetooth: Query LE tx power on startup")
---
v7 :- Added Tested-by.
include/net/bluetooth/hci.h | 9 +++++++++
net/bluetooth/hci_core.c | 3 ++-
2 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 63065bc01b766c..383342efcdc464 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -246,6 +246,15 @@ enum {
* HCI after resume.
*/
HCI_QUIRK_NO_SUSPEND_NOTIFIER,
+
+ /*
+ * When this quirk is set, LE tx power is not queried on startup
+ * and the min/max tx power values default to HCI_TX_POWER_INVALID.
+ *
+ * This quirk can be set before hci_register_dev is called or
+ * during the hdev->setup vendor callback.
+ */
+ HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER,
};
/* HCI device flags */
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 8d33aa64846b1c..434c6878fe9640 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -619,7 +619,8 @@ static int hci_init3_req(struct hci_request *req, unsigned long opt)
hci_req_add(req, HCI_OP_LE_READ_ADV_TX_POWER, 0, NULL);
}
- if (hdev->commands[38] & 0x80) {
+ if ((hdev->commands[38] & 0x80) &&
+ !test_bit(HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER, &hdev->quirks)) {
/* Read LE Min/Max Tx Power*/
hci_req_add(req, HCI_OP_LE_READ_TRANSMIT_POWER,
0, NULL);
From: Aditya Garg <[email protected]>
Some Macs with the T2 security chip had Bluetooth not working.
To fix it we add DMI based quirks to disable querying of LE Tx power.
Signed-off-by: Aditya Garg <[email protected]>
Reported-by: Orlando Chamberlain <[email protected]>
Tested-by: Orlando Chamberlain <[email protected]>
Link:
https://lore.kernel.org/r/[email protected]
Fixes: 7c395ea521e6 ("Bluetooth: Query LE tx power on startup")
---
v7 :- Removed unused variable and added Tested-by.
drivers/bluetooth/btbcm.c | 39 +++++++++++++++++++++++++++++++++++++++
1 file changed, 39 insertions(+)
diff --git a/drivers/bluetooth/btbcm.c b/drivers/bluetooth/btbcm.c
index e4182acee488c5..07fabaa5aa2979 100644
--- a/drivers/bluetooth/btbcm.c
+++ b/drivers/bluetooth/btbcm.c
@@ -8,6 +8,7 @@
#include <linux/module.h>
#include <linux/firmware.h>
+#include <linux/dmi.h>
#include <asm/unaligned.h>
#include <net/bluetooth/bluetooth.h>
@@ -343,6 +344,40 @@ static struct sk_buff *btbcm_read_usb_product(struct hci_dev *hdev)
return skb;
}
+static const struct dmi_system_id disable_broken_read_transmit_power[] = {
+ {
+ .matches = {
+ DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro16,1"),
+ },
+ },
+ {
+ .matches = {
+ DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro16,2"),
+ },
+ },
+ {
+ .matches = {
+ DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro16,4"),
+ },
+ },
+ {
+ .matches = {
+ DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "iMac20,1"),
+ },
+ },
+ {
+ .matches = {
+ DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "iMac20,2"),
+ },
+ },
+ { }
+};
+
static int btbcm_read_info(struct hci_dev *hdev)
{
struct sk_buff *skb;
@@ -363,6 +398,10 @@ static int btbcm_read_info(struct hci_dev *hdev)
bt_dev_info(hdev, "BCM: features 0x%2.2x", skb->data[1]);
kfree_skb(skb);
+ /* Read DMI and disable broken Read LE Min/Max Tx Power */
+ if (dmi_first_match(disable_broken_read_transmit_power))
+ set_bit(HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER, &hdev->quirks);
+
return 0;
}
> On 30-Nov-2021, at 5:11 PM, Greg KH <[email protected]> wrote:
>
> On Tue, Nov 30, 2021 at 11:38:58AM +0000, Aditya Garg wrote:
>> From: Aditya Garg <[email protected]>
>>
>> Some devices have a bug causing them to not work if they query
>> LE tx power on startup. Thus we add a quirk in order to not query it
>> and default min/max tx power values to HCI_TX_POWER_INVALID.
>>
>> Signed-off-by: Aditya Garg <[email protected]>
>> Reported-by: Orlando Chamberlain <[email protected]>
>> Tested-by: Orlando Chamberlain <[email protected]>
>> Link:
>> https://lore.kernel.org/r/[email protected]
>> Fixes: 7c395ea521e6 ("Bluetooth: Query LE tx power on startup")
>> ---
>> v7 :- Added Tested-by.
>> include/net/bluetooth/hci.h | 9 +++++++++
>> net/bluetooth/hci_core.c | 3 ++-
>> 2 files changed, 11 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
>> index 63065bc01b766c..383342efcdc464 100644
>> --- a/include/net/bluetooth/hci.h
>> +++ b/include/net/bluetooth/hci.h
>> @@ -246,6 +246,15 @@ enum {
>> * HCI after resume.
>> */
>> HCI_QUIRK_NO_SUSPEND_NOTIFIER,
>> +
>> + /*
>> + * When this quirk is set, LE tx power is not queried on startup
>> + * and the min/max tx power values default to HCI_TX_POWER_INVALID.
>> + *
>> + * This quirk can be set before hci_register_dev is called or
>> + * during the hdev->setup vendor callback.
>> + */
>> + HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER,
>> };
>>
>> /* HCI device flags */
>> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
>> index 8d33aa64846b1c..434c6878fe9640 100644
>> --- a/net/bluetooth/hci_core.c
>> +++ b/net/bluetooth/hci_core.c
>> @@ -619,7 +619,8 @@ static int hci_init3_req(struct hci_request *req, unsigned long opt)
>> hci_req_add(req, HCI_OP_LE_READ_ADV_TX_POWER, 0, NULL);
>> }
>>
>> - if (hdev->commands[38] & 0x80) {
>> + if ((hdev->commands[38] & 0x80) &&
>> + !test_bit(HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER, &hdev->quirks)) {
>
> This line is still not properly formatted.
I hope its fine in the resent patch. I sent the patch as HTML by mistake.
>
> Please always use scripts/checkpatch.pl to find issues like this.
>
> thanks,
>
> greg k-h
On Tue, Nov 30, 2021 at 11:48:25AM +0000, Aditya Garg wrote:
> From: Aditya Garg <[email protected]>
>
> Some devices have a bug causing them to not work if they query
> LE tx power on startup. Thus we add a quirk in order to not query it
> and default min/max tx power values to HCI_TX_POWER_INVALID.
>
> Signed-off-by: Aditya Garg <[email protected]>
> Reported-by: Orlando Chamberlain <[email protected]>
> Tested-by: Orlando Chamberlain <[email protected]>
> Link:
> https://lore.kernel.org/r/[email protected]
> Fixes: 7c395ea521e6 ("Bluetooth: Query LE tx power on startup")
> ---
> v7 :- Added Tested-by.
> include/net/bluetooth/hci.h | 9 +++++++++
> net/bluetooth/hci_core.c | 3 ++-
> 2 files changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> index 63065bc01b766c..383342efcdc464 100644
> --- a/include/net/bluetooth/hci.h
> +++ b/include/net/bluetooth/hci.h
> @@ -246,6 +246,15 @@ enum {
> * HCI after resume.
> */
> HCI_QUIRK_NO_SUSPEND_NOTIFIER,
> +
> + /*
> + * When this quirk is set, LE tx power is not queried on startup
> + * and the min/max tx power values default to HCI_TX_POWER_INVALID.
> + *
> + * This quirk can be set before hci_register_dev is called or
> + * during the hdev->setup vendor callback.
> + */
> + HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER,
> };
>
> /* HCI device flags */
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index 8d33aa64846b1c..434c6878fe9640 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -619,7 +619,8 @@ static int hci_init3_req(struct hci_request *req, unsigned long opt)
> hci_req_add(req, HCI_OP_LE_READ_ADV_TX_POWER, 0, NULL);
> }
>
> - if (hdev->commands[38] & 0x80) {
> + if ((hdev->commands[38] & 0x80) &&
> + !test_bit(HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER, &hdev->quirks)) {
I am getting tired of saying this, but PLEASE use scripts/checkpatch.pl
before you send out your patches.
If you had done so, you would see the following output in it:
CHECK: Alignment should match open parenthesis
#49: FILE: net/bluetooth/hci_core.c:623:
+ if ((hdev->commands[38] & 0x80) &&
+ !test_bit(HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER, &hdev->quirks)) {
Please fix.
> On 30-Nov-2021, at 5:33 PM, Greg KH <[email protected]> wrote:
>
> On Tue, Nov 30, 2021 at 11:48:25AM +0000, Aditya Garg wrote:
>> From: Aditya Garg <[email protected]>
>>
>> Some devices have a bug causing them to not work if they query
>> LE tx power on startup. Thus we add a quirk in order to not query it
>> and default min/max tx power values to HCI_TX_POWER_INVALID.
>>
>> Signed-off-by: Aditya Garg <[email protected]>
>> Reported-by: Orlando Chamberlain <[email protected]>
>> Tested-by: Orlando Chamberlain <[email protected]>
>> Link:
>> https://lore.kernel.org/r/[email protected]
>> Fixes: 7c395ea521e6 ("Bluetooth: Query LE tx power on startup")
>> ---
>> v7 :- Added Tested-by.
>> include/net/bluetooth/hci.h | 9 +++++++++
>> net/bluetooth/hci_core.c | 3 ++-
>> 2 files changed, 11 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
>> index 63065bc01b766c..383342efcdc464 100644
>> --- a/include/net/bluetooth/hci.h
>> +++ b/include/net/bluetooth/hci.h
>> @@ -246,6 +246,15 @@ enum {
>> * HCI after resume.
>> */
>> HCI_QUIRK_NO_SUSPEND_NOTIFIER,
>> +
>> + /*
>> + * When this quirk is set, LE tx power is not queried on startup
>> + * and the min/max tx power values default to HCI_TX_POWER_INVALID.
>> + *
>> + * This quirk can be set before hci_register_dev is called or
>> + * during the hdev->setup vendor callback.
>> + */
>> + HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER,
>> };
>>
>> /* HCI device flags */
>> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
>> index 8d33aa64846b1c..434c6878fe9640 100644
>> --- a/net/bluetooth/hci_core.c
>> +++ b/net/bluetooth/hci_core.c
>> @@ -619,7 +619,8 @@ static int hci_init3_req(struct hci_request *req, unsigned long opt)
>> hci_req_add(req, HCI_OP_LE_READ_ADV_TX_POWER, 0, NULL);
>> }
>>
>> - if (hdev->commands[38] & 0x80) {
>> + if ((hdev->commands[38] & 0x80) &&
>> + !test_bit(HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER, &hdev->quirks)) {
>
> I am getting tired of saying this, but PLEASE use scripts/checkpatch.pl
> before you send out your patches.
Sorry for offending you. The thing is that I am inexperienced in the field of submitting patches upstream. v8 shouldn't disappoint you.
>
> If you had done so, you would see the following output in it:
>
> CHECK: Alignment should match open parenthesis
> #49: FILE: net/bluetooth/hci_core.c:623:
> + if ((hdev->commands[38] & 0x80) &&
> + !test_bit(HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER, &hdev->quirks)) {
>
>
> Please fix.
From: Aditya Garg <[email protected]>
Some devices have a bug causing them to not work if they query
LE tx power on startup. Thus we add a quirk in order to not query it
and default min/max tx power values to HCI_TX_POWER_INVALID.
Signed-off-by: Aditya Garg <[email protected]>
Reported-by: Orlando Chamberlain <[email protected]>
Tested-by: Orlando Chamberlain <[email protected]>
Link:
https://lore.kernel.org/r/[email protected]
Fixes: 7c395ea521e6 ("Bluetooth: Query LE tx power on startup")
---
v7 :- Added Tested-by.
v8 :- Fix checkpatch error.
include/net/bluetooth/hci.h | 9 +++++++++
net/bluetooth/hci_core.c | 3 ++-
2 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 63065bc01b766c..383342efcdc464 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -246,6 +246,15 @@ enum {
* HCI after resume.
*/
HCI_QUIRK_NO_SUSPEND_NOTIFIER,
+
+ /*
+ * When this quirk is set, LE tx power is not queried on startup
+ * and the min/max tx power values default to HCI_TX_POWER_INVALID.
+ *
+ * This quirk can be set before hci_register_dev is called or
+ * during the hdev->setup vendor callback.
+ */
+ HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER,
};
/* HCI device flags */
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 8d33aa64846b1c..434c6878fe9640 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -619,7 +619,8 @@ static int hci_init3_req(struct hci_request *req, unsigned long opt)
hci_req_add(req, HCI_OP_LE_READ_ADV_TX_POWER, 0, NULL);
}
- if (hdev->commands[38] & 0x80) {
+ if ((hdev->commands[38] & 0x80) &&
+ !test_bit(HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER, &hdev->quirks)) {
/* Read LE Min/Max Tx Power*/
hci_req_add(req, HCI_OP_LE_READ_TRANSMIT_POWER,
0, NULL);
From: Aditya Garg <[email protected]>
Some Macs with the T2 security chip had Bluetooth not working.
To fix it we add DMI based quirks to disable querying of LE Tx power.
Signed-off-by: Aditya Garg <[email protected]>
Reported-by: Orlando Chamberlain <[email protected]>
Tested-by: Orlando Chamberlain <[email protected]>
Link:
https://lore.kernel.org/r/[email protected]
Fixes: 7c395ea521e6 ("Bluetooth: Query LE tx power on startup")
---
v7 :- Removed unused variable and added Tested-by.
v8 :- No change.
drivers/bluetooth/btbcm.c | 39 +++++++++++++++++++++++++++++++++++++++
1 file changed, 39 insertions(+)
diff --git a/drivers/bluetooth/btbcm.c b/drivers/bluetooth/btbcm.c
index e4182acee488c5..07fabaa5aa2979 100644
--- a/drivers/bluetooth/btbcm.c
+++ b/drivers/bluetooth/btbcm.c
@@ -8,6 +8,7 @@
#include <linux/module.h>
#include <linux/firmware.h>
+#include <linux/dmi.h>
#include <asm/unaligned.h>
#include <net/bluetooth/bluetooth.h>
@@ -343,6 +344,40 @@ static struct sk_buff *btbcm_read_usb_product(struct hci_dev *hdev)
return skb;
}
+static const struct dmi_system_id disable_broken_read_transmit_power[] = {
+ {
+ .matches = {
+ DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro16,1"),
+ },
+ },
+ {
+ .matches = {
+ DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro16,2"),
+ },
+ },
+ {
+ .matches = {
+ DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro16,4"),
+ },
+ },
+ {
+ .matches = {
+ DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "iMac20,1"),
+ },
+ },
+ {
+ .matches = {
+ DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "iMac20,2"),
+ },
+ },
+ { }
+};
+
static int btbcm_read_info(struct hci_dev *hdev)
{
struct sk_buff *skb;
@@ -363,6 +398,10 @@ static int btbcm_read_info(struct hci_dev *hdev)
bt_dev_info(hdev, "BCM: features 0x%2.2x", skb->data[1]);
kfree_skb(skb);
+ /* Read DMI and disable broken Read LE Min/Max Tx Power */
+ if (dmi_first_match(disable_broken_read_transmit_power))
+ set_bit(HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER, &hdev->quirks);
+
return 0;
}
Hi Aditya,
> Some devices have a bug causing them to not work if they query
> LE tx power on startup. Thus we add a quirk in order to not query it
> and default min/max tx power values to HCI_TX_POWER_INVALID.
>
> Signed-off-by: Aditya Garg <[email protected]>
> Reported-by: Orlando Chamberlain <[email protected]>
> Tested-by: Orlando Chamberlain <[email protected]>
> Link:
> https://lore.kernel.org/r/[email protected]
> Fixes: 7c395ea521e6 ("Bluetooth: Query LE tx power on startup")
> ---
> v7 :- Added Tested-by.
> v8 :- Fix checkpatch error.
> include/net/bluetooth/hci.h | 9 +++++++++
> net/bluetooth/hci_core.c | 3 ++-
> 2 files changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> index 63065bc01b766c..383342efcdc464 100644
> --- a/include/net/bluetooth/hci.h
> +++ b/include/net/bluetooth/hci.h
> @@ -246,6 +246,15 @@ enum {
> * HCI after resume.
> */
> HCI_QUIRK_NO_SUSPEND_NOTIFIER,
> +
> + /*
> + * When this quirk is set, LE tx power is not queried on startup
> + * and the min/max tx power values default to HCI_TX_POWER_INVALID.
> + *
> + * This quirk can be set before hci_register_dev is called or
> + * during the hdev->setup vendor callback.
> + */
> + HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER,
> };
>
> /* HCI device flags */
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index 8d33aa64846b1c..434c6878fe9640 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -619,7 +619,8 @@ static int hci_init3_req(struct hci_request *req, unsigned long opt)
> hci_req_add(req, HCI_OP_LE_READ_ADV_TX_POWER, 0, NULL);
> }
>
> - if (hdev->commands[38] & 0x80) {
> + if ((hdev->commands[38] & 0x80) &&
> + !test_bit(HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER, &hdev->quirks)) {
> /* Read LE Min/Max Tx Power*/
> hci_req_add(req, HCI_OP_LE_READ_TRANSMIT_POWER,
> 0, NULL);
>
while patch and indentation look good now, it doesn’t actually apply
cleanly against bluetooth-next tree. So you need to re-spin it.
Regards
Marcel
From: Aditya Garg <[email protected]>
Some devices have a bug causing them to not work if they query
LE tx power on startup. Thus we add a quirk in order to not query it
and default min/max tx power values to HCI_TX_POWER_INVALID.
Signed-off-by: Aditya Garg <[email protected]>
Reported-by: Orlando Chamberlain <[email protected]>
Tested-by: Orlando Chamberlain <[email protected]>
Link:
https://lore.kernel.org/r/[email protected]
Fixes: 7c395ea521e6 ("Bluetooth: Query LE tx power on startup")
Cc: [email protected]
---
v7 :- Added Tested-by.
v8 :- Fix checkpatch error.
v9 :- Remake patch for Bluetooth-next tree and add Cc: [email protected]
include/net/bluetooth/hci.h | 9 +++++++++
net/bluetooth/hci_sync.c | 3 ++-
2 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 0d2a92168..c4959cf9a 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -246,6 +246,15 @@ enum {
* HCI after resume.
*/
HCI_QUIRK_NO_SUSPEND_NOTIFIER,
+
+ /*
+ * When this quirk is set, LE tx power is not queried on startup
+ * and the min/max tx power values default to HCI_TX_POWER_INVALID.
+ *
+ * This quirk can be set before hci_register_dev is called or
+ * during the hdev->setup vendor callback.
+ */
+ HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER,
};
/* HCI device flags */
diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
index ad86caf41..52e6b5dae 100644
--- a/net/bluetooth/hci_sync.c
+++ b/net/bluetooth/hci_sync.c
@@ -3283,7 +3283,8 @@ static int hci_le_read_adv_tx_power_sync(struct hci_dev *hdev)
/* Read LE Min/Max Tx Power*/
static int hci_le_read_tx_power_sync(struct hci_dev *hdev)
{
- if (!(hdev->commands[38] & 0x80))
+ if (!(hdev->commands[38] & 0x80) ||
+ test_bit(HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER, &hdev->quirks))
return 0;
return __hci_cmd_sync_status(hdev, HCI_OP_LE_READ_TRANSMIT_POWER,
--
2.25.1
From: Aditya Garg <[email protected]>
Some Macs with the T2 security chip had Bluetooth not working.
To fix it we add DMI based quirks to disable querying of LE Tx power.
Signed-off-by: Aditya Garg <[email protected]>
Reported-by: Orlando Chamberlain <[email protected]>
Tested-by: Orlando Chamberlain <[email protected]>
Link:
https://lore.kernel.org/r/[email protected]
Fixes: 7c395ea521e6 ("Bluetooth: Query LE tx power on startup")
Cc: [email protected]
---
v7 :- Removed unused variable and added Tested-by.
v8 :- No change.
v9 :- Add Cc: [email protected]
drivers/bluetooth/btbcm.c | 39 +++++++++++++++++++++++++++++++++++++++
1 file changed, 39 insertions(+)
diff --git a/drivers/bluetooth/btbcm.c b/drivers/bluetooth/btbcm.c
index e4182acee488c5..07fabaa5aa2979 100644
--- a/drivers/bluetooth/btbcm.c
+++ b/drivers/bluetooth/btbcm.c
@@ -8,6 +8,7 @@
#include <linux/module.h>
#include <linux/firmware.h>
+#include <linux/dmi.h>
#include <asm/unaligned.h>
#include <net/bluetooth/bluetooth.h>
@@ -343,6 +344,40 @@ static struct sk_buff *btbcm_read_usb_product(struct hci_dev *hdev)
return skb;
}
+static const struct dmi_system_id disable_broken_read_transmit_power[] = {
+ {
+ .matches = {
+ DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro16,1"),
+ },
+ },
+ {
+ .matches = {
+ DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro16,2"),
+ },
+ },
+ {
+ .matches = {
+ DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro16,4"),
+ },
+ },
+ {
+ .matches = {
+ DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "iMac20,1"),
+ },
+ },
+ {
+ .matches = {
+ DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "iMac20,2"),
+ },
+ },
+ { }
+};
+
static int btbcm_read_info(struct hci_dev *hdev)
{
struct sk_buff *skb;
@@ -363,6 +398,10 @@ static int btbcm_read_info(struct hci_dev *hdev)
bt_dev_info(hdev, "BCM: features 0x%2.2x", skb->data[1]);
kfree_skb(skb);
+ /* Read DMI and disable broken Read LE Min/Max Tx Power */
+ if (dmi_first_match(disable_broken_read_transmit_power))
+ set_bit(HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER, &hdev->quirks);
+
return 0;
}
From: Aditya Garg <[email protected]>
Some devices have a bug causing them to not work if they query
LE tx power on startup. Thus we add a quirk in order to not query it
and default min/max tx power values to HCI_TX_POWER_INVALID.
Signed-off-by: Aditya Garg <[email protected]>
Reported-by: Orlando Chamberlain <[email protected]>
Tested-by: Orlando Chamberlain <[email protected]>
Link:
https://lore.kernel.org/r/[email protected]
Fixes: 7c395ea521e6 ("Bluetooth: Query LE tx power on startup")
Cc: [email protected]
---
v7 :- Added Tested-by.
v8 :- Fix checkpatch error.
v9 :- Remake patch for Bluetooth-next tree and add Cc: [email protected]
v10 :- Fix gitlint
include/net/bluetooth/hci.h | 9 +++++++++
net/bluetooth/hci_sync.c | 3 ++-
2 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 0d2a92168..c4959cf9a 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -246,6 +246,15 @@ enum {
* HCI after resume.
*/
HCI_QUIRK_NO_SUSPEND_NOTIFIER,
+
+ /*
+ * When this quirk is set, LE tx power is not queried on startup
+ * and the min/max tx power values default to HCI_TX_POWER_INVALID.
+ *
+ * This quirk can be set before hci_register_dev is called or
+ * during the hdev->setup vendor callback.
+ */
+ HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER,
};
/* HCI device flags */
diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
index ad86caf41..52e6b5dae 100644
--- a/net/bluetooth/hci_sync.c
+++ b/net/bluetooth/hci_sync.c
@@ -3283,7 +3283,8 @@ static int hci_le_read_adv_tx_power_sync(struct hci_dev *hdev)
/* Read LE Min/Max Tx Power*/
static int hci_le_read_tx_power_sync(struct hci_dev *hdev)
{
- if (!(hdev->commands[38] & 0x80))
+ if (!(hdev->commands[38] & 0x80) ||
+ test_bit(HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER, &hdev->quirks))
return 0;
return __hci_cmd_sync_status(hdev, HCI_OP_LE_READ_TRANSMIT_POWER,
--
2.25.1
From: Aditya Garg <[email protected]>
Some Macs with the T2 security chip had Bluetooth not working.
To fix it we add DMI based quirks to disable querying of LE Tx power.
Signed-off-by: Aditya Garg <[email protected]>
Reported-by: Orlando Chamberlain <[email protected]>
Tested-by: Orlando Chamberlain <[email protected]>
Link:
https://lore.kernel.org/r/[email protected]
Fixes: 7c395ea521e6 ("Bluetooth: Query LE tx power on startup")
Cc: [email protected]
---
v7 :- Removed unused variable and added Tested-by.
v8 :- No change.
v9 :- Add Cc: [email protected]
v10 :- Fix gitlint
drivers/bluetooth/btbcm.c | 39 +++++++++++++++++++++++++++++++++++++++
1 file changed, 39 insertions(+)
diff --git a/drivers/bluetooth/btbcm.c b/drivers/bluetooth/btbcm.c
index e4182acee488c5..07fabaa5aa2979 100644
--- a/drivers/bluetooth/btbcm.c
+++ b/drivers/bluetooth/btbcm.c
@@ -8,6 +8,7 @@
#include <linux/module.h>
#include <linux/firmware.h>
+#include <linux/dmi.h>
#include <asm/unaligned.h>
#include <net/bluetooth/bluetooth.h>
@@ -343,6 +344,40 @@ static struct sk_buff *btbcm_read_usb_product(struct hci_dev *hdev)
return skb;
}
+static const struct dmi_system_id disable_broken_read_transmit_power[] = {
+ {
+ .matches = {
+ DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro16,1"),
+ },
+ },
+ {
+ .matches = {
+ DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro16,2"),
+ },
+ },
+ {
+ .matches = {
+ DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro16,4"),
+ },
+ },
+ {
+ .matches = {
+ DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "iMac20,1"),
+ },
+ },
+ {
+ .matches = {
+ DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "iMac20,2"),
+ },
+ },
+ { }
+};
+
static int btbcm_read_info(struct hci_dev *hdev)
{
struct sk_buff *skb;
@@ -363,6 +398,10 @@ static int btbcm_read_info(struct hci_dev *hdev)
bt_dev_info(hdev, "BCM: features 0x%2.2x", skb->data[1]);
kfree_skb(skb);
+ /* Read DMI and disable broken Read LE Min/Max Tx Power */
+ if (dmi_first_match(disable_broken_read_transmit_power))
+ set_bit(HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER, &hdev->quirks);
+
return 0;
}
Hi Aditya,
> Some Macs with the T2 security chip had Bluetooth not working.
> To fix it we add DMI based quirks to disable querying of LE Tx power.
>
> Signed-off-by: Aditya Garg <[email protected]>
> Reported-by: Orlando Chamberlain <[email protected]>
> Tested-by: Orlando Chamberlain <[email protected]>
> Link:
> https://lore.kernel.org/r/[email protected]
> Fixes: 7c395ea521e6 ("Bluetooth: Query LE tx power on startup")
> Cc: [email protected]
> ---
> v7 :- Removed unused variable and added Tested-by.
> v8 :- No change.
> v9 :- Add Cc: [email protected]
> v10 :- Fix gitlint
> drivers/bluetooth/btbcm.c | 39 +++++++++++++++++++++++++++++++++++++++
> 1 file changed, 39 insertions(+)
patch has been applied to bluetooth-next tree.
Regards
Marcel
Hi Aditya,
> Some devices have a bug causing them to not work if they query
> LE tx power on startup. Thus we add a quirk in order to not query it
> and default min/max tx power values to HCI_TX_POWER_INVALID.
>
> Signed-off-by: Aditya Garg <[email protected]>
> Reported-by: Orlando Chamberlain <[email protected]>
> Tested-by: Orlando Chamberlain <[email protected]>
> Link:
> https://lore.kernel.org/r/[email protected]
> Fixes: 7c395ea521e6 ("Bluetooth: Query LE tx power on startup")
> Cc: [email protected]
> ---
> v7 :- Added Tested-by.
> v8 :- Fix checkpatch error.
> v9 :- Remake patch for Bluetooth-next tree and add Cc: [email protected]
> v10 :- Fix gitlint
> include/net/bluetooth/hci.h | 9 +++++++++
> net/bluetooth/hci_sync.c | 3 ++-
> 2 files changed, 11 insertions(+), 1 deletion(-)
patch has been applied to bluetooth-next tree.
Regards
Marcel
Hi, this once again is your Linux kernel regression tracker speaking.
On 03.12.21 22:28, Marcel Holtmann wrote:
>> Some Macs with the T2 security chip had Bluetooth not working.
>> To fix it we add DMI based quirks to disable querying of LE Tx power.
>>
>> Signed-off-by: Aditya Garg <[email protected]>
>> Reported-by: Orlando Chamberlain <[email protected]>
>> Tested-by: Orlando Chamberlain <[email protected]>
>> Link:
>> https://lore.kernel.org/r/[email protected]
>> Fixes: 7c395ea521e6 ("Bluetooth: Query LE tx power on startup")
If anyone wonders: this was for v5.11-rc1.
>> Cc: [email protected]
>> ---
>> v7 :- Removed unused variable and added Tested-by.
>> v8 :- No change.
>> v9 :- Add Cc: [email protected]
>> v10 :- Fix gitlint
>> drivers/bluetooth/btbcm.c | 39 +++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 39 insertions(+)
>
> patch has been applied to bluetooth-next tree.
And there are these two pages now for 19 days. What's the hold-up? Or do
you consider the changes to dangerous to merge now?
Sure, it's not a recent regression, so it might not look that urgent.
But it OTOH affects everyone that can't go back to v5.10 (the newest
Longterm kernel without the regression) -- for example if a user needs a
post-v5.10 feature or upgrades to a distro with a newer kernel.
That's why this fix (unless you considerer it to dangerous) IMHO should
be merged rather sooner than later and and not wait for the merge window
of v5.17. Another aspect against waiting for the next merge window: it
contributes to piling up a large number of changes that need to be
backported to stable and longterm kernels once v5.17-rc1 is out,
resulting in stable and longterm releases with a huge pile of changes:
https://lwn.net/Articles/863505/
Ciao, Thorsten
#regzbot poke