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

Fix for c4 spi #7263

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

hattorihanso
Copy link

Description

This pull requests adds a device tree overlay for the Odroid C4 enabling hardware SPI on pins 19, 21, 23 and 24 on the GPIO header. It doesn't require any other changes, except one additional line in the Makefile.
Besides, it can replace the old device tree meson-sm1-odroid-c4-spidev.dtb (which activates wrong pins and doesn't work due to this) while offering more flexibility.

How Has This Been Tested?

The SPI communication has been successfully tested with a LogicPort analyzer and some custom made FPGA board (SPI readout of a Mamamatsu silicon spectrometer).

  • Logic Analyzer: timing of MOSI vs. SCK has been verified, maximum speed is about 12MHz for SCK
  • data transfer between C4 and spectrometer FPGA has been tested

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new warnings
  • Any dependent changes have been merged and published in downstream modules

@github-actions github-actions bot added size/small PR with less then 50 lines Hardware Hardware related like kernel, U-Boot, ... Patches Patches related to kernel, U-Boot, ... labels Sep 16, 2024
@Tonymac32
Copy link
Member

Directionally I agree with this, we shouldn't have any "spidev" or other special devices trees in any kernel that supports overlays. If we can get a second check by anyone with a C4 I think this is a good move.

@Tonymac32 Tonymac32 requested a review from a team September 17, 2024 02:13
@hattorihanso
Copy link
Author

One question not related to that... in case being approved, would it make sense to transfer this patch to 6.11 kernel to make sure it doesn't get lost?

@rpardini
Copy link
Member

we shouldn't have any "spidev" or other special devices trees in any kernel that supports overlays

My fault. I can't spew out overlays, as the syntax remains a mystery to me.

would it make sense to transfer this patch to 6.11 kernel to make sure it doesn't get lost?

Indeed. I think exactly the same change can be done to 6.11/edge.
If not done, eventually edge will replace current, and this will be lost.

@hattorihanso
Copy link
Author

My fault. I can't spew out overlays, as the syntax remains a mystery to me.

No. Your effort! I took the stuff you did and fiddled into an overlay. I don't understand that syntax (I assume only few people really do), and I had some teachings from other gurus here so I could barely do it.

@adeepn
Copy link
Member

adeepn commented Sep 18, 2024

would it make sense to transfer this patch to 6.11 kernel to make sure it doesn't get lost

add patch for 6.11 into this PR, please.

@hattorihanso
Copy link
Author

would it make sense to transfer this patch to 6.11 kernel to make sure it doesn't get lost

add patch for 6.11 into this PR, please.

Could you please advise how to do this? This is my first PR ever...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Hardware Hardware related like kernel, U-Boot, ... Patches Patches related to kernel, U-Boot, ... size/small PR with less then 50 lines
Development

Successfully merging this pull request may close these issues.

4 participants