Skip to content

Initial flash support, empty code structure#20

Merged
erhankur merged 2 commits intoespressif:masterfrom
antmak:feat/spiflash_initial
Aug 25, 2025
Merged

Initial flash support, empty code structure#20
erhankur merged 2 commits intoespressif:masterfrom
antmak:feat/spiflash_initial

Conversation

@antmak
Copy link
Copy Markdown
Collaborator

@antmak antmak commented Jun 20, 2025

Initial code structure for SPI Flash support

  • adds target/common code structure
  • adds empty code structure for SPI Flash support for all targets

@antmak antmak requested a review from erhankur June 20, 2025 08:12
@github-actions
Copy link
Copy Markdown

github-actions bot commented Jun 20, 2025

Messages
📖 🎉 Good Job! All checks are passing!

👋 Hello antmak, we appreciate your contribution to this project!


Click to see more instructions ...


This automated output is generated by the PR linter DangerJS, which checks if your Pull Request meets the project's requirements and helps you fix potential issues.

DangerJS is triggered with each push event to a Pull Request and modify the contents of this comment.

Please consider the following:
- Danger mainly focuses on the PR structure and formatting and can't understand the meaning behind your code or changes.
- Danger is not a substitute for human code reviews; it's still important to request a code review from your colleagues.
- To manually retry these Danger checks, please navigate to the Actions tab and re-run last Danger workflow.

Review and merge process you can expect ...


We do welcome contributions in the form of bug reports, feature requests and pull requests.

1. An internal issue has been created for the PR, we assign it to the relevant engineer.
2. They review the PR and either approve it or ask you for changes or clarifications.
3. Once the GitHub PR is approved we do the final review, collect approvals from core owners and make sure all the automated tests are passing.
- At this point we may do some adjustments to the proposed change, or extend it by adding tests or documentation.
4. If the change is approved and passes the tests it is merged into the default branch.

Generated by 🚫 dangerJS against f54ee9e

@dobairoland dobairoland requested a review from radimkarnis June 20, 2025 08:21
@antmak antmak force-pushed the feat/spiflash_initial branch 2 times, most recently from 67c53ad to e086f2e Compare June 23, 2025 06:19
@erhankur
Copy link
Copy Markdown
Collaborator

@antmak new structure LGTM.

We are still having internal discussions regarding the use of esp_flash APIs, so I haven't looked into the flash implementation in detail yet. A few notes:

  • Please fix the build errors before the review process. I usually build and run each PR.
  • We should maintain consistency in Doxygen comments. Some functions have them, others don't.
  • In general, the flash code looks messy due to the trace logs. I understand you'll remove them soon. It's ok for now.
  • As you mentioned in the comments, we should avoid increasing the .rodata section size of the library. You can start by introducing new error codes instead. Otherwise, we're accumulating technical debt from the beginning.
  • I would prefer to see JIRA ticket IDs created for the TODOs.

@antmak antmak force-pushed the feat/spiflash_initial branch from e086f2e to df81832 Compare August 19, 2025 10:08
@antmak antmak changed the title FLASH support, initial structure FLASH support, empty code structure Aug 19, 2025
@antmak antmak force-pushed the feat/spiflash_initial branch 2 times, most recently from 9e9d185 to 45c5776 Compare August 21, 2025 05:55
@antmak
Copy link
Copy Markdown
Collaborator Author

antmak commented Aug 21, 2025

@erhankur PTAL

I did not add much implementation details here to make it easier to review the code structure

Doxygen

I sorted out comments, paid attention to return values.

error codes

Iinitial return codes have been add. Other changes being made on OOCD side. Split to #22

@antmak antmak force-pushed the feat/spiflash_initial branch 3 times, most recently from 677594d to bedb2cd Compare August 21, 2025 18:33
@antmak antmak changed the title FLASH support, empty code structure Initial flash support, empty code structure Aug 21, 2025
Copy link
Copy Markdown
Collaborator

@erhankur erhankur left a comment

Choose a reason for hiding this comment

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

@antmak Some minor comments. New common folder LGTM

@antmak antmak force-pushed the feat/spiflash_initial branch from bedb2cd to 5fb49f1 Compare August 25, 2025 12:25
@antmak antmak force-pushed the feat/spiflash_initial branch from 5fb49f1 to fffd93d Compare August 25, 2025 12:34
@antmak antmak force-pushed the feat/spiflash_initial branch from fffd93d to f54ee9e Compare August 25, 2025 12:40
@antmak antmak requested a review from erhankur August 25, 2025 12:50
@erhankur erhankur merged commit 6af3d44 into espressif:master Aug 25, 2025
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants