-
-
Notifications
You must be signed in to change notification settings - Fork 8.2k
stm32: Add analog switches configuration. #12706
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
cc8f9d1
to
5d0b10c
Compare
@iabdalkader which packages do the Arduino boards use? |
This is complicated! I need to think about it some more but here are some initial thoughts. The change in the first commit promotes "pads" to "pins", which I'm not sure makes sense. For example, on a part with both Would someone ever want to change the switch state at runtime. i.e. a generic board that has both On a part with the pad So the switches would default to open (except on parts with a On the Nicla, would a user ever need/want to open the switch, thereby disconnecting ULPI NXT and DIR pins? My feeling is that we really need to address this by making the ADC more sophisticated (we already want to do this anyway via ADCBlock -- e.g. you cannot currently use more than the lowest-numbered ADC on the channel that has the most ADCs). So for example: |
Damien suggested that rather than waiting for ADCBlock to exist in order to access the So in the af.csv, the PC2 line for example would be This would work the same way I will add this to the boardgen.py PR. I think we should keep the second commit from this PR though (with the |
No, this is definitely a board design characteristic, that depends on what those pins were intended to do. If
@jimmo @dpgeorge I don't think this needs to be any more complex than it is already, this PR lets you map board pins to dual pads as you normally would in However note that the sole purpose of this PR is to support mapping board pins to dual pads, for example on Portenta board pin adc = ADC("A0") It's important to use "A0" because it makes examples portable (on Giga board pin |
I see you've configure the switch for this pad to be open. That means the user can only use ADC on this board-level |
Replicating @iabdalkader's comment from #12211 here to keep the conversation in one place.
I left a comment on #12706. This feels like a workaround to access those channels, my goal is not just access the extra ADC channels, the main goal is to support mapping "board pins" to adc = ADC("A0") # ADC12_IN0 channel It's important to use NOTE: This fact that Also, if the above won't be supported, what should I map the following board pins to ? If I map them to their +A4,PC3
+A5,PC2
+A7,PA0
+A8,PC2_C
+A9,PC3_C
+A11,PA0_C A bigger problem here with the Portenta, those 4 pins their +A0,PA0_C
+A1,PA1_C
+A2,PC2_C
+A3,PC3_C |
The problem is that (It's especially confusing that "A0" is a board name in this case, which takes precendence over the cpu name I see your point about the fact that there needs to be a portable way to be able to use the ADC on the thing on the board labeled "A0". What if we were to support a special case on H7, where
So on the portenta,
I think the above suggestion addresses this? You could have e.g.
And with the switch open, |
To put this another way:
|
Yes that's the intended use, the |
@jimmo I'm very happy with this solution, in fact I think it fixes one more issue, as I've mentioned those pins should Not be allowed to be used as GPIOs otherwise they'll break things. |
7873533
to
3274d08
Compare
Signed-off-by: iabdalkader <i.abdalkader@gmail.com>
3274d08
to
e5014a4
Compare
Is
|
@iabdalkader Different PR, which I had just started on and was looking at the same thing. The main issue is that it used to suppress cpu-hidden (i.e. That part is easy to fix. The output should be:
So the problem is the double-up of A0 and A1 to channel 16. I think this worked essentially by accident before (and incorrectly, because it was assigning A0 and A1 to the _C channels). We probably need the logic that chooses a default channel for a pin to be aware that it's already used that channel for a different pin. (I guess one could argue it was working correctly before because it would have worked to use the _C channels, via the A0 pin, but I think this would still have incorrectly put the pin into ADC mode). But also I don't think A0,A1,C2,C3 should be in these tables, as you do not use these pins for ADC (only their _C pads). Also these tables are only used for pyb.ADC. Anyway, I'm working on it now. |
Ignoring dual-pad pins for now, what should the following config in af.csv do:
i.e. I should be able to do both Right now it will put both on ADC1 channel 16 (under the criteria that make-pins.py has always used -- first channel entry with the greatest number of units). Should we try and put them on different channels (in which case channel 16 will be excluded for PA1), or should we always take only the P channels? (I think that there is at most 1 P or N channel for a given pin?) |
I think I answered my own question... the previous code only got the P channels. |
Yes this particular issue was discussed somewhere here before and fixed at some point after 1.20 I think.
That would make sense, the stm32 ADC driver doesn't support differential mode, |
Sent #12867 to fix the immediate issue. |
Hi @jimmo just wondering if dual pads support is done somewhere and I missed it, or if it's still in the works ? |
This PR adds support for dual-pad pins, allowing access to the extra ADC channels on the second pads (when the analog switch is open). Additionally the second commit adds new config options to configure the analog switches, this is not strictly necessary I can move them back to
board_init.c
files, but figured that eventually more boards will need to configure them.