[PATCH 0/2] iio: mxs-lradc: check ranges of ts properties

[PATCH 0/2] iio: mxs-lradc: check ranges of ts properties

[Stefan Wahren]

These patch series fixes off-by-one issues of the touchscreen DT properties
and finally adds a range check during probe.

Only the range check has been tested by myself, because i don't have a
touchscreen. So a functional test is very welcome.

Changes since RFC:
- add fix for off-by-one issues
- first check DT properties and then assign a value
- adapt range check to better represent DT binding
- add important note from the reference manual as comment

Stefan Wahren (2):
  DT: mxs-lradc: fix ranges of ts properties
  iio: mxs-lradc: check ranges of ts properties

 .../bindings/staging/iio/adc/mxs-lradc.txt         |    4 +-
 drivers/staging/iio/adc/mxs-lradc.c                |   44 +++++++++++++++-----
 2 files changed, 36 insertions(+), 12 deletions(-)

-- 
1.7.9.5

[PATCH 1/2] DT: mxs-lradc: fix ranges of ts properties

[Stefan Wahren]

This patch fixes off-by-one issues in the devicetree binding of
mxs-lradc.

According to the i.MX23 and i.MX28 reference manuals [1][2] the range of
NUM_SAMPLES is 0..31, but property ave-ctrl is substracted by 1 before used.

Considering all limitations the range of DELAY is 1..2047, but also
property ave-delay is substracted by 1 before used.

The patch has been suggested by Hartmut Knaack and Kristina Martsenko.

[1] - http://cache.freescale.com/files/dsp/doc/ref_manual/IMX23RM.pdf
[2] - http://cache.freescale.com/files/dsp/doc/ref_manual/MCIMX28RM.pdf

Signed-off-by: Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org>
---
 .../bindings/staging/iio/adc/mxs-lradc.txt         |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/staging/iio/adc/mxs-lradc.txt b/Documentation/devicetree/bindings/staging/iio/adc/mxs-lradc.txt
index ee05dc3..3075377 100644
--- a/Documentation/devicetree/bindings/staging/iio/adc/mxs-lradc.txt
+++ b/Documentation/devicetree/bindings/staging/iio/adc/mxs-lradc.txt
@@ -12,9 +12,9 @@ Optional properties:
                                property is not present, then the touchscreen is
                                disabled. 5 wires is valid for i.MX28 SoC only.
 - fsl,ave-ctrl: number of samples per direction to calculate an average value.
-                Allowed value is 1 ... 31, default is 4
+                Allowed value is 1 ... 32, default is 4
 - fsl,ave-delay: delay between consecutive samples. Allowed value is
-                 1 ... 2047. It is used if 'fsl,ave-ctrl' > 1, counts at
+                 2 ... 2048. It is used if 'fsl,ave-ctrl' > 1, counts at
                  2 kHz and its default is 2 (= 1 ms)
 - fsl,settling: delay between plate switch to next sample. Allowed value is
                 1 ... 2047. It counts at 2 kHz and its default is
-- 
1.7.9.5

[PATCH 2/2] iio: mxs-lradc: check ranges of ts properties

[Stefan Wahren]

The devicetree binding for mxs-lradc defines ranges for the
touchscreen properties. In order to avoid unexpected behavior like
division by zero, we better check these ranges during probe and
abort in error case.

Additionally this patch adds an important note from the reference
manual about the range of sample delay.

Signed-off-by: Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org>
---
 drivers/staging/iio/adc/mxs-lradc.c |   44 +++++++++++++++++++++++++++--------
 1 file changed, 34 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/iio/adc/mxs-lradc.c b/drivers/staging/iio/adc/mxs-lradc.c
index f053535..990e945 100644
--- a/drivers/staging/iio/adc/mxs-lradc.c
+++ b/drivers/staging/iio/adc/mxs-lradc.c
@@ -436,7 +436,13 @@ static void mxs_lradc_setup_ts_channel(struct mxs_lradc *lradc, unsigned ch)
 	 */
 	mxs_lradc_reg_clear(lradc, LRADC_CH_VALUE_MASK, LRADC_CH(ch));
 
-	/* prepare the delay/loop unit according to the oversampling count */
+	/* prepare the delay/loop unit according to the oversampling count
+	 *
+	 * from the datasheet:
+	 * "The DELAY fields in HW_LRADC_DELAY0, HW_LRADC_DELAY1,
+	 * HW_LRADC_DELAY2, and HW_LRADC_DELAY3 must be non-zero; otherwise,
+	 * the LRADC will not trigger the delay group."
+	 */
 	mxs_lradc_reg_wrt(lradc, LRADC_DELAY_TRIGGER(1 << ch) |
 		LRADC_DELAY_TRIGGER_DELAYS(0) |
 		LRADC_DELAY_LOOP(lradc->over_sample_cnt - 1) |
@@ -1495,20 +1501,38 @@ static int mxs_lradc_probe_touchscreen(struct mxs_lradc *lradc,
 		return -EINVAL;
 	}
 
-	lradc->over_sample_cnt = 4;
-	ret = of_property_read_u32(lradc_node, "fsl,ave-ctrl", &adapt);
-	if (ret == 0)
+	if (of_property_read_u32(lradc_node, "fsl,ave-ctrl", &adapt)) {
+		lradc->over_sample_cnt = 4;
+	} else {
+		if (adapt < 1 || adapt > 32) {
+			dev_err(lradc->dev, "Invalid sample count (%lu)\n",
+				adapt);
+			return -EINVAL;
+		}
 		lradc->over_sample_cnt = adapt;
+	}
 
-	lradc->over_sample_delay = 2;
-	ret = of_property_read_u32(lradc_node, "fsl,ave-delay", &adapt);
-	if (ret == 0)
+	if (of_property_read_u32(lradc_node, "fsl,ave-delay", &adapt)) {
+		lradc->over_sample_delay = 2;
+	} else {
+		if (adapt < 2 || adapt > LRADC_DELAY_DELAY_MASK+1) {
+			dev_err(lradc->dev, "Invalid sample delay (%u)\n",
+				adapt);
+			return -EINVAL;
+		}
 		lradc->over_sample_delay = adapt;
+	}
 
-	lradc->settling_delay = 10;
-	ret = of_property_read_u32(lradc_node, "fsl,settling", &adapt);
-	if (ret == 0)
+	if (of_property_read_u32(lradc_node, "fsl,settling", &adapt)) {
+		lradc->settling_delay = 10;
+	} else {
+		if (adapt < 1 || adapt > LRADC_DELAY_DELAY_MASK) {
+			dev_err(lradc->dev, "Invalid settling delay (%u)\n",
+				adapt);
+			return -EINVAL;
+		}
 		lradc->settling_delay = adapt;
+	}
 
 	return 0;
 }
-- 
1.7.9.5

Re: [PATCH 2/2] iio: mxs-lradc: check ranges of ts properties

[Marek Vasut]

On Monday, December 22, 2014 at 01:14:36 PM, Stefan Wahren wrote:
> The devicetree binding for mxs-lradc defines ranges for the
> touchscreen properties. In order to avoid unexpected behavior like
> division by zero, we better check these ranges during probe and
> abort in error case.
> 
> Additionally this patch adds an important note from the reference
> manual about the range of sample delay.
> 
> Signed-off-by: Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org>
> ---
>  drivers/staging/iio/adc/mxs-lradc.c |   44
> +++++++++++++++++++++++++++-------- 1 file changed, 34 insertions(+), 10
> deletions(-)
> 
> diff --git a/drivers/staging/iio/adc/mxs-lradc.c
> b/drivers/staging/iio/adc/mxs-lradc.c index f053535..990e945 100644
> --- a/drivers/staging/iio/adc/mxs-lradc.c
> +++ b/drivers/staging/iio/adc/mxs-lradc.c
> @@ -436,7 +436,13 @@ static void mxs_lradc_setup_ts_channel(struct
> mxs_lradc *lradc, unsigned ch) */
>  	mxs_lradc_reg_clear(lradc, LRADC_CH_VALUE_MASK, LRADC_CH(ch));
> 
> -	/* prepare the delay/loop unit according to the oversampling count */
> +	/* prepare the delay/loop unit according to the oversampling count

Very minor coding style flub in this comment above. Multi-line comments should
start with /* and a newline after that ;-)

> +	 * from the datasheet:
> +	 * "The DELAY fields in HW_LRADC_DELAY0, HW_LRADC_DELAY1,
> +	 * HW_LRADC_DELAY2, and HW_LRADC_DELAY3 must be non-zero; otherwise,
> +	 * the LRADC will not trigger the delay group."
> +	 */
>  	mxs_lradc_reg_wrt(lradc, LRADC_DELAY_TRIGGER(1 << ch) |
>  		LRADC_DELAY_TRIGGER_DELAYS(0) |
>  		LRADC_DELAY_LOOP(lradc->over_sample_cnt - 1) |
> @@ -1495,20 +1501,38 @@ static int mxs_lradc_probe_touchscreen(struct
> mxs_lradc *lradc, return -EINVAL;
>  	}
> 
> -	lradc->over_sample_cnt = 4;
> -	ret = of_property_read_u32(lradc_node, "fsl,ave-ctrl", &adapt);
> -	if (ret == 0)
> +	if (of_property_read_u32(lradc_node, "fsl,ave-ctrl", &adapt)) {
> +		lradc->over_sample_cnt = 4;
> +	} else {
> +		if (adapt < 1 || adapt > 32) {

This is just an idea, but do we not have some kind of a 
"of_property_read_u32_range()" thingie, which would include this kind of range 
checking ? Would it be worth implementing such thing ? What do you think
please ?

[...]

Otherwise,

Reviewed-by: Marek Vasut <marex-ynQEQJNshbs@public.gmane.org>

Re: [PATCH 2/2] iio: mxs-lradc: check ranges of ts properties

[Stefan Wahren]

Hi Marek,

> Marek Vasut <marex-ynQEQJNshbs@public.gmane.org> hat am 23. Dezember 2014 um 14:37 geschrieben:
>
>
> On Monday, December 22, 2014 at 01:14:36 PM, Stefan Wahren wrote:
> [...]
>
> Very minor coding style flub in this comment above. Multi-line comments should
> start with /* and a newline after that ;-)

Thanks for your advice.

>
> > + * from the datasheet:
> > + * "The DELAY fields in HW_LRADC_DELAY0, HW_LRADC_DELAY1,
> > + * HW_LRADC_DELAY2, and HW_LRADC_DELAY3 must be non-zero; otherwise,
> > + * the LRADC will not trigger the delay group."
> > + */
> > mxs_lradc_reg_wrt(lradc, LRADC_DELAY_TRIGGER(1 << ch) |
> > LRADC_DELAY_TRIGGER_DELAYS(0) |
> > LRADC_DELAY_LOOP(lradc->over_sample_cnt - 1) |
> > @@ -1495,20 +1501,38 @@ static int mxs_lradc_probe_touchscreen(struct
> > mxs_lradc *lradc, return -EINVAL;
> > }
> >
> > - lradc->over_sample_cnt = 4;
> > - ret = of_property_read_u32(lradc_node, "fsl,ave-ctrl", &adapt);
> > - if (ret == 0)
> > + if (of_property_read_u32(lradc_node, "fsl,ave-ctrl", &adapt)) {
> > + lradc->over_sample_cnt = 4;
> > + } else {
> > + if (adapt < 1 || adapt > 32) {
>
> This is just an idea, but do we not have some kind of a
> "of_property_read_u32_range()" thingie, which would include this kind of range
> checking ? Would it be worth implementing such thing ? What do you think
> please ?

I never heard of such a function. I think it's not the best idea of mixing dt
parsing and range checking in a general function.

But this code does nearly the same thing 3 times. How about defining an array of
property range structures:

static const struct property_value_range mxs_lradc_properties[] = {
	{
		.name = "fsl,ave-ctrl",
		.min_value = 1,
		.max_value = 32,
		.default_value = 4,
	},
	{
		.name = "fsl,ave-delay",
		.min_value = 2,
		.max_value = LRADC_DELAY_DELAY_MASK+1,
		.default_value = 2,
	},
	{
		.name = "fsl,settling",
		.min_value = 1,
		.max_value = LRADC_DELAY_DELAY_MASK,
		.default_value = 10,
	},
};

and a local validate function for these optional parameters.

>
> [...]
>
> Otherwise,
>
> Reviewed-by: Marek Vasut <marex-ynQEQJNshbs@public.gmane.org>

Thanks

Btw i found an error in the format strings :(

Stefan

Re: [PATCH 2/2] iio: mxs-lradc: check ranges of ts properties

[Marek Vasut]

On Tuesday, December 23, 2014 at 11:45:59 PM, Stefan Wahren wrote:
> Hi Marek,

Hi!

> > Marek Vasut <marex-ynQEQJNshbs@public.gmane.org> hat am 23. Dezember 2014 um 14:37
> > geschrieben:
> > 
> > 
> > On Monday, December 22, 2014 at 01:14:36 PM, Stefan Wahren wrote:
> > [...]
> > 
> > Very minor coding style flub in this comment above. Multi-line comments
> > should start with /* and a newline after that ;-)
> 
> Thanks for your advice.

Sure, it's really a minor thing.

> > > + * from the datasheet:
> > > + * "The DELAY fields in HW_LRADC_DELAY0, HW_LRADC_DELAY1,
> > > + * HW_LRADC_DELAY2, and HW_LRADC_DELAY3 must be non-zero; otherwise,
> > > + * the LRADC will not trigger the delay group."
> > > + */
> > > mxs_lradc_reg_wrt(lradc, LRADC_DELAY_TRIGGER(1 << ch) |
> > > LRADC_DELAY_TRIGGER_DELAYS(0) |
> > > LRADC_DELAY_LOOP(lradc->over_sample_cnt - 1) |
> > > @@ -1495,20 +1501,38 @@ static int mxs_lradc_probe_touchscreen(struct
> > > mxs_lradc *lradc, return -EINVAL;
> > > }
> > > 
> > > - lradc->over_sample_cnt = 4;
> > > - ret = of_property_read_u32(lradc_node, "fsl,ave-ctrl", &adapt);
> > > - if (ret == 0)
> > > + if (of_property_read_u32(lradc_node, "fsl,ave-ctrl", &adapt)) {
> > > + lradc->over_sample_cnt = 4;
> > > + } else {
> > > + if (adapt < 1 || adapt > 32) {
> > 
> > This is just an idea, but do we not have some kind of a
> > "of_property_read_u32_range()" thingie, which would include this kind of
> > range checking ? Would it be worth implementing such thing ? What do you
> > think please ?
> 
> I never heard of such a function. I think it's not the best idea of mixing
> dt parsing and range checking in a general function.

It was just an idea, since it would trim down the code duplication a bit.

> But this code does nearly the same thing 3 times. How about defining an
> array of property range structures:
> 
> static const struct property_value_range mxs_lradc_properties[] = {
> 	{
> 		.name = "fsl,ave-ctrl",
> 		.min_value = 1,
> 		.max_value = 32,
> 		.default_value = 4,
> 	},
> 	{
> 		.name = "fsl,ave-delay",
> 		.min_value = 2,
> 		.max_value = LRADC_DELAY_DELAY_MASK+1,
> 		.default_value = 2,
> 	},
> 	{
> 		.name = "fsl,settling",
> 		.min_value = 1,
> 		.max_value = LRADC_DELAY_DELAY_MASK,
> 		.default_value = 10,
> 	},
> };
> 
> and a local validate function for these optional parameters.

That's becoming a bit too complex for such a simple task. I cannot tell right 
now, so I'd prefer of others chimed in.

Have a nice holiday!

Re: [PATCH 2/2] iio: mxs-lradc: check ranges of ts properties

[Jonathan Cameron]

On 24/12/14 00:35, Marek Vasut wrote:
> On Tuesday, December 23, 2014 at 11:45:59 PM, Stefan Wahren wrote:
>> Hi Marek,
> 
> Hi!
> 
>>> Marek Vasut <marex-ynQEQJNshbs@public.gmane.org> hat am 23. Dezember 2014 um 14:37
>>> geschrieben:
>>>
>>>
>>> On Monday, December 22, 2014 at 01:14:36 PM, Stefan Wahren wrote:
>>> [...]
>>>
>>> Very minor coding style flub in this comment above. Multi-line comments
>>> should start with /* and a newline after that ;-)
>>
>> Thanks for your advice.
> 
> Sure, it's really a minor thing.
> 
>>>> + * from the datasheet:
>>>> + * "The DELAY fields in HW_LRADC_DELAY0, HW_LRADC_DELAY1,
>>>> + * HW_LRADC_DELAY2, and HW_LRADC_DELAY3 must be non-zero; otherwise,
>>>> + * the LRADC will not trigger the delay group."
>>>> + */
>>>> mxs_lradc_reg_wrt(lradc, LRADC_DELAY_TRIGGER(1 << ch) |
>>>> LRADC_DELAY_TRIGGER_DELAYS(0) |
>>>> LRADC_DELAY_LOOP(lradc->over_sample_cnt - 1) |
>>>> @@ -1495,20 +1501,38 @@ static int mxs_lradc_probe_touchscreen(struct
>>>> mxs_lradc *lradc, return -EINVAL;
>>>> }
>>>>
>>>> - lradc->over_sample_cnt = 4;
>>>> - ret = of_property_read_u32(lradc_node, "fsl,ave-ctrl", &adapt);
>>>> - if (ret == 0)
>>>> + if (of_property_read_u32(lradc_node, "fsl,ave-ctrl", &adapt)) {
>>>> + lradc->over_sample_cnt = 4;
>>>> + } else {
>>>> + if (adapt < 1 || adapt > 32) {
>>>
>>> This is just an idea, but do we not have some kind of a
>>> "of_property_read_u32_range()" thingie, which would include this kind of
>>> range checking ? Would it be worth implementing such thing ? What do you
>>> think please ?
>>
>> I never heard of such a function. I think it's not the best idea of mixing
>> dt parsing and range checking in a general function.
> 
> It was just an idea, since it would trim down the code duplication a bit.
It's an interesting thought certainly as this must be a fairly common case...

> 
>> But this code does nearly the same thing 3 times. How about defining an
>> array of property range structures:
>>
>> static const struct property_value_range mxs_lradc_properties[] = {
>> 	{
>> 		.name = "fsl,ave-ctrl",
>> 		.min_value = 1,
>> 		.max_value = 32,
>> 		.default_value = 4,
>> 	},
>> 	{
>> 		.name = "fsl,ave-delay",
>> 		.min_value = 2,
>> 		.max_value = LRADC_DELAY_DELAY_MASK+1,
>> 		.default_value = 2,
>> 	},
>> 	{
>> 		.name = "fsl,settling",
>> 		.min_value = 1,
>> 		.max_value = LRADC_DELAY_DELAY_MASK,
>> 		.default_value = 10,
>> 	},
>> };
>>
>> and a local validate function for these optional parameters.
> 
> That's becoming a bit too complex for such a simple task. I cannot tell right 
> now, so I'd prefer of others chimed in.
I'd leave as it is.  If there were a few more cases it might be worth the
function, but probably not for these 3.

A shorter option might be to just use a function with the constants passed
in as parameters for each call.
> 
> Have a nice holiday!
> 

Re: [PATCH 2/2] iio: mxs-lradc: check ranges of ts properties

[Jonathan Cameron]

On 22/12/14 12:14, Stefan Wahren wrote:
> The devicetree binding for mxs-lradc defines ranges for the
> touchscreen properties. In order to avoid unexpected behavior like
> division by zero, we better check these ranges during probe and
> abort in error case.
> 
> Additionally this patch adds an important note from the reference
> manual about the range of sample delay.
> 
> Signed-off-by: Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org>
> ---
>  drivers/staging/iio/adc/mxs-lradc.c |   44 +++++++++++++++++++++++++++--------
>  1 file changed, 34 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/staging/iio/adc/mxs-lradc.c b/drivers/staging/iio/adc/mxs-lradc.c
> index f053535..990e945 100644
> --- a/drivers/staging/iio/adc/mxs-lradc.c
> +++ b/drivers/staging/iio/adc/mxs-lradc.c
> @@ -436,7 +436,13 @@ static void mxs_lradc_setup_ts_channel(struct mxs_lradc *lradc, unsigned ch)
>  	 */
>  	mxs_lradc_reg_clear(lradc, LRADC_CH_VALUE_MASK, LRADC_CH(ch));
>  
> -	/* prepare the delay/loop unit according to the oversampling count */
> +	/* prepare the delay/loop unit according to the oversampling count
> +	 *
> +	 * from the datasheet:
> +	 * "The DELAY fields in HW_LRADC_DELAY0, HW_LRADC_DELAY1,
> +	 * HW_LRADC_DELAY2, and HW_LRADC_DELAY3 must be non-zero; otherwise,
> +	 * the LRADC will not trigger the delay group."
> +	 */
>  	mxs_lradc_reg_wrt(lradc, LRADC_DELAY_TRIGGER(1 << ch) |
>  		LRADC_DELAY_TRIGGER_DELAYS(0) |
>  		LRADC_DELAY_LOOP(lradc->over_sample_cnt - 1) |
> @@ -1495,20 +1501,38 @@ static int mxs_lradc_probe_touchscreen(struct mxs_lradc *lradc,
>  		return -EINVAL;
>  	}
>  
> -	lradc->over_sample_cnt = 4;
> -	ret = of_property_read_u32(lradc_node, "fsl,ave-ctrl", &adapt);
> -	if (ret == 0)
> +	if (of_property_read_u32(lradc_node, "fsl,ave-ctrl", &adapt)) {
> +		lradc->over_sample_cnt = 4;
> +	} else {
> +		if (adapt < 1 || adapt > 32) {
> +			dev_err(lradc->dev, "Invalid sample count (%lu)\n",
> +				adapt);
> +			return -EINVAL;
> +		}
>  		lradc->over_sample_cnt = adapt;
> +	}
>  
> -	lradc->over_sample_delay = 2;
> -	ret = of_property_read_u32(lradc_node, "fsl,ave-delay", &adapt);
> -	if (ret == 0)
> +	if (of_property_read_u32(lradc_node, "fsl,ave-delay", &adapt)) {
> +		lradc->over_sample_delay = 2;
> +	} else {
> +		if (adapt < 2 || adapt > LRADC_DELAY_DELAY_MASK+1) {
please run checkpatch.pl over these.  Should be spaces around the +
> +			dev_err(lradc->dev, "Invalid sample delay (%u)\n",
> +				adapt);
> +			return -EINVAL;
> +		}
>  		lradc->over_sample_delay = adapt;
> +	}
>  
> -	lradc->settling_delay = 10;
> -	ret = of_property_read_u32(lradc_node, "fsl,settling", &adapt);
> -	if (ret == 0)
> +	if (of_property_read_u32(lradc_node, "fsl,settling", &adapt)) {
> +		lradc->settling_delay = 10;
> +	} else {
> +		if (adapt < 1 || adapt > LRADC_DELAY_DELAY_MASK) {
> +			dev_err(lradc->dev, "Invalid settling delay (%u)\n",
> +				adapt);
> +			return -EINVAL;
> +		}
>  		lradc->settling_delay = adapt;
> +	}
>  
>  	return 0;
>  }
> 

Re: [PATCH 2/2] iio: mxs-lradc: check ranges of ts properties

[Stefan Wahren]

Hi Jonathan,

> Jonathan Cameron <jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> hat am 26. Dezember 2014 um 10:23
> geschrieben:
>
> [...]
> >
> > - lradc->over_sample_delay = 2;
> > - ret = of_property_read_u32(lradc_node, "fsl,ave-delay", &adapt);
> > - if (ret == 0)
> > + if (of_property_read_u32(lradc_node, "fsl,ave-delay", &adapt)) {
> > + lradc->over_sample_delay = 2;
> > + } else {
> > + if (adapt < 2 || adapt > LRADC_DELAY_DELAY_MASK+1) {
> please run checkpatch.pl over these. Should be spaces around the +

i'm afraid my checkpatch.pl doesn't find this issue. I'll fix it in the next
version.

Stefan

Re: [PATCH 2/2] iio: mxs-lradc: check ranges of ts properties

[Jonathan Cameron]

On 26/12/14 20:50, Stefan Wahren wrote:
> Hi Jonathan,
> 
>> Jonathan Cameron <jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> hat am 26. Dezember 2014 um 10:23
>> geschrieben:
>>
>> [...]
>>>
>>> - lradc->over_sample_delay = 2;
>>> - ret = of_property_read_u32(lradc_node, "fsl,ave-delay", &adapt);
>>> - if (ret == 0)
>>> + if (of_property_read_u32(lradc_node, "fsl,ave-delay", &adapt)) {
>>> + lradc->over_sample_delay = 2;
>>> + } else {
>>> + if (adapt < 2 || adapt > LRADC_DELAY_DELAY_MASK+1) {
>> please run checkpatch.pl over these. Should be spaces around the +
> 
> i'm afraid my checkpatch.pl doesn't find this issue. I'll fix it in the next
> version.
> 
So it doesn't!  Sorry about the false comment.

Hmm. I wonder why...