2012-01-27 15:47:22

by David Herrmann

[permalink] [raw]
Subject: [PATCH 0/4] Integrate better device support

Hi

"struct device" provides a drvdata-field that we should use properly to save
_driver-data_. This series makes the hci-core use pointer-arithmetic to avoid
using this field in the bus-core and instead converts the drivers to use the
drvdata field.
This also reduces the hci_dev structure by 4/8 bytes, yeah.

This behavior is very common and I tried to stick to naming-conventions. See the
usb-bus or input-bus for other examples.

Regards
David

David Herrmann (4):
Bluetooth: Introduce to_hci_dev()
Bluetooth: Remove hci_dev->driver_data
Bluetooth: Introduce to_hci_conn
Bluetooth: Use proper datatypes in release-callbacks

drivers/bluetooth/bfusb.c | 10 ++++----
drivers/bluetooth/bluecard_cs.c | 12 ++++----
drivers/bluetooth/bpa10x.c | 18 +++++++-------
drivers/bluetooth/bt3c_cs.c | 6 ++--
drivers/bluetooth/btmrvl_debugfs.c | 26 +++++++++----------
drivers/bluetooth/btmrvl_main.c | 10 ++++----
drivers/bluetooth/btsdio.c | 10 ++++----
drivers/bluetooth/btuart_cs.c | 6 ++--
drivers/bluetooth/btusb.c | 28 ++++++++++----------
drivers/bluetooth/btwilink.c | 8 +++---
drivers/bluetooth/dtl1_cs.c | 6 ++--
drivers/bluetooth/hci_ldisc.c | 6 ++--
drivers/bluetooth/hci_vhci.c | 8 +++---
include/net/bluetooth/hci_core.h | 14 ++++++++++-
net/bluetooth/hci_sysfs.c | 47 ++++++++++++++++-------------------
15 files changed, 111 insertions(+), 104 deletions(-)

--
1.7.8.4


2012-01-30 22:00:42

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH 4/4] Bluetooth: Use proper datatypes in release-callbacks

Hi David,

> This enhances code readability a lot and avoids using void* even though
> we know the type of the variable.
>
> Signed-off-by: David Herrmann <[email protected]>
> ---
> net/bluetooth/hci_sysfs.c | 8 ++++----
> 1 files changed, 4 insertions(+), 4 deletions(-)

Acked-by: Marcel Holtmann <[email protected]>

Regards

Marcel



2012-01-30 21:59:56

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH 3/4] Bluetooth: Introduce to_hci_conn

Hi David,

> This avoids using the dev_set/get_drvdata() functions to retrieve a
> pointer to our own structure. We can use simple pointer arithmetic here.
> The drvdata field is actually not needed by any other code-path but this
> makes the code more consistent with hci_dev.
>
> Signed-off-by: David Herrmann <[email protected]>
> ---
> include/net/bluetooth/hci_core.h | 1 +
> net/bluetooth/hci_sysfs.c | 10 ++++------
> 2 files changed, 5 insertions(+), 6 deletions(-)

I read through the comment and I am fine with to_hci_conn.

Acked-by: Marcel Holtmann <[email protected]>

Regards

Marcel



2012-01-30 21:57:35

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH 2/4 v2] Bluetooth: Remove hci_dev->driver_data

Hi David,

> The linux device model provides dev_set/get_drvdata so we can use this
> to save private driver data.
> This also removes several unnecessary casts.
>
> Signed-off-by: David Herrmann <[email protected]>
> ---
> v2: Fixed a bug spotted by Anderson Lizardo. I checked the variables in
> btmrvl_main.c in the wrong order.
>
> drivers/bluetooth/bfusb.c | 10 +++++-----
> drivers/bluetooth/bluecard_cs.c | 12 ++++++------
> drivers/bluetooth/bpa10x.c | 18 +++++++++---------
> drivers/bluetooth/bt3c_cs.c | 6 +++---
> drivers/bluetooth/btmrvl_debugfs.c | 26 ++++++++++++--------------
> drivers/bluetooth/btmrvl_main.c | 11 ++++++-----
> drivers/bluetooth/btsdio.c | 10 +++++-----
> drivers/bluetooth/btuart_cs.c | 6 +++---
> drivers/bluetooth/btusb.c | 28 ++++++++++++++--------------
> drivers/bluetooth/btwilink.c | 8 ++++----
> drivers/bluetooth/dtl1_cs.c | 6 +++---
> drivers/bluetooth/hci_ldisc.c | 6 +++---
> drivers/bluetooth/hci_vhci.c | 8 ++++----
> include/net/bluetooth/hci_core.h | 11 ++++++++++-
> 14 files changed, 87 insertions(+), 79 deletions(-)

Acked-by: Marcel Holtmann <[email protected]>

Regards

Marcel



2012-01-30 21:55:25

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH 1/4] Bluetooth: Introduce to_hci_dev()

Hi David,

> We currently use dev_set_drvdata to keep a pointer to ourself. This
> doesn't make sense as we are the bus and not a driver. Therefore,
> introduce to_hci_dev() so we can get a struct hci_dev pointer from a
> struct device pointer.
>
> dev_set/get_drvdata() is reserved for drivers that provide a device and
> not for the bus using the device. The bus can use simple pointer
> arithmetic to retrieve its private data.
>
> Signed-off-by: David Herrmann <[email protected]>
> ---
> include/net/bluetooth/hci_core.h | 2 ++
> net/bluetooth/hci_sysfs.c | 33 ++++++++++++++++-----------------
> 2 files changed, 18 insertions(+), 17 deletions(-)

Acked-by: Marcel Holtmann <[email protected]>

Regards

Marcel



2012-01-30 21:37:31

by Ulisses Furquim

[permalink] [raw]
Subject: Re: [PATCH 3/4] Bluetooth: Introduce to_hci_conn

Hi David,

On Fri, Jan 27, 2012 at 3:20 PM, David Herrmann
<[email protected]> wrote:
> Hi Andrei
>
> On Fri, Jan 27, 2012 at 6:01 PM, Andrei Emeltchenko
> <[email protected]> wrote:
>> Hi David,
>>
>>> This avoids using the dev_set/get_drvdata() functions to retrieve a
>>> pointer to our own structure. We can use simple pointer arithmetic here=
.
>>> The drvdata field is actually not needed by any other code-path but thi=
s
>>> makes the code more consistent with hci_dev.
>>>
>>> Signed-off-by: David Herrmann <[email protected]>
>>> ---
>>> =A0include/net/bluetooth/hci_core.h | =A0 =A01 +
>>> =A0net/bluetooth/hci_sysfs.c =A0 =A0 =A0 =A0| =A0 10 ++++------
>>> =A02 files changed, 5 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/include/net/bluetooth/hci_core.h
>>> b/include/net/bluetooth/hci_core.h
>>> index 9780f42..8784da1 100644
>>> --- a/include/net/bluetooth/hci_core.h
>>> +++ b/include/net/bluetooth/hci_core.h
>>> @@ -612,6 +612,7 @@ static inline struct hci_dev *hci_dev_hold(struct
>>> hci_dev *d)
>>> =A0#define hci_dev_unlock(d) =A0 =A0 =A0mutex_unlock(&d->lock)
>>>
>>> =A0#define to_hci_dev(d) container_of(d, struct hci_dev, dev)
>>> +#define to_hci_conn(c) container_of(c, struct hci_conn, dev)
>>>
>>> =A0static inline void *hci_get_drvdata(struct hci_dev *hdev)
>>> =A0{
>>> diff --git a/net/bluetooth/hci_sysfs.c b/net/bluetooth/hci_sysfs.c
>>> index 2a0243a..17e6cd4 100644
>>> --- a/net/bluetooth/hci_sysfs.c
>>> +++ b/net/bluetooth/hci_sysfs.c
>>> @@ -33,19 +33,19 @@ static inline char *link_typetostr(int type)
>>>
>>> =A0static ssize_t show_link_type(struct device *dev, struct device_attr=
ibute
>>> *attr, char *buf)
>>> =A0{
>>> - =A0 =A0 =A0 struct hci_conn *conn =3D dev_get_drvdata(dev);
>>> + =A0 =A0 =A0 struct hci_conn *conn =3D to_hci_conn(dev);
>>
>> I personally think it was more readable before.
>
> I didn't write it to make it more readable but to make it conform to
> other subsystems using the drvdata field. I think using a pointer in a
> structure if we could also use container_of is a waste of memory.
> Anyway, in this case we will keep the drvdata field anyway as we
> cannot remove it so I have no objections if you drop this patch.
> Personally I'd prefer to_hci_conn, though, as we are no driver but a
> bus so "get_drvdata" sounds not right here.
>
> Regards
> David
>
>> --Andrei

Using container_of is pretty much common in the kernel. Maybe the
macro names can be better? I think the series is an improvement but
Marcel is the best one to rule on this.

Regards,

--=20
Ulisses Furquim
ProFUSION embedded systems
http://profusion.mobi
Mobile: +55 19 9250 0942
Skype: ulissesffs

2012-01-27 17:20:29

by David Herrmann

[permalink] [raw]
Subject: Re: [PATCH 3/4] Bluetooth: Introduce to_hci_conn

Hi Andrei

On Fri, Jan 27, 2012 at 6:01 PM, Andrei Emeltchenko
<[email protected]> wrote:
> Hi David,
>
>> This avoids using the dev_set/get_drvdata() functions to retrieve a
>> pointer to our own structure. We can use simple pointer arithmetic here.
>> The drvdata field is actually not needed by any other code-path but this
>> makes the code more consistent with hci_dev.
>>
>> Signed-off-by: David Herrmann <[email protected]>
>> ---
>> =A0include/net/bluetooth/hci_core.h | =A0 =A01 +
>> =A0net/bluetooth/hci_sysfs.c =A0 =A0 =A0 =A0| =A0 10 ++++------
>> =A02 files changed, 5 insertions(+), 6 deletions(-)
>>
>> diff --git a/include/net/bluetooth/hci_core.h
>> b/include/net/bluetooth/hci_core.h
>> index 9780f42..8784da1 100644
>> --- a/include/net/bluetooth/hci_core.h
>> +++ b/include/net/bluetooth/hci_core.h
>> @@ -612,6 +612,7 @@ static inline struct hci_dev *hci_dev_hold(struct
>> hci_dev *d)
>> =A0#define hci_dev_unlock(d) =A0 =A0 =A0mutex_unlock(&d->lock)
>>
>> =A0#define to_hci_dev(d) container_of(d, struct hci_dev, dev)
>> +#define to_hci_conn(c) container_of(c, struct hci_conn, dev)
>>
>> =A0static inline void *hci_get_drvdata(struct hci_dev *hdev)
>> =A0{
>> diff --git a/net/bluetooth/hci_sysfs.c b/net/bluetooth/hci_sysfs.c
>> index 2a0243a..17e6cd4 100644
>> --- a/net/bluetooth/hci_sysfs.c
>> +++ b/net/bluetooth/hci_sysfs.c
>> @@ -33,19 +33,19 @@ static inline char *link_typetostr(int type)
>>
>> =A0static ssize_t show_link_type(struct device *dev, struct device_attri=
bute
>> *attr, char *buf)
>> =A0{
>> - =A0 =A0 =A0 struct hci_conn *conn =3D dev_get_drvdata(dev);
>> + =A0 =A0 =A0 struct hci_conn *conn =3D to_hci_conn(dev);
>
> I personally think it was more readable before.

I didn't write it to make it more readable but to make it conform to
other subsystems using the drvdata field. I think using a pointer in a
structure if we could also use container_of is a waste of memory.
Anyway, in this case we will keep the drvdata field anyway as we
cannot remove it so I have no objections if you drop this patch.
Personally I'd prefer to_hci_conn, though, as we are no driver but a
bus so "get_drvdata" sounds not right here.

Regards
David

> --Andrei

2012-01-27 17:01:30

by Andrei Emeltchenko

[permalink] [raw]
Subject: Re: [PATCH 3/4] Bluetooth: Introduce to_hci_conn

Hi David,

> This avoids using the dev_set/get_drvdata() functions to retrieve a
> pointer to our own structure. We can use simple pointer arithmetic here.
> The drvdata field is actually not needed by any other code-path but this
> makes the code more consistent with hci_dev.
>
> Signed-off-by: David Herrmann <[email protected]>
> ---
> include/net/bluetooth/hci_core.h | 1 +
> net/bluetooth/hci_sysfs.c | 10 ++++------
> 2 files changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/include/net/bluetooth/hci_core.h
b/include/net/bluetooth/hci_core.h
> index 9780f42..8784da1 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -612,6 +612,7 @@ static inline struct hci_dev *hci_dev_hold(struct
hci_dev *d)
> #define hci_dev_unlock(d) mutex_unlock(&d->lock)
>
> #define to_hci_dev(d) container_of(d, struct hci_dev, dev)
> +#define to_hci_conn(c) container_of(c, struct hci_conn, dev)
>
> static inline void *hci_get_drvdata(struct hci_dev *hdev)
> {
> diff --git a/net/bluetooth/hci_sysfs.c b/net/bluetooth/hci_sysfs.c
> index 2a0243a..17e6cd4 100644
> --- a/net/bluetooth/hci_sysfs.c
> +++ b/net/bluetooth/hci_sysfs.c
> @@ -33,19 +33,19 @@ static inline char *link_typetostr(int type)
>
> static ssize_t show_link_type(struct device *dev, struct
device_attribute *attr, char *buf)
> {
> - struct hci_conn *conn = dev_get_drvdata(dev);
> + struct hci_conn *conn = to_hci_conn(dev);

I personally think it was more readable before.

--Andrei

2012-01-27 17:01:10

by David Herrmann

[permalink] [raw]
Subject: [PATCH 2/4 v2] Bluetooth: Remove hci_dev->driver_data

The linux device model provides dev_set/get_drvdata so we can use this
to save private driver data.
This also removes several unnecessary casts.

Signed-off-by: David Herrmann <[email protected]>
---
v2: Fixed a bug spotted by Anderson Lizardo. I checked the variables in
btmrvl_main.c in the wrong order.

drivers/bluetooth/bfusb.c | 10 +++++-----
drivers/bluetooth/bluecard_cs.c | 12 ++++++------
drivers/bluetooth/bpa10x.c | 18 +++++++++---------
drivers/bluetooth/bt3c_cs.c | 6 +++---
drivers/bluetooth/btmrvl_debugfs.c | 26 ++++++++++++--------------
drivers/bluetooth/btmrvl_main.c | 11 ++++++-----
drivers/bluetooth/btsdio.c | 10 +++++-----
drivers/bluetooth/btuart_cs.c | 6 +++---
drivers/bluetooth/btusb.c | 28 ++++++++++++++--------------
drivers/bluetooth/btwilink.c | 8 ++++----
drivers/bluetooth/dtl1_cs.c | 6 +++---
drivers/bluetooth/hci_ldisc.c | 6 +++---
drivers/bluetooth/hci_vhci.c | 8 ++++----
include/net/bluetooth/hci_core.h | 11 ++++++++++-
14 files changed, 87 insertions(+), 79 deletions(-)

diff --git a/drivers/bluetooth/bfusb.c b/drivers/bluetooth/bfusb.c
index c7d6ff0..b8ac1c5 100644
--- a/drivers/bluetooth/bfusb.c
+++ b/drivers/bluetooth/bfusb.c
@@ -411,7 +411,7 @@ unlock:

static int bfusb_open(struct hci_dev *hdev)
{
- struct bfusb_data *data = hdev->driver_data;
+ struct bfusb_data *data = hci_get_drvdata(hdev);
unsigned long flags;
int i, err;

@@ -437,7 +437,7 @@ static int bfusb_open(struct hci_dev *hdev)

static int bfusb_flush(struct hci_dev *hdev)
{
- struct bfusb_data *data = hdev->driver_data;
+ struct bfusb_data *data = hci_get_drvdata(hdev);

BT_DBG("hdev %p bfusb %p", hdev, data);

@@ -448,7 +448,7 @@ static int bfusb_flush(struct hci_dev *hdev)

static int bfusb_close(struct hci_dev *hdev)
{
- struct bfusb_data *data = hdev->driver_data;
+ struct bfusb_data *data = hci_get_drvdata(hdev);
unsigned long flags;

BT_DBG("hdev %p bfusb %p", hdev, data);
@@ -483,7 +483,7 @@ static int bfusb_send_frame(struct sk_buff *skb)
if (!test_bit(HCI_RUNNING, &hdev->flags))
return -EBUSY;

- data = hdev->driver_data;
+ data = hci_get_drvdata(hdev);

switch (bt_cb(skb)->pkt_type) {
case HCI_COMMAND_PKT:
@@ -696,7 +696,7 @@ static int bfusb_probe(struct usb_interface *intf, const struct usb_device_id *i
data->hdev = hdev;

hdev->bus = HCI_USB;
- hdev->driver_data = data;
+ hci_set_drvdata(hdev, data);
SET_HCIDEV_DEV(hdev, &intf->dev);

hdev->open = bfusb_open;
diff --git a/drivers/bluetooth/bluecard_cs.c b/drivers/bluetooth/bluecard_cs.c
index 6b1261f..1fcd923 100644
--- a/drivers/bluetooth/bluecard_cs.c
+++ b/drivers/bluetooth/bluecard_cs.c
@@ -561,7 +561,7 @@ static irqreturn_t bluecard_interrupt(int irq, void *dev_inst)

static int bluecard_hci_set_baud_rate(struct hci_dev *hdev, int baud)
{
- bluecard_info_t *info = (bluecard_info_t *)(hdev->driver_data);
+ bluecard_info_t *info = hci_get_drvdata(hdev);
struct sk_buff *skb;

/* Ericsson baud rate command */
@@ -609,7 +609,7 @@ static int bluecard_hci_set_baud_rate(struct hci_dev *hdev, int baud)

static int bluecard_hci_flush(struct hci_dev *hdev)
{
- bluecard_info_t *info = (bluecard_info_t *)(hdev->driver_data);
+ bluecard_info_t *info = hci_get_drvdata(hdev);

/* Drop TX queue */
skb_queue_purge(&(info->txq));
@@ -620,7 +620,7 @@ static int bluecard_hci_flush(struct hci_dev *hdev)

static int bluecard_hci_open(struct hci_dev *hdev)
{
- bluecard_info_t *info = (bluecard_info_t *)(hdev->driver_data);
+ bluecard_info_t *info = hci_get_drvdata(hdev);
unsigned int iobase = info->p_dev->resource[0]->start;

if (test_bit(CARD_HAS_PCCARD_ID, &(info->hw_state)))
@@ -640,7 +640,7 @@ static int bluecard_hci_open(struct hci_dev *hdev)

static int bluecard_hci_close(struct hci_dev *hdev)
{
- bluecard_info_t *info = (bluecard_info_t *)(hdev->driver_data);
+ bluecard_info_t *info = hci_get_drvdata(hdev);
unsigned int iobase = info->p_dev->resource[0]->start;

if (!test_and_clear_bit(HCI_RUNNING, &(hdev->flags)))
@@ -667,7 +667,7 @@ static int bluecard_hci_send_frame(struct sk_buff *skb)
return -ENODEV;
}

- info = (bluecard_info_t *)(hdev->driver_data);
+ info = hci_get_drvdata(hdev);

switch (bt_cb(skb)->pkt_type) {
case HCI_COMMAND_PKT:
@@ -729,7 +729,7 @@ static int bluecard_open(bluecard_info_t *info)
info->hdev = hdev;

hdev->bus = HCI_PCCARD;
- hdev->driver_data = info;
+ hci_set_drvdata(hdev, info);
SET_HCIDEV_DEV(hdev, &info->p_dev->dev);

hdev->open = bluecard_hci_open;
diff --git a/drivers/bluetooth/bpa10x.c b/drivers/bluetooth/bpa10x.c
index 9d63514..d894340 100644
--- a/drivers/bluetooth/bpa10x.c
+++ b/drivers/bluetooth/bpa10x.c
@@ -66,7 +66,7 @@ struct hci_vendor_hdr {

static int bpa10x_recv(struct hci_dev *hdev, int queue, void *buf, int count)
{
- struct bpa10x_data *data = hdev->driver_data;
+ struct bpa10x_data *data = hci_get_drvdata(hdev);

BT_DBG("%s queue %d buffer %p count %d", hdev->name,
queue, buf, count);
@@ -189,7 +189,7 @@ done:
static void bpa10x_rx_complete(struct urb *urb)
{
struct hci_dev *hdev = urb->context;
- struct bpa10x_data *data = hdev->driver_data;
+ struct bpa10x_data *data = hci_get_drvdata(hdev);
int err;

BT_DBG("%s urb %p status %d count %d", hdev->name,
@@ -219,7 +219,7 @@ static void bpa10x_rx_complete(struct urb *urb)

static inline int bpa10x_submit_intr_urb(struct hci_dev *hdev)
{
- struct bpa10x_data *data = hdev->driver_data;
+ struct bpa10x_data *data = hci_get_drvdata(hdev);
struct urb *urb;
unsigned char *buf;
unsigned int pipe;
@@ -260,7 +260,7 @@ static inline int bpa10x_submit_intr_urb(struct hci_dev *hdev)

static inline int bpa10x_submit_bulk_urb(struct hci_dev *hdev)
{
- struct bpa10x_data *data = hdev->driver_data;
+ struct bpa10x_data *data = hci_get_drvdata(hdev);
struct urb *urb;
unsigned char *buf;
unsigned int pipe;
@@ -301,7 +301,7 @@ static inline int bpa10x_submit_bulk_urb(struct hci_dev *hdev)

static int bpa10x_open(struct hci_dev *hdev)
{
- struct bpa10x_data *data = hdev->driver_data;
+ struct bpa10x_data *data = hci_get_drvdata(hdev);
int err;

BT_DBG("%s", hdev->name);
@@ -329,7 +329,7 @@ error:

static int bpa10x_close(struct hci_dev *hdev)
{
- struct bpa10x_data *data = hdev->driver_data;
+ struct bpa10x_data *data = hci_get_drvdata(hdev);

BT_DBG("%s", hdev->name);

@@ -343,7 +343,7 @@ static int bpa10x_close(struct hci_dev *hdev)

static int bpa10x_flush(struct hci_dev *hdev)
{
- struct bpa10x_data *data = hdev->driver_data;
+ struct bpa10x_data *data = hci_get_drvdata(hdev);

BT_DBG("%s", hdev->name);

@@ -355,7 +355,7 @@ static int bpa10x_flush(struct hci_dev *hdev)
static int bpa10x_send_frame(struct sk_buff *skb)
{
struct hci_dev *hdev = (struct hci_dev *) skb->dev;
- struct bpa10x_data *data = hdev->driver_data;
+ struct bpa10x_data *data = hci_get_drvdata(hdev);
struct usb_ctrlrequest *dr;
struct urb *urb;
unsigned int pipe;
@@ -459,7 +459,7 @@ static int bpa10x_probe(struct usb_interface *intf, const struct usb_device_id *
}

hdev->bus = HCI_USB;
- hdev->driver_data = data;
+ hci_set_drvdata(hdev, data);

data->hdev = hdev;

diff --git a/drivers/bluetooth/bt3c_cs.c b/drivers/bluetooth/bt3c_cs.c
index 0e304cb..9c09d6f 100644
--- a/drivers/bluetooth/bt3c_cs.c
+++ b/drivers/bluetooth/bt3c_cs.c
@@ -389,7 +389,7 @@ static irqreturn_t bt3c_interrupt(int irq, void *dev_inst)

static int bt3c_hci_flush(struct hci_dev *hdev)
{
- bt3c_info_t *info = (bt3c_info_t *)(hdev->driver_data);
+ bt3c_info_t *info = hci_get_drvdata(hdev);

/* Drop TX queue */
skb_queue_purge(&(info->txq));
@@ -428,7 +428,7 @@ static int bt3c_hci_send_frame(struct sk_buff *skb)
return -ENODEV;
}

- info = (bt3c_info_t *) (hdev->driver_data);
+ info = hci_get_drvdata(hdev);

switch (bt_cb(skb)->pkt_type) {
case HCI_COMMAND_PKT:
@@ -575,7 +575,7 @@ static int bt3c_open(bt3c_info_t *info)
info->hdev = hdev;

hdev->bus = HCI_PCCARD;
- hdev->driver_data = info;
+ hci_set_drvdata(hdev, info);
SET_HCIDEV_DEV(hdev, &info->p_dev->dev);

hdev->open = bt3c_hci_open;
diff --git a/drivers/bluetooth/btmrvl_debugfs.c b/drivers/bluetooth/btmrvl_debugfs.c
index 8ecf4c6..60fe333 100644
--- a/drivers/bluetooth/btmrvl_debugfs.c
+++ b/drivers/bluetooth/btmrvl_debugfs.c
@@ -384,7 +384,7 @@ static const struct file_operations btmrvl_txdnldready_fops = {

void btmrvl_debugfs_init(struct hci_dev *hdev)
{
- struct btmrvl_private *priv = hdev->driver_data;
+ struct btmrvl_private *priv = hci_get_drvdata(hdev);
struct btmrvl_debugfs_data *dbg;

if (!hdev->debugfs)
@@ -401,36 +401,34 @@ void btmrvl_debugfs_init(struct hci_dev *hdev)
dbg->config_dir = debugfs_create_dir("config", hdev->debugfs);

dbg->psmode = debugfs_create_file("psmode", 0644, dbg->config_dir,
- hdev->driver_data, &btmrvl_psmode_fops);
+ priv, &btmrvl_psmode_fops);
dbg->pscmd = debugfs_create_file("pscmd", 0644, dbg->config_dir,
- hdev->driver_data, &btmrvl_pscmd_fops);
+ priv, &btmrvl_pscmd_fops);
dbg->gpiogap = debugfs_create_file("gpiogap", 0644, dbg->config_dir,
- hdev->driver_data, &btmrvl_gpiogap_fops);
+ priv, &btmrvl_gpiogap_fops);
dbg->hsmode = debugfs_create_file("hsmode", 0644, dbg->config_dir,
- hdev->driver_data, &btmrvl_hsmode_fops);
+ priv, &btmrvl_hsmode_fops);
dbg->hscmd = debugfs_create_file("hscmd", 0644, dbg->config_dir,
- hdev->driver_data, &btmrvl_hscmd_fops);
+ priv, &btmrvl_hscmd_fops);
dbg->hscfgcmd = debugfs_create_file("hscfgcmd", 0644, dbg->config_dir,
- hdev->driver_data, &btmrvl_hscfgcmd_fops);
+ priv, &btmrvl_hscfgcmd_fops);

dbg->status_dir = debugfs_create_dir("status", hdev->debugfs);
dbg->curpsmode = debugfs_create_file("curpsmode", 0444,
- dbg->status_dir,
- hdev->driver_data,
- &btmrvl_curpsmode_fops);
+ dbg->status_dir, priv, &btmrvl_curpsmode_fops);
dbg->psstate = debugfs_create_file("psstate", 0444, dbg->status_dir,
- hdev->driver_data, &btmrvl_psstate_fops);
+ priv, &btmrvl_psstate_fops);
dbg->hsstate = debugfs_create_file("hsstate", 0444, dbg->status_dir,
- hdev->driver_data, &btmrvl_hsstate_fops);
+ priv, &btmrvl_hsstate_fops);
dbg->txdnldready = debugfs_create_file("txdnldready", 0444,
dbg->status_dir,
- hdev->driver_data,
+ priv,
&btmrvl_txdnldready_fops);
}

void btmrvl_debugfs_remove(struct hci_dev *hdev)
{
- struct btmrvl_private *priv = hdev->driver_data;
+ struct btmrvl_private *priv = hci_get_drvdata(hdev);
struct btmrvl_debugfs_data *dbg = priv->debugfs_data;

if (!dbg)
diff --git a/drivers/bluetooth/btmrvl_main.c b/drivers/bluetooth/btmrvl_main.c
index 66b58fd..d1209ad 100644
--- a/drivers/bluetooth/btmrvl_main.c
+++ b/drivers/bluetooth/btmrvl_main.c
@@ -394,12 +394,13 @@ static int btmrvl_send_frame(struct sk_buff *skb)

BT_DBG("type=%d, len=%d", skb->pkt_type, skb->len);

- if (!hdev || !hdev->driver_data) {
+ if (!hdev) {
BT_ERR("Frame for unknown HCI device");
return -ENODEV;
}

- priv = (struct btmrvl_private *) hdev->driver_data;
+ priv = hci_get_drvdata(hdev);
+
if (!test_bit(HCI_RUNNING, &hdev->flags)) {
BT_ERR("Failed testing HCI_RUNING, flags=%lx", hdev->flags);
print_hex_dump_bytes("data: ", DUMP_PREFIX_OFFSET,
@@ -430,7 +431,7 @@ static int btmrvl_send_frame(struct sk_buff *skb)

static int btmrvl_flush(struct hci_dev *hdev)
{
- struct btmrvl_private *priv = hdev->driver_data;
+ struct btmrvl_private *priv = hci_get_drvdata(hdev);

skb_queue_purge(&priv->adapter->tx_queue);

@@ -439,7 +440,7 @@ static int btmrvl_flush(struct hci_dev *hdev)

static int btmrvl_close(struct hci_dev *hdev)
{
- struct btmrvl_private *priv = hdev->driver_data;
+ struct btmrvl_private *priv = hci_get_drvdata(hdev);

if (!test_and_clear_bit(HCI_RUNNING, &hdev->flags))
return 0;
@@ -542,7 +543,7 @@ int btmrvl_register_hdev(struct btmrvl_private *priv)
}

priv->btmrvl_dev.hcidev = hdev;
- hdev->driver_data = priv;
+ hci_set_drvdata(hdev, priv);

hdev->bus = HCI_SDIO;
hdev->open = btmrvl_open;
diff --git a/drivers/bluetooth/btsdio.c b/drivers/bluetooth/btsdio.c
index 2d6e4ed..e10ea03 100644
--- a/drivers/bluetooth/btsdio.c
+++ b/drivers/bluetooth/btsdio.c
@@ -189,7 +189,7 @@ static void btsdio_interrupt(struct sdio_func *func)

static int btsdio_open(struct hci_dev *hdev)
{
- struct btsdio_data *data = hdev->driver_data;
+ struct btsdio_data *data = hci_get_drvdata(hdev);
int err;

BT_DBG("%s", hdev->name);
@@ -225,7 +225,7 @@ release:

static int btsdio_close(struct hci_dev *hdev)
{
- struct btsdio_data *data = hdev->driver_data;
+ struct btsdio_data *data = hci_get_drvdata(hdev);

BT_DBG("%s", hdev->name);

@@ -246,7 +246,7 @@ static int btsdio_close(struct hci_dev *hdev)

static int btsdio_flush(struct hci_dev *hdev)
{
- struct btsdio_data *data = hdev->driver_data;
+ struct btsdio_data *data = hci_get_drvdata(hdev);

BT_DBG("%s", hdev->name);

@@ -258,7 +258,7 @@ static int btsdio_flush(struct hci_dev *hdev)
static int btsdio_send_frame(struct sk_buff *skb)
{
struct hci_dev *hdev = (struct hci_dev *) skb->dev;
- struct btsdio_data *data = hdev->driver_data;
+ struct btsdio_data *data = hci_get_drvdata(hdev);

BT_DBG("%s", hdev->name);

@@ -321,7 +321,7 @@ static int btsdio_probe(struct sdio_func *func,
}

hdev->bus = HCI_SDIO;
- hdev->driver_data = data;
+ hci_set_drvdata(hdev, data);

if (id->class == SDIO_CLASS_BT_AMP)
hdev->dev_type = HCI_AMP;
diff --git a/drivers/bluetooth/btuart_cs.c b/drivers/bluetooth/btuart_cs.c
index 80ad2b9..194224d 100644
--- a/drivers/bluetooth/btuart_cs.c
+++ b/drivers/bluetooth/btuart_cs.c
@@ -397,7 +397,7 @@ static void btuart_change_speed(btuart_info_t *info, unsigned int speed)

static int btuart_hci_flush(struct hci_dev *hdev)
{
- btuart_info_t *info = (btuart_info_t *)(hdev->driver_data);
+ btuart_info_t *info = hci_get_drvdata(hdev);

/* Drop TX queue */
skb_queue_purge(&(info->txq));
@@ -435,7 +435,7 @@ static int btuart_hci_send_frame(struct sk_buff *skb)
return -ENODEV;
}

- info = (btuart_info_t *)(hdev->driver_data);
+ info = hci_get_drvdata(hdev);

switch (bt_cb(skb)->pkt_type) {
case HCI_COMMAND_PKT:
@@ -493,7 +493,7 @@ static int btuart_open(btuart_info_t *info)
info->hdev = hdev;

hdev->bus = HCI_PCCARD;
- hdev->driver_data = info;
+ hci_set_drvdata(hdev, info);
SET_HCIDEV_DEV(hdev, &info->p_dev->dev);

hdev->open = btuart_hci_open;
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index d7664ff..0de3f5e 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -242,7 +242,7 @@ static int inc_tx(struct btusb_data *data)
static void btusb_intr_complete(struct urb *urb)
{
struct hci_dev *hdev = urb->context;
- struct btusb_data *data = hdev->driver_data;
+ struct btusb_data *data = hci_get_drvdata(hdev);
int err;

BT_DBG("%s urb %p status %d count %d", hdev->name,
@@ -281,7 +281,7 @@ static void btusb_intr_complete(struct urb *urb)

static int btusb_submit_intr_urb(struct hci_dev *hdev, gfp_t mem_flags)
{
- struct btusb_data *data = hdev->driver_data;
+ struct btusb_data *data = hci_get_drvdata(hdev);
struct urb *urb;
unsigned char *buf;
unsigned int pipe;
@@ -330,7 +330,7 @@ static int btusb_submit_intr_urb(struct hci_dev *hdev, gfp_t mem_flags)
static void btusb_bulk_complete(struct urb *urb)
{
struct hci_dev *hdev = urb->context;
- struct btusb_data *data = hdev->driver_data;
+ struct btusb_data *data = hci_get_drvdata(hdev);
int err;

BT_DBG("%s urb %p status %d count %d", hdev->name,
@@ -369,7 +369,7 @@ static void btusb_bulk_complete(struct urb *urb)

static int btusb_submit_bulk_urb(struct hci_dev *hdev, gfp_t mem_flags)
{
- struct btusb_data *data = hdev->driver_data;
+ struct btusb_data *data = hci_get_drvdata(hdev);
struct urb *urb;
unsigned char *buf;
unsigned int pipe;
@@ -416,7 +416,7 @@ static int btusb_submit_bulk_urb(struct hci_dev *hdev, gfp_t mem_flags)
static void btusb_isoc_complete(struct urb *urb)
{
struct hci_dev *hdev = urb->context;
- struct btusb_data *data = hdev->driver_data;
+ struct btusb_data *data = hci_get_drvdata(hdev);
int i, err;

BT_DBG("%s urb %p status %d count %d", hdev->name,
@@ -483,7 +483,7 @@ static inline void __fill_isoc_descriptor(struct urb *urb, int len, int mtu)

static int btusb_submit_isoc_urb(struct hci_dev *hdev, gfp_t mem_flags)
{
- struct btusb_data *data = hdev->driver_data;
+ struct btusb_data *data = hci_get_drvdata(hdev);
struct urb *urb;
unsigned char *buf;
unsigned int pipe;
@@ -536,7 +536,7 @@ static void btusb_tx_complete(struct urb *urb)
{
struct sk_buff *skb = urb->context;
struct hci_dev *hdev = (struct hci_dev *) skb->dev;
- struct btusb_data *data = hdev->driver_data;
+ struct btusb_data *data = hci_get_drvdata(hdev);

BT_DBG("%s urb %p status %d count %d", hdev->name,
urb, urb->status, urb->actual_length);
@@ -583,7 +583,7 @@ done:

static int btusb_open(struct hci_dev *hdev)
{
- struct btusb_data *data = hdev->driver_data;
+ struct btusb_data *data = hci_get_drvdata(hdev);
int err;

BT_DBG("%s", hdev->name);
@@ -633,7 +633,7 @@ static void btusb_stop_traffic(struct btusb_data *data)

static int btusb_close(struct hci_dev *hdev)
{
- struct btusb_data *data = hdev->driver_data;
+ struct btusb_data *data = hci_get_drvdata(hdev);
int err;

BT_DBG("%s", hdev->name);
@@ -663,7 +663,7 @@ failed:

static int btusb_flush(struct hci_dev *hdev)
{
- struct btusb_data *data = hdev->driver_data;
+ struct btusb_data *data = hci_get_drvdata(hdev);

BT_DBG("%s", hdev->name);

@@ -675,7 +675,7 @@ static int btusb_flush(struct hci_dev *hdev)
static int btusb_send_frame(struct sk_buff *skb)
{
struct hci_dev *hdev = (struct hci_dev *) skb->dev;
- struct btusb_data *data = hdev->driver_data;
+ struct btusb_data *data = hci_get_drvdata(hdev);
struct usb_ctrlrequest *dr;
struct urb *urb;
unsigned int pipe;
@@ -785,7 +785,7 @@ done:

static void btusb_notify(struct hci_dev *hdev, unsigned int evt)
{
- struct btusb_data *data = hdev->driver_data;
+ struct btusb_data *data = hci_get_drvdata(hdev);

BT_DBG("%s evt %d", hdev->name, evt);

@@ -797,7 +797,7 @@ static void btusb_notify(struct hci_dev *hdev, unsigned int evt)

static inline int __set_isoc_interface(struct hci_dev *hdev, int altsetting)
{
- struct btusb_data *data = hdev->driver_data;
+ struct btusb_data *data = hci_get_drvdata(hdev);
struct usb_interface *intf = data->isoc;
struct usb_endpoint_descriptor *ep_desc;
int i, err;
@@ -985,7 +985,7 @@ static int btusb_probe(struct usb_interface *intf,
}

hdev->bus = HCI_USB;
- hdev->driver_data = data;
+ hci_set_drvdata(hdev, data);

data->hdev = hdev;

diff --git a/drivers/bluetooth/btwilink.c b/drivers/bluetooth/btwilink.c
index b81b32e..8869469 100644
--- a/drivers/bluetooth/btwilink.c
+++ b/drivers/bluetooth/btwilink.c
@@ -161,7 +161,7 @@ static int ti_st_open(struct hci_dev *hdev)
return -EBUSY;

/* provide contexts for callbacks from ST */
- hst = hdev->driver_data;
+ hst = hci_get_drvdata(hdev);

for (i = 0; i < MAX_BT_CHNL_IDS; i++) {
ti_st_proto[i].priv_data = hst;
@@ -236,7 +236,7 @@ done:
static int ti_st_close(struct hci_dev *hdev)
{
int err, i;
- struct ti_st *hst = hdev->driver_data;
+ struct ti_st *hst = hci_get_drvdata(hdev);

if (!test_and_clear_bit(HCI_RUNNING, &hdev->flags))
return 0;
@@ -264,7 +264,7 @@ static int ti_st_send_frame(struct sk_buff *skb)
if (!test_bit(HCI_RUNNING, &hdev->flags))
return -EBUSY;

- hst = hdev->driver_data;
+ hst = hci_get_drvdata(hdev);

/* Prepend skb with frame type */
memcpy(skb_push(skb, 1), &bt_cb(skb)->pkt_type, 1);
@@ -312,7 +312,7 @@ static int bt_ti_probe(struct platform_device *pdev)

hst->hdev = hdev;
hdev->bus = HCI_UART;
- hdev->driver_data = hst;
+ hci_set_drvdata(hdev, hst);
hdev->open = ti_st_open;
hdev->close = ti_st_close;
hdev->flush = NULL;
diff --git a/drivers/bluetooth/dtl1_cs.c b/drivers/bluetooth/dtl1_cs.c
index 295cf1b..049c059 100644
--- a/drivers/bluetooth/dtl1_cs.c
+++ b/drivers/bluetooth/dtl1_cs.c
@@ -364,7 +364,7 @@ static int dtl1_hci_open(struct hci_dev *hdev)

static int dtl1_hci_flush(struct hci_dev *hdev)
{
- dtl1_info_t *info = (dtl1_info_t *)(hdev->driver_data);
+ dtl1_info_t *info = hci_get_drvdata(hdev);

/* Drop TX queue */
skb_queue_purge(&(info->txq));
@@ -396,7 +396,7 @@ static int dtl1_hci_send_frame(struct sk_buff *skb)
return -ENODEV;
}

- info = (dtl1_info_t *)(hdev->driver_data);
+ info = hci_get_drvdata(hdev);

switch (bt_cb(skb)->pkt_type) {
case HCI_COMMAND_PKT:
@@ -475,7 +475,7 @@ static int dtl1_open(dtl1_info_t *info)
info->hdev = hdev;

hdev->bus = HCI_PCCARD;
- hdev->driver_data = info;
+ hci_set_drvdata(hdev, info);
SET_HCIDEV_DEV(hdev, &info->p_dev->dev);

hdev->open = dtl1_hci_open;
diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
index 459ff0b..01c23df 100644
--- a/drivers/bluetooth/hci_ldisc.c
+++ b/drivers/bluetooth/hci_ldisc.c
@@ -174,7 +174,7 @@ static int hci_uart_open(struct hci_dev *hdev)
/* Reset device */
static int hci_uart_flush(struct hci_dev *hdev)
{
- struct hci_uart *hu = (struct hci_uart *) hdev->driver_data;
+ struct hci_uart *hu = hci_get_drvdata(hdev);
struct tty_struct *tty = hu->tty;

BT_DBG("hdev %p tty %p", hdev, tty);
@@ -220,7 +220,7 @@ static int hci_uart_send_frame(struct sk_buff *skb)
if (!test_bit(HCI_RUNNING, &hdev->flags))
return -EBUSY;

- hu = (struct hci_uart *) hdev->driver_data;
+ hu = hci_get_drvdata(hdev);

BT_DBG("%s: type %d len %d", hdev->name, bt_cb(skb)->pkt_type, skb->len);

@@ -384,7 +384,7 @@ static int hci_uart_register_dev(struct hci_uart *hu)
hu->hdev = hdev;

hdev->bus = HCI_UART;
- hdev->driver_data = hu;
+ hci_set_drvdata(hdev, hu);

hdev->open = hci_uart_open;
hdev->close = hci_uart_close;
diff --git a/drivers/bluetooth/hci_vhci.c b/drivers/bluetooth/hci_vhci.c
index 5f305c1..158bfe5 100644
--- a/drivers/bluetooth/hci_vhci.c
+++ b/drivers/bluetooth/hci_vhci.c
@@ -61,7 +61,7 @@ static int vhci_open_dev(struct hci_dev *hdev)

static int vhci_close_dev(struct hci_dev *hdev)
{
- struct vhci_data *data = hdev->driver_data;
+ struct vhci_data *data = hci_get_drvdata(hdev);

if (!test_and_clear_bit(HCI_RUNNING, &hdev->flags))
return 0;
@@ -73,7 +73,7 @@ static int vhci_close_dev(struct hci_dev *hdev)

static int vhci_flush(struct hci_dev *hdev)
{
- struct vhci_data *data = hdev->driver_data;
+ struct vhci_data *data = hci_get_drvdata(hdev);

skb_queue_purge(&data->readq);

@@ -93,7 +93,7 @@ static int vhci_send_frame(struct sk_buff *skb)
if (!test_bit(HCI_RUNNING, &hdev->flags))
return -EBUSY;

- data = hdev->driver_data;
+ data = hci_get_drvdata(hdev);

memcpy(skb_push(skb, 1), &bt_cb(skb)->pkt_type, 1);
skb_queue_tail(&data->readq, skb);
@@ -234,7 +234,7 @@ static int vhci_open(struct inode *inode, struct file *file)
data->hdev = hdev;

hdev->bus = HCI_VIRTUAL;
- hdev->driver_data = data;
+ hci_set_drvdata(hdev, data);

if (amp)
hdev->dev_type = HCI_AMP;
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 01fdd2c..9780f42 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -248,7 +248,6 @@ struct hci_dev {

struct sk_buff_head driver_init;

- void *driver_data;
void *core_data;

atomic_t promisc;
@@ -614,6 +613,16 @@ static inline struct hci_dev *hci_dev_hold(struct hci_dev *d)

#define to_hci_dev(d) container_of(d, struct hci_dev, dev)

+static inline void *hci_get_drvdata(struct hci_dev *hdev)
+{
+ return dev_get_drvdata(&hdev->dev);
+}
+
+static inline void hci_set_drvdata(struct hci_dev *hdev, void *data)
+{
+ dev_set_drvdata(&hdev->dev, data);
+}
+
struct hci_dev *hci_dev_get(int index);
struct hci_dev *hci_get_route(bdaddr_t *src, bdaddr_t *dst);

--
1.7.8.4

2012-01-27 16:56:50

by David Herrmann

[permalink] [raw]
Subject: Re: [PATCH 2/4] Bluetooth: Remove hci_dev->driver_data

Hi Anderson

On Fri, Jan 27, 2012 at 5:51 PM, Anderson Lizardo
<[email protected]> wrote:
> Hi David,
>
> On Fri, Jan 27, 2012 at 11:47 AM, David Herrmann
> <[email protected]> wrote:
>> diff --git a/drivers/bluetooth/btmrvl_main.c b/drivers/bluetooth/btmrvl_=
main.c
>> index 66b58fd..8d0e090 100644
>> --- a/drivers/bluetooth/btmrvl_main.c
>> +++ b/drivers/bluetooth/btmrvl_main.c
>> @@ -394,12 +394,12 @@ static int btmrvl_send_frame(struct sk_buff *skb)
>>
>> =A0 =A0 =A0 =A0BT_DBG("type=3D%d, len=3D%d", skb->pkt_type, skb->len);
>>
>> - =A0 =A0 =A0 if (!hdev || !hdev->driver_data) {
>> + =A0 =A0 =A0 priv =3D hci_get_drvdata(hdev);
>> + =A0 =A0 =A0 if (!hdev || !priv) {
>
> The change above will cause problems if hdev is NULL.
> hci_get_drvdata() dereferences hdev without checking for NULL.

Thanks for reviewing. I will resend it without the check for !priv. It
doesn't make sense to check this. It can never be NULL, anyway.

>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0BT_ERR("Frame for unknown HCI device");
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0return -ENODEV;
>> =A0 =A0 =A0 =A0}
>>
>> - =A0 =A0 =A0 priv =3D (struct btmrvl_private *) hdev->driver_data;
>> =A0 =A0 =A0 =A0if (!test_bit(HCI_RUNNING, &hdev->flags)) {
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0BT_ERR("Failed testing HCI_RUNING, flags=
=3D%lx", hdev->flags);
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0print_hex_dump_bytes("data: ", DUMP_PREFI=
X_OFFSET,
> [...]
>> @@ -614,6 +613,16 @@ static inline struct hci_dev *hci_dev_hold(struct h=
ci_dev *d)
>>
>> =A0#define to_hci_dev(d) container_of(d, struct hci_dev, dev)
>>
>> +static inline void *hci_get_drvdata(struct hci_dev *hdev)
>> +{
>> + =A0 =A0 =A0 return dev_get_drvdata(&hdev->dev);
>> +}
>> +
>> +static inline void hci_set_drvdata(struct hci_dev *hdev, void *data)
>> +{
>> + =A0 =A0 =A0 dev_set_drvdata(&hdev->dev, data);
>> +}
>> +
>> =A0struct hci_dev *hci_dev_get(int index);
>> =A0struct hci_dev *hci_get_route(bdaddr_t *src, bdaddr_t *dst);
>
> Regards,

Regards
David

2012-01-27 16:51:21

by Anderson Lizardo

[permalink] [raw]
Subject: Re: [PATCH 2/4] Bluetooth: Remove hci_dev->driver_data

Hi David,

On Fri, Jan 27, 2012 at 11:47 AM, David Herrmann
<[email protected]> wrote:
> diff --git a/drivers/bluetooth/btmrvl_main.c b/drivers/bluetooth/btmrvl_m=
ain.c
> index 66b58fd..8d0e090 100644
> --- a/drivers/bluetooth/btmrvl_main.c
> +++ b/drivers/bluetooth/btmrvl_main.c
> @@ -394,12 +394,12 @@ static int btmrvl_send_frame(struct sk_buff *skb)
>
> =A0 =A0 =A0 =A0BT_DBG("type=3D%d, len=3D%d", skb->pkt_type, skb->len);
>
> - =A0 =A0 =A0 if (!hdev || !hdev->driver_data) {
> + =A0 =A0 =A0 priv =3D hci_get_drvdata(hdev);
> + =A0 =A0 =A0 if (!hdev || !priv) {

The change above will cause problems if hdev is NULL.
hci_get_drvdata() dereferences hdev without checking for NULL.

> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0BT_ERR("Frame for unknown HCI device");
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0return -ENODEV;
> =A0 =A0 =A0 =A0}
>
> - =A0 =A0 =A0 priv =3D (struct btmrvl_private *) hdev->driver_data;
> =A0 =A0 =A0 =A0if (!test_bit(HCI_RUNNING, &hdev->flags)) {
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0BT_ERR("Failed testing HCI_RUNING, flags=
=3D%lx", hdev->flags);
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0print_hex_dump_bytes("data: ", DUMP_PREFIX=
_OFFSET,
[...]
> @@ -614,6 +613,16 @@ static inline struct hci_dev *hci_dev_hold(struct hc=
i_dev *d)
>
> =A0#define to_hci_dev(d) container_of(d, struct hci_dev, dev)
>
> +static inline void *hci_get_drvdata(struct hci_dev *hdev)
> +{
> + =A0 =A0 =A0 return dev_get_drvdata(&hdev->dev);
> +}
> +
> +static inline void hci_set_drvdata(struct hci_dev *hdev, void *data)
> +{
> + =A0 =A0 =A0 dev_set_drvdata(&hdev->dev, data);
> +}
> +
> =A0struct hci_dev *hci_dev_get(int index);
> =A0struct hci_dev *hci_get_route(bdaddr_t *src, bdaddr_t *dst);

Regards,
--=20
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil

2012-01-27 15:47:26

by David Herrmann

[permalink] [raw]
Subject: [PATCH 4/4] Bluetooth: Use proper datatypes in release-callbacks

This enhances code readability a lot and avoids using void* even though
we know the type of the variable.

Signed-off-by: David Herrmann <[email protected]>
---
net/bluetooth/hci_sysfs.c | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/net/bluetooth/hci_sysfs.c b/net/bluetooth/hci_sysfs.c
index 17e6cd4..bc15429 100644
--- a/net/bluetooth/hci_sysfs.c
+++ b/net/bluetooth/hci_sysfs.c
@@ -79,8 +79,8 @@ static const struct attribute_group *bt_link_groups[] = {

static void bt_link_release(struct device *dev)
{
- void *data = to_hci_conn(dev);
- kfree(data);
+ struct hci_conn *conn = to_hci_conn(dev);
+ kfree(conn);
}

static struct device_type bt_link = {
@@ -368,8 +368,8 @@ static const struct attribute_group *bt_host_groups[] = {

static void bt_host_release(struct device *dev)
{
- void *data = to_hci_dev(dev);
- kfree(data);
+ struct hci_dev *hdev = to_hci_dev(dev);
+ kfree(hdev);
module_put(THIS_MODULE);
}

--
1.7.8.4

2012-01-27 15:47:24

by David Herrmann

[permalink] [raw]
Subject: [PATCH 2/4] Bluetooth: Remove hci_dev->driver_data

The linux device model provides dev_set/get_drvdata so we can use this
to save private driver data.
This also removes several unnecessary casts.

Signed-off-by: David Herrmann <[email protected]>
---
drivers/bluetooth/bfusb.c | 10 +++++-----
drivers/bluetooth/bluecard_cs.c | 12 ++++++------
drivers/bluetooth/bpa10x.c | 18 +++++++++---------
drivers/bluetooth/bt3c_cs.c | 6 +++---
drivers/bluetooth/btmrvl_debugfs.c | 26 ++++++++++++--------------
drivers/bluetooth/btmrvl_main.c | 10 +++++-----
drivers/bluetooth/btsdio.c | 10 +++++-----
drivers/bluetooth/btuart_cs.c | 6 +++---
drivers/bluetooth/btusb.c | 28 ++++++++++++++--------------
drivers/bluetooth/btwilink.c | 8 ++++----
drivers/bluetooth/dtl1_cs.c | 6 +++---
drivers/bluetooth/hci_ldisc.c | 6 +++---
drivers/bluetooth/hci_vhci.c | 8 ++++----
include/net/bluetooth/hci_core.h | 11 ++++++++++-
14 files changed, 86 insertions(+), 79 deletions(-)

diff --git a/drivers/bluetooth/bfusb.c b/drivers/bluetooth/bfusb.c
index c7d6ff0..b8ac1c5 100644
--- a/drivers/bluetooth/bfusb.c
+++ b/drivers/bluetooth/bfusb.c
@@ -411,7 +411,7 @@ unlock:

static int bfusb_open(struct hci_dev *hdev)
{
- struct bfusb_data *data = hdev->driver_data;
+ struct bfusb_data *data = hci_get_drvdata(hdev);
unsigned long flags;
int i, err;

@@ -437,7 +437,7 @@ static int bfusb_open(struct hci_dev *hdev)

static int bfusb_flush(struct hci_dev *hdev)
{
- struct bfusb_data *data = hdev->driver_data;
+ struct bfusb_data *data = hci_get_drvdata(hdev);

BT_DBG("hdev %p bfusb %p", hdev, data);

@@ -448,7 +448,7 @@ static int bfusb_flush(struct hci_dev *hdev)

static int bfusb_close(struct hci_dev *hdev)
{
- struct bfusb_data *data = hdev->driver_data;
+ struct bfusb_data *data = hci_get_drvdata(hdev);
unsigned long flags;

BT_DBG("hdev %p bfusb %p", hdev, data);
@@ -483,7 +483,7 @@ static int bfusb_send_frame(struct sk_buff *skb)
if (!test_bit(HCI_RUNNING, &hdev->flags))
return -EBUSY;

- data = hdev->driver_data;
+ data = hci_get_drvdata(hdev);

switch (bt_cb(skb)->pkt_type) {
case HCI_COMMAND_PKT:
@@ -696,7 +696,7 @@ static int bfusb_probe(struct usb_interface *intf, const struct usb_device_id *i
data->hdev = hdev;

hdev->bus = HCI_USB;
- hdev->driver_data = data;
+ hci_set_drvdata(hdev, data);
SET_HCIDEV_DEV(hdev, &intf->dev);

hdev->open = bfusb_open;
diff --git a/drivers/bluetooth/bluecard_cs.c b/drivers/bluetooth/bluecard_cs.c
index 6b1261f..1fcd923 100644
--- a/drivers/bluetooth/bluecard_cs.c
+++ b/drivers/bluetooth/bluecard_cs.c
@@ -561,7 +561,7 @@ static irqreturn_t bluecard_interrupt(int irq, void *dev_inst)

static int bluecard_hci_set_baud_rate(struct hci_dev *hdev, int baud)
{
- bluecard_info_t *info = (bluecard_info_t *)(hdev->driver_data);
+ bluecard_info_t *info = hci_get_drvdata(hdev);
struct sk_buff *skb;

/* Ericsson baud rate command */
@@ -609,7 +609,7 @@ static int bluecard_hci_set_baud_rate(struct hci_dev *hdev, int baud)

static int bluecard_hci_flush(struct hci_dev *hdev)
{
- bluecard_info_t *info = (bluecard_info_t *)(hdev->driver_data);
+ bluecard_info_t *info = hci_get_drvdata(hdev);

/* Drop TX queue */
skb_queue_purge(&(info->txq));
@@ -620,7 +620,7 @@ static int bluecard_hci_flush(struct hci_dev *hdev)

static int bluecard_hci_open(struct hci_dev *hdev)
{
- bluecard_info_t *info = (bluecard_info_t *)(hdev->driver_data);
+ bluecard_info_t *info = hci_get_drvdata(hdev);
unsigned int iobase = info->p_dev->resource[0]->start;

if (test_bit(CARD_HAS_PCCARD_ID, &(info->hw_state)))
@@ -640,7 +640,7 @@ static int bluecard_hci_open(struct hci_dev *hdev)

static int bluecard_hci_close(struct hci_dev *hdev)
{
- bluecard_info_t *info = (bluecard_info_t *)(hdev->driver_data);
+ bluecard_info_t *info = hci_get_drvdata(hdev);
unsigned int iobase = info->p_dev->resource[0]->start;

if (!test_and_clear_bit(HCI_RUNNING, &(hdev->flags)))
@@ -667,7 +667,7 @@ static int bluecard_hci_send_frame(struct sk_buff *skb)
return -ENODEV;
}

- info = (bluecard_info_t *)(hdev->driver_data);
+ info = hci_get_drvdata(hdev);

switch (bt_cb(skb)->pkt_type) {
case HCI_COMMAND_PKT:
@@ -729,7 +729,7 @@ static int bluecard_open(bluecard_info_t *info)
info->hdev = hdev;

hdev->bus = HCI_PCCARD;
- hdev->driver_data = info;
+ hci_set_drvdata(hdev, info);
SET_HCIDEV_DEV(hdev, &info->p_dev->dev);

hdev->open = bluecard_hci_open;
diff --git a/drivers/bluetooth/bpa10x.c b/drivers/bluetooth/bpa10x.c
index 9d63514..d894340 100644
--- a/drivers/bluetooth/bpa10x.c
+++ b/drivers/bluetooth/bpa10x.c
@@ -66,7 +66,7 @@ struct hci_vendor_hdr {

static int bpa10x_recv(struct hci_dev *hdev, int queue, void *buf, int count)
{
- struct bpa10x_data *data = hdev->driver_data;
+ struct bpa10x_data *data = hci_get_drvdata(hdev);

BT_DBG("%s queue %d buffer %p count %d", hdev->name,
queue, buf, count);
@@ -189,7 +189,7 @@ done:
static void bpa10x_rx_complete(struct urb *urb)
{
struct hci_dev *hdev = urb->context;
- struct bpa10x_data *data = hdev->driver_data;
+ struct bpa10x_data *data = hci_get_drvdata(hdev);
int err;

BT_DBG("%s urb %p status %d count %d", hdev->name,
@@ -219,7 +219,7 @@ static void bpa10x_rx_complete(struct urb *urb)

static inline int bpa10x_submit_intr_urb(struct hci_dev *hdev)
{
- struct bpa10x_data *data = hdev->driver_data;
+ struct bpa10x_data *data = hci_get_drvdata(hdev);
struct urb *urb;
unsigned char *buf;
unsigned int pipe;
@@ -260,7 +260,7 @@ static inline int bpa10x_submit_intr_urb(struct hci_dev *hdev)

static inline int bpa10x_submit_bulk_urb(struct hci_dev *hdev)
{
- struct bpa10x_data *data = hdev->driver_data;
+ struct bpa10x_data *data = hci_get_drvdata(hdev);
struct urb *urb;
unsigned char *buf;
unsigned int pipe;
@@ -301,7 +301,7 @@ static inline int bpa10x_submit_bulk_urb(struct hci_dev *hdev)

static int bpa10x_open(struct hci_dev *hdev)
{
- struct bpa10x_data *data = hdev->driver_data;
+ struct bpa10x_data *data = hci_get_drvdata(hdev);
int err;

BT_DBG("%s", hdev->name);
@@ -329,7 +329,7 @@ error:

static int bpa10x_close(struct hci_dev *hdev)
{
- struct bpa10x_data *data = hdev->driver_data;
+ struct bpa10x_data *data = hci_get_drvdata(hdev);

BT_DBG("%s", hdev->name);

@@ -343,7 +343,7 @@ static int bpa10x_close(struct hci_dev *hdev)

static int bpa10x_flush(struct hci_dev *hdev)
{
- struct bpa10x_data *data = hdev->driver_data;
+ struct bpa10x_data *data = hci_get_drvdata(hdev);

BT_DBG("%s", hdev->name);

@@ -355,7 +355,7 @@ static int bpa10x_flush(struct hci_dev *hdev)
static int bpa10x_send_frame(struct sk_buff *skb)
{
struct hci_dev *hdev = (struct hci_dev *) skb->dev;
- struct bpa10x_data *data = hdev->driver_data;
+ struct bpa10x_data *data = hci_get_drvdata(hdev);
struct usb_ctrlrequest *dr;
struct urb *urb;
unsigned int pipe;
@@ -459,7 +459,7 @@ static int bpa10x_probe(struct usb_interface *intf, const struct usb_device_id *
}

hdev->bus = HCI_USB;
- hdev->driver_data = data;
+ hci_set_drvdata(hdev, data);

data->hdev = hdev;

diff --git a/drivers/bluetooth/bt3c_cs.c b/drivers/bluetooth/bt3c_cs.c
index 0e304cb..9c09d6f 100644
--- a/drivers/bluetooth/bt3c_cs.c
+++ b/drivers/bluetooth/bt3c_cs.c
@@ -389,7 +389,7 @@ static irqreturn_t bt3c_interrupt(int irq, void *dev_inst)

static int bt3c_hci_flush(struct hci_dev *hdev)
{
- bt3c_info_t *info = (bt3c_info_t *)(hdev->driver_data);
+ bt3c_info_t *info = hci_get_drvdata(hdev);

/* Drop TX queue */
skb_queue_purge(&(info->txq));
@@ -428,7 +428,7 @@ static int bt3c_hci_send_frame(struct sk_buff *skb)
return -ENODEV;
}

- info = (bt3c_info_t *) (hdev->driver_data);
+ info = hci_get_drvdata(hdev);

switch (bt_cb(skb)->pkt_type) {
case HCI_COMMAND_PKT:
@@ -575,7 +575,7 @@ static int bt3c_open(bt3c_info_t *info)
info->hdev = hdev;

hdev->bus = HCI_PCCARD;
- hdev->driver_data = info;
+ hci_set_drvdata(hdev, info);
SET_HCIDEV_DEV(hdev, &info->p_dev->dev);

hdev->open = bt3c_hci_open;
diff --git a/drivers/bluetooth/btmrvl_debugfs.c b/drivers/bluetooth/btmrvl_debugfs.c
index 8ecf4c6..60fe333 100644
--- a/drivers/bluetooth/btmrvl_debugfs.c
+++ b/drivers/bluetooth/btmrvl_debugfs.c
@@ -384,7 +384,7 @@ static const struct file_operations btmrvl_txdnldready_fops = {

void btmrvl_debugfs_init(struct hci_dev *hdev)
{
- struct btmrvl_private *priv = hdev->driver_data;
+ struct btmrvl_private *priv = hci_get_drvdata(hdev);
struct btmrvl_debugfs_data *dbg;

if (!hdev->debugfs)
@@ -401,36 +401,34 @@ void btmrvl_debugfs_init(struct hci_dev *hdev)
dbg->config_dir = debugfs_create_dir("config", hdev->debugfs);

dbg->psmode = debugfs_create_file("psmode", 0644, dbg->config_dir,
- hdev->driver_data, &btmrvl_psmode_fops);
+ priv, &btmrvl_psmode_fops);
dbg->pscmd = debugfs_create_file("pscmd", 0644, dbg->config_dir,
- hdev->driver_data, &btmrvl_pscmd_fops);
+ priv, &btmrvl_pscmd_fops);
dbg->gpiogap = debugfs_create_file("gpiogap", 0644, dbg->config_dir,
- hdev->driver_data, &btmrvl_gpiogap_fops);
+ priv, &btmrvl_gpiogap_fops);
dbg->hsmode = debugfs_create_file("hsmode", 0644, dbg->config_dir,
- hdev->driver_data, &btmrvl_hsmode_fops);
+ priv, &btmrvl_hsmode_fops);
dbg->hscmd = debugfs_create_file("hscmd", 0644, dbg->config_dir,
- hdev->driver_data, &btmrvl_hscmd_fops);
+ priv, &btmrvl_hscmd_fops);
dbg->hscfgcmd = debugfs_create_file("hscfgcmd", 0644, dbg->config_dir,
- hdev->driver_data, &btmrvl_hscfgcmd_fops);
+ priv, &btmrvl_hscfgcmd_fops);

dbg->status_dir = debugfs_create_dir("status", hdev->debugfs);
dbg->curpsmode = debugfs_create_file("curpsmode", 0444,
- dbg->status_dir,
- hdev->driver_data,
- &btmrvl_curpsmode_fops);
+ dbg->status_dir, priv, &btmrvl_curpsmode_fops);
dbg->psstate = debugfs_create_file("psstate", 0444, dbg->status_dir,
- hdev->driver_data, &btmrvl_psstate_fops);
+ priv, &btmrvl_psstate_fops);
dbg->hsstate = debugfs_create_file("hsstate", 0444, dbg->status_dir,
- hdev->driver_data, &btmrvl_hsstate_fops);
+ priv, &btmrvl_hsstate_fops);
dbg->txdnldready = debugfs_create_file("txdnldready", 0444,
dbg->status_dir,
- hdev->driver_data,
+ priv,
&btmrvl_txdnldready_fops);
}

void btmrvl_debugfs_remove(struct hci_dev *hdev)
{
- struct btmrvl_private *priv = hdev->driver_data;
+ struct btmrvl_private *priv = hci_get_drvdata(hdev);
struct btmrvl_debugfs_data *dbg = priv->debugfs_data;

if (!dbg)
diff --git a/drivers/bluetooth/btmrvl_main.c b/drivers/bluetooth/btmrvl_main.c
index 66b58fd..8d0e090 100644
--- a/drivers/bluetooth/btmrvl_main.c
+++ b/drivers/bluetooth/btmrvl_main.c
@@ -394,12 +394,12 @@ static int btmrvl_send_frame(struct sk_buff *skb)

BT_DBG("type=%d, len=%d", skb->pkt_type, skb->len);

- if (!hdev || !hdev->driver_data) {
+ priv = hci_get_drvdata(hdev);
+ if (!hdev || !priv) {
BT_ERR("Frame for unknown HCI device");
return -ENODEV;
}

- priv = (struct btmrvl_private *) hdev->driver_data;
if (!test_bit(HCI_RUNNING, &hdev->flags)) {
BT_ERR("Failed testing HCI_RUNING, flags=%lx", hdev->flags);
print_hex_dump_bytes("data: ", DUMP_PREFIX_OFFSET,
@@ -430,7 +430,7 @@ static int btmrvl_send_frame(struct sk_buff *skb)

static int btmrvl_flush(struct hci_dev *hdev)
{
- struct btmrvl_private *priv = hdev->driver_data;
+ struct btmrvl_private *priv = hci_get_drvdata(hdev);

skb_queue_purge(&priv->adapter->tx_queue);

@@ -439,7 +439,7 @@ static int btmrvl_flush(struct hci_dev *hdev)

static int btmrvl_close(struct hci_dev *hdev)
{
- struct btmrvl_private *priv = hdev->driver_data;
+ struct btmrvl_private *priv = hci_get_drvdata(hdev);

if (!test_and_clear_bit(HCI_RUNNING, &hdev->flags))
return 0;
@@ -542,7 +542,7 @@ int btmrvl_register_hdev(struct btmrvl_private *priv)
}

priv->btmrvl_dev.hcidev = hdev;
- hdev->driver_data = priv;
+ hci_set_drvdata(hdev, priv);

hdev->bus = HCI_SDIO;
hdev->open = btmrvl_open;
diff --git a/drivers/bluetooth/btsdio.c b/drivers/bluetooth/btsdio.c
index 2d6e4ed..e10ea03 100644
--- a/drivers/bluetooth/btsdio.c
+++ b/drivers/bluetooth/btsdio.c
@@ -189,7 +189,7 @@ static void btsdio_interrupt(struct sdio_func *func)

static int btsdio_open(struct hci_dev *hdev)
{
- struct btsdio_data *data = hdev->driver_data;
+ struct btsdio_data *data = hci_get_drvdata(hdev);
int err;

BT_DBG("%s", hdev->name);
@@ -225,7 +225,7 @@ release:

static int btsdio_close(struct hci_dev *hdev)
{
- struct btsdio_data *data = hdev->driver_data;
+ struct btsdio_data *data = hci_get_drvdata(hdev);

BT_DBG("%s", hdev->name);

@@ -246,7 +246,7 @@ static int btsdio_close(struct hci_dev *hdev)

static int btsdio_flush(struct hci_dev *hdev)
{
- struct btsdio_data *data = hdev->driver_data;
+ struct btsdio_data *data = hci_get_drvdata(hdev);

BT_DBG("%s", hdev->name);

@@ -258,7 +258,7 @@ static int btsdio_flush(struct hci_dev *hdev)
static int btsdio_send_frame(struct sk_buff *skb)
{
struct hci_dev *hdev = (struct hci_dev *) skb->dev;
- struct btsdio_data *data = hdev->driver_data;
+ struct btsdio_data *data = hci_get_drvdata(hdev);

BT_DBG("%s", hdev->name);

@@ -321,7 +321,7 @@ static int btsdio_probe(struct sdio_func *func,
}

hdev->bus = HCI_SDIO;
- hdev->driver_data = data;
+ hci_set_drvdata(hdev, data);

if (id->class == SDIO_CLASS_BT_AMP)
hdev->dev_type = HCI_AMP;
diff --git a/drivers/bluetooth/btuart_cs.c b/drivers/bluetooth/btuart_cs.c
index 80ad2b9..194224d 100644
--- a/drivers/bluetooth/btuart_cs.c
+++ b/drivers/bluetooth/btuart_cs.c
@@ -397,7 +397,7 @@ static void btuart_change_speed(btuart_info_t *info, unsigned int speed)

static int btuart_hci_flush(struct hci_dev *hdev)
{
- btuart_info_t *info = (btuart_info_t *)(hdev->driver_data);
+ btuart_info_t *info = hci_get_drvdata(hdev);

/* Drop TX queue */
skb_queue_purge(&(info->txq));
@@ -435,7 +435,7 @@ static int btuart_hci_send_frame(struct sk_buff *skb)
return -ENODEV;
}

- info = (btuart_info_t *)(hdev->driver_data);
+ info = hci_get_drvdata(hdev);

switch (bt_cb(skb)->pkt_type) {
case HCI_COMMAND_PKT:
@@ -493,7 +493,7 @@ static int btuart_open(btuart_info_t *info)
info->hdev = hdev;

hdev->bus = HCI_PCCARD;
- hdev->driver_data = info;
+ hci_set_drvdata(hdev, info);
SET_HCIDEV_DEV(hdev, &info->p_dev->dev);

hdev->open = btuart_hci_open;
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index d7664ff..0de3f5e 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -242,7 +242,7 @@ static int inc_tx(struct btusb_data *data)
static void btusb_intr_complete(struct urb *urb)
{
struct hci_dev *hdev = urb->context;
- struct btusb_data *data = hdev->driver_data;
+ struct btusb_data *data = hci_get_drvdata(hdev);
int err;

BT_DBG("%s urb %p status %d count %d", hdev->name,
@@ -281,7 +281,7 @@ static void btusb_intr_complete(struct urb *urb)

static int btusb_submit_intr_urb(struct hci_dev *hdev, gfp_t mem_flags)
{
- struct btusb_data *data = hdev->driver_data;
+ struct btusb_data *data = hci_get_drvdata(hdev);
struct urb *urb;
unsigned char *buf;
unsigned int pipe;
@@ -330,7 +330,7 @@ static int btusb_submit_intr_urb(struct hci_dev *hdev, gfp_t mem_flags)
static void btusb_bulk_complete(struct urb *urb)
{
struct hci_dev *hdev = urb->context;
- struct btusb_data *data = hdev->driver_data;
+ struct btusb_data *data = hci_get_drvdata(hdev);
int err;

BT_DBG("%s urb %p status %d count %d", hdev->name,
@@ -369,7 +369,7 @@ static void btusb_bulk_complete(struct urb *urb)

static int btusb_submit_bulk_urb(struct hci_dev *hdev, gfp_t mem_flags)
{
- struct btusb_data *data = hdev->driver_data;
+ struct btusb_data *data = hci_get_drvdata(hdev);
struct urb *urb;
unsigned char *buf;
unsigned int pipe;
@@ -416,7 +416,7 @@ static int btusb_submit_bulk_urb(struct hci_dev *hdev, gfp_t mem_flags)
static void btusb_isoc_complete(struct urb *urb)
{
struct hci_dev *hdev = urb->context;
- struct btusb_data *data = hdev->driver_data;
+ struct btusb_data *data = hci_get_drvdata(hdev);
int i, err;

BT_DBG("%s urb %p status %d count %d", hdev->name,
@@ -483,7 +483,7 @@ static inline void __fill_isoc_descriptor(struct urb *urb, int len, int mtu)

static int btusb_submit_isoc_urb(struct hci_dev *hdev, gfp_t mem_flags)
{
- struct btusb_data *data = hdev->driver_data;
+ struct btusb_data *data = hci_get_drvdata(hdev);
struct urb *urb;
unsigned char *buf;
unsigned int pipe;
@@ -536,7 +536,7 @@ static void btusb_tx_complete(struct urb *urb)
{
struct sk_buff *skb = urb->context;
struct hci_dev *hdev = (struct hci_dev *) skb->dev;
- struct btusb_data *data = hdev->driver_data;
+ struct btusb_data *data = hci_get_drvdata(hdev);

BT_DBG("%s urb %p status %d count %d", hdev->name,
urb, urb->status, urb->actual_length);
@@ -583,7 +583,7 @@ done:

static int btusb_open(struct hci_dev *hdev)
{
- struct btusb_data *data = hdev->driver_data;
+ struct btusb_data *data = hci_get_drvdata(hdev);
int err;

BT_DBG("%s", hdev->name);
@@ -633,7 +633,7 @@ static void btusb_stop_traffic(struct btusb_data *data)

static int btusb_close(struct hci_dev *hdev)
{
- struct btusb_data *data = hdev->driver_data;
+ struct btusb_data *data = hci_get_drvdata(hdev);
int err;

BT_DBG("%s", hdev->name);
@@ -663,7 +663,7 @@ failed:

static int btusb_flush(struct hci_dev *hdev)
{
- struct btusb_data *data = hdev->driver_data;
+ struct btusb_data *data = hci_get_drvdata(hdev);

BT_DBG("%s", hdev->name);

@@ -675,7 +675,7 @@ static int btusb_flush(struct hci_dev *hdev)
static int btusb_send_frame(struct sk_buff *skb)
{
struct hci_dev *hdev = (struct hci_dev *) skb->dev;
- struct btusb_data *data = hdev->driver_data;
+ struct btusb_data *data = hci_get_drvdata(hdev);
struct usb_ctrlrequest *dr;
struct urb *urb;
unsigned int pipe;
@@ -785,7 +785,7 @@ done:

static void btusb_notify(struct hci_dev *hdev, unsigned int evt)
{
- struct btusb_data *data = hdev->driver_data;
+ struct btusb_data *data = hci_get_drvdata(hdev);

BT_DBG("%s evt %d", hdev->name, evt);

@@ -797,7 +797,7 @@ static void btusb_notify(struct hci_dev *hdev, unsigned int evt)

static inline int __set_isoc_interface(struct hci_dev *hdev, int altsetting)
{
- struct btusb_data *data = hdev->driver_data;
+ struct btusb_data *data = hci_get_drvdata(hdev);
struct usb_interface *intf = data->isoc;
struct usb_endpoint_descriptor *ep_desc;
int i, err;
@@ -985,7 +985,7 @@ static int btusb_probe(struct usb_interface *intf,
}

hdev->bus = HCI_USB;
- hdev->driver_data = data;
+ hci_set_drvdata(hdev, data);

data->hdev = hdev;

diff --git a/drivers/bluetooth/btwilink.c b/drivers/bluetooth/btwilink.c
index b81b32e..8869469 100644
--- a/drivers/bluetooth/btwilink.c
+++ b/drivers/bluetooth/btwilink.c
@@ -161,7 +161,7 @@ static int ti_st_open(struct hci_dev *hdev)
return -EBUSY;

/* provide contexts for callbacks from ST */
- hst = hdev->driver_data;
+ hst = hci_get_drvdata(hdev);

for (i = 0; i < MAX_BT_CHNL_IDS; i++) {
ti_st_proto[i].priv_data = hst;
@@ -236,7 +236,7 @@ done:
static int ti_st_close(struct hci_dev *hdev)
{
int err, i;
- struct ti_st *hst = hdev->driver_data;
+ struct ti_st *hst = hci_get_drvdata(hdev);

if (!test_and_clear_bit(HCI_RUNNING, &hdev->flags))
return 0;
@@ -264,7 +264,7 @@ static int ti_st_send_frame(struct sk_buff *skb)
if (!test_bit(HCI_RUNNING, &hdev->flags))
return -EBUSY;

- hst = hdev->driver_data;
+ hst = hci_get_drvdata(hdev);

/* Prepend skb with frame type */
memcpy(skb_push(skb, 1), &bt_cb(skb)->pkt_type, 1);
@@ -312,7 +312,7 @@ static int bt_ti_probe(struct platform_device *pdev)

hst->hdev = hdev;
hdev->bus = HCI_UART;
- hdev->driver_data = hst;
+ hci_set_drvdata(hdev, hst);
hdev->open = ti_st_open;
hdev->close = ti_st_close;
hdev->flush = NULL;
diff --git a/drivers/bluetooth/dtl1_cs.c b/drivers/bluetooth/dtl1_cs.c
index 295cf1b..049c059 100644
--- a/drivers/bluetooth/dtl1_cs.c
+++ b/drivers/bluetooth/dtl1_cs.c
@@ -364,7 +364,7 @@ static int dtl1_hci_open(struct hci_dev *hdev)

static int dtl1_hci_flush(struct hci_dev *hdev)
{
- dtl1_info_t *info = (dtl1_info_t *)(hdev->driver_data);
+ dtl1_info_t *info = hci_get_drvdata(hdev);

/* Drop TX queue */
skb_queue_purge(&(info->txq));
@@ -396,7 +396,7 @@ static int dtl1_hci_send_frame(struct sk_buff *skb)
return -ENODEV;
}

- info = (dtl1_info_t *)(hdev->driver_data);
+ info = hci_get_drvdata(hdev);

switch (bt_cb(skb)->pkt_type) {
case HCI_COMMAND_PKT:
@@ -475,7 +475,7 @@ static int dtl1_open(dtl1_info_t *info)
info->hdev = hdev;

hdev->bus = HCI_PCCARD;
- hdev->driver_data = info;
+ hci_set_drvdata(hdev, info);
SET_HCIDEV_DEV(hdev, &info->p_dev->dev);

hdev->open = dtl1_hci_open;
diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
index 459ff0b..01c23df 100644
--- a/drivers/bluetooth/hci_ldisc.c
+++ b/drivers/bluetooth/hci_ldisc.c
@@ -174,7 +174,7 @@ static int hci_uart_open(struct hci_dev *hdev)
/* Reset device */
static int hci_uart_flush(struct hci_dev *hdev)
{
- struct hci_uart *hu = (struct hci_uart *) hdev->driver_data;
+ struct hci_uart *hu = hci_get_drvdata(hdev);
struct tty_struct *tty = hu->tty;

BT_DBG("hdev %p tty %p", hdev, tty);
@@ -220,7 +220,7 @@ static int hci_uart_send_frame(struct sk_buff *skb)
if (!test_bit(HCI_RUNNING, &hdev->flags))
return -EBUSY;

- hu = (struct hci_uart *) hdev->driver_data;
+ hu = hci_get_drvdata(hdev);

BT_DBG("%s: type %d len %d", hdev->name, bt_cb(skb)->pkt_type, skb->len);

@@ -384,7 +384,7 @@ static int hci_uart_register_dev(struct hci_uart *hu)
hu->hdev = hdev;

hdev->bus = HCI_UART;
- hdev->driver_data = hu;
+ hci_set_drvdata(hdev, hu);

hdev->open = hci_uart_open;
hdev->close = hci_uart_close;
diff --git a/drivers/bluetooth/hci_vhci.c b/drivers/bluetooth/hci_vhci.c
index 5f305c1..158bfe5 100644
--- a/drivers/bluetooth/hci_vhci.c
+++ b/drivers/bluetooth/hci_vhci.c
@@ -61,7 +61,7 @@ static int vhci_open_dev(struct hci_dev *hdev)

static int vhci_close_dev(struct hci_dev *hdev)
{
- struct vhci_data *data = hdev->driver_data;
+ struct vhci_data *data = hci_get_drvdata(hdev);

if (!test_and_clear_bit(HCI_RUNNING, &hdev->flags))
return 0;
@@ -73,7 +73,7 @@ static int vhci_close_dev(struct hci_dev *hdev)

static int vhci_flush(struct hci_dev *hdev)
{
- struct vhci_data *data = hdev->driver_data;
+ struct vhci_data *data = hci_get_drvdata(hdev);

skb_queue_purge(&data->readq);

@@ -93,7 +93,7 @@ static int vhci_send_frame(struct sk_buff *skb)
if (!test_bit(HCI_RUNNING, &hdev->flags))
return -EBUSY;

- data = hdev->driver_data;
+ data = hci_get_drvdata(hdev);

memcpy(skb_push(skb, 1), &bt_cb(skb)->pkt_type, 1);
skb_queue_tail(&data->readq, skb);
@@ -234,7 +234,7 @@ static int vhci_open(struct inode *inode, struct file *file)
data->hdev = hdev;

hdev->bus = HCI_VIRTUAL;
- hdev->driver_data = data;
+ hci_set_drvdata(hdev, data);

if (amp)
hdev->dev_type = HCI_AMP;
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 01fdd2c..9780f42 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -248,7 +248,6 @@ struct hci_dev {

struct sk_buff_head driver_init;

- void *driver_data;
void *core_data;

atomic_t promisc;
@@ -614,6 +613,16 @@ static inline struct hci_dev *hci_dev_hold(struct hci_dev *d)

#define to_hci_dev(d) container_of(d, struct hci_dev, dev)

+static inline void *hci_get_drvdata(struct hci_dev *hdev)
+{
+ return dev_get_drvdata(&hdev->dev);
+}
+
+static inline void hci_set_drvdata(struct hci_dev *hdev, void *data)
+{
+ dev_set_drvdata(&hdev->dev, data);
+}
+
struct hci_dev *hci_dev_get(int index);
struct hci_dev *hci_get_route(bdaddr_t *src, bdaddr_t *dst);

--
1.7.8.4

2012-01-27 15:47:25

by David Herrmann

[permalink] [raw]
Subject: [PATCH 3/4] Bluetooth: Introduce to_hci_conn

This avoids using the dev_set/get_drvdata() functions to retrieve a
pointer to our own structure. We can use simple pointer arithmetic here.
The drvdata field is actually not needed by any other code-path but this
makes the code more consistent with hci_dev.

Signed-off-by: David Herrmann <[email protected]>
---
include/net/bluetooth/hci_core.h | 1 +
net/bluetooth/hci_sysfs.c | 10 ++++------
2 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 9780f42..8784da1 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -612,6 +612,7 @@ static inline struct hci_dev *hci_dev_hold(struct hci_dev *d)
#define hci_dev_unlock(d) mutex_unlock(&d->lock)

#define to_hci_dev(d) container_of(d, struct hci_dev, dev)
+#define to_hci_conn(c) container_of(c, struct hci_conn, dev)

static inline void *hci_get_drvdata(struct hci_dev *hdev)
{
diff --git a/net/bluetooth/hci_sysfs.c b/net/bluetooth/hci_sysfs.c
index 2a0243a..17e6cd4 100644
--- a/net/bluetooth/hci_sysfs.c
+++ b/net/bluetooth/hci_sysfs.c
@@ -33,19 +33,19 @@ static inline char *link_typetostr(int type)

static ssize_t show_link_type(struct device *dev, struct device_attribute *attr, char *buf)
{
- struct hci_conn *conn = dev_get_drvdata(dev);
+ struct hci_conn *conn = to_hci_conn(dev);
return sprintf(buf, "%s\n", link_typetostr(conn->type));
}

static ssize_t show_link_address(struct device *dev, struct device_attribute *attr, char *buf)
{
- struct hci_conn *conn = dev_get_drvdata(dev);
+ struct hci_conn *conn = to_hci_conn(dev);
return sprintf(buf, "%s\n", batostr(&conn->dst));
}

static ssize_t show_link_features(struct device *dev, struct device_attribute *attr, char *buf)
{
- struct hci_conn *conn = dev_get_drvdata(dev);
+ struct hci_conn *conn = to_hci_conn(dev);

return sprintf(buf, "0x%02x%02x%02x%02x%02x%02x%02x%02x\n",
conn->features[0], conn->features[1],
@@ -79,7 +79,7 @@ static const struct attribute_group *bt_link_groups[] = {

static void bt_link_release(struct device *dev)
{
- void *data = dev_get_drvdata(dev);
+ void *data = to_hci_conn(dev);
kfree(data);
}

@@ -120,8 +120,6 @@ void hci_conn_add_sysfs(struct hci_conn *conn)

dev_set_name(&conn->dev, "%s:%d", hdev->name, conn->handle);

- dev_set_drvdata(&conn->dev, conn);
-
if (device_add(&conn->dev) < 0) {
BT_ERR("Failed to register connection device");
return;
--
1.7.8.4

2012-01-27 15:47:23

by David Herrmann

[permalink] [raw]
Subject: [PATCH 1/4] Bluetooth: Introduce to_hci_dev()

We currently use dev_set_drvdata to keep a pointer to ourself. This
doesn't make sense as we are the bus and not a driver. Therefore,
introduce to_hci_dev() so we can get a struct hci_dev pointer from a
struct device pointer.

dev_set/get_drvdata() is reserved for drivers that provide a device and
not for the bus using the device. The bus can use simple pointer
arithmetic to retrieve its private data.

Signed-off-by: David Herrmann <[email protected]>
---
include/net/bluetooth/hci_core.h | 2 ++
net/bluetooth/hci_sysfs.c | 33 ++++++++++++++++-----------------
2 files changed, 18 insertions(+), 17 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 25f449f..01fdd2c 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -612,6 +612,8 @@ static inline struct hci_dev *hci_dev_hold(struct hci_dev *d)
#define hci_dev_lock(d) mutex_lock(&d->lock)
#define hci_dev_unlock(d) mutex_unlock(&d->lock)

+#define to_hci_dev(d) container_of(d, struct hci_dev, dev)
+
struct hci_dev *hci_dev_get(int index);
struct hci_dev *hci_get_route(bdaddr_t *src, bdaddr_t *dst);

diff --git a/net/bluetooth/hci_sysfs.c b/net/bluetooth/hci_sysfs.c
index ec03ee2..2a0243a 100644
--- a/net/bluetooth/hci_sysfs.c
+++ b/net/bluetooth/hci_sysfs.c
@@ -189,19 +189,19 @@ static inline char *host_typetostr(int type)

static ssize_t show_bus(struct device *dev, struct device_attribute *attr, char *buf)
{
- struct hci_dev *hdev = dev_get_drvdata(dev);
+ struct hci_dev *hdev = to_hci_dev(dev);
return sprintf(buf, "%s\n", host_bustostr(hdev->bus));
}

static ssize_t show_type(struct device *dev, struct device_attribute *attr, char *buf)
{
- struct hci_dev *hdev = dev_get_drvdata(dev);
+ struct hci_dev *hdev = to_hci_dev(dev);
return sprintf(buf, "%s\n", host_typetostr(hdev->dev_type));
}

static ssize_t show_name(struct device *dev, struct device_attribute *attr, char *buf)
{
- struct hci_dev *hdev = dev_get_drvdata(dev);
+ struct hci_dev *hdev = to_hci_dev(dev);
char name[HCI_MAX_NAME_LENGTH + 1];
int i;

@@ -214,20 +214,20 @@ static ssize_t show_name(struct device *dev, struct device_attribute *attr, char

static ssize_t show_class(struct device *dev, struct device_attribute *attr, char *buf)
{
- struct hci_dev *hdev = dev_get_drvdata(dev);
+ struct hci_dev *hdev = to_hci_dev(dev);
return sprintf(buf, "0x%.2x%.2x%.2x\n",
hdev->dev_class[2], hdev->dev_class[1], hdev->dev_class[0]);
}

static ssize_t show_address(struct device *dev, struct device_attribute *attr, char *buf)
{
- struct hci_dev *hdev = dev_get_drvdata(dev);
+ struct hci_dev *hdev = to_hci_dev(dev);
return sprintf(buf, "%s\n", batostr(&hdev->bdaddr));
}

static ssize_t show_features(struct device *dev, struct device_attribute *attr, char *buf)
{
- struct hci_dev *hdev = dev_get_drvdata(dev);
+ struct hci_dev *hdev = to_hci_dev(dev);

return sprintf(buf, "0x%02x%02x%02x%02x%02x%02x%02x%02x\n",
hdev->features[0], hdev->features[1],
@@ -238,31 +238,31 @@ static ssize_t show_features(struct device *dev, struct device_attribute *attr,

static ssize_t show_manufacturer(struct device *dev, struct device_attribute *attr, char *buf)
{
- struct hci_dev *hdev = dev_get_drvdata(dev);
+ struct hci_dev *hdev = to_hci_dev(dev);
return sprintf(buf, "%d\n", hdev->manufacturer);
}

static ssize_t show_hci_version(struct device *dev, struct device_attribute *attr, char *buf)
{
- struct hci_dev *hdev = dev_get_drvdata(dev);
+ struct hci_dev *hdev = to_hci_dev(dev);
return sprintf(buf, "%d\n", hdev->hci_ver);
}

static ssize_t show_hci_revision(struct device *dev, struct device_attribute *attr, char *buf)
{
- struct hci_dev *hdev = dev_get_drvdata(dev);
+ struct hci_dev *hdev = to_hci_dev(dev);
return sprintf(buf, "%d\n", hdev->hci_rev);
}

static ssize_t show_idle_timeout(struct device *dev, struct device_attribute *attr, char *buf)
{
- struct hci_dev *hdev = dev_get_drvdata(dev);
+ struct hci_dev *hdev = to_hci_dev(dev);
return sprintf(buf, "%d\n", hdev->idle_timeout);
}

static ssize_t store_idle_timeout(struct device *dev, struct device_attribute *attr, const char *buf, size_t count)
{
- struct hci_dev *hdev = dev_get_drvdata(dev);
+ struct hci_dev *hdev = to_hci_dev(dev);
unsigned int val;
int rv;

@@ -280,13 +280,13 @@ static ssize_t store_idle_timeout(struct device *dev, struct device_attribute *a

static ssize_t show_sniff_max_interval(struct device *dev, struct device_attribute *attr, char *buf)
{
- struct hci_dev *hdev = dev_get_drvdata(dev);
+ struct hci_dev *hdev = to_hci_dev(dev);
return sprintf(buf, "%d\n", hdev->sniff_max_interval);
}

static ssize_t store_sniff_max_interval(struct device *dev, struct device_attribute *attr, const char *buf, size_t count)
{
- struct hci_dev *hdev = dev_get_drvdata(dev);
+ struct hci_dev *hdev = to_hci_dev(dev);
u16 val;
int rv;

@@ -304,13 +304,13 @@ static ssize_t store_sniff_max_interval(struct device *dev, struct device_attrib

static ssize_t show_sniff_min_interval(struct device *dev, struct device_attribute *attr, char *buf)
{
- struct hci_dev *hdev = dev_get_drvdata(dev);
+ struct hci_dev *hdev = to_hci_dev(dev);
return sprintf(buf, "%d\n", hdev->sniff_min_interval);
}

static ssize_t store_sniff_min_interval(struct device *dev, struct device_attribute *attr, const char *buf, size_t count)
{
- struct hci_dev *hdev = dev_get_drvdata(dev);
+ struct hci_dev *hdev = to_hci_dev(dev);
u16 val;
int rv;

@@ -370,7 +370,7 @@ static const struct attribute_group *bt_host_groups[] = {

static void bt_host_release(struct device *dev)
{
- void *data = dev_get_drvdata(dev);
+ void *data = to_hci_dev(dev);
kfree(data);
module_put(THIS_MODULE);
}
@@ -525,7 +525,6 @@ void hci_init_sysfs(struct hci_dev *hdev)
dev->class = bt_class;

__module_get(THIS_MODULE);
- dev_set_drvdata(dev, hdev);
device_initialize(dev);
}

--
1.7.8.4

2012-02-09 21:12:11

by David Herrmann

[permalink] [raw]
Subject: Re: [PATCH 0/4] Integrate better device support

Hi Andrei

On Mon, Feb 6, 2012 at 1:34 PM, Andrei Emeltchenko
<[email protected]> wrote:
> Hi David,
>
> On Fri, Jan 27, 2012 at 04:47:22PM +0100, David Herrmann wrote:
>> Hi
>>
>> "struct device" provides a drvdata-field that we should use properly to =
save
>> _driver-data_. This series makes the hci-core use pointer-arithmetic to =
avoid
>> using this field in the bus-core and instead converts the drivers to use=
the
>> drvdata field.
>> This also reduces the hci_dev structure by 4/8 bytes, yeah.
>
> I do not know does it related to those changes but recently I got several
> dumps like shown below:
>
> [ =A0276.028121] Bluetooth: Virtual HCI driver ver 1.3
> [ =A0277.028692] Bluetooth: BNEP (Ethernet Emulation) ver 1.3
> [ =A0277.054874] Bluetooth: BNEP filters: protocol multicast
>
> < here comes module unloading>
>
> [ =A0302.632063] usbcore: deregistering interface driver btusb
> [ =A0302.664760] BUG: unable to handle kernel paging request at c16bef8f
> [ =A0302.668371] IP: [<c12a9011>] kobject_get+0x11/0x30
> [ =A0302.668371] *pde =3D 36785063 *pte =3D 016be161
> [ =A0302.668371] Oops: 0003 [#1] SMP
> [ =A0302.668371] Modules linked in: hci_vhci(O) btusb(-) bluetooth(O)
> snd_intel8x0 joydev snd_ac97_codec ac97_bus snd_pcm ppdev snd_seq
> snd_timer snd_seq_device parport_pc snd binfmt_misc psmouse serio_raw
> soundcore snd_page_alloc i2c_piix4 lp parport usbhid hid ahci libahci
> e1000 [last unloaded: bnep]
> [ =A0302.668371]
> [ =A0302.668371] Pid: 4310, comm: rmmod Tainted: G =A0 =A0 =A0 =A0 =A0 O =
3.2.0niko+
> #74 innotek GmbH VirtualBox
> [ =A0302.668371] EIP: 0060:[<c12a9011>] EFLAGS: 00010206 CPU: 0
> [ =A0302.668371] EIP is at kobject_get+0x11/0x30
> [ =A0302.668371] EAX: 6f6c2e71 EBX: c16bef73 ECX: 00000006 EDX: 00000000
> [ =A0302.668371] ESI: c16be46b EDI: f5655c00 EBP: e9415e5c ESP: e9415e58
> [ =A0302.668371] =A0DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068
> [ =A0302.668371] Process rmmod (pid: 4310, ti=3De9414000 task=3De982bea0
> task.ti=3De9414000)
> [ =A0302.668371] Stack:
> [ =A0302.668371] =A0f45c6400 e9415e64 c1366a16 e9415e90 f81589d8 00000002
> f67e1d88 f67a6000
> [ =A0302.668371] =A0e9415e90 c16bef6b 00000000 f5655c1c f5655c00 f815c1d8
> e9415eac c13e7bef
> [ =A0302.668371] =A000000000 f67a6000 f5655c1c f815c1d8 f5655c50 e9415ebc
> c136aaaa f5655c1c
> [ =A0302.668371] Call Trace:
> [ =A0302.668371] =A0[<c1366a16>] get_device+0x16/0x20
> [ =A0302.668371] =A0[<f81589d8>] btusb_disconnect+0x48/0xe0 [btusb]
> [ =A0302.668371] =A0[<c13e7bef>] usb_unbind_interface+0x3f/0x150
> [ =A0302.668371] =A0[<c136aaaa>] __device_release_driver+0x6a/0xc0
> [ =A0302.668371] =A0[<c136b2e7>] driver_detach+0x97/0xa0

Do you know which "get_device" call that is? I can assume its
triggered by hci_dev_hold() in btusb_disconnect which only calls
get_device, but if this call fails, the following call to
hci_unregister_dev() must fail, too. I still cannot trigger this so
can you provide some more information here?
If "hdev" really is invalid here, we have a serious problem but if
this get_device() is not at all related to the bt-core but rather in
the usb-core I would have to search at a totally different place. If
you can reliably trigger this BUG could you try to compile with
frame-pointers/no-inline or sth. like that so we can get a better
stack-trace?
Anyway, its definitely not related to this series as they have not
been applied yet but another patch I sent to the list "btusb: Remove
device lock on release" touches this so could you try that one and see
what hci_unregister_dev() does?

> [ =A0302.668371] =A0[<c136a944>] bus_remove_driver+0x74/0xe0
> [ =A0302.668371] =A0[<c136b988>] driver_unregister+0x48/0x80
> [ =A0302.668371] =A0[<c13e701c>] usb_deregister+0xac/0xc0
> [ =A0302.668371] =A0[<f815a0ed>] btusb_driver_exit+0xd/0xf20 [btusb]
> [ =A0302.668371] =A0[<c108f455>] sys_delete_module+0x135/0x250
> [ =A0302.668371] =A0[<c113f0bd>] ? vfs_write+0xed/0x160
> [ =A0302.668371] =A0[<c113e4a0>] ? wait_on_retry_sync_kiocb+0x50/0x50
> [ =A0302.668371] =A0[<c15668ad>] ? restore_all+0xf/0xf
> [ =A0302.668371] =A0[<c156d71f>] sysenter_do_call+0x12/0x38
> [ =A0302.668371] Code: 24 08 c7 04 24 28 7a 7e c1 e8 1c 5c 01 00 8b 4c 24=
18
> e9 d0 fe ff ff 8d 76 00 55 85 c0 89 e5 53 89 c3 74 0b 8b 40 1c 85 c0 74 0=
9
> <3e> ff 43 1c 89 d8 5b 5d c3 ba 28 00 00 00 b8 9d 7b 6b c1 e8 17
> [ =A0302.668371] EIP: [<c12a9011>] kobject_get+0x11/0x30 SS:ESP
> 0068:e9415e58
> [ =A0302.668371] CR2: 00000000c16bef8f
> [ =A0302.668371] ---[ end trace de038ac80f57694f ]---
>
> I have to say that usually I recompile only bluetooth modules and reload
> them, IMO this should not trigger it.
>
> Best regards
> Andrei Emeltchenko

Cheers
David

2012-02-06 14:03:20

by David Herrmann

[permalink] [raw]
Subject: Re: [PATCH 0/4] Integrate better device support

Hi Andrei

On Mon, Feb 6, 2012 at 1:34 PM, Andrei Emeltchenko
<[email protected]> wrote:
> Hi David,
>
> On Fri, Jan 27, 2012 at 04:47:22PM +0100, David Herrmann wrote:
>> Hi
>>
>> "struct device" provides a drvdata-field that we should use properly to =
save
>> _driver-data_. This series makes the hci-core use pointer-arithmetic to =
avoid
>> using this field in the bus-core and instead converts the drivers to use=
the
>> drvdata field.
>> This also reduces the hci_dev structure by 4/8 bytes, yeah.
>
> I do not know does it related to those changes but recently I got several
> dumps like shown below:

As far as I know Johan hasn't applied them yet so it is probably not
related to this patchset. I'd rather guess that it is related to my
previous changes. I will look at this later but I wasn't able to
trigger it, yet.

> [ =A0276.028121] Bluetooth: Virtual HCI driver ver 1.3
> [ =A0277.028692] Bluetooth: BNEP (Ethernet Emulation) ver 1.3
> [ =A0277.054874] Bluetooth: BNEP filters: protocol multicast
>
> < here comes module unloading>
>
> [ =A0302.632063] usbcore: deregistering interface driver btusb
> [ =A0302.664760] BUG: unable to handle kernel paging request at c16bef8f
> [ =A0302.668371] IP: [<c12a9011>] kobject_get+0x11/0x30
> [ =A0302.668371] *pde =3D 36785063 *pte =3D 016be161
> [ =A0302.668371] Oops: 0003 [#1] SMP
> [ =A0302.668371] Modules linked in: hci_vhci(O) btusb(-) bluetooth(O)
> snd_intel8x0 joydev snd_ac97_codec ac97_bus snd_pcm ppdev snd_seq
> snd_timer snd_seq_device parport_pc snd binfmt_misc psmouse serio_raw
> soundcore snd_page_alloc i2c_piix4 lp parport usbhid hid ahci libahci
> e1000 [last unloaded: bnep]
> [ =A0302.668371]
> [ =A0302.668371] Pid: 4310, comm: rmmod Tainted: G =A0 =A0 =A0 =A0 =A0 O =
3.2.0niko+
> #74 innotek GmbH VirtualBox
> [ =A0302.668371] EIP: 0060:[<c12a9011>] EFLAGS: 00010206 CPU: 0
> [ =A0302.668371] EIP is at kobject_get+0x11/0x30
> [ =A0302.668371] EAX: 6f6c2e71 EBX: c16bef73 ECX: 00000006 EDX: 00000000
> [ =A0302.668371] ESI: c16be46b EDI: f5655c00 EBP: e9415e5c ESP: e9415e58
> [ =A0302.668371] =A0DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068
> [ =A0302.668371] Process rmmod (pid: 4310, ti=3De9414000 task=3De982bea0
> task.ti=3De9414000)
> [ =A0302.668371] Stack:
> [ =A0302.668371] =A0f45c6400 e9415e64 c1366a16 e9415e90 f81589d8 00000002
> f67e1d88 f67a6000
> [ =A0302.668371] =A0e9415e90 c16bef6b 00000000 f5655c1c f5655c00 f815c1d8
> e9415eac c13e7bef
> [ =A0302.668371] =A000000000 f67a6000 f5655c1c f815c1d8 f5655c50 e9415ebc
> c136aaaa f5655c1c
> [ =A0302.668371] Call Trace:
> [ =A0302.668371] =A0[<c1366a16>] get_device+0x16/0x20
> [ =A0302.668371] =A0[<f81589d8>] btusb_disconnect+0x48/0xe0 [btusb]
> [ =A0302.668371] =A0[<c13e7bef>] usb_unbind_interface+0x3f/0x150
> [ =A0302.668371] =A0[<c136aaaa>] __device_release_driver+0x6a/0xc0
> [ =A0302.668371] =A0[<c136b2e7>] driver_detach+0x97/0xa0
> [ =A0302.668371] =A0[<c136a944>] bus_remove_driver+0x74/0xe0
> [ =A0302.668371] =A0[<c136b988>] driver_unregister+0x48/0x80
> [ =A0302.668371] =A0[<c13e701c>] usb_deregister+0xac/0xc0
> [ =A0302.668371] =A0[<f815a0ed>] btusb_driver_exit+0xd/0xf20 [btusb]
> [ =A0302.668371] =A0[<c108f455>] sys_delete_module+0x135/0x250
> [ =A0302.668371] =A0[<c113f0bd>] ? vfs_write+0xed/0x160
> [ =A0302.668371] =A0[<c113e4a0>] ? wait_on_retry_sync_kiocb+0x50/0x50
> [ =A0302.668371] =A0[<c15668ad>] ? restore_all+0xf/0xf
> [ =A0302.668371] =A0[<c156d71f>] sysenter_do_call+0x12/0x38
> [ =A0302.668371] Code: 24 08 c7 04 24 28 7a 7e c1 e8 1c 5c 01 00 8b 4c 24=
18
> e9 d0 fe ff ff 8d 76 00 55 85 c0 89 e5 53 89 c3 74 0b 8b 40 1c 85 c0 74 0=
9
> <3e> ff 43 1c 89 d8 5b 5d c3 ba 28 00 00 00 b8 9d 7b 6b c1 e8 17
> [ =A0302.668371] EIP: [<c12a9011>] kobject_get+0x11/0x30 SS:ESP
> 0068:e9415e58
> [ =A0302.668371] CR2: 00000000c16bef8f
> [ =A0302.668371] ---[ end trace de038ac80f57694f ]---
>
> I have to say that usually I recompile only bluetooth modules and reload
> them, IMO this should not trigger it.
>
> Best regards
> Andrei Emeltchenko

Regards
David

2012-02-06 12:34:24

by Andrei Emeltchenko

[permalink] [raw]
Subject: Re: [PATCH 0/4] Integrate better device support

Hi David,

On Fri, Jan 27, 2012 at 04:47:22PM +0100, David Herrmann wrote:
> Hi
>
> "struct device" provides a drvdata-field that we should use properly to save
> _driver-data_. This series makes the hci-core use pointer-arithmetic to avoid
> using this field in the bus-core and instead converts the drivers to use the
> drvdata field.
> This also reduces the hci_dev structure by 4/8 bytes, yeah.

I do not know does it related to those changes but recently I got several
dumps like shown below:

[ 276.028121] Bluetooth: Virtual HCI driver ver 1.3
[ 277.028692] Bluetooth: BNEP (Ethernet Emulation) ver 1.3
[ 277.054874] Bluetooth: BNEP filters: protocol multicast

< here comes module unloading>

[ 302.632063] usbcore: deregistering interface driver btusb
[ 302.664760] BUG: unable to handle kernel paging request at c16bef8f
[ 302.668371] IP: [<c12a9011>] kobject_get+0x11/0x30
[ 302.668371] *pde = 36785063 *pte = 016be161
[ 302.668371] Oops: 0003 [#1] SMP
[ 302.668371] Modules linked in: hci_vhci(O) btusb(-) bluetooth(O)
snd_intel8x0 joydev snd_ac97_codec ac97_bus snd_pcm ppdev snd_seq
snd_timer snd_seq_device parport_pc snd binfmt_misc psmouse serio_raw
soundcore snd_page_alloc i2c_piix4 lp parport usbhid hid ahci libahci
e1000 [last unloaded: bnep]
[ 302.668371]
[ 302.668371] Pid: 4310, comm: rmmod Tainted: G O 3.2.0niko+
#74 innotek GmbH VirtualBox
[ 302.668371] EIP: 0060:[<c12a9011>] EFLAGS: 00010206 CPU: 0
[ 302.668371] EIP is at kobject_get+0x11/0x30
[ 302.668371] EAX: 6f6c2e71 EBX: c16bef73 ECX: 00000006 EDX: 00000000
[ 302.668371] ESI: c16be46b EDI: f5655c00 EBP: e9415e5c ESP: e9415e58
[ 302.668371] DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068
[ 302.668371] Process rmmod (pid: 4310, ti=e9414000 task=e982bea0
task.ti=e9414000)
[ 302.668371] Stack:
[ 302.668371] f45c6400 e9415e64 c1366a16 e9415e90 f81589d8 00000002
f67e1d88 f67a6000
[ 302.668371] e9415e90 c16bef6b 00000000 f5655c1c f5655c00 f815c1d8
e9415eac c13e7bef
[ 302.668371] 00000000 f67a6000 f5655c1c f815c1d8 f5655c50 e9415ebc
c136aaaa f5655c1c
[ 302.668371] Call Trace:
[ 302.668371] [<c1366a16>] get_device+0x16/0x20
[ 302.668371] [<f81589d8>] btusb_disconnect+0x48/0xe0 [btusb]
[ 302.668371] [<c13e7bef>] usb_unbind_interface+0x3f/0x150
[ 302.668371] [<c136aaaa>] __device_release_driver+0x6a/0xc0
[ 302.668371] [<c136b2e7>] driver_detach+0x97/0xa0
[ 302.668371] [<c136a944>] bus_remove_driver+0x74/0xe0
[ 302.668371] [<c136b988>] driver_unregister+0x48/0x80
[ 302.668371] [<c13e701c>] usb_deregister+0xac/0xc0
[ 302.668371] [<f815a0ed>] btusb_driver_exit+0xd/0xf20 [btusb]
[ 302.668371] [<c108f455>] sys_delete_module+0x135/0x250
[ 302.668371] [<c113f0bd>] ? vfs_write+0xed/0x160
[ 302.668371] [<c113e4a0>] ? wait_on_retry_sync_kiocb+0x50/0x50
[ 302.668371] [<c15668ad>] ? restore_all+0xf/0xf
[ 302.668371] [<c156d71f>] sysenter_do_call+0x12/0x38
[ 302.668371] Code: 24 08 c7 04 24 28 7a 7e c1 e8 1c 5c 01 00 8b 4c 24 18
e9 d0 fe ff ff 8d 76 00 55 85 c0 89 e5 53 89 c3 74 0b 8b 40 1c 85 c0 74 09
<3e> ff 43 1c 89 d8 5b 5d c3 ba 28 00 00 00 b8 9d 7b 6b c1 e8 17
[ 302.668371] EIP: [<c12a9011>] kobject_get+0x11/0x30 SS:ESP
0068:e9415e58
[ 302.668371] CR2: 00000000c16bef8f
[ 302.668371] ---[ end trace de038ac80f57694f ]---

I have to say that usually I recompile only bluetooth modules and reload
them, IMO this should not trigger it.

Best regards
Andrei Emeltchenko