Re: [PATCH 07/16] HID: wiimote: Parse accelerometer data

On Thu, 28 Jul 2011, David Herrmann wrote:

quoted

quoted

+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@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html