On Mon, Jul 19, 2021 at 2:39 AM Yizhuo Zhai <yzhai003@xxxxxxx> wrote:
>
> Hi All:
> Inside function stm32_dfsdm_irq(), the variable "status", "int_en"
> could be uninitialized if the regmap_read() fails and returns an error
> code. However, they are directly used in the later context to decide
> the control flow, which is potentially unsafe. However,
> stm32_dfsdm_irq() returns the type irqreturn_t and I could not return
Just curious: are you seeing any issues with these variables being
uninitialized?
> the error code directly. Could you please advise me here?
The correct way to do it, would be:
ret = regmap_read(regmap, DFSDM_ISR(adc->fl_id), &status);
if (ret)
return IRQ_HANDLED;
IRQ handlers should return one of
enum irqreturn {
IRQ_NONE = (0 << 0),
IRQ_HANDLED = (1 << 0),
IRQ_WAKE_THREAD = (1 << 1),
};
If you want to fully optimize/correct this, then it may be something like:
ret = regmap_read(regmap, DFSDM_ISR(adc->fl_id), &status);
if (ret)
return IRQ_HANDLED;
if (status & DFSDM_ISR_REOCF_MASK) {
/* Read the data register clean the IRQ status */
regmap_read(regmap, DFSDM_RDATAR(adc->fl_id), adc->buffer);
// in this point, we could check for regmap_read(), but it won't make
sense; we should call the complete() handler, either way
complete(&adc->completion);
}
if (status & DFSDM_ISR_ROVRF_MASK) {
ret = regmap_read(regmap, DFSDM_CR2(adc->fl_id), &int_en);
if (ret)
return IRQ_HANDLED;
if (int_en & DFSDM_CR2_ROVRIE_MASK)
dev_warn(&indio_dev->dev, "Overrun detected\n");
regmap_update_bits(regmap, DFSDM_ICR(adc->fl_id),
DFSDM_ICR_CLRROVRF_MASK,
DFSDM_ICR_CLRROVRF_MASK);
// in this point, we could also check the ret code; but we still need
to call IRQ_HANDLED anyway;
}
Quite often, when regmap_read() returns errors, then something is
seriously wrong in the system.
Something else would usually fail or crash worse than this interrupt handler.
That being said, properly handling regmap_read() here is a good idea.
>
> The related code:
>
> static irqreturn_t stm32_dfsdm_irq(int irq, void *arg) {
> unsigned int status, int_en;
>
> regmap_read(regmap, DFSDM_ISR(adc->fl_id), &status);
> regmap_read(regmap, DFSDM_CR2(adc->fl_id), &int_en);
>
> if (status & DFSDM_ISR_REOCF_MASK) {}
> if (status & DFSDM_ISR_ROVRF_MASK) {}
> }
>
>
> --
> Kind Regards,
>
> Yizhuo Zhai
>
> Computer Science, Graduate Student
> University of California, Riverside
