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

Support non-standard I2C timings on Pi 5 #5853

Merged
merged 6 commits into from
Jan 29, 2024
Merged

Conversation

pelwell
Copy link
Contributor

@pelwell pelwell commented Jan 16, 2024

This patchset reverts the previous attempt to get more accurate I2C clocks (using hard-coded cycle counts for each standard speed) and replaces it with a simple calculation based on the rise and fall times of the clock signal and the aim of a 50/50 mark/space ratio. With this in place, it removes the requirement to only use standard rates by allowing them to be detuned.

The result appears to give accurate timing over a wide range of speeds - ~3kHz - 3.4MHz.

This reverts commit 660d569.

Signed-off-by: Phil Elwell <phil@raspberrypi.com>
This reverts commit 0a09088.

Signed-off-by: Phil Elwell <phil@raspberrypi.com>
Calculate the HCNT and LCNT values for all modes using the rise and
fall times of SCL, the aim being a 50/50 mark/space ratio.

Signed-off-by: Phil Elwell <phil@raspberrypi.com>
Add support for non-standard bus speeds by treating them as detuned
versions of the slowest standard speed not less than the requested
speed.

Signed-off-by: Phil Elwell <phil@raspberrypi.com>
Add SCL rise and fall times, to allow the derivation of timings at
arbitrary speeds.

Signed-off-by: Phil Elwell <phil@raspberrypi.com>
@JinShil
Copy link
Contributor

JinShil commented Jan 17, 2024

This PR is giving me trouble. I'm testing on i2c-3 and I've only been able to get successful communcation once at 100kHz. But even that's not reproducible. Most of the time, I can't even see a signal on SCL.

I reverted back to #5802 and that seems to work. However, I haven't yet ruled out a problem with my apparatus.

I'll keep testing and report if I learn anything.

@pelwell
Copy link
Contributor Author

pelwell commented Jan 17, 2024

I do hope it's your equipment, otherwise I'll be baffled as well as disappointed.

@JinShil
Copy link
Contributor

JinShil commented Jan 17, 2024

Ok, I was able to get things working again at 100kHz and then 400kHz, but 1MHz is definitely not working.

My config is dtoverlay=i2c3,pins_6_7,baudrate=1000000. I am working with some new equipment from our EEs, but every time I revert back to #5802, everything works. I'll keep looking for something actionable to report.

@JinShil
Copy link
Contributor

JinShil commented Jan 17, 2024

When communication fails, logs show this:

Jan 17 04:40:38 raspberrypi kernel: i2c_designware 1f0007c000.i2c: controller timed out
Jan 17 04:40:36 raspberrypi kernel: i2c_designware 1f0007c000.i2c: controller timed out
Jan 17 04:40:35 raspberrypi kernel: i2c_designware 1f0007c000.i2c: controller timed out

@JinShil
Copy link
Contributor

JinShil commented Jan 17, 2024

I was able to successfully communicate at 100kHz ~ 900kHz in increments of 100kHz. However, once configured at 1MHz communication failed. The most recent failure had this additional error in the logs:

Jan 17 05:19:05 raspberrypi kernel: i2c_designware 1f0007c000.i2c: controller timed out
Jan 17 05:19:04 raspberrypi kernel: i2c_designware 1f0007c000.i2c: i2c_dw_handle_tx_abort: lost arbitration

@pelwell
Copy link
Contributor Author

pelwell commented Jan 17, 2024

Do you have any external pull-ups on GPIOs 6 & 7, and if so what size? Trying without them, I can see that the rise time of SCL takes most of the period, leaving very narrow high pulses. Interestingly, without this PR (i.e. with the I2C driver as of #5802), the trace looks worse.

@JinShil
Copy link
Contributor

JinShil commented Jan 17, 2024

Here's the relevant part of our circuit:

image

As can be seen we have both a level translator and pullups on both ends. Maybe that explains it. Looking at the datasheet for the PCA9306, I'm wondering if we need to find a better part. Still it works fine with #5802, which is curious. I'll chat with our EEs tomorrow.

@pelwell
Copy link
Contributor Author

pelwell commented Jan 17, 2024

When reading all my comments, bear in mind that I'm open to the possibility that this PR might be making things worse and could be improved. Having said that...

Does the PCA9306 really support 1MHz operation? The datasheet subtitle is "2-bit bidirectional 400-kHz I2C/SMBus voltage level translator", and the only references to 1MHz it in the datasheet are about disabling the 9306 if you want to address 1MHz-capable devices on the bus.

@JinShil
Copy link
Contributor

JinShil commented Jan 17, 2024

Right. I noticed the same thing. Reading the datasheet it appears it does not support anything above 400kHz, and even explicitly mentions problems at 1MHz. I'll try to convince our EEs to find a more suitable part and test again.

@pelwell
Copy link
Contributor Author

pelwell commented Jan 29, 2024

Merging to get some wider testing.

@pelwell pelwell merged commit 4c7a8e9 into raspberrypi:rpi-6.1.y Jan 29, 2024
11 of 12 checks passed
@dgedgedge
Copy link

dgedgedge commented Jan 30, 2024

Not yet tested the pull. But comparing PI4 behaviour and PI5 behaviour we found that SDA rising and falling edge where not correctly separated from the rizing falling edge of the SCL as it should.

This is leading to difficulties with addresing components at some quite standard frequencies. But I do imagine that it may have an impact on other frequenceies.
See discussion and conclusion here on raspi forum : "PI5 miss to detect I2c devices where PI4 does".

does the pull corrects this issue ?

@pelwell
Copy link
Contributor Author

pelwell commented Jan 31, 2024

does the pull corrects this issue ?

I've not seen that thread before, so thanks for the link. Proper (suspected is OK) bug reports are best posted here - they are less likely to get missed and easier to track.

No - that potential issue wasn't paid any specific attention when the patch was written, and looking at a scope trace I do see SCL falling at the same time as SDA. The timing looks better when SDA is rising, but it's still not good.

I've opened issue #5914 to track this.

popcornmix added a commit to raspberrypi/firmware that referenced this pull request Feb 8, 2024
See: raspberrypi/linux#5923

kernel: overlays: Delete deprecated overlay mpu6050

kernel: overlays: Correct some compatible strings

kernel: ASoC: DACplusADCPro - fix 16bit sample support in clock consumer mode
See: raspberrypi/linux#5919

kernel: ASoC: adds support for Hifiberry AMP4Pro to the dacplus driver
See: raspberrypi/linux#5918

kernel: ASoC: DACplus - fix 16bit sample support in clock consumer mode
See: raspberrypi/linux#5917

kernel: Improve I2C timing (again)
See: raspberrypi/linux#5916

kernel: Update PiTFT overlays for compatibility and consistency
See: raspberrypi/linux#5903

kernel: Support non-standard I2C timings on Pi 5
See: raspberrypi/linux#5853

kernel: overlays: Add pcie-32bit-dma-pi5-overlay to enable 32bit DMA on the Pi 5's external PCIe interface
See: raspberrypi/linux#5897

kernel: Improvement on backup-switchover-mode overlay value definitions
See: raspberrypi/linux#5884

kernel: Pisound updates for Pi 5
See: raspberrypi/linux#5872
popcornmix added a commit to raspberrypi/rpi-firmware that referenced this pull request Feb 8, 2024
See: raspberrypi/linux#5923

kernel: overlays: Delete deprecated overlay mpu6050

kernel: overlays: Correct some compatible strings

kernel: ASoC: DACplusADCPro - fix 16bit sample support in clock consumer mode
See: raspberrypi/linux#5919

kernel: ASoC: adds support for Hifiberry AMP4Pro to the dacplus driver
See: raspberrypi/linux#5918

kernel: ASoC: DACplus - fix 16bit sample support in clock consumer mode
See: raspberrypi/linux#5917

kernel: Improve I2C timing (again)
See: raspberrypi/linux#5916

kernel: Update PiTFT overlays for compatibility and consistency
See: raspberrypi/linux#5903

kernel: Support non-standard I2C timings on Pi 5
See: raspberrypi/linux#5853

kernel: overlays: Add pcie-32bit-dma-pi5-overlay to enable 32bit DMA on the Pi 5's external PCIe interface
See: raspberrypi/linux#5897

kernel: Improvement on backup-switchover-mode overlay value definitions
See: raspberrypi/linux#5884

kernel: Pisound updates for Pi 5
See: raspberrypi/linux#5872
@JinShil
Copy link
Contributor

JinShil commented Mar 13, 2024

I've received new 1MHz compatible hardware, but I am still having trouble, especially with an MCP23017.

I want to take my unique hardware out of the equation, so I've ordered an MCP23017 DIP package and connected it directly to I2C-1 on the PI5 without any level translator in between. I've tested at 400kHz, and I'm still getting communication errors, but I've noticed something that might help. See the following waveform captures:

Using PR #5916
image

Using PR #5802
image

Could that be the issue?

@6by9
Copy link
Contributor

6by9 commented Mar 13, 2024

The ACK is coming from the slave device, and is the slave pulling the line low.

Seeing the blip go higher is the slave taking slightly longer to respond to the clock edge, so the pull up resistors have slightly longer to discharge the capacitance of SDA and pull it slightly higher, before the slave pulls it low to denote the ACK.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants