On Thu, 28 Jul 2011, David Herrmann wrote:
> >> +static void handler_accel(struct wiimote_data *wdata, const __u8 *payload)
> >> +{
> >> + __u16 x, y, z;
> >> +
> >> + if (!(wdata->state.flags & WIIPROTO_FLAG_ACCEL))
> >> + return;
> >> +
> >> + /*
> >> + * payload is: BB BB XX YY ZZ
> >> + * Buttons data contains LSBs
> >> + */
> >> +
> >> + x = payload[2] << 2;
> >> + y = payload[3] << 2;
> >> + z = payload[4] << 2;
> >> +
> >> + x |= (payload[0] >> 5) & 0x3;
> >> + y |= (payload[1] >> 4) & 0x2;
> >> + z |= (payload[1] >> 5) & 0x2;
> >
> > Could you make the comments a bit clearer. Those last lines are impossible
> > to understand.
>
> The LSBs are encoded in the "BB BB" data and I am extracting them. I
> have documented the whole protocol in a separated document but if it
> is common practice to add those comments to the code, I will add it in
> the next version.
In some cases, the usual way is to put a brief description of the
protocol/data structures mandated by hardware in the comments at the very
beginning of the driver file.
The description below seems like a perfect fit for such purpose. If you
have much more elaborate description, it should probably go into
Documentation/.
Thanks,
--
Jiri Kosina
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
