* [PATCH] iio: st_sensors: miscellaneous cleanup
@ 2018-10-15 0:27 Martin Kelly
2018-10-15 16:03 ` Jonathan Cameron
0 siblings, 1 reply; 5+ messages in thread
From: Martin Kelly @ 2018-10-15 0:27 UTC (permalink / raw)
To: linux-iio
Cc: Jonathan Cameron, Lars-Peter Clausen, Peter Meerwald-Stadler,
Martin Kelly
From: Martin Kelly <martin@martingkelly.com>
Miscellaneous cleanup to fix minor consistency, grammar, and spelling
issues.
Signed-off-by: Martin Kelly <martin@martingkelly.com>
---
drivers/iio/common/st_sensors/st_sensors_core.c | 4 ++--
drivers/iio/common/st_sensors/st_sensors_trigger.c | 4 ++--
drivers/iio/magnetometer/st_magn_core.c | 6 +++---
3 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/iio/common/st_sensors/st_sensors_core.c b/drivers/iio/common/st_sensors/st_sensors_core.c
index 26fbd1bd9413..82882bc505f5 100644
--- a/drivers/iio/common/st_sensors/st_sensors_core.c
+++ b/drivers/iio/common/st_sensors/st_sensors_core.c
@@ -133,7 +133,7 @@ static int st_sensors_match_fs(struct st_sensor_settings *sensor_settings,
for (i = 0; i < ST_SENSORS_FULLSCALE_AVL_MAX; i++) {
if (sensor_settings->fs.fs_avl[i].num == 0)
- goto st_sensors_match_odr_error;
+ goto st_sensors_match_fs_error;
if (sensor_settings->fs.fs_avl[i].num == fs) {
*index_fs_avl = i;
@@ -142,7 +142,7 @@ static int st_sensors_match_fs(struct st_sensor_settings *sensor_settings,
}
}
-st_sensors_match_odr_error:
+st_sensors_match_fs_error:
return ret;
}
diff --git a/drivers/iio/common/st_sensors/st_sensors_trigger.c b/drivers/iio/common/st_sensors/st_sensors_trigger.c
index fdcc5a891958..224596b0e189 100644
--- a/drivers/iio/common/st_sensors/st_sensors_trigger.c
+++ b/drivers/iio/common/st_sensors/st_sensors_trigger.c
@@ -104,7 +104,7 @@ static irqreturn_t st_sensors_irq_thread(int irq, void *p)
return IRQ_HANDLED;
/*
- * If we are using egde IRQs, new samples arrived while processing
+ * If we are using edge IRQs, new samples arrived while processing
* the IRQ and those may be missed unless we pick them here, so poll
* again. If the sensor delivery frequency is very high, this thread
* turns into a polled loop handler.
@@ -148,7 +148,7 @@ int st_sensors_allocate_trigger(struct iio_dev *indio_dev,
if (!sdata->sensor_settings->drdy_irq.addr_ihl) {
dev_err(&indio_dev->dev,
"falling/low specified for IRQ "
- "but hardware only support rising/high: "
+ "but hardware supports only rising/high: "
"will request rising/high\n");
if (irq_trig == IRQF_TRIGGER_FALLING)
irq_trig = IRQF_TRIGGER_RISING;
diff --git a/drivers/iio/magnetometer/st_magn_core.c b/drivers/iio/magnetometer/st_magn_core.c
index 72f6d1335a04..880c11c7f1cb 100644
--- a/drivers/iio/magnetometer/st_magn_core.c
+++ b/drivers/iio/magnetometer/st_magn_core.c
@@ -29,9 +29,9 @@
#define ST_MAGN_NUMBER_DATA_CHANNELS 3
/* DEFAULT VALUE FOR SENSORS */
-#define ST_MAGN_DEFAULT_OUT_X_H_ADDR 0X03
-#define ST_MAGN_DEFAULT_OUT_Y_H_ADDR 0X07
-#define ST_MAGN_DEFAULT_OUT_Z_H_ADDR 0X05
+#define ST_MAGN_DEFAULT_OUT_X_H_ADDR 0x03
+#define ST_MAGN_DEFAULT_OUT_Y_H_ADDR 0x07
+#define ST_MAGN_DEFAULT_OUT_Z_H_ADDR 0x05
/* FULLSCALE */
#define ST_MAGN_FS_AVL_1300MG 1300
--
2.11.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] iio: st_sensors: miscellaneous cleanup
2018-10-15 0:27 [PATCH] iio: st_sensors: miscellaneous cleanup Martin Kelly
@ 2018-10-15 16:03 ` Jonathan Cameron
2018-10-15 16:32 ` Martin Kelly
0 siblings, 1 reply; 5+ messages in thread
From: Jonathan Cameron @ 2018-10-15 16:03 UTC (permalink / raw)
To: Martin Kelly
Cc: linux-iio, Jonathan Cameron, Lars-Peter Clausen, Peter Meerwald-Stadler
On Sun, 14 Oct 2018 17:27:05 -0700
Martin Kelly <martin@martingkelly.com> wrote:
> From: Martin Kelly <martin@martingkelly.com>
>
> Miscellaneous cleanup to fix minor consistency, grammar, and spelling
> issues.
>
> Signed-off-by: Martin Kelly <martin@martingkelly.com>
Hi Martin,
Always nice to tidy up the corners.
A few comments inline...
> ---
> drivers/iio/common/st_sensors/st_sensors_core.c | 4 ++--
> drivers/iio/common/st_sensors/st_sensors_trigger.c | 4 ++--
> drivers/iio/magnetometer/st_magn_core.c | 6 +++---
> 3 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/iio/common/st_sensors/st_sensors_core.c b/drivers/iio/common/st_sensors/st_sensors_core.c
> index 26fbd1bd9413..82882bc505f5 100644
> --- a/drivers/iio/common/st_sensors/st_sensors_core.c
> +++ b/drivers/iio/common/st_sensors/st_sensors_core.c
> @@ -133,7 +133,7 @@ static int st_sensors_match_fs(struct st_sensor_settings *sensor_settings,
>
> for (i = 0; i < ST_SENSORS_FULLSCALE_AVL_MAX; i++) {
> if (sensor_settings->fs.fs_avl[i].num == 0)
> - goto st_sensors_match_odr_error;
> + goto st_sensors_match_fs_error;
>
> if (sensor_settings->fs.fs_avl[i].num == fs) {
> *index_fs_avl = i;
> @@ -142,7 +142,7 @@ static int st_sensors_match_fs(struct st_sensor_settings *sensor_settings,
> }
> }
>
> -st_sensors_match_odr_error:
> +st_sensors_match_fs_error:
> return ret;
This goto seems a little pointless in general. Let's just directly return instead of coming here
in the first place.
> }
>
> diff --git a/drivers/iio/common/st_sensors/st_sensors_trigger.c b/drivers/iio/common/st_sensors/st_sensors_trigger.c
> index fdcc5a891958..224596b0e189 100644
> --- a/drivers/iio/common/st_sensors/st_sensors_trigger.c
> +++ b/drivers/iio/common/st_sensors/st_sensors_trigger.c
> @@ -104,7 +104,7 @@ static irqreturn_t st_sensors_irq_thread(int irq, void *p)
> return IRQ_HANDLED;
>
> /*
> - * If we are using egde IRQs, new samples arrived while processing
> + * If we are using edge IRQs, new samples arrived while processing
> * the IRQ and those may be missed unless we pick them here, so poll
> * again. If the sensor delivery frequency is very high, this thread
> * turns into a polled loop handler.
> @@ -148,7 +148,7 @@ int st_sensors_allocate_trigger(struct iio_dev *indio_dev,
> if (!sdata->sensor_settings->drdy_irq.addr_ihl) {
> dev_err(&indio_dev->dev,
> "falling/low specified for IRQ "
> - "but hardware only support rising/high: "
> + "but hardware supports only rising/high: "
Either of those two is fine to my mind so please leave this one alone.
Good thing we never adopted an English language guide for the kernel ;)
> "will request rising/high\n");
> if (irq_trig == IRQF_TRIGGER_FALLING)
> irq_trig = IRQF_TRIGGER_RISING;
> diff --git a/drivers/iio/magnetometer/st_magn_core.c b/drivers/iio/magnetometer/st_magn_core.c
> index 72f6d1335a04..880c11c7f1cb 100644
> --- a/drivers/iio/magnetometer/st_magn_core.c
> +++ b/drivers/iio/magnetometer/st_magn_core.c
> @@ -29,9 +29,9 @@
> #define ST_MAGN_NUMBER_DATA_CHANNELS 3
>
> /* DEFAULT VALUE FOR SENSORS */
> -#define ST_MAGN_DEFAULT_OUT_X_H_ADDR 0X03
> -#define ST_MAGN_DEFAULT_OUT_Y_H_ADDR 0X07
> -#define ST_MAGN_DEFAULT_OUT_Z_H_ADDR 0X05
> +#define ST_MAGN_DEFAULT_OUT_X_H_ADDR 0x03
> +#define ST_MAGN_DEFAULT_OUT_Y_H_ADDR 0x07
> +#define ST_MAGN_DEFAULT_OUT_Z_H_ADDR 0x05
>
> /* FULLSCALE */
> #define ST_MAGN_FS_AVL_1300MG 1300
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] iio: st_sensors: miscellaneous cleanup
2018-10-15 16:03 ` Jonathan Cameron
@ 2018-10-15 16:32 ` Martin Kelly
2018-10-15 16:37 ` Jonathan Cameron
0 siblings, 1 reply; 5+ messages in thread
From: Martin Kelly @ 2018-10-15 16:32 UTC (permalink / raw)
To: Jonathan Cameron
Cc: linux-iio, Jonathan Cameron, Lars-Peter Clausen, Peter Meerwald-Stadler
On 10/15/18 9:03 AM, Jonathan Cameron wrote:
> On Sun, 14 Oct 2018 17:27:05 -0700
> Martin Kelly <martin@martingkelly.com> wrote:
>
>> From: Martin Kelly <martin@martingkelly.com>
>>
>> Miscellaneous cleanup to fix minor consistency, grammar, and spelling
>> issues.
>>
>> Signed-off-by: Martin Kelly <martin@martingkelly.com>
> Hi Martin,
>
> Always nice to tidy up the corners.
>
> A few comments inline...
>
>> ---
>> drivers/iio/common/st_sensors/st_sensors_core.c | 4 ++--
>> drivers/iio/common/st_sensors/st_sensors_trigger.c | 4 ++--
>> drivers/iio/magnetometer/st_magn_core.c | 6 +++---
>> 3 files changed, 7 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/iio/common/st_sensors/st_sensors_core.c b/drivers/iio/common/st_sensors/st_sensors_core.c
>> index 26fbd1bd9413..82882bc505f5 100644
>> --- a/drivers/iio/common/st_sensors/st_sensors_core.c
>> +++ b/drivers/iio/common/st_sensors/st_sensors_core.c
>> @@ -133,7 +133,7 @@ static int st_sensors_match_fs(struct st_sensor_settings *sensor_settings,
>>
>> for (i = 0; i < ST_SENSORS_FULLSCALE_AVL_MAX; i++) {
>> if (sensor_settings->fs.fs_avl[i].num == 0)
>> - goto st_sensors_match_odr_error;
>> + goto st_sensors_match_fs_error;
>>
>> if (sensor_settings->fs.fs_avl[i].num == fs) {
>> *index_fs_avl = i;
>> @@ -142,7 +142,7 @@ static int st_sensors_match_fs(struct st_sensor_settings *sensor_settings,
>> }
>> }
>>
>> -st_sensors_match_odr_error:
>> +st_sensors_match_fs_error:
>> return ret;
> This goto seems a little pointless in general. Let's just directly return instead of coming here
> in the first place.
>
I think the goto is in place in case later logic is added that needs
explicit cleanup. However, it's fine to just add that logic later, so
it's a judgment call. If you prefer, I'll change it to an explicit return.
>> }
>>
>> diff --git a/drivers/iio/common/st_sensors/st_sensors_trigger.c b/drivers/iio/common/st_sensors/st_sensors_trigger.c
>> index fdcc5a891958..224596b0e189 100644
>> --- a/drivers/iio/common/st_sensors/st_sensors_trigger.c
>> +++ b/drivers/iio/common/st_sensors/st_sensors_trigger.c
>> @@ -104,7 +104,7 @@ static irqreturn_t st_sensors_irq_thread(int irq, void *p)
>> return IRQ_HANDLED;
>>
>> /*
>> - * If we are using egde IRQs, new samples arrived while processing
>> + * If we are using edge IRQs, new samples arrived while processing
>> * the IRQ and those may be missed unless we pick them here, so poll
>> * again. If the sensor delivery frequency is very high, this thread
>> * turns into a polled loop handler.
>> @@ -148,7 +148,7 @@ int st_sensors_allocate_trigger(struct iio_dev *indio_dev,
>> if (!sdata->sensor_settings->drdy_irq.addr_ihl) {
>> dev_err(&indio_dev->dev,
>> "falling/low specified for IRQ "
>> - "but hardware only support rising/high: "
>> + "but hardware supports only rising/high: "
> Either of those two is fine to my mind so please leave this one alone.
>
> Good thing we never adopted an English language guide for the kernel ;)
>
This is a case in which I wouldn't issue a standalone patch just for it,
but since I'm fixing other things, I might as well fix this too. There
are two grammar issues here, one more important than the other:
(1) "hardware support" --> "hardware supports". This is the main one I
wanted to fix.
(2) While I'm at it, technically the hardware "only supports rising/high
interrupts" means the *only* thing it supports is rising/high
interrupts, while we actually want to say that the only type of
*interrupt* it supports is rising/high. "supports only" does that.
Again, these things are minor, so if you'd like to fix (1), (2), or
neither, it's all fine with me. Just let me know how to proceed.
>> "will request rising/high\n");
>> if (irq_trig == IRQF_TRIGGER_FALLING)
>> irq_trig = IRQF_TRIGGER_RISING;
>> diff --git a/drivers/iio/magnetometer/st_magn_core.c b/drivers/iio/magnetometer/st_magn_core.c
>> index 72f6d1335a04..880c11c7f1cb 100644
>> --- a/drivers/iio/magnetometer/st_magn_core.c
>> +++ b/drivers/iio/magnetometer/st_magn_core.c
>> @@ -29,9 +29,9 @@
>> #define ST_MAGN_NUMBER_DATA_CHANNELS 3
>>
>> /* DEFAULT VALUE FOR SENSORS */
>> -#define ST_MAGN_DEFAULT_OUT_X_H_ADDR 0X03
>> -#define ST_MAGN_DEFAULT_OUT_Y_H_ADDR 0X07
>> -#define ST_MAGN_DEFAULT_OUT_Z_H_ADDR 0X05
>> +#define ST_MAGN_DEFAULT_OUT_X_H_ADDR 0x03
>> +#define ST_MAGN_DEFAULT_OUT_Y_H_ADDR 0x07
>> +#define ST_MAGN_DEFAULT_OUT_Z_H_ADDR 0x05
>>
>> /* FULLSCALE */
>> #define ST_MAGN_FS_AVL_1300MG 1300
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] iio: st_sensors: miscellaneous cleanup
2018-10-15 16:32 ` Martin Kelly
@ 2018-10-15 16:37 ` Jonathan Cameron
2018-10-16 3:28 ` Martin Kelly
0 siblings, 1 reply; 5+ messages in thread
From: Jonathan Cameron @ 2018-10-15 16:37 UTC (permalink / raw)
To: Martin Kelly
Cc: linux-iio, Jonathan Cameron, Lars-Peter Clausen, Peter Meerwald-Stadler
On Mon, 15 Oct 2018 09:32:24 -0700
Martin Kelly <martin@martingkelly.com> wrote:
> On 10/15/18 9:03 AM, Jonathan Cameron wrote:
> > On Sun, 14 Oct 2018 17:27:05 -0700
> > Martin Kelly <martin@martingkelly.com> wrote:
> >
> >> From: Martin Kelly <martin@martingkelly.com>
> >>
> >> Miscellaneous cleanup to fix minor consistency, grammar, and spelling
> >> issues.
> >>
> >> Signed-off-by: Martin Kelly <martin@martingkelly.com>
> > Hi Martin,
> >
> > Always nice to tidy up the corners.
> >
> > A few comments inline...
> >
> >> ---
> >> drivers/iio/common/st_sensors/st_sensors_core.c | 4 ++--
> >> drivers/iio/common/st_sensors/st_sensors_trigger.c | 4 ++--
> >> drivers/iio/magnetometer/st_magn_core.c | 6 +++---
> >> 3 files changed, 7 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/drivers/iio/common/st_sensors/st_sensors_core.c b/drivers/iio/common/st_sensors/st_sensors_core.c
> >> index 26fbd1bd9413..82882bc505f5 100644
> >> --- a/drivers/iio/common/st_sensors/st_sensors_core.c
> >> +++ b/drivers/iio/common/st_sensors/st_sensors_core.c
> >> @@ -133,7 +133,7 @@ static int st_sensors_match_fs(struct st_sensor_settings *sensor_settings,
> >>
> >> for (i = 0; i < ST_SENSORS_FULLSCALE_AVL_MAX; i++) {
> >> if (sensor_settings->fs.fs_avl[i].num == 0)
> >> - goto st_sensors_match_odr_error;
> >> + goto st_sensors_match_fs_error;
> >>
> >> if (sensor_settings->fs.fs_avl[i].num == fs) {
> >> *index_fs_avl = i;
> >> @@ -142,7 +142,7 @@ static int st_sensors_match_fs(struct st_sensor_settings *sensor_settings,
> >> }
> >> }
> >>
> >> -st_sensors_match_odr_error:
> >> +st_sensors_match_fs_error:
> >> return ret;
> > This goto seems a little pointless in general. Let's just directly return instead of coming here
> > in the first place.
> >
>
> I think the goto is in place in case later logic is added that needs
> explicit cleanup. However, it's fine to just add that logic later, so
> it's a judgment call. If you prefer, I'll change it to an explicit return.
Please do.
>
> >> }
> >>
> >> diff --git a/drivers/iio/common/st_sensors/st_sensors_trigger.c b/drivers/iio/common/st_sensors/st_sensors_trigger.c
> >> index fdcc5a891958..224596b0e189 100644
> >> --- a/drivers/iio/common/st_sensors/st_sensors_trigger.c
> >> +++ b/drivers/iio/common/st_sensors/st_sensors_trigger.c
> >> @@ -104,7 +104,7 @@ static irqreturn_t st_sensors_irq_thread(int irq, void *p)
> >> return IRQ_HANDLED;
> >>
> >> /*
> >> - * If we are using egde IRQs, new samples arrived while processing
> >> + * If we are using edge IRQs, new samples arrived while processing
> >> * the IRQ and those may be missed unless we pick them here, so poll
> >> * again. If the sensor delivery frequency is very high, this thread
> >> * turns into a polled loop handler.
> >> @@ -148,7 +148,7 @@ int st_sensors_allocate_trigger(struct iio_dev *indio_dev,
> >> if (!sdata->sensor_settings->drdy_irq.addr_ihl) {
> >> dev_err(&indio_dev->dev,
> >> "falling/low specified for IRQ "
> >> - "but hardware only support rising/high: "
> >> + "but hardware supports only rising/high: "
> > Either of those two is fine to my mind so please leave this one alone.
> >
> > Good thing we never adopted an English language guide for the kernel ;)
> >
>
> This is a case in which I wouldn't issue a standalone patch just for it,
> but since I'm fixing other things, I might as well fix this too. There
> are two grammar issues here, one more important than the other:
>
> (1) "hardware support" --> "hardware supports". This is the main one I
> wanted to fix.
>
> (2) While I'm at it, technically the hardware "only supports rising/high
> interrupts" means the *only* thing it supports is rising/high
> interrupts, while we actually want to say that the only type of
> *interrupt* it supports is rising/high. "supports only" does that.
Yeah, maybe, but that's very much a specific context thing. If we were
writing a specification then that level of clarity would be needed -
actually you'd go further and probably word it something like.
"but the only interrupt types that the hardware supports are rising/high."
I don't really feel that strongly on it. If you want to go with it,
then fair enough - it's arguably more correct as you say though
it's not really open to being misinterpreted as it stands..
>
> Again, these things are minor, so if you'd like to fix (1), (2), or
> neither, it's all fine with me. Just let me know how to proceed.
>
> >> "will request rising/high\n");
> >> if (irq_trig == IRQF_TRIGGER_FALLING)
> >> irq_trig = IRQF_TRIGGER_RISING;
> >> diff --git a/drivers/iio/magnetometer/st_magn_core.c b/drivers/iio/magnetometer/st_magn_core.c
> >> index 72f6d1335a04..880c11c7f1cb 100644
> >> --- a/drivers/iio/magnetometer/st_magn_core.c
> >> +++ b/drivers/iio/magnetometer/st_magn_core.c
> >> @@ -29,9 +29,9 @@
> >> #define ST_MAGN_NUMBER_DATA_CHANNELS 3
> >>
> >> /* DEFAULT VALUE FOR SENSORS */
> >> -#define ST_MAGN_DEFAULT_OUT_X_H_ADDR 0X03
> >> -#define ST_MAGN_DEFAULT_OUT_Y_H_ADDR 0X07
> >> -#define ST_MAGN_DEFAULT_OUT_Z_H_ADDR 0X05
> >> +#define ST_MAGN_DEFAULT_OUT_X_H_ADDR 0x03
> >> +#define ST_MAGN_DEFAULT_OUT_Y_H_ADDR 0x07
> >> +#define ST_MAGN_DEFAULT_OUT_Z_H_ADDR 0x05
> >>
> >> /* FULLSCALE */
> >> #define ST_MAGN_FS_AVL_1300MG 1300
> >
> >
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] iio: st_sensors: miscellaneous cleanup
2018-10-15 16:37 ` Jonathan Cameron
@ 2018-10-16 3:28 ` Martin Kelly
0 siblings, 0 replies; 5+ messages in thread
From: Martin Kelly @ 2018-10-16 3:28 UTC (permalink / raw)
To: Jonathan Cameron
Cc: linux-iio, Jonathan Cameron, Lars-Peter Clausen, Peter Meerwald-Stadler
On 10/15/18 9:37 AM, Jonathan Cameron wrote:
> On Mon, 15 Oct 2018 09:32:24 -0700
> Martin Kelly <martin@martingkelly.com> wrote:
>
>> On 10/15/18 9:03 AM, Jonathan Cameron wrote:
>>> On Sun, 14 Oct 2018 17:27:05 -0700
>>> Martin Kelly <martin@martingkelly.com> wrote:
>>>
>>>> From: Martin Kelly <martin@martingkelly.com>
>>>>
>>>> Miscellaneous cleanup to fix minor consistency, grammar, and spelling
>>>> issues.
>>>>
>>>> Signed-off-by: Martin Kelly <martin@martingkelly.com>
>>> Hi Martin,
>>>
>>> Always nice to tidy up the corners.
>>>
>>> A few comments inline...
>>>
>>>> ---
>>>> drivers/iio/common/st_sensors/st_sensors_core.c | 4 ++--
>>>> drivers/iio/common/st_sensors/st_sensors_trigger.c | 4 ++--
>>>> drivers/iio/magnetometer/st_magn_core.c | 6 +++---
>>>> 3 files changed, 7 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/drivers/iio/common/st_sensors/st_sensors_core.c b/drivers/iio/common/st_sensors/st_sensors_core.c
>>>> index 26fbd1bd9413..82882bc505f5 100644
>>>> --- a/drivers/iio/common/st_sensors/st_sensors_core.c
>>>> +++ b/drivers/iio/common/st_sensors/st_sensors_core.c
>>>> @@ -133,7 +133,7 @@ static int st_sensors_match_fs(struct st_sensor_settings *sensor_settings,
>>>>
>>>> for (i = 0; i < ST_SENSORS_FULLSCALE_AVL_MAX; i++) {
>>>> if (sensor_settings->fs.fs_avl[i].num == 0)
>>>> - goto st_sensors_match_odr_error;
>>>> + goto st_sensors_match_fs_error;
>>>>
>>>> if (sensor_settings->fs.fs_avl[i].num == fs) {
>>>> *index_fs_avl = i;
>>>> @@ -142,7 +142,7 @@ static int st_sensors_match_fs(struct st_sensor_settings *sensor_settings,
>>>> }
>>>> }
>>>>
>>>> -st_sensors_match_odr_error:
>>>> +st_sensors_match_fs_error:
>>>> return ret;
>>> This goto seems a little pointless in general. Let's just directly return instead of coming here
>>> in the first place.
>>>
>>
>> I think the goto is in place in case later logic is added that needs
>> explicit cleanup. However, it's fine to just add that logic later, so
>> it's a judgment call. If you prefer, I'll change it to an explicit return.
>
> Please do.
>
>>
>>>> }
>>>>
>>>> diff --git a/drivers/iio/common/st_sensors/st_sensors_trigger.c b/drivers/iio/common/st_sensors/st_sensors_trigger.c
>>>> index fdcc5a891958..224596b0e189 100644
>>>> --- a/drivers/iio/common/st_sensors/st_sensors_trigger.c
>>>> +++ b/drivers/iio/common/st_sensors/st_sensors_trigger.c
>>>> @@ -104,7 +104,7 @@ static irqreturn_t st_sensors_irq_thread(int irq, void *p)
>>>> return IRQ_HANDLED;
>>>>
>>>> /*
>>>> - * If we are using egde IRQs, new samples arrived while processing
>>>> + * If we are using edge IRQs, new samples arrived while processing
>>>> * the IRQ and those may be missed unless we pick them here, so poll
>>>> * again. If the sensor delivery frequency is very high, this thread
>>>> * turns into a polled loop handler.
>>>> @@ -148,7 +148,7 @@ int st_sensors_allocate_trigger(struct iio_dev *indio_dev,
>>>> if (!sdata->sensor_settings->drdy_irq.addr_ihl) {
>>>> dev_err(&indio_dev->dev,
>>>> "falling/low specified for IRQ "
>>>> - "but hardware only support rising/high: "
>>>> + "but hardware supports only rising/high: "
>>> Either of those two is fine to my mind so please leave this one alone.
>>>
>>> Good thing we never adopted an English language guide for the kernel ;)
>>>
>>
>> This is a case in which I wouldn't issue a standalone patch just for it,
>> but since I'm fixing other things, I might as well fix this too. There
>> are two grammar issues here, one more important than the other:
>>
>> (1) "hardware support" --> "hardware supports". This is the main one I
>> wanted to fix.
>>
>> (2) While I'm at it, technically the hardware "only supports rising/high
>> interrupts" means the *only* thing it supports is rising/high
>> interrupts, while we actually want to say that the only type of
>> *interrupt* it supports is rising/high. "supports only" does that.
>
> Yeah, maybe, but that's very much a specific context thing. If we were
> writing a specification then that level of clarity would be needed -
> actually you'd go further and probably word it something like.
>
> "but the only interrupt types that the hardware supports are rising/high."
>
> I don't really feel that strongly on it. If you want to go with it,
> then fair enough - it's arguably more correct as you say though
> it's not really open to being misinterpreted as it stands..
>
>
Yeah, I think it's fine either way. I issued a v2 leaving it in.
>>
>> Again, these things are minor, so if you'd like to fix (1), (2), or
>> neither, it's all fine with me. Just let me know how to proceed.
>>
>>>> "will request rising/high\n");
>>>> if (irq_trig == IRQF_TRIGGER_FALLING)
>>>> irq_trig = IRQF_TRIGGER_RISING;
>>>> diff --git a/drivers/iio/magnetometer/st_magn_core.c b/drivers/iio/magnetometer/st_magn_core.c
>>>> index 72f6d1335a04..880c11c7f1cb 100644
>>>> --- a/drivers/iio/magnetometer/st_magn_core.c
>>>> +++ b/drivers/iio/magnetometer/st_magn_core.c
>>>> @@ -29,9 +29,9 @@
>>>> #define ST_MAGN_NUMBER_DATA_CHANNELS 3
>>>>
>>>> /* DEFAULT VALUE FOR SENSORS */
>>>> -#define ST_MAGN_DEFAULT_OUT_X_H_ADDR 0X03
>>>> -#define ST_MAGN_DEFAULT_OUT_Y_H_ADDR 0X07
>>>> -#define ST_MAGN_DEFAULT_OUT_Z_H_ADDR 0X05
>>>> +#define ST_MAGN_DEFAULT_OUT_X_H_ADDR 0x03
>>>> +#define ST_MAGN_DEFAULT_OUT_Y_H_ADDR 0x07
>>>> +#define ST_MAGN_DEFAULT_OUT_Z_H_ADDR 0x05
>>>>
>>>> /* FULLSCALE */
>>>> #define ST_MAGN_FS_AVL_1300MG 1300
>>>
>>>
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-10-16 11:16 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-15 0:27 [PATCH] iio: st_sensors: miscellaneous cleanup Martin Kelly
2018-10-15 16:03 ` Jonathan Cameron
2018-10-15 16:32 ` Martin Kelly
2018-10-15 16:37 ` Jonathan Cameron
2018-10-16 3:28 ` Martin Kelly
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.