linux-kernel - Re: [PATCH v2 3/6] iio: adc: Add support for STM32 ADC

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <535ffb14-a92f-1556-a4cb-f2be0508289a@metafoo.de>
Date:   Mon, 14 Nov 2016 13:11:54 +0100
From:   Lars-Peter Clausen <lars@...afoo.de>
To:     Fabrice Gasnier <fabrice.gasnier@...com>,
        linux-iio@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Cc:     jic23@...nel.org, lee.jones@...aro.org, linux@...linux.org.uk,
        robh+dt@...nel.org, mark.rutland@....com,
        mcoquelin.stm32@...il.com, alexandre.torgue@...com,
        knaack.h@....de, pmeerw@...erw.net
Subject: Re: [PATCH v2 3/6] iio: adc: Add support for STM32 ADC

On 11/10/2016 05:18 PM, Fabrice Gasnier wrote:
[...]
> + static int stm32_adc_single_conv(struct iio_dev *indio_dev,
> +				 const struct iio_chan_spec *chan,
> +				 int *res)
> +{
> +	struct stm32_adc *adc = iio_priv(indio_dev);
> +	long timeout;
> +	u32 val;
> +	u16 result;
> +	int ret;
> +
> +	reinit_completion(&adc->completion);
> +
> +	adc->buffer = &result;
> +
> +	/* Program chan number in regular sequence */
> +	val = stm32_adc_readl(adc, STM32F4_ADCX_SQR3);
> +	val &= ~STM32F4_SQ1_MASK;
> +	val |= chan->channel << STM32F4_SQ1_SHIFT;
> +	stm32_adc_writel(adc, STM32F4_ADCX_SQR3, val);
> +
> +	/* Set regular sequence len (0 for 1 conversion) */
> +	stm32_adc_clr_bits(adc, STM32F4_ADCX_SQR1, STM32F4_L_MASK);
> +
> +	/* Trigger detection disabled (conversion can be launched in SW) */
> +	stm32_adc_clr_bits(adc, STM32F4_ADCX_CR2, STM32F4_EXTEN_MASK);
> +
> +	stm32_adc_conv_irq_enable(adc);
> +
> +	stm32_adc_start_conv(adc);
> +
> +	timeout = wait_for_completion_interruptible_timeout(
> +					&adc->completion, STM32_ADC_TIMEOUT);
> +	if (timeout == 0) {
> +		dev_warn(&indio_dev->dev, "Conversion timed out!\n");

This should be dev_dbg() at most. This out of band reporting is not
particular useful for applications as it is impossible to match the error to
the action that triggered it. And you also report the error through the
error code, so the applications knows what is going on.

> +		ret = -ETIMEDOUT;
> +	} else if (timeout < 0) {
> +		dev_warn(&indio_dev->dev, "Interrupted conversion!\n");
> +		ret = -EINTR;

This should just propagate the error returned by wait_for_completion...().
This will make sure that the right behavior occurs based on the SA_RESTART
policy.

> +	} else {
> +		*res = result;
> +		ret = IIO_VAL_INT;
> +	}
> +
> +	stm32_adc_stop_conv(adc);
> +
> +	stm32_adc_conv_irq_disable(adc);
> +
> +	return ret;