2022-09-26 23:56:24

by Abhishek Pandit-Subedi

[permalink] [raw]
Subject: [PATCH v2] Bluetooth: Call shutdown for HCI_USER_CHANNEL

From: Abhishek Pandit-Subedi <[email protected]>

Some drivers depend on shutdown being called for proper operation.
Unset HCI_USER_CHANNEL and call the full close routine since shutdown is
complementary to setup.

Signed-off-by: Abhishek Pandit-Subedi <[email protected]>
---

Using hci_qca, we can get the controller into a bad state simply by
trying to bind to userchannel twice (open+bind+close, then open+bind).
Without running the shutdown routine, the device seems to get into a bad
state. A similar bug also occurs with btmtksdio (using MT7921).

This change properly runs the shutdown routine, which should be
complementary to setup. The reason it unsets the HCI_USER_CHANNEL flag
is that some drivers have complex operations in their shutdown routine
(including sending hci packets) and we need to support the normal data
path for them (including cmd_timeout + recovery mechanisms).

Note for v2: I've gotten a chance to test this on more devices
and figure out why it wasn't working before in v1. I found two problems:
I had a signal pending (SIGTERM) that was messing things up in the
socket release function and the HCI_USER_CHANNEL flag was preventing
hci_sync from operating properly during shutdown on Intel chipsets
(which use the sync functions to send a reset command + other commands
sometimes).

This was tested with hci_qca (QCA6174-A-3), btmtksdio (MT7921-SDIO)
and btusb (with AX200).


Changes in v2:
- Clear HCI_USER_CHANNEL flag at start of close and restore at end.
- Add comment explaning why we need to clear flag and run shutdown.

net/bluetooth/hci_sync.c | 19 ++++++++++++++++---
1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
index 422f7c6911d9..f9591fcefb8d 100644
--- a/net/bluetooth/hci_sync.c
+++ b/net/bluetooth/hci_sync.c
@@ -4731,9 +4731,18 @@ int hci_dev_close_sync(struct hci_dev *hdev)
{
bool auto_off;
int err = 0;
+ bool was_userchannel;

bt_dev_dbg(hdev, "");

+ /* Similar to how we first do setup and then set the exclusive access
+ * bit for userspace, we must first unset userchannel and then clean up.
+ * Otherwise, the kernel can't properly use the hci channel to clean up
+ * the controller (some shutdown routines require sending additional
+ * commands to the controller for example).
+ */
+ was_userchannel = hci_dev_test_and_clear_flag(hdev, HCI_USER_CHANNEL);
+
cancel_delayed_work(&hdev->power_off);
cancel_delayed_work(&hdev->ncmd_timer);
cancel_delayed_work(&hdev->le_scan_disable);
@@ -4747,7 +4756,6 @@ int hci_dev_close_sync(struct hci_dev *hdev)
}

if (!hci_dev_test_flag(hdev, HCI_UNREGISTER) &&
- !hci_dev_test_flag(hdev, HCI_USER_CHANNEL) &&
test_bit(HCI_UP, &hdev->flags)) {
/* Execute vendor specific shutdown routine */
if (hdev->shutdown)
@@ -4756,6 +4764,8 @@ int hci_dev_close_sync(struct hci_dev *hdev)

if (!test_and_clear_bit(HCI_UP, &hdev->flags)) {
cancel_delayed_work_sync(&hdev->cmd_timer);
+ if (was_userchannel)
+ hci_dev_set_flag(hdev, HCI_USER_CHANNEL);
return err;
}

@@ -4795,7 +4805,7 @@ int hci_dev_close_sync(struct hci_dev *hdev)
auto_off = hci_dev_test_and_clear_flag(hdev, HCI_AUTO_OFF);

if (!auto_off && hdev->dev_type == HCI_PRIMARY &&
- !hci_dev_test_flag(hdev, HCI_USER_CHANNEL) &&
+ !was_userchannel &&
hci_dev_test_flag(hdev, HCI_MGMT))
__mgmt_power_off(hdev);

@@ -4808,7 +4818,7 @@ int hci_dev_close_sync(struct hci_dev *hdev)

hci_sock_dev_event(hdev, HCI_DEV_DOWN);

- if (!hci_dev_test_flag(hdev, HCI_USER_CHANNEL)) {
+ if (!was_userchannel)
aosp_do_close(hdev);
msft_do_close(hdev);
}
@@ -4858,6 +4868,9 @@ int hci_dev_close_sync(struct hci_dev *hdev)
memset(hdev->dev_class, 0, sizeof(hdev->dev_class));
bacpy(&hdev->random_addr, BDADDR_ANY);

+ if (was_userchannel)
+ hci_dev_set_flag(hdev, HCI_USER_CHANNEL);
+
hci_dev_put(hdev);
return err;
}
--
2.37.3.998.g577e59143f-goog


2022-09-27 00:26:05

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH v2] Bluetooth: Call shutdown for HCI_USER_CHANNEL

Hi Abhishek,

On Mon, Sep 26, 2022 at 4:44 PM Abhishek Pandit-Subedi
<[email protected]> wrote:
>
> From: Abhishek Pandit-Subedi <[email protected]>
>
> Some drivers depend on shutdown being called for proper operation.
> Unset HCI_USER_CHANNEL and call the full close routine since shutdown is
> complementary to setup.
>
> Signed-off-by: Abhishek Pandit-Subedi <[email protected]>
> ---
>
> Using hci_qca, we can get the controller into a bad state simply by
> trying to bind to userchannel twice (open+bind+close, then open+bind).
> Without running the shutdown routine, the device seems to get into a bad
> state. A similar bug also occurs with btmtksdio (using MT7921).
>
> This change properly runs the shutdown routine, which should be
> complementary to setup. The reason it unsets the HCI_USER_CHANNEL flag
> is that some drivers have complex operations in their shutdown routine
> (including sending hci packets) and we need to support the normal data
> path for them (including cmd_timeout + recovery mechanisms).
>
> Note for v2: I've gotten a chance to test this on more devices
> and figure out why it wasn't working before in v1. I found two problems:
> I had a signal pending (SIGTERM) that was messing things up in the
> socket release function and the HCI_USER_CHANNEL flag was preventing
> hci_sync from operating properly during shutdown on Intel chipsets
> (which use the sync functions to send a reset command + other commands
> sometimes).
>
> This was tested with hci_qca (QCA6174-A-3), btmtksdio (MT7921-SDIO)
> and btusb (with AX200).
>
>
> Changes in v2:
> - Clear HCI_USER_CHANNEL flag at start of close and restore at end.
> - Add comment explaning why we need to clear flag and run shutdown.
>
> net/bluetooth/hci_sync.c | 19 ++++++++++++++++---
> 1 file changed, 16 insertions(+), 3 deletions(-)
>
> diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
> index 422f7c6911d9..f9591fcefb8d 100644
> --- a/net/bluetooth/hci_sync.c
> +++ b/net/bluetooth/hci_sync.c
> @@ -4731,9 +4731,18 @@ int hci_dev_close_sync(struct hci_dev *hdev)
> {
> bool auto_off;
> int err = 0;
> + bool was_userchannel;
>
> bt_dev_dbg(hdev, "");
>
> + /* Similar to how we first do setup and then set the exclusive access
> + * bit for userspace, we must first unset userchannel and then clean up.
> + * Otherwise, the kernel can't properly use the hci channel to clean up
> + * the controller (some shutdown routines require sending additional
> + * commands to the controller for example).
> + */
> + was_userchannel = hci_dev_test_and_clear_flag(hdev, HCI_USER_CHANNEL);
> +
> cancel_delayed_work(&hdev->power_off);
> cancel_delayed_work(&hdev->ncmd_timer);
> cancel_delayed_work(&hdev->le_scan_disable);
> @@ -4747,7 +4756,6 @@ int hci_dev_close_sync(struct hci_dev *hdev)
> }
>
> if (!hci_dev_test_flag(hdev, HCI_UNREGISTER) &&
> - !hci_dev_test_flag(hdev, HCI_USER_CHANNEL) &&
> test_bit(HCI_UP, &hdev->flags)) {
> /* Execute vendor specific shutdown routine */
> if (hdev->shutdown)

I guess the idea here is that shutdown can be run without the
HCI_USER_CHANNEL flag since the hdev is closing we don't expect any
traffic from socket/user channel? In that case I'd probably suggest
having this on its own function e.g. hci_dev_shutdown which can have
the logic of resetting the flag and restoring at the end. Also it is
probably a good idea to have some test mimicking this behavior on
userchan-tester so we do not accidentally break it.

> @@ -4756,6 +4764,8 @@ int hci_dev_close_sync(struct hci_dev *hdev)
>
> if (!test_and_clear_bit(HCI_UP, &hdev->flags)) {
> cancel_delayed_work_sync(&hdev->cmd_timer);
> + if (was_userchannel)
> + hci_dev_set_flag(hdev, HCI_USER_CHANNEL);
> return err;
> }
>
> @@ -4795,7 +4805,7 @@ int hci_dev_close_sync(struct hci_dev *hdev)
> auto_off = hci_dev_test_and_clear_flag(hdev, HCI_AUTO_OFF);
>
> if (!auto_off && hdev->dev_type == HCI_PRIMARY &&
> - !hci_dev_test_flag(hdev, HCI_USER_CHANNEL) &&
> + !was_userchannel &&
> hci_dev_test_flag(hdev, HCI_MGMT))
> __mgmt_power_off(hdev);
>
> @@ -4808,7 +4818,7 @@ int hci_dev_close_sync(struct hci_dev *hdev)
>
> hci_sock_dev_event(hdev, HCI_DEV_DOWN);
>
> - if (!hci_dev_test_flag(hdev, HCI_USER_CHANNEL)) {
> + if (!was_userchannel)
> aosp_do_close(hdev);
> msft_do_close(hdev);
> }
> @@ -4858,6 +4868,9 @@ int hci_dev_close_sync(struct hci_dev *hdev)
> memset(hdev->dev_class, 0, sizeof(hdev->dev_class));
> bacpy(&hdev->random_addr, BDADDR_ANY);
>
> + if (was_userchannel)
> + hci_dev_set_flag(hdev, HCI_USER_CHANNEL);
> +
> hci_dev_put(hdev);
> return err;
> }
> --
> 2.37.3.998.g577e59143f-goog
>


--
Luiz Augusto von Dentz

2022-09-27 00:26:31

by Abhishek Pandit-Subedi

[permalink] [raw]
Subject: Re: [PATCH v2] Bluetooth: Call shutdown for HCI_USER_CHANNEL

On Mon, Sep 26, 2022 at 5:10 PM Luiz Augusto von Dentz
<[email protected]> wrote:
>
> Hi Abhishek,
>
> On Mon, Sep 26, 2022 at 4:44 PM Abhishek Pandit-Subedi
> <[email protected]> wrote:
> >
> > From: Abhishek Pandit-Subedi <[email protected]>
> >
> > Some drivers depend on shutdown being called for proper operation.
> > Unset HCI_USER_CHANNEL and call the full close routine since shutdown is
> > complementary to setup.
> >
> > Signed-off-by: Abhishek Pandit-Subedi <[email protected]>
> > ---
> >
> > Using hci_qca, we can get the controller into a bad state simply by
> > trying to bind to userchannel twice (open+bind+close, then open+bind).
> > Without running the shutdown routine, the device seems to get into a bad
> > state. A similar bug also occurs with btmtksdio (using MT7921).
> >
> > This change properly runs the shutdown routine, which should be
> > complementary to setup. The reason it unsets the HCI_USER_CHANNEL flag
> > is that some drivers have complex operations in their shutdown routine
> > (including sending hci packets) and we need to support the normal data
> > path for them (including cmd_timeout + recovery mechanisms).
> >
> > Note for v2: I've gotten a chance to test this on more devices
> > and figure out why it wasn't working before in v1. I found two problems:
> > I had a signal pending (SIGTERM) that was messing things up in the
> > socket release function and the HCI_USER_CHANNEL flag was preventing
> > hci_sync from operating properly during shutdown on Intel chipsets
> > (which use the sync functions to send a reset command + other commands
> > sometimes).
> >
> > This was tested with hci_qca (QCA6174-A-3), btmtksdio (MT7921-SDIO)
> > and btusb (with AX200).
> >
> >
> > Changes in v2:
> > - Clear HCI_USER_CHANNEL flag at start of close and restore at end.
> > - Add comment explaning why we need to clear flag and run shutdown.
> >
> > net/bluetooth/hci_sync.c | 19 ++++++++++++++++---
> > 1 file changed, 16 insertions(+), 3 deletions(-)
> >
> > diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
> > index 422f7c6911d9..f9591fcefb8d 100644
> > --- a/net/bluetooth/hci_sync.c
> > +++ b/net/bluetooth/hci_sync.c
> > @@ -4731,9 +4731,18 @@ int hci_dev_close_sync(struct hci_dev *hdev)
> > {
> > bool auto_off;
> > int err = 0;
> > + bool was_userchannel;
> >
> > bt_dev_dbg(hdev, "");
> >
> > + /* Similar to how we first do setup and then set the exclusive access
> > + * bit for userspace, we must first unset userchannel and then clean up.
> > + * Otherwise, the kernel can't properly use the hci channel to clean up
> > + * the controller (some shutdown routines require sending additional
> > + * commands to the controller for example).
> > + */
> > + was_userchannel = hci_dev_test_and_clear_flag(hdev, HCI_USER_CHANNEL);
> > +
> > cancel_delayed_work(&hdev->power_off);
> > cancel_delayed_work(&hdev->ncmd_timer);
> > cancel_delayed_work(&hdev->le_scan_disable);
> > @@ -4747,7 +4756,6 @@ int hci_dev_close_sync(struct hci_dev *hdev)
> > }
> >
> > if (!hci_dev_test_flag(hdev, HCI_UNREGISTER) &&
> > - !hci_dev_test_flag(hdev, HCI_USER_CHANNEL) &&
> > test_bit(HCI_UP, &hdev->flags)) {
> > /* Execute vendor specific shutdown routine */
> > if (hdev->shutdown)
>
> I guess the idea here is that shutdown can be run without the
> HCI_USER_CHANNEL flag since the hdev is closing we don't expect any
> traffic from socket/user channel? In that case I'd probably suggest
> having this on its own function e.g. hci_dev_shutdown which can have
> the logic of resetting the flag and restoring at the end. Also it is
> probably a good idea to have some test mimicking this behavior on
> userchan-tester so we do not accidentally break it.

Yup, that sounds reasonable. I'll look into userchan-tester before
sending up a v3.

>
> > @@ -4756,6 +4764,8 @@ int hci_dev_close_sync(struct hci_dev *hdev)
> >
> > if (!test_and_clear_bit(HCI_UP, &hdev->flags)) {
> > cancel_delayed_work_sync(&hdev->cmd_timer);
> > + if (was_userchannel)
> > + hci_dev_set_flag(hdev, HCI_USER_CHANNEL);
> > return err;
> > }
> >
> > @@ -4795,7 +4805,7 @@ int hci_dev_close_sync(struct hci_dev *hdev)
> > auto_off = hci_dev_test_and_clear_flag(hdev, HCI_AUTO_OFF);
> >
> > if (!auto_off && hdev->dev_type == HCI_PRIMARY &&
> > - !hci_dev_test_flag(hdev, HCI_USER_CHANNEL) &&
> > + !was_userchannel &&
> > hci_dev_test_flag(hdev, HCI_MGMT))
> > __mgmt_power_off(hdev);
> >
> > @@ -4808,7 +4818,7 @@ int hci_dev_close_sync(struct hci_dev *hdev)
> >
> > hci_sock_dev_event(hdev, HCI_DEV_DOWN);
> >
> > - if (!hci_dev_test_flag(hdev, HCI_USER_CHANNEL)) {
> > + if (!was_userchannel)
> > aosp_do_close(hdev);
> > msft_do_close(hdev);
> > }
> > @@ -4858,6 +4868,9 @@ int hci_dev_close_sync(struct hci_dev *hdev)
> > memset(hdev->dev_class, 0, sizeof(hdev->dev_class));
> > bacpy(&hdev->random_addr, BDADDR_ANY);
> >
> > + if (was_userchannel)
> > + hci_dev_set_flag(hdev, HCI_USER_CHANNEL);
> > +
> > hci_dev_put(hdev);
> > return err;
> > }
> > --
> > 2.37.3.998.g577e59143f-goog
> >
>
>
> --
> Luiz Augusto von Dentz

2022-09-27 08:15:12

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH v2] Bluetooth: Call shutdown for HCI_USER_CHANNEL

Hi Abhishek,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on bluetooth-next/master]
[also build test WARNING on bluetooth/master net-next/master net/master linus/master v6.0-rc7 next-20220923]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/Abhishek-Pandit-Subedi/Bluetooth-Call-shutdown-for-HCI_USER_CHANNEL/20220927-120257
base: https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git master
config: parisc-allyesconfig
compiler: hppa-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/0c60eab323f044b8bc6ab401d691e1d47c117bbd
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Abhishek-Pandit-Subedi/Bluetooth-Call-shutdown-for-HCI_USER_CHANNEL/20220927-120257
git checkout 0c60eab323f044b8bc6ab401d691e1d47c117bbd
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=parisc SHELL=/bin/bash net/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>

All warnings (new ones prefixed by >>):

net/bluetooth/hci_sync.c: In function 'hci_dev_close_sync':
>> net/bluetooth/hci_sync.c:4821:9: warning: this 'if' clause does not guard... [-Wmisleading-indentation]
4821 | if (!was_userchannel)
| ^~
net/bluetooth/hci_sync.c:4823:17: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the 'if'
4823 | msft_do_close(hdev);
| ^~~~~~~~~~~~~
net/bluetooth/hci_sync.c: At top level:
net/bluetooth/hci_sync.c:4826:9: error: expected identifier or '(' before 'if'
4826 | if (hdev->flush)
| ^~
net/bluetooth/hci_sync.c:4830:25: error: expected declaration specifiers or '...' before '&' token
4830 | skb_queue_purge(&hdev->cmd_q);
| ^
net/bluetooth/hci_sync.c:4831:20: error: expected declaration specifiers or '...' before '&' token
4831 | atomic_set(&hdev->cmd_cnt, 1);
| ^
net/bluetooth/hci_sync.c:4831:36: error: expected declaration specifiers or '...' before numeric constant
4831 | atomic_set(&hdev->cmd_cnt, 1);
| ^
net/bluetooth/hci_sync.c:4832:9: error: expected identifier or '(' before 'if'
4832 | if (test_bit(HCI_QUIRK_RESET_ON_CLOSE, &hdev->quirks) &&
| ^~
net/bluetooth/hci_sync.c:4840:20: error: expected declaration specifiers or '...' before '&' token
4840 | flush_work(&hdev->cmd_work);
| ^
net/bluetooth/hci_sync.c:4843:25: error: expected declaration specifiers or '...' before '&' token
4843 | skb_queue_purge(&hdev->rx_q);
| ^
net/bluetooth/hci_sync.c:4844:25: error: expected declaration specifiers or '...' before '&' token
4844 | skb_queue_purge(&hdev->cmd_q);
| ^
net/bluetooth/hci_sync.c:4845:25: error: expected declaration specifiers or '...' before '&' token
4845 | skb_queue_purge(&hdev->raw_q);
| ^
net/bluetooth/hci_sync.c:4848:9: error: expected identifier or '(' before 'if'
4848 | if (hdev->sent_cmd) {
| ^~
net/bluetooth/hci_sync.c:4854:31: error: expected ')' before '&' token
4854 | clear_bit(HCI_RUNNING, &hdev->flags);
| ^~
| )
net/bluetooth/hci_sync.c:4855:33: error: expected ')' before numeric constant
4855 | hci_sock_dev_event(hdev, HCI_DEV_CLOSE);
| ^
| )
net/bluetooth/hci_sync.c:4858:13: error: expected '=', ',', ';', 'asm' or '__attribute__' before '->' token
4858 | hdev->close(hdev);
| ^~
net/bluetooth/hci_sync.c:4861:13: error: expected '=', ',', ';', 'asm' or '__attribute__' before '->' token
4861 | hdev->flags &= BIT(HCI_RAW);
| ^~
In file included from net/bluetooth/hci_sync.c:11:
include/net/bluetooth/hci_core.h:827:9: error: expected identifier or '(' before 'do'
827 | do { \
| ^~
net/bluetooth/hci_sync.c:4862:9: note: in expansion of macro 'hci_dev_clear_volatile_flags'
4862 | hci_dev_clear_volatile_flags(hdev);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/net/bluetooth/hci_core.h:833:11: error: expected identifier or '(' before 'while'
833 | } while (0)
| ^~~~~
net/bluetooth/hci_sync.c:4862:9: note: in expansion of macro 'hci_dev_clear_volatile_flags'
4862 | hci_dev_clear_volatile_flags(hdev);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
net/bluetooth/hci_sync.c:4865:13: error: expected '=', ',', ';', 'asm' or '__attribute__' before '->' token
4865 | hdev->amp_status = AMP_STATUS_POWERED_DOWN;
| ^~
net/bluetooth/hci_sync.c:4867:20: error: expected ')' before '->' token
4867 | memset(hdev->eir, 0, sizeof(hdev->eir));
| ^~
| )
net/bluetooth/hci_sync.c:4868:20: error: expected ')' before '->' token
4868 | memset(hdev->dev_class, 0, sizeof(hdev->dev_class));
| ^~
| )
net/bluetooth/hci_sync.c:4869:15: error: expected declaration specifiers or '...' before '&' token
4869 | bacpy(&hdev->random_addr, BDADDR_ANY);
| ^
In file included from net/bluetooth/hci_sync.c:10:
include/net/bluetooth/bluetooth.h:341:21: error: expected declaration specifiers or '...' before '(' token
341 | #define BDADDR_ANY (&(bdaddr_t) {{0, 0, 0, 0, 0, 0}})
| ^
net/bluetooth/hci_sync.c:4869:35: note: in expansion of macro 'BDADDR_ANY'
4869 | bacpy(&hdev->random_addr, BDADDR_ANY);
| ^~~~~~~~~~
net/bluetooth/hci_sync.c:4871:9: error: expected identifier or '(' before 'if'
4871 | if (was_userchannel)
| ^~
>> net/bluetooth/hci_sync.c:4874:9: warning: data definition has no type or storage class
4874 | hci_dev_put(hdev);
| ^~~~~~~~~~~
net/bluetooth/hci_sync.c:4874:9: error: type defaults to 'int' in declaration of 'hci_dev_put' [-Werror=implicit-int]
>> net/bluetooth/hci_sync.c:4874:9: warning: parameter names (without types) in function declaration
net/bluetooth/hci_sync.c:4874:9: error: conflicting types for 'hci_dev_put'; have 'int()'
include/net/bluetooth/hci_core.h:1422:20: note: previous definition of 'hci_dev_put' with type 'void(struct hci_dev *)'
1422 | static inline void hci_dev_put(struct hci_dev *d)
| ^~~~~~~~~~~
net/bluetooth/hci_sync.c:4875:9: error: expected identifier or '(' before 'return'
4875 | return err;
| ^~~~~~
net/bluetooth/hci_sync.c:4876:1: error: expected identifier or '(' before '}' token
4876 | }
| ^
net/bluetooth/hci_sync.c: In function 'hci_dev_close_sync':
net/bluetooth/hci_sync.c:4824:9: error: control reaches end of non-void function [-Werror=return-type]
4824 | }
| ^
cc1: some warnings being treated as errors


vim +/if +4821 net/bluetooth/hci_sync.c

4729
4730 int hci_dev_close_sync(struct hci_dev *hdev)
4731 {
4732 bool auto_off;
4733 int err = 0;
4734 bool was_userchannel;
4735
4736 bt_dev_dbg(hdev, "");
4737
4738 /* Similar to how we first do setup and then set the exclusive access
4739 * bit for userspace, we must first unset userchannel and then clean up.
4740 * Otherwise, the kernel can't properly use the hci channel to clean up
4741 * the controller (some shutdown routines require sending additional
4742 * commands to the controller for example).
4743 */
4744 was_userchannel = hci_dev_test_and_clear_flag(hdev, HCI_USER_CHANNEL);
4745
4746 cancel_delayed_work(&hdev->power_off);
4747 cancel_delayed_work(&hdev->ncmd_timer);
4748 cancel_delayed_work(&hdev->le_scan_disable);
4749 cancel_delayed_work(&hdev->le_scan_restart);
4750
4751 hci_request_cancel_all(hdev);
4752
4753 if (hdev->adv_instance_timeout) {
4754 cancel_delayed_work_sync(&hdev->adv_instance_expire);
4755 hdev->adv_instance_timeout = 0;
4756 }
4757
4758 if (!hci_dev_test_flag(hdev, HCI_UNREGISTER) &&
4759 test_bit(HCI_UP, &hdev->flags)) {
4760 /* Execute vendor specific shutdown routine */
4761 if (hdev->shutdown)
4762 err = hdev->shutdown(hdev);
4763 }
4764
4765 if (!test_and_clear_bit(HCI_UP, &hdev->flags)) {
4766 cancel_delayed_work_sync(&hdev->cmd_timer);
4767 if (was_userchannel)
4768 hci_dev_set_flag(hdev, HCI_USER_CHANNEL);
4769 return err;
4770 }
4771
4772 hci_leds_update_powered(hdev, false);
4773
4774 /* Flush RX and TX works */
4775 flush_work(&hdev->tx_work);
4776 flush_work(&hdev->rx_work);
4777
4778 if (hdev->discov_timeout > 0) {
4779 hdev->discov_timeout = 0;
4780 hci_dev_clear_flag(hdev, HCI_DISCOVERABLE);
4781 hci_dev_clear_flag(hdev, HCI_LIMITED_DISCOVERABLE);
4782 }
4783
4784 if (hci_dev_test_and_clear_flag(hdev, HCI_SERVICE_CACHE))
4785 cancel_delayed_work(&hdev->service_cache);
4786
4787 if (hci_dev_test_flag(hdev, HCI_MGMT)) {
4788 struct adv_info *adv_instance;
4789
4790 cancel_delayed_work_sync(&hdev->rpa_expired);
4791
4792 list_for_each_entry(adv_instance, &hdev->adv_instances, list)
4793 cancel_delayed_work_sync(&adv_instance->rpa_expired_cb);
4794 }
4795
4796 /* Avoid potential lockdep warnings from the *_flush() calls by
4797 * ensuring the workqueue is empty up front.
4798 */
4799 drain_workqueue(hdev->workqueue);
4800
4801 hci_dev_lock(hdev);
4802
4803 hci_discovery_set_state(hdev, DISCOVERY_STOPPED);
4804
4805 auto_off = hci_dev_test_and_clear_flag(hdev, HCI_AUTO_OFF);
4806
4807 if (!auto_off && hdev->dev_type == HCI_PRIMARY &&
4808 !was_userchannel &&
4809 hci_dev_test_flag(hdev, HCI_MGMT))
4810 __mgmt_power_off(hdev);
4811
4812 hci_inquiry_cache_flush(hdev);
4813 hci_pend_le_actions_clear(hdev);
4814 hci_conn_hash_flush(hdev);
4815 /* Prevent data races on hdev->smp_data or hdev->smp_bredr_data */
4816 smp_unregister(hdev);
4817 hci_dev_unlock(hdev);
4818
4819 hci_sock_dev_event(hdev, HCI_DEV_DOWN);
4820
> 4821 if (!was_userchannel)
4822 aosp_do_close(hdev);
4823 msft_do_close(hdev);
4824 }
4825
4826 if (hdev->flush)
4827 hdev->flush(hdev);
4828
4829 /* Reset device */
4830 skb_queue_purge(&hdev->cmd_q);
4831 atomic_set(&hdev->cmd_cnt, 1);
4832 if (test_bit(HCI_QUIRK_RESET_ON_CLOSE, &hdev->quirks) &&
4833 !auto_off && !hci_dev_test_flag(hdev, HCI_UNCONFIGURED)) {
4834 set_bit(HCI_INIT, &hdev->flags);
4835 hci_reset_sync(hdev);
4836 clear_bit(HCI_INIT, &hdev->flags);
4837 }
4838
4839 /* flush cmd work */
4840 flush_work(&hdev->cmd_work);
4841
4842 /* Drop queues */
4843 skb_queue_purge(&hdev->rx_q);
4844 skb_queue_purge(&hdev->cmd_q);
4845 skb_queue_purge(&hdev->raw_q);
4846
4847 /* Drop last sent command */
4848 if (hdev->sent_cmd) {
4849 cancel_delayed_work_sync(&hdev->cmd_timer);
4850 kfree_skb(hdev->sent_cmd);
4851 hdev->sent_cmd = NULL;
4852 }
4853
4854 clear_bit(HCI_RUNNING, &hdev->flags);
4855 hci_sock_dev_event(hdev, HCI_DEV_CLOSE);
4856
4857 /* After this point our queues are empty and no tasks are scheduled. */
4858 hdev->close(hdev);
4859
4860 /* Clear flags */
4861 hdev->flags &= BIT(HCI_RAW);
4862 hci_dev_clear_volatile_flags(hdev);
4863
4864 /* Controller radio is available but is currently powered down */
4865 hdev->amp_status = AMP_STATUS_POWERED_DOWN;
4866
4867 memset(hdev->eir, 0, sizeof(hdev->eir));
4868 memset(hdev->dev_class, 0, sizeof(hdev->dev_class));
4869 bacpy(&hdev->random_addr, BDADDR_ANY);
4870
4871 if (was_userchannel)
4872 hci_dev_set_flag(hdev, HCI_USER_CHANNEL);
4873
> 4874 hci_dev_put(hdev);
4875 return err;
4876 }
4877

--
0-DAY CI Kernel Test Service
https://01.org/lkp


Attachments:
(No filename) (13.20 kB)
config (318.52 kB)
Download all attachments