Re: [PATCH v1] i2c-hid: introduce HID over i2c specification implementation

2012-10-15 Thread Benjamin Tissoires
On Sun, Oct 7, 2012 at 9:03 PM, Jean Delvare wrote: > On Sun, 7 Oct 2012 18:57:36 +0200, Benjamin Tissoires wrote: >> On Sun, Oct 7, 2012 at 4:28 PM, Jean Delvare wrote: >> > On Fri, 14 Sep 2012 15:41:43 +0200, benjamin.tissoires wrote: >> >> + u16 readRegister = ihid->hdesc.wDataRegister; >>

Re: [PATCH v1] i2c-hid: introduce HID over i2c specification implementation

2012-10-08 Thread Jiri Kosina
On Sun, 7 Oct 2012, Benjamin Tissoires wrote: > It seems that hid transport layers should go in drivers/hid. > However, I don't like mixing the transport layer and the final > drivers. Maybe this is the time to rework a little bit the tree. > To minimize the moves, we could introduce: > drivers/hi

Re: [PATCH v1] i2c-hid: introduce HID over i2c specification implementation

2012-10-07 Thread Jean Delvare
On Sun, 7 Oct 2012 18:57:36 +0200, Benjamin Tissoires wrote: > On Sun, Oct 7, 2012 at 4:28 PM, Jean Delvare wrote: > > On Fri, 14 Sep 2012 15:41:43 +0200, benjamin.tissoires wrote: > >> + u16 readRegister = ihid->hdesc.wDataRegister; > > > > This is missing le16_to_cpu(). > > I agree this is

Re: [PATCH v1] i2c-hid: introduce HID over i2c specification implementation

2012-10-07 Thread Benjamin Tissoires
Hello Jean, many thanks for this detailed review. I agree to almost all of your comments, so I didn't add an 'ok' after each remark. This review will give me some work, but the driver will be much better. On Sun, Oct 7, 2012 at 4:28 PM, Jean Delvare wrote: > Salut Benjamin, > > On Fri, 14 Sep 20

Re: [PATCH v1] i2c-hid: introduce HID over i2c specification implementation

2012-10-07 Thread Stéphane Chatty
Le 7 oct. 2012 à 18:07, Benjamin Tissoires a écrit : >>> >>> Basically, to me this all boils down to the question -- what is more >>> important: low-level transport being used, or the general function of the >>> device? >>> >>> To me, it's the latter, and as such, everything would belong under >

Re: [PATCH v1] i2c-hid: introduce HID over i2c specification implementation

2012-10-07 Thread Benjamin Tissoires
On Sat, Oct 6, 2012 at 11:39 PM, Stéphane Chatty wrote: > > Le 6 oct. 2012 à 23:28, Jiri Kosina a écrit : > >> On Sat, 6 Oct 2012, Jiri Kosina wrote: >> My vote is a clear 3. It took me a few years to kick all users (as opposed to implementers) of i2c from drivers/i2c and finding them a

Re: [PATCH v1] i2c-hid: introduce HID over i2c specification implementation

2012-10-07 Thread Benjamin Tissoires
Hi Jean, Thanks for the comments, the tests and the review. I'm going to try to answer most of the remarks, so here is the first: On Sat, Oct 6, 2012 at 10:04 PM, Jean Delvare wrote: > Hi Benjamin, > [...] >a > ERROR: "hiddev_report_event" [drivers/i2c/i2c-hid.ko] undefined! > ERROR: "hiddev_dis

Re: [PATCH v1] i2c-hid: introduce HID over i2c specification implementation

2012-10-07 Thread Jean Delvare
Salut Benjamin, On Fri, 14 Sep 2012 15:41:43 +0200, benjamin.tissoires wrote: > From: Benjamin Tissoires > > Microsoft published the protocol specification of HID over i2c: > http://msdn.microsoft.com/en-us/library/windows/hardware/hh852380.aspx > > This patch introduces an implementation of th

Re: [PATCH v1] i2c-hid: introduce HID over i2c specification implementation

2012-10-07 Thread Jean Delvare
On Sat, 6 Oct 2012 23:27:47 +0200, Stéphane Chatty wrote: > Le 6 oct. 2012 à 23:11, Jean Delvare a écrit : > > On Sat, 6 Oct 2012 22:30:00 +0200, Stéphane Chatty wrote: > >> This is a question I asked a few months back, but apparently not all > >> points of views had been expressed at the time. Cu

Re: [PATCH v1] i2c-hid: introduce HID over i2c specification implementation

2012-10-06 Thread Stéphane Chatty
Le 6 oct. 2012 à 23:28, Jiri Kosina a écrit : > On Sat, 6 Oct 2012, Jiri Kosina wrote: > >>> My vote is a clear 3. It took me a few years to kick all users (as >>> opposed to implementers) of i2c from drivers/i2c and finding them a >>> proper home, I'm not going to accept new intruders. Grouping

Re: [PATCH v1] i2c-hid: introduce HID over i2c specification implementation

2012-10-06 Thread Jiri Kosina
On Sat, 6 Oct 2012, Jiri Kosina wrote: > > My vote is a clear 3. It took me a few years to kick all users (as > > opposed to implementers) of i2c from drivers/i2c and finding them a > > proper home, I'm not going to accept new intruders. Grouping drivers > > according to what they implement makes

Re: [PATCH v1] i2c-hid: introduce HID over i2c specification implementation

2012-10-06 Thread Stéphane Chatty
Le 6 oct. 2012 à 23:11, Jean Delvare a écrit : > On Sat, 6 Oct 2012 22:30:00 +0200, Stéphane Chatty wrote: >> Le 6 oct. 2012 à 22:04, Jean Delvare a écrit : >>> Looks like the wrong place for this driver. HID-over-USB support lives >>> under drivers/hid, so your driver should go there as well. No

Re: [PATCH v1] i2c-hid: introduce HID over i2c specification implementation

2012-10-06 Thread Jiri Kosina
On Sat, 6 Oct 2012, Jean Delvare wrote: > > The question is what drives the choice of where to put HID-over-XXX, among > > the following > > 1- who the maintainer is. Here, Benjamin will probably maintain this > > so it does not help. > > 2- dependencies. HID-over-XXX depends on HID as much as

Re: [PATCH v1] i2c-hid: introduce HID over i2c specification implementation

2012-10-06 Thread Jean Delvare
On Sat, 6 Oct 2012 22:30:00 +0200, Stéphane Chatty wrote: > Le 6 oct. 2012 à 22:04, Jean Delvare a écrit : > > Looks like the wrong place for this driver. HID-over-USB support lives > > under drivers/hid, so your driver should go there as well. Not only > > this will be more consistent, but it also

Re: [PATCH v1] i2c-hid: introduce HID over i2c specification implementation

2012-10-06 Thread Stéphane Chatty
Hi Jean (I cc Marcel Holtmann because BT is involved too) Le 6 oct. 2012 à 22:04, Jean Delvare a écrit : > Hi Benjamin, > > On Fri, 14 Sep 2012 15:41:43 +0200, benjamin.tissoires wrote: >> From: Benjamin Tissoires >> >> Microsoft published the protocol specification of HID over i2c: >> http:/

Re: [PATCH v1] i2c-hid: introduce HID over i2c specification implementation

2012-10-06 Thread Jean Delvare
Hi Benjamin, On Fri, 14 Sep 2012 15:41:43 +0200, benjamin.tissoires wrote: > From: Benjamin Tissoires > > Microsoft published the protocol specification of HID over i2c: > http://msdn.microsoft.com/en-us/library/windows/hardware/hh852380.aspx > > This patch introduces an implementation of this

Re: [PATCH v1] i2c-hid: introduce HID over i2c specification implementation

2012-10-06 Thread Jean Delvare
On Wed, 3 Oct 2012 09:43:12 -0700, Dmitry Torokhov wrote: > On Fri, Sep 14, 2012 at 03:41:43PM +0200, benjamin.tissoires wrote: > > + } > > + > > + do { > > + ret = i2c_transfer(client->adapter, msg, msg_num); > > + if (ret > 0) > > + break; > > +

Re: [PATCH v1] i2c-hid: introduce HID over i2c specification implementation

2012-10-04 Thread Jiri Kosina
On Thu, 4 Oct 2012, Benjamin Tissoires wrote: > >> + > >> + hid->claimed = 0; > > > > Should it be here and not in core? > > This is a line that was copied/pasted from usbhid. I'll check how can > I do that without interfering with core. Well, we are calling ll_driver->stop at multiple plac

Re: [PATCH v1] i2c-hid: introduce HID over i2c specification implementation

2012-10-04 Thread Benjamin Tissoires
Hi Dmitry, thanks for the review. On Wed, Oct 3, 2012 at 6:43 PM, Dmitry Torokhov wrote: > Hi Benjamin, > > A few random comments... > > On Fri, Sep 14, 2012 at 03:41:43PM +0200, benjamin.tissoires wrote: >> From: Benjamin Tissoires >> >> Microsoft published the protocol specification of HID ov

Re: [PATCH v1] i2c-hid: introduce HID over i2c specification implementation

2012-10-03 Thread Dmitry Torokhov
Hi Benjamin, A few random comments... On Fri, Sep 14, 2012 at 03:41:43PM +0200, benjamin.tissoires wrote: > From: Benjamin Tissoires > > Microsoft published the protocol specification of HID over i2c: > http://msdn.microsoft.com/en-us/library/windows/hardware/hh852380.aspx > > This patch intro

Re: [PATCH v1] i2c-hid: introduce HID over i2c specification implementation

2012-10-03 Thread Shubhrajyoti Datta
On Wed, Oct 3, 2012 at 9:03 PM, Benjamin Tissoires wrote: > Hi, > > thanks also for the review. Two in the same day! I was about to send a > ping on that patch ;-) > > On Wed, Oct 3, 2012 at 8:05 AM, Shubhrajyoti Datta > wrote: >> On Fri, Sep 14, 2012 at 7:11 PM, benjamin.tissoires >> wrote:

Re: [PATCH v1] i2c-hid: introduce HID over i2c specification implementation

2012-10-03 Thread Benjamin Tissoires
Hi, thanks also for the review. Two in the same day! I was about to send a ping on that patch ;-) On Wed, Oct 3, 2012 at 8:05 AM, Shubhrajyoti Datta wrote: > On Fri, Sep 14, 2012 at 7:11 PM, benjamin.tissoires > wrote: >> From: Benjamin Tissoires >> >> Microsoft published the protocol spec

Re: [PATCH v1] i2c-hid: introduce HID over i2c specification implementation

2012-10-03 Thread Benjamin Tissoires
Hi JJ, On Wed, Oct 3, 2012 at 5:08 AM, Jian-Jhong Ding wrote: > Hi Benjamin, > > I have one little question about __i2chid_command(), please see below. > > "benjamin.tissoires" writes: >> From: Benjamin Tissoires >> >> Microsoft published the protocol specification of HID over i2c: >> http://ms

Re: [PATCH v1] i2c-hid: introduce HID over i2c specification implementation

2012-10-02 Thread Shubhrajyoti Datta
On Fri, Sep 14, 2012 at 7:11 PM, benjamin.tissoires wrote: > From: Benjamin Tissoires > > Microsoft published the protocol specification of HID over i2c: > http://msdn.microsoft.com/en-us/library/windows/hardware/hh852380.aspx > > This patch introduces an implementation of this protocol. > > This

Re: [PATCH v1] i2c-hid: introduce HID over i2c specification implementation

2012-10-02 Thread Jian-Jhong Ding
Hi Benjamin, I have one little question about __i2chid_command(), please see below. "benjamin.tissoires" writes: > From: Benjamin Tissoires > > Microsoft published the protocol specification of HID over i2c: > http://msdn.microsoft.com/en-us/library/windows/hardware/hh852380.aspx > > This patch