Jonathan Cameron writes: > On 03/07/16 11:37, Jonathan Cameron wrote: >> On 29/06/16 16:41, Srinivas Pandruvada wrote: >>> On Wed, 2016-06-29 at 17:13 +0200, Steffen Trumtrar wrote: >>>> On Wed, May 04, 2016 at 10:55:56AM +0100, Jonathan Cameron wrote: >>>>> >>>>> On 01/05/16 21:02, Jonathan Cameron wrote: >>>>>> >>>>>> On 26/04/16 22:36, Srinivas Pandruvada wrote: >>>>>>> >>>>>>> On Tue, 2016-04-26 at 22:15 +0100, Jonathan Cameron wrote: >>>>>>>> >>>>>>>> >>>>>>>> On 26 April 2016 21:25:22 BST, Srinivas Pandruvada >>>>>>>> <srinivas.pandruva >>>>>>>> da@xxxxxxxxxxxxxxx> wrote: >>>>>>>>> >>>>>>>>> >>>>>>>>> On Mon, 2016-04-25 at 20:31 +0100, Jonathan Cameron wrote: >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> On 21/04/16 11:49, Steffen Trumtrar wrote: >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> The filter frequency and sample rate have a fixed >>>>>>>>>>> relationship. >>>>>>>>>>> Only the filter frequency is unique, however. >>>>>>>>>>> Currently the driver ignores the filter settings for 32 >>>>>>>>>>> Hz and >>>>>>>>>>> 64 Hz. >>>>>>>>>>> >>>>>>>>>>> This patch adds the necessary callbacks to be able to >>>>>>>>>>> configure >>>>>>>>>>> and read the filter setting from sysfs. >>>>>>>>>>> >>>>>>>>>>> Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix >>>>>>>>>>> .de> >>>>>>>>>> cc'd Srinivas as it's his driver... Looks superficially >>>>>>>>>> fine to >>>>>>>>>> me. >>>>>>>>>> >>>>>>>>>> Jonathan >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> --- >>>>>>>>>>> >>>>>>>>>>> Changing the sample rate will result in using the first >>>>>>>>>>> match >>>>>>>>>>> and therefore selecting the filter accordingly. Is this >>>>>>>>>>> a >>>>>>>>>>> misuse >>>>>>>>>>> of the ABI and should be handled differently or is this >>>>>>>>>>> okay? >>>>>>>>>>> >>>>>>>>> This is the reason they were omitted. Now you can't >>>>>>>>> uniquely set >>>>>>>>> 100Hz >>>>>>>>> sampling frequency. Depending on filter it will have >>>>>>>>> different >>>>>>>>> results. >>>>>>>>> >>>>>>>>> I think this needs some ABI level changes, where you >>>>>>>>> display >>>>>>>>> available >>>>>>>>> and allow to specify both ODR and Filter to uniquely >>>>>>>>> select. >>>>>>>> Unfortunately the moment the ABI allows for combined elements >>>>>>>> it >>>>>>>> becomes a >>>>>>>> nightmare for complexity. In some devices a single parameter >>>>>>>> change >>>>>>>> can >>>>>>>> change everything else. There are no simple rules >>>>>>>> unfortunately. >>>>>>>> >>>>>>>> The way we avoid this being a problem is that we very >>>>>>>> deliberately >>>>>>>> allow any ABI element >>>>>>>> to be able to result in a change in any other. This includes >>>>>>>> changing the >>>>>>>> available values list as here. It might be slightly nicer to >>>>>>>> jump to >>>>>>>> the nearest >>>>>>>> available option though. >>>>>>>> >>>>>>>> An alternative would be to have an interface to fake such >>>>>>>> changes >>>>>>>> then >>>>>>>> apply them atomically if possible. >>>>>>>> That level of complexity just does seem warranted here and >>>>>>>> would >>>>>>>> still need userspace to check valid ranges as it >>>>>>>> pretends to change the state. Hence no real gain.... >>>>>>>> >>>>>>> I think we should add some documentation for this driver about >>>>>>> this. >>>>>>> They should rather change filer rather than sampling freq to >>>>>>> have >>>>>>> unique setting. >>>>>> whilst that would get around the problem, people are going to be >>>>>> expecting >>>>>> to have explicit control of sampling frequency if they see it is >>>>>> variable for >>>>>> the part... >>>>>> >>>>>> Tricky unfortunately. >>>>> So Srinivas, I'm in favour of the patch as it stands. Have I >>>>> convinced you? >>>> So, any conclusion ? :-) >>> Sorry, I missed this. I am fine with this change. >> Cool. >> >> Applied to the togreg branch of iio.git and initially pushed out as >> testing for the autobuilders to play cricket with it... >> >> Thanks, >> >> Jonathan > I completely failed to notice that data->dev got dropped in the meantime > fixed up and pushed out. Please sanity check the testing branch of iio.git. \o/ Looks good to me. Thanks, Steffen -- Pengutronix e.K. | Steffen Trumtrar | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | -- To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
