2012-11-23 20:30:29

by Vinicius Costa Gomes

[permalink] [raw]
Subject: [RFC BlueZ 0/3] Only create the uhid device when PNP info is available

Hi,

Some input drivers require that the input device is created with the
correct product and vendor ids.

So we have a race condition between the DIS (Device Information)
profile and the HoG profile, this series aims to solve this particular
problem by adding a mechanism to notify the interested parties that
PNP information is ready, the HoG plugin uses this to only create the
uhid device when that information arrives.

Patch 1/3 should be considered for inclusion even if this idea doesn't
make sense.

Cheers,

Vinicius Costa Gomes (3):
device: Add btd_ prefix to device_set_pnpid()
device: Add a way to be notified that PNP information is present
hog: Fix registering HoG devices without vendor information

profiles/deviceinfo/deviceinfo.c | 2 +-
profiles/input/hog_device.c | 75 ++++++++++++++++++++++++++++------------
src/device.c | 45 ++++++++++++++++++++++--
src/device.h | 7 +++-
4 files changed, 103 insertions(+), 26 deletions(-)

--
1.8.0



2012-11-23 20:30:32

by Vinicius Costa Gomes

[permalink] [raw]
Subject: [RFC BlueZ 3/3] hog: Fix registering HoG devices without vendor information

We use the newly added support for notifying when the PNP ID
information is ready before registering the HoG device.
---
profiles/input/hog_device.c | 75 ++++++++++++++++++++++++++++++++-------------
1 file changed, 53 insertions(+), 22 deletions(-)

diff --git a/profiles/input/hog_device.c b/profiles/input/hog_device.c
index 0a5fb58..28e3b50 100644
--- a/profiles/input/hog_device.c
+++ b/profiles/input/hog_device.c
@@ -78,6 +78,8 @@ struct hog_device {
guint attioid;
struct gatt_primary *hog_primary;
GSList *reports;
+ uint8_t *reportmap;
+ ssize_t reportmap_size;
int uhid_fd;
gboolean prepend_id;
guint uhid_watch_id;
@@ -335,13 +337,54 @@ static void external_report_reference_cb(guint8 status, const guint8 *pdu,
external_service_char_cb, hogdev);
}

+static void create_uhid_device(struct hog_device *hogdev, uint8_t *reportmap,
+ uint16_t rsize)
+{
+ struct uhid_event ev;
+ uint16_t vendor_src, vendor, product, version;
+
+ vendor_src = btd_device_get_vendor_src(hogdev->device);
+ vendor = btd_device_get_vendor(hogdev->device);
+ product = btd_device_get_product(hogdev->device);
+ version = btd_device_get_version(hogdev->device);
+ DBG("DIS information: vendor_src=0x%X, vendor=0x%X, product=0x%X, "
+ "version=0x%X", vendor_src, vendor, product, version);
+
+ /* create uHID device */
+ memset(&ev, 0, sizeof(ev));
+ ev.type = UHID_CREATE;
+ strcpy((char *) ev.u.create.name, "bluez-hog-device");
+ ev.u.create.vendor = vendor;
+ ev.u.create.product = product;
+ ev.u.create.version = version;
+ ev.u.create.country = hogdev->bcountrycode;
+ ev.u.create.bus = BUS_BLUETOOTH;
+ ev.u.create.rd_data = reportmap;
+ ev.u.create.rd_size = rsize;
+
+ if (write(hogdev->uhid_fd, &ev, sizeof(ev)) < 0)
+ error("Failed to create uHID device: %s", strerror(errno));
+}
+
+static void pnpid_ready(gpointer user_data)
+{
+ struct hog_device *hogdev = user_data;
+
+ if (hogdev->reportmap == NULL)
+ return;
+
+ create_uhid_device(hogdev, hogdev->reportmap, hogdev->reportmap_size);
+
+ g_free(hogdev->reportmap);
+ hogdev->reportmap = NULL;
+ hogdev->reportmap_size = 0;
+}
+
static void report_map_read_cb(guint8 status, const guint8 *pdu, guint16 plen,
gpointer user_data)
{
struct hog_device *hogdev = user_data;
uint8_t value[HOG_REPORT_MAP_MAX_SIZE];
- struct uhid_event ev;
- uint16_t vendor_src, vendor, product, version;
ssize_t vlen;
int i;

@@ -373,27 +416,14 @@ static void report_map_read_cb(guint8 status, const guint8 *pdu, guint16 plen,
}
}

- vendor_src = btd_device_get_vendor_src(hogdev->device);
- vendor = btd_device_get_vendor(hogdev->device);
- product = btd_device_get_product(hogdev->device);
- version = btd_device_get_version(hogdev->device);
- DBG("DIS information: vendor_src=0x%X, vendor=0x%X, product=0x%X, "
- "version=0x%X", vendor_src, vendor, product, version);
-
- /* create uHID device */
- memset(&ev, 0, sizeof(ev));
- ev.type = UHID_CREATE;
- strcpy((char *) ev.u.create.name, "bluez-hog-device");
- ev.u.create.vendor = vendor;
- ev.u.create.product = product;
- ev.u.create.version = version;
- ev.u.create.country = hogdev->bcountrycode;
- ev.u.create.bus = BUS_BLUETOOTH;
- ev.u.create.rd_data = value;
- ev.u.create.rd_size = vlen;
+ if (btd_device_register_pnpid_notifier(hogdev->device,
+ pnpid_ready, hogdev)) {
+ hogdev->reportmap = g_memdup(value, vlen);
+ hogdev->reportmap_size = vlen;
+ return;
+ }

- if (write(hogdev->uhid_fd, &ev, sizeof(ev)) < 0)
- error("Failed to create uHID device: %s", strerror(errno));
+ create_uhid_device(hogdev, value, vlen);
}

static void info_read_cb(guint8 status, const guint8 *pdu, guint16 plen,
@@ -706,6 +736,7 @@ static void hog_device_free(struct hog_device *hogdev)
btd_device_unref(hogdev->device);
g_slist_free_full(hogdev->reports, report_free);
g_free(hogdev->hog_primary);
+ g_free(hogdev->reportmap);
g_free(hogdev);
}

--
1.8.0


2012-11-23 20:30:31

by Vinicius Costa Gomes

[permalink] [raw]
Subject: [RFC BlueZ 2/3] device: Add a way to be notified that PNP information is present

It will be used inside the HoG profile to only create the uhid device
after the PNP information is present, for the HID subsystem may use the
vendor and product ids to load the correct module.

btd_device_register_pnpid_notifier() will return 'false' when the
information is already present.
---
src/device.c | 41 +++++++++++++++++++++++++++++++++++++++++
src/device.h | 5 +++++
2 files changed, 46 insertions(+)

diff --git a/src/device.c b/src/device.c
index a196af4..ec890fa 100644
--- a/src/device.c
+++ b/src/device.c
@@ -132,6 +132,11 @@ struct attio_data {
gpointer user_data;
};

+struct pnpid_notifier {
+ pnpid_ready_func func;
+ void *user_data;
+};
+
typedef void (*attio_error_cb) (const GError *gerr, gpointer user_data);
typedef void (*attio_success_cb) (gpointer user_data);

@@ -195,6 +200,8 @@ struct btd_device {
GIOChannel *att_io;
guint cleanup_id;
guint store_id;
+
+ GSList *pnpid_notifiers;
};

static uint16_t uuid_list[] = {
@@ -347,6 +354,7 @@ static void device_free(gpointer user_data)
g_slist_free_full(device->primaries, g_free);
g_slist_free_full(device->attios, g_free);
g_slist_free_full(device->attios_offline, g_free);
+ g_slist_free_full(device->pnpid_notifiers, g_free);

attio_cleanup(device);

@@ -4105,10 +4113,43 @@ void btd_device_set_pnpid(struct btd_device *device, uint8_t vendor_id_src,
uint16_t vendor_id, uint16_t product_id,
uint16_t product_ver)
{
+ GSList *l;
+
device_set_vendor(device, vendor_id);
device_set_vendor_src(device, vendor_id_src);
device_set_product(device, product_id);
device_set_version(device, product_ver);

store_device_info(device);
+
+ for (l = device->pnpid_notifiers; l; l = l->next) {
+ struct pnpid_notifier *notifier = l->data;
+
+ notifier->func(notifier->user_data);
+ g_free(notifier);
+ }
+
+ g_slist_free_full(device->pnpid_notifiers, g_free);
+ device->pnpid_notifiers = NULL;
+}
+
+bool btd_device_register_pnpid_notifier(struct btd_device *device,
+ pnpid_ready_func notify, void *user_data)
+{
+ struct pnpid_notifier *notifier;
+
+ if (btd_device_get_vendor(device) ||
+ btd_device_get_vendor_src(device) ||
+ btd_device_get_product(device) ||
+ btd_device_get_version(device))
+ return false;
+
+ notifier = g_new0(struct pnpid_notifier, 1);
+
+ notifier->func = notify;
+ notifier->user_data = user_data;
+
+ device->pnpid_notifiers = g_slist_append(device->pnpid_notifiers,
+ notifier);
+ return true;
}
diff --git a/src/device.h b/src/device.h
index 703dfcf..914ee38 100644
--- a/src/device.h
+++ b/src/device.h
@@ -119,4 +119,9 @@ int device_unblock(struct btd_device *device, gboolean silent,
void btd_device_set_pnpid(struct btd_device *device, uint8_t vendor_id_src,
uint16_t vendor_id, uint16_t product_id,
uint16_t product_ver);
+
+typedef void (*pnpid_ready_func) (void *user_data);
+
+bool btd_device_register_pnpid_notifier(struct btd_device *device,
+ pnpid_ready_func notify, void *user_data);
GIOChannel *device_att_connect(gpointer user_data);
--
1.8.0


2012-11-23 20:30:30

by Vinicius Costa Gomes

[permalink] [raw]
Subject: [RFC BlueZ 1/3] device: Add btd_ prefix to device_set_pnpid()

As device_set_pnpid() is used inside a plugin it should have the btd_
prefix.
---
profiles/deviceinfo/deviceinfo.c | 2 +-
src/device.c | 4 ++--
src/device.h | 2 +-
3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/profiles/deviceinfo/deviceinfo.c b/profiles/deviceinfo/deviceinfo.c
index 9910533..da27df8 100644
--- a/profiles/deviceinfo/deviceinfo.c
+++ b/profiles/deviceinfo/deviceinfo.c
@@ -106,7 +106,7 @@ static void read_pnpid_cb(guint8 status, const guint8 *pdu, guint16 len,
return;
}

- device_set_pnpid(ch->d->dev, value[0], att_get_u16(&value[1]),
+ btd_device_set_pnpid(ch->d->dev, value[0], att_get_u16(&value[1]),
att_get_u16(&value[3]), att_get_u16(&value[5]));
}

diff --git a/src/device.c b/src/device.c
index f0223c8..a196af4 100644
--- a/src/device.c
+++ b/src/device.c
@@ -2349,7 +2349,7 @@ static void update_bredr_services(struct browse_req *req, sdp_list_t *recs)
version = pdlist ? pdlist->val.uint16 : 0x0000;

if (source || vendor || product || version)
- device_set_pnpid(device, source, vendor,
+ btd_device_set_pnpid(device, source, vendor,
product, version);
}

@@ -4101,7 +4101,7 @@ gboolean btd_device_remove_attio_callback(struct btd_device *device, guint id)
return TRUE;
}

-void device_set_pnpid(struct btd_device *device, uint8_t vendor_id_src,
+void btd_device_set_pnpid(struct btd_device *device, uint8_t vendor_id_src,
uint16_t vendor_id, uint16_t product_id,
uint16_t product_ver)
{
diff --git a/src/device.h b/src/device.h
index 3715698..703dfcf 100644
--- a/src/device.h
+++ b/src/device.h
@@ -116,7 +116,7 @@ void btd_device_unref(struct btd_device *device);
int device_block(struct btd_device *device, gboolean update_only);
int device_unblock(struct btd_device *device, gboolean silent,
gboolean update_only);
-void device_set_pnpid(struct btd_device *device, uint8_t vendor_id_src,
+void btd_device_set_pnpid(struct btd_device *device, uint8_t vendor_id_src,
uint16_t vendor_id, uint16_t product_id,
uint16_t product_ver);
GIOChannel *device_att_connect(gpointer user_data);
--
1.8.0