Skip to content

SPIF - Fix command to unlock Global Block-Protection register #9408

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
Merged
merged 3 commits into from
Jan 23, 2019
Merged

SPIF - Fix command to unlock Global Block-Protection register #9408

merged 3 commits into from
Jan 23, 2019

Conversation

DBS06
Copy link
Contributor

@DBS06 DBS06 commented Jan 17, 2019

Description

As described in the code of SPIFBlockDevice.cpp line 168:

SST devices come preset with block protection enabled for some regions, issue write disable instruction to clear

But during the commit ARMmbed/spif-driver@a821b69 the command (0x98) was falsely replaced with "SPIF_WRDI" -> classical copy and paste error 😏

Therefore I added the commend to the SPIF-Default-Instructions and replaced the wrong command "SPIF_WRDI" with "SPIF_ULBPR".

Pull request type

[x] Fix
[ ] Refactor
[ ] Target update
[ ] Functionality change
[ ] Docs update
[ ] Test update
[ ] Breaking change

Reviewers

@ciarmcom ciarmcom requested review from a team January 17, 2019 10:00
@ciarmcom
Copy link
Member

@DBS06, thank you for your changes.
@ARMmbed/mbed-os-storage @ARMmbed/mbed-os-maintainers please review.

@DBS06
Copy link
Contributor Author

DBS06 commented Jan 17, 2019

Interesting fail

Local doxy-spellcheck testing has failed

@0xc0170
Copy link
Contributor

0xc0170 commented Jan 17, 2019

It's bug on master ,there's PR already fixing it and will rebase PR once it's on master

offirko
offirko approved these changes Jan 17, 2019
View reviewed changes
Copy link
Contributor

@offirko offirko left a comment •
edited
Loading

Choose a reason for hiding this comment

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

@DBS06 Good catch - Thanks!! :$
I'd really appreciate it if you could perform the identical change for QSPIFBlockDevice.cpp.
https://github.com/ARMmbed/mbed-os/blob/a50ba6e71d203409102cd1be0a2dd011c67b9d64/components/storage/blockdevice/COMPONENT_QSPIF/QSPIFBlockDevice.cpp#L209

components/storage/blockdevice/COMPONENT_SPIF/SPIFBlockDevice.cpp Outdated
@@ -169,7 +170,7 @@ int SPIFBlockDevice::init()
// SST devices come preset with block protection
// enabled for some regions, issue write disable instruction to clear
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be changed to "issue global protection unlock to clear"

Copy link
Contributor Author

@DBS06 DBS06 Jan 17, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

@offirko Done 😃
But what do you mean by:

> Should be changed to "issue global protection unlock to clear"

@DBS06
Copy link
Contributor Author

DBS06 commented Jan 17, 2019

Furthermore I added the command to SPIFReducedBlockDevice.cpp, it was there but only as a "magic number" and not as a command.

@offirko
Copy link
Contributor

offirko commented Jan 17, 2019

@DBS06 - much appreciated

@cmonr
Copy link
Contributor

cmonr commented Jan 17, 2019

NOTE: This PR has now been rebased.

If this was made in error, feel free to force-push your local branch to restore the PR.

@cmonr cmonr force-pushed the feature/SPIF_ULBPR branch from eac0e4e to f0ec813 Compare January 17, 2019 17:13
@DBS06
Copy link
Contributor Author

DBS06 commented Jan 21, 2019

@0xc0170 is work still required?

@0xc0170
Copy link
Contributor

0xc0170 commented Jan 21, 2019

Ready for CI, starting now

@mbed-ci
Copy link

mbed-ci commented Jan 21, 2019

Test run: FAILED

Summary: 1 of 7 test jobs failed
Build number : 1
Build artifacts

Failed test jobs:

  • jenkins-ci/mbed-os-ci_build-ARM

@0xc0170
Copy link
Contributor

0xc0170 commented Jan 22, 2019

CI restarted

@mbed-ci
Copy link

mbed-ci commented Jan 22, 2019

Test run: FAILED

Summary: 1 of 11 test jobs failed
Build number : 3
Build artifacts

Failed test jobs:

  • jenkins-ci/mbed-os-ci_dynamic-memory-usage

@0xc0170
Copy link
Contributor

0xc0170 commented Jan 22, 2019

We are investigating the dynamic memory usage ( last 3 jobs failed)

@cmonr cmonr merged commit 0c85d44 into ARMmbed:master Jan 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@0xc0170 0xc0170 0xc0170 approved these changes

@offirko offirko offirko approved these changes

Assignees
No one assigned
Labels
release-version: 5.11.3
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@DBS06 @ciarmcom @0xc0170 @offirko @cmonr @mbed-ci