Re: [PATCH v5 3/4] iio: trigger: Introduce IIO hrtimer based trigger

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 05/05/2015 04:51 PM, Jonathan Cameron wrote:


On 4 May 2015 20:54:08 GMT+01:00, Lars-Peter Clausen <lars@xxxxxxxxxx> wrote:
On 05/04/2015 12:50 PM, Daniel Baluta wrote:
[...]
+IIO_HRTIMER_INFO_ATTR(sampling_frequency, S_IRUGO | S_IWUSR,
+		      iio_hrtimer_info_show_sampling_frequency,
+		      iio_hrtimer_info_store_sampling_frequency);

I wonder if the sampling frequency should be configurable the regular
IIO
API, just like any other IIO device. But things like min/max sampling
frequency should be configured in configfs.
Would have to be in the trigger dir rather than device... Makes sense to put it there.
Limits on it here seem like a sensible idea.

But then each trigger will have sampling_frequency right? This is not what we want.


[...]
+#endif /* CONFIGFS_FS */
+
[...]
+static struct iio_sw_trigger *iio_trig_hrtimer_probe(const char
*name)
+{
[...]
+#ifdef CONFIG_CONFIGFS_FS
+	config_group_init_type_name(&trig_info->swt.group, name,
+				    &iio_hrtimer_type);
+#endif

This should probably have a helper function in the sw trigger core,
that
gets stubbed out when CONFIG_FS is disabled. Otherwise we'll see the
same
#ifdef in every software trigger driver.
[...]

Agree with this. Will fix.

+}
+
+static int iio_trig_hrtimer_remove(struct iio_sw_trigger *swt)
+{
+	struct iio_hrtimer_info *trig_info;
+
+	trig_info = iio_trigger_get_drvdata(swt->trigger);
+
+	hrtimer_cancel(&trig_info->timer);
+
+	iio_trigger_unregister(swt->trigger);
+	iio_trigger_free(swt->trigger);

There is a bit of a race condition here. hrtimer_cancel() should be
called
between unregister and free, otherwise it might be re-armed before it
is
unregistered.

So this can be re-armed only if the buffer is re-enabled between hrtimer_cancel and iio_trigger_unregister :). I'm trying to understand how the race can happen.



+	kfree(trig_info);
+
+	return 0;
+}
+
+struct iio_sw_trigger_ops iio_trig_hrtimer_ops = {

const

Agree.

+	.probe		= iio_trig_hrtimer_probe,
+	.remove		= iio_trig_hrtimer_remove,
+};
[...]
--
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




[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Input]     [Linux Kernel]     [Linux SCSI]     [X.org]

  Powered by Linux