Re: [PATCH v2] add sur40 driver for Samsung SUR40 (aka MS Surface 2.0/Pixelsense)

[Dmitry Torokhov <dmitry.torokhov@gmail.com>]

On Tue, Oct 22, 2013 at 05:22:08PM +0200, David Herrmann wrote:
> Hi
> 
> On Tue, Oct 22, 2013 at 8:08 AM, Florian Echtler <floe@butterbrot.org> wrote:
> > Hello Dmitry,
> >
> > thanks for your quick feedback, a few questions below:
> >
> > On 21.10.2013 18:20, Dmitry Torokhov wrote:
> >> On Sun, Oct 20, 2013 at 06:49:11PM +0200, Florian Echtler wrote:
> >>> +/* read 512 bytes from endpoint 0x86 -> get header + blobs */
> >>> +struct sur40_header {
> >>> +
> >>> +    uint16_t type;       /* always 0x0001 */
> >>> +    uint16_t count;      /* count of blobs (if 0: continue prev. packet) */
> >>> +
> >>> +    uint32_t packet_id;
> >>> +
> >>> +    uint32_t timestamp;  /* milliseconds (inc. by 16 or 17 each frame) */
> >>> +    uint32_t unknown;    /* "epoch?" always 02/03 00 00 00 */
> >>
> >> Proper internal kernel types are u8, u16, u32. For user-facing APIs
> >> __u8, __u16, and __u32 should be used. Also, since this is data coming
> >> directly off the wire, you should be using __le16, __le32, etc, and then
> >> do __leXX_to_cpu() conversion before using it in calculations.
> > OK, I'll switch to u32 throughout (also for the float, I'll explain in a
> > commment). However, I haven't found a single other touchscreen driver
> > which uses __le32, even though they all probably process raw wire data -
> > can you suggest an example?
> 
> These are probably all broken or the hardware guarantees
> cpu-byte-order. Anyway, what you should do is use __le16/32 for your
> types which represent data from the device. Then call le16_to_cpu() on
> these values to convert it to host byte-order. Something like this:
> 
> struct sur40_raw_header {
>   __le16 type;
>   __le32 unused;
>   __le8 count;

Not the last one please ;)

Thanks.

-- 
Dmitry