Skip to content

drivers: spi: cdns: fix missing fifo config #88777

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
Jun 4, 2025
Merged

drivers: spi: cdns: fix missing fifo config #88777

merged 3 commits into from
Jun 4, 2025

Conversation

XenuIsWatching
Copy link
Member

@XenuIsWatching XenuIsWatching commented Apr 17, 2025 •
edited
Loading

This adds the missing fifo config from the dts which was missed in the initial revision. This also adds the spi rtio fallback api, and fixes an issue with calculating the spi clock divisor

@XenuIsWatching
drivers: spi: cdns: fix missing fifo config
add0cc9 
This adds the missing fifo config from the dts which was missed in
the initial revision. This also adds the spi rtio fallback api.

Signed-off-by: Ryan McClelland <ryanmcclelland@meta.com>
@github-actions github-actions bot added the area: SPI SPI bus label Apr 17, 2025
@github-actions github-actions bot requested review from tbursztyka and teburd April 17, 2025 19:20
teburd
teburd previously approved these changes Apr 18, 2025
View reviewed changes
kartben
kartben previously approved these changes Apr 25, 2025
View reviewed changes
@XenuIsWatching XenuIsWatching dismissed stale reviews from kartben and teburd via 2274ae6 April 26, 2025 02:49
teburd
teburd previously approved these changes Apr 26, 2025
View reviewed changes
@XenuIsWatching
drivers: spi: cdns: remove pm device call
500de16 
The PM Device callbacks is rather unimplemented. There currently is
no device agnostic clock management api (yet), and the pinctrl isn't
fully implemented in this driver. Remove it all.

Signed-off-by: Ryan McClelland <ryanmcclelland@meta.com>
kartben
kartben previously approved these changes Apr 26, 2025
View reviewed changes
@XenuIsWatching XenuIsWatching mentioned this pull request Apr 28, 2025
Merged
@XenuIsWatching XenuIsWatching added the DNM This PR should not be merged (Do Not Merge) label Apr 29, 2025
@XenuIsWatching XenuIsWatching removed the DNM This PR should not be merged (Do Not Merge) label Apr 29, 2025
@XenuIsWatching XenuIsWatching force-pushed the spi-cdns-fixup branch 2 times, most recently from 7ceb0fc to 2a970cd Compare April 29, 2025 21:06
@XenuIsWatching
drivers: spi: cdns: fixup spi clk divisor
ee0547c 
Remove the auto setting of the external spi clock if its not there,
also fix the calculation of calucation the spi divisor value.

Signed-off-by: Ryan McClelland <ryanmcclelland@meta.com>
@XenuIsWatching
Copy link
Member Author

Sorry for the butter churning here after the approvals have already happened, but hopefully that should be the last commit to be stacked in to this PR

@SrikanthGoud123
Copy link

SrikanthGoud123 commented May 5, 2025 •
edited
Loading

Hi @XenuIsWatching with your latest changes I see child(flash) attached under spi , spi-nor flash initialization is working fine and its could able to read Flash ID. But as mentioned in #88777 still I see all test cases in tests/drivers/flash/common/ failing, from the failure log, its looks like driver is could able to handle small transfers and failing at large transfers

START - test_read_unaligned_address
Test will run on device flash@2
E: Timeout waiting for transfer complete

Assertion failed at WEST_TOPDIR/zephyr/tests/drivers/flash/common/src/main.c:91: flash_driver_before: (rc not equal to 0)

Cannot read flash
FAIL - test_read_unaligned_address in 0.220 seconds

TESTSUITE flash_driver failed.

------ TESTSUITE SUMMARY START ------

SUITE FAIL - 0.00% [flash_driver]: pass = 0, fail = 7, skip = 0, total = 7 duration = 1.540 seconds

  • FAIL - [flash_driver.test_flash_copy] duration = 0.220 seconds
  • FAIL - [flash_driver.test_flash_erase] duration = 0.220 seconds
  • FAIL - [flash_driver.test_flash_fill] duration = 0.220 seconds
  • FAIL - [flash_driver.test_flash_flatten] duration = 0.220 seconds
  • FAIL - [flash_driver.test_flash_page_layout] duration = 0.220 seconds
  • FAIL - [flash_driver.test_get_size] duration = 0.220 seconds
  • FAIL - [flash_driver.test_read_unaligned_address] duration = 0.220 seconds

------ TESTSUITE SUMMARY END ------

may I know how you are testing.

@XenuIsWatching
Copy link
Member Author

XenuIsWatching commented May 5, 2025 •
edited
Loading

Hi @XenuIsWatching with your latest changes I see child(flash) attached under spi , spi-nor flash initialization is working fine and its could able to read Flash ID. But as mentioned in #88777 still I see all test cases in tests/drivers/flash/common/ failing, from the failure log, its looks like driver is could able to handle small transfers and failing at large transfers

START - test_read_unaligned_address Test will run on device flash@2 E: Timeout waiting for transfer complete

Assertion failed at WEST_TOPDIR/zephyr/tests/drivers/flash/common/src/main.c:91: flash_driver_before: (rc not equal to 0)

Cannot read flash FAIL - test_read_unaligned_address in 0.220 seconds

TESTSUITE flash_driver failed.

------ TESTSUITE SUMMARY START ------

SUITE FAIL - 0.00% [flash_driver]: pass = 0, fail = 7, skip = 0, total = 7 duration = 1.540 seconds

  • FAIL - [flash_driver.test_flash_copy] duration = 0.220 seconds
  • FAIL - [flash_driver.test_flash_erase] duration = 0.220 seconds
  • FAIL - [flash_driver.test_flash_fill] duration = 0.220 seconds
  • FAIL - [flash_driver.test_flash_flatten] duration = 0.220 seconds
  • FAIL - [flash_driver.test_flash_page_layout] duration = 0.220 seconds
  • FAIL - [flash_driver.test_get_size] duration = 0.220 seconds
  • FAIL - [flash_driver.test_read_unaligned_address] duration = 0.220 seconds

------ TESTSUITE SUMMARY END ------

may I know how you are testing.

Duee to a Non-Disclosure Agreement (NDA), I am limited in the information I can share.
To facilitate a resolution for your specific case, I recommend just debugging this code with this PR stacked and submitting a Pull Request (PR) with the necessary changes if you find anything. This is also a very configurable hardware from the RTL configurations and I dunno what you have versus what I have. I will be happy to review the PR and provide feedback accordingly. See

@singular0770
Copy link
Contributor

singular0770 commented Jun 1, 2025 •
edited
Loading

@XenuIsWatching I'm testing your Cadence SPI driver on a Zynq Ultrascale+ (Cadence SPI R109) and seeing the same timeout waiting for transfer complete as Srikanth. I saw your comment on your previous PR regarding some small changes needed for the R109 version of the IP - can you advise where to look for the differences between the IP versions? I'm struggling to find it on Cadence's website, or are you comparing the Versal/Ultrascale+ TRMs?
Seems to work as expected if I remove cs-gpios from the device tree. With the CS GPIO manually configured I was reading all 0xFF from the SPI device.

@XenuIsWatching
Copy link
Member Author

XenuIsWatching commented Jun 1, 2025 •
edited
Loading

@XenuIsWatching I'm testing your Cadence SPI driver on a Zynq Ultrascale+ (Cadence SPI R109) and seeing the same timeout waiting for transfer complete as Srikanth. I saw your comment on your previous PR regarding some small changes needed for the R109 version of the IP - can you advise where to look for the differences between the IP versions? I'm struggling to find it on Cadence's website, or are you comparing the Versal/Ultrascale+ TRMs?

Seems to work as expected if I remove cs-gpios from the device tree. With the CS GPIO manually configured I was reading all 0xFF from the SPI device.

This IP has Hardware chip selects. If you are using those, you can not be using the cs gpio definitions, otherwise it will be calling the gpio api to toggle the cs pin.

@singular0770
Copy link
Contributor

@XenuIsWatching I'm testing your Cadence SPI driver on a Zynq Ultrascale+ (Cadence SPI R109) and seeing the same timeout waiting for transfer complete as Srikanth. I saw your comment on your previous PR regarding some small changes needed for the R109 version of the IP - can you advise where to look for the differences between the IP versions? I'm struggling to find it on Cadence's website, or are you comparing the Versal/Ultrascale+ TRMs?
Seems to work as expected if I remove cs-gpios from the device tree. With the CS GPIO manually configured I was reading all 0xFF from the SPI device.

This IP has Hardware chip selects. If you are using those, you can not be using the cs gpio definitions, otherwise it will be calling the gpio api to toggle the cs pin.

Realized this upon revisiting this morning. Driver appears to work as is on the Ultrascale+ with R109, even with writing to some bits marked as reserved in UG1085 with SPI_CONF_TXCLR, SPI_CONF_RXCLR, and SPI_CONF_SPSE. Thanks for the work!

@singular0770 singular0770 mentioned this pull request Jun 2, 2025
Open
@dkalowsk dkalowsk merged commit 2bda9ad into zephyrproject-rtos:main Jun 4, 2025
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kartben kartben kartben approved these changes

@teburd teburd teburd approved these changes

@tbursztyka tbursztyka tbursztyka approved these changes

Assignees

@tbursztyka tbursztyka

Labels
area: SPI SPI bus
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@XenuIsWatching @SrikanthGoud123 @singular0770 @kartben @teburd @tbursztyka @dkalowsk