Add ADC family MAX11102-MAX11117 #60328
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
benediktibk
force-pushed
the
add_max11102
branch
3 times, most recently
from
c8f7da9
to
ea87f35
Compare
benediktibk
force-pushed
the
add_max11102
branch
7 times, most recently
from
a463310
to
92156db
Compare
benediktibk
requested review from
nashif,
tbursztyka,
galak and
anangl
as code owners
zephyrbot
added
area: Devicetree Binding
benediktibk
force-pushed
the
add_max11102
branch
from
92156db
to
bb71651
Compare
|
I've tested this driver with a MAX11102. The other ones are untested, but as they are fairly similar I'm confident enough to push them as well. |
benediktibk
force-pushed
the
add_max11102
branch
5 times, most recently
from
6de5b01
to
be461bf
Compare
benediktibk
force-pushed
the
add_max11102
branch
2 times, most recently
from
4335e24
to
f6a6a1c
Compare
benediktibk
force-pushed
the
add_max11102
branch
from
c05fe1b
to
74b29d6
Compare
benediktibk
force-pushed
the
add_max11102
branch
from
74b29d6
to
c17d547
Compare
MaureenHelm
previously approved these changes
Sep 6, 2023
benediktibk
force-pushed
the
add_max11102
branch
from
c17d547
to
19f5814
Compare
|
Rebased to resolve merge conflicts. |
benediktibk
force-pushed
the
add_max11102
branch
from
19f5814
to
0529deb
Compare
|
Rebased to resolve CI issues. |
MaureenHelm
previously approved these changes
Sep 7, 2023
anangl
requested changes
Sep 13, 2023
drivers/adc/adc_max11102_17.c
Outdated
| #endif | ||
|
|
||
| #if CONFIG_ADC_ASYNC | ||
| static void max11102_17_acquisition_thread(struct device *dev) |
There was a problem hiding this comment.
Suggested change
| static void max11102_17_acquisition_thread(struct device *dev) | |
| static void max11102_17_acquisition_thread(const struct device *dev) |
There was a problem hiding this comment.
Done, BUT: Looking into this I realized that k_thread_entry_t is actually defined as (void)(void*, void*, void*). At this point I am not sure if it might fuck up the stack if the signature of the actual function and the function pointer through which the call happens do not match?
Anyway, this seems to be the common approach, so at least it is now consistently dangerous ;-).
There was a problem hiding this comment.
outsourced my concerns to #62637
anangl
reviewed
Sep 13, 2023
benediktibk
force-pushed
the
add_max11102
branch
from
0529deb
to
aa72da6
Compare
Add bindings for the following ADCs: - MAX11102 - MAX11103 - MAX11105 - MAX11106 - MAX11110 - MAX11111 - MAX11115 - MAX11116 - MAX11117 Signed-off-by: Benedikt Schmidt <benedikt.schmidt@embedded-solutions.at>
Add a driver for the following ADCs: - MAX11102 - MAX11103 - MAX11105 - MAX11106 - MAX11110 - MAX11111 - MAX11115 - MAX11116 - MAX11117 Signed-off-by: Benedikt Schmidt <benedikt.schmidt@embedded-solutions.at>
Add all available instances of the ADC series MAX11102-MAX11117 to the ADC shell. Signed-off-by: Benedikt Schmidt <benedikt.schmidt@embedded-solutions.at>
Add instances of the ADC family MAX11102-MAX11117 to the build all tests. Signed-off-by: Benedikt Schmidt <benedikt.schmidt@embedded-solutions.at>
Increase the rodata section for build_all/adc to successfully build on the platform atsame54_xpro. Signed-off-by: Benedikt Schmidt <benedikt.schmidt@embedded-solutions.at>
Add myself as codeowner of the previously committed ADC driver for the MAX11102-17 series. Signed-off-by: Benedikt Schmidt <benedikt.schmidt@embedded-solutions.at>
benediktibk
force-pushed
the
add_max11102
branch
from
aa72da6
to
950b6cd
Compare
anangl
approved these changes
Sep 14, 2023
Comment on lines
+160
to
+162
| if (sequence_channel_count > 1) { | ||
| LOG_ERR("multiple channels selected"); | ||
| return -EINVAL; |
There was a problem hiding this comment.
Why? Would it be that hard to support sequences with multiple channels?
There was a problem hiding this comment.
I thought this feature was intended for ADCs which can actually read out multiple channels at once, which this IC definitely cannot. Of course I could simulate this via multiple reads in a row.
MaureenHelm
approved these changes
Sep 14, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add support for devices of the ADC family MAX11102-MAX11117.
Unfortunately these ADCs require to select the channel a GPIO change within a SPI transaction. Therefore, I had to extend the SPI subsystem with an intermediate callback which is triggered after a SPI buffer has been transferred.
The naming of the driver files themselves is also a little bit unfortunate with max11102_17, but max111xx would be problematic as there are also exist the devices MAX11120-MAX11128 which will require a totally different driver implementation.