Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

QSPI : Define default pins at drivers level #11444

Merged
merged 5 commits into from
Oct 21, 2019
Merged

QSPI : Define default pins at drivers level #11444

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
bec463d
QSPI : Define default pins at drivers level
jeromecoutant Sep 9, 2019
03837a7
QSPIF component should use defined QSPI pins
jeromecoutant Sep 10, 2019
5c1c558
QSPI test should use json defined QSPI pins
jeromecoutant Sep 10, 2019
f117d35
QSPIF frequency setting depends on QSPI memory not target.
jeromecoutant Oct 15, 2019
23e6840
QSPI HAL test: flash configuration file selection update
jeromecoutant Oct 15, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 12 additions & 20 deletions TESTS/mbed_hal/qspi/flash_configs/flash_configs.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,23 +17,8 @@
#ifndef MBED_FLASH_CONFIGS_H
#define MBED_FLASH_CONFIGS_H

#if defined(TARGET_DISCO_L475VG_IOT01A)
#include "MX25RXX35F_config.h" // MX25R6435F

#elif defined(TARGET_DISCO_F413ZH)
#include "N25Q128A_config.h" // N25Q128A13EF840F

#elif defined(TARGET_DISCO_F746NG)
#include "N25Q128A_config.h" // N25Q128A13EF840E

#elif defined(TARGET_DISCO_F469NI)
#include "N25Q128A_config.h" // N25Q128A13EF840E

#elif defined(TARGET_DISCO_F769NI)
#include "MX25L51245G_config.h" // MX25L51245G

#elif defined(TARGET_DISCO_L4R9I)
#include "MX25LM51245G_config.h" // MX25LM51245G
#if defined(TARGET_MX25R6435F)
#include "MX25RXX35F_config.h"

#elif defined(TARGET_DISCO_L476VG)
#include "N25Q128A_config.h" // N25Q128A13EF840E
Expand All @@ -43,6 +28,15 @@
#undef QSPI_CMD_WRITE_DPI
#undef QSPI_CMD_WRITE_QPI

#elif defined(TARGET_N25Q128A)
#include "N25Q128A_config.h"

#elif defined(TARGET_MX25L51245G)
#include "MX25L51245G_config.h"

#elif defined(TARGET_MX25LM51245G)
#include "MX25LM51245G_config.h"

#elif defined(TARGET_RHOMBIO_L476DMW1K)
#include "MT25Q_config.h" // MT25QL128ABA1EW7
/* See STM32L476 Errata Sheet, it is not possible to use Dual-/Quad-mode for the command phase */
Expand All @@ -51,9 +45,6 @@
#undef QSPI_CMD_WRITE_DPI
#undef QSPI_CMD_WRITE_QPI

#elif defined(TARGET_DISCO_L496AG)
#include "MX25RXX35F_config.h" // MX25R6435F

#elif defined(TARGET_NRF52840)
#include "NORDIC/NRF52840_DK/flash_config.h"

Expand Down Expand Up @@ -88,5 +79,6 @@
#include "S25FL128S_config.h"

#endif

#endif // MBED_FLASH_CONFIGS_H

12 changes: 6 additions & 6 deletions TESTS/mbed_hal/qspi/main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,12 +70,12 @@ uint8_t rx_buf[DATA_SIZE_1024];


// some target defines QSPI pins as integers thus conversion needed
#define QPIN_0 static_cast<PinName>(QSPI_FLASH1_IO0)
#define QPIN_1 static_cast<PinName>(QSPI_FLASH1_IO1)
#define QPIN_2 static_cast<PinName>(QSPI_FLASH1_IO2)
#define QPIN_3 static_cast<PinName>(QSPI_FLASH1_IO3)
#define QSCK static_cast<PinName>(QSPI_FLASH1_SCK)
#define QCSN static_cast<PinName>(QSPI_FLASH1_CSN)
#define QPIN_0 static_cast<PinName>(MBED_CONF_DRIVERS_QSPI_IO0)
#define QPIN_1 static_cast<PinName>(MBED_CONF_DRIVERS_QSPI_IO1)
#define QPIN_2 static_cast<PinName>(MBED_CONF_DRIVERS_QSPI_IO2)
#define QPIN_3 static_cast<PinName>(MBED_CONF_DRIVERS_QSPI_IO3)
#define QSCK static_cast<PinName>(MBED_CONF_DRIVERS_QSPI_SCK)
#define QCSN static_cast<PinName>(MBED_CONF_DRIVERS_QSPI_CSN)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was designed that tests-mbed_hal-qspi uses default qspi interface available on dev boards.
This should look rather like below to maintain hal test working:

#ifdef MBED_CONF_DRIVERS_QSPI_IO0
#define QPIN_0 static_cast<PinName>(MBED_CONF_DRIVERS_QSPI_IO0)
#else 
#define QPIN_0 static_cast<PinName>(QSPI_FLASH1_IO0)
#endif

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see the difference as MBED_CONF_DRIVERS_QSPI_IO0 is QSPI_FLASH1_IO0...



static uint32_t gen_flash_address()
Expand Down
30 changes: 9 additions & 21 deletions components/storage/blockdevice/COMPONENT_QSPIF/mbed_lib.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,39 +1,27 @@
{
"name": "qspif",
"config": {
"QSPI_IO0": "QSPI_FLASH1_IO0",
"QSPI_IO1": "QSPI_FLASH1_IO1",
"QSPI_IO2": "QSPI_FLASH1_IO2",
"QSPI_IO3": "QSPI_FLASH1_IO3",
"QSPI_SCK": "QSPI_FLASH1_SCK",
"QSPI_CSN": "QSPI_FLASH1_CSN",
"QSPI_IO0": "MBED_CONF_DRIVERS_QSPI_IO0",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jeromecoutant what I'm saying is these new configs like MBED_CONF_DRIVERS_QSPI_IO0 are not defined by default as far as i can tell, while the old ones like QSPI_FLASH1_IO0 were defined as part of pinmap. Am I missing something?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MBED_CONF_DRIVERS_QSPI_IO0 is set by default to QSPI_FLASH1_IO0,
so no impact for QSPIF

Difference, now, is that when user adds some external QSPI memory, he only has to update his mbed_app.json file. Then he could execute QSPIF AND QSPI tests.

Thx

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks it work
mbed_config.h:

// Automatically generated configuration file.
// DO NOT EDIT, content will be overwritten.

#ifndef __MBED_CONFIG_DATA__
#define __MBED_CONFIG_DATA__

// Configuration parameters
...
#define MBED_CONF_DRIVERS_QSPI_CSN QSPI_FLASH1_CSN // set by library:drivers
#define MBED_CONF_DRIVERS_QSPI_IO0 QSPI_FLASH1_IO0 // set by library:drivers
#define MBED_CONF_DRIVERS_QSPI_IO1 QSPI_FLASH1_IO1 // set by library:drivers
#define MBED_CONF_DRIVERS_QSPI_IO2 QSPI_FLASH1_IO2 // set by library:drivers
#define MBED_CONF_DRIVERS_QSPI_IO3 QSPI_FLASH1_IO3 // set by library:drivers
#define MBED_CONF_DRIVERS_QSPI_SCK QSPI_FLASH1_SCK // set by library:drivers
...

"QSPI_IO1": "MBED_CONF_DRIVERS_QSPI_IO1",
"QSPI_IO2": "MBED_CONF_DRIVERS_QSPI_IO2",
"QSPI_IO3": "MBED_CONF_DRIVERS_QSPI_IO3",
"QSPI_SCK": "MBED_CONF_DRIVERS_QSPI_SCK",
"QSPI_CSN": "MBED_CONF_DRIVERS_QSPI_CSN",
"QSPI_POLARITY_MODE": 0,
"QSPI_FREQ": "40000000",
"QSPI_MIN_READ_SIZE": "1",
"QSPI_MIN_PROG_SIZE": "1"
},
"target_overrides": {
"DISCO_F413ZH": {
"QSPI_FREQ": "80000000"
},
"DISCO_L475VG_IOT01A": {
"MX25R6435F": {
"QSPI_FREQ": "8000000"
},
"DISCO_L476VG": {
"QSPI_FREQ": "80000000"
},
"DISCO_L496AG": {
"MX25L51245G": {
"QSPI_FREQ": "8000000"
},
"DISCO_F469NI": {
"N25Q128A": {
"QSPI_FREQ": "80000000"
},
"DISCO_F746NG": {
"QSPI_FREQ": "80000000"
},
"DISCO_F769NI": {
"QSPI_FREQ": "8000000"
},
"MCU_NRF52840": {
"QSPI_FREQ": "32000000",
"QSPI_MIN_READ_SIZE": "4",
Expand Down
24 changes: 24 additions & 0 deletions drivers/mbed_lib.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,30 @@
"spi_count_max": {
"help": "The maximum number of SPI peripherals used at the same time. Determines RAM allocated for SPI peripheral management. If null, limit determined by hardware.",
"value": null
},
"qspi_io0": {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have QSPI_FLASH1_IO0 and now introducing QSPI_FLASH1_IO0 ?

Why are not these in PinNames and defined for all boards having QSPI? There are QSPI pins or not? They should provide default pins, that could be overriden in tests/apps via config. Is this what is it adding?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Goal is to use an external QSPI,
and add pins value in mbed_app.json file

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand now. This is a new approach here in this drivers config. This brings QSPI definitions even for a board that does not have it. Shouldn' t this be by default null, not even created if not used , and a target would override it and set?

@maciejbocianski what do you think?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Goal of this approach is to be full compatible with current implementation.
There is no impact with current targets with and without QSPI pins

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only impact having QSPI defined always even if QSPI is not available?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

4 new define added and not used in the mbed_config.h file

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Help text is missing.

"help": "QSPI data I/O 0 pin",
"value": "QSPI_FLASH1_IO0"
},
"qspi_io1": {
"help": "QSPI data I/O 1 pin",
"value": "QSPI_FLASH1_IO1"
},
"qspi_io2": {
"help": "QSPI data I/O 2 pin",
"value": "QSPI_FLASH1_IO2"
},
"qspi_io3": {
"help": "QSPI data I/O 3 pin",
"value": "QSPI_FLASH1_IO3"
},
"qspi_sck": {
"help": "QSPI clock pin",
"value": "QSPI_FLASH1_SCK"
},
"qspi_csn": {
"help": "QSPI chip select pin",
"value": "QSPI_FLASH1_CSN"
}
}
}
7 changes: 7 additions & 0 deletions targets/targets.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2745,6 +2745,7 @@
"supported_form_factors": ["ARDUINO"],
"core": "Cortex-M4F",
"extra_labels_add": [
"N25Q128A",
"STM32F4",
"STM32F413xx",
"STM32F413ZH",
Expand Down Expand Up @@ -4072,6 +4073,7 @@
"supported_form_factors": ["ARDUINO"],
"core": "Cortex-M4F",
"extra_labels_add": [
"N25Q128A",
"STM32F4",
"STM32F469",
"STM32F469NI",
Expand Down Expand Up @@ -4233,6 +4235,7 @@
"inherits": ["FAMILY_STM32"],
"core": "Cortex-M7F",
"extra_labels_add": [
"N25Q128A",
"STM32F7",
"STM32F746",
"STM32F746xG",
Expand Down Expand Up @@ -4287,6 +4290,7 @@
"inherits": ["FAMILY_STM32"],
"core": "Cortex-M7FD",
"extra_labels_add": [
"MX25L51245G",
"STM32F7",
"STM32F769",
"STM32F769xI",
Expand Down Expand Up @@ -4345,6 +4349,7 @@
"inherits": ["FAMILY_STM32"],
"core": "Cortex-M4F",
"extra_labels_add": [
"MX25R6435F",
"STM32L4",
"STM32L475xG",
"STM32L475VG"
Expand Down Expand Up @@ -4418,6 +4423,7 @@
"inherits": ["FAMILY_STM32"],
"core": "Cortex-M4F",
"extra_labels_add": [
"N25Q128A",
"STM32L4",
"STM32L476xG",
"STM32L476VG"
Expand Down Expand Up @@ -8248,6 +8254,7 @@
"supported_form_factors": ["ARDUINO", "STMOD", "PMOD"],
"core": "Cortex-M4F",
"extra_labels_add": [
"MX25R6435F",
"STM32L4",
"STM32L496AG",
"STM32L496xG"
Expand Down