You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mynewt.apache.org by GitBox <gi...@apache.org> on 2022/05/02 22:30:25 UTC
[GitHub] [mynewt-core] agross-korg opened a new pull request, #2827: da1469x: hal_flash: Autodetect and configure flash
agross-korg opened a new pull request, #2827:
URL: https://github.com/apache/mynewt-core/pull/2827
This patch adds in auto-detection and configuration of Gigadevice and Macronix
flash devices based on the manufacturer and device ID. The intent in this is to
allow for a single binary to work on devices with different flash.
Normally, the flash is initialized by the ROM bootloader on the device. However, if the image being loaded is RAM_RESIDENT, that header will not exist and the ROM bootloader will not provision the device to have a working flash configuration.
Signed-off-by: Andy Gross <an...@juul.com>
--
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@mynewt.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-core] utzig commented on pull request #2827: da1469x: hal_flash: Autodetect and configure flash
Posted by GitBox <gi...@apache.org>.
utzig commented on PR #2827:
URL: https://github.com/apache/mynewt-core/pull/2827#issuecomment-1129636994
<!-- style-bot -->
## Style check summary
#### No suggestions at this time!
--
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@mynewt.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-core] utzig commented on pull request #2827: da1469x: hal_flash: Autodetect and configure flash
Posted by GitBox <gi...@apache.org>.
utzig commented on PR #2827:
URL: https://github.com/apache/mynewt-core/pull/2827#issuecomment-1132348475
<!-- style-bot -->
## Style check summary
#### No suggestions at this time!
--
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@mynewt.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-core] utzig commented on pull request #2827: da1469x: hal_flash: Autodetect and configure flash
Posted by GitBox <gi...@apache.org>.
utzig commented on PR #2827:
URL: https://github.com/apache/mynewt-core/pull/2827#issuecomment-1132215006
<!-- style-bot -->
## Style check summary
### Our coding style is [here!](https://github.com/apache/mynewt-core/blob/master/CODING_STANDARDS.md)
#### hw/bsp/dialog_da14695-dk-usb/src/hal_bsp.c
<details>
```diff
@@ -50,8 +50,7 @@
{ .id = 0xc22536, /* macronix */
.cmda = 0xa8a500eb,
.cmdb = 0x66,
- .qe = {1, {0x40}},
- },
+ .qe = {1, {0x40}},},
};
const int qspi_flash_config_array_size = ARRAY_SIZE(rdids);
```
</details>
#### hw/bsp/dialog_da1469x-dk-pro/src/hal_bsp.c
<details>
```diff
@@ -51,8 +51,7 @@
{ .id = 0xc22536, /* macronix */
.cmda = 0xa8a500eb,
.cmdb = 0x66,
- .qe = {1, {0x40}},
- },
+ .qe = {1, {0x40}},},
};
const int qspi_flash_config_array_size = ARRAY_SIZE(rdids);
```
</details>
--
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@mynewt.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-core] agross-korg commented on a diff in pull request #2827: da1469x: hal_flash: Autodetect and configure flash
Posted by GitBox <gi...@apache.org>.
agross-korg commented on code in PR #2827:
URL: https://github.com/apache/mynewt-core/pull/2827#discussion_r876167183
##########
hw/bsp/dialog_da1469x-dk-pro/src/hal_bsp.c:
##########
@@ -41,6 +41,15 @@ static const struct hal_bsp_mem_dump dump_cfg[] = {
*/
static char hw_id[] = "DA1469X_HW_ID";
+
+/*
+ * Configure empty RDIDs for use by flash init.
+ */
+#if MYNEWT_VAL(RAM_RESIDENT)
+const struct qspi_flash_config rdids[] = 0;
+const int qspi_flash_config_array_size = ARRAY_SIZE(rdids);
Review Comment:
Either works, but I'll update that to {}.
##########
hw/mcu/dialog/da1469x/src/hal_flash.c:
##########
@@ -24,6 +24,7 @@
#include "mcu/da1469x_hal.h"
#include "hal/hal_flash_int.h"
#include "mcu/mcu.h"
+#include <os/util.h>
Review Comment:
Done. Will push refresh.
--
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@mynewt.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-core] agross-korg commented on a diff in pull request #2827: da1469x: hal_flash: Autodetect and configure flash
Posted by GitBox <gi...@apache.org>.
agross-korg commented on code in PR #2827:
URL: https://github.com/apache/mynewt-core/pull/2827#discussion_r875501198
##########
hw/mcu/dialog/da1469x/src/hal_flash.c:
##########
@@ -395,35 +396,121 @@ da1469x_hff_sector_info(const struct hal_flash *dev, int idx,
return 0;
}
-#if MYNEWT_VAL(MCU_QSPIC_APP_CFG)
+#if MYNEWT_VAL(RAM_RESIDENT)
+struct qspi_qe_config {
+ uint8_t length;
+ uint8_t bytes[2];
+};
+
+struct qspi_flash_config {
+ uint32_t id;
+ uint32_t cmda;
+ uint32_t cmdb;
+ struct qspi_qe_config qe;
+};
+
+static const struct qspi_flash_config rdids[] =
+{
+ { .id = 0xc86015, /* gigaflash */
Review Comment:
Updated commit to implement the suggestions and to fix the gigadevice comment.
--
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@mynewt.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-core] agross-korg commented on a diff in pull request #2827: da1469x: hal_flash: Autodetect and configure flash
Posted by GitBox <gi...@apache.org>.
agross-korg commented on code in PR #2827:
URL: https://github.com/apache/mynewt-core/pull/2827#discussion_r877160346
##########
hw/mcu/dialog/da1469x/src/hal_flash.c:
##########
@@ -24,6 +24,7 @@
#include "mcu/da1469x_hal.h"
#include "hal/hal_flash_int.h"
#include "mcu/mcu.h"
+#include <os/util.h>
Review Comment:
Well that's embarrassing. Moving the rdids from mcu to bsp, I forgot to update the other bsp. We use our own external bsp code. Fixed and pushed up.
--
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@mynewt.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-core] kasjer commented on a diff in pull request #2827: da1469x: hal_flash: Autodetect and configure flash
Posted by GitBox <gi...@apache.org>.
kasjer commented on code in PR #2827:
URL: https://github.com/apache/mynewt-core/pull/2827#discussion_r864843767
##########
hw/mcu/dialog/da1469x/src/hal_flash.c:
##########
@@ -395,35 +396,121 @@ da1469x_hff_sector_info(const struct hal_flash *dev, int idx,
return 0;
}
-#if MYNEWT_VAL(MCU_QSPIC_APP_CFG)
+#if MYNEWT_VAL(RAM_RESIDENT)
+struct qspi_qe_config {
+ uint8_t length;
+ uint8_t bytes[2];
+};
+
+struct qspi_flash_config {
+ uint32_t id;
+ uint32_t cmda;
+ uint32_t cmdb;
+ struct qspi_qe_config qe;
+};
+
+static const struct qspi_flash_config rdids[] =
+{
+ { .id = 0xc86015, /* gigaflash */
Review Comment:
Probably should be gigadevice in comment.
I would also move rdids to bsp as it is very picky about what flash can be detected and I think that BSP is more suitable for this. If someone wanted to have different flash we would need to modify this file again.
Alternatively maybe it is enough to check vendor id (c8, or c2 and maybe Winbond) to determine how to enable quad mode. I guess vendor devices with different sizes still have same bit for enabling quad mode.
--
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@mynewt.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-core] utzig commented on pull request #2827: da1469x: hal_flash: Autodetect and configure flash
Posted by GitBox <gi...@apache.org>.
utzig commented on PR #2827:
URL: https://github.com/apache/mynewt-core/pull/2827#issuecomment-1115596071
<!-- style-bot -->
## Style check summary
### Our coding style is [here!](https://github.com/apache/mynewt-core/blob/master/CODING_STANDARDS.md)
#### hw/mcu/dialog/da1469x/src/hal_flash.c
<details>
```diff
@@ -409,21 +409,19 @@
struct qspi_qe_config qe;
};
-static const struct qspi_flash_config rdids[] =
-{
+static const struct qspi_flash_config rdids[] = {
{ .id = 0xc86015, /* gigaflash */
.cmda = 0xa82000eb,
.cmdb = 0x66,
- .qe = {2, {0x0, 0x2}},
- },
+ .qe = {2, {0x0, 0x2}},},
{ .id = 0xc22535, /* macronix */
.cmda = 0xa8a500eb,
.cmdb = 0x66,
- .qe = {1, {0x40}},
- },
+ .qe = {1, {0x40}},},
};
-static void qspi_qe_enable(const struct hal_flash *dev, const struct qspi_qe_config *config)
+static void
+qspi_qe_enable(const struct hal_flash *dev, const struct qspi_qe_config *config)
{
int i;
@@ -449,7 +447,8 @@
QSPIC->QSPIC_CTRLBUS_REG = QSPIC_QSPIC_CTRLBUS_REG_QSPIC_DIS_CS_Msk;
}
-static const struct qspi_flash_config *qspi_read_rdid(const struct hal_flash *dev)
+static const struct qspi_flash_config *
+qspi_read_rdid(const struct hal_flash *dev)
{
int i;
uint32_t result;
@@ -508,7 +507,7 @@
#endif
static int
- da1469x_hff_init(const struct hal_flash *dev)
+da1469x_hff_init(const struct hal_flash *dev)
{
#if MYNEWT_VAL(RAM_RESIDENT)
da1469x_hff_mcu_custom_init(dev);
```
</details>
--
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@mynewt.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-core] kasjer commented on a diff in pull request #2827: da1469x: hal_flash: Autodetect and configure flash
Posted by GitBox <gi...@apache.org>.
kasjer commented on code in PR #2827:
URL: https://github.com/apache/mynewt-core/pull/2827#discussion_r875527960
##########
hw/mcu/dialog/da1469x/src/hal_flash.c:
##########
@@ -24,6 +24,7 @@
#include "mcu/da1469x_hal.h"
#include "hal/hal_flash_int.h"
#include "mcu/mcu.h"
+#include <os/util.h>
Review Comment:
This probably should be moved to hal_bsp.c
##########
hw/bsp/dialog_da1469x-dk-pro/src/hal_bsp.c:
##########
@@ -41,6 +41,15 @@ static const struct hal_bsp_mem_dump dump_cfg[] = {
*/
static char hw_id[] = "DA1469X_HW_ID";
+
+/*
+ * Configure empty RDIDs for use by flash init.
+ */
+#if MYNEWT_VAL(RAM_RESIDENT)
+const struct qspi_flash_config rdids[] = 0;
+const int qspi_flash_config_array_size = ARRAY_SIZE(rdids);
Review Comment:
```
Error: repos/apache-mynewt-core/hw/bsp/dialog_da1469x-dk-pro/src/hal_bsp.c:49:42: error: invalid initializer
const struct qspi_flash_config rdids[] = 0;
^
repos/apache-mynewt-core/hw/bsp/dialog_da1469x-dk-pro/src/hal_bsp.c:50:42: error: implicit declaration of function 'ARRAY_SIZE'; did you mean 'RAM_SIZE'? [-Werror=implicit-function-declaration]
const int qspi_flash_config_array_size = ARRAY_SIZE(rdids);
^~~~~~~~~~
RAM_SIZE
repos/apache-mynewt-core/hw/bsp/dialog_da1469x-dk-pro/src/hal_bsp.c:50:42: error: initializer element is not constant
```
I guess you meant:
```c
const struct qspi_flash_config rdids[] = {};
```
--
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@mynewt.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-core] agross-korg merged pull request #2827: da1469x: hal_flash: Autodetect and configure flash
Posted by GitBox <gi...@apache.org>.
agross-korg merged PR #2827:
URL: https://github.com/apache/mynewt-core/pull/2827
--
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@mynewt.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-core] kasjer commented on a diff in pull request #2827: da1469x: hal_flash: Autodetect and configure flash
Posted by GitBox <gi...@apache.org>.
kasjer commented on code in PR #2827:
URL: https://github.com/apache/mynewt-core/pull/2827#discussion_r876748519
##########
hw/mcu/dialog/da1469x/src/hal_flash.c:
##########
@@ -24,6 +24,7 @@
#include "mcu/da1469x_hal.h"
#include "hal/hal_flash_int.h"
#include "mcu/mcu.h"
+#include <os/util.h>
Review Comment:
Error: repos/apache-mynewt-core/hw/bsp/dialog_da1469x-dk-pro/src/hal_bsp.c:50:42: error: implicit declaration of function 'ARRAY_SIZE'; did you mean 'RAM_SIZE'? [-Werror=implicit-function-declaration]
const int qspi_flash_config_array_size = ARRAY_SIZE(rdids);
^~~~~~~~~~
RAM_SIZE
repos/apache-mynewt-core/hw/bsp/dialog_da1469x-dk-pro/src/hal_bsp.c:50:42: error: initializer element is not constant
--
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@mynewt.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-core] utzig commented on pull request #2827: da1469x: hal_flash: Autodetect and configure flash
Posted by GitBox <gi...@apache.org>.
utzig commented on PR #2827:
URL: https://github.com/apache/mynewt-core/pull/2827#issuecomment-1130349899
<!-- style-bot -->
## Style check summary
#### No suggestions at this time!
--
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@mynewt.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-core] agross-korg commented on a diff in pull request #2827: da1469x: hal_flash: Autodetect and configure flash
Posted by GitBox <gi...@apache.org>.
agross-korg commented on code in PR #2827:
URL: https://github.com/apache/mynewt-core/pull/2827#discussion_r865125326
##########
hw/mcu/dialog/da1469x/src/hal_flash.c:
##########
@@ -395,35 +396,121 @@ da1469x_hff_sector_info(const struct hal_flash *dev, int idx,
return 0;
}
-#if MYNEWT_VAL(MCU_QSPIC_APP_CFG)
+#if MYNEWT_VAL(RAM_RESIDENT)
+struct qspi_qe_config {
+ uint8_t length;
+ uint8_t bytes[2];
+};
+
+struct qspi_flash_config {
+ uint32_t id;
+ uint32_t cmda;
+ uint32_t cmdb;
+ struct qspi_qe_config qe;
+};
+
+static const struct qspi_flash_config rdids[] =
+{
+ { .id = 0xc86015, /* gigaflash */
Review Comment:
I'll fix that. As for moving this to BSP, that's fine. I am unsure about the possibility of the QE enable stuff changing between products in the same manufacturer. I imagine it could be possible, but it might suffice to just use the vendor id. The alternative is just compiling a list of complete RDIDs (vendor/product), which is what other projects like linux do.
--
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@mynewt.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org