* [PATCH] iio: accel: bmc150: decouple buffer and triggers
@ 2015-05-12 14:03 Vlad Dogaru
2015-05-12 16:17 ` Octavian Purdila
0 siblings, 1 reply; 4+ messages in thread
From: Vlad Dogaru @ 2015-05-12 14:03 UTC (permalink / raw)
To: jic23, octavian.purdila, linux-iio; +Cc: Vlad Dogaru
If the interrupt pins are not available, we should still be able to use
the buffer with an external trigger. However, we won't be able to use
the hardware fifo since we have no means of signalling when the
watermark is reached.
I also added a comment to indicate that the timestamps in
bmc150_accel_data are only used for hardware fifo, since initially I was
confused about duplication with pf->timestamp.
Signed-off-by: Vlad Dogaru <vlad.dogaru@intel.com>
---
drivers/iio/accel/bmc150-accel.c | 55 +++++++++++++++++++++++++---------------
1 file changed, 35 insertions(+), 20 deletions(-)
diff --git a/drivers/iio/accel/bmc150-accel.c b/drivers/iio/accel/bmc150-accel.c
index 73e8773..4e70f51 100644
--- a/drivers/iio/accel/bmc150-accel.c
+++ b/drivers/iio/accel/bmc150-accel.c
@@ -196,7 +196,7 @@ struct bmc150_accel_data {
u32 slope_thres;
u32 range;
int ev_enable_state;
- int64_t timestamp, old_timestamp;
+ int64_t timestamp, old_timestamp; /* Only used in hw fifo mode. */
const struct bmc150_accel_chip_info *chip_info;
};
@@ -1183,7 +1183,6 @@ static const struct iio_info bmc150_accel_info = {
.write_event_value = bmc150_accel_write_event,
.write_event_config = bmc150_accel_write_event_config,
.read_event_config = bmc150_accel_read_event_config,
- .validate_trigger = bmc150_accel_validate_trigger,
.driver_module = THIS_MODULE,
};
@@ -1222,7 +1221,7 @@ static irqreturn_t bmc150_accel_trigger_handler(int irq, void *p)
mutex_unlock(&data->mutex);
iio_push_to_buffers_with_timestamp(indio_dev, data->buffer,
- data->timestamp);
+ pf->timestamp);
err_read:
iio_trigger_notify_done(indio_dev->trig);
@@ -1535,6 +1534,13 @@ static int bmc150_accel_fifo_set_mode(struct bmc150_accel_data *data)
return ret;
}
+static int bmc150_accel_buffer_preenable(struct iio_dev *indio_dev)
+{
+ struct bmc150_accel_data *data = iio_priv(indio_dev);
+
+ return bmc150_accel_set_power_state(data, true);
+}
+
static int bmc150_accel_buffer_postenable(struct iio_dev *indio_dev)
{
struct bmc150_accel_data *data = iio_priv(indio_dev);
@@ -1591,9 +1597,18 @@ out:
return 0;
}
+static int bmc150_accel_buffer_postdisable(struct iio_dev *indio_dev)
+{
+ struct bmc150_accel_data *data = iio_priv(indio_dev);
+
+ return bmc150_accel_set_power_state(data, false);
+}
+
static const struct iio_buffer_setup_ops bmc150_accel_buffer_ops = {
+ .preenable = bmc150_accel_buffer_preenable,
.postenable = bmc150_accel_buffer_postenable,
.predisable = bmc150_accel_buffer_predisable,
+ .postdisable = bmc150_accel_buffer_postdisable,
};
static int bmc150_accel_probe(struct i2c_client *client,
@@ -1636,6 +1651,15 @@ static int bmc150_accel_probe(struct i2c_client *client,
indio_dev->modes = INDIO_DIRECT_MODE;
indio_dev->info = &bmc150_accel_info;
+ ret = iio_triggered_buffer_setup(indio_dev,
+ &iio_pollfunc_store_time,
+ bmc150_accel_trigger_handler,
+ &bmc150_accel_buffer_ops);
+ if (ret < 0) {
+ dev_err(&client->dev, "Failed: iio triggered buffer setup\n");
+ return ret;
+ }
+
if (client->irq < 0)
client->irq = bmc150_accel_gpio_probe(client, data);
@@ -1648,7 +1672,7 @@ static int bmc150_accel_probe(struct i2c_client *client,
BMC150_ACCEL_IRQ_NAME,
indio_dev);
if (ret)
- return ret;
+ goto err_buffer_cleanup;
/*
* Set latched mode interrupt. While certain interrupts are
@@ -1661,24 +1685,14 @@ static int bmc150_accel_probe(struct i2c_client *client,
BMC150_ACCEL_INT_MODE_LATCH_RESET);
if (ret < 0) {
dev_err(&data->client->dev, "Error writing reg_int_rst_latch\n");
- return ret;
+ goto err_buffer_cleanup;
}
bmc150_accel_interrupts_setup(indio_dev, data);
ret = bmc150_accel_triggers_setup(indio_dev, data);
if (ret)
- return ret;
-
- ret = iio_triggered_buffer_setup(indio_dev,
- &iio_pollfunc_store_time,
- bmc150_accel_trigger_handler,
- &bmc150_accel_buffer_ops);
- if (ret < 0) {
- dev_err(&client->dev,
- "Failed: iio triggered buffer setup\n");
- goto err_trigger_unregister;
- }
+ goto err_buffer_cleanup;
if (i2c_check_functionality(client->adapter, I2C_FUNC_I2C) ||
i2c_check_functionality(client->adapter,
@@ -1692,7 +1706,7 @@ static int bmc150_accel_probe(struct i2c_client *client,
ret = iio_device_register(indio_dev);
if (ret < 0) {
dev_err(&client->dev, "Unable to register iio device\n");
- goto err_buffer_cleanup;
+ goto err_trigger_unregister;
}
ret = pm_runtime_set_active(&client->dev);
@@ -1708,11 +1722,10 @@ static int bmc150_accel_probe(struct i2c_client *client,
err_iio_unregister:
iio_device_unregister(indio_dev);
-err_buffer_cleanup:
- if (indio_dev->pollfunc)
- iio_triggered_buffer_cleanup(indio_dev);
err_trigger_unregister:
bmc150_accel_unregister_triggers(data, BMC150_ACCEL_TRIGGERS - 1);
+err_buffer_cleanup:
+ iio_triggered_buffer_cleanup(indio_dev);
return ret;
}
@@ -1730,6 +1743,8 @@ static int bmc150_accel_remove(struct i2c_client *client)
bmc150_accel_unregister_triggers(data, BMC150_ACCEL_TRIGGERS - 1);
+ iio_triggered_buffer_cleanup(indio_dev);
+
mutex_lock(&data->mutex);
bmc150_accel_set_mode(data, BMC150_ACCEL_SLEEP_MODE_DEEP_SUSPEND, 0);
mutex_unlock(&data->mutex);
--
1.9.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] iio: accel: bmc150: decouple buffer and triggers
2015-05-12 14:03 [PATCH] iio: accel: bmc150: decouple buffer and triggers Vlad Dogaru
@ 2015-05-12 16:17 ` Octavian Purdila
2015-05-12 20:51 ` Pandruvada, Srinivas
0 siblings, 1 reply; 4+ messages in thread
From: Octavian Purdila @ 2015-05-12 16:17 UTC (permalink / raw)
To: Vlad Dogaru
Cc: Jonathan Cameron, linux-iio@vger.kernel.org, Srinivas Pandruvada,
Srinivas Pandruvada
On Tue, May 12, 2015 at 5:03 PM, Vlad Dogaru <vlad.dogaru@intel.com> wrote:
>
> If the interrupt pins are not available, we should still be able to use
> the buffer with an external trigger. However, we won't be able to use
> the hardware fifo since we have no means of signalling when the
> watermark is reached.
>
> I also added a comment to indicate that the timestamps in
> bmc150_accel_data are only used for hardware fifo, since initially I was
> confused about duplication with pf->timestamp.
>
> Signed-off-by: Vlad Dogaru <vlad.dogaru@intel.com>
Srinivas can you also please take a look?
The timestamp now has a slight delay (we take in the poll irq handler
instead of the irq handler) but I think that is negligible, so:
Reviewed-by: Octavian Purdila <octavian.purdila@intel.com>
> ---
> drivers/iio/accel/bmc150-accel.c | 55 +++++++++++++++++++++++++---------------
> 1 file changed, 35 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/iio/accel/bmc150-accel.c b/drivers/iio/accel/bmc150-accel.c
> index 73e8773..4e70f51 100644
> --- a/drivers/iio/accel/bmc150-accel.c
> +++ b/drivers/iio/accel/bmc150-accel.c
> @@ -196,7 +196,7 @@ struct bmc150_accel_data {
> u32 slope_thres;
> u32 range;
> int ev_enable_state;
> - int64_t timestamp, old_timestamp;
> + int64_t timestamp, old_timestamp; /* Only used in hw fifo mode. */
> const struct bmc150_accel_chip_info *chip_info;
> };
>
> @@ -1183,7 +1183,6 @@ static const struct iio_info bmc150_accel_info = {
> .write_event_value = bmc150_accel_write_event,
> .write_event_config = bmc150_accel_write_event_config,
> .read_event_config = bmc150_accel_read_event_config,
> - .validate_trigger = bmc150_accel_validate_trigger,
> .driver_module = THIS_MODULE,
> };
>
> @@ -1222,7 +1221,7 @@ static irqreturn_t bmc150_accel_trigger_handler(int irq, void *p)
> mutex_unlock(&data->mutex);
>
> iio_push_to_buffers_with_timestamp(indio_dev, data->buffer,
> - data->timestamp);
> + pf->timestamp);
> err_read:
> iio_trigger_notify_done(indio_dev->trig);
>
> @@ -1535,6 +1534,13 @@ static int bmc150_accel_fifo_set_mode(struct bmc150_accel_data *data)
> return ret;
> }
>
> +static int bmc150_accel_buffer_preenable(struct iio_dev *indio_dev)
> +{
> + struct bmc150_accel_data *data = iio_priv(indio_dev);
> +
> + return bmc150_accel_set_power_state(data, true);
> +}
> +
> static int bmc150_accel_buffer_postenable(struct iio_dev *indio_dev)
> {
> struct bmc150_accel_data *data = iio_priv(indio_dev);
> @@ -1591,9 +1597,18 @@ out:
> return 0;
> }
>
> +static int bmc150_accel_buffer_postdisable(struct iio_dev *indio_dev)
> +{
> + struct bmc150_accel_data *data = iio_priv(indio_dev);
> +
> + return bmc150_accel_set_power_state(data, false);
> +}
> +
> static const struct iio_buffer_setup_ops bmc150_accel_buffer_ops = {
> + .preenable = bmc150_accel_buffer_preenable,
> .postenable = bmc150_accel_buffer_postenable,
> .predisable = bmc150_accel_buffer_predisable,
> + .postdisable = bmc150_accel_buffer_postdisable,
> };
>
> static int bmc150_accel_probe(struct i2c_client *client,
> @@ -1636,6 +1651,15 @@ static int bmc150_accel_probe(struct i2c_client *client,
> indio_dev->modes = INDIO_DIRECT_MODE;
> indio_dev->info = &bmc150_accel_info;
>
> + ret = iio_triggered_buffer_setup(indio_dev,
> + &iio_pollfunc_store_time,
> + bmc150_accel_trigger_handler,
> + &bmc150_accel_buffer_ops);
> + if (ret < 0) {
> + dev_err(&client->dev, "Failed: iio triggered buffer setup\n");
> + return ret;
> + }
> +
> if (client->irq < 0)
> client->irq = bmc150_accel_gpio_probe(client, data);
>
> @@ -1648,7 +1672,7 @@ static int bmc150_accel_probe(struct i2c_client *client,
> BMC150_ACCEL_IRQ_NAME,
> indio_dev);
> if (ret)
> - return ret;
> + goto err_buffer_cleanup;
>
> /*
> * Set latched mode interrupt. While certain interrupts are
> @@ -1661,24 +1685,14 @@ static int bmc150_accel_probe(struct i2c_client *client,
> BMC150_ACCEL_INT_MODE_LATCH_RESET);
> if (ret < 0) {
> dev_err(&data->client->dev, "Error writing reg_int_rst_latch\n");
> - return ret;
> + goto err_buffer_cleanup;
> }
>
> bmc150_accel_interrupts_setup(indio_dev, data);
>
> ret = bmc150_accel_triggers_setup(indio_dev, data);
> if (ret)
> - return ret;
> -
> - ret = iio_triggered_buffer_setup(indio_dev,
> - &iio_pollfunc_store_time,
> - bmc150_accel_trigger_handler,
> - &bmc150_accel_buffer_ops);
> - if (ret < 0) {
> - dev_err(&client->dev,
> - "Failed: iio triggered buffer setup\n");
> - goto err_trigger_unregister;
> - }
> + goto err_buffer_cleanup;
>
> if (i2c_check_functionality(client->adapter, I2C_FUNC_I2C) ||
> i2c_check_functionality(client->adapter,
> @@ -1692,7 +1706,7 @@ static int bmc150_accel_probe(struct i2c_client *client,
> ret = iio_device_register(indio_dev);
> if (ret < 0) {
> dev_err(&client->dev, "Unable to register iio device\n");
> - goto err_buffer_cleanup;
> + goto err_trigger_unregister;
> }
>
> ret = pm_runtime_set_active(&client->dev);
> @@ -1708,11 +1722,10 @@ static int bmc150_accel_probe(struct i2c_client *client,
>
> err_iio_unregister:
> iio_device_unregister(indio_dev);
> -err_buffer_cleanup:
> - if (indio_dev->pollfunc)
> - iio_triggered_buffer_cleanup(indio_dev);
> err_trigger_unregister:
> bmc150_accel_unregister_triggers(data, BMC150_ACCEL_TRIGGERS - 1);
> +err_buffer_cleanup:
> + iio_triggered_buffer_cleanup(indio_dev);
>
> return ret;
> }
> @@ -1730,6 +1743,8 @@ static int bmc150_accel_remove(struct i2c_client *client)
>
> bmc150_accel_unregister_triggers(data, BMC150_ACCEL_TRIGGERS - 1);
>
> + iio_triggered_buffer_cleanup(indio_dev);
> +
> mutex_lock(&data->mutex);
> bmc150_accel_set_mode(data, BMC150_ACCEL_SLEEP_MODE_DEEP_SUSPEND, 0);
> mutex_unlock(&data->mutex);
> --
> 1.9.1
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] iio: accel: bmc150: decouple buffer and triggers
2015-05-12 16:17 ` Octavian Purdila
@ 2015-05-12 20:51 ` Pandruvada, Srinivas
2015-05-17 9:58 ` Jonathan Cameron
0 siblings, 1 reply; 4+ messages in thread
From: Pandruvada, Srinivas @ 2015-05-12 20:51 UTC (permalink / raw)
To: Purdila, Octavian
Cc: linux-iio@vger.kernel.org, jic23@kernel.org, Dogaru, Vlad
T24gVHVlLCAyMDE1LTA1LTEyIGF0IDE5OjE3ICswMzAwLCBPY3RhdmlhbiBQdXJkaWxhIHdyb3Rl
Og0KPiBPbiBUdWUsIE1heSAxMiwgMjAxNSBhdCA1OjAzIFBNLCBWbGFkIERvZ2FydSA8dmxhZC5k
b2dhcnVAaW50ZWwuY29tPiB3cm90ZToNCj4gPg0KPiA+IElmIHRoZSBpbnRlcnJ1cHQgcGlucyBh
cmUgbm90IGF2YWlsYWJsZSwgd2Ugc2hvdWxkIHN0aWxsIGJlIGFibGUgdG8gdXNlDQo+ID4gdGhl
IGJ1ZmZlciB3aXRoIGFuIGV4dGVybmFsIHRyaWdnZXIuICBIb3dldmVyLCB3ZSB3b24ndCBiZSBh
YmxlIHRvIHVzZQ0KPiA+IHRoZSBoYXJkd2FyZSBmaWZvIHNpbmNlIHdlIGhhdmUgbm8gbWVhbnMg
b2Ygc2lnbmFsbGluZyB3aGVuIHRoZQ0KPiA+IHdhdGVybWFyayBpcyByZWFjaGVkLg0KPiA+DQo+
ID4gSSBhbHNvIGFkZGVkIGEgY29tbWVudCB0byBpbmRpY2F0ZSB0aGF0IHRoZSB0aW1lc3RhbXBz
IGluDQo+ID4gYm1jMTUwX2FjY2VsX2RhdGEgYXJlIG9ubHkgdXNlZCBmb3IgaGFyZHdhcmUgZmlm
bywgc2luY2UgaW5pdGlhbGx5IEkgd2FzDQo+ID4gY29uZnVzZWQgYWJvdXQgZHVwbGljYXRpb24g
d2l0aCBwZi0+dGltZXN0YW1wLg0KPiA+DQo+ID4gU2lnbmVkLW9mZi1ieTogVmxhZCBEb2dhcnUg
PHZsYWQuZG9nYXJ1QGludGVsLmNvbT4NCj4gDQo+IFNyaW5pdmFzIGNhbiB5b3UgYWxzbyBwbGVh
c2UgdGFrZSBhIGxvb2s/DQo+IA0KPiBUaGUgdGltZXN0YW1wIG5vdyBoYXMgYSBzbGlnaHQgZGVs
YXkgKHdlIHRha2UgaW4gdGhlIHBvbGwgaXJxIGhhbmRsZXINCj4gaW5zdGVhZCBvZiB0aGUgaXJx
IGhhbmRsZXIpIGJ1dCBJIHRoaW5rIHRoYXQgaXMgbmVnbGlnaWJsZSwgc286DQpUaGlzIHNob3Vs
ZCBiZSBmaW5lLg0KUmV2aWV3ZWQtYnk6IFNyaW5pdmFzIFBhbmRydXZhZGEgPHNyaW5pdmFzLnBh
bmRydXZhZGFAbGludXguaW50ZWwuY29tPg0KPiANCj4gUmV2aWV3ZWQtYnk6IE9jdGF2aWFuIFB1
cmRpbGEgPG9jdGF2aWFuLnB1cmRpbGFAaW50ZWwuY29tPg0KPiANCj4gPiAtLS0NCj4gPiAgZHJp
dmVycy9paW8vYWNjZWwvYm1jMTUwLWFjY2VsLmMgfCA1NSArKysrKysrKysrKysrKysrKysrKysr
KysrLS0tLS0tLS0tLS0tLS0tDQo+ID4gIDEgZmlsZSBjaGFuZ2VkLCAzNSBpbnNlcnRpb25zKCsp
LCAyMCBkZWxldGlvbnMoLSkNCj4gPg0KPiA+IGRpZmYgLS1naXQgYS9kcml2ZXJzL2lpby9hY2Nl
bC9ibWMxNTAtYWNjZWwuYyBiL2RyaXZlcnMvaWlvL2FjY2VsL2JtYzE1MC1hY2NlbC5jDQo+ID4g
aW5kZXggNzNlODc3My4uNGU3MGY1MSAxMDA2NDQNCj4gPiAtLS0gYS9kcml2ZXJzL2lpby9hY2Nl
bC9ibWMxNTAtYWNjZWwuYw0KPiA+ICsrKyBiL2RyaXZlcnMvaWlvL2FjY2VsL2JtYzE1MC1hY2Nl
bC5jDQo+ID4gQEAgLTE5Niw3ICsxOTYsNyBAQCBzdHJ1Y3QgYm1jMTUwX2FjY2VsX2RhdGEgew0K
PiA+ICAgICAgICAgdTMyIHNsb3BlX3RocmVzOw0KPiA+ICAgICAgICAgdTMyIHJhbmdlOw0KPiA+
ICAgICAgICAgaW50IGV2X2VuYWJsZV9zdGF0ZTsNCj4gPiAtICAgICAgIGludDY0X3QgdGltZXN0
YW1wLCBvbGRfdGltZXN0YW1wOw0KPiA+ICsgICAgICAgaW50NjRfdCB0aW1lc3RhbXAsIG9sZF90
aW1lc3RhbXA7IC8qIE9ubHkgdXNlZCBpbiBodyBmaWZvIG1vZGUuICovDQo+ID4gICAgICAgICBj
b25zdCBzdHJ1Y3QgYm1jMTUwX2FjY2VsX2NoaXBfaW5mbyAqY2hpcF9pbmZvOw0KPiA+ICB9Ow0K
PiA+DQo+ID4gQEAgLTExODMsNyArMTE4Myw2IEBAIHN0YXRpYyBjb25zdCBzdHJ1Y3QgaWlvX2lu
Zm8gYm1jMTUwX2FjY2VsX2luZm8gPSB7DQo+ID4gICAgICAgICAud3JpdGVfZXZlbnRfdmFsdWUg
ICAgICA9IGJtYzE1MF9hY2NlbF93cml0ZV9ldmVudCwNCj4gPiAgICAgICAgIC53cml0ZV9ldmVu
dF9jb25maWcgICAgID0gYm1jMTUwX2FjY2VsX3dyaXRlX2V2ZW50X2NvbmZpZywNCj4gPiAgICAg
ICAgIC5yZWFkX2V2ZW50X2NvbmZpZyAgICAgID0gYm1jMTUwX2FjY2VsX3JlYWRfZXZlbnRfY29u
ZmlnLA0KPiA+IC0gICAgICAgLnZhbGlkYXRlX3RyaWdnZXIgICAgICAgPSBibWMxNTBfYWNjZWxf
dmFsaWRhdGVfdHJpZ2dlciwNCj4gPiAgICAgICAgIC5kcml2ZXJfbW9kdWxlICAgICAgICAgID0g
VEhJU19NT0RVTEUsDQo+ID4gIH07DQo+ID4NCj4gPiBAQCAtMTIyMiw3ICsxMjIxLDcgQEAgc3Rh
dGljIGlycXJldHVybl90IGJtYzE1MF9hY2NlbF90cmlnZ2VyX2hhbmRsZXIoaW50IGlycSwgdm9p
ZCAqcCkNCj4gPiAgICAgICAgIG11dGV4X3VubG9jaygmZGF0YS0+bXV0ZXgpOw0KPiA+DQo+ID4g
ICAgICAgICBpaW9fcHVzaF90b19idWZmZXJzX3dpdGhfdGltZXN0YW1wKGluZGlvX2RldiwgZGF0
YS0+YnVmZmVyLA0KPiA+IC0gICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg
ICBkYXRhLT50aW1lc3RhbXApOw0KPiA+ICsgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg
ICAgICAgICAgICBwZi0+dGltZXN0YW1wKTsNCj4gPiAgZXJyX3JlYWQ6DQo+ID4gICAgICAgICBp
aW9fdHJpZ2dlcl9ub3RpZnlfZG9uZShpbmRpb19kZXYtPnRyaWcpOw0KPiA+DQo+ID4gQEAgLTE1
MzUsNiArMTUzNCwxMyBAQCBzdGF0aWMgaW50IGJtYzE1MF9hY2NlbF9maWZvX3NldF9tb2RlKHN0
cnVjdCBibWMxNTBfYWNjZWxfZGF0YSAqZGF0YSkNCj4gPiAgICAgICAgIHJldHVybiByZXQ7DQo+
ID4gIH0NCj4gPg0KPiA+ICtzdGF0aWMgaW50IGJtYzE1MF9hY2NlbF9idWZmZXJfcHJlZW5hYmxl
KHN0cnVjdCBpaW9fZGV2ICppbmRpb19kZXYpDQo+ID4gK3sNCj4gPiArICAgICAgIHN0cnVjdCBi
bWMxNTBfYWNjZWxfZGF0YSAqZGF0YSA9IGlpb19wcml2KGluZGlvX2Rldik7DQo+ID4gKw0KPiA+
ICsgICAgICAgcmV0dXJuIGJtYzE1MF9hY2NlbF9zZXRfcG93ZXJfc3RhdGUoZGF0YSwgdHJ1ZSk7
DQo+ID4gK30NCj4gPiArDQo+ID4gIHN0YXRpYyBpbnQgYm1jMTUwX2FjY2VsX2J1ZmZlcl9wb3N0
ZW5hYmxlKHN0cnVjdCBpaW9fZGV2ICppbmRpb19kZXYpDQo+ID4gIHsNCj4gPiAgICAgICAgIHN0
cnVjdCBibWMxNTBfYWNjZWxfZGF0YSAqZGF0YSA9IGlpb19wcml2KGluZGlvX2Rldik7DQo+ID4g
QEAgLTE1OTEsOSArMTU5NywxOCBAQCBvdXQ6DQo+ID4gICAgICAgICByZXR1cm4gMDsNCj4gPiAg
fQ0KPiA+DQo+ID4gK3N0YXRpYyBpbnQgYm1jMTUwX2FjY2VsX2J1ZmZlcl9wb3N0ZGlzYWJsZShz
dHJ1Y3QgaWlvX2RldiAqaW5kaW9fZGV2KQ0KPiA+ICt7DQo+ID4gKyAgICAgICBzdHJ1Y3QgYm1j
MTUwX2FjY2VsX2RhdGEgKmRhdGEgPSBpaW9fcHJpdihpbmRpb19kZXYpOw0KPiA+ICsNCj4gPiAr
ICAgICAgIHJldHVybiBibWMxNTBfYWNjZWxfc2V0X3Bvd2VyX3N0YXRlKGRhdGEsIGZhbHNlKTsN
Cj4gPiArfQ0KPiA+ICsNCj4gPiAgc3RhdGljIGNvbnN0IHN0cnVjdCBpaW9fYnVmZmVyX3NldHVw
X29wcyBibWMxNTBfYWNjZWxfYnVmZmVyX29wcyA9IHsNCj4gPiArICAgICAgIC5wcmVlbmFibGUg
PSBibWMxNTBfYWNjZWxfYnVmZmVyX3ByZWVuYWJsZSwNCj4gPiAgICAgICAgIC5wb3N0ZW5hYmxl
ID0gYm1jMTUwX2FjY2VsX2J1ZmZlcl9wb3N0ZW5hYmxlLA0KPiA+ICAgICAgICAgLnByZWRpc2Fi
bGUgPSBibWMxNTBfYWNjZWxfYnVmZmVyX3ByZWRpc2FibGUsDQo+ID4gKyAgICAgICAucG9zdGRp
c2FibGUgPSBibWMxNTBfYWNjZWxfYnVmZmVyX3Bvc3RkaXNhYmxlLA0KPiA+ICB9Ow0KPiA+DQo+
ID4gIHN0YXRpYyBpbnQgYm1jMTUwX2FjY2VsX3Byb2JlKHN0cnVjdCBpMmNfY2xpZW50ICpjbGll
bnQsDQo+ID4gQEAgLTE2MzYsNiArMTY1MSwxNSBAQCBzdGF0aWMgaW50IGJtYzE1MF9hY2NlbF9w
cm9iZShzdHJ1Y3QgaTJjX2NsaWVudCAqY2xpZW50LA0KPiA+ICAgICAgICAgaW5kaW9fZGV2LT5t
b2RlcyA9IElORElPX0RJUkVDVF9NT0RFOw0KPiA+ICAgICAgICAgaW5kaW9fZGV2LT5pbmZvID0g
JmJtYzE1MF9hY2NlbF9pbmZvOw0KPiA+DQo+ID4gKyAgICAgICByZXQgPSBpaW9fdHJpZ2dlcmVk
X2J1ZmZlcl9zZXR1cChpbmRpb19kZXYsDQo+ID4gKyAgICAgICAgICAgICAgICAgICAgICAgICAg
ICAgICAgICAgICAgICAmaWlvX3BvbGxmdW5jX3N0b3JlX3RpbWUsDQo+ID4gKyAgICAgICAgICAg
ICAgICAgICAgICAgICAgICAgICAgICAgICAgICBibWMxNTBfYWNjZWxfdHJpZ2dlcl9oYW5kbGVy
LA0KPiA+ICsgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgJmJtYzE1MF9h
Y2NlbF9idWZmZXJfb3BzKTsNCj4gPiArICAgICAgIGlmIChyZXQgPCAwKSB7DQo+ID4gKyAgICAg
ICAgICAgICAgIGRldl9lcnIoJmNsaWVudC0+ZGV2LCAiRmFpbGVkOiBpaW8gdHJpZ2dlcmVkIGJ1
ZmZlciBzZXR1cFxuIik7DQo+ID4gKyAgICAgICAgICAgICAgIHJldHVybiByZXQ7DQo+ID4gKyAg
ICAgICB9DQo+ID4gKw0KPiA+ICAgICAgICAgaWYgKGNsaWVudC0+aXJxIDwgMCkNCj4gPiAgICAg
ICAgICAgICAgICAgY2xpZW50LT5pcnEgPSBibWMxNTBfYWNjZWxfZ3Bpb19wcm9iZShjbGllbnQs
IGRhdGEpOw0KPiA+DQo+ID4gQEAgLTE2NDgsNyArMTY3Miw3IEBAIHN0YXRpYyBpbnQgYm1jMTUw
X2FjY2VsX3Byb2JlKHN0cnVjdCBpMmNfY2xpZW50ICpjbGllbnQsDQo+ID4gICAgICAgICAgICAg
ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgQk1DMTUwX0FDQ0VMX0lSUV9OQU1F
LA0KPiA+ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIGlu
ZGlvX2Rldik7DQo+ID4gICAgICAgICAgICAgICAgIGlmIChyZXQpDQo+ID4gLSAgICAgICAgICAg
ICAgICAgICAgICAgcmV0dXJuIHJldDsNCj4gPiArICAgICAgICAgICAgICAgICAgICAgICBnb3Rv
IGVycl9idWZmZXJfY2xlYW51cDsNCj4gPg0KPiA+ICAgICAgICAgICAgICAgICAvKg0KPiA+ICAg
ICAgICAgICAgICAgICAgKiBTZXQgbGF0Y2hlZCBtb2RlIGludGVycnVwdC4gV2hpbGUgY2VydGFp
biBpbnRlcnJ1cHRzIGFyZQ0KPiA+IEBAIC0xNjYxLDI0ICsxNjg1LDE0IEBAIHN0YXRpYyBpbnQg
Ym1jMTUwX2FjY2VsX3Byb2JlKHN0cnVjdCBpMmNfY2xpZW50ICpjbGllbnQsDQo+ID4gICAgICAg
ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgQk1DMTUwX0FDQ0VMX0lOVF9N
T0RFX0xBVENIX1JFU0VUKTsNCj4gPiAgICAgICAgICAgICAgICAgaWYgKHJldCA8IDApIHsNCj4g
PiAgICAgICAgICAgICAgICAgICAgICAgICBkZXZfZXJyKCZkYXRhLT5jbGllbnQtPmRldiwgIkVy
cm9yIHdyaXRpbmcgcmVnX2ludF9yc3RfbGF0Y2hcbiIpOw0KPiA+IC0gICAgICAgICAgICAgICAg
ICAgICAgIHJldHVybiByZXQ7DQo+ID4gKyAgICAgICAgICAgICAgICAgICAgICAgZ290byBlcnJf
YnVmZmVyX2NsZWFudXA7DQo+ID4gICAgICAgICAgICAgICAgIH0NCj4gPg0KPiA+ICAgICAgICAg
ICAgICAgICBibWMxNTBfYWNjZWxfaW50ZXJydXB0c19zZXR1cChpbmRpb19kZXYsIGRhdGEpOw0K
PiA+DQo+ID4gICAgICAgICAgICAgICAgIHJldCA9IGJtYzE1MF9hY2NlbF90cmlnZ2Vyc19zZXR1
cChpbmRpb19kZXYsIGRhdGEpOw0KPiA+ICAgICAgICAgICAgICAgICBpZiAocmV0KQ0KPiA+IC0g
ICAgICAgICAgICAgICAgICAgICAgIHJldHVybiByZXQ7DQo+ID4gLQ0KPiA+IC0gICAgICAgICAg
ICAgICByZXQgPSBpaW9fdHJpZ2dlcmVkX2J1ZmZlcl9zZXR1cChpbmRpb19kZXYsDQo+ID4gLSAg
ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICZpaW9fcG9sbGZ1
bmNfc3RvcmVfdGltZSwNCj4gPiAtICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg
ICAgICAgICAgICAgYm1jMTUwX2FjY2VsX3RyaWdnZXJfaGFuZGxlciwNCj4gPiAtICAgICAgICAg
ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgJmJtYzE1MF9hY2NlbF9idWZm
ZXJfb3BzKTsNCj4gPiAtICAgICAgICAgICAgICAgaWYgKHJldCA8IDApIHsNCj4gPiAtICAgICAg
ICAgICAgICAgICAgICAgICBkZXZfZXJyKCZjbGllbnQtPmRldiwNCj4gPiAtICAgICAgICAgICAg
ICAgICAgICAgICAgICAgICAgICJGYWlsZWQ6IGlpbyB0cmlnZ2VyZWQgYnVmZmVyIHNldHVwXG4i
KTsNCj4gPiAtICAgICAgICAgICAgICAgICAgICAgICBnb3RvIGVycl90cmlnZ2VyX3VucmVnaXN0
ZXI7DQo+ID4gLSAgICAgICAgICAgICAgIH0NCj4gPiArICAgICAgICAgICAgICAgICAgICAgICBn
b3RvIGVycl9idWZmZXJfY2xlYW51cDsNCj4gPg0KPiA+ICAgICAgICAgICAgICAgICBpZiAoaTJj
X2NoZWNrX2Z1bmN0aW9uYWxpdHkoY2xpZW50LT5hZGFwdGVyLCBJMkNfRlVOQ19JMkMpIHx8DQo+
ID4gICAgICAgICAgICAgICAgICAgICBpMmNfY2hlY2tfZnVuY3Rpb25hbGl0eShjbGllbnQtPmFk
YXB0ZXIsDQo+ID4gQEAgLTE2OTIsNyArMTcwNiw3IEBAIHN0YXRpYyBpbnQgYm1jMTUwX2FjY2Vs
X3Byb2JlKHN0cnVjdCBpMmNfY2xpZW50ICpjbGllbnQsDQo+ID4gICAgICAgICByZXQgPSBpaW9f
ZGV2aWNlX3JlZ2lzdGVyKGluZGlvX2Rldik7DQo+ID4gICAgICAgICBpZiAocmV0IDwgMCkgew0K
PiA+ICAgICAgICAgICAgICAgICBkZXZfZXJyKCZjbGllbnQtPmRldiwgIlVuYWJsZSB0byByZWdp
c3RlciBpaW8gZGV2aWNlXG4iKTsNCj4gPiAtICAgICAgICAgICAgICAgZ290byBlcnJfYnVmZmVy
X2NsZWFudXA7DQo+ID4gKyAgICAgICAgICAgICAgIGdvdG8gZXJyX3RyaWdnZXJfdW5yZWdpc3Rl
cjsNCj4gPiAgICAgICAgIH0NCj4gPg0KPiA+ICAgICAgICAgcmV0ID0gcG1fcnVudGltZV9zZXRf
YWN0aXZlKCZjbGllbnQtPmRldik7DQo+ID4gQEAgLTE3MDgsMTEgKzE3MjIsMTAgQEAgc3RhdGlj
IGludCBibWMxNTBfYWNjZWxfcHJvYmUoc3RydWN0IGkyY19jbGllbnQgKmNsaWVudCwNCj4gPg0K
PiA+ICBlcnJfaWlvX3VucmVnaXN0ZXI6DQo+ID4gICAgICAgICBpaW9fZGV2aWNlX3VucmVnaXN0
ZXIoaW5kaW9fZGV2KTsNCj4gPiAtZXJyX2J1ZmZlcl9jbGVhbnVwOg0KPiA+IC0gICAgICAgaWYg
KGluZGlvX2Rldi0+cG9sbGZ1bmMpDQo+ID4gLSAgICAgICAgICAgICAgIGlpb190cmlnZ2VyZWRf
YnVmZmVyX2NsZWFudXAoaW5kaW9fZGV2KTsNCj4gPiAgZXJyX3RyaWdnZXJfdW5yZWdpc3RlcjoN
Cj4gPiAgICAgICAgIGJtYzE1MF9hY2NlbF91bnJlZ2lzdGVyX3RyaWdnZXJzKGRhdGEsIEJNQzE1
MF9BQ0NFTF9UUklHR0VSUyAtIDEpOw0KPiA+ICtlcnJfYnVmZmVyX2NsZWFudXA6DQo+ID4gKyAg
ICAgICBpaW9fdHJpZ2dlcmVkX2J1ZmZlcl9jbGVhbnVwKGluZGlvX2Rldik7DQo+ID4NCj4gPiAg
ICAgICAgIHJldHVybiByZXQ7DQo+ID4gIH0NCj4gPiBAQCAtMTczMCw2ICsxNzQzLDggQEAgc3Rh
dGljIGludCBibWMxNTBfYWNjZWxfcmVtb3ZlKHN0cnVjdCBpMmNfY2xpZW50ICpjbGllbnQpDQo+
ID4NCj4gPiAgICAgICAgIGJtYzE1MF9hY2NlbF91bnJlZ2lzdGVyX3RyaWdnZXJzKGRhdGEsIEJN
QzE1MF9BQ0NFTF9UUklHR0VSUyAtIDEpOw0KPiA+DQo+ID4gKyAgICAgICBpaW9fdHJpZ2dlcmVk
X2J1ZmZlcl9jbGVhbnVwKGluZGlvX2Rldik7DQo+ID4gKw0KPiA+ICAgICAgICAgbXV0ZXhfbG9j
aygmZGF0YS0+bXV0ZXgpOw0KPiA+ICAgICAgICAgYm1jMTUwX2FjY2VsX3NldF9tb2RlKGRhdGEs
IEJNQzE1MF9BQ0NFTF9TTEVFUF9NT0RFX0RFRVBfU1VTUEVORCwgMCk7DQo+ID4gICAgICAgICBt
dXRleF91bmxvY2soJmRhdGEtPm11dGV4KTsNCj4gPiAtLQ0KPiA+IDEuOS4xDQo+ID4NCg0K
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] iio: accel: bmc150: decouple buffer and triggers
2015-05-12 20:51 ` Pandruvada, Srinivas
@ 2015-05-17 9:58 ` Jonathan Cameron
0 siblings, 0 replies; 4+ messages in thread
From: Jonathan Cameron @ 2015-05-17 9:58 UTC (permalink / raw)
To: Pandruvada, Srinivas, Purdila, Octavian
Cc: linux-iio@vger.kernel.org, Dogaru, Vlad
On 12/05/15 21:51, Pandruvada, Srinivas wrote:
> On Tue, 2015-05-12 at 19:17 +0300, Octavian Purdila wrote:
>> On Tue, May 12, 2015 at 5:03 PM, Vlad Dogaru <vlad.dogaru@intel.com> wrote:
>>>
>>> If the interrupt pins are not available, we should still be able to use
>>> the buffer with an external trigger. However, we won't be able to use
>>> the hardware fifo since we have no means of signalling when the
>>> watermark is reached.
>>>
>>> I also added a comment to indicate that the timestamps in
>>> bmc150_accel_data are only used for hardware fifo, since initially I was
>>> confused about duplication with pf->timestamp.
>>>
>>> Signed-off-by: Vlad Dogaru <vlad.dogaru@intel.com>
>>
>> Srinivas can you also please take a look?
>>
>> The timestamp now has a slight delay (we take in the poll irq handler
>> instead of the irq handler) but I think that is negligible, so:
> This should be fine.
Should be very small as it's a very short chain of function calls later.
Applied to the togreg branch of iio.git.
Thanks,
Jonathan
> Reviewed-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
>>
>> Reviewed-by: Octavian Purdila <octavian.purdila@intel.com>
>>
>>> ---
>>> drivers/iio/accel/bmc150-accel.c | 55 +++++++++++++++++++++++++---------------
>>> 1 file changed, 35 insertions(+), 20 deletions(-)
>>>
>>> diff --git a/drivers/iio/accel/bmc150-accel.c b/drivers/iio/accel/bmc150-accel.c
>>> index 73e8773..4e70f51 100644
>>> --- a/drivers/iio/accel/bmc150-accel.c
>>> +++ b/drivers/iio/accel/bmc150-accel.c
>>> @@ -196,7 +196,7 @@ struct bmc150_accel_data {
>>> u32 slope_thres;
>>> u32 range;
>>> int ev_enable_state;
>>> - int64_t timestamp, old_timestamp;
>>> + int64_t timestamp, old_timestamp; /* Only used in hw fifo mode. */
>>> const struct bmc150_accel_chip_info *chip_info;
>>> };
>>>
>>> @@ -1183,7 +1183,6 @@ static const struct iio_info bmc150_accel_info = {
>>> .write_event_value = bmc150_accel_write_event,
>>> .write_event_config = bmc150_accel_write_event_config,
>>> .read_event_config = bmc150_accel_read_event_config,
>>> - .validate_trigger = bmc150_accel_validate_trigger,
>>> .driver_module = THIS_MODULE,
>>> };
>>>
>>> @@ -1222,7 +1221,7 @@ static irqreturn_t bmc150_accel_trigger_handler(int irq, void *p)
>>> mutex_unlock(&data->mutex);
>>>
>>> iio_push_to_buffers_with_timestamp(indio_dev, data->buffer,
>>> - data->timestamp);
>>> + pf->timestamp);
>>> err_read:
>>> iio_trigger_notify_done(indio_dev->trig);
>>>
>>> @@ -1535,6 +1534,13 @@ static int bmc150_accel_fifo_set_mode(struct bmc150_accel_data *data)
>>> return ret;
>>> }
>>>
>>> +static int bmc150_accel_buffer_preenable(struct iio_dev *indio_dev)
>>> +{
>>> + struct bmc150_accel_data *data = iio_priv(indio_dev);
>>> +
>>> + return bmc150_accel_set_power_state(data, true);
>>> +}
>>> +
>>> static int bmc150_accel_buffer_postenable(struct iio_dev *indio_dev)
>>> {
>>> struct bmc150_accel_data *data = iio_priv(indio_dev);
>>> @@ -1591,9 +1597,18 @@ out:
>>> return 0;
>>> }
>>>
>>> +static int bmc150_accel_buffer_postdisable(struct iio_dev *indio_dev)
>>> +{
>>> + struct bmc150_accel_data *data = iio_priv(indio_dev);
>>> +
>>> + return bmc150_accel_set_power_state(data, false);
>>> +}
>>> +
>>> static const struct iio_buffer_setup_ops bmc150_accel_buffer_ops = {
>>> + .preenable = bmc150_accel_buffer_preenable,
>>> .postenable = bmc150_accel_buffer_postenable,
>>> .predisable = bmc150_accel_buffer_predisable,
>>> + .postdisable = bmc150_accel_buffer_postdisable,
>>> };
>>>
>>> static int bmc150_accel_probe(struct i2c_client *client,
>>> @@ -1636,6 +1651,15 @@ static int bmc150_accel_probe(struct i2c_client *client,
>>> indio_dev->modes = INDIO_DIRECT_MODE;
>>> indio_dev->info = &bmc150_accel_info;
>>>
>>> + ret = iio_triggered_buffer_setup(indio_dev,
>>> + &iio_pollfunc_store_time,
>>> + bmc150_accel_trigger_handler,
>>> + &bmc150_accel_buffer_ops);
>>> + if (ret < 0) {
>>> + dev_err(&client->dev, "Failed: iio triggered buffer setup\n");
>>> + return ret;
>>> + }
>>> +
>>> if (client->irq < 0)
>>> client->irq = bmc150_accel_gpio_probe(client, data);
>>>
>>> @@ -1648,7 +1672,7 @@ static int bmc150_accel_probe(struct i2c_client *client,
>>> BMC150_ACCEL_IRQ_NAME,
>>> indio_dev);
>>> if (ret)
>>> - return ret;
>>> + goto err_buffer_cleanup;
>>>
>>> /*
>>> * Set latched mode interrupt. While certain interrupts are
>>> @@ -1661,24 +1685,14 @@ static int bmc150_accel_probe(struct i2c_client *client,
>>> BMC150_ACCEL_INT_MODE_LATCH_RESET);
>>> if (ret < 0) {
>>> dev_err(&data->client->dev, "Error writing reg_int_rst_latch\n");
>>> - return ret;
>>> + goto err_buffer_cleanup;
>>> }
>>>
>>> bmc150_accel_interrupts_setup(indio_dev, data);
>>>
>>> ret = bmc150_accel_triggers_setup(indio_dev, data);
>>> if (ret)
>>> - return ret;
>>> -
>>> - ret = iio_triggered_buffer_setup(indio_dev,
>>> - &iio_pollfunc_store_time,
>>> - bmc150_accel_trigger_handler,
>>> - &bmc150_accel_buffer_ops);
>>> - if (ret < 0) {
>>> - dev_err(&client->dev,
>>> - "Failed: iio triggered buffer setup\n");
>>> - goto err_trigger_unregister;
>>> - }
>>> + goto err_buffer_cleanup;
>>>
>>> if (i2c_check_functionality(client->adapter, I2C_FUNC_I2C) ||
>>> i2c_check_functionality(client->adapter,
>>> @@ -1692,7 +1706,7 @@ static int bmc150_accel_probe(struct i2c_client *client,
>>> ret = iio_device_register(indio_dev);
>>> if (ret < 0) {
>>> dev_err(&client->dev, "Unable to register iio device\n");
>>> - goto err_buffer_cleanup;
>>> + goto err_trigger_unregister;
>>> }
>>>
>>> ret = pm_runtime_set_active(&client->dev);
>>> @@ -1708,11 +1722,10 @@ static int bmc150_accel_probe(struct i2c_client *client,
>>>
>>> err_iio_unregister:
>>> iio_device_unregister(indio_dev);
>>> -err_buffer_cleanup:
>>> - if (indio_dev->pollfunc)
>>> - iio_triggered_buffer_cleanup(indio_dev);
>>> err_trigger_unregister:
>>> bmc150_accel_unregister_triggers(data, BMC150_ACCEL_TRIGGERS - 1);
>>> +err_buffer_cleanup:
>>> + iio_triggered_buffer_cleanup(indio_dev);
>>>
>>> return ret;
>>> }
>>> @@ -1730,6 +1743,8 @@ static int bmc150_accel_remove(struct i2c_client *client)
>>>
>>> bmc150_accel_unregister_triggers(data, BMC150_ACCEL_TRIGGERS - 1);
>>>
>>> + iio_triggered_buffer_cleanup(indio_dev);
>>> +
>>> mutex_lock(&data->mutex);
>>> bmc150_accel_set_mode(data, BMC150_ACCEL_SLEEP_MODE_DEEP_SUSPEND, 0);
>>> mutex_unlock(&data->mutex);
>>> --
>>> 1.9.1
>>>
>
> N�����r��y���b�X��ǧv�^�){.n�+����{��*"��^n�r���z�\x1a��h����&��\x1e�G���h�\x03(�階�ݢj"��\x1a�^[m�����z�ޖ���f���h���~�mml==
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-05-17 9:58 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-05-12 14:03 [PATCH] iio: accel: bmc150: decouple buffer and triggers Vlad Dogaru
2015-05-12 16:17 ` Octavian Purdila
2015-05-12 20:51 ` Pandruvada, Srinivas
2015-05-17 9:58 ` 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).