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@xxxxxxxxxxxxxx> > >>>>> 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 ? :-) Best regards, Steffen -- Pengutronix e.K. | | 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
