On 07/15/11 09:23, Eric Andersson wrote: >>> + values represent the range given as +/- G. >>> + Possible values are: 2, 4, 8. >>> + >>> + Reading: returns the current acceleration range. >>> + >>> + Writing: sets a new acceleration range. >> >> Same comment as last time - these values are not discoverable so it >> should set the nearest bigger range. > > And same answer as last time - The values can be retreived by doing a: > cat range. This whole idea comes from a review comment that Jonathan Cameron > gave on your bma023 driver [1]. Needs documenting. Right now there is no mention of that here. > If we could agree on how these values should be represented I will be glad > to fix it! Dmitry, any preferences? > > [1] http://www.spinics.net/lists/linux-input/msg14271.html > >>> + for (i = 0, ret = 0; i < ARRAY_SIZE(bw_val); i++) >>> + ret += sprintf(buf + ret, >>> + (bw_val[i].reg == bw) ? "[%d] " : "%d ", >>> + bw_val[i].value); >> >> sysfs nodes should really be single values > > Once again it's a previous comment from Jonathan [1]. Please specify how > you want this and I'll fix it. Yes in general to the single value, but this is a pretty common syntax for choosing one of a set. Got to indicate it somehow and it's either this or a list of available ones in a separate attribute. There was an LWN article on this a while back that lays out the options very clearly http://lwn.net/Articles/378884/ > >>> +static const struct i2c_device_id bma150_id[] = { >>> + { "bma150", 0 }, >>> + { "smb380", 0 }, >> >> bma023 ? > > As mentioned earlier bma023 is not an official product name from Bosch Sensortec. Hence, > there will be no datasheet available or references from Bosch Sensortec. IMHO it would be > confusing to add it. Just ignore the marketing and add it anyway :) > >> It also doesn't expose the thresholds or support interrupts which the >> one I posted did. > > We were told by Bosch S that the interrupt is triggered every 333 us (data ready) which would > pretty much flood the irq handler. Somewhat quick, but maybe still worth having... Just use a one shot threaded interrupt and it will at least run safely even if it isn't keeping up with all of them. Alan, do you have a device that has frequency control on chip? (very unusual not to see this on an accelerometer). > However, if this is stopping the patch we can add it. Would it be acceptable to have a driver > that supports both a polling and irq driven setup, e.g. by checking if the .irq is non zero? Yes: common situation. See the recent kionix driver for an example. You definitely want the theshold stuff from Alan's driver though. > >> So while its better it still seems incomplete - its certainly nowhere >> near where it can replace the one I posted months ago and doesn't seem >> to >> be making any headway. >> >> Dmitry shall I repost the intel one - at this point I think it would >> be a >> better starting point as it supports more features, interrupts and the >> like although it's not perfect either. > > My hope is that we can sort out the differences before I post v4. Dmitry, if you have time > could you please give an objective review of the patch? > > Best regards, > Eric > > http://www.unixphere.com > -- > 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 > -- 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