Hello Jonathan On Tue, Oct 18, 2011 at 10:54, Jonathan Cameron <jic23@xxxxxxxxx> 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. >> +#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 >> +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. >> + * 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@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html