Skip to content

Conversation

@JimMacA
Copy link
Contributor

@JimMacA JimMacA commented Nov 8, 2024

2 changes in pio_i2s.pio (stamped 2584) to delay the transitions on LRCK until the falling edge of BCK. Only for input mode. TI ADCs usually prefer that timing.

Fixing Issue 2584 bug: in input mode, LRCK needs to change on falling edge of BCK
Fixes Issue 2584: I2S input, LRCK should change on falling edge of BCK
@Andy2No
Copy link
Contributor

Andy2No commented Nov 8, 2024

Does the comment on line 153 of the .pio file need updating?

 		.program pio_i2s_in ; Note this is the same as _out, just "in" and not "out"

@JimMacA
Copy link
Contributor Author

JimMacA commented Nov 8, 2024

Not to my mind. My comment was that the change was "only for input mode" because the output mode already has the correct timing.

Copy link
Owner

@earlephilhower earlephilhower left a comment

Choose a reason for hiding this comment

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

Awesome, thank you very much!

Could you apply the same fix to pio_i2s_in_swap for completeness? It's the same program with the BCLK/LRCLK pins reversed.

@JimMacA
Copy link
Contributor Author

JimMacA commented Nov 8, 2024

I'd be happy to make that fix, but.... can someone confirm that swapClocks is working for inputs? It works fine for I2S outputs, but appears to have no effect for inputs. The bits look properly swapped in the PIO input code, so either it's in the C code or I'm doing something silly.

@earlephilhower
Copy link
Owner

Not sure I understand the question, sorry. The "swap clocks" versions here refer to just changing the order of the BCLK/WCLK in the PIO sidesets. So 01 in normal mode becomes 10 in swapped. It's not actually changing the protocol, just swapping the BCLK and LRCLK in the hardware pinout. So a line-for-line copy of the fixed version, but reverse the sideset pins...

@JimMacA
Copy link
Contributor Author

JimMacA commented Nov 9, 2024

You're right; it's a trivial change. The problem came when I tried to test it and discovered that, for whatever reason, the clock-swapped PIO code wasn't being run. Just to make sure, I then ran swapClocks() when setting up the output I2S, which did swap the clocks. So either I'm doing something wrong or there's a bug in the library C code. If this isn't an open issue, I can dive into the library code and see why it's not calling the right PIO routine.
Meanwhile, I can also just make the literal 2-bit change in the PIO code, with the understanding that I can't test it in vivo.

@earlephilhower
Copy link
Owner

Yes, please do the change. We can debug the swapclocks bit elsewhere. Thx!

Issue 2584, extended LRCK for swapped clock input version
Delayed LRCK for swapped I2S in
Copy link
Owner

@earlephilhower earlephilhower left a comment

Choose a reason for hiding this comment

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

Thanks again! With HW-related bugs like this it really helps to have a user submit a tested solution!

@earlephilhower earlephilhower merged commit 64156c4 into earlephilhower:master Nov 9, 2024
26 checks passed
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.

3 participants