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/07/16 15:03:32 UTC
[GitHub] [incubator-nuttx] gustavonihei opened a new pull request #4166: mtd: Add MTDIOC_ERASESTATE command for retrieving erase state value
gustavonihei opened a new pull request #4166:
URL: https://github.com/apache/incubator-nuttx/pull/4166
## Summary
This PR intends to add a new `ioctl` command to the MTD subsystem named `MTDIOC_ERASESTATE` for retrieving the erase state value of the underlying MTD.
## Impact
New `ioctl` command to the MTD API.
## Testing
Tested using `esp32-wrover-kit`, which will be submitted in a later PR.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #4166: mtd: Add MTDIOC_ERASESTATE command for retrieving erase state value
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #4166:
URL: https://github.com/apache/incubator-nuttx/pull/4166#discussion_r671688254
##########
File path: arch/xtensa/src/esp32/esp32_spiflash.c
##########
@@ -1933,6 +1934,15 @@ static int esp32_ioctl(FAR struct mtd_dev_s *dev, int cmd,
}
break;
+ case MTDIOC_ERASESTATE:
+ {
+ FAR uint8_t *result = (uint8_t *)arg;
Review comment:
The cast need add FAR too:)
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] gustavonihei commented on a change in pull request #4166: mtd: Add MTDIOC_ERASESTATE command for retrieving erase state value
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #4166:
URL: https://github.com/apache/incubator-nuttx/pull/4166#discussion_r671699595
##########
File path: arch/xtensa/src/esp32/esp32_spiflash.c
##########
@@ -1933,6 +1934,15 @@ static int esp32_ioctl(FAR struct mtd_dev_s *dev, int cmd,
}
break;
+ case MTDIOC_ERASESTATE:
+ {
+ FAR uint8_t *result = (uint8_t *)arg;
Review comment:
That's the price for doing things in a hurry hehehe
Thanks, now it is properly fixed.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #4166: mtd: Add MTDIOC_ERASESTATE command for retrieving erase state value
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #4166:
URL: https://github.com/apache/incubator-nuttx/pull/4166#discussion_r671331362
##########
File path: include/nuttx/mtd/mtd.h
##########
@@ -71,6 +71,9 @@
#define MTDIOC_FLUSH _MTDIOC(0x0009) /* IN: None
* OUT: None (ioctl return value provides
* success/failure indication). */
+#define MTDIOC_ERASESTATE _MTDIOC(0x000a) /* IN: Pointer to uint8_t
+ * OUT: Byte value of the FLASH erased
Review comment:
should we standardize the returned value meaning here?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] gustavonihei commented on a change in pull request #4166: mtd: Add MTDIOC_ERASESTATE command for retrieving erase state value
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #4166:
URL: https://github.com/apache/incubator-nuttx/pull/4166#discussion_r671418517
##########
File path: include/nuttx/mtd/mtd.h
##########
@@ -71,6 +71,9 @@
#define MTDIOC_FLUSH _MTDIOC(0x0009) /* IN: None
* OUT: None (ioctl return value provides
* success/failure indication). */
+#define MTDIOC_ERASESTATE _MTDIOC(0x000a) /* IN: Pointer to uint8_t
+ * OUT: Byte value of the FLASH erased
Review comment:
I tried to explain it on the OUT description to follow the pattern from this file.
Shall I add more information?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] gustavonihei commented on a change in pull request #4166: mtd: Add MTDIOC_ERASESTATE command for retrieving erase state value
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #4166:
URL: https://github.com/apache/incubator-nuttx/pull/4166#discussion_r671687312
##########
File path: drivers/mtd/filemtd.c
##########
@@ -449,6 +449,15 @@ static int filemtd_ioctl(FAR struct mtd_dev_s *dev, int cmd,
}
break;
+ case MTDIOC_ERASESTATE:
+ {
+ uint8_t *result = (uint8_t *)arg;
Review comment:
You're right, I missed that. I've also added to chip-specific drivers, since they all follow the pattern of adding FAR to every pointer to data.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] gustavonihei commented on a change in pull request #4166: mtd: Add MTDIOC_ERASESTATE command for retrieving erase state value
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #4166:
URL: https://github.com/apache/incubator-nuttx/pull/4166#discussion_r671418517
##########
File path: include/nuttx/mtd/mtd.h
##########
@@ -71,6 +71,9 @@
#define MTDIOC_FLUSH _MTDIOC(0x0009) /* IN: None
* OUT: None (ioctl return value provides
* success/failure indication). */
+#define MTDIOC_ERASESTATE _MTDIOC(0x000a) /* IN: Pointer to uint8_t
+ * OUT: Byte value of the FLASH erased
Review comment:
I tried to explain it on the OUT description to follow the pattern from this file.
Which further info you think should be included here?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #4166: mtd: Add MTDIOC_ERASESTATE command for retrieving erase state value
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #4166:
URL: https://github.com/apache/incubator-nuttx/pull/4166#discussion_r671613676
##########
File path: include/nuttx/mtd/mtd.h
##########
@@ -71,6 +71,9 @@
#define MTDIOC_FLUSH _MTDIOC(0x0009) /* IN: None
* OUT: None (ioctl return value provides
* success/failure indication). */
+#define MTDIOC_ERASESTATE _MTDIOC(0x000a) /* IN: Pointer to uint8_t
+ * OUT: Byte value of the FLASH erased
Review comment:
Ok, I misunderstand the meaning of ERASESTATE. ERASESTATE doesn't represent a real state with the limited flags, but the storage value after the flash erase.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 merged pull request #4166: mtd: Add MTDIOC_ERASESTATE command for retrieving erase state value
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged pull request #4166:
URL: https://github.com/apache/incubator-nuttx/pull/4166
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #4166: mtd: Add MTDIOC_ERASESTATE command for retrieving erase state value
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #4166:
URL: https://github.com/apache/incubator-nuttx/pull/4166#discussion_r671331362
##########
File path: include/nuttx/mtd/mtd.h
##########
@@ -71,6 +71,9 @@
#define MTDIOC_FLUSH _MTDIOC(0x0009) /* IN: None
* OUT: None (ioctl return value provides
* success/failure indication). */
+#define MTDIOC_ERASESTATE _MTDIOC(0x000a) /* IN: Pointer to uint8_t
+ * OUT: Byte value of the FLASH erased
Review comment:
should we specify the value meaning here?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #4166: mtd: Add MTDIOC_ERASESTATE command for retrieving erase state value
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #4166:
URL: https://github.com/apache/incubator-nuttx/pull/4166#discussion_r671614463
##########
File path: drivers/mtd/filemtd.c
##########
@@ -449,6 +449,15 @@ static int filemtd_ioctl(FAR struct mtd_dev_s *dev, int cmd,
}
break;
+ case MTDIOC_ERASESTATE:
+ {
+ uint8_t *result = (uint8_t *)arg;
Review comment:
should we add FAR for all pointer at least in the common code?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] gustavonihei commented on a change in pull request #4166: mtd: Add MTDIOC_ERASESTATE command for retrieving erase state value
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #4166:
URL: https://github.com/apache/incubator-nuttx/pull/4166#discussion_r671687312
##########
File path: drivers/mtd/filemtd.c
##########
@@ -449,6 +449,15 @@ static int filemtd_ioctl(FAR struct mtd_dev_s *dev, int cmd,
}
break;
+ case MTDIOC_ERASESTATE:
+ {
+ uint8_t *result = (uint8_t *)arg;
Review comment:
You're right, I missed that. I've also added to chip-specific drivers, since they all follow the pattern of adding FAR to every pointer to data.
Thanks, it's fixed.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] gustavonihei commented on a change in pull request #4166: mtd: Add MTDIOC_ERASESTATE command for retrieving erase state value
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #4166:
URL: https://github.com/apache/incubator-nuttx/pull/4166#discussion_r671687433
##########
File path: include/nuttx/mtd/mtd.h
##########
@@ -71,6 +71,9 @@
#define MTDIOC_FLUSH _MTDIOC(0x0009) /* IN: None
* OUT: None (ioctl return value provides
* success/failure indication). */
+#define MTDIOC_ERASESTATE _MTDIOC(0x000a) /* IN: Pointer to uint8_t
+ * OUT: Byte value of the FLASH erased
Review comment:
I think your confusion is genuine. I've rephrased the documentation to clarify it. Please, take another 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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org