| Message ID | 20230822191444.3741307-1-luiz.dentz@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | Bluetooth: HCI: Introduce HCI_QUIRK_BROKEN_LE_CODED | expand |
Dnia środa, 23 sierpnia 2023 18:35:27 CEST Luiz Augusto von Dentz pisze: > Hi Bruno, > > On Wed, Aug 23, 2023 at 2:21 AM Bruno Pitrus <brunopitrus@hotmail.com> wrote: > > > > Dnia środa, 23 sierpnia 2023 10:26:31 CEST Paul Menzel pisze: > > > Dear Luiz, > > > > > > > > > Thank you for your answer. > > > > > > Am 22.08.23 um 22:20 schrieb Luiz Augusto von Dentz: > > > > Hi Paul, > > > > > > > > On Tue, Aug 22, 2023 at 1:01 PM Paul Menzel <pmenzel@molgen.mpg.de> wrote: > > > >> > > > >> [CC: +Bruno] > > > >> > > > >> Dear Luiz, > > > >> > > > >> > > > >> Thank you for the patch. > > > >> > > > >> Am 22.08.23 um 21:14 schrieb Luiz Augusto von Dentz: > > > >>> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> > > > >>> > > > >>> This introduces HCI_QUIRK_BROKEN_LE_CODED is is used to indicate that > > > >> > > > >> …. It is used … > > > >> > > > >>> LE Coded PHY shall not be used, it is then set for some Intel models > > > >>> that claims to support it but when used causes many problems. > > > >> > > > >> that claim to … > > > >> > > > >>> Link: https://github.com/bluez/bluez/issues/577 > > > >>> Link: https://github.com/bluez/bluez/issues/582 > > > >>> Link: https://lore.kernel.org/linux-bluetooth/CABBYNZKco-v7wkjHHexxQbgwwSz-S=GZ=dZKbRE1qxT1h4fFbQ@mail.gmail.com/T/# > > > >>> Fixes: 288c90224eec ("Bluetooth: Enable all supported LE PHY by default") > > > >>> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> > > > >>> --- > > > >>> drivers/bluetooth/btintel.c | 2 ++ > > > >>> include/net/bluetooth/hci.h | 10 ++++++++++ > > > >>> include/net/bluetooth/hci_core.h | 4 +++- > > > >>> 3 files changed, 15 insertions(+), 1 deletion(-) > > > >>> > > > >>> diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c > > > >>> index 9b239ce96fa4..3ed60b2b0340 100644 > > > >>> --- a/drivers/bluetooth/btintel.c > > > >>> +++ b/drivers/bluetooth/btintel.c > > > >>> @@ -2776,6 +2776,8 @@ static int btintel_setup_combined(struct hci_dev *hdev) > > > >>> case 0x11: /* JfP */ > > > >>> case 0x12: /* ThP */ > > > >>> case 0x13: /* HrP */ > > > >>> + set_bit(HCI_QUIRK_BROKEN_LE_CODED, &hdev->quirks); > > > >>> + fallthrough; > > > >>> case 0x14: /* CcP */ > > > >>> set_bit(HCI_QUIRK_VALID_LE_STATES, &hdev->quirks); > > > >>> fallthrough; > > > >>> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h > > > >>> index c58425d4c592..767921d7f5c1 100644 > > > >>> --- a/include/net/bluetooth/hci.h > > > >>> +++ b/include/net/bluetooth/hci.h > > > >>> @@ -319,6 +319,16 @@ enum { > > > >>> * This quirk must be set before hci_register_dev is called. > > > >>> */ > > > >>> HCI_QUIRK_USE_MSFT_EXT_ADDRESS_FILTER, > > > >>> + > > > >>> + /* > > > >>> + * When this quirk is set, LE Coded PHY is shall not be used. This is > > > >> > > > >> s/is shall/shall/ > > > >> > > > >>> + * required for some Intel controllers which erroneously claim to > > > >>> + * support it but it causes problems with extended scanning. > > > >>> + * > > > >>> + * This quirk can be set before hci_register_dev is called or > > > >>> + * during the hdev->setup vendor callback. > > > >>> + */ > > > >>> + HCI_QUIRK_BROKEN_LE_CODED, > > > >>> }; > > > >>> > > > >>> /* HCI device flags */ > > > >>> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h > > > >>> index 6e2988b11f99..e6359f7346f1 100644 > > > >>> --- a/include/net/bluetooth/hci_core.h > > > >>> +++ b/include/net/bluetooth/hci_core.h > > > >>> @@ -1817,7 +1817,9 @@ void hci_conn_del_sysfs(struct hci_conn *conn); > > > >>> #define scan_2m(dev) (((dev)->le_tx_def_phys & HCI_LE_SET_PHY_2M) || \ > > > >>> ((dev)->le_rx_def_phys & HCI_LE_SET_PHY_2M)) > > > >>> > > > >>> -#define le_coded_capable(dev) (((dev)->le_features[1] & HCI_LE_PHY_CODED)) > > > >>> +#define le_coded_capable(dev) (((dev)->le_features[1] & HCI_LE_PHY_CODED) && \ > > > >>> + !test_bit(HCI_QUIRK_BROKEN_LE_CODED, \ > > > >>> + &(dev)->quirks)) > > > >>> > > > >>> #define scan_coded(dev) (((dev)->le_tx_def_phys & HCI_LE_SET_PHY_CODED) || \ > > > >>> ((dev)->le_rx_def_phys & HCI_LE_SET_PHY_CODED)) > > > >> > > > >> Will this be future proof, once firmware for the broken controllers are > > > >> fixed? > > > > > > > > Yes, it won't cause any regressions if the firmware is fixed in the > > > > future, but LE coded PHY will need to be re-enabled by removing the > > > > quirks, this is why we say it is broken but we can't depend on runtime > > > > detection and should probably print a warning until we have a proper > > > > fix for it lands at the firmware side. We could in theory set > > > > HCI_QUIRK_BROKEN_LE_CODED based on the firmware version but firmware > > > > versioning schemes tend to change so I'm afraid that could also cause > > > > regression like this in the future. > > > > > > Understood. Maybe you could add the known broken firmware versions to > > > the commit message for posterity though. > > > > > > > > > Kind regards, > > > > > > Paul > > > > > Thanks, > > This patch partly works for me. The mouse now takes several dozen seconds to detect where the computer did not find it at all before. > > But note that in kernel 6.3.x it was always detected immediately. > > What version did you try, v2 didn't have any effect for me, only v3 > worked which had the same behavior of AX210 so it discovered it quite > quickly. > > I only tried v1 patch ( https://www.spinics.net/lists/linux-bluetooth/msg106406.html ). Should i try a later version instead?
diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c index 9b239ce96fa4..3ed60b2b0340 100644 --- a/drivers/bluetooth/btintel.c +++ b/drivers/bluetooth/btintel.c @@ -2776,6 +2776,8 @@ static int btintel_setup_combined(struct hci_dev *hdev) case 0x11: /* JfP */ case 0x12: /* ThP */ case 0x13: /* HrP */ + set_bit(HCI_QUIRK_BROKEN_LE_CODED, &hdev->quirks); + fallthrough; case 0x14: /* CcP */ set_bit(HCI_QUIRK_VALID_LE_STATES, &hdev->quirks); fallthrough; diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h index c58425d4c592..767921d7f5c1 100644 --- a/include/net/bluetooth/hci.h +++ b/include/net/bluetooth/hci.h @@ -319,6 +319,16 @@ enum { * This quirk must be set before hci_register_dev is called. */ HCI_QUIRK_USE_MSFT_EXT_ADDRESS_FILTER, + + /* + * When this quirk is set, LE Coded PHY is shall not be used. This is + * required for some Intel controllers which erroneously claim to + * support it but it causes problems with extended scanning. + * + * This quirk can be set before hci_register_dev is called or + * during the hdev->setup vendor callback. + */ + HCI_QUIRK_BROKEN_LE_CODED, }; /* HCI device flags */ diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h index 6e2988b11f99..e6359f7346f1 100644 --- a/include/net/bluetooth/hci_core.h +++ b/include/net/bluetooth/hci_core.h @@ -1817,7 +1817,9 @@ void hci_conn_del_sysfs(struct hci_conn *conn); #define scan_2m(dev) (((dev)->le_tx_def_phys & HCI_LE_SET_PHY_2M) || \ ((dev)->le_rx_def_phys & HCI_LE_SET_PHY_2M)) -#define le_coded_capable(dev) (((dev)->le_features[1] & HCI_LE_PHY_CODED)) +#define le_coded_capable(dev) (((dev)->le_features[1] & HCI_LE_PHY_CODED) && \ + !test_bit(HCI_QUIRK_BROKEN_LE_CODED, \ + &(dev)->quirks)) #define scan_coded(dev) (((dev)->le_tx_def_phys & HCI_LE_SET_PHY_CODED) || \ ((dev)->le_rx_def_phys & HCI_LE_SET_PHY_CODED))