try to enable ohci for lpc55 but not working, probably clock issue #3294
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR attempts to enable OHCI (Open Host Controller Interface) support for the LPC55 microcontroller family but notes it's not working properly, likely due to clock configuration issues. The implementation refactors the OHCI code to be more modular and adds LPC55 support alongside existing LPC17/40 support.
Key changes include:
- Refactoring OHCI hardware abstraction with a new
ohci_nxp.hheader - Adding LPC55 OHCI configuration and USB0 host mode support
- Restructuring port/speed selection logic in the LPC55 BSP
Reviewed Changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
src/portable/ohci/ohci_nxp.h |
New hardware abstraction layer separating LPC17/40 and LPC55 OHCI implementations |
src/portable/ohci/ohci.h |
Added register offset comments for documentation |
src/portable/ohci/ohci.c |
Replaced direct hardware includes with abstracted header |
src/common/tusb_mcu.h |
Added OHCI support flags for LPC55 |
hw/bsp/lpc55/family.cmake |
Restructured port/speed selection and added OHCI source |
hw/bsp/lpc55/family.c |
Added USB0 host mode initialization with pin configuration |
| Board configuration files | Removed hardcoded PORT settings in favor of new RHPORT system |
| BSP makefiles | Removed old LPC17/40 HCD driver references |
README.rst |
Updated documentation to reflect OHCI/EHCI controller types |
| */ | ||
| #ifndef TUSB_OHCI_NXP_H | ||
| #define TUSB_OHCI_NXP_H | ||
|
|
There was a problem hiding this comment.
Missing include for tusb_option.h which is required for the TU_CHECK_MCU macro. This will cause compilation errors.
| #include "tusb_option.h" |
Copilot uses AI. Check for mistakes.
| #else | ||
|
|
||
| #include "fsl_device_registers.h" |
There was a problem hiding this comment.
The #else block assumes any non-LPC17/40 MCU is LPC55, but this may not be true for future OHCI implementations. Consider adding an explicit MCU check for LPC55 to make the condition more specific.
Copilot uses AI. Check for mistakes.
| #endif | ||
|
|
||
| // USB VBUS | ||
| #if (CFG_TUD_ENABLED && BOARD_TUD_RHPORT == 0) || (CFG_TUH_ENABLED && BOARD_TUH_RHPORT == 0) |
There was a problem hiding this comment.
This complex conditional logic is repeated multiple times in the file. Consider extracting it into a macro or helper function to improve readability and maintainability.
Copilot uses AI. Check for mistakes.
|
|
||
| /* enable USB Device clock */ | ||
| CLOCK_EnableUsbfs0DeviceClock(kCLOCK_UsbfsSrcFro, CLOCK_GetFreq(kCLOCK_FroHf)); | ||
| if (BOARD_TUD_RHPORT == 0) { |
There was a problem hiding this comment.
This condition should also check if device mode is enabled (CFG_TUD_ENABLED) to avoid unnecessary initialization when only host mode is used.
Copilot uses AI. Check for mistakes.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting
| #if (CFG_TUD_ENABLED && BOARD_TUD_RHPORT == 1) || (CFG_TUH_ENABLED && BOARD_TUH_RHPORT == 1) | ||
| // Port1 is High Speed | ||
|
|
||
| /* Turn on USB1 Phy */ |
There was a problem hiding this comment.
The initialization for USB1 now runs when either the device or host selects port 1, but the body still configures the PHY and clocks for device mode (USBHSH->PORTMODE |= USBHSH_PORTMODE_DEV_ENABLE_MASK, device clock enables). For host builds that set BOARD_TUH_RHPORT == 1, this code flips the controller into device mode and never enables host clocks, which will prevent the OHCI host from ever running. Previously the block only executed for device builds.
Useful? React with 👍 / 👎.
1 participant
No description provided.