Thread (1 message) 1 message, 1 author, 2014-12-23

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

From: Marek Vasut <hidden>
Date: 2014-12-23 13:37:22
Also in: linux-iio

On Monday, December 22, 2014 at 01:14:36 PM, Stefan Wahren wrote:
quoted hunk
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 <redacted>
---
 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 ;-)
quoted hunk
+	 * 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 <redacted>
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help