Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1391868 > unrolled thread

[PATCH 00/13] Rework for AFE440x drivers to prepare for AFE4405

Started by"Andrew F. Davis" <afd@ti.com>
First post2016-05-01 22:40 +0200
Last post2016-05-04 16:50 +0200
Articles 5 on this page of 25 — 3 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH 00/13] Rework for AFE440x drivers to prepare for AFE4405 "Andrew F. Davis" <afd@ti.com> - 2016-05-01 22:40 +0200
    [PATCH 10/13] iio: health/afe440x: Make gain settings a modifier for the stages "Andrew F. Davis" <afd@ti.com> - 2016-05-01 22:40 +0200
      Re: [PATCH 10/13] iio: health/afe440x: Make gain settings a modifier  for the stages Jonathan Cameron <jic23@kernel.org> - 2016-05-04 16:50 +0200
    [PATCH 01/13] iio: health/afe440x: Fix kernel-doc format "Andrew F. Davis" <afd@ti.com> - 2016-05-01 22:40 +0200
      Re: [PATCH 01/13] iio: health/afe440x: Fix kernel-doc format Jonathan Cameron <jic23@kernel.org> - 2016-05-04 16:50 +0200
    [PATCH 02/13] iio: health/afe440x: Remove of_match_ptr and ifdefs "Andrew F. Davis" <afd@ti.com> - 2016-05-01 22:40 +0200
      Re: [PATCH 02/13] iio: health/afe440x: Remove of_match_ptr and ifdefs Jonathan Cameron <jic23@kernel.org> - 2016-05-04 16:50 +0200
    [PATCH 13/13] iio: health/afe4404: ENSEPGAIN is part of CONTROL2 register "Andrew F. Davis" <afd@ti.com> - 2016-05-01 22:40 +0200
      Re: [PATCH 13/13] iio: health/afe4404: ENSEPGAIN is part of CONTROL2  register Jonathan Cameron <jic23@kernel.org> - 2016-05-04 16:50 +0200
    [PATCH 07/13] iio: health/afe4404: Remove LED3 input channel "Andrew F. Davis" <afd@ti.com> - 2016-05-01 22:40 +0200
      Re: [PATCH 07/13] iio: health/afe4404: Remove LED3 input channel Jonathan Cameron <jic23@kernel.org> - 2016-05-04 16:50 +0200
    [PATCH 09/13] iio: health/afe440x: Use regmap fields "Andrew F. Davis" <afd@ti.com> - 2016-05-01 22:40 +0200
      Re: [PATCH 09/13] iio: health/afe440x: Use regmap fields Jonathan Cameron <jic23@kernel.org> - 2016-05-04 16:50 +0200
        Re: [PATCH 09/13] iio: health/afe440x: Use regmap fields "Andrew F. Davis" <afd@ti.com> - 2016-05-04 17:40 +0200
    [PATCH 04/13] iio: health/afe440x: Always use separate gain values "Andrew F. Davis" <afd@ti.com> - 2016-05-01 22:50 +0200
      Re: [PATCH 04/13] iio: health/afe440x: Always use separate gain  values Jonathan Cameron <jic23@kernel.org> - 2016-05-04 16:50 +0200
        Re: [PATCH 04/13] iio: health/afe440x: Always use separate gain  values "Andrew F. Davis" <afd@ti.com> - 2016-05-04 17:20 +0200
          Re: [PATCH 04/13] iio: health/afe440x: Always use separate gain values Jonathan Cameron <jic23@jic23.retrosnub.co.uk> - 2016-05-04 20:40 +0200
    [PATCH 11/13] iio: health/afe440x: Match LED currents to stages "Andrew F. Davis" <afd@ti.com> - 2016-05-01 22:50 +0200
    [PATCH 12/13] iio: health/afe440x: Remove unused definitions "Andrew F. Davis" <afd@ti.com> - 2016-05-01 22:50 +0200
      Re: [PATCH 12/13] iio: health/afe440x: Remove unused definitions Jonathan Cameron <jic23@kernel.org> - 2016-05-04 16:50 +0200
    [PATCH 08/13] iio: health/afe440x: Remove channel names "Andrew F. Davis" <afd@ti.com> - 2016-05-01 22:50 +0200
      Re: [PATCH 08/13] iio: health/afe440x: Remove channel names Jonathan Cameron <jic23@kernel.org> - 2016-05-04 16:50 +0200
        Re: [PATCH 08/13] iio: health/afe440x: Remove channel names "Andrew F. Davis" <afd@ti.com> - 2016-05-04 17:30 +0200
    Re: [PATCH 00/13] Rework for AFE440x drivers to prepare for AFE4405 Jonathan Cameron <jic23@kernel.org> - 2016-05-04 16:50 +0200

Page 2 of 2 — ← Prev page 1 [2]

#1394383 — Re: [PATCH 12/13] iio: health/afe440x: Remove unused definitions

FromJonathan Cameron <jic23@kernel.org>
Date2016-05-04 16:50 +0200
SubjectRe: [PATCH 12/13] iio: health/afe440x: Remove unused definitions
Message-ID<rv6hH-5S0-5@gated-at.bofh.it>
In reply to#1391877
On 01/05/16 21:37, Andrew F. Davis wrote:
> These definitions are not currently used and if the functionality
> they represent is needed the values should be added back to a table
> for easy userspace use.
> 
> Signed-off-by: Andrew F. Davis <afd@ti.com>
Applied.
> ---
>  drivers/iio/health/afe4403.c | 37 -------------------------------------
>  drivers/iio/health/afe4404.c | 20 --------------------
>  2 files changed, 57 deletions(-)
> 
> diff --git a/drivers/iio/health/afe4403.c b/drivers/iio/health/afe4403.c
> index 059d521..9a08146 100644
> --- a/drivers/iio/health/afe4403.c
> +++ b/drivers/iio/health/afe4403.c
> @@ -39,43 +39,6 @@
>  #define AFE4403_TIAGAIN			0x20
>  #define AFE4403_TIA_AMB_GAIN		0x21
>  
> -/* AFE4403 LEDCNTRL values */
> -#define AFE440X_LEDCNTRL_RANGE_TX_HALF	0x1
> -#define AFE440X_LEDCNTRL_RANGE_TX_FULL	0x2
> -#define AFE440X_LEDCNTRL_RANGE_TX_OFF	0x3
> -
> -/* AFE4403 CONTROL2 values */
> -#define AFE440X_CONTROL2_TX_REF_025	0x0
> -#define AFE440X_CONTROL2_TX_REF_050	0x1
> -#define AFE440X_CONTROL2_TX_REF_100	0x2
> -#define AFE440X_CONTROL2_TX_REF_075	0x3
> -
> -/* AFE4403 CONTROL3 values */
> -#define AFE440X_CONTROL3_CLK_DIV_2	0x0
> -#define AFE440X_CONTROL3_CLK_DIV_4	0x2
> -#define AFE440X_CONTROL3_CLK_DIV_6	0x3
> -#define AFE440X_CONTROL3_CLK_DIV_8	0x4
> -#define AFE440X_CONTROL3_CLK_DIV_12	0x5
> -#define AFE440X_CONTROL3_CLK_DIV_1	0x7
> -
> -/* AFE4403 TIAGAIN_CAP values */
> -#define AFE4403_TIAGAIN_CAP_5_P		0x0
> -#define AFE4403_TIAGAIN_CAP_10_P	0x1
> -#define AFE4403_TIAGAIN_CAP_20_P	0x2
> -#define AFE4403_TIAGAIN_CAP_30_P	0x3
> -#define AFE4403_TIAGAIN_CAP_55_P	0x8
> -#define AFE4403_TIAGAIN_CAP_155_P	0x10
> -
> -/* AFE4403 TIAGAIN_RES values */
> -#define AFE4403_TIAGAIN_RES_500_K	0x0
> -#define AFE4403_TIAGAIN_RES_250_K	0x1
> -#define AFE4403_TIAGAIN_RES_100_K	0x2
> -#define AFE4403_TIAGAIN_RES_50_K	0x3
> -#define AFE4403_TIAGAIN_RES_25_K	0x4
> -#define AFE4403_TIAGAIN_RES_10_K	0x5
> -#define AFE4403_TIAGAIN_RES_1_M		0x6
> -#define AFE4403_TIAGAIN_RES_NONE	0x7
> -
>  enum afe4403_fields {
>  	/* Gains */
>  	F_RF_LED1, F_CF_LED1,
> diff --git a/drivers/iio/health/afe4404.c b/drivers/iio/health/afe4404.c
> index aa8770b..3a8131d 100644
> --- a/drivers/iio/health/afe4404.c
> +++ b/drivers/iio/health/afe4404.c
> @@ -51,26 +51,6 @@
>  /* AFE4404 CONTROL3 register fields */
>  #define AFE440X_CONTROL3_OSC_ENABLE	BIT(9)
>  
> -/* AFE4404 TIA_GAIN_CAP values */
> -#define AFE4404_TIA_GAIN_CAP_5_P	0x0
> -#define AFE4404_TIA_GAIN_CAP_2_5_P	0x1
> -#define AFE4404_TIA_GAIN_CAP_10_P	0x2
> -#define AFE4404_TIA_GAIN_CAP_7_5_P	0x3
> -#define AFE4404_TIA_GAIN_CAP_20_P	0x4
> -#define AFE4404_TIA_GAIN_CAP_17_5_P	0x5
> -#define AFE4404_TIA_GAIN_CAP_25_P	0x6
> -#define AFE4404_TIA_GAIN_CAP_22_5_P	0x7
> -
> -/* AFE4404 TIA_GAIN_RES values */
> -#define AFE4404_TIA_GAIN_RES_500_K	0x0
> -#define AFE4404_TIA_GAIN_RES_250_K	0x1
> -#define AFE4404_TIA_GAIN_RES_100_K	0x2
> -#define AFE4404_TIA_GAIN_RES_50_K	0x3
> -#define AFE4404_TIA_GAIN_RES_25_K	0x4
> -#define AFE4404_TIA_GAIN_RES_10_K	0x5
> -#define AFE4404_TIA_GAIN_RES_1_M	0x6
> -#define AFE4404_TIA_GAIN_RES_2_M	0x7
> -
>  enum afe4404_fields {
>  	/* Gains */
>  	F_TIA_GAIN_SEP, F_TIA_CF_SEP,
>

[toc] | [prev] | [next] | [standalone]

#1391879 — [PATCH 08/13] iio: health/afe440x: Remove channel names

From"Andrew F. Davis" <afd@ti.com>
Date2016-05-01 22:50 +0200
Subject[PATCH 08/13] iio: health/afe440x: Remove channel names
Message-ID<ru6tt-7tP-23@gated-at.bofh.it>
In reply to#1391868
These AFEs have 4 ADC mesuring stages (called LED2, ALED2, LED1, and
ALED1 in the datasheet), we map these as channels, these stages can serve
different purposes depending on the application. For instance the AFE4404
has an additional LED (LED3), this LED can be timed to be active during
stage 2 (or anystage, but the datasheet describes this case and the name
of the stage reflects this use). This ability is used further in upcoming
parts that tie the front-end gain and the LED timings together. For these
reasons we remove explicit naming the channels.

Without channel names it is best that the index numbers are in order to
match the stage number, reorder the channel numbers.

Signed-off-by: Andrew F. Davis <afd@ti.com>
---
 .../ABI/testing/sysfs-bus-iio-health-afe440x       | 39 ++++++++++------------
 drivers/iio/health/afe4403.c                       | 28 ++++++++--------
 drivers/iio/health/afe4404.c                       | 30 ++++++++---------
 drivers/iio/health/afe440x.h                       |  8 ++---
 4 files changed, 51 insertions(+), 54 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-bus-iio-health-afe440x b/Documentation/ABI/testing/sysfs-bus-iio-health-afe440x
index b19053a..a067073 100644
--- a/Documentation/ABI/testing/sysfs-bus-iio-health-afe440x
+++ b/Documentation/ABI/testing/sysfs-bus-iio-health-afe440x
@@ -8,38 +8,35 @@ Description:
 		Transimpedance Amplifier. Y is 1 for Rf1 and Cf1, Y is 2 for
 		Rf2 and Cf2 values.
 
-What:		/sys/bus/iio/devices/iio:deviceX/in_intensity_ledY_raw
-		/sys/bus/iio/devices/iio:deviceX/in_intensity_ledY_ambient_raw
-Date:		December 2015
+What:		/sys/bus/iio/devices/iio:deviceX/in_intensityY_raw
+Date:		May 2016
 KernelVersion:
 Contact:	Andrew F. Davis <afd@ti.com>
 Description:
 		Get measured values from the ADC for these stages. Y is the
-		specific LED number. The values are expressed in 24-bit twos
-		complement.
-
-What:		/sys/bus/iio/devices/iio:deviceX/in_intensity_ledY-ledY_ambient_raw
-Date:		December 2015
-KernelVersion:
-Contact:	Andrew F. Davis <afd@ti.com>
-Description:
-		Get differential values from the ADC for these stages. Y is the
-		specific LED number. The values are expressed in 24-bit twos
-		complement for the specified LEDs.
+		specific stage number corresponding to datasheet stage names
+		as follows:
+		1 -> LED2
+		2 -> ALED2/LED3
+		3 -> LED1
+		4 -> ALED1/LED4
+		Note that channels 5 and 6 represent LED2-ALED2 and LED1-ALED1
+		respectively which simply helper channels containing the
+		calculated difference in the value of stage 1 - 2 and 3 - 4.
+		The values are expressed in 24-bit twos complement.
 
-What:		/sys/bus/iio/devices/iio:deviceX/out_current_ledY_offset
-		/sys/bus/iio/devices/iio:deviceX/out_current_ledY_ambient_offset
-Date:		December 2015
+What:		/sys/bus/iio/devices/iio:deviceX/in_intensityY_offset
+Date:		May 2016
 KernelVersion:
 Contact:	Andrew F. Davis <afd@ti.com>
 Description:
 		Get and set the offset cancellation DAC setting for these
 		stages. The values are expressed in 5-bit sign-magnitude.
 
-What:		/sys/bus/iio/devices/iio:deviceX/out_current_ledY_raw
-Date:		December 2015
+What:		/sys/bus/iio/devices/iio:deviceX/out_currentY_raw
+Date:		May 2016
 KernelVersion:
 Contact:	Andrew F. Davis <afd@ti.com>
 Description:
-		Get and set the LED current for the specified LED. Y is the
-		specific LED number.
+		Get and set the LED current for the specified LED active during
+		this stage. Y is the specific stage number.
diff --git a/drivers/iio/health/afe4403.c b/drivers/iio/health/afe4403.c
index cac6090..4a58064 100644
--- a/drivers/iio/health/afe4403.c
+++ b/drivers/iio/health/afe4403.c
@@ -121,38 +121,38 @@ struct afe4403_data {
 };
 
 enum afe4403_chan_id {
+	LED2 = 1,
+	ALED2,
 	LED1,
 	ALED1,
-	LED2,
-	ALED2,
-	LED1_ALED1,
 	LED2_ALED2,
+	LED1_ALED1,
 	ILED1,
 	ILED2,
 };
 
 static const struct afe440x_reg_info afe4403_reg_info[] = {
-	[LED1] = AFE440X_REG_INFO(AFE440X_LED1VAL, 0, NULL),
-	[ALED1] = AFE440X_REG_INFO(AFE440X_ALED1VAL, 0, NULL),
 	[LED2] = AFE440X_REG_INFO(AFE440X_LED2VAL, 0, NULL),
 	[ALED2] = AFE440X_REG_INFO(AFE440X_ALED2VAL, 0, NULL),
-	[LED1_ALED1] = AFE440X_REG_INFO(AFE440X_LED1_ALED1VAL, 0, NULL),
+	[LED1] = AFE440X_REG_INFO(AFE440X_LED1VAL, 0, NULL),
+	[ALED1] = AFE440X_REG_INFO(AFE440X_ALED1VAL, 0, NULL),
 	[LED2_ALED2] = AFE440X_REG_INFO(AFE440X_LED2_ALED2VAL, 0, NULL),
+	[LED1_ALED1] = AFE440X_REG_INFO(AFE440X_LED1_ALED1VAL, 0, NULL),
 	[ILED1] = AFE440X_REG_INFO(AFE440X_LEDCNTRL, 0, AFE440X_LEDCNTRL_LED1),
 	[ILED2] = AFE440X_REG_INFO(AFE440X_LEDCNTRL, 0, AFE440X_LEDCNTRL_LED2),
 };
 
 static const struct iio_chan_spec afe4403_channels[] = {
 	/* ADC values */
-	AFE440X_INTENSITY_CHAN(LED1, "led1", 0),
-	AFE440X_INTENSITY_CHAN(ALED1, "led1_ambient", 0),
-	AFE440X_INTENSITY_CHAN(LED2, "led2", 0),
-	AFE440X_INTENSITY_CHAN(ALED2, "led2_ambient", 0),
-	AFE440X_INTENSITY_CHAN(LED1_ALED1, "led1-led1_ambient", 0),
-	AFE440X_INTENSITY_CHAN(LED2_ALED2, "led2-led2_ambient", 0),
+	AFE440X_INTENSITY_CHAN(LED2, 0),
+	AFE440X_INTENSITY_CHAN(ALED2, 0),
+	AFE440X_INTENSITY_CHAN(LED1, 0),
+	AFE440X_INTENSITY_CHAN(ALED1, 0),
+	AFE440X_INTENSITY_CHAN(LED2_ALED2, 0),
+	AFE440X_INTENSITY_CHAN(LED1_ALED1, 0),
 	/* LED current */
-	AFE440X_CURRENT_CHAN(ILED1, "led1"),
-	AFE440X_CURRENT_CHAN(ILED2, "led2"),
+	AFE440X_CURRENT_CHAN(ILED1),
+	AFE440X_CURRENT_CHAN(ILED2),
 };
 
 static const struct afe440x_val_table afe4403_res_table[] = {
diff --git a/drivers/iio/health/afe4404.c b/drivers/iio/health/afe4404.c
index 2edb7d7..7806a45 100644
--- a/drivers/iio/health/afe4404.c
+++ b/drivers/iio/health/afe4404.c
@@ -122,24 +122,24 @@ struct afe4404_data {
 };
 
 enum afe4404_chan_id {
+	LED2 = 1,
+	ALED2,
 	LED1,
 	ALED1,
-	LED2,
-	ALED2,
-	LED1_ALED1,
 	LED2_ALED2,
+	LED1_ALED1,
 	ILED1,
 	ILED2,
 	ILED3,
 };
 
 static const struct afe440x_reg_info afe4404_reg_info[] = {
-	[LED1] = AFE440X_REG_INFO(AFE440X_LED1VAL, AFE4404_OFFDAC, AFE4404_OFFDAC_CURR_LED1),
-	[ALED1] = AFE440X_REG_INFO(AFE440X_ALED1VAL, AFE4404_OFFDAC, AFE4404_OFFDAC_CURR_ALED1),
 	[LED2] = AFE440X_REG_INFO(AFE440X_LED2VAL, AFE4404_OFFDAC, AFE4404_OFFDAC_CURR_LED2),
 	[ALED2] = AFE440X_REG_INFO(AFE440X_ALED2VAL, AFE4404_OFFDAC, AFE4404_OFFDAC_CURR_ALED2),
-	[LED1_ALED1] = AFE440X_REG_INFO(AFE440X_LED1_ALED1VAL, 0, NULL),
+	[LED1] = AFE440X_REG_INFO(AFE440X_LED1VAL, AFE4404_OFFDAC, AFE4404_OFFDAC_CURR_LED1),
+	[ALED1] = AFE440X_REG_INFO(AFE440X_ALED1VAL, AFE4404_OFFDAC, AFE4404_OFFDAC_CURR_ALED1),
 	[LED2_ALED2] = AFE440X_REG_INFO(AFE440X_LED2_ALED2VAL, 0, NULL),
+	[LED1_ALED1] = AFE440X_REG_INFO(AFE440X_LED1_ALED1VAL, 0, NULL),
 	[ILED1] = AFE440X_REG_INFO(AFE440X_LEDCNTRL, 0, AFE4404_LEDCNTRL_ILED1),
 	[ILED2] = AFE440X_REG_INFO(AFE440X_LEDCNTRL, 0, AFE4404_LEDCNTRL_ILED2),
 	[ILED3] = AFE440X_REG_INFO(AFE440X_LEDCNTRL, 0, AFE4404_LEDCNTRL_ILED3),
@@ -147,16 +147,16 @@ static const struct afe440x_reg_info afe4404_reg_info[] = {
 
 static const struct iio_chan_spec afe4404_channels[] = {
 	/* ADC values */
-	AFE440X_INTENSITY_CHAN(LED1, "led1", BIT(IIO_CHAN_INFO_OFFSET)),
-	AFE440X_INTENSITY_CHAN(ALED1, "led1_ambient", BIT(IIO_CHAN_INFO_OFFSET)),
-	AFE440X_INTENSITY_CHAN(LED2, "led2", BIT(IIO_CHAN_INFO_OFFSET)),
-	AFE440X_INTENSITY_CHAN(ALED2, "led2_ambient", BIT(IIO_CHAN_INFO_OFFSET)),
-	AFE440X_INTENSITY_CHAN(LED1_ALED1, "led1-led1_ambient", 0),
-	AFE440X_INTENSITY_CHAN(LED2_ALED2, "led2-led2_ambient", 0),
+	AFE440X_INTENSITY_CHAN(LED2, BIT(IIO_CHAN_INFO_OFFSET)),
+	AFE440X_INTENSITY_CHAN(ALED2, BIT(IIO_CHAN_INFO_OFFSET)),
+	AFE440X_INTENSITY_CHAN(LED1, BIT(IIO_CHAN_INFO_OFFSET)),
+	AFE440X_INTENSITY_CHAN(ALED1, BIT(IIO_CHAN_INFO_OFFSET)),
+	AFE440X_INTENSITY_CHAN(LED2_ALED2, 0),
+	AFE440X_INTENSITY_CHAN(LED1_ALED1, 0),
 	/* LED current */
-	AFE440X_CURRENT_CHAN(ILED1, "led1"),
-	AFE440X_CURRENT_CHAN(ILED2, "led2"),
-	AFE440X_CURRENT_CHAN(ILED3, "led3"),
+	AFE440X_CURRENT_CHAN(ILED1),
+	AFE440X_CURRENT_CHAN(ILED2),
+	AFE440X_CURRENT_CHAN(ILED3),
 };
 
 static const struct afe440x_val_table afe4404_res_table[] = {
diff --git a/drivers/iio/health/afe440x.h b/drivers/iio/health/afe440x.h
index 583d071..713972f 100644
--- a/drivers/iio/health/afe440x.h
+++ b/drivers/iio/health/afe440x.h
@@ -103,7 +103,7 @@ struct afe440x_reg_info {
 		.mask = _sm ## _MASK,				\
 	}
 
-#define AFE440X_INTENSITY_CHAN(_index, _name, _mask)		\
+#define AFE440X_INTENSITY_CHAN(_index, _mask)			\
 	{							\
 		.type = IIO_INTENSITY,				\
 		.channel = _index,				\
@@ -115,20 +115,20 @@ struct afe440x_reg_info {
 				.storagebits = 32,		\
 				.endianness = IIO_CPU,		\
 		},						\
-		.extend_name = _name,				\
 		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |	\
 			_mask,					\
+		.indexed = true,				\
 	}
 
-#define AFE440X_CURRENT_CHAN(_index, _name)			\
+#define AFE440X_CURRENT_CHAN(_index)				\
 	{							\
 		.type = IIO_CURRENT,				\
 		.channel = _index,				\
 		.address = _index,				\
 		.scan_index = -1,				\
-		.extend_name = _name,				\
 		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |	\
 			BIT(IIO_CHAN_INFO_SCALE),		\
+		.indexed = true,				\
 		.output = true,					\
 	}
 
-- 
2.8.1

[toc] | [prev] | [next] | [standalone]

#1394387 — Re: [PATCH 08/13] iio: health/afe440x: Remove channel names

FromJonathan Cameron <jic23@kernel.org>
Date2016-05-04 16:50 +0200
SubjectRe: [PATCH 08/13] iio: health/afe440x: Remove channel names
Message-ID<rv6hI-5S0-13@gated-at.bofh.it>
In reply to#1391879
On 01/05/16 21:36, Andrew F. Davis wrote:
> These AFEs have 4 ADC mesuring stages (called LED2, ALED2, LED1, and
> ALED1 in the datasheet), we map these as channels, these stages can serve
> different purposes depending on the application. For instance the AFE4404
> has an additional LED (LED3), this LED can be timed to be active during
> stage 2 (or anystage, but the datasheet describes this case and the name
> of the stage reflects this use). This ability is used further in upcoming
> parts that tie the front-end gain and the LED timings together. For these
> reasons we remove explicit naming the channels.
> 
> Without channel names it is best that the index numbers are in order to
> match the stage number, reorder the channel numbers.
> 
> Signed-off-by: Andrew F. Davis <afd@ti.com>
Youch - this is rather major surgery.

Hmm. Given how non standard an ABI we ended up with in the first place
again, let us cross our fingers that all the userspace for this is
effectively under your control currently.

Getting a little more nervous about this - but worth the risk I think
for the simpler result.

Applied

Jonathan
> ---
>  .../ABI/testing/sysfs-bus-iio-health-afe440x       | 39 ++++++++++------------
>  drivers/iio/health/afe4403.c                       | 28 ++++++++--------
>  drivers/iio/health/afe4404.c                       | 30 ++++++++---------
>  drivers/iio/health/afe440x.h                       |  8 ++---
>  4 files changed, 51 insertions(+), 54 deletions(-)
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-health-afe440x b/Documentation/ABI/testing/sysfs-bus-iio-health-afe440x
> index b19053a..a067073 100644
> --- a/Documentation/ABI/testing/sysfs-bus-iio-health-afe440x
> +++ b/Documentation/ABI/testing/sysfs-bus-iio-health-afe440x
> @@ -8,38 +8,35 @@ Description:
>  		Transimpedance Amplifier. Y is 1 for Rf1 and Cf1, Y is 2 for
>  		Rf2 and Cf2 values.
>  
> -What:		/sys/bus/iio/devices/iio:deviceX/in_intensity_ledY_raw
> -		/sys/bus/iio/devices/iio:deviceX/in_intensity_ledY_ambient_raw
> -Date:		December 2015
> +What:		/sys/bus/iio/devices/iio:deviceX/in_intensityY_raw
> +Date:		May 2016
>  KernelVersion:
>  Contact:	Andrew F. Davis <afd@ti.com>
>  Description:
>  		Get measured values from the ADC for these stages. Y is the
> -		specific LED number. The values are expressed in 24-bit twos
> -		complement.
> -
> -What:		/sys/bus/iio/devices/iio:deviceX/in_intensity_ledY-ledY_ambient_raw
> -Date:		December 2015
> -KernelVersion:
> -Contact:	Andrew F. Davis <afd@ti.com>
> -Description:
> -		Get differential values from the ADC for these stages. Y is the
> -		specific LED number. The values are expressed in 24-bit twos
> -		complement for the specified LEDs.
> +		specific stage number corresponding to datasheet stage names
> +		as follows:
> +		1 -> LED2
> +		2 -> ALED2/LED3
> +		3 -> LED1
> +		4 -> ALED1/LED4
> +		Note that channels 5 and 6 represent LED2-ALED2 and LED1-ALED1
> +		respectively which simply helper channels containing the
> +		calculated difference in the value of stage 1 - 2 and 3 - 4.
> +		The values are expressed in 24-bit twos complement.
>  
> -What:		/sys/bus/iio/devices/iio:deviceX/out_current_ledY_offset
> -		/sys/bus/iio/devices/iio:deviceX/out_current_ledY_ambient_offset
> -Date:		December 2015
> +What:		/sys/bus/iio/devices/iio:deviceX/in_intensityY_offset
> +Date:		May 2016
>  KernelVersion:
>  Contact:	Andrew F. Davis <afd@ti.com>
>  Description:
>  		Get and set the offset cancellation DAC setting for these
>  		stages. The values are expressed in 5-bit sign-magnitude.
>  
> -What:		/sys/bus/iio/devices/iio:deviceX/out_current_ledY_raw
> -Date:		December 2015
> +What:		/sys/bus/iio/devices/iio:deviceX/out_currentY_raw
> +Date:		May 2016
>  KernelVersion:
>  Contact:	Andrew F. Davis <afd@ti.com>
>  Description:
> -		Get and set the LED current for the specified LED. Y is the
> -		specific LED number.
> +		Get and set the LED current for the specified LED active during
> +		this stage. Y is the specific stage number.
> diff --git a/drivers/iio/health/afe4403.c b/drivers/iio/health/afe4403.c
> index cac6090..4a58064 100644
> --- a/drivers/iio/health/afe4403.c
> +++ b/drivers/iio/health/afe4403.c
> @@ -121,38 +121,38 @@ struct afe4403_data {
>  };
>  
>  enum afe4403_chan_id {
> +	LED2 = 1,
> +	ALED2,
>  	LED1,
>  	ALED1,
> -	LED2,
> -	ALED2,
> -	LED1_ALED1,
>  	LED2_ALED2,
> +	LED1_ALED1,
>  	ILED1,
>  	ILED2,
>  };
>  
>  static const struct afe440x_reg_info afe4403_reg_info[] = {
> -	[LED1] = AFE440X_REG_INFO(AFE440X_LED1VAL, 0, NULL),
> -	[ALED1] = AFE440X_REG_INFO(AFE440X_ALED1VAL, 0, NULL),
>  	[LED2] = AFE440X_REG_INFO(AFE440X_LED2VAL, 0, NULL),
>  	[ALED2] = AFE440X_REG_INFO(AFE440X_ALED2VAL, 0, NULL),
> -	[LED1_ALED1] = AFE440X_REG_INFO(AFE440X_LED1_ALED1VAL, 0, NULL),
> +	[LED1] = AFE440X_REG_INFO(AFE440X_LED1VAL, 0, NULL),
> +	[ALED1] = AFE440X_REG_INFO(AFE440X_ALED1VAL, 0, NULL),
>  	[LED2_ALED2] = AFE440X_REG_INFO(AFE440X_LED2_ALED2VAL, 0, NULL),
> +	[LED1_ALED1] = AFE440X_REG_INFO(AFE440X_LED1_ALED1VAL, 0, NULL),
>  	[ILED1] = AFE440X_REG_INFO(AFE440X_LEDCNTRL, 0, AFE440X_LEDCNTRL_LED1),
>  	[ILED2] = AFE440X_REG_INFO(AFE440X_LEDCNTRL, 0, AFE440X_LEDCNTRL_LED2),
>  };
>  
>  static const struct iio_chan_spec afe4403_channels[] = {
>  	/* ADC values */
> -	AFE440X_INTENSITY_CHAN(LED1, "led1", 0),
> -	AFE440X_INTENSITY_CHAN(ALED1, "led1_ambient", 0),
> -	AFE440X_INTENSITY_CHAN(LED2, "led2", 0),
> -	AFE440X_INTENSITY_CHAN(ALED2, "led2_ambient", 0),
> -	AFE440X_INTENSITY_CHAN(LED1_ALED1, "led1-led1_ambient", 0),
> -	AFE440X_INTENSITY_CHAN(LED2_ALED2, "led2-led2_ambient", 0),
> +	AFE440X_INTENSITY_CHAN(LED2, 0),
> +	AFE440X_INTENSITY_CHAN(ALED2, 0),
> +	AFE440X_INTENSITY_CHAN(LED1, 0),
> +	AFE440X_INTENSITY_CHAN(ALED1, 0),
> +	AFE440X_INTENSITY_CHAN(LED2_ALED2, 0),
> +	AFE440X_INTENSITY_CHAN(LED1_ALED1, 0),
>  	/* LED current */
> -	AFE440X_CURRENT_CHAN(ILED1, "led1"),
> -	AFE440X_CURRENT_CHAN(ILED2, "led2"),
> +	AFE440X_CURRENT_CHAN(ILED1),
> +	AFE440X_CURRENT_CHAN(ILED2),
>  };
>  
>  static const struct afe440x_val_table afe4403_res_table[] = {
> diff --git a/drivers/iio/health/afe4404.c b/drivers/iio/health/afe4404.c
> index 2edb7d7..7806a45 100644
> --- a/drivers/iio/health/afe4404.c
> +++ b/drivers/iio/health/afe4404.c
> @@ -122,24 +122,24 @@ struct afe4404_data {
>  };
>  
>  enum afe4404_chan_id {
> +	LED2 = 1,
> +	ALED2,
>  	LED1,
>  	ALED1,
> -	LED2,
> -	ALED2,
> -	LED1_ALED1,
>  	LED2_ALED2,
> +	LED1_ALED1,
>  	ILED1,
>  	ILED2,
>  	ILED3,
>  };
>  
>  static const struct afe440x_reg_info afe4404_reg_info[] = {
> -	[LED1] = AFE440X_REG_INFO(AFE440X_LED1VAL, AFE4404_OFFDAC, AFE4404_OFFDAC_CURR_LED1),
> -	[ALED1] = AFE440X_REG_INFO(AFE440X_ALED1VAL, AFE4404_OFFDAC, AFE4404_OFFDAC_CURR_ALED1),
>  	[LED2] = AFE440X_REG_INFO(AFE440X_LED2VAL, AFE4404_OFFDAC, AFE4404_OFFDAC_CURR_LED2),
>  	[ALED2] = AFE440X_REG_INFO(AFE440X_ALED2VAL, AFE4404_OFFDAC, AFE4404_OFFDAC_CURR_ALED2),
> -	[LED1_ALED1] = AFE440X_REG_INFO(AFE440X_LED1_ALED1VAL, 0, NULL),
> +	[LED1] = AFE440X_REG_INFO(AFE440X_LED1VAL, AFE4404_OFFDAC, AFE4404_OFFDAC_CURR_LED1),
> +	[ALED1] = AFE440X_REG_INFO(AFE440X_ALED1VAL, AFE4404_OFFDAC, AFE4404_OFFDAC_CURR_ALED1),
>  	[LED2_ALED2] = AFE440X_REG_INFO(AFE440X_LED2_ALED2VAL, 0, NULL),
> +	[LED1_ALED1] = AFE440X_REG_INFO(AFE440X_LED1_ALED1VAL, 0, NULL),
>  	[ILED1] = AFE440X_REG_INFO(AFE440X_LEDCNTRL, 0, AFE4404_LEDCNTRL_ILED1),
>  	[ILED2] = AFE440X_REG_INFO(AFE440X_LEDCNTRL, 0, AFE4404_LEDCNTRL_ILED2),
>  	[ILED3] = AFE440X_REG_INFO(AFE440X_LEDCNTRL, 0, AFE4404_LEDCNTRL_ILED3),
> @@ -147,16 +147,16 @@ static const struct afe440x_reg_info afe4404_reg_info[] = {
>  
>  static const struct iio_chan_spec afe4404_channels[] = {
>  	/* ADC values */
> -	AFE440X_INTENSITY_CHAN(LED1, "led1", BIT(IIO_CHAN_INFO_OFFSET)),
> -	AFE440X_INTENSITY_CHAN(ALED1, "led1_ambient", BIT(IIO_CHAN_INFO_OFFSET)),
> -	AFE440X_INTENSITY_CHAN(LED2, "led2", BIT(IIO_CHAN_INFO_OFFSET)),
> -	AFE440X_INTENSITY_CHAN(ALED2, "led2_ambient", BIT(IIO_CHAN_INFO_OFFSET)),
> -	AFE440X_INTENSITY_CHAN(LED1_ALED1, "led1-led1_ambient", 0),
> -	AFE440X_INTENSITY_CHAN(LED2_ALED2, "led2-led2_ambient", 0),
> +	AFE440X_INTENSITY_CHAN(LED2, BIT(IIO_CHAN_INFO_OFFSET)),
> +	AFE440X_INTENSITY_CHAN(ALED2, BIT(IIO_CHAN_INFO_OFFSET)),
> +	AFE440X_INTENSITY_CHAN(LED1, BIT(IIO_CHAN_INFO_OFFSET)),
> +	AFE440X_INTENSITY_CHAN(ALED1, BIT(IIO_CHAN_INFO_OFFSET)),
> +	AFE440X_INTENSITY_CHAN(LED2_ALED2, 0),
> +	AFE440X_INTENSITY_CHAN(LED1_ALED1, 0),
>  	/* LED current */
> -	AFE440X_CURRENT_CHAN(ILED1, "led1"),
> -	AFE440X_CURRENT_CHAN(ILED2, "led2"),
> -	AFE440X_CURRENT_CHAN(ILED3, "led3"),
> +	AFE440X_CURRENT_CHAN(ILED1),
> +	AFE440X_CURRENT_CHAN(ILED2),
> +	AFE440X_CURRENT_CHAN(ILED3),
>  };
>  
>  static const struct afe440x_val_table afe4404_res_table[] = {
> diff --git a/drivers/iio/health/afe440x.h b/drivers/iio/health/afe440x.h
> index 583d071..713972f 100644
> --- a/drivers/iio/health/afe440x.h
> +++ b/drivers/iio/health/afe440x.h
> @@ -103,7 +103,7 @@ struct afe440x_reg_info {
>  		.mask = _sm ## _MASK,				\
>  	}
>  
> -#define AFE440X_INTENSITY_CHAN(_index, _name, _mask)		\
> +#define AFE440X_INTENSITY_CHAN(_index, _mask)			\
>  	{							\
>  		.type = IIO_INTENSITY,				\
>  		.channel = _index,				\
> @@ -115,20 +115,20 @@ struct afe440x_reg_info {
>  				.storagebits = 32,		\
>  				.endianness = IIO_CPU,		\
>  		},						\
> -		.extend_name = _name,				\
>  		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |	\
>  			_mask,					\
> +		.indexed = true,				\
>  	}
>  
> -#define AFE440X_CURRENT_CHAN(_index, _name)			\
> +#define AFE440X_CURRENT_CHAN(_index)				\
>  	{							\
>  		.type = IIO_CURRENT,				\
>  		.channel = _index,				\
>  		.address = _index,				\
>  		.scan_index = -1,				\
> -		.extend_name = _name,				\
>  		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |	\
>  			BIT(IIO_CHAN_INFO_SCALE),		\
> +		.indexed = true,				\
>  		.output = true,					\
>  	}
>  
>

[toc] | [prev] | [next] | [standalone]

#1394468 — Re: [PATCH 08/13] iio: health/afe440x: Remove channel names

From"Andrew F. Davis" <afd@ti.com>
Date2016-05-04 17:30 +0200
SubjectRe: [PATCH 08/13] iio: health/afe440x: Remove channel names
Message-ID<rv6Ur-6wS-39@gated-at.bofh.it>
In reply to#1394387
On 05/04/2016 05:08 AM, Jonathan Cameron wrote:
> On 01/05/16 21:36, Andrew F. Davis wrote:
>> These AFEs have 4 ADC mesuring stages (called LED2, ALED2, LED1, and
>> ALED1 in the datasheet), we map these as channels, these stages can serve
>> different purposes depending on the application. For instance the AFE4404
>> has an additional LED (LED3), this LED can be timed to be active during
>> stage 2 (or anystage, but the datasheet describes this case and the name
>> of the stage reflects this use). This ability is used further in upcoming
>> parts that tie the front-end gain and the LED timings together. For these
>> reasons we remove explicit naming the channels.
>>
>> Without channel names it is best that the index numbers are in order to
>> match the stage number, reorder the channel numbers.
>>
>> Signed-off-by: Andrew F. Davis <afd@ti.com>
> Youch - this is rather major surgery.
> 
> Hmm. Given how non standard an ABI we ended up with in the first place
> again, let us cross our fingers that all the userspace for this is
> effectively under your control currently.
> 

Yeah, as far as I know we are the first to start building something on
top of this, and that is going to just be an Android HAL, so after that
any usage should still be abstracted a bit from the ABI, at least until
native users appear.

> Getting a little more nervous about this - but worth the risk I think
> for the simpler result.
> 

Using the datasheet names in the sysfs entries was probably wrong from
the start, if I remember correctly you even suggested against it
originally (probably should have listened :)).

in_intensity_ledY_raw -> in_intensityY_raw

works much better, especially with all the different names/uses they
give each stage in the upcoming AFE4405.

> Applied
> 

Thanks,
Andrew

> Jonathan
>> ---
>>  .../ABI/testing/sysfs-bus-iio-health-afe440x       | 39 ++++++++++------------
>>  drivers/iio/health/afe4403.c                       | 28 ++++++++--------
>>  drivers/iio/health/afe4404.c                       | 30 ++++++++---------
>>  drivers/iio/health/afe440x.h                       |  8 ++---
>>  4 files changed, 51 insertions(+), 54 deletions(-)
>>
>> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-health-afe440x b/Documentation/ABI/testing/sysfs-bus-iio-health-afe440x
>> index b19053a..a067073 100644
>> --- a/Documentation/ABI/testing/sysfs-bus-iio-health-afe440x
>> +++ b/Documentation/ABI/testing/sysfs-bus-iio-health-afe440x
>> @@ -8,38 +8,35 @@ Description:
>>  		Transimpedance Amplifier. Y is 1 for Rf1 and Cf1, Y is 2 for
>>  		Rf2 and Cf2 values.
>>  
>> -What:		/sys/bus/iio/devices/iio:deviceX/in_intensity_ledY_raw
>> -		/sys/bus/iio/devices/iio:deviceX/in_intensity_ledY_ambient_raw
>> -Date:		December 2015
>> +What:		/sys/bus/iio/devices/iio:deviceX/in_intensityY_raw
>> +Date:		May 2016
>>  KernelVersion:
>>  Contact:	Andrew F. Davis <afd@ti.com>
>>  Description:
>>  		Get measured values from the ADC for these stages. Y is the
>> -		specific LED number. The values are expressed in 24-bit twos
>> -		complement.
>> -
>> -What:		/sys/bus/iio/devices/iio:deviceX/in_intensity_ledY-ledY_ambient_raw
>> -Date:		December 2015
>> -KernelVersion:
>> -Contact:	Andrew F. Davis <afd@ti.com>
>> -Description:
>> -		Get differential values from the ADC for these stages. Y is the
>> -		specific LED number. The values are expressed in 24-bit twos
>> -		complement for the specified LEDs.
>> +		specific stage number corresponding to datasheet stage names
>> +		as follows:
>> +		1 -> LED2
>> +		2 -> ALED2/LED3
>> +		3 -> LED1
>> +		4 -> ALED1/LED4
>> +		Note that channels 5 and 6 represent LED2-ALED2 and LED1-ALED1
>> +		respectively which simply helper channels containing the
>> +		calculated difference in the value of stage 1 - 2 and 3 - 4.
>> +		The values are expressed in 24-bit twos complement.
>>  
>> -What:		/sys/bus/iio/devices/iio:deviceX/out_current_ledY_offset
>> -		/sys/bus/iio/devices/iio:deviceX/out_current_ledY_ambient_offset
>> -Date:		December 2015
>> +What:		/sys/bus/iio/devices/iio:deviceX/in_intensityY_offset
>> +Date:		May 2016
>>  KernelVersion:
>>  Contact:	Andrew F. Davis <afd@ti.com>
>>  Description:
>>  		Get and set the offset cancellation DAC setting for these
>>  		stages. The values are expressed in 5-bit sign-magnitude.
>>  
>> -What:		/sys/bus/iio/devices/iio:deviceX/out_current_ledY_raw
>> -Date:		December 2015
>> +What:		/sys/bus/iio/devices/iio:deviceX/out_currentY_raw
>> +Date:		May 2016
>>  KernelVersion:
>>  Contact:	Andrew F. Davis <afd@ti.com>
>>  Description:
>> -		Get and set the LED current for the specified LED. Y is the
>> -		specific LED number.
>> +		Get and set the LED current for the specified LED active during
>> +		this stage. Y is the specific stage number.
>> diff --git a/drivers/iio/health/afe4403.c b/drivers/iio/health/afe4403.c
>> index cac6090..4a58064 100644
>> --- a/drivers/iio/health/afe4403.c
>> +++ b/drivers/iio/health/afe4403.c
>> @@ -121,38 +121,38 @@ struct afe4403_data {
>>  };
>>  
>>  enum afe4403_chan_id {
>> +	LED2 = 1,
>> +	ALED2,
>>  	LED1,
>>  	ALED1,
>> -	LED2,
>> -	ALED2,
>> -	LED1_ALED1,
>>  	LED2_ALED2,
>> +	LED1_ALED1,
>>  	ILED1,
>>  	ILED2,
>>  };
>>  
>>  static const struct afe440x_reg_info afe4403_reg_info[] = {
>> -	[LED1] = AFE440X_REG_INFO(AFE440X_LED1VAL, 0, NULL),
>> -	[ALED1] = AFE440X_REG_INFO(AFE440X_ALED1VAL, 0, NULL),
>>  	[LED2] = AFE440X_REG_INFO(AFE440X_LED2VAL, 0, NULL),
>>  	[ALED2] = AFE440X_REG_INFO(AFE440X_ALED2VAL, 0, NULL),
>> -	[LED1_ALED1] = AFE440X_REG_INFO(AFE440X_LED1_ALED1VAL, 0, NULL),
>> +	[LED1] = AFE440X_REG_INFO(AFE440X_LED1VAL, 0, NULL),
>> +	[ALED1] = AFE440X_REG_INFO(AFE440X_ALED1VAL, 0, NULL),
>>  	[LED2_ALED2] = AFE440X_REG_INFO(AFE440X_LED2_ALED2VAL, 0, NULL),
>> +	[LED1_ALED1] = AFE440X_REG_INFO(AFE440X_LED1_ALED1VAL, 0, NULL),
>>  	[ILED1] = AFE440X_REG_INFO(AFE440X_LEDCNTRL, 0, AFE440X_LEDCNTRL_LED1),
>>  	[ILED2] = AFE440X_REG_INFO(AFE440X_LEDCNTRL, 0, AFE440X_LEDCNTRL_LED2),
>>  };
>>  
>>  static const struct iio_chan_spec afe4403_channels[] = {
>>  	/* ADC values */
>> -	AFE440X_INTENSITY_CHAN(LED1, "led1", 0),
>> -	AFE440X_INTENSITY_CHAN(ALED1, "led1_ambient", 0),
>> -	AFE440X_INTENSITY_CHAN(LED2, "led2", 0),
>> -	AFE440X_INTENSITY_CHAN(ALED2, "led2_ambient", 0),
>> -	AFE440X_INTENSITY_CHAN(LED1_ALED1, "led1-led1_ambient", 0),
>> -	AFE440X_INTENSITY_CHAN(LED2_ALED2, "led2-led2_ambient", 0),
>> +	AFE440X_INTENSITY_CHAN(LED2, 0),
>> +	AFE440X_INTENSITY_CHAN(ALED2, 0),
>> +	AFE440X_INTENSITY_CHAN(LED1, 0),
>> +	AFE440X_INTENSITY_CHAN(ALED1, 0),
>> +	AFE440X_INTENSITY_CHAN(LED2_ALED2, 0),
>> +	AFE440X_INTENSITY_CHAN(LED1_ALED1, 0),
>>  	/* LED current */
>> -	AFE440X_CURRENT_CHAN(ILED1, "led1"),
>> -	AFE440X_CURRENT_CHAN(ILED2, "led2"),
>> +	AFE440X_CURRENT_CHAN(ILED1),
>> +	AFE440X_CURRENT_CHAN(ILED2),
>>  };
>>  
>>  static const struct afe440x_val_table afe4403_res_table[] = {
>> diff --git a/drivers/iio/health/afe4404.c b/drivers/iio/health/afe4404.c
>> index 2edb7d7..7806a45 100644
>> --- a/drivers/iio/health/afe4404.c
>> +++ b/drivers/iio/health/afe4404.c
>> @@ -122,24 +122,24 @@ struct afe4404_data {
>>  };
>>  
>>  enum afe4404_chan_id {
>> +	LED2 = 1,
>> +	ALED2,
>>  	LED1,
>>  	ALED1,
>> -	LED2,
>> -	ALED2,
>> -	LED1_ALED1,
>>  	LED2_ALED2,
>> +	LED1_ALED1,
>>  	ILED1,
>>  	ILED2,
>>  	ILED3,
>>  };
>>  
>>  static const struct afe440x_reg_info afe4404_reg_info[] = {
>> -	[LED1] = AFE440X_REG_INFO(AFE440X_LED1VAL, AFE4404_OFFDAC, AFE4404_OFFDAC_CURR_LED1),
>> -	[ALED1] = AFE440X_REG_INFO(AFE440X_ALED1VAL, AFE4404_OFFDAC, AFE4404_OFFDAC_CURR_ALED1),
>>  	[LED2] = AFE440X_REG_INFO(AFE440X_LED2VAL, AFE4404_OFFDAC, AFE4404_OFFDAC_CURR_LED2),
>>  	[ALED2] = AFE440X_REG_INFO(AFE440X_ALED2VAL, AFE4404_OFFDAC, AFE4404_OFFDAC_CURR_ALED2),
>> -	[LED1_ALED1] = AFE440X_REG_INFO(AFE440X_LED1_ALED1VAL, 0, NULL),
>> +	[LED1] = AFE440X_REG_INFO(AFE440X_LED1VAL, AFE4404_OFFDAC, AFE4404_OFFDAC_CURR_LED1),
>> +	[ALED1] = AFE440X_REG_INFO(AFE440X_ALED1VAL, AFE4404_OFFDAC, AFE4404_OFFDAC_CURR_ALED1),
>>  	[LED2_ALED2] = AFE440X_REG_INFO(AFE440X_LED2_ALED2VAL, 0, NULL),
>> +	[LED1_ALED1] = AFE440X_REG_INFO(AFE440X_LED1_ALED1VAL, 0, NULL),
>>  	[ILED1] = AFE440X_REG_INFO(AFE440X_LEDCNTRL, 0, AFE4404_LEDCNTRL_ILED1),
>>  	[ILED2] = AFE440X_REG_INFO(AFE440X_LEDCNTRL, 0, AFE4404_LEDCNTRL_ILED2),
>>  	[ILED3] = AFE440X_REG_INFO(AFE440X_LEDCNTRL, 0, AFE4404_LEDCNTRL_ILED3),
>> @@ -147,16 +147,16 @@ static const struct afe440x_reg_info afe4404_reg_info[] = {
>>  
>>  static const struct iio_chan_spec afe4404_channels[] = {
>>  	/* ADC values */
>> -	AFE440X_INTENSITY_CHAN(LED1, "led1", BIT(IIO_CHAN_INFO_OFFSET)),
>> -	AFE440X_INTENSITY_CHAN(ALED1, "led1_ambient", BIT(IIO_CHAN_INFO_OFFSET)),
>> -	AFE440X_INTENSITY_CHAN(LED2, "led2", BIT(IIO_CHAN_INFO_OFFSET)),
>> -	AFE440X_INTENSITY_CHAN(ALED2, "led2_ambient", BIT(IIO_CHAN_INFO_OFFSET)),
>> -	AFE440X_INTENSITY_CHAN(LED1_ALED1, "led1-led1_ambient", 0),
>> -	AFE440X_INTENSITY_CHAN(LED2_ALED2, "led2-led2_ambient", 0),
>> +	AFE440X_INTENSITY_CHAN(LED2, BIT(IIO_CHAN_INFO_OFFSET)),
>> +	AFE440X_INTENSITY_CHAN(ALED2, BIT(IIO_CHAN_INFO_OFFSET)),
>> +	AFE440X_INTENSITY_CHAN(LED1, BIT(IIO_CHAN_INFO_OFFSET)),
>> +	AFE440X_INTENSITY_CHAN(ALED1, BIT(IIO_CHAN_INFO_OFFSET)),
>> +	AFE440X_INTENSITY_CHAN(LED2_ALED2, 0),
>> +	AFE440X_INTENSITY_CHAN(LED1_ALED1, 0),
>>  	/* LED current */
>> -	AFE440X_CURRENT_CHAN(ILED1, "led1"),
>> -	AFE440X_CURRENT_CHAN(ILED2, "led2"),
>> -	AFE440X_CURRENT_CHAN(ILED3, "led3"),
>> +	AFE440X_CURRENT_CHAN(ILED1),
>> +	AFE440X_CURRENT_CHAN(ILED2),
>> +	AFE440X_CURRENT_CHAN(ILED3),
>>  };
>>  
>>  static const struct afe440x_val_table afe4404_res_table[] = {
>> diff --git a/drivers/iio/health/afe440x.h b/drivers/iio/health/afe440x.h
>> index 583d071..713972f 100644
>> --- a/drivers/iio/health/afe440x.h
>> +++ b/drivers/iio/health/afe440x.h
>> @@ -103,7 +103,7 @@ struct afe440x_reg_info {
>>  		.mask = _sm ## _MASK,				\
>>  	}
>>  
>> -#define AFE440X_INTENSITY_CHAN(_index, _name, _mask)		\
>> +#define AFE440X_INTENSITY_CHAN(_index, _mask)			\
>>  	{							\
>>  		.type = IIO_INTENSITY,				\
>>  		.channel = _index,				\
>> @@ -115,20 +115,20 @@ struct afe440x_reg_info {
>>  				.storagebits = 32,		\
>>  				.endianness = IIO_CPU,		\
>>  		},						\
>> -		.extend_name = _name,				\
>>  		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |	\
>>  			_mask,					\
>> +		.indexed = true,				\
>>  	}
>>  
>> -#define AFE440X_CURRENT_CHAN(_index, _name)			\
>> +#define AFE440X_CURRENT_CHAN(_index)				\
>>  	{							\
>>  		.type = IIO_CURRENT,				\
>>  		.channel = _index,				\
>>  		.address = _index,				\
>>  		.scan_index = -1,				\
>> -		.extend_name = _name,				\
>>  		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |	\
>>  			BIT(IIO_CHAN_INFO_SCALE),		\
>> +		.indexed = true,				\
>>  		.output = true,					\
>>  	}
>>  
>>
>

[toc] | [prev] | [next] | [standalone]

#1394396

FromJonathan Cameron <jic23@kernel.org>
Date2016-05-04 16:50 +0200
Message-ID<rv6hJ-5S0-39@gated-at.bofh.it>
In reply to#1391868
On 01/05/16 21:36, Andrew F. Davis wrote:
> Hello all,
> 
> I will be posting a driver for the AFE4405 soon and in preparation
> for this I have made some changes to the existing drivers to better
> align them with the new part. Some of these changes are trivial, others
> change the sysfs entries, I understand these are considered ABIs and
> changes to even testing ABIs should not be made lightly, but my hope is
> that these changes will more accurately reflect the devices intended use
> and improve functionality as the first users begin to appear.
> 
> Thanks,
> Andrew

Hi Andrew,

Just a quick not to explain my reasoning for taking these which involve
some pretty major ABI surgery, whilst bouncing back much more trivial
changes.

Ultimately, whether we can change ABI comes down to 1 simple question.
'Will anyone notice and if so will it be an issue for them?'
We must not break userspace.

For the vast majority of IIO drivers, only simple tools along the lines
of generic_buffer or just sysfs file reads are all that is needed to
make 'use' of the data.  Obviously lots of users will go through one
of the more advanced libraries, but these are still mostly generic.

We had a long debate through numerous versions when Andrew was
originally submitting these two drivers to try and pin down a vaguely
standard ABI for them.  The result was indeed 'Vaguely' standard.
We moved in a good direction but sometimes a device is just too
specialized (unfortunately).

The very nature of these devices is that the data is only meaningful
(i.e. measuring what they are supposedly for) once a non trivial
amount of computation has been performed in userspace. Now, there
probably is enough information available to do pulse oximetry using
one of these devices, but it is certainly non trivial.

As such the interface is hopefully tightly coupled to the userspace
tools that TI provide allowing us to make changes without it
adversely effecting anyone.  As time moves on this may well not be
the case.  If anyone knows that there is existing non TI code out
there for this and we will be causing pain, let me know ASAP so that
we can work out a way forward.

So really it's a case of an educated guess that no one cares and
crossing fingers!  I think the improvements made here are worth
the risk.

Jonathan
> 
> Andrew F. Davis (13):
>   iio: health/afe440x: Fix kernel-doc format
>   iio: health/afe440x: Remove of_match_ptr and ifdefs
>   iio: health/afe440x: Remove unneeded initializers
>   iio: health/afe440x: Always use separate gain values
>   iio: health/afe440x: Fix scan_index assignment
>   iio: health/afe440x: Remove unneeded offset handling
>   iio: health/afe4404: Remove LED3 input channel
>   iio: health/afe440x: Remove channel names
>   iio: health/afe440x: Use regmap fields
>   iio: health/afe440x: Make gain settings a modifier for the stages
>   iio: health/afe440x: Match LED currents to stages
>   iio: health/afe440x: Remove unused definitions
>   iio: health/afe4404: ENSEPGAIN is part of CONTROL2 register
> 
>  .../ABI/testing/sysfs-bus-iio-health-afe440x       |  95 +++----
>  drivers/iio/health/afe4403.c                       | 299 ++++++++------------
>  drivers/iio/health/afe4404.c                       | 308 +++++++++------------
>  drivers/iio/health/afe440x.h                       |  48 +---
>  4 files changed, 295 insertions(+), 455 deletions(-)
>  rewrite Documentation/ABI/testing/sysfs-bus-iio-health-afe440x (72%)
>

[toc] | [prev] | [standalone]

Page 2 of 2 — ← Prev page 1 [2]

Back to top | Article view | linux.kernel

csiph-web