drivers: spi: siwx91x: Fix frequency > 40MHz #100762
Open
+24
−64
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The bit_rate variable does not bring any benefit. Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
In functions requiring to lock/release resources, it is less error prone to have only one exit point and user goto to manage errors. The behavior of the new code is exactly identical to the initial one. Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
zephyrbot
requested review from
Martinhoff-maker,
asmellby,
decsny,
jhedberg,
tbursztyka and
teburd
jerome-pouiller
force-pushed
the
fix-spi
branch
from
363c46e to
7802f8e
Compare
Until now, GSPI_DATA_SAMPLE_EDGE was enabled as soon as the user requested
> 40Mhz (even if the actual frequency was in fact 40Mhz). However, at 40MHz
and at 80MHz, use of GSPI_DATA_SAMPLE_EDGE generated read errors on the
last bit of the transaction:
Buffer contents are different:
[...],0xaa,0xaa,0xaa,0xaa,
vs:
[...],0xaa,0xaa,0xaa,0xab,
I have not found any case where GSPI_DATA_SAMPLE_EDGE is useful, so this
patch just remove this parameter.
Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
In gspi_siwx91x_config(), clk_div_factor can't be < 1. Therefore, we can remove the dead code. This code has been tested with tests/drivers/spi/spi_loopback, with a PLL clock configured to 160MHz and a bus clock to 80MHz with success. I have not found the case where change in GSPI_CLK_CONFIG are required. Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
Since actual_hz is no more needed, we can simplify gspi_siwx91x_pick_lower_freq(). Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
Currently, clk_div_factor is force to 1 if user request more than 110MHz. However, in this case, gspi_siwx91x_get_divider() will never return 2 or more, unless the input clock is >= 220MHz. The si91x is not designed for such high clock frequency. So, this case has never been tested. Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
jerome-pouiller
force-pushed
the
fix-spi
branch
from
7802f8e to
dc3f4f1
Compare
|
jhedberg
approved these changes
Dec 10, 2025
asmellby
approved these changes
Dec 10, 2025
asmellby
pushed a commit
to asmellby/zephyr
that referenced
this pull request
Dec 10, 2025
The bit_rate variable does not bring any benefit. Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com> Upstream-status: pr <zephyrproject-rtos#100762>
asmellby
pushed a commit
to asmellby/zephyr
that referenced
this pull request
Dec 10, 2025
In functions requiring to lock/release resources, it is less error prone to have only one exit point and user goto to manage errors. The behavior of the new code is exactly identical to the initial one. Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com> Upstream-status: pr <zephyrproject-rtos#100762>
asmellby
pushed a commit
to asmellby/zephyr
that referenced
this pull request
Dec 10, 2025
Until now, GSPI_DATA_SAMPLE_EDGE was enabled as soon as the user requested
> 40Mhz (even if the actual frequency was in fact 40Mhz). However, at 40MHz
and at 80MHz, use of GSPI_DATA_SAMPLE_EDGE generated read errors on the
last bit of the transaction:
Buffer contents are different:
[...],0xaa,0xaa,0xaa,0xaa,
vs:
[...],0xaa,0xaa,0xaa,0xab,
I have not found any case where GSPI_DATA_SAMPLE_EDGE is useful, so this
patch just remove this parameter.
Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
Upstream-status: pr <zephyrproject-rtos#100762>
asmellby
pushed a commit
to asmellby/zephyr
that referenced
this pull request
Dec 10, 2025
In gspi_siwx91x_config(), clk_div_factor can't be < 1. Therefore, we can remove the dead code. This code has been tested with tests/drivers/spi/spi_loopback, with a PLL clock configured to 160MHz and a bus clock to 80MHz with success. I have not found the case where change in GSPI_CLK_CONFIG are required. Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com> Upstream-status: pr <zephyrproject-rtos#100762>
asmellby
pushed a commit
to asmellby/zephyr
that referenced
this pull request
Dec 10, 2025
Since actual_hz is no more needed, we can simplify gspi_siwx91x_pick_lower_freq(). Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com> Upstream-status: pr <zephyrproject-rtos#100762>
asmellby
pushed a commit
to asmellby/zephyr
that referenced
this pull request
Dec 10, 2025
Currently, clk_div_factor is force to 1 if user request more than 110MHz. However, in this case, gspi_siwx91x_get_divider() will never return 2 or more, unless the input clock is >= 220MHz. The si91x is not designed for such high clock frequency. So, this case has never been tested. Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com> Upstream-status: pr <zephyrproject-rtos#100762>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
tests/drivers/spi/spi_loopback reported Rx errors on the last bit when requested frequency was > 40MHz. This was mainly caused by misuse of
GSPI_DATA_SAMPLE_EDGE.This PR take advantage of this change to slightly clean up the SPI driver.
I have successfully tested with code at 40Mhz and 80Mhz.