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

STM32F4 USB OTG Host msc #20299

Closed

Conversation

bigtreetech
Copy link
Contributor

@bigtreetech bigtreetech commented Nov 27, 2020

Requirements

  • the USB OTG host and USB CDC emulated serial port share the same hardware port on SKR Pro and GTR (OTG_FS: PA11, PA12), So U disk and emulated serial port cannot be used at the same time. you can't set SERIAL_PORT or SERIAL_PORT_2 to -1 when enabled USB_HOST_MSC_FLASH_SUPPORT. and you need comment out -DUSBCON -DUSBD_USE_CDC in platformio.ini
    -DUSBCON -DUSBD_USE_CDC

    Since this is enabled, the dependency library of STM32 will occupy void OTG_FS_IRQHandler (void) function, resulting in conflict, compilation failed.

Description

  • This PR realizes the feature of printing gcode from U disk on STM32F4 motherboard(SKR Pro, GTR, etc..).
  • When USB OTG Host is available in STM32 dependency library, Marlin\src\HAL\STM32\usb_hostlib\STM32_USB_Host_Library can be deleted in Marlin, See USB Host + MSC stm32duino/Arduino_Core_STM32#1196

Benefits

Configurations

uncomment

#define SDSUPPORT
#define USB_HOST_MSC_FLASH_SUPPORT

Related Issues

* USB Flash mounted in MCU USB OTG Host port
* you need uncomment both SDSUPPORT and following option at the same time or it won't work.
*/
//#define USB_HOST_MSC_FLASH_SUPPORT
Copy link
Member

Choose a reason for hiding this comment

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

Rather than leave it up to the user to set this, it would be better to enable it automatically when it is required.

@rhapsodyv rhapsodyv added Needs: Discussion Discussion is needed S: Don't Merge Work in progress or under discussion. labels Nov 27, 2020
@thisiskeithb
Copy link
Member

I requested a review from @rhapsodyv because I knew he was working on a different (cleaner) implementation that doesn't pull in code from the stm32duino/Arduino_Core_STM32 framework.

I tested some of his early code on an SKR Pro and GTR and thumbdrive support seemed to work fine, but there is still more work to be done within Marlin & in the referenced Arduino_Core_STM32 PR.

@rhapsodyv
Copy link
Member

Hi @bigtreetech,

As I already have this work done and I'm the autor of the stm32duino PR, I would like to ask some questions. The idea is to sum your work with mine, so we can get the best of both implementations.

  1. Why are you coping external code to Marlin? Recent PIO versions have a way to just point the a lib in a ZIP format from GitHub. In this case, we can just create a new env pointing to the stm32duino PR.
    platform_packages = framework-arduinoststm32@https://github.com/rhapsodyv/Arduino_Core_STM32/archive/usb-host-msc.zip

  2. Why are you creating a new config, when Marlin already have USB_FLASH_DRIVE_SUPPORT? I used USB_FLASH_DRIVE_SUPPORT, just adding a new option to it: USE_OTG_USB

  3. Giving instructions to users change platformio.ini don't work in practice and lead to unnecessary support. We can create a new env for custom behaviour/libs, or preferably, use BOARD_CUSTOM_BUILD_FLAGS to change build flags according with user options.

  4. Do you need this feature in Marlin asap? I'm asking that, because my PR to stm32duino is about to be merged.
    It's only wasn't merged it, because stm32duino maintainer ask me to do a new test: How OTG FS/HS work when mixed with HOST and DEVICE mode (HS in HCD and FS in PCD). In about two weeks I will receive a board to test both and mixed modes, so stm32duino can do all changes at once and merge my PR.
    Do you need it merged before that? Or can wait?

Thanks!

@bigtreetech
Copy link
Contributor Author

  1. Do you need this feature in Marlin asap? I'm asking that, because my PR to stm32duino is about to be merged.
    It's only wasn't merged it, because stm32duino maintainer ask me to do a new test: How OTG FS/HS work when mixed with HOST and DEVICE mode (HS in HCD and FS in PCD). In about two weeks I will receive a board to test both and mixed modes, so stm32duino can do all changes at once and merge my PR.
    Do you need it merged before that? Or can wait?

@rhapsodyv Hello
For 4. Can you confirm the approximate time of stm32duino/Arduino_Core_STM32#1196 merged? It will be great if it doesn't take long time.

And for 1,2,3. I can clean it up and pull it again. And one more thing when your PR is merged. Do we need to specify a new platform version number to take it effect?

If your PR doesn't take too long to merge, this PR can remain open so that someone needs to use it.

@rhapsodyv
Copy link
Member

I just received the board to complete the PR to stm32duino. Next week I guess everything will be merged.

@rhapsodyv
Copy link
Member

#20571 was merged. GTR and SKR PRO support were added too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs: Discussion Discussion is needed S: Don't Merge Work in progress or under discussion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants