Skip to content

QuadSPI API documentation #354

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

Merged
merged 5 commits into from
Aug 31, 2018

Conversation

SenRamakri
Copy link
Contributor

This provides Quad SPI driver API documentation. Note that the QuadSPI implementation will be in a feature branch for 5.7 release.

Make minor copy edits, mostly for active voice and formatting for rendering.
Copy link
Contributor

@AnotherButler AnotherButler left a comment

Choose a reason for hiding this comment

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

Please review my copy edits, and make sure I didn't accidentally change the meaning of anything.

@AnotherButler
Copy link
Contributor

AnotherButler commented Dec 4, 2017

Note to self: Merge after the updated Doxy kicks in. Also, inform web team of image when merging.

Copy link
Contributor Author

@SenRamakri SenRamakri left a comment

Choose a reason for hiding this comment

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

Copy edit looks good to me, Thanks..

@0xc0170
Copy link
Contributor

0xc0170 commented Dec 5, 2017

@AnotherButler Where do we define this is feature driver, it is on the branch. Means its under development (contributions welcome) ? I expect that as its feature branch, we receive feedback from partners, community that could potentially lead to changes.

I get an impression from reading this document that QSPI was released and it's ready for production

@AnotherButler
Copy link
Contributor

@0xc0170 This is a placeholder. Because we don't know when or how this will be delivered, we created it here for now. It will not be merged or added to the .json. Instead, it will be moved to the decided delivery method once we know what that is.

@AnotherButler AnotherButler changed the base branch from new_engine to development June 15, 2018 15:47
@kegilbert
Copy link
Contributor

kegilbert commented Aug 21, 2018

Some questions primarily in regards to the linked example for now:

https://os.mbed.com/teams/mbed_example/code/mbed-os-example-qspi/file/cc0165946232/main.cpp/

@AnotherButler Will ping an appropriate engineer when I hear back about who that is, should be tomorrow. EDIT: @0xc0170 @maciejbocianski

Is this the final example we plan on using? I like the actual functionality of it rather than just creating a QSPI instance and doing nothing with it, but the example seems like a bit much at first blush compared to our other examples. While we need more examples like this, should we have a smaller stepping stone example not tied to specific hardware in addition to this? I'm not too familiar with the QSPI feature so may not really be reasonable just throwing it out there.
EDIT: the inline doxygen actually has more streamlined examples, may be fine:
https://github.com/ARMmbed/mbed-os/pull/7783/files#diff-ea46523e3936b422e08016fa70ff15d7R41

In case this is the final example proposal, a few minor nits:

  1. Minor: Line 40: Why is the QSPI object being allocated here with new out of curiosity? Our other examples don't typically do this. Is it primarily to be able to check for memory allocation or QSPI ctor failures?
 myQspi = new QSPI((PinName)QSPI_PIN_IO0, (PinName)QSPI_PIN_IO1, (PinName)QSPI_PIN_IO2, (PinName)QSPI_PIN_IO3, (PinName)QSPI_PIN_SCK, (PinName)QSPI_PIN_CSN);
  1. Minor: Line 72: Why is the tx_buf initiated manually character by character rather than with
tx_buf[] = "HELLO QUAD-SPI!";
  1. Minor: Line 227: Argument comments for the final
    myQspi->command_transfer
    aren't lined up like the other calls are.

Copy link
Contributor

@maciejbocianski maciejbocianski left a comment

Choose a reason for hiding this comment

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

Certainly sample app should be improved.

@AnotherButler
Copy link
Contributor

Who can fix the example?

@kegilbert
Copy link
Contributor

@maciejbocianski is listed as the developer in the teams example, would you be able to clean up the example? Senthil will be out for another ~1.5 weeks.

@maciejbocianski
Copy link
Contributor

@kegilbert @AnotherButler sure thing, I will do it tomorrow

@kegilbert
Copy link
Contributor

Thanks!

Fix links for proper rendering.
Fix broken link.
Standardize naming across documents.
@AnotherButler AnotherButler merged commit 19a71b9 into ARMmbed:development Aug 31, 2018
@maciejbocianski
Copy link
Contributor

@AnotherButler @kegilbert @0xc0170 @SenRamakri
QSPI example and README has been updated (please https://os.mbed.com/teams/mbed_example/code/mbed-os-example-qspi/)

Please review

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.

5 participants