Hi Luiz, Do you need the original patch re-generated, or can you find it in this thread? On Fri, 2023-02-17 at 20:31 +0100, Mateusz Jończyk wrote: > W dniu 21.11.2022 o 19:48, Gix, Brian pisze: > > Hi Mateusz, > > > > On Sat, 2022-11-19 at 19:26 +0100, Mateusz Jończyk wrote: > > > W dniu 2.11.2022 o 18:59, Brian Gix pisze: > > > > The msft_set_filter_enable() command was using the deprecated > > > > hci_request mechanism rather than hci_sync. This caused the > > > > warning > > > > error: > > > > hci0: HCI_REQ-0xfcf0 > > > > > > > > Signed-off-by: Brian Gix <brian.gix@xxxxxxxxx> > > > > --- > > > > net/bluetooth/msft.c | 36 +++++++++++------------------------- > > > > 1 file changed, 11 insertions(+), 25 deletions(-) > > > > > > Hello, > > > > > > On the first time I was testing this patch, I was heavily testing > > > various Bluetooth functionality. Eventually, something stopped > > > working > > > and I was no longer able to establish a Bluetooth connection > > > between > > > my mobile phone and my laptop. To try to resolve this issue, I > > > tried > > > suspending > > > my laptop, but after resume, I got a kernel panic (dmesg attached > > > below, I may > > > provide journal extracts on request). > > Hello, > > I have to apologize for the delay. > > This kernel panic happened to me again, without this patch so now I > can > say that it is unrelated. I have again tested the patch you provided > and > Bluetooth with it appeared to be working correctly. I have even > tested > with a mobile app called "Beacon Simulator" [1] and the beacon was > successfully found on my laptop. > > I have tested this on top of 6.2.0-rc8. > > Additionally, to see how it worked, I tested it with some debugging > printks and additional calls to > msft_set_filter_enable(hdev, X); > (patch attached below) and the code appears to do as it is intended, > but > when I combined two > msft_set_filter_enable(hdev, true); > calls one after the other, I got something like this: > > Bluetooth: hci0: BEGIN msft_set_filter_enable(hdev, 1) > Bluetooth: hci0: BEGIN > msft_le_set_advertisement_filter_enable_cb(hdev, user_data, 0) > Bluetooth: hci0: END msft_set_filter_enable(hdev, 1), err = 0 > Bluetooth: hci0: BEGIN msft_set_filter_enable(hdev, 1) > Bluetooth: hci0: Opcode 0xfcf0 failed: -16 > Bluetooth: hci0: BEGIN > msft_le_set_advertisement_filter_enable_cb(hdev, user_data, f0) > Bluetooth: hci0: END msft_set_filter_enable(hdev, 1), err = - > 16 > > instead of the error code 0x0C as is described in a comment in > msft_le_set_advertisement_filter_enable_cb() and the Microsoft > specification. Is this expected? > > In msft_set_filter_enable() your patch also casts err from int to u8 > while calling msft_le_set_advertisement_filter_enable_cb() without > checking its sign. > > Greetings, > > Mateusz > > [1] > https://play.google.com/store/apps/details?id=net.alea.beaconsimulator > > Cc: Luiz Von Dentz <luiz.von.dentz@xxxxxxxxx> > > --- > net/bluetooth/msft.c | 13 +++++++++++++ > 1 file changed, 13 insertions(+) > > diff --git a/net/bluetooth/msft.c b/net/bluetooth/msft.c > index bf5cee48916c..a6f594bd0722 100644 > --- a/net/bluetooth/msft.c > +++ b/net/bluetooth/msft.c > @@ -6,6 +6,7 @@ > #include <net/bluetooth/bluetooth.h> > #include <net/bluetooth/hci_core.h> > #include <net/bluetooth/mgmt.h> > +#include <linux/delay.h> > > #include "hci_request.h" > #include "mgmt_util.h" > @@ -527,6 +528,12 @@ void msft_do_open(struct hci_dev *hdev) > if (msft_monitor_supported(hdev)) { > msft->resuming = true; > msft_set_filter_enable(hdev, true); > + mdelay(1); > + msft_set_filter_enable(hdev, false); > + mdelay(1); > + msft_set_filter_enable(hdev, true); > + mdelay(1); > + msft_set_filter_enable(hdev, true); > /* Monitors get removed on power off, so we need to > explicitly > * tell the controller to re-monitor. > */ > @@ -749,6 +756,8 @@ static void > msft_le_set_advertisement_filter_enable_cb(struct hci_dev *hdev, > struct msft_cp_le_set_advertisement_filter_enable *cp = > user_data; > struct msft_data *msft = hdev->msft_data; > > + bt_dev_warn(hdev, "BEGIN > msft_le_set_advertisement_filter_enable_cb(hdev, user_data, %x)", > (unsigned) status); > + > /* Error 0x0C would be returned if the filter enabled status > is > * already set to whatever we were trying to set. > * Although the default state should be disabled, some > controller set > @@ -804,6 +813,8 @@ int msft_set_filter_enable(struct hci_dev *hdev, > bool enable) > struct msft_data *msft = hdev->msft_data; > int err; > > + bt_dev_warn(hdev, "BEGIN msft_set_filter_enable(hdev, %d)", > (int) enable); > + > if (!msft) > return -EOPNOTSUPP; > > @@ -814,6 +825,8 @@ int msft_set_filter_enable(struct hci_dev *hdev, > bool enable) > > msft_le_set_advertisement_filter_enable_cb(hdev, &cp, err); > > + bt_dev_warn(hdev, "END msft_set_filter_enable(hdev, %d), err > = %d", (int) enable, err); > + > return 0; > } >
