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.

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

Add QSPI mode and update spi driver. #171

Merged
merged 1 commit into from
Sep 3, 2019
Merged

Add QSPI mode and update spi driver. #171

merged 1 commit into from
Sep 3, 2019

Conversation

reclusejack
Copy link
Contributor

QSPI_ALL
_ _
CS |_________________________________________|
_ _ _ _ _ _ _ _ _ _ _ _ _ _
SCL | || |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |

Phase ---cmd--|-addr--|-dummy-|-data----------------------------
DQ0 0 4 | 0 4 | 0 4 | 0 4 8 12...
| | |
DQ1 1 5 | 1 5 | 1 5 | 1 5 9 13...
| | |
DQ2 2 6 | 2 6 | 2 6 | 2 6 10 14...
| | |
DQ3 3 7 | 3 7 | 3 7 | 3 7 11 15...

QSPI_ADDR_DATA
_ _
CS |_________________________________________|
_ _ _ _ _ _ _ _ _ _ _ _ _ _
SCL | || |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |

Phase ------cmd------|-addr--|-dummy-|-data--------------------
DQ0 0 1 2 3 | 0 4 | 0 4 | 0 4 8 12...
| | |
DQ1 | 1 5 | 1 5 | 1 5 9 13...
| | |
DQ2 | 2 6 | 2 6 | 2 6 10 14...
| | |
DQ3 | 3 7 | 3 7 | 3 7 11 15...

QSPI_DATA_ONLY
_ _
CS |_________________________________________|
_ _ _ _ _ _ _ _ _ _ _ _ _ _
SCL | || |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |

Phase ------cmd------|------addr-----|-----dummy-----|-data----
DQ0 0 1 2 3 | 0 1 2 3 | 0 1 2 3 | 0 4
| | |
DQ1 | | | 1 5
| | |
DQ2 | | | 2 6
| | |
DQ3 | | | 3 7

metal/spi.h Outdated
/*! @brief The spi dummy frame number */
unsigned int dummy_num;
/*! @brief The Dual/Quad spi mode selection.*/
enum { QSPI_ALL, QSPI_DATA_ONLY, QSPI_ADDR_DATA } qspi_mode;
Copy link
Contributor

Choose a reason for hiding this comment

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

If this applies to both dual and quad-SPI, we should probably not name it "qspi_mode".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Any suggestion for this variable name? How about "multi_wire" or "multi_wire_mode"?

long control_base =
__metal_driver_sifive_spi0_control_base((struct metal_spi *)spi);

if (config->qspi_mode == qspi_mode) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this check here? If they have to be equal, why not just use the one already in config?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That' my mistake, I should not use the same name for the two variables.
I've renamed these two variables.
First one is used for SPI configuration, the second is for tagging the data transfer stage.
The function spi_mode_switch will be active either after sending out cmd or before sending data.

MULTI_WIRE_ALL
      _                                                        _
CS     |______________________________________________________|
         _   _   _   _   _   _   _   _   _   _   _   _   _   _
SCL    _| |_| |_| |_| |_| |_| |_| |_| |_| |_| |_| |_| |_| |_| |_

Phase ---cmd--|-addr--|-dummy-|-data----------------------------
DQ0     0   4 | 0   4 | 0   4 | 0   4   8  12...
              |       |       |
DQ1     1   5 | 1   5 | 1   5 | 1   5   9  13...
              |       |       |
DQ2     2   6 | 2   6 | 2   6 | 2   6  10  14...
              |       |       |
DQ3     3   7 | 3   7 | 3   7 | 3   7  11  15...

MULTI_WIRE_ADDR_DATA
      _                                                        _
CS     |______________________________________________________|
         _   _   _   _   _   _   _   _   _   _   _   _   _   _
SCL    _| |_| |_| |_| |_| |_| |_| |_| |_| |_| |_| |_| |_| |_| |_

Phase  ------cmd------|-addr--|-dummy-|-data--------------------
DQ0     0   1   2   3 | 0   4 | 0   4 | 0   4   8  12...
                      |       |       |
DQ1                   | 1   5 | 1   5 | 1   5   9  13...
                      |       |       |
DQ2                   | 2   6 | 2   6 | 2   6  10  14...
                      |       |       |
DQ3                   | 3   7 | 3   7 | 3   7  11  15...

MULTI_WIRE_DATA_ONLY
      _                                                        _
CS     |______________________________________________________|
         _   _   _   _   _   _   _   _   _   _   _   _   _   _
SCL    _| |_| |_| |_| |_| |_| |_| |_| |_| |_| |_| |_| |_| |_| |_

Phase  ------cmd------|------addr-----|-----dummy-----|-data----
DQ0     0   1   2   3 | 0   1   2   3 | 0   1   2   3 | 0   4
                      |               |               |
DQ1                   |               |               | 1   5
                      |               |               |
DQ2                   |               |               | 2   6
                      |               |               |
DQ3                   |               |               | 3   7
Copy link
Contributor

@nategraff-sifive nategraff-sifive left a comment

Choose a reason for hiding this comment

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

LGTM, thank you!

@nategraff-sifive
Copy link
Contributor

@reclusejack Can you issue a similar PR against v201908-branch? @dbarbi1 would like this in v19.08 as well.

@nategraff-sifive nategraff-sifive merged commit 7707e18 into master Sep 3, 2019
@nategraff-sifive nategraff-sifive deleted the add_qspi branch September 3, 2019 17:39
bsousi5 pushed a commit that referenced this pull request Apr 8, 2020
Enable Freedom Metal builds in standalone projects
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NEW API Adds public API surface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants