* [PATCHv2 0/3] IIO: Add support for compass north usage attribute
@ 2014-06-02 23:14 Reyad Attiyat
[not found] ` <1401750890-31854-1-git-send-email-reyad.attiyat-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2014-06-02 23:14 ` [PATCHv2 3/3] IIO: hid-sensor-magn-3d: Add in support " Reyad Attiyat
0 siblings, 2 replies; 23+ messages in thread
From: Reyad Attiyat @ 2014-06-02 23:14 UTC (permalink / raw)
To: linux-kernel, linux-iio, jic23, srinivas.pandruvada, linux-input,
jkosina
Cc: Reyad Attiyat
The following patches allow the hid-sensor-magn-3d driver to handle
the north usage attribute. It allows the driver to dynamically
create IIO channels depending on which HID reports are found.
Reyad Attiyat (3):
IIO: Documentation: Add north attribute to ABI docs
IIO: Add iio_chan modifier for True/Magnetic North HID usages
IIO: hid-sensor-magn-3d: Add in support for True/Magnetic North HID
usages
Documentation/ABI/testing/sysfs-bus-iio | 76 ++++++++
drivers/iio/industrialio-core.c | 4 +
drivers/iio/magnetometer/hid-sensor-magn-3d.c | 271 +++++++++++++++++---------
include/linux/iio/types.h | 4 +
4 files changed, 261 insertions(+), 94 deletions(-)
--
1.9.3
^ permalink raw reply [flat|nested] 23+ messages in thread[parent not found: <1401750890-31854-1-git-send-email-reyad.attiyat-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]
* [PATCHv2 1/3] IIO: Documentation: Add north attribute to ABI docs 2014-06-02 23:14 [PATCHv2 0/3] IIO: Add support for compass north usage attribute Reyad Attiyat @ 2014-06-02 23:14 ` Reyad Attiyat 2014-06-02 23:14 ` [PATCHv2 3/3] IIO: hid-sensor-magn-3d: Add in support " Reyad Attiyat 1 sibling, 0 replies; 23+ messages in thread From: Reyad Attiyat @ 2014-06-02 23:14 UTC (permalink / raw) To: linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-iio-u79uwXL29TY76Z2rM5mHXA, jic23-DgEjT+Ai2ygdnm+yROfE0A, srinivas.pandruvada-VuQAYsv1563Yd54FQh9/CA, linux-input-u79uwXL29TY76Z2rM5mHXA, jkosina-AlSwsSmVLrQ Cc: Reyad Attiyat Updated ABI documentation to inculde compass north usages. Signed-off-by: Reyad Attiyat <reyad.attiyat-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> --- Documentation/ABI/testing/sysfs-bus-iio | 76 +++++++++++++++++++++++++++++++++ 1 file changed, 76 insertions(+) diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio index 6e02c50..251c2bb 100644 --- a/Documentation/ABI/testing/sysfs-bus-iio +++ b/Documentation/ABI/testing/sysfs-bus-iio @@ -165,6 +165,10 @@ Description: What: /sys/bus/iio/devices/iio:deviceX/in_magn_x_raw What: /sys/bus/iio/devices/iio:deviceX/in_magn_y_raw What: /sys/bus/iio/devices/iio:deviceX/in_magn_z_raw +What: /sys/bus/iio/devices/iio:deviceX/in_north_magnetic_raw +What: /sys/bus/iio/devices/iio:deviceX/in_north_true_raw +What: /sys/bus/iio/devices/iio:deviceX/in_north_magnetic_tilt_comp_raw +What: /sys/bus/iio/devices/iio:deviceX/in_north_true_tilt_comp_raw KernelVersion: 2.6.35 Contact: linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org Description: @@ -249,6 +253,10 @@ What: /sys/bus/iio/devices/iio:deviceX/in_magn_scale What: /sys/bus/iio/devices/iio:deviceX/in_magn_x_scale What: /sys/bus/iio/devices/iio:deviceX/in_magn_y_scale What: /sys/bus/iio/devices/iio:deviceX/in_magn_z_scale +What: /sys/bus/iio/devices/iio:deviceX/in_north_magnetic_scale +What: /sys/bus/iio/devices/iio:deviceX/in_north_true_scale +What: /sys/bus/iio/devices/iio:deviceX/in_north_magnetic_tilt_comp_scale +What: /sys/bus/iio/devices/iio:deviceX/in_north_true_tilt_comp_scale What: /sys/bus/iio/devices/iio:deviceX/in_pressureY_scale What: /sys/bus/iio/devices/iio:deviceX/in_pressure_scale KernelVersion: 2.6.35 @@ -436,6 +444,14 @@ What: /sys/.../iio:deviceX/events/in_magn_y_thresh_rising_en What: /sys/.../iio:deviceX/events/in_magn_y_thresh_falling_en What: /sys/.../iio:deviceX/events/in_magn_z_thresh_rising_en What: /sys/.../iio:deviceX/events/in_magn_z_thresh_falling_en +What: /sys/.../iio:deviceX/events/in_north_magnetic_thresh_rising_en +What: /sys/.../iio:deviceX/events/in_north_magnetic_thresh_falling_en +What: /sys/.../iio:deviceX/events/in_north_true_thresh_rising_en +What: /sys/.../iio:deviceX/events/in_north_true_thresh_falling_en +What: /sys/.../iio:deviceX/events/in_north_magnetic_tilt_comp_thresh_rising_en +What: /sys/.../iio:deviceX/events/in_north_magnetic_tilt_comp_thresh_falling_en +What: /sys/.../iio:deviceX/events/in_north_true_tilt_comp_thresh_rising_en +What: /sys/.../iio:deviceX/events/in_north_true_tilt_comp_thresh_falling_en What: /sys/.../iio:deviceX/events/in_voltageY_supply_thresh_rising_en What: /sys/.../iio:deviceX/events/in_voltageY_supply_thresh_falling_en What: /sys/.../iio:deviceX/events/in_voltageY_thresh_rising_en @@ -481,6 +497,14 @@ What: /sys/.../iio:deviceX/events/in_magn_y_roc_rising_en What: /sys/.../iio:deviceX/events/in_magn_y_roc_falling_en What: /sys/.../iio:deviceX/events/in_magn_z_roc_rising_en What: /sys/.../iio:deviceX/events/in_magn_z_roc_falling_en +What: /sys/.../iio:deviceX/events/in_north_magnetic_roc_rising_en +What: /sys/.../iio:deviceX/events/in_north_magnetic_roc_falling_en +What: /sys/.../iio:deviceX/events/in_north_true_roc_rising_en +What: /sys/.../iio:deviceX/events/in_north_true_roc_falling_en +What: /sys/.../iio:deviceX/events/in_north_magnetic_tilt_comp_roc_rising_en +What: /sys/.../iio:deviceX/events/in_north_magnetic_tilt_comp_roc_falling_en +What: /sys/.../iio:deviceX/events/in_north_true_tilt_comp_roc_rising_en +What: /sys/.../iio:deviceX/events/in_north_true_tilt_comp_roc_falling_en What: /sys/.../iio:deviceX/events/in_voltageY_supply_roc_rising_en What: /sys/.../iio:deviceX/events/in_voltageY_supply_roc_falling_en What: /sys/.../iio:deviceX/events/in_voltageY_roc_rising_en @@ -527,6 +551,14 @@ What: /sys/.../events/in_magn_y_raw_thresh_rising_value What: /sys/.../events/in_magn_y_raw_thresh_falling_value What: /sys/.../events/in_magn_z_raw_thresh_rising_value What: /sys/.../events/in_magn_z_raw_thresh_falling_value +What: /sys/.../events/in_north_magnetic_raw_thresh_rising_value +What: /sys/.../events/in_north_magnetic_raw_thresh_falling_value +What: /sys/.../events/in_north_true_raw_thresh_rising_value +What: /sys/.../events/in_north_true_raw_thresh_falling_value +What: /sys/.../events/in_north_magnetic_tilt_comp_raw_thresh_rising_value +What: /sys/.../events/in_north_magnetic_tilt_comp_raw_thresh_falling_value +What: /sys/.../events/in_north_true_tilt_comp_raw_thresh_rising_value +What: /sys/.../events/in_north_true_tilt_comp_raw_thresh_falling_value What: /sys/.../events/in_voltageY_supply_raw_thresh_rising_value What: /sys/.../events/in_voltageY_supply_raw_thresh_falling_value What: /sys/.../events/in_voltageY_raw_thresh_rising_value @@ -577,6 +609,18 @@ What: /sys/.../events/in_magn_y_thresh_either_hysteresis What: /sys/.../events/in_magn_z_thresh_rising_hysteresis What: /sys/.../events/in_magn_z_thresh_falling_hysteresis What: /sys/.../events/in_magn_z_thresh_either_hysteresis +What: /sys/.../events/in_north_magnetic_thresh_rising_hysteresis +What: /sys/.../events/in_north_magnetic_thresh_falling_hysteresis +What: /sys/.../events/in_north_magnetic_thresh_either_hysteresis +What: /sys/.../events/in_north_true_thresh_rising_hysteresis +What: /sys/.../events/in_north_true_thresh_falling_hysteresis +What: /sys/.../events/in_north_true_thresh_either_hysteresis +What: /sys/.../events/in_north_magnetic_tilt_comp_thresh_rising_hysteresis +What: /sys/.../events/in_north_magnetic_tilt_comp_thresh_falling_hysteresis +What: /sys/.../events/in_north_magnetic_tilt_comp_thresh_either_hysteresis +What: /sys/.../events/in_north_true_tilt_comp_thresh_rising_hysteresis +What: /sys/.../events/in_north_true_tilt_comp_thresh_falling_hysteresis +What: /sys/.../events/in_north_true_tilt_comp_thresh_either_hysteresis What: /sys/.../events/in_voltageY_thresh_rising_hysteresis What: /sys/.../events/in_voltageY_thresh_falling_hysteresis What: /sys/.../events/in_voltageY_thresh_either_hysteresis @@ -624,6 +668,14 @@ What: /sys/.../events/in_magn_y_raw_roc_rising_value What: /sys/.../events/in_magn_y_raw_roc_falling_value What: /sys/.../events/in_magn_z_raw_roc_rising_value What: /sys/.../events/in_magn_z_raw_roc_falling_value +What: /sys/.../events/in_north_magnetic_raw_roc_rising_value +What: /sys/.../events/in_north_magnetic_raw_roc_falling_value +What: /sys/.../events/in_north_true_raw_roc_rising_value +What: /sys/.../events/in_north_true_raw_roc_falling_value +What: /sys/.../events/in_north_magnetic_tilt_comp_raw_roc_rising_value +What: /sys/.../events/in_north_magnetic_tilt_comp_raw_roc_falling_value +What: /sys/.../events/in_north_true_tilt_comp_raw_roc_rising_value +What: /sys/.../events/in_north_true_tilt_comp_raw_roc_falling_value What: /sys/.../events/in_voltageY_supply_raw_roc_rising_value What: /sys/.../events/in_voltageY_supply_raw_roc_falling_value What: /sys/.../events/in_voltageY_raw_roc_rising_value @@ -679,6 +731,22 @@ What: /sys/.../events/in_magn_z_thresh_rising_period What: /sys/.../events/in_magn_z_thresh_falling_period What: /sys/.../events/in_magn_z_roc_rising_period What: /sys/.../events/in_magn_z_roc_falling_period +What: /sys/.../events/in_north_magnetic_thresh_rising_period +What: /sys/.../events/in_north_magnetic_thresh_falling_period +What: /sys/.../events/in_north_magnetic_roc_rising_period +What: /sys/.../events/in_north_magnetic_roc_falling_period +What: /sys/.../events/in_north_true_thresh_rising_period +What: /sys/.../events/in_north_true_thresh_falling_period +What: /sys/.../events/in_north_true_roc_rising_period +What: /sys/.../events/in_north_true_roc_falling_period +What: /sys/.../events/in_north_magnetic_tilt_comp_thresh_rising_period +What: /sys/.../events/in_north_magnetic_tilt_comp_thresh_falling_period +What: /sys/.../events/in_north_magnetic_tilt_comp_roc_rising_period +What: /sys/.../events/in_north_magnetic_tilt_comp_roc_falling_period +What: /sys/.../events/in_north_true_tilt_comp_thresh_rising_period +What: /sys/.../events/in_north_true_tilt_comp_thresh_falling_period +What: /sys/.../events/in_north_true_tilt_comp_roc_rising_period +What: /sys/.../events/in_north_true_tilt_comp_roc_falling_period What: /sys/.../events/in_voltageY_supply_thresh_rising_period What: /sys/.../events/in_voltageY_supply_thresh_falling_period What: /sys/.../events/in_voltageY_supply_roc_rising_period @@ -776,6 +844,10 @@ What: /sys/.../iio:deviceX/scan_elements/in_anglvel_z_en What: /sys/.../iio:deviceX/scan_elements/in_magn_x_en What: /sys/.../iio:deviceX/scan_elements/in_magn_y_en What: /sys/.../iio:deviceX/scan_elements/in_magn_z_en +What: /sys/.../iio:deviceX/scan_elements/in_north_magnetic_en +What: /sys/.../iio:deviceX/scan_elements/in_north_true_en +What: /sys/.../iio:deviceX/scan_elements/in_north_magnetic_tilt_comp_en +What: /sys/.../iio:deviceX/scan_elements/in_north_true_tilt_comp_en What: /sys/.../iio:deviceX/scan_elements/in_timestamp_en What: /sys/.../iio:deviceX/scan_elements/in_voltageY_supply_en What: /sys/.../iio:deviceX/scan_elements/in_voltageY_en @@ -840,6 +912,10 @@ What: /sys/.../iio:deviceX/scan_elements/in_anglvel_z_index What: /sys/.../iio:deviceX/scan_elements/in_magn_x_index What: /sys/.../iio:deviceX/scan_elements/in_magn_y_index What: /sys/.../iio:deviceX/scan_elements/in_magn_z_index +What: /sys/.../iio:deviceX/scan_elements/in_north_magnetic_index +What: /sys/.../iio:deviceX/scan_elements/in_north_true_index +What: /sys/.../iio:deviceX/scan_elements/in_north_magnetic_tilt_comp_index +What: /sys/.../iio:deviceX/scan_elements/in_north_true_tilt_comp_index What: /sys/.../iio:deviceX/scan_elements/in_incli_x_index What: /sys/.../iio:deviceX/scan_elements/in_incli_y_index What: /sys/.../iio:deviceX/scan_elements/in_timestamp_index -- 1.9.3 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCHv2 1/3] IIO: Documentation: Add north attribute to ABI docs @ 2014-06-02 23:14 ` Reyad Attiyat 0 siblings, 0 replies; 23+ messages in thread From: Reyad Attiyat @ 2014-06-02 23:14 UTC (permalink / raw) To: linux-kernel, linux-iio, jic23, srinivas.pandruvada, linux-input, jkosina Cc: Reyad Attiyat Updated ABI documentation to inculde compass north usages. Signed-off-by: Reyad Attiyat <reyad.attiyat@gmail.com> --- Documentation/ABI/testing/sysfs-bus-iio | 76 +++++++++++++++++++++++++++++++++ 1 file changed, 76 insertions(+) diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio index 6e02c50..251c2bb 100644 --- a/Documentation/ABI/testing/sysfs-bus-iio +++ b/Documentation/ABI/testing/sysfs-bus-iio @@ -165,6 +165,10 @@ Description: What: /sys/bus/iio/devices/iio:deviceX/in_magn_x_raw What: /sys/bus/iio/devices/iio:deviceX/in_magn_y_raw What: /sys/bus/iio/devices/iio:deviceX/in_magn_z_raw +What: /sys/bus/iio/devices/iio:deviceX/in_north_magnetic_raw +What: /sys/bus/iio/devices/iio:deviceX/in_north_true_raw +What: /sys/bus/iio/devices/iio:deviceX/in_north_magnetic_tilt_comp_raw +What: /sys/bus/iio/devices/iio:deviceX/in_north_true_tilt_comp_raw KernelVersion: 2.6.35 Contact: linux-iio@vger.kernel.org Description: @@ -249,6 +253,10 @@ What: /sys/bus/iio/devices/iio:deviceX/in_magn_scale What: /sys/bus/iio/devices/iio:deviceX/in_magn_x_scale What: /sys/bus/iio/devices/iio:deviceX/in_magn_y_scale What: /sys/bus/iio/devices/iio:deviceX/in_magn_z_scale +What: /sys/bus/iio/devices/iio:deviceX/in_north_magnetic_scale +What: /sys/bus/iio/devices/iio:deviceX/in_north_true_scale +What: /sys/bus/iio/devices/iio:deviceX/in_north_magnetic_tilt_comp_scale +What: /sys/bus/iio/devices/iio:deviceX/in_north_true_tilt_comp_scale What: /sys/bus/iio/devices/iio:deviceX/in_pressureY_scale What: /sys/bus/iio/devices/iio:deviceX/in_pressure_scale KernelVersion: 2.6.35 @@ -436,6 +444,14 @@ What: /sys/.../iio:deviceX/events/in_magn_y_thresh_rising_en What: /sys/.../iio:deviceX/events/in_magn_y_thresh_falling_en What: /sys/.../iio:deviceX/events/in_magn_z_thresh_rising_en What: /sys/.../iio:deviceX/events/in_magn_z_thresh_falling_en +What: /sys/.../iio:deviceX/events/in_north_magnetic_thresh_rising_en +What: /sys/.../iio:deviceX/events/in_north_magnetic_thresh_falling_en +What: /sys/.../iio:deviceX/events/in_north_true_thresh_rising_en +What: /sys/.../iio:deviceX/events/in_north_true_thresh_falling_en +What: /sys/.../iio:deviceX/events/in_north_magnetic_tilt_comp_thresh_rising_en +What: /sys/.../iio:deviceX/events/in_north_magnetic_tilt_comp_thresh_falling_en +What: /sys/.../iio:deviceX/events/in_north_true_tilt_comp_thresh_rising_en +What: /sys/.../iio:deviceX/events/in_north_true_tilt_comp_thresh_falling_en What: /sys/.../iio:deviceX/events/in_voltageY_supply_thresh_rising_en What: /sys/.../iio:deviceX/events/in_voltageY_supply_thresh_falling_en What: /sys/.../iio:deviceX/events/in_voltageY_thresh_rising_en @@ -481,6 +497,14 @@ What: /sys/.../iio:deviceX/events/in_magn_y_roc_rising_en What: /sys/.../iio:deviceX/events/in_magn_y_roc_falling_en What: /sys/.../iio:deviceX/events/in_magn_z_roc_rising_en What: /sys/.../iio:deviceX/events/in_magn_z_roc_falling_en +What: /sys/.../iio:deviceX/events/in_north_magnetic_roc_rising_en +What: /sys/.../iio:deviceX/events/in_north_magnetic_roc_falling_en +What: /sys/.../iio:deviceX/events/in_north_true_roc_rising_en +What: /sys/.../iio:deviceX/events/in_north_true_roc_falling_en +What: /sys/.../iio:deviceX/events/in_north_magnetic_tilt_comp_roc_rising_en +What: /sys/.../iio:deviceX/events/in_north_magnetic_tilt_comp_roc_falling_en +What: /sys/.../iio:deviceX/events/in_north_true_tilt_comp_roc_rising_en +What: /sys/.../iio:deviceX/events/in_north_true_tilt_comp_roc_falling_en What: /sys/.../iio:deviceX/events/in_voltageY_supply_roc_rising_en What: /sys/.../iio:deviceX/events/in_voltageY_supply_roc_falling_en What: /sys/.../iio:deviceX/events/in_voltageY_roc_rising_en @@ -527,6 +551,14 @@ What: /sys/.../events/in_magn_y_raw_thresh_rising_value What: /sys/.../events/in_magn_y_raw_thresh_falling_value What: /sys/.../events/in_magn_z_raw_thresh_rising_value What: /sys/.../events/in_magn_z_raw_thresh_falling_value +What: /sys/.../events/in_north_magnetic_raw_thresh_rising_value +What: /sys/.../events/in_north_magnetic_raw_thresh_falling_value +What: /sys/.../events/in_north_true_raw_thresh_rising_value +What: /sys/.../events/in_north_true_raw_thresh_falling_value +What: /sys/.../events/in_north_magnetic_tilt_comp_raw_thresh_rising_value +What: /sys/.../events/in_north_magnetic_tilt_comp_raw_thresh_falling_value +What: /sys/.../events/in_north_true_tilt_comp_raw_thresh_rising_value +What: /sys/.../events/in_north_true_tilt_comp_raw_thresh_falling_value What: /sys/.../events/in_voltageY_supply_raw_thresh_rising_value What: /sys/.../events/in_voltageY_supply_raw_thresh_falling_value What: /sys/.../events/in_voltageY_raw_thresh_rising_value @@ -577,6 +609,18 @@ What: /sys/.../events/in_magn_y_thresh_either_hysteresis What: /sys/.../events/in_magn_z_thresh_rising_hysteresis What: /sys/.../events/in_magn_z_thresh_falling_hysteresis What: /sys/.../events/in_magn_z_thresh_either_hysteresis +What: /sys/.../events/in_north_magnetic_thresh_rising_hysteresis +What: /sys/.../events/in_north_magnetic_thresh_falling_hysteresis +What: /sys/.../events/in_north_magnetic_thresh_either_hysteresis +What: /sys/.../events/in_north_true_thresh_rising_hysteresis +What: /sys/.../events/in_north_true_thresh_falling_hysteresis +What: /sys/.../events/in_north_true_thresh_either_hysteresis +What: /sys/.../events/in_north_magnetic_tilt_comp_thresh_rising_hysteresis +What: /sys/.../events/in_north_magnetic_tilt_comp_thresh_falling_hysteresis +What: /sys/.../events/in_north_magnetic_tilt_comp_thresh_either_hysteresis +What: /sys/.../events/in_north_true_tilt_comp_thresh_rising_hysteresis +What: /sys/.../events/in_north_true_tilt_comp_thresh_falling_hysteresis +What: /sys/.../events/in_north_true_tilt_comp_thresh_either_hysteresis What: /sys/.../events/in_voltageY_thresh_rising_hysteresis What: /sys/.../events/in_voltageY_thresh_falling_hysteresis What: /sys/.../events/in_voltageY_thresh_either_hysteresis @@ -624,6 +668,14 @@ What: /sys/.../events/in_magn_y_raw_roc_rising_value What: /sys/.../events/in_magn_y_raw_roc_falling_value What: /sys/.../events/in_magn_z_raw_roc_rising_value What: /sys/.../events/in_magn_z_raw_roc_falling_value +What: /sys/.../events/in_north_magnetic_raw_roc_rising_value +What: /sys/.../events/in_north_magnetic_raw_roc_falling_value +What: /sys/.../events/in_north_true_raw_roc_rising_value +What: /sys/.../events/in_north_true_raw_roc_falling_value +What: /sys/.../events/in_north_magnetic_tilt_comp_raw_roc_rising_value +What: /sys/.../events/in_north_magnetic_tilt_comp_raw_roc_falling_value +What: /sys/.../events/in_north_true_tilt_comp_raw_roc_rising_value +What: /sys/.../events/in_north_true_tilt_comp_raw_roc_falling_value What: /sys/.../events/in_voltageY_supply_raw_roc_rising_value What: /sys/.../events/in_voltageY_supply_raw_roc_falling_value What: /sys/.../events/in_voltageY_raw_roc_rising_value @@ -679,6 +731,22 @@ What: /sys/.../events/in_magn_z_thresh_rising_period What: /sys/.../events/in_magn_z_thresh_falling_period What: /sys/.../events/in_magn_z_roc_rising_period What: /sys/.../events/in_magn_z_roc_falling_period +What: /sys/.../events/in_north_magnetic_thresh_rising_period +What: /sys/.../events/in_north_magnetic_thresh_falling_period +What: /sys/.../events/in_north_magnetic_roc_rising_period +What: /sys/.../events/in_north_magnetic_roc_falling_period +What: /sys/.../events/in_north_true_thresh_rising_period +What: /sys/.../events/in_north_true_thresh_falling_period +What: /sys/.../events/in_north_true_roc_rising_period +What: /sys/.../events/in_north_true_roc_falling_period +What: /sys/.../events/in_north_magnetic_tilt_comp_thresh_rising_period +What: /sys/.../events/in_north_magnetic_tilt_comp_thresh_falling_period +What: /sys/.../events/in_north_magnetic_tilt_comp_roc_rising_period +What: /sys/.../events/in_north_magnetic_tilt_comp_roc_falling_period +What: /sys/.../events/in_north_true_tilt_comp_thresh_rising_period +What: /sys/.../events/in_north_true_tilt_comp_thresh_falling_period +What: /sys/.../events/in_north_true_tilt_comp_roc_rising_period +What: /sys/.../events/in_north_true_tilt_comp_roc_falling_period What: /sys/.../events/in_voltageY_supply_thresh_rising_period What: /sys/.../events/in_voltageY_supply_thresh_falling_period What: /sys/.../events/in_voltageY_supply_roc_rising_period @@ -776,6 +844,10 @@ What: /sys/.../iio:deviceX/scan_elements/in_anglvel_z_en What: /sys/.../iio:deviceX/scan_elements/in_magn_x_en What: /sys/.../iio:deviceX/scan_elements/in_magn_y_en What: /sys/.../iio:deviceX/scan_elements/in_magn_z_en +What: /sys/.../iio:deviceX/scan_elements/in_north_magnetic_en +What: /sys/.../iio:deviceX/scan_elements/in_north_true_en +What: /sys/.../iio:deviceX/scan_elements/in_north_magnetic_tilt_comp_en +What: /sys/.../iio:deviceX/scan_elements/in_north_true_tilt_comp_en What: /sys/.../iio:deviceX/scan_elements/in_timestamp_en What: /sys/.../iio:deviceX/scan_elements/in_voltageY_supply_en What: /sys/.../iio:deviceX/scan_elements/in_voltageY_en @@ -840,6 +912,10 @@ What: /sys/.../iio:deviceX/scan_elements/in_anglvel_z_index What: /sys/.../iio:deviceX/scan_elements/in_magn_x_index What: /sys/.../iio:deviceX/scan_elements/in_magn_y_index What: /sys/.../iio:deviceX/scan_elements/in_magn_z_index +What: /sys/.../iio:deviceX/scan_elements/in_north_magnetic_index +What: /sys/.../iio:deviceX/scan_elements/in_north_true_index +What: /sys/.../iio:deviceX/scan_elements/in_north_magnetic_tilt_comp_index +What: /sys/.../iio:deviceX/scan_elements/in_north_true_tilt_comp_index What: /sys/.../iio:deviceX/scan_elements/in_incli_x_index What: /sys/.../iio:deviceX/scan_elements/in_incli_y_index What: /sys/.../iio:deviceX/scan_elements/in_timestamp_index -- 1.9.3 ^ permalink raw reply related [flat|nested] 23+ messages in thread
[parent not found: <1401750890-31854-2-git-send-email-reyad.attiyat-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]
* Re: [PATCHv2 1/3] IIO: Documentation: Add north attribute to ABI docs 2014-06-02 23:14 ` Reyad Attiyat @ 2014-06-09 17:50 ` Jonathan Cameron -1 siblings, 0 replies; 23+ messages in thread From: Jonathan Cameron @ 2014-06-09 17:50 UTC (permalink / raw) To: Reyad Attiyat, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-iio-u79uwXL29TY76Z2rM5mHXA, srinivas.pandruvada-VuQAYsv1563Yd54FQh9/CA, linux-input-u79uwXL29TY76Z2rM5mHXA, jkosina-AlSwsSmVLrQ On 03/06/14 00:14, Reyad Attiyat wrote: > Updated ABI documentation to inculde compass north usages. > > Signed-off-by: Reyad Attiyat <reyad.attiyat-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> Amazing how many lines of documentation two small additions can make! Anyhow, I'm keener on having these as as modifiers on a rotation rather than there own type. in_rot_from_north_magnetic_raw and friends (as per your suggestion elsewhere). Otherwise, whilst obvious, perhaps a line or two in the main _raw attribute description to state the obvious and say what this is... J > --- > Documentation/ABI/testing/sysfs-bus-iio | 76 +++++++++++++++++++++++++++++++++ > 1 file changed, 76 insertions(+) > > diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio > index 6e02c50..251c2bb 100644 > --- a/Documentation/ABI/testing/sysfs-bus-iio > +++ b/Documentation/ABI/testing/sysfs-bus-iio > @@ -165,6 +165,10 @@ Description: > What: /sys/bus/iio/devices/iio:deviceX/in_magn_x_raw > What: /sys/bus/iio/devices/iio:deviceX/in_magn_y_raw > What: /sys/bus/iio/devices/iio:deviceX/in_magn_z_raw > +What: /sys/bus/iio/devices/iio:deviceX/in_north_magnetic_raw > +What: /sys/bus/iio/devices/iio:deviceX/in_north_true_raw > +What: /sys/bus/iio/devices/iio:deviceX/in_north_magnetic_tilt_comp_raw > +What: /sys/bus/iio/devices/iio:deviceX/in_north_true_tilt_comp_raw > KernelVersion: 2.6.35 > Contact: linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org > Description: > @@ -249,6 +253,10 @@ What: /sys/bus/iio/devices/iio:deviceX/in_magn_scale > What: /sys/bus/iio/devices/iio:deviceX/in_magn_x_scale > What: /sys/bus/iio/devices/iio:deviceX/in_magn_y_scale > What: /sys/bus/iio/devices/iio:deviceX/in_magn_z_scale > +What: /sys/bus/iio/devices/iio:deviceX/in_north_magnetic_scale > +What: /sys/bus/iio/devices/iio:deviceX/in_north_true_scale > +What: /sys/bus/iio/devices/iio:deviceX/in_north_magnetic_tilt_comp_scale > +What: /sys/bus/iio/devices/iio:deviceX/in_north_true_tilt_comp_scale > What: /sys/bus/iio/devices/iio:deviceX/in_pressureY_scale > What: /sys/bus/iio/devices/iio:deviceX/in_pressure_scale > KernelVersion: 2.6.35 > @@ -436,6 +444,14 @@ What: /sys/.../iio:deviceX/events/in_magn_y_thresh_rising_en > What: /sys/.../iio:deviceX/events/in_magn_y_thresh_falling_en > What: /sys/.../iio:deviceX/events/in_magn_z_thresh_rising_en > What: /sys/.../iio:deviceX/events/in_magn_z_thresh_falling_en > +What: /sys/.../iio:deviceX/events/in_north_magnetic_thresh_rising_en > +What: /sys/.../iio:deviceX/events/in_north_magnetic_thresh_falling_en > +What: /sys/.../iio:deviceX/events/in_north_true_thresh_rising_en > +What: /sys/.../iio:deviceX/events/in_north_true_thresh_falling_en > +What: /sys/.../iio:deviceX/events/in_north_magnetic_tilt_comp_thresh_rising_en > +What: /sys/.../iio:deviceX/events/in_north_magnetic_tilt_comp_thresh_falling_en > +What: /sys/.../iio:deviceX/events/in_north_true_tilt_comp_thresh_rising_en > +What: /sys/.../iio:deviceX/events/in_north_true_tilt_comp_thresh_falling_en > What: /sys/.../iio:deviceX/events/in_voltageY_supply_thresh_rising_en > What: /sys/.../iio:deviceX/events/in_voltageY_supply_thresh_falling_en > What: /sys/.../iio:deviceX/events/in_voltageY_thresh_rising_en > @@ -481,6 +497,14 @@ What: /sys/.../iio:deviceX/events/in_magn_y_roc_rising_en > What: /sys/.../iio:deviceX/events/in_magn_y_roc_falling_en > What: /sys/.../iio:deviceX/events/in_magn_z_roc_rising_en > What: /sys/.../iio:deviceX/events/in_magn_z_roc_falling_en > +What: /sys/.../iio:deviceX/events/in_north_magnetic_roc_rising_en > +What: /sys/.../iio:deviceX/events/in_north_magnetic_roc_falling_en > +What: /sys/.../iio:deviceX/events/in_north_true_roc_rising_en > +What: /sys/.../iio:deviceX/events/in_north_true_roc_falling_en > +What: /sys/.../iio:deviceX/events/in_north_magnetic_tilt_comp_roc_rising_en > +What: /sys/.../iio:deviceX/events/in_north_magnetic_tilt_comp_roc_falling_en > +What: /sys/.../iio:deviceX/events/in_north_true_tilt_comp_roc_rising_en > +What: /sys/.../iio:deviceX/events/in_north_true_tilt_comp_roc_falling_en > What: /sys/.../iio:deviceX/events/in_voltageY_supply_roc_rising_en > What: /sys/.../iio:deviceX/events/in_voltageY_supply_roc_falling_en > What: /sys/.../iio:deviceX/events/in_voltageY_roc_rising_en > @@ -527,6 +551,14 @@ What: /sys/.../events/in_magn_y_raw_thresh_rising_value > What: /sys/.../events/in_magn_y_raw_thresh_falling_value > What: /sys/.../events/in_magn_z_raw_thresh_rising_value > What: /sys/.../events/in_magn_z_raw_thresh_falling_value > +What: /sys/.../events/in_north_magnetic_raw_thresh_rising_value > +What: /sys/.../events/in_north_magnetic_raw_thresh_falling_value > +What: /sys/.../events/in_north_true_raw_thresh_rising_value > +What: /sys/.../events/in_north_true_raw_thresh_falling_value > +What: /sys/.../events/in_north_magnetic_tilt_comp_raw_thresh_rising_value > +What: /sys/.../events/in_north_magnetic_tilt_comp_raw_thresh_falling_value > +What: /sys/.../events/in_north_true_tilt_comp_raw_thresh_rising_value > +What: /sys/.../events/in_north_true_tilt_comp_raw_thresh_falling_value > What: /sys/.../events/in_voltageY_supply_raw_thresh_rising_value > What: /sys/.../events/in_voltageY_supply_raw_thresh_falling_value > What: /sys/.../events/in_voltageY_raw_thresh_rising_value > @@ -577,6 +609,18 @@ What: /sys/.../events/in_magn_y_thresh_either_hysteresis > What: /sys/.../events/in_magn_z_thresh_rising_hysteresis > What: /sys/.../events/in_magn_z_thresh_falling_hysteresis > What: /sys/.../events/in_magn_z_thresh_either_hysteresis > +What: /sys/.../events/in_north_magnetic_thresh_rising_hysteresis > +What: /sys/.../events/in_north_magnetic_thresh_falling_hysteresis > +What: /sys/.../events/in_north_magnetic_thresh_either_hysteresis > +What: /sys/.../events/in_north_true_thresh_rising_hysteresis > +What: /sys/.../events/in_north_true_thresh_falling_hysteresis > +What: /sys/.../events/in_north_true_thresh_either_hysteresis > +What: /sys/.../events/in_north_magnetic_tilt_comp_thresh_rising_hysteresis > +What: /sys/.../events/in_north_magnetic_tilt_comp_thresh_falling_hysteresis > +What: /sys/.../events/in_north_magnetic_tilt_comp_thresh_either_hysteresis > +What: /sys/.../events/in_north_true_tilt_comp_thresh_rising_hysteresis > +What: /sys/.../events/in_north_true_tilt_comp_thresh_falling_hysteresis > +What: /sys/.../events/in_north_true_tilt_comp_thresh_either_hysteresis > What: /sys/.../events/in_voltageY_thresh_rising_hysteresis > What: /sys/.../events/in_voltageY_thresh_falling_hysteresis > What: /sys/.../events/in_voltageY_thresh_either_hysteresis > @@ -624,6 +668,14 @@ What: /sys/.../events/in_magn_y_raw_roc_rising_value > What: /sys/.../events/in_magn_y_raw_roc_falling_value > What: /sys/.../events/in_magn_z_raw_roc_rising_value > What: /sys/.../events/in_magn_z_raw_roc_falling_value > +What: /sys/.../events/in_north_magnetic_raw_roc_rising_value > +What: /sys/.../events/in_north_magnetic_raw_roc_falling_value > +What: /sys/.../events/in_north_true_raw_roc_rising_value > +What: /sys/.../events/in_north_true_raw_roc_falling_value > +What: /sys/.../events/in_north_magnetic_tilt_comp_raw_roc_rising_value > +What: /sys/.../events/in_north_magnetic_tilt_comp_raw_roc_falling_value > +What: /sys/.../events/in_north_true_tilt_comp_raw_roc_rising_value > +What: /sys/.../events/in_north_true_tilt_comp_raw_roc_falling_value > What: /sys/.../events/in_voltageY_supply_raw_roc_rising_value > What: /sys/.../events/in_voltageY_supply_raw_roc_falling_value > What: /sys/.../events/in_voltageY_raw_roc_rising_value > @@ -679,6 +731,22 @@ What: /sys/.../events/in_magn_z_thresh_rising_period > What: /sys/.../events/in_magn_z_thresh_falling_period > What: /sys/.../events/in_magn_z_roc_rising_period > What: /sys/.../events/in_magn_z_roc_falling_period > +What: /sys/.../events/in_north_magnetic_thresh_rising_period > +What: /sys/.../events/in_north_magnetic_thresh_falling_period > +What: /sys/.../events/in_north_magnetic_roc_rising_period > +What: /sys/.../events/in_north_magnetic_roc_falling_period > +What: /sys/.../events/in_north_true_thresh_rising_period > +What: /sys/.../events/in_north_true_thresh_falling_period > +What: /sys/.../events/in_north_true_roc_rising_period > +What: /sys/.../events/in_north_true_roc_falling_period > +What: /sys/.../events/in_north_magnetic_tilt_comp_thresh_rising_period > +What: /sys/.../events/in_north_magnetic_tilt_comp_thresh_falling_period > +What: /sys/.../events/in_north_magnetic_tilt_comp_roc_rising_period > +What: /sys/.../events/in_north_magnetic_tilt_comp_roc_falling_period > +What: /sys/.../events/in_north_true_tilt_comp_thresh_rising_period > +What: /sys/.../events/in_north_true_tilt_comp_thresh_falling_period > +What: /sys/.../events/in_north_true_tilt_comp_roc_rising_period > +What: /sys/.../events/in_north_true_tilt_comp_roc_falling_period > What: /sys/.../events/in_voltageY_supply_thresh_rising_period > What: /sys/.../events/in_voltageY_supply_thresh_falling_period > What: /sys/.../events/in_voltageY_supply_roc_rising_period > @@ -776,6 +844,10 @@ What: /sys/.../iio:deviceX/scan_elements/in_anglvel_z_en > What: /sys/.../iio:deviceX/scan_elements/in_magn_x_en > What: /sys/.../iio:deviceX/scan_elements/in_magn_y_en > What: /sys/.../iio:deviceX/scan_elements/in_magn_z_en > +What: /sys/.../iio:deviceX/scan_elements/in_north_magnetic_en > +What: /sys/.../iio:deviceX/scan_elements/in_north_true_en > +What: /sys/.../iio:deviceX/scan_elements/in_north_magnetic_tilt_comp_en > +What: /sys/.../iio:deviceX/scan_elements/in_north_true_tilt_comp_en > What: /sys/.../iio:deviceX/scan_elements/in_timestamp_en > What: /sys/.../iio:deviceX/scan_elements/in_voltageY_supply_en > What: /sys/.../iio:deviceX/scan_elements/in_voltageY_en > @@ -840,6 +912,10 @@ What: /sys/.../iio:deviceX/scan_elements/in_anglvel_z_index > What: /sys/.../iio:deviceX/scan_elements/in_magn_x_index > What: /sys/.../iio:deviceX/scan_elements/in_magn_y_index > What: /sys/.../iio:deviceX/scan_elements/in_magn_z_index > +What: /sys/.../iio:deviceX/scan_elements/in_north_magnetic_index > +What: /sys/.../iio:deviceX/scan_elements/in_north_true_index > +What: /sys/.../iio:deviceX/scan_elements/in_north_magnetic_tilt_comp_index > +What: /sys/.../iio:deviceX/scan_elements/in_north_true_tilt_comp_index > What: /sys/.../iio:deviceX/scan_elements/in_incli_x_index > What: /sys/.../iio:deviceX/scan_elements/in_incli_y_index > What: /sys/.../iio:deviceX/scan_elements/in_timestamp_index > ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCHv2 1/3] IIO: Documentation: Add north attribute to ABI docs @ 2014-06-09 17:50 ` Jonathan Cameron 0 siblings, 0 replies; 23+ messages in thread From: Jonathan Cameron @ 2014-06-09 17:50 UTC (permalink / raw) To: Reyad Attiyat, linux-kernel, linux-iio, srinivas.pandruvada, linux-input, jkosina On 03/06/14 00:14, Reyad Attiyat wrote: > Updated ABI documentation to inculde compass north usages. > > Signed-off-by: Reyad Attiyat <reyad.attiyat@gmail.com> Amazing how many lines of documentation two small additions can make! Anyhow, I'm keener on having these as as modifiers on a rotation rather than there own type. in_rot_from_north_magnetic_raw and friends (as per your suggestion elsewhere). Otherwise, whilst obvious, perhaps a line or two in the main _raw attribute description to state the obvious and say what this is... J > --- > Documentation/ABI/testing/sysfs-bus-iio | 76 +++++++++++++++++++++++++++++++++ > 1 file changed, 76 insertions(+) > > diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio > index 6e02c50..251c2bb 100644 > --- a/Documentation/ABI/testing/sysfs-bus-iio > +++ b/Documentation/ABI/testing/sysfs-bus-iio > @@ -165,6 +165,10 @@ Description: > What: /sys/bus/iio/devices/iio:deviceX/in_magn_x_raw > What: /sys/bus/iio/devices/iio:deviceX/in_magn_y_raw > What: /sys/bus/iio/devices/iio:deviceX/in_magn_z_raw > +What: /sys/bus/iio/devices/iio:deviceX/in_north_magnetic_raw > +What: /sys/bus/iio/devices/iio:deviceX/in_north_true_raw > +What: /sys/bus/iio/devices/iio:deviceX/in_north_magnetic_tilt_comp_raw > +What: /sys/bus/iio/devices/iio:deviceX/in_north_true_tilt_comp_raw > KernelVersion: 2.6.35 > Contact: linux-iio@vger.kernel.org > Description: > @@ -249,6 +253,10 @@ What: /sys/bus/iio/devices/iio:deviceX/in_magn_scale > What: /sys/bus/iio/devices/iio:deviceX/in_magn_x_scale > What: /sys/bus/iio/devices/iio:deviceX/in_magn_y_scale > What: /sys/bus/iio/devices/iio:deviceX/in_magn_z_scale > +What: /sys/bus/iio/devices/iio:deviceX/in_north_magnetic_scale > +What: /sys/bus/iio/devices/iio:deviceX/in_north_true_scale > +What: /sys/bus/iio/devices/iio:deviceX/in_north_magnetic_tilt_comp_scale > +What: /sys/bus/iio/devices/iio:deviceX/in_north_true_tilt_comp_scale > What: /sys/bus/iio/devices/iio:deviceX/in_pressureY_scale > What: /sys/bus/iio/devices/iio:deviceX/in_pressure_scale > KernelVersion: 2.6.35 > @@ -436,6 +444,14 @@ What: /sys/.../iio:deviceX/events/in_magn_y_thresh_rising_en > What: /sys/.../iio:deviceX/events/in_magn_y_thresh_falling_en > What: /sys/.../iio:deviceX/events/in_magn_z_thresh_rising_en > What: /sys/.../iio:deviceX/events/in_magn_z_thresh_falling_en > +What: /sys/.../iio:deviceX/events/in_north_magnetic_thresh_rising_en > +What: /sys/.../iio:deviceX/events/in_north_magnetic_thresh_falling_en > +What: /sys/.../iio:deviceX/events/in_north_true_thresh_rising_en > +What: /sys/.../iio:deviceX/events/in_north_true_thresh_falling_en > +What: /sys/.../iio:deviceX/events/in_north_magnetic_tilt_comp_thresh_rising_en > +What: /sys/.../iio:deviceX/events/in_north_magnetic_tilt_comp_thresh_falling_en > +What: /sys/.../iio:deviceX/events/in_north_true_tilt_comp_thresh_rising_en > +What: /sys/.../iio:deviceX/events/in_north_true_tilt_comp_thresh_falling_en > What: /sys/.../iio:deviceX/events/in_voltageY_supply_thresh_rising_en > What: /sys/.../iio:deviceX/events/in_voltageY_supply_thresh_falling_en > What: /sys/.../iio:deviceX/events/in_voltageY_thresh_rising_en > @@ -481,6 +497,14 @@ What: /sys/.../iio:deviceX/events/in_magn_y_roc_rising_en > What: /sys/.../iio:deviceX/events/in_magn_y_roc_falling_en > What: /sys/.../iio:deviceX/events/in_magn_z_roc_rising_en > What: /sys/.../iio:deviceX/events/in_magn_z_roc_falling_en > +What: /sys/.../iio:deviceX/events/in_north_magnetic_roc_rising_en > +What: /sys/.../iio:deviceX/events/in_north_magnetic_roc_falling_en > +What: /sys/.../iio:deviceX/events/in_north_true_roc_rising_en > +What: /sys/.../iio:deviceX/events/in_north_true_roc_falling_en > +What: /sys/.../iio:deviceX/events/in_north_magnetic_tilt_comp_roc_rising_en > +What: /sys/.../iio:deviceX/events/in_north_magnetic_tilt_comp_roc_falling_en > +What: /sys/.../iio:deviceX/events/in_north_true_tilt_comp_roc_rising_en > +What: /sys/.../iio:deviceX/events/in_north_true_tilt_comp_roc_falling_en > What: /sys/.../iio:deviceX/events/in_voltageY_supply_roc_rising_en > What: /sys/.../iio:deviceX/events/in_voltageY_supply_roc_falling_en > What: /sys/.../iio:deviceX/events/in_voltageY_roc_rising_en > @@ -527,6 +551,14 @@ What: /sys/.../events/in_magn_y_raw_thresh_rising_value > What: /sys/.../events/in_magn_y_raw_thresh_falling_value > What: /sys/.../events/in_magn_z_raw_thresh_rising_value > What: /sys/.../events/in_magn_z_raw_thresh_falling_value > +What: /sys/.../events/in_north_magnetic_raw_thresh_rising_value > +What: /sys/.../events/in_north_magnetic_raw_thresh_falling_value > +What: /sys/.../events/in_north_true_raw_thresh_rising_value > +What: /sys/.../events/in_north_true_raw_thresh_falling_value > +What: /sys/.../events/in_north_magnetic_tilt_comp_raw_thresh_rising_value > +What: /sys/.../events/in_north_magnetic_tilt_comp_raw_thresh_falling_value > +What: /sys/.../events/in_north_true_tilt_comp_raw_thresh_rising_value > +What: /sys/.../events/in_north_true_tilt_comp_raw_thresh_falling_value > What: /sys/.../events/in_voltageY_supply_raw_thresh_rising_value > What: /sys/.../events/in_voltageY_supply_raw_thresh_falling_value > What: /sys/.../events/in_voltageY_raw_thresh_rising_value > @@ -577,6 +609,18 @@ What: /sys/.../events/in_magn_y_thresh_either_hysteresis > What: /sys/.../events/in_magn_z_thresh_rising_hysteresis > What: /sys/.../events/in_magn_z_thresh_falling_hysteresis > What: /sys/.../events/in_magn_z_thresh_either_hysteresis > +What: /sys/.../events/in_north_magnetic_thresh_rising_hysteresis > +What: /sys/.../events/in_north_magnetic_thresh_falling_hysteresis > +What: /sys/.../events/in_north_magnetic_thresh_either_hysteresis > +What: /sys/.../events/in_north_true_thresh_rising_hysteresis > +What: /sys/.../events/in_north_true_thresh_falling_hysteresis > +What: /sys/.../events/in_north_true_thresh_either_hysteresis > +What: /sys/.../events/in_north_magnetic_tilt_comp_thresh_rising_hysteresis > +What: /sys/.../events/in_north_magnetic_tilt_comp_thresh_falling_hysteresis > +What: /sys/.../events/in_north_magnetic_tilt_comp_thresh_either_hysteresis > +What: /sys/.../events/in_north_true_tilt_comp_thresh_rising_hysteresis > +What: /sys/.../events/in_north_true_tilt_comp_thresh_falling_hysteresis > +What: /sys/.../events/in_north_true_tilt_comp_thresh_either_hysteresis > What: /sys/.../events/in_voltageY_thresh_rising_hysteresis > What: /sys/.../events/in_voltageY_thresh_falling_hysteresis > What: /sys/.../events/in_voltageY_thresh_either_hysteresis > @@ -624,6 +668,14 @@ What: /sys/.../events/in_magn_y_raw_roc_rising_value > What: /sys/.../events/in_magn_y_raw_roc_falling_value > What: /sys/.../events/in_magn_z_raw_roc_rising_value > What: /sys/.../events/in_magn_z_raw_roc_falling_value > +What: /sys/.../events/in_north_magnetic_raw_roc_rising_value > +What: /sys/.../events/in_north_magnetic_raw_roc_falling_value > +What: /sys/.../events/in_north_true_raw_roc_rising_value > +What: /sys/.../events/in_north_true_raw_roc_falling_value > +What: /sys/.../events/in_north_magnetic_tilt_comp_raw_roc_rising_value > +What: /sys/.../events/in_north_magnetic_tilt_comp_raw_roc_falling_value > +What: /sys/.../events/in_north_true_tilt_comp_raw_roc_rising_value > +What: /sys/.../events/in_north_true_tilt_comp_raw_roc_falling_value > What: /sys/.../events/in_voltageY_supply_raw_roc_rising_value > What: /sys/.../events/in_voltageY_supply_raw_roc_falling_value > What: /sys/.../events/in_voltageY_raw_roc_rising_value > @@ -679,6 +731,22 @@ What: /sys/.../events/in_magn_z_thresh_rising_period > What: /sys/.../events/in_magn_z_thresh_falling_period > What: /sys/.../events/in_magn_z_roc_rising_period > What: /sys/.../events/in_magn_z_roc_falling_period > +What: /sys/.../events/in_north_magnetic_thresh_rising_period > +What: /sys/.../events/in_north_magnetic_thresh_falling_period > +What: /sys/.../events/in_north_magnetic_roc_rising_period > +What: /sys/.../events/in_north_magnetic_roc_falling_period > +What: /sys/.../events/in_north_true_thresh_rising_period > +What: /sys/.../events/in_north_true_thresh_falling_period > +What: /sys/.../events/in_north_true_roc_rising_period > +What: /sys/.../events/in_north_true_roc_falling_period > +What: /sys/.../events/in_north_magnetic_tilt_comp_thresh_rising_period > +What: /sys/.../events/in_north_magnetic_tilt_comp_thresh_falling_period > +What: /sys/.../events/in_north_magnetic_tilt_comp_roc_rising_period > +What: /sys/.../events/in_north_magnetic_tilt_comp_roc_falling_period > +What: /sys/.../events/in_north_true_tilt_comp_thresh_rising_period > +What: /sys/.../events/in_north_true_tilt_comp_thresh_falling_period > +What: /sys/.../events/in_north_true_tilt_comp_roc_rising_period > +What: /sys/.../events/in_north_true_tilt_comp_roc_falling_period > What: /sys/.../events/in_voltageY_supply_thresh_rising_period > What: /sys/.../events/in_voltageY_supply_thresh_falling_period > What: /sys/.../events/in_voltageY_supply_roc_rising_period > @@ -776,6 +844,10 @@ What: /sys/.../iio:deviceX/scan_elements/in_anglvel_z_en > What: /sys/.../iio:deviceX/scan_elements/in_magn_x_en > What: /sys/.../iio:deviceX/scan_elements/in_magn_y_en > What: /sys/.../iio:deviceX/scan_elements/in_magn_z_en > +What: /sys/.../iio:deviceX/scan_elements/in_north_magnetic_en > +What: /sys/.../iio:deviceX/scan_elements/in_north_true_en > +What: /sys/.../iio:deviceX/scan_elements/in_north_magnetic_tilt_comp_en > +What: /sys/.../iio:deviceX/scan_elements/in_north_true_tilt_comp_en > What: /sys/.../iio:deviceX/scan_elements/in_timestamp_en > What: /sys/.../iio:deviceX/scan_elements/in_voltageY_supply_en > What: /sys/.../iio:deviceX/scan_elements/in_voltageY_en > @@ -840,6 +912,10 @@ What: /sys/.../iio:deviceX/scan_elements/in_anglvel_z_index > What: /sys/.../iio:deviceX/scan_elements/in_magn_x_index > What: /sys/.../iio:deviceX/scan_elements/in_magn_y_index > What: /sys/.../iio:deviceX/scan_elements/in_magn_z_index > +What: /sys/.../iio:deviceX/scan_elements/in_north_magnetic_index > +What: /sys/.../iio:deviceX/scan_elements/in_north_true_index > +What: /sys/.../iio:deviceX/scan_elements/in_north_magnetic_tilt_comp_index > +What: /sys/.../iio:deviceX/scan_elements/in_north_true_tilt_comp_index > What: /sys/.../iio:deviceX/scan_elements/in_incli_x_index > What: /sys/.../iio:deviceX/scan_elements/in_incli_y_index > What: /sys/.../iio:deviceX/scan_elements/in_timestamp_index > ^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCHv2 2/3] IIO: Add iio_chan modifier for True/Magnetic North HID usages 2014-06-02 23:14 [PATCHv2 0/3] IIO: Add support for compass north usage attribute Reyad Attiyat @ 2014-06-02 23:14 ` Reyad Attiyat 2014-06-02 23:14 ` [PATCHv2 3/3] IIO: hid-sensor-magn-3d: Add in support " Reyad Attiyat 1 sibling, 0 replies; 23+ messages in thread From: Reyad Attiyat @ 2014-06-02 23:14 UTC (permalink / raw) To: linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-iio-u79uwXL29TY76Z2rM5mHXA, jic23-DgEjT+Ai2ygdnm+yROfE0A, srinivas.pandruvada-VuQAYsv1563Yd54FQh9/CA, linux-input-u79uwXL29TY76Z2rM5mHXA, jkosina-AlSwsSmVLrQ Cc: Reyad Attiyat Updated iio modifier enum for compass north usages, including magnetic/true north with tilt compensation. Signed-off-by: Reyad Attiyat <reyad.attiyat-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> --- drivers/iio/industrialio-core.c | 4 ++++ include/linux/iio/types.h | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c index ede16aec..2f523b5 100644 --- a/drivers/iio/industrialio-core.c +++ b/drivers/iio/industrialio-core.c @@ -84,6 +84,10 @@ static const char * const iio_modifier_names[] = { [IIO_MOD_LIGHT_RED] = "red", [IIO_MOD_LIGHT_GREEN] = "green", [IIO_MOD_LIGHT_BLUE] = "blue", + [IIO_MOD_NORTH_MAGN] = "north_magnetic", + [IIO_MOD_NORTH_TRUE] = "north_true", + [IIO_MOD_NORTH_MAGN_TILT_COMP] = "north_magnetic_tilt_comp", + [IIO_MOD_NORTH_TRUE_TILT_COMP] = "north_true_tilt_comp", }; /* relies on pairs of these shared then separate */ diff --git a/include/linux/iio/types.h b/include/linux/iio/types.h index 084d882..5bf8847 100644 --- a/include/linux/iio/types.h +++ b/include/linux/iio/types.h @@ -53,6 +53,10 @@ enum iio_modifier { IIO_MOD_LIGHT_RED, IIO_MOD_LIGHT_GREEN, IIO_MOD_LIGHT_BLUE, + IIO_MOD_NORTH_MAGN, + IIO_MOD_NORTH_TRUE, + IIO_MOD_NORTH_MAGN_TILT_COMP, + IIO_MOD_NORTH_TRUE_TILT_COMP }; enum iio_event_type { -- 1.9.3 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCHv2 2/3] IIO: Add iio_chan modifier for True/Magnetic North HID usages @ 2014-06-02 23:14 ` Reyad Attiyat 0 siblings, 0 replies; 23+ messages in thread From: Reyad Attiyat @ 2014-06-02 23:14 UTC (permalink / raw) To: linux-kernel, linux-iio, jic23, srinivas.pandruvada, linux-input, jkosina Cc: Reyad Attiyat Updated iio modifier enum for compass north usages, including magnetic/true north with tilt compensation. Signed-off-by: Reyad Attiyat <reyad.attiyat@gmail.com> --- drivers/iio/industrialio-core.c | 4 ++++ include/linux/iio/types.h | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c index ede16aec..2f523b5 100644 --- a/drivers/iio/industrialio-core.c +++ b/drivers/iio/industrialio-core.c @@ -84,6 +84,10 @@ static const char * const iio_modifier_names[] = { [IIO_MOD_LIGHT_RED] = "red", [IIO_MOD_LIGHT_GREEN] = "green", [IIO_MOD_LIGHT_BLUE] = "blue", + [IIO_MOD_NORTH_MAGN] = "north_magnetic", + [IIO_MOD_NORTH_TRUE] = "north_true", + [IIO_MOD_NORTH_MAGN_TILT_COMP] = "north_magnetic_tilt_comp", + [IIO_MOD_NORTH_TRUE_TILT_COMP] = "north_true_tilt_comp", }; /* relies on pairs of these shared then separate */ diff --git a/include/linux/iio/types.h b/include/linux/iio/types.h index 084d882..5bf8847 100644 --- a/include/linux/iio/types.h +++ b/include/linux/iio/types.h @@ -53,6 +53,10 @@ enum iio_modifier { IIO_MOD_LIGHT_RED, IIO_MOD_LIGHT_GREEN, IIO_MOD_LIGHT_BLUE, + IIO_MOD_NORTH_MAGN, + IIO_MOD_NORTH_TRUE, + IIO_MOD_NORTH_MAGN_TILT_COMP, + IIO_MOD_NORTH_TRUE_TILT_COMP }; enum iio_event_type { -- 1.9.3 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCHv2 2/3] IIO: Add iio_chan modifier for True/Magnetic North HID usages 2014-06-02 23:14 ` Reyad Attiyat (?) @ 2014-06-09 17:51 ` Jonathan Cameron [not found] ` <5395F424.4070407-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> -1 siblings, 1 reply; 23+ messages in thread From: Jonathan Cameron @ 2014-06-09 17:51 UTC (permalink / raw) To: Reyad Attiyat, linux-kernel, linux-iio, srinivas.pandruvada, linux-input, jkosina On 03/06/14 00:14, Reyad Attiyat wrote: > Updated iio modifier enum for compass north usages, > including magnetic/true north with tilt compensation. > > Signed-off-by: Reyad Attiyat <reyad.attiyat@gmail.com> This looks fine. > --- > drivers/iio/industrialio-core.c | 4 ++++ > include/linux/iio/types.h | 4 ++++ > 2 files changed, 8 insertions(+) > > diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c > index ede16aec..2f523b5 100644 > --- a/drivers/iio/industrialio-core.c > +++ b/drivers/iio/industrialio-core.c > @@ -84,6 +84,10 @@ static const char * const iio_modifier_names[] = { > [IIO_MOD_LIGHT_RED] = "red", > [IIO_MOD_LIGHT_GREEN] = "green", > [IIO_MOD_LIGHT_BLUE] = "blue", > + [IIO_MOD_NORTH_MAGN] = "north_magnetic", > + [IIO_MOD_NORTH_TRUE] = "north_true", > + [IIO_MOD_NORTH_MAGN_TILT_COMP] = "north_magnetic_tilt_comp", > + [IIO_MOD_NORTH_TRUE_TILT_COMP] = "north_true_tilt_comp", > }; > > /* relies on pairs of these shared then separate */ > diff --git a/include/linux/iio/types.h b/include/linux/iio/types.h > index 084d882..5bf8847 100644 > --- a/include/linux/iio/types.h > +++ b/include/linux/iio/types.h > @@ -53,6 +53,10 @@ enum iio_modifier { > IIO_MOD_LIGHT_RED, > IIO_MOD_LIGHT_GREEN, > IIO_MOD_LIGHT_BLUE, > + IIO_MOD_NORTH_MAGN, > + IIO_MOD_NORTH_TRUE, > + IIO_MOD_NORTH_MAGN_TILT_COMP, > + IIO_MOD_NORTH_TRUE_TILT_COMP > }; > > enum iio_event_type { > ^ permalink raw reply [flat|nested] 23+ messages in thread
[parent not found: <5395F424.4070407-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>]
* Re: [PATCHv2 2/3] IIO: Add iio_chan modifier for True/Magnetic North HID usages 2014-06-09 17:51 ` Jonathan Cameron @ 2014-06-09 17:52 ` Jonathan Cameron 0 siblings, 0 replies; 23+ messages in thread From: Jonathan Cameron @ 2014-06-09 17:52 UTC (permalink / raw) To: Reyad Attiyat, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-iio-u79uwXL29TY76Z2rM5mHXA, srinivas.pandruvada-VuQAYsv1563Yd54FQh9/CA, linux-input-u79uwXL29TY76Z2rM5mHXA, jkosina-AlSwsSmVLrQ On 09/06/14 18:51, Jonathan Cameron wrote: > On 03/06/14 00:14, Reyad Attiyat wrote: >> Updated iio modifier enum for compass north usages, >> including magnetic/true north with tilt compensation. >> >> Signed-off-by: Reyad Attiyat <reyad.attiyat-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> > This looks fine. gah. With the from_ prefix you suggested of course! >> --- >> drivers/iio/industrialio-core.c | 4 ++++ >> include/linux/iio/types.h | 4 ++++ >> 2 files changed, 8 insertions(+) >> >> diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c >> index ede16aec..2f523b5 100644 >> --- a/drivers/iio/industrialio-core.c >> +++ b/drivers/iio/industrialio-core.c >> @@ -84,6 +84,10 @@ static const char * const iio_modifier_names[] = { >> [IIO_MOD_LIGHT_RED] = "red", >> [IIO_MOD_LIGHT_GREEN] = "green", >> [IIO_MOD_LIGHT_BLUE] = "blue", >> + [IIO_MOD_NORTH_MAGN] = "north_magnetic", >> + [IIO_MOD_NORTH_TRUE] = "north_true", >> + [IIO_MOD_NORTH_MAGN_TILT_COMP] = "north_magnetic_tilt_comp", >> + [IIO_MOD_NORTH_TRUE_TILT_COMP] = "north_true_tilt_comp", >> }; >> >> /* relies on pairs of these shared then separate */ >> diff --git a/include/linux/iio/types.h b/include/linux/iio/types.h >> index 084d882..5bf8847 100644 >> --- a/include/linux/iio/types.h >> +++ b/include/linux/iio/types.h >> @@ -53,6 +53,10 @@ enum iio_modifier { >> IIO_MOD_LIGHT_RED, >> IIO_MOD_LIGHT_GREEN, >> IIO_MOD_LIGHT_BLUE, >> + IIO_MOD_NORTH_MAGN, >> + IIO_MOD_NORTH_TRUE, >> + IIO_MOD_NORTH_MAGN_TILT_COMP, >> + IIO_MOD_NORTH_TRUE_TILT_COMP >> }; >> >> enum iio_event_type { >> > ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCHv2 2/3] IIO: Add iio_chan modifier for True/Magnetic North HID usages @ 2014-06-09 17:52 ` Jonathan Cameron 0 siblings, 0 replies; 23+ messages in thread From: Jonathan Cameron @ 2014-06-09 17:52 UTC (permalink / raw) To: Reyad Attiyat, linux-kernel, linux-iio, srinivas.pandruvada, linux-input, jkosina On 09/06/14 18:51, Jonathan Cameron wrote: > On 03/06/14 00:14, Reyad Attiyat wrote: >> Updated iio modifier enum for compass north usages, >> including magnetic/true north with tilt compensation. >> >> Signed-off-by: Reyad Attiyat <reyad.attiyat@gmail.com> > This looks fine. gah. With the from_ prefix you suggested of course! >> --- >> drivers/iio/industrialio-core.c | 4 ++++ >> include/linux/iio/types.h | 4 ++++ >> 2 files changed, 8 insertions(+) >> >> diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c >> index ede16aec..2f523b5 100644 >> --- a/drivers/iio/industrialio-core.c >> +++ b/drivers/iio/industrialio-core.c >> @@ -84,6 +84,10 @@ static const char * const iio_modifier_names[] = { >> [IIO_MOD_LIGHT_RED] = "red", >> [IIO_MOD_LIGHT_GREEN] = "green", >> [IIO_MOD_LIGHT_BLUE] = "blue", >> + [IIO_MOD_NORTH_MAGN] = "north_magnetic", >> + [IIO_MOD_NORTH_TRUE] = "north_true", >> + [IIO_MOD_NORTH_MAGN_TILT_COMP] = "north_magnetic_tilt_comp", >> + [IIO_MOD_NORTH_TRUE_TILT_COMP] = "north_true_tilt_comp", >> }; >> >> /* relies on pairs of these shared then separate */ >> diff --git a/include/linux/iio/types.h b/include/linux/iio/types.h >> index 084d882..5bf8847 100644 >> --- a/include/linux/iio/types.h >> +++ b/include/linux/iio/types.h >> @@ -53,6 +53,10 @@ enum iio_modifier { >> IIO_MOD_LIGHT_RED, >> IIO_MOD_LIGHT_GREEN, >> IIO_MOD_LIGHT_BLUE, >> + IIO_MOD_NORTH_MAGN, >> + IIO_MOD_NORTH_TRUE, >> + IIO_MOD_NORTH_MAGN_TILT_COMP, >> + IIO_MOD_NORTH_TRUE_TILT_COMP >> }; >> >> enum iio_event_type { >> > ^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCHv2 3/3] IIO: hid-sensor-magn-3d: Add in support for True/Magnetic North HID usages 2014-06-02 23:14 [PATCHv2 0/3] IIO: Add support for compass north usage attribute Reyad Attiyat [not found] ` <1401750890-31854-1-git-send-email-reyad.attiyat-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> @ 2014-06-02 23:14 ` Reyad Attiyat [not found] ` <1401750890-31854-4-git-send-email-reyad.attiyat-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 1 sibling, 1 reply; 23+ messages in thread From: Reyad Attiyat @ 2014-06-02 23:14 UTC (permalink / raw) To: linux-kernel, linux-iio, jic23, srinivas.pandruvada, linux-input, jkosina Cc: Reyad Attiyat Updated magn_3d_channel enum for all possible north channels Added functions to setup iio_chan_spec array depending on which hid usage reports are found Renamed magn_val to iio_val to differentiate the index being used Updated magn_3d_state struct to hold pointer array (magn_val_addr[]) which points to iio_val[] Updated magn_3d_parse_report to scan for all compass usages and create iio channels for each Signed-off-by: Reyad Attiyat <reyad.attiyat@gmail.com> --- drivers/iio/magnetometer/hid-sensor-magn-3d.c | 271 +++++++++++++++++--------- 1 file changed, 177 insertions(+), 94 deletions(-) diff --git a/drivers/iio/magnetometer/hid-sensor-magn-3d.c b/drivers/iio/magnetometer/hid-sensor-magn-3d.c index 6d162b7..32f4d90 100644 --- a/drivers/iio/magnetometer/hid-sensor-magn-3d.c +++ b/drivers/iio/magnetometer/hid-sensor-magn-3d.c @@ -34,63 +34,54 @@ enum magn_3d_channel { CHANNEL_SCAN_INDEX_X, CHANNEL_SCAN_INDEX_Y, CHANNEL_SCAN_INDEX_Z, + CHANNEL_SCAN_INDEX_NORTH_MAGN, + CHANNEL_SCAN_INDEX_NORTH_TRUE, + CHANNEL_SCAN_INDEX_NORTH_TILT_COMP, + CHANNEL_SCAN_INDEX_NORTH_TRUE_TILT_COMP, MAGN_3D_CHANNEL_MAX, }; +#define IIO_CHANNEL_MAX MAGN_3D_CHANNEL_MAX + struct magn_3d_state { struct hid_sensor_hub_callbacks callbacks; struct hid_sensor_common common_attributes; struct hid_sensor_hub_attribute_info magn[MAGN_3D_CHANNEL_MAX]; - u32 magn_val[MAGN_3D_CHANNEL_MAX]; -}; + u32 *magn_val_addr[MAGN_3D_CHANNEL_MAX]; -static const u32 magn_3d_addresses[MAGN_3D_CHANNEL_MAX] = { - HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_X_AXIS, - HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Y_AXIS, - HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Z_AXIS + u32 iio_val[IIO_CHANNEL_MAX]; + int num_iio_channels; }; -/* Channel definitions */ -static const struct iio_chan_spec magn_3d_channels[] = { - { - .type = IIO_MAGN, - .modified = 1, - .channel2 = IIO_MOD_X, - .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) | - BIT(IIO_CHAN_INFO_SCALE) | - BIT(IIO_CHAN_INFO_SAMP_FREQ) | - BIT(IIO_CHAN_INFO_HYSTERESIS), - .scan_index = CHANNEL_SCAN_INDEX_X, - }, { - .type = IIO_MAGN, - .modified = 1, - .channel2 = IIO_MOD_Y, - .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) | - BIT(IIO_CHAN_INFO_SCALE) | - BIT(IIO_CHAN_INFO_SAMP_FREQ) | - BIT(IIO_CHAN_INFO_HYSTERESIS), - .scan_index = CHANNEL_SCAN_INDEX_Y, - }, { - .type = IIO_MAGN, - .modified = 1, - .channel2 = IIO_MOD_Z, - .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) | - BIT(IIO_CHAN_INFO_SCALE) | - BIT(IIO_CHAN_INFO_SAMP_FREQ) | - BIT(IIO_CHAN_INFO_HYSTERESIS), - .scan_index = CHANNEL_SCAN_INDEX_Z, +/* Find index into magn_3d_state magn[] and magn_val_addr[] from HID Usage */ +static int magn_3d_usage_id_to_chan_index(unsigned usage_id){ + int offset = -1; + + switch (usage_id) { + case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_X_AXIS: + offset = CHANNEL_SCAN_INDEX_X; + break; + case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Y_AXIS: + offset = CHANNEL_SCAN_INDEX_Y; + break; + case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Z_AXIS: + offset = CHANNEL_SCAN_INDEX_Z; + break; + case HID_USAGE_SENSOR_ORIENT_COMP_MAGN_NORTH: + offset = CHANNEL_SCAN_INDEX_NORTH_TILT_COMP; + break; + case HID_USAGE_SENSOR_ORIENT_COMP_TRUE_NORTH: + offset = CHANNEL_SCAN_INDEX_NORTH_TRUE_TILT_COMP; + break; + case HID_USAGE_SENSOR_ORIENT_MAGN_NORTH: + offset = CHANNEL_SCAN_INDEX_NORTH_MAGN; + break; + case HID_USAGE_SENSOR_ORIENT_TRUE_NORTH: + offset = CHANNEL_SCAN_INDEX_NORTH_TRUE; + break; } -}; -/* Adjust channel real bits based on report descriptor */ -static void magn_3d_adjust_channel_bit_mask(struct iio_chan_spec *channels, - int channel, int size) -{ - channels[channel].scan_type.sign = 's'; - /* Real storage bits will change based on the report desc. */ - channels[channel].scan_type.realbits = size * 8; - /* Maximum size of a sample to capture is u32 */ - channels[channel].scan_type.storagebits = sizeof(u32) * 8; + return offset; } /* Channel read_raw handler */ @@ -101,21 +92,31 @@ static int magn_3d_read_raw(struct iio_dev *indio_dev, { struct magn_3d_state *magn_state = iio_priv(indio_dev); int report_id = -1; - u32 address; + unsigned usage_id; + int chan_index = -1; int ret; int ret_type; + dev_dbg(&indio_dev->dev, "magn_3d_read_raw\n"); + *val = 0; *val2 = 0; switch (mask) { case 0: + /* We store the HID usage ID of the iio channel + * in its address field + */ + usage_id = chan->address; + chan_index = magn_3d_usage_id_to_chan_index(usage_id); + if(chan_index < 0) + return -EINVAL; + report_id = - magn_state->magn[chan->scan_index].report_id; - address = magn_3d_addresses[chan->scan_index]; + magn_state->magn[chan_index].report_id; if (report_id >= 0) *val = sensor_hub_input_attr_get_raw_value( magn_state->common_attributes.hsdev, - HID_USAGE_SENSOR_COMPASS_3D, address, + HID_USAGE_SENSOR_COMPASS_3D, usage_id, report_id); else { *val = 0; @@ -202,12 +203,13 @@ static int magn_3d_proc_event(struct hid_sensor_hub_device *hsdev, magn_state->common_attributes.data_ready); if (magn_state->common_attributes.data_ready) hid_sensor_push_data(indio_dev, - magn_state->magn_val, - sizeof(magn_state->magn_val)); + &(magn_state->iio_val), + sizeof(magn_state->iio_val)); return 0; } + /* Capture samples in local storage */ static int magn_3d_capture_sample(struct hid_sensor_hub_device *hsdev, unsigned usage_id, @@ -217,63 +219,143 @@ static int magn_3d_capture_sample(struct hid_sensor_hub_device *hsdev, struct iio_dev *indio_dev = platform_get_drvdata(priv); struct magn_3d_state *magn_state = iio_priv(indio_dev); int offset; + u32 *magn_val; int ret = -EINVAL; - switch (usage_id) { + offset = magn_3d_usage_id_to_chan_index(usage_id); + if(offset < 0) + return ret; + + magn_val = magn_state->magn_val_addr[offset]; + if(!magn_val) + return ret; + + *(magn_val) = *(u32 *)raw_data; + + return 0; +} + +/* Setup the iio_chan_spec for HID Usage ID */ +static int magn_3d_setup_new_iio_chan(struct hid_sensor_hub_device *hsdev, + unsigned usage_id, + unsigned attr_usage_id, + struct iio_chan_spec *iio_chans, + struct magn_3d_state *st) +{ + int ret = -1; + int iio_index; + int magn_index; + struct iio_chan_spec *channel; + + /* Setup magn_3d_state for new channel */ + magn_index = magn_3d_usage_id_to_chan_index(attr_usage_id); + if(magn_index < 0 || magn_index >= MAGN_3D_CHANNEL_MAX){ + return -1; + } + + /* Check if usage attribute exists in the sensor hub device */ + ret = sensor_hub_input_get_attribute_info(hsdev, + HID_INPUT_REPORT, + usage_id, + attr_usage_id, + &(st->magn[magn_index])); + if(ret != 0){ + /* Usage not found. Nothing to setup.*/ + return 0; + } + + iio_index = st->num_iio_channels; + if(iio_index < 0 || iio_index >= IIO_CHANNEL_MAX){ + return -2; + } + + /* Check if this usage hasn't been setup */ + if(st->magn_val_addr[magn_index] != NULL){ + return -3; + } + st->magn_val_addr[magn_index] = &(st->iio_val[iio_index]); + + /* Setup IIO Channel spec */ + channel = &(iio_chans[iio_index]); + + channel->type = IIO_MAGN; + channel->address = attr_usage_id; + channel->modified = 1; + + switch (attr_usage_id){ + case HID_USAGE_SENSOR_ORIENT_COMP_MAGN_NORTH: + channel->channel2 = IIO_MOD_NORTH_MAGN_TILT_COMP; + break; + case HID_USAGE_SENSOR_ORIENT_COMP_TRUE_NORTH: + channel->channel2 = IIO_MOD_NORTH_TRUE_TILT_COMP; + break; + case HID_USAGE_SENSOR_ORIENT_MAGN_NORTH: + channel->channel2 = IIO_MOD_NORTH_MAGN; + break; + case HID_USAGE_SENSOR_ORIENT_TRUE_NORTH: + channel->channel2 = IIO_MOD_NORTH_TRUE; + break; case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_X_AXIS: + channel->channel2 = IIO_MOD_X; + break; case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Y_AXIS: + channel->channel2 = IIO_MOD_Y; + break; case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Z_AXIS: - offset = usage_id - HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_X_AXIS; - magn_state->magn_val[CHANNEL_SCAN_INDEX_X + offset] = - *(u32 *)raw_data; - ret = 0; - break; - default: + channel->channel2 = IIO_MOD_Z; break; + default: + return -4; } + channel->info_mask_shared_by_type = + BIT(IIO_CHAN_INFO_OFFSET) | + BIT(IIO_CHAN_INFO_SCALE) | + BIT(IIO_CHAN_INFO_SAMP_FREQ) | + BIT(IIO_CHAN_INFO_HYSTERESIS); + + channel->scan_type.sign = 's'; + /* Real storage bits will change based on the report desc. */ + channel->scan_type.realbits = st->magn[magn_index].size * 8; + /* Maximum size of a sample to capture is u32 */ + channel->scan_type.storagebits = sizeof(u32) * 8; + + (st->num_iio_channels)++; + ret = 0; + return ret; } -/* Parse report which is specific to an usage id*/ +/* Read the HID reports and setup IIO Channels */ static int magn_3d_parse_report(struct platform_device *pdev, struct hid_sensor_hub_device *hsdev, - struct iio_chan_spec *channels, + struct iio_chan_spec *iio_chans, unsigned usage_id, struct magn_3d_state *st) { - int ret; + int ret = 0; int i; - for (i = 0; i <= CHANNEL_SCAN_INDEX_Z; ++i) { - ret = sensor_hub_input_get_attribute_info(hsdev, - HID_INPUT_REPORT, - usage_id, - HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_X_AXIS + i, - &st->magn[CHANNEL_SCAN_INDEX_X + i]); - if (ret < 0) - break; - magn_3d_adjust_channel_bit_mask(channels, - CHANNEL_SCAN_INDEX_X + i, - st->magn[CHANNEL_SCAN_INDEX_X + i].size); - } - dev_dbg(&pdev->dev, "magn_3d %x:%x, %x:%x, %x:%x\n", - st->magn[0].index, - st->magn[0].report_id, - st->magn[1].index, st->magn[1].report_id, - st->magn[2].index, st->magn[2].report_id); - - /* Set Sensitivity field ids, when there is no individual modifier */ - if (st->common_attributes.sensitivity.index < 0) { - sensor_hub_input_get_attribute_info(hsdev, - HID_FEATURE_REPORT, usage_id, - HID_USAGE_SENSOR_DATA_MOD_CHANGE_SENSITIVITY_ABS | - HID_USAGE_SENSOR_DATA_ORIENTATION, - &st->common_attributes.sensitivity); - dev_dbg(&pdev->dev, "Sensitivity index:report %d:%d\n", - st->common_attributes.sensitivity.index, - st->common_attributes.sensitivity.report_id); - } + dev_dbg(&pdev->dev, "magn_3d_parse_reports Usage ID: %x\n", usage_id); + for(i = HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_X_AXIS; + i <= HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Z_AXIS && ret == 0; + i++) + ret = magn_3d_setup_new_iio_chan(hsdev, + usage_id, + i, + iio_chans, + st); + + dev_dbg(&pdev->dev, "magn_3d_parse_reports Found %d\n magnetic axis. Returned: %d", st->num_iio_channels, ret); + for(i = HID_USAGE_SENSOR_ORIENT_COMP_MAGN_NORTH; + i <= HID_USAGE_SENSOR_ORIENT_TRUE_NORTH && ret == 0; + i++) + ret = magn_3d_setup_new_iio_chan(hsdev, + usage_id, + i, + iio_chans, + st); + dev_dbg(&pdev->dev, "magn_3d_parse_reports Found %d\n magnetic channels. Returned: %d", st->num_iio_channels, ret); return ret; } @@ -307,10 +389,11 @@ static int hid_magn_3d_probe(struct platform_device *pdev) return ret; } - channels = kmemdup(magn_3d_channels, sizeof(magn_3d_channels), - GFP_KERNEL); + channels = kcalloc(MAGN_3D_CHANNEL_MAX, + sizeof(struct iio_chan_spec), + GFP_KERNEL); if (!channels) { - dev_err(&pdev->dev, "failed to duplicate channels\n"); + dev_err(&pdev->dev, "failed to allocate memory for iio channel\n"); return -ENOMEM; } @@ -322,7 +405,7 @@ static int hid_magn_3d_probe(struct platform_device *pdev) } indio_dev->channels = channels; - indio_dev->num_channels = ARRAY_SIZE(magn_3d_channels); + indio_dev->num_channels = magn_state->num_iio_channels; indio_dev->dev.parent = &pdev->dev; indio_dev->info = &magn_3d_info; indio_dev->name = name; -- 1.9.3 ^ permalink raw reply related [flat|nested] 23+ messages in thread
[parent not found: <1401750890-31854-4-git-send-email-reyad.attiyat-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]
* Re: [PATCHv2 3/3] IIO: hid-sensor-magn-3d: Add in support for True/Magnetic North HID usages 2014-06-02 23:14 ` [PATCHv2 3/3] IIO: hid-sensor-magn-3d: Add in support " Reyad Attiyat @ 2014-06-03 17:43 ` Srinivas Pandruvada 0 siblings, 0 replies; 23+ messages in thread From: Srinivas Pandruvada @ 2014-06-03 17:43 UTC (permalink / raw) To: Reyad Attiyat Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-iio-u79uwXL29TY76Z2rM5mHXA, jic23-DgEjT+Ai2ygdnm+yROfE0A, linux-input-u79uwXL29TY76Z2rM5mHXA, jkosina-AlSwsSmVLrQ On 06/02/2014 04:14 PM, Reyad Attiyat wrote: > Updated magn_3d_channel enum for all possible north channels > > Added functions to setup iio_chan_spec array depending on which hid usage reports are found > > Renamed magn_val to iio_val to differentiate the index being used > > Updated magn_3d_state struct to hold pointer array (magn_val_addr[]) which points to iio_val[] > > Updated magn_3d_parse_report to scan for all compass usages and create iio channels for each > > Signed-off-by: Reyad Attiyat <reyad.attiyat-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> > --- > drivers/iio/magnetometer/hid-sensor-magn-3d.c | 271 +++++++++++++++++--------- > 1 file changed, 177 insertions(+), 94 deletions(-) > > diff --git a/drivers/iio/magnetometer/hid-sensor-magn-3d.c b/drivers/iio/magnetometer/hid-sensor-magn-3d.c > index 6d162b7..32f4d90 100644 > --- a/drivers/iio/magnetometer/hid-sensor-magn-3d.c > +++ b/drivers/iio/magnetometer/hid-sensor-magn-3d.c > @@ -34,63 +34,54 @@ enum magn_3d_channel { > CHANNEL_SCAN_INDEX_X, > CHANNEL_SCAN_INDEX_Y, > CHANNEL_SCAN_INDEX_Z, > + CHANNEL_SCAN_INDEX_NORTH_MAGN, > + CHANNEL_SCAN_INDEX_NORTH_TRUE, > + CHANNEL_SCAN_INDEX_NORTH_TILT_COMP, > + CHANNEL_SCAN_INDEX_NORTH_TRUE_TILT_COMP, > MAGN_3D_CHANNEL_MAX, > }; > > +#define IIO_CHANNEL_MAX MAGN_3D_CHANNEL_MAX > + > struct magn_3d_state { > struct hid_sensor_hub_callbacks callbacks; > struct hid_sensor_common common_attributes; > struct hid_sensor_hub_attribute_info magn[MAGN_3D_CHANNEL_MAX]; > - u32 magn_val[MAGN_3D_CHANNEL_MAX]; > -}; > + u32 *magn_val_addr[MAGN_3D_CHANNEL_MAX]; > > -static const u32 magn_3d_addresses[MAGN_3D_CHANNEL_MAX] = { > - HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_X_AXIS, > - HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Y_AXIS, > - HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Z_AXIS > + u32 iio_val[IIO_CHANNEL_MAX]; > + int num_iio_channels; > }; > > -/* Channel definitions */ > -static const struct iio_chan_spec magn_3d_channels[] = { > - { > - .type = IIO_MAGN, > - .modified = 1, > - .channel2 = IIO_MOD_X, > - .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) | > - BIT(IIO_CHAN_INFO_SCALE) | > - BIT(IIO_CHAN_INFO_SAMP_FREQ) | > - BIT(IIO_CHAN_INFO_HYSTERESIS), > - .scan_index = CHANNEL_SCAN_INDEX_X, > - }, { > - .type = IIO_MAGN, > - .modified = 1, > - .channel2 = IIO_MOD_Y, > - .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) | > - BIT(IIO_CHAN_INFO_SCALE) | > - BIT(IIO_CHAN_INFO_SAMP_FREQ) | > - BIT(IIO_CHAN_INFO_HYSTERESIS), > - .scan_index = CHANNEL_SCAN_INDEX_Y, > - }, { > - .type = IIO_MAGN, > - .modified = 1, > - .channel2 = IIO_MOD_Z, > - .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) | > - BIT(IIO_CHAN_INFO_SCALE) | > - BIT(IIO_CHAN_INFO_SAMP_FREQ) | > - BIT(IIO_CHAN_INFO_HYSTERESIS), > - .scan_index = CHANNEL_SCAN_INDEX_Z, May be we should have a field in const struct iio_chan_spec, so that we can dynamically enable disable channels. In this way we can statically define channels, but can be enabled/disabled dynamically. > +/* Find index into magn_3d_state magn[] and magn_val_addr[] from HID Usage */ > +static int magn_3d_usage_id_to_chan_index(unsigned usage_id){ > + int offset = -1; > + > + switch (usage_id) { > + case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_X_AXIS: > + offset = CHANNEL_SCAN_INDEX_X; > + break; > + case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Y_AXIS: > + offset = CHANNEL_SCAN_INDEX_Y; > + break; > + case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Z_AXIS: > + offset = CHANNEL_SCAN_INDEX_Z; > + break; > + case HID_USAGE_SENSOR_ORIENT_COMP_MAGN_NORTH: > + offset = CHANNEL_SCAN_INDEX_NORTH_TILT_COMP; > + break; > + case HID_USAGE_SENSOR_ORIENT_COMP_TRUE_NORTH: > + offset = CHANNEL_SCAN_INDEX_NORTH_TRUE_TILT_COMP; > + break; > + case HID_USAGE_SENSOR_ORIENT_MAGN_NORTH: > + offset = CHANNEL_SCAN_INDEX_NORTH_MAGN; > + break; > + case HID_USAGE_SENSOR_ORIENT_TRUE_NORTH: > + offset = CHANNEL_SCAN_INDEX_NORTH_TRUE; > + break; > } > -}; > > -/* Adjust channel real bits based on report descriptor */ > -static void magn_3d_adjust_channel_bit_mask(struct iio_chan_spec *channels, > - int channel, int size) > -{ > - channels[channel].scan_type.sign = 's'; > - /* Real storage bits will change based on the report desc. */ > - channels[channel].scan_type.realbits = size * 8; > - /* Maximum size of a sample to capture is u32 */ > - channels[channel].scan_type.storagebits = sizeof(u32) * 8; > + return offset; > } > > /* Channel read_raw handler */ > @@ -101,21 +92,31 @@ static int magn_3d_read_raw(struct iio_dev *indio_dev, > { > struct magn_3d_state *magn_state = iio_priv(indio_dev); > int report_id = -1; > - u32 address; > + unsigned usage_id; > + int chan_index = -1; > int ret; > int ret_type; > > + dev_dbg(&indio_dev->dev, "magn_3d_read_raw\n"); > + > *val = 0; > *val2 = 0; > switch (mask) { > case 0: > + /* We store the HID usage ID of the iio channel > + * in its address field > + */ > + usage_id = chan->address; > + chan_index = magn_3d_usage_id_to_chan_index(usage_id); > + if(chan_index < 0) > + return -EINVAL; > + > report_id = > - magn_state->magn[chan->scan_index].report_id; > - address = magn_3d_addresses[chan->scan_index]; > + magn_state->magn[chan_index].report_id; > if (report_id >= 0) > *val = sensor_hub_input_attr_get_raw_value( > magn_state->common_attributes.hsdev, > - HID_USAGE_SENSOR_COMPASS_3D, address, > + HID_USAGE_SENSOR_COMPASS_3D, usage_id, > report_id); > else { > *val = 0; > @@ -202,12 +203,13 @@ static int magn_3d_proc_event(struct hid_sensor_hub_device *hsdev, > magn_state->common_attributes.data_ready); > if (magn_state->common_attributes.data_ready) > hid_sensor_push_data(indio_dev, > - magn_state->magn_val, > - sizeof(magn_state->magn_val)); > + &(magn_state->iio_val), > + sizeof(magn_state->iio_val)); > > return 0; > } > > + > /* Capture samples in local storage */ > static int magn_3d_capture_sample(struct hid_sensor_hub_device *hsdev, > unsigned usage_id, > @@ -217,63 +219,143 @@ static int magn_3d_capture_sample(struct hid_sensor_hub_device *hsdev, > struct iio_dev *indio_dev = platform_get_drvdata(priv); > struct magn_3d_state *magn_state = iio_priv(indio_dev); > int offset; > + u32 *magn_val; > int ret = -EINVAL; > > - switch (usage_id) { > + offset = magn_3d_usage_id_to_chan_index(usage_id); > + if(offset < 0) > + return ret; > + > + magn_val = magn_state->magn_val_addr[offset]; > + if(!magn_val) > + return ret; > + > + *(magn_val) = *(u32 *)raw_data; > + > + return 0; > +} > + > +/* Setup the iio_chan_spec for HID Usage ID */ > +static int magn_3d_setup_new_iio_chan(struct hid_sensor_hub_device *hsdev, > + unsigned usage_id, > + unsigned attr_usage_id, > + struct iio_chan_spec *iio_chans, > + struct magn_3d_state *st) > +{ > + int ret = -1; > + int iio_index; > + int magn_index; > + struct iio_chan_spec *channel; > + > + /* Setup magn_3d_state for new channel */ > + magn_index = magn_3d_usage_id_to_chan_index(attr_usage_id); > + if(magn_index < 0 || magn_index >= MAGN_3D_CHANNEL_MAX){ > + return -1; > + } > + > + /* Check if usage attribute exists in the sensor hub device */ > + ret = sensor_hub_input_get_attribute_info(hsdev, > + HID_INPUT_REPORT, > + usage_id, > + attr_usage_id, > + &(st->magn[magn_index])); > + if(ret != 0){ > + /* Usage not found. Nothing to setup.*/ > + return 0; > + } > + > + iio_index = st->num_iio_channels; > + if(iio_index < 0 || iio_index >= IIO_CHANNEL_MAX){ > + return -2; > + } > + > + /* Check if this usage hasn't been setup */ > + if(st->magn_val_addr[magn_index] != NULL){ > + return -3; > + } > + st->magn_val_addr[magn_index] = &(st->iio_val[iio_index]); > + > + /* Setup IIO Channel spec */ > + channel = &(iio_chans[iio_index]); > + > + channel->type = IIO_MAGN; > + channel->address = attr_usage_id; > + channel->modified = 1; > + > + switch (attr_usage_id){ > + case HID_USAGE_SENSOR_ORIENT_COMP_MAGN_NORTH: > + channel->channel2 = IIO_MOD_NORTH_MAGN_TILT_COMP; > + break; > + case HID_USAGE_SENSOR_ORIENT_COMP_TRUE_NORTH: > + channel->channel2 = IIO_MOD_NORTH_TRUE_TILT_COMP; > + break; > + case HID_USAGE_SENSOR_ORIENT_MAGN_NORTH: > + channel->channel2 = IIO_MOD_NORTH_MAGN; > + break; > + case HID_USAGE_SENSOR_ORIENT_TRUE_NORTH: > + channel->channel2 = IIO_MOD_NORTH_TRUE; > + break; > case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_X_AXIS: > + channel->channel2 = IIO_MOD_X; > + break; > case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Y_AXIS: > + channel->channel2 = IIO_MOD_Y; > + break; > case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Z_AXIS: > - offset = usage_id - HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_X_AXIS; > - magn_state->magn_val[CHANNEL_SCAN_INDEX_X + offset] = > - *(u32 *)raw_data; > - ret = 0; > - break; > - default: > + channel->channel2 = IIO_MOD_Z; > break; > + default: > + return -4; > } > > + channel->info_mask_shared_by_type = > + BIT(IIO_CHAN_INFO_OFFSET) | > + BIT(IIO_CHAN_INFO_SCALE) | > + BIT(IIO_CHAN_INFO_SAMP_FREQ) | > + BIT(IIO_CHAN_INFO_HYSTERESIS); > + > + channel->scan_type.sign = 's'; > + /* Real storage bits will change based on the report desc. */ > + channel->scan_type.realbits = st->magn[magn_index].size * 8; > + /* Maximum size of a sample to capture is u32 */ > + channel->scan_type.storagebits = sizeof(u32) * 8; > + > + (st->num_iio_channels)++; > + ret = 0; > + > return ret; > } > > -/* Parse report which is specific to an usage id*/ > +/* Read the HID reports and setup IIO Channels */ > static int magn_3d_parse_report(struct platform_device *pdev, > struct hid_sensor_hub_device *hsdev, > - struct iio_chan_spec *channels, > + struct iio_chan_spec *iio_chans, > unsigned usage_id, > struct magn_3d_state *st) > { > - int ret; > + int ret = 0; > int i; > > - for (i = 0; i <= CHANNEL_SCAN_INDEX_Z; ++i) { > - ret = sensor_hub_input_get_attribute_info(hsdev, > - HID_INPUT_REPORT, > - usage_id, > - HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_X_AXIS + i, > - &st->magn[CHANNEL_SCAN_INDEX_X + i]); > - if (ret < 0) > - break; > - magn_3d_adjust_channel_bit_mask(channels, > - CHANNEL_SCAN_INDEX_X + i, > - st->magn[CHANNEL_SCAN_INDEX_X + i].size); > - } > - dev_dbg(&pdev->dev, "magn_3d %x:%x, %x:%x, %x:%x\n", > - st->magn[0].index, > - st->magn[0].report_id, > - st->magn[1].index, st->magn[1].report_id, > - st->magn[2].index, st->magn[2].report_id); > - > - /* Set Sensitivity field ids, when there is no individual modifier */ > - if (st->common_attributes.sensitivity.index < 0) { > - sensor_hub_input_get_attribute_info(hsdev, > - HID_FEATURE_REPORT, usage_id, > - HID_USAGE_SENSOR_DATA_MOD_CHANGE_SENSITIVITY_ABS | > - HID_USAGE_SENSOR_DATA_ORIENTATION, > - &st->common_attributes.sensitivity); > - dev_dbg(&pdev->dev, "Sensitivity index:report %d:%d\n", > - st->common_attributes.sensitivity.index, > - st->common_attributes.sensitivity.report_id); > - } > + dev_dbg(&pdev->dev, "magn_3d_parse_reports Usage ID: %x\n", usage_id); > + for(i = HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_X_AXIS; > + i <= HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Z_AXIS && ret == 0; > + i++) > + ret = magn_3d_setup_new_iio_chan(hsdev, > + usage_id, > + i, > + iio_chans, > + st); > + > + dev_dbg(&pdev->dev, "magn_3d_parse_reports Found %d\n magnetic axis. Returned: %d", st->num_iio_channels, ret); > + for(i = HID_USAGE_SENSOR_ORIENT_COMP_MAGN_NORTH; > + i <= HID_USAGE_SENSOR_ORIENT_TRUE_NORTH && ret == 0; > + i++) > + ret = magn_3d_setup_new_iio_chan(hsdev, > + usage_id, > + i, > + iio_chans, > + st); > + dev_dbg(&pdev->dev, "magn_3d_parse_reports Found %d\n magnetic channels. Returned: %d", st->num_iio_channels, ret); > I think we need to present angle in radians. Are you basing change present in linux-next? This will automatically do in this function. > return ret; > } > @@ -307,10 +389,11 @@ static int hid_magn_3d_probe(struct platform_device *pdev) > return ret; > } > > - channels = kmemdup(magn_3d_channels, sizeof(magn_3d_channels), > - GFP_KERNEL); > + channels = kcalloc(MAGN_3D_CHANNEL_MAX, > + sizeof(struct iio_chan_spec), > + GFP_KERNEL); I don't see kfree. Try devm_kcalloc? > if (!channels) { > - dev_err(&pdev->dev, "failed to duplicate channels\n"); > + dev_err(&pdev->dev, "failed to allocate memory for iio channel\n"); > return -ENOMEM; > } > > @@ -322,7 +405,7 @@ static int hid_magn_3d_probe(struct platform_device *pdev) > } > > indio_dev->channels = channels; > - indio_dev->num_channels = ARRAY_SIZE(magn_3d_channels); > + indio_dev->num_channels = magn_state->num_iio_channels; > indio_dev->dev.parent = &pdev->dev; > indio_dev->info = &magn_3d_info; > indio_dev->name = name; > Thanks, Srinivas ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCHv2 3/3] IIO: hid-sensor-magn-3d: Add in support for True/Magnetic North HID usages @ 2014-06-03 17:43 ` Srinivas Pandruvada 0 siblings, 0 replies; 23+ messages in thread From: Srinivas Pandruvada @ 2014-06-03 17:43 UTC (permalink / raw) To: Reyad Attiyat; +Cc: linux-kernel, linux-iio, jic23, linux-input, jkosina On 06/02/2014 04:14 PM, Reyad Attiyat wrote: > Updated magn_3d_channel enum for all possible north channels > > Added functions to setup iio_chan_spec array depending on which hid usage reports are found > > Renamed magn_val to iio_val to differentiate the index being used > > Updated magn_3d_state struct to hold pointer array (magn_val_addr[]) which points to iio_val[] > > Updated magn_3d_parse_report to scan for all compass usages and create iio channels for each > > Signed-off-by: Reyad Attiyat <reyad.attiyat@gmail.com> > --- > drivers/iio/magnetometer/hid-sensor-magn-3d.c | 271 +++++++++++++++++--------- > 1 file changed, 177 insertions(+), 94 deletions(-) > > diff --git a/drivers/iio/magnetometer/hid-sensor-magn-3d.c b/drivers/iio/magnetometer/hid-sensor-magn-3d.c > index 6d162b7..32f4d90 100644 > --- a/drivers/iio/magnetometer/hid-sensor-magn-3d.c > +++ b/drivers/iio/magnetometer/hid-sensor-magn-3d.c > @@ -34,63 +34,54 @@ enum magn_3d_channel { > CHANNEL_SCAN_INDEX_X, > CHANNEL_SCAN_INDEX_Y, > CHANNEL_SCAN_INDEX_Z, > + CHANNEL_SCAN_INDEX_NORTH_MAGN, > + CHANNEL_SCAN_INDEX_NORTH_TRUE, > + CHANNEL_SCAN_INDEX_NORTH_TILT_COMP, > + CHANNEL_SCAN_INDEX_NORTH_TRUE_TILT_COMP, > MAGN_3D_CHANNEL_MAX, > }; > > +#define IIO_CHANNEL_MAX MAGN_3D_CHANNEL_MAX > + > struct magn_3d_state { > struct hid_sensor_hub_callbacks callbacks; > struct hid_sensor_common common_attributes; > struct hid_sensor_hub_attribute_info magn[MAGN_3D_CHANNEL_MAX]; > - u32 magn_val[MAGN_3D_CHANNEL_MAX]; > -}; > + u32 *magn_val_addr[MAGN_3D_CHANNEL_MAX]; > > -static const u32 magn_3d_addresses[MAGN_3D_CHANNEL_MAX] = { > - HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_X_AXIS, > - HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Y_AXIS, > - HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Z_AXIS > + u32 iio_val[IIO_CHANNEL_MAX]; > + int num_iio_channels; > }; > > -/* Channel definitions */ > -static const struct iio_chan_spec magn_3d_channels[] = { > - { > - .type = IIO_MAGN, > - .modified = 1, > - .channel2 = IIO_MOD_X, > - .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) | > - BIT(IIO_CHAN_INFO_SCALE) | > - BIT(IIO_CHAN_INFO_SAMP_FREQ) | > - BIT(IIO_CHAN_INFO_HYSTERESIS), > - .scan_index = CHANNEL_SCAN_INDEX_X, > - }, { > - .type = IIO_MAGN, > - .modified = 1, > - .channel2 = IIO_MOD_Y, > - .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) | > - BIT(IIO_CHAN_INFO_SCALE) | > - BIT(IIO_CHAN_INFO_SAMP_FREQ) | > - BIT(IIO_CHAN_INFO_HYSTERESIS), > - .scan_index = CHANNEL_SCAN_INDEX_Y, > - }, { > - .type = IIO_MAGN, > - .modified = 1, > - .channel2 = IIO_MOD_Z, > - .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) | > - BIT(IIO_CHAN_INFO_SCALE) | > - BIT(IIO_CHAN_INFO_SAMP_FREQ) | > - BIT(IIO_CHAN_INFO_HYSTERESIS), > - .scan_index = CHANNEL_SCAN_INDEX_Z, May be we should have a field in const struct iio_chan_spec, so that we can dynamically enable disable channels. In this way we can statically define channels, but can be enabled/disabled dynamically. > +/* Find index into magn_3d_state magn[] and magn_val_addr[] from HID Usage */ > +static int magn_3d_usage_id_to_chan_index(unsigned usage_id){ > + int offset = -1; > + > + switch (usage_id) { > + case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_X_AXIS: > + offset = CHANNEL_SCAN_INDEX_X; > + break; > + case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Y_AXIS: > + offset = CHANNEL_SCAN_INDEX_Y; > + break; > + case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Z_AXIS: > + offset = CHANNEL_SCAN_INDEX_Z; > + break; > + case HID_USAGE_SENSOR_ORIENT_COMP_MAGN_NORTH: > + offset = CHANNEL_SCAN_INDEX_NORTH_TILT_COMP; > + break; > + case HID_USAGE_SENSOR_ORIENT_COMP_TRUE_NORTH: > + offset = CHANNEL_SCAN_INDEX_NORTH_TRUE_TILT_COMP; > + break; > + case HID_USAGE_SENSOR_ORIENT_MAGN_NORTH: > + offset = CHANNEL_SCAN_INDEX_NORTH_MAGN; > + break; > + case HID_USAGE_SENSOR_ORIENT_TRUE_NORTH: > + offset = CHANNEL_SCAN_INDEX_NORTH_TRUE; > + break; > } > -}; > > -/* Adjust channel real bits based on report descriptor */ > -static void magn_3d_adjust_channel_bit_mask(struct iio_chan_spec *channels, > - int channel, int size) > -{ > - channels[channel].scan_type.sign = 's'; > - /* Real storage bits will change based on the report desc. */ > - channels[channel].scan_type.realbits = size * 8; > - /* Maximum size of a sample to capture is u32 */ > - channels[channel].scan_type.storagebits = sizeof(u32) * 8; > + return offset; > } > > /* Channel read_raw handler */ > @@ -101,21 +92,31 @@ static int magn_3d_read_raw(struct iio_dev *indio_dev, > { > struct magn_3d_state *magn_state = iio_priv(indio_dev); > int report_id = -1; > - u32 address; > + unsigned usage_id; > + int chan_index = -1; > int ret; > int ret_type; > > + dev_dbg(&indio_dev->dev, "magn_3d_read_raw\n"); > + > *val = 0; > *val2 = 0; > switch (mask) { > case 0: > + /* We store the HID usage ID of the iio channel > + * in its address field > + */ > + usage_id = chan->address; > + chan_index = magn_3d_usage_id_to_chan_index(usage_id); > + if(chan_index < 0) > + return -EINVAL; > + > report_id = > - magn_state->magn[chan->scan_index].report_id; > - address = magn_3d_addresses[chan->scan_index]; > + magn_state->magn[chan_index].report_id; > if (report_id >= 0) > *val = sensor_hub_input_attr_get_raw_value( > magn_state->common_attributes.hsdev, > - HID_USAGE_SENSOR_COMPASS_3D, address, > + HID_USAGE_SENSOR_COMPASS_3D, usage_id, > report_id); > else { > *val = 0; > @@ -202,12 +203,13 @@ static int magn_3d_proc_event(struct hid_sensor_hub_device *hsdev, > magn_state->common_attributes.data_ready); > if (magn_state->common_attributes.data_ready) > hid_sensor_push_data(indio_dev, > - magn_state->magn_val, > - sizeof(magn_state->magn_val)); > + &(magn_state->iio_val), > + sizeof(magn_state->iio_val)); > > return 0; > } > > + > /* Capture samples in local storage */ > static int magn_3d_capture_sample(struct hid_sensor_hub_device *hsdev, > unsigned usage_id, > @@ -217,63 +219,143 @@ static int magn_3d_capture_sample(struct hid_sensor_hub_device *hsdev, > struct iio_dev *indio_dev = platform_get_drvdata(priv); > struct magn_3d_state *magn_state = iio_priv(indio_dev); > int offset; > + u32 *magn_val; > int ret = -EINVAL; > > - switch (usage_id) { > + offset = magn_3d_usage_id_to_chan_index(usage_id); > + if(offset < 0) > + return ret; > + > + magn_val = magn_state->magn_val_addr[offset]; > + if(!magn_val) > + return ret; > + > + *(magn_val) = *(u32 *)raw_data; > + > + return 0; > +} > + > +/* Setup the iio_chan_spec for HID Usage ID */ > +static int magn_3d_setup_new_iio_chan(struct hid_sensor_hub_device *hsdev, > + unsigned usage_id, > + unsigned attr_usage_id, > + struct iio_chan_spec *iio_chans, > + struct magn_3d_state *st) > +{ > + int ret = -1; > + int iio_index; > + int magn_index; > + struct iio_chan_spec *channel; > + > + /* Setup magn_3d_state for new channel */ > + magn_index = magn_3d_usage_id_to_chan_index(attr_usage_id); > + if(magn_index < 0 || magn_index >= MAGN_3D_CHANNEL_MAX){ > + return -1; > + } > + > + /* Check if usage attribute exists in the sensor hub device */ > + ret = sensor_hub_input_get_attribute_info(hsdev, > + HID_INPUT_REPORT, > + usage_id, > + attr_usage_id, > + &(st->magn[magn_index])); > + if(ret != 0){ > + /* Usage not found. Nothing to setup.*/ > + return 0; > + } > + > + iio_index = st->num_iio_channels; > + if(iio_index < 0 || iio_index >= IIO_CHANNEL_MAX){ > + return -2; > + } > + > + /* Check if this usage hasn't been setup */ > + if(st->magn_val_addr[magn_index] != NULL){ > + return -3; > + } > + st->magn_val_addr[magn_index] = &(st->iio_val[iio_index]); > + > + /* Setup IIO Channel spec */ > + channel = &(iio_chans[iio_index]); > + > + channel->type = IIO_MAGN; > + channel->address = attr_usage_id; > + channel->modified = 1; > + > + switch (attr_usage_id){ > + case HID_USAGE_SENSOR_ORIENT_COMP_MAGN_NORTH: > + channel->channel2 = IIO_MOD_NORTH_MAGN_TILT_COMP; > + break; > + case HID_USAGE_SENSOR_ORIENT_COMP_TRUE_NORTH: > + channel->channel2 = IIO_MOD_NORTH_TRUE_TILT_COMP; > + break; > + case HID_USAGE_SENSOR_ORIENT_MAGN_NORTH: > + channel->channel2 = IIO_MOD_NORTH_MAGN; > + break; > + case HID_USAGE_SENSOR_ORIENT_TRUE_NORTH: > + channel->channel2 = IIO_MOD_NORTH_TRUE; > + break; > case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_X_AXIS: > + channel->channel2 = IIO_MOD_X; > + break; > case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Y_AXIS: > + channel->channel2 = IIO_MOD_Y; > + break; > case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Z_AXIS: > - offset = usage_id - HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_X_AXIS; > - magn_state->magn_val[CHANNEL_SCAN_INDEX_X + offset] = > - *(u32 *)raw_data; > - ret = 0; > - break; > - default: > + channel->channel2 = IIO_MOD_Z; > break; > + default: > + return -4; > } > > + channel->info_mask_shared_by_type = > + BIT(IIO_CHAN_INFO_OFFSET) | > + BIT(IIO_CHAN_INFO_SCALE) | > + BIT(IIO_CHAN_INFO_SAMP_FREQ) | > + BIT(IIO_CHAN_INFO_HYSTERESIS); > + > + channel->scan_type.sign = 's'; > + /* Real storage bits will change based on the report desc. */ > + channel->scan_type.realbits = st->magn[magn_index].size * 8; > + /* Maximum size of a sample to capture is u32 */ > + channel->scan_type.storagebits = sizeof(u32) * 8; > + > + (st->num_iio_channels)++; > + ret = 0; > + > return ret; > } > > -/* Parse report which is specific to an usage id*/ > +/* Read the HID reports and setup IIO Channels */ > static int magn_3d_parse_report(struct platform_device *pdev, > struct hid_sensor_hub_device *hsdev, > - struct iio_chan_spec *channels, > + struct iio_chan_spec *iio_chans, > unsigned usage_id, > struct magn_3d_state *st) > { > - int ret; > + int ret = 0; > int i; > > - for (i = 0; i <= CHANNEL_SCAN_INDEX_Z; ++i) { > - ret = sensor_hub_input_get_attribute_info(hsdev, > - HID_INPUT_REPORT, > - usage_id, > - HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_X_AXIS + i, > - &st->magn[CHANNEL_SCAN_INDEX_X + i]); > - if (ret < 0) > - break; > - magn_3d_adjust_channel_bit_mask(channels, > - CHANNEL_SCAN_INDEX_X + i, > - st->magn[CHANNEL_SCAN_INDEX_X + i].size); > - } > - dev_dbg(&pdev->dev, "magn_3d %x:%x, %x:%x, %x:%x\n", > - st->magn[0].index, > - st->magn[0].report_id, > - st->magn[1].index, st->magn[1].report_id, > - st->magn[2].index, st->magn[2].report_id); > - > - /* Set Sensitivity field ids, when there is no individual modifier */ > - if (st->common_attributes.sensitivity.index < 0) { > - sensor_hub_input_get_attribute_info(hsdev, > - HID_FEATURE_REPORT, usage_id, > - HID_USAGE_SENSOR_DATA_MOD_CHANGE_SENSITIVITY_ABS | > - HID_USAGE_SENSOR_DATA_ORIENTATION, > - &st->common_attributes.sensitivity); > - dev_dbg(&pdev->dev, "Sensitivity index:report %d:%d\n", > - st->common_attributes.sensitivity.index, > - st->common_attributes.sensitivity.report_id); > - } > + dev_dbg(&pdev->dev, "magn_3d_parse_reports Usage ID: %x\n", usage_id); > + for(i = HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_X_AXIS; > + i <= HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Z_AXIS && ret == 0; > + i++) > + ret = magn_3d_setup_new_iio_chan(hsdev, > + usage_id, > + i, > + iio_chans, > + st); > + > + dev_dbg(&pdev->dev, "magn_3d_parse_reports Found %d\n magnetic axis. Returned: %d", st->num_iio_channels, ret); > + for(i = HID_USAGE_SENSOR_ORIENT_COMP_MAGN_NORTH; > + i <= HID_USAGE_SENSOR_ORIENT_TRUE_NORTH && ret == 0; > + i++) > + ret = magn_3d_setup_new_iio_chan(hsdev, > + usage_id, > + i, > + iio_chans, > + st); > + dev_dbg(&pdev->dev, "magn_3d_parse_reports Found %d\n magnetic channels. Returned: %d", st->num_iio_channels, ret); > I think we need to present angle in radians. Are you basing change present in linux-next? This will automatically do in this function. > return ret; > } > @@ -307,10 +389,11 @@ static int hid_magn_3d_probe(struct platform_device *pdev) > return ret; > } > > - channels = kmemdup(magn_3d_channels, sizeof(magn_3d_channels), > - GFP_KERNEL); > + channels = kcalloc(MAGN_3D_CHANNEL_MAX, > + sizeof(struct iio_chan_spec), > + GFP_KERNEL); I don't see kfree. Try devm_kcalloc? > if (!channels) { > - dev_err(&pdev->dev, "failed to duplicate channels\n"); > + dev_err(&pdev->dev, "failed to allocate memory for iio channel\n"); > return -ENOMEM; > } > > @@ -322,7 +405,7 @@ static int hid_magn_3d_probe(struct platform_device *pdev) > } > > indio_dev->channels = channels; > - indio_dev->num_channels = ARRAY_SIZE(magn_3d_channels); > + indio_dev->num_channels = magn_state->num_iio_channels; > indio_dev->dev.parent = &pdev->dev; > indio_dev->info = &magn_3d_info; > indio_dev->name = name; > Thanks, Srinivas ^ permalink raw reply [flat|nested] 23+ messages in thread
[parent not found: <538E092F.9040004-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>]
* Re: [PATCHv2 3/3] IIO: hid-sensor-magn-3d: Add in support for True/Magnetic North HID usages 2014-06-03 17:43 ` Srinivas Pandruvada @ 2014-06-04 15:23 ` Reyad Attiyat -1 siblings, 0 replies; 23+ messages in thread From: Reyad Attiyat @ 2014-06-04 15:23 UTC (permalink / raw) To: Srinivas Pandruvada Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-iio-u79uwXL29TY76Z2rM5mHXA, Jonathan Cameron, linux-input, Jiri Kosina Hey Srinivas, On Tue, Jun 3, 2014 at 12:43 PM, Srinivas Pandruvada <srinivas.pandruvada-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> wrote: > > May be we should have a field in const struct iio_chan_spec, so that we > can dynamically enable disable channels. In this way we can statically > define channels, but can be enabled/disabled dynamically. > Would this require changing iio subsystem to create sysfs entries only when enabled? Would we need to add functions to disable and enable later on? > > I think we need to present angle in radians. Are you basing change present > in linux-next? This will automatically do in this function. > I'll look into this. What function should I use to make the iio chanel to report radians? > > > I don't see kfree. Try devm_kcalloc? > I changed kmemdup to kcalloc so there is still a kfree in the exsiting code. I can change this to devm_kcalloc but only if we don't go with static channels that are enabled as found. Thanks, Reyad Attiyat ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCHv2 3/3] IIO: hid-sensor-magn-3d: Add in support for True/Magnetic North HID usages @ 2014-06-04 15:23 ` Reyad Attiyat 0 siblings, 0 replies; 23+ messages in thread From: Reyad Attiyat @ 2014-06-04 15:23 UTC (permalink / raw) To: Srinivas Pandruvada Cc: linux-kernel, linux-iio, Jonathan Cameron, linux-input, Jiri Kosina Hey Srinivas, On Tue, Jun 3, 2014 at 12:43 PM, Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> wrote: > > May be we should have a field in const struct iio_chan_spec, so that we > can dynamically enable disable channels. In this way we can statically > define channels, but can be enabled/disabled dynamically. > Would this require changing iio subsystem to create sysfs entries only when enabled? Would we need to add functions to disable and enable later on? > > I think we need to present angle in radians. Are you basing change present > in linux-next? This will automatically do in this function. > I'll look into this. What function should I use to make the iio chanel to report radians? > > > I don't see kfree. Try devm_kcalloc? > I changed kmemdup to kcalloc so there is still a kfree in the exsiting code. I can change this to devm_kcalloc but only if we don't go with static channels that are enabled as found. Thanks, Reyad Attiyat ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCHv2 3/3] IIO: hid-sensor-magn-3d: Add in support for True/Magnetic North HID usages 2014-06-04 15:23 ` Reyad Attiyat (?) @ 2014-06-04 15:42 ` Srinivas Pandruvada 2014-06-09 17:47 ` Jonathan Cameron -1 siblings, 1 reply; 23+ messages in thread From: Srinivas Pandruvada @ 2014-06-04 15:42 UTC (permalink / raw) To: Reyad Attiyat Cc: linux-kernel, linux-iio, Jonathan Cameron, linux-input, Jiri Kosina On 06/04/2014 08:23 AM, Reyad Attiyat wrote: > Hey Srinivas, > > On Tue, Jun 3, 2014 at 12:43 PM, Srinivas Pandruvada > <srinivas.pandruvada@linux.intel.com> wrote: > >> >> May be we should have a field in const struct iio_chan_spec, so that we >> can dynamically enable disable channels. In this way we can statically >> define channels, but can be enabled/disabled dynamically. >> > Would this require changing iio subsystem to create sysfs entries only > when enabled? Would we need to add functions to disable and enable > later on? This is just a thought. You don't have to change it. I am sure Jonathan will have some opinion this. >> >> I think we need to present angle in radians. Are you basing change present >> in linux-next? This will automatically do in this function. >> > I'll look into this. What function should I use to make the iio chanel > to report radians? I think it will work if the existing sequence is maintained st->scale_precision = hid_sensor_format_scale( HID_USAGE_SENSOR_COMPASS_3D, &st->magn[CHANNEL_SCAN_INDEX_X], &st->scale_pre_decml, &st->scale_post_decml); So as long as you call this function, the scale value to user space will be returned correctly. >> >> >> I don't see kfree. Try devm_kcalloc? >> > I changed kmemdup to kcalloc so there is still a kfree in the exsiting > code. I can change this to devm_kcalloc but only if we don't go with > static channels that are enabled as found. > Since you are changing this part, devm_ calls are preferred, I think. Thanks, Srinivas > Thanks, > Reyad Attiyat > ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCHv2 3/3] IIO: hid-sensor-magn-3d: Add in support for True/Magnetic North HID usages 2014-06-04 15:42 ` Srinivas Pandruvada @ 2014-06-09 17:47 ` Jonathan Cameron 0 siblings, 0 replies; 23+ messages in thread From: Jonathan Cameron @ 2014-06-09 17:47 UTC (permalink / raw) To: Srinivas Pandruvada, Reyad Attiyat Cc: linux-kernel, linux-iio, linux-input, Jiri Kosina On 04/06/14 16:42, Srinivas Pandruvada wrote: > On 06/04/2014 08:23 AM, Reyad Attiyat wrote: >> Hey Srinivas, >> >> On Tue, Jun 3, 2014 at 12:43 PM, Srinivas Pandruvada >> <srinivas.pandruvada@linux.intel.com> wrote: >> >>> >>> May be we should have a field in const struct iio_chan_spec, so that we >>> can dynamically enable disable channels. In this way we can statically >>> define channels, but can be enabled/disabled dynamically. >>> >> Would this require changing iio subsystem to create sysfs entries only >> when enabled? Would we need to add functions to disable and enable >> later on? > > This is just a thought. You don't have to change it. I am sure Jonathan will have some opinion this. Replied to earlier message. If there is some common code we can factor out into the core then I'm happy to do so, but I don't see an advantage in using a field, rather than just tailoring the copy in the first place. e.g. 1) Pass whatever is needed to figure out how many channels are present. 2) Allocate space for that many channels. 3) Copy said channels from predefined versions, perhaps amending as necessary. This is a reasonably common pattern in complex parts or those with hugely variable numbers of channels... > >>> >>> I think we need to present angle in radians. Are you basing change present >>> in linux-next? This will automatically do in this function. >>> >> I'll look into this. What function should I use to make the iio chanel >> to report radians? > I think it will work if the existing sequence is maintained > st->scale_precision = hid_sensor_format_scale( > HID_USAGE_SENSOR_COMPASS_3D, > &st->magn[CHANNEL_SCAN_INDEX_X], > &st->scale_pre_decml, &st->scale_post_decml); > > So as long as you call this function, the scale value to user space will > be returned correctly. > >>> >>> >>> I don't see kfree. Try devm_kcalloc? >>> >> I changed kmemdup to kcalloc so there is still a kfree in the exsiting >> code. I can change this to devm_kcalloc but only if we don't go with >> static channels that are enabled as found. >> > Since you are changing this part, devm_ calls are preferred, I think. As long as it doesn't add complexity or possibility of race conditions, devm calls are usually a good idea. > > Thanks, > Srinivas > > >> Thanks, >> Reyad Attiyat >> > ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCHv2 3/3] IIO: hid-sensor-magn-3d: Add in support for True/Magnetic North HID usages 2014-06-03 17:43 ` Srinivas Pandruvada @ 2014-06-09 17:43 ` Jonathan Cameron -1 siblings, 0 replies; 23+ messages in thread From: Jonathan Cameron @ 2014-06-09 17:43 UTC (permalink / raw) To: Srinivas Pandruvada, Reyad Attiyat Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-iio-u79uwXL29TY76Z2rM5mHXA, linux-input-u79uwXL29TY76Z2rM5mHXA, jkosina-AlSwsSmVLrQ On 03/06/14 18:43, Srinivas Pandruvada wrote: > On 06/02/2014 04:14 PM, Reyad Attiyat wrote: >> Updated magn_3d_channel enum for all possible north channels >> >> Added functions to setup iio_chan_spec array depending on which hid usage reports are found >> >> Renamed magn_val to iio_val to differentiate the index being used >> >> Updated magn_3d_state struct to hold pointer array (magn_val_addr[]) which points to iio_val[] >> >> Updated magn_3d_parse_report to scan for all compass usages and create iio channels for each >> >> Signed-off-by: Reyad Attiyat <reyad.attiyat-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> >> --- >> drivers/iio/magnetometer/hid-sensor-magn-3d.c | 271 +++++++++++++++++--------- >> 1 file changed, 177 insertions(+), 94 deletions(-) >> >> diff --git a/drivers/iio/magnetometer/hid-sensor-magn-3d.c b/drivers/iio/magnetometer/hid-sensor-magn-3d.c >> index 6d162b7..32f4d90 100644 >> --- a/drivers/iio/magnetometer/hid-sensor-magn-3d.c >> +++ b/drivers/iio/magnetometer/hid-sensor-magn-3d.c >> @@ -34,63 +34,54 @@ enum magn_3d_channel { >> CHANNEL_SCAN_INDEX_X, >> CHANNEL_SCAN_INDEX_Y, >> CHANNEL_SCAN_INDEX_Z, >> + CHANNEL_SCAN_INDEX_NORTH_MAGN, >> + CHANNEL_SCAN_INDEX_NORTH_TRUE, >> + CHANNEL_SCAN_INDEX_NORTH_TILT_COMP, >> + CHANNEL_SCAN_INDEX_NORTH_TRUE_TILT_COMP, >> MAGN_3D_CHANNEL_MAX, >> }; >> >> +#define IIO_CHANNEL_MAX MAGN_3D_CHANNEL_MAX >> + >> struct magn_3d_state { >> struct hid_sensor_hub_callbacks callbacks; >> struct hid_sensor_common common_attributes; >> struct hid_sensor_hub_attribute_info magn[MAGN_3D_CHANNEL_MAX]; >> - u32 magn_val[MAGN_3D_CHANNEL_MAX]; >> -}; >> + u32 *magn_val_addr[MAGN_3D_CHANNEL_MAX]; >> >> -static const u32 magn_3d_addresses[MAGN_3D_CHANNEL_MAX] = { >> - HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_X_AXIS, >> - HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Y_AXIS, >> - HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Z_AXIS >> + u32 iio_val[IIO_CHANNEL_MAX]; >> + int num_iio_channels; >> }; >> >> -/* Channel definitions */ >> -static const struct iio_chan_spec magn_3d_channels[] = { >> - { >> - .type = IIO_MAGN, >> - .modified = 1, >> - .channel2 = IIO_MOD_X, >> - .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) | >> - BIT(IIO_CHAN_INFO_SCALE) | >> - BIT(IIO_CHAN_INFO_SAMP_FREQ) | >> - BIT(IIO_CHAN_INFO_HYSTERESIS), >> - .scan_index = CHANNEL_SCAN_INDEX_X, >> - }, { >> - .type = IIO_MAGN, >> - .modified = 1, >> - .channel2 = IIO_MOD_Y, >> - .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) | >> - BIT(IIO_CHAN_INFO_SCALE) | >> - BIT(IIO_CHAN_INFO_SAMP_FREQ) | >> - BIT(IIO_CHAN_INFO_HYSTERESIS), >> - .scan_index = CHANNEL_SCAN_INDEX_Y, >> - }, { >> - .type = IIO_MAGN, >> - .modified = 1, >> - .channel2 = IIO_MOD_Z, >> - .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) | >> - BIT(IIO_CHAN_INFO_SCALE) | >> - BIT(IIO_CHAN_INFO_SAMP_FREQ) | >> - BIT(IIO_CHAN_INFO_HYSTERESIS), >> - .scan_index = CHANNEL_SCAN_INDEX_Z, > > May be we should have a field in const struct iio_chan_spec, so that we > can dynamically enable disable channels. In this way we can statically define channels, but can be enabled/disabled dynamically. But then it's not const to you have to take a per instance copy. Once you are doing that you might as well just pull out into the copy those channels that are actually present, at which point the field doesn't have any purpose. > > >> +/* Find index into magn_3d_state magn[] and magn_val_addr[] from HID Usage */ >> +static int magn_3d_usage_id_to_chan_index(unsigned usage_id){ >> + int offset = -1; >> + >> + switch (usage_id) { >> + case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_X_AXIS: >> + offset = CHANNEL_SCAN_INDEX_X; >> + break; >> + case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Y_AXIS: >> + offset = CHANNEL_SCAN_INDEX_Y; >> + break; >> + case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Z_AXIS: >> + offset = CHANNEL_SCAN_INDEX_Z; >> + break; >> + case HID_USAGE_SENSOR_ORIENT_COMP_MAGN_NORTH: >> + offset = CHANNEL_SCAN_INDEX_NORTH_TILT_COMP; >> + break; >> + case HID_USAGE_SENSOR_ORIENT_COMP_TRUE_NORTH: >> + offset = CHANNEL_SCAN_INDEX_NORTH_TRUE_TILT_COMP; >> + break; >> + case HID_USAGE_SENSOR_ORIENT_MAGN_NORTH: >> + offset = CHANNEL_SCAN_INDEX_NORTH_MAGN; >> + break; >> + case HID_USAGE_SENSOR_ORIENT_TRUE_NORTH: >> + offset = CHANNEL_SCAN_INDEX_NORTH_TRUE; >> + break; >> } >> -}; >> >> -/* Adjust channel real bits based on report descriptor */ >> -static void magn_3d_adjust_channel_bit_mask(struct iio_chan_spec *channels, >> - int channel, int size) >> -{ >> - channels[channel].scan_type.sign = 's'; >> - /* Real storage bits will change based on the report desc. */ >> - channels[channel].scan_type.realbits = size * 8; >> - /* Maximum size of a sample to capture is u32 */ >> - channels[channel].scan_type.storagebits = sizeof(u32) * 8; >> + return offset; >> } >> >> /* Channel read_raw handler */ >> @@ -101,21 +92,31 @@ static int magn_3d_read_raw(struct iio_dev *indio_dev, >> { >> struct magn_3d_state *magn_state = iio_priv(indio_dev); >> int report_id = -1; >> - u32 address; >> + unsigned usage_id; >> + int chan_index = -1; >> int ret; >> int ret_type; >> >> + dev_dbg(&indio_dev->dev, "magn_3d_read_raw\n"); >> + >> *val = 0; >> *val2 = 0; >> switch (mask) { >> case 0: >> + /* We store the HID usage ID of the iio channel >> + * in its address field >> + */ >> + usage_id = chan->address; >> + chan_index = magn_3d_usage_id_to_chan_index(usage_id); >> + if(chan_index < 0) >> + return -EINVAL; >> + >> report_id = >> - magn_state->magn[chan->scan_index].report_id; >> - address = magn_3d_addresses[chan->scan_index]; >> + magn_state->magn[chan_index].report_id; >> if (report_id >= 0) >> *val = sensor_hub_input_attr_get_raw_value( >> magn_state->common_attributes.hsdev, >> - HID_USAGE_SENSOR_COMPASS_3D, address, >> + HID_USAGE_SENSOR_COMPASS_3D, usage_id, >> report_id); >> else { >> *val = 0; >> @@ -202,12 +203,13 @@ static int magn_3d_proc_event(struct hid_sensor_hub_device *hsdev, >> magn_state->common_attributes.data_ready); >> if (magn_state->common_attributes.data_ready) >> hid_sensor_push_data(indio_dev, >> - magn_state->magn_val, >> - sizeof(magn_state->magn_val)); >> + &(magn_state->iio_val), >> + sizeof(magn_state->iio_val)); >> >> return 0; >> } >> >> + >> /* Capture samples in local storage */ >> static int magn_3d_capture_sample(struct hid_sensor_hub_device *hsdev, >> unsigned usage_id, >> @@ -217,63 +219,143 @@ static int magn_3d_capture_sample(struct hid_sensor_hub_device *hsdev, >> struct iio_dev *indio_dev = platform_get_drvdata(priv); >> struct magn_3d_state *magn_state = iio_priv(indio_dev); >> int offset; >> + u32 *magn_val; >> int ret = -EINVAL; >> >> - switch (usage_id) { >> + offset = magn_3d_usage_id_to_chan_index(usage_id); >> + if(offset < 0) >> + return ret; >> + >> + magn_val = magn_state->magn_val_addr[offset]; >> + if(!magn_val) >> + return ret; >> + >> + *(magn_val) = *(u32 *)raw_data; >> + >> + return 0; >> +} >> + >> +/* Setup the iio_chan_spec for HID Usage ID */ >> +static int magn_3d_setup_new_iio_chan(struct hid_sensor_hub_device *hsdev, >> + unsigned usage_id, >> + unsigned attr_usage_id, >> + struct iio_chan_spec *iio_chans, >> + struct magn_3d_state *st) >> +{ >> + int ret = -1; >> + int iio_index; >> + int magn_index; >> + struct iio_chan_spec *channel; >> + >> + /* Setup magn_3d_state for new channel */ >> + magn_index = magn_3d_usage_id_to_chan_index(attr_usage_id); >> + if(magn_index < 0 || magn_index >= MAGN_3D_CHANNEL_MAX){ >> + return -1; >> + } >> + >> + /* Check if usage attribute exists in the sensor hub device */ >> + ret = sensor_hub_input_get_attribute_info(hsdev, >> + HID_INPUT_REPORT, >> + usage_id, >> + attr_usage_id, >> + &(st->magn[magn_index])); >> + if(ret != 0){ >> + /* Usage not found. Nothing to setup.*/ >> + return 0; >> + } >> + >> + iio_index = st->num_iio_channels; >> + if(iio_index < 0 || iio_index >= IIO_CHANNEL_MAX){ >> + return -2; >> + } >> + >> + /* Check if this usage hasn't been setup */ >> + if(st->magn_val_addr[magn_index] != NULL){ >> + return -3; >> + } >> + st->magn_val_addr[magn_index] = &(st->iio_val[iio_index]); >> + >> + /* Setup IIO Channel spec */ >> + channel = &(iio_chans[iio_index]); >> + >> + channel->type = IIO_MAGN; >> + channel->address = attr_usage_id; >> + channel->modified = 1; >> + >> + switch (attr_usage_id){ >> + case HID_USAGE_SENSOR_ORIENT_COMP_MAGN_NORTH: >> + channel->channel2 = IIO_MOD_NORTH_MAGN_TILT_COMP; >> + break; >> + case HID_USAGE_SENSOR_ORIENT_COMP_TRUE_NORTH: >> + channel->channel2 = IIO_MOD_NORTH_TRUE_TILT_COMP; >> + break; >> + case HID_USAGE_SENSOR_ORIENT_MAGN_NORTH: >> + channel->channel2 = IIO_MOD_NORTH_MAGN; >> + break; >> + case HID_USAGE_SENSOR_ORIENT_TRUE_NORTH: >> + channel->channel2 = IIO_MOD_NORTH_TRUE; >> + break; >> case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_X_AXIS: >> + channel->channel2 = IIO_MOD_X; >> + break; >> case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Y_AXIS: >> + channel->channel2 = IIO_MOD_Y; >> + break; >> case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Z_AXIS: >> - offset = usage_id - HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_X_AXIS; >> - magn_state->magn_val[CHANNEL_SCAN_INDEX_X + offset] = >> - *(u32 *)raw_data; >> - ret = 0; >> - break; >> - default: >> + channel->channel2 = IIO_MOD_Z; >> break; >> + default: >> + return -4; >> } >> >> + channel->info_mask_shared_by_type = >> + BIT(IIO_CHAN_INFO_OFFSET) | >> + BIT(IIO_CHAN_INFO_SCALE) | >> + BIT(IIO_CHAN_INFO_SAMP_FREQ) | >> + BIT(IIO_CHAN_INFO_HYSTERESIS); >> + >> + channel->scan_type.sign = 's'; >> + /* Real storage bits will change based on the report desc. */ >> + channel->scan_type.realbits = st->magn[magn_index].size * 8; >> + /* Maximum size of a sample to capture is u32 */ >> + channel->scan_type.storagebits = sizeof(u32) * 8; >> + >> + (st->num_iio_channels)++; >> + ret = 0; >> + >> return ret; >> } >> >> -/* Parse report which is specific to an usage id*/ >> +/* Read the HID reports and setup IIO Channels */ >> static int magn_3d_parse_report(struct platform_device *pdev, >> struct hid_sensor_hub_device *hsdev, >> - struct iio_chan_spec *channels, >> + struct iio_chan_spec *iio_chans, >> unsigned usage_id, >> struct magn_3d_state *st) >> { >> - int ret; >> + int ret = 0; >> int i; >> >> - for (i = 0; i <= CHANNEL_SCAN_INDEX_Z; ++i) { >> - ret = sensor_hub_input_get_attribute_info(hsdev, >> - HID_INPUT_REPORT, >> - usage_id, >> - HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_X_AXIS + i, >> - &st->magn[CHANNEL_SCAN_INDEX_X + i]); >> - if (ret < 0) >> - break; >> - magn_3d_adjust_channel_bit_mask(channels, >> - CHANNEL_SCAN_INDEX_X + i, >> - st->magn[CHANNEL_SCAN_INDEX_X + i].size); >> - } >> - dev_dbg(&pdev->dev, "magn_3d %x:%x, %x:%x, %x:%x\n", >> - st->magn[0].index, >> - st->magn[0].report_id, >> - st->magn[1].index, st->magn[1].report_id, >> - st->magn[2].index, st->magn[2].report_id); >> - >> - /* Set Sensitivity field ids, when there is no individual modifier */ >> - if (st->common_attributes.sensitivity.index < 0) { >> - sensor_hub_input_get_attribute_info(hsdev, >> - HID_FEATURE_REPORT, usage_id, >> - HID_USAGE_SENSOR_DATA_MOD_CHANGE_SENSITIVITY_ABS | >> - HID_USAGE_SENSOR_DATA_ORIENTATION, >> - &st->common_attributes.sensitivity); >> - dev_dbg(&pdev->dev, "Sensitivity index:report %d:%d\n", >> - st->common_attributes.sensitivity.index, >> - st->common_attributes.sensitivity.report_id); >> - } >> + dev_dbg(&pdev->dev, "magn_3d_parse_reports Usage ID: %x\n", usage_id); >> + for(i = HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_X_AXIS; >> + i <= HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Z_AXIS && ret == 0; >> + i++) >> + ret = magn_3d_setup_new_iio_chan(hsdev, >> + usage_id, >> + i, >> + iio_chans, >> + st); >> + >> + dev_dbg(&pdev->dev, "magn_3d_parse_reports Found %d\n magnetic axis. Returned: %d", st->num_iio_channels, ret); >> + for(i = HID_USAGE_SENSOR_ORIENT_COMP_MAGN_NORTH; >> + i <= HID_USAGE_SENSOR_ORIENT_TRUE_NORTH && ret == 0; >> + i++) >> + ret = magn_3d_setup_new_iio_chan(hsdev, >> + usage_id, >> + i, >> + iio_chans, >> + st); >> + dev_dbg(&pdev->dev, "magn_3d_parse_reports Found %d\n magnetic channels. Returned: %d", st->num_iio_channels, ret); >> > I think we need to present angle in radians. Are you basing change present in linux-next? This will automatically do in this function. Yes, angles need to be in radians. > >> return ret; >> } >> @@ -307,10 +389,11 @@ static int hid_magn_3d_probe(struct platform_device *pdev) >> return ret; >> } >> >> - channels = kmemdup(magn_3d_channels, sizeof(magn_3d_channels), >> - GFP_KERNEL); >> + channels = kcalloc(MAGN_3D_CHANNEL_MAX, >> + sizeof(struct iio_chan_spec), >> + GFP_KERNEL); > > I don't see kfree. Try devm_kcalloc? > >> if (!channels) { >> - dev_err(&pdev->dev, "failed to duplicate channels\n"); >> + dev_err(&pdev->dev, "failed to allocate memory for iio channel\n"); >> return -ENOMEM; >> } >> >> @@ -322,7 +405,7 @@ static int hid_magn_3d_probe(struct platform_device *pdev) >> } >> >> indio_dev->channels = channels; >> - indio_dev->num_channels = ARRAY_SIZE(magn_3d_channels); >> + indio_dev->num_channels = magn_state->num_iio_channels; >> indio_dev->dev.parent = &pdev->dev; >> indio_dev->info = &magn_3d_info; >> indio_dev->name = name; >> > Thanks, > Srinivas > ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCHv2 3/3] IIO: hid-sensor-magn-3d: Add in support for True/Magnetic North HID usages @ 2014-06-09 17:43 ` Jonathan Cameron 0 siblings, 0 replies; 23+ messages in thread From: Jonathan Cameron @ 2014-06-09 17:43 UTC (permalink / raw) To: Srinivas Pandruvada, Reyad Attiyat Cc: linux-kernel, linux-iio, linux-input, jkosina On 03/06/14 18:43, Srinivas Pandruvada wrote: > On 06/02/2014 04:14 PM, Reyad Attiyat wrote: >> Updated magn_3d_channel enum for all possible north channels >> >> Added functions to setup iio_chan_spec array depending on which hid usage reports are found >> >> Renamed magn_val to iio_val to differentiate the index being used >> >> Updated magn_3d_state struct to hold pointer array (magn_val_addr[]) which points to iio_val[] >> >> Updated magn_3d_parse_report to scan for all compass usages and create iio channels for each >> >> Signed-off-by: Reyad Attiyat <reyad.attiyat@gmail.com> >> --- >> drivers/iio/magnetometer/hid-sensor-magn-3d.c | 271 +++++++++++++++++--------- >> 1 file changed, 177 insertions(+), 94 deletions(-) >> >> diff --git a/drivers/iio/magnetometer/hid-sensor-magn-3d.c b/drivers/iio/magnetometer/hid-sensor-magn-3d.c >> index 6d162b7..32f4d90 100644 >> --- a/drivers/iio/magnetometer/hid-sensor-magn-3d.c >> +++ b/drivers/iio/magnetometer/hid-sensor-magn-3d.c >> @@ -34,63 +34,54 @@ enum magn_3d_channel { >> CHANNEL_SCAN_INDEX_X, >> CHANNEL_SCAN_INDEX_Y, >> CHANNEL_SCAN_INDEX_Z, >> + CHANNEL_SCAN_INDEX_NORTH_MAGN, >> + CHANNEL_SCAN_INDEX_NORTH_TRUE, >> + CHANNEL_SCAN_INDEX_NORTH_TILT_COMP, >> + CHANNEL_SCAN_INDEX_NORTH_TRUE_TILT_COMP, >> MAGN_3D_CHANNEL_MAX, >> }; >> >> +#define IIO_CHANNEL_MAX MAGN_3D_CHANNEL_MAX >> + >> struct magn_3d_state { >> struct hid_sensor_hub_callbacks callbacks; >> struct hid_sensor_common common_attributes; >> struct hid_sensor_hub_attribute_info magn[MAGN_3D_CHANNEL_MAX]; >> - u32 magn_val[MAGN_3D_CHANNEL_MAX]; >> -}; >> + u32 *magn_val_addr[MAGN_3D_CHANNEL_MAX]; >> >> -static const u32 magn_3d_addresses[MAGN_3D_CHANNEL_MAX] = { >> - HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_X_AXIS, >> - HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Y_AXIS, >> - HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Z_AXIS >> + u32 iio_val[IIO_CHANNEL_MAX]; >> + int num_iio_channels; >> }; >> >> -/* Channel definitions */ >> -static const struct iio_chan_spec magn_3d_channels[] = { >> - { >> - .type = IIO_MAGN, >> - .modified = 1, >> - .channel2 = IIO_MOD_X, >> - .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) | >> - BIT(IIO_CHAN_INFO_SCALE) | >> - BIT(IIO_CHAN_INFO_SAMP_FREQ) | >> - BIT(IIO_CHAN_INFO_HYSTERESIS), >> - .scan_index = CHANNEL_SCAN_INDEX_X, >> - }, { >> - .type = IIO_MAGN, >> - .modified = 1, >> - .channel2 = IIO_MOD_Y, >> - .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) | >> - BIT(IIO_CHAN_INFO_SCALE) | >> - BIT(IIO_CHAN_INFO_SAMP_FREQ) | >> - BIT(IIO_CHAN_INFO_HYSTERESIS), >> - .scan_index = CHANNEL_SCAN_INDEX_Y, >> - }, { >> - .type = IIO_MAGN, >> - .modified = 1, >> - .channel2 = IIO_MOD_Z, >> - .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) | >> - BIT(IIO_CHAN_INFO_SCALE) | >> - BIT(IIO_CHAN_INFO_SAMP_FREQ) | >> - BIT(IIO_CHAN_INFO_HYSTERESIS), >> - .scan_index = CHANNEL_SCAN_INDEX_Z, > > May be we should have a field in const struct iio_chan_spec, so that we > can dynamically enable disable channels. In this way we can statically define channels, but can be enabled/disabled dynamically. But then it's not const to you have to take a per instance copy. Once you are doing that you might as well just pull out into the copy those channels that are actually present, at which point the field doesn't have any purpose. > > >> +/* Find index into magn_3d_state magn[] and magn_val_addr[] from HID Usage */ >> +static int magn_3d_usage_id_to_chan_index(unsigned usage_id){ >> + int offset = -1; >> + >> + switch (usage_id) { >> + case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_X_AXIS: >> + offset = CHANNEL_SCAN_INDEX_X; >> + break; >> + case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Y_AXIS: >> + offset = CHANNEL_SCAN_INDEX_Y; >> + break; >> + case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Z_AXIS: >> + offset = CHANNEL_SCAN_INDEX_Z; >> + break; >> + case HID_USAGE_SENSOR_ORIENT_COMP_MAGN_NORTH: >> + offset = CHANNEL_SCAN_INDEX_NORTH_TILT_COMP; >> + break; >> + case HID_USAGE_SENSOR_ORIENT_COMP_TRUE_NORTH: >> + offset = CHANNEL_SCAN_INDEX_NORTH_TRUE_TILT_COMP; >> + break; >> + case HID_USAGE_SENSOR_ORIENT_MAGN_NORTH: >> + offset = CHANNEL_SCAN_INDEX_NORTH_MAGN; >> + break; >> + case HID_USAGE_SENSOR_ORIENT_TRUE_NORTH: >> + offset = CHANNEL_SCAN_INDEX_NORTH_TRUE; >> + break; >> } >> -}; >> >> -/* Adjust channel real bits based on report descriptor */ >> -static void magn_3d_adjust_channel_bit_mask(struct iio_chan_spec *channels, >> - int channel, int size) >> -{ >> - channels[channel].scan_type.sign = 's'; >> - /* Real storage bits will change based on the report desc. */ >> - channels[channel].scan_type.realbits = size * 8; >> - /* Maximum size of a sample to capture is u32 */ >> - channels[channel].scan_type.storagebits = sizeof(u32) * 8; >> + return offset; >> } >> >> /* Channel read_raw handler */ >> @@ -101,21 +92,31 @@ static int magn_3d_read_raw(struct iio_dev *indio_dev, >> { >> struct magn_3d_state *magn_state = iio_priv(indio_dev); >> int report_id = -1; >> - u32 address; >> + unsigned usage_id; >> + int chan_index = -1; >> int ret; >> int ret_type; >> >> + dev_dbg(&indio_dev->dev, "magn_3d_read_raw\n"); >> + >> *val = 0; >> *val2 = 0; >> switch (mask) { >> case 0: >> + /* We store the HID usage ID of the iio channel >> + * in its address field >> + */ >> + usage_id = chan->address; >> + chan_index = magn_3d_usage_id_to_chan_index(usage_id); >> + if(chan_index < 0) >> + return -EINVAL; >> + >> report_id = >> - magn_state->magn[chan->scan_index].report_id; >> - address = magn_3d_addresses[chan->scan_index]; >> + magn_state->magn[chan_index].report_id; >> if (report_id >= 0) >> *val = sensor_hub_input_attr_get_raw_value( >> magn_state->common_attributes.hsdev, >> - HID_USAGE_SENSOR_COMPASS_3D, address, >> + HID_USAGE_SENSOR_COMPASS_3D, usage_id, >> report_id); >> else { >> *val = 0; >> @@ -202,12 +203,13 @@ static int magn_3d_proc_event(struct hid_sensor_hub_device *hsdev, >> magn_state->common_attributes.data_ready); >> if (magn_state->common_attributes.data_ready) >> hid_sensor_push_data(indio_dev, >> - magn_state->magn_val, >> - sizeof(magn_state->magn_val)); >> + &(magn_state->iio_val), >> + sizeof(magn_state->iio_val)); >> >> return 0; >> } >> >> + >> /* Capture samples in local storage */ >> static int magn_3d_capture_sample(struct hid_sensor_hub_device *hsdev, >> unsigned usage_id, >> @@ -217,63 +219,143 @@ static int magn_3d_capture_sample(struct hid_sensor_hub_device *hsdev, >> struct iio_dev *indio_dev = platform_get_drvdata(priv); >> struct magn_3d_state *magn_state = iio_priv(indio_dev); >> int offset; >> + u32 *magn_val; >> int ret = -EINVAL; >> >> - switch (usage_id) { >> + offset = magn_3d_usage_id_to_chan_index(usage_id); >> + if(offset < 0) >> + return ret; >> + >> + magn_val = magn_state->magn_val_addr[offset]; >> + if(!magn_val) >> + return ret; >> + >> + *(magn_val) = *(u32 *)raw_data; >> + >> + return 0; >> +} >> + >> +/* Setup the iio_chan_spec for HID Usage ID */ >> +static int magn_3d_setup_new_iio_chan(struct hid_sensor_hub_device *hsdev, >> + unsigned usage_id, >> + unsigned attr_usage_id, >> + struct iio_chan_spec *iio_chans, >> + struct magn_3d_state *st) >> +{ >> + int ret = -1; >> + int iio_index; >> + int magn_index; >> + struct iio_chan_spec *channel; >> + >> + /* Setup magn_3d_state for new channel */ >> + magn_index = magn_3d_usage_id_to_chan_index(attr_usage_id); >> + if(magn_index < 0 || magn_index >= MAGN_3D_CHANNEL_MAX){ >> + return -1; >> + } >> + >> + /* Check if usage attribute exists in the sensor hub device */ >> + ret = sensor_hub_input_get_attribute_info(hsdev, >> + HID_INPUT_REPORT, >> + usage_id, >> + attr_usage_id, >> + &(st->magn[magn_index])); >> + if(ret != 0){ >> + /* Usage not found. Nothing to setup.*/ >> + return 0; >> + } >> + >> + iio_index = st->num_iio_channels; >> + if(iio_index < 0 || iio_index >= IIO_CHANNEL_MAX){ >> + return -2; >> + } >> + >> + /* Check if this usage hasn't been setup */ >> + if(st->magn_val_addr[magn_index] != NULL){ >> + return -3; >> + } >> + st->magn_val_addr[magn_index] = &(st->iio_val[iio_index]); >> + >> + /* Setup IIO Channel spec */ >> + channel = &(iio_chans[iio_index]); >> + >> + channel->type = IIO_MAGN; >> + channel->address = attr_usage_id; >> + channel->modified = 1; >> + >> + switch (attr_usage_id){ >> + case HID_USAGE_SENSOR_ORIENT_COMP_MAGN_NORTH: >> + channel->channel2 = IIO_MOD_NORTH_MAGN_TILT_COMP; >> + break; >> + case HID_USAGE_SENSOR_ORIENT_COMP_TRUE_NORTH: >> + channel->channel2 = IIO_MOD_NORTH_TRUE_TILT_COMP; >> + break; >> + case HID_USAGE_SENSOR_ORIENT_MAGN_NORTH: >> + channel->channel2 = IIO_MOD_NORTH_MAGN; >> + break; >> + case HID_USAGE_SENSOR_ORIENT_TRUE_NORTH: >> + channel->channel2 = IIO_MOD_NORTH_TRUE; >> + break; >> case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_X_AXIS: >> + channel->channel2 = IIO_MOD_X; >> + break; >> case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Y_AXIS: >> + channel->channel2 = IIO_MOD_Y; >> + break; >> case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Z_AXIS: >> - offset = usage_id - HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_X_AXIS; >> - magn_state->magn_val[CHANNEL_SCAN_INDEX_X + offset] = >> - *(u32 *)raw_data; >> - ret = 0; >> - break; >> - default: >> + channel->channel2 = IIO_MOD_Z; >> break; >> + default: >> + return -4; >> } >> >> + channel->info_mask_shared_by_type = >> + BIT(IIO_CHAN_INFO_OFFSET) | >> + BIT(IIO_CHAN_INFO_SCALE) | >> + BIT(IIO_CHAN_INFO_SAMP_FREQ) | >> + BIT(IIO_CHAN_INFO_HYSTERESIS); >> + >> + channel->scan_type.sign = 's'; >> + /* Real storage bits will change based on the report desc. */ >> + channel->scan_type.realbits = st->magn[magn_index].size * 8; >> + /* Maximum size of a sample to capture is u32 */ >> + channel->scan_type.storagebits = sizeof(u32) * 8; >> + >> + (st->num_iio_channels)++; >> + ret = 0; >> + >> return ret; >> } >> >> -/* Parse report which is specific to an usage id*/ >> +/* Read the HID reports and setup IIO Channels */ >> static int magn_3d_parse_report(struct platform_device *pdev, >> struct hid_sensor_hub_device *hsdev, >> - struct iio_chan_spec *channels, >> + struct iio_chan_spec *iio_chans, >> unsigned usage_id, >> struct magn_3d_state *st) >> { >> - int ret; >> + int ret = 0; >> int i; >> >> - for (i = 0; i <= CHANNEL_SCAN_INDEX_Z; ++i) { >> - ret = sensor_hub_input_get_attribute_info(hsdev, >> - HID_INPUT_REPORT, >> - usage_id, >> - HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_X_AXIS + i, >> - &st->magn[CHANNEL_SCAN_INDEX_X + i]); >> - if (ret < 0) >> - break; >> - magn_3d_adjust_channel_bit_mask(channels, >> - CHANNEL_SCAN_INDEX_X + i, >> - st->magn[CHANNEL_SCAN_INDEX_X + i].size); >> - } >> - dev_dbg(&pdev->dev, "magn_3d %x:%x, %x:%x, %x:%x\n", >> - st->magn[0].index, >> - st->magn[0].report_id, >> - st->magn[1].index, st->magn[1].report_id, >> - st->magn[2].index, st->magn[2].report_id); >> - >> - /* Set Sensitivity field ids, when there is no individual modifier */ >> - if (st->common_attributes.sensitivity.index < 0) { >> - sensor_hub_input_get_attribute_info(hsdev, >> - HID_FEATURE_REPORT, usage_id, >> - HID_USAGE_SENSOR_DATA_MOD_CHANGE_SENSITIVITY_ABS | >> - HID_USAGE_SENSOR_DATA_ORIENTATION, >> - &st->common_attributes.sensitivity); >> - dev_dbg(&pdev->dev, "Sensitivity index:report %d:%d\n", >> - st->common_attributes.sensitivity.index, >> - st->common_attributes.sensitivity.report_id); >> - } >> + dev_dbg(&pdev->dev, "magn_3d_parse_reports Usage ID: %x\n", usage_id); >> + for(i = HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_X_AXIS; >> + i <= HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Z_AXIS && ret == 0; >> + i++) >> + ret = magn_3d_setup_new_iio_chan(hsdev, >> + usage_id, >> + i, >> + iio_chans, >> + st); >> + >> + dev_dbg(&pdev->dev, "magn_3d_parse_reports Found %d\n magnetic axis. Returned: %d", st->num_iio_channels, ret); >> + for(i = HID_USAGE_SENSOR_ORIENT_COMP_MAGN_NORTH; >> + i <= HID_USAGE_SENSOR_ORIENT_TRUE_NORTH && ret == 0; >> + i++) >> + ret = magn_3d_setup_new_iio_chan(hsdev, >> + usage_id, >> + i, >> + iio_chans, >> + st); >> + dev_dbg(&pdev->dev, "magn_3d_parse_reports Found %d\n magnetic channels. Returned: %d", st->num_iio_channels, ret); >> > I think we need to present angle in radians. Are you basing change present in linux-next? This will automatically do in this function. Yes, angles need to be in radians. > >> return ret; >> } >> @@ -307,10 +389,11 @@ static int hid_magn_3d_probe(struct platform_device *pdev) >> return ret; >> } >> >> - channels = kmemdup(magn_3d_channels, sizeof(magn_3d_channels), >> - GFP_KERNEL); >> + channels = kcalloc(MAGN_3D_CHANNEL_MAX, >> + sizeof(struct iio_chan_spec), >> + GFP_KERNEL); > > I don't see kfree. Try devm_kcalloc? > >> if (!channels) { >> - dev_err(&pdev->dev, "failed to duplicate channels\n"); >> + dev_err(&pdev->dev, "failed to allocate memory for iio channel\n"); >> return -ENOMEM; >> } >> >> @@ -322,7 +405,7 @@ static int hid_magn_3d_probe(struct platform_device *pdev) >> } >> >> indio_dev->channels = channels; >> - indio_dev->num_channels = ARRAY_SIZE(magn_3d_channels); >> + indio_dev->num_channels = magn_state->num_iio_channels; >> indio_dev->dev.parent = &pdev->dev; >> indio_dev->info = &magn_3d_info; >> indio_dev->name = name; >> > Thanks, > Srinivas > ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCHv2 3/3] IIO: hid-sensor-magn-3d: Add in support for True/Magnetic North HID usages 2014-06-02 23:14 ` [PATCHv2 3/3] IIO: hid-sensor-magn-3d: Add in support " Reyad Attiyat @ 2014-06-09 19:55 ` Jonathan Cameron 0 siblings, 0 replies; 23+ messages in thread From: Jonathan Cameron @ 2014-06-09 19:55 UTC (permalink / raw) To: Reyad Attiyat, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-iio-u79uwXL29TY76Z2rM5mHXA, srinivas.pandruvada-VuQAYsv1563Yd54FQh9/CA, linux-input-u79uwXL29TY76Z2rM5mHXA, jkosina-AlSwsSmVLrQ On 03/06/14 00:14, Reyad Attiyat wrote: > Updated magn_3d_channel enum for all possible north channels > > Added functions to setup iio_chan_spec array depending on which hid usage reports are found > > Renamed magn_val to iio_val to differentiate the index being used > > Updated magn_3d_state struct to hold pointer array (magn_val_addr[]) which points to iio_val[] > > Updated magn_3d_parse_report to scan for all compass usages and create iio channels for each > > Signed-off-by: Reyad Attiyat <reyad.attiyat-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> Hi Reyad, I've taken a rather quick look at this. Will probably want to take one more close look at a newer version. Various bits and bobs inline - mostly fine! Jonathan > --- > drivers/iio/magnetometer/hid-sensor-magn-3d.c | 271 +++++++++++++++++--------- > 1 file changed, 177 insertions(+), 94 deletions(-) > > diff --git a/drivers/iio/magnetometer/hid-sensor-magn-3d.c b/drivers/iio/magnetometer/hid-sensor-magn-3d.c > index 6d162b7..32f4d90 100644 > --- a/drivers/iio/magnetometer/hid-sensor-magn-3d.c > +++ b/drivers/iio/magnetometer/hid-sensor-magn-3d.c > @@ -34,63 +34,54 @@ enum magn_3d_channel { > CHANNEL_SCAN_INDEX_X, > CHANNEL_SCAN_INDEX_Y, > CHANNEL_SCAN_INDEX_Z, > + CHANNEL_SCAN_INDEX_NORTH_MAGN, > + CHANNEL_SCAN_INDEX_NORTH_TRUE, > + CHANNEL_SCAN_INDEX_NORTH_TILT_COMP, > + CHANNEL_SCAN_INDEX_NORTH_TRUE_TILT_COMP, > MAGN_3D_CHANNEL_MAX, > }; > > +#define IIO_CHANNEL_MAX MAGN_3D_CHANNEL_MAX Please don't define a generic sounding name for a local constant. Why not use MAGN_3D_CHANNEL_MAX everywhere? > + > struct magn_3d_state { > struct hid_sensor_hub_callbacks callbacks; > struct hid_sensor_common common_attributes; > struct hid_sensor_hub_attribute_info magn[MAGN_3D_CHANNEL_MAX]; > - u32 magn_val[MAGN_3D_CHANNEL_MAX]; > -}; > + u32 *magn_val_addr[MAGN_3D_CHANNEL_MAX]; > > -static const u32 magn_3d_addresses[MAGN_3D_CHANNEL_MAX] = { > - HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_X_AXIS, > - HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Y_AXIS, > - HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Z_AXIS > + u32 iio_val[IIO_CHANNEL_MAX]; > + int num_iio_channels; > }; > > -/* Channel definitions */ > -static const struct iio_chan_spec magn_3d_channels[] = { > - { > - .type = IIO_MAGN, > - .modified = 1, > - .channel2 = IIO_MOD_X, > - .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) | > - BIT(IIO_CHAN_INFO_SCALE) | > - BIT(IIO_CHAN_INFO_SAMP_FREQ) | > - BIT(IIO_CHAN_INFO_HYSTERESIS), > - .scan_index = CHANNEL_SCAN_INDEX_X, > - }, { > - .type = IIO_MAGN, > - .modified = 1, > - .channel2 = IIO_MOD_Y, > - .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) | > - BIT(IIO_CHAN_INFO_SCALE) | > - BIT(IIO_CHAN_INFO_SAMP_FREQ) | > - BIT(IIO_CHAN_INFO_HYSTERESIS), > - .scan_index = CHANNEL_SCAN_INDEX_Y, > - }, { > - .type = IIO_MAGN, > - .modified = 1, > - .channel2 = IIO_MOD_Z, > - .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) | > - BIT(IIO_CHAN_INFO_SCALE) | > - BIT(IIO_CHAN_INFO_SAMP_FREQ) | > - BIT(IIO_CHAN_INFO_HYSTERESIS), > - .scan_index = CHANNEL_SCAN_INDEX_Z, > +/* Find index into magn_3d_state magn[] and magn_val_addr[] from HID Usage */ > +static int magn_3d_usage_id_to_chan_index(unsigned usage_id){ > + int offset = -1; I'd personally prefer offset = -EINVAL and to have it assigned as a default element in the switch statement rather that here. > + > + switch (usage_id) { > + case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_X_AXIS: > + offset = CHANNEL_SCAN_INDEX_X; > + break; > + case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Y_AXIS: > + offset = CHANNEL_SCAN_INDEX_Y; > + break; > + case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Z_AXIS: > + offset = CHANNEL_SCAN_INDEX_Z; > + break; > + case HID_USAGE_SENSOR_ORIENT_COMP_MAGN_NORTH: > + offset = CHANNEL_SCAN_INDEX_NORTH_TILT_COMP; > + break; > + case HID_USAGE_SENSOR_ORIENT_COMP_TRUE_NORTH: > + offset = CHANNEL_SCAN_INDEX_NORTH_TRUE_TILT_COMP; > + break; > + case HID_USAGE_SENSOR_ORIENT_MAGN_NORTH: > + offset = CHANNEL_SCAN_INDEX_NORTH_MAGN; > + break; > + case HID_USAGE_SENSOR_ORIENT_TRUE_NORTH: > + offset = CHANNEL_SCAN_INDEX_NORTH_TRUE; > + break; > } > -}; > > -/* Adjust channel real bits based on report descriptor */ > -static void magn_3d_adjust_channel_bit_mask(struct iio_chan_spec *channels, > - int channel, int size) > -{ > - channels[channel].scan_type.sign = 's'; > - /* Real storage bits will change based on the report desc. */ > - channels[channel].scan_type.realbits = size * 8; > - /* Maximum size of a sample to capture is u32 */ > - channels[channel].scan_type.storagebits = sizeof(u32) * 8; > + return offset; > } > > /* Channel read_raw handler */ > @@ -101,21 +92,31 @@ static int magn_3d_read_raw(struct iio_dev *indio_dev, > { > struct magn_3d_state *magn_state = iio_priv(indio_dev); > int report_id = -1; > - u32 address; > + unsigned usage_id; > + int chan_index = -1; > int ret; > int ret_type; > > + dev_dbg(&indio_dev->dev, "magn_3d_read_raw\n"); > + > *val = 0; > *val2 = 0; > switch (mask) { > case 0: > + /* We store the HID usage ID of the iio channel > + * in its address field > + */ > + usage_id = chan->address; > + chan_index = magn_3d_usage_id_to_chan_index(usage_id); > + if(chan_index < 0) > + return -EINVAL; > + > report_id = > - magn_state->magn[chan->scan_index].report_id; > - address = magn_3d_addresses[chan->scan_index]; > + magn_state->magn[chan_index].report_id; > if (report_id >= 0) > *val = sensor_hub_input_attr_get_raw_value( > magn_state->common_attributes.hsdev, > - HID_USAGE_SENSOR_COMPASS_3D, address, > + HID_USAGE_SENSOR_COMPASS_3D, usage_id, > report_id); > else { > *val = 0; > @@ -202,12 +203,13 @@ static int magn_3d_proc_event(struct hid_sensor_hub_device *hsdev, > magn_state->common_attributes.data_ready); > if (magn_state->common_attributes.data_ready) > hid_sensor_push_data(indio_dev, > - magn_state->magn_val, > - sizeof(magn_state->magn_val)); > + &(magn_state->iio_val), > + sizeof(magn_state->iio_val)); > > return 0; > } > > + > /* Capture samples in local storage */ > static int magn_3d_capture_sample(struct hid_sensor_hub_device *hsdev, > unsigned usage_id, > @@ -217,63 +219,143 @@ static int magn_3d_capture_sample(struct hid_sensor_hub_device *hsdev, > struct iio_dev *indio_dev = platform_get_drvdata(priv); > struct magn_3d_state *magn_state = iio_priv(indio_dev); > int offset; > + u32 *magn_val; > int ret = -EINVAL; > > - switch (usage_id) { > + offset = magn_3d_usage_id_to_chan_index(usage_id); > + if(offset < 0) > + return ret; > + > + magn_val = magn_state->magn_val_addr[offset]; > + if(!magn_val) > + return ret; > + > + *(magn_val) = *(u32 *)raw_data; > + > + return 0; > +} > + > +/* Setup the iio_chan_spec for HID Usage ID */ > +static int magn_3d_setup_new_iio_chan(struct hid_sensor_hub_device *hsdev, > + unsigned usage_id, > + unsigned attr_usage_id, > + struct iio_chan_spec *iio_chans, > + struct magn_3d_state *st) > +{ > + int ret = -1; This is kind of backwards to normal practice for this sort of function. Better to maintain ret in the correct state at all times. Hence start with it being 0 and set to negative when there is an error rather than the other way around. > + int iio_index; > + int magn_index; > + struct iio_chan_spec *channel; > + > + /* Setup magn_3d_state for new channel */ > + magn_index = magn_3d_usage_id_to_chan_index(attr_usage_id); > + if(magn_index < 0 || magn_index >= MAGN_3D_CHANNEL_MAX){ > + return -1; > + } > + > + /* Check if usage attribute exists in the sensor hub device */ > + ret = sensor_hub_input_get_attribute_info(hsdev, > + HID_INPUT_REPORT, > + usage_id, > + attr_usage_id, > + &(st->magn[magn_index])); > + if(ret != 0){ > + /* Usage not found. Nothing to setup.*/ > + return 0; > + } > + > + iio_index = st->num_iio_channels; > + if(iio_index < 0 || iio_index >= IIO_CHANNEL_MAX){ I'd be tempted to allocate space dynamically but I guess this works. > + return -2; > + } > + > + /* Check if this usage hasn't been setup */ > + if(st->magn_val_addr[magn_index] != NULL){ Space before that bracket here and elsewhere. Don't return your own error codes. Use standard ones. I guess thees are left from debugging. > + return -3; > + } > + st->magn_val_addr[magn_index] = &(st->iio_val[iio_index]); > + > + /* Setup IIO Channel spec */ > + channel = &(iio_chans[iio_index]); Just pass this is in as a pointer in the first place? That is a pointer to the current channel location, that may or may not be filled by this function. > + > + channel->type = IIO_MAGN; > + channel->address = attr_usage_id; > + channel->modified = 1; > + > + switch (attr_usage_id){ > + case HID_USAGE_SENSOR_ORIENT_COMP_MAGN_NORTH: > + channel->channel2 = IIO_MOD_NORTH_MAGN_TILT_COMP; > + break; > + case HID_USAGE_SENSOR_ORIENT_COMP_TRUE_NORTH: > + channel->channel2 = IIO_MOD_NORTH_TRUE_TILT_COMP; > + break; > + case HID_USAGE_SENSOR_ORIENT_MAGN_NORTH: > + channel->channel2 = IIO_MOD_NORTH_MAGN; > + break; > + case HID_USAGE_SENSOR_ORIENT_TRUE_NORTH: > + channel->channel2 = IIO_MOD_NORTH_TRUE; > + break; > case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_X_AXIS: > + channel->channel2 = IIO_MOD_X; > + break; > case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Y_AXIS: > + channel->channel2 = IIO_MOD_Y; > + break; > case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Z_AXIS: > - offset = usage_id - HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_X_AXIS; > - magn_state->magn_val[CHANNEL_SCAN_INDEX_X + offset] = > - *(u32 *)raw_data; > - ret = 0; > - break; > - default: > + channel->channel2 = IIO_MOD_Z; > break; > + default: > + return -4; huh? why -4. -EINVAL or -ENOSUP or something similar perhaps. > } > > + channel->info_mask_shared_by_type = > + BIT(IIO_CHAN_INFO_OFFSET) | > + BIT(IIO_CHAN_INFO_SCALE) | > + BIT(IIO_CHAN_INFO_SAMP_FREQ) | > + BIT(IIO_CHAN_INFO_HYSTERESIS); > + > + channel->scan_type.sign = 's'; > + /* Real storage bits will change based on the report desc. */ > + channel->scan_type.realbits = st->magn[magn_index].size * 8; > + /* Maximum size of a sample to capture is u32 */ > + channel->scan_type.storagebits = sizeof(u32) * 8; > + I'd keen num_iio_channels outside this function and increment it if this function does not return an error. > + (st->num_iio_channels)++; Don't do this. ret should have been valid throughout... if not something odd is going on with your use of ret. Hmm. I see it is in the original code :( feel free to clean this up. > + ret = 0; > + > return ret; > } > > -/* Parse report which is specific to an usage id*/ > +/* Read the HID reports and setup IIO Channels */ > static int magn_3d_parse_report(struct platform_device *pdev, > struct hid_sensor_hub_device *hsdev, > - struct iio_chan_spec *channels, > + struct iio_chan_spec *iio_chans, > unsigned usage_id, > struct magn_3d_state *st) > { > - int ret; > + int ret = 0; > int i; > > - for (i = 0; i <= CHANNEL_SCAN_INDEX_Z; ++i) { > - ret = sensor_hub_input_get_attribute_info(hsdev, > - HID_INPUT_REPORT, > - usage_id, > - HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_X_AXIS + i, > - &st->magn[CHANNEL_SCAN_INDEX_X + i]); > - if (ret < 0) > - break; > - magn_3d_adjust_channel_bit_mask(channels, > - CHANNEL_SCAN_INDEX_X + i, > - st->magn[CHANNEL_SCAN_INDEX_X + i].size); > - } > - dev_dbg(&pdev->dev, "magn_3d %x:%x, %x:%x, %x:%x\n", > - st->magn[0].index, > - st->magn[0].report_id, > - st->magn[1].index, st->magn[1].report_id, > - st->magn[2].index, st->magn[2].report_id); > - > - /* Set Sensitivity field ids, when there is no individual modifier */ > - if (st->common_attributes.sensitivity.index < 0) { > - sensor_hub_input_get_attribute_info(hsdev, > - HID_FEATURE_REPORT, usage_id, > - HID_USAGE_SENSOR_DATA_MOD_CHANGE_SENSITIVITY_ABS | > - HID_USAGE_SENSOR_DATA_ORIENTATION, > - &st->common_attributes.sensitivity); > - dev_dbg(&pdev->dev, "Sensitivity index:report %d:%d\n", > - st->common_attributes.sensitivity.index, > - st->common_attributes.sensitivity.report_id); > - } > + dev_dbg(&pdev->dev, "magn_3d_parse_reports Usage ID: %x\n", usage_id); > + for(i = HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_X_AXIS; > + i <= HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Z_AXIS && ret == 0; > + i++) > + ret = magn_3d_setup_new_iio_chan(hsdev, > + usage_id, > + i, > + iio_chans, > + st); > + > + dev_dbg(&pdev->dev, "magn_3d_parse_reports Found %d\n magnetic axis. Returned: %d", st->num_iio_channels, ret); > + for(i = HID_USAGE_SENSOR_ORIENT_COMP_MAGN_NORTH; > + i <= HID_USAGE_SENSOR_ORIENT_TRUE_NORTH && ret == 0; > + i++) > + ret = magn_3d_setup_new_iio_chan(hsdev, > + usage_id, > + i, > + iio_chans, > + st); As stated above, I'd use whether this succeeds for a given channel to increment the location in iio_chans and then pass in only the 'next' channel location on each pass. Moving the bounds checks out here as well would be make for a cleaner magn_3d_setup_new_iio_chan function. > + dev_dbg(&pdev->dev, "magn_3d_parse_reports Found %d\n magnetic channels. Returned: %d", st->num_iio_channels, ret); > > return ret; > } > @@ -307,10 +389,11 @@ static int hid_magn_3d_probe(struct platform_device *pdev) > return ret; > } > > - channels = kmemdup(magn_3d_channels, sizeof(magn_3d_channels), > - GFP_KERNEL); > + channels = kcalloc(MAGN_3D_CHANNEL_MAX, > + sizeof(struct iio_chan_spec), > + GFP_KERNEL); > if (!channels) { > - dev_err(&pdev->dev, "failed to duplicate channels\n"); > + dev_err(&pdev->dev, "failed to allocate memory for iio channel\n"); > return -ENOMEM; > } > > @@ -322,7 +405,7 @@ static int hid_magn_3d_probe(struct platform_device *pdev) > } > > indio_dev->channels = channels; > - indio_dev->num_channels = ARRAY_SIZE(magn_3d_channels); > + indio_dev->num_channels = magn_state->num_iio_channels; With a bit of rearranging it strikes me that you can fill num_channels directly rather than having another copy of it.... > indio_dev->dev.parent = &pdev->dev; > indio_dev->info = &magn_3d_info; > indio_dev->name = name; > ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCHv2 3/3] IIO: hid-sensor-magn-3d: Add in support for True/Magnetic North HID usages @ 2014-06-09 19:55 ` Jonathan Cameron 0 siblings, 0 replies; 23+ messages in thread From: Jonathan Cameron @ 2014-06-09 19:55 UTC (permalink / raw) To: Reyad Attiyat, linux-kernel, linux-iio, srinivas.pandruvada, linux-input, jkosina On 03/06/14 00:14, Reyad Attiyat wrote: > Updated magn_3d_channel enum for all possible north channels > > Added functions to setup iio_chan_spec array depending on which hid usage reports are found > > Renamed magn_val to iio_val to differentiate the index being used > > Updated magn_3d_state struct to hold pointer array (magn_val_addr[]) which points to iio_val[] > > Updated magn_3d_parse_report to scan for all compass usages and create iio channels for each > > Signed-off-by: Reyad Attiyat <reyad.attiyat@gmail.com> Hi Reyad, I've taken a rather quick look at this. Will probably want to take one more close look at a newer version. Various bits and bobs inline - mostly fine! Jonathan > --- > drivers/iio/magnetometer/hid-sensor-magn-3d.c | 271 +++++++++++++++++--------- > 1 file changed, 177 insertions(+), 94 deletions(-) > > diff --git a/drivers/iio/magnetometer/hid-sensor-magn-3d.c b/drivers/iio/magnetometer/hid-sensor-magn-3d.c > index 6d162b7..32f4d90 100644 > --- a/drivers/iio/magnetometer/hid-sensor-magn-3d.c > +++ b/drivers/iio/magnetometer/hid-sensor-magn-3d.c > @@ -34,63 +34,54 @@ enum magn_3d_channel { > CHANNEL_SCAN_INDEX_X, > CHANNEL_SCAN_INDEX_Y, > CHANNEL_SCAN_INDEX_Z, > + CHANNEL_SCAN_INDEX_NORTH_MAGN, > + CHANNEL_SCAN_INDEX_NORTH_TRUE, > + CHANNEL_SCAN_INDEX_NORTH_TILT_COMP, > + CHANNEL_SCAN_INDEX_NORTH_TRUE_TILT_COMP, > MAGN_3D_CHANNEL_MAX, > }; > > +#define IIO_CHANNEL_MAX MAGN_3D_CHANNEL_MAX Please don't define a generic sounding name for a local constant. Why not use MAGN_3D_CHANNEL_MAX everywhere? > + > struct magn_3d_state { > struct hid_sensor_hub_callbacks callbacks; > struct hid_sensor_common common_attributes; > struct hid_sensor_hub_attribute_info magn[MAGN_3D_CHANNEL_MAX]; > - u32 magn_val[MAGN_3D_CHANNEL_MAX]; > -}; > + u32 *magn_val_addr[MAGN_3D_CHANNEL_MAX]; > > -static const u32 magn_3d_addresses[MAGN_3D_CHANNEL_MAX] = { > - HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_X_AXIS, > - HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Y_AXIS, > - HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Z_AXIS > + u32 iio_val[IIO_CHANNEL_MAX]; > + int num_iio_channels; > }; > > -/* Channel definitions */ > -static const struct iio_chan_spec magn_3d_channels[] = { > - { > - .type = IIO_MAGN, > - .modified = 1, > - .channel2 = IIO_MOD_X, > - .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) | > - BIT(IIO_CHAN_INFO_SCALE) | > - BIT(IIO_CHAN_INFO_SAMP_FREQ) | > - BIT(IIO_CHAN_INFO_HYSTERESIS), > - .scan_index = CHANNEL_SCAN_INDEX_X, > - }, { > - .type = IIO_MAGN, > - .modified = 1, > - .channel2 = IIO_MOD_Y, > - .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) | > - BIT(IIO_CHAN_INFO_SCALE) | > - BIT(IIO_CHAN_INFO_SAMP_FREQ) | > - BIT(IIO_CHAN_INFO_HYSTERESIS), > - .scan_index = CHANNEL_SCAN_INDEX_Y, > - }, { > - .type = IIO_MAGN, > - .modified = 1, > - .channel2 = IIO_MOD_Z, > - .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) | > - BIT(IIO_CHAN_INFO_SCALE) | > - BIT(IIO_CHAN_INFO_SAMP_FREQ) | > - BIT(IIO_CHAN_INFO_HYSTERESIS), > - .scan_index = CHANNEL_SCAN_INDEX_Z, > +/* Find index into magn_3d_state magn[] and magn_val_addr[] from HID Usage */ > +static int magn_3d_usage_id_to_chan_index(unsigned usage_id){ > + int offset = -1; I'd personally prefer offset = -EINVAL and to have it assigned as a default element in the switch statement rather that here. > + > + switch (usage_id) { > + case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_X_AXIS: > + offset = CHANNEL_SCAN_INDEX_X; > + break; > + case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Y_AXIS: > + offset = CHANNEL_SCAN_INDEX_Y; > + break; > + case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Z_AXIS: > + offset = CHANNEL_SCAN_INDEX_Z; > + break; > + case HID_USAGE_SENSOR_ORIENT_COMP_MAGN_NORTH: > + offset = CHANNEL_SCAN_INDEX_NORTH_TILT_COMP; > + break; > + case HID_USAGE_SENSOR_ORIENT_COMP_TRUE_NORTH: > + offset = CHANNEL_SCAN_INDEX_NORTH_TRUE_TILT_COMP; > + break; > + case HID_USAGE_SENSOR_ORIENT_MAGN_NORTH: > + offset = CHANNEL_SCAN_INDEX_NORTH_MAGN; > + break; > + case HID_USAGE_SENSOR_ORIENT_TRUE_NORTH: > + offset = CHANNEL_SCAN_INDEX_NORTH_TRUE; > + break; > } > -}; > > -/* Adjust channel real bits based on report descriptor */ > -static void magn_3d_adjust_channel_bit_mask(struct iio_chan_spec *channels, > - int channel, int size) > -{ > - channels[channel].scan_type.sign = 's'; > - /* Real storage bits will change based on the report desc. */ > - channels[channel].scan_type.realbits = size * 8; > - /* Maximum size of a sample to capture is u32 */ > - channels[channel].scan_type.storagebits = sizeof(u32) * 8; > + return offset; > } > > /* Channel read_raw handler */ > @@ -101,21 +92,31 @@ static int magn_3d_read_raw(struct iio_dev *indio_dev, > { > struct magn_3d_state *magn_state = iio_priv(indio_dev); > int report_id = -1; > - u32 address; > + unsigned usage_id; > + int chan_index = -1; > int ret; > int ret_type; > > + dev_dbg(&indio_dev->dev, "magn_3d_read_raw\n"); > + > *val = 0; > *val2 = 0; > switch (mask) { > case 0: > + /* We store the HID usage ID of the iio channel > + * in its address field > + */ > + usage_id = chan->address; > + chan_index = magn_3d_usage_id_to_chan_index(usage_id); > + if(chan_index < 0) > + return -EINVAL; > + > report_id = > - magn_state->magn[chan->scan_index].report_id; > - address = magn_3d_addresses[chan->scan_index]; > + magn_state->magn[chan_index].report_id; > if (report_id >= 0) > *val = sensor_hub_input_attr_get_raw_value( > magn_state->common_attributes.hsdev, > - HID_USAGE_SENSOR_COMPASS_3D, address, > + HID_USAGE_SENSOR_COMPASS_3D, usage_id, > report_id); > else { > *val = 0; > @@ -202,12 +203,13 @@ static int magn_3d_proc_event(struct hid_sensor_hub_device *hsdev, > magn_state->common_attributes.data_ready); > if (magn_state->common_attributes.data_ready) > hid_sensor_push_data(indio_dev, > - magn_state->magn_val, > - sizeof(magn_state->magn_val)); > + &(magn_state->iio_val), > + sizeof(magn_state->iio_val)); > > return 0; > } > > + > /* Capture samples in local storage */ > static int magn_3d_capture_sample(struct hid_sensor_hub_device *hsdev, > unsigned usage_id, > @@ -217,63 +219,143 @@ static int magn_3d_capture_sample(struct hid_sensor_hub_device *hsdev, > struct iio_dev *indio_dev = platform_get_drvdata(priv); > struct magn_3d_state *magn_state = iio_priv(indio_dev); > int offset; > + u32 *magn_val; > int ret = -EINVAL; > > - switch (usage_id) { > + offset = magn_3d_usage_id_to_chan_index(usage_id); > + if(offset < 0) > + return ret; > + > + magn_val = magn_state->magn_val_addr[offset]; > + if(!magn_val) > + return ret; > + > + *(magn_val) = *(u32 *)raw_data; > + > + return 0; > +} > + > +/* Setup the iio_chan_spec for HID Usage ID */ > +static int magn_3d_setup_new_iio_chan(struct hid_sensor_hub_device *hsdev, > + unsigned usage_id, > + unsigned attr_usage_id, > + struct iio_chan_spec *iio_chans, > + struct magn_3d_state *st) > +{ > + int ret = -1; This is kind of backwards to normal practice for this sort of function. Better to maintain ret in the correct state at all times. Hence start with it being 0 and set to negative when there is an error rather than the other way around. > + int iio_index; > + int magn_index; > + struct iio_chan_spec *channel; > + > + /* Setup magn_3d_state for new channel */ > + magn_index = magn_3d_usage_id_to_chan_index(attr_usage_id); > + if(magn_index < 0 || magn_index >= MAGN_3D_CHANNEL_MAX){ > + return -1; > + } > + > + /* Check if usage attribute exists in the sensor hub device */ > + ret = sensor_hub_input_get_attribute_info(hsdev, > + HID_INPUT_REPORT, > + usage_id, > + attr_usage_id, > + &(st->magn[magn_index])); > + if(ret != 0){ > + /* Usage not found. Nothing to setup.*/ > + return 0; > + } > + > + iio_index = st->num_iio_channels; > + if(iio_index < 0 || iio_index >= IIO_CHANNEL_MAX){ I'd be tempted to allocate space dynamically but I guess this works. > + return -2; > + } > + > + /* Check if this usage hasn't been setup */ > + if(st->magn_val_addr[magn_index] != NULL){ Space before that bracket here and elsewhere. Don't return your own error codes. Use standard ones. I guess thees are left from debugging. > + return -3; > + } > + st->magn_val_addr[magn_index] = &(st->iio_val[iio_index]); > + > + /* Setup IIO Channel spec */ > + channel = &(iio_chans[iio_index]); Just pass this is in as a pointer in the first place? That is a pointer to the current channel location, that may or may not be filled by this function. > + > + channel->type = IIO_MAGN; > + channel->address = attr_usage_id; > + channel->modified = 1; > + > + switch (attr_usage_id){ > + case HID_USAGE_SENSOR_ORIENT_COMP_MAGN_NORTH: > + channel->channel2 = IIO_MOD_NORTH_MAGN_TILT_COMP; > + break; > + case HID_USAGE_SENSOR_ORIENT_COMP_TRUE_NORTH: > + channel->channel2 = IIO_MOD_NORTH_TRUE_TILT_COMP; > + break; > + case HID_USAGE_SENSOR_ORIENT_MAGN_NORTH: > + channel->channel2 = IIO_MOD_NORTH_MAGN; > + break; > + case HID_USAGE_SENSOR_ORIENT_TRUE_NORTH: > + channel->channel2 = IIO_MOD_NORTH_TRUE; > + break; > case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_X_AXIS: > + channel->channel2 = IIO_MOD_X; > + break; > case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Y_AXIS: > + channel->channel2 = IIO_MOD_Y; > + break; > case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Z_AXIS: > - offset = usage_id - HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_X_AXIS; > - magn_state->magn_val[CHANNEL_SCAN_INDEX_X + offset] = > - *(u32 *)raw_data; > - ret = 0; > - break; > - default: > + channel->channel2 = IIO_MOD_Z; > break; > + default: > + return -4; huh? why -4. -EINVAL or -ENOSUP or something similar perhaps. > } > > + channel->info_mask_shared_by_type = > + BIT(IIO_CHAN_INFO_OFFSET) | > + BIT(IIO_CHAN_INFO_SCALE) | > + BIT(IIO_CHAN_INFO_SAMP_FREQ) | > + BIT(IIO_CHAN_INFO_HYSTERESIS); > + > + channel->scan_type.sign = 's'; > + /* Real storage bits will change based on the report desc. */ > + channel->scan_type.realbits = st->magn[magn_index].size * 8; > + /* Maximum size of a sample to capture is u32 */ > + channel->scan_type.storagebits = sizeof(u32) * 8; > + I'd keen num_iio_channels outside this function and increment it if this function does not return an error. > + (st->num_iio_channels)++; Don't do this. ret should have been valid throughout... if not something odd is going on with your use of ret. Hmm. I see it is in the original code :( feel free to clean this up. > + ret = 0; > + > return ret; > } > > -/* Parse report which is specific to an usage id*/ > +/* Read the HID reports and setup IIO Channels */ > static int magn_3d_parse_report(struct platform_device *pdev, > struct hid_sensor_hub_device *hsdev, > - struct iio_chan_spec *channels, > + struct iio_chan_spec *iio_chans, > unsigned usage_id, > struct magn_3d_state *st) > { > - int ret; > + int ret = 0; > int i; > > - for (i = 0; i <= CHANNEL_SCAN_INDEX_Z; ++i) { > - ret = sensor_hub_input_get_attribute_info(hsdev, > - HID_INPUT_REPORT, > - usage_id, > - HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_X_AXIS + i, > - &st->magn[CHANNEL_SCAN_INDEX_X + i]); > - if (ret < 0) > - break; > - magn_3d_adjust_channel_bit_mask(channels, > - CHANNEL_SCAN_INDEX_X + i, > - st->magn[CHANNEL_SCAN_INDEX_X + i].size); > - } > - dev_dbg(&pdev->dev, "magn_3d %x:%x, %x:%x, %x:%x\n", > - st->magn[0].index, > - st->magn[0].report_id, > - st->magn[1].index, st->magn[1].report_id, > - st->magn[2].index, st->magn[2].report_id); > - > - /* Set Sensitivity field ids, when there is no individual modifier */ > - if (st->common_attributes.sensitivity.index < 0) { > - sensor_hub_input_get_attribute_info(hsdev, > - HID_FEATURE_REPORT, usage_id, > - HID_USAGE_SENSOR_DATA_MOD_CHANGE_SENSITIVITY_ABS | > - HID_USAGE_SENSOR_DATA_ORIENTATION, > - &st->common_attributes.sensitivity); > - dev_dbg(&pdev->dev, "Sensitivity index:report %d:%d\n", > - st->common_attributes.sensitivity.index, > - st->common_attributes.sensitivity.report_id); > - } > + dev_dbg(&pdev->dev, "magn_3d_parse_reports Usage ID: %x\n", usage_id); > + for(i = HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_X_AXIS; > + i <= HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Z_AXIS && ret == 0; > + i++) > + ret = magn_3d_setup_new_iio_chan(hsdev, > + usage_id, > + i, > + iio_chans, > + st); > + > + dev_dbg(&pdev->dev, "magn_3d_parse_reports Found %d\n magnetic axis. Returned: %d", st->num_iio_channels, ret); > + for(i = HID_USAGE_SENSOR_ORIENT_COMP_MAGN_NORTH; > + i <= HID_USAGE_SENSOR_ORIENT_TRUE_NORTH && ret == 0; > + i++) > + ret = magn_3d_setup_new_iio_chan(hsdev, > + usage_id, > + i, > + iio_chans, > + st); As stated above, I'd use whether this succeeds for a given channel to increment the location in iio_chans and then pass in only the 'next' channel location on each pass. Moving the bounds checks out here as well would be make for a cleaner magn_3d_setup_new_iio_chan function. > + dev_dbg(&pdev->dev, "magn_3d_parse_reports Found %d\n magnetic channels. Returned: %d", st->num_iio_channels, ret); > > return ret; > } > @@ -307,10 +389,11 @@ static int hid_magn_3d_probe(struct platform_device *pdev) > return ret; > } > > - channels = kmemdup(magn_3d_channels, sizeof(magn_3d_channels), > - GFP_KERNEL); > + channels = kcalloc(MAGN_3D_CHANNEL_MAX, > + sizeof(struct iio_chan_spec), > + GFP_KERNEL); > if (!channels) { > - dev_err(&pdev->dev, "failed to duplicate channels\n"); > + dev_err(&pdev->dev, "failed to allocate memory for iio channel\n"); > return -ENOMEM; > } > > @@ -322,7 +405,7 @@ static int hid_magn_3d_probe(struct platform_device *pdev) > } > > indio_dev->channels = channels; > - indio_dev->num_channels = ARRAY_SIZE(magn_3d_channels); > + indio_dev->num_channels = magn_state->num_iio_channels; With a bit of rearranging it strikes me that you can fill num_channels directly rather than having another copy of it.... > indio_dev->dev.parent = &pdev->dev; > indio_dev->info = &magn_3d_info; > indio_dev->name = name; > ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCHv2 3/3] IIO: hid-sensor-magn-3d: Add in support for True/Magnetic North HID usages 2014-06-09 19:55 ` Jonathan Cameron (?) @ 2014-06-10 17:33 ` Reyad Attiyat 2014-06-10 18:29 ` Jonathan Cameron -1 siblings, 1 reply; 23+ messages in thread From: Reyad Attiyat @ 2014-06-10 17:33 UTC (permalink / raw) To: Jonathan Cameron; +Cc: linux-kernel, linux-iio, Srinivas Pandruvada Hey Jonathan, I will make the changes you suggested thanks for reviewing this. Any conclusions on the naming, did you want me to change the sysfs name to "in_rot_from_north_true/magnetic"? On Mon, Jun 9, 2014 at 2:55 PM, Jonathan Cameron <jic23@kernel.org> wrote: > On 03/06/14 00:14, Reyad Attiyat wrote: >> >> Updated magn_3d_channel enum for all possible north channels >> >> Added functions to setup iio_chan_spec array depending on which hid usage >> reports are found >> >> Renamed magn_val to iio_val to differentiate the index being used >> >> Updated magn_3d_state struct to hold pointer array (magn_val_addr[]) which >> points to iio_val[] >> >> Updated magn_3d_parse_report to scan for all compass usages and create iio >> channels for each >> >> Signed-off-by: Reyad Attiyat <reyad.attiyat@gmail.com> > > Hi Reyad, > > I've taken a rather quick look at this. Will probably want to take > one more close look at a newer version. > > Various bits and bobs inline - mostly fine! > Jonathan > >> --- >> drivers/iio/magnetometer/hid-sensor-magn-3d.c | 271 >> +++++++++++++++++--------- >> 1 file changed, 177 insertions(+), 94 deletions(-) >> >> diff --git a/drivers/iio/magnetometer/hid-sensor-magn-3d.c >> b/drivers/iio/magnetometer/hid-sensor-magn-3d.c >> index 6d162b7..32f4d90 100644 >> --- a/drivers/iio/magnetometer/hid-sensor-magn-3d.c >> +++ b/drivers/iio/magnetometer/hid-sensor-magn-3d.c >> @@ -34,63 +34,54 @@ enum magn_3d_channel { >> CHANNEL_SCAN_INDEX_X, >> CHANNEL_SCAN_INDEX_Y, >> CHANNEL_SCAN_INDEX_Z, >> + CHANNEL_SCAN_INDEX_NORTH_MAGN, >> + CHANNEL_SCAN_INDEX_NORTH_TRUE, >> + CHANNEL_SCAN_INDEX_NORTH_TILT_COMP, >> + CHANNEL_SCAN_INDEX_NORTH_TRUE_TILT_COMP, >> MAGN_3D_CHANNEL_MAX, >> }; >> >> +#define IIO_CHANNEL_MAX MAGN_3D_CHANNEL_MAX > > Please don't define a generic sounding name for a local > constant. Why not use MAGN_3D_CHANNEL_MAX everywhere? > >> + >> struct magn_3d_state { >> struct hid_sensor_hub_callbacks callbacks; >> struct hid_sensor_common common_attributes; >> struct hid_sensor_hub_attribute_info magn[MAGN_3D_CHANNEL_MAX]; >> - u32 magn_val[MAGN_3D_CHANNEL_MAX]; >> -}; >> + u32 *magn_val_addr[MAGN_3D_CHANNEL_MAX]; >> >> -static const u32 magn_3d_addresses[MAGN_3D_CHANNEL_MAX] = { >> - HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_X_AXIS, >> - HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Y_AXIS, >> - HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Z_AXIS >> + u32 iio_val[IIO_CHANNEL_MAX]; >> + int num_iio_channels; >> }; >> >> -/* Channel definitions */ >> -static const struct iio_chan_spec magn_3d_channels[] = { >> - { >> - .type = IIO_MAGN, >> - .modified = 1, >> - .channel2 = IIO_MOD_X, >> - .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) | >> - BIT(IIO_CHAN_INFO_SCALE) | >> - BIT(IIO_CHAN_INFO_SAMP_FREQ) | >> - BIT(IIO_CHAN_INFO_HYSTERESIS), >> - .scan_index = CHANNEL_SCAN_INDEX_X, >> - }, { >> - .type = IIO_MAGN, >> - .modified = 1, >> - .channel2 = IIO_MOD_Y, >> - .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) | >> - BIT(IIO_CHAN_INFO_SCALE) | >> - BIT(IIO_CHAN_INFO_SAMP_FREQ) | >> - BIT(IIO_CHAN_INFO_HYSTERESIS), >> - .scan_index = CHANNEL_SCAN_INDEX_Y, >> - }, { >> - .type = IIO_MAGN, >> - .modified = 1, >> - .channel2 = IIO_MOD_Z, >> - .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) | >> - BIT(IIO_CHAN_INFO_SCALE) | >> - BIT(IIO_CHAN_INFO_SAMP_FREQ) | >> - BIT(IIO_CHAN_INFO_HYSTERESIS), >> - .scan_index = CHANNEL_SCAN_INDEX_Z, >> +/* Find index into magn_3d_state magn[] and magn_val_addr[] from HID >> Usage */ >> +static int magn_3d_usage_id_to_chan_index(unsigned usage_id){ >> + int offset = -1; > > I'd personally prefer offset = -EINVAL and to have it assigned as > a default element in the switch statement rather that here. > >> + >> + switch (usage_id) { >> + case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_X_AXIS: >> + offset = CHANNEL_SCAN_INDEX_X; >> + break; >> + case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Y_AXIS: >> + offset = CHANNEL_SCAN_INDEX_Y; >> + break; >> + case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Z_AXIS: >> + offset = CHANNEL_SCAN_INDEX_Z; >> + break; >> + case HID_USAGE_SENSOR_ORIENT_COMP_MAGN_NORTH: >> + offset = CHANNEL_SCAN_INDEX_NORTH_TILT_COMP; >> + break; >> + case HID_USAGE_SENSOR_ORIENT_COMP_TRUE_NORTH: >> + offset = CHANNEL_SCAN_INDEX_NORTH_TRUE_TILT_COMP; >> + break; >> + case HID_USAGE_SENSOR_ORIENT_MAGN_NORTH: >> + offset = CHANNEL_SCAN_INDEX_NORTH_MAGN; >> + break; >> + case HID_USAGE_SENSOR_ORIENT_TRUE_NORTH: >> + offset = CHANNEL_SCAN_INDEX_NORTH_TRUE; >> + break; >> } >> -}; >> >> -/* Adjust channel real bits based on report descriptor */ >> -static void magn_3d_adjust_channel_bit_mask(struct iio_chan_spec >> *channels, >> - int channel, int size) >> -{ >> - channels[channel].scan_type.sign = 's'; >> - /* Real storage bits will change based on the report desc. */ >> - channels[channel].scan_type.realbits = size * 8; >> - /* Maximum size of a sample to capture is u32 */ >> - channels[channel].scan_type.storagebits = sizeof(u32) * 8; >> + return offset; >> } >> >> /* Channel read_raw handler */ >> @@ -101,21 +92,31 @@ static int magn_3d_read_raw(struct iio_dev >> *indio_dev, >> { >> struct magn_3d_state *magn_state = iio_priv(indio_dev); >> int report_id = -1; >> - u32 address; >> + unsigned usage_id; >> + int chan_index = -1; >> int ret; >> int ret_type; >> >> + dev_dbg(&indio_dev->dev, "magn_3d_read_raw\n"); >> + >> *val = 0; >> *val2 = 0; >> switch (mask) { >> case 0: >> + /* We store the HID usage ID of the iio channel >> + * in its address field >> + */ >> + usage_id = chan->address; >> + chan_index = magn_3d_usage_id_to_chan_index(usage_id); > > >> + if(chan_index < 0) >> + return -EINVAL; >> + >> report_id = >> - magn_state->magn[chan->scan_index].report_id; >> - address = magn_3d_addresses[chan->scan_index]; >> + magn_state->magn[chan_index].report_id; >> if (report_id >= 0) >> *val = sensor_hub_input_attr_get_raw_value( >> magn_state->common_attributes.hsdev, >> - HID_USAGE_SENSOR_COMPASS_3D, address, >> + HID_USAGE_SENSOR_COMPASS_3D, usage_id, >> report_id); >> else { >> *val = 0; >> @@ -202,12 +203,13 @@ static int magn_3d_proc_event(struct >> hid_sensor_hub_device *hsdev, >> magn_state->common_attributes.data_ready); >> if (magn_state->common_attributes.data_ready) >> hid_sensor_push_data(indio_dev, >> - magn_state->magn_val, >> - sizeof(magn_state->magn_val)); >> + &(magn_state->iio_val), >> + sizeof(magn_state->iio_val)); >> >> return 0; >> } >> >> + >> /* Capture samples in local storage */ >> static int magn_3d_capture_sample(struct hid_sensor_hub_device *hsdev, >> unsigned usage_id, >> @@ -217,63 +219,143 @@ static int magn_3d_capture_sample(struct >> hid_sensor_hub_device *hsdev, >> struct iio_dev *indio_dev = platform_get_drvdata(priv); >> struct magn_3d_state *magn_state = iio_priv(indio_dev); >> int offset; >> + u32 *magn_val; >> int ret = -EINVAL; >> >> - switch (usage_id) { >> + offset = magn_3d_usage_id_to_chan_index(usage_id); >> + if(offset < 0) >> + return ret; >> + >> + magn_val = magn_state->magn_val_addr[offset]; >> + if(!magn_val) >> + return ret; >> + >> + *(magn_val) = *(u32 *)raw_data; >> + >> + return 0; >> +} >> + >> +/* Setup the iio_chan_spec for HID Usage ID */ >> +static int magn_3d_setup_new_iio_chan(struct hid_sensor_hub_device >> *hsdev, >> + unsigned usage_id, >> + unsigned attr_usage_id, >> + struct iio_chan_spec *iio_chans, >> + struct magn_3d_state *st) >> +{ >> + int ret = -1; > > This is kind of backwards to normal practice for this sort of function. > Better to maintain ret in the correct state at all times. Hence > start with it being 0 and set to negative when there is an error rather > than the other way around. > >> + int iio_index; >> + int magn_index; >> + struct iio_chan_spec *channel; >> + >> + /* Setup magn_3d_state for new channel */ >> + magn_index = magn_3d_usage_id_to_chan_index(attr_usage_id); >> + if(magn_index < 0 || magn_index >= MAGN_3D_CHANNEL_MAX){ >> + return -1; >> + } >> + >> + /* Check if usage attribute exists in the sensor hub device */ >> + ret = sensor_hub_input_get_attribute_info(hsdev, >> + HID_INPUT_REPORT, >> + usage_id, >> + attr_usage_id, >> + &(st->magn[magn_index])); >> + if(ret != 0){ >> + /* Usage not found. Nothing to setup.*/ >> + return 0; >> + } >> + >> + iio_index = st->num_iio_channels; >> + if(iio_index < 0 || iio_index >= IIO_CHANNEL_MAX){ > > I'd be tempted to allocate space dynamically but I guess this works. > > >> + return -2; >> + } >> + >> + /* Check if this usage hasn't been setup */ >> + if(st->magn_val_addr[magn_index] != NULL){ > > Space before that bracket here and elsewhere. > Don't return your own error codes. Use standard ones. I guess thees > are left from debugging. > >> + return -3; >> + } >> + st->magn_val_addr[magn_index] = &(st->iio_val[iio_index]); >> + >> + /* Setup IIO Channel spec */ >> + channel = &(iio_chans[iio_index]); > > Just pass this is in as a pointer in the first place? > That is a pointer to the current channel location, that may or may > not be filled by this function. > >> + >> + channel->type = IIO_MAGN; >> + channel->address = attr_usage_id; >> + channel->modified = 1; >> + >> + switch (attr_usage_id){ >> + case HID_USAGE_SENSOR_ORIENT_COMP_MAGN_NORTH: >> + channel->channel2 = IIO_MOD_NORTH_MAGN_TILT_COMP; >> + break; >> + case HID_USAGE_SENSOR_ORIENT_COMP_TRUE_NORTH: >> + channel->channel2 = IIO_MOD_NORTH_TRUE_TILT_COMP; >> + break; >> + case HID_USAGE_SENSOR_ORIENT_MAGN_NORTH: >> + channel->channel2 = IIO_MOD_NORTH_MAGN; >> + break; >> + case HID_USAGE_SENSOR_ORIENT_TRUE_NORTH: >> + channel->channel2 = IIO_MOD_NORTH_TRUE; >> + break; >> case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_X_AXIS: >> + channel->channel2 = IIO_MOD_X; >> + break; >> case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Y_AXIS: >> + channel->channel2 = IIO_MOD_Y; >> + break; >> case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Z_AXIS: >> - offset = usage_id - >> HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_X_AXIS; >> - magn_state->magn_val[CHANNEL_SCAN_INDEX_X + offset] = >> - *(u32 *)raw_data; >> - ret = 0; >> - break; >> - default: >> + channel->channel2 = IIO_MOD_Z; >> break; >> + default: >> + return -4; > > huh? why -4. -EINVAL or -ENOSUP or something similar perhaps. > >> } >> >> + channel->info_mask_shared_by_type = >> + BIT(IIO_CHAN_INFO_OFFSET) | >> + BIT(IIO_CHAN_INFO_SCALE) | >> + BIT(IIO_CHAN_INFO_SAMP_FREQ) | >> + BIT(IIO_CHAN_INFO_HYSTERESIS); >> + >> + channel->scan_type.sign = 's'; >> + /* Real storage bits will change based on the report desc. */ >> + channel->scan_type.realbits = st->magn[magn_index].size * 8; >> + /* Maximum size of a sample to capture is u32 */ >> + channel->scan_type.storagebits = sizeof(u32) * 8; >> + > > I'd keen num_iio_channels outside this function and increment it if > this function does not return an error. >> >> + (st->num_iio_channels)++; > > Don't do this. ret should have been valid throughout... if not something > odd is going on with your use of ret. > > Hmm. I see it is in the original code :( feel free to clean this up. > >> + ret = 0; >> + >> return ret; >> } >> >> -/* Parse report which is specific to an usage id*/ >> +/* Read the HID reports and setup IIO Channels */ >> static int magn_3d_parse_report(struct platform_device *pdev, >> struct hid_sensor_hub_device *hsdev, >> - struct iio_chan_spec *channels, >> + struct iio_chan_spec *iio_chans, >> unsigned usage_id, >> struct magn_3d_state *st) >> { >> - int ret; >> + int ret = 0; >> int i; >> >> - for (i = 0; i <= CHANNEL_SCAN_INDEX_Z; ++i) { >> - ret = sensor_hub_input_get_attribute_info(hsdev, >> - HID_INPUT_REPORT, >> - usage_id, >> - HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_X_AXIS + >> i, >> - &st->magn[CHANNEL_SCAN_INDEX_X + i]); >> - if (ret < 0) >> - break; >> - magn_3d_adjust_channel_bit_mask(channels, >> - CHANNEL_SCAN_INDEX_X + i, >> - st->magn[CHANNEL_SCAN_INDEX_X + i].size); >> - } >> - dev_dbg(&pdev->dev, "magn_3d %x:%x, %x:%x, %x:%x\n", >> - st->magn[0].index, >> - st->magn[0].report_id, >> - st->magn[1].index, st->magn[1].report_id, >> - st->magn[2].index, st->magn[2].report_id); >> - >> - /* Set Sensitivity field ids, when there is no individual modifier >> */ >> - if (st->common_attributes.sensitivity.index < 0) { >> - sensor_hub_input_get_attribute_info(hsdev, >> - HID_FEATURE_REPORT, usage_id, >> - HID_USAGE_SENSOR_DATA_MOD_CHANGE_SENSITIVITY_ABS | >> - HID_USAGE_SENSOR_DATA_ORIENTATION, >> - &st->common_attributes.sensitivity); >> - dev_dbg(&pdev->dev, "Sensitivity index:report %d:%d\n", >> - st->common_attributes.sensitivity.index, >> - st->common_attributes.sensitivity.report_id); >> - } >> + dev_dbg(&pdev->dev, "magn_3d_parse_reports Usage ID: %x\n", >> usage_id); >> + for(i = HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_X_AXIS; >> + i <= HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Z_AXIS && ret == 0; >> + i++) >> + ret = magn_3d_setup_new_iio_chan(hsdev, >> + usage_id, >> + i, >> + iio_chans, >> + st); >> + >> + dev_dbg(&pdev->dev, "magn_3d_parse_reports Found %d\n magnetic >> axis. Returned: %d", st->num_iio_channels, ret); >> + for(i = HID_USAGE_SENSOR_ORIENT_COMP_MAGN_NORTH; >> + i <= HID_USAGE_SENSOR_ORIENT_TRUE_NORTH && ret == 0; >> + i++) >> + ret = magn_3d_setup_new_iio_chan(hsdev, >> + usage_id, >> + i, >> + iio_chans, >> + st); > > As stated above, I'd use whether this succeeds for a given channel to > increment the location in iio_chans and then pass in only the 'next' channel > location on each pass. Moving the bounds checks out here as well would be > make for a cleaner magn_3d_setup_new_iio_chan function. > > >> + dev_dbg(&pdev->dev, "magn_3d_parse_reports Found %d\n magnetic >> channels. Returned: %d", st->num_iio_channels, ret); >> >> return ret; >> } >> @@ -307,10 +389,11 @@ static int hid_magn_3d_probe(struct platform_device >> *pdev) >> return ret; >> } >> >> - channels = kmemdup(magn_3d_channels, sizeof(magn_3d_channels), >> - GFP_KERNEL); >> + channels = kcalloc(MAGN_3D_CHANNEL_MAX, >> + sizeof(struct iio_chan_spec), >> + GFP_KERNEL); >> if (!channels) { >> - dev_err(&pdev->dev, "failed to duplicate channels\n"); >> + dev_err(&pdev->dev, "failed to allocate memory for iio >> channel\n"); >> return -ENOMEM; >> } >> >> @@ -322,7 +405,7 @@ static int hid_magn_3d_probe(struct platform_device >> *pdev) >> } >> >> indio_dev->channels = channels; >> - indio_dev->num_channels = ARRAY_SIZE(magn_3d_channels); >> + indio_dev->num_channels = magn_state->num_iio_channels; > > With a bit of rearranging it strikes me that you can fill num_channels > directly rather than having another copy of it.... > >> indio_dev->dev.parent = &pdev->dev; >> indio_dev->info = &magn_3d_info; >> indio_dev->name = name; >> > ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCHv2 3/3] IIO: hid-sensor-magn-3d: Add in support for True/Magnetic North HID usages 2014-06-10 17:33 ` Reyad Attiyat @ 2014-06-10 18:29 ` Jonathan Cameron 0 siblings, 0 replies; 23+ messages in thread From: Jonathan Cameron @ 2014-06-10 18:29 UTC (permalink / raw) To: Reyad Attiyat; +Cc: linux-kernel, linux-iio, Srinivas Pandruvada On June 10, 2014 6:33:25 PM GMT+01:00, Reyad Attiyat <reyad.attiyat@gmail.com> wrote: >Hey Jonathan, > >I will make the changes you suggested thanks for reviewing this. Any >conclusions on the naming, did you want me to change the sysfs name to >"in_rot_from_north_true/magnetic"? Yes. I think that is the cleanest option from those we have considered! > >On Mon, Jun 9, 2014 at 2:55 PM, Jonathan Cameron <jic23@kernel.org> >wrote: >> On 03/06/14 00:14, Reyad Attiyat wrote: >>> >>> Updated magn_3d_channel enum for all possible north channels >>> >>> Added functions to setup iio_chan_spec array depending on which hid >usage >>> reports are found >>> >>> Renamed magn_val to iio_val to differentiate the index being used >>> >>> Updated magn_3d_state struct to hold pointer array (magn_val_addr[]) >which >>> points to iio_val[] >>> >>> Updated magn_3d_parse_report to scan for all compass usages and >create iio >>> channels for each >>> >>> Signed-off-by: Reyad Attiyat <reyad.attiyat@gmail.com> >> >> Hi Reyad, >> >> I've taken a rather quick look at this. Will probably want to take >> one more close look at a newer version. >> >> Various bits and bobs inline - mostly fine! >> Jonathan >> >>> --- >>> drivers/iio/magnetometer/hid-sensor-magn-3d.c | 271 >>> +++++++++++++++++--------- >>> 1 file changed, 177 insertions(+), 94 deletions(-) >>> >>> diff --git a/drivers/iio/magnetometer/hid-sensor-magn-3d.c >>> b/drivers/iio/magnetometer/hid-sensor-magn-3d.c >>> index 6d162b7..32f4d90 100644 >>> --- a/drivers/iio/magnetometer/hid-sensor-magn-3d.c >>> +++ b/drivers/iio/magnetometer/hid-sensor-magn-3d.c >>> @@ -34,63 +34,54 @@ enum magn_3d_channel { >>> CHANNEL_SCAN_INDEX_X, >>> CHANNEL_SCAN_INDEX_Y, >>> CHANNEL_SCAN_INDEX_Z, >>> + CHANNEL_SCAN_INDEX_NORTH_MAGN, >>> + CHANNEL_SCAN_INDEX_NORTH_TRUE, >>> + CHANNEL_SCAN_INDEX_NORTH_TILT_COMP, >>> + CHANNEL_SCAN_INDEX_NORTH_TRUE_TILT_COMP, >>> MAGN_3D_CHANNEL_MAX, >>> }; >>> >>> +#define IIO_CHANNEL_MAX MAGN_3D_CHANNEL_MAX >> >> Please don't define a generic sounding name for a local >> constant. Why not use MAGN_3D_CHANNEL_MAX everywhere? >> >>> + >>> struct magn_3d_state { >>> struct hid_sensor_hub_callbacks callbacks; >>> struct hid_sensor_common common_attributes; >>> struct hid_sensor_hub_attribute_info >magn[MAGN_3D_CHANNEL_MAX]; >>> - u32 magn_val[MAGN_3D_CHANNEL_MAX]; >>> -}; >>> + u32 *magn_val_addr[MAGN_3D_CHANNEL_MAX]; >>> >>> -static const u32 magn_3d_addresses[MAGN_3D_CHANNEL_MAX] = { >>> - HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_X_AXIS, >>> - HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Y_AXIS, >>> - HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Z_AXIS >>> + u32 iio_val[IIO_CHANNEL_MAX]; >>> + int num_iio_channels; >>> }; >>> >>> -/* Channel definitions */ >>> -static const struct iio_chan_spec magn_3d_channels[] = { >>> - { >>> - .type = IIO_MAGN, >>> - .modified = 1, >>> - .channel2 = IIO_MOD_X, >>> - .info_mask_shared_by_type = >BIT(IIO_CHAN_INFO_OFFSET) | >>> - BIT(IIO_CHAN_INFO_SCALE) | >>> - BIT(IIO_CHAN_INFO_SAMP_FREQ) | >>> - BIT(IIO_CHAN_INFO_HYSTERESIS), >>> - .scan_index = CHANNEL_SCAN_INDEX_X, >>> - }, { >>> - .type = IIO_MAGN, >>> - .modified = 1, >>> - .channel2 = IIO_MOD_Y, >>> - .info_mask_shared_by_type = >BIT(IIO_CHAN_INFO_OFFSET) | >>> - BIT(IIO_CHAN_INFO_SCALE) | >>> - BIT(IIO_CHAN_INFO_SAMP_FREQ) | >>> - BIT(IIO_CHAN_INFO_HYSTERESIS), >>> - .scan_index = CHANNEL_SCAN_INDEX_Y, >>> - }, { >>> - .type = IIO_MAGN, >>> - .modified = 1, >>> - .channel2 = IIO_MOD_Z, >>> - .info_mask_shared_by_type = >BIT(IIO_CHAN_INFO_OFFSET) | >>> - BIT(IIO_CHAN_INFO_SCALE) | >>> - BIT(IIO_CHAN_INFO_SAMP_FREQ) | >>> - BIT(IIO_CHAN_INFO_HYSTERESIS), >>> - .scan_index = CHANNEL_SCAN_INDEX_Z, >>> +/* Find index into magn_3d_state magn[] and magn_val_addr[] from >HID >>> Usage */ >>> +static int magn_3d_usage_id_to_chan_index(unsigned usage_id){ >>> + int offset = -1; >> >> I'd personally prefer offset = -EINVAL and to have it assigned as >> a default element in the switch statement rather that here. >> >>> + >>> + switch (usage_id) { >>> + case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_X_AXIS: >>> + offset = CHANNEL_SCAN_INDEX_X; >>> + break; >>> + case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Y_AXIS: >>> + offset = CHANNEL_SCAN_INDEX_Y; >>> + break; >>> + case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Z_AXIS: >>> + offset = CHANNEL_SCAN_INDEX_Z; >>> + break; >>> + case HID_USAGE_SENSOR_ORIENT_COMP_MAGN_NORTH: >>> + offset = CHANNEL_SCAN_INDEX_NORTH_TILT_COMP; >>> + break; >>> + case HID_USAGE_SENSOR_ORIENT_COMP_TRUE_NORTH: >>> + offset = CHANNEL_SCAN_INDEX_NORTH_TRUE_TILT_COMP; >>> + break; >>> + case HID_USAGE_SENSOR_ORIENT_MAGN_NORTH: >>> + offset = CHANNEL_SCAN_INDEX_NORTH_MAGN; >>> + break; >>> + case HID_USAGE_SENSOR_ORIENT_TRUE_NORTH: >>> + offset = CHANNEL_SCAN_INDEX_NORTH_TRUE; >>> + break; >>> } >>> -}; >>> >>> -/* Adjust channel real bits based on report descriptor */ >>> -static void magn_3d_adjust_channel_bit_mask(struct iio_chan_spec >>> *channels, >>> - int channel, int >size) >>> -{ >>> - channels[channel].scan_type.sign = 's'; >>> - /* Real storage bits will change based on the report desc. >*/ >>> - channels[channel].scan_type.realbits = size * 8; >>> - /* Maximum size of a sample to capture is u32 */ >>> - channels[channel].scan_type.storagebits = sizeof(u32) * 8; >>> + return offset; >>> } >>> >>> /* Channel read_raw handler */ >>> @@ -101,21 +92,31 @@ static int magn_3d_read_raw(struct iio_dev >>> *indio_dev, >>> { >>> struct magn_3d_state *magn_state = iio_priv(indio_dev); >>> int report_id = -1; >>> - u32 address; >>> + unsigned usage_id; >>> + int chan_index = -1; >>> int ret; >>> int ret_type; >>> >>> + dev_dbg(&indio_dev->dev, "magn_3d_read_raw\n"); >>> + >>> *val = 0; >>> *val2 = 0; >>> switch (mask) { >>> case 0: >>> + /* We store the HID usage ID of the iio channel >>> + * in its address field >>> + */ >>> + usage_id = chan->address; >>> + chan_index = >magn_3d_usage_id_to_chan_index(usage_id); >> >> >>> + if(chan_index < 0) >>> + return -EINVAL; >>> + >>> report_id = >>> - >magn_state->magn[chan->scan_index].report_id; >>> - address = magn_3d_addresses[chan->scan_index]; >>> + magn_state->magn[chan_index].report_id; >>> if (report_id >= 0) >>> *val = sensor_hub_input_attr_get_raw_value( >>> magn_state->common_attributes.hsdev, >>> - HID_USAGE_SENSOR_COMPASS_3D, >address, >>> + HID_USAGE_SENSOR_COMPASS_3D, >usage_id, >>> report_id); >>> else { >>> *val = 0; >>> @@ -202,12 +203,13 @@ static int magn_3d_proc_event(struct >>> hid_sensor_hub_device *hsdev, >>> >magn_state->common_attributes.data_ready); >>> if (magn_state->common_attributes.data_ready) >>> hid_sensor_push_data(indio_dev, >>> - magn_state->magn_val, >>> - sizeof(magn_state->magn_val)); >>> + &(magn_state->iio_val), >>> + sizeof(magn_state->iio_val)); >>> >>> return 0; >>> } >>> >>> + >>> /* Capture samples in local storage */ >>> static int magn_3d_capture_sample(struct hid_sensor_hub_device >*hsdev, >>> unsigned usage_id, >>> @@ -217,63 +219,143 @@ static int magn_3d_capture_sample(struct >>> hid_sensor_hub_device *hsdev, >>> struct iio_dev *indio_dev = platform_get_drvdata(priv); >>> struct magn_3d_state *magn_state = iio_priv(indio_dev); >>> int offset; >>> + u32 *magn_val; >>> int ret = -EINVAL; >>> >>> - switch (usage_id) { >>> + offset = magn_3d_usage_id_to_chan_index(usage_id); >>> + if(offset < 0) >>> + return ret; >>> + >>> + magn_val = magn_state->magn_val_addr[offset]; >>> + if(!magn_val) >>> + return ret; >>> + >>> + *(magn_val) = *(u32 *)raw_data; >>> + >>> + return 0; >>> +} >>> + >>> +/* Setup the iio_chan_spec for HID Usage ID */ >>> +static int magn_3d_setup_new_iio_chan(struct hid_sensor_hub_device >>> *hsdev, >>> + unsigned usage_id, >>> + unsigned attr_usage_id, >>> + struct iio_chan_spec *iio_chans, >>> + struct magn_3d_state *st) >>> +{ >>> + int ret = -1; >> >> This is kind of backwards to normal practice for this sort of >function. >> Better to maintain ret in the correct state at all times. Hence >> start with it being 0 and set to negative when there is an error >rather >> than the other way around. >> >>> + int iio_index; >>> + int magn_index; >>> + struct iio_chan_spec *channel; >>> + >>> + /* Setup magn_3d_state for new channel */ >>> + magn_index = magn_3d_usage_id_to_chan_index(attr_usage_id); >>> + if(magn_index < 0 || magn_index >= MAGN_3D_CHANNEL_MAX){ >>> + return -1; >>> + } >>> + >>> + /* Check if usage attribute exists in the sensor hub device >*/ >>> + ret = sensor_hub_input_get_attribute_info(hsdev, >>> + HID_INPUT_REPORT, >>> + usage_id, >>> + attr_usage_id, >>> + &(st->magn[magn_index])); >>> + if(ret != 0){ >>> + /* Usage not found. Nothing to setup.*/ >>> + return 0; >>> + } >>> + >>> + iio_index = st->num_iio_channels; >>> + if(iio_index < 0 || iio_index >= IIO_CHANNEL_MAX){ >> >> I'd be tempted to allocate space dynamically but I guess this works. >> >> >>> + return -2; >>> + } >>> + >>> + /* Check if this usage hasn't been setup */ >>> + if(st->magn_val_addr[magn_index] != NULL){ >> >> Space before that bracket here and elsewhere. >> Don't return your own error codes. Use standard ones. I guess thees >> are left from debugging. >> >>> + return -3; >>> + } >>> + st->magn_val_addr[magn_index] = &(st->iio_val[iio_index]); >>> + >>> + /* Setup IIO Channel spec */ >>> + channel = &(iio_chans[iio_index]); >> >> Just pass this is in as a pointer in the first place? >> That is a pointer to the current channel location, that may or may >> not be filled by this function. >> >>> + >>> + channel->type = IIO_MAGN; >>> + channel->address = attr_usage_id; >>> + channel->modified = 1; >>> + >>> + switch (attr_usage_id){ >>> + case HID_USAGE_SENSOR_ORIENT_COMP_MAGN_NORTH: >>> + channel->channel2 = IIO_MOD_NORTH_MAGN_TILT_COMP; >>> + break; >>> + case HID_USAGE_SENSOR_ORIENT_COMP_TRUE_NORTH: >>> + channel->channel2 = IIO_MOD_NORTH_TRUE_TILT_COMP; >>> + break; >>> + case HID_USAGE_SENSOR_ORIENT_MAGN_NORTH: >>> + channel->channel2 = IIO_MOD_NORTH_MAGN; >>> + break; >>> + case HID_USAGE_SENSOR_ORIENT_TRUE_NORTH: >>> + channel->channel2 = IIO_MOD_NORTH_TRUE; >>> + break; >>> case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_X_AXIS: >>> + channel->channel2 = IIO_MOD_X; >>> + break; >>> case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Y_AXIS: >>> + channel->channel2 = IIO_MOD_Y; >>> + break; >>> case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Z_AXIS: >>> - offset = usage_id - >>> HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_X_AXIS; >>> - magn_state->magn_val[CHANNEL_SCAN_INDEX_X + offset] >= >>> - *(u32 *)raw_data; >>> - ret = 0; >>> - break; >>> - default: >>> + channel->channel2 = IIO_MOD_Z; >>> break; >>> + default: >>> + return -4; >> >> huh? why -4. -EINVAL or -ENOSUP or something similar perhaps. >> >>> } >>> >>> + channel->info_mask_shared_by_type = >>> + BIT(IIO_CHAN_INFO_OFFSET) | >>> + BIT(IIO_CHAN_INFO_SCALE) | >>> + BIT(IIO_CHAN_INFO_SAMP_FREQ) | >>> + BIT(IIO_CHAN_INFO_HYSTERESIS); >>> + >>> + channel->scan_type.sign = 's'; >>> + /* Real storage bits will change based on the report desc. >*/ >>> + channel->scan_type.realbits = st->magn[magn_index].size * 8; >>> + /* Maximum size of a sample to capture is u32 */ >>> + channel->scan_type.storagebits = sizeof(u32) * 8; >>> + >> >> I'd keen num_iio_channels outside this function and increment it if >> this function does not return an error. >>> >>> + (st->num_iio_channels)++; >> >> Don't do this. ret should have been valid throughout... if not >something >> odd is going on with your use of ret. >> >> Hmm. I see it is in the original code :( feel free to clean this up. >> >>> + ret = 0; >>> + >>> return ret; >>> } >>> >>> -/* Parse report which is specific to an usage id*/ >>> +/* Read the HID reports and setup IIO Channels */ >>> static int magn_3d_parse_report(struct platform_device *pdev, >>> struct hid_sensor_hub_device *hsdev, >>> - struct iio_chan_spec *channels, >>> + struct iio_chan_spec *iio_chans, >>> unsigned usage_id, >>> struct magn_3d_state *st) >>> { >>> - int ret; >>> + int ret = 0; >>> int i; >>> >>> - for (i = 0; i <= CHANNEL_SCAN_INDEX_Z; ++i) { >>> - ret = sensor_hub_input_get_attribute_info(hsdev, >>> - HID_INPUT_REPORT, >>> - usage_id, >>> - >HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_X_AXIS + >>> i, >>> - &st->magn[CHANNEL_SCAN_INDEX_X + >i]); >>> - if (ret < 0) >>> - break; >>> - magn_3d_adjust_channel_bit_mask(channels, >>> - CHANNEL_SCAN_INDEX_X + i, >>> - st->magn[CHANNEL_SCAN_INDEX_X + >i].size); >>> - } >>> - dev_dbg(&pdev->dev, "magn_3d %x:%x, %x:%x, %x:%x\n", >>> - st->magn[0].index, >>> - st->magn[0].report_id, >>> - st->magn[1].index, st->magn[1].report_id, >>> - st->magn[2].index, st->magn[2].report_id); >>> - >>> - /* Set Sensitivity field ids, when there is no individual >modifier >>> */ >>> - if (st->common_attributes.sensitivity.index < 0) { >>> - sensor_hub_input_get_attribute_info(hsdev, >>> - HID_FEATURE_REPORT, usage_id, >>> - >HID_USAGE_SENSOR_DATA_MOD_CHANGE_SENSITIVITY_ABS | >>> - HID_USAGE_SENSOR_DATA_ORIENTATION, >>> - &st->common_attributes.sensitivity); >>> - dev_dbg(&pdev->dev, "Sensitivity index:report >%d:%d\n", >>> - st->common_attributes.sensitivity.index, >>> - >st->common_attributes.sensitivity.report_id); >>> - } >>> + dev_dbg(&pdev->dev, "magn_3d_parse_reports Usage ID: %x\n", >>> usage_id); >>> + for(i = HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_X_AXIS; >>> + i <= HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Z_AXIS && ret == >0; >>> + i++) >>> + ret = magn_3d_setup_new_iio_chan(hsdev, >>> + usage_id, >>> + i, >>> + iio_chans, >>> + st); >>> + >>> + dev_dbg(&pdev->dev, "magn_3d_parse_reports Found %d\n >magnetic >>> axis. Returned: %d", st->num_iio_channels, ret); >>> + for(i = HID_USAGE_SENSOR_ORIENT_COMP_MAGN_NORTH; >>> + i <= HID_USAGE_SENSOR_ORIENT_TRUE_NORTH && ret == 0; >>> + i++) >>> + ret = magn_3d_setup_new_iio_chan(hsdev, >>> + usage_id, >>> + i, >>> + iio_chans, >>> + st); >> >> As stated above, I'd use whether this succeeds for a given channel to >> increment the location in iio_chans and then pass in only the 'next' >channel >> location on each pass. Moving the bounds checks out here as well >would be >> make for a cleaner magn_3d_setup_new_iio_chan function. >> >> >>> + dev_dbg(&pdev->dev, "magn_3d_parse_reports Found %d\n >magnetic >>> channels. Returned: %d", st->num_iio_channels, ret); >>> >>> return ret; >>> } >>> @@ -307,10 +389,11 @@ static int hid_magn_3d_probe(struct >platform_device >>> *pdev) >>> return ret; >>> } >>> >>> - channels = kmemdup(magn_3d_channels, >sizeof(magn_3d_channels), >>> - GFP_KERNEL); >>> + channels = kcalloc(MAGN_3D_CHANNEL_MAX, >>> + sizeof(struct iio_chan_spec), >>> + GFP_KERNEL); >>> if (!channels) { >>> - dev_err(&pdev->dev, "failed to duplicate >channels\n"); >>> + dev_err(&pdev->dev, "failed to allocate memory for >iio >>> channel\n"); >>> return -ENOMEM; >>> } >>> >>> @@ -322,7 +405,7 @@ static int hid_magn_3d_probe(struct >platform_device >>> *pdev) >>> } >>> >>> indio_dev->channels = channels; >>> - indio_dev->num_channels = ARRAY_SIZE(magn_3d_channels); >>> + indio_dev->num_channels = magn_state->num_iio_channels; >> >> With a bit of rearranging it strikes me that you can fill >num_channels >> directly rather than having another copy of it.... >> >>> indio_dev->dev.parent = &pdev->dev; >>> indio_dev->info = &magn_3d_info; >>> indio_dev->name = name; >>> >> >-- >To unsubscribe from this list: send the line "unsubscribe linux-iio" in >the body of a message to majordomo@vger.kernel.org >More majordomo info at http://vger.kernel.org/majordomo-info.html -- Sent from my Android phone with K-9 Mail. Please excuse my brevity. ^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2014-06-10 18:29 UTC | newest]
Thread overview: 23+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-06-02 23:14 [PATCHv2 0/3] IIO: Add support for compass north usage attribute Reyad Attiyat
[not found] ` <1401750890-31854-1-git-send-email-reyad.attiyat-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2014-06-02 23:14 ` [PATCHv2 1/3] IIO: Documentation: Add north attribute to ABI docs Reyad Attiyat
2014-06-02 23:14 ` Reyad Attiyat
[not found] ` <1401750890-31854-2-git-send-email-reyad.attiyat-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2014-06-09 17:50 ` Jonathan Cameron
2014-06-09 17:50 ` Jonathan Cameron
2014-06-02 23:14 ` [PATCHv2 2/3] IIO: Add iio_chan modifier for True/Magnetic North HID usages Reyad Attiyat
2014-06-02 23:14 ` Reyad Attiyat
2014-06-09 17:51 ` Jonathan Cameron
[not found] ` <5395F424.4070407-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2014-06-09 17:52 ` Jonathan Cameron
2014-06-09 17:52 ` Jonathan Cameron
2014-06-02 23:14 ` [PATCHv2 3/3] IIO: hid-sensor-magn-3d: Add in support " Reyad Attiyat
[not found] ` <1401750890-31854-4-git-send-email-reyad.attiyat-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2014-06-03 17:43 ` Srinivas Pandruvada
2014-06-03 17:43 ` Srinivas Pandruvada
[not found] ` <538E092F.9040004-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
2014-06-04 15:23 ` Reyad Attiyat
2014-06-04 15:23 ` Reyad Attiyat
2014-06-04 15:42 ` Srinivas Pandruvada
2014-06-09 17:47 ` Jonathan Cameron
2014-06-09 17:43 ` Jonathan Cameron
2014-06-09 17:43 ` Jonathan Cameron
2014-06-09 19:55 ` Jonathan Cameron
2014-06-09 19:55 ` Jonathan Cameron
2014-06-10 17:33 ` Reyad Attiyat
2014-06-10 18:29 ` Jonathan Cameron
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.