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

Jump to bottom

boards: arm: use QSPI on XIAO BLE Sense #55199

Merged
merged 1 commit into from Apr 7, 2023
Merged

boards: arm: use QSPI on XIAO BLE Sense #55199

merged 1 commit into from Apr 7, 2023

Conversation

oryjkov
Copy link
Contributor

@oryjkov oryjkov commented Feb 26, 2023

Tested by running usb/mass demo on a XIAO ble board.

@mniestroj

@mniestroj mniestroj self-requested a review February 27, 2023 04:42
mniestroj
mniestroj requested changes Feb 27, 2023
View reviewed changes
boards/arm/xiao_ble/xiao_ble_common.dtsi Outdated Show resolved Hide resolved
boards/arm/xiao_ble/xiao_ble_common.dtsi Outdated Show resolved Hide resolved
@mniestroj
Copy link
Member

@oryjkov Please squash your changes to a single commit.

@oryjkov
Copy link
Contributor Author

oryjkov commented Feb 27, 2023

Done. Thanks for the review

@mniestroj
Copy link
Member

@oryjkov Please add commit body according to contribution guidelines: https://docs.zephyrproject.org/3.3.0/contribute/guidelines.html#commit-message-guidelines

@teburd teburd requested a review from anangl March 2, 2023 16:48
@oryjkov oryjkov requested review from mniestroj and removed request for MaureenHelm, galak and anangl March 4, 2023 10:04
mniestroj
mniestroj previously approved these changes Mar 4, 2023
View reviewed changes
@mniestroj
Copy link
Member

@oryjkov Please update commit author to match your signed-off line. You can probably do this with git commit --amend --author='Oleg Ryjkov <oryjkov@gmail.com>'.

@oryjkov @mniestroj
boards: arm: use QSPI on XIAO BLE Sense
a7f543b 
The external flash chip is wired for QSPI. The chip, P25Q16H, has its
"Quad enable" bit in bit 9 of the Read Status Register (section 10.5 in
the datasheet), hence needs this particular quad enable requirement.

Co-authored-by: Marcin Niestroj <m.niestroj@emb.dev>
Signed-off-by: Oleg Ryjkov <oryjkov@gmail.com>
@oryjkov
Copy link
Contributor Author

oryjkov commented Mar 4, 2023

Dang, thanks. I haven't set it git config properly on this machine yet.

@oryjkov oryjkov requested a review from mniestroj March 4, 2023 11:22
@oryjkov
Copy link
Contributor Author

oryjkov commented Mar 4, 2023 •
edited

Actually I want to hold this off for now. I tried doing system off with the qspi config and it fails:
pm: Device p25q16h@0 did not enter suspended state (-140)

It works OK when used with SPI. Not sure what's up. LMK if you have any ideas.
@mniestroj

@oryjkov
Copy link
Contributor Author

oryjkov commented Mar 4, 2023 •
edited

Hm, removing "has-dpd;" and friends is one way to go. SPI mode does not use them anyway and does not do deep power-down, so this won't be a regression.
But would have been nice to be able to use power down.

Later edit: The reason for saying that SPI mode is not doing power down is because when stepping through with debugger, I'm seeing that device p25q16h_spi in pm_device_action_run() at https://github.com/zephyrproject-rtos/zephyr/blob/main/subsys/pm/device.c#L44 has pm=NULL. Whereas when it runs as QSPI pm is not null.

I have not actually verified whether flash is off with a power meter.

@oryjkov
Copy link
Contributor Author

oryjkov commented Mar 5, 2023 •
edited

I had another look and hopefully someone here could tell me what's the problem.

This change as it is now works fine, however if I enter system off and then wake up (without losing power, so flash does not reset), then the flash device p25q16h@0 becomes unusable. Power cycling fixes it up again. I believe this is because the nrf driver can't get the flash chip out of deep sleep.

Here is what I tried:

  1. Once the flash is bricked, I reconfigured zephyr to use the SPI driver and re-flashed, without losing power to the flash chip. The SPI driver did not manage to get the chip working. (That's one reason to believe that the DPD is not working now, even though it seems to be configured in the board's dts.).
  2. I then stepped through the SPI NOR driver initialization and noticed that it fails at spi_nor_rdsr().
  3. This seemed strange given that the chip could be in deep sleep and from the datasheet I got the impression that it can only respond to RDPD and RES commands. I changed the call to spi_nor_rdsr to first do spi_nor_cmd_read(dev, SPI_NOR_CMD_RDPD, &reg, sizeof(reg)); (as a read, not a write). This made the flash chip usable again!
  4. QSPI, however, seems more opinionated on how flash chips are supposed to behave and I was not able to find a way to get it to do an RDPD command. After a cursory look at the documentation and debug output, it seems that custom instruction is the way to do this, but a custom instruction requires the QSPI STATUS register to be set, which (I think) has to query the device for its status via an ACTIVATE task (which is where its failing). Forcing an CINSTR seems to just do nothing.

(All of this was using ncs-2.3.0-rc1, I have not tried the latest zephyr)

I'd appreciate any help :)

@mniestroj given that the driver now has dpd options. How certain are you that it actually puts the chip into deep sleep?

@mniestroj
Copy link
Member

Hm, removing "has-dpd;" and friends is one way to go. SPI mode does not use them anyway and does not do deep power-down, so this won't be a regression.

Looking at the spi_nor.c implementation it seems that CONFIG_SPI_NOR_IDLE_IN_DPD=y is needed in order to take has-dpd; devicetree property in effect and actually attempt deep power down.

given that the driver now has dpd options. How certain are you that it actually puts the chip into deep sleep?

Tested it with flash test shell command and CONFIG_SPI_NOR_IDLE_IN_DPD=y. Though I didn't check if the power goes down.

@oryjkov
Copy link
Contributor Author

oryjkov commented Mar 6, 2023

Tested it with flash test shell command and CONFIG_SPI_NOR_IDLE_IN_DPD=y. Though I didn't check if the power goes down.

You're right. With SPI mode and IDLE_IN_DPD option enabled the current in SYSTEM_OFF was down to <4 uA. Without IDLE_IN_DPD - it was around 320 uA. Interestingly, flash itself is only 20uA, 300 seem to come from the fact that CS pin was driven high.

Anyways, with QSPI without deep sleep it is possible to bring system off current down to ~25 uA. With deeps sleep it goes down to the same 4uA, but it won't resume. So it still feels like a regression.

What do you think is the best course of action to get someone from Nordic to look at this, I'm guessing a DevZone ticket?

@mniestroj
Copy link
Member

What do you think is the best course of action to get someone from Nordic to look at this, I'm guessing a DevZone ticket?

@anangl @carlescufi could advice on above comment?

@anangl
Copy link
Member

anangl commented Mar 7, 2023

What do you think is the best course of action to get someone from Nordic to look at this, I'm guessing a DevZone ticket?

@oryjkov Please create a Zephyr issue for this.

@oryjkov oryjkov mentioned this pull request Mar 7, 2023
Closed
@oryjkov
Copy link
Contributor Author

oryjkov commented Mar 7, 2023

@anangl done in #55543

@carlescufi carlescufi merged commit 01bc0c3 into zephyrproject-rtos:main Apr 7, 2023
@xudongzheng xudongzheng mentioned this pull request Jan 4, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mniestroj mniestroj mniestroj approved these changes

@carlescufi carlescufi carlescufi approved these changes

Assignees

@anangl anangl

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@oryjkov @mniestroj @anangl @carlescufi