From 0906a51dacc947c5fa19a80e208549f9039556cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andrzej=20G=C5=82=C4=85bek?= Date: Mon, 17 Dec 2018 08:04:10 +0100 Subject: [PATCH] drivers: adc: Fix handling of invalid sampling requests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Commit aad21ecb3167479b113df009f8dbe89c966aebcc introduced an incorrect pattern of handling ADC sampling requests with invalid parameters in both nRF ADC drivers. After discarding such request, the drivers do not release properly the access lock and therefore become unusable. Unfortunately, this pattern were later on copied in all other ADC drivers in the source tree. This commit adds the proper lock releasing in all the affected drivers. Signed-off-by: Andrzej Głąbek --- drivers/adc/adc_dw.c | 12 ++++++++---- drivers/adc/adc_intel_quark_d2000.c | 15 ++++++++++----- drivers/adc/adc_mcux_adc16.c | 15 +++++++++++---- drivers/adc/adc_nrfx_adc.c | 24 +++++++++++++++--------- drivers/adc/adc_nrfx_saadc.c | 20 ++++++++++++++------ drivers/adc/adc_sam_afec.c | 14 ++++++++++---- 6 files changed, 68 insertions(+), 32 deletions(-) diff --git a/drivers/adc/adc_dw.c b/drivers/adc/adc_dw.c index ab7afc37178..119a9a8f227 100644 --- a/drivers/adc/adc_dw.c +++ b/drivers/adc/adc_dw.c @@ -302,14 +302,13 @@ static int adc_dw_read_request(struct device *dev, adc_context_start_read(&info->ctx, seq_tbl); error = adc_context_wait_for_completion(&info->ctx); - adc_context_release(&info->ctx, error); if (info->state == ADC_STATE_ERROR) { info->state = ADC_STATE_IDLE; return -EIO; } - return 0; + return error; } static int adc_dw_read(struct device *dev, const struct adc_sequence *seq_tbl) @@ -318,8 +317,9 @@ static int adc_dw_read(struct device *dev, const struct adc_sequence *seq_tbl) int ret; adc_context_lock(&info->ctx, false, NULL); - ret = adc_dw_read_request(dev, seq_tbl); + adc_context_release(&info->ctx, ret); + return ret; } @@ -330,9 +330,13 @@ static int adc_dw_read_async(struct device *dev, struct k_poll_signal *async) { struct adc_info *info = dev->driver_data; + int ret; adc_context_lock(&info->ctx, true, async); - return adc_dw_read_request(dev, sequence); + ret = adc_dw_read_request(dev, sequence); + adc_context_release(&info->ctx, ret); + + return ret; } #endif diff --git a/drivers/adc/adc_intel_quark_d2000.c b/drivers/adc/adc_intel_quark_d2000.c index 9ab4076831d..5c7ca5d20d4 100644 --- a/drivers/adc/adc_intel_quark_d2000.c +++ b/drivers/adc/adc_intel_quark_d2000.c @@ -289,20 +289,22 @@ static int adc_quark_d2000_read_request(struct device *dev, } adc_context_start_read(&info->ctx, seq_tbl); - error = adc_context_wait_for_completion(&info->ctx); - adc_context_release(&info->ctx, error); - return 0; + error = adc_context_wait_for_completion(&info->ctx); + return error; } static int adc_quark_d2000_read(struct device *dev, const struct adc_sequence *sequence) { struct adc_quark_d2000_info *info = dev->driver_data; + int error; adc_context_lock(&info->ctx, false, NULL); + error = adc_quark_d2000_read_request(dev, sequence); + adc_context_release(&info->ctx, error); - return adc_quark_d2000_read_request(dev, sequence); + return error; } #ifdef CONFIG_ADC_ASYNC @@ -311,10 +313,13 @@ static int adc_quark_d2000_read_async(struct device *dev, struct k_poll_signal *async) { struct adc_quark_d2000_info *info = dev->driver_data; + int error; adc_context_lock(&info->ctx, true, async); + error = adc_quark_d2000_read_request(dev, sequence); + adc_context_release(&info->ctx, error); - return adc_quark_d2000_read_request(dev, sequence); + return error; } #endif diff --git a/drivers/adc/adc_mcux_adc16.c b/drivers/adc/adc_mcux_adc16.c index 7f214d9904e..86fde47ef85 100644 --- a/drivers/adc/adc_mcux_adc16.c +++ b/drivers/adc/adc_mcux_adc16.c @@ -124,9 +124,8 @@ static int start_read(struct device *dev, const struct adc_sequence *sequence) data->buffer = sequence->buffer; adc_context_start_read(&data->ctx, sequence); - error = adc_context_wait_for_completion(&data->ctx); - adc_context_release(&data->ctx, error); + error = adc_context_wait_for_completion(&data->ctx); return error; } @@ -134,9 +133,13 @@ static int mcux_adc16_read(struct device *dev, const struct adc_sequence *sequence) { struct mcux_adc16_data *data = dev->driver_data; + int error; adc_context_lock(&data->ctx, false, NULL); - return start_read(dev, sequence); + error = start_read(dev, sequence); + adc_context_release(&data->ctx, error); + + return error; } #ifdef CONFIG_ADC_ASYNC @@ -145,9 +148,13 @@ static int mcux_adc16_read_async(struct device *dev, struct k_poll_signal *async) { struct mcux_adc16_data *data = dev->driver_data; + int error; adc_context_lock(&data->ctx, true, async); - return start_read(dev, sequence); + error = start_read(dev, sequence); + adc_context_release(&data->ctx, error); + + return error; } #endif diff --git a/drivers/adc/adc_nrfx_adc.c b/drivers/adc/adc_nrfx_adc.c index b4a18a2ba88..49ca6ab4aaf 100644 --- a/drivers/adc/adc_nrfx_adc.c +++ b/drivers/adc/adc_nrfx_adc.c @@ -136,7 +136,7 @@ static int check_buffer_size(const struct adc_sequence *sequence, static int start_read(struct device *dev, const struct adc_sequence *sequence) { - int error = 0; + int error; u32_t selected_channels = sequence->channels; u8_t active_channels; u8_t channel_id; @@ -203,11 +203,7 @@ static int start_read(struct device *dev, const struct adc_sequence *sequence) adc_context_start_read(&m_data.ctx, sequence); - if (!error) { - error = adc_context_wait_for_completion(&m_data.ctx); - adc_context_release(&m_data.ctx, error); - } - + error = adc_context_wait_for_completion(&m_data.ctx); return error; } @@ -215,8 +211,13 @@ static int start_read(struct device *dev, const struct adc_sequence *sequence) static int adc_nrfx_read(struct device *dev, const struct adc_sequence *sequence) { + int error; + adc_context_lock(&m_data.ctx, false, NULL); - return start_read(dev, sequence); + error = start_read(dev, sequence); + adc_context_release(&m_data.ctx, error); + + return error; } #ifdef CONFIG_ADC_ASYNC @@ -225,10 +226,15 @@ static int adc_nrfx_read_async(struct device *dev, const struct adc_sequence *sequence, struct k_poll_signal *async) { + int error; + adc_context_lock(&m_data.ctx, true, async); - return start_read(dev, sequence); + error = start_read(dev, sequence); + adc_context_release(&m_data.ctx, error); + + return error; } -#endif +#endif /* CONFIG_ADC_ASYNC */ DEVICE_DECLARE(adc_0); diff --git a/drivers/adc/adc_nrfx_saadc.c b/drivers/adc/adc_nrfx_saadc.c index ab2ba8167a6..3abb737d38b 100644 --- a/drivers/adc/adc_nrfx_saadc.c +++ b/drivers/adc/adc_nrfx_saadc.c @@ -247,7 +247,7 @@ static int check_buffer_size(const struct adc_sequence *sequence, static int start_read(struct device *dev, const struct adc_sequence *sequence) { - int error = 0; + int error; u32_t selected_channels = sequence->channels; u8_t active_channels; u8_t channel_id; @@ -322,8 +322,6 @@ static int start_read(struct device *dev, const struct adc_sequence *sequence) adc_context_start_read(&m_data.ctx, sequence); error = adc_context_wait_for_completion(&m_data.ctx); - adc_context_release(&m_data.ctx, error); - return error; } @@ -331,8 +329,13 @@ static int start_read(struct device *dev, const struct adc_sequence *sequence) static int adc_nrfx_read(struct device *dev, const struct adc_sequence *sequence) { + int error; + adc_context_lock(&m_data.ctx, false, NULL); - return start_read(dev, sequence); + error = start_read(dev, sequence); + adc_context_release(&m_data.ctx, error); + + return error; } #ifdef CONFIG_ADC_ASYNC @@ -341,10 +344,15 @@ static int adc_nrfx_read_async(struct device *dev, const struct adc_sequence *sequence, struct k_poll_signal *async) { + int error; + adc_context_lock(&m_data.ctx, true, async); - return start_read(dev, sequence); + error = start_read(dev, sequence); + adc_context_release(&m_data.ctx, error); + + return error; } -#endif +#endif /* CONFIG_ADC_ASYNC */ static void saadc_irq_handler(void *param) { diff --git a/drivers/adc/adc_sam_afec.c b/drivers/adc/adc_sam_afec.c index ac09e3df527..072a02b42e8 100644 --- a/drivers/adc/adc_sam_afec.c +++ b/drivers/adc/adc_sam_afec.c @@ -242,8 +242,6 @@ static int start_read(struct device *dev, const struct adc_sequence *sequence) adc_context_start_read(&data->ctx, sequence); error = adc_context_wait_for_completion(&data->ctx); - adc_context_release(&data->ctx, error); - return error; } @@ -251,9 +249,13 @@ static int adc_sam_read(struct device *dev, const struct adc_sequence *sequence) { struct adc_sam_data *data = DEV_DATA(dev); + int error; adc_context_lock(&data->ctx, false, NULL); - return start_read(dev, sequence); + error = start_read(dev, sequence); + adc_context_release(&data->ctx, error); + + return error; } static int adc_sam_init(struct device *dev) @@ -302,9 +304,13 @@ static int adc_sam_read_async(struct device *dev, struct k_poll_signal *async) { struct adc_sam_data *data = DEV_DATA(dev); + int error; adc_context_lock(&data->ctx, true, async); - return start_read(dev, sequence); + error = start_read(dev, sequence); + adc_context_release(&data->ctx, error); + + return error; } #endif