Skip to content

drivers/mtd: fix Kconfig dependencies #16836

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
Merged
merged 2 commits into from
Sep 22, 2021
Merged

drivers/mtd: fix Kconfig dependencies #16836

merged 2 commits into from
Sep 22, 2021

Conversation

fjmolinas
Copy link
Contributor

Contribution description

Currently, implementations of the MTD api are selecting the module,
this makes it easy for default backends to be selected. But the
correct modeling is simply selecting the MTD api and then selecting
a backend. BOARDs providing one of the backends can expose this and
then that backend can be selected by default.

There is also currently nothing preventing from using the MTD api
with no backend since a mock backend can easily be provided as is
done in all mtd tests.

Testing procedure

  • Murdock should take care of it
  • Test the change on native with tests/mtd_raw

Issues/PRs references

Split out of #16780

@github-actions github-actions bot added Area: cpu Area: CPU/MCU ports Area: drivers Area: Device drivers Area: Kconfig Area: Kconfig integration Area: tests Area: tests and testing framework Platform: native Platform: This PR/issue effects the native platform labels Sep 10, 2021
leandrolanzieri
leandrolanzieri reviewed Sep 13, 2021
View reviewed changes
fjmolinas
fjmolinas commented Sep 13, 2021
View reviewed changes
leandrolanzieri
leandrolanzieri reviewed Sep 13, 2021
View reviewed changes
@MrKevinWeiss MrKevinWeiss mentioned this pull request Sep 14, 2021
Merged
leandrolanzieri
leandrolanzieri reviewed Sep 14, 2021
View reviewed changes
@fjmolinas fjmolinas added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Sep 19, 2021
@fjmolinas
Copy link
Contributor Author

@leandrolanzieri murdock is green, do you still have concerns?

@leandrolanzieri
Copy link
Contributor

@leandrolanzieri murdock is green, do you still have concerns?

No, I think the changes are good! Please squash

@leandrolanzieri leandrolanzieri added this to the Release 2021.10 milestone Sep 22, 2021
@leandrolanzieri leandrolanzieri added Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 3-testing The PR was tested according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines Reviewed: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines labels Sep 22, 2021
@fjmolinas
tests/mtd_raw: add Kconfig
af74620
@fjmolinas
drivers/mtd: fix Kconfig dependencies
14ef09b 
Currently implementations of the MTD api are selecting the module,
this makes it easy for default backends to be selected. But the
correct modeling is simply selecting the MTD api and then selecting
a backend. BOARDs providing one of the backends can expose this and
then that backend can be selected by default.

There is also currently nothing preventing from using the MTD api
with no backend since a mock backend can easily be provided as is
done in all mtd tests.
@leandrolanzieri leandrolanzieri added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Sep 22, 2021
leandrolanzieri
leandrolanzieri approved these changes Sep 22, 2021
View reviewed changes
Copy link
Contributor

@leandrolanzieri leandrolanzieri left a comment

Choose a reason for hiding this comment

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

ACK!

@fjmolinas
Copy link
Contributor Author

go!

@fjmolinas fjmolinas merged commit 0b7eb2b into RIOT-OS:master Sep 22, 2021
@fjmolinas fjmolinas deleted the pr_kconfig_mtd branch September 22, 2021 08:08
@fjmolinas
Copy link
Contributor Author

Thanks for the review @leandrolanzieri!

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

@leandrolanzieri leandrolanzieri leandrolanzieri approved these changes

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

@cgundogan cgundogan Awaiting requested review from cgundogan

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

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

@smlng smlng Awaiting requested review from smlng

Assignees
No one assigned
Labels
Area: cpu Area: CPU/MCU ports Area: drivers Area: Device drivers Area: Kconfig Area: Kconfig integration Area: tests Area: tests and testing framework CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Platform: native Platform: This PR/issue effects the native platform Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 3-testing The PR was tested according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines Reviewed: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines
Projects
None yet
Milestone
Release 2021.10
Development

Successfully merging this pull request may close these issues.
2 participants
@fjmolinas @leandrolanzieri