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 some regressions on feather M7 #7953

Merged
merged 1 commit into from
May 12, 2023
Merged

Conversation

jepler
Copy link
Member

@jepler jepler commented May 9, 2023

This closes #7944 but it's fishy -- why is such a large buffer required? @hathach your feedback would be appreciated about this question.

It also includes a board-specific fix for #7952 but this may be needed on other boards. @tannewt do you know what's going on here? The USB_DM/USB_DP pins seem to just be dummy objects on 1011 but adding these to the list (as you did on the 1011 evk) does allow CP to start. If this is correct, the fix should be propagated to ALL boards, or incorporated at a better location.

ports/mimxrt10xx/peripherals/mimxrt10xx/MIMXRT1011/pins.c:
const mcu_pin_obj_t pin_USB_OTG1_DN = { { &mcu_pin_type }, };

@tannewt
Copy link
Member

tannewt commented May 9, 2023

@tannewt do you know what's going on here? The USB_DM/USB_DP pins seem to just be dummy objects on 1011 but adding these to the list (as you did on the 1011 evk) does allow CP to start. If this is correct, the fix should be propagated to ALL boards, or incorporated at a better location.

I don't. I didn't think that the reset code would get this far because it should be bounded by the IOMUXC count. Must be some loop bounds error.

@hathach
Copy link
Member

hathach commented May 9, 2023

This closes #7944 but it's fishy -- why is such a large buffer required? @hathach your feedback would be appreciated about this question.

because usb highspeed endpoint size for bulk is 512 bytes. Maybe the buffer is overflowed or something. I will try to pull out and test tomorrow to see if we could still shrink the memory and not get into trouble.

@jepler jepler requested a review from tannewt May 11, 2023 14:29
@jepler
Copy link
Member Author

jepler commented May 11, 2023

@tannewt can we merge this even if the reason the USB_DM/DP change fixes anything is murky? It's unfortunate and frustrating that metro m7 seems to simply not work at present.

@jepler jepler force-pushed the issue7944-v3 branch 2 times, most recently from 095e020 to ba0735c Compare May 11, 2023 14:52
@gamblor21
Copy link
Member

Tested and this did give me access back to the Metro M7. If things work out I will get a new MIDI keyboard tomorrow and could try the MIDI part.

It's not clear why this is required, but otherwise it stops
midi_intest1 from ever receiving any MIDI events.

Closes: adafruit#7944
@jepler
Copy link
Member Author

jepler commented May 12, 2023

#7964 was the better fix for the not-booting-at-all problem. This is now just to fix the MIDI problem. I only increased the RX (to microcontroller) buffer, and only tested this direction. Before merging, both directions should be tested to find out if they work.

@jepler
Copy link
Member Author

jepler commented May 12, 2023

In my testing, the M7 can now receive (from linux vkeybd) and send (to linux timidity) midi events. So, the send buffer doesn't currently need a size increase, just the receive buffer.

Copy link
Member

@tannewt tannewt left a comment

Choose a reason for hiding this comment

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

Thank you!

@tannewt tannewt merged commit 02d4c81 into adafruit:main May 12, 2023
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.

metro m7 no longer gets midi events
4 participants