Thanks for the review Alan.
On 2011-02-22, 17:27 +0000, Alan Cox wrote:
> > 1. exports 2 platform device attributes files in
> > /sys/devices/platform/intel_oaktrail/:
> > camera - Camera subsystem enabled: contains either 0 or 1. (rw)
> > touchscreen - Touchscreen subsystem enabled: contains either 0 or
> > 1. (ro)
>
> Is there a reason to expose these this way rather than just enabling
> them when the user opens the device ?
The reason I can think of is: this is oaktrail platform specific
code/interface.
Code sitting here seems more reasonable, comparing to the way we
provide interfaces for Camera driver to invoke when opened. Make
sense?
> > +static const struct name_mapping i2c_device_names[] = {
> > + { "SMO8601", "lsm303dlh_a" },
> > + #if 1
> > + { "COOLDEV", "bma150" },
> > + #endif
> > +
> > + /* other sensors not exposed by BIOS yet */
> > + #if 0
> > + { "SMO8601", "lsm303dlh_m" },
> > + { "xxx", "AK8975" },
> > + { "xxx", "L3G4200D" },
> > + { "xxx", "mpu3050" },
> > + #endif
>
> I would submit with the #if's cleaned up
>
>
> > +#if 1
> > + /* Add this bma150 temporary before BIOS exported it. */
> > + oaktrail_i2c_device_register("COOLDEV", 0x38);
> > +#endif
>
> Ditto
Yes of course, these code was just waiting for the new BIOS to be
removed. :)
> Looks basically sound to me otherwise
Thanks,
Kangkai
_______________________________________________
MeeGo-kernel mailing list
[email protected]
http://lists.meego.com/listinfo/meego-kernel
