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

flash: add docs for user defined data #6373

Merged
merged 1 commit into from Mar 28, 2018

Conversation

Projects
None yet
6 participants
@0xc0170
Member

0xc0170 commented Mar 15, 2018

This should make some of the data more clear to a user

Description

Documentation update for flash algo CMSIS

Feedback received here #6318 (comment)

cc @li-ho

Pull request type

  • Fix
  • Refactor
  • New target
  • Feature
  • Breaking change

@0xc0170 0xc0170 requested a review from AnotherButler Mar 15, 2018

const uint32_t start;
const uint32_t size;
const uint32_t start; /**< Sector start address */
const uint32_t size; /**< Sector size */

This comment has been minimized.

@li-ho

li-ho Mar 15, 2018

The typical range of size is between xxx and yyyy. It should not be less than xxx.

const uint32_t flash_size;
const sector_info_t *sectors;
const uint32_t sector_info_count;
const uint32_t page_size; /**< The minimum program page size */

This comment has been minimized.

@li-ho

li-ho Mar 15, 2018

The typical range of page_size is between x and yy, and it should not be more than yy.

This comment has been minimized.

@0xc0170

0xc0170 Mar 20, 2018

Member

Similar to sector size, how would upper and bottom limits help? I can list typical size, But that might be a better to include in the porting guide?

const sector_info_t *sectors;
const uint32_t sector_info_count;
const uint32_t page_size; /**< The minimum program page size */
const uint32_t flash_start; /**< Program page size */

This comment has been minimized.

@li-ho

li-ho Mar 15, 2018

Is flash_start a 32 bit memory address like 0xabcdef98? Should it always be 0?

This comment has been minimized.

@0xc0170

0xc0170 Mar 20, 2018

Member

not always 0,, and yes 32bit address. How would you change the description?

typedef struct {
const uint32_t start;
const uint32_t size;
const uint32_t start; /**< Sector start address */

This comment has been minimized.

@li-ho

li-ho Mar 15, 2018

Is start the start address of a flash memory block like 0x02000000? is it 32 bit address?

const uint32_t page_size; /**< The minimum program page size */
const uint32_t flash_start; /**< Program page size */
const uint32_t flash_size; /**< Flash size */
const sector_info_t *sectors; /**< List of sectors */

This comment has been minimized.

@li-ho

li-ho Mar 15, 2018

Does *sectors contain a list of flash memory block sector info if these memory blocks don't have the same sector size?

This comment has been minimized.

@0xc0170

0xc0170 Mar 20, 2018

Member

Yes, isn't that obvious from sector_info_t ? It defines sector size, and start

const uint32_t sector_info_count;
const uint32_t page_size; /**< The minimum program page size */
const uint32_t flash_start; /**< Program page size */
const uint32_t flash_size; /**< Flash size */

This comment has been minimized.

@li-ho

li-ho Mar 15, 2018

Is flash_size in sizeof(char) or sizeof(int)?

This comment has been minimized.

@0xc0170

0xc0170 Mar 20, 2018

Member

What is the question here?

const uint32_t flash_start; /**< Program page size */
const uint32_t flash_size; /**< Flash size */
const sector_info_t *sectors; /**< List of sectors */
const uint32_t sector_info_count; /**< Number of sectors */

This comment has been minimized.

@li-ho

li-ho Mar 15, 2018

The meanings of sector and page are so ambiguous unless you define the relationship among size , page_size and flash_size by an equation.

This comment has been minimized.

@0xc0170

0xc0170 Mar 20, 2018

Member

I added more info there

@0xc0170 0xc0170 force-pushed the 0xc0170:fix_flash_docs branch from 65c9dbf to 49d3a07 Mar 20, 2018

flash: add docs for user defined data
This should make some of the data more clear to a user

@0xc0170 0xc0170 force-pushed the 0xc0170:fix_flash_docs branch from 49d3a07 to e828b39 Mar 20, 2018

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Mar 20, 2018

I pushed an update to add more info to some members

@li-ho I think the level of detail you are looking for might be better for handbook, to describe what is sector, page , each size details ,etc.

@li-ho

li-ho approved these changes Mar 21, 2018

I have questions because I am a novice.

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Mar 21, 2018

I have questions because I am a novice.

👍 The code should provide details you can understand. Thanks for the feedback !

We got FlashIAP documentation plus flash HAL porting. I checked those, they do not touch the concept of what flash actually is (sectors vs page, write/read/erase - how they operate). This might be a good proposal for improvement. However, I do not think we cover the basic description of protocols, or drivers?
@AnotherButler what do you think?

@li-ho

This comment has been minimized.

li-ho commented Mar 22, 2018

In the document http://www.analog.com/media/en/technical-documentation/application-notes/EE381v01.pdf
the page it talks about is exactly sector here. You are welcome to clarify the overused jargon
which cannot be misunderstood in your context.

@AnotherButler

This comment has been minimized.

Contributor

AnotherButler commented Mar 22, 2018

@0xc0170 This sounds like great content for the Handbook. I'll create a task to complete this. Can I assign it to you, or can you recommend someone who can work on the content with me?

@0xc0170 0xc0170 requested a review from c1728p9 Mar 23, 2018

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Mar 26, 2018

@AnotherButler Happy with the docs changes here?

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Mar 26, 2018

/morph build

@mbed-ci

This comment has been minimized.

mbed-ci commented Mar 26, 2018

Build : SUCCESS

Build number : 1574
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/6373/

Triggering tests

/morph test
/morph uvisor-test
/morph export-build
/morph mbed2-build

@cmonr cmonr added needs: CI and removed needs: review labels Mar 26, 2018

@mbed-ci

This comment has been minimized.

@mbed-ci

This comment has been minimized.

@cmonr cmonr added ready for merge and removed needs: CI labels Mar 28, 2018

@adbridge adbridge merged commit 773fb90 into ARMmbed:master Mar 28, 2018

11 checks passed

AWS-CI uVisor Build & Test Success
Details
ci-morph-build build completed
Details
ci-morph-exporter build completed
Details
ci-morph-mbed2-build build completed
Details
ci-morph-test test completed
Details
continuous-integration/jenkins/pr-head This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
travis-ci/docs Local docs testing has passed
Details
travis-ci/events Local events testing has passed
Details
travis-ci/littlefs Passed, code size is 10060B
Details
travis-ci/tools Local tools testing has passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment