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. thanks, Jonathan >> >> Thanks, >> Srinivas >>> >>> Best regards, >>> Steffen >>> >> -- >> 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 >> > -- 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
