Qspi flash pin names standardization #7639

maciejbocianski · 2018-07-30T09:15:28Z

Description

Normalize QSPI flash pin names to new format:

QSPI_FLASH1_XXX for targets with flash onboard
QSPI1_XXX for targets with no flash onboard (to be discussed)

Some targets support Dual-Flash, hence added port indexing (1, 2) to distinct which port is actually utilized

TODO:

add pin names to targets with no flash onboard
update docs (https://github.com/ARMmbed/mbed-os-5-docs/blob/development/docs/reference/contributing/target/QuadSPI.md)

Pull request type

[ ] Fix
[ ] Refactor
[ ] New target
[X] Feature
[ ] Breaking change

Adding new QSPI HAL header file. This should help to use memory-maped devices as memories, graphical displays. The API consist of few functions, most important are read/write/write_command functions. The command format is: ``` ---------------------------------------------- | Instruction | Address | Alt | Dummy | Data | ---------------------------------------------- ``` We define only synch API at the moment.

Fixing by adding NONE values for both

This provides a way to return how many bytes have been written/read (as status codes are returned via func ret value)

SPI mode means Clock polarity and phase mode (0 - 3)

This commit adds QSPI HAL implementation for nrf52840 MCU targets

…TDIO definitions

if address is skipped, used size NONE

Not used anymore, not defined. ctor default initializes the object

Should be: instr, alt, address or inst, addr or just addr

If phase is being skipped, set disabled to true, otherwise false.

…m_workaround hal-qspi_test: remove STM workaround

Add reset internal state before call to HAL_QspiInit function

maciejbocianski · 2018-07-30T09:18:28Z

FYI @adustm @offirko @donatieng @0xc0170

maciejbocianski · 2018-07-30T09:55:33Z

@donatieng @jamesbeyond @0xc0170
What do you think about having only one naming scheme for QSPI pins (QSPI_FLASH1_XXX), whatever the availability of the on board flash

offirko · 2018-07-30T12:04:39Z

Hi Maciej - I've added you to a PR submitted yesterday for QSPI SFDP BlockDevice .
I've resolved in that PR the PIN layout per target with json config.
I'm not sure which is better .h or the json, please take a look .
Anyhow, I believe if we use this .h configuration the json will no longer be needed.

0xc0170 · 2018-07-30T12:15:00Z

target configuration , in this case it would be PinNames - being part of the target itself like LEDx, etc

Anyhow, I believe if we use this .h configuration the json will no longer be needed.

👍

adustm · 2018-07-30T12:52:52Z

Hello,
I agree to use PinNames.h to get generic pin names that can be used in tests, for platforms that have a flash memory connected to qspi pins.

QSPI1_XXX for targets with no flash onboard (to be discussed)
I would not use a generic pin name for boards that don't have a flash memory on it.
For such platforms, the user will be able to use official pin names, that are defined in PeripheralPins.c directly.
Refer to

mbed-os/targets/TARGET_STM/TARGET_STM32L4/TARGET_STM32L476xG/TARGET_NUCLEO_L476RG/PeripheralPins.c

Line 309 in 1c38d24

//*** QUADSPI ***

NUCLEO_L476RG platform does not have a QSPI flash memory on it. If user wants to connect a QSPI memory on it, he will be able to use PA_6 / PB_11 etc...

won't he ?
Maybe your request it not clear to me.
Kind regards

donatieng · 2018-07-30T15:46:45Z

Hi @maciejbocianski, for platforms with onboard flash, could we also have a QSPI_FLASH_COUNT definition to be able to test for correct macro defines for boards with more than 1 QSPI chip?

Standard pin names (QSPI1_IO0, etc) should be given to all targets (including the ones with onboard flash).

What do you think about having only one naming scheme for QSPI pins (QSPI_FLASH1_XXX), whatever the availability of the on board flash.

I think we need to be very clear on whether pins names refer to a peripheral on the MCU or to pins connected to an on-board memory chip.

@adustm

NUCLEO_L476RG platform does not have a QSPI flash memory on it. If user wants to connect a QSPI memory on it, he will be able to use PA_6 / PB_11 etc...

I think having standard naming where possible would help in many standard cases and avoid confusion.

QSPIFlash flash(QSPI1_SCK, QSPI1_IO0, ...) feels more readable than QSPIFlash flash(PE_6, PB_3, ...).

maciejbocianski · 2018-07-31T10:00:04Z

@donatieng
Not sure if QSPI_FLASH_COUNT is worth adding since the max config is two memory chips (so QSPI2_IO0 will be the max)
I haven't seen similar approach in any other interface in mbed

So finally we will have

For all QSPI capable target:

    QSPI1_IO0 = PD_11,
    QSPI1_IO1 = PD_12,
    QSPI1_IO2 = PE_2,
    QSPI1_IO3 = PD_13,
    QSPI1_SCK = PB_2,
    QSPI1_CSN = PB_6,
    // for dual QSPI inteface
    QSPI2_IO0 = XX,
    QSPI2_IO1 = XX,
    ...

all target with onboard memory will additionally define:

    QSPI_FLASH1_IO0 = QSPI1_IO0,
    QSPI_FLASH1_IO1 = QSPI1_IO1,
    QSPI_FLASH1_IO2 = QSPI1_IO2,
    QSPI_FLASH1_IO3 = QSPI1_IO3,
    QSPI_FLASH1_SCK = QSPI1_SCK,
    QSPI_FLASH1_CSN = QSPI1_CSN,
    // for dual QSPI flash onboard
    QSPI_FLASH2_IO0 = QSPI2_IO0,
    QSPI_FLASH2_IO1 = QSPI2_IO1,
    ...

bulislaw · 2018-08-02T13:44:24Z

I agree, we should aim to provide users with default pin names for all interfaces. On top of that having pins defined for on board memory and sdcards etc should be a good idea. As for "count" i'm not sure how useful it is, how would that be used?

donatieng · 2018-08-07T17:35:01Z

@maciejbocianski @bulislaw my rationale behind QSPI_FLASH_COUNT is that it gives us the ability to test that pin names are properly defined

maciejbocianski · 2018-08-08T12:41:24Z

use case:

#if DEVICE_QSPI
	// use/test QSPI_FLASH1_XXX
	#if QSPI_FLASH_COUNT == 2
		// use/test use QSPI_FLASH2_XXX
	#endif
#endif

cmonr · 2018-08-14T01:27:32Z

@maciejbocianski @donatieng @bulislaw Is there any more work ongoing with this?

maciejbocianski · 2018-08-14T06:04:28Z

Since qspi feature branch will be merged to master soon (#7783).
This PR will be closed and work will be moved to new one

adbridge · 2018-08-14T12:15:19Z

@maciejbocianski please close this if it is no longer needed. Thanks.

maciejbocianski · 2018-08-17T20:40:50Z

Further work moved to #7817

0xc0170 and others added 30 commits July 17, 2018 09:19

QSPI: fix address/alt variable sizes (can be skipped)

1b17da5

Fixing by adding NONE values for both

QSPI: change length to be in/out parameter

c2fbac4

This provides a way to return how many bytes have been written/read (as status codes are returned via func ret value)

QSPI: improve mode documentation

e7b55d3

SPI mode means Clock polarity and phase mode (0 - 3)

QSPI: fix command declaration names

86899ea

QSPI: initial HAL nrf52840 version

6c221dc

This commit adds QSPI HAL implementation for nrf52840 MCU targets

Modify QSPI HAL API to include an API for command-transfer operations

54f6237

Enabling QSPI headers in Nordic HAL implementation and fix for UART S…

91dcbbe

…TDIO definitions

QSPI driver implementation

1103f80

Review fixes and doxygen changes

d0e84d6

Fix code style issues

f94c6fb

Add support for 1_1_2 and 1_2_2 modes in HAL

b091d74

Changing config and return definitions to adhere to HAL defs

d0b6112

Remove explicit initialize API and coding style fixes

5c973cb

Fix bracket placements

d58d980

Remove changes to Nordic SDK and modify HAL to track qspi init

41104e1

Minor optimizations and code style fixes

dacfa9f

Review fixes

3a7b331

QSPI: fix address size for build qspi command

993c3ed

if address is skipped, used size NONE

QSPI: remove initialize method

34f3fe2

Not used anymore, not defined. ctor default initializes the object

QSPI: remove spaces on empty lines

eef50cd

QSPI HAL: fix alternative comment

764a1a6

QSPI: fix arguments for write/read when alt is defined

3b75274

Should be: instr, alt, address or inst, addr or just addr

QSPI: fix driver style issues

c9516a7

QSPI: fix alt size NONE instead 0

74b7666

QSPI HAL: add disabled flag to format phase

22091d9

If phase is being skipped, set disabled to true, otherwise false.

QSPI: add STM32 implementation

67a0e78

QSPI STM32: fix ssel af selection

beee925

QSPI STM32: fix return value in frequency

63b1507

QSPI STM32: set default command values to none

5b23dbe

Cruz Monrreal and others added 6 commits July 23, 2018 23:13

Merge pull request ARMmbed#7580 from maciejbocianski/qspi_test_rem_st…

74307ff

…m_workaround hal-qspi_test: remove STM workaround

Implement qspi_free function

488baf9

Add reset internal state before call to HAL_QspiInit function

5821e6c

Merge pull request ARMmbed#7586 from adustm/qspistm_fix_arm_toolchain

1c38d24

Add reset internal state before call to HAL_QspiInit function

normalize QSPI flash pin names

a2da175

align QSPI test to new pin names

1e1d57c

0xc0170 added the needs: review label Jul 30, 2018

0xc0170 requested review from adustm, offirko and a team July 30, 2018 09:50

0xc0170 approved these changes Jul 30, 2018

View reviewed changes

0xc0170 added the scope: feature label Jul 30, 2018

0xc0170 force-pushed the feature-qspi branch from 8dc4b8a to 606bfb5 Compare August 10, 2018 09:40

maciejbocianski mentioned this pull request Aug 17, 2018

standardise QSPI pin names #7817

Merged

maciejbocianski closed this Aug 17, 2018

0xc0170 removed the needs: review label Aug 17, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Qspi flash pin names standardization #7639

Qspi flash pin names standardization #7639

Uh oh!

maciejbocianski commented Jul 30, 2018

Uh oh!

maciejbocianski commented Jul 30, 2018 •

edited

Loading

Uh oh!

maciejbocianski commented Jul 30, 2018

Uh oh!

offirko commented Jul 30, 2018

Uh oh!

0xc0170 commented Jul 30, 2018

Uh oh!

adustm commented Jul 30, 2018 •

edited

Loading

Uh oh!

donatieng commented Jul 30, 2018 •

edited

Loading

Uh oh!

maciejbocianski commented Jul 31, 2018 •

edited

Loading

Uh oh!

bulislaw commented Aug 2, 2018

Uh oh!

donatieng commented Aug 7, 2018

Uh oh!

maciejbocianski commented Aug 8, 2018

Uh oh!

cmonr commented Aug 14, 2018

Uh oh!

maciejbocianski commented Aug 14, 2018

Uh oh!

adbridge commented Aug 14, 2018

Uh oh!

maciejbocianski commented Aug 17, 2018

Uh oh!

Uh oh!

Qspi flash pin names standardization #7639

Qspi flash pin names standardization #7639

Uh oh!

Conversation

maciejbocianski commented Jul 30, 2018

Description

Pull request type

Uh oh!

maciejbocianski commented Jul 30, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

maciejbocianski commented Jul 30, 2018

Uh oh!

offirko commented Jul 30, 2018

Uh oh!

0xc0170 commented Jul 30, 2018

Uh oh!

adustm commented Jul 30, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

donatieng commented Jul 30, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

maciejbocianski commented Jul 31, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bulislaw commented Aug 2, 2018

Uh oh!

donatieng commented Aug 7, 2018

Uh oh!

maciejbocianski commented Aug 8, 2018

Uh oh!

cmonr commented Aug 14, 2018

Uh oh!

maciejbocianski commented Aug 14, 2018

Uh oh!

adbridge commented Aug 14, 2018

Uh oh!

maciejbocianski commented Aug 17, 2018

Uh oh!

Uh oh!

maciejbocianski commented Jul 30, 2018 •

edited

Loading

adustm commented Jul 30, 2018 •

edited

Loading

donatieng commented Jul 30, 2018 •

edited

Loading

maciejbocianski commented Jul 31, 2018 •

edited

Loading