Re: [PATCH] bma023: Support for Bosch BMA023, BMA150 and SMB380 accelerometers

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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


[Index of Archives]     [Linux Media Devel]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Linux Wireless Networking]     [Linux Omap]

  Powered by Linux