Skip to contentSkip to conversation

iio: add channel type for frequency

PATCH In review David Lechner 3 participants 5 messages Jul 1, 2018 – Jul 2, 2018 IIO

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 iio: add channel type for frequency

This adds a new type for frequency to the IIO channel type enumeration.

Units are in Hz.

---
 drivers/iio/industrialio-core.c | 1 +
 include/uapi/linux/iio/types.h  | 1 +
 tools/iio/iio_event_monitor.c   | 1 +
 3 files changed, 3 insertions(+)
drivers/iio/industrialio-core.c +1 -0
index 19bdf3d2962a..f3c2c9e4b997 100644
@@ -85,6 +85,7 @@ static const char * const iio_chan_type_name_spec[] = {
 	[IIO_COUNT] = "count",
 	[IIO_INDEX] = "index",
 	[IIO_GRAVITY]  = "gravity",
+	[IIO_FREQUENCY] = "frequency",
 };
 
 static const char * const iio_modifier_names[] = {
include/uapi/linux/iio/types.h +1 -0
index 4213cdf88e3c..9fee86e2046f 100644
@@ -44,6 +44,7 @@ enum iio_chan_type {
 	IIO_COUNT,
 	IIO_INDEX,
 	IIO_GRAVITY,
+	IIO_FREQUENCY,
 };
 
 enum iio_modifier {
tools/iio/iio_event_monitor.c +1 -0
index b61245e1181d..c3ef20057d52 100644
@@ -58,6 +58,7 @@ static const char * const iio_chan_type_name_spec[] = {
 	[IIO_PH] = "ph",
 	[IIO_UVINDEX] = "uvindex",
 	[IIO_GRAVITY] = "gravity",
+	[IIO_FREQUENCY] = "frequency",
 };
 
 static const char * const iio_ev_type_text[] = {
Signature · 1 line
2.17.1
#1 lore raw
Quoted 4 lines · David Lechner

This adds a new type for frequency to the IIO channel type enumeration.

Units are in Hz.

Documentation?

We already have the altvoltage channel type with the frequency attribute. Difficult to say if there are any overlaps without documentation on how this new attribute is supposed to be used.

Quoted 44 lines
---
 drivers/iio/industrialio-core.c | 1 +
 include/uapi/linux/iio/types.h  | 1 +
 tools/iio/iio_event_monitor.c   | 1 +
 3 files changed, 3 insertions(+)
drivers/iio/industrialio-core.c +1 -0
index 19bdf3d2962a..f3c2c9e4b997 100644
@@ -85,6 +85,7 @@ static const char * const iio_chan_type_name_spec[] = {
 	[IIO_COUNT] = "count",
 	[IIO_INDEX] = "index",
 	[IIO_GRAVITY]  = "gravity",
+	[IIO_FREQUENCY] = "frequency",
 };
 
 static const char * const iio_modifier_names[] = {
include/uapi/linux/iio/types.h +1 -0
index 4213cdf88e3c..9fee86e2046f 100644
@@ -44,6 +44,7 @@ enum iio_chan_type {
 	IIO_COUNT,
 	IIO_INDEX,
 	IIO_GRAVITY,
+	IIO_FREQUENCY,
 };
 
 enum iio_modifier {
tools/iio/iio_event_monitor.c +1 -0
index b61245e1181d..c3ef20057d52 100644
@@ -58,6 +58,7 @@ static const char * const iio_chan_type_name_spec[] = {
 	[IIO_PH] = "ph",
 	[IIO_UVINDEX] = "uvindex",
 	[IIO_GRAVITY] = "gravity",
+	[IIO_FREQUENCY] = "frequency",
 };
 
 static const char * const iio_ev_type_text[] = {
#2 lore raw
Quoted 7 lines · Lars-Peter Clausen
Quoted 4 lines · David Lechner

This adds a new type for frequency to the IIO channel type enumeration.

Units are in Hz.

Documentation?

I take it that you mean Documentation/ABI/testing/sysfs-bus-iio? Or somewhere else too?

Quoted 4 lines

We already have the altvoltage channel type with the frequency attribute. Difficult to say if there are any overlaps without documentation on how this new attribute is supposed to be used.

I'm basically trying to implement a quadrature encoder in iio. I want to be able to use it in-kernel to get a rotational speed value for a motor.

The motors (and encoder wheels) are hot-swapable, so we don't know how many counts from the quadrature encoder equals one rotation because it depends on which motor is being used. So using IIO_ANGL_VEL doesn't work for this case.

It seems to me that the proper generic unit for "speed" from the quadrature encoder would be counts per second, hence the suggestion of a frequency unit. I'm not sure if voltage frequency works here since a "count" on a quadrature encoder is derived from two different voltage signals (and may vary depending on how the encoder determines what one count it).

Also, unrelated to my quadrature encoder project, I was thinking that frequency counters would use this unit as well (although the voltage alt makes more sense for this case).

There are also sound sensors that measure frequency that could use this unit.

Quoted 46 lines
Quoted 45 lines
---
  drivers/iio/industrialio-core.c | 1 +
  include/uapi/linux/iio/types.h  | 1 +
  tools/iio/iio_event_monitor.c   | 1 +
  3 files changed, 3 insertions(+)
drivers/iio/industrialio-core.c +1 -0
index 19bdf3d2962a..f3c2c9e4b997 100644
@@ -85,6 +85,7 @@ static const char * const iio_chan_type_name_spec[] = {
  	[IIO_COUNT] = "count",
  	[IIO_INDEX] = "index",
  	[IIO_GRAVITY]  = "gravity",
+	[IIO_FREQUENCY] = "frequency",
  };
 
  static const char * const iio_modifier_names[] = {
include/uapi/linux/iio/types.h +1 -0
index 4213cdf88e3c..9fee86e2046f 100644
@@ -44,6 +44,7 @@ enum iio_chan_type {
  	IIO_COUNT,
  	IIO_INDEX,
  	IIO_GRAVITY,
+	IIO_FREQUENCY,
  };
 
  enum iio_modifier {
tools/iio/iio_event_monitor.c +1 -0
index b61245e1181d..c3ef20057d52 100644
@@ -58,6 +58,7 @@ static const char * const iio_chan_type_name_spec[] = {
  	[IIO_PH] = "ph",
  	[IIO_UVINDEX] = "uvindex",
  	[IIO_GRAVITY] = "gravity",
+	[IIO_FREQUENCY] = "frequency",
  };
 
  static const char * const iio_ev_type_text[] = {
#3 lore raw
Quoted 11 lines · On Sun, 1 Jul 2018 17:24:34 -0500 David Lechner <[email protected]> wrote:
Quoted 7 lines · Lars-Peter Clausen
Quoted 4 lines · David Lechner

This adds a new type for frequency to the IIO channel type enumeration.

Units are in Hz.

Documentation?

I take it that you mean Documentation/ABI/testing/sysfs-bus-iio? Or somewhere else too?

Yes.

Quoted 8 lines
Quoted 5 lines

We already have the altvoltage channel type with the frequency attribute. Difficult to say if there are any overlaps without documentation on how this new attribute is supposed to be used.

I'm basically trying to implement a quadrature encoder in iio. I want to be able to use it in-kernel to get a rotational speed value for a motor.

Have you seen the counters subsystem work that is pretty much ready to merge?

https://lkml.org/lkml/2018/6/21/659

Quoted 4 lines

The motors (and encoder wheels) are hot-swapable, so we don't know how many counts from the quadrature encoder equals one rotation because it depends on which motor is being used. So using IIO_ANGL_VEL doesn't work for this case.

Hmm. This always ugly when we have hotswappable external devices. Ideal is to describe them with device tree or similar none the less as this stuff isn't really something userspace should have to figure out.

Quoted 3 lines

It seems to me that the proper generic unit for "speed" from the quadrature encoder would be counts per second, hence the suggestion of a frequency unit.

It is probably not that clear cut, as what is it frequency of? All depends on what mode the quadrature counter is working in x4 or x1 for simplest options.

Quoted 9 lines

I'm not sure if voltage frequency works here since a "count" on a quadrature encoder is derived from two different voltage signals (and may vary depending on how the encoder determines what one count it).

Also, unrelated to my quadrature encoder project, I was thinking that frequency counters would use this unit as well (although the voltage alt makes more sense for this case).

There are also sound sensors that measure frequency that could use this unit.

I have not fundamental issue with having a frequency channel, but only if we have a clear cut use case. I suspect the right option here is to look at what extensions are needed to William's counter subsystem instead of in IIO (which frankly failed to stretch far enough to support counters well).

Jonathan

Quoted 52 lines
Quoted 44 lines
Quoted 44 lines
---
  drivers/iio/industrialio-core.c | 1 +
  include/uapi/linux/iio/types.h  | 1 +
  tools/iio/iio_event_monitor.c   | 1 +
  3 files changed, 3 insertions(+)
drivers/iio/industrialio-core.c +1 -0
index 19bdf3d2962a..f3c2c9e4b997 100644
@@ -85,6 +85,7 @@ static const char * const iio_chan_type_name_spec[] = {
  	[IIO_COUNT] = "count",
  	[IIO_INDEX] = "index",
  	[IIO_GRAVITY]  = "gravity",
+	[IIO_FREQUENCY] = "frequency",
  };
 
  static const char * const iio_modifier_names[] = {
include/uapi/linux/iio/types.h +1 -0
index 4213cdf88e3c..9fee86e2046f 100644
@@ -44,6 +44,7 @@ enum iio_chan_type {
  	IIO_COUNT,
  	IIO_INDEX,
  	IIO_GRAVITY,
+	IIO_FREQUENCY,
  };
 
  enum iio_modifier {
tools/iio/iio_event_monitor.c +1 -0
index b61245e1181d..c3ef20057d52 100644
@@ -58,6 +58,7 @@ static const char * const iio_chan_type_name_spec[] = {
  	[IIO_PH] = "ph",
  	[IIO_UVINDEX] = "uvindex",
  	[IIO_GRAVITY] = "gravity",
+	[IIO_FREQUENCY] = "frequency",
  };
 
Quoted 3 lines

tools/iio/iio_event_monitor.c static const char * const iio_chan_type_name_spec[] = { · line 64

Quoted 2 lines
tools/iio/iio_event_monitor.c +0 -0
  static const char * const iio_ev_type_text[] = {

--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#4 lore raw
Quoted 19 lines · Jonathan Cameron
Quoted 11 lines · On Sun, 1 Jul 2018 17:24:34 -0500 David Lechner <[email protected]> wrote:
Quoted 7 lines · Lars-Peter Clausen
Quoted 4 lines · David Lechner

This adds a new type for frequency to the IIO channel type enumeration.

Units are in Hz.

Documentation?

I take it that you mean Documentation/ABI/testing/sysfs-bus-iio? Or somewhere else too?

Yes.

Quoted 2 lines
Quoted 2 lines
Quoted 8 lines
Quoted 6 lines
Quoted 3 lines

We already have the altvoltage channel type with the frequency attribute. Difficult to say if there are any overlaps without documentation on how this new attribute is supposed to be used.

I'm basically trying to implement a quadrature encoder in iio. I want to be able to use it in-kernel to get a rotational speed value for a motor.

Have you seen the counters subsystem work that is pretty much ready to merge?

I hadn't seen that. Thanks. It sounds like the way to go since I am dealing with a quadrature encoder.

Quoted 28 lines

https://lkml.org/lkml/2018/6/21/659

Quoted 4 lines

The motors (and encoder wheels) are hot-swapable, so we don't know how many counts from the quadrature encoder equals one rotation because it depends on which motor is being used. So using IIO_ANGL_VEL doesn't work for this case.

Hmm. This always ugly when we have hotswappable external devices. Ideal is to describe them with device tree or similar none the less as this stuff isn't really something userspace should have to figure out.

Quoted 3 lines

It seems to me that the proper generic unit for "speed" from the quadrature encoder would be counts per second, hence the suggestion of a frequency unit.

It is probably not that clear cut, as what is it frequency of? All depends on what mode the quadrature counter is working in x4 or x1 for simplest options.

Quoted 8 lines

I'm not sure if voltage frequency works here since a "count" on a quadrature encoder is derived from two different voltage signals (and may vary depending on how the encoder determines what one count it).

Also, unrelated to my quadrature encoder project, I was thinking that frequency counters would use this unit as well (although the voltage alt makes more sense for this case).

Quoted 8 lines
Quoted 1 line

There are also sound sensors that measure frequency that could use this unit.

I have not fundamental issue with having a frequency channel, but only if we have a clear cut use case. I suspect the right option here is to look at what extensions are needed to William's counter subsystem instead of in IIO (which frankly failed to stretch far enough to support counters well).

Agreed. Let's drop this patch.