You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2021/06/02 23:20:30 UTC
[GitHub] [incubator-nuttx] antmerlino opened a new pull request #3834: RFQ: progmem.h:
antmerlino opened a new pull request #3834:
URL: https://github.com/apache/incubator-nuttx/pull/3834
## Summary
## Impact
## Testing
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] jlaitine commented on pull request #3834: RFQ: progmem.h: Modify interface to avoid terms like page, sector, and block, as these terms are used interchangeably by different silicon vendors.
Posted by GitBox <gi...@apache.org>.
jlaitine commented on pull request #3834:
URL: https://github.com/apache/incubator-nuttx/pull/3834#issuecomment-854937825
> @jlaitine As I mentioned, the H7 was the only platform that interpreted getpage as the index of writable units instead of erasable units. Can you explain why this was necessary? It stands out to me that no one needed this information before and makes me wonder what the use-case is for this info is.
I don't think the above is true; you need to look into other devices which use the progmem interface on NAND flash (with ECC correction).
I am not saying that everything in stm32h7 flash driver is good :) But it is extremely important that we maintain clear distinction between the eraseblocks and write/read unit. Otherwise it is impossible to use the interface with NAND flash devices, where the smallest read/write unit actually matters. On NOR flash devices it pretty much makes no difference. On some NAND flash devices the smallest erasable unit is so small that you can actually mix these up by also reading/writing the same size.
On stm32h7 there is no room for compromise, since it has got a 128KB (!) smallest erase size and 32 byte read/write size. However, I have seen it mapped to both PX4 flashparams and also nuttx littlefs (latter turned out to make no sense, but worked in principle). So the it can't be all broken...
It is also that this distinction is there on purpose in progmem.h; see the topmost commit touching it:
"
commit 0f18e8cc328f1a420135531eb2afe57b27d3332f
Author: EunBong Song <eu...@samsung.com>
Date: Fri Sep 21 03:18:38 2018 +0000
"
Also check out the commit history of "drivers/mtd/mtd_progmem.c"
I don't object naming changes as such, if there is a common view that it is important. I just 1) see the need of distinction between the "eraseblock" and "write/read unit" for NAND flash support and 2) personally liked the nuttx-way of naming these "block" and "page".
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] antmerlino commented on pull request #3834: RFQ: progmem.h: Modify interface to avoid terms like page, sector, and block, as these terms are used interchangeably by different silicon vendors.
Posted by GitBox <gi...@apache.org>.
antmerlino commented on pull request #3834:
URL: https://github.com/apache/incubator-nuttx/pull/3834#issuecomment-854849401
@jlaitine As I mentioned, the H7 was the only platform that interpreted getpage as the index of writable units instead of erasable units. Can you explain why this was necessary? It stands out to me that no one needed this information before and makes me wonder what the use-case is for this info is.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] davids5 commented on a change in pull request #3834: RFQ: progmem.h: Modify interface to avoid terms like page, sector, and block, as these terms are used interchangeably by different silicon vendors.
Posted by GitBox <gi...@apache.org>.
davids5 commented on a change in pull request #3834:
URL: https://github.com/apache/incubator-nuttx/pull/3834#discussion_r644997995
##########
File path: include/nuttx/progmem.h
##########
@@ -147,48 +170,50 @@ size_t up_progmem_getaddress(size_t page);
*
****************************************************************************/
-ssize_t up_progmem_eraseblock(size_t block);
+ssize_t up_progmem_erase(size_t index);
/****************************************************************************
- * Name: up_progmem_ispageerased
+ * Name: up_progmem_issectionerased
*
* Description:
- * Checks whether erase page is erased
+ * Checks whether a specified unit of memory is erased
*
* Input Parameters:
- * page - The page index to be checked.
+ * index - ordinal number of the set of erasable units of memory
*
* Returned Value:
* Returns number of bytes NOT erased or negative value on error. If it
- * returns zero then complete page is erased.
+ * returns zero then complete section is erased.
*
* The following errors are reported:
- * -EFAULT: On invalid page
+ * -EFAULT: On invalid index
*
****************************************************************************/
-ssize_t up_progmem_ispageerased(size_t page);
+ssize_t up_progmem_iserased(size_t index);
/****************************************************************************
* Name: up_progmem_write
*
* Description:
* Program data at given address
*
- * Note: this function is not limited to single page and nor it requires
- * the address be aligned inside the page boundaries.
+ * Note: This function may cross memory boundaries so long as the caller
+ * has ensured all units of memory that will be programmed have been
+ * erased. It also does not require that the write be aligned with the
+ * beginning of a unit of memory.
*
* Input Parameters:
* addr - Address with or without flash offset
- * (absolute or aligned to page0)
+ * (absolute or aligned to sector0)
Review comment:
IIRC the H7 has 2 banks of FLASH. How would the user get the second one or know it is the second own without including arch specific code. Maybe we should talk this one through.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] jlaitine commented on a change in pull request #3834: RFQ: progmem.h: Modify interface to avoid terms like page, sector, and block, as these terms are used interchangeably by different silicon vendors.
Posted by GitBox <gi...@apache.org>.
jlaitine commented on a change in pull request #3834:
URL: https://github.com/apache/incubator-nuttx/pull/3834#discussion_r645304976
##########
File path: include/nuttx/progmem.h
##########
@@ -49,96 +49,127 @@ extern "C"
* Public Functions Definitions
****************************************************************************/
+/* NOTE: Since vendors use the terms page, block, and sector interchangably,
+ * it causes confusion and therefore a neutral term is used. We use the term
+ * "index" to refer to the ordinal number of erasable units of memory.
+ */
+
/****************************************************************************
- * Name: up_progmem_neraseblocks
+ * Name: up_progmem_maxeraseindex
*
* Description:
- * Return number of erase blocks
+ * Return the total number of erasable units of memory.
+ *
+ * Returned Value:
+ * Returns number of erasable units of memory, or 0 if there are no units
+ * available that can be programmed.
*
****************************************************************************/
-size_t up_progmem_neraseblocks(void);
+size_t up_progmem_maxeraseindex(void);
/****************************************************************************
* Name: up_progmem_isuniform
*
* Description:
- * Is program memory uniform or erase page and read/write page size differs?
+ * Are all erasable units of memory the same size?
+ *
+ * Returned Value:
+ * true if all erasable units are the same size, false if they vary in size
*
****************************************************************************/
bool up_progmem_isuniform(void);
/****************************************************************************
- * Name: up_progmem_pagesize
+ * Name: up_progmem_writegranularity
*
* Description:
- * Return read/write page size
+ * Return the number of bytes of the smallest allowable write operation.
+ * All writes MUST be a multiple of the granularity. Some FLASH memories
+ * require write operations in multiples of 8-bit, 16-bit, 32-bit, 128-bit,
+ * 256-bit, etc. The caller must ensure the size of the data passed into
+ * up_progmem_write is an even multiple of the writegranularity. Padding
+ * may be required.
+ *
+ * Returned Value:
+ * Required granularity of a write operation for the FLASH memory.
*
****************************************************************************/
-size_t up_progmem_pagesize(size_t page);
+size_t up_progmem_writegranularity(void);
/****************************************************************************
* Name: up_progmem_erasesize
*
* Description:
- * Return erase block size. Must be a multiple of the read/write page size.
+ * Return the erase size for a specified index. Will always be a multiple
+ * of the write granularity.
+ *
+ * Input Parameters:
+ * index - ordinal number of the set of erasable units of memory
+ *
+ * Returned Value:
+ * Erase size for erasable unit of memory, or negative value on error.
+ *
+ * The following errors are reported (errno is not set!):
+ *
+ * -EFAULT: On invalid unit of memory
*
****************************************************************************/
-size_t up_progmem_erasesize(size_t block);
+ssize_t up_progmem_erasesize(size_t index);
/****************************************************************************
- * Name: up_progmem_getpage
+ * Name: up_progmem_getindex
*
* Description:
- * Address to read/write page conversion
+ * Get the index of memory, given a FLASH memory address
*
* Input Parameters:
* addr - Address with or without flash offset
- * (absolute or aligned to page0)
+ * (absolute or aligned to the zeroeth unit of memory)
*
* Returned Value:
- * Page or negative value on error.
+ * Index or negative value on error.
* The following errors are reported (errno is not set!):
*
* -EFAULT: On invalid address
*
****************************************************************************/
-ssize_t up_progmem_getpage(size_t addr);
+ssize_t up_progmem_getindex(size_t addr);
Review comment:
In the original naming, the word "block" referred to smallest erasable unit of flash, and "page" referred to the smallest writable unit. This separation in naming should be kept consistent; On this line the function name uses the word "index" referring to original "block" (smallest erasable unit). This should really return the "write granule" and not the eraseblock!
So the function name here would be wrong.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] jlaitine commented on a change in pull request #3834: RFQ: progmem.h: Modify interface to avoid terms like page, sector, and block, as these terms are used interchangeably by different silicon vendors.
Posted by GitBox <gi...@apache.org>.
jlaitine commented on a change in pull request #3834:
URL: https://github.com/apache/incubator-nuttx/pull/3834#discussion_r645305535
##########
File path: include/nuttx/progmem.h
##########
@@ -49,96 +49,127 @@ extern "C"
* Public Functions Definitions
****************************************************************************/
+/* NOTE: Since vendors use the terms page, block, and sector interchangably,
+ * it causes confusion and therefore a neutral term is used. We use the term
+ * "index" to refer to the ordinal number of erasable units of memory.
+ */
+
/****************************************************************************
- * Name: up_progmem_neraseblocks
+ * Name: up_progmem_maxeraseindex
*
* Description:
- * Return number of erase blocks
+ * Return the total number of erasable units of memory.
+ *
+ * Returned Value:
+ * Returns number of erasable units of memory, or 0 if there are no units
+ * available that can be programmed.
*
****************************************************************************/
-size_t up_progmem_neraseblocks(void);
+size_t up_progmem_maxeraseindex(void);
/****************************************************************************
* Name: up_progmem_isuniform
*
* Description:
- * Is program memory uniform or erase page and read/write page size differs?
+ * Are all erasable units of memory the same size?
+ *
+ * Returned Value:
+ * true if all erasable units are the same size, false if they vary in size
*
****************************************************************************/
bool up_progmem_isuniform(void);
/****************************************************************************
- * Name: up_progmem_pagesize
+ * Name: up_progmem_writegranularity
*
* Description:
- * Return read/write page size
+ * Return the number of bytes of the smallest allowable write operation.
+ * All writes MUST be a multiple of the granularity. Some FLASH memories
+ * require write operations in multiples of 8-bit, 16-bit, 32-bit, 128-bit,
+ * 256-bit, etc. The caller must ensure the size of the data passed into
+ * up_progmem_write is an even multiple of the writegranularity. Padding
+ * may be required.
+ *
+ * Returned Value:
+ * Required granularity of a write operation for the FLASH memory.
*
****************************************************************************/
-size_t up_progmem_pagesize(size_t page);
+size_t up_progmem_writegranularity(void);
/****************************************************************************
* Name: up_progmem_erasesize
*
* Description:
- * Return erase block size. Must be a multiple of the read/write page size.
+ * Return the erase size for a specified index. Will always be a multiple
+ * of the write granularity.
+ *
+ * Input Parameters:
+ * index - ordinal number of the set of erasable units of memory
+ *
+ * Returned Value:
+ * Erase size for erasable unit of memory, or negative value on error.
+ *
+ * The following errors are reported (errno is not set!):
+ *
+ * -EFAULT: On invalid unit of memory
*
****************************************************************************/
-size_t up_progmem_erasesize(size_t block);
+ssize_t up_progmem_erasesize(size_t index);
/****************************************************************************
- * Name: up_progmem_getpage
+ * Name: up_progmem_getindex
*
* Description:
- * Address to read/write page conversion
+ * Get the index of memory, given a FLASH memory address
*
* Input Parameters:
* addr - Address with or without flash offset
- * (absolute or aligned to page0)
+ * (absolute or aligned to the zeroeth unit of memory)
*
* Returned Value:
- * Page or negative value on error.
+ * Index or negative value on error.
* The following errors are reported (errno is not set!):
*
* -EFAULT: On invalid address
*
****************************************************************************/
-ssize_t up_progmem_getpage(size_t addr);
+ssize_t up_progmem_getindex(size_t addr);
/****************************************************************************
* Name: up_progmem_getaddress
*
* Description:
- * Read/write page to address conversion
+ * Get the base address of a unit of memory, given an index
*
* Input Parameters:
- * page - page index
+ * index - ordinal number of the set of erasable units of memory
*
* Returned Value:
- * Base address of given page, SIZE_MAX if page index is not valid.
+ * Base address of given unit of memory, SIZE_MAX if index is not valid.
*
****************************************************************************/
-size_t up_progmem_getaddress(size_t page);
Review comment:
Same comment as above; word "index" is confusing, it is not obvious whether this returns the address of the smalles erasable unit, or address of the smalles writable unit
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] antmerlino commented on a change in pull request #3834: RFQ: progmem.h: Modify interface to avoid terms like page, sector, and block, as these terms are used interchangeably by different silicon vendors.
Posted by GitBox <gi...@apache.org>.
antmerlino commented on a change in pull request #3834:
URL: https://github.com/apache/incubator-nuttx/pull/3834#discussion_r645689393
##########
File path: include/nuttx/progmem.h
##########
@@ -49,96 +49,127 @@ extern "C"
* Public Functions Definitions
****************************************************************************/
+/* NOTE: Since vendors use the terms page, block, and sector interchangably,
+ * it causes confusion and therefore a neutral term is used. We use the term
+ * "index" to refer to the ordinal number of erasable units of memory.
+ */
+
/****************************************************************************
- * Name: up_progmem_neraseblocks
+ * Name: up_progmem_maxeraseindex
*
* Description:
- * Return number of erase blocks
+ * Return the total number of erasable units of memory.
+ *
+ * Returned Value:
+ * Returns number of erasable units of memory, or 0 if there are no units
+ * available that can be programmed.
*
****************************************************************************/
-size_t up_progmem_neraseblocks(void);
+size_t up_progmem_maxeraseindex(void);
/****************************************************************************
* Name: up_progmem_isuniform
*
* Description:
- * Is program memory uniform or erase page and read/write page size differs?
+ * Are all erasable units of memory the same size?
+ *
+ * Returned Value:
+ * true if all erasable units are the same size, false if they vary in size
*
****************************************************************************/
bool up_progmem_isuniform(void);
/****************************************************************************
- * Name: up_progmem_pagesize
+ * Name: up_progmem_writegranularity
*
* Description:
- * Return read/write page size
+ * Return the number of bytes of the smallest allowable write operation.
+ * All writes MUST be a multiple of the granularity. Some FLASH memories
+ * require write operations in multiples of 8-bit, 16-bit, 32-bit, 128-bit,
+ * 256-bit, etc. The caller must ensure the size of the data passed into
+ * up_progmem_write is an even multiple of the writegranularity. Padding
+ * may be required.
+ *
+ * Returned Value:
+ * Required granularity of a write operation for the FLASH memory.
*
****************************************************************************/
-size_t up_progmem_pagesize(size_t page);
+size_t up_progmem_writegranularity(void);
/****************************************************************************
* Name: up_progmem_erasesize
*
* Description:
- * Return erase block size. Must be a multiple of the read/write page size.
+ * Return the erase size for a specified index. Will always be a multiple
+ * of the write granularity.
+ *
+ * Input Parameters:
+ * index - ordinal number of the set of erasable units of memory
+ *
+ * Returned Value:
+ * Erase size for erasable unit of memory, or negative value on error.
+ *
+ * The following errors are reported (errno is not set!):
+ *
+ * -EFAULT: On invalid unit of memory
*
****************************************************************************/
-size_t up_progmem_erasesize(size_t block);
+ssize_t up_progmem_erasesize(size_t index);
/****************************************************************************
- * Name: up_progmem_getpage
+ * Name: up_progmem_getindex
*
* Description:
- * Address to read/write page conversion
+ * Get the index of memory, given a FLASH memory address
*
* Input Parameters:
* addr - Address with or without flash offset
- * (absolute or aligned to page0)
+ * (absolute or aligned to the zeroeth unit of memory)
*
* Returned Value:
- * Page or negative value on error.
+ * Index or negative value on error.
* The following errors are reported (errno is not set!):
*
* -EFAULT: On invalid address
*
****************************************************************************/
-ssize_t up_progmem_getpage(size_t addr);
+ssize_t up_progmem_getindex(size_t addr);
Review comment:
No.. this is literally the problem. You interpreted the getpage call as the smallest writable block. But if you look at the majority of the existing drivers, this returned the index of the erasable unit.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] antmerlino commented on a change in pull request #3834: RFQ: progmem.h: Modify interface to avoid terms like page, sector, and block, as these terms are used interchangeably by different silicon vendors.
Posted by GitBox <gi...@apache.org>.
antmerlino commented on a change in pull request #3834:
URL: https://github.com/apache/incubator-nuttx/pull/3834#discussion_r644981932
##########
File path: include/nuttx/progmem.h
##########
@@ -147,48 +170,50 @@ size_t up_progmem_getaddress(size_t page);
*
****************************************************************************/
-ssize_t up_progmem_eraseblock(size_t block);
+ssize_t up_progmem_erase(size_t index);
/****************************************************************************
- * Name: up_progmem_ispageerased
+ * Name: up_progmem_issectionerased
*
* Description:
- * Checks whether erase page is erased
+ * Checks whether a specified unit of memory is erased
*
* Input Parameters:
- * page - The page index to be checked.
+ * index - ordinal number of the set of erasable units of memory
*
* Returned Value:
* Returns number of bytes NOT erased or negative value on error. If it
- * returns zero then complete page is erased.
+ * returns zero then complete section is erased.
*
* The following errors are reported:
- * -EFAULT: On invalid page
+ * -EFAULT: On invalid index
*
****************************************************************************/
-ssize_t up_progmem_ispageerased(size_t page);
+ssize_t up_progmem_iserased(size_t index);
/****************************************************************************
* Name: up_progmem_write
*
* Description:
* Program data at given address
*
- * Note: this function is not limited to single page and nor it requires
- * the address be aligned inside the page boundaries.
+ * Note: This function may cross memory boundaries so long as the caller
+ * has ensured all units of memory that will be programmed have been
+ * erased. It also does not require that the write be aligned with the
+ * beginning of a unit of memory.
*
* Input Parameters:
* addr - Address with or without flash offset
- * (absolute or aligned to page0)
+ * (absolute or aligned to sector0)
Review comment:
@davids5 I believe I have addressed all of your comments except this one. The up_progmem_getaddress() interface could be used to get the base addresses of memory. Is this not sufficient? Or perhaps I don't understand your concern fully.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] davids5 commented on pull request #3834: RFQ: progmem.h: Modify interface to avoid terms like page, sector, and block, as these terms are used interchangeably by different silicon vendors.
Posted by GitBox <gi...@apache.org>.
davids5 commented on pull request #3834:
URL: https://github.com/apache/incubator-nuttx/pull/3834#issuecomment-853815799
@jlaitine Please have a look.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] antmerlino closed pull request #3834: RFQ: progmem.h:
Posted by GitBox <gi...@apache.org>.
antmerlino closed pull request #3834:
URL: https://github.com/apache/incubator-nuttx/pull/3834
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] davids5 commented on a change in pull request #3834: RFQ: progmem.h: Modify interface to avoid terms like page, sector, and block, as these terms are used interchangeably by different silicon vendors.
Posted by GitBox <gi...@apache.org>.
davids5 commented on a change in pull request #3834:
URL: https://github.com/apache/incubator-nuttx/pull/3834#discussion_r644734148
##########
File path: include/nuttx/progmem.h
##########
@@ -49,96 +49,119 @@ extern "C"
* Public Functions Definitions
****************************************************************************/
+/* NOTE: Since vendors use the terms page, block, and sector interchangably,
+ * it causes confusion and therefore a neutral term is used. We use the term
+ * "index" to refer to the ordinal number of erasable units of memory.
+ */
+
/****************************************************************************
- * Name: up_progmem_neraseblocks
+ * Name: up_progmem_maxeraseindex
*
* Description:
- * Return number of erase blocks
+ * Return the total number of erasable units of memory.
*
****************************************************************************/
-size_t up_progmem_neraseblocks(void);
+size_t up_progmem_maxeraseindex(void);
/****************************************************************************
* Name: up_progmem_isuniform
*
* Description:
- * Is program memory uniform or erase page and read/write page size differs?
+ * Are all erasable units of memory the same size?
+ *
+ * Returned Value:
+ * true if all erasable units are the same size, false if they vary in size
*
****************************************************************************/
bool up_progmem_isuniform(void);
/****************************************************************************
- * Name: up_progmem_pagesize
+ * Name: up_progmem_writegranularity
*
* Description:
- * Return read/write page size
+ * Return the number of bytes of the smallest allowable write operation.
+ * All writes MUST be a multiple of the granularity. Some FLASH memories
+ * require write operations in multiples of 8-bit, 16-bit, 32-bit, 128-bit,
+ * 256-bit, etc. The caller must ensure the size of the data passed into
+ * up_progmem_write is an even multiple of the writegranularity. Padding
+ * may be required.
+ *
+ * Returned Value:
+ * Required granularity of a write operation for the FLASH memory.
*
****************************************************************************/
-size_t up_progmem_pagesize(size_t page);
+size_t up_progmem_writegranularity(void);
/****************************************************************************
* Name: up_progmem_erasesize
*
* Description:
- * Return erase block size. Must be a multiple of the read/write page size.
+ * Return the erase size for a specified index. Will always be a multiple of
Review comment:
Needs Wrapping.
##########
File path: include/nuttx/progmem.h
##########
@@ -49,96 +49,119 @@ extern "C"
* Public Functions Definitions
****************************************************************************/
+/* NOTE: Since vendors use the terms page, block, and sector interchangably,
+ * it causes confusion and therefore a neutral term is used. We use the term
+ * "index" to refer to the ordinal number of erasable units of memory.
+ */
+
/****************************************************************************
- * Name: up_progmem_neraseblocks
+ * Name: up_progmem_maxeraseindex
*
* Description:
- * Return number of erase blocks
+ * Return the total number of erasable units of memory.
*
****************************************************************************/
-size_t up_progmem_neraseblocks(void);
+size_t up_progmem_maxeraseindex(void);
/****************************************************************************
* Name: up_progmem_isuniform
*
* Description:
- * Is program memory uniform or erase page and read/write page size differs?
+ * Are all erasable units of memory the same size?
+ *
+ * Returned Value:
+ * true if all erasable units are the same size, false if they vary in size
*
****************************************************************************/
bool up_progmem_isuniform(void);
/****************************************************************************
- * Name: up_progmem_pagesize
+ * Name: up_progmem_writegranularity
*
* Description:
- * Return read/write page size
+ * Return the number of bytes of the smallest allowable write operation.
+ * All writes MUST be a multiple of the granularity. Some FLASH memories
+ * require write operations in multiples of 8-bit, 16-bit, 32-bit, 128-bit,
+ * 256-bit, etc. The caller must ensure the size of the data passed into
+ * up_progmem_write is an even multiple of the writegranularity. Padding
+ * may be required.
+ *
+ * Returned Value:
+ * Required granularity of a write operation for the FLASH memory.
*
****************************************************************************/
-size_t up_progmem_pagesize(size_t page);
+size_t up_progmem_writegranularity(void);
/****************************************************************************
* Name: up_progmem_erasesize
*
* Description:
- * Return erase block size. Must be a multiple of the read/write page size.
+ * Return the erase size for a specified index. Will always be a multiple of
+ * the write granularity.
+ *
+ * Input Parameters:
+ * index - ordinal number of the set of erasable units of memory
+ *
+ * Returned Value:
+ * Erase size for erasable unit of memory, SIZE_MAX if index is invalid.
*
****************************************************************************/
-size_t up_progmem_erasesize(size_t block);
+size_t up_progmem_erasesize(size_t index);
Review comment:
```suggestion
ssize_t up_progmem_erasesize(size_t index);
```
##########
File path: include/nuttx/progmem.h
##########
@@ -207,12 +232,14 @@ ssize_t up_progmem_write(size_t addr, FAR const void *buf, size_t count);
* Description:
* Read data at given address
*
- * Note: this function is not limited to single page and nor it requires
- * the address be aligned inside the page boundaries.
+ * NOTE: This function does not require the address to be aligned with a
+ * unit of memory. It is also not limited to single unit of memory
+ * (i.e. multiple units of memory may be read, including partial unit
+ * reads)
*
* Input Parameters:
* addr - Address with or without flash offset
- * (absolute or aligned to page0)
+ * (absolute or aligned to sector0)
Review comment:
```suggestion
* (absolute or aligned to the zeroth unit of memory)
```
##########
File path: include/nuttx/progmem.h
##########
@@ -147,48 +170,50 @@ size_t up_progmem_getaddress(size_t page);
*
****************************************************************************/
-ssize_t up_progmem_eraseblock(size_t block);
+ssize_t up_progmem_erase(size_t index);
/****************************************************************************
- * Name: up_progmem_ispageerased
+ * Name: up_progmem_issectionerased
*
* Description:
- * Checks whether erase page is erased
+ * Checks whether a specified unit of memory is erased
*
* Input Parameters:
- * page - The page index to be checked.
+ * index - ordinal number of the set of erasable units of memory
*
* Returned Value:
* Returns number of bytes NOT erased or negative value on error. If it
- * returns zero then complete page is erased.
+ * returns zero then complete section is erased.
Review comment:
```suggestion
* returns zero then complete unit of memory is erased.
```
##########
File path: include/nuttx/progmem.h
##########
@@ -49,96 +49,119 @@ extern "C"
* Public Functions Definitions
****************************************************************************/
+/* NOTE: Since vendors use the terms page, block, and sector interchangably,
+ * it causes confusion and therefore a neutral term is used. We use the term
+ * "index" to refer to the ordinal number of erasable units of memory.
+ */
+
/****************************************************************************
- * Name: up_progmem_neraseblocks
+ * Name: up_progmem_maxeraseindex
*
* Description:
- * Return number of erase blocks
+ * Return the total number of erasable units of memory.
*
****************************************************************************/
-size_t up_progmem_neraseblocks(void);
+size_t up_progmem_maxeraseindex(void);
/****************************************************************************
* Name: up_progmem_isuniform
*
* Description:
- * Is program memory uniform or erase page and read/write page size differs?
+ * Are all erasable units of memory the same size?
+ *
+ * Returned Value:
+ * true if all erasable units are the same size, false if they vary in size
*
****************************************************************************/
bool up_progmem_isuniform(void);
/****************************************************************************
- * Name: up_progmem_pagesize
+ * Name: up_progmem_writegranularity
*
* Description:
- * Return read/write page size
+ * Return the number of bytes of the smallest allowable write operation.
+ * All writes MUST be a multiple of the granularity. Some FLASH memories
+ * require write operations in multiples of 8-bit, 16-bit, 32-bit, 128-bit,
+ * 256-bit, etc. The caller must ensure the size of the data passed into
+ * up_progmem_write is an even multiple of the writegranularity. Padding
+ * may be required.
+ *
+ * Returned Value:
+ * Required granularity of a write operation for the FLASH memory.
*
****************************************************************************/
-size_t up_progmem_pagesize(size_t page);
+size_t up_progmem_writegranularity(void);
/****************************************************************************
* Name: up_progmem_erasesize
*
* Description:
- * Return erase block size. Must be a multiple of the read/write page size.
+ * Return the erase size for a specified index. Will always be a multiple of
+ * the write granularity.
+ *
+ * Input Parameters:
+ * index - ordinal number of the set of erasable units of memory
+ *
+ * Returned Value:
+ * Erase size for erasable unit of memory, SIZE_MAX if index is invalid.
*
****************************************************************************/
-size_t up_progmem_erasesize(size_t block);
+size_t up_progmem_erasesize(size_t index);
/****************************************************************************
- * Name: up_progmem_getpage
+ * Name: up_progmem_getindex
*
* Description:
- * Address to read/write page conversion
+ * Get the index of memory, given a FLASH memory address
*
* Input Parameters:
* addr - Address with or without flash offset
- * (absolute or aligned to page0)
+ * (absolute or aligned to sector0)
*
* Returned Value:
- * Page or negative value on error.
+ * Index or negative value on error.
* The following errors are reported (errno is not set!):
*
* -EFAULT: On invalid address
*
****************************************************************************/
-ssize_t up_progmem_getpage(size_t addr);
+ssize_t up_progmem_getindex(size_t addr);
/****************************************************************************
* Name: up_progmem_getaddress
*
* Description:
- * Read/write page to address conversion
+ * Get the base address of a unit of memory, given an index
*
* Input Parameters:
- * page - page index
+ * index - ordinal number of the set of erasable units of memory
*
* Returned Value:
- * Base address of given page, SIZE_MAX if page index is not valid.
+ * Base address of given section, SIZE_MAX if index is not valid.
Review comment:
```suggestion
* Base address of given unit of memory, SIZE_MAX if index is not valid.
```
##########
File path: include/nuttx/progmem.h
##########
@@ -49,96 +49,119 @@ extern "C"
* Public Functions Definitions
****************************************************************************/
+/* NOTE: Since vendors use the terms page, block, and sector interchangably,
+ * it causes confusion and therefore a neutral term is used. We use the term
+ * "index" to refer to the ordinal number of erasable units of memory.
+ */
+
/****************************************************************************
- * Name: up_progmem_neraseblocks
+ * Name: up_progmem_maxeraseindex
*
* Description:
- * Return number of erase blocks
+ * Return the total number of erasable units of memory.
*
****************************************************************************/
-size_t up_progmem_neraseblocks(void);
+size_t up_progmem_maxeraseindex(void);
/****************************************************************************
* Name: up_progmem_isuniform
*
* Description:
- * Is program memory uniform or erase page and read/write page size differs?
+ * Are all erasable units of memory the same size?
+ *
+ * Returned Value:
+ * true if all erasable units are the same size, false if they vary in size
*
****************************************************************************/
bool up_progmem_isuniform(void);
/****************************************************************************
- * Name: up_progmem_pagesize
+ * Name: up_progmem_writegranularity
*
* Description:
- * Return read/write page size
+ * Return the number of bytes of the smallest allowable write operation.
+ * All writes MUST be a multiple of the granularity. Some FLASH memories
+ * require write operations in multiples of 8-bit, 16-bit, 32-bit, 128-bit,
+ * 256-bit, etc. The caller must ensure the size of the data passed into
+ * up_progmem_write is an even multiple of the writegranularity. Padding
+ * may be required.
+ *
+ * Returned Value:
+ * Required granularity of a write operation for the FLASH memory.
*
****************************************************************************/
-size_t up_progmem_pagesize(size_t page);
+size_t up_progmem_writegranularity(void);
/****************************************************************************
* Name: up_progmem_erasesize
*
* Description:
- * Return erase block size. Must be a multiple of the read/write page size.
+ * Return the erase size for a specified index. Will always be a multiple of
+ * the write granularity.
+ *
+ * Input Parameters:
+ * index - ordinal number of the set of erasable units of memory
+ *
+ * Returned Value:
+ * Erase size for erasable unit of memory, SIZE_MAX if index is invalid.
Review comment:
Would 0 be a better choice, or change the return type to ssize_t and return negative
##########
File path: include/nuttx/progmem.h
##########
@@ -49,96 +49,119 @@ extern "C"
* Public Functions Definitions
****************************************************************************/
+/* NOTE: Since vendors use the terms page, block, and sector interchangably,
+ * it causes confusion and therefore a neutral term is used. We use the term
+ * "index" to refer to the ordinal number of erasable units of memory.
+ */
+
/****************************************************************************
- * Name: up_progmem_neraseblocks
+ * Name: up_progmem_maxeraseindex
*
* Description:
- * Return number of erase blocks
+ * Return the total number of erasable units of memory.
Review comment:
Add the fail return value 0
##########
File path: include/nuttx/progmem.h
##########
@@ -49,96 +49,119 @@ extern "C"
* Public Functions Definitions
****************************************************************************/
+/* NOTE: Since vendors use the terms page, block, and sector interchangably,
+ * it causes confusion and therefore a neutral term is used. We use the term
+ * "index" to refer to the ordinal number of erasable units of memory.
+ */
+
/****************************************************************************
- * Name: up_progmem_neraseblocks
+ * Name: up_progmem_maxeraseindex
*
* Description:
- * Return number of erase blocks
+ * Return the total number of erasable units of memory.
*
****************************************************************************/
-size_t up_progmem_neraseblocks(void);
+size_t up_progmem_maxeraseindex(void);
/****************************************************************************
* Name: up_progmem_isuniform
*
* Description:
- * Is program memory uniform or erase page and read/write page size differs?
+ * Are all erasable units of memory the same size?
+ *
+ * Returned Value:
+ * true if all erasable units are the same size, false if they vary in size
*
****************************************************************************/
bool up_progmem_isuniform(void);
/****************************************************************************
- * Name: up_progmem_pagesize
+ * Name: up_progmem_writegranularity
*
* Description:
- * Return read/write page size
+ * Return the number of bytes of the smallest allowable write operation.
+ * All writes MUST be a multiple of the granularity. Some FLASH memories
+ * require write operations in multiples of 8-bit, 16-bit, 32-bit, 128-bit,
+ * 256-bit, etc. The caller must ensure the size of the data passed into
+ * up_progmem_write is an even multiple of the writegranularity. Padding
+ * may be required.
+ *
+ * Returned Value:
+ * Required granularity of a write operation for the FLASH memory.
*
****************************************************************************/
-size_t up_progmem_pagesize(size_t page);
+size_t up_progmem_writegranularity(void);
/****************************************************************************
* Name: up_progmem_erasesize
*
* Description:
- * Return erase block size. Must be a multiple of the read/write page size.
+ * Return the erase size for a specified index. Will always be a multiple of
+ * the write granularity.
+ *
+ * Input Parameters:
+ * index - ordinal number of the set of erasable units of memory
+ *
+ * Returned Value:
+ * Erase size for erasable unit of memory, SIZE_MAX if index is invalid.
*
****************************************************************************/
-size_t up_progmem_erasesize(size_t block);
+size_t up_progmem_erasesize(size_t index);
/****************************************************************************
- * Name: up_progmem_getpage
+ * Name: up_progmem_getindex
*
* Description:
- * Address to read/write page conversion
+ * Get the index of memory, given a FLASH memory address
*
* Input Parameters:
* addr - Address with or without flash offset
- * (absolute or aligned to page0)
+ * (absolute or aligned to sector0)
*
* Returned Value:
- * Page or negative value on error.
+ * Index or negative value on error.
* The following errors are reported (errno is not set!):
*
* -EFAULT: On invalid address
*
****************************************************************************/
-ssize_t up_progmem_getpage(size_t addr);
+ssize_t up_progmem_getindex(size_t addr);
/****************************************************************************
* Name: up_progmem_getaddress
*
* Description:
- * Read/write page to address conversion
+ * Get the base address of a unit of memory, given an index
*
* Input Parameters:
- * page - page index
+ * index - ordinal number of the set of erasable units of memory
*
* Returned Value:
- * Base address of given page, SIZE_MAX if page index is not valid.
+ * Base address of given section, SIZE_MAX if index is not valid.
*
****************************************************************************/
-size_t up_progmem_getaddress(size_t page);
+size_t up_progmem_getaddress(size_t index);
/****************************************************************************
- * Name: up_progmem_eraseblock
+ * Name: up_progmem_erase
*
* Description:
- * Erase selected erase block.
+ * Erase a specified unit of memory, given an index
*
* Input Parameters:
- * block - The erase block index to be erased.
+ * index - ordinal number of the set of erasable units of memory
*
* Returned Value:
- * block size or negative value on error.
+ * section size or negative value on error.
Review comment:
```suggestion
* The unit of memory's size or negative value on error.
```
return type needs to be ssize_t if negative return or use 0
##########
File path: include/nuttx/progmem.h
##########
@@ -234,6 +261,67 @@ ssize_t up_progmem_write(size_t addr, FAR const void *buf, size_t count);
ssize_t up_progmem_read(size_t addr, FAR void *buf, size_t count);
#endif
+/****************************************************************************
+ * Name: up_progmem_enableprogramming
+ *
+ * Description:
+ * Enable the ability to erase/write the FLASH memory.
+ *
Review comment:
Add: Typically done by "Unlocking" the FLASH programming IP Block
##########
File path: include/nuttx/progmem.h
##########
@@ -147,48 +170,50 @@ size_t up_progmem_getaddress(size_t page);
*
****************************************************************************/
-ssize_t up_progmem_eraseblock(size_t block);
+ssize_t up_progmem_erase(size_t index);
/****************************************************************************
- * Name: up_progmem_ispageerased
+ * Name: up_progmem_issectionerased
*
* Description:
- * Checks whether erase page is erased
+ * Checks whether a specified unit of memory is erased
*
* Input Parameters:
- * page - The page index to be checked.
+ * index - ordinal number of the set of erasable units of memory
*
* Returned Value:
* Returns number of bytes NOT erased or negative value on error. If it
- * returns zero then complete page is erased.
+ * returns zero then complete section is erased.
*
* The following errors are reported:
- * -EFAULT: On invalid page
+ * -EFAULT: On invalid index
*
****************************************************************************/
-ssize_t up_progmem_ispageerased(size_t page);
+ssize_t up_progmem_iserased(size_t index);
/****************************************************************************
* Name: up_progmem_write
*
* Description:
* Program data at given address
*
- * Note: this function is not limited to single page and nor it requires
- * the address be aligned inside the page boundaries.
+ * Note: This function may cross memory boundaries so long as the caller
+ * has ensured all units of memory that will be programmed have been
+ * erased. It also does not require that the write be aligned with the
+ * beginning of a unit of memory.
*
* Input Parameters:
* addr - Address with or without flash offset
- * (absolute or aligned to page0)
+ * (absolute or aligned to sector0)
Review comment:
```suggestion
* (absolute or aligned to the zeroth unit of memory)
```
##########
File path: include/nuttx/progmem.h
##########
@@ -234,6 +261,67 @@ ssize_t up_progmem_write(size_t addr, FAR const void *buf, size_t count);
ssize_t up_progmem_read(size_t addr, FAR void *buf, size_t count);
#endif
+/****************************************************************************
+ * Name: up_progmem_enableprogramming
+ *
+ * Description:
+ * Enable the ability to erase/write the FLASH memory.
+ *
+ * NOTE: Write-protection may still be required for an individual
+ * unit of memory.
+ *
+ * Returned Value:
+ * 0 on success, or negative value on error.
+ *
+ ****************************************************************************/
+
+int up_progmem_enableprogramming(void);
+
+/****************************************************************************
+ * Name: up_progmem_disableprogramming
+ *
+ * Description:
+ * Disable the ability to erase/write the FLASH memory.
+ *
Review comment:
"
##########
File path: include/nuttx/progmem.h
##########
@@ -49,96 +49,119 @@ extern "C"
* Public Functions Definitions
****************************************************************************/
+/* NOTE: Since vendors use the terms page, block, and sector interchangably,
+ * it causes confusion and therefore a neutral term is used. We use the term
+ * "index" to refer to the ordinal number of erasable units of memory.
+ */
+
/****************************************************************************
- * Name: up_progmem_neraseblocks
+ * Name: up_progmem_maxeraseindex
*
* Description:
- * Return number of erase blocks
+ * Return the total number of erasable units of memory.
*
****************************************************************************/
-size_t up_progmem_neraseblocks(void);
+size_t up_progmem_maxeraseindex(void);
/****************************************************************************
* Name: up_progmem_isuniform
*
* Description:
- * Is program memory uniform or erase page and read/write page size differs?
+ * Are all erasable units of memory the same size?
+ *
+ * Returned Value:
+ * true if all erasable units are the same size, false if they vary in size
*
****************************************************************************/
bool up_progmem_isuniform(void);
/****************************************************************************
- * Name: up_progmem_pagesize
+ * Name: up_progmem_writegranularity
*
* Description:
- * Return read/write page size
+ * Return the number of bytes of the smallest allowable write operation.
+ * All writes MUST be a multiple of the granularity. Some FLASH memories
+ * require write operations in multiples of 8-bit, 16-bit, 32-bit, 128-bit,
+ * 256-bit, etc. The caller must ensure the size of the data passed into
+ * up_progmem_write is an even multiple of the writegranularity. Padding
+ * may be required.
+ *
+ * Returned Value:
+ * Required granularity of a write operation for the FLASH memory.
*
****************************************************************************/
-size_t up_progmem_pagesize(size_t page);
+size_t up_progmem_writegranularity(void);
/****************************************************************************
* Name: up_progmem_erasesize
*
* Description:
- * Return erase block size. Must be a multiple of the read/write page size.
+ * Return the erase size for a specified index. Will always be a multiple of
+ * the write granularity.
+ *
+ * Input Parameters:
+ * index - ordinal number of the set of erasable units of memory
+ *
+ * Returned Value:
+ * Erase size for erasable unit of memory, SIZE_MAX if index is invalid.
*
****************************************************************************/
-size_t up_progmem_erasesize(size_t block);
+size_t up_progmem_erasesize(size_t index);
/****************************************************************************
- * Name: up_progmem_getpage
+ * Name: up_progmem_getindex
*
* Description:
- * Address to read/write page conversion
+ * Get the index of memory, given a FLASH memory address
*
* Input Parameters:
* addr - Address with or without flash offset
- * (absolute or aligned to page0)
+ * (absolute or aligned to sector0)
*
* Returned Value:
- * Page or negative value on error.
+ * Index or negative value on error.
* The following errors are reported (errno is not set!):
*
* -EFAULT: On invalid address
*
****************************************************************************/
-ssize_t up_progmem_getpage(size_t addr);
+ssize_t up_progmem_getindex(size_t addr);
/****************************************************************************
* Name: up_progmem_getaddress
*
* Description:
- * Read/write page to address conversion
+ * Get the base address of a unit of memory, given an index
*
* Input Parameters:
- * page - page index
+ * index - ordinal number of the set of erasable units of memory
*
* Returned Value:
- * Base address of given page, SIZE_MAX if page index is not valid.
+ * Base address of given section, SIZE_MAX if index is not valid.
*
****************************************************************************/
-size_t up_progmem_getaddress(size_t page);
+size_t up_progmem_getaddress(size_t index);
/****************************************************************************
- * Name: up_progmem_eraseblock
+ * Name: up_progmem_erase
*
* Description:
- * Erase selected erase block.
+ * Erase a specified unit of memory, given an index
*
* Input Parameters:
- * block - The erase block index to be erased.
+ * index - ordinal number of the set of erasable units of memory
*
* Returned Value:
- * block size or negative value on error.
+ * section size or negative value on error.
* The following errors are reported (errno is not set!):
*
- * -EFAULT: On invalid page
+ * -EFAULT: On invalid section
Review comment:
```suggestion
* -EFAULT: On invalid index
```
##########
File path: include/nuttx/progmem.h
##########
@@ -147,48 +170,50 @@ size_t up_progmem_getaddress(size_t page);
*
****************************************************************************/
-ssize_t up_progmem_eraseblock(size_t block);
+ssize_t up_progmem_erase(size_t index);
/****************************************************************************
- * Name: up_progmem_ispageerased
+ * Name: up_progmem_issectionerased
*
* Description:
- * Checks whether erase page is erased
+ * Checks whether a specified unit of memory is erased
*
* Input Parameters:
- * page - The page index to be checked.
+ * index - ordinal number of the set of erasable units of memory
*
* Returned Value:
* Returns number of bytes NOT erased or negative value on error. If it
- * returns zero then complete page is erased.
+ * returns zero then complete section is erased.
*
* The following errors are reported:
- * -EFAULT: On invalid page
+ * -EFAULT: On invalid index
*
****************************************************************************/
-ssize_t up_progmem_ispageerased(size_t page);
+ssize_t up_progmem_iserased(size_t index);
/****************************************************************************
* Name: up_progmem_write
*
* Description:
* Program data at given address
*
- * Note: this function is not limited to single page and nor it requires
- * the address be aligned inside the page boundaries.
+ * Note: This function may cross memory boundaries so long as the caller
+ * has ensured all units of memory that will be programmed have been
+ * erased. It also does not require that the write be aligned with the
+ * beginning of a unit of memory.
*
* Input Parameters:
* addr - Address with or without flash offset
- * (absolute or aligned to page0)
+ * (absolute or aligned to sector0)
Review comment:
I find this confusing.
If it is an address the caller has to know the base address of the memory, so the arch data has to be exposed to the caller. Would could add a function that returns the the base(s) as any counted array.
If it is BASE address less it is an offset.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] gustavonihei commented on a change in pull request #3834: RFQ: progmem.h: Modify interface to avoid terms like page, sector, and block, as these terms are used interchangeably by different silicon vendors.
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #3834:
URL: https://github.com/apache/incubator-nuttx/pull/3834#discussion_r646675017
##########
File path: include/nuttx/progmem.h
##########
@@ -49,96 +49,127 @@ extern "C"
* Public Functions Definitions
****************************************************************************/
+/* NOTE: Since vendors use the terms page, block, and sector interchangably,
+ * it causes confusion and therefore a neutral term is used. We use the term
+ * "index" to refer to the ordinal number of erasable units of memory.
+ */
+
/****************************************************************************
- * Name: up_progmem_neraseblocks
+ * Name: up_progmem_maxeraseindex
*
* Description:
- * Return number of erase blocks
+ * Return the total number of erasable units of memory.
+ *
+ * Returned Value:
+ * Returns number of erasable units of memory, or 0 if there are no units
+ * available that can be programmed.
*
****************************************************************************/
-size_t up_progmem_neraseblocks(void);
+size_t up_progmem_maxeraseindex(void);
/****************************************************************************
* Name: up_progmem_isuniform
*
* Description:
- * Is program memory uniform or erase page and read/write page size differs?
+ * Are all erasable units of memory the same size?
Review comment:
```suggestion
* Check whether all erasable units of memory are equally sized.
```
Uniformize alignment of description text.
Also, I think a description text with a question does not seem too fit for documentation.
Here is a suggestion for a direct statement.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #3834: RFQ: progmem.h: Modify interface to avoid terms like page, sector, and block, as these terms are used interchangeably by different silicon vendors.
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #3834:
URL: https://github.com/apache/incubator-nuttx/pull/3834#discussion_r644466631
##########
File path: include/nuttx/progmem.h
##########
@@ -147,48 +170,50 @@ size_t up_progmem_getaddress(size_t page);
*
****************************************************************************/
-ssize_t up_progmem_eraseblock(size_t block);
+ssize_t up_progmem_erase(size_t index);
/****************************************************************************
- * Name: up_progmem_ispageerased
+ * Name: up_progmem_issectionerased
Review comment:
up_progmem_iserased
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] antmerlino closed pull request #3834: RFQ: progmem.h:
Posted by GitBox <gi...@apache.org>.
antmerlino closed pull request #3834:
URL: https://github.com/apache/incubator-nuttx/pull/3834
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] antmerlino commented on pull request #3834: RFQ: progmem.h: Modify interface to avoid terms like page, sector, and block, as these terms are used interchangeably by different silicon vendors.
Posted by GitBox <gi...@apache.org>.
antmerlino commented on pull request #3834:
URL: https://github.com/apache/incubator-nuttx/pull/3834#issuecomment-854841346
"I think that mapping the "nuttx specified" naming should be done in the implementing code. For example, in stm32h7 stm_flash.c ( which @davids5 mentioned ), the mapping seems to be done correctly. STM32H7 name "SECTOR" really means smallest erasable unit (block) and "PAGE" means the write-granule (page). I guess there are drivers which map these differently (wrong) still; so these should also be fixed in order not to get burned."
Well, to be fair. It is really the STM32H7 that did not follow what all the other drivers did. It is the one that really introduced the disparity. In all other drivers, the getaddress and getpage calls returned what erasable unit you were in. On the H7 though, you got back the index for what writable unit you were in. And the claim that one is right and one is wrong is EXACTLY why I don't want to use sector or page terminology.
I hear what you are saying that we may need to expand the interface to include functions for both erasable and writable, but I refuse to accept that reintroducing the term page or sector as a good idea, because it's how the drivers got out of line in the first place.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] antmerlino commented on a change in pull request #3834: RFQ: progmem.h: Modify interface to avoid terms like page, sector, and block, as these terms are used interchangeably by different silicon vendors.
Posted by GitBox <gi...@apache.org>.
antmerlino commented on a change in pull request #3834:
URL: https://github.com/apache/incubator-nuttx/pull/3834#discussion_r644879328
##########
File path: include/nuttx/progmem.h
##########
@@ -49,96 +49,119 @@ extern "C"
* Public Functions Definitions
****************************************************************************/
+/* NOTE: Since vendors use the terms page, block, and sector interchangably,
+ * it causes confusion and therefore a neutral term is used. We use the term
+ * "index" to refer to the ordinal number of erasable units of memory.
+ */
+
/****************************************************************************
- * Name: up_progmem_neraseblocks
+ * Name: up_progmem_maxeraseindex
*
* Description:
- * Return number of erase blocks
+ * Return the total number of erasable units of memory.
Review comment:
@davids5 Could this even fail? I suppose 0 means that there are no programmable units of memory?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] jlaitine commented on a change in pull request #3834: RFQ: progmem.h: Modify interface to avoid terms like page, sector, and block, as these terms are used interchangeably by different silicon vendors.
Posted by GitBox <gi...@apache.org>.
jlaitine commented on a change in pull request #3834:
URL: https://github.com/apache/incubator-nuttx/pull/3834#discussion_r645305970
##########
File path: include/nuttx/progmem.h
##########
@@ -147,48 +178,50 @@ size_t up_progmem_getaddress(size_t page);
*
****************************************************************************/
-ssize_t up_progmem_eraseblock(size_t block);
+ssize_t up_progmem_erase(size_t index);
/****************************************************************************
- * Name: up_progmem_ispageerased
+ * Name: up_progmem_iserased
*
* Description:
- * Checks whether erase page is erased
+ * Checks whether a specified unit of memory is erased
*
* Input Parameters:
- * page - The page index to be checked.
+ * index - ordinal number of the set of erasable units of memory
*
* Returned Value:
* Returns number of bytes NOT erased or negative value on error. If it
- * returns zero then complete page is erased.
+ * returns zero then complete unit of memory is erased.
*
* The following errors are reported:
- * -EFAULT: On invalid page
+ * -EFAULT: On invalid index
*
****************************************************************************/
-ssize_t up_progmem_ispageerased(size_t page);
+ssize_t up_progmem_iserased(size_t index);
Review comment:
Same comment as above
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] antmerlino commented on a change in pull request #3834: RFQ: progmem.h: Modify interface to avoid terms like page, sector, and block, as these terms are used interchangeably by different silicon vendors.
Posted by GitBox <gi...@apache.org>.
antmerlino commented on a change in pull request #3834:
URL: https://github.com/apache/incubator-nuttx/pull/3834#discussion_r644879328
##########
File path: include/nuttx/progmem.h
##########
@@ -49,96 +49,119 @@ extern "C"
* Public Functions Definitions
****************************************************************************/
+/* NOTE: Since vendors use the terms page, block, and sector interchangably,
+ * it causes confusion and therefore a neutral term is used. We use the term
+ * "index" to refer to the ordinal number of erasable units of memory.
+ */
+
/****************************************************************************
- * Name: up_progmem_neraseblocks
+ * Name: up_progmem_maxeraseindex
*
* Description:
- * Return number of erase blocks
+ * Return the total number of erasable units of memory.
Review comment:
@davids5 Could this even fail? I suppose 0 means that there are no programmable units of memory?
##########
File path: include/nuttx/progmem.h
##########
@@ -147,48 +170,50 @@ size_t up_progmem_getaddress(size_t page);
*
****************************************************************************/
-ssize_t up_progmem_eraseblock(size_t block);
+ssize_t up_progmem_erase(size_t index);
/****************************************************************************
- * Name: up_progmem_ispageerased
+ * Name: up_progmem_issectionerased
*
* Description:
- * Checks whether erase page is erased
+ * Checks whether a specified unit of memory is erased
*
* Input Parameters:
- * page - The page index to be checked.
+ * index - ordinal number of the set of erasable units of memory
*
* Returned Value:
* Returns number of bytes NOT erased or negative value on error. If it
- * returns zero then complete page is erased.
+ * returns zero then complete section is erased.
*
* The following errors are reported:
- * -EFAULT: On invalid page
+ * -EFAULT: On invalid index
*
****************************************************************************/
-ssize_t up_progmem_ispageerased(size_t page);
+ssize_t up_progmem_iserased(size_t index);
/****************************************************************************
* Name: up_progmem_write
*
* Description:
* Program data at given address
*
- * Note: this function is not limited to single page and nor it requires
- * the address be aligned inside the page boundaries.
+ * Note: This function may cross memory boundaries so long as the caller
+ * has ensured all units of memory that will be programmed have been
+ * erased. It also does not require that the write be aligned with the
+ * beginning of a unit of memory.
*
* Input Parameters:
* addr - Address with or without flash offset
- * (absolute or aligned to page0)
+ * (absolute or aligned to sector0)
Review comment:
@davids5 I believe I have addressed all of your comments except this one. The up_progmem_getaddress() interface could be used to get the base addresses of memory. Is this not sufficient? Or perhaps I don't understand your concern fully.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] gustavonihei commented on a change in pull request #3834: RFQ: progmem.h: Modify interface to avoid terms like page, sector, and block, as these terms are used interchangeably by different silicon vendors.
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #3834:
URL: https://github.com/apache/incubator-nuttx/pull/3834#discussion_r646692432
##########
File path: include/nuttx/progmem.h
##########
@@ -49,96 +49,127 @@ extern "C"
* Public Functions Definitions
****************************************************************************/
+/* NOTE: Since vendors use the terms page, block, and sector interchangably,
+ * it causes confusion and therefore a neutral term is used. We use the term
+ * "index" to refer to the ordinal number of erasable units of memory.
+ */
+
/****************************************************************************
- * Name: up_progmem_neraseblocks
+ * Name: up_progmem_maxeraseindex
*
* Description:
- * Return number of erase blocks
+ * Return the total number of erasable units of memory.
+ *
+ * Returned Value:
+ * Returns number of erasable units of memory, or 0 if there are no units
+ * available that can be programmed.
*
****************************************************************************/
-size_t up_progmem_neraseblocks(void);
+size_t up_progmem_maxeraseindex(void);
/****************************************************************************
* Name: up_progmem_isuniform
*
* Description:
- * Is program memory uniform or erase page and read/write page size differs?
+ * Are all erasable units of memory the same size?
+ *
+ * Returned Value:
+ * true if all erasable units are the same size, false if they vary in size
*
****************************************************************************/
bool up_progmem_isuniform(void);
/****************************************************************************
- * Name: up_progmem_pagesize
+ * Name: up_progmem_writegranularity
*
* Description:
- * Return read/write page size
+ * Return the number of bytes of the smallest allowable write operation.
+ * All writes MUST be a multiple of the granularity. Some FLASH memories
+ * require write operations in multiples of 8-bit, 16-bit, 32-bit, 128-bit,
+ * 256-bit, etc. The caller must ensure the size of the data passed into
+ * up_progmem_write is an even multiple of the writegranularity. Padding
+ * may be required.
+ *
+ * Returned Value:
+ * Required granularity of a write operation for the FLASH memory.
*
****************************************************************************/
-size_t up_progmem_pagesize(size_t page);
+size_t up_progmem_writegranularity(void);
/****************************************************************************
* Name: up_progmem_erasesize
*
* Description:
- * Return erase block size. Must be a multiple of the read/write page size.
+ * Return the erase size for a specified index. Will always be a multiple
+ * of the write granularity.
Review comment:
```suggestion
* Return the erase size for a specified index. Will always be a multiple
* of the write granularity indicated by up_progmem_writegranularity.
```
It may sound repetitive, but it is helpful for making the link between those functions explicit.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] gustavonihei commented on a change in pull request #3834: RFQ: progmem.h: Modify interface to avoid terms like page, sector, and block, as these terms are used interchangeably by different silicon vendors.
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #3834:
URL: https://github.com/apache/incubator-nuttx/pull/3834#discussion_r646675017
##########
File path: include/nuttx/progmem.h
##########
@@ -49,96 +49,127 @@ extern "C"
* Public Functions Definitions
****************************************************************************/
+/* NOTE: Since vendors use the terms page, block, and sector interchangably,
+ * it causes confusion and therefore a neutral term is used. We use the term
+ * "index" to refer to the ordinal number of erasable units of memory.
+ */
+
/****************************************************************************
- * Name: up_progmem_neraseblocks
+ * Name: up_progmem_maxeraseindex
*
* Description:
- * Return number of erase blocks
+ * Return the total number of erasable units of memory.
+ *
+ * Returned Value:
+ * Returns number of erasable units of memory, or 0 if there are no units
+ * available that can be programmed.
*
****************************************************************************/
-size_t up_progmem_neraseblocks(void);
+size_t up_progmem_maxeraseindex(void);
/****************************************************************************
* Name: up_progmem_isuniform
*
* Description:
- * Is program memory uniform or erase page and read/write page size differs?
+ * Are all erasable units of memory the same size?
Review comment:
```suggestion
* Are all erasable units of memory the same size?
```
nit: Uniformize alignment of description text.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] gustavonihei commented on a change in pull request #3834: RFQ: progmem.h: Modify interface to avoid terms like page, sector, and block, as these terms are used interchangeably by different silicon vendors.
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #3834:
URL: https://github.com/apache/incubator-nuttx/pull/3834#discussion_r646692432
##########
File path: include/nuttx/progmem.h
##########
@@ -49,96 +49,127 @@ extern "C"
* Public Functions Definitions
****************************************************************************/
+/* NOTE: Since vendors use the terms page, block, and sector interchangably,
+ * it causes confusion and therefore a neutral term is used. We use the term
+ * "index" to refer to the ordinal number of erasable units of memory.
+ */
+
/****************************************************************************
- * Name: up_progmem_neraseblocks
+ * Name: up_progmem_maxeraseindex
*
* Description:
- * Return number of erase blocks
+ * Return the total number of erasable units of memory.
+ *
+ * Returned Value:
+ * Returns number of erasable units of memory, or 0 if there are no units
+ * available that can be programmed.
*
****************************************************************************/
-size_t up_progmem_neraseblocks(void);
+size_t up_progmem_maxeraseindex(void);
/****************************************************************************
* Name: up_progmem_isuniform
*
* Description:
- * Is program memory uniform or erase page and read/write page size differs?
+ * Are all erasable units of memory the same size?
+ *
+ * Returned Value:
+ * true if all erasable units are the same size, false if they vary in size
*
****************************************************************************/
bool up_progmem_isuniform(void);
/****************************************************************************
- * Name: up_progmem_pagesize
+ * Name: up_progmem_writegranularity
*
* Description:
- * Return read/write page size
+ * Return the number of bytes of the smallest allowable write operation.
+ * All writes MUST be a multiple of the granularity. Some FLASH memories
+ * require write operations in multiples of 8-bit, 16-bit, 32-bit, 128-bit,
+ * 256-bit, etc. The caller must ensure the size of the data passed into
+ * up_progmem_write is an even multiple of the writegranularity. Padding
+ * may be required.
+ *
+ * Returned Value:
+ * Required granularity of a write operation for the FLASH memory.
*
****************************************************************************/
-size_t up_progmem_pagesize(size_t page);
+size_t up_progmem_writegranularity(void);
/****************************************************************************
* Name: up_progmem_erasesize
*
* Description:
- * Return erase block size. Must be a multiple of the read/write page size.
+ * Return the erase size for a specified index. Will always be a multiple
+ * of the write granularity.
Review comment:
```suggestion
* Return the erase size for a specified index. Will always be a multiple
* of the write granularity indicated by up_progmem_writegranularity.
```
It may sound repetitive, but it is helpful for making the link between them explicit.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] jlaitine commented on a change in pull request #3834: RFQ: progmem.h: Modify interface to avoid terms like page, sector, and block, as these terms are used interchangeably by different silicon vendors.
Posted by GitBox <gi...@apache.org>.
jlaitine commented on a change in pull request #3834:
URL: https://github.com/apache/incubator-nuttx/pull/3834#discussion_r645304976
##########
File path: include/nuttx/progmem.h
##########
@@ -49,96 +49,127 @@ extern "C"
* Public Functions Definitions
****************************************************************************/
+/* NOTE: Since vendors use the terms page, block, and sector interchangably,
+ * it causes confusion and therefore a neutral term is used. We use the term
+ * "index" to refer to the ordinal number of erasable units of memory.
+ */
+
/****************************************************************************
- * Name: up_progmem_neraseblocks
+ * Name: up_progmem_maxeraseindex
*
* Description:
- * Return number of erase blocks
+ * Return the total number of erasable units of memory.
+ *
+ * Returned Value:
+ * Returns number of erasable units of memory, or 0 if there are no units
+ * available that can be programmed.
*
****************************************************************************/
-size_t up_progmem_neraseblocks(void);
+size_t up_progmem_maxeraseindex(void);
/****************************************************************************
* Name: up_progmem_isuniform
*
* Description:
- * Is program memory uniform or erase page and read/write page size differs?
+ * Are all erasable units of memory the same size?
+ *
+ * Returned Value:
+ * true if all erasable units are the same size, false if they vary in size
*
****************************************************************************/
bool up_progmem_isuniform(void);
/****************************************************************************
- * Name: up_progmem_pagesize
+ * Name: up_progmem_writegranularity
*
* Description:
- * Return read/write page size
+ * Return the number of bytes of the smallest allowable write operation.
+ * All writes MUST be a multiple of the granularity. Some FLASH memories
+ * require write operations in multiples of 8-bit, 16-bit, 32-bit, 128-bit,
+ * 256-bit, etc. The caller must ensure the size of the data passed into
+ * up_progmem_write is an even multiple of the writegranularity. Padding
+ * may be required.
+ *
+ * Returned Value:
+ * Required granularity of a write operation for the FLASH memory.
*
****************************************************************************/
-size_t up_progmem_pagesize(size_t page);
+size_t up_progmem_writegranularity(void);
/****************************************************************************
* Name: up_progmem_erasesize
*
* Description:
- * Return erase block size. Must be a multiple of the read/write page size.
+ * Return the erase size for a specified index. Will always be a multiple
+ * of the write granularity.
+ *
+ * Input Parameters:
+ * index - ordinal number of the set of erasable units of memory
+ *
+ * Returned Value:
+ * Erase size for erasable unit of memory, or negative value on error.
+ *
+ * The following errors are reported (errno is not set!):
+ *
+ * -EFAULT: On invalid unit of memory
*
****************************************************************************/
-size_t up_progmem_erasesize(size_t block);
+ssize_t up_progmem_erasesize(size_t index);
/****************************************************************************
- * Name: up_progmem_getpage
+ * Name: up_progmem_getindex
*
* Description:
- * Address to read/write page conversion
+ * Get the index of memory, given a FLASH memory address
*
* Input Parameters:
* addr - Address with or without flash offset
- * (absolute or aligned to page0)
+ * (absolute or aligned to the zeroeth unit of memory)
*
* Returned Value:
- * Page or negative value on error.
+ * Index or negative value on error.
* The following errors are reported (errno is not set!):
*
* -EFAULT: On invalid address
*
****************************************************************************/
-ssize_t up_progmem_getpage(size_t addr);
+ssize_t up_progmem_getindex(size_t addr);
Review comment:
In the original naming, the word "block" referred to smallest erasable unit of flash, and "page" referred to the smallest writable unit. This separation in naming should be kept consistent; On this line the function name uses the word "index" referring to original "block" (smallest erasable unit). This should really return the "write granule" and not the eraseblock!
So the function name here would be wrong.
##########
File path: include/nuttx/progmem.h
##########
@@ -49,96 +49,127 @@ extern "C"
* Public Functions Definitions
****************************************************************************/
+/* NOTE: Since vendors use the terms page, block, and sector interchangably,
+ * it causes confusion and therefore a neutral term is used. We use the term
+ * "index" to refer to the ordinal number of erasable units of memory.
+ */
+
/****************************************************************************
- * Name: up_progmem_neraseblocks
+ * Name: up_progmem_maxeraseindex
*
* Description:
- * Return number of erase blocks
+ * Return the total number of erasable units of memory.
+ *
+ * Returned Value:
+ * Returns number of erasable units of memory, or 0 if there are no units
+ * available that can be programmed.
*
****************************************************************************/
-size_t up_progmem_neraseblocks(void);
+size_t up_progmem_maxeraseindex(void);
/****************************************************************************
* Name: up_progmem_isuniform
*
* Description:
- * Is program memory uniform or erase page and read/write page size differs?
+ * Are all erasable units of memory the same size?
+ *
+ * Returned Value:
+ * true if all erasable units are the same size, false if they vary in size
*
****************************************************************************/
bool up_progmem_isuniform(void);
/****************************************************************************
- * Name: up_progmem_pagesize
+ * Name: up_progmem_writegranularity
*
* Description:
- * Return read/write page size
+ * Return the number of bytes of the smallest allowable write operation.
+ * All writes MUST be a multiple of the granularity. Some FLASH memories
+ * require write operations in multiples of 8-bit, 16-bit, 32-bit, 128-bit,
+ * 256-bit, etc. The caller must ensure the size of the data passed into
+ * up_progmem_write is an even multiple of the writegranularity. Padding
+ * may be required.
+ *
+ * Returned Value:
+ * Required granularity of a write operation for the FLASH memory.
*
****************************************************************************/
-size_t up_progmem_pagesize(size_t page);
+size_t up_progmem_writegranularity(void);
/****************************************************************************
* Name: up_progmem_erasesize
*
* Description:
- * Return erase block size. Must be a multiple of the read/write page size.
+ * Return the erase size for a specified index. Will always be a multiple
+ * of the write granularity.
+ *
+ * Input Parameters:
+ * index - ordinal number of the set of erasable units of memory
+ *
+ * Returned Value:
+ * Erase size for erasable unit of memory, or negative value on error.
+ *
+ * The following errors are reported (errno is not set!):
+ *
+ * -EFAULT: On invalid unit of memory
*
****************************************************************************/
-size_t up_progmem_erasesize(size_t block);
+ssize_t up_progmem_erasesize(size_t index);
/****************************************************************************
- * Name: up_progmem_getpage
+ * Name: up_progmem_getindex
*
* Description:
- * Address to read/write page conversion
+ * Get the index of memory, given a FLASH memory address
*
* Input Parameters:
* addr - Address with or without flash offset
- * (absolute or aligned to page0)
+ * (absolute or aligned to the zeroeth unit of memory)
*
* Returned Value:
- * Page or negative value on error.
+ * Index or negative value on error.
* The following errors are reported (errno is not set!):
*
* -EFAULT: On invalid address
*
****************************************************************************/
-ssize_t up_progmem_getpage(size_t addr);
+ssize_t up_progmem_getindex(size_t addr);
/****************************************************************************
* Name: up_progmem_getaddress
*
* Description:
- * Read/write page to address conversion
+ * Get the base address of a unit of memory, given an index
*
* Input Parameters:
- * page - page index
+ * index - ordinal number of the set of erasable units of memory
*
* Returned Value:
- * Base address of given page, SIZE_MAX if page index is not valid.
+ * Base address of given unit of memory, SIZE_MAX if index is not valid.
*
****************************************************************************/
-size_t up_progmem_getaddress(size_t page);
Review comment:
Same comment as above; word "index" is confusing, it is not obvious whether this returns the address of the smalles erasable unit, or address of the smalles writable unit
##########
File path: include/nuttx/progmem.h
##########
@@ -147,48 +178,50 @@ size_t up_progmem_getaddress(size_t page);
*
****************************************************************************/
-ssize_t up_progmem_eraseblock(size_t block);
+ssize_t up_progmem_erase(size_t index);
/****************************************************************************
- * Name: up_progmem_ispageerased
+ * Name: up_progmem_iserased
*
* Description:
- * Checks whether erase page is erased
+ * Checks whether a specified unit of memory is erased
*
* Input Parameters:
- * page - The page index to be checked.
+ * index - ordinal number of the set of erasable units of memory
*
* Returned Value:
* Returns number of bytes NOT erased or negative value on error. If it
- * returns zero then complete page is erased.
+ * returns zero then complete unit of memory is erased.
*
* The following errors are reported:
- * -EFAULT: On invalid page
+ * -EFAULT: On invalid index
*
****************************************************************************/
-ssize_t up_progmem_ispageerased(size_t page);
+ssize_t up_progmem_iserased(size_t index);
Review comment:
Same comment as above
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] davids5 commented on pull request #3834: RFQ: progmem.h: Modify interface to avoid terms like page, sector, and block, as these terms are used interchangeably by different silicon vendors.
Posted by GitBox <gi...@apache.org>.
davids5 commented on pull request #3834:
URL: https://github.com/apache/incubator-nuttx/pull/3834#issuecomment-853815799
@jlaitine Please have a look.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] gustavonihei commented on a change in pull request #3834: RFQ: progmem.h: Modify interface to avoid terms like page, sector, and block, as these terms are used interchangeably by different silicon vendors.
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #3834:
URL: https://github.com/apache/incubator-nuttx/pull/3834#discussion_r646682929
##########
File path: include/nuttx/progmem.h
##########
@@ -49,96 +49,127 @@ extern "C"
* Public Functions Definitions
****************************************************************************/
+/* NOTE: Since vendors use the terms page, block, and sector interchangably,
+ * it causes confusion and therefore a neutral term is used. We use the term
+ * "index" to refer to the ordinal number of erasable units of memory.
+ */
+
/****************************************************************************
- * Name: up_progmem_neraseblocks
+ * Name: up_progmem_maxeraseindex
*
* Description:
- * Return number of erase blocks
+ * Return the total number of erasable units of memory.
+ *
+ * Returned Value:
+ * Returns number of erasable units of memory, or 0 if there are no units
+ * available that can be programmed.
*
****************************************************************************/
-size_t up_progmem_neraseblocks(void);
+size_t up_progmem_maxeraseindex(void);
/****************************************************************************
* Name: up_progmem_isuniform
*
* Description:
- * Is program memory uniform or erase page and read/write page size differs?
+ * Are all erasable units of memory the same size?
+ *
+ * Returned Value:
+ * true if all erasable units are the same size, false if they vary in size
*
****************************************************************************/
bool up_progmem_isuniform(void);
/****************************************************************************
- * Name: up_progmem_pagesize
+ * Name: up_progmem_writegranularity
*
* Description:
- * Return read/write page size
+ * Return the number of bytes of the smallest allowable write operation.
+ * All writes MUST be a multiple of the granularity. Some FLASH memories
+ * require write operations in multiples of 8-bit, 16-bit, 32-bit, 128-bit,
+ * 256-bit, etc. The caller must ensure the size of the data passed into
+ * up_progmem_write is an even multiple of the writegranularity. Padding
+ * may be required.
Review comment:
```suggestion
* up_progmem_write is an even multiple of the up_progmem_writegranularity.
* Padding may be required.
```
Reference to the full name of the function.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #3834: RFQ: progmem.h: Modify interface to avoid terms like page, sector, and block, as these terms are used interchangeably by different silicon vendors.
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #3834:
URL: https://github.com/apache/incubator-nuttx/pull/3834#discussion_r644466631
##########
File path: include/nuttx/progmem.h
##########
@@ -147,48 +170,50 @@ size_t up_progmem_getaddress(size_t page);
*
****************************************************************************/
-ssize_t up_progmem_eraseblock(size_t block);
+ssize_t up_progmem_erase(size_t index);
/****************************************************************************
- * Name: up_progmem_ispageerased
+ * Name: up_progmem_issectionerased
Review comment:
up_progmem_iserased
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] davids5 commented on a change in pull request #3834: RFQ: progmem.h: Modify interface to avoid terms like page, sector, and block, as these terms are used interchangeably by different silicon vendors.
Posted by GitBox <gi...@apache.org>.
davids5 commented on a change in pull request #3834:
URL: https://github.com/apache/incubator-nuttx/pull/3834#discussion_r644896626
##########
File path: include/nuttx/progmem.h
##########
@@ -49,96 +49,119 @@ extern "C"
* Public Functions Definitions
****************************************************************************/
+/* NOTE: Since vendors use the terms page, block, and sector interchangably,
+ * it causes confusion and therefore a neutral term is used. We use the term
+ * "index" to refer to the ordinal number of erasable units of memory.
+ */
+
/****************************************************************************
- * Name: up_progmem_neraseblocks
+ * Name: up_progmem_maxeraseindex
*
* Description:
- * Return number of erase blocks
+ * Return the total number of erasable units of memory.
Review comment:
yep
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] jlaitine commented on a change in pull request #3834: RFQ: progmem.h: Modify interface to avoid terms like page, sector, and block, as these terms are used interchangeably by different silicon vendors.
Posted by GitBox <gi...@apache.org>.
jlaitine commented on a change in pull request #3834:
URL: https://github.com/apache/incubator-nuttx/pull/3834#discussion_r645776166
##########
File path: include/nuttx/progmem.h
##########
@@ -49,96 +49,127 @@ extern "C"
* Public Functions Definitions
****************************************************************************/
+/* NOTE: Since vendors use the terms page, block, and sector interchangably,
+ * it causes confusion and therefore a neutral term is used. We use the term
+ * "index" to refer to the ordinal number of erasable units of memory.
+ */
+
/****************************************************************************
- * Name: up_progmem_neraseblocks
+ * Name: up_progmem_maxeraseindex
*
* Description:
- * Return number of erase blocks
+ * Return the total number of erasable units of memory.
+ *
+ * Returned Value:
+ * Returns number of erasable units of memory, or 0 if there are no units
+ * available that can be programmed.
*
****************************************************************************/
-size_t up_progmem_neraseblocks(void);
+size_t up_progmem_maxeraseindex(void);
/****************************************************************************
* Name: up_progmem_isuniform
*
* Description:
- * Is program memory uniform or erase page and read/write page size differs?
+ * Are all erasable units of memory the same size?
+ *
+ * Returned Value:
+ * true if all erasable units are the same size, false if they vary in size
*
****************************************************************************/
bool up_progmem_isuniform(void);
/****************************************************************************
- * Name: up_progmem_pagesize
+ * Name: up_progmem_writegranularity
*
* Description:
- * Return read/write page size
+ * Return the number of bytes of the smallest allowable write operation.
+ * All writes MUST be a multiple of the granularity. Some FLASH memories
+ * require write operations in multiples of 8-bit, 16-bit, 32-bit, 128-bit,
+ * 256-bit, etc. The caller must ensure the size of the data passed into
+ * up_progmem_write is an even multiple of the writegranularity. Padding
+ * may be required.
+ *
+ * Returned Value:
+ * Required granularity of a write operation for the FLASH memory.
*
****************************************************************************/
-size_t up_progmem_pagesize(size_t page);
+size_t up_progmem_writegranularity(void);
/****************************************************************************
* Name: up_progmem_erasesize
*
* Description:
- * Return erase block size. Must be a multiple of the read/write page size.
+ * Return the erase size for a specified index. Will always be a multiple
+ * of the write granularity.
+ *
+ * Input Parameters:
+ * index - ordinal number of the set of erasable units of memory
+ *
+ * Returned Value:
+ * Erase size for erasable unit of memory, or negative value on error.
+ *
+ * The following errors are reported (errno is not set!):
+ *
+ * -EFAULT: On invalid unit of memory
*
****************************************************************************/
-size_t up_progmem_erasesize(size_t block);
+ssize_t up_progmem_erasesize(size_t index);
/****************************************************************************
- * Name: up_progmem_getpage
+ * Name: up_progmem_getindex
*
* Description:
- * Address to read/write page conversion
+ * Get the index of memory, given a FLASH memory address
*
* Input Parameters:
* addr - Address with or without flash offset
- * (absolute or aligned to page0)
+ * (absolute or aligned to the zeroeth unit of memory)
*
* Returned Value:
- * Page or negative value on error.
+ * Index or negative value on error.
* The following errors are reported (errno is not set!):
*
* -EFAULT: On invalid address
*
****************************************************************************/
-ssize_t up_progmem_getpage(size_t addr);
+ssize_t up_progmem_getindex(size_t addr);
Review comment:
If this is the intention, the function should be "up_progmem_getblock"
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] jlaitine commented on a change in pull request #3834: RFQ: progmem.h: Modify interface to avoid terms like page, sector, and block, as these terms are used interchangeably by different silicon vendors.
Posted by GitBox <gi...@apache.org>.
jlaitine commented on a change in pull request #3834:
URL: https://github.com/apache/incubator-nuttx/pull/3834#discussion_r645776166
##########
File path: include/nuttx/progmem.h
##########
@@ -49,96 +49,127 @@ extern "C"
* Public Functions Definitions
****************************************************************************/
+/* NOTE: Since vendors use the terms page, block, and sector interchangably,
+ * it causes confusion and therefore a neutral term is used. We use the term
+ * "index" to refer to the ordinal number of erasable units of memory.
+ */
+
/****************************************************************************
- * Name: up_progmem_neraseblocks
+ * Name: up_progmem_maxeraseindex
*
* Description:
- * Return number of erase blocks
+ * Return the total number of erasable units of memory.
+ *
+ * Returned Value:
+ * Returns number of erasable units of memory, or 0 if there are no units
+ * available that can be programmed.
*
****************************************************************************/
-size_t up_progmem_neraseblocks(void);
+size_t up_progmem_maxeraseindex(void);
/****************************************************************************
* Name: up_progmem_isuniform
*
* Description:
- * Is program memory uniform or erase page and read/write page size differs?
+ * Are all erasable units of memory the same size?
+ *
+ * Returned Value:
+ * true if all erasable units are the same size, false if they vary in size
*
****************************************************************************/
bool up_progmem_isuniform(void);
/****************************************************************************
- * Name: up_progmem_pagesize
+ * Name: up_progmem_writegranularity
*
* Description:
- * Return read/write page size
+ * Return the number of bytes of the smallest allowable write operation.
+ * All writes MUST be a multiple of the granularity. Some FLASH memories
+ * require write operations in multiples of 8-bit, 16-bit, 32-bit, 128-bit,
+ * 256-bit, etc. The caller must ensure the size of the data passed into
+ * up_progmem_write is an even multiple of the writegranularity. Padding
+ * may be required.
+ *
+ * Returned Value:
+ * Required granularity of a write operation for the FLASH memory.
*
****************************************************************************/
-size_t up_progmem_pagesize(size_t page);
+size_t up_progmem_writegranularity(void);
/****************************************************************************
* Name: up_progmem_erasesize
*
* Description:
- * Return erase block size. Must be a multiple of the read/write page size.
+ * Return the erase size for a specified index. Will always be a multiple
+ * of the write granularity.
+ *
+ * Input Parameters:
+ * index - ordinal number of the set of erasable units of memory
+ *
+ * Returned Value:
+ * Erase size for erasable unit of memory, or negative value on error.
+ *
+ * The following errors are reported (errno is not set!):
+ *
+ * -EFAULT: On invalid unit of memory
*
****************************************************************************/
-size_t up_progmem_erasesize(size_t block);
+ssize_t up_progmem_erasesize(size_t index);
/****************************************************************************
- * Name: up_progmem_getpage
+ * Name: up_progmem_getindex
*
* Description:
- * Address to read/write page conversion
+ * Get the index of memory, given a FLASH memory address
*
* Input Parameters:
* addr - Address with or without flash offset
- * (absolute or aligned to page0)
+ * (absolute or aligned to the zeroeth unit of memory)
*
* Returned Value:
- * Page or negative value on error.
+ * Index or negative value on error.
* The following errors are reported (errno is not set!):
*
* -EFAULT: On invalid address
*
****************************************************************************/
-ssize_t up_progmem_getpage(size_t addr);
+ssize_t up_progmem_getindex(size_t addr);
Review comment:
If this is the intention, the function should be "up_progmem_getblock", so it would be wrong originally.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] davids5 commented on a change in pull request #3834: RFQ: progmem.h: Modify interface to avoid terms like page, sector, and block, as these terms are used interchangeably by different silicon vendors.
Posted by GitBox <gi...@apache.org>.
davids5 commented on a change in pull request #3834:
URL: https://github.com/apache/incubator-nuttx/pull/3834#discussion_r644734148
##########
File path: include/nuttx/progmem.h
##########
@@ -49,96 +49,119 @@ extern "C"
* Public Functions Definitions
****************************************************************************/
+/* NOTE: Since vendors use the terms page, block, and sector interchangably,
+ * it causes confusion and therefore a neutral term is used. We use the term
+ * "index" to refer to the ordinal number of erasable units of memory.
+ */
+
/****************************************************************************
- * Name: up_progmem_neraseblocks
+ * Name: up_progmem_maxeraseindex
*
* Description:
- * Return number of erase blocks
+ * Return the total number of erasable units of memory.
*
****************************************************************************/
-size_t up_progmem_neraseblocks(void);
+size_t up_progmem_maxeraseindex(void);
/****************************************************************************
* Name: up_progmem_isuniform
*
* Description:
- * Is program memory uniform or erase page and read/write page size differs?
+ * Are all erasable units of memory the same size?
+ *
+ * Returned Value:
+ * true if all erasable units are the same size, false if they vary in size
*
****************************************************************************/
bool up_progmem_isuniform(void);
/****************************************************************************
- * Name: up_progmem_pagesize
+ * Name: up_progmem_writegranularity
*
* Description:
- * Return read/write page size
+ * Return the number of bytes of the smallest allowable write operation.
+ * All writes MUST be a multiple of the granularity. Some FLASH memories
+ * require write operations in multiples of 8-bit, 16-bit, 32-bit, 128-bit,
+ * 256-bit, etc. The caller must ensure the size of the data passed into
+ * up_progmem_write is an even multiple of the writegranularity. Padding
+ * may be required.
+ *
+ * Returned Value:
+ * Required granularity of a write operation for the FLASH memory.
*
****************************************************************************/
-size_t up_progmem_pagesize(size_t page);
+size_t up_progmem_writegranularity(void);
/****************************************************************************
* Name: up_progmem_erasesize
*
* Description:
- * Return erase block size. Must be a multiple of the read/write page size.
+ * Return the erase size for a specified index. Will always be a multiple of
Review comment:
Needs Wrapping.
##########
File path: include/nuttx/progmem.h
##########
@@ -49,96 +49,119 @@ extern "C"
* Public Functions Definitions
****************************************************************************/
+/* NOTE: Since vendors use the terms page, block, and sector interchangably,
+ * it causes confusion and therefore a neutral term is used. We use the term
+ * "index" to refer to the ordinal number of erasable units of memory.
+ */
+
/****************************************************************************
- * Name: up_progmem_neraseblocks
+ * Name: up_progmem_maxeraseindex
*
* Description:
- * Return number of erase blocks
+ * Return the total number of erasable units of memory.
*
****************************************************************************/
-size_t up_progmem_neraseblocks(void);
+size_t up_progmem_maxeraseindex(void);
/****************************************************************************
* Name: up_progmem_isuniform
*
* Description:
- * Is program memory uniform or erase page and read/write page size differs?
+ * Are all erasable units of memory the same size?
+ *
+ * Returned Value:
+ * true if all erasable units are the same size, false if they vary in size
*
****************************************************************************/
bool up_progmem_isuniform(void);
/****************************************************************************
- * Name: up_progmem_pagesize
+ * Name: up_progmem_writegranularity
*
* Description:
- * Return read/write page size
+ * Return the number of bytes of the smallest allowable write operation.
+ * All writes MUST be a multiple of the granularity. Some FLASH memories
+ * require write operations in multiples of 8-bit, 16-bit, 32-bit, 128-bit,
+ * 256-bit, etc. The caller must ensure the size of the data passed into
+ * up_progmem_write is an even multiple of the writegranularity. Padding
+ * may be required.
+ *
+ * Returned Value:
+ * Required granularity of a write operation for the FLASH memory.
*
****************************************************************************/
-size_t up_progmem_pagesize(size_t page);
+size_t up_progmem_writegranularity(void);
/****************************************************************************
* Name: up_progmem_erasesize
*
* Description:
- * Return erase block size. Must be a multiple of the read/write page size.
+ * Return the erase size for a specified index. Will always be a multiple of
+ * the write granularity.
+ *
+ * Input Parameters:
+ * index - ordinal number of the set of erasable units of memory
+ *
+ * Returned Value:
+ * Erase size for erasable unit of memory, SIZE_MAX if index is invalid.
*
****************************************************************************/
-size_t up_progmem_erasesize(size_t block);
+size_t up_progmem_erasesize(size_t index);
Review comment:
```suggestion
ssize_t up_progmem_erasesize(size_t index);
```
##########
File path: include/nuttx/progmem.h
##########
@@ -207,12 +232,14 @@ ssize_t up_progmem_write(size_t addr, FAR const void *buf, size_t count);
* Description:
* Read data at given address
*
- * Note: this function is not limited to single page and nor it requires
- * the address be aligned inside the page boundaries.
+ * NOTE: This function does not require the address to be aligned with a
+ * unit of memory. It is also not limited to single unit of memory
+ * (i.e. multiple units of memory may be read, including partial unit
+ * reads)
*
* Input Parameters:
* addr - Address with or without flash offset
- * (absolute or aligned to page0)
+ * (absolute or aligned to sector0)
Review comment:
```suggestion
* (absolute or aligned to the zeroth unit of memory)
```
##########
File path: include/nuttx/progmem.h
##########
@@ -147,48 +170,50 @@ size_t up_progmem_getaddress(size_t page);
*
****************************************************************************/
-ssize_t up_progmem_eraseblock(size_t block);
+ssize_t up_progmem_erase(size_t index);
/****************************************************************************
- * Name: up_progmem_ispageerased
+ * Name: up_progmem_issectionerased
*
* Description:
- * Checks whether erase page is erased
+ * Checks whether a specified unit of memory is erased
*
* Input Parameters:
- * page - The page index to be checked.
+ * index - ordinal number of the set of erasable units of memory
*
* Returned Value:
* Returns number of bytes NOT erased or negative value on error. If it
- * returns zero then complete page is erased.
+ * returns zero then complete section is erased.
Review comment:
```suggestion
* returns zero then complete unit of memory is erased.
```
##########
File path: include/nuttx/progmem.h
##########
@@ -49,96 +49,119 @@ extern "C"
* Public Functions Definitions
****************************************************************************/
+/* NOTE: Since vendors use the terms page, block, and sector interchangably,
+ * it causes confusion and therefore a neutral term is used. We use the term
+ * "index" to refer to the ordinal number of erasable units of memory.
+ */
+
/****************************************************************************
- * Name: up_progmem_neraseblocks
+ * Name: up_progmem_maxeraseindex
*
* Description:
- * Return number of erase blocks
+ * Return the total number of erasable units of memory.
*
****************************************************************************/
-size_t up_progmem_neraseblocks(void);
+size_t up_progmem_maxeraseindex(void);
/****************************************************************************
* Name: up_progmem_isuniform
*
* Description:
- * Is program memory uniform or erase page and read/write page size differs?
+ * Are all erasable units of memory the same size?
+ *
+ * Returned Value:
+ * true if all erasable units are the same size, false if they vary in size
*
****************************************************************************/
bool up_progmem_isuniform(void);
/****************************************************************************
- * Name: up_progmem_pagesize
+ * Name: up_progmem_writegranularity
*
* Description:
- * Return read/write page size
+ * Return the number of bytes of the smallest allowable write operation.
+ * All writes MUST be a multiple of the granularity. Some FLASH memories
+ * require write operations in multiples of 8-bit, 16-bit, 32-bit, 128-bit,
+ * 256-bit, etc. The caller must ensure the size of the data passed into
+ * up_progmem_write is an even multiple of the writegranularity. Padding
+ * may be required.
+ *
+ * Returned Value:
+ * Required granularity of a write operation for the FLASH memory.
*
****************************************************************************/
-size_t up_progmem_pagesize(size_t page);
+size_t up_progmem_writegranularity(void);
/****************************************************************************
* Name: up_progmem_erasesize
*
* Description:
- * Return erase block size. Must be a multiple of the read/write page size.
+ * Return the erase size for a specified index. Will always be a multiple of
+ * the write granularity.
+ *
+ * Input Parameters:
+ * index - ordinal number of the set of erasable units of memory
+ *
+ * Returned Value:
+ * Erase size for erasable unit of memory, SIZE_MAX if index is invalid.
*
****************************************************************************/
-size_t up_progmem_erasesize(size_t block);
+size_t up_progmem_erasesize(size_t index);
/****************************************************************************
- * Name: up_progmem_getpage
+ * Name: up_progmem_getindex
*
* Description:
- * Address to read/write page conversion
+ * Get the index of memory, given a FLASH memory address
*
* Input Parameters:
* addr - Address with or without flash offset
- * (absolute or aligned to page0)
+ * (absolute or aligned to sector0)
*
* Returned Value:
- * Page or negative value on error.
+ * Index or negative value on error.
* The following errors are reported (errno is not set!):
*
* -EFAULT: On invalid address
*
****************************************************************************/
-ssize_t up_progmem_getpage(size_t addr);
+ssize_t up_progmem_getindex(size_t addr);
/****************************************************************************
* Name: up_progmem_getaddress
*
* Description:
- * Read/write page to address conversion
+ * Get the base address of a unit of memory, given an index
*
* Input Parameters:
- * page - page index
+ * index - ordinal number of the set of erasable units of memory
*
* Returned Value:
- * Base address of given page, SIZE_MAX if page index is not valid.
+ * Base address of given section, SIZE_MAX if index is not valid.
Review comment:
```suggestion
* Base address of given unit of memory, SIZE_MAX if index is not valid.
```
##########
File path: include/nuttx/progmem.h
##########
@@ -49,96 +49,119 @@ extern "C"
* Public Functions Definitions
****************************************************************************/
+/* NOTE: Since vendors use the terms page, block, and sector interchangably,
+ * it causes confusion and therefore a neutral term is used. We use the term
+ * "index" to refer to the ordinal number of erasable units of memory.
+ */
+
/****************************************************************************
- * Name: up_progmem_neraseblocks
+ * Name: up_progmem_maxeraseindex
*
* Description:
- * Return number of erase blocks
+ * Return the total number of erasable units of memory.
*
****************************************************************************/
-size_t up_progmem_neraseblocks(void);
+size_t up_progmem_maxeraseindex(void);
/****************************************************************************
* Name: up_progmem_isuniform
*
* Description:
- * Is program memory uniform or erase page and read/write page size differs?
+ * Are all erasable units of memory the same size?
+ *
+ * Returned Value:
+ * true if all erasable units are the same size, false if they vary in size
*
****************************************************************************/
bool up_progmem_isuniform(void);
/****************************************************************************
- * Name: up_progmem_pagesize
+ * Name: up_progmem_writegranularity
*
* Description:
- * Return read/write page size
+ * Return the number of bytes of the smallest allowable write operation.
+ * All writes MUST be a multiple of the granularity. Some FLASH memories
+ * require write operations in multiples of 8-bit, 16-bit, 32-bit, 128-bit,
+ * 256-bit, etc. The caller must ensure the size of the data passed into
+ * up_progmem_write is an even multiple of the writegranularity. Padding
+ * may be required.
+ *
+ * Returned Value:
+ * Required granularity of a write operation for the FLASH memory.
*
****************************************************************************/
-size_t up_progmem_pagesize(size_t page);
+size_t up_progmem_writegranularity(void);
/****************************************************************************
* Name: up_progmem_erasesize
*
* Description:
- * Return erase block size. Must be a multiple of the read/write page size.
+ * Return the erase size for a specified index. Will always be a multiple of
+ * the write granularity.
+ *
+ * Input Parameters:
+ * index - ordinal number of the set of erasable units of memory
+ *
+ * Returned Value:
+ * Erase size for erasable unit of memory, SIZE_MAX if index is invalid.
Review comment:
Would 0 be a better choice, or change the return type to ssize_t and return negative
##########
File path: include/nuttx/progmem.h
##########
@@ -49,96 +49,119 @@ extern "C"
* Public Functions Definitions
****************************************************************************/
+/* NOTE: Since vendors use the terms page, block, and sector interchangably,
+ * it causes confusion and therefore a neutral term is used. We use the term
+ * "index" to refer to the ordinal number of erasable units of memory.
+ */
+
/****************************************************************************
- * Name: up_progmem_neraseblocks
+ * Name: up_progmem_maxeraseindex
*
* Description:
- * Return number of erase blocks
+ * Return the total number of erasable units of memory.
Review comment:
Add the fail return value 0
##########
File path: include/nuttx/progmem.h
##########
@@ -49,96 +49,119 @@ extern "C"
* Public Functions Definitions
****************************************************************************/
+/* NOTE: Since vendors use the terms page, block, and sector interchangably,
+ * it causes confusion and therefore a neutral term is used. We use the term
+ * "index" to refer to the ordinal number of erasable units of memory.
+ */
+
/****************************************************************************
- * Name: up_progmem_neraseblocks
+ * Name: up_progmem_maxeraseindex
*
* Description:
- * Return number of erase blocks
+ * Return the total number of erasable units of memory.
*
****************************************************************************/
-size_t up_progmem_neraseblocks(void);
+size_t up_progmem_maxeraseindex(void);
/****************************************************************************
* Name: up_progmem_isuniform
*
* Description:
- * Is program memory uniform or erase page and read/write page size differs?
+ * Are all erasable units of memory the same size?
+ *
+ * Returned Value:
+ * true if all erasable units are the same size, false if they vary in size
*
****************************************************************************/
bool up_progmem_isuniform(void);
/****************************************************************************
- * Name: up_progmem_pagesize
+ * Name: up_progmem_writegranularity
*
* Description:
- * Return read/write page size
+ * Return the number of bytes of the smallest allowable write operation.
+ * All writes MUST be a multiple of the granularity. Some FLASH memories
+ * require write operations in multiples of 8-bit, 16-bit, 32-bit, 128-bit,
+ * 256-bit, etc. The caller must ensure the size of the data passed into
+ * up_progmem_write is an even multiple of the writegranularity. Padding
+ * may be required.
+ *
+ * Returned Value:
+ * Required granularity of a write operation for the FLASH memory.
*
****************************************************************************/
-size_t up_progmem_pagesize(size_t page);
+size_t up_progmem_writegranularity(void);
/****************************************************************************
* Name: up_progmem_erasesize
*
* Description:
- * Return erase block size. Must be a multiple of the read/write page size.
+ * Return the erase size for a specified index. Will always be a multiple of
+ * the write granularity.
+ *
+ * Input Parameters:
+ * index - ordinal number of the set of erasable units of memory
+ *
+ * Returned Value:
+ * Erase size for erasable unit of memory, SIZE_MAX if index is invalid.
*
****************************************************************************/
-size_t up_progmem_erasesize(size_t block);
+size_t up_progmem_erasesize(size_t index);
/****************************************************************************
- * Name: up_progmem_getpage
+ * Name: up_progmem_getindex
*
* Description:
- * Address to read/write page conversion
+ * Get the index of memory, given a FLASH memory address
*
* Input Parameters:
* addr - Address with or without flash offset
- * (absolute or aligned to page0)
+ * (absolute or aligned to sector0)
*
* Returned Value:
- * Page or negative value on error.
+ * Index or negative value on error.
* The following errors are reported (errno is not set!):
*
* -EFAULT: On invalid address
*
****************************************************************************/
-ssize_t up_progmem_getpage(size_t addr);
+ssize_t up_progmem_getindex(size_t addr);
/****************************************************************************
* Name: up_progmem_getaddress
*
* Description:
- * Read/write page to address conversion
+ * Get the base address of a unit of memory, given an index
*
* Input Parameters:
- * page - page index
+ * index - ordinal number of the set of erasable units of memory
*
* Returned Value:
- * Base address of given page, SIZE_MAX if page index is not valid.
+ * Base address of given section, SIZE_MAX if index is not valid.
*
****************************************************************************/
-size_t up_progmem_getaddress(size_t page);
+size_t up_progmem_getaddress(size_t index);
/****************************************************************************
- * Name: up_progmem_eraseblock
+ * Name: up_progmem_erase
*
* Description:
- * Erase selected erase block.
+ * Erase a specified unit of memory, given an index
*
* Input Parameters:
- * block - The erase block index to be erased.
+ * index - ordinal number of the set of erasable units of memory
*
* Returned Value:
- * block size or negative value on error.
+ * section size or negative value on error.
Review comment:
```suggestion
* The unit of memory's size or negative value on error.
```
return type needs to be ssize_t if negative return or use 0
##########
File path: include/nuttx/progmem.h
##########
@@ -234,6 +261,67 @@ ssize_t up_progmem_write(size_t addr, FAR const void *buf, size_t count);
ssize_t up_progmem_read(size_t addr, FAR void *buf, size_t count);
#endif
+/****************************************************************************
+ * Name: up_progmem_enableprogramming
+ *
+ * Description:
+ * Enable the ability to erase/write the FLASH memory.
+ *
Review comment:
Add: Typically done by "Unlocking" the FLASH programming IP Block
##########
File path: include/nuttx/progmem.h
##########
@@ -147,48 +170,50 @@ size_t up_progmem_getaddress(size_t page);
*
****************************************************************************/
-ssize_t up_progmem_eraseblock(size_t block);
+ssize_t up_progmem_erase(size_t index);
/****************************************************************************
- * Name: up_progmem_ispageerased
+ * Name: up_progmem_issectionerased
*
* Description:
- * Checks whether erase page is erased
+ * Checks whether a specified unit of memory is erased
*
* Input Parameters:
- * page - The page index to be checked.
+ * index - ordinal number of the set of erasable units of memory
*
* Returned Value:
* Returns number of bytes NOT erased or negative value on error. If it
- * returns zero then complete page is erased.
+ * returns zero then complete section is erased.
*
* The following errors are reported:
- * -EFAULT: On invalid page
+ * -EFAULT: On invalid index
*
****************************************************************************/
-ssize_t up_progmem_ispageerased(size_t page);
+ssize_t up_progmem_iserased(size_t index);
/****************************************************************************
* Name: up_progmem_write
*
* Description:
* Program data at given address
*
- * Note: this function is not limited to single page and nor it requires
- * the address be aligned inside the page boundaries.
+ * Note: This function may cross memory boundaries so long as the caller
+ * has ensured all units of memory that will be programmed have been
+ * erased. It also does not require that the write be aligned with the
+ * beginning of a unit of memory.
*
* Input Parameters:
* addr - Address with or without flash offset
- * (absolute or aligned to page0)
+ * (absolute or aligned to sector0)
Review comment:
```suggestion
* (absolute or aligned to the zeroth unit of memory)
```
##########
File path: include/nuttx/progmem.h
##########
@@ -234,6 +261,67 @@ ssize_t up_progmem_write(size_t addr, FAR const void *buf, size_t count);
ssize_t up_progmem_read(size_t addr, FAR void *buf, size_t count);
#endif
+/****************************************************************************
+ * Name: up_progmem_enableprogramming
+ *
+ * Description:
+ * Enable the ability to erase/write the FLASH memory.
+ *
+ * NOTE: Write-protection may still be required for an individual
+ * unit of memory.
+ *
+ * Returned Value:
+ * 0 on success, or negative value on error.
+ *
+ ****************************************************************************/
+
+int up_progmem_enableprogramming(void);
+
+/****************************************************************************
+ * Name: up_progmem_disableprogramming
+ *
+ * Description:
+ * Disable the ability to erase/write the FLASH memory.
+ *
Review comment:
"
##########
File path: include/nuttx/progmem.h
##########
@@ -49,96 +49,119 @@ extern "C"
* Public Functions Definitions
****************************************************************************/
+/* NOTE: Since vendors use the terms page, block, and sector interchangably,
+ * it causes confusion and therefore a neutral term is used. We use the term
+ * "index" to refer to the ordinal number of erasable units of memory.
+ */
+
/****************************************************************************
- * Name: up_progmem_neraseblocks
+ * Name: up_progmem_maxeraseindex
*
* Description:
- * Return number of erase blocks
+ * Return the total number of erasable units of memory.
*
****************************************************************************/
-size_t up_progmem_neraseblocks(void);
+size_t up_progmem_maxeraseindex(void);
/****************************************************************************
* Name: up_progmem_isuniform
*
* Description:
- * Is program memory uniform or erase page and read/write page size differs?
+ * Are all erasable units of memory the same size?
+ *
+ * Returned Value:
+ * true if all erasable units are the same size, false if they vary in size
*
****************************************************************************/
bool up_progmem_isuniform(void);
/****************************************************************************
- * Name: up_progmem_pagesize
+ * Name: up_progmem_writegranularity
*
* Description:
- * Return read/write page size
+ * Return the number of bytes of the smallest allowable write operation.
+ * All writes MUST be a multiple of the granularity. Some FLASH memories
+ * require write operations in multiples of 8-bit, 16-bit, 32-bit, 128-bit,
+ * 256-bit, etc. The caller must ensure the size of the data passed into
+ * up_progmem_write is an even multiple of the writegranularity. Padding
+ * may be required.
+ *
+ * Returned Value:
+ * Required granularity of a write operation for the FLASH memory.
*
****************************************************************************/
-size_t up_progmem_pagesize(size_t page);
+size_t up_progmem_writegranularity(void);
/****************************************************************************
* Name: up_progmem_erasesize
*
* Description:
- * Return erase block size. Must be a multiple of the read/write page size.
+ * Return the erase size for a specified index. Will always be a multiple of
+ * the write granularity.
+ *
+ * Input Parameters:
+ * index - ordinal number of the set of erasable units of memory
+ *
+ * Returned Value:
+ * Erase size for erasable unit of memory, SIZE_MAX if index is invalid.
*
****************************************************************************/
-size_t up_progmem_erasesize(size_t block);
+size_t up_progmem_erasesize(size_t index);
/****************************************************************************
- * Name: up_progmem_getpage
+ * Name: up_progmem_getindex
*
* Description:
- * Address to read/write page conversion
+ * Get the index of memory, given a FLASH memory address
*
* Input Parameters:
* addr - Address with or without flash offset
- * (absolute or aligned to page0)
+ * (absolute or aligned to sector0)
*
* Returned Value:
- * Page or negative value on error.
+ * Index or negative value on error.
* The following errors are reported (errno is not set!):
*
* -EFAULT: On invalid address
*
****************************************************************************/
-ssize_t up_progmem_getpage(size_t addr);
+ssize_t up_progmem_getindex(size_t addr);
/****************************************************************************
* Name: up_progmem_getaddress
*
* Description:
- * Read/write page to address conversion
+ * Get the base address of a unit of memory, given an index
*
* Input Parameters:
- * page - page index
+ * index - ordinal number of the set of erasable units of memory
*
* Returned Value:
- * Base address of given page, SIZE_MAX if page index is not valid.
+ * Base address of given section, SIZE_MAX if index is not valid.
*
****************************************************************************/
-size_t up_progmem_getaddress(size_t page);
+size_t up_progmem_getaddress(size_t index);
/****************************************************************************
- * Name: up_progmem_eraseblock
+ * Name: up_progmem_erase
*
* Description:
- * Erase selected erase block.
+ * Erase a specified unit of memory, given an index
*
* Input Parameters:
- * block - The erase block index to be erased.
+ * index - ordinal number of the set of erasable units of memory
*
* Returned Value:
- * block size or negative value on error.
+ * section size or negative value on error.
* The following errors are reported (errno is not set!):
*
- * -EFAULT: On invalid page
+ * -EFAULT: On invalid section
Review comment:
```suggestion
* -EFAULT: On invalid index
```
##########
File path: include/nuttx/progmem.h
##########
@@ -147,48 +170,50 @@ size_t up_progmem_getaddress(size_t page);
*
****************************************************************************/
-ssize_t up_progmem_eraseblock(size_t block);
+ssize_t up_progmem_erase(size_t index);
/****************************************************************************
- * Name: up_progmem_ispageerased
+ * Name: up_progmem_issectionerased
*
* Description:
- * Checks whether erase page is erased
+ * Checks whether a specified unit of memory is erased
*
* Input Parameters:
- * page - The page index to be checked.
+ * index - ordinal number of the set of erasable units of memory
*
* Returned Value:
* Returns number of bytes NOT erased or negative value on error. If it
- * returns zero then complete page is erased.
+ * returns zero then complete section is erased.
*
* The following errors are reported:
- * -EFAULT: On invalid page
+ * -EFAULT: On invalid index
*
****************************************************************************/
-ssize_t up_progmem_ispageerased(size_t page);
+ssize_t up_progmem_iserased(size_t index);
/****************************************************************************
* Name: up_progmem_write
*
* Description:
* Program data at given address
*
- * Note: this function is not limited to single page and nor it requires
- * the address be aligned inside the page boundaries.
+ * Note: This function may cross memory boundaries so long as the caller
+ * has ensured all units of memory that will be programmed have been
+ * erased. It also does not require that the write be aligned with the
+ * beginning of a unit of memory.
*
* Input Parameters:
* addr - Address with or without flash offset
- * (absolute or aligned to page0)
+ * (absolute or aligned to sector0)
Review comment:
I find this confusing.
If it is an address the caller has to know the base address of the memory, so the arch data has to be exposed to the caller. Would could add a function that returns the the base(s) as any counted array.
If it is BASE address less it is an offset.
##########
File path: include/nuttx/progmem.h
##########
@@ -49,96 +49,119 @@ extern "C"
* Public Functions Definitions
****************************************************************************/
+/* NOTE: Since vendors use the terms page, block, and sector interchangably,
+ * it causes confusion and therefore a neutral term is used. We use the term
+ * "index" to refer to the ordinal number of erasable units of memory.
+ */
+
/****************************************************************************
- * Name: up_progmem_neraseblocks
+ * Name: up_progmem_maxeraseindex
*
* Description:
- * Return number of erase blocks
+ * Return the total number of erasable units of memory.
Review comment:
yep
##########
File path: include/nuttx/progmem.h
##########
@@ -147,48 +170,50 @@ size_t up_progmem_getaddress(size_t page);
*
****************************************************************************/
-ssize_t up_progmem_eraseblock(size_t block);
+ssize_t up_progmem_erase(size_t index);
/****************************************************************************
- * Name: up_progmem_ispageerased
+ * Name: up_progmem_issectionerased
*
* Description:
- * Checks whether erase page is erased
+ * Checks whether a specified unit of memory is erased
*
* Input Parameters:
- * page - The page index to be checked.
+ * index - ordinal number of the set of erasable units of memory
*
* Returned Value:
* Returns number of bytes NOT erased or negative value on error. If it
- * returns zero then complete page is erased.
+ * returns zero then complete section is erased.
*
* The following errors are reported:
- * -EFAULT: On invalid page
+ * -EFAULT: On invalid index
*
****************************************************************************/
-ssize_t up_progmem_ispageerased(size_t page);
+ssize_t up_progmem_iserased(size_t index);
/****************************************************************************
* Name: up_progmem_write
*
* Description:
* Program data at given address
*
- * Note: this function is not limited to single page and nor it requires
- * the address be aligned inside the page boundaries.
+ * Note: This function may cross memory boundaries so long as the caller
+ * has ensured all units of memory that will be programmed have been
+ * erased. It also does not require that the write be aligned with the
+ * beginning of a unit of memory.
*
* Input Parameters:
* addr - Address with or without flash offset
- * (absolute or aligned to page0)
+ * (absolute or aligned to sector0)
Review comment:
IIRC the H7 has 2 banks of FLASH. How would the user get the second one or know it is the second own without including arch specific code. Maybe we should talk this one through.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org