Skip to content

drivers:dac:Add support for MAX22007 #2787

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

drivers:dac:Add support for MAX22007 #2787

wants to merge 5 commits into from

Conversation

@jansunil
Copy link
Collaborator

@jansunil jansunil commented Oct 16, 2025 •
edited
Loading

Added driver, example project and documentation for MAX22007

Added a new API in the no_os_crc.c/.h to populate CRC table with LSB first logic

Pull Request Description

Please replace this with a detailed description and motivation of the changes.
You can tick the checkboxes below with an 'x' between square brackets or just check them after publishing the PR.
If this PR contains a breaking change, list dependent PRs and try to push all related PRs at the same time.

PR Type

  • Bug fix (change that fixes an issue)
  • New feature (change that adds new functionality)
  • Breaking change (has dependencies in other repos or will cause CI to fail)

PR Checklist

  • I have followed the Coding style guidelines
  • I have complied with the Submission Checklist
  • I have performed a self-review of the changes
  • I have commented my code, at least hard-to-understand parts
  • I have build all projects affected by the changes in this PR
  • I have tested in hardware affected projects, at the relevant boards
  • I have signed off all commits from this PR
  • I have updated the documentation (wiki pages, ReadMe etc), if applies

SPI Read Operation
image

SPI Write Operation
image

@jansunil jansunil force-pushed the max22007-dev branch 5 times, most recently from 9e41b68 to e9b599b Compare October 17, 2025 10:33
@jansunil jansunil marked this pull request as ready for review October 17, 2025 10:34
amiclaus
amiclaus reviewed Oct 17, 2025
View reviewed changes
Copy link
Contributor

@amiclaus amiclaus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

prior to the in-depth review, please separate your commits into: driver implementation, driver documentation, project implementation, project documentation. Also the crc implementation should be in its own commit.
Example: https://github.com/analogdevicesinc/no-OS/pull/2429/commits

drivers/dac/max22007/max22007.c Outdated
no_os_free(dev);

return 0;
} No newline at end of file
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: new line at the end of the file. check all files for this issue.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've updated the files to add a new line at the end

projects/max22007/builds.json
@@ -0,0 +1,7 @@
{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should be builds.json. Remove the underscore _ from the filename. Otherwise it won't be parsed by the CI. Unless you have the same issue as here: df789f6

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Renamed the file

@jansunil
drivers: util: Add LSB first implementation
34bc1e3 
Add LSB first implementation for populating CRC table

Signed-off-by: Janani Sunil <janani.sunil@analog.com>
@jansunil jansunil force-pushed the max22007-dev branch 4 times, most recently from c31703a to 26f99ce Compare October 17, 2025 12:48
amiclaus
amiclaus reviewed Oct 21, 2025
View reviewed changes
Copy link
Contributor

@amiclaus amiclaus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks solid overall, minor comments inline.

@jansunil
drivers: dac: MAX22007: Add support for MAX22007
65cd2ba 
Add driver support for MAX22007

Signed-off-by: Janani Sunil <janani.sunil@analog.com>
@jansunil
drivers: dac: max22007: Add README for MAX22007
360e7b4 
Add driver documentation for MAX22007

Signed-off-by: Janani Sunil <janani.sunil@analog.com>
@jansunil jansunil force-pushed the max22007-dev branch 2 times, most recently from ccb4baf to e2c0d9b Compare October 22, 2025 07:29
amiclaus
amiclaus reviewed Oct 23, 2025
View reviewed changes
projects/max22007/builds.json Outdated
{
"stm32": {
"basic_example": {
"flags" : "EXAMPLE=basic_example HARDWARE=sdp-k1.ioc"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

EXAMPLE=basic_example mismatches what's in the makefile. That's most probably what causes the build fail.

Copy link
Contributor

@amiclaus amiclaus Oct 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

EXAMPLE = basic

amiclaus
amiclaus reviewed Oct 24, 2025
View reviewed changes
projects/max22007/Makefile Outdated
@@ -0,0 +1,12 @@
EXAMPLE ?= basic_example
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

EXAMPLE ?= basic

projects/max22007/builds.json Outdated
{
"stm32": {
"basic_example": {
"flags" : "EXAMPLE=basic_example HARDWARE=sdp-k1.ioc"
Copy link
Contributor

@amiclaus amiclaus Oct 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

EXAMPLE = basic

@jansunil
projects:max22007: Add example project for max22007
9adbaf9 
Add basic example project for MAX22007

Add make based CI build support

Signed-off-by: Janani Sunil <janani.sunil@analog.com>
@jansunil
projects: max22007: Add README for MAX22007 project
76237b8 
Add documentation for MAX22007 example project

Signed-off-by: Janani Sunil <janani.sunil@analog.com>
buha
buha reviewed Oct 27, 2025
View reviewed changes
drivers/dac/max22007/max22007.h

/* Channel Mode Register Masks */
#define MAX22007_CH_MODE_MASK NO_OS_GENMASK(15, 12)
#define MAX22007_CH_MODE_CH_MASK(channel) (NO_OS_BIT(12) << (channel))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this doesn't look right to me, are you really trying to shift 0x1000 by 0, 1, 2, 3 ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@buha buha buha left review comments

@amiclaus amiclaus amiclaus left review comments

@CiprianRegus CiprianRegus Awaiting requested review from CiprianRegus CiprianRegus is a code owner

@dbogdan dbogdan Awaiting requested review from dbogdan dbogdan is a code owner

@rbolboac rbolboac Awaiting requested review from rbolboac rbolboac is a code owner

@danmois danmois Awaiting requested review from danmois danmois is a code owner

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@jansunil @buha @amiclaus