* [PATCH 00/13] Rework for AFE440x drivers to prepare for AFE4405
@ 2016-05-01 20:36 Andrew F. Davis
2016-05-01 20:36 ` [PATCH 01/13] iio: health/afe440x: Fix kernel-doc format Andrew F. Davis
` (7 more replies)
0 siblings, 8 replies; 32+ messages in thread
From: Andrew F. Davis @ 2016-05-01 20:36 UTC (permalink / raw)
To: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald
Cc: linux-iio, linux-api, linux-kernel, Andrew F. Davis
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
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%)
--
2.8.1
^ permalink raw reply [flat|nested] 32+ messages in thread* [PATCH 01/13] iio: health/afe440x: Fix kernel-doc format 2016-05-01 20:36 [PATCH 00/13] Rework for AFE440x drivers to prepare for AFE4405 Andrew F. Davis @ 2016-05-01 20:36 ` Andrew F. Davis [not found] ` <1462135023-14445-2-git-send-email-afd-l0cyMroinI0@public.gmane.org> 2016-05-01 20:36 ` [PATCH 02/13] iio: health/afe440x: Remove of_match_ptr and ifdefs Andrew F. Davis ` (6 subsequent siblings) 7 siblings, 1 reply; 32+ messages in thread From: Andrew F. Davis @ 2016-05-01 20:36 UTC (permalink / raw) To: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald Cc: linux-iio, linux-api, linux-kernel, Andrew F. Davis Fix kernel-doc formatting for structs, and while we are making little fixes, clarify the module description and update the copywrite. Signed-off-by: Andrew F. Davis <afd@ti.com> --- drivers/iio/health/afe4403.c | 18 +++++++++--------- drivers/iio/health/afe4404.c | 16 ++++++++-------- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/drivers/iio/health/afe4403.c b/drivers/iio/health/afe4403.c index 88e43f8..2094113 100644 --- a/drivers/iio/health/afe4403.c +++ b/drivers/iio/health/afe4403.c @@ -1,7 +1,7 @@ /* * AFE4403 Heart Rate Monitors and Low-Cost Pulse Oximeters * - * Copyright (C) 2015 Texas Instruments Incorporated - http://www.ti.com/ + * Copyright (C) 2015-2016 Texas Instruments Incorporated - http://www.ti.com/ * Andrew F. Davis <afd@ti.com> * * This program is free software; you can redistribute it and/or modify @@ -103,13 +103,13 @@ #define AFE4403_TIAGAIN_RES_NONE 0x7 /** - * struct afe4403_data - * @dev - Device structure - * @spi - SPI device handle - * @regmap - Register map of the device - * @regulator - Pointer to the regulator for the IC - * @trig - IIO trigger for this device - * @irq - ADC_RDY line interrupt number + * struct afe4403_data - AFE4403 device instance data + * @dev: Device structure + * @spi: SPI device handle + * @regmap: Register map of the device + * @regulator: Pointer to the regulator for the IC + * @trig: IIO trigger for this device + * @irq: ADC_RDY line interrupt number */ struct afe4403_data { struct device *dev; @@ -704,5 +704,5 @@ static struct spi_driver afe4403_spi_driver = { module_spi_driver(afe4403_spi_driver); MODULE_AUTHOR("Andrew F. Davis <afd@ti.com>"); -MODULE_DESCRIPTION("TI AFE4403 Heart Rate and Pulse Oximeter"); +MODULE_DESCRIPTION("TI AFE4403 Heart Rate Monitor and Pulse Oximeter AFE"); MODULE_LICENSE("GPL v2"); diff --git a/drivers/iio/health/afe4404.c b/drivers/iio/health/afe4404.c index 5096a46..7127d03 100644 --- a/drivers/iio/health/afe4404.c +++ b/drivers/iio/health/afe4404.c @@ -1,7 +1,7 @@ /* * AFE4404 Heart Rate Monitors and Low-Cost Pulse Oximeters * - * Copyright (C) 2015 Texas Instruments Incorporated - http://www.ti.com/ + * Copyright (C) 2015-2016 Texas Instruments Incorporated - http://www.ti.com/ * Andrew F. Davis <afd@ti.com> * * This program is free software; you can redistribute it and/or modify @@ -106,12 +106,12 @@ #define AFE4404_TIA_GAIN_RES_2_M 0x7 /** - * struct afe4404_data - * @dev - Device structure - * @regmap - Register map of the device - * @regulator - Pointer to the regulator for the IC - * @trig - IIO trigger for this device - * @irq - ADC_RDY line interrupt number + * struct afe4404_data - AFE4404 device instance data + * @dev: Device structure + * @regmap: Register map of the device + * @regulator: Pointer to the regulator for the IC + * @trig: IIO trigger for this device + * @irq: ADC_RDY line interrupt number */ struct afe4404_data { struct device *dev; @@ -675,5 +675,5 @@ static struct i2c_driver afe4404_i2c_driver = { module_i2c_driver(afe4404_i2c_driver); MODULE_AUTHOR("Andrew F. Davis <afd@ti.com>"); -MODULE_DESCRIPTION("TI AFE4404 Heart Rate and Pulse Oximeter"); +MODULE_DESCRIPTION("TI AFE4404 Heart Rate Monitor and Pulse Oximeter AFE"); MODULE_LICENSE("GPL v2"); -- 2.8.1 ^ permalink raw reply related [flat|nested] 32+ messages in thread
[parent not found: <1462135023-14445-2-git-send-email-afd-l0cyMroinI0@public.gmane.org>]
* Re: [PATCH 01/13] iio: health/afe440x: Fix kernel-doc format [not found] ` <1462135023-14445-2-git-send-email-afd-l0cyMroinI0@public.gmane.org> @ 2016-05-04 9:58 ` Jonathan Cameron 0 siblings, 0 replies; 32+ messages in thread From: Jonathan Cameron @ 2016-05-04 9:58 UTC (permalink / raw) To: Andrew F. Davis, Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA, linux-api-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA On 01/05/16 21:36, Andrew F. Davis wrote: > Fix kernel-doc formatting for structs, and while we are making little > fixes, clarify the module description and update the copywrite. > > Signed-off-by: Andrew F. Davis <afd-l0cyMroinI0@public.gmane.org> Applied to the togreg branch of iio.git - to be pushed out in a day or two as testing. Thanks, Jonathan > --- > drivers/iio/health/afe4403.c | 18 +++++++++--------- > drivers/iio/health/afe4404.c | 16 ++++++++-------- > 2 files changed, 17 insertions(+), 17 deletions(-) > > diff --git a/drivers/iio/health/afe4403.c b/drivers/iio/health/afe4403.c > index 88e43f8..2094113 100644 > --- a/drivers/iio/health/afe4403.c > +++ b/drivers/iio/health/afe4403.c > @@ -1,7 +1,7 @@ > /* > * AFE4403 Heart Rate Monitors and Low-Cost Pulse Oximeters > * > - * Copyright (C) 2015 Texas Instruments Incorporated - http://www.ti.com/ > + * Copyright (C) 2015-2016 Texas Instruments Incorporated - http://www.ti.com/ > * Andrew F. Davis <afd-l0cyMroinI0@public.gmane.org> > * > * This program is free software; you can redistribute it and/or modify > @@ -103,13 +103,13 @@ > #define AFE4403_TIAGAIN_RES_NONE 0x7 > > /** > - * struct afe4403_data > - * @dev - Device structure > - * @spi - SPI device handle > - * @regmap - Register map of the device > - * @regulator - Pointer to the regulator for the IC > - * @trig - IIO trigger for this device > - * @irq - ADC_RDY line interrupt number > + * struct afe4403_data - AFE4403 device instance data > + * @dev: Device structure > + * @spi: SPI device handle > + * @regmap: Register map of the device > + * @regulator: Pointer to the regulator for the IC > + * @trig: IIO trigger for this device > + * @irq: ADC_RDY line interrupt number > */ > struct afe4403_data { > struct device *dev; > @@ -704,5 +704,5 @@ static struct spi_driver afe4403_spi_driver = { > module_spi_driver(afe4403_spi_driver); > > MODULE_AUTHOR("Andrew F. Davis <afd-l0cyMroinI0@public.gmane.org>"); > -MODULE_DESCRIPTION("TI AFE4403 Heart Rate and Pulse Oximeter"); > +MODULE_DESCRIPTION("TI AFE4403 Heart Rate Monitor and Pulse Oximeter AFE"); > MODULE_LICENSE("GPL v2"); > diff --git a/drivers/iio/health/afe4404.c b/drivers/iio/health/afe4404.c > index 5096a46..7127d03 100644 > --- a/drivers/iio/health/afe4404.c > +++ b/drivers/iio/health/afe4404.c > @@ -1,7 +1,7 @@ > /* > * AFE4404 Heart Rate Monitors and Low-Cost Pulse Oximeters > * > - * Copyright (C) 2015 Texas Instruments Incorporated - http://www.ti.com/ > + * Copyright (C) 2015-2016 Texas Instruments Incorporated - http://www.ti.com/ > * Andrew F. Davis <afd-l0cyMroinI0@public.gmane.org> > * > * This program is free software; you can redistribute it and/or modify > @@ -106,12 +106,12 @@ > #define AFE4404_TIA_GAIN_RES_2_M 0x7 > > /** > - * struct afe4404_data > - * @dev - Device structure > - * @regmap - Register map of the device > - * @regulator - Pointer to the regulator for the IC > - * @trig - IIO trigger for this device > - * @irq - ADC_RDY line interrupt number > + * struct afe4404_data - AFE4404 device instance data > + * @dev: Device structure > + * @regmap: Register map of the device > + * @regulator: Pointer to the regulator for the IC > + * @trig: IIO trigger for this device > + * @irq: ADC_RDY line interrupt number > */ > struct afe4404_data { > struct device *dev; > @@ -675,5 +675,5 @@ static struct i2c_driver afe4404_i2c_driver = { > module_i2c_driver(afe4404_i2c_driver); > > MODULE_AUTHOR("Andrew F. Davis <afd-l0cyMroinI0@public.gmane.org>"); > -MODULE_DESCRIPTION("TI AFE4404 Heart Rate and Pulse Oximeter"); > +MODULE_DESCRIPTION("TI AFE4404 Heart Rate Monitor and Pulse Oximeter AFE"); > MODULE_LICENSE("GPL v2"); > ^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 02/13] iio: health/afe440x: Remove of_match_ptr and ifdefs 2016-05-01 20:36 [PATCH 00/13] Rework for AFE440x drivers to prepare for AFE4405 Andrew F. Davis 2016-05-01 20:36 ` [PATCH 01/13] iio: health/afe440x: Fix kernel-doc format Andrew F. Davis @ 2016-05-01 20:36 ` Andrew F. Davis 2016-05-04 9:59 ` Jonathan Cameron 2016-05-01 20:36 ` [PATCH 04/13] iio: health/afe440x: Always use separate gain values Andrew F. Davis ` (5 subsequent siblings) 7 siblings, 1 reply; 32+ messages in thread From: Andrew F. Davis @ 2016-05-01 20:36 UTC (permalink / raw) To: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald Cc: linux-iio, linux-api, linux-kernel, Andrew F. Davis The drivers DT tables are not built-in when OF is not enabled, this does not save us enough to justify ugly ifdefs. Clean this up. Signed-off-by: Andrew F. Davis <afd@ti.com> --- drivers/iio/health/afe4403.c | 4 +--- drivers/iio/health/afe4404.c | 4 +--- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/drivers/iio/health/afe4403.c b/drivers/iio/health/afe4403.c index 2094113..e3700cf 100644 --- a/drivers/iio/health/afe4403.c +++ b/drivers/iio/health/afe4403.c @@ -498,13 +498,11 @@ static const struct regmap_config afe4403_regmap_config = { .volatile_table = &afe4403_volatile_table, }; -#ifdef CONFIG_OF static const struct of_device_id afe4403_of_match[] = { { .compatible = "ti,afe4403", }, { /* sentinel */ } }; MODULE_DEVICE_TABLE(of, afe4403_of_match); -#endif static int __maybe_unused afe4403_suspend(struct device *dev) { @@ -694,7 +692,7 @@ MODULE_DEVICE_TABLE(spi, afe4403_ids); static struct spi_driver afe4403_spi_driver = { .driver = { .name = AFE4403_DRIVER_NAME, - .of_match_table = of_match_ptr(afe4403_of_match), + .of_match_table = afe4403_of_match, .pm = &afe4403_pm_ops, }, .probe = afe4403_probe, diff --git a/drivers/iio/health/afe4404.c b/drivers/iio/health/afe4404.c index 7127d03..783b9b4 100644 --- a/drivers/iio/health/afe4404.c +++ b/drivers/iio/health/afe4404.c @@ -469,13 +469,11 @@ static const struct regmap_config afe4404_regmap_config = { .volatile_table = &afe4404_volatile_table, }; -#ifdef CONFIG_OF static const struct of_device_id afe4404_of_match[] = { { .compatible = "ti,afe4404", }, { /* sentinel */ } }; MODULE_DEVICE_TABLE(of, afe4404_of_match); -#endif static int __maybe_unused afe4404_suspend(struct device *dev) { @@ -665,7 +663,7 @@ MODULE_DEVICE_TABLE(i2c, afe4404_ids); static struct i2c_driver afe4404_i2c_driver = { .driver = { .name = AFE4404_DRIVER_NAME, - .of_match_table = of_match_ptr(afe4404_of_match), + .of_match_table = afe4404_of_match, .pm = &afe4404_pm_ops, }, .probe = afe4404_probe, -- 2.8.1 ^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: [PATCH 02/13] iio: health/afe440x: Remove of_match_ptr and ifdefs 2016-05-01 20:36 ` [PATCH 02/13] iio: health/afe440x: Remove of_match_ptr and ifdefs Andrew F. Davis @ 2016-05-04 9:59 ` Jonathan Cameron 0 siblings, 0 replies; 32+ messages in thread From: Jonathan Cameron @ 2016-05-04 9:59 UTC (permalink / raw) To: Andrew F. Davis, Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald Cc: linux-iio, linux-api, linux-kernel On 01/05/16 21:36, Andrew F. Davis wrote: > The drivers DT tables are not built-in when OF is not enabled, this does > not save us enough to justify ugly ifdefs. Clean this up. > > Signed-off-by: Andrew F. Davis <afd@ti.com> Can't argue with that. Applied. Jonathan > --- > drivers/iio/health/afe4403.c | 4 +--- > drivers/iio/health/afe4404.c | 4 +--- > 2 files changed, 2 insertions(+), 6 deletions(-) > > diff --git a/drivers/iio/health/afe4403.c b/drivers/iio/health/afe4403.c > index 2094113..e3700cf 100644 > --- a/drivers/iio/health/afe4403.c > +++ b/drivers/iio/health/afe4403.c > @@ -498,13 +498,11 @@ static const struct regmap_config afe4403_regmap_config = { > .volatile_table = &afe4403_volatile_table, > }; > > -#ifdef CONFIG_OF > static const struct of_device_id afe4403_of_match[] = { > { .compatible = "ti,afe4403", }, > { /* sentinel */ } > }; > MODULE_DEVICE_TABLE(of, afe4403_of_match); > -#endif > > static int __maybe_unused afe4403_suspend(struct device *dev) > { > @@ -694,7 +692,7 @@ MODULE_DEVICE_TABLE(spi, afe4403_ids); > static struct spi_driver afe4403_spi_driver = { > .driver = { > .name = AFE4403_DRIVER_NAME, > - .of_match_table = of_match_ptr(afe4403_of_match), > + .of_match_table = afe4403_of_match, > .pm = &afe4403_pm_ops, > }, > .probe = afe4403_probe, > diff --git a/drivers/iio/health/afe4404.c b/drivers/iio/health/afe4404.c > index 7127d03..783b9b4 100644 > --- a/drivers/iio/health/afe4404.c > +++ b/drivers/iio/health/afe4404.c > @@ -469,13 +469,11 @@ static const struct regmap_config afe4404_regmap_config = { > .volatile_table = &afe4404_volatile_table, > }; > > -#ifdef CONFIG_OF > static const struct of_device_id afe4404_of_match[] = { > { .compatible = "ti,afe4404", }, > { /* sentinel */ } > }; > MODULE_DEVICE_TABLE(of, afe4404_of_match); > -#endif > > static int __maybe_unused afe4404_suspend(struct device *dev) > { > @@ -665,7 +663,7 @@ MODULE_DEVICE_TABLE(i2c, afe4404_ids); > static struct i2c_driver afe4404_i2c_driver = { > .driver = { > .name = AFE4404_DRIVER_NAME, > - .of_match_table = of_match_ptr(afe4404_of_match), > + .of_match_table = afe4404_of_match, > .pm = &afe4404_pm_ops, > }, > .probe = afe4404_probe, > ^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 04/13] iio: health/afe440x: Always use separate gain values 2016-05-01 20:36 [PATCH 00/13] Rework for AFE440x drivers to prepare for AFE4405 Andrew F. Davis 2016-05-01 20:36 ` [PATCH 01/13] iio: health/afe440x: Fix kernel-doc format Andrew F. Davis 2016-05-01 20:36 ` [PATCH 02/13] iio: health/afe440x: Remove of_match_ptr and ifdefs Andrew F. Davis @ 2016-05-01 20:36 ` Andrew F. Davis 2016-05-04 10:02 ` Jonathan Cameron [not found] ` <1462135023-14445-1-git-send-email-afd-l0cyMroinI0@public.gmane.org> ` (4 subsequent siblings) 7 siblings, 1 reply; 32+ messages in thread From: Andrew F. Davis @ 2016-05-01 20:36 UTC (permalink / raw) To: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald Cc: linux-iio, linux-api, linux-kernel, Andrew F. Davis Locking the two gain stages to the same setting adds no value for us, so initialize them as unlocked and remove the sysfs for unlocking them. This also allows us to greatly simplify showing and setting the gain registers. Signed-off-by: Andrew F. Davis <afd@ti.com> --- .../ABI/testing/sysfs-bus-iio-health-afe440x | 9 ---- drivers/iio/health/afe4403.c | 60 ++++++---------------- drivers/iio/health/afe4404.c | 60 ++++++---------------- drivers/iio/health/afe440x.h | 15 ++---- 4 files changed, 37 insertions(+), 107 deletions(-) diff --git a/Documentation/ABI/testing/sysfs-bus-iio-health-afe440x b/Documentation/ABI/testing/sysfs-bus-iio-health-afe440x index 3740f25..b19053a 100644 --- a/Documentation/ABI/testing/sysfs-bus-iio-health-afe440x +++ b/Documentation/ABI/testing/sysfs-bus-iio-health-afe440x @@ -8,15 +8,6 @@ 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/tia_separate_en -Date: December 2015 -KernelVersion: -Contact: Andrew F. Davis <afd@ti.com> -Description: - Enable or disable separate settings for the TransImpedance - Amplifier above, when disabled both values are set by the - first channel. - 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 diff --git a/drivers/iio/health/afe4403.c b/drivers/iio/health/afe4403.c index 5484785..bcff528 100644 --- a/drivers/iio/health/afe4403.c +++ b/drivers/iio/health/afe4403.c @@ -180,9 +180,9 @@ static ssize_t afe440x_show_register(struct device *dev, struct iio_dev *indio_dev = dev_to_iio_dev(dev); struct afe4403_data *afe = iio_priv(indio_dev); struct afe440x_attr *afe440x_attr = to_afe440x_attr(attr); - unsigned int reg_val, type; + unsigned int reg_val; int vals[2]; - int ret, val_len; + int ret; ret = regmap_read(afe->regmap, afe440x_attr->reg, ®_val); if (ret) @@ -191,27 +191,13 @@ static ssize_t afe440x_show_register(struct device *dev, reg_val &= afe440x_attr->mask; reg_val >>= afe440x_attr->shift; - switch (afe440x_attr->type) { - case SIMPLE: - type = IIO_VAL_INT; - val_len = 1; - vals[0] = reg_val; - break; - case RESISTANCE: - case CAPACITANCE: - type = IIO_VAL_INT_PLUS_MICRO; - val_len = 2; - if (reg_val < afe440x_attr->table_size) { - vals[0] = afe440x_attr->val_table[reg_val].integer; - vals[1] = afe440x_attr->val_table[reg_val].fract; - break; - } + if (reg_val >= afe440x_attr->table_size) return -EINVAL; - default: - return -EINVAL; - } - return iio_format_value(buf, type, val_len, vals); + vals[0] = afe440x_attr->val_table[reg_val].integer; + vals[1] = afe440x_attr->val_table[reg_val].fract; + + return iio_format_value(buf, IIO_VAL_INT_PLUS_MICRO, 2, vals); } static ssize_t afe440x_store_register(struct device *dev, @@ -227,22 +213,12 @@ static ssize_t afe440x_store_register(struct device *dev, if (ret) return ret; - switch (afe440x_attr->type) { - case SIMPLE: - val = integer; - break; - case RESISTANCE: - case CAPACITANCE: - for (val = 0; val < afe440x_attr->table_size; val++) - if (afe440x_attr->val_table[val].integer == integer && - afe440x_attr->val_table[val].fract == fract) - break; - if (val == afe440x_attr->table_size) - return -EINVAL; - break; - default: + for (val = 0; val < afe440x_attr->table_size; val++) + if (afe440x_attr->val_table[val].integer == integer && + afe440x_attr->val_table[val].fract == fract) + break; + if (val == afe440x_attr->table_size) return -EINVAL; - } ret = regmap_update_bits(afe->regmap, afe440x_attr->reg, afe440x_attr->mask, @@ -253,16 +229,13 @@ static ssize_t afe440x_store_register(struct device *dev, return count; } -static AFE440X_ATTR(tia_separate_en, AFE4403_TIAGAIN, AFE440X_TIAGAIN_ENSEPGAIN, SIMPLE, NULL, 0); - -static AFE440X_ATTR(tia_resistance1, AFE4403_TIAGAIN, AFE4403_TIAGAIN_RES, RESISTANCE, afe4403_res_table, ARRAY_SIZE(afe4403_res_table)); -static AFE440X_ATTR(tia_capacitance1, AFE4403_TIAGAIN, AFE4403_TIAGAIN_CAP, CAPACITANCE, afe4403_cap_table, ARRAY_SIZE(afe4403_cap_table)); +static AFE440X_ATTR(tia_resistance1, AFE4403_TIAGAIN, AFE4403_TIAGAIN_RES, afe4403_res_table); +static AFE440X_ATTR(tia_capacitance1, AFE4403_TIAGAIN, AFE4403_TIAGAIN_CAP, afe4403_cap_table); -static AFE440X_ATTR(tia_resistance2, AFE4403_TIA_AMB_GAIN, AFE4403_TIAGAIN_RES, RESISTANCE, afe4403_res_table, ARRAY_SIZE(afe4403_res_table)); -static AFE440X_ATTR(tia_capacitance2, AFE4403_TIA_AMB_GAIN, AFE4403_TIAGAIN_RES, CAPACITANCE, afe4403_cap_table, ARRAY_SIZE(afe4403_cap_table)); +static AFE440X_ATTR(tia_resistance2, AFE4403_TIA_AMB_GAIN, AFE4403_TIAGAIN_RES, afe4403_res_table); +static AFE440X_ATTR(tia_capacitance2, AFE4403_TIA_AMB_GAIN, AFE4403_TIAGAIN_RES, afe4403_cap_table); static struct attribute *afe440x_attributes[] = { - &afe440x_attr_tia_separate_en.dev_attr.attr, &afe440x_attr_tia_resistance1.dev_attr.attr, &afe440x_attr_tia_capacitance1.dev_attr.attr, &afe440x_attr_tia_resistance2.dev_attr.attr, @@ -473,6 +446,7 @@ static const struct iio_trigger_ops afe4403_trigger_ops = { static const struct reg_sequence afe4403_reg_sequences[] = { AFE4403_TIMING_PAIRS, { AFE440X_CONTROL1, AFE440X_CONTROL1_TIMEREN }, + { AFE4403_TIAGAIN, AFE440X_TIAGAIN_ENSEPGAIN }, }; static const struct regmap_range afe4403_yes_ranges[] = { diff --git a/drivers/iio/health/afe4404.c b/drivers/iio/health/afe4404.c index 2d4c522..b9c1666 100644 --- a/drivers/iio/health/afe4404.c +++ b/drivers/iio/health/afe4404.c @@ -193,9 +193,9 @@ static ssize_t afe440x_show_register(struct device *dev, struct iio_dev *indio_dev = dev_to_iio_dev(dev); struct afe4404_data *afe = iio_priv(indio_dev); struct afe440x_attr *afe440x_attr = to_afe440x_attr(attr); - unsigned int reg_val, type; + unsigned int reg_val; int vals[2]; - int ret, val_len; + int ret; ret = regmap_read(afe->regmap, afe440x_attr->reg, ®_val); if (ret) @@ -204,27 +204,13 @@ static ssize_t afe440x_show_register(struct device *dev, reg_val &= afe440x_attr->mask; reg_val >>= afe440x_attr->shift; - switch (afe440x_attr->type) { - case SIMPLE: - type = IIO_VAL_INT; - val_len = 1; - vals[0] = reg_val; - break; - case RESISTANCE: - case CAPACITANCE: - type = IIO_VAL_INT_PLUS_MICRO; - val_len = 2; - if (reg_val < afe440x_attr->table_size) { - vals[0] = afe440x_attr->val_table[reg_val].integer; - vals[1] = afe440x_attr->val_table[reg_val].fract; - break; - } + if (reg_val >= afe440x_attr->table_size) return -EINVAL; - default: - return -EINVAL; - } - return iio_format_value(buf, type, val_len, vals); + vals[0] = afe440x_attr->val_table[reg_val].integer; + vals[1] = afe440x_attr->val_table[reg_val].fract; + + return iio_format_value(buf, IIO_VAL_INT_PLUS_MICRO, 2, vals); } static ssize_t afe440x_store_register(struct device *dev, @@ -240,22 +226,12 @@ static ssize_t afe440x_store_register(struct device *dev, if (ret) return ret; - switch (afe440x_attr->type) { - case SIMPLE: - val = integer; - break; - case RESISTANCE: - case CAPACITANCE: - for (val = 0; val < afe440x_attr->table_size; val++) - if (afe440x_attr->val_table[val].integer == integer && - afe440x_attr->val_table[val].fract == fract) - break; - if (val == afe440x_attr->table_size) - return -EINVAL; - break; - default: + for (val = 0; val < afe440x_attr->table_size; val++) + if (afe440x_attr->val_table[val].integer == integer && + afe440x_attr->val_table[val].fract == fract) + break; + if (val == afe440x_attr->table_size) return -EINVAL; - } ret = regmap_update_bits(afe->regmap, afe440x_attr->reg, afe440x_attr->mask, @@ -266,16 +242,13 @@ static ssize_t afe440x_store_register(struct device *dev, return count; } -static AFE440X_ATTR(tia_separate_en, AFE4404_TIA_GAIN_SEP, AFE440X_TIAGAIN_ENSEPGAIN, SIMPLE, NULL, 0); - -static AFE440X_ATTR(tia_resistance1, AFE4404_TIA_GAIN, AFE4404_TIA_GAIN_RES, RESISTANCE, afe4404_res_table, ARRAY_SIZE(afe4404_res_table)); -static AFE440X_ATTR(tia_capacitance1, AFE4404_TIA_GAIN, AFE4404_TIA_GAIN_CAP, CAPACITANCE, afe4404_cap_table, ARRAY_SIZE(afe4404_cap_table)); +static AFE440X_ATTR(tia_resistance1, AFE4404_TIA_GAIN, AFE4404_TIA_GAIN_RES, afe4404_res_table); +static AFE440X_ATTR(tia_capacitance1, AFE4404_TIA_GAIN, AFE4404_TIA_GAIN_CAP, afe4404_cap_table); -static AFE440X_ATTR(tia_resistance2, AFE4404_TIA_GAIN_SEP, AFE4404_TIA_GAIN_RES, RESISTANCE, afe4404_res_table, ARRAY_SIZE(afe4404_res_table)); -static AFE440X_ATTR(tia_capacitance2, AFE4404_TIA_GAIN_SEP, AFE4404_TIA_GAIN_CAP, CAPACITANCE, afe4404_cap_table, ARRAY_SIZE(afe4404_cap_table)); +static AFE440X_ATTR(tia_resistance2, AFE4404_TIA_GAIN_SEP, AFE4404_TIA_GAIN_RES, afe4404_res_table); +static AFE440X_ATTR(tia_capacitance2, AFE4404_TIA_GAIN_SEP, AFE4404_TIA_GAIN_CAP, afe4404_cap_table); static struct attribute *afe440x_attributes[] = { - &afe440x_attr_tia_separate_en.dev_attr.attr, &afe440x_attr_tia_resistance1.dev_attr.attr, &afe440x_attr_tia_capacitance1.dev_attr.attr, &afe440x_attr_tia_resistance2.dev_attr.attr, @@ -443,6 +416,7 @@ static const struct iio_trigger_ops afe4404_trigger_ops = { static const struct reg_sequence afe4404_reg_sequences[] = { AFE4404_TIMING_PAIRS, { AFE440X_CONTROL1, AFE440X_CONTROL1_TIMEREN }, + { AFE4404_TIA_GAIN_SEP, AFE440X_TIAGAIN_ENSEPGAIN }, { AFE440X_CONTROL2, AFE440X_CONTROL3_OSC_ENABLE }, }; diff --git a/drivers/iio/health/afe440x.h b/drivers/iio/health/afe440x.h index c671ab7..544bbab 100644 --- a/drivers/iio/health/afe440x.h +++ b/drivers/iio/health/afe440x.h @@ -71,8 +71,7 @@ #define AFE440X_CONTROL1_TIMEREN BIT(8) /* TIAGAIN register fields */ -#define AFE440X_TIAGAIN_ENSEPGAIN_MASK BIT(15) -#define AFE440X_TIAGAIN_ENSEPGAIN_SHIFT 15 +#define AFE440X_TIAGAIN_ENSEPGAIN BIT(15) /* CONTROL2 register fields */ #define AFE440X_CONTROL2_PDN_AFE BIT(0) @@ -133,12 +132,6 @@ struct afe440x_reg_info { .output = true, \ } -enum afe440x_reg_type { - SIMPLE, - RESISTANCE, - CAPACITANCE, -}; - struct afe440x_val_table { int integer; int fract; @@ -167,7 +160,6 @@ struct afe440x_attr { unsigned int reg; unsigned int shift; unsigned int mask; - enum afe440x_reg_type type; const struct afe440x_val_table *val_table; unsigned int table_size; }; @@ -175,7 +167,7 @@ struct afe440x_attr { #define to_afe440x_attr(_dev_attr) \ container_of(_dev_attr, struct afe440x_attr, dev_attr) -#define AFE440X_ATTR(_name, _reg, _field, _type, _table, _size) \ +#define AFE440X_ATTR(_name, _reg, _field, _table) \ struct afe440x_attr afe440x_attr_##_name = { \ .dev_attr = __ATTR(_name, (S_IRUGO | S_IWUSR), \ afe440x_show_register, \ @@ -183,9 +175,8 @@ struct afe440x_attr { .reg = _reg, \ .shift = _field ## _SHIFT, \ .mask = _field ## _MASK, \ - .type = _type, \ .val_table = _table, \ - .table_size = _size, \ + .table_size = ARRAY_SIZE(_table), \ } #endif /* _AFE440X_H */ -- 2.8.1 ^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: [PATCH 04/13] iio: health/afe440x: Always use separate gain values 2016-05-01 20:36 ` [PATCH 04/13] iio: health/afe440x: Always use separate gain values Andrew F. Davis @ 2016-05-04 10:02 ` Jonathan Cameron 2016-05-04 15:13 ` Andrew F. Davis 0 siblings, 1 reply; 32+ messages in thread From: Jonathan Cameron @ 2016-05-04 10:02 UTC (permalink / raw) To: Andrew F. Davis, Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald Cc: linux-iio, linux-api, linux-kernel On 01/05/16 21:36, Andrew F. Davis wrote: > Locking the two gain stages to the same setting adds no value for us, > so initialize them as unlocked and remove the sysfs for unlocking them. > This also allows us to greatly simplify showing and setting the gain > registers. > > Signed-off-by: Andrew F. Davis <afd@ti.com> Hmm. ABI change but as you said it's an improvement. Honestly I doubt anyone is using this device without also using userspace that you are providing so lets apply this an cross our fingers that no one minds. Applied. > --- > .../ABI/testing/sysfs-bus-iio-health-afe440x | 9 ---- > drivers/iio/health/afe4403.c | 60 ++++++---------------- > drivers/iio/health/afe4404.c | 60 ++++++---------------- > drivers/iio/health/afe440x.h | 15 ++---- > 4 files changed, 37 insertions(+), 107 deletions(-) > > diff --git a/Documentation/ABI/testing/sysfs-bus-iio-health-afe440x b/Documentation/ABI/testing/sysfs-bus-iio-health-afe440x > index 3740f25..b19053a 100644 > --- a/Documentation/ABI/testing/sysfs-bus-iio-health-afe440x > +++ b/Documentation/ABI/testing/sysfs-bus-iio-health-afe440x > @@ -8,15 +8,6 @@ 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/tia_separate_en > -Date: December 2015 > -KernelVersion: > -Contact: Andrew F. Davis <afd@ti.com> > -Description: > - Enable or disable separate settings for the TransImpedance > - Amplifier above, when disabled both values are set by the > - first channel. > - > 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 > diff --git a/drivers/iio/health/afe4403.c b/drivers/iio/health/afe4403.c > index 5484785..bcff528 100644 > --- a/drivers/iio/health/afe4403.c > +++ b/drivers/iio/health/afe4403.c > @@ -180,9 +180,9 @@ static ssize_t afe440x_show_register(struct device *dev, > struct iio_dev *indio_dev = dev_to_iio_dev(dev); > struct afe4403_data *afe = iio_priv(indio_dev); > struct afe440x_attr *afe440x_attr = to_afe440x_attr(attr); > - unsigned int reg_val, type; > + unsigned int reg_val; > int vals[2]; > - int ret, val_len; > + int ret; > > ret = regmap_read(afe->regmap, afe440x_attr->reg, ®_val); > if (ret) > @@ -191,27 +191,13 @@ static ssize_t afe440x_show_register(struct device *dev, > reg_val &= afe440x_attr->mask; > reg_val >>= afe440x_attr->shift; > > - switch (afe440x_attr->type) { > - case SIMPLE: > - type = IIO_VAL_INT; > - val_len = 1; > - vals[0] = reg_val; > - break; > - case RESISTANCE: > - case CAPACITANCE: > - type = IIO_VAL_INT_PLUS_MICRO; > - val_len = 2; > - if (reg_val < afe440x_attr->table_size) { > - vals[0] = afe440x_attr->val_table[reg_val].integer; > - vals[1] = afe440x_attr->val_table[reg_val].fract; > - break; > - } > + if (reg_val >= afe440x_attr->table_size) > return -EINVAL; > - default: > - return -EINVAL; > - } > > - return iio_format_value(buf, type, val_len, vals); > + vals[0] = afe440x_attr->val_table[reg_val].integer; > + vals[1] = afe440x_attr->val_table[reg_val].fract; > + > + return iio_format_value(buf, IIO_VAL_INT_PLUS_MICRO, 2, vals); > } > > static ssize_t afe440x_store_register(struct device *dev, > @@ -227,22 +213,12 @@ static ssize_t afe440x_store_register(struct device *dev, > if (ret) > return ret; > > - switch (afe440x_attr->type) { > - case SIMPLE: > - val = integer; > - break; > - case RESISTANCE: > - case CAPACITANCE: > - for (val = 0; val < afe440x_attr->table_size; val++) > - if (afe440x_attr->val_table[val].integer == integer && > - afe440x_attr->val_table[val].fract == fract) > - break; > - if (val == afe440x_attr->table_size) > - return -EINVAL; > - break; > - default: > + for (val = 0; val < afe440x_attr->table_size; val++) > + if (afe440x_attr->val_table[val].integer == integer && > + afe440x_attr->val_table[val].fract == fract) > + break; > + if (val == afe440x_attr->table_size) > return -EINVAL; > - } > > ret = regmap_update_bits(afe->regmap, afe440x_attr->reg, > afe440x_attr->mask, > @@ -253,16 +229,13 @@ static ssize_t afe440x_store_register(struct device *dev, > return count; > } > > -static AFE440X_ATTR(tia_separate_en, AFE4403_TIAGAIN, AFE440X_TIAGAIN_ENSEPGAIN, SIMPLE, NULL, 0); > - > -static AFE440X_ATTR(tia_resistance1, AFE4403_TIAGAIN, AFE4403_TIAGAIN_RES, RESISTANCE, afe4403_res_table, ARRAY_SIZE(afe4403_res_table)); > -static AFE440X_ATTR(tia_capacitance1, AFE4403_TIAGAIN, AFE4403_TIAGAIN_CAP, CAPACITANCE, afe4403_cap_table, ARRAY_SIZE(afe4403_cap_table)); > +static AFE440X_ATTR(tia_resistance1, AFE4403_TIAGAIN, AFE4403_TIAGAIN_RES, afe4403_res_table); > +static AFE440X_ATTR(tia_capacitance1, AFE4403_TIAGAIN, AFE4403_TIAGAIN_CAP, afe4403_cap_table); > > -static AFE440X_ATTR(tia_resistance2, AFE4403_TIA_AMB_GAIN, AFE4403_TIAGAIN_RES, RESISTANCE, afe4403_res_table, ARRAY_SIZE(afe4403_res_table)); > -static AFE440X_ATTR(tia_capacitance2, AFE4403_TIA_AMB_GAIN, AFE4403_TIAGAIN_RES, CAPACITANCE, afe4403_cap_table, ARRAY_SIZE(afe4403_cap_table)); > +static AFE440X_ATTR(tia_resistance2, AFE4403_TIA_AMB_GAIN, AFE4403_TIAGAIN_RES, afe4403_res_table); > +static AFE440X_ATTR(tia_capacitance2, AFE4403_TIA_AMB_GAIN, AFE4403_TIAGAIN_RES, afe4403_cap_table); > > static struct attribute *afe440x_attributes[] = { > - &afe440x_attr_tia_separate_en.dev_attr.attr, > &afe440x_attr_tia_resistance1.dev_attr.attr, > &afe440x_attr_tia_capacitance1.dev_attr.attr, > &afe440x_attr_tia_resistance2.dev_attr.attr, > @@ -473,6 +446,7 @@ static const struct iio_trigger_ops afe4403_trigger_ops = { > static const struct reg_sequence afe4403_reg_sequences[] = { > AFE4403_TIMING_PAIRS, > { AFE440X_CONTROL1, AFE440X_CONTROL1_TIMEREN }, > + { AFE4403_TIAGAIN, AFE440X_TIAGAIN_ENSEPGAIN }, > }; > > static const struct regmap_range afe4403_yes_ranges[] = { > diff --git a/drivers/iio/health/afe4404.c b/drivers/iio/health/afe4404.c > index 2d4c522..b9c1666 100644 > --- a/drivers/iio/health/afe4404.c > +++ b/drivers/iio/health/afe4404.c > @@ -193,9 +193,9 @@ static ssize_t afe440x_show_register(struct device *dev, > struct iio_dev *indio_dev = dev_to_iio_dev(dev); > struct afe4404_data *afe = iio_priv(indio_dev); > struct afe440x_attr *afe440x_attr = to_afe440x_attr(attr); > - unsigned int reg_val, type; > + unsigned int reg_val; > int vals[2]; > - int ret, val_len; > + int ret; > > ret = regmap_read(afe->regmap, afe440x_attr->reg, ®_val); > if (ret) > @@ -204,27 +204,13 @@ static ssize_t afe440x_show_register(struct device *dev, > reg_val &= afe440x_attr->mask; > reg_val >>= afe440x_attr->shift; > > - switch (afe440x_attr->type) { > - case SIMPLE: > - type = IIO_VAL_INT; > - val_len = 1; > - vals[0] = reg_val; > - break; > - case RESISTANCE: > - case CAPACITANCE: > - type = IIO_VAL_INT_PLUS_MICRO; > - val_len = 2; > - if (reg_val < afe440x_attr->table_size) { > - vals[0] = afe440x_attr->val_table[reg_val].integer; > - vals[1] = afe440x_attr->val_table[reg_val].fract; > - break; > - } > + if (reg_val >= afe440x_attr->table_size) > return -EINVAL; > - default: > - return -EINVAL; > - } > > - return iio_format_value(buf, type, val_len, vals); > + vals[0] = afe440x_attr->val_table[reg_val].integer; > + vals[1] = afe440x_attr->val_table[reg_val].fract; > + > + return iio_format_value(buf, IIO_VAL_INT_PLUS_MICRO, 2, vals); > } > > static ssize_t afe440x_store_register(struct device *dev, > @@ -240,22 +226,12 @@ static ssize_t afe440x_store_register(struct device *dev, > if (ret) > return ret; > > - switch (afe440x_attr->type) { > - case SIMPLE: > - val = integer; > - break; > - case RESISTANCE: > - case CAPACITANCE: > - for (val = 0; val < afe440x_attr->table_size; val++) > - if (afe440x_attr->val_table[val].integer == integer && > - afe440x_attr->val_table[val].fract == fract) > - break; > - if (val == afe440x_attr->table_size) > - return -EINVAL; > - break; > - default: > + for (val = 0; val < afe440x_attr->table_size; val++) > + if (afe440x_attr->val_table[val].integer == integer && > + afe440x_attr->val_table[val].fract == fract) > + break; > + if (val == afe440x_attr->table_size) > return -EINVAL; > - } > > ret = regmap_update_bits(afe->regmap, afe440x_attr->reg, > afe440x_attr->mask, > @@ -266,16 +242,13 @@ static ssize_t afe440x_store_register(struct device *dev, > return count; > } > > -static AFE440X_ATTR(tia_separate_en, AFE4404_TIA_GAIN_SEP, AFE440X_TIAGAIN_ENSEPGAIN, SIMPLE, NULL, 0); > - > -static AFE440X_ATTR(tia_resistance1, AFE4404_TIA_GAIN, AFE4404_TIA_GAIN_RES, RESISTANCE, afe4404_res_table, ARRAY_SIZE(afe4404_res_table)); > -static AFE440X_ATTR(tia_capacitance1, AFE4404_TIA_GAIN, AFE4404_TIA_GAIN_CAP, CAPACITANCE, afe4404_cap_table, ARRAY_SIZE(afe4404_cap_table)); > +static AFE440X_ATTR(tia_resistance1, AFE4404_TIA_GAIN, AFE4404_TIA_GAIN_RES, afe4404_res_table); > +static AFE440X_ATTR(tia_capacitance1, AFE4404_TIA_GAIN, AFE4404_TIA_GAIN_CAP, afe4404_cap_table); > > -static AFE440X_ATTR(tia_resistance2, AFE4404_TIA_GAIN_SEP, AFE4404_TIA_GAIN_RES, RESISTANCE, afe4404_res_table, ARRAY_SIZE(afe4404_res_table)); > -static AFE440X_ATTR(tia_capacitance2, AFE4404_TIA_GAIN_SEP, AFE4404_TIA_GAIN_CAP, CAPACITANCE, afe4404_cap_table, ARRAY_SIZE(afe4404_cap_table)); > +static AFE440X_ATTR(tia_resistance2, AFE4404_TIA_GAIN_SEP, AFE4404_TIA_GAIN_RES, afe4404_res_table); > +static AFE440X_ATTR(tia_capacitance2, AFE4404_TIA_GAIN_SEP, AFE4404_TIA_GAIN_CAP, afe4404_cap_table); > > static struct attribute *afe440x_attributes[] = { > - &afe440x_attr_tia_separate_en.dev_attr.attr, > &afe440x_attr_tia_resistance1.dev_attr.attr, > &afe440x_attr_tia_capacitance1.dev_attr.attr, > &afe440x_attr_tia_resistance2.dev_attr.attr, > @@ -443,6 +416,7 @@ static const struct iio_trigger_ops afe4404_trigger_ops = { > static const struct reg_sequence afe4404_reg_sequences[] = { > AFE4404_TIMING_PAIRS, > { AFE440X_CONTROL1, AFE440X_CONTROL1_TIMEREN }, > + { AFE4404_TIA_GAIN_SEP, AFE440X_TIAGAIN_ENSEPGAIN }, > { AFE440X_CONTROL2, AFE440X_CONTROL3_OSC_ENABLE }, > }; > > diff --git a/drivers/iio/health/afe440x.h b/drivers/iio/health/afe440x.h > index c671ab7..544bbab 100644 > --- a/drivers/iio/health/afe440x.h > +++ b/drivers/iio/health/afe440x.h > @@ -71,8 +71,7 @@ > #define AFE440X_CONTROL1_TIMEREN BIT(8) > > /* TIAGAIN register fields */ > -#define AFE440X_TIAGAIN_ENSEPGAIN_MASK BIT(15) > -#define AFE440X_TIAGAIN_ENSEPGAIN_SHIFT 15 > +#define AFE440X_TIAGAIN_ENSEPGAIN BIT(15) > > /* CONTROL2 register fields */ > #define AFE440X_CONTROL2_PDN_AFE BIT(0) > @@ -133,12 +132,6 @@ struct afe440x_reg_info { > .output = true, \ > } > > -enum afe440x_reg_type { > - SIMPLE, > - RESISTANCE, > - CAPACITANCE, > -}; > - > struct afe440x_val_table { > int integer; > int fract; > @@ -167,7 +160,6 @@ struct afe440x_attr { > unsigned int reg; > unsigned int shift; > unsigned int mask; > - enum afe440x_reg_type type; > const struct afe440x_val_table *val_table; > unsigned int table_size; > }; > @@ -175,7 +167,7 @@ struct afe440x_attr { > #define to_afe440x_attr(_dev_attr) \ > container_of(_dev_attr, struct afe440x_attr, dev_attr) > > -#define AFE440X_ATTR(_name, _reg, _field, _type, _table, _size) \ > +#define AFE440X_ATTR(_name, _reg, _field, _table) \ > struct afe440x_attr afe440x_attr_##_name = { \ > .dev_attr = __ATTR(_name, (S_IRUGO | S_IWUSR), \ > afe440x_show_register, \ > @@ -183,9 +175,8 @@ struct afe440x_attr { > .reg = _reg, \ > .shift = _field ## _SHIFT, \ > .mask = _field ## _MASK, \ > - .type = _type, \ > .val_table = _table, \ > - .table_size = _size, \ > + .table_size = ARRAY_SIZE(_table), \ > } > > #endif /* _AFE440X_H */ > ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 04/13] iio: health/afe440x: Always use separate gain values 2016-05-04 10:02 ` Jonathan Cameron @ 2016-05-04 15:13 ` Andrew F. Davis 2016-05-04 18:33 ` Jonathan Cameron 0 siblings, 1 reply; 32+ messages in thread From: Andrew F. Davis @ 2016-05-04 15:13 UTC (permalink / raw) To: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald Cc: linux-iio, linux-api, linux-kernel On 05/04/2016 05:02 AM, Jonathan Cameron wrote: > On 01/05/16 21:36, Andrew F. Davis wrote: >> Locking the two gain stages to the same setting adds no value for us, >> so initialize them as unlocked and remove the sysfs for unlocking them. >> This also allows us to greatly simplify showing and setting the gain >> registers. >> >> Signed-off-by: Andrew F. Davis <afd@ti.com> > Hmm. ABI change but as you said it's an improvement. > > Honestly I doubt anyone is using this device without also using userspace that > you are providing so lets apply this an cross our fingers that no one minds. > Actually we don't even provide a Linux userspace for these yet, starting to work on that is what prompted these changes. These parts have traditionally been paired with a microcontroller to perform the work in dedicated devices. This was put on my desk as they are hoping to start enabling these to be integrated into smart devices running Linux (Android). I'm not an expert in pulse metering, so my initial attempt at an ABI was a bit off compared to what I now believe to be the "correct" way to expose these parts. Thanks again for tolerating all this madness :), Andrew > Applied. >> --- >> .../ABI/testing/sysfs-bus-iio-health-afe440x | 9 ---- >> drivers/iio/health/afe4403.c | 60 ++++++---------------- >> drivers/iio/health/afe4404.c | 60 ++++++---------------- >> drivers/iio/health/afe440x.h | 15 ++---- >> 4 files changed, 37 insertions(+), 107 deletions(-) >> >> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-health-afe440x b/Documentation/ABI/testing/sysfs-bus-iio-health-afe440x >> index 3740f25..b19053a 100644 >> --- a/Documentation/ABI/testing/sysfs-bus-iio-health-afe440x >> +++ b/Documentation/ABI/testing/sysfs-bus-iio-health-afe440x >> @@ -8,15 +8,6 @@ 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/tia_separate_en >> -Date: December 2015 >> -KernelVersion: >> -Contact: Andrew F. Davis <afd@ti.com> >> -Description: >> - Enable or disable separate settings for the TransImpedance >> - Amplifier above, when disabled both values are set by the >> - first channel. >> - >> 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 >> diff --git a/drivers/iio/health/afe4403.c b/drivers/iio/health/afe4403.c >> index 5484785..bcff528 100644 >> --- a/drivers/iio/health/afe4403.c >> +++ b/drivers/iio/health/afe4403.c >> @@ -180,9 +180,9 @@ static ssize_t afe440x_show_register(struct device *dev, >> struct iio_dev *indio_dev = dev_to_iio_dev(dev); >> struct afe4403_data *afe = iio_priv(indio_dev); >> struct afe440x_attr *afe440x_attr = to_afe440x_attr(attr); >> - unsigned int reg_val, type; >> + unsigned int reg_val; >> int vals[2]; >> - int ret, val_len; >> + int ret; >> >> ret = regmap_read(afe->regmap, afe440x_attr->reg, ®_val); >> if (ret) >> @@ -191,27 +191,13 @@ static ssize_t afe440x_show_register(struct device *dev, >> reg_val &= afe440x_attr->mask; >> reg_val >>= afe440x_attr->shift; >> >> - switch (afe440x_attr->type) { >> - case SIMPLE: >> - type = IIO_VAL_INT; >> - val_len = 1; >> - vals[0] = reg_val; >> - break; >> - case RESISTANCE: >> - case CAPACITANCE: >> - type = IIO_VAL_INT_PLUS_MICRO; >> - val_len = 2; >> - if (reg_val < afe440x_attr->table_size) { >> - vals[0] = afe440x_attr->val_table[reg_val].integer; >> - vals[1] = afe440x_attr->val_table[reg_val].fract; >> - break; >> - } >> + if (reg_val >= afe440x_attr->table_size) >> return -EINVAL; >> - default: >> - return -EINVAL; >> - } >> >> - return iio_format_value(buf, type, val_len, vals); >> + vals[0] = afe440x_attr->val_table[reg_val].integer; >> + vals[1] = afe440x_attr->val_table[reg_val].fract; >> + >> + return iio_format_value(buf, IIO_VAL_INT_PLUS_MICRO, 2, vals); >> } >> >> static ssize_t afe440x_store_register(struct device *dev, >> @@ -227,22 +213,12 @@ static ssize_t afe440x_store_register(struct device *dev, >> if (ret) >> return ret; >> >> - switch (afe440x_attr->type) { >> - case SIMPLE: >> - val = integer; >> - break; >> - case RESISTANCE: >> - case CAPACITANCE: >> - for (val = 0; val < afe440x_attr->table_size; val++) >> - if (afe440x_attr->val_table[val].integer == integer && >> - afe440x_attr->val_table[val].fract == fract) >> - break; >> - if (val == afe440x_attr->table_size) >> - return -EINVAL; >> - break; >> - default: >> + for (val = 0; val < afe440x_attr->table_size; val++) >> + if (afe440x_attr->val_table[val].integer == integer && >> + afe440x_attr->val_table[val].fract == fract) >> + break; >> + if (val == afe440x_attr->table_size) >> return -EINVAL; >> - } >> >> ret = regmap_update_bits(afe->regmap, afe440x_attr->reg, >> afe440x_attr->mask, >> @@ -253,16 +229,13 @@ static ssize_t afe440x_store_register(struct device *dev, >> return count; >> } >> >> -static AFE440X_ATTR(tia_separate_en, AFE4403_TIAGAIN, AFE440X_TIAGAIN_ENSEPGAIN, SIMPLE, NULL, 0); >> - >> -static AFE440X_ATTR(tia_resistance1, AFE4403_TIAGAIN, AFE4403_TIAGAIN_RES, RESISTANCE, afe4403_res_table, ARRAY_SIZE(afe4403_res_table)); >> -static AFE440X_ATTR(tia_capacitance1, AFE4403_TIAGAIN, AFE4403_TIAGAIN_CAP, CAPACITANCE, afe4403_cap_table, ARRAY_SIZE(afe4403_cap_table)); >> +static AFE440X_ATTR(tia_resistance1, AFE4403_TIAGAIN, AFE4403_TIAGAIN_RES, afe4403_res_table); >> +static AFE440X_ATTR(tia_capacitance1, AFE4403_TIAGAIN, AFE4403_TIAGAIN_CAP, afe4403_cap_table); >> >> -static AFE440X_ATTR(tia_resistance2, AFE4403_TIA_AMB_GAIN, AFE4403_TIAGAIN_RES, RESISTANCE, afe4403_res_table, ARRAY_SIZE(afe4403_res_table)); >> -static AFE440X_ATTR(tia_capacitance2, AFE4403_TIA_AMB_GAIN, AFE4403_TIAGAIN_RES, CAPACITANCE, afe4403_cap_table, ARRAY_SIZE(afe4403_cap_table)); >> +static AFE440X_ATTR(tia_resistance2, AFE4403_TIA_AMB_GAIN, AFE4403_TIAGAIN_RES, afe4403_res_table); >> +static AFE440X_ATTR(tia_capacitance2, AFE4403_TIA_AMB_GAIN, AFE4403_TIAGAIN_RES, afe4403_cap_table); >> >> static struct attribute *afe440x_attributes[] = { >> - &afe440x_attr_tia_separate_en.dev_attr.attr, >> &afe440x_attr_tia_resistance1.dev_attr.attr, >> &afe440x_attr_tia_capacitance1.dev_attr.attr, >> &afe440x_attr_tia_resistance2.dev_attr.attr, >> @@ -473,6 +446,7 @@ static const struct iio_trigger_ops afe4403_trigger_ops = { >> static const struct reg_sequence afe4403_reg_sequences[] = { >> AFE4403_TIMING_PAIRS, >> { AFE440X_CONTROL1, AFE440X_CONTROL1_TIMEREN }, >> + { AFE4403_TIAGAIN, AFE440X_TIAGAIN_ENSEPGAIN }, >> }; >> >> static const struct regmap_range afe4403_yes_ranges[] = { >> diff --git a/drivers/iio/health/afe4404.c b/drivers/iio/health/afe4404.c >> index 2d4c522..b9c1666 100644 >> --- a/drivers/iio/health/afe4404.c >> +++ b/drivers/iio/health/afe4404.c >> @@ -193,9 +193,9 @@ static ssize_t afe440x_show_register(struct device *dev, >> struct iio_dev *indio_dev = dev_to_iio_dev(dev); >> struct afe4404_data *afe = iio_priv(indio_dev); >> struct afe440x_attr *afe440x_attr = to_afe440x_attr(attr); >> - unsigned int reg_val, type; >> + unsigned int reg_val; >> int vals[2]; >> - int ret, val_len; >> + int ret; >> >> ret = regmap_read(afe->regmap, afe440x_attr->reg, ®_val); >> if (ret) >> @@ -204,27 +204,13 @@ static ssize_t afe440x_show_register(struct device *dev, >> reg_val &= afe440x_attr->mask; >> reg_val >>= afe440x_attr->shift; >> >> - switch (afe440x_attr->type) { >> - case SIMPLE: >> - type = IIO_VAL_INT; >> - val_len = 1; >> - vals[0] = reg_val; >> - break; >> - case RESISTANCE: >> - case CAPACITANCE: >> - type = IIO_VAL_INT_PLUS_MICRO; >> - val_len = 2; >> - if (reg_val < afe440x_attr->table_size) { >> - vals[0] = afe440x_attr->val_table[reg_val].integer; >> - vals[1] = afe440x_attr->val_table[reg_val].fract; >> - break; >> - } >> + if (reg_val >= afe440x_attr->table_size) >> return -EINVAL; >> - default: >> - return -EINVAL; >> - } >> >> - return iio_format_value(buf, type, val_len, vals); >> + vals[0] = afe440x_attr->val_table[reg_val].integer; >> + vals[1] = afe440x_attr->val_table[reg_val].fract; >> + >> + return iio_format_value(buf, IIO_VAL_INT_PLUS_MICRO, 2, vals); >> } >> >> static ssize_t afe440x_store_register(struct device *dev, >> @@ -240,22 +226,12 @@ static ssize_t afe440x_store_register(struct device *dev, >> if (ret) >> return ret; >> >> - switch (afe440x_attr->type) { >> - case SIMPLE: >> - val = integer; >> - break; >> - case RESISTANCE: >> - case CAPACITANCE: >> - for (val = 0; val < afe440x_attr->table_size; val++) >> - if (afe440x_attr->val_table[val].integer == integer && >> - afe440x_attr->val_table[val].fract == fract) >> - break; >> - if (val == afe440x_attr->table_size) >> - return -EINVAL; >> - break; >> - default: >> + for (val = 0; val < afe440x_attr->table_size; val++) >> + if (afe440x_attr->val_table[val].integer == integer && >> + afe440x_attr->val_table[val].fract == fract) >> + break; >> + if (val == afe440x_attr->table_size) >> return -EINVAL; >> - } >> >> ret = regmap_update_bits(afe->regmap, afe440x_attr->reg, >> afe440x_attr->mask, >> @@ -266,16 +242,13 @@ static ssize_t afe440x_store_register(struct device *dev, >> return count; >> } >> >> -static AFE440X_ATTR(tia_separate_en, AFE4404_TIA_GAIN_SEP, AFE440X_TIAGAIN_ENSEPGAIN, SIMPLE, NULL, 0); >> - >> -static AFE440X_ATTR(tia_resistance1, AFE4404_TIA_GAIN, AFE4404_TIA_GAIN_RES, RESISTANCE, afe4404_res_table, ARRAY_SIZE(afe4404_res_table)); >> -static AFE440X_ATTR(tia_capacitance1, AFE4404_TIA_GAIN, AFE4404_TIA_GAIN_CAP, CAPACITANCE, afe4404_cap_table, ARRAY_SIZE(afe4404_cap_table)); >> +static AFE440X_ATTR(tia_resistance1, AFE4404_TIA_GAIN, AFE4404_TIA_GAIN_RES, afe4404_res_table); >> +static AFE440X_ATTR(tia_capacitance1, AFE4404_TIA_GAIN, AFE4404_TIA_GAIN_CAP, afe4404_cap_table); >> >> -static AFE440X_ATTR(tia_resistance2, AFE4404_TIA_GAIN_SEP, AFE4404_TIA_GAIN_RES, RESISTANCE, afe4404_res_table, ARRAY_SIZE(afe4404_res_table)); >> -static AFE440X_ATTR(tia_capacitance2, AFE4404_TIA_GAIN_SEP, AFE4404_TIA_GAIN_CAP, CAPACITANCE, afe4404_cap_table, ARRAY_SIZE(afe4404_cap_table)); >> +static AFE440X_ATTR(tia_resistance2, AFE4404_TIA_GAIN_SEP, AFE4404_TIA_GAIN_RES, afe4404_res_table); >> +static AFE440X_ATTR(tia_capacitance2, AFE4404_TIA_GAIN_SEP, AFE4404_TIA_GAIN_CAP, afe4404_cap_table); >> >> static struct attribute *afe440x_attributes[] = { >> - &afe440x_attr_tia_separate_en.dev_attr.attr, >> &afe440x_attr_tia_resistance1.dev_attr.attr, >> &afe440x_attr_tia_capacitance1.dev_attr.attr, >> &afe440x_attr_tia_resistance2.dev_attr.attr, >> @@ -443,6 +416,7 @@ static const struct iio_trigger_ops afe4404_trigger_ops = { >> static const struct reg_sequence afe4404_reg_sequences[] = { >> AFE4404_TIMING_PAIRS, >> { AFE440X_CONTROL1, AFE440X_CONTROL1_TIMEREN }, >> + { AFE4404_TIA_GAIN_SEP, AFE440X_TIAGAIN_ENSEPGAIN }, >> { AFE440X_CONTROL2, AFE440X_CONTROL3_OSC_ENABLE }, >> }; >> >> diff --git a/drivers/iio/health/afe440x.h b/drivers/iio/health/afe440x.h >> index c671ab7..544bbab 100644 >> --- a/drivers/iio/health/afe440x.h >> +++ b/drivers/iio/health/afe440x.h >> @@ -71,8 +71,7 @@ >> #define AFE440X_CONTROL1_TIMEREN BIT(8) >> >> /* TIAGAIN register fields */ >> -#define AFE440X_TIAGAIN_ENSEPGAIN_MASK BIT(15) >> -#define AFE440X_TIAGAIN_ENSEPGAIN_SHIFT 15 >> +#define AFE440X_TIAGAIN_ENSEPGAIN BIT(15) >> >> /* CONTROL2 register fields */ >> #define AFE440X_CONTROL2_PDN_AFE BIT(0) >> @@ -133,12 +132,6 @@ struct afe440x_reg_info { >> .output = true, \ >> } >> >> -enum afe440x_reg_type { >> - SIMPLE, >> - RESISTANCE, >> - CAPACITANCE, >> -}; >> - >> struct afe440x_val_table { >> int integer; >> int fract; >> @@ -167,7 +160,6 @@ struct afe440x_attr { >> unsigned int reg; >> unsigned int shift; >> unsigned int mask; >> - enum afe440x_reg_type type; >> const struct afe440x_val_table *val_table; >> unsigned int table_size; >> }; >> @@ -175,7 +167,7 @@ struct afe440x_attr { >> #define to_afe440x_attr(_dev_attr) \ >> container_of(_dev_attr, struct afe440x_attr, dev_attr) >> >> -#define AFE440X_ATTR(_name, _reg, _field, _type, _table, _size) \ >> +#define AFE440X_ATTR(_name, _reg, _field, _table) \ >> struct afe440x_attr afe440x_attr_##_name = { \ >> .dev_attr = __ATTR(_name, (S_IRUGO | S_IWUSR), \ >> afe440x_show_register, \ >> @@ -183,9 +175,8 @@ struct afe440x_attr { >> .reg = _reg, \ >> .shift = _field ## _SHIFT, \ >> .mask = _field ## _MASK, \ >> - .type = _type, \ >> .val_table = _table, \ >> - .table_size = _size, \ >> + .table_size = ARRAY_SIZE(_table), \ >> } >> >> #endif /* _AFE440X_H */ >> > ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 04/13] iio: health/afe440x: Always use separate gain values 2016-05-04 15:13 ` Andrew F. Davis @ 2016-05-04 18:33 ` Jonathan Cameron 0 siblings, 0 replies; 32+ messages in thread From: Jonathan Cameron @ 2016-05-04 18:33 UTC (permalink / raw) To: Andrew F. Davis, Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald Cc: linux-iio, linux-api, linux-kernel On 4 May 2016 16:13:29 BST, "Andrew F. Davis" <afd@ti.com> wrote: >On 05/04/2016 05:02 AM, Jonathan Cameron wrote: >> On 01/05/16 21:36, Andrew F. Davis wrote: >>> Locking the two gain stages to the same setting adds no value for >us, >>> so initialize them as unlocked and remove the sysfs for unlocking >them. >>> This also allows us to greatly simplify showing and setting the gain >>> registers. >>> >>> Signed-off-by: Andrew F. Davis <afd@ti.com> >> Hmm. ABI change but as you said it's an improvement. >> >> Honestly I doubt anyone is using this device without also using >userspace that >> you are providing so lets apply this an cross our fingers that no one >minds. >> > >Actually we don't even provide a Linux userspace for these yet, >starting >to work on that is what prompted these changes. These parts have >traditionally been paired with a microcontroller to perform the work in >dedicated devices. This was put on my desk as they are hoping to start >enabling these to be integrated into smart devices running Linux >(Android). I'm not an expert in pulse metering, so my initial attempt >at >an ABI was a bit off compared to what I now believe to be the "correct" >way to expose these parts. *laughs* sometimes the only way to find out is developing real products. > >Thanks again for tolerating all this madness :), It's cool. I worked with some medics who loved pulse oximeters a few years back. Not sure what kit they were using though. We were doing breathing measurement with structure light. Turns out you can get pulse from that (sort of). Plus all sorts of weird plots that medics love which make no sense at all from an engineering POV. I like weird devices even if they can cause headaches :) Weird comes in many forms! Jonathan >Andrew > >> Applied. >>> --- >>> .../ABI/testing/sysfs-bus-iio-health-afe440x | 9 ---- >>> drivers/iio/health/afe4403.c | 60 >++++++---------------- >>> drivers/iio/health/afe4404.c | 60 >++++++---------------- >>> drivers/iio/health/afe440x.h | 15 ++---- >>> 4 files changed, 37 insertions(+), 107 deletions(-) >>> >>> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-health-afe440x >b/Documentation/ABI/testing/sysfs-bus-iio-health-afe440x >>> index 3740f25..b19053a 100644 >>> --- a/Documentation/ABI/testing/sysfs-bus-iio-health-afe440x >>> +++ b/Documentation/ABI/testing/sysfs-bus-iio-health-afe440x >>> @@ -8,15 +8,6 @@ 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/tia_separate_en >>> -Date: December 2015 >>> -KernelVersion: >>> -Contact: Andrew F. Davis <afd@ti.com> >>> -Description: >>> - Enable or disable separate settings for the TransImpedance >>> - Amplifier above, when disabled both values are set by the >>> - first channel. >>> - >>> 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 >>> diff --git a/drivers/iio/health/afe4403.c >b/drivers/iio/health/afe4403.c >>> index 5484785..bcff528 100644 >>> --- a/drivers/iio/health/afe4403.c >>> +++ b/drivers/iio/health/afe4403.c >>> @@ -180,9 +180,9 @@ static ssize_t afe440x_show_register(struct >device *dev, >>> struct iio_dev *indio_dev = dev_to_iio_dev(dev); >>> struct afe4403_data *afe = iio_priv(indio_dev); >>> struct afe440x_attr *afe440x_attr = to_afe440x_attr(attr); >>> - unsigned int reg_val, type; >>> + unsigned int reg_val; >>> int vals[2]; >>> - int ret, val_len; >>> + int ret; >>> >>> ret = regmap_read(afe->regmap, afe440x_attr->reg, ®_val); >>> if (ret) >>> @@ -191,27 +191,13 @@ static ssize_t afe440x_show_register(struct >device *dev, >>> reg_val &= afe440x_attr->mask; >>> reg_val >>= afe440x_attr->shift; >>> >>> - switch (afe440x_attr->type) { >>> - case SIMPLE: >>> - type = IIO_VAL_INT; >>> - val_len = 1; >>> - vals[0] = reg_val; >>> - break; >>> - case RESISTANCE: >>> - case CAPACITANCE: >>> - type = IIO_VAL_INT_PLUS_MICRO; >>> - val_len = 2; >>> - if (reg_val < afe440x_attr->table_size) { >>> - vals[0] = afe440x_attr->val_table[reg_val].integer; >>> - vals[1] = afe440x_attr->val_table[reg_val].fract; >>> - break; >>> - } >>> + if (reg_val >= afe440x_attr->table_size) >>> return -EINVAL; >>> - default: >>> - return -EINVAL; >>> - } >>> >>> - return iio_format_value(buf, type, val_len, vals); >>> + vals[0] = afe440x_attr->val_table[reg_val].integer; >>> + vals[1] = afe440x_attr->val_table[reg_val].fract; >>> + >>> + return iio_format_value(buf, IIO_VAL_INT_PLUS_MICRO, 2, vals); >>> } >>> >>> static ssize_t afe440x_store_register(struct device *dev, >>> @@ -227,22 +213,12 @@ static ssize_t afe440x_store_register(struct >device *dev, >>> if (ret) >>> return ret; >>> >>> - switch (afe440x_attr->type) { >>> - case SIMPLE: >>> - val = integer; >>> - break; >>> - case RESISTANCE: >>> - case CAPACITANCE: >>> - for (val = 0; val < afe440x_attr->table_size; val++) >>> - if (afe440x_attr->val_table[val].integer == integer && >>> - afe440x_attr->val_table[val].fract == fract) >>> - break; >>> - if (val == afe440x_attr->table_size) >>> - return -EINVAL; >>> - break; >>> - default: >>> + for (val = 0; val < afe440x_attr->table_size; val++) >>> + if (afe440x_attr->val_table[val].integer == integer && >>> + afe440x_attr->val_table[val].fract == fract) >>> + break; >>> + if (val == afe440x_attr->table_size) >>> return -EINVAL; >>> - } >>> >>> ret = regmap_update_bits(afe->regmap, afe440x_attr->reg, >>> afe440x_attr->mask, >>> @@ -253,16 +229,13 @@ static ssize_t afe440x_store_register(struct >device *dev, >>> return count; >>> } >>> >>> -static AFE440X_ATTR(tia_separate_en, AFE4403_TIAGAIN, >AFE440X_TIAGAIN_ENSEPGAIN, SIMPLE, NULL, 0); >>> - >>> -static AFE440X_ATTR(tia_resistance1, AFE4403_TIAGAIN, >AFE4403_TIAGAIN_RES, RESISTANCE, afe4403_res_table, >ARRAY_SIZE(afe4403_res_table)); >>> -static AFE440X_ATTR(tia_capacitance1, AFE4403_TIAGAIN, >AFE4403_TIAGAIN_CAP, CAPACITANCE, afe4403_cap_table, >ARRAY_SIZE(afe4403_cap_table)); >>> +static AFE440X_ATTR(tia_resistance1, AFE4403_TIAGAIN, >AFE4403_TIAGAIN_RES, afe4403_res_table); >>> +static AFE440X_ATTR(tia_capacitance1, AFE4403_TIAGAIN, >AFE4403_TIAGAIN_CAP, afe4403_cap_table); >>> >>> -static AFE440X_ATTR(tia_resistance2, AFE4403_TIA_AMB_GAIN, >AFE4403_TIAGAIN_RES, RESISTANCE, afe4403_res_table, >ARRAY_SIZE(afe4403_res_table)); >>> -static AFE440X_ATTR(tia_capacitance2, AFE4403_TIA_AMB_GAIN, >AFE4403_TIAGAIN_RES, CAPACITANCE, afe4403_cap_table, >ARRAY_SIZE(afe4403_cap_table)); >>> +static AFE440X_ATTR(tia_resistance2, AFE4403_TIA_AMB_GAIN, >AFE4403_TIAGAIN_RES, afe4403_res_table); >>> +static AFE440X_ATTR(tia_capacitance2, AFE4403_TIA_AMB_GAIN, >AFE4403_TIAGAIN_RES, afe4403_cap_table); >>> >>> static struct attribute *afe440x_attributes[] = { >>> - &afe440x_attr_tia_separate_en.dev_attr.attr, >>> &afe440x_attr_tia_resistance1.dev_attr.attr, >>> &afe440x_attr_tia_capacitance1.dev_attr.attr, >>> &afe440x_attr_tia_resistance2.dev_attr.attr, >>> @@ -473,6 +446,7 @@ static const struct iio_trigger_ops >afe4403_trigger_ops = { >>> static const struct reg_sequence afe4403_reg_sequences[] = { >>> AFE4403_TIMING_PAIRS, >>> { AFE440X_CONTROL1, AFE440X_CONTROL1_TIMEREN }, >>> + { AFE4403_TIAGAIN, AFE440X_TIAGAIN_ENSEPGAIN }, >>> }; >>> >>> static const struct regmap_range afe4403_yes_ranges[] = { >>> diff --git a/drivers/iio/health/afe4404.c >b/drivers/iio/health/afe4404.c >>> index 2d4c522..b9c1666 100644 >>> --- a/drivers/iio/health/afe4404.c >>> +++ b/drivers/iio/health/afe4404.c >>> @@ -193,9 +193,9 @@ static ssize_t afe440x_show_register(struct >device *dev, >>> struct iio_dev *indio_dev = dev_to_iio_dev(dev); >>> struct afe4404_data *afe = iio_priv(indio_dev); >>> struct afe440x_attr *afe440x_attr = to_afe440x_attr(attr); >>> - unsigned int reg_val, type; >>> + unsigned int reg_val; >>> int vals[2]; >>> - int ret, val_len; >>> + int ret; >>> >>> ret = regmap_read(afe->regmap, afe440x_attr->reg, ®_val); >>> if (ret) >>> @@ -204,27 +204,13 @@ static ssize_t afe440x_show_register(struct >device *dev, >>> reg_val &= afe440x_attr->mask; >>> reg_val >>= afe440x_attr->shift; >>> >>> - switch (afe440x_attr->type) { >>> - case SIMPLE: >>> - type = IIO_VAL_INT; >>> - val_len = 1; >>> - vals[0] = reg_val; >>> - break; >>> - case RESISTANCE: >>> - case CAPACITANCE: >>> - type = IIO_VAL_INT_PLUS_MICRO; >>> - val_len = 2; >>> - if (reg_val < afe440x_attr->table_size) { >>> - vals[0] = afe440x_attr->val_table[reg_val].integer; >>> - vals[1] = afe440x_attr->val_table[reg_val].fract; >>> - break; >>> - } >>> + if (reg_val >= afe440x_attr->table_size) >>> return -EINVAL; >>> - default: >>> - return -EINVAL; >>> - } >>> >>> - return iio_format_value(buf, type, val_len, vals); >>> + vals[0] = afe440x_attr->val_table[reg_val].integer; >>> + vals[1] = afe440x_attr->val_table[reg_val].fract; >>> + >>> + return iio_format_value(buf, IIO_VAL_INT_PLUS_MICRO, 2, vals); >>> } >>> >>> static ssize_t afe440x_store_register(struct device *dev, >>> @@ -240,22 +226,12 @@ static ssize_t afe440x_store_register(struct >device *dev, >>> if (ret) >>> return ret; >>> >>> - switch (afe440x_attr->type) { >>> - case SIMPLE: >>> - val = integer; >>> - break; >>> - case RESISTANCE: >>> - case CAPACITANCE: >>> - for (val = 0; val < afe440x_attr->table_size; val++) >>> - if (afe440x_attr->val_table[val].integer == integer && >>> - afe440x_attr->val_table[val].fract == fract) >>> - break; >>> - if (val == afe440x_attr->table_size) >>> - return -EINVAL; >>> - break; >>> - default: >>> + for (val = 0; val < afe440x_attr->table_size; val++) >>> + if (afe440x_attr->val_table[val].integer == integer && >>> + afe440x_attr->val_table[val].fract == fract) >>> + break; >>> + if (val == afe440x_attr->table_size) >>> return -EINVAL; >>> - } >>> >>> ret = regmap_update_bits(afe->regmap, afe440x_attr->reg, >>> afe440x_attr->mask, >>> @@ -266,16 +242,13 @@ static ssize_t afe440x_store_register(struct >device *dev, >>> return count; >>> } >>> >>> -static AFE440X_ATTR(tia_separate_en, AFE4404_TIA_GAIN_SEP, >AFE440X_TIAGAIN_ENSEPGAIN, SIMPLE, NULL, 0); >>> - >>> -static AFE440X_ATTR(tia_resistance1, AFE4404_TIA_GAIN, >AFE4404_TIA_GAIN_RES, RESISTANCE, afe4404_res_table, >ARRAY_SIZE(afe4404_res_table)); >>> -static AFE440X_ATTR(tia_capacitance1, AFE4404_TIA_GAIN, >AFE4404_TIA_GAIN_CAP, CAPACITANCE, afe4404_cap_table, >ARRAY_SIZE(afe4404_cap_table)); >>> +static AFE440X_ATTR(tia_resistance1, AFE4404_TIA_GAIN, >AFE4404_TIA_GAIN_RES, afe4404_res_table); >>> +static AFE440X_ATTR(tia_capacitance1, AFE4404_TIA_GAIN, >AFE4404_TIA_GAIN_CAP, afe4404_cap_table); >>> >>> -static AFE440X_ATTR(tia_resistance2, AFE4404_TIA_GAIN_SEP, >AFE4404_TIA_GAIN_RES, RESISTANCE, afe4404_res_table, >ARRAY_SIZE(afe4404_res_table)); >>> -static AFE440X_ATTR(tia_capacitance2, AFE4404_TIA_GAIN_SEP, >AFE4404_TIA_GAIN_CAP, CAPACITANCE, afe4404_cap_table, >ARRAY_SIZE(afe4404_cap_table)); >>> +static AFE440X_ATTR(tia_resistance2, AFE4404_TIA_GAIN_SEP, >AFE4404_TIA_GAIN_RES, afe4404_res_table); >>> +static AFE440X_ATTR(tia_capacitance2, AFE4404_TIA_GAIN_SEP, >AFE4404_TIA_GAIN_CAP, afe4404_cap_table); >>> >>> static struct attribute *afe440x_attributes[] = { >>> - &afe440x_attr_tia_separate_en.dev_attr.attr, >>> &afe440x_attr_tia_resistance1.dev_attr.attr, >>> &afe440x_attr_tia_capacitance1.dev_attr.attr, >>> &afe440x_attr_tia_resistance2.dev_attr.attr, >>> @@ -443,6 +416,7 @@ static const struct iio_trigger_ops >afe4404_trigger_ops = { >>> static const struct reg_sequence afe4404_reg_sequences[] = { >>> AFE4404_TIMING_PAIRS, >>> { AFE440X_CONTROL1, AFE440X_CONTROL1_TIMEREN }, >>> + { AFE4404_TIA_GAIN_SEP, AFE440X_TIAGAIN_ENSEPGAIN }, >>> { AFE440X_CONTROL2, AFE440X_CONTROL3_OSC_ENABLE }, >>> }; >>> >>> diff --git a/drivers/iio/health/afe440x.h >b/drivers/iio/health/afe440x.h >>> index c671ab7..544bbab 100644 >>> --- a/drivers/iio/health/afe440x.h >>> +++ b/drivers/iio/health/afe440x.h >>> @@ -71,8 +71,7 @@ >>> #define AFE440X_CONTROL1_TIMEREN BIT(8) >>> >>> /* TIAGAIN register fields */ >>> -#define AFE440X_TIAGAIN_ENSEPGAIN_MASK BIT(15) >>> -#define AFE440X_TIAGAIN_ENSEPGAIN_SHIFT 15 >>> +#define AFE440X_TIAGAIN_ENSEPGAIN BIT(15) >>> >>> /* CONTROL2 register fields */ >>> #define AFE440X_CONTROL2_PDN_AFE BIT(0) >>> @@ -133,12 +132,6 @@ struct afe440x_reg_info { >>> .output = true, \ >>> } >>> >>> -enum afe440x_reg_type { >>> - SIMPLE, >>> - RESISTANCE, >>> - CAPACITANCE, >>> -}; >>> - >>> struct afe440x_val_table { >>> int integer; >>> int fract; >>> @@ -167,7 +160,6 @@ struct afe440x_attr { >>> unsigned int reg; >>> unsigned int shift; >>> unsigned int mask; >>> - enum afe440x_reg_type type; >>> const struct afe440x_val_table *val_table; >>> unsigned int table_size; >>> }; >>> @@ -175,7 +167,7 @@ struct afe440x_attr { >>> #define to_afe440x_attr(_dev_attr) \ >>> container_of(_dev_attr, struct afe440x_attr, dev_attr) >>> >>> -#define AFE440X_ATTR(_name, _reg, _field, _type, _table, _size) \ >>> +#define AFE440X_ATTR(_name, _reg, _field, _table) \ >>> struct afe440x_attr afe440x_attr_##_name = { \ >>> .dev_attr = __ATTR(_name, (S_IRUGO | S_IWUSR), \ >>> afe440x_show_register, \ >>> @@ -183,9 +175,8 @@ struct afe440x_attr { >>> .reg = _reg, \ >>> .shift = _field ## _SHIFT, \ >>> .mask = _field ## _MASK, \ >>> - .type = _type, \ >>> .val_table = _table, \ >>> - .table_size = _size, \ >>> + .table_size = ARRAY_SIZE(_table), \ >>> } >>> >>> #endif /* _AFE440X_H */ >>> >> >-- >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 device with K-9 Mail. Please excuse my brevity. ^ permalink raw reply [flat|nested] 32+ messages in thread
[parent not found: <1462135023-14445-1-git-send-email-afd-l0cyMroinI0@public.gmane.org>]
* [PATCH 03/13] iio: health/afe440x: Remove unneeded initializers [not found] ` <1462135023-14445-1-git-send-email-afd-l0cyMroinI0@public.gmane.org> @ 2016-05-01 20:36 ` Andrew F. Davis 2016-05-04 9:59 ` Jonathan Cameron 2016-05-01 20:36 ` [PATCH 05/13] iio: health/afe440x: Fix scan_index assignment Andrew F. Davis ` (5 subsequent siblings) 6 siblings, 1 reply; 32+ messages in thread From: Andrew F. Davis @ 2016-05-01 20:36 UTC (permalink / raw) To: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA, linux-api-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, Andrew F. Davis The drivers set some register values during initialization that can be set at runtime, these defaults were used in testing but are not necessary, remove these. Signed-off-by: Andrew F. Davis <afd-l0cyMroinI0@public.gmane.org> --- drivers/iio/health/afe4403.c | 7 +------ drivers/iio/health/afe4404.c | 4 ---- 2 files changed, 1 insertion(+), 10 deletions(-) diff --git a/drivers/iio/health/afe4403.c b/drivers/iio/health/afe4403.c index e3700cf..5484785 100644 --- a/drivers/iio/health/afe4403.c +++ b/drivers/iio/health/afe4403.c @@ -472,12 +472,7 @@ static const struct iio_trigger_ops afe4403_trigger_ops = { static const struct reg_sequence afe4403_reg_sequences[] = { AFE4403_TIMING_PAIRS, - { AFE440X_CONTROL1, AFE440X_CONTROL1_TIMEREN | 0x000007}, - { AFE4403_TIA_AMB_GAIN, AFE4403_TIAGAIN_RES_1_M }, - { AFE440X_LEDCNTRL, (0x14 << AFE440X_LEDCNTRL_LED1_SHIFT) | - (0x14 << AFE440X_LEDCNTRL_LED2_SHIFT) }, - { AFE440X_CONTROL2, AFE440X_CONTROL2_TX_REF_050 << - AFE440X_CONTROL2_TX_REF_SHIFT }, + { AFE440X_CONTROL1, AFE440X_CONTROL1_TIMEREN }, }; static const struct regmap_range afe4403_yes_ranges[] = { diff --git a/drivers/iio/health/afe4404.c b/drivers/iio/health/afe4404.c index 783b9b4..2d4c522 100644 --- a/drivers/iio/health/afe4404.c +++ b/drivers/iio/health/afe4404.c @@ -443,10 +443,6 @@ static const struct iio_trigger_ops afe4404_trigger_ops = { static const struct reg_sequence afe4404_reg_sequences[] = { AFE4404_TIMING_PAIRS, { AFE440X_CONTROL1, AFE440X_CONTROL1_TIMEREN }, - { AFE4404_TIA_GAIN, AFE4404_TIA_GAIN_RES_50_K }, - { AFE440X_LEDCNTRL, (0xf << AFE4404_LEDCNTRL_ILED1_SHIFT) | - (0x3 << AFE4404_LEDCNTRL_ILED2_SHIFT) | - (0x3 << AFE4404_LEDCNTRL_ILED3_SHIFT) }, { AFE440X_CONTROL2, AFE440X_CONTROL3_OSC_ENABLE }, }; -- 2.8.1 ^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: [PATCH 03/13] iio: health/afe440x: Remove unneeded initializers 2016-05-01 20:36 ` [PATCH 03/13] iio: health/afe440x: Remove unneeded initializers Andrew F. Davis @ 2016-05-04 9:59 ` Jonathan Cameron 0 siblings, 0 replies; 32+ messages in thread From: Jonathan Cameron @ 2016-05-04 9:59 UTC (permalink / raw) To: Andrew F. Davis, Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald Cc: linux-iio, linux-api, linux-kernel On 01/05/16 21:36, Andrew F. Davis wrote: > The drivers set some register values during initialization that can be > set at runtime, these defaults were used in testing but are not > necessary, remove these. > > Signed-off-by: Andrew F. Davis <afd@ti.com> Fair enough - applied. > --- > drivers/iio/health/afe4403.c | 7 +------ > drivers/iio/health/afe4404.c | 4 ---- > 2 files changed, 1 insertion(+), 10 deletions(-) > > diff --git a/drivers/iio/health/afe4403.c b/drivers/iio/health/afe4403.c > index e3700cf..5484785 100644 > --- a/drivers/iio/health/afe4403.c > +++ b/drivers/iio/health/afe4403.c > @@ -472,12 +472,7 @@ static const struct iio_trigger_ops afe4403_trigger_ops = { > > static const struct reg_sequence afe4403_reg_sequences[] = { > AFE4403_TIMING_PAIRS, > - { AFE440X_CONTROL1, AFE440X_CONTROL1_TIMEREN | 0x000007}, > - { AFE4403_TIA_AMB_GAIN, AFE4403_TIAGAIN_RES_1_M }, > - { AFE440X_LEDCNTRL, (0x14 << AFE440X_LEDCNTRL_LED1_SHIFT) | > - (0x14 << AFE440X_LEDCNTRL_LED2_SHIFT) }, > - { AFE440X_CONTROL2, AFE440X_CONTROL2_TX_REF_050 << > - AFE440X_CONTROL2_TX_REF_SHIFT }, > + { AFE440X_CONTROL1, AFE440X_CONTROL1_TIMEREN }, > }; > > static const struct regmap_range afe4403_yes_ranges[] = { > diff --git a/drivers/iio/health/afe4404.c b/drivers/iio/health/afe4404.c > index 783b9b4..2d4c522 100644 > --- a/drivers/iio/health/afe4404.c > +++ b/drivers/iio/health/afe4404.c > @@ -443,10 +443,6 @@ static const struct iio_trigger_ops afe4404_trigger_ops = { > static const struct reg_sequence afe4404_reg_sequences[] = { > AFE4404_TIMING_PAIRS, > { AFE440X_CONTROL1, AFE440X_CONTROL1_TIMEREN }, > - { AFE4404_TIA_GAIN, AFE4404_TIA_GAIN_RES_50_K }, > - { AFE440X_LEDCNTRL, (0xf << AFE4404_LEDCNTRL_ILED1_SHIFT) | > - (0x3 << AFE4404_LEDCNTRL_ILED2_SHIFT) | > - (0x3 << AFE4404_LEDCNTRL_ILED3_SHIFT) }, > { AFE440X_CONTROL2, AFE440X_CONTROL3_OSC_ENABLE }, > }; > > ^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 05/13] iio: health/afe440x: Fix scan_index assignment [not found] ` <1462135023-14445-1-git-send-email-afd-l0cyMroinI0@public.gmane.org> 2016-05-01 20:36 ` [PATCH 03/13] iio: health/afe440x: Remove unneeded initializers Andrew F. Davis @ 2016-05-01 20:36 ` Andrew F. Davis [not found] ` <1462135023-14445-6-git-send-email-afd-l0cyMroinI0@public.gmane.org> 2016-05-01 20:36 ` [PATCH 08/13] iio: health/afe440x: Remove channel names Andrew F. Davis ` (4 subsequent siblings) 6 siblings, 1 reply; 32+ messages in thread From: Andrew F. Davis @ 2016-05-01 20:36 UTC (permalink / raw) To: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA, linux-api-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, Andrew F. Davis The LED channels are not scannable and so scan_index should be negative, fix this here. Signed-off-by: Andrew F. Davis <afd-l0cyMroinI0@public.gmane.org> --- drivers/iio/health/afe440x.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/iio/health/afe440x.h b/drivers/iio/health/afe440x.h index 544bbab..583d071 100644 --- a/drivers/iio/health/afe440x.h +++ b/drivers/iio/health/afe440x.h @@ -125,7 +125,7 @@ struct afe440x_reg_info { .type = IIO_CURRENT, \ .channel = _index, \ .address = _index, \ - .scan_index = _index, \ + .scan_index = -1, \ .extend_name = _name, \ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \ BIT(IIO_CHAN_INFO_SCALE), \ -- 2.8.1 ^ permalink raw reply related [flat|nested] 32+ messages in thread
[parent not found: <1462135023-14445-6-git-send-email-afd-l0cyMroinI0@public.gmane.org>]
* Re: [PATCH 05/13] iio: health/afe440x: Fix scan_index assignment [not found] ` <1462135023-14445-6-git-send-email-afd-l0cyMroinI0@public.gmane.org> @ 2016-05-04 10:03 ` Jonathan Cameron 0 siblings, 0 replies; 32+ messages in thread From: Jonathan Cameron @ 2016-05-04 10:03 UTC (permalink / raw) To: Andrew F. Davis, Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA, linux-api-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA On 01/05/16 21:36, Andrew F. Davis wrote: > The LED channels are not scannable and so scan_index should > be negative, fix this here. > > Signed-off-by: Andrew F. Davis <afd-l0cyMroinI0@public.gmane.org> Good point. I've applied this to the togreg branch, but if you want me to push this one out as a fix it definitely qualifies. Jonathan > --- > drivers/iio/health/afe440x.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/iio/health/afe440x.h b/drivers/iio/health/afe440x.h > index 544bbab..583d071 100644 > --- a/drivers/iio/health/afe440x.h > +++ b/drivers/iio/health/afe440x.h > @@ -125,7 +125,7 @@ struct afe440x_reg_info { > .type = IIO_CURRENT, \ > .channel = _index, \ > .address = _index, \ > - .scan_index = _index, \ > + .scan_index = -1, \ > .extend_name = _name, \ > .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \ > BIT(IIO_CHAN_INFO_SCALE), \ > ^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 08/13] iio: health/afe440x: Remove channel names [not found] ` <1462135023-14445-1-git-send-email-afd-l0cyMroinI0@public.gmane.org> 2016-05-01 20:36 ` [PATCH 03/13] iio: health/afe440x: Remove unneeded initializers Andrew F. Davis 2016-05-01 20:36 ` [PATCH 05/13] iio: health/afe440x: Fix scan_index assignment Andrew F. Davis @ 2016-05-01 20:36 ` Andrew F. Davis 2016-05-04 10:08 ` Jonathan Cameron 2016-05-01 20:36 ` [PATCH 09/13] iio: health/afe440x: Use regmap fields Andrew F. Davis ` (3 subsequent siblings) 6 siblings, 1 reply; 32+ messages in thread From: Andrew F. Davis @ 2016-05-01 20:36 UTC (permalink / raw) To: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA, linux-api-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, Andrew F. Davis 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-l0cyMroinI0@public.gmane.org> --- .../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-l0cyMroinI0@public.gmane.org> 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-l0cyMroinI0@public.gmane.org> -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-l0cyMroinI0@public.gmane.org> 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-l0cyMroinI0@public.gmane.org> 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 ^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: [PATCH 08/13] iio: health/afe440x: Remove channel names 2016-05-01 20:36 ` [PATCH 08/13] iio: health/afe440x: Remove channel names Andrew F. Davis @ 2016-05-04 10:08 ` Jonathan Cameron [not found] ` <f0679a4f-c8f0-1712-e7fc-a61350734a03-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> 0 siblings, 1 reply; 32+ messages in thread From: Jonathan Cameron @ 2016-05-04 10:08 UTC (permalink / raw) To: Andrew F. Davis, Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald Cc: linux-iio, linux-api, linux-kernel 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, \ > } > > ^ permalink raw reply [flat|nested] 32+ messages in thread
[parent not found: <f0679a4f-c8f0-1712-e7fc-a61350734a03-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>]
* Re: [PATCH 08/13] iio: health/afe440x: Remove channel names [not found] ` <f0679a4f-c8f0-1712-e7fc-a61350734a03-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> @ 2016-05-04 15:28 ` Andrew F. Davis 0 siblings, 0 replies; 32+ messages in thread From: Andrew F. Davis @ 2016-05-04 15:28 UTC (permalink / raw) To: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA, linux-api-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA 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-l0cyMroinI0@public.gmane.org> > 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-l0cyMroinI0@public.gmane.org> >> 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-l0cyMroinI0@public.gmane.org> >> -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-l0cyMroinI0@public.gmane.org> >> 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-l0cyMroinI0@public.gmane.org> >> 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, \ >> } >> >> > ^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 09/13] iio: health/afe440x: Use regmap fields [not found] ` <1462135023-14445-1-git-send-email-afd-l0cyMroinI0@public.gmane.org> ` (2 preceding siblings ...) 2016-05-01 20:36 ` [PATCH 08/13] iio: health/afe440x: Remove channel names Andrew F. Davis @ 2016-05-01 20:36 ` Andrew F. Davis 2016-05-04 10:10 ` Jonathan Cameron 2016-05-01 20:37 ` [PATCH 11/13] iio: health/afe440x: Match LED currents to stages Andrew F. Davis ` (2 subsequent siblings) 6 siblings, 1 reply; 32+ messages in thread From: Andrew F. Davis @ 2016-05-01 20:36 UTC (permalink / raw) To: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA, linux-api-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, Andrew F. Davis These drivers can use regmap fields to access fields in registers, this allows us to remove some macros/defines and simplify code, do this here. Signed-off-by: Andrew F. Davis <afd-l0cyMroinI0@public.gmane.org> --- drivers/iio/health/afe4403.c | 118 ++++++++++++++++---------------- drivers/iio/health/afe4404.c | 156 ++++++++++++++++++++++--------------------- drivers/iio/health/afe440x.h | 25 +------ 3 files changed, 144 insertions(+), 155 deletions(-) diff --git a/drivers/iio/health/afe4403.c b/drivers/iio/health/afe4403.c index 4a58064..1950155 100644 --- a/drivers/iio/health/afe4403.c +++ b/drivers/iio/health/afe4403.c @@ -39,32 +39,6 @@ #define AFE4403_TIAGAIN 0x20 #define AFE4403_TIA_AMB_GAIN 0x21 -/* AFE4403 GAIN register fields */ -#define AFE4403_TIAGAIN_RES_MASK GENMASK(2, 0) -#define AFE4403_TIAGAIN_RES_SHIFT 0 -#define AFE4403_TIAGAIN_CAP_MASK GENMASK(7, 3) -#define AFE4403_TIAGAIN_CAP_SHIFT 3 - -/* AFE4403 LEDCNTRL register fields */ -#define AFE440X_LEDCNTRL_LED1_MASK GENMASK(15, 8) -#define AFE440X_LEDCNTRL_LED1_SHIFT 8 -#define AFE440X_LEDCNTRL_LED2_MASK GENMASK(7, 0) -#define AFE440X_LEDCNTRL_LED2_SHIFT 0 -#define AFE440X_LEDCNTRL_LED_RANGE_MASK GENMASK(17, 16) -#define AFE440X_LEDCNTRL_LED_RANGE_SHIFT 16 - -/* AFE4403 CONTROL2 register fields */ -#define AFE440X_CONTROL2_PWR_DWN_TX BIT(2) -#define AFE440X_CONTROL2_EN_SLOW_DIAG BIT(8) -#define AFE440X_CONTROL2_DIAG_OUT_TRI BIT(10) -#define AFE440X_CONTROL2_TX_BRDG_MOD BIT(11) -#define AFE440X_CONTROL2_TX_REF_MASK GENMASK(18, 17) -#define AFE440X_CONTROL2_TX_REF_SHIFT 17 - -/* AFE4404 NULL fields */ -#define NULL_MASK 0 -#define NULL_SHIFT 0 - /* AFE4403 LEDCNTRL values */ #define AFE440X_LEDCNTRL_RANGE_TX_HALF 0x1 #define AFE440X_LEDCNTRL_RANGE_TX_FULL 0x2 @@ -102,11 +76,35 @@ #define AFE4403_TIAGAIN_RES_1_M 0x6 #define AFE4403_TIAGAIN_RES_NONE 0x7 +enum afe4403_fields { + /* Gains */ + F_RF_LED1, F_CF_LED1, + F_RF_LED, F_CF_LED, + + /* LED Current */ + F_ILED1, F_ILED2, + + /* sentinel */ + F_MAX_FIELDS +}; + +static const struct reg_field afe4403_reg_fields[] = { + /* Gains */ + [F_RF_LED1] = REG_FIELD(AFE4403_TIAGAIN, 0, 2), + [F_CF_LED1] = REG_FIELD(AFE4403_TIAGAIN, 3, 7), + [F_RF_LED] = REG_FIELD(AFE4403_TIA_AMB_GAIN, 0, 2), + [F_CF_LED] = REG_FIELD(AFE4403_TIA_AMB_GAIN, 3, 7), + /* LED Current */ + [F_ILED1] = REG_FIELD(AFE440X_LEDCNTRL, 0, 7), + [F_ILED2] = REG_FIELD(AFE440X_LEDCNTRL, 8, 15), +}; + /** * struct afe4403_data - AFE4403 device instance data * @dev: Device structure * @spi: SPI device handle * @regmap: Register map of the device + * @fields: Register fields of the device * @regulator: Pointer to the regulator for the IC * @trig: IIO trigger for this device * @irq: ADC_RDY line interrupt number @@ -115,6 +113,7 @@ struct afe4403_data { struct device *dev; struct spi_device *spi; struct regmap *regmap; + struct regmap_field *fields[F_MAX_FIELDS]; struct regulator *regulator; struct iio_trigger *trig; int irq; @@ -131,15 +130,18 @@ enum afe4403_chan_id { ILED2, }; -static const struct afe440x_reg_info afe4403_reg_info[] = { - [LED2] = AFE440X_REG_INFO(AFE440X_LED2VAL, 0, NULL), - [ALED2] = AFE440X_REG_INFO(AFE440X_ALED2VAL, 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 unsigned int afe4403_channel_values[] = { + [LED2] = AFE440X_LED2VAL, + [ALED2] = AFE440X_ALED2VAL, + [LED1] = AFE440X_LED1VAL, + [ALED1] = AFE440X_ALED1VAL, + [LED2_ALED2] = AFE440X_LED2_ALED2VAL, + [LED1_ALED1] = AFE440X_LED1_ALED1VAL, +}; + +static const unsigned int afe4403_channel_leds[] = { + [ILED1] = F_ILED1, + [ILED2] = F_ILED2, }; static const struct iio_chan_spec afe4403_channels[] = { @@ -184,13 +186,10 @@ static ssize_t afe440x_show_register(struct device *dev, int vals[2]; int ret; - ret = regmap_read(afe->regmap, afe440x_attr->reg, ®_val); + ret = regmap_field_read(afe->fields[afe440x_attr->field], ®_val); if (ret) return ret; - reg_val &= afe440x_attr->mask; - reg_val >>= afe440x_attr->shift; - if (reg_val >= afe440x_attr->table_size) return -EINVAL; @@ -220,20 +219,18 @@ static ssize_t afe440x_store_register(struct device *dev, if (val == afe440x_attr->table_size) return -EINVAL; - ret = regmap_update_bits(afe->regmap, afe440x_attr->reg, - afe440x_attr->mask, - (val << afe440x_attr->shift)); + ret = regmap_field_write(afe->fields[afe440x_attr->field], val); if (ret) return ret; return count; } -static AFE440X_ATTR(tia_resistance1, AFE4403_TIAGAIN, AFE4403_TIAGAIN_RES, afe4403_res_table); -static AFE440X_ATTR(tia_capacitance1, AFE4403_TIAGAIN, AFE4403_TIAGAIN_CAP, afe4403_cap_table); +static AFE440X_ATTR(tia_resistance1, F_RF_LED1, afe4403_res_table); +static AFE440X_ATTR(tia_capacitance1, F_CF_LED1, afe4403_cap_table); -static AFE440X_ATTR(tia_resistance2, AFE4403_TIA_AMB_GAIN, AFE4403_TIAGAIN_RES, afe4403_res_table); -static AFE440X_ATTR(tia_capacitance2, AFE4403_TIA_AMB_GAIN, AFE4403_TIAGAIN_RES, afe4403_cap_table); +static AFE440X_ATTR(tia_resistance2, F_RF_LED, afe4403_res_table); +static AFE440X_ATTR(tia_capacitance2, F_CF_LED, afe4403_cap_table); static struct attribute *afe440x_attributes[] = { &afe440x_attr_tia_resistance1.dev_attr.attr, @@ -282,14 +279,15 @@ static int afe4403_read_raw(struct iio_dev *indio_dev, int *val, int *val2, long mask) { struct afe4403_data *afe = iio_priv(indio_dev); - const struct afe440x_reg_info reg_info = afe4403_reg_info[chan->address]; + unsigned int reg = afe4403_channel_values[chan->address]; + unsigned int field = afe4403_channel_leds[chan->address]; int ret; switch (chan->type) { case IIO_INTENSITY: switch (mask) { case IIO_CHAN_INFO_RAW: - ret = afe4403_read(afe, reg_info.reg, val); + ret = afe4403_read(afe, reg, val); if (ret) return ret; return IIO_VAL_INT; @@ -298,11 +296,9 @@ static int afe4403_read_raw(struct iio_dev *indio_dev, case IIO_CURRENT: switch (mask) { case IIO_CHAN_INFO_RAW: - ret = regmap_read(afe->regmap, reg_info.reg, val); + ret = regmap_field_read(afe->fields[field], val); if (ret) return ret; - *val &= reg_info.mask; - *val >>= reg_info.shift; return IIO_VAL_INT; case IIO_CHAN_INFO_SCALE: *val = 0; @@ -322,16 +318,13 @@ static int afe4403_write_raw(struct iio_dev *indio_dev, int val, int val2, long mask) { struct afe4403_data *afe = iio_priv(indio_dev); - const struct afe440x_reg_info reg_info = afe4403_reg_info[chan->address]; + unsigned int field = afe4403_channel_leds[chan->address]; switch (chan->type) { case IIO_CURRENT: switch (mask) { case IIO_CHAN_INFO_RAW: - return regmap_update_bits(afe->regmap, - reg_info.reg, - reg_info.mask, - (val << reg_info.shift)); + return regmap_field_write(afe->fields[field], val); } break; default: @@ -366,7 +359,7 @@ static irqreturn_t afe4403_trigger_handler(int irq, void *private) for_each_set_bit(bit, indio_dev->active_scan_mask, indio_dev->masklength) { ret = spi_write_then_read(afe->spi, - &afe4403_reg_info[bit].reg, 1, + &afe4403_channel_values[bit], 1, rx, 3); if (ret) goto err; @@ -503,7 +496,7 @@ static int afe4403_probe(struct spi_device *spi) { struct iio_dev *indio_dev; struct afe4403_data *afe; - int ret; + int i, ret; indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*afe)); if (!indio_dev) @@ -522,6 +515,15 @@ static int afe4403_probe(struct spi_device *spi) return PTR_ERR(afe->regmap); } + for (i = 0; i < F_MAX_FIELDS; i++) { + afe->fields[i] = devm_regmap_field_alloc(afe->dev, afe->regmap, + afe4403_reg_fields[i]); + if (IS_ERR(afe->fields[i])) { + dev_err(afe->dev, "Unable to allocate regmap fields\n"); + return PTR_ERR(afe->fields[i]); + } + } + afe->regulator = devm_regulator_get(afe->dev, "tx_sup"); if (IS_ERR(afe->regulator)) { dev_err(afe->dev, "Unable to get regulator\n"); diff --git a/drivers/iio/health/afe4404.c b/drivers/iio/health/afe4404.c index 7806a45..0d1af4a 100644 --- a/drivers/iio/health/afe4404.c +++ b/drivers/iio/health/afe4404.c @@ -48,43 +48,9 @@ #define AFE4404_AVG_LED2_ALED2VAL 0x3f #define AFE4404_AVG_LED1_ALED1VAL 0x40 -/* AFE4404 GAIN register fields */ -#define AFE4404_TIA_GAIN_RES_MASK GENMASK(2, 0) -#define AFE4404_TIA_GAIN_RES_SHIFT 0 -#define AFE4404_TIA_GAIN_CAP_MASK GENMASK(5, 3) -#define AFE4404_TIA_GAIN_CAP_SHIFT 3 - -/* AFE4404 LEDCNTRL register fields */ -#define AFE4404_LEDCNTRL_ILED1_MASK GENMASK(5, 0) -#define AFE4404_LEDCNTRL_ILED1_SHIFT 0 -#define AFE4404_LEDCNTRL_ILED2_MASK GENMASK(11, 6) -#define AFE4404_LEDCNTRL_ILED2_SHIFT 6 -#define AFE4404_LEDCNTRL_ILED3_MASK GENMASK(17, 12) -#define AFE4404_LEDCNTRL_ILED3_SHIFT 12 - -/* AFE4404 CONTROL2 register fields */ -#define AFE440X_CONTROL2_ILED_2X_MASK BIT(17) -#define AFE440X_CONTROL2_ILED_2X_SHIFT 17 - /* AFE4404 CONTROL3 register fields */ #define AFE440X_CONTROL3_OSC_ENABLE BIT(9) -/* AFE4404 OFFDAC register current fields */ -#define AFE4404_OFFDAC_CURR_LED1_MASK GENMASK(9, 5) -#define AFE4404_OFFDAC_CURR_LED1_SHIFT 5 -#define AFE4404_OFFDAC_CURR_LED2_MASK GENMASK(19, 15) -#define AFE4404_OFFDAC_CURR_LED2_SHIFT 15 -#define AFE4404_OFFDAC_CURR_LED3_MASK GENMASK(4, 0) -#define AFE4404_OFFDAC_CURR_LED3_SHIFT 0 -#define AFE4404_OFFDAC_CURR_ALED1_MASK GENMASK(14, 10) -#define AFE4404_OFFDAC_CURR_ALED1_SHIFT 10 -#define AFE4404_OFFDAC_CURR_ALED2_MASK GENMASK(4, 0) -#define AFE4404_OFFDAC_CURR_ALED2_SHIFT 0 - -/* AFE4404 NULL fields */ -#define NULL_MASK 0 -#define NULL_SHIFT 0 - /* AFE4404 TIA_GAIN_CAP values */ #define AFE4404_TIA_GAIN_CAP_5_P 0x0 #define AFE4404_TIA_GAIN_CAP_2_5_P 0x1 @@ -105,10 +71,43 @@ #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, + F_TIA_GAIN, TIA_CF, + + /* LED Current */ + F_ILED1, F_ILED2, F_ILED3, + + /* Offset DAC */ + F_OFFDAC_AMB2, F_OFFDAC_LED1, F_OFFDAC_AMB1, F_OFFDAC_LED2, + + /* sentinel */ + F_MAX_FIELDS +}; + +static const struct reg_field afe4404_reg_fields[] = { + /* Gains */ + [F_TIA_GAIN_SEP] = REG_FIELD(AFE4404_TIA_GAIN_SEP, 0, 2), + [F_TIA_CF_SEP] = REG_FIELD(AFE4404_TIA_GAIN_SEP, 3, 5), + [F_TIA_GAIN] = REG_FIELD(AFE4404_TIA_GAIN, 0, 2), + [TIA_CF] = REG_FIELD(AFE4404_TIA_GAIN, 3, 5), + /* LED Current */ + [F_ILED1] = REG_FIELD(AFE440X_LEDCNTRL, 0, 5), + [F_ILED2] = REG_FIELD(AFE440X_LEDCNTRL, 6, 11), + [F_ILED3] = REG_FIELD(AFE440X_LEDCNTRL, 12, 17), + /* Offset DAC */ + [F_OFFDAC_AMB2] = REG_FIELD(AFE4404_OFFDAC, 0, 4), + [F_OFFDAC_LED1] = REG_FIELD(AFE4404_OFFDAC, 5, 9), + [F_OFFDAC_AMB1] = REG_FIELD(AFE4404_OFFDAC, 10, 14), + [F_OFFDAC_LED2] = REG_FIELD(AFE4404_OFFDAC, 15, 19), +}; + /** * struct afe4404_data - AFE4404 device instance data * @dev: Device structure * @regmap: Register map of the device + * @fields: Register fields of the device * @regulator: Pointer to the regulator for the IC * @trig: IIO trigger for this device * @irq: ADC_RDY line interrupt number @@ -116,6 +115,7 @@ struct afe4404_data { struct device *dev; struct regmap *regmap; + struct regmap_field *fields[F_MAX_FIELDS]; struct regulator *regulator; struct iio_trigger *trig; int irq; @@ -133,16 +133,26 @@ enum afe4404_chan_id { ILED3, }; -static const struct afe440x_reg_info afe4404_reg_info[] = { - [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] = 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), +static const unsigned int afe4404_channel_values[] = { + [LED2] = AFE440X_LED2VAL, + [ALED2] = AFE440X_ALED2VAL, + [LED1] = AFE440X_LED1VAL, + [ALED1] = AFE440X_ALED1VAL, + [LED2_ALED2] = AFE440X_LED2_ALED2VAL, + [LED1_ALED1] = AFE440X_LED1_ALED1VAL, +}; + +static const unsigned int afe4404_channel_leds[] = { + [ILED1] = F_ILED1, + [ILED2] = F_ILED2, + [ILED3] = F_ILED3, +}; + +static const unsigned int afe4404_channel_offdacs[] = { + [LED2] = F_OFFDAC_LED2, + [ALED2] = F_OFFDAC_AMB2, + [LED1] = F_OFFDAC_LED1, + [ALED1] = F_OFFDAC_AMB1, }; static const struct iio_chan_spec afe4404_channels[] = { @@ -194,13 +204,10 @@ static ssize_t afe440x_show_register(struct device *dev, int vals[2]; int ret; - ret = regmap_read(afe->regmap, afe440x_attr->reg, ®_val); + ret = regmap_field_read(afe->fields[afe440x_attr->field], ®_val); if (ret) return ret; - reg_val &= afe440x_attr->mask; - reg_val >>= afe440x_attr->shift; - if (reg_val >= afe440x_attr->table_size) return -EINVAL; @@ -230,20 +237,18 @@ static ssize_t afe440x_store_register(struct device *dev, if (val == afe440x_attr->table_size) return -EINVAL; - ret = regmap_update_bits(afe->regmap, afe440x_attr->reg, - afe440x_attr->mask, - (val << afe440x_attr->shift)); + ret = regmap_field_write(afe->fields[afe440x_attr->field], val); if (ret) return ret; return count; } -static AFE440X_ATTR(tia_resistance1, AFE4404_TIA_GAIN, AFE4404_TIA_GAIN_RES, afe4404_res_table); -static AFE440X_ATTR(tia_capacitance1, AFE4404_TIA_GAIN, AFE4404_TIA_GAIN_CAP, afe4404_cap_table); +static AFE440X_ATTR(tia_resistance1, F_TIA_GAIN, afe4404_res_table); +static AFE440X_ATTR(tia_capacitance1, TIA_CF, afe4404_cap_table); -static AFE440X_ATTR(tia_resistance2, AFE4404_TIA_GAIN_SEP, AFE4404_TIA_GAIN_RES, afe4404_res_table); -static AFE440X_ATTR(tia_capacitance2, AFE4404_TIA_GAIN_SEP, AFE4404_TIA_GAIN_CAP, afe4404_cap_table); +static AFE440X_ATTR(tia_resistance2, F_TIA_GAIN_SEP, afe4404_res_table); +static AFE440X_ATTR(tia_capacitance2, F_TIA_CF_SEP, afe4404_cap_table); static struct attribute *afe440x_attributes[] = { &afe440x_attr_tia_resistance1.dev_attr.attr, @@ -264,35 +269,32 @@ static int afe4404_read_raw(struct iio_dev *indio_dev, int *val, int *val2, long mask) { struct afe4404_data *afe = iio_priv(indio_dev); - const struct afe440x_reg_info reg_info = afe4404_reg_info[chan->address]; + unsigned int value_reg = afe4404_channel_values[chan->address]; + unsigned int led_field = afe4404_channel_leds[chan->address]; + unsigned int offdac_field = afe4404_channel_offdacs[chan->address]; int ret; switch (chan->type) { case IIO_INTENSITY: switch (mask) { case IIO_CHAN_INFO_RAW: - ret = regmap_read(afe->regmap, reg_info.reg, val); + ret = regmap_read(afe->regmap, value_reg, val); if (ret) return ret; return IIO_VAL_INT; case IIO_CHAN_INFO_OFFSET: - ret = regmap_read(afe->regmap, reg_info.offreg, - val); + ret = regmap_field_read(afe->fields[offdac_field], val); if (ret) return ret; - *val &= reg_info.mask; - *val >>= reg_info.shift; return IIO_VAL_INT; } break; case IIO_CURRENT: switch (mask) { case IIO_CHAN_INFO_RAW: - ret = regmap_read(afe->regmap, reg_info.reg, val); + ret = regmap_field_read(afe->fields[led_field], val); if (ret) return ret; - *val &= reg_info.mask; - *val >>= reg_info.shift; return IIO_VAL_INT; case IIO_CHAN_INFO_SCALE: *val = 0; @@ -312,25 +314,20 @@ static int afe4404_write_raw(struct iio_dev *indio_dev, int val, int val2, long mask) { struct afe4404_data *afe = iio_priv(indio_dev); - const struct afe440x_reg_info reg_info = afe4404_reg_info[chan->address]; + unsigned int led_field = afe4404_channel_leds[chan->address]; + unsigned int offdac_field = afe4404_channel_offdacs[chan->address]; switch (chan->type) { case IIO_INTENSITY: switch (mask) { case IIO_CHAN_INFO_OFFSET: - return regmap_update_bits(afe->regmap, - reg_info.offreg, - reg_info.mask, - (val << reg_info.shift)); + return regmap_field_write(afe->fields[offdac_field], val); } break; case IIO_CURRENT: switch (mask) { case IIO_CHAN_INFO_RAW: - return regmap_update_bits(afe->regmap, - reg_info.reg, - reg_info.mask, - (val << reg_info.shift)); + return regmap_field_write(afe->fields[led_field], val); } break; default: @@ -357,7 +354,7 @@ static irqreturn_t afe4404_trigger_handler(int irq, void *private) for_each_set_bit(bit, indio_dev->active_scan_mask, indio_dev->masklength) { - ret = regmap_read(afe->regmap, afe4404_reg_info[bit].reg, + ret = regmap_read(afe->regmap, afe4404_channel_values[bit], &buffer[i++]); if (ret) goto err; @@ -490,7 +487,7 @@ static int afe4404_probe(struct i2c_client *client, { struct iio_dev *indio_dev; struct afe4404_data *afe; - int ret; + int i, ret; indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*afe)); if (!indio_dev) @@ -508,6 +505,15 @@ static int afe4404_probe(struct i2c_client *client, return PTR_ERR(afe->regmap); } + for (i = 0; i < F_MAX_FIELDS; i++) { + afe->fields[i] = devm_regmap_field_alloc(afe->dev, afe->regmap, + afe4404_reg_fields[i]); + if (IS_ERR(afe->fields[i])) { + dev_err(afe->dev, "Unable to allocate regmap fields\n"); + return PTR_ERR(afe->fields[i]); + } + } + afe->regulator = devm_regulator_get(afe->dev, "tx_sup"); if (IS_ERR(afe->regulator)) { dev_err(afe->dev, "Unable to get regulator\n"); diff --git a/drivers/iio/health/afe440x.h b/drivers/iio/health/afe440x.h index 713972f..1a0f247 100644 --- a/drivers/iio/health/afe440x.h +++ b/drivers/iio/health/afe440x.h @@ -88,21 +88,6 @@ #define AFE440X_CONTROL0_WRITE 0x0 #define AFE440X_CONTROL0_READ 0x1 -struct afe440x_reg_info { - unsigned int reg; - unsigned int offreg; - unsigned int shift; - unsigned int mask; -}; - -#define AFE440X_REG_INFO(_reg, _offreg, _sm) \ - { \ - .reg = _reg, \ - .offreg = _offreg, \ - .shift = _sm ## _SHIFT, \ - .mask = _sm ## _MASK, \ - } - #define AFE440X_INTENSITY_CHAN(_index, _mask) \ { \ .type = IIO_INTENSITY, \ @@ -157,9 +142,7 @@ static DEVICE_ATTR_RO(_name) struct afe440x_attr { struct device_attribute dev_attr; - unsigned int reg; - unsigned int shift; - unsigned int mask; + unsigned int field; const struct afe440x_val_table *val_table; unsigned int table_size; }; @@ -167,14 +150,12 @@ struct afe440x_attr { #define to_afe440x_attr(_dev_attr) \ container_of(_dev_attr, struct afe440x_attr, dev_attr) -#define AFE440X_ATTR(_name, _reg, _field, _table) \ +#define AFE440X_ATTR(_name, _field, _table) \ struct afe440x_attr afe440x_attr_##_name = { \ .dev_attr = __ATTR(_name, (S_IRUGO | S_IWUSR), \ afe440x_show_register, \ afe440x_store_register), \ - .reg = _reg, \ - .shift = _field ## _SHIFT, \ - .mask = _field ## _MASK, \ + .field = _field, \ .val_table = _table, \ .table_size = ARRAY_SIZE(_table), \ } -- 2.8.1 ^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: [PATCH 09/13] iio: health/afe440x: Use regmap fields 2016-05-01 20:36 ` [PATCH 09/13] iio: health/afe440x: Use regmap fields Andrew F. Davis @ 2016-05-04 10:10 ` Jonathan Cameron [not found] ` <45523060-7ae8-076f-dece-34e76e8a740a-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> 0 siblings, 1 reply; 32+ messages in thread From: Jonathan Cameron @ 2016-05-04 10:10 UTC (permalink / raw) To: Andrew F. Davis, Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald Cc: linux-iio, linux-api, linux-kernel On 01/05/16 21:36, Andrew F. Davis wrote: > These drivers can use regmap fields to access fields in registers, this > allows us to remove some macros/defines and simplify code, do this here. > > Signed-off-by: Andrew F. Davis <afd@ti.com> This could almost be a poster boy for why regmap field stuff is good ;) So often it ends up more marginal than this. Applied. Jonathan > --- > drivers/iio/health/afe4403.c | 118 ++++++++++++++++---------------- > drivers/iio/health/afe4404.c | 156 ++++++++++++++++++++++--------------------- > drivers/iio/health/afe440x.h | 25 +------ > 3 files changed, 144 insertions(+), 155 deletions(-) > > diff --git a/drivers/iio/health/afe4403.c b/drivers/iio/health/afe4403.c > index 4a58064..1950155 100644 > --- a/drivers/iio/health/afe4403.c > +++ b/drivers/iio/health/afe4403.c > @@ -39,32 +39,6 @@ > #define AFE4403_TIAGAIN 0x20 > #define AFE4403_TIA_AMB_GAIN 0x21 > > -/* AFE4403 GAIN register fields */ > -#define AFE4403_TIAGAIN_RES_MASK GENMASK(2, 0) > -#define AFE4403_TIAGAIN_RES_SHIFT 0 > -#define AFE4403_TIAGAIN_CAP_MASK GENMASK(7, 3) > -#define AFE4403_TIAGAIN_CAP_SHIFT 3 > - > -/* AFE4403 LEDCNTRL register fields */ > -#define AFE440X_LEDCNTRL_LED1_MASK GENMASK(15, 8) > -#define AFE440X_LEDCNTRL_LED1_SHIFT 8 > -#define AFE440X_LEDCNTRL_LED2_MASK GENMASK(7, 0) > -#define AFE440X_LEDCNTRL_LED2_SHIFT 0 > -#define AFE440X_LEDCNTRL_LED_RANGE_MASK GENMASK(17, 16) > -#define AFE440X_LEDCNTRL_LED_RANGE_SHIFT 16 > - > -/* AFE4403 CONTROL2 register fields */ > -#define AFE440X_CONTROL2_PWR_DWN_TX BIT(2) > -#define AFE440X_CONTROL2_EN_SLOW_DIAG BIT(8) > -#define AFE440X_CONTROL2_DIAG_OUT_TRI BIT(10) > -#define AFE440X_CONTROL2_TX_BRDG_MOD BIT(11) > -#define AFE440X_CONTROL2_TX_REF_MASK GENMASK(18, 17) > -#define AFE440X_CONTROL2_TX_REF_SHIFT 17 > - > -/* AFE4404 NULL fields */ > -#define NULL_MASK 0 > -#define NULL_SHIFT 0 > - > /* AFE4403 LEDCNTRL values */ > #define AFE440X_LEDCNTRL_RANGE_TX_HALF 0x1 > #define AFE440X_LEDCNTRL_RANGE_TX_FULL 0x2 > @@ -102,11 +76,35 @@ > #define AFE4403_TIAGAIN_RES_1_M 0x6 > #define AFE4403_TIAGAIN_RES_NONE 0x7 > > +enum afe4403_fields { > + /* Gains */ > + F_RF_LED1, F_CF_LED1, > + F_RF_LED, F_CF_LED, > + > + /* LED Current */ > + F_ILED1, F_ILED2, > + > + /* sentinel */ > + F_MAX_FIELDS > +}; > + > +static const struct reg_field afe4403_reg_fields[] = { > + /* Gains */ > + [F_RF_LED1] = REG_FIELD(AFE4403_TIAGAIN, 0, 2), > + [F_CF_LED1] = REG_FIELD(AFE4403_TIAGAIN, 3, 7), > + [F_RF_LED] = REG_FIELD(AFE4403_TIA_AMB_GAIN, 0, 2), > + [F_CF_LED] = REG_FIELD(AFE4403_TIA_AMB_GAIN, 3, 7), > + /* LED Current */ > + [F_ILED1] = REG_FIELD(AFE440X_LEDCNTRL, 0, 7), > + [F_ILED2] = REG_FIELD(AFE440X_LEDCNTRL, 8, 15), > +}; > + > /** > * struct afe4403_data - AFE4403 device instance data > * @dev: Device structure > * @spi: SPI device handle > * @regmap: Register map of the device > + * @fields: Register fields of the device > * @regulator: Pointer to the regulator for the IC > * @trig: IIO trigger for this device > * @irq: ADC_RDY line interrupt number > @@ -115,6 +113,7 @@ struct afe4403_data { > struct device *dev; > struct spi_device *spi; > struct regmap *regmap; > + struct regmap_field *fields[F_MAX_FIELDS]; > struct regulator *regulator; > struct iio_trigger *trig; > int irq; > @@ -131,15 +130,18 @@ enum afe4403_chan_id { > ILED2, > }; > > -static const struct afe440x_reg_info afe4403_reg_info[] = { > - [LED2] = AFE440X_REG_INFO(AFE440X_LED2VAL, 0, NULL), > - [ALED2] = AFE440X_REG_INFO(AFE440X_ALED2VAL, 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 unsigned int afe4403_channel_values[] = { > + [LED2] = AFE440X_LED2VAL, > + [ALED2] = AFE440X_ALED2VAL, > + [LED1] = AFE440X_LED1VAL, > + [ALED1] = AFE440X_ALED1VAL, > + [LED2_ALED2] = AFE440X_LED2_ALED2VAL, > + [LED1_ALED1] = AFE440X_LED1_ALED1VAL, > +}; > + > +static const unsigned int afe4403_channel_leds[] = { > + [ILED1] = F_ILED1, > + [ILED2] = F_ILED2, > }; > > static const struct iio_chan_spec afe4403_channels[] = { > @@ -184,13 +186,10 @@ static ssize_t afe440x_show_register(struct device *dev, > int vals[2]; > int ret; > > - ret = regmap_read(afe->regmap, afe440x_attr->reg, ®_val); > + ret = regmap_field_read(afe->fields[afe440x_attr->field], ®_val); > if (ret) > return ret; > > - reg_val &= afe440x_attr->mask; > - reg_val >>= afe440x_attr->shift; > - > if (reg_val >= afe440x_attr->table_size) > return -EINVAL; > > @@ -220,20 +219,18 @@ static ssize_t afe440x_store_register(struct device *dev, > if (val == afe440x_attr->table_size) > return -EINVAL; > > - ret = regmap_update_bits(afe->regmap, afe440x_attr->reg, > - afe440x_attr->mask, > - (val << afe440x_attr->shift)); > + ret = regmap_field_write(afe->fields[afe440x_attr->field], val); > if (ret) > return ret; > > return count; > } > > -static AFE440X_ATTR(tia_resistance1, AFE4403_TIAGAIN, AFE4403_TIAGAIN_RES, afe4403_res_table); > -static AFE440X_ATTR(tia_capacitance1, AFE4403_TIAGAIN, AFE4403_TIAGAIN_CAP, afe4403_cap_table); > +static AFE440X_ATTR(tia_resistance1, F_RF_LED1, afe4403_res_table); > +static AFE440X_ATTR(tia_capacitance1, F_CF_LED1, afe4403_cap_table); > > -static AFE440X_ATTR(tia_resistance2, AFE4403_TIA_AMB_GAIN, AFE4403_TIAGAIN_RES, afe4403_res_table); > -static AFE440X_ATTR(tia_capacitance2, AFE4403_TIA_AMB_GAIN, AFE4403_TIAGAIN_RES, afe4403_cap_table); > +static AFE440X_ATTR(tia_resistance2, F_RF_LED, afe4403_res_table); > +static AFE440X_ATTR(tia_capacitance2, F_CF_LED, afe4403_cap_table); > > static struct attribute *afe440x_attributes[] = { > &afe440x_attr_tia_resistance1.dev_attr.attr, > @@ -282,14 +279,15 @@ static int afe4403_read_raw(struct iio_dev *indio_dev, > int *val, int *val2, long mask) > { > struct afe4403_data *afe = iio_priv(indio_dev); > - const struct afe440x_reg_info reg_info = afe4403_reg_info[chan->address]; > + unsigned int reg = afe4403_channel_values[chan->address]; > + unsigned int field = afe4403_channel_leds[chan->address]; > int ret; > > switch (chan->type) { > case IIO_INTENSITY: > switch (mask) { > case IIO_CHAN_INFO_RAW: > - ret = afe4403_read(afe, reg_info.reg, val); > + ret = afe4403_read(afe, reg, val); > if (ret) > return ret; > return IIO_VAL_INT; > @@ -298,11 +296,9 @@ static int afe4403_read_raw(struct iio_dev *indio_dev, > case IIO_CURRENT: > switch (mask) { > case IIO_CHAN_INFO_RAW: > - ret = regmap_read(afe->regmap, reg_info.reg, val); > + ret = regmap_field_read(afe->fields[field], val); > if (ret) > return ret; > - *val &= reg_info.mask; > - *val >>= reg_info.shift; > return IIO_VAL_INT; > case IIO_CHAN_INFO_SCALE: > *val = 0; > @@ -322,16 +318,13 @@ static int afe4403_write_raw(struct iio_dev *indio_dev, > int val, int val2, long mask) > { > struct afe4403_data *afe = iio_priv(indio_dev); > - const struct afe440x_reg_info reg_info = afe4403_reg_info[chan->address]; > + unsigned int field = afe4403_channel_leds[chan->address]; > > switch (chan->type) { > case IIO_CURRENT: > switch (mask) { > case IIO_CHAN_INFO_RAW: > - return regmap_update_bits(afe->regmap, > - reg_info.reg, > - reg_info.mask, > - (val << reg_info.shift)); > + return regmap_field_write(afe->fields[field], val); > } > break; > default: > @@ -366,7 +359,7 @@ static irqreturn_t afe4403_trigger_handler(int irq, void *private) > for_each_set_bit(bit, indio_dev->active_scan_mask, > indio_dev->masklength) { > ret = spi_write_then_read(afe->spi, > - &afe4403_reg_info[bit].reg, 1, > + &afe4403_channel_values[bit], 1, > rx, 3); > if (ret) > goto err; > @@ -503,7 +496,7 @@ static int afe4403_probe(struct spi_device *spi) > { > struct iio_dev *indio_dev; > struct afe4403_data *afe; > - int ret; > + int i, ret; > > indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*afe)); > if (!indio_dev) > @@ -522,6 +515,15 @@ static int afe4403_probe(struct spi_device *spi) > return PTR_ERR(afe->regmap); > } > > + for (i = 0; i < F_MAX_FIELDS; i++) { > + afe->fields[i] = devm_regmap_field_alloc(afe->dev, afe->regmap, > + afe4403_reg_fields[i]); > + if (IS_ERR(afe->fields[i])) { > + dev_err(afe->dev, "Unable to allocate regmap fields\n"); > + return PTR_ERR(afe->fields[i]); > + } > + } > + > afe->regulator = devm_regulator_get(afe->dev, "tx_sup"); > if (IS_ERR(afe->regulator)) { > dev_err(afe->dev, "Unable to get regulator\n"); > diff --git a/drivers/iio/health/afe4404.c b/drivers/iio/health/afe4404.c > index 7806a45..0d1af4a 100644 > --- a/drivers/iio/health/afe4404.c > +++ b/drivers/iio/health/afe4404.c > @@ -48,43 +48,9 @@ > #define AFE4404_AVG_LED2_ALED2VAL 0x3f > #define AFE4404_AVG_LED1_ALED1VAL 0x40 > > -/* AFE4404 GAIN register fields */ > -#define AFE4404_TIA_GAIN_RES_MASK GENMASK(2, 0) > -#define AFE4404_TIA_GAIN_RES_SHIFT 0 > -#define AFE4404_TIA_GAIN_CAP_MASK GENMASK(5, 3) > -#define AFE4404_TIA_GAIN_CAP_SHIFT 3 > - > -/* AFE4404 LEDCNTRL register fields */ > -#define AFE4404_LEDCNTRL_ILED1_MASK GENMASK(5, 0) > -#define AFE4404_LEDCNTRL_ILED1_SHIFT 0 > -#define AFE4404_LEDCNTRL_ILED2_MASK GENMASK(11, 6) > -#define AFE4404_LEDCNTRL_ILED2_SHIFT 6 > -#define AFE4404_LEDCNTRL_ILED3_MASK GENMASK(17, 12) > -#define AFE4404_LEDCNTRL_ILED3_SHIFT 12 > - > -/* AFE4404 CONTROL2 register fields */ > -#define AFE440X_CONTROL2_ILED_2X_MASK BIT(17) > -#define AFE440X_CONTROL2_ILED_2X_SHIFT 17 > - > /* AFE4404 CONTROL3 register fields */ > #define AFE440X_CONTROL3_OSC_ENABLE BIT(9) > > -/* AFE4404 OFFDAC register current fields */ > -#define AFE4404_OFFDAC_CURR_LED1_MASK GENMASK(9, 5) > -#define AFE4404_OFFDAC_CURR_LED1_SHIFT 5 > -#define AFE4404_OFFDAC_CURR_LED2_MASK GENMASK(19, 15) > -#define AFE4404_OFFDAC_CURR_LED2_SHIFT 15 > -#define AFE4404_OFFDAC_CURR_LED3_MASK GENMASK(4, 0) > -#define AFE4404_OFFDAC_CURR_LED3_SHIFT 0 > -#define AFE4404_OFFDAC_CURR_ALED1_MASK GENMASK(14, 10) > -#define AFE4404_OFFDAC_CURR_ALED1_SHIFT 10 > -#define AFE4404_OFFDAC_CURR_ALED2_MASK GENMASK(4, 0) > -#define AFE4404_OFFDAC_CURR_ALED2_SHIFT 0 > - > -/* AFE4404 NULL fields */ > -#define NULL_MASK 0 > -#define NULL_SHIFT 0 > - > /* AFE4404 TIA_GAIN_CAP values */ > #define AFE4404_TIA_GAIN_CAP_5_P 0x0 > #define AFE4404_TIA_GAIN_CAP_2_5_P 0x1 > @@ -105,10 +71,43 @@ > #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, > + F_TIA_GAIN, TIA_CF, > + > + /* LED Current */ > + F_ILED1, F_ILED2, F_ILED3, > + > + /* Offset DAC */ > + F_OFFDAC_AMB2, F_OFFDAC_LED1, F_OFFDAC_AMB1, F_OFFDAC_LED2, > + > + /* sentinel */ > + F_MAX_FIELDS > +}; > + > +static const struct reg_field afe4404_reg_fields[] = { > + /* Gains */ > + [F_TIA_GAIN_SEP] = REG_FIELD(AFE4404_TIA_GAIN_SEP, 0, 2), > + [F_TIA_CF_SEP] = REG_FIELD(AFE4404_TIA_GAIN_SEP, 3, 5), > + [F_TIA_GAIN] = REG_FIELD(AFE4404_TIA_GAIN, 0, 2), > + [TIA_CF] = REG_FIELD(AFE4404_TIA_GAIN, 3, 5), > + /* LED Current */ > + [F_ILED1] = REG_FIELD(AFE440X_LEDCNTRL, 0, 5), > + [F_ILED2] = REG_FIELD(AFE440X_LEDCNTRL, 6, 11), > + [F_ILED3] = REG_FIELD(AFE440X_LEDCNTRL, 12, 17), > + /* Offset DAC */ > + [F_OFFDAC_AMB2] = REG_FIELD(AFE4404_OFFDAC, 0, 4), > + [F_OFFDAC_LED1] = REG_FIELD(AFE4404_OFFDAC, 5, 9), > + [F_OFFDAC_AMB1] = REG_FIELD(AFE4404_OFFDAC, 10, 14), > + [F_OFFDAC_LED2] = REG_FIELD(AFE4404_OFFDAC, 15, 19), > +}; > + > /** > * struct afe4404_data - AFE4404 device instance data > * @dev: Device structure > * @regmap: Register map of the device > + * @fields: Register fields of the device > * @regulator: Pointer to the regulator for the IC > * @trig: IIO trigger for this device > * @irq: ADC_RDY line interrupt number > @@ -116,6 +115,7 @@ > struct afe4404_data { > struct device *dev; > struct regmap *regmap; > + struct regmap_field *fields[F_MAX_FIELDS]; > struct regulator *regulator; > struct iio_trigger *trig; > int irq; > @@ -133,16 +133,26 @@ enum afe4404_chan_id { > ILED3, > }; > > -static const struct afe440x_reg_info afe4404_reg_info[] = { > - [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] = 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), > +static const unsigned int afe4404_channel_values[] = { > + [LED2] = AFE440X_LED2VAL, > + [ALED2] = AFE440X_ALED2VAL, > + [LED1] = AFE440X_LED1VAL, > + [ALED1] = AFE440X_ALED1VAL, > + [LED2_ALED2] = AFE440X_LED2_ALED2VAL, > + [LED1_ALED1] = AFE440X_LED1_ALED1VAL, > +}; > + > +static const unsigned int afe4404_channel_leds[] = { > + [ILED1] = F_ILED1, > + [ILED2] = F_ILED2, > + [ILED3] = F_ILED3, > +}; > + > +static const unsigned int afe4404_channel_offdacs[] = { > + [LED2] = F_OFFDAC_LED2, > + [ALED2] = F_OFFDAC_AMB2, > + [LED1] = F_OFFDAC_LED1, > + [ALED1] = F_OFFDAC_AMB1, > }; > > static const struct iio_chan_spec afe4404_channels[] = { > @@ -194,13 +204,10 @@ static ssize_t afe440x_show_register(struct device *dev, > int vals[2]; > int ret; > > - ret = regmap_read(afe->regmap, afe440x_attr->reg, ®_val); > + ret = regmap_field_read(afe->fields[afe440x_attr->field], ®_val); > if (ret) > return ret; > > - reg_val &= afe440x_attr->mask; > - reg_val >>= afe440x_attr->shift; > - > if (reg_val >= afe440x_attr->table_size) > return -EINVAL; > > @@ -230,20 +237,18 @@ static ssize_t afe440x_store_register(struct device *dev, > if (val == afe440x_attr->table_size) > return -EINVAL; > > - ret = regmap_update_bits(afe->regmap, afe440x_attr->reg, > - afe440x_attr->mask, > - (val << afe440x_attr->shift)); > + ret = regmap_field_write(afe->fields[afe440x_attr->field], val); > if (ret) > return ret; > > return count; > } > > -static AFE440X_ATTR(tia_resistance1, AFE4404_TIA_GAIN, AFE4404_TIA_GAIN_RES, afe4404_res_table); > -static AFE440X_ATTR(tia_capacitance1, AFE4404_TIA_GAIN, AFE4404_TIA_GAIN_CAP, afe4404_cap_table); > +static AFE440X_ATTR(tia_resistance1, F_TIA_GAIN, afe4404_res_table); > +static AFE440X_ATTR(tia_capacitance1, TIA_CF, afe4404_cap_table); > > -static AFE440X_ATTR(tia_resistance2, AFE4404_TIA_GAIN_SEP, AFE4404_TIA_GAIN_RES, afe4404_res_table); > -static AFE440X_ATTR(tia_capacitance2, AFE4404_TIA_GAIN_SEP, AFE4404_TIA_GAIN_CAP, afe4404_cap_table); > +static AFE440X_ATTR(tia_resistance2, F_TIA_GAIN_SEP, afe4404_res_table); > +static AFE440X_ATTR(tia_capacitance2, F_TIA_CF_SEP, afe4404_cap_table); > > static struct attribute *afe440x_attributes[] = { > &afe440x_attr_tia_resistance1.dev_attr.attr, > @@ -264,35 +269,32 @@ static int afe4404_read_raw(struct iio_dev *indio_dev, > int *val, int *val2, long mask) > { > struct afe4404_data *afe = iio_priv(indio_dev); > - const struct afe440x_reg_info reg_info = afe4404_reg_info[chan->address]; > + unsigned int value_reg = afe4404_channel_values[chan->address]; > + unsigned int led_field = afe4404_channel_leds[chan->address]; > + unsigned int offdac_field = afe4404_channel_offdacs[chan->address]; > int ret; > > switch (chan->type) { > case IIO_INTENSITY: > switch (mask) { > case IIO_CHAN_INFO_RAW: > - ret = regmap_read(afe->regmap, reg_info.reg, val); > + ret = regmap_read(afe->regmap, value_reg, val); > if (ret) > return ret; > return IIO_VAL_INT; > case IIO_CHAN_INFO_OFFSET: > - ret = regmap_read(afe->regmap, reg_info.offreg, > - val); > + ret = regmap_field_read(afe->fields[offdac_field], val); > if (ret) > return ret; > - *val &= reg_info.mask; > - *val >>= reg_info.shift; > return IIO_VAL_INT; > } > break; > case IIO_CURRENT: > switch (mask) { > case IIO_CHAN_INFO_RAW: > - ret = regmap_read(afe->regmap, reg_info.reg, val); > + ret = regmap_field_read(afe->fields[led_field], val); > if (ret) > return ret; > - *val &= reg_info.mask; > - *val >>= reg_info.shift; > return IIO_VAL_INT; > case IIO_CHAN_INFO_SCALE: > *val = 0; > @@ -312,25 +314,20 @@ static int afe4404_write_raw(struct iio_dev *indio_dev, > int val, int val2, long mask) > { > struct afe4404_data *afe = iio_priv(indio_dev); > - const struct afe440x_reg_info reg_info = afe4404_reg_info[chan->address]; > + unsigned int led_field = afe4404_channel_leds[chan->address]; > + unsigned int offdac_field = afe4404_channel_offdacs[chan->address]; > > switch (chan->type) { > case IIO_INTENSITY: > switch (mask) { > case IIO_CHAN_INFO_OFFSET: > - return regmap_update_bits(afe->regmap, > - reg_info.offreg, > - reg_info.mask, > - (val << reg_info.shift)); > + return regmap_field_write(afe->fields[offdac_field], val); > } > break; > case IIO_CURRENT: > switch (mask) { > case IIO_CHAN_INFO_RAW: > - return regmap_update_bits(afe->regmap, > - reg_info.reg, > - reg_info.mask, > - (val << reg_info.shift)); > + return regmap_field_write(afe->fields[led_field], val); > } > break; > default: > @@ -357,7 +354,7 @@ static irqreturn_t afe4404_trigger_handler(int irq, void *private) > > for_each_set_bit(bit, indio_dev->active_scan_mask, > indio_dev->masklength) { > - ret = regmap_read(afe->regmap, afe4404_reg_info[bit].reg, > + ret = regmap_read(afe->regmap, afe4404_channel_values[bit], > &buffer[i++]); > if (ret) > goto err; > @@ -490,7 +487,7 @@ static int afe4404_probe(struct i2c_client *client, > { > struct iio_dev *indio_dev; > struct afe4404_data *afe; > - int ret; > + int i, ret; > > indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*afe)); > if (!indio_dev) > @@ -508,6 +505,15 @@ static int afe4404_probe(struct i2c_client *client, > return PTR_ERR(afe->regmap); > } > > + for (i = 0; i < F_MAX_FIELDS; i++) { > + afe->fields[i] = devm_regmap_field_alloc(afe->dev, afe->regmap, > + afe4404_reg_fields[i]); > + if (IS_ERR(afe->fields[i])) { > + dev_err(afe->dev, "Unable to allocate regmap fields\n"); > + return PTR_ERR(afe->fields[i]); > + } > + } > + > afe->regulator = devm_regulator_get(afe->dev, "tx_sup"); > if (IS_ERR(afe->regulator)) { > dev_err(afe->dev, "Unable to get regulator\n"); > diff --git a/drivers/iio/health/afe440x.h b/drivers/iio/health/afe440x.h > index 713972f..1a0f247 100644 > --- a/drivers/iio/health/afe440x.h > +++ b/drivers/iio/health/afe440x.h > @@ -88,21 +88,6 @@ > #define AFE440X_CONTROL0_WRITE 0x0 > #define AFE440X_CONTROL0_READ 0x1 > > -struct afe440x_reg_info { > - unsigned int reg; > - unsigned int offreg; > - unsigned int shift; > - unsigned int mask; > -}; > - > -#define AFE440X_REG_INFO(_reg, _offreg, _sm) \ > - { \ > - .reg = _reg, \ > - .offreg = _offreg, \ > - .shift = _sm ## _SHIFT, \ > - .mask = _sm ## _MASK, \ > - } > - > #define AFE440X_INTENSITY_CHAN(_index, _mask) \ > { \ > .type = IIO_INTENSITY, \ > @@ -157,9 +142,7 @@ static DEVICE_ATTR_RO(_name) > > struct afe440x_attr { > struct device_attribute dev_attr; > - unsigned int reg; > - unsigned int shift; > - unsigned int mask; > + unsigned int field; > const struct afe440x_val_table *val_table; > unsigned int table_size; > }; > @@ -167,14 +150,12 @@ struct afe440x_attr { > #define to_afe440x_attr(_dev_attr) \ > container_of(_dev_attr, struct afe440x_attr, dev_attr) > > -#define AFE440X_ATTR(_name, _reg, _field, _table) \ > +#define AFE440X_ATTR(_name, _field, _table) \ > struct afe440x_attr afe440x_attr_##_name = { \ > .dev_attr = __ATTR(_name, (S_IRUGO | S_IWUSR), \ > afe440x_show_register, \ > afe440x_store_register), \ > - .reg = _reg, \ > - .shift = _field ## _SHIFT, \ > - .mask = _field ## _MASK, \ > + .field = _field, \ > .val_table = _table, \ > .table_size = ARRAY_SIZE(_table), \ > } > ^ permalink raw reply [flat|nested] 32+ messages in thread
[parent not found: <45523060-7ae8-076f-dece-34e76e8a740a-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>]
* Re: [PATCH 09/13] iio: health/afe440x: Use regmap fields [not found] ` <45523060-7ae8-076f-dece-34e76e8a740a-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> @ 2016-05-04 15:30 ` Andrew F. Davis 0 siblings, 0 replies; 32+ messages in thread From: Andrew F. Davis @ 2016-05-04 15:30 UTC (permalink / raw) To: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA, linux-api-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA On 05/04/2016 05:10 AM, Jonathan Cameron wrote: > On 01/05/16 21:36, Andrew F. Davis wrote: >> These drivers can use regmap fields to access fields in registers, this >> allows us to remove some macros/defines and simplify code, do this here. >> >> Signed-off-by: Andrew F. Davis <afd-l0cyMroinI0@public.gmane.org> > This could almost be a poster boy for why regmap field stuff is good ;) > So often it ends up more marginal than this. > I know, I am really pleased how well it worked out here. Andrew > Applied. > > Jonathan >> --- >> drivers/iio/health/afe4403.c | 118 ++++++++++++++++---------------- >> drivers/iio/health/afe4404.c | 156 ++++++++++++++++++++++--------------------- >> drivers/iio/health/afe440x.h | 25 +------ >> 3 files changed, 144 insertions(+), 155 deletions(-) >> >> diff --git a/drivers/iio/health/afe4403.c b/drivers/iio/health/afe4403.c >> index 4a58064..1950155 100644 >> --- a/drivers/iio/health/afe4403.c >> +++ b/drivers/iio/health/afe4403.c >> @@ -39,32 +39,6 @@ >> #define AFE4403_TIAGAIN 0x20 >> #define AFE4403_TIA_AMB_GAIN 0x21 >> >> -/* AFE4403 GAIN register fields */ >> -#define AFE4403_TIAGAIN_RES_MASK GENMASK(2, 0) >> -#define AFE4403_TIAGAIN_RES_SHIFT 0 >> -#define AFE4403_TIAGAIN_CAP_MASK GENMASK(7, 3) >> -#define AFE4403_TIAGAIN_CAP_SHIFT 3 >> - >> -/* AFE4403 LEDCNTRL register fields */ >> -#define AFE440X_LEDCNTRL_LED1_MASK GENMASK(15, 8) >> -#define AFE440X_LEDCNTRL_LED1_SHIFT 8 >> -#define AFE440X_LEDCNTRL_LED2_MASK GENMASK(7, 0) >> -#define AFE440X_LEDCNTRL_LED2_SHIFT 0 >> -#define AFE440X_LEDCNTRL_LED_RANGE_MASK GENMASK(17, 16) >> -#define AFE440X_LEDCNTRL_LED_RANGE_SHIFT 16 >> - >> -/* AFE4403 CONTROL2 register fields */ >> -#define AFE440X_CONTROL2_PWR_DWN_TX BIT(2) >> -#define AFE440X_CONTROL2_EN_SLOW_DIAG BIT(8) >> -#define AFE440X_CONTROL2_DIAG_OUT_TRI BIT(10) >> -#define AFE440X_CONTROL2_TX_BRDG_MOD BIT(11) >> -#define AFE440X_CONTROL2_TX_REF_MASK GENMASK(18, 17) >> -#define AFE440X_CONTROL2_TX_REF_SHIFT 17 >> - >> -/* AFE4404 NULL fields */ >> -#define NULL_MASK 0 >> -#define NULL_SHIFT 0 >> - >> /* AFE4403 LEDCNTRL values */ >> #define AFE440X_LEDCNTRL_RANGE_TX_HALF 0x1 >> #define AFE440X_LEDCNTRL_RANGE_TX_FULL 0x2 >> @@ -102,11 +76,35 @@ >> #define AFE4403_TIAGAIN_RES_1_M 0x6 >> #define AFE4403_TIAGAIN_RES_NONE 0x7 >> >> +enum afe4403_fields { >> + /* Gains */ >> + F_RF_LED1, F_CF_LED1, >> + F_RF_LED, F_CF_LED, >> + >> + /* LED Current */ >> + F_ILED1, F_ILED2, >> + >> + /* sentinel */ >> + F_MAX_FIELDS >> +}; >> + >> +static const struct reg_field afe4403_reg_fields[] = { >> + /* Gains */ >> + [F_RF_LED1] = REG_FIELD(AFE4403_TIAGAIN, 0, 2), >> + [F_CF_LED1] = REG_FIELD(AFE4403_TIAGAIN, 3, 7), >> + [F_RF_LED] = REG_FIELD(AFE4403_TIA_AMB_GAIN, 0, 2), >> + [F_CF_LED] = REG_FIELD(AFE4403_TIA_AMB_GAIN, 3, 7), >> + /* LED Current */ >> + [F_ILED1] = REG_FIELD(AFE440X_LEDCNTRL, 0, 7), >> + [F_ILED2] = REG_FIELD(AFE440X_LEDCNTRL, 8, 15), >> +}; >> + >> /** >> * struct afe4403_data - AFE4403 device instance data >> * @dev: Device structure >> * @spi: SPI device handle >> * @regmap: Register map of the device >> + * @fields: Register fields of the device >> * @regulator: Pointer to the regulator for the IC >> * @trig: IIO trigger for this device >> * @irq: ADC_RDY line interrupt number >> @@ -115,6 +113,7 @@ struct afe4403_data { >> struct device *dev; >> struct spi_device *spi; >> struct regmap *regmap; >> + struct regmap_field *fields[F_MAX_FIELDS]; >> struct regulator *regulator; >> struct iio_trigger *trig; >> int irq; >> @@ -131,15 +130,18 @@ enum afe4403_chan_id { >> ILED2, >> }; >> >> -static const struct afe440x_reg_info afe4403_reg_info[] = { >> - [LED2] = AFE440X_REG_INFO(AFE440X_LED2VAL, 0, NULL), >> - [ALED2] = AFE440X_REG_INFO(AFE440X_ALED2VAL, 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 unsigned int afe4403_channel_values[] = { >> + [LED2] = AFE440X_LED2VAL, >> + [ALED2] = AFE440X_ALED2VAL, >> + [LED1] = AFE440X_LED1VAL, >> + [ALED1] = AFE440X_ALED1VAL, >> + [LED2_ALED2] = AFE440X_LED2_ALED2VAL, >> + [LED1_ALED1] = AFE440X_LED1_ALED1VAL, >> +}; >> + >> +static const unsigned int afe4403_channel_leds[] = { >> + [ILED1] = F_ILED1, >> + [ILED2] = F_ILED2, >> }; >> >> static const struct iio_chan_spec afe4403_channels[] = { >> @@ -184,13 +186,10 @@ static ssize_t afe440x_show_register(struct device *dev, >> int vals[2]; >> int ret; >> >> - ret = regmap_read(afe->regmap, afe440x_attr->reg, ®_val); >> + ret = regmap_field_read(afe->fields[afe440x_attr->field], ®_val); >> if (ret) >> return ret; >> >> - reg_val &= afe440x_attr->mask; >> - reg_val >>= afe440x_attr->shift; >> - >> if (reg_val >= afe440x_attr->table_size) >> return -EINVAL; >> >> @@ -220,20 +219,18 @@ static ssize_t afe440x_store_register(struct device *dev, >> if (val == afe440x_attr->table_size) >> return -EINVAL; >> >> - ret = regmap_update_bits(afe->regmap, afe440x_attr->reg, >> - afe440x_attr->mask, >> - (val << afe440x_attr->shift)); >> + ret = regmap_field_write(afe->fields[afe440x_attr->field], val); >> if (ret) >> return ret; >> >> return count; >> } >> >> -static AFE440X_ATTR(tia_resistance1, AFE4403_TIAGAIN, AFE4403_TIAGAIN_RES, afe4403_res_table); >> -static AFE440X_ATTR(tia_capacitance1, AFE4403_TIAGAIN, AFE4403_TIAGAIN_CAP, afe4403_cap_table); >> +static AFE440X_ATTR(tia_resistance1, F_RF_LED1, afe4403_res_table); >> +static AFE440X_ATTR(tia_capacitance1, F_CF_LED1, afe4403_cap_table); >> >> -static AFE440X_ATTR(tia_resistance2, AFE4403_TIA_AMB_GAIN, AFE4403_TIAGAIN_RES, afe4403_res_table); >> -static AFE440X_ATTR(tia_capacitance2, AFE4403_TIA_AMB_GAIN, AFE4403_TIAGAIN_RES, afe4403_cap_table); >> +static AFE440X_ATTR(tia_resistance2, F_RF_LED, afe4403_res_table); >> +static AFE440X_ATTR(tia_capacitance2, F_CF_LED, afe4403_cap_table); >> >> static struct attribute *afe440x_attributes[] = { >> &afe440x_attr_tia_resistance1.dev_attr.attr, >> @@ -282,14 +279,15 @@ static int afe4403_read_raw(struct iio_dev *indio_dev, >> int *val, int *val2, long mask) >> { >> struct afe4403_data *afe = iio_priv(indio_dev); >> - const struct afe440x_reg_info reg_info = afe4403_reg_info[chan->address]; >> + unsigned int reg = afe4403_channel_values[chan->address]; >> + unsigned int field = afe4403_channel_leds[chan->address]; >> int ret; >> >> switch (chan->type) { >> case IIO_INTENSITY: >> switch (mask) { >> case IIO_CHAN_INFO_RAW: >> - ret = afe4403_read(afe, reg_info.reg, val); >> + ret = afe4403_read(afe, reg, val); >> if (ret) >> return ret; >> return IIO_VAL_INT; >> @@ -298,11 +296,9 @@ static int afe4403_read_raw(struct iio_dev *indio_dev, >> case IIO_CURRENT: >> switch (mask) { >> case IIO_CHAN_INFO_RAW: >> - ret = regmap_read(afe->regmap, reg_info.reg, val); >> + ret = regmap_field_read(afe->fields[field], val); >> if (ret) >> return ret; >> - *val &= reg_info.mask; >> - *val >>= reg_info.shift; >> return IIO_VAL_INT; >> case IIO_CHAN_INFO_SCALE: >> *val = 0; >> @@ -322,16 +318,13 @@ static int afe4403_write_raw(struct iio_dev *indio_dev, >> int val, int val2, long mask) >> { >> struct afe4403_data *afe = iio_priv(indio_dev); >> - const struct afe440x_reg_info reg_info = afe4403_reg_info[chan->address]; >> + unsigned int field = afe4403_channel_leds[chan->address]; >> >> switch (chan->type) { >> case IIO_CURRENT: >> switch (mask) { >> case IIO_CHAN_INFO_RAW: >> - return regmap_update_bits(afe->regmap, >> - reg_info.reg, >> - reg_info.mask, >> - (val << reg_info.shift)); >> + return regmap_field_write(afe->fields[field], val); >> } >> break; >> default: >> @@ -366,7 +359,7 @@ static irqreturn_t afe4403_trigger_handler(int irq, void *private) >> for_each_set_bit(bit, indio_dev->active_scan_mask, >> indio_dev->masklength) { >> ret = spi_write_then_read(afe->spi, >> - &afe4403_reg_info[bit].reg, 1, >> + &afe4403_channel_values[bit], 1, >> rx, 3); >> if (ret) >> goto err; >> @@ -503,7 +496,7 @@ static int afe4403_probe(struct spi_device *spi) >> { >> struct iio_dev *indio_dev; >> struct afe4403_data *afe; >> - int ret; >> + int i, ret; >> >> indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*afe)); >> if (!indio_dev) >> @@ -522,6 +515,15 @@ static int afe4403_probe(struct spi_device *spi) >> return PTR_ERR(afe->regmap); >> } >> >> + for (i = 0; i < F_MAX_FIELDS; i++) { >> + afe->fields[i] = devm_regmap_field_alloc(afe->dev, afe->regmap, >> + afe4403_reg_fields[i]); >> + if (IS_ERR(afe->fields[i])) { >> + dev_err(afe->dev, "Unable to allocate regmap fields\n"); >> + return PTR_ERR(afe->fields[i]); >> + } >> + } >> + >> afe->regulator = devm_regulator_get(afe->dev, "tx_sup"); >> if (IS_ERR(afe->regulator)) { >> dev_err(afe->dev, "Unable to get regulator\n"); >> diff --git a/drivers/iio/health/afe4404.c b/drivers/iio/health/afe4404.c >> index 7806a45..0d1af4a 100644 >> --- a/drivers/iio/health/afe4404.c >> +++ b/drivers/iio/health/afe4404.c >> @@ -48,43 +48,9 @@ >> #define AFE4404_AVG_LED2_ALED2VAL 0x3f >> #define AFE4404_AVG_LED1_ALED1VAL 0x40 >> >> -/* AFE4404 GAIN register fields */ >> -#define AFE4404_TIA_GAIN_RES_MASK GENMASK(2, 0) >> -#define AFE4404_TIA_GAIN_RES_SHIFT 0 >> -#define AFE4404_TIA_GAIN_CAP_MASK GENMASK(5, 3) >> -#define AFE4404_TIA_GAIN_CAP_SHIFT 3 >> - >> -/* AFE4404 LEDCNTRL register fields */ >> -#define AFE4404_LEDCNTRL_ILED1_MASK GENMASK(5, 0) >> -#define AFE4404_LEDCNTRL_ILED1_SHIFT 0 >> -#define AFE4404_LEDCNTRL_ILED2_MASK GENMASK(11, 6) >> -#define AFE4404_LEDCNTRL_ILED2_SHIFT 6 >> -#define AFE4404_LEDCNTRL_ILED3_MASK GENMASK(17, 12) >> -#define AFE4404_LEDCNTRL_ILED3_SHIFT 12 >> - >> -/* AFE4404 CONTROL2 register fields */ >> -#define AFE440X_CONTROL2_ILED_2X_MASK BIT(17) >> -#define AFE440X_CONTROL2_ILED_2X_SHIFT 17 >> - >> /* AFE4404 CONTROL3 register fields */ >> #define AFE440X_CONTROL3_OSC_ENABLE BIT(9) >> >> -/* AFE4404 OFFDAC register current fields */ >> -#define AFE4404_OFFDAC_CURR_LED1_MASK GENMASK(9, 5) >> -#define AFE4404_OFFDAC_CURR_LED1_SHIFT 5 >> -#define AFE4404_OFFDAC_CURR_LED2_MASK GENMASK(19, 15) >> -#define AFE4404_OFFDAC_CURR_LED2_SHIFT 15 >> -#define AFE4404_OFFDAC_CURR_LED3_MASK GENMASK(4, 0) >> -#define AFE4404_OFFDAC_CURR_LED3_SHIFT 0 >> -#define AFE4404_OFFDAC_CURR_ALED1_MASK GENMASK(14, 10) >> -#define AFE4404_OFFDAC_CURR_ALED1_SHIFT 10 >> -#define AFE4404_OFFDAC_CURR_ALED2_MASK GENMASK(4, 0) >> -#define AFE4404_OFFDAC_CURR_ALED2_SHIFT 0 >> - >> -/* AFE4404 NULL fields */ >> -#define NULL_MASK 0 >> -#define NULL_SHIFT 0 >> - >> /* AFE4404 TIA_GAIN_CAP values */ >> #define AFE4404_TIA_GAIN_CAP_5_P 0x0 >> #define AFE4404_TIA_GAIN_CAP_2_5_P 0x1 >> @@ -105,10 +71,43 @@ >> #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, >> + F_TIA_GAIN, TIA_CF, >> + >> + /* LED Current */ >> + F_ILED1, F_ILED2, F_ILED3, >> + >> + /* Offset DAC */ >> + F_OFFDAC_AMB2, F_OFFDAC_LED1, F_OFFDAC_AMB1, F_OFFDAC_LED2, >> + >> + /* sentinel */ >> + F_MAX_FIELDS >> +}; >> + >> +static const struct reg_field afe4404_reg_fields[] = { >> + /* Gains */ >> + [F_TIA_GAIN_SEP] = REG_FIELD(AFE4404_TIA_GAIN_SEP, 0, 2), >> + [F_TIA_CF_SEP] = REG_FIELD(AFE4404_TIA_GAIN_SEP, 3, 5), >> + [F_TIA_GAIN] = REG_FIELD(AFE4404_TIA_GAIN, 0, 2), >> + [TIA_CF] = REG_FIELD(AFE4404_TIA_GAIN, 3, 5), >> + /* LED Current */ >> + [F_ILED1] = REG_FIELD(AFE440X_LEDCNTRL, 0, 5), >> + [F_ILED2] = REG_FIELD(AFE440X_LEDCNTRL, 6, 11), >> + [F_ILED3] = REG_FIELD(AFE440X_LEDCNTRL, 12, 17), >> + /* Offset DAC */ >> + [F_OFFDAC_AMB2] = REG_FIELD(AFE4404_OFFDAC, 0, 4), >> + [F_OFFDAC_LED1] = REG_FIELD(AFE4404_OFFDAC, 5, 9), >> + [F_OFFDAC_AMB1] = REG_FIELD(AFE4404_OFFDAC, 10, 14), >> + [F_OFFDAC_LED2] = REG_FIELD(AFE4404_OFFDAC, 15, 19), >> +}; >> + >> /** >> * struct afe4404_data - AFE4404 device instance data >> * @dev: Device structure >> * @regmap: Register map of the device >> + * @fields: Register fields of the device >> * @regulator: Pointer to the regulator for the IC >> * @trig: IIO trigger for this device >> * @irq: ADC_RDY line interrupt number >> @@ -116,6 +115,7 @@ >> struct afe4404_data { >> struct device *dev; >> struct regmap *regmap; >> + struct regmap_field *fields[F_MAX_FIELDS]; >> struct regulator *regulator; >> struct iio_trigger *trig; >> int irq; >> @@ -133,16 +133,26 @@ enum afe4404_chan_id { >> ILED3, >> }; >> >> -static const struct afe440x_reg_info afe4404_reg_info[] = { >> - [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] = 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), >> +static const unsigned int afe4404_channel_values[] = { >> + [LED2] = AFE440X_LED2VAL, >> + [ALED2] = AFE440X_ALED2VAL, >> + [LED1] = AFE440X_LED1VAL, >> + [ALED1] = AFE440X_ALED1VAL, >> + [LED2_ALED2] = AFE440X_LED2_ALED2VAL, >> + [LED1_ALED1] = AFE440X_LED1_ALED1VAL, >> +}; >> + >> +static const unsigned int afe4404_channel_leds[] = { >> + [ILED1] = F_ILED1, >> + [ILED2] = F_ILED2, >> + [ILED3] = F_ILED3, >> +}; >> + >> +static const unsigned int afe4404_channel_offdacs[] = { >> + [LED2] = F_OFFDAC_LED2, >> + [ALED2] = F_OFFDAC_AMB2, >> + [LED1] = F_OFFDAC_LED1, >> + [ALED1] = F_OFFDAC_AMB1, >> }; >> >> static const struct iio_chan_spec afe4404_channels[] = { >> @@ -194,13 +204,10 @@ static ssize_t afe440x_show_register(struct device *dev, >> int vals[2]; >> int ret; >> >> - ret = regmap_read(afe->regmap, afe440x_attr->reg, ®_val); >> + ret = regmap_field_read(afe->fields[afe440x_attr->field], ®_val); >> if (ret) >> return ret; >> >> - reg_val &= afe440x_attr->mask; >> - reg_val >>= afe440x_attr->shift; >> - >> if (reg_val >= afe440x_attr->table_size) >> return -EINVAL; >> >> @@ -230,20 +237,18 @@ static ssize_t afe440x_store_register(struct device *dev, >> if (val == afe440x_attr->table_size) >> return -EINVAL; >> >> - ret = regmap_update_bits(afe->regmap, afe440x_attr->reg, >> - afe440x_attr->mask, >> - (val << afe440x_attr->shift)); >> + ret = regmap_field_write(afe->fields[afe440x_attr->field], val); >> if (ret) >> return ret; >> >> return count; >> } >> >> -static AFE440X_ATTR(tia_resistance1, AFE4404_TIA_GAIN, AFE4404_TIA_GAIN_RES, afe4404_res_table); >> -static AFE440X_ATTR(tia_capacitance1, AFE4404_TIA_GAIN, AFE4404_TIA_GAIN_CAP, afe4404_cap_table); >> +static AFE440X_ATTR(tia_resistance1, F_TIA_GAIN, afe4404_res_table); >> +static AFE440X_ATTR(tia_capacitance1, TIA_CF, afe4404_cap_table); >> >> -static AFE440X_ATTR(tia_resistance2, AFE4404_TIA_GAIN_SEP, AFE4404_TIA_GAIN_RES, afe4404_res_table); >> -static AFE440X_ATTR(tia_capacitance2, AFE4404_TIA_GAIN_SEP, AFE4404_TIA_GAIN_CAP, afe4404_cap_table); >> +static AFE440X_ATTR(tia_resistance2, F_TIA_GAIN_SEP, afe4404_res_table); >> +static AFE440X_ATTR(tia_capacitance2, F_TIA_CF_SEP, afe4404_cap_table); >> >> static struct attribute *afe440x_attributes[] = { >> &afe440x_attr_tia_resistance1.dev_attr.attr, >> @@ -264,35 +269,32 @@ static int afe4404_read_raw(struct iio_dev *indio_dev, >> int *val, int *val2, long mask) >> { >> struct afe4404_data *afe = iio_priv(indio_dev); >> - const struct afe440x_reg_info reg_info = afe4404_reg_info[chan->address]; >> + unsigned int value_reg = afe4404_channel_values[chan->address]; >> + unsigned int led_field = afe4404_channel_leds[chan->address]; >> + unsigned int offdac_field = afe4404_channel_offdacs[chan->address]; >> int ret; >> >> switch (chan->type) { >> case IIO_INTENSITY: >> switch (mask) { >> case IIO_CHAN_INFO_RAW: >> - ret = regmap_read(afe->regmap, reg_info.reg, val); >> + ret = regmap_read(afe->regmap, value_reg, val); >> if (ret) >> return ret; >> return IIO_VAL_INT; >> case IIO_CHAN_INFO_OFFSET: >> - ret = regmap_read(afe->regmap, reg_info.offreg, >> - val); >> + ret = regmap_field_read(afe->fields[offdac_field], val); >> if (ret) >> return ret; >> - *val &= reg_info.mask; >> - *val >>= reg_info.shift; >> return IIO_VAL_INT; >> } >> break; >> case IIO_CURRENT: >> switch (mask) { >> case IIO_CHAN_INFO_RAW: >> - ret = regmap_read(afe->regmap, reg_info.reg, val); >> + ret = regmap_field_read(afe->fields[led_field], val); >> if (ret) >> return ret; >> - *val &= reg_info.mask; >> - *val >>= reg_info.shift; >> return IIO_VAL_INT; >> case IIO_CHAN_INFO_SCALE: >> *val = 0; >> @@ -312,25 +314,20 @@ static int afe4404_write_raw(struct iio_dev *indio_dev, >> int val, int val2, long mask) >> { >> struct afe4404_data *afe = iio_priv(indio_dev); >> - const struct afe440x_reg_info reg_info = afe4404_reg_info[chan->address]; >> + unsigned int led_field = afe4404_channel_leds[chan->address]; >> + unsigned int offdac_field = afe4404_channel_offdacs[chan->address]; >> >> switch (chan->type) { >> case IIO_INTENSITY: >> switch (mask) { >> case IIO_CHAN_INFO_OFFSET: >> - return regmap_update_bits(afe->regmap, >> - reg_info.offreg, >> - reg_info.mask, >> - (val << reg_info.shift)); >> + return regmap_field_write(afe->fields[offdac_field], val); >> } >> break; >> case IIO_CURRENT: >> switch (mask) { >> case IIO_CHAN_INFO_RAW: >> - return regmap_update_bits(afe->regmap, >> - reg_info.reg, >> - reg_info.mask, >> - (val << reg_info.shift)); >> + return regmap_field_write(afe->fields[led_field], val); >> } >> break; >> default: >> @@ -357,7 +354,7 @@ static irqreturn_t afe4404_trigger_handler(int irq, void *private) >> >> for_each_set_bit(bit, indio_dev->active_scan_mask, >> indio_dev->masklength) { >> - ret = regmap_read(afe->regmap, afe4404_reg_info[bit].reg, >> + ret = regmap_read(afe->regmap, afe4404_channel_values[bit], >> &buffer[i++]); >> if (ret) >> goto err; >> @@ -490,7 +487,7 @@ static int afe4404_probe(struct i2c_client *client, >> { >> struct iio_dev *indio_dev; >> struct afe4404_data *afe; >> - int ret; >> + int i, ret; >> >> indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*afe)); >> if (!indio_dev) >> @@ -508,6 +505,15 @@ static int afe4404_probe(struct i2c_client *client, >> return PTR_ERR(afe->regmap); >> } >> >> + for (i = 0; i < F_MAX_FIELDS; i++) { >> + afe->fields[i] = devm_regmap_field_alloc(afe->dev, afe->regmap, >> + afe4404_reg_fields[i]); >> + if (IS_ERR(afe->fields[i])) { >> + dev_err(afe->dev, "Unable to allocate regmap fields\n"); >> + return PTR_ERR(afe->fields[i]); >> + } >> + } >> + >> afe->regulator = devm_regulator_get(afe->dev, "tx_sup"); >> if (IS_ERR(afe->regulator)) { >> dev_err(afe->dev, "Unable to get regulator\n"); >> diff --git a/drivers/iio/health/afe440x.h b/drivers/iio/health/afe440x.h >> index 713972f..1a0f247 100644 >> --- a/drivers/iio/health/afe440x.h >> +++ b/drivers/iio/health/afe440x.h >> @@ -88,21 +88,6 @@ >> #define AFE440X_CONTROL0_WRITE 0x0 >> #define AFE440X_CONTROL0_READ 0x1 >> >> -struct afe440x_reg_info { >> - unsigned int reg; >> - unsigned int offreg; >> - unsigned int shift; >> - unsigned int mask; >> -}; >> - >> -#define AFE440X_REG_INFO(_reg, _offreg, _sm) \ >> - { \ >> - .reg = _reg, \ >> - .offreg = _offreg, \ >> - .shift = _sm ## _SHIFT, \ >> - .mask = _sm ## _MASK, \ >> - } >> - >> #define AFE440X_INTENSITY_CHAN(_index, _mask) \ >> { \ >> .type = IIO_INTENSITY, \ >> @@ -157,9 +142,7 @@ static DEVICE_ATTR_RO(_name) >> >> struct afe440x_attr { >> struct device_attribute dev_attr; >> - unsigned int reg; >> - unsigned int shift; >> - unsigned int mask; >> + unsigned int field; >> const struct afe440x_val_table *val_table; >> unsigned int table_size; >> }; >> @@ -167,14 +150,12 @@ struct afe440x_attr { >> #define to_afe440x_attr(_dev_attr) \ >> container_of(_dev_attr, struct afe440x_attr, dev_attr) >> >> -#define AFE440X_ATTR(_name, _reg, _field, _table) \ >> +#define AFE440X_ATTR(_name, _field, _table) \ >> struct afe440x_attr afe440x_attr_##_name = { \ >> .dev_attr = __ATTR(_name, (S_IRUGO | S_IWUSR), \ >> afe440x_show_register, \ >> afe440x_store_register), \ >> - .reg = _reg, \ >> - .shift = _field ## _SHIFT, \ >> - .mask = _field ## _MASK, \ >> + .field = _field, \ >> .val_table = _table, \ >> .table_size = ARRAY_SIZE(_table), \ >> } >> > ^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 11/13] iio: health/afe440x: Match LED currents to stages [not found] ` <1462135023-14445-1-git-send-email-afd-l0cyMroinI0@public.gmane.org> ` (3 preceding siblings ...) 2016-05-01 20:36 ` [PATCH 09/13] iio: health/afe440x: Use regmap fields Andrew F. Davis @ 2016-05-01 20:37 ` Andrew F. Davis 2016-05-04 10:13 ` Jonathan Cameron 2016-05-01 20:37 ` [PATCH 12/13] iio: health/afe440x: Remove unused definitions Andrew F. Davis 2016-05-01 20:37 ` [PATCH 13/13] iio: health/afe4404: ENSEPGAIN is part of CONTROL2 register Andrew F. Davis 6 siblings, 1 reply; 32+ messages in thread From: Andrew F. Davis @ 2016-05-01 20:37 UTC (permalink / raw) To: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA, linux-api-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, Andrew F. Davis The current channel number for the LEDs should match the stage number that they are active during, fix this here. Signed-off-by: Andrew F. Davis <afd-l0cyMroinI0@public.gmane.org> --- drivers/iio/health/afe4403.c | 10 ++++------ drivers/iio/health/afe4404.c | 15 ++++++--------- 2 files changed, 10 insertions(+), 15 deletions(-) diff --git a/drivers/iio/health/afe4403.c b/drivers/iio/health/afe4403.c index 610631b..059d521 100644 --- a/drivers/iio/health/afe4403.c +++ b/drivers/iio/health/afe4403.c @@ -126,8 +126,6 @@ enum afe4403_chan_id { ALED1, LED2_ALED2, LED1_ALED1, - ILED1, - ILED2, }; static const unsigned int afe4403_channel_values[] = { @@ -140,8 +138,8 @@ static const unsigned int afe4403_channel_values[] = { }; static const unsigned int afe4403_channel_leds[] = { - [ILED1] = F_ILED1, - [ILED2] = F_ILED2, + [LED2] = F_ILED2, + [LED1] = F_ILED1, }; static const struct iio_chan_spec afe4403_channels[] = { @@ -153,8 +151,8 @@ static const struct iio_chan_spec afe4403_channels[] = { AFE440X_INTENSITY_CHAN(LED2_ALED2, 0), AFE440X_INTENSITY_CHAN(LED1_ALED1, 0), /* LED current */ - AFE440X_CURRENT_CHAN(ILED1), - AFE440X_CURRENT_CHAN(ILED2), + AFE440X_CURRENT_CHAN(LED2), + AFE440X_CURRENT_CHAN(LED1), }; static const struct afe440x_val_table afe4403_res_table[] = { diff --git a/drivers/iio/health/afe4404.c b/drivers/iio/health/afe4404.c index 69116cd..aa8770b 100644 --- a/drivers/iio/health/afe4404.c +++ b/drivers/iio/health/afe4404.c @@ -128,9 +128,6 @@ enum afe4404_chan_id { ALED1, LED2_ALED2, LED1_ALED1, - ILED1, - ILED2, - ILED3, }; static const unsigned int afe4404_channel_values[] = { @@ -143,9 +140,9 @@ static const unsigned int afe4404_channel_values[] = { }; static const unsigned int afe4404_channel_leds[] = { - [ILED1] = F_ILED1, - [ILED2] = F_ILED2, - [ILED3] = F_ILED3, + [LED2] = F_ILED2, + [ALED2] = F_ILED3, + [LED1] = F_ILED1, }; static const unsigned int afe4404_channel_offdacs[] = { @@ -164,9 +161,9 @@ static const struct iio_chan_spec afe4404_channels[] = { AFE440X_INTENSITY_CHAN(LED2_ALED2, 0), AFE440X_INTENSITY_CHAN(LED1_ALED1, 0), /* LED current */ - AFE440X_CURRENT_CHAN(ILED1), - AFE440X_CURRENT_CHAN(ILED2), - AFE440X_CURRENT_CHAN(ILED3), + AFE440X_CURRENT_CHAN(LED2), + AFE440X_CURRENT_CHAN(ALED2), + AFE440X_CURRENT_CHAN(LED1), }; static const struct afe440x_val_table afe4404_res_table[] = { -- 2.8.1 ^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: [PATCH 11/13] iio: health/afe440x: Match LED currents to stages 2016-05-01 20:37 ` [PATCH 11/13] iio: health/afe440x: Match LED currents to stages Andrew F. Davis @ 2016-05-04 10:13 ` Jonathan Cameron 0 siblings, 0 replies; 32+ messages in thread From: Jonathan Cameron @ 2016-05-04 10:13 UTC (permalink / raw) To: Andrew F. Davis, Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald Cc: linux-iio, linux-api, linux-kernel On 01/05/16 21:37, Andrew F. Davis wrote: > The current channel number for the LEDs should match the stage > number that they are active during, fix this here. > > Signed-off-by: Andrew F. Davis <afd@ti.com> Applied. > --- > drivers/iio/health/afe4403.c | 10 ++++------ > drivers/iio/health/afe4404.c | 15 ++++++--------- > 2 files changed, 10 insertions(+), 15 deletions(-) > > diff --git a/drivers/iio/health/afe4403.c b/drivers/iio/health/afe4403.c > index 610631b..059d521 100644 > --- a/drivers/iio/health/afe4403.c > +++ b/drivers/iio/health/afe4403.c > @@ -126,8 +126,6 @@ enum afe4403_chan_id { > ALED1, > LED2_ALED2, > LED1_ALED1, > - ILED1, > - ILED2, > }; > > static const unsigned int afe4403_channel_values[] = { > @@ -140,8 +138,8 @@ static const unsigned int afe4403_channel_values[] = { > }; > > static const unsigned int afe4403_channel_leds[] = { > - [ILED1] = F_ILED1, > - [ILED2] = F_ILED2, > + [LED2] = F_ILED2, > + [LED1] = F_ILED1, > }; > > static const struct iio_chan_spec afe4403_channels[] = { > @@ -153,8 +151,8 @@ static const struct iio_chan_spec afe4403_channels[] = { > AFE440X_INTENSITY_CHAN(LED2_ALED2, 0), > AFE440X_INTENSITY_CHAN(LED1_ALED1, 0), > /* LED current */ > - AFE440X_CURRENT_CHAN(ILED1), > - AFE440X_CURRENT_CHAN(ILED2), > + AFE440X_CURRENT_CHAN(LED2), > + AFE440X_CURRENT_CHAN(LED1), > }; > > static const struct afe440x_val_table afe4403_res_table[] = { > diff --git a/drivers/iio/health/afe4404.c b/drivers/iio/health/afe4404.c > index 69116cd..aa8770b 100644 > --- a/drivers/iio/health/afe4404.c > +++ b/drivers/iio/health/afe4404.c > @@ -128,9 +128,6 @@ enum afe4404_chan_id { > ALED1, > LED2_ALED2, > LED1_ALED1, > - ILED1, > - ILED2, > - ILED3, > }; > > static const unsigned int afe4404_channel_values[] = { > @@ -143,9 +140,9 @@ static const unsigned int afe4404_channel_values[] = { > }; > > static const unsigned int afe4404_channel_leds[] = { > - [ILED1] = F_ILED1, > - [ILED2] = F_ILED2, > - [ILED3] = F_ILED3, > + [LED2] = F_ILED2, > + [ALED2] = F_ILED3, > + [LED1] = F_ILED1, > }; > > static const unsigned int afe4404_channel_offdacs[] = { > @@ -164,9 +161,9 @@ static const struct iio_chan_spec afe4404_channels[] = { > AFE440X_INTENSITY_CHAN(LED2_ALED2, 0), > AFE440X_INTENSITY_CHAN(LED1_ALED1, 0), > /* LED current */ > - AFE440X_CURRENT_CHAN(ILED1), > - AFE440X_CURRENT_CHAN(ILED2), > - AFE440X_CURRENT_CHAN(ILED3), > + AFE440X_CURRENT_CHAN(LED2), > + AFE440X_CURRENT_CHAN(ALED2), > + AFE440X_CURRENT_CHAN(LED1), > }; > > static const struct afe440x_val_table afe4404_res_table[] = { > ^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 12/13] iio: health/afe440x: Remove unused definitions [not found] ` <1462135023-14445-1-git-send-email-afd-l0cyMroinI0@public.gmane.org> ` (4 preceding siblings ...) 2016-05-01 20:37 ` [PATCH 11/13] iio: health/afe440x: Match LED currents to stages Andrew F. Davis @ 2016-05-01 20:37 ` Andrew F. Davis 2016-05-04 10:14 ` Jonathan Cameron 2016-05-01 20:37 ` [PATCH 13/13] iio: health/afe4404: ENSEPGAIN is part of CONTROL2 register Andrew F. Davis 6 siblings, 1 reply; 32+ messages in thread From: Andrew F. Davis @ 2016-05-01 20:37 UTC (permalink / raw) To: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA, linux-api-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, Andrew F. Davis 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-l0cyMroinI0@public.gmane.org> --- 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, -- 2.8.1 ^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: [PATCH 12/13] iio: health/afe440x: Remove unused definitions 2016-05-01 20:37 ` [PATCH 12/13] iio: health/afe440x: Remove unused definitions Andrew F. Davis @ 2016-05-04 10:14 ` Jonathan Cameron 0 siblings, 0 replies; 32+ messages in thread From: Jonathan Cameron @ 2016-05-04 10:14 UTC (permalink / raw) To: Andrew F. Davis, Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald Cc: linux-iio, linux-api, linux-kernel 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, > ^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 13/13] iio: health/afe4404: ENSEPGAIN is part of CONTROL2 register [not found] ` <1462135023-14445-1-git-send-email-afd-l0cyMroinI0@public.gmane.org> ` (5 preceding siblings ...) 2016-05-01 20:37 ` [PATCH 12/13] iio: health/afe440x: Remove unused definitions Andrew F. Davis @ 2016-05-01 20:37 ` Andrew F. Davis 2016-05-04 10:14 ` Jonathan Cameron 6 siblings, 1 reply; 32+ messages in thread From: Andrew F. Davis @ 2016-05-01 20:37 UTC (permalink / raw) To: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA, linux-api-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, Andrew F. Davis Rename this definition, no functional changes. Signed-off-by: Andrew F. Davis <afd-l0cyMroinI0@public.gmane.org> --- drivers/iio/health/afe4404.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/iio/health/afe4404.c b/drivers/iio/health/afe4404.c index 3a8131d..4526640 100644 --- a/drivers/iio/health/afe4404.c +++ b/drivers/iio/health/afe4404.c @@ -48,8 +48,8 @@ #define AFE4404_AVG_LED2_ALED2VAL 0x3f #define AFE4404_AVG_LED1_ALED1VAL 0x40 -/* AFE4404 CONTROL3 register fields */ -#define AFE440X_CONTROL3_OSC_ENABLE BIT(9) +/* AFE4404 CONTROL2 register fields */ +#define AFE440X_CONTROL2_OSC_ENABLE BIT(9) enum afe4404_fields { /* Gains */ @@ -398,7 +398,7 @@ static const struct reg_sequence afe4404_reg_sequences[] = { AFE4404_TIMING_PAIRS, { AFE440X_CONTROL1, AFE440X_CONTROL1_TIMEREN }, { AFE4404_TIA_GAIN_SEP, AFE440X_TIAGAIN_ENSEPGAIN }, - { AFE440X_CONTROL2, AFE440X_CONTROL3_OSC_ENABLE }, + { AFE440X_CONTROL2, AFE440X_CONTROL2_OSC_ENABLE }, }; static const struct regmap_range afe4404_yes_ranges[] = { -- 2.8.1 ^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: [PATCH 13/13] iio: health/afe4404: ENSEPGAIN is part of CONTROL2 register 2016-05-01 20:37 ` [PATCH 13/13] iio: health/afe4404: ENSEPGAIN is part of CONTROL2 register Andrew F. Davis @ 2016-05-04 10:14 ` Jonathan Cameron 0 siblings, 0 replies; 32+ messages in thread From: Jonathan Cameron @ 2016-05-04 10:14 UTC (permalink / raw) To: Andrew F. Davis, Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald Cc: linux-iio, linux-api, linux-kernel On 01/05/16 21:37, Andrew F. Davis wrote: > Rename this definition, no functional changes. > > Signed-off-by: Andrew F. Davis <afd@ti.com> Applied. > --- > drivers/iio/health/afe4404.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/iio/health/afe4404.c b/drivers/iio/health/afe4404.c > index 3a8131d..4526640 100644 > --- a/drivers/iio/health/afe4404.c > +++ b/drivers/iio/health/afe4404.c > @@ -48,8 +48,8 @@ > #define AFE4404_AVG_LED2_ALED2VAL 0x3f > #define AFE4404_AVG_LED1_ALED1VAL 0x40 > > -/* AFE4404 CONTROL3 register fields */ > -#define AFE440X_CONTROL3_OSC_ENABLE BIT(9) > +/* AFE4404 CONTROL2 register fields */ > +#define AFE440X_CONTROL2_OSC_ENABLE BIT(9) > > enum afe4404_fields { > /* Gains */ > @@ -398,7 +398,7 @@ static const struct reg_sequence afe4404_reg_sequences[] = { > AFE4404_TIMING_PAIRS, > { AFE440X_CONTROL1, AFE440X_CONTROL1_TIMEREN }, > { AFE4404_TIA_GAIN_SEP, AFE440X_TIAGAIN_ENSEPGAIN }, > - { AFE440X_CONTROL2, AFE440X_CONTROL3_OSC_ENABLE }, > + { AFE440X_CONTROL2, AFE440X_CONTROL2_OSC_ENABLE }, > }; > > static const struct regmap_range afe4404_yes_ranges[] = { > ^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 06/13] iio: health/afe440x: Remove unneeded offset handling 2016-05-01 20:36 [PATCH 00/13] Rework for AFE440x drivers to prepare for AFE4405 Andrew F. Davis ` (3 preceding siblings ...) [not found] ` <1462135023-14445-1-git-send-email-afd-l0cyMroinI0@public.gmane.org> @ 2016-05-01 20:36 ` Andrew F. Davis [not found] ` <1462135023-14445-7-git-send-email-afd-l0cyMroinI0@public.gmane.org> 2016-05-01 20:36 ` [PATCH 07/13] iio: health/afe4404: Remove LED3 input channel Andrew F. Davis ` (2 subsequent siblings) 7 siblings, 1 reply; 32+ messages in thread From: Andrew F. Davis @ 2016-05-01 20:36 UTC (permalink / raw) To: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald Cc: linux-iio, linux-api, linux-kernel, Andrew F. Davis No channel in the afe4403 driver has IIO_CHAN_INFO_OFFSET set so remove the handlers for this. Signed-off-by: Andrew F. Davis <afd@ti.com> --- drivers/iio/health/afe4403.c | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/drivers/iio/health/afe4403.c b/drivers/iio/health/afe4403.c index bcff528..cac6090 100644 --- a/drivers/iio/health/afe4403.c +++ b/drivers/iio/health/afe4403.c @@ -293,14 +293,6 @@ static int afe4403_read_raw(struct iio_dev *indio_dev, if (ret) return ret; return IIO_VAL_INT; - case IIO_CHAN_INFO_OFFSET: - ret = regmap_read(afe->regmap, reg_info.offreg, - val); - if (ret) - return ret; - *val &= reg_info.mask; - *val >>= reg_info.shift; - return IIO_VAL_INT; } break; case IIO_CURRENT: @@ -333,15 +325,6 @@ static int afe4403_write_raw(struct iio_dev *indio_dev, const struct afe440x_reg_info reg_info = afe4403_reg_info[chan->address]; switch (chan->type) { - case IIO_INTENSITY: - switch (mask) { - case IIO_CHAN_INFO_OFFSET: - return regmap_update_bits(afe->regmap, - reg_info.offreg, - reg_info.mask, - (val << reg_info.shift)); - } - break; case IIO_CURRENT: switch (mask) { case IIO_CHAN_INFO_RAW: -- 2.8.1 ^ permalink raw reply related [flat|nested] 32+ messages in thread
[parent not found: <1462135023-14445-7-git-send-email-afd-l0cyMroinI0@public.gmane.org>]
* Re: [PATCH 06/13] iio: health/afe440x: Remove unneeded offset handling [not found] ` <1462135023-14445-7-git-send-email-afd-l0cyMroinI0@public.gmane.org> @ 2016-05-04 10:04 ` Jonathan Cameron 0 siblings, 0 replies; 32+ messages in thread From: Jonathan Cameron @ 2016-05-04 10:04 UTC (permalink / raw) To: Andrew F. Davis, Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA, linux-api-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA On 01/05/16 21:36, Andrew F. Davis wrote: > No channel in the afe4403 driver has IIO_CHAN_INFO_OFFSET set so > remove the handlers for this. > > Signed-off-by: Andrew F. Davis <afd-l0cyMroinI0@public.gmane.org> Oops :) Applied. > --- > drivers/iio/health/afe4403.c | 17 ----------------- > 1 file changed, 17 deletions(-) > > diff --git a/drivers/iio/health/afe4403.c b/drivers/iio/health/afe4403.c > index bcff528..cac6090 100644 > --- a/drivers/iio/health/afe4403.c > +++ b/drivers/iio/health/afe4403.c > @@ -293,14 +293,6 @@ static int afe4403_read_raw(struct iio_dev *indio_dev, > if (ret) > return ret; > return IIO_VAL_INT; > - case IIO_CHAN_INFO_OFFSET: > - ret = regmap_read(afe->regmap, reg_info.offreg, > - val); > - if (ret) > - return ret; > - *val &= reg_info.mask; > - *val >>= reg_info.shift; > - return IIO_VAL_INT; > } > break; > case IIO_CURRENT: > @@ -333,15 +325,6 @@ static int afe4403_write_raw(struct iio_dev *indio_dev, > const struct afe440x_reg_info reg_info = afe4403_reg_info[chan->address]; > > switch (chan->type) { > - case IIO_INTENSITY: > - switch (mask) { > - case IIO_CHAN_INFO_OFFSET: > - return regmap_update_bits(afe->regmap, > - reg_info.offreg, > - reg_info.mask, > - (val << reg_info.shift)); > - } > - break; > case IIO_CURRENT: > switch (mask) { > case IIO_CHAN_INFO_RAW: > ^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 07/13] iio: health/afe4404: Remove LED3 input channel 2016-05-01 20:36 [PATCH 00/13] Rework for AFE440x drivers to prepare for AFE4405 Andrew F. Davis ` (4 preceding siblings ...) 2016-05-01 20:36 ` [PATCH 06/13] iio: health/afe440x: Remove unneeded offset handling Andrew F. Davis @ 2016-05-01 20:36 ` Andrew F. Davis 2016-05-04 10:04 ` Jonathan Cameron 2016-05-01 20:37 ` [PATCH 10/13] iio: health/afe440x: Make gain settings a modifier for the stages Andrew F. Davis 2016-05-04 10:25 ` [PATCH 00/13] Rework for AFE440x drivers to prepare for AFE4405 Jonathan Cameron 7 siblings, 1 reply; 32+ messages in thread From: Andrew F. Davis @ 2016-05-01 20:36 UTC (permalink / raw) To: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald Cc: linux-iio, linux-api, linux-kernel, Andrew F. Davis Input channel LED3 is only an alias for stage ALED2, this virtual channel does nothing for us, remove this channel. Signed-off-by: Andrew F. Davis <afd@ti.com> --- drivers/iio/health/afe4404.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/iio/health/afe4404.c b/drivers/iio/health/afe4404.c index b9c1666..2edb7d7 100644 --- a/drivers/iio/health/afe4404.c +++ b/drivers/iio/health/afe4404.c @@ -126,7 +126,6 @@ enum afe4404_chan_id { ALED1, LED2, ALED2, - LED3, LED1_ALED1, LED2_ALED2, ILED1, @@ -139,7 +138,6 @@ static const struct afe440x_reg_info afe4404_reg_info[] = { [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), - [LED3] = AFE440X_REG_INFO(AFE440X_ALED2VAL, 0, NULL), [LED1_ALED1] = AFE440X_REG_INFO(AFE440X_LED1_ALED1VAL, 0, NULL), [LED2_ALED2] = AFE440X_REG_INFO(AFE440X_LED2_ALED2VAL, 0, NULL), [ILED1] = AFE440X_REG_INFO(AFE440X_LEDCNTRL, 0, AFE4404_LEDCNTRL_ILED1), @@ -153,7 +151,6 @@ static const struct iio_chan_spec afe4404_channels[] = { 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(LED3, "led3", BIT(IIO_CHAN_INFO_OFFSET)), AFE440X_INTENSITY_CHAN(LED1_ALED1, "led1-led1_ambient", 0), AFE440X_INTENSITY_CHAN(LED2_ALED2, "led2-led2_ambient", 0), /* LED current */ -- 2.8.1 ^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: [PATCH 07/13] iio: health/afe4404: Remove LED3 input channel 2016-05-01 20:36 ` [PATCH 07/13] iio: health/afe4404: Remove LED3 input channel Andrew F. Davis @ 2016-05-04 10:04 ` Jonathan Cameron 0 siblings, 0 replies; 32+ messages in thread From: Jonathan Cameron @ 2016-05-04 10:04 UTC (permalink / raw) To: Andrew F. Davis, Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald Cc: linux-iio, linux-api, linux-kernel On 01/05/16 21:36, Andrew F. Davis wrote: > Input channel LED3 is only an alias for stage ALED2, this virtual channel > does nothing for us, remove this channel. > > Signed-off-by: Andrew F. Davis <afd@ti.com> Again, abi change, but I doubt anyone will notice *fingers crossed* ;) Jonathan > --- > drivers/iio/health/afe4404.c | 3 --- > 1 file changed, 3 deletions(-) > > diff --git a/drivers/iio/health/afe4404.c b/drivers/iio/health/afe4404.c > index b9c1666..2edb7d7 100644 > --- a/drivers/iio/health/afe4404.c > +++ b/drivers/iio/health/afe4404.c > @@ -126,7 +126,6 @@ enum afe4404_chan_id { > ALED1, > LED2, > ALED2, > - LED3, > LED1_ALED1, > LED2_ALED2, > ILED1, > @@ -139,7 +138,6 @@ static const struct afe440x_reg_info afe4404_reg_info[] = { > [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), > - [LED3] = AFE440X_REG_INFO(AFE440X_ALED2VAL, 0, NULL), > [LED1_ALED1] = AFE440X_REG_INFO(AFE440X_LED1_ALED1VAL, 0, NULL), > [LED2_ALED2] = AFE440X_REG_INFO(AFE440X_LED2_ALED2VAL, 0, NULL), > [ILED1] = AFE440X_REG_INFO(AFE440X_LEDCNTRL, 0, AFE4404_LEDCNTRL_ILED1), > @@ -153,7 +151,6 @@ static const struct iio_chan_spec afe4404_channels[] = { > 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(LED3, "led3", BIT(IIO_CHAN_INFO_OFFSET)), > AFE440X_INTENSITY_CHAN(LED1_ALED1, "led1-led1_ambient", 0), > AFE440X_INTENSITY_CHAN(LED2_ALED2, "led2-led2_ambient", 0), > /* LED current */ > ^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 10/13] iio: health/afe440x: Make gain settings a modifier for the stages 2016-05-01 20:36 [PATCH 00/13] Rework for AFE440x drivers to prepare for AFE4405 Andrew F. Davis ` (5 preceding siblings ...) 2016-05-01 20:36 ` [PATCH 07/13] iio: health/afe4404: Remove LED3 input channel Andrew F. Davis @ 2016-05-01 20:37 ` Andrew F. Davis 2016-05-04 10:12 ` Jonathan Cameron 2016-05-04 10:25 ` [PATCH 00/13] Rework for AFE440x drivers to prepare for AFE4405 Jonathan Cameron 7 siblings, 1 reply; 32+ messages in thread From: Andrew F. Davis @ 2016-05-01 20:37 UTC (permalink / raw) To: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald Cc: linux-iio, linux-api, linux-kernel, Andrew F. Davis Currently the TIA gain settings are exported to userspace as sysfs entries that do not clearly represent their internal relation to the sampling stages. The gain settings are enabled on a per-stage basis, this can be seen in figure 24 of the current AFE4404 datasheet. These gain settings should therefore be tied to the channels that are read during these stages. Make this change here. Signed-off-by: Andrew F. Davis <afd@ti.com> --- .../ABI/testing/sysfs-bus-iio-health-afe440x | 19 ++++++------ drivers/iio/health/afe4403.c | 34 ++++++++++++++-------- drivers/iio/health/afe4404.c | 34 ++++++++++++++-------- 3 files changed, 53 insertions(+), 34 deletions(-) diff --git a/Documentation/ABI/testing/sysfs-bus-iio-health-afe440x b/Documentation/ABI/testing/sysfs-bus-iio-health-afe440x index a067073..6adba90 100644 --- a/Documentation/ABI/testing/sysfs-bus-iio-health-afe440x +++ b/Documentation/ABI/testing/sysfs-bus-iio-health-afe440x @@ -1,13 +1,3 @@ -What: /sys/bus/iio/devices/iio:deviceX/tia_resistanceY - /sys/bus/iio/devices/iio:deviceX/tia_capacitanceY -Date: December 2015 -KernelVersion: -Contact: Andrew F. Davis <afd@ti.com> -Description: - Get and set the resistance and the capacitance settings for the - 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_intensityY_raw Date: May 2016 KernelVersion: @@ -33,6 +23,15 @@ 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/in_intensityY_resistance +What: /sys/bus/iio/devices/iio:deviceX/in_intensityY_capacitance +Date: May 2016 +KernelVersion: +Contact: Andrew F. Davis <afd@ti.com> +Description: + Get and set the resistance and the capacitance settings for the + Transimpedance Amplifier during the associated stage. + What: /sys/bus/iio/devices/iio:deviceX/out_currentY_raw Date: May 2016 KernelVersion: diff --git a/drivers/iio/health/afe4403.c b/drivers/iio/health/afe4403.c index 1950155..610631b 100644 --- a/drivers/iio/health/afe4403.c +++ b/drivers/iio/health/afe4403.c @@ -161,7 +161,7 @@ static const struct afe440x_val_table afe4403_res_table[] = { { 500000 }, { 250000 }, { 100000 }, { 50000 }, { 25000 }, { 10000 }, { 1000000 }, { 0 }, }; -AFE440X_TABLE_ATTR(tia_resistance_available, afe4403_res_table); +AFE440X_TABLE_ATTR(in_intensity_resistance_available, afe4403_res_table); static const struct afe440x_val_table afe4403_cap_table[] = { { 0, 5000 }, { 0, 10000 }, { 0, 20000 }, { 0, 25000 }, @@ -173,7 +173,7 @@ static const struct afe440x_val_table afe4403_cap_table[] = { { 0, 205000 }, { 0, 210000 }, { 0, 220000 }, { 0, 225000 }, { 0, 230000 }, { 0, 235000 }, { 0, 245000 }, { 0, 250000 }, }; -AFE440X_TABLE_ATTR(tia_capacitance_available, afe4403_cap_table); +AFE440X_TABLE_ATTR(in_intensity_capacitance_available, afe4403_cap_table); static ssize_t afe440x_show_register(struct device *dev, struct device_attribute *attr, @@ -226,19 +226,29 @@ static ssize_t afe440x_store_register(struct device *dev, return count; } -static AFE440X_ATTR(tia_resistance1, F_RF_LED1, afe4403_res_table); -static AFE440X_ATTR(tia_capacitance1, F_CF_LED1, afe4403_cap_table); +static AFE440X_ATTR(in_intensity1_resistance, F_RF_LED, afe4403_res_table); +static AFE440X_ATTR(in_intensity1_capacitance, F_CF_LED, afe4403_cap_table); -static AFE440X_ATTR(tia_resistance2, F_RF_LED, afe4403_res_table); -static AFE440X_ATTR(tia_capacitance2, F_CF_LED, afe4403_cap_table); +static AFE440X_ATTR(in_intensity2_resistance, F_RF_LED, afe4403_res_table); +static AFE440X_ATTR(in_intensity2_capacitance, F_CF_LED, afe4403_cap_table); + +static AFE440X_ATTR(in_intensity3_resistance, F_RF_LED1, afe4403_res_table); +static AFE440X_ATTR(in_intensity3_capacitance, F_CF_LED1, afe4403_cap_table); + +static AFE440X_ATTR(in_intensity4_resistance, F_RF_LED1, afe4403_res_table); +static AFE440X_ATTR(in_intensity4_capacitance, F_CF_LED1, afe4403_cap_table); static struct attribute *afe440x_attributes[] = { - &afe440x_attr_tia_resistance1.dev_attr.attr, - &afe440x_attr_tia_capacitance1.dev_attr.attr, - &afe440x_attr_tia_resistance2.dev_attr.attr, - &afe440x_attr_tia_capacitance2.dev_attr.attr, - &dev_attr_tia_resistance_available.attr, - &dev_attr_tia_capacitance_available.attr, + &dev_attr_in_intensity_resistance_available.attr, + &dev_attr_in_intensity_capacitance_available.attr, + &afe440x_attr_in_intensity1_resistance.dev_attr.attr, + &afe440x_attr_in_intensity1_capacitance.dev_attr.attr, + &afe440x_attr_in_intensity2_resistance.dev_attr.attr, + &afe440x_attr_in_intensity2_capacitance.dev_attr.attr, + &afe440x_attr_in_intensity3_resistance.dev_attr.attr, + &afe440x_attr_in_intensity3_capacitance.dev_attr.attr, + &afe440x_attr_in_intensity4_resistance.dev_attr.attr, + &afe440x_attr_in_intensity4_capacitance.dev_attr.attr, NULL }; diff --git a/drivers/iio/health/afe4404.c b/drivers/iio/health/afe4404.c index 0d1af4a..69116cd 100644 --- a/drivers/iio/health/afe4404.c +++ b/drivers/iio/health/afe4404.c @@ -179,7 +179,7 @@ static const struct afe440x_val_table afe4404_res_table[] = { { .integer = 1000000, .fract = 0 }, { .integer = 2000000, .fract = 0 }, }; -AFE440X_TABLE_ATTR(tia_resistance_available, afe4404_res_table); +AFE440X_TABLE_ATTR(in_intensity_resistance_available, afe4404_res_table); static const struct afe440x_val_table afe4404_cap_table[] = { { .integer = 0, .fract = 5000 }, @@ -191,7 +191,7 @@ static const struct afe440x_val_table afe4404_cap_table[] = { { .integer = 0, .fract = 25000 }, { .integer = 0, .fract = 22500 }, }; -AFE440X_TABLE_ATTR(tia_capacitance_available, afe4404_cap_table); +AFE440X_TABLE_ATTR(in_intensity_capacitance_available, afe4404_cap_table); static ssize_t afe440x_show_register(struct device *dev, struct device_attribute *attr, @@ -244,19 +244,29 @@ static ssize_t afe440x_store_register(struct device *dev, return count; } -static AFE440X_ATTR(tia_resistance1, F_TIA_GAIN, afe4404_res_table); -static AFE440X_ATTR(tia_capacitance1, TIA_CF, afe4404_cap_table); +static AFE440X_ATTR(in_intensity1_resistance, F_TIA_GAIN_SEP, afe4404_res_table); +static AFE440X_ATTR(in_intensity1_capacitance, F_TIA_CF_SEP, afe4404_cap_table); -static AFE440X_ATTR(tia_resistance2, F_TIA_GAIN_SEP, afe4404_res_table); -static AFE440X_ATTR(tia_capacitance2, F_TIA_CF_SEP, afe4404_cap_table); +static AFE440X_ATTR(in_intensity2_resistance, F_TIA_GAIN_SEP, afe4404_res_table); +static AFE440X_ATTR(in_intensity2_capacitance, F_TIA_CF_SEP, afe4404_cap_table); + +static AFE440X_ATTR(in_intensity3_resistance, F_TIA_GAIN, afe4404_res_table); +static AFE440X_ATTR(in_intensity3_capacitance, TIA_CF, afe4404_cap_table); + +static AFE440X_ATTR(in_intensity4_resistance, F_TIA_GAIN, afe4404_res_table); +static AFE440X_ATTR(in_intensity4_capacitance, TIA_CF, afe4404_cap_table); static struct attribute *afe440x_attributes[] = { - &afe440x_attr_tia_resistance1.dev_attr.attr, - &afe440x_attr_tia_capacitance1.dev_attr.attr, - &afe440x_attr_tia_resistance2.dev_attr.attr, - &afe440x_attr_tia_capacitance2.dev_attr.attr, - &dev_attr_tia_resistance_available.attr, - &dev_attr_tia_capacitance_available.attr, + &dev_attr_in_intensity_resistance_available.attr, + &dev_attr_in_intensity_capacitance_available.attr, + &afe440x_attr_in_intensity1_resistance.dev_attr.attr, + &afe440x_attr_in_intensity1_capacitance.dev_attr.attr, + &afe440x_attr_in_intensity2_resistance.dev_attr.attr, + &afe440x_attr_in_intensity2_capacitance.dev_attr.attr, + &afe440x_attr_in_intensity3_resistance.dev_attr.attr, + &afe440x_attr_in_intensity3_capacitance.dev_attr.attr, + &afe440x_attr_in_intensity4_resistance.dev_attr.attr, + &afe440x_attr_in_intensity4_capacitance.dev_attr.attr, NULL }; -- 2.8.1 ^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: [PATCH 10/13] iio: health/afe440x: Make gain settings a modifier for the stages 2016-05-01 20:37 ` [PATCH 10/13] iio: health/afe440x: Make gain settings a modifier for the stages Andrew F. Davis @ 2016-05-04 10:12 ` Jonathan Cameron 0 siblings, 0 replies; 32+ messages in thread From: Jonathan Cameron @ 2016-05-04 10:12 UTC (permalink / raw) To: Andrew F. Davis, Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald Cc: linux-iio, linux-api, linux-kernel On 01/05/16 21:37, Andrew F. Davis wrote: > Currently the TIA gain settings are exported to userspace as sysfs > entries that do not clearly represent their internal relation to the > sampling stages. The gain settings are enabled on a per-stage basis, > this can be seen in figure 24 of the current AFE4404 datasheet. > These gain settings should therefore be tied to the channels that are > read during these stages. Make this change here. > > Signed-off-by: Andrew F. Davis <afd@ti.com> I'd not understood this right originally. Good to clear this up! Applied. I'll add a reply to the cover letter later explaining why I am risking these ABI changes, whilst bouncing back much smaller ones. Jonathan > --- > .../ABI/testing/sysfs-bus-iio-health-afe440x | 19 ++++++------ > drivers/iio/health/afe4403.c | 34 ++++++++++++++-------- > drivers/iio/health/afe4404.c | 34 ++++++++++++++-------- > 3 files changed, 53 insertions(+), 34 deletions(-) > > diff --git a/Documentation/ABI/testing/sysfs-bus-iio-health-afe440x b/Documentation/ABI/testing/sysfs-bus-iio-health-afe440x > index a067073..6adba90 100644 > --- a/Documentation/ABI/testing/sysfs-bus-iio-health-afe440x > +++ b/Documentation/ABI/testing/sysfs-bus-iio-health-afe440x > @@ -1,13 +1,3 @@ > -What: /sys/bus/iio/devices/iio:deviceX/tia_resistanceY > - /sys/bus/iio/devices/iio:deviceX/tia_capacitanceY > -Date: December 2015 > -KernelVersion: > -Contact: Andrew F. Davis <afd@ti.com> > -Description: > - Get and set the resistance and the capacitance settings for the > - 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_intensityY_raw > Date: May 2016 > KernelVersion: > @@ -33,6 +23,15 @@ 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/in_intensityY_resistance > +What: /sys/bus/iio/devices/iio:deviceX/in_intensityY_capacitance > +Date: May 2016 > +KernelVersion: > +Contact: Andrew F. Davis <afd@ti.com> > +Description: > + Get and set the resistance and the capacitance settings for the > + Transimpedance Amplifier during the associated stage. > + > What: /sys/bus/iio/devices/iio:deviceX/out_currentY_raw > Date: May 2016 > KernelVersion: > diff --git a/drivers/iio/health/afe4403.c b/drivers/iio/health/afe4403.c > index 1950155..610631b 100644 > --- a/drivers/iio/health/afe4403.c > +++ b/drivers/iio/health/afe4403.c > @@ -161,7 +161,7 @@ static const struct afe440x_val_table afe4403_res_table[] = { > { 500000 }, { 250000 }, { 100000 }, { 50000 }, > { 25000 }, { 10000 }, { 1000000 }, { 0 }, > }; > -AFE440X_TABLE_ATTR(tia_resistance_available, afe4403_res_table); > +AFE440X_TABLE_ATTR(in_intensity_resistance_available, afe4403_res_table); > > static const struct afe440x_val_table afe4403_cap_table[] = { > { 0, 5000 }, { 0, 10000 }, { 0, 20000 }, { 0, 25000 }, > @@ -173,7 +173,7 @@ static const struct afe440x_val_table afe4403_cap_table[] = { > { 0, 205000 }, { 0, 210000 }, { 0, 220000 }, { 0, 225000 }, > { 0, 230000 }, { 0, 235000 }, { 0, 245000 }, { 0, 250000 }, > }; > -AFE440X_TABLE_ATTR(tia_capacitance_available, afe4403_cap_table); > +AFE440X_TABLE_ATTR(in_intensity_capacitance_available, afe4403_cap_table); > > static ssize_t afe440x_show_register(struct device *dev, > struct device_attribute *attr, > @@ -226,19 +226,29 @@ static ssize_t afe440x_store_register(struct device *dev, > return count; > } > > -static AFE440X_ATTR(tia_resistance1, F_RF_LED1, afe4403_res_table); > -static AFE440X_ATTR(tia_capacitance1, F_CF_LED1, afe4403_cap_table); > +static AFE440X_ATTR(in_intensity1_resistance, F_RF_LED, afe4403_res_table); > +static AFE440X_ATTR(in_intensity1_capacitance, F_CF_LED, afe4403_cap_table); > > -static AFE440X_ATTR(tia_resistance2, F_RF_LED, afe4403_res_table); > -static AFE440X_ATTR(tia_capacitance2, F_CF_LED, afe4403_cap_table); > +static AFE440X_ATTR(in_intensity2_resistance, F_RF_LED, afe4403_res_table); > +static AFE440X_ATTR(in_intensity2_capacitance, F_CF_LED, afe4403_cap_table); > + > +static AFE440X_ATTR(in_intensity3_resistance, F_RF_LED1, afe4403_res_table); > +static AFE440X_ATTR(in_intensity3_capacitance, F_CF_LED1, afe4403_cap_table); > + > +static AFE440X_ATTR(in_intensity4_resistance, F_RF_LED1, afe4403_res_table); > +static AFE440X_ATTR(in_intensity4_capacitance, F_CF_LED1, afe4403_cap_table); > > static struct attribute *afe440x_attributes[] = { > - &afe440x_attr_tia_resistance1.dev_attr.attr, > - &afe440x_attr_tia_capacitance1.dev_attr.attr, > - &afe440x_attr_tia_resistance2.dev_attr.attr, > - &afe440x_attr_tia_capacitance2.dev_attr.attr, > - &dev_attr_tia_resistance_available.attr, > - &dev_attr_tia_capacitance_available.attr, > + &dev_attr_in_intensity_resistance_available.attr, > + &dev_attr_in_intensity_capacitance_available.attr, > + &afe440x_attr_in_intensity1_resistance.dev_attr.attr, > + &afe440x_attr_in_intensity1_capacitance.dev_attr.attr, > + &afe440x_attr_in_intensity2_resistance.dev_attr.attr, > + &afe440x_attr_in_intensity2_capacitance.dev_attr.attr, > + &afe440x_attr_in_intensity3_resistance.dev_attr.attr, > + &afe440x_attr_in_intensity3_capacitance.dev_attr.attr, > + &afe440x_attr_in_intensity4_resistance.dev_attr.attr, > + &afe440x_attr_in_intensity4_capacitance.dev_attr.attr, > NULL > }; > > diff --git a/drivers/iio/health/afe4404.c b/drivers/iio/health/afe4404.c > index 0d1af4a..69116cd 100644 > --- a/drivers/iio/health/afe4404.c > +++ b/drivers/iio/health/afe4404.c > @@ -179,7 +179,7 @@ static const struct afe440x_val_table afe4404_res_table[] = { > { .integer = 1000000, .fract = 0 }, > { .integer = 2000000, .fract = 0 }, > }; > -AFE440X_TABLE_ATTR(tia_resistance_available, afe4404_res_table); > +AFE440X_TABLE_ATTR(in_intensity_resistance_available, afe4404_res_table); > > static const struct afe440x_val_table afe4404_cap_table[] = { > { .integer = 0, .fract = 5000 }, > @@ -191,7 +191,7 @@ static const struct afe440x_val_table afe4404_cap_table[] = { > { .integer = 0, .fract = 25000 }, > { .integer = 0, .fract = 22500 }, > }; > -AFE440X_TABLE_ATTR(tia_capacitance_available, afe4404_cap_table); > +AFE440X_TABLE_ATTR(in_intensity_capacitance_available, afe4404_cap_table); > > static ssize_t afe440x_show_register(struct device *dev, > struct device_attribute *attr, > @@ -244,19 +244,29 @@ static ssize_t afe440x_store_register(struct device *dev, > return count; > } > > -static AFE440X_ATTR(tia_resistance1, F_TIA_GAIN, afe4404_res_table); > -static AFE440X_ATTR(tia_capacitance1, TIA_CF, afe4404_cap_table); > +static AFE440X_ATTR(in_intensity1_resistance, F_TIA_GAIN_SEP, afe4404_res_table); > +static AFE440X_ATTR(in_intensity1_capacitance, F_TIA_CF_SEP, afe4404_cap_table); > > -static AFE440X_ATTR(tia_resistance2, F_TIA_GAIN_SEP, afe4404_res_table); > -static AFE440X_ATTR(tia_capacitance2, F_TIA_CF_SEP, afe4404_cap_table); > +static AFE440X_ATTR(in_intensity2_resistance, F_TIA_GAIN_SEP, afe4404_res_table); > +static AFE440X_ATTR(in_intensity2_capacitance, F_TIA_CF_SEP, afe4404_cap_table); > + > +static AFE440X_ATTR(in_intensity3_resistance, F_TIA_GAIN, afe4404_res_table); > +static AFE440X_ATTR(in_intensity3_capacitance, TIA_CF, afe4404_cap_table); > + > +static AFE440X_ATTR(in_intensity4_resistance, F_TIA_GAIN, afe4404_res_table); > +static AFE440X_ATTR(in_intensity4_capacitance, TIA_CF, afe4404_cap_table); > > static struct attribute *afe440x_attributes[] = { > - &afe440x_attr_tia_resistance1.dev_attr.attr, > - &afe440x_attr_tia_capacitance1.dev_attr.attr, > - &afe440x_attr_tia_resistance2.dev_attr.attr, > - &afe440x_attr_tia_capacitance2.dev_attr.attr, > - &dev_attr_tia_resistance_available.attr, > - &dev_attr_tia_capacitance_available.attr, > + &dev_attr_in_intensity_resistance_available.attr, > + &dev_attr_in_intensity_capacitance_available.attr, > + &afe440x_attr_in_intensity1_resistance.dev_attr.attr, > + &afe440x_attr_in_intensity1_capacitance.dev_attr.attr, > + &afe440x_attr_in_intensity2_resistance.dev_attr.attr, > + &afe440x_attr_in_intensity2_capacitance.dev_attr.attr, > + &afe440x_attr_in_intensity3_resistance.dev_attr.attr, > + &afe440x_attr_in_intensity3_capacitance.dev_attr.attr, > + &afe440x_attr_in_intensity4_resistance.dev_attr.attr, > + &afe440x_attr_in_intensity4_capacitance.dev_attr.attr, > NULL > }; > > ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 00/13] Rework for AFE440x drivers to prepare for AFE4405 2016-05-01 20:36 [PATCH 00/13] Rework for AFE440x drivers to prepare for AFE4405 Andrew F. Davis ` (6 preceding siblings ...) 2016-05-01 20:37 ` [PATCH 10/13] iio: health/afe440x: Make gain settings a modifier for the stages Andrew F. Davis @ 2016-05-04 10:25 ` Jonathan Cameron 7 siblings, 0 replies; 32+ messages in thread From: Jonathan Cameron @ 2016-05-04 10:25 UTC (permalink / raw) To: Andrew F. Davis, Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald Cc: linux-iio, linux-api, linux-kernel 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%) > ^ permalink raw reply [flat|nested] 32+ messages in thread
end of thread, other threads:[~2016-05-04 18:33 UTC | newest]
Thread overview: 32+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-05-01 20:36 [PATCH 00/13] Rework for AFE440x drivers to prepare for AFE4405 Andrew F. Davis
2016-05-01 20:36 ` [PATCH 01/13] iio: health/afe440x: Fix kernel-doc format Andrew F. Davis
[not found] ` <1462135023-14445-2-git-send-email-afd-l0cyMroinI0@public.gmane.org>
2016-05-04 9:58 ` Jonathan Cameron
2016-05-01 20:36 ` [PATCH 02/13] iio: health/afe440x: Remove of_match_ptr and ifdefs Andrew F. Davis
2016-05-04 9:59 ` Jonathan Cameron
2016-05-01 20:36 ` [PATCH 04/13] iio: health/afe440x: Always use separate gain values Andrew F. Davis
2016-05-04 10:02 ` Jonathan Cameron
2016-05-04 15:13 ` Andrew F. Davis
2016-05-04 18:33 ` Jonathan Cameron
[not found] ` <1462135023-14445-1-git-send-email-afd-l0cyMroinI0@public.gmane.org>
2016-05-01 20:36 ` [PATCH 03/13] iio: health/afe440x: Remove unneeded initializers Andrew F. Davis
2016-05-04 9:59 ` Jonathan Cameron
2016-05-01 20:36 ` [PATCH 05/13] iio: health/afe440x: Fix scan_index assignment Andrew F. Davis
[not found] ` <1462135023-14445-6-git-send-email-afd-l0cyMroinI0@public.gmane.org>
2016-05-04 10:03 ` Jonathan Cameron
2016-05-01 20:36 ` [PATCH 08/13] iio: health/afe440x: Remove channel names Andrew F. Davis
2016-05-04 10:08 ` Jonathan Cameron
[not found] ` <f0679a4f-c8f0-1712-e7fc-a61350734a03-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2016-05-04 15:28 ` Andrew F. Davis
2016-05-01 20:36 ` [PATCH 09/13] iio: health/afe440x: Use regmap fields Andrew F. Davis
2016-05-04 10:10 ` Jonathan Cameron
[not found] ` <45523060-7ae8-076f-dece-34e76e8a740a-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2016-05-04 15:30 ` Andrew F. Davis
2016-05-01 20:37 ` [PATCH 11/13] iio: health/afe440x: Match LED currents to stages Andrew F. Davis
2016-05-04 10:13 ` Jonathan Cameron
2016-05-01 20:37 ` [PATCH 12/13] iio: health/afe440x: Remove unused definitions Andrew F. Davis
2016-05-04 10:14 ` Jonathan Cameron
2016-05-01 20:37 ` [PATCH 13/13] iio: health/afe4404: ENSEPGAIN is part of CONTROL2 register Andrew F. Davis
2016-05-04 10:14 ` Jonathan Cameron
2016-05-01 20:36 ` [PATCH 06/13] iio: health/afe440x: Remove unneeded offset handling Andrew F. Davis
[not found] ` <1462135023-14445-7-git-send-email-afd-l0cyMroinI0@public.gmane.org>
2016-05-04 10:04 ` Jonathan Cameron
2016-05-01 20:36 ` [PATCH 07/13] iio: health/afe4404: Remove LED3 input channel Andrew F. Davis
2016-05-04 10:04 ` Jonathan Cameron
2016-05-01 20:37 ` [PATCH 10/13] iio: health/afe440x: Make gain settings a modifier for the stages Andrew F. Davis
2016-05-04 10:12 ` Jonathan Cameron
2016-05-04 10:25 ` [PATCH 00/13] Rework for AFE440x drivers to prepare for AFE4405 Jonathan Cameron
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).