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

spi: sam: Fix gpio chip select usage #57130

Merged
merged 1 commit into from
Apr 29, 2023

Conversation

teburd
Copy link
Collaborator

@teburd teburd commented Apr 21, 2023

When using gpio chip select the clock line seems to get stuck low after some transactions. When attempting to use other SPI_CSR registers the peripheral fails to work as expected.

Always using SPI_CSR[0] when using gpio chip selects resolves the issue.

@teburd teburd requested a review from tbursztyka as a code owner April 21, 2023 14:51
@teburd teburd requested review from pdgendt and yperess April 21, 2023 14:51
@zephyrbot zephyrbot added area: SPI SPI bus platform: Microchip SAM Microchip SAM Platform (formerly Atmel SAM) labels Apr 21, 2023
@@ -128,7 +128,11 @@ static int spi_sam_configure(const struct device *dev,
* select mode.
*/
spi_mr |= (SPI_MR_MSTR | SPI_MR_MODFDIS);
spi_mr |= SPI_MR_PCS(spi_slave_to_mr_pcs(config->slave));
if (config->cs) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe use a ternary operator?

yperess
yperess previously approved these changes Apr 21, 2023
drivers/spi/spi_sam.c Outdated Show resolved Hide resolved
yperess added a commit to yperess/zephyr that referenced this pull request Apr 27, 2023
Once zephyrproject-rtos#57130 merges this will not be needed.

Signed-off-by: Yuval Peress <peress@google.com>
yperess added a commit to yperess/zephyr that referenced this pull request Apr 27, 2023
Once zephyrproject-rtos#57130 merges this will not be needed.

Signed-off-by: Yuval Peress <peress@google.com>
yperess added a commit to yperess/zephyr that referenced this pull request Apr 27, 2023
Once zephyrproject-rtos#57130 merges this will not be needed.

Signed-off-by: Yuval Peress <peress@google.com>
yperess added a commit to yperess/zephyr that referenced this pull request Apr 27, 2023
Once zephyrproject-rtos#57130 merges this will not be needed.

Signed-off-by: Yuval Peress <peress@google.com>
yperess added a commit to yperess/zephyr that referenced this pull request Apr 28, 2023
Once zephyrproject-rtos#57130 merges this will not be needed.

Signed-off-by: Yuval Peress <peress@google.com>
Comment on lines 131 to 135
if (spi_cs_is_gpio(config)) {
spi_mr |= SPI_MR_PCS(spi_slave_to_mr_pcs(0));
} else {
spi_mr |= SPI_MR_PCS(spi_slave_to_mr_pcs(config->slave));
}
Copy link
Collaborator

@pdgendt pdgendt Apr 28, 2023

Choose a reason for hiding this comment

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

I still fail to understand why it wouldn't work without this change, but in any case you can shorten the statement:

Suggested change
if (spi_cs_is_gpio(config)) {
spi_mr |= SPI_MR_PCS(spi_slave_to_mr_pcs(0));
} else {
spi_mr |= SPI_MR_PCS(spi_slave_to_mr_pcs(config->slave));
}
spi_mr |= SPI_MR_PCS(spi_slave_to_mr_pcs(spi_cs_is_gpio(config) ? 0 : config->slave));

Same for the CSR.

EDIT: I guess the explanation here makes sense

drivers/spi/spi_sam.c Outdated Show resolved Hide resolved
When using gpio chip select the clock line seems to get stuck low after
some transactions. When attempting to use other SPI_CSR registers
the peripheral fails to work as expected.

Always using SPI_CSR[0] when using gpio chip selects resolves the issue.

Signed-off-by: Tom Burdick <thomas.burdick@intel.com>
@carlescufi carlescufi merged commit f923bf8 into zephyrproject-rtos:main Apr 29, 2023
yperess added a commit to yperess/zephyr that referenced this pull request May 1, 2023
Once zephyrproject-rtos#57130 merges this will not be needed.

Signed-off-by: Yuval Peress <peress@google.com>
yperess added a commit to yperess/zephyr that referenced this pull request May 1, 2023
Once zephyrproject-rtos#57130 merges this will not be needed.

Signed-off-by: Yuval Peress <peress@google.com>
yperess added a commit to yperess/zephyr that referenced this pull request May 4, 2023
Once zephyrproject-rtos#57130 merges this will not be needed.

Signed-off-by: Yuval Peress <peress@google.com>
yperess added a commit to yperess/zephyr that referenced this pull request May 4, 2023
Once zephyrproject-rtos#57130 merges this will not be needed.

Signed-off-by: Yuval Peress <peress@google.com>
yperess added a commit to yperess/zephyr that referenced this pull request May 11, 2023
Once zephyrproject-rtos#57130 merges this will not be needed.

Signed-off-by: Yuval Peress <peress@google.com>
yperess added a commit to yperess/zephyr that referenced this pull request May 12, 2023
Once zephyrproject-rtos#57130 merges this will not be needed.

Signed-off-by: Yuval Peress <peress@google.com>
yperess added a commit to yperess/zephyr that referenced this pull request May 12, 2023
Once zephyrproject-rtos#57130 merges this will not be needed.

Signed-off-by: Yuval Peress <peress@google.com>
yperess added a commit to yperess/zephyr that referenced this pull request May 12, 2023
Once zephyrproject-rtos#57130 merges this will not be needed.

Signed-off-by: Yuval Peress <peress@google.com>
yperess added a commit to yperess/zephyr that referenced this pull request May 15, 2023
Once zephyrproject-rtos#57130 merges this will not be needed.

Signed-off-by: Yuval Peress <peress@google.com>
yperess added a commit to yperess/zephyr that referenced this pull request May 15, 2023
Once zephyrproject-rtos#57130 merges this will not be needed.

Signed-off-by: Yuval Peress <peress@google.com>
yperess added a commit to yperess/zephyr that referenced this pull request May 15, 2023
Once zephyrproject-rtos#57130 merges this will not be needed.

Signed-off-by: Yuval Peress <peress@google.com>
yperess added a commit to yperess/zephyr that referenced this pull request May 16, 2023
Once zephyrproject-rtos#57130 merges this will not be needed.

Signed-off-by: Yuval Peress <peress@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: SPI SPI bus platform: Microchip SAM Microchip SAM Platform (formerly Atmel SAM)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants