Thanks for your input Jonathan! > >>> + 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. True. I'll add a comment on how to get the valid values in v4. > > 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/ An extra attribute does sound a bit over-kill to me. Would it be acceptable to keep it the way it is? > >>> +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 :) You are probably right. I'll add it. > >> 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). Ok, I will add irq handling for v4. It will be threshold triggered, meaning I will also add the threshold parameters from bma023. If no .irq is given it will use polldev. Does that work for you Alan? Regarding the sysfs paramters - how about removing them and use platform data only? 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
