On Mon, 14 Feb 2011 17:20:13 +0530 Shubhrajyoti <shubhrajyoti@xxxxxx> wrote: > Hello, > Some comments inlined. Please trim stuff when you add comments inline like that. Firstly it saves bandwidth but more importantly it makes them easier to find ! > > obj-$(CONFIG_INPUT_SGI_BTNS) += sgi_btns.o > > +obj-$(CONFIG_INPUT_BMA023) += bma023.o > May consider alphabetical order oops - yes good point. I didn't check that when merging. > > + * bma023_xyz_read_reg - read the axes values > > + * @reg: register being updated > > + * @val: value to write > Didnât understand this That would be because the comments need updating. Will fix that now. > > + struct bma023_sensor *sensor = dev_get_drvdata(dev); > > + > > + pm_runtime_get_sync(dev); > Not a comment really , what does pm_runtime_get_sync(dev); do ? We need to be sure it's powered up when we read it via sysfs. > > + pm_runtime_put(dev); > > Also did you pm_runtime_put(dev); here? Didnt understand this did you > mean sync? No - as we don't care if it powers off immediately just that it gets around to it. > > + > > + return sprintf(buf, "(%d,%d,%d)\n", > > + sensor->data.x, sensor->data.y, sensor->data.z); > > +} > > +static DEVICE_ATTR(accel_data, S_IRUGO, bma023_show_xyz, NULL); > Since it is reported as an input device do you need the sysfs entry? Yes - the device is usable both as an input device (think about tilt based games) and for other purposes like orientation or drop detection. The sysfs interface covers the latter. > > + * Add an input device to the sensor. This will be used to report > > + * events from the sensor itself. > > + */ > > +static int bma023_register_input_device(struct bma023_sensor *sensor) > this could be devinit as well as called only from probe. Looks like it could will check > > + if (idev) > > + input_free_device(idev); > This check is there in the function Yep -- 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