LinuxLists.cc - [PATCH 0/9] iio: use regmap to retrieve struct device

2016-04-06 05:15:39

Subject: [PATCH 0/9] iio: use regmap to retrieve struct device

These drivers include both struct regmap and struct device in their
global data. Remove the redundant copy by deleting struct device
from the drivers global data and using regmap API to retrieve when
needed.

This patchset removes all such redundancies in drivers/iio/.

Patches created using Coccinelle plus hand edits for whitespace and
one local var naming collision.

Alison Schofield (9):
iio: adc: exynos_adc: use regmap to retrieve struct device
iio: adc: qcom-spmi-iadc: use regmap to retrieve struct device
iio: adc: qcom-spmi-vadc: use regmap to retrieve struct device
iio: accel: bmc150: use regmap to retrieve struct device
iio: accel: mma7455: use regmap to retrieve struct device
iio: accel: mxc4005: use regmap to retrieve struct device
iio: health: afe4403: use regmap to retrieve struct device
iio: health: afe4404: use regmap to retrieve struct device
iio: gyro: bmg160_core: use regmap to retrieve struct device

drivers/iio/accel/bmc150-accel-core.c | 99 +++++++++++++++++++----------------
drivers/iio/accel/mma7455_core.c | 5 +-
drivers/iio/accel/mxc4005.c | 26 +++++----
drivers/iio/adc/exynos_adc.c | 18 +++----
drivers/iio/adc/qcom-spmi-iadc.c | 32 +++++------
drivers/iio/adc/qcom-spmi-vadc.c | 37 +++++++------
drivers/iio/gyro/bmg160_core.c | 86 +++++++++++++++---------------
drivers/iio/health/afe4403.c | 36 ++++++-------
drivers/iio/health/afe4404.c | 38 +++++++-------
9 files changed, 197 insertions(+), 180 deletions(-)

--
2.1.4

2016-04-06 05:16:00

by Alison Schofield

[permalink] [raw]

Subject: [PATCH 1/9] iio: adc: exynos_adc: use regmap to retrieve struct device

2016-04-06 05:16:48

by Alison Schofield

[permalink] [raw]

Subject: [PATCH 2/9] iio: adc: qcom-spmi-iadc: use regmap to retrieve struct device

2016-04-06 05:17:35

by Alison Schofield

[permalink] [raw]

Subject: [PATCH 3/9] iio: adc: qcom-spmi-vadc: use regmap to retrieve struct device

2016-04-06 05:18:16

by Alison Schofield

[permalink] [raw]

Subject: [PATCH 4/9] iio: accel: bmc150: use regmap to retrieve struct device

Driver includes struct regmap and struct device in its global data.
Remove the struct device and use regmap API to retrieve device info.

Patch created using Coccinelle plus manual edits.

Signed-off-by: Alison Schofield <[email protected]>
---
drivers/iio/accel/bmc150-accel-core.c | 99 +++++++++++++++++++----------------
1 file changed, 54 insertions(+), 45 deletions(-)

diff --git a/drivers/iio/accel/bmc150-accel-core.c b/drivers/iio/accel/bmc150-accel-core.c
index f3d096f..ad5310b 100644
--- a/drivers/iio/accel/bmc150-accel-core.c
+++ b/drivers/iio/accel/bmc150-accel-core.c
@@ -188,7 +188,6 @@ enum bmc150_accel_trigger_id {

struct bmc150_accel_data {
struct regmap *regmap;
- struct device *dev;
int irq;
struct bmc150_accel_interrupt interrupts[BMC150_ACCEL_INTERRUPTS];
atomic_t active_intr;
@@ -256,6 +255,7 @@ static int bmc150_accel_set_mode(struct bmc150_accel_data *data,
enum bmc150_power_modes mode,
int dur_us)
{
+ struct device *dev = regmap_get_device(data->regmap);
int i;
int ret;
u8 lpw_bits;
@@ -279,11 +279,11 @@ static int bmc150_accel_set_mode(struct bmc150_accel_data *data,
lpw_bits = mode << BMC150_ACCEL_PMU_MODE_SHIFT;
lpw_bits |= (dur_val << BMC150_ACCEL_PMU_BIT_SLEEP_DUR_SHIFT);

- dev_dbg(data->dev, "Set Mode bits %x\n", lpw_bits);
+ dev_dbg(dev, "Set Mode bits %x\n", lpw_bits);

ret = regmap_write(data->regmap, BMC150_ACCEL_REG_PMU_LPW, lpw_bits);
if (ret < 0) {
- dev_err(data->dev, "Error writing reg_pmu_lpw\n");
+ dev_err(dev, "Error writing reg_pmu_lpw\n");
return ret;
}

@@ -316,23 +316,24 @@ static int bmc150_accel_set_bw(struct bmc150_accel_data *data, int val,

static int bmc150_accel_update_slope(struct bmc150_accel_data *data)
{
+ struct device *dev = regmap_get_device(data->regmap);
int ret;

ret = regmap_write(data->regmap, BMC150_ACCEL_REG_INT_6,
data->slope_thres);
if (ret < 0) {
- dev_err(data->dev, "Error writing reg_int_6\n");
+ dev_err(dev, "Error writing reg_int_6\n");
return ret;
}

ret = regmap_update_bits(data->regmap, BMC150_ACCEL_REG_INT_5,
BMC150_ACCEL_SLOPE_DUR_MASK, data->slope_dur);
if (ret < 0) {
- dev_err(data->dev, "Error updating reg_int_5\n");
+ dev_err(dev, "Error updating reg_int_5\n");
return ret;
}

- dev_dbg(data->dev, "%s: %x %x\n", __func__, data->slope_thres,
+ dev_dbg(dev, "%s: %x %x\n", __func__, data->slope_thres,
data->slope_dur);

return ret;
@@ -378,20 +379,21 @@ static int bmc150_accel_get_startup_times(struct bmc150_accel_data *data)

static int bmc150_accel_set_power_state(struct bmc150_accel_data *data, bool on)
{
+ struct device *dev = regmap_get_device(data->regmap);
int ret;

if (on) {
- ret = pm_runtime_get_sync(data->dev);
+ ret = pm_runtime_get_sync(dev);
} else {
- pm_runtime_mark_last_busy(data->dev);
- ret = pm_runtime_put_autosuspend(data->dev);
+ pm_runtime_mark_last_busy(dev);
+ ret = pm_runtime_put_autosuspend(dev);
}

if (ret < 0) {
- dev_err(data->dev,
+ dev_err(dev,
"Failed: bmc150_accel_set_power_state for %d\n", on);
if (on)
- pm_runtime_put_noidle(data->dev);
+ pm_runtime_put_noidle(dev);

return ret;
}
@@ -445,6 +447,7 @@ static void bmc150_accel_interrupts_setup(struct iio_dev *indio_dev,
static int bmc150_accel_set_interrupt(struct bmc150_accel_data *data, int i,
bool state)
{
+ struct device *dev = regmap_get_device(data->regmap);
struct bmc150_accel_interrupt *intr = &data->interrupts[i];
const struct bmc150_accel_interrupt_info *info = intr->info;
int ret;
@@ -474,7 +477,7 @@ static int bmc150_accel_set_interrupt(struct bmc150_accel_data *data, int i,
ret = regmap_update_bits(data->regmap, info->map_reg, info->map_bitmask,
(state ? info->map_bitmask : 0));
if (ret < 0) {
- dev_err(data->dev, "Error updating reg_int_map\n");
+ dev_err(dev, "Error updating reg_int_map\n");
goto out_fix_power_state;
}

@@ -482,7 +485,7 @@ static int bmc150_accel_set_interrupt(struct bmc150_accel_data *data, int i,
ret = regmap_update_bits(data->regmap, info->en_reg, info->en_bitmask,
(state ? info->en_bitmask : 0));
if (ret < 0) {
- dev_err(data->dev, "Error updating reg_int_en\n");
+ dev_err(dev, "Error updating reg_int_en\n");
goto out_fix_power_state;
}

@@ -500,6 +503,7 @@ out_fix_power_state:

static int bmc150_accel_set_scale(struct bmc150_accel_data *data, int val)
{
+ struct device *dev = regmap_get_device(data->regmap);
int ret, i;

for (i = 0; i < ARRAY_SIZE(data->chip_info->scale_table); ++i) {
@@ -508,8 +512,7 @@ static int bmc150_accel_set_scale(struct bmc150_accel_data *data, int val)
BMC150_ACCEL_REG_PMU_RANGE,
data->chip_info->scale_table[i].reg_range);
if (ret < 0) {
- dev_err(data->dev,
- "Error writing pmu_range\n");
+ dev_err(dev, "Error writing pmu_range\n");
return ret;
}

@@ -523,6 +526,7 @@ static int bmc150_accel_set_scale(struct bmc150_accel_data *data, int val)

static int bmc150_accel_get_temp(struct bmc150_accel_data *data, int *val)
{
+ struct device *dev = regmap_get_device(data->regmap);
int ret;
unsigned int value;

@@ -530,7 +534,7 @@ static int bmc150_accel_get_temp(struct bmc150_accel_data *data, int *val)

ret = regmap_read(data->regmap, BMC150_ACCEL_REG_TEMP, &value);
if (ret < 0) {
- dev_err(data->dev, "Error reading reg_temp\n");
+ dev_err(dev, "Error reading reg_temp\n");
mutex_unlock(&data->mutex);
return ret;
}
@@ -545,6 +549,7 @@ static int bmc150_accel_get_axis(struct bmc150_accel_data *data,
struct iio_chan_spec const *chan,
int *val)
{
+ struct device *dev = regmap_get_device(data->regmap);
int ret;
int axis = chan->scan_index;
unsigned int raw_val;
@@ -559,7 +564,7 @@ static int bmc150_accel_get_axis(struct bmc150_accel_data *data,
ret = regmap_bulk_read(data->regmap, BMC150_ACCEL_AXIS_TO_REG(axis),
&raw_val, 2);
if (ret < 0) {
- dev_err(data->dev, "Error reading axis %d\n", axis);
+ dev_err(dev, "Error reading axis %d\n", axis);
bmc150_accel_set_power_state(data, false);
mutex_unlock(&data->mutex);
return ret;
@@ -831,6 +836,7 @@ static int bmc150_accel_set_watermark(struct iio_dev *indio_dev, unsigned val)
static int bmc150_accel_fifo_transfer(struct bmc150_accel_data *data,
char *buffer, int samples)
{
+ struct device *dev = regmap_get_device(data->regmap);
int sample_length = 3 * 2;
int ret;
int total_length = samples * sample_length;
@@ -854,7 +860,8 @@ static int bmc150_accel_fifo_transfer(struct bmc150_accel_data *data,
}

if (ret)
- dev_err(data->dev, "Error transferring data from fifo in single steps of %zu\n",
+ dev_err(dev,
+ "Error transferring data from fifo in single steps of %zu\n",
step);

return ret;
@@ -864,6 +871,7 @@ static int __bmc150_accel_fifo_flush(struct iio_dev *indio_dev,
unsigned samples, bool irq)
{
struct bmc150_accel_data *data = iio_priv(indio_dev);
+ struct device *dev = regmap_get_device(data->regmap);
int ret, i;
u8 count;
u16 buffer[BMC150_ACCEL_FIFO_LENGTH * 3];
@@ -873,7 +881,7 @@ static int __bmc150_accel_fifo_flush(struct iio_dev *indio_dev,

ret = regmap_read(data->regmap, BMC150_ACCEL_REG_FIFO_STATUS, &val);
if (ret < 0) {
- dev_err(data->dev, "Error reading reg_fifo_status\n");
+ dev_err(dev, "Error reading reg_fifo_status\n");
return ret;
}

@@ -1135,6 +1143,7 @@ static int bmc150_accel_trig_try_reen(struct iio_trigger *trig)
{
struct bmc150_accel_trigger *t = iio_trigger_get_drvdata(trig);
struct bmc150_accel_data *data = t->data;
+ struct device *dev = regmap_get_device(data->regmap);
int ret;

/* new data interrupts don't need ack */
@@ -1148,8 +1157,7 @@ static int bmc150_accel_trig_try_reen(struct iio_trigger *trig)
BMC150_ACCEL_INT_MODE_LATCH_RESET);
mutex_unlock(&data->mutex);
if (ret < 0) {
- dev_err(data->dev,
- "Error writing reg_int_rst_latch\n");
+ dev_err(dev, "Error writing reg_int_rst_latch\n");
return ret;
}

@@ -1200,13 +1208,14 @@ static const struct iio_trigger_ops bmc150_accel_trigger_ops = {
static int bmc150_accel_handle_roc_event(struct iio_dev *indio_dev)
{
struct bmc150_accel_data *data = iio_priv(indio_dev);
+ struct device *dev = regmap_get_device(data->regmap);
int dir;
int ret;
unsigned int val;

ret = regmap_read(data->regmap, BMC150_ACCEL_REG_INT_STATUS_2, &val);
if (ret < 0) {
- dev_err(data->dev, "Error reading reg_int_status_2\n");
+ dev_err(dev, "Error reading reg_int_status_2\n");
return ret;
}

@@ -1249,6 +1258,7 @@ static irqreturn_t bmc150_accel_irq_thread_handler(int irq, void *private)
{
struct iio_dev *indio_dev = private;
struct bmc150_accel_data *data = iio_priv(indio_dev);
+ struct device *dev = regmap_get_device(data->regmap);
bool ack = false;
int ret;

@@ -1272,7 +1282,7 @@ static irqreturn_t bmc150_accel_irq_thread_handler(int irq, void *private)
BMC150_ACCEL_INT_MODE_LATCH_INT |
BMC150_ACCEL_INT_MODE_LATCH_RESET);
if (ret)
- dev_err(data->dev, "Error writing reg_int_rst_latch\n");
+ dev_err(dev, "Error writing reg_int_rst_latch\n");

ret = IRQ_HANDLED;
} else {
@@ -1343,13 +1353,14 @@ static void bmc150_accel_unregister_triggers(struct bmc150_accel_data *data,
static int bmc150_accel_triggers_setup(struct iio_dev *indio_dev,
struct bmc150_accel_data *data)
{
+ struct device *dev = regmap_get_device(data->regmap);
int i, ret;

for (i = 0; i < BMC150_ACCEL_TRIGGERS; i++) {
struct bmc150_accel_trigger *t = &data->triggers[i];

- t->indio_trig = devm_iio_trigger_alloc(data->dev,
- bmc150_accel_triggers[i].name,
+ t->indio_trig = devm_iio_trigger_alloc(dev,
+ bmc150_accel_triggers[i].name,
indio_dev->name,
indio_dev->id);
if (!t->indio_trig) {
@@ -1357,7 +1368,7 @@ static int bmc150_accel_triggers_setup(struct iio_dev *indio_dev,
break;
}

- t->indio_trig->dev.parent = data->dev;
+ t->indio_trig->dev.parent = dev;
t->indio_trig->ops = &bmc150_accel_trigger_ops;
t->intr = bmc150_accel_triggers[i].intr;
t->data = data;
@@ -1381,12 +1392,13 @@ static int bmc150_accel_triggers_setup(struct iio_dev *indio_dev,

static int bmc150_accel_fifo_set_mode(struct bmc150_accel_data *data)
{
+ struct device *dev = regmap_get_device(data->regmap);
u8 reg = BMC150_ACCEL_REG_FIFO_CONFIG1;
int ret;

ret = regmap_write(data->regmap, reg, data->fifo_mode);
if (ret < 0) {
- dev_err(data->dev, "Error writing reg_fifo_config1\n");
+ dev_err(dev, "Error writing reg_fifo_config1\n");
return ret;
}

@@ -1396,7 +1408,7 @@ static int bmc150_accel_fifo_set_mode(struct bmc150_accel_data *data)
ret = regmap_write(data->regmap, BMC150_ACCEL_REG_FIFO_CONFIG0,
data->watermark);
if (ret < 0)
- dev_err(data->dev, "Error writing reg_fifo_config0\n");
+ dev_err(dev, "Error writing reg_fifo_config0\n");

return ret;
}
@@ -1480,17 +1492,17 @@ static const struct iio_buffer_setup_ops bmc150_accel_buffer_ops = {

static int bmc150_accel_chip_init(struct bmc150_accel_data *data)
{
+ struct device *dev = regmap_get_device(data->regmap);
int ret, i;
unsigned int val;

ret = regmap_read(data->regmap, BMC150_ACCEL_REG_CHIP_ID, &val);
if (ret < 0) {
- dev_err(data->dev,
- "Error: Reading chip id\n");
+ dev_err(dev, "Error: Reading chip id\n");
return ret;
}

- dev_dbg(data->dev, "Chip Id %x\n", val);
+ dev_dbg(dev, "Chip Id %x\n", val);
for (i = 0; i < ARRAY_SIZE(bmc150_accel_chip_info_tbl); i++) {
if (bmc150_accel_chip_info_tbl[i].chip_id == val) {
data->chip_info = &bmc150_accel_chip_info_tbl[i];
@@ -1499,7 +1511,7 @@ static int bmc150_accel_chip_init(struct bmc150_accel_data *data)
}

if (!data->chip_info) {
- dev_err(data->dev, "Invalid chip %x\n", val);
+ dev_err(dev, "Invalid chip %x\n", val);
return -ENODEV;
}

@@ -1516,8 +1528,7 @@ static int bmc150_accel_chip_init(struct bmc150_accel_data *data)
ret = regmap_write(data->regmap, BMC150_ACCEL_REG_PMU_RANGE,
BMC150_ACCEL_DEF_RANGE_4G);
if (ret < 0) {
- dev_err(data->dev,
- "Error writing reg_pmu_range\n");
+ dev_err(dev, "Error writing reg_pmu_range\n");
return ret;
}

@@ -1535,8 +1546,7 @@ static int bmc150_accel_chip_init(struct bmc150_accel_data *data)
BMC150_ACCEL_INT_MODE_LATCH_INT |
BMC150_ACCEL_INT_MODE_LATCH_RESET);
if (ret < 0) {
- dev_err(data->dev,
- "Error writing reg_int_rst_latch\n");
+ dev_err(dev, "Error writing reg_int_rst_latch\n");
return ret;
}

@@ -1556,7 +1566,6 @@ int bmc150_accel_core_probe(struct device *dev, struct regmap *regmap, int irq,

data = iio_priv(indio_dev);
dev_set_drvdata(dev, indio_dev);
- data->dev = dev;
data->irq = irq;

data->regmap = regmap;
@@ -1580,13 +1589,13 @@ int bmc150_accel_core_probe(struct device *dev, struct regmap *regmap, int irq,
bmc150_accel_trigger_handler,
&bmc150_accel_buffer_ops);
if (ret < 0) {
- dev_err(data->dev, "Failed: iio triggered buffer setup\n");
+ dev_err(dev, "Failed: iio triggered buffer setup\n");
return ret;
}

if (data->irq > 0) {
ret = devm_request_threaded_irq(
- data->dev, data->irq,
+ dev, data->irq,
bmc150_accel_irq_handler,
bmc150_accel_irq_thread_handler,
IRQF_TRIGGER_RISING,
@@ -1604,7 +1613,7 @@ int bmc150_accel_core_probe(struct device *dev, struct regmap *regmap, int irq,
ret = regmap_write(data->regmap, BMC150_ACCEL_REG_INT_RST_LATCH,
BMC150_ACCEL_INT_MODE_LATCH_RESET);
if (ret < 0) {
- dev_err(data->dev, "Error writing reg_int_rst_latch\n");
+ dev_err(dev, "Error writing reg_int_rst_latch\n");
goto err_buffer_cleanup;
}

@@ -1653,9 +1662,9 @@ int bmc150_accel_core_remove(struct device *dev)

iio_device_unregister(indio_dev);

- pm_runtime_disable(data->dev);
- pm_runtime_set_suspended(data->dev);
- pm_runtime_put_noidle(data->dev);
+ pm_runtime_disable(dev);
+ pm_runtime_set_suspended(dev);
+ pm_runtime_put_noidle(dev);

bmc150_accel_unregister_triggers(data, BMC150_ACCEL_TRIGGERS - 1);

@@ -1704,7 +1713,7 @@ static int bmc150_accel_runtime_suspend(struct device *dev)
struct bmc150_accel_data *data = iio_priv(indio_dev);
int ret;

- dev_dbg(data->dev, __func__);
+ dev_dbg(dev, __func__);
ret = bmc150_accel_set_mode(data, BMC150_ACCEL_SLEEP_MODE_SUSPEND, 0);
if (ret < 0)
return -EAGAIN;
@@ -1719,7 +1728,7 @@ static int bmc150_accel_runtime_resume(struct device *dev)
int ret;
int sleep_val;

- dev_dbg(data->dev, __func__);
+ dev_dbg(dev, __func__);

ret = bmc150_accel_set_mode(data, BMC150_ACCEL_SLEEP_MODE_NORMAL, 0);
if (ret < 0)
--
2.1.4

2016-04-06 05:18:49

by Alison Schofield

[permalink] [raw]

Subject: [PATCH 5/9] iio: accel: mma7455: use regmap to retrieve struct device

2016-04-06 05:19:30

by Alison Schofield

[permalink] [raw]

Subject: [PATCH 6/9] iio: accel: mxc4005: use regmap to retrieve struct device

2016-04-06 05:20:06

by Alison Schofield

[permalink] [raw]

Subject: [PATCH 7/9] iio: health: afe4403: use regmap to retrieve struct device

2016-04-06 05:20:58

by Alison Schofield

[permalink] [raw]

These drivers include both struct regmap and struct device in their
global data. Remove the redundant copy by deleting struct device
from the drivers global data and using regmap API to retrieve when
needed.

This patchset removes all such redundancies in drivers/iio/.

Patches created using Coccinelle plus hand edits for whitespace.

Changes v1->v2:
This set is updated following reviews teaching me that not
all regmaps in global data will be of that device. Discards
and keepers are noted below. No actual code changes just a
remake of the patchset.

Jonathan - I saw you want to let it simmer, just want to
let it simmer cleaner ;)

accel: mma7455
Removing: Joachim ack'd, Jonathan took v1

adc: exynos_adc
Discard: regmap is of PMU parent

adc: qcom-spmi-iadc
adc: qcom-spmi-vadc
Discard: first line of _probe gets regmap from parent

accel: bmc150
gyro: bmg160
Keep: regmap passed into _probe from i2c,spi_probe where
it was initialized for this device

accel: mxc4005
health: afe4403
health: afe4404
Keep: regmap initialized in _probe for this device

Alison Schofield (5):
iio: accel: bmc150: use regmap to retrieve struct device
iio: accel: mxc4005: use regmap to retrieve struct device
iio: health: afe4403: use regmap to retrieve struct device
iio: health: afe4404: use regmap to retrieve struct device
iio: gyro: bmg160: use regmap to retrieve struct device

drivers/iio/accel/bmc150-accel-core.c | 99 +++++++++++++++++++----------------
drivers/iio/accel/mxc4005.c | 26 +++++----
drivers/iio/gyro/bmg160_core.c | 86 +++++++++++++++---------------
drivers/iio/health/afe4403.c | 36 ++++++-------
drivers/iio/health/afe4404.c | 38 +++++++-------
5 files changed, 149 insertions(+), 136 deletions(-)

--
2.1.4

2016-04-10 19:05:31

by Alison Schofield

[permalink] [raw]

Subject: [PATCH v2 1/5] iio: accel: bmc150: use regmap to retrieve struct device

2016-04-10 19:07:01

by Alison Schofield

[permalink] [raw]

Subject: [PATCH v2 2/5] iio: accel: mxc4005: use regmap to retrieve struct device

2016-04-10 19:07:22

by Alison Schofield

[permalink] [raw]

Subject: [PATCH v2 3/5] iio: health: afe4403: use regmap to retrieve struct device

2016-04-10 19:07:53

by Alison Schofield

[permalink] [raw]

On 18/04/16 16:53, Andrew F. Davis wrote:
> On 04/17/2016 11:56 PM, Alison Schofield wrote:
>> On Sun, Apr 17, 2016 at 01:07:52PM -0500, Andrew F. Davis wrote:
>>> On 04/16/2016 02:22 PM, Jonathan Cameron wrote:
>>>> On 10/04/16 20:07, Alison Schofield wrote:
>>>>> Driver includes struct regmap and struct device in its global data.
>>>>> Remove the struct device and use regmap API to retrieve device info.
>>>>>
>>>
>>> Why? This adds nothing but more code to get dev through some
>>> container_of trickery when we could just keep a dev pointer in the data
>>> structure.
>>>
>>> Andrew
>>
>> Thanks for the review and response. The why would be for
>> simplification and uniformity across IIO.
>>
>
> I'm all for simplification and uniformity but I think this will take us
> in the wrong direction, a lot drivers do not use regmap for instance and
> so will need another method to lookup the device struct.
>
>> I think I see your point in general, but not sure I get your
>> specific concerns with these afe4403/04 drivers.
>>
>> The drivers only use the device struct in probe and then
>> again at device remove time. At probe, the change no
>> longer stores it in the global data. At remove the
>> regmap_get_device() func is a simple dereference to retrieve
>> the device struct. That's the simplification: we don't carry
>> that ptr in global data waiting for the opportunity to use it
>> at device remove. We just find it when we need it at device
>> remove.
>
> A lot of what drivers store in their per-instance data structure is only
> held for use in remove. Using afe4404 as an example trig and irq are
> also only stored for removal. A good driver framework will pass in
> relevant information to the driver callbacks, and so instance data
> structures have gotten very small, for afe4404 only regmap and regulator
> are actually used during runtime currently.
The difference here is that we are handing over the device access to
entirely be via regmap - so to my mind we should probably also hand
over the dev pointer itself (which in fact we are doing anyway) as such
it makes sense to retrieve it from regmap when needed. Anyhow, whilst
I originally suggested this (in a driver review a while back) it's not
an important issue and can be left to the tastes of individual authors.

Alison, you win some you loose some with this sort of cleanup patch!
In this particular case the fact that it was used as a convenient
place to get hold of it in the probe function made the patch a lot
more 'noisy' than it would otherwise have been.
>
> The direction that would really be of best simplification would be to
> remove the need for the remove function from these drivers, if
> iio_trigger_register and iio_triggered_buffer_setup had devm_ versions
> (like most other setup functions) everything would automatically be
> cleaned up properly without needing a remove function. Then I would have
> no problem removing the unused device structure pointer, but for now
> their will never be a simpler trick to getting the device pointer than
> simply storing it in the instance data.
Whilst that might be nice.... Actually most devices need to do something
very much device specific in their shutdown - such as turning themselves off
so devm still only takes us so far. Still I don't mind patches proposing
moving more stuff over to devm - they do help for some devices
(though the devm_iio_device_register still causes me a enough issues in
incorrect usage that I'm doubtful that accepting that one was ever a good
idea!)
>
>> (Perhaps these devices are getting removed frequently?)
>>
>
> Quite the opposite, same for many drivers, the remove path is
> dangerously untested in the real world, even more reason it's nice to
> not need remove and let devm_ cleanup for us :)

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