STM32Fx issues using multiple ADCs with a common HW reset bit

[bob.fe...@rafresearch.com]

The original ADC low level driver for the STM32Fx series seems to have been written for the STM32F1. The code notes that it is written for ADC controllers
that have independent HW reset bits. This code was copied to STM32Fx ports that
have a common HW reset bit for all controllers. This code fails on the chips
with common reset bits.

I have seen this problem on the STM32F7.
Michał Łyszczek reported this problem on the STM32F3.

We should choose a common fix that can be applied across all STM32Fx chips that
have a common

In arch/arm/src/stm32f7/stm32_adc.c...
The current adc_reset function unconditionally calls adc_rccreset().
All STM32F7 MCUs have a single RCC_RSTR bit that is common to all three ADCs.
The high level ADC driver, which has an instance for each of the three ADCs,
causes adc_rccreset() to be called the first time each of the ADC has its
open() function executed.
open("/dev/adc0",...) resets all three ADCs and initializes only ADC1.
open("/dev/adc1",...) resets all three ADCs and initializes only ADC2.
open("/dev/adc2",...) resets all three ADCs and initializes only ADC3.
Therefore, it is impossible to have more than one ADC open and functioning.

Possible fixes:
a) Remove the adc_rccreset() function. This means that there would be no easy
way to place an ADC into its power-on-reset state after power-on. In my
application, all three ADCs are opened shortly after power-on and never
closed. So this solution is acceptable to me. I also prefer it, because it
is the simplest fix.

b) Permit the HW reset to occur only when no other ADC controllers are
operating (opened).
Define a "struct adc_common_dev_s *dev" struct for the common ADC
hardware. Then...
1) have adc_rccreset() obtain a mutually exclusive use semaphore in this
struct;
2) sum the dev->ad_ocount fields across all configured ADCs;
3) conditionally pulse the RCC_RSTR bit only when the sum is zero;
4) release the mutually exclusive use semaphore.
Note that since nested semaphores will now being used, we must establish
the rule that the "struct adc_common_dev_s *dev" semaphore must always be
obtained before the "struct adc_common_dev_s *dev" semaphore is requested.
(otherwise deadlock will occur).
Also note that close() also causes the adc_rccreset() function to be
invoked, so this fix will also prevents the RCC_RSTR bit from being pulsed
on close() until the sum of the dev->ad_ocount fields is zero.
This fix is more complicated to implement and may be difficult for
applications to use. (The application would need to force the close() of
all ADCs to have the RCC_RSTR reset occur.)
This solution is also acceptable to me.

If you may have already implemented a different solution, please share it.
Otherwise, please provide your guidance on a common solution regarding this
issue?

Regards,
Bob Feretich

[raiden00pl]

Hi, I fixed it for stm32f33xx chip with this pull request https://bitbucket.org/nuttx/nuttx/pull-requests/607/stm32f33xxx_adcc-fix-bug-in-rcc-reset/diff

[bob.fe...@rafresearch.com]

It's very similar to my option 'b', but it seems to not be thread safe.
It seems that simultaneous opens and/or closes to different ADCs on different threads could cause unexpected adc_rccresets to occur.

[Michał Łyszczek]

wtorek, 3 kwietnia 2018 11:17:31 CEST bob.fe...@rafresearch.com:

Good solution would probably be to pair these 2 ADCs into one single device in
fs. Just like when you open ADC1 with multiple channels you get single
"/dev/adc0" device and on read() you get struct with information what channel
that read corresponds to. In that solution you would open register ADC1 and
ADC2 into single "/dev/adc0" device and on read() you would receive
information about channel *and* adc device. That would force user to open and
close both devices at once.

While this seems good it has one big issue. It would require API changes in
ADC communication and there would be problems with ioctls - after first glance
at this, it looks impossible to implement with current API so it's rather a
no-go.

Well, looks like noone came up with any good solution to fix this and since
this is big issue I would suggest adding #error or at least #warning if
someone enables two or more ADCs with same RST pin. It doesn't really work
anyway (not without hacks in stm32_adc.c driver) so it shouldn't be possible
to set such configuration.

Thoughts?

--
Best regards
Łyszczek Michał