QSPI: add contributing guide #454

0xc0170 · 2018-03-15T14:09:56Z

Tests still needs to be written (To be implemented in that section)

SenRamakri

This is good start, please add more documentation around APIs.

SenRamakri · 2018-03-15T16:29:26Z

docs/reference/contributing/target/QuadSPI.md

-[Include implementation information here.]
+The target needs to define `qspi_s` structure - target specific QSPI object, enable QSPI in targets.json file `device_has` and implement QSPI HAL functions defined in `hal/qspi_api.h` header file.
+
+`qspi_write` and `qspi_read` are used for data transfers. For communicating with device, `qspi_command_transfer` should be used.



This is a good start, please add detailed documentation for each API. Also mention how the peripheral is configured, (like different operating modes, and what's supported with this implementation) and I would also like to see more info around what switches needs to be used in json files to configure this feature.

The API is documented in the header file, do we duplicate it here as well? I dont think so (neither can find anywhere in this contribution folder this ). I would expect the API to be documented in the header file (not certain what else we could add here about the functions) and we can share the pointer here

Also mention how the peripheral is configured, (like different operating modes, and what's supported with this implementation) and I would also like to see more info around what switches needs to be used in json files to configure this feature.

I'll add some info

0xc0170 · 2018-03-20T13:14:51Z

I pushed an update. I would like to reference qspi header file (similar to what RTC porting guide does).

@AnotherButler Can you help? This is what I would like to have here : https://github.com/ARMmbed/Handbook/blob/new_engine/docs/reference/contributing/target/rtc.md#implementing-the-rtc-api. I would then replace Functions to implement: to point to the header file.

That should answer documenting API.

What else is missing here?

0xc0170 · 2018-04-03T13:15:41Z

How can I help to make this in? Is there anything outstanding?

AnotherButler · 2018-04-03T15:34:58Z

docs/reference/contributing/target/QuadSPI.md

@@ -1,25 +1,50 @@
 <h3 id="quadspi-port">QuadSPI</h3>


Sometimes, we use "QuadSPI," and sometimes we use "QSPI". Which is the API in the code?

QSPI is an object in the drivers API The full name should be Quad Serial Peripheral Interface.

This then should be changed to QSPI?

Note: the name of the file is QuadSPI , also here in the port it is as it was. So which naming we should use?

AnotherButler · 2018-04-03T15:35:19Z

docs/reference/contributing/target/QuadSPI.md

@@ -1,25 +1,50 @@
 <h3 id="quadspi-port">QuadSPI</h3>

-[Include a brief description here.]
+The QSPI HAL defines API for targets that contain QSPI capable peripheral. Developers often use the QSPI interface for data storage.


The QSPI HAL defines API for targets? What does this mean? Please clarify.

AnotherButler · 2018-04-03T15:36:10Z

docs/reference/contributing/target/QuadSPI.md


 ##### Undefined behavior

-[Include any undefined behavior in bullet format here.]
+- Calling any function other than `qspi_init` before the initialization of the QSPI.



Please add a "Notes" section with any common trouble areas developers may come across when implementing this API (if applicable).

Not yet, will be filled later once we get more targets and questions about the porting

AnotherButler · 2018-04-03T15:37:51Z

docs/reference/contributing/target/QuadSPI.md

+"TARGET_NAME": {
+   "device_has": ["QSPI"]
+}
+```



Query: Is this on master? If so, please link to its Doxy on master. Then link to its Doxy on the feature branch. If not, please link only to the Doxy on the feature branch.

Not on master, only on feature branch. how to link (an example) ?

Ah. If it's not on master, then the link toward the top of this section is the correct one. I'm assuming any testing pages will be on the same branch, too: https://os-doc-builder.test.mbed.com/docs/development/feature-hal-spec-qspi-doxy/annotated.html?

We generate the feature-branch Doxy from the .json. You can find the page by replacing the "mbed-os-api-doxy" of the usual Doxy URL with the slug from the .json. (You probably don't need to know how to do that. I'm just telling you in case you're curious.)

AnotherButler · 2018-04-03T15:38:46Z

docs/reference/contributing/target/QuadSPI.md


 #### Testing

-[Include testing information here.]
+To be implemented


Please complete this section and add links to the Doxy tests.

There are no tests at the moment, what shall I put in here?

- use device_has to enable it - add API to be ported

Copy edit file for active voice and inclusion of articles.

0xc0170 · 2018-04-09T12:16:37Z

Rebased, conflicts resolved

Add link to class reference.

0xc0170 · 2018-04-17T10:54:12Z

Is there anything else I can help with to make this ready?

AnotherButler · 2018-04-17T20:02:48Z

@SenRamakri Is this OK to merge?

SenRamakri · 2018-04-17T20:06:20Z

docs/reference/contributing/target/QuadSPI.md


 ### Testing

-[Include testing information here.]
+To be implemented


@0xc0170 Are we planning to expand on this "Testing" section?

Yes, this should be added once the tests in the codebase (currently not implemented therefore this is empty)

AnotherButler · 2018-04-23T19:52:49Z

docs/reference/contributing/target/QuadSPI.md

@@ -1,25 +1,54 @@
 <h2 id="quadspi-port">QuadSPI</h2>

-[Include a brief description here.]
+Implementing QSPI enables Mbed OS to communicate with external memories much faster than via SPI.


@0xc0170 One outstanding item: Please expand this paragraph. What does this API do, and what are its use cases?

Added in cefda8b

Add content from PR #454 to live site after reviewing on test site.

AnotherButler requested a review from SenRamakri March 15, 2018 15:26

AnotherButler added the wait to merge label Mar 15, 2018

SenRamakri suggested changes Mar 15, 2018

View reviewed changes

0xc0170 force-pushed the dev_qspi_hal branch from ae73c1c to 9fa88b8 Compare March 20, 2018 13:16

AnotherButler reviewed Apr 3, 2018

View reviewed changes

0xc0170 and others added 4 commits April 9, 2018 13:07

QSPI: add contributing guide

b24fb01

QSPI: add API and how to enable it

1706bc0

- use device_has to enable it - add API to be ported

Copy edit QuadSPI.md

5905571

Copy edit file for active voice and inclusion of articles.

QSPI: fix introduction

898d1c6

0xc0170 force-pushed the dev_qspi_hal branch from 73a8129 to 898d1c6 Compare April 9, 2018 12:14

Update QuadSPI.md

a6c5c01

Add link to class reference.

AnotherButler removed the wait to merge label Apr 17, 2018

SenRamakri reviewed Apr 17, 2018

View reviewed changes

SenRamakri approved these changes Apr 23, 2018

View reviewed changes

AnotherButler reviewed Apr 23, 2018

View reviewed changes

QSPI: add use case + 4 data lines for communcation

a938c27

0xc0170 force-pushed the dev_qspi_hal branch from cefda8b to a938c27 Compare April 27, 2018 08:58

AnotherButler approved these changes Apr 30, 2018

View reviewed changes

AnotherButler merged commit 576444d into ARMmbed:new_engine Apr 30, 2018

AnotherButler pushed a commit that referenced this pull request May 1, 2018

Create QuadSPI.md

9b1ee45

Add content from PR #454 to live site after reviewing on test site.

QSPI: add contributing guide #454

QSPI: add contributing guide #454

Uh oh!

Conversation

0xc0170 commented Mar 15, 2018

Uh oh!

SenRamakri left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

0xc0170 Mar 20, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

0xc0170 commented Mar 20, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

0xc0170 commented Apr 3, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

0xc0170 Apr 9, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

0xc0170 commented Apr 9, 2018

Uh oh!

0xc0170 commented Apr 17, 2018

Uh oh!

AnotherButler commented Apr 17, 2018

Uh oh!

SenRamakri Apr 17, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

0xc0170 Mar 20, 2018 •

edited

Loading

0xc0170 commented Mar 20, 2018 •

edited

Loading

0xc0170 Apr 9, 2018 •

edited

Loading

SenRamakri Apr 17, 2018 •

edited

Loading