2013-12-30 21:59:39

by Gordon

[permalink] [raw]
Subject: PATCH hid2hci for CSR 8510 A10

Hi,

ich had a problem with the Sitecom CNT-524 as described by someone else
here:
http://blog.ruecker.fi/2013/10/06/adventures-in-bluetooth-4-0-part-i/

so i tried to fix that by introducing --mode csr2 for hid2hci

I neither know how to switch back to hid nor how to detect the EALREADY
state, otherwise the patch is very small.

I works on my machine in ubuntu 13.04 with bluez 4.101-0ubuntu8b1 and
said sitecom usb dongle. the patch is against bluez commit
fd00064e0bb2c81e53e9d0b7d22ce919b41dbe60

Could someone please review.

Cheers,

Gordon



Attachments:
bluez_hid2hci.patch (2.41 kB)

2014-03-31 12:17:06

by Anderson Lizardo

[permalink] [raw]
Subject: Re: [PATCH] hid2hci add CSR 8510 A10 support

Hi Gordon,

A few more coding style suggestions below.

On Sat, Mar 29, 2014 at 7:37 AM, Gordon <[email protected]> wrote:
> diff --git a/tools/hid2hci.c b/tools/hid2hci.c
> index 2dbfca7..1f64b41 100644
> --- a/tools/hid2hci.c
> +++ b/tools/hid2hci.c
> @@ -119,6 +119,49 @@ static int usb_switch_csr(int fd, enum mode mode)
> return err;
> }
>
> +static int usb_switch_csr2(int fd, enum mode mode)
> +{
> + int err;
> + struct usbfs_disconnect disconnect;
> + uint8_t report[] = {
> + 0x01, 0x05, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
> + };

Add an empty line here to separate variable declarations.

> + switch (mode) {
> + case HCI:
> + /* send report as is */
> + disconnect.interface = 0;
> + disconnect.flags = USBFS_DISCONNECT_EXCEPT_DRIVER;
> + strcpy(disconnect.driver, "usbfs");
> +
> + if (ioctl(fd, USBFS_IOCTL_DISCONNECT, &disconnect) < 0) {
> + fprintf(stderr, "Can't claim interface: %s (%d)\n",
> + strerror(errno), errno);
> + return -1;
> + }

Add an empty line here.

> + /* Set_report request with
> + * report id: 0x01, report type: feature (0x03)
> + * on interface 0
> + */
> + err = control_message(fd,
> + USB_ENDPOINT_OUT | USB_TYPE_CLASS |
> + USB_RECIP_INTERFACE,
> + USB_REQ_SET_CONFIGURATION,
> + 0x01 | (0x03 << 8),
> + 0, report, sizeof(report), 5000);
> + /* unable to detect whether the previous state
> + * already was HCI (EALREADY)
> + */
> + break;
> + case HID:
> + /* currently unknown how to switch to HID */
> + fprintf(stderr,
> + "csr2: Switching to hid mode is not implemented\n");
> + return -1;
> + break;

Instead of "return -1; break;" you can do:

err = -1;
break;

> + }

Add an empty line here to separate the "return err" from the rest of the code.

> + return err;
> +}
> +
> static int hid_logitech_send_report(int fd, const char *buf, size_t size)
> {
> struct hiddev_report_info rinfo;

Best Regards,
--
Anderson Lizardo
http://www.indt.org/?lang=en
INdT - Manaus - Brazil

2014-03-29 11:37:06

by Gordon

[permalink] [raw]
Subject: [PATCH] hid2hci add CSR 8510 A10 support

add a new method "csr2" to hid2hci that sends a control message
to the usb device which switches the Cambridge Silicon Radio 8510 A10
as used in Sitecom CNT-524

fixes: https://bugzilla.kernel.org/show_bug.cgi?id=69181
---
tools/hid2hci.1 | 2 +-
tools/hid2hci.c | 48 +++++++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 48 insertions(+), 2 deletions(-)

diff --git a/tools/hid2hci.1 b/tools/hid2hci.1
index 8c5d520..c6876a3 100644
--- a/tools/hid2hci.1
+++ b/tools/hid2hci.1
@@ -32,7 +32,7 @@ mode and back.
.B --mode= [hid, hci]
Sets the mode to switch the device into
.TP
-.B --method= [csr, logitech-hid, dell]
+.B --method= [csr, csr2, logitech-hid, dell]
Which vendor method to use for switching the device.
.TP
.B --devpath=
diff --git a/tools/hid2hci.c b/tools/hid2hci.c
index 2dbfca7..1f64b41 100644
--- a/tools/hid2hci.c
+++ b/tools/hid2hci.c
@@ -119,6 +119,49 @@ static int usb_switch_csr(int fd, enum mode mode)
return err;
}

+static int usb_switch_csr2(int fd, enum mode mode)
+{
+ int err;
+ struct usbfs_disconnect disconnect;
+ uint8_t report[] = {
+ 0x01, 0x05, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
+ };
+ switch (mode) {
+ case HCI:
+ /* send report as is */
+ disconnect.interface = 0;
+ disconnect.flags = USBFS_DISCONNECT_EXCEPT_DRIVER;
+ strcpy(disconnect.driver, "usbfs");
+
+ if (ioctl(fd, USBFS_IOCTL_DISCONNECT, &disconnect) < 0) {
+ fprintf(stderr, "Can't claim interface: %s (%d)\n",
+ strerror(errno), errno);
+ return -1;
+ }
+ /* Set_report request with
+ * report id: 0x01, report type: feature (0x03)
+ * on interface 0
+ */
+ err = control_message(fd,
+ USB_ENDPOINT_OUT | USB_TYPE_CLASS |
+ USB_RECIP_INTERFACE,
+ USB_REQ_SET_CONFIGURATION,
+ 0x01 | (0x03 << 8),
+ 0, report, sizeof(report), 5000);
+ /* unable to detect whether the previous state
+ * already was HCI (EALREADY)
+ */
+ break;
+ case HID:
+ /* currently unknown how to switch to HID */
+ fprintf(stderr,
+ "csr2: Switching to hid mode is not implemented\n");
+ return -1;
+ break;
+ }
+ return err;
+}
+
static int hid_logitech_send_report(int fd, const char *buf, size_t size)
{
struct hiddev_report_info rinfo;
@@ -258,7 +301,7 @@ static void usage(const char *error)
printf("Usage: hid2hci [options]\n"
" --mode= mode to switch to [hid|hci] (default hci)\n"
" --devpath= sys device path\n"
- " --method= method to use to switch [csr|logitech-hid|dell]\n"
+ " --method= method to use to switch [csr|csr2|logitech-hid|dell]\n"
" --help\n\n");
}

@@ -311,6 +354,9 @@ int main(int argc, char *argv[])
if (!strcmp(optarg, "csr")) {
method = METHOD_CSR;
usb_switch = usb_switch_csr;
+ } else if (!strcmp(optarg, "csr2")) {
+ method = METHOD_CSR;
+ usb_switch = usb_switch_csr2;
} else if (!strcmp(optarg, "logitech-hid")) {
method = METHOD_LOGITECH_HID;
} else if (!strcmp(optarg, "dell")) {
--
1.8.3.2


2014-03-29 11:37:05

by Gordon

[permalink] [raw]
Subject: Re: PATCH hid2hci for CSR 8510 A10


second try fo this patch, with remarks from Johan Hedberg addressed.
code style checked with linux's scripts/Lindent.


Gordon

2014-03-28 20:23:58

by Gordon

[permalink] [raw]
Subject: Re: PATCH hid2hci for CSR 8510 A10

Hi Johan,

On Fr, 2014-03-28 at 21:49 +0200, Johan Hedberg wrote:
> Hi Gordon,
>
> On Fri, Mar 28, 2014, Gordon wrote:
> > some time ago I sent a patch attached to following mail.
> >
> > On Mo, 2013-12-30 at 22:59 +0100, Gordon wrote:
> > > Hi,
> > >
> > > Could someone please review.
> >
> > I didn't get a reply yet. Since it has been my first mail to this list I
> > wonder whether there is anything wrong with this request, or just nobody
> > is interested ?
>
> A couple of things regarding the presentation of the patch should at
> least be fixed:
>
> - Send it as a proper git patch, i.e. using git commit, format-patch
> and send-email

ok, will try.

> - Try to follow the bluez.git coding style which is roughly the same as
> the Linux kernel coding style. I realize the rest of hid2hci.c is
> probably the worst reference for good BlueZ coding style (being one
> of the oldest pieces of code with very little maintenance over the
> years).
>

thanks for the detailed remarks, although removed from the reply, I will
address them.

> > + //unable to detect EALREADY
>
> We don't generally use C++ style comments in the tree. Prefer /* .. */
> where possible. In this case I'd elaborate the comment a bit since I
> don't quite understand what it means.

I am not sure, other functions in hid2hci presumably return this when
the device is already in the mode that should be switched to.

>
> > + case HID:
> > + // currently unknown what to do here
>

the method for switching back to hid mode is unknown, should I remove
the cases or just be more verbose in the comments ?

Thanks,

Gordon


2014-03-28 19:49:07

by Johan Hedberg

[permalink] [raw]
Subject: Re: PATCH hid2hci for CSR 8510 A10

Hi Gordon,

On Fri, Mar 28, 2014, Gordon wrote:
> some time ago I sent a patch attached to following mail.
>
> On Mo, 2013-12-30 at 22:59 +0100, Gordon wrote:
> > Hi,
> >
> > ich had a problem with the Sitecom CNT-524 as described by someone else
> > here:
> > http://blog.ruecker.fi/2013/10/06/adventures-in-bluetooth-4-0-part-i/
> >
> > so i tried to fix that by introducing --mode csr2 for hid2hci
> >
> > I neither know how to switch back to hid nor how to detect the EALREADY
> > state, otherwise the patch is very small.
> >
> > I works on my machine in ubuntu 13.04 with bluez 4.101-0ubuntu8b1 and
> > said sitecom usb dongle. the patch is against bluez commit
> > fd00064e0bb2c81e53e9d0b7d22ce919b41dbe60
> >
> > Could someone please review.
>
> I didn't get a reply yet. Since it has been my first mail to this list I
> wonder whether there is anything wrong with this request, or just nobody
> is interested ?

A couple of things regarding the presentation of the patch should at
least be fixed:

- Send it as a proper git patch, i.e. using git commit, format-patch
and send-email
- Try to follow the bluez.git coding style which is roughly the same as
the Linux kernel coding style. I realize the rest of hid2hci.c is
probably the worst reference for good BlueZ coding style (being one
of the oldest pieces of code with very little maintenance over the
years).

A couple of things that could be improved coding style-wise:

> +static int usb_switch_csr2(int fd, enum mode mode)
> +{
> + int err = 0;

Does this really need to be initialized to 0 upon declaration? Doing
this prevents the compiler from pointing out places where we should be
setting the variable to something else but dot (i.e.. in the case of
errors).

> + struct usbfs_disconnect disconnect;
> + char report[] = { 0x1 , 0x5 , 0x0 , 0x0 , 0x0 , 0x0 , 0x0 , 0x0 , 0x0 };

This should be an unsigned integer, uint8_t is probably best choice.
Spell out the individual values with two digits to make clear they are
bytes and not 4-bit values.


> + switch (mode) {
> + case HCI:
> + //send report as is
> + disconnect.interface = 0;
> + disconnect.flags = USBFS_DISCONNECT_EXCEPT_DRIVER;
> + strcpy(disconnect.driver, "usbfs");
> +
> + if (ioctl(fd, USBFS_IOCTL_DISCONNECT, &disconnect) < 0) {
> + fprintf(stderr, "Can't claim interface: %s (%d)\n",
> + strerror(errno), errno);
> + return -1;
> + }
> +
> + err = control_message(fd, USB_ENDPOINT_OUT | USB_TYPE_CLASS | USB_RECIP_INTERFACE,
> + USB_REQ_SET_CONFIGURATION, //Set_Report Request
> + 0x01 | (0x03 << 8), //report id: 0x01, report type: feature (0x03)
> + 0, //interface
> + report, sizeof(report), 5000);

Looks like you're hitting the 80-character line limits here. I know
other places in hid2hci.c violate against it but it'd be nice to at
least follow the rule for new code.

> + //unable to detect EALREADY

We don't generally use C++ style comments in the tree. Prefer /* .. */
where possible. In this case I'd elaborate the comment a bit since I
don't quite understand what it means.

> + case HID:
> + // currently unknown what to do here

Same here.

Johan

2014-03-28 19:21:02

by Gordon

[permalink] [raw]
Subject: Re: PATCH hid2hci for CSR 8510 A10

Hi,

some time ago I sent a patch attached to following mail.

On Mo, 2013-12-30 at 22:59 +0100, Gordon wrote:
> Hi,
>
> ich had a problem with the Sitecom CNT-524 as described by someone else
> here:
> http://blog.ruecker.fi/2013/10/06/adventures-in-bluetooth-4-0-part-i/
>
> so i tried to fix that by introducing --mode csr2 for hid2hci
>
> I neither know how to switch back to hid nor how to detect the EALREADY
> state, otherwise the patch is very small.
>
> I works on my machine in ubuntu 13.04 with bluez 4.101-0ubuntu8b1 and
> said sitecom usb dongle. the patch is against bluez commit
> fd00064e0bb2c81e53e9d0b7d22ce919b41dbe60
>
> Could someone please review.

I didn't get a reply yet. Since it has been my first mail to this list I
wonder whether there is anything wrong with this request, or just nobody
is interested ?


Cheers,

Gordon


Attachments:
bluez_hid2hci.patch (2.41 kB)

2014-04-01 21:38:59

by Gordon

[permalink] [raw]
Subject: [PATCH v3] hid2hci add CSR 8510 A10 support

add a new method "csr2" to hid2hci that sends a control message
to the usb device which switches the Cambridge Silicon Radio 8510 A10
as used in Sitecom CNT-524

fixes: https://bugzilla.kernel.org/show_bug.cgi?id=69181
---

Notes:
3rd version, coding style corrections from Johan Hedberg and Anderson Lizardo

tools/hid2hci.1 | 2 +-
tools/hid2hci.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 51 insertions(+), 2 deletions(-)

diff --git a/tools/hid2hci.1 b/tools/hid2hci.1
index 8c5d520..c6876a3 100644
--- a/tools/hid2hci.1
+++ b/tools/hid2hci.1
@@ -32,7 +32,7 @@ mode and back.
.B --mode= [hid, hci]
Sets the mode to switch the device into
.TP
-.B --method= [csr, logitech-hid, dell]
+.B --method= [csr, csr2, logitech-hid, dell]
Which vendor method to use for switching the device.
.TP
.B --devpath=
diff --git a/tools/hid2hci.c b/tools/hid2hci.c
index 2dbfca7..a83bace 100644
--- a/tools/hid2hci.c
+++ b/tools/hid2hci.c
@@ -119,6 +119,52 @@ static int usb_switch_csr(int fd, enum mode mode)
return err;
}

+static int usb_switch_csr2(int fd, enum mode mode)
+{
+ int err;
+ struct usbfs_disconnect disconnect;
+ uint8_t report[] = {
+ 0x01, 0x05, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
+ };
+
+ switch (mode) {
+ case HCI:
+ /* send report as is */
+ disconnect.interface = 0;
+ disconnect.flags = USBFS_DISCONNECT_EXCEPT_DRIVER;
+ strcpy(disconnect.driver, "usbfs");
+
+ if (ioctl(fd, USBFS_IOCTL_DISCONNECT, &disconnect) < 0) {
+ fprintf(stderr, "Can't claim interface: %s (%d)\n",
+ strerror(errno), errno);
+ return -1;
+ }
+
+ /* Set_report request with
+ * report id: 0x01, report type: feature (0x03)
+ * on interface 0
+ */
+ err = control_message(fd,
+ USB_ENDPOINT_OUT | USB_TYPE_CLASS |
+ USB_RECIP_INTERFACE,
+ USB_REQ_SET_CONFIGURATION,
+ 0x01 | (0x03 << 8),
+ 0, report, sizeof(report), 5000);
+ /* unable to detect whether the previous state
+ * already was HCI (EALREADY)
+ */
+ break;
+ case HID:
+ /* currently unknown how to switch to HID */
+ fprintf(stderr,
+ "csr2: Switching to hid mode is not implemented\n");
+ err = -1;
+ break;
+ }
+
+ return err;
+}
+
static int hid_logitech_send_report(int fd, const char *buf, size_t size)
{
struct hiddev_report_info rinfo;
@@ -258,7 +304,7 @@ static void usage(const char *error)
printf("Usage: hid2hci [options]\n"
" --mode= mode to switch to [hid|hci] (default hci)\n"
" --devpath= sys device path\n"
- " --method= method to use to switch [csr|logitech-hid|dell]\n"
+ " --method= method to use to switch [csr|csr2|logitech-hid|dell]\n"
" --help\n\n");
}

@@ -311,6 +357,9 @@ int main(int argc, char *argv[])
if (!strcmp(optarg, "csr")) {
method = METHOD_CSR;
usb_switch = usb_switch_csr;
+ } else if (!strcmp(optarg, "csr2")) {
+ method = METHOD_CSR;
+ usb_switch = usb_switch_csr2;
} else if (!strcmp(optarg, "logitech-hid")) {
method = METHOD_LOGITECH_HID;
} else if (!strcmp(optarg, "dell")) {
--
1.8.3.2


2015-01-08 10:02:16

by Johan Hedberg

[permalink] [raw]
Subject: Re: [PATCH v5 0/2] hid2hci add CSR 8510 A10 support

Hi Gordon,

On Wed, Jan 07, 2015, [email protected] wrote:
> fixed the compilation bug. using following config
> configure --enable-maintainer-mode --disable-systemd --disable-cups --disable-monitor
>
> i also only compile with
> make tools/hid2hci
>
> added a second patch that implements your suggestions with const uint_8 *
>
> Thanks!
>
> Gordon Kramer (2):
> hid2hci add CSR 8510 A10 support
> use const uint_8* for pointer in control_message
>
> tools/hid2hci.1 | 2 +-
> tools/hid2hci.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++++++++----
> 2 files changed, 54 insertions(+), 5 deletions(-)

I applied both of these patches now, however the first one still wasn't
completely clean and needed auto-fixing from git:

Applying: hid2hci add CSR 8510 A10 support
/home/jh/src/bluez/.git/rebase-apply/patch:52: trailing whitespace.
/* Set_report request with
/home/jh/src/bluez/.git/rebase-apply/patch:53: trailing whitespace.
* report id: 0x01, report type: feature (0x03)
/home/jh/src/bluez/.git/rebase-apply/patch:62: trailing whitespace.
/* unable to detect whether the previous state
/home/jh/src/bluez/.git/rebase-apply/patch:63: trailing whitespace.
* already was HCI (EALREADY)
warning: 4 lines applied after fixing whitespace errors.

Johan

2015-01-07 22:04:00

by Gordon

[permalink] [raw]
Subject: [PATCH v5 2/2] use const uint_8* for pointer in control_message

From: Gordon Kramer <[email protected]>

change control_message signature
requires const void* usbfs_ctrltransfer for ioctl
use uint8 * in usb_switch_dell
---
tools/hid2hci.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/hid2hci.c b/tools/hid2hci.c
index 3eedf4e..033fcfd 100644
--- a/tools/hid2hci.c
+++ b/tools/hid2hci.c
@@ -59,7 +59,7 @@ struct usbfs_ctrltransfer {
uint16_t wIndex;
uint16_t wLength;
uint32_t timeout; /* in milliseconds */
- void *data; /* pointer to data */
+ const void *data; /* pointer to data */
};


@@ -77,7 +77,7 @@ struct usbfs_disconnect{

static int control_message(int fd, int requesttype, int request,
int value, int index,
- char *bytes, int size, int timeout)
+ const uint8_t *bytes, int size, int timeout)
{
struct usbfs_ctrltransfer transfer;

@@ -123,7 +123,7 @@ static int usb_switch_csr2(int fd, enum mode mode)
{
int err = 0;
struct usbfs_disconnect disconnect;
- char report[] = {
+ const uint8_t report[] = {
0x01, 0x05, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
};

@@ -226,7 +226,7 @@ out:

static int usb_switch_dell(int fd, enum mode mode)
{
- char report[] = { 0x7f, 0x00, 0x00, 0x00 };
+ uint8_t report[] = { 0x7f, 0x00, 0x00, 0x00 };
struct usbfs_disconnect disconnect;
int err;

--
1.9.1


2015-01-07 22:03:59

by Gordon

[permalink] [raw]
Subject: [PATCH v5 1/2] hid2hci add CSR 8510 A10 support

From: Gordon Kramer <[email protected]>

add a new method "csr2" to hid2hci that sends a control message
to the usb device which switches the Cambridge Silicon Radio 8510 A10
as used in Sitecom CNT-524

fixes: https://bugzilla.kernel.org/show_bug.cgi?id=69181
---

Notes:
5th version, coding style corrections from Johan Hedberg and Anderson Lizardo, compilation fix

tools/hid2hci.1 | 2 +-
tools/hid2hci.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 51 insertions(+), 2 deletions(-)

diff --git a/tools/hid2hci.1 b/tools/hid2hci.1
index 8c5d520..c6876a3 100644
--- a/tools/hid2hci.1
+++ b/tools/hid2hci.1
@@ -32,7 +32,7 @@ mode and back.
.B --mode= [hid, hci]
Sets the mode to switch the device into
.TP
-.B --method= [csr, logitech-hid, dell]
+.B --method= [csr, csr2, logitech-hid, dell]
Which vendor method to use for switching the device.
.TP
.B --devpath=
diff --git a/tools/hid2hci.c b/tools/hid2hci.c
index a183bfa..3eedf4e 100644
--- a/tools/hid2hci.c
+++ b/tools/hid2hci.c
@@ -119,6 +119,52 @@ static int usb_switch_csr(int fd, enum mode mode)
return err;
}

+static int usb_switch_csr2(int fd, enum mode mode)
+{
+ int err = 0;
+ struct usbfs_disconnect disconnect;
+ char report[] = {
+ 0x01, 0x05, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
+ };
+
+ switch (mode) {
+ case HCI:
+ /* send report as is */
+ disconnect.interface = 0;
+ disconnect.flags = USBFS_DISCONNECT_EXCEPT_DRIVER;
+ strcpy(disconnect.driver, "usbfs");
+
+ if (ioctl(fd, USBFS_IOCTL_DISCONNECT, &disconnect) < 0) {
+ fprintf(stderr, "Can't claim interface: %s (%d)\n",
+ strerror(errno), errno);
+ return -1;
+ }
+
+ /* Set_report request with
+ * report id: 0x01, report type: feature (0x03)
+ * on interface 0
+ */
+ err = control_message(fd,
+ USB_ENDPOINT_OUT | USB_TYPE_CLASS |
+ USB_RECIP_INTERFACE,
+ USB_REQ_SET_CONFIGURATION,
+ 0x01 | (0x03 << 8),
+ 0, report, sizeof(report), 5000);
+ /* unable to detect whether the previous state
+ * already was HCI (EALREADY)
+ */
+ break;
+ case HID:
+ /* currently unknown how to switch to HID */
+ fprintf(stderr,
+ "csr2: Switching to hid mode is not implemented\n");
+ err = -1;
+ break;
+ }
+
+ return err;
+}
+
static int hid_logitech_send_report(int fd, const char *buf, size_t size)
{
struct hiddev_report_info rinfo;
@@ -258,7 +304,7 @@ static void usage(const char *error)
printf("Usage: hid2hci [options]\n"
" --mode= mode to switch to [hid|hci] (default hci)\n"
" --devpath= sys device path\n"
- " --method= method to use to switch [csr|logitech-hid|dell]\n"
+ " --method= method to use to switch [csr|csr2|logitech-hid|dell]\n"
" --help\n\n");
}

@@ -311,6 +357,9 @@ int main(int argc, char *argv[])
if (!strcmp(optarg, "csr")) {
method = METHOD_CSR;
usb_switch = usb_switch_csr;
+ } else if (!strcmp(optarg, "csr2")) {
+ method = METHOD_CSR;
+ usb_switch = usb_switch_csr2;
} else if (!strcmp(optarg, "logitech-hid")) {
method = METHOD_LOGITECH_HID;
} else if (!strcmp(optarg, "dell")) {
--
1.9.1


2015-01-07 22:03:58

by Gordon

[permalink] [raw]
Subject: [PATCH v5 0/2] hid2hci add CSR 8510 A10 support

From: Gordon Kramer <[email protected]>

Hi Johan,

fixed the compilation bug. using following config
configure --enable-maintainer-mode --disable-systemd --disable-cups --disable-monitor

i also only compile with
make tools/hid2hci

added a second patch that implements your suggestions with const uint_8 *

Thanks!

Gordon Kramer (2):
hid2hci add CSR 8510 A10 support
use const uint_8* for pointer in control_message

tools/hid2hci.1 | 2 +-
tools/hid2hci.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++++++++----
2 files changed, 54 insertions(+), 5 deletions(-)

--
1.9.1


2015-01-07 21:21:16

by Gordon

[permalink] [raw]
Subject: Re: [PATCH v4] hid2hci add CSR 8510 A10 support

Hi Johan,

forget my earlier mail, this was due to using a different system.

Am Mittwoch, 7. Januar 2015, 22:31:59 schrieb Johan Hedberg:
> Hi Gordon,
>
> On Wed, Jan 07, 2015, [email protected] wrote:
> > add a new method "csr2" to hid2hci that sends a control message
> > to the usb device which switches the Cambridge Silicon Radio 8510 A10
> > as used in Sitecom CNT-524
> >
> > fixes: https://bugzilla.kernel.org/show_bug.cgi?id=69181
> > ---
> >
> > Notes:
> > 4th version, coding style corrections from Johan Hedberg and Anderson
> > Lizardo, resend>
> > tools/hid2hci.1 | 2 +-
> > tools/hid2hci.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++++++-
> > 2 files changed, 51 insertions(+), 2 deletions(-)
>
> I was about to apply this but it doesn't compile:
>
> make --no-print-directory all-am
> CC tools/hid2hci.o
> tools/hid2hci.c: In function ‘usb_switch_csr2’:
> tools/hid2hci.c:152:14: error: pointer targets in passing argument 6 of
> ‘control_message’ differ in signedness [-Werror=pointer-sign] 0, report,
> sizeof(report), 5000);
> ^
> tools/hid2hci.c:78:12: note: expected ‘char *’ but argument is of type
> ‘uint8_t *’ static int control_message(int fd, int requesttype, int
> request,
> ^

what do you prefer? char * or uint_8 * in the function declaration ? when
switching to unit_8 there will be changes in other functions as well.


> tools/hid2hci.c:124:6: error: ‘err’ may be used uninitialized in this
> function [-Werror=maybe-uninitialized] int err;
> ^
> cc1: all warnings being treated as errors

ok easy

>
> Please use ./configure --enable-maintainer-mode to verify that the build
> is clean (or ./bootstrap-configure && make).
>
> > + uint8_t report[] = {
> > + 0x01, 0x05, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
> > + };
>
> Minor improvement: you might want to declare this as const.

const also means a change to the method declaration or an extra cast. What do
you prefer ?

Thanks,

Gordon


2015-01-07 20:52:24

by Gordon

[permalink] [raw]
Subject: Re: [PATCH v4] hid2hci add CSR 8510 A10 support

Am Mittwoch, 7. Januar 2015, 22:31:59 schrieb Johan Hedberg:
> Hi Gordon,
>
> On Wed, Jan 07, 2015, [email protected] wrote:
> > add a new method "csr2" to hid2hci that sends a control message
> > to the usb device which switches the Cambridge Silicon Radio 8510 A10
> > as used in Sitecom CNT-524
> >
> > fixes: https://bugzilla.kernel.org/show_bug.cgi?id=69181
> > ---
> >
> > Notes:
> > 4th version, coding style corrections from Johan Hedberg and Anderson
> > Lizardo, resend>
> > tools/hid2hci.1 | 2 +-
> > tools/hid2hci.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++++++-
> > 2 files changed, 51 insertions(+), 2 deletions(-)
>
> I was about to apply this but it doesn't compile:
>

sorry, i didn't test after rebasing, last year the compiler wasn't
complaining, and now i even need a new glib. is there a recommended developer
platform/OS distribution ?

Thanks,

Gordon

2015-01-07 20:31:59

by Johan Hedberg

[permalink] [raw]
Subject: Re: [PATCH v4] hid2hci add CSR 8510 A10 support

Hi Gordon,

On Wed, Jan 07, 2015, [email protected] wrote:
> add a new method "csr2" to hid2hci that sends a control message
> to the usb device which switches the Cambridge Silicon Radio 8510 A10
> as used in Sitecom CNT-524
>
> fixes: https://bugzilla.kernel.org/show_bug.cgi?id=69181
> ---
>
> Notes:
> 4th version, coding style corrections from Johan Hedberg and Anderson Lizardo, resend
>
> tools/hid2hci.1 | 2 +-
> tools/hid2hci.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 51 insertions(+), 2 deletions(-)

I was about to apply this but it doesn't compile:

make --no-print-directory all-am
CC tools/hid2hci.o
tools/hid2hci.c: In function ‘usb_switch_csr2’:
tools/hid2hci.c:152:14: error: pointer targets in passing argument 6 of ‘control_message’ differ in signedness [-Werror=pointer-sign]
0, report, sizeof(report), 5000);
^
tools/hid2hci.c:78:12: note: expected ‘char *’ but argument is of type ‘uint8_t *’
static int control_message(int fd, int requesttype, int request,
^
tools/hid2hci.c:124:6: error: ‘err’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
int err;
^
cc1: all warnings being treated as errors

Please use ./configure --enable-maintainer-mode to verify that the build
is clean (or ./bootstrap-configure && make).

> + uint8_t report[] = {
> + 0x01, 0x05, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
> + };

Minor improvement: you might want to declare this as const.

Johan

2015-01-07 10:10:17

by Gordon

[permalink] [raw]
Subject: [PATCH v4] hid2hci add CSR 8510 A10 support

From: Gordon Kramer <[email protected]>

add a new method "csr2" to hid2hci that sends a control message
to the usb device which switches the Cambridge Silicon Radio 8510 A10
as used in Sitecom CNT-524

fixes: https://bugzilla.kernel.org/show_bug.cgi?id=69181
---

Notes:
4th version, coding style corrections from Johan Hedberg and Anderson Lizardo, resend

tools/hid2hci.1 | 2 +-
tools/hid2hci.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 51 insertions(+), 2 deletions(-)

diff --git a/tools/hid2hci.1 b/tools/hid2hci.1
index 8c5d520..c6876a3 100644
--- a/tools/hid2hci.1
+++ b/tools/hid2hci.1
@@ -32,7 +32,7 @@ mode and back.
.B --mode= [hid, hci]
Sets the mode to switch the device into
.TP
-.B --method= [csr, logitech-hid, dell]
+.B --method= [csr, csr2, logitech-hid, dell]
Which vendor method to use for switching the device.
.TP
.B --devpath=
diff --git a/tools/hid2hci.c b/tools/hid2hci.c
index a183bfa..49dcada 100644
--- a/tools/hid2hci.c
+++ b/tools/hid2hci.c
@@ -119,6 +119,52 @@ static int usb_switch_csr(int fd, enum mode mode)
return err;
}

+static int usb_switch_csr2(int fd, enum mode mode)
+{
+ int err;
+ struct usbfs_disconnect disconnect;
+ uint8_t report[] = {
+ 0x01, 0x05, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
+ };
+
+ switch (mode) {
+ case HCI:
+ /* send report as is */
+ disconnect.interface = 0;
+ disconnect.flags = USBFS_DISCONNECT_EXCEPT_DRIVER;
+ strcpy(disconnect.driver, "usbfs");
+
+ if (ioctl(fd, USBFS_IOCTL_DISCONNECT, &disconnect) < 0) {
+ fprintf(stderr, "Can't claim interface: %s (%d)\n",
+ strerror(errno), errno);
+ return -1;
+ }
+
+ /* Set_report request with
+ * report id: 0x01, report type: feature (0x03)
+ * on interface 0
+ */
+ err = control_message(fd,
+ USB_ENDPOINT_OUT | USB_TYPE_CLASS |
+ USB_RECIP_INTERFACE,
+ USB_REQ_SET_CONFIGURATION,
+ 0x01 | (0x03 << 8),
+ 0, report, sizeof(report), 5000);
+ /* unable to detect whether the previous state
+ * already was HCI (EALREADY)
+ */
+ break;
+ case HID:
+ /* currently unknown how to switch to HID */
+ fprintf(stderr,
+ "csr2: Switching to hid mode is not implemented\n");
+ err = -1;
+ break;
+ }
+
+ return err;
+}
+
static int hid_logitech_send_report(int fd, const char *buf, size_t size)
{
struct hiddev_report_info rinfo;
@@ -258,7 +304,7 @@ static void usage(const char *error)
printf("Usage: hid2hci [options]\n"
" --mode= mode to switch to [hid|hci] (default hci)\n"
" --devpath= sys device path\n"
- " --method= method to use to switch [csr|logitech-hid|dell]\n"
+ " --method= method to use to switch [csr|csr2|logitech-hid|dell]\n"
" --help\n\n");
}

@@ -311,6 +357,9 @@ int main(int argc, char *argv[])
if (!strcmp(optarg, "csr")) {
method = METHOD_CSR;
usb_switch = usb_switch_csr;
+ } else if (!strcmp(optarg, "csr2")) {
+ method = METHOD_CSR;
+ usb_switch = usb_switch_csr2;
} else if (!strcmp(optarg, "logitech-hid")) {
method = METHOD_LOGITECH_HID;
} else if (!strcmp(optarg, "dell")) {
--
1.9.1


2015-01-07 09:12:48

by Johan Hedberg

[permalink] [raw]
Subject: Re: [PATCH v3] hid2hci add CSR 8510 A10 support

Hi Gordon,

On Wed, Jan 07, 2015, Gordon wrote:
> is there a reason this is not yet in bluez ?
>
> http://article.gmane.org/gmane.linux.bluez.kernel/46985

Probably because it's simply been forgotten about (feel free to send a
reminder email if a patch hasn't been responded to in a week or so).
However, before applying this I'd appreciate if you could re-send it
with your full name in the author information. We generally prefer not
to include anonymous commits in the tree.

Johan

2015-01-07 08:24:36

by Gordon

[permalink] [raw]
Subject: Re: [PATCH v3] hid2hci add CSR 8510 A10 support

Hi,

is there a reason this is not yet in bluez ?

http://article.gmane.org/gmane.linux.bluez.kernel/46985

Gordon