Nrf52 qspi flash #1188

hathach · 2018-09-13T12:19:31Z

It is still work in progress but current code works with 8 MB MX25R6435F on pca10056. The read speed is ok, but the write is painfully slow (will improve later). I would like to have as many review/suggestion as possible.

added flash_api directory and move all the flash there
flash_api.c implement generic read/write blocks API with page caching along with python glueing.
flash_api use flash_hal_erase/read/program which are implemented by internal or qspi flash.
flash hal will be dynamic binding later ( e.g switch from qspi to internal should we couldn't read/write to external flash in case of soldering issue etc ..)

#1049

- also move usb_init() from board_init() to port_init(0

qspi flash is still corrupted occasionally

- move write caching to flash_api for both internal and qspi flash - fix flash write blocks for qspi

hathach · 2018-09-13T12:20:11Z

the super slow write speed really need some improvement !! I will try to implement

non-blocking API, since the current write caching with erase + write + read is not that good at all.
~~Also we don't really need read if the whole 4KB is written~~ only read data that we are not writing does not increase write speed with testbench.

tannewt · 2018-09-13T16:28:09Z

Write speed isn't critical to using CircuitPython. Yes, it'd be nice if it was faster but let's ignore it for now and focus on getting all of the APIs going.

I suggest filing a separate issue for it for the full 4.0.0 release milestone and moving onto other tasks for now.

hathach · 2018-09-13T16:58:14Z

@tannewt phew, I am trying to increase usb msc buffer size to 8KB but it doesn't help as well. probably need to dive deeper into the flash datasheet timing etc ... I will try to wrap it up now.

Update: just check datasheet of the flash on pca10056, we may need to enable high performance mode, will give it a try later.

dhalbert · 2018-09-13T17:47:56Z

@hathach We'd like to make this implementation be as similar as possible to the atmel-samd QSPI and external flash implementation, so that we can factor out the common code as much as possible and only implement the lowest level routines in a chip-specific way. For instance, that code has parameterized chip support, and it already has caching logic. Did you try copying that impl? This impl is more different than we thought it would be.

hathach · 2018-09-13T18:08:36Z

@dhalbert many of the spi std command for flash is handled by nrfx qspi driver. Since nrf52840 has enough memory (4K) to cache the whole page. I just refactor both caching for internal + external flash leaving it to only basic hal erase, read, program. At anywhere you could use block API flash_read_blocks/flash_write_blocks instead of internal/external_flash_write_blocks. The flash_ will use either internal or external depending on the config. Those can be changed to function pointer for concurrent internal + external uses if needed.

tannewt · 2018-09-13T19:13:54Z

There is additional logic in the atmel-samd port that isn't in this one because it wasn't started from a copy. For example, the SPI flash chip is detected rather than hard coded. Ideally this logic would be shared but for now it can be copied if that's easier.

Ideally you'd only need to replace the implementations here but it hasn't been tested yet.

hathach · 2018-09-13T19:21:46Z

@tannewt ah yeah, I am adding support device lookup table with JEDEC ID. Instead of using 3 bytes response from 0x9F for manufacture id, memory type and capacity, can we use only 2 bytes response from 0x90 command (manufacture id, device id) ?

…f flash device is not one of supported. change nrf qspi to blocking model

Rename config pin name from QSPI_FLASH_ --> MICROPY_QSPI_

…g of DATAn pin

hathach · 2018-09-15T17:35:30Z

I tried to enable high performance bit but couldn't get it written any faster. It is probably default mode already. Note: nrf52 SCK clock speed is only up to 32 Mhz.

I post an issue on Nordic devzone to see if there is anyone got more lucky than us.
https://devzone.nordicsemi.com/f/nordic-q-a/38570/write-speed-with-mx25r6435f-on-nrf52840dk

hathach · 2018-09-19T08:50:17Z

Nordic testing is a bit higher than us for write speed (different tool can be a bit different), but seem like write speed will be somewhat 1/10 of read speed. We may not improve it much with pca10056.

dhalbert · 2018-10-03T13:18:52Z

@hathach can you resolve the conflicts that have arisen? Thanks.

hathach · 2018-10-12T06:39:16Z

@dhalbert I am making more improvement while porting this for Arduino, I will clean up and update the flash code then resolve the conflict for this a bit later.

dhalbert · 2018-10-12T21:18:44Z

@hathach I stumbled on this while looking at the datasheet for something else. It says to use high drive strength to drive the QSPI pins:

[page 267, section 6.19.1]

2. To ensure stable operation, set the GPIO drive strength to “high drive”. See the GPIO — General
purpose input/output on page 141 chapter for details on how to configure GPIO drive strength.

arturo182 · 2018-10-16T18:57:00Z

ports/nrf/boards/feather_nrf52840_express/mpconfigboard.h

+#define MICROPY_QSPI_DATA2                NRF_GPIO_PIN_MAP(0, 12)
+#define MICROPY_QSPI_DATA3                NRF_GPIO_PIN_MAP(0, 14)
+#define MICROPY_QSPI_SCK                  NRF_GPIO_PIN_MAP(0, 8)
+#define MICROPY_QSPI_CS                   NRF_GPIO_PIN_MAP(1, 8)


Seems inconsistent that all other defines in this file use the pin__ objects but for QSPI we use the pin number

arturo182 · 2018-10-16T18:57:19Z

ports/nrf/boards/pca10056/board.c

+    nrf_gpio_cfg_input(11, NRF_GPIO_PIN_PULLUP);
+    nrf_gpio_cfg_input(12, NRF_GPIO_PIN_PULLUP);
+    nrf_gpio_cfg_input(24, NRF_GPIO_PIN_PULLUP);
+    nrf_gpio_cfg_input(25, NRF_GPIO_PIN_PULLUP);


This looks like test code that shouldn't be merged.

arturo182 · 2018-10-16T19:07:25Z

ports/nrf/flash_api/flash_devices.h

+
+
+// Settings for the Gigadevice GD25Q16C 2MiB SPI flash.
+// Datasheet: http://www.gigadevice.com/wp-content/uploads/2017/12/DS-00086-GD25Q16C-Rev2.6.pdf


This link returns 404 for me

arturo182 · 2018-10-16T19:07:33Z

ports/nrf/flash_api/flash_devices.h

+    .status_quad_enable = (1 << 9)\
+}
+
+#define MX25R6435F {\


Add a datasheet link as well?

arturo182 · 2018-10-16T19:42:54Z

I'm trying to get this patch working on my board. I use a different flash chip but I added it to flash_devices.h, it seems to get detected properly because I get past the if ( _flash_devices_arr[i].manufacturer_id == mfgr_id && _flash_devices_arr[i].device_id == dev_id ) { check with proper values, however the MSD does not get mounted by the system (Linux Mint), trying to mount it manually gives me an error:

> sudo mount /dev/sdb /media/arturo182/usb
mount: /media/arturo182/usb: wrong fs type, bad option, bad superblock on /dev/sdb, missing codepage or helper program, or other error.

The CDC is properly detected and works just fine, but it also doesn't seem to be aware of the flash storage:

>>> import os
>>> os.listdir('.')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
OSError: [Errno 19] Unsupported operation

Also, the chip i use is 128Mbit/16Mbyte and I specified it as so .total_size = 16*1024*1024, but when I connect the board I get this message sd 4:0:0:0: [sdb] 32768 512-byte logical blocks: (16.8 MB/16.0 MiB), doesn't that mean that the system think there is more space than the chip has so either we might overwrite something or discard it at some boundary check? I think it's possible the total_size units are wrong.

dhalbert · 2018-10-16T20:18:38Z

@arturo182 What chip is it, and does it truly have 16MiB (binary megabytes), or is it smaller?

Also 16MB is just at the boundary between FAT12 and FAT16, and it's possible either Linux or FatFS is confused about just where that boundary is: one might be think it's FAT12 and the other might think it's FAT16.

arturo182 · 2018-10-16T20:50:31Z

Ah yes, I should've written the model, it's IS25LP128F, datasheet available here: http://www.issi.com/WW/pdf/25LP-WP128F.pdf
I also tried setting total_size to 8MB and that didn't work either, but I'm not sure if mismatching like that should work at all.

hathach · 2018-10-17T04:07:27Z

@aturo182 is the flash wired for quad mode ? If yes, Also please check the status registers mask in device config to make sure it is correct value to put flash device into quad mode.

arturo182 · 2018-10-17T10:55:30Z

I use (1 << 6) for the quad enable value, I think it's correct according to the datasheet:

As for the design, I'm not sure what more is needed for quad mode wiring, I pretty much copied the PCA10056 except with a different chip, most qspi flash ICs seem to be pin compatible.

Datasheet:

My design:

@ladyada do you think it would be possible to post the nrf52840 feather schematic part with the QSPI flash so I could compare?

tannewt · 2018-10-17T19:03:50Z

@arturo182 I think most of the QSPI testing has been done on the Nordic dev boards. The feather exists but I'm not sure if anyone has tested it at all.

arturo182 · 2018-10-17T19:15:27Z

I see, I thought @hathach was using some early nRF52840 feather prototypes to test the qspi :D

Looking at the PCA10056 schematic the pinout seems the same, I don't think there is any limit to which pins can be used as QSPI, so maybe I'll double check my soldering, but seems odd that it managed so read the chip id without a problem.

hathach · 2018-10-18T04:43:57Z

@arturo182 I only tested with pca10056, maybe you should start there to see if that works for you first and then moving to your custom board.

arturo182 · 2018-10-18T05:41:09Z

Good idea, I'll try that :D

arturo182 · 2018-10-19T16:59:50Z

I tested the code on PCA10056 and it works there so must be something with either the soldering, the chip or my gpio choice.

tannewt · 2018-10-26T22:21:51Z

FYI I'm moving the atmel-samd port to tiny usb and factoring out all of the usb stuff from the ports. This includes MSC so I plan on factoring out the spi flash chip support as well. I'll add support for QSPI on nRF based on this PR (thanks for it!) but will not merge it directly. Expect to see a PR for it next week sometime. Thanks!

hathach added 11 commits September 12, 2018 14:24

adding flash_api, qspi flash

835582b

- also move usb_init() from board_init() to port_init(0

clean up

81ec1d9

udate tinyusb msc flash size config

fae7f50

qspi on pca10056 seems to work but write speed is slow

fc80e75

move internal_flash to flash_api

570b315

add MICROPY_HW_LED_MSC

2dee813

qspi flash is still corrupted occasionally

add flash_hal API

155ea3e

- move write caching to flash_api for both internal and qspi flash - fix flash write blocks for qspi

rename vfs to user_vfs

19abe37

update qspi config

78a2e4e

clean up

3661b40

clean up

c65de95

hathach requested review from arturo182, dhalbert, ladyada, microbuilder and tannewt September 13, 2018 12:19

hathach added 2 commits September 13, 2018 22:47

only read existing flash data that we are not writing

c48e507

fix previous commit issue

1b498c2

add flash devices

6386d3c

hathach added 4 commits September 14, 2018 02:41

clean up

0be09f4

add JEDEC manufacturer & device ID detection, fall back to internal i…

3297ca5

…f flash device is not one of supported. change nrf qspi to blocking model

update qspi mpconfigboard.h for feather nrf52840

c0c89e3

Rename config pin name from QSPI_FLASH_ --> MICROPY_QSPI_

determine QSPI operation mode (quad, dual, single) based on the confi…

6a3e70a

…g of DATAn pin

add ALL_EXTERNAL_FLASH_DEVICES define

67161a5

arturo182 requested changes Oct 16, 2018

View reviewed changes

tannewt closed this Oct 26, 2018

hathach deleted the nrf52_qspi_flash branch December 17, 2019 03:38



		// Settings for the Gigadevice GD25Q16C 2MiB SPI flash.
		// Datasheet: http://www.gigadevice.com/wp-content/uploads/2017/12/DS-00086-GD25Q16C-Rev2.6.pdf

Nrf52 qspi flash #1188

Nrf52 qspi flash #1188

Uh oh!

Conversation

hathach commented Sep 13, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hathach commented Sep 13, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tannewt commented Sep 13, 2018

Uh oh!

hathach commented Sep 13, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dhalbert commented Sep 13, 2018

Uh oh!

hathach commented Sep 13, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tannewt commented Sep 13, 2018

Uh oh!

hathach commented Sep 13, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hathach commented Sep 15, 2018

Uh oh!

hathach commented Sep 19, 2018

Uh oh!

dhalbert commented Oct 3, 2018

Uh oh!

hathach commented Oct 12, 2018

Uh oh!

dhalbert commented Oct 12, 2018

Uh oh!

arturo182 Oct 16, 2018

Choose a reason for hiding this comment

Uh oh!

arturo182 Oct 16, 2018

Choose a reason for hiding this comment

Uh oh!

arturo182 Oct 16, 2018

Choose a reason for hiding this comment

Uh oh!

arturo182 Oct 16, 2018

Choose a reason for hiding this comment

Uh oh!

arturo182 commented Oct 16, 2018

Uh oh!

dhalbert commented Oct 16, 2018

Uh oh!

arturo182 commented Oct 16, 2018

Uh oh!

hathach commented Oct 17, 2018

Uh oh!

arturo182 commented Oct 17, 2018

Uh oh!

tannewt commented Oct 17, 2018

Uh oh!

arturo182 commented Oct 17, 2018

Uh oh!

hathach commented Oct 18, 2018

Uh oh!

arturo182 commented Oct 18, 2018

Uh oh!

arturo182 commented Oct 19, 2018

Uh oh!

tannewt commented Oct 26, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

hathach commented Sep 13, 2018 •

edited

Loading

hathach commented Sep 13, 2018 •

edited

Loading

hathach commented Sep 13, 2018 •

edited

Loading

hathach commented Sep 13, 2018 •

edited

Loading

hathach commented Sep 13, 2018 •

edited

Loading