Skip to content
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

test: dac: esp32s2_devkitc bugfix #73015

Closed
wants to merge 1 commit into from

Conversation

wmrsouza
Copy link
Contributor

add dts and dac setting for tests/driver/dac/dac_api

add dts and dac setting for tests/driver/dac/dac_api

Signed-off-by: Marcio Ribeiro <marcio.ribeiro@espressif.com>
@hakehuang
Copy link
Collaborator

looks like there is a cycling dependency on the two PRs #72802, do we need merge them to in PR?

@sylvioalves
Copy link
Collaborator

looks like there is a cycling dependency on the two PRs #72802, do we need merge them to in PR?

@hakehuang I think so. Would you update your with this change or you want us to update this?

Comment on lines +7 to +9
&dac {
status = "okay";
};
Copy link
Member

Choose a reason for hiding this comment

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

Can we add this directly to the board file in order to avoid so many overlays here?

Copy link
Collaborator

Choose a reason for hiding this comment

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

this will case the dac is enabled by default for any application, which may increase your code size. but you can make decision.

Copy link
Member

Choose a reason for hiding this comment

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

No, it is only compiled if enabled via Kconfig. I prefer enabling it in the board file (similar to most other boards).

I realized that some ESP32 boards do indeed use an overlay as well, but that should probably be fixed.

@hakehuang
Copy link
Collaborator

@hakehuang I think so. Would you update your with this change or you want us to update this?

let me do this

@hakehuang
Copy link
Collaborator

I cherry-pick the commit to my pr #72984, thanks @martinjaeger and @wmrsouza , you can close this PR once merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: DAC Digital-to-Analog Converter
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants