| Message ID | 20220506225617.1774604-2-yannick.brosseau@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | iio: adc: stm32: Fix ADC IRQ handling on STM32F4 | expand |
On 5/7/22 00:56, Yannick Brosseau wrote: > The irq handler was only checking the mask for the first ADCs in the case of the > F4 and H7 generation, since it was using the num_irq value. This patch add > the number of ADC in the common register, which map to the number of entries of > eoc_msk and ovr_msk in stm32_adc_common_regs. > > Tested on a STM32F429NIH6 > > Signed-off-by: Yannick Brosseau <yannick.brosseau@gmail.com> > --- > drivers/iio/adc/stm32-adc-core.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > Hi Yannick, I confirm you've identified and analyzed here a regression. This is something that I noticed also earlier (Olivier and I had some patches in our downstream tree for that. Shame on me that I haven't sent them right away). So, I'm fine with your current patch. Please add a Fixes: tag as suggested by Jonathan and Paul in the cover letter. Fixes: 695e2f5c289b ("iio: adc: stm32-adc: fix a regression when using dma and irq") While you're at it, I suggest to also mention "fix" in the commit tittle, to make it clear: e.g. it fixes a regression in irq handling on stm32f4 and stm32h7. > diff --git a/drivers/iio/adc/stm32-adc-core.c b/drivers/iio/adc/stm32-adc-core.c > index 142656232157..11c08c56acb0 100644 > --- a/drivers/iio/adc/stm32-adc-core.c > +++ b/drivers/iio/adc/stm32-adc-core.c > @@ -64,6 +64,7 @@ struct stm32_adc_priv; > * @max_clk_rate_hz: maximum analog clock rate (Hz, from datasheet) > * @has_syscfg: SYSCFG capability flags > * @num_irqs: number of interrupt lines > + * @num_adcs: number of ADC instances in the common registers Minor comment here, this rather is the 'maximum' number of ADC instances. E.g. on stm32h7, there are two ADC blocks: ADC12 with 2 ADCs and ADC3 with 1 ADC instantiated separately. So you could update the comment and/or variable name. Thanks & Best Regards, Fabrice > */ > struct stm32_adc_priv_cfg { > const struct stm32_adc_common_regs *regs; > @@ -71,6 +72,7 @@ struct stm32_adc_priv_cfg { > u32 max_clk_rate_hz; > unsigned int has_syscfg; > unsigned int num_irqs; > + unsigned int num_adcs; > }; > > /** > @@ -352,7 +354,7 @@ static void stm32_adc_irq_handler(struct irq_desc *desc) > * before invoking the interrupt handler (e.g. call ISR only for > * IRQ-enabled ADCs). > */ > - for (i = 0; i < priv->cfg->num_irqs; i++) { > + for (i = 0; i < priv->cfg->num_adcs; i++) { > if ((status & priv->cfg->regs->eoc_msk[i] && > stm32_adc_eoc_enabled(priv, i)) || > (status & priv->cfg->regs->ovr_msk[i])) > @@ -792,6 +794,7 @@ static const struct stm32_adc_priv_cfg stm32f4_adc_priv_cfg = { > .clk_sel = stm32f4_adc_clk_sel, > .max_clk_rate_hz = 36000000, > .num_irqs = 1, > + .num_adcs = 3, > }; > > static const struct stm32_adc_priv_cfg stm32h7_adc_priv_cfg = { > @@ -800,6 +803,7 @@ static const struct stm32_adc_priv_cfg stm32h7_adc_priv_cfg = { > .max_clk_rate_hz = 36000000, > .has_syscfg = HAS_VBOOSTER, > .num_irqs = 1, > + .num_adcs = 2, > }; > > static const struct stm32_adc_priv_cfg stm32mp1_adc_priv_cfg = { > @@ -808,6 +812,7 @@ static const struct stm32_adc_priv_cfg stm32mp1_adc_priv_cfg = { > .max_clk_rate_hz = 40000000, > .has_syscfg = HAS_VBOOSTER | HAS_ANASWVDD, > .num_irqs = 2, > + .num_adcs = 2, > }; > > static const struct of_device_id stm32_adc_of_match[] = {
diff --git a/drivers/iio/adc/stm32-adc-core.c b/drivers/iio/adc/stm32-adc-core.c index 142656232157..11c08c56acb0 100644 --- a/drivers/iio/adc/stm32-adc-core.c +++ b/drivers/iio/adc/stm32-adc-core.c @@ -64,6 +64,7 @@ struct stm32_adc_priv; * @max_clk_rate_hz: maximum analog clock rate (Hz, from datasheet) * @has_syscfg: SYSCFG capability flags * @num_irqs: number of interrupt lines + * @num_adcs: number of ADC instances in the common registers */ struct stm32_adc_priv_cfg { const struct stm32_adc_common_regs *regs; @@ -71,6 +72,7 @@ struct stm32_adc_priv_cfg { u32 max_clk_rate_hz; unsigned int has_syscfg; unsigned int num_irqs; + unsigned int num_adcs; }; /** @@ -352,7 +354,7 @@ static void stm32_adc_irq_handler(struct irq_desc *desc) * before invoking the interrupt handler (e.g. call ISR only for * IRQ-enabled ADCs). */ - for (i = 0; i < priv->cfg->num_irqs; i++) { + for (i = 0; i < priv->cfg->num_adcs; i++) { if ((status & priv->cfg->regs->eoc_msk[i] && stm32_adc_eoc_enabled(priv, i)) || (status & priv->cfg->regs->ovr_msk[i])) @@ -792,6 +794,7 @@ static const struct stm32_adc_priv_cfg stm32f4_adc_priv_cfg = { .clk_sel = stm32f4_adc_clk_sel, .max_clk_rate_hz = 36000000, .num_irqs = 1, + .num_adcs = 3, }; static const struct stm32_adc_priv_cfg stm32h7_adc_priv_cfg = { @@ -800,6 +803,7 @@ static const struct stm32_adc_priv_cfg stm32h7_adc_priv_cfg = { .max_clk_rate_hz = 36000000, .has_syscfg = HAS_VBOOSTER, .num_irqs = 1, + .num_adcs = 2, }; static const struct stm32_adc_priv_cfg stm32mp1_adc_priv_cfg = { @@ -808,6 +812,7 @@ static const struct stm32_adc_priv_cfg stm32mp1_adc_priv_cfg = { .max_clk_rate_hz = 40000000, .has_syscfg = HAS_VBOOSTER | HAS_ANASWVDD, .num_irqs = 2, + .num_adcs = 2, }; static const struct of_device_id stm32_adc_of_match[] = {
The irq handler was only checking the mask for the first ADCs in the case of the F4 and H7 generation, since it was using the num_irq value. This patch add the number of ADC in the common register, which map to the number of entries of eoc_msk and ovr_msk in stm32_adc_common_regs. Tested on a STM32F429NIH6 Signed-off-by: Yannick Brosseau <yannick.brosseau@gmail.com> --- drivers/iio/adc/stm32-adc-core.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)