Bluetooth: remove unneeded hci_conn_hold/put_device()

hci_conn_hold/put_device() is used to control when hci_conn->dev is no
longer needed and can be deleted from the system. Lets first look how they
are currently used throughout the code (excluding HIDP!).

All code that uses hci_conn_hold_device() looks like this:
    ...
    hci_conn_hold_device();
    hci_conn_add_sysfs();
    ...
On the other side, hci_conn_put_device() is exclusively used in
hci_conn_del().

So, considering that hci_conn_del() must not be called twice (which would
fail horribly), we know that hci_conn_put_device() is only called _once_
(which is in hci_conn_del()).
On the other hand, hci_conn_add_sysfs() must not be called twice, either
(it would call device_add twice, which breaks the device, see
drivers/base/core.c). So we know that hci_conn_hold_device() is also
called only once (it's only called directly before hci_conn_add_sysfs()).

So hold and put are known to be called only once. That means we can safely
remove them and directly call hci_conn_del_sysfs() in hci_conn_del().

But there is one issue left: HIDP also uses hci_conn_hold/put_device().
However, this case can be ignored and simply removed as it is totally
broken. The issue is, the only thing HIDP delays with
hci_conn_hold_device() is the removal of the hci_conn->dev from sysfs.
But, the hci_conn device has no mechanism to get notified when its own
parent (hci_dev) gets removed from sysfs. hci_dev_hold/put() does _not_
control when it is removed but only when the device object is created
and destroyed.
And hci_dev calls hci_conn_flush_*() when it removes itself from sysfs,
which itself causes hci_conn_del() to be called, but it does _not_ cause
hci_conn_del_sysfs() to be called, which is wrong.

Hence, we fix it to call hci_conn_del_sysfs() in hci_conn_del(). This
guarantees that a hci_conn object is removed from sysfs _before_ its
parent hci_dev is removed.

The changes to HIDP look scary, wrong and broken. However, if you look at
the HIDP session management, you will notice they're already broken in the
exact _same_ way (ever tried "unplugging" HIDP devices? Breaks _all_ the
time).
So this patch only makes HIDP look _scary_ and _obviously broken_. It does
not break HIDP itself, it already is!

See later patches in this series which fix HIDP to use proper
session-management.

Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
Acked-by: Marcel Holtmann <marcel@holtmann.org>
Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

1 files changed, 0 insertions, 4 deletions

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 78ea9c7c202c..5590cc4412c6 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -345,7 +345,6 @@ struct hci_conn {
         struct timer_list auto_accept_timer;
 
         struct device   dev;
-        atomic_t        devref;
 
         struct hci_dev  *hdev;
         void            *l2cap_data;
@@ -601,9 +600,6 @@ int hci_conn_switch_role(struct hci_conn *conn, __u8 role);
 
 void hci_conn_enter_active_mode(struct hci_conn *conn, __u8 force_active);
 
-void hci_conn_hold_device(struct hci_conn *conn);
-void hci_conn_put_device(struct hci_conn *conn);
-
 static inline void hci_conn_hold(struct hci_conn *conn)
 {
         BT_DBG("hcon %p orig refcnt %d", conn, atomic_read(&conn->refcnt));