driver: adc: stm32: combine shared and separate irqs

[mrrosen]

Several STM32 variants include both shared IRQs for some ADCs and separate IRQs for others (for example, STM32G473 has 5 ADCs, ADC1 and ADC2 share one IRQ while ADC3, ADC4 and ADC5 each have unique IRQs). The STM32 ADC driver however previously only supported either separate IRQ lines for each operational ADC in the devicetree or a single shared IRQ for all operational ADCs in the devicetree which prevented all ADCs from being usable at the same time when the variant contained a mix of both shared and separate ADC IRQ lines (only either all the shared or all the separate and one of the shared might be used at most for one application).

To allow for all ADCs in an STM32 variant to be usable in a single application, generate an ISR and initialization function for each unique IRQn as defined in the devicetree and give the task of initialization to the first ADC which connects to that particular IRQ. Each ISR function will generate code to check each ADC associated with its IRQn as was previously done for CONFIG_ADC_STM32_SHARED_IRQS, allowing an ISR to be shared for the ADCs sharing an IRQ while simultaneously providing separate ISRs for each IRQ. Thus, the only information required to have ADCs either share an ISR or not is provided by the devicetree.

[mrrosen]

Has been tried on an STM32G473 which fixed an issue encountered when using ADC1,2,4 in a single application; however, as there are a wide number of STM32 variants, I am unsure if there isnt something in one of that that is still unaccounted for

[erwango]

Thanks for working on this.
Initially I thought you used #61422, but I can see this isn't the case. Did you explore this solution ?
I guess your option might be more optimized but maybe #61422 could bring some code simplification ?

Regarding testing, i'm not sure what's the best option. We can perform a non regression on few boards, but it won't help testing all combinations.

Also, please the issue reported by bot.

[mrrosen]

Thanks for working on this. Initially I thought you used #61422, but I can see this isn't the case. Did you explore this solution ? I guess your option might be more optimized but maybe #61422 could bring some code simplification ?

Regarding testing, i'm not sure what's the best option. We can perform a non regression on few boards, but it won't help testing all combinations.

Also, please the issue reported by bot.

Since we were working off of v3.4.0, I didn't realize that a more general shared interrupts was available in mainline though I did start looking at it - this why I haven't yet fixed all the issues here.

With this solution, I am unsure as to whether when CONFIG_SHARED_INTERRUPTS is set if it should not do this change and just use the common infrastructure or always just use this new isr setup as it is more optimal (at least for this driver, other more mixed interrupts do need the shared solution; though I wonder if the shared solution shouldn't just figure itself out from the devicetree as it's easy to forget to turn it on when activating various controllers for some SoC that requires it). Thoughts?

[erwango]

With this solution, I am unsure as to whether when CONFIG_SHARED_INTERRUPTS is set if it should not do this change and just use the common infrastructure or always just use this new isr setup as it is more optimal (at least for this driver, other more mixed interrupts do need the shared solution; though I wonder if the shared solution shouldn't just figure itself out from the devicetree as it's easy to forget to turn it on when activating various controllers for some SoC that requires it). Thoughts?

I tend to agree. My only concern with current solution is the complexity and readability issue involved. But there are ways to enhance this.

[mrrosen]

@erwango @ycsin I have made some of the suggestions mentioned and added significantly more comments to clarify exactly what all the macros are doing. I do want to retest this on our platform to make sure it works as expected but that is limited to the STM32G473...

[mrrosen]

@erwango @ycsin Sorry for the delay I was able to test this fix using two different STM32 platforms (STM32F042K6 and STM32G473QE; custom boards), one with a single ADC for a simple check and the other using 3 ADCs (1, 2 which share an interrupt; 3 which uses a separate interrupt) with the ADC driver sample application and it all checked out.

I do agree with the discussion here regarding shared interrupts infrastructure and how this PR might fit in or not but as mentioned, I do think it's easy to miss enabling it, it would be hard to enable automatically when the set of enabled devicetree devices requires it, and there is a code size penalty for it. I think it might be worth merging this in as it does fix the currently broken ADC_STM32_SHARED_IRQ, and maybe open a discussion on how to take the next step for the shared interrupt framework in enabling and figuring itself out more from the devicetree.

[mrrosen]

@erwango @ycsin Any thoughts on this PR?

[erwango]

@mrrosen Back from holidays, please give me some time.

[erwango]

I'll try to review this week. Sorry for the delay