* [PATCH] iio:st_magn: enable trigger before enabling sensor
@ 2018-10-25 22:32 Denis Ciocca
2018-10-26 1:28 ` Martin Kelly
2018-10-28 16:20 ` Jonathan Cameron
0 siblings, 2 replies; 7+ messages in thread
From: Denis Ciocca @ 2018-10-25 22:32 UTC (permalink / raw)
To: linux-iio, lars, jic23, lorenzo.bianconi83, pmeerw, martin, knaack.h
Cc: Denis Ciocca
>From logical point of view driver should be ready to
receive irqs before enabling the sensor itself.
This patch is fixing also an issue related to
sensors that generate interrupts unconditionally,
(DRDY pads) when interrupt level is used.
Signed-off-by: Martin Kelly <martin@martingkelly.com>
Signed-off-by: Denis Ciocca <denis.ciocca@st.com>
---
drivers/iio/magnetometer/st_magn_buffer.c | 33 +++++++++++------------
1 file changed, 16 insertions(+), 17 deletions(-)
diff --git a/drivers/iio/magnetometer/st_magn_buffer.c b/drivers/iio/magnetometer/st_magn_buffer.c
index 0a9e8fadfa9d..097e6e88a464 100644
--- a/drivers/iio/magnetometer/st_magn_buffer.c
+++ b/drivers/iio/magnetometer/st_magn_buffer.c
@@ -30,11 +30,6 @@ int st_magn_trig_set_state(struct iio_trigger *trig, bool state)
return st_sensors_set_dataready_irq(indio_dev, state);
}
-static int st_magn_buffer_preenable(struct iio_dev *indio_dev)
-{
- return st_sensors_set_enable(indio_dev, true);
-}
-
static int st_magn_buffer_postenable(struct iio_dev *indio_dev)
{
int err;
@@ -42,40 +37,44 @@ static int st_magn_buffer_postenable(struct iio_dev *indio_dev)
mdata->buffer_data = kmalloc(indio_dev->scan_bytes, GFP_KERNEL);
if (mdata->buffer_data == NULL) {
- err = -ENOMEM;
- goto allocate_memory_error;
+ return -ENOMEM;
}
err = iio_triggered_buffer_postenable(indio_dev);
if (err < 0)
- goto st_magn_buffer_postenable_error;
+ goto st_magn_buffer_free_buffer_data;
+
+ err = st_sensors_set_enable(indio_dev, true);
+ if (err < 0)
+ goto st_magn_buffer_buffer_predisable;
return err;
-st_magn_buffer_postenable_error:
+st_magn_buffer_buffer_predisable:
+ iio_triggered_buffer_predisable(indio_dev);
+st_magn_buffer_free_buffer_data:
kfree(mdata->buffer_data);
-allocate_memory_error:
return err;
}
static int st_magn_buffer_predisable(struct iio_dev *indio_dev)
{
- int err;
+ int err = 0, err2;
struct st_sensor_data *mdata = iio_priv(indio_dev);
- err = iio_triggered_buffer_predisable(indio_dev);
- if (err < 0)
- goto st_magn_buffer_predisable_error;
+ err2 = st_sensors_set_enable(indio_dev, false);
+ if (err2 < 0)
+ err = err2;
- err = st_sensors_set_enable(indio_dev, false);
+ err2 = iio_triggered_buffer_predisable(indio_dev);
+ if (err2 < 0)
+ err = err2;
-st_magn_buffer_predisable_error:
kfree(mdata->buffer_data);
return err;
}
static const struct iio_buffer_setup_ops st_magn_buffer_setup_ops = {
- .preenable = &st_magn_buffer_preenable,
.postenable = &st_magn_buffer_postenable,
.predisable = &st_magn_buffer_predisable,
};
--
2.19.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] iio:st_magn: enable trigger before enabling sensor
2018-10-25 22:32 [PATCH] iio:st_magn: enable trigger before enabling sensor Denis Ciocca
@ 2018-10-26 1:28 ` Martin Kelly
2018-10-28 16:20 ` Jonathan Cameron
1 sibling, 0 replies; 7+ messages in thread
From: Martin Kelly @ 2018-10-26 1:28 UTC (permalink / raw)
To: Denis Ciocca, linux-iio, lars, jic23, lorenzo.bianconi83, pmeerw,
knaack.h
On 10/25/18 3:32 PM, Denis Ciocca wrote:
> From logical point of view driver should be ready to
> receive irqs before enabling the sensor itself.
> This patch is fixing also an issue related to
> sensors that generate interrupts unconditionally,
> (DRDY pads) when interrupt level is used.
>
> Signed-off-by: Martin Kelly <martin@martingkelly.com>
> Signed-off-by: Denis Ciocca <denis.ciocca@st.com>
> ---
> drivers/iio/magnetometer/st_magn_buffer.c | 33 +++++++++++------------
> 1 file changed, 16 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/iio/magnetometer/st_magn_buffer.c b/drivers/iio/magnetometer/st_magn_buffer.c
> index 0a9e8fadfa9d..097e6e88a464 100644
> --- a/drivers/iio/magnetometer/st_magn_buffer.c
> +++ b/drivers/iio/magnetometer/st_magn_buffer.c
> @@ -30,11 +30,6 @@ int st_magn_trig_set_state(struct iio_trigger *trig, bool state)
> return st_sensors_set_dataready_irq(indio_dev, state);
> }
>
> -static int st_magn_buffer_preenable(struct iio_dev *indio_dev)
> -{
> - return st_sensors_set_enable(indio_dev, true);
> -}
> -
> static int st_magn_buffer_postenable(struct iio_dev *indio_dev)
> {
> int err;
> @@ -42,40 +37,44 @@ static int st_magn_buffer_postenable(struct iio_dev *indio_dev)
>
> mdata->buffer_data = kmalloc(indio_dev->scan_bytes, GFP_KERNEL);
> if (mdata->buffer_data == NULL) {
> - err = -ENOMEM;
> - goto allocate_memory_error;
> + return -ENOMEM;
> }
>
> err = iio_triggered_buffer_postenable(indio_dev);
> if (err < 0)
> - goto st_magn_buffer_postenable_error;
> + goto st_magn_buffer_free_buffer_data;
> +
> + err = st_sensors_set_enable(indio_dev, true);
> + if (err < 0)
> + goto st_magn_buffer_buffer_predisable;
>
> return err;
>
> -st_magn_buffer_postenable_error:
> +st_magn_buffer_buffer_predisable:
> + iio_triggered_buffer_predisable(indio_dev);
> +st_magn_buffer_free_buffer_data:
> kfree(mdata->buffer_data);
> -allocate_memory_error:
> return err;
> }
>
> static int st_magn_buffer_predisable(struct iio_dev *indio_dev)
> {
> - int err;
> + int err = 0, err2;
> struct st_sensor_data *mdata = iio_priv(indio_dev);
>
> - err = iio_triggered_buffer_predisable(indio_dev);
> - if (err < 0)
> - goto st_magn_buffer_predisable_error;
> + err2 = st_sensors_set_enable(indio_dev, false);
> + if (err2 < 0)
> + err = err2;
>
> - err = st_sensors_set_enable(indio_dev, false);
> + err2 = iio_triggered_buffer_predisable(indio_dev);
> + if (err2 < 0)
> + err = err2;
I think it would be useful to log an error message when we fail, to help
distinguish in the logs between the two different failure cases.
>
> -st_magn_buffer_predisable_error:
> kfree(mdata->buffer_data);
> return err;
> }
>
> static const struct iio_buffer_setup_ops st_magn_buffer_setup_ops = {
> - .preenable = &st_magn_buffer_preenable,
> .postenable = &st_magn_buffer_postenable,
> .predisable = &st_magn_buffer_predisable,
> };
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] iio:st_magn: enable trigger before enabling sensor
2018-10-25 22:32 [PATCH] iio:st_magn: enable trigger before enabling sensor Denis Ciocca
2018-10-26 1:28 ` Martin Kelly
@ 2018-10-28 16:20 ` Jonathan Cameron
2018-10-28 19:07 ` Martin Kelly
1 sibling, 1 reply; 7+ messages in thread
From: Jonathan Cameron @ 2018-10-28 16:20 UTC (permalink / raw)
To: Denis Ciocca
Cc: linux-iio, lars, lorenzo.bianconi83, pmeerw, martin, knaack.h
On Thu, 25 Oct 2018 15:32:26 -0700
Denis Ciocca <denis.ciocca@st.com> wrote:
> From logical point of view driver should be ready to
> receive irqs before enabling the sensor itself.
> This patch is fixing also an issue related to
> sensors that generate interrupts unconditionally,
> (DRDY pads) when interrupt level is used.
>
> Signed-off-by: Martin Kelly <martin@martingkelly.com>
> Signed-off-by: Denis Ciocca <denis.ciocca@st.com>
A couple of very minor points inline.
Also, author should match the first sign off. I kind of lost touch of
how much got modified, but a Reported-by: or the one you occasionally get
is Originated-by: for martin would be preferred if the changes are major,
if they are minor, then the author should be martin with a sign off
from Denis.
Also, as I understand it this is a fix for an issue seen, so make that
clear in the naming of the patch and give a fixes-tag to indicate
how far back we should apply this in stable.
Thanks,
Jonathan
> ---
> drivers/iio/magnetometer/st_magn_buffer.c | 33 +++++++++++------------
> 1 file changed, 16 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/iio/magnetometer/st_magn_buffer.c b/drivers/iio/magnetometer/st_magn_buffer.c
> index 0a9e8fadfa9d..097e6e88a464 100644
> --- a/drivers/iio/magnetometer/st_magn_buffer.c
> +++ b/drivers/iio/magnetometer/st_magn_buffer.c
> @@ -30,11 +30,6 @@ int st_magn_trig_set_state(struct iio_trigger *trig, bool state)
> return st_sensors_set_dataready_irq(indio_dev, state);
> }
>
> -static int st_magn_buffer_preenable(struct iio_dev *indio_dev)
> -{
> - return st_sensors_set_enable(indio_dev, true);
> -}
> -
> static int st_magn_buffer_postenable(struct iio_dev *indio_dev)
> {
> int err;
> @@ -42,40 +37,44 @@ static int st_magn_buffer_postenable(struct iio_dev *indio_dev)
>
> mdata->buffer_data = kmalloc(indio_dev->scan_bytes, GFP_KERNEL);
> if (mdata->buffer_data == NULL) {
> - err = -ENOMEM;
> - goto allocate_memory_error;
> + return -ENOMEM;
I would have a very slight preference for that having been in a separate patch.
Good cleanup but not part of the main point of this patch.
> }
>
> err = iio_triggered_buffer_postenable(indio_dev);
> if (err < 0)
> - goto st_magn_buffer_postenable_error;
> + goto st_magn_buffer_free_buffer_data;
> +
> + err = st_sensors_set_enable(indio_dev, true);
> + if (err < 0)
> + goto st_magn_buffer_buffer_predisable;
>
> return err;
>
> -st_magn_buffer_postenable_error:
> +st_magn_buffer_buffer_predisable:
> + iio_triggered_buffer_predisable(indio_dev);
> +st_magn_buffer_free_buffer_data:
> kfree(mdata->buffer_data);
> -allocate_memory_error:
> return err;
> }
>
> static int st_magn_buffer_predisable(struct iio_dev *indio_dev)
> {
> - int err;
> + int err = 0, err2;
> struct st_sensor_data *mdata = iio_priv(indio_dev);
>
> - err = iio_triggered_buffer_predisable(indio_dev);
> - if (err < 0)
> - goto st_magn_buffer_predisable_error;
> + err2 = st_sensors_set_enable(indio_dev, false);
> + if (err2 < 0)
> + err = err2;
>
> - err = st_sensors_set_enable(indio_dev, false);
> + err2 = iio_triggered_buffer_predisable(indio_dev);
> + if (err2 < 0)
> + err = err2;
There is a small argument that we should have some visibility of
the value of the error from st_sensors_set_enable if both
error paths trigger. I think the suggestion made in the
other review of an error comment would solve that fairly well.
>
> -st_magn_buffer_predisable_error:
> kfree(mdata->buffer_data);
> return err;
> }
>
> static const struct iio_buffer_setup_ops st_magn_buffer_setup_ops = {
> - .preenable = &st_magn_buffer_preenable,
> .postenable = &st_magn_buffer_postenable,
> .predisable = &st_magn_buffer_predisable,
> };
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] iio:st_magn: enable trigger before enabling sensor
2018-10-28 16:20 ` Jonathan Cameron
@ 2018-10-28 19:07 ` Martin Kelly
2018-10-28 22:55 ` Denis CIOCCA
0 siblings, 1 reply; 7+ messages in thread
From: Martin Kelly @ 2018-10-28 19:07 UTC (permalink / raw)
To: Jonathan Cameron, Denis Ciocca
Cc: linux-iio, lars, lorenzo.bianconi83, pmeerw, knaack.h
On 10/28/18 9:20 AM, Jonathan Cameron wrote:
> On Thu, 25 Oct 2018 15:32:26 -0700
> Denis Ciocca <denis.ciocca@st.com> wrote:
>
>> From logical point of view driver should be ready to
>> receive irqs before enabling the sensor itself.
>> This patch is fixing also an issue related to
>> sensors that generate interrupts unconditionally,
>> (DRDY pads) when interrupt level is used.
>>
>> Signed-off-by: Martin Kelly <martin@martingkelly.com>
>> Signed-off-by: Denis Ciocca <denis.ciocca@st.com>
> A couple of very minor points inline.
>
> Also, author should match the first sign off. I kind of lost touch of
> how much got modified, but a Reported-by: or the one you occasionally get
> is Originated-by: for martin would be preferred if the changes are major,
> if they are minor, then the author should be martin with a sign off
> from Denis.
>
I would say the changes are minor, although it's subjective exactly what
"minor" vs. "major" are, of course. In this case, Denis added some
goto-error cleanup that I had missed, but the bug fix was from the
original patch.
> Also, as I understand it this is a fix for an issue seen, so make that
> clear in the naming of the patch and give a fixes-tag to indicate
> how far back we should apply this in stable.
>
> Thanks,
>
> Jonathan
>
>> ---
>> drivers/iio/magnetometer/st_magn_buffer.c | 33 +++++++++++------------
>> 1 file changed, 16 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/iio/magnetometer/st_magn_buffer.c b/drivers/iio/magnetometer/st_magn_buffer.c
>> index 0a9e8fadfa9d..097e6e88a464 100644
>> --- a/drivers/iio/magnetometer/st_magn_buffer.c
>> +++ b/drivers/iio/magnetometer/st_magn_buffer.c
>> @@ -30,11 +30,6 @@ int st_magn_trig_set_state(struct iio_trigger *trig, bool state)
>> return st_sensors_set_dataready_irq(indio_dev, state);
>> }
>>
>> -static int st_magn_buffer_preenable(struct iio_dev *indio_dev)
>> -{
>> - return st_sensors_set_enable(indio_dev, true);
>> -}
>> -
>> static int st_magn_buffer_postenable(struct iio_dev *indio_dev)
>> {
>> int err;
>> @@ -42,40 +37,44 @@ static int st_magn_buffer_postenable(struct iio_dev *indio_dev)
>>
>> mdata->buffer_data = kmalloc(indio_dev->scan_bytes, GFP_KERNEL);
>> if (mdata->buffer_data == NULL) {
>> - err = -ENOMEM;
>> - goto allocate_memory_error;
>> + return -ENOMEM;
> I would have a very slight preference for that having been in a separate patch.
> Good cleanup but not part of the main point of this patch.
>
>> }
>>
>> err = iio_triggered_buffer_postenable(indio_dev);
>> if (err < 0)
>> - goto st_magn_buffer_postenable_error;
>> + goto st_magn_buffer_free_buffer_data;
>> +
>> + err = st_sensors_set_enable(indio_dev, true);
>> + if (err < 0)
>> + goto st_magn_buffer_buffer_predisable;
>>
>> return err;
>>
>> -st_magn_buffer_postenable_error:
>> +st_magn_buffer_buffer_predisable:
>> + iio_triggered_buffer_predisable(indio_dev);
>> +st_magn_buffer_free_buffer_data:
>> kfree(mdata->buffer_data);
>> -allocate_memory_error:
>> return err;
>> }
>>
>> static int st_magn_buffer_predisable(struct iio_dev *indio_dev)
>> {
>> - int err;
>> + int err = 0, err2;
>> struct st_sensor_data *mdata = iio_priv(indio_dev);
>>
>> - err = iio_triggered_buffer_predisable(indio_dev);
>> - if (err < 0)
>> - goto st_magn_buffer_predisable_error;
>> + err2 = st_sensors_set_enable(indio_dev, false);
>> + if (err2 < 0)
>> + err = err2;
>>
>> - err = st_sensors_set_enable(indio_dev, false);
>> + err2 = iio_triggered_buffer_predisable(indio_dev);
>> + if (err2 < 0)
>> + err = err2;
> There is a small argument that we should have some visibility of
> the value of the error from st_sensors_set_enable if both
> error paths trigger. I think the suggestion made in the
> other review of an error comment would solve that fairly well.
>
>>
>> -st_magn_buffer_predisable_error:
>> kfree(mdata->buffer_data);
>> return err;
>> }
>>
>> static const struct iio_buffer_setup_ops st_magn_buffer_setup_ops = {
>> - .preenable = &st_magn_buffer_preenable,
>> .postenable = &st_magn_buffer_postenable,
>> .predisable = &st_magn_buffer_predisable,
>> };
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH] iio:st_magn: enable trigger before enabling sensor
2018-10-28 19:07 ` Martin Kelly
@ 2018-10-28 22:55 ` Denis CIOCCA
2018-10-28 23:15 ` Martin Kelly
0 siblings, 1 reply; 7+ messages in thread
From: Denis CIOCCA @ 2018-10-28 22:55 UTC (permalink / raw)
To: Martin Kelly, Jonathan Cameron
Cc: linux-iio, lars, lorenzo.bianconi83, pmeerw, knaack.h
SGV5IE1hcnRpbiwgSm9uYXRoYW4sDQoNClNvcnJ5IG15IGJhZC4gSSBoYWQgcHVzaGVkIHdoYXQg
SSd2ZSBtb2RpZmllZCBwZXJmb3JtaW5nIHRoZSB0ZXN0cyBub3QgY29uc2lkZXJpbmcgYXQgYWxs
IHRoZSBwcm9jZWR1cmVzLiBJIGRpZG4ndCBtZWFuIHRvIHRha2UgYWR2YW50YWdlLiBJIGFwb2xv
Z2l6ZS4NCg0KTWFydGluLCBpZiB5b3UgY2FuLCBwbGVhc2UganVzdCB0YWtlIHRoZSBwYXJ0IG5l
ZWRlZCBhbmQgdXBkYXRlIGFjY29yZGFudGx5IHRoZSBmaXJzdCBwYXRjaCB5b3Ugc2VudCBwbGVh
c2UuIEkgdGhpbmsgSSBqdXN0IGFwcGx5IGEgJ21pbm9yJyBjaGFuZ2UgaW4gdGhlcmUuIFRoZSBt
YWpvciBzdHVmZiBoZXJlIHdhcyB0aGUgZGV0ZWN0aW9uIG9mIHRoZSB3cm9uZyBvcmRlciBhbmQg
TWFydGluIGRpZCB0aGUgam9iIQ0KDQpUaGFua3MhDQpEZW5pcw0KDQoNCg0KLS0tLS1PcmlnaW5h
bCBNZXNzYWdlLS0tLS0NCkZyb206IE1hcnRpbiBLZWxseSA8bWFydGluQG1hcnRpbmdrZWxseS5j
b20+IA0KU2VudDogU3VuZGF5LCBPY3RvYmVyIDI4LCAyMDE4IDEyOjA3IFBNDQpUbzogSm9uYXRo
YW4gQ2FtZXJvbiA8amljMjNAa2VybmVsLm9yZz47IERlbmlzIENJT0NDQSA8ZGVuaXMuY2lvY2Nh
QHN0LmNvbT4NCkNjOiBsaW51eC1paW9Admdlci5rZXJuZWwub3JnOyBsYXJzQG1ldGFmb28uZGU7
IGxvcmVuem8uYmlhbmNvbmk4M0BnbWFpbC5jb207IHBtZWVyd0BwbWVlcncubmV0OyBrbmFhY2su
aEBnbXguZGUNClN1YmplY3Q6IFJlOiBbUEFUQ0hdIGlpbzpzdF9tYWduOiBlbmFibGUgdHJpZ2dl
ciBiZWZvcmUgZW5hYmxpbmcgc2Vuc29yDQoNCk9uIDEwLzI4LzE4IDk6MjAgQU0sIEpvbmF0aGFu
IENhbWVyb24gd3JvdGU6DQo+IE9uIFRodSwgMjUgT2N0IDIwMTggMTU6MzI6MjYgLTA3MDANCj4g
RGVuaXMgQ2lvY2NhIDxkZW5pcy5jaW9jY2FAc3QuY29tPiB3cm90ZToNCj4gDQo+PiAgRnJvbSBs
b2dpY2FsIHBvaW50IG9mIHZpZXcgZHJpdmVyIHNob3VsZCBiZSByZWFkeSB0byByZWNlaXZlIGly
cXMgDQo+PiBiZWZvcmUgZW5hYmxpbmcgdGhlIHNlbnNvciBpdHNlbGYuDQo+PiBUaGlzIHBhdGNo
IGlzIGZpeGluZyBhbHNvIGFuIGlzc3VlIHJlbGF0ZWQgdG8gc2Vuc29ycyB0aGF0IGdlbmVyYXRl
IA0KPj4gaW50ZXJydXB0cyB1bmNvbmRpdGlvbmFsbHksIChEUkRZIHBhZHMpIHdoZW4gaW50ZXJy
dXB0IGxldmVsIGlzIHVzZWQuDQo+Pg0KPj4gU2lnbmVkLW9mZi1ieTogTWFydGluIEtlbGx5IDxt
YXJ0aW5AbWFydGluZ2tlbGx5LmNvbT4NCj4+IFNpZ25lZC1vZmYtYnk6IERlbmlzIENpb2NjYSA8
ZGVuaXMuY2lvY2NhQHN0LmNvbT4NCj4gQSBjb3VwbGUgb2YgdmVyeSBtaW5vciBwb2ludHMgaW5s
aW5lLg0KPiANCj4gQWxzbywgYXV0aG9yIHNob3VsZCBtYXRjaCB0aGUgZmlyc3Qgc2lnbiBvZmYu
ICBJIGtpbmQgb2YgbG9zdCB0b3VjaCBvZiANCj4gaG93IG11Y2ggZ290IG1vZGlmaWVkLCBidXQg
YSBSZXBvcnRlZC1ieTogb3IgdGhlIG9uZSB5b3Ugb2NjYXNpb25hbGx5IA0KPiBnZXQgaXMgT3Jp
Z2luYXRlZC1ieTogZm9yIG1hcnRpbiB3b3VsZCBiZSBwcmVmZXJyZWQgaWYgdGhlIGNoYW5nZXMg
YXJlIA0KPiBtYWpvciwgaWYgdGhleSBhcmUgbWlub3IsIHRoZW4gdGhlIGF1dGhvciBzaG91bGQg
YmUgbWFydGluIHdpdGggYSBzaWduIA0KPiBvZmYgZnJvbSBEZW5pcy4NCj4gDQoNCkkgd291bGQg
c2F5IHRoZSBjaGFuZ2VzIGFyZSBtaW5vciwgYWx0aG91Z2ggaXQncyBzdWJqZWN0aXZlIGV4YWN0
bHkgd2hhdCAibWlub3IiIHZzLiAibWFqb3IiIGFyZSwgb2YgY291cnNlLiBJbiB0aGlzIGNhc2Us
IERlbmlzIGFkZGVkIHNvbWUgZ290by1lcnJvciBjbGVhbnVwIHRoYXQgSSBoYWQgbWlzc2VkLCBi
dXQgdGhlIGJ1ZyBmaXggd2FzIGZyb20gdGhlIG9yaWdpbmFsIHBhdGNoLg0KDQo+IEFsc28sIGFz
IEkgdW5kZXJzdGFuZCBpdCB0aGlzIGlzIGEgZml4IGZvciBhbiBpc3N1ZSBzZWVuLCBzbyBtYWtl
IHRoYXQgDQo+IGNsZWFyIGluIHRoZSBuYW1pbmcgb2YgdGhlIHBhdGNoIGFuZCBnaXZlIGEgZml4
ZXMtdGFnIHRvIGluZGljYXRlIGhvdyANCj4gZmFyIGJhY2sgd2Ugc2hvdWxkIGFwcGx5IHRoaXMg
aW4gc3RhYmxlLg0KPiANCj4gVGhhbmtzLA0KPiANCj4gSm9uYXRoYW4NCj4gDQo+PiAtLS0NCj4+
ICAgZHJpdmVycy9paW8vbWFnbmV0b21ldGVyL3N0X21hZ25fYnVmZmVyLmMgfCAzMyArKysrKysr
KysrKy0tLS0tLS0tLS0tLQ0KPj4gICAxIGZpbGUgY2hhbmdlZCwgMTYgaW5zZXJ0aW9ucygrKSwg
MTcgZGVsZXRpb25zKC0pDQo+Pg0KPj4gZGlmZiAtLWdpdCBhL2RyaXZlcnMvaWlvL21hZ25ldG9t
ZXRlci9zdF9tYWduX2J1ZmZlci5jIA0KPj4gYi9kcml2ZXJzL2lpby9tYWduZXRvbWV0ZXIvc3Rf
bWFnbl9idWZmZXIuYw0KPj4gaW5kZXggMGE5ZThmYWRmYTlkLi4wOTdlNmU4OGE0NjQgMTAwNjQ0
DQo+PiAtLS0gYS9kcml2ZXJzL2lpby9tYWduZXRvbWV0ZXIvc3RfbWFnbl9idWZmZXIuYw0KPj4g
KysrIGIvZHJpdmVycy9paW8vbWFnbmV0b21ldGVyL3N0X21hZ25fYnVmZmVyLmMNCj4+IEBAIC0z
MCwxMSArMzAsNiBAQCBpbnQgc3RfbWFnbl90cmlnX3NldF9zdGF0ZShzdHJ1Y3QgaWlvX3RyaWdn
ZXIgKnRyaWcsIGJvb2wgc3RhdGUpDQo+PiAgIAlyZXR1cm4gc3Rfc2Vuc29yc19zZXRfZGF0YXJl
YWR5X2lycShpbmRpb19kZXYsIHN0YXRlKTsNCj4+ICAgfQ0KPj4gICANCj4+IC1zdGF0aWMgaW50
IHN0X21hZ25fYnVmZmVyX3ByZWVuYWJsZShzdHJ1Y3QgaWlvX2RldiAqaW5kaW9fZGV2KSAtew0K
Pj4gLQlyZXR1cm4gc3Rfc2Vuc29yc19zZXRfZW5hYmxlKGluZGlvX2RldiwgdHJ1ZSk7DQo+PiAt
fQ0KPj4gLQ0KPj4gICBzdGF0aWMgaW50IHN0X21hZ25fYnVmZmVyX3Bvc3RlbmFibGUoc3RydWN0
IGlpb19kZXYgKmluZGlvX2RldikNCj4+ICAgew0KPj4gICAJaW50IGVycjsNCj4+IEBAIC00Miw0
MCArMzcsNDQgQEAgc3RhdGljIGludCBzdF9tYWduX2J1ZmZlcl9wb3N0ZW5hYmxlKHN0cnVjdCAN
Cj4+IGlpb19kZXYgKmluZGlvX2RldikNCj4+ICAgDQo+PiAgIAltZGF0YS0+YnVmZmVyX2RhdGEg
PSBrbWFsbG9jKGluZGlvX2Rldi0+c2Nhbl9ieXRlcywgR0ZQX0tFUk5FTCk7DQo+PiAgIAlpZiAo
bWRhdGEtPmJ1ZmZlcl9kYXRhID09IE5VTEwpIHsNCj4+IC0JCWVyciA9IC1FTk9NRU07DQo+PiAt
CQlnb3RvIGFsbG9jYXRlX21lbW9yeV9lcnJvcjsNCj4+ICsJCXJldHVybiAtRU5PTUVNOw0KPiBJ
IHdvdWxkIGhhdmUgYSB2ZXJ5IHNsaWdodCBwcmVmZXJlbmNlIGZvciB0aGF0IGhhdmluZyBiZWVu
IGluIGEgc2VwYXJhdGUgcGF0Y2guDQo+IEdvb2QgY2xlYW51cCBidXQgbm90IHBhcnQgb2YgdGhl
IG1haW4gcG9pbnQgb2YgdGhpcyBwYXRjaC4NCj4gDQo+PiAgIAl9DQo+PiAgIA0KPj4gICAJZXJy
ID0gaWlvX3RyaWdnZXJlZF9idWZmZXJfcG9zdGVuYWJsZShpbmRpb19kZXYpOw0KPj4gICAJaWYg
KGVyciA8IDApDQo+PiAtCQlnb3RvIHN0X21hZ25fYnVmZmVyX3Bvc3RlbmFibGVfZXJyb3I7DQo+
PiArCQlnb3RvIHN0X21hZ25fYnVmZmVyX2ZyZWVfYnVmZmVyX2RhdGE7DQo+PiArDQo+PiArCWVy
ciA9IHN0X3NlbnNvcnNfc2V0X2VuYWJsZShpbmRpb19kZXYsIHRydWUpOw0KPj4gKwlpZiAoZXJy
IDwgMCkNCj4+ICsJCWdvdG8gc3RfbWFnbl9idWZmZXJfYnVmZmVyX3ByZWRpc2FibGU7DQo+PiAg
IA0KPj4gICAJcmV0dXJuIGVycjsNCj4+ICAgDQo+PiAtc3RfbWFnbl9idWZmZXJfcG9zdGVuYWJs
ZV9lcnJvcjoNCj4+ICtzdF9tYWduX2J1ZmZlcl9idWZmZXJfcHJlZGlzYWJsZToNCj4+ICsJaWlv
X3RyaWdnZXJlZF9idWZmZXJfcHJlZGlzYWJsZShpbmRpb19kZXYpOw0KPj4gK3N0X21hZ25fYnVm
ZmVyX2ZyZWVfYnVmZmVyX2RhdGE6DQo+PiAgIAlrZnJlZShtZGF0YS0+YnVmZmVyX2RhdGEpOw0K
Pj4gLWFsbG9jYXRlX21lbW9yeV9lcnJvcjoNCj4+ICAgCXJldHVybiBlcnI7DQo+PiAgIH0NCj4+
ICAgDQo+PiAgIHN0YXRpYyBpbnQgc3RfbWFnbl9idWZmZXJfcHJlZGlzYWJsZShzdHJ1Y3QgaWlv
X2RldiAqaW5kaW9fZGV2KQ0KPj4gICB7DQo+PiAtCWludCBlcnI7DQo+PiArCWludCBlcnIgPSAw
LCBlcnIyOw0KPj4gICAJc3RydWN0IHN0X3NlbnNvcl9kYXRhICptZGF0YSA9IGlpb19wcml2KGlu
ZGlvX2Rldik7DQo+PiAgIA0KPj4gLQllcnIgPSBpaW9fdHJpZ2dlcmVkX2J1ZmZlcl9wcmVkaXNh
YmxlKGluZGlvX2Rldik7DQo+PiAtCWlmIChlcnIgPCAwKQ0KPj4gLQkJZ290byBzdF9tYWduX2J1
ZmZlcl9wcmVkaXNhYmxlX2Vycm9yOw0KPj4gKwllcnIyID0gc3Rfc2Vuc29yc19zZXRfZW5hYmxl
KGluZGlvX2RldiwgZmFsc2UpOw0KPj4gKwlpZiAoZXJyMiA8IDApDQo+PiArCQllcnIgPSBlcnIy
Ow0KPj4gICANCj4+IC0JZXJyID0gc3Rfc2Vuc29yc19zZXRfZW5hYmxlKGluZGlvX2RldiwgZmFs
c2UpOw0KPj4gKwllcnIyID0gaWlvX3RyaWdnZXJlZF9idWZmZXJfcHJlZGlzYWJsZShpbmRpb19k
ZXYpOw0KPj4gKwlpZiAoZXJyMiA8IDApDQo+PiArCQllcnIgPSBlcnIyOw0KPiBUaGVyZSBpcyBh
IHNtYWxsIGFyZ3VtZW50IHRoYXQgd2Ugc2hvdWxkIGhhdmUgc29tZSB2aXNpYmlsaXR5IG9mIHRo
ZSANCj4gdmFsdWUgb2YgdGhlIGVycm9yIGZyb20gc3Rfc2Vuc29yc19zZXRfZW5hYmxlIGlmIGJv
dGggZXJyb3IgcGF0aHMgDQo+IHRyaWdnZXIuICBJIHRoaW5rIHRoZSBzdWdnZXN0aW9uIG1hZGUg
aW4gdGhlIG90aGVyIHJldmlldyBvZiBhbiBlcnJvciANCj4gY29tbWVudCB3b3VsZCBzb2x2ZSB0
aGF0IGZhaXJseSB3ZWxsLg0KPiANCj4+ICAgDQo+PiAtc3RfbWFnbl9idWZmZXJfcHJlZGlzYWJs
ZV9lcnJvcjoNCj4+ICAgCWtmcmVlKG1kYXRhLT5idWZmZXJfZGF0YSk7DQo+PiAgIAlyZXR1cm4g
ZXJyOw0KPj4gICB9DQo+PiAgIA0KPj4gICBzdGF0aWMgY29uc3Qgc3RydWN0IGlpb19idWZmZXJf
c2V0dXBfb3BzIHN0X21hZ25fYnVmZmVyX3NldHVwX29wcyA9IHsNCj4+IC0JLnByZWVuYWJsZSA9
ICZzdF9tYWduX2J1ZmZlcl9wcmVlbmFibGUsDQo+PiAgIAkucG9zdGVuYWJsZSA9ICZzdF9tYWdu
X2J1ZmZlcl9wb3N0ZW5hYmxlLA0KPj4gICAJLnByZWRpc2FibGUgPSAmc3RfbWFnbl9idWZmZXJf
cHJlZGlzYWJsZSwNCj4+ICAgfTsNCj4gDQoNCg==
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] iio:st_magn: enable trigger before enabling sensor
2018-10-28 22:55 ` Denis CIOCCA
@ 2018-10-28 23:15 ` Martin Kelly
2018-11-03 10:26 ` Jonathan Cameron
0 siblings, 1 reply; 7+ messages in thread
From: Martin Kelly @ 2018-10-28 23:15 UTC (permalink / raw)
To: Denis CIOCCA, Jonathan Cameron
Cc: linux-iio, lars, lorenzo.bianconi83, pmeerw, knaack.h
On 10/28/18 3:55 PM, Denis CIOCCA wrote:
> Hey Martin, Jonathan,
>
> Sorry my bad. I had pushed what I've modified performing the tests not considering at all the procedures. I didn't mean to take advantage. I apologize.
>
> Martin, if you can, please just take the part needed and update accordantly the first patch you sent please. I think I just apply a 'minor' change in there. The major stuff here was the detection of the wrong order and Martin did the job!
>
> Thanks!
> Denis
>
No problem, patch "authorship" can be a rather fuzzy concept in a
collaborative open source project. I sometimes wish git would allow for
multiple authors like academic papers have, as it's sometimes the most
correct expression of how a patch comes to be.
I'll send the original patch in the next few days, making sure I cover
the goto-errors properly and re-testing (I think I may have missed them
the first time). I'll add a Fixes tag too.
After that, Denis may want to issue some of the cleanups that were
included here as a separate patch.
>
>
> -----Original Message-----
> From: Martin Kelly <martin@martingkelly.com>
> Sent: Sunday, October 28, 2018 12:07 PM
> To: Jonathan Cameron <jic23@kernel.org>; Denis CIOCCA <denis.ciocca@st.com>
> Cc: linux-iio@vger.kernel.org; lars@metafoo.de; lorenzo.bianconi83@gmail.com; pmeerw@pmeerw.net; knaack.h@gmx.de
> Subject: Re: [PATCH] iio:st_magn: enable trigger before enabling sensor
>
> On 10/28/18 9:20 AM, Jonathan Cameron wrote:
>> On Thu, 25 Oct 2018 15:32:26 -0700
>> Denis Ciocca <denis.ciocca@st.com> wrote:
>>
>>> From logical point of view driver should be ready to receive irqs
>>> before enabling the sensor itself.
>>> This patch is fixing also an issue related to sensors that generate
>>> interrupts unconditionally, (DRDY pads) when interrupt level is used.
>>>
>>> Signed-off-by: Martin Kelly <martin@martingkelly.com>
>>> Signed-off-by: Denis Ciocca <denis.ciocca@st.com>
>> A couple of very minor points inline.
>>
>> Also, author should match the first sign off. I kind of lost touch of
>> how much got modified, but a Reported-by: or the one you occasionally
>> get is Originated-by: for martin would be preferred if the changes are
>> major, if they are minor, then the author should be martin with a sign
>> off from Denis.
>>
>
> I would say the changes are minor, although it's subjective exactly what "minor" vs. "major" are, of course. In this case, Denis added some goto-error cleanup that I had missed, but the bug fix was from the original patch.
>
>> Also, as I understand it this is a fix for an issue seen, so make that
>> clear in the naming of the patch and give a fixes-tag to indicate how
>> far back we should apply this in stable.
>>
>> Thanks,
>>
>> Jonathan
>>
>>> ---
>>> drivers/iio/magnetometer/st_magn_buffer.c | 33 +++++++++++------------
>>> 1 file changed, 16 insertions(+), 17 deletions(-)
>>>
>>> diff --git a/drivers/iio/magnetometer/st_magn_buffer.c
>>> b/drivers/iio/magnetometer/st_magn_buffer.c
>>> index 0a9e8fadfa9d..097e6e88a464 100644
>>> --- a/drivers/iio/magnetometer/st_magn_buffer.c
>>> +++ b/drivers/iio/magnetometer/st_magn_buffer.c
>>> @@ -30,11 +30,6 @@ int st_magn_trig_set_state(struct iio_trigger *trig, bool state)
>>> return st_sensors_set_dataready_irq(indio_dev, state);
>>> }
>>>
>>> -static int st_magn_buffer_preenable(struct iio_dev *indio_dev) -{
>>> - return st_sensors_set_enable(indio_dev, true);
>>> -}
>>> -
>>> static int st_magn_buffer_postenable(struct iio_dev *indio_dev)
>>> {
>>> int err;
>>> @@ -42,40 +37,44 @@ static int st_magn_buffer_postenable(struct
>>> iio_dev *indio_dev)
>>>
>>> mdata->buffer_data = kmalloc(indio_dev->scan_bytes, GFP_KERNEL);
>>> if (mdata->buffer_data == NULL) {
>>> - err = -ENOMEM;
>>> - goto allocate_memory_error;
>>> + return -ENOMEM;
>> I would have a very slight preference for that having been in a separate patch.
>> Good cleanup but not part of the main point of this patch.
>>
>>> }
>>>
>>> err = iio_triggered_buffer_postenable(indio_dev);
>>> if (err < 0)
>>> - goto st_magn_buffer_postenable_error;
>>> + goto st_magn_buffer_free_buffer_data;
>>> +
>>> + err = st_sensors_set_enable(indio_dev, true);
>>> + if (err < 0)
>>> + goto st_magn_buffer_buffer_predisable;
>>>
>>> return err;
>>>
>>> -st_magn_buffer_postenable_error:
>>> +st_magn_buffer_buffer_predisable:
>>> + iio_triggered_buffer_predisable(indio_dev);
>>> +st_magn_buffer_free_buffer_data:
>>> kfree(mdata->buffer_data);
>>> -allocate_memory_error:
>>> return err;
>>> }
>>>
>>> static int st_magn_buffer_predisable(struct iio_dev *indio_dev)
>>> {
>>> - int err;
>>> + int err = 0, err2;
>>> struct st_sensor_data *mdata = iio_priv(indio_dev);
>>>
>>> - err = iio_triggered_buffer_predisable(indio_dev);
>>> - if (err < 0)
>>> - goto st_magn_buffer_predisable_error;
>>> + err2 = st_sensors_set_enable(indio_dev, false);
>>> + if (err2 < 0)
>>> + err = err2;
>>>
>>> - err = st_sensors_set_enable(indio_dev, false);
>>> + err2 = iio_triggered_buffer_predisable(indio_dev);
>>> + if (err2 < 0)
>>> + err = err2;
>> There is a small argument that we should have some visibility of the
>> value of the error from st_sensors_set_enable if both error paths
>> trigger. I think the suggestion made in the other review of an error
>> comment would solve that fairly well.
>>
>>>
>>> -st_magn_buffer_predisable_error:
>>> kfree(mdata->buffer_data);
>>> return err;
>>> }
>>>
>>> static const struct iio_buffer_setup_ops st_magn_buffer_setup_ops = {
>>> - .preenable = &st_magn_buffer_preenable,
>>> .postenable = &st_magn_buffer_postenable,
>>> .predisable = &st_magn_buffer_predisable,
>>> };
>>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] iio:st_magn: enable trigger before enabling sensor
2018-10-28 23:15 ` Martin Kelly
@ 2018-11-03 10:26 ` Jonathan Cameron
0 siblings, 0 replies; 7+ messages in thread
From: Jonathan Cameron @ 2018-11-03 10:26 UTC (permalink / raw)
To: Martin Kelly
Cc: Denis CIOCCA, linux-iio, lars, lorenzo.bianconi83, pmeerw, knaack.h
On Sun, 28 Oct 2018 16:15:21 -0700
Martin Kelly <martin@martingkelly.com> wrote:
> On 10/28/18 3:55 PM, Denis CIOCCA wrote:
> > Hey Martin, Jonathan,
> >
> > Sorry my bad. I had pushed what I've modified performing the tests not considering at all the procedures. I didn't mean to take advantage. I apologize.
> >
> > Martin, if you can, please just take the part needed and update accordantly the first patch you sent please. I think I just apply a 'minor' change in there. The major stuff here was the detection of the wrong order and Martin did the job!
> >
> > Thanks!
> > Denis
> >
>
> No problem, patch "authorship" can be a rather fuzzy concept in a
> collaborative open source project. I sometimes wish git would allow for
> multiple authors like academic papers have, as it's sometimes the most
> correct expression of how a patch comes to be.
Agreed. It would indeed be nice if that were possible.
>
> I'll send the original patch in the next few days, making sure I cover
> the goto-errors properly and re-testing (I think I may have missed them
> the first time). I'll add a Fixes tag too.
>
> After that, Denis may want to issue some of the cleanups that were
> included here as a separate patch.
Cool. Thanks for sorting this.
Jonathan
>
> >
> >
> > -----Original Message-----
> > From: Martin Kelly <martin@martingkelly.com>
> > Sent: Sunday, October 28, 2018 12:07 PM
> > To: Jonathan Cameron <jic23@kernel.org>; Denis CIOCCA <denis.ciocca@st.com>
> > Cc: linux-iio@vger.kernel.org; lars@metafoo.de; lorenzo.bianconi83@gmail.com; pmeerw@pmeerw.net; knaack.h@gmx.de
> > Subject: Re: [PATCH] iio:st_magn: enable trigger before enabling sensor
> >
> > On 10/28/18 9:20 AM, Jonathan Cameron wrote:
> >> On Thu, 25 Oct 2018 15:32:26 -0700
> >> Denis Ciocca <denis.ciocca@st.com> wrote:
> >>
> >>> From logical point of view driver should be ready to receive irqs
> >>> before enabling the sensor itself.
> >>> This patch is fixing also an issue related to sensors that generate
> >>> interrupts unconditionally, (DRDY pads) when interrupt level is used.
> >>>
> >>> Signed-off-by: Martin Kelly <martin@martingkelly.com>
> >>> Signed-off-by: Denis Ciocca <denis.ciocca@st.com>
> >> A couple of very minor points inline.
> >>
> >> Also, author should match the first sign off. I kind of lost touch of
> >> how much got modified, but a Reported-by: or the one you occasionally
> >> get is Originated-by: for martin would be preferred if the changes are
> >> major, if they are minor, then the author should be martin with a sign
> >> off from Denis.
> >>
> >
> > I would say the changes are minor, although it's subjective exactly what "minor" vs. "major" are, of course. In this case, Denis added some goto-error cleanup that I had missed, but the bug fix was from the original patch.
> >
> >> Also, as I understand it this is a fix for an issue seen, so make that
> >> clear in the naming of the patch and give a fixes-tag to indicate how
> >> far back we should apply this in stable.
> >>
> >> Thanks,
> >>
> >> Jonathan
> >>
> >>> ---
> >>> drivers/iio/magnetometer/st_magn_buffer.c | 33 +++++++++++------------
> >>> 1 file changed, 16 insertions(+), 17 deletions(-)
> >>>
> >>> diff --git a/drivers/iio/magnetometer/st_magn_buffer.c
> >>> b/drivers/iio/magnetometer/st_magn_buffer.c
> >>> index 0a9e8fadfa9d..097e6e88a464 100644
> >>> --- a/drivers/iio/magnetometer/st_magn_buffer.c
> >>> +++ b/drivers/iio/magnetometer/st_magn_buffer.c
> >>> @@ -30,11 +30,6 @@ int st_magn_trig_set_state(struct iio_trigger *trig, bool state)
> >>> return st_sensors_set_dataready_irq(indio_dev, state);
> >>> }
> >>>
> >>> -static int st_magn_buffer_preenable(struct iio_dev *indio_dev) -{
> >>> - return st_sensors_set_enable(indio_dev, true);
> >>> -}
> >>> -
> >>> static int st_magn_buffer_postenable(struct iio_dev *indio_dev)
> >>> {
> >>> int err;
> >>> @@ -42,40 +37,44 @@ static int st_magn_buffer_postenable(struct
> >>> iio_dev *indio_dev)
> >>>
> >>> mdata->buffer_data = kmalloc(indio_dev->scan_bytes, GFP_KERNEL);
> >>> if (mdata->buffer_data == NULL) {
> >>> - err = -ENOMEM;
> >>> - goto allocate_memory_error;
> >>> + return -ENOMEM;
> >> I would have a very slight preference for that having been in a separate patch.
> >> Good cleanup but not part of the main point of this patch.
> >>
> >>> }
> >>>
> >>> err = iio_triggered_buffer_postenable(indio_dev);
> >>> if (err < 0)
> >>> - goto st_magn_buffer_postenable_error;
> >>> + goto st_magn_buffer_free_buffer_data;
> >>> +
> >>> + err = st_sensors_set_enable(indio_dev, true);
> >>> + if (err < 0)
> >>> + goto st_magn_buffer_buffer_predisable;
> >>>
> >>> return err;
> >>>
> >>> -st_magn_buffer_postenable_error:
> >>> +st_magn_buffer_buffer_predisable:
> >>> + iio_triggered_buffer_predisable(indio_dev);
> >>> +st_magn_buffer_free_buffer_data:
> >>> kfree(mdata->buffer_data);
> >>> -allocate_memory_error:
> >>> return err;
> >>> }
> >>>
> >>> static int st_magn_buffer_predisable(struct iio_dev *indio_dev)
> >>> {
> >>> - int err;
> >>> + int err = 0, err2;
> >>> struct st_sensor_data *mdata = iio_priv(indio_dev);
> >>>
> >>> - err = iio_triggered_buffer_predisable(indio_dev);
> >>> - if (err < 0)
> >>> - goto st_magn_buffer_predisable_error;
> >>> + err2 = st_sensors_set_enable(indio_dev, false);
> >>> + if (err2 < 0)
> >>> + err = err2;
> >>>
> >>> - err = st_sensors_set_enable(indio_dev, false);
> >>> + err2 = iio_triggered_buffer_predisable(indio_dev);
> >>> + if (err2 < 0)
> >>> + err = err2;
> >> There is a small argument that we should have some visibility of the
> >> value of the error from st_sensors_set_enable if both error paths
> >> trigger. I think the suggestion made in the other review of an error
> >> comment would solve that fairly well.
> >>
> >>>
> >>> -st_magn_buffer_predisable_error:
> >>> kfree(mdata->buffer_data);
> >>> return err;
> >>> }
> >>>
> >>> static const struct iio_buffer_setup_ops st_magn_buffer_setup_ops = {
> >>> - .preenable = &st_magn_buffer_preenable,
> >>> .postenable = &st_magn_buffer_postenable,
> >>> .predisable = &st_magn_buffer_predisable,
> >>> };
> >>
> >
>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2018-11-03 19:37 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-25 22:32 [PATCH] iio:st_magn: enable trigger before enabling sensor Denis Ciocca
2018-10-26 1:28 ` Martin Kelly
2018-10-28 16:20 ` Jonathan Cameron
2018-10-28 19:07 ` Martin Kelly
2018-10-28 22:55 ` Denis CIOCCA
2018-10-28 23:15 ` Martin Kelly
2018-11-03 10:26 ` Jonathan Cameron
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.