Re: [PATCH 5/5] Input: add CMR3000 gyrsocope driver

Hello Jonathan

On Tue, Oct 18, 2011 at 10:54, Jonathan Cameron [off-list ref] wrote:

On 10/18/11 09:28, Ricardo Ribalda Delgado wrote:

Quick review done below.  Quite a bit of overlap with earlier reviews.
Sorry I couldn't take a closer look.

I am fixing also previous errors

accl?  I thought this was a gyro so this seems odd.

copy paste :), fixing it

I'd much rather see these done explicity in the code.
This sort of macro just obscures the works to save a few lines
of code.

quoted

+#define CMR3000_READ(data, reg, msg) \
+     (data->bus_ops->read(data->dev, reg, msg))
+#define CMR3000_SET(data, reg, val, msg) \
+     ((data)->bus_ops->write(data->dev, reg, val, msg))
+

Fixing it

quoted

+void cmr3000_exit(struct cmr3000_accl_data *data)
+{
+     free_irq(data->irq, data);
+     input_unregister_device(data->input_dev);

cleaner to unwind in reverse order of setup unless you have
a good reason and that should be commented.

Fixing it, and also fixing it on the cma3000

Match comment to name fo structure.

quoted

+ * struct cmr3000_i2c_platform_data - CMR3000 Platform data
+ * @fuzz_x: Noise on X Axis
+ * @fuzz_y: Noise on Y Axis
+ * @fuzz_z: Noise on Z Axis
+ * @mode: Operating mode
+ * @irq_level: Irq level
+ */
+struct cmr3000_platform_data {
+     int fuzz_x;
+     int fuzz_y;
+     int fuzz_z;
+     uint8_t irq_level;
+     uint8_t mode;
+     unsigned long irqflags;
+};

Fixing it, and also fixing it on the cma3000


Thanks!



-- 
Ricardo Ribalda
--
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