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

Add dfu-util as a standard upload method #718

Open
wants to merge 10 commits into
base: master
from

Conversation

@jgillick
Copy link
Contributor

jgillick commented Oct 22, 2019

Summary

This PR implements the following features:

  • Adds dfu-util as a standard menu upload option.

This is a follow-up to (and dependant on) PR stm32duino/Arduino_Tools#46.

Motivation
The dfu-util program is already bundled with the stm32duino packages, so setting it as a primary supported upload tool reduces the need for outside software dependencies and gets the end-user setup and running quicker. Additionally, the STM32CubeProgrammer installer is known to have issues on the mac -- there are workarounds, but it's one more hurdle for the end-user to jump through.

Validation

  • Ensure Travis CI build is passed.
  • Upload a sketch to a board using the dfu-util upload option.
Copy link
Member

fpistm left a comment

Some remarks about the PR.
Anyway, I will not merge this until the automatic reset in BL mode is not finished.
I'm also thinking about merge the STM32CubeProgrammer script and the dfu-util.

platform.txt Outdated Show resolved Hide resolved
platform.txt Outdated Show resolved Hide resolved
boards.txt Outdated
GenF4.menu.upload_method.dfuUtil.upload.tool=dfu_util
GenF4.menu.upload_method.dfuUtil.upload.protocol=
GenF4.menu.upload_method.dfuUtil.upload.usbID=0483:df11
GenF4.menu.upload_method.dfuUtil.upload.address=0x08000000:leave

This comment has been minimized.

Copy link
@fpistm

fpistm Oct 23, 2019

Member

I guess this should not be hardcoded if we want flash at another address (ex: user have a bootloader)

This comment has been minimized.

Copy link
@jgillick

jgillick Oct 23, 2019

Author Contributor

That works. How's the best way to make this value dynamic? If the user had a custom bootloader or wanted to flash to a different address, I'd assume they'd be running the command manually from the terminal. (wouldn't it also require the linker script to be updated?) To make it dynamic, would we create a new menu option to choose addresses from?

This comment has been minimized.

Copy link
@fpistm

fpistm Oct 23, 2019

Member

Linker script can already manage this kind of feature as it is preprocessed.
This is already done for BP F103 for example.
flash_offset is currently hardcoded but can be override by a platform.local.txt
Well, this need some investigation 😉

This comment has been minimized.

Copy link
@jgillick

jgillick Oct 24, 2019

Author Contributor

If I understand the flash_offset example, the advice would be to change the variable from upload.address to build.address and likely move the :leave flag to the command in platform.txt. I feel like I must be missing something, though.

Originally I was going to rename this to build. flash_offset to match the examples, but that would offset where in the program flash it was writing to (which would reduce the overall size of the program).

This comment has been minimized.

Copy link
@fpistm

fpistm Oct 25, 2019

Member

If a custom bootloader (or any other custom data) is present the max size of the program is reduced.
It assumes that flash_offset is from the start of the flash.
I wonder if by default the --dfuse-address {upload.address} is required maybe use it only if offset is not null.
This is what is done in maple_upload IIRW.

This comment has been minimized.

Copy link
@matthijskooijman

matthijskooijman Nov 6, 2019

Contributor

It looks like this might be fixed with dfu-util 1.0, which isn't released yet.

I could not find anything in the changelog you linked about this. Are you perhaps referring to this commit? If so, I do not think that fixes this, since it only prevents an address of 0 to be treated the same as "no address specified", but when you specify 0 it will actually try to download to address 0, not the start of the area.

I guess dfu-util could be made smarter to default to the start of the first segment (it already parses the DFU altsetting name (e.g. @Internal Flash /0x08000000/04*016Kg,01*064Kg,03*128Kg) to discover the memory layout (needed for STM32-specific erase additions to the DFU protocol). It does not look the current code already does this (assume the first address), though.

This comment has been minimized.

Copy link
@jgillick

jgillick Nov 7, 2019

Author Contributor

Originally I was referencing this line:

Improve alternate interface index matching (Aleks Chakin)

But, after some research, it doesn't look this would help...

I think I figured out why the alt flag works for Maple. It seems that maple boards use the 1.1 version of the DFU bootloader protocol (source, discussion) and STM32 chips, by default, use 1.1a. The difference is that 1.1 gets the memory addresses from the chip, while the 1.1a protocol requires the host to specify the address explicitly. (more info)

I wish the dfu-util would provide an option to override this -- since it already has these addresses from the USB descriptor -- but alas.

The solution preferred by dfu-util is to provide a dfu file, instead of a bin, which includes all this information embedded in the file (source code). STM32 provides a bin -> dfu file converter, however, that program also requires the starting flash address to work.

Conclusion
So, ultimately, it seems we're left with the requirement to send the flash address with the command (unless I'm missing something). It would be possible to write a script which uses dfu-util list to parse the flash addresses first, but that sounds like overkill and error-prone.

This comment has been minimized.

Copy link
@matthijskooijman

matthijskooijman Nov 7, 2019

Contributor

I think I figured out why the alt flag works for Maple. It seems that maple boards use the 1.1 version of the DFU bootloader protocol (source , discussion) and STM32 chips, by default, use 1.1a. The difference is that 1.1 gets the memory addresses from the chip, while the 1.1a protocol requires the host to specify the address explicitly. (more info)

I actually think it is a little bit different. In the official 1.1 protocol, there is no concept of addresses. You just pass a blob of binary to the bootloader, and the bootloader will decide where to place it. The only way to choose between different placements is using different alts (but again, there is no standard way to map these onto addresses).

The "1.1a" version of the protocol (which, AFAICS is not an official standard but something STM came up with and labeled with a non-standard version number which can now no longer be used for future standard versions) does things differently: You must explicitly tell the bootloader where the binary should be uploaded (in addition, sectors must be explicitly erased before writing, which the DFU standard states should be handled by the bootloader automatically). For this, the programmer needs info about addresses and sector layout, which is encoded in the "name" of the alt (e.g. @Internal Flash /0x08000000/04*016Kg,01*064Kg,03*128Kg encodes the name, address and sector layout in an STM-specific format).

I wish the dfu-util would provide an option to override this -- since it already has these addresses from the USB descriptor -- but alas.

Yes, I think all info should be present and such a change to dfu-util is feasible (probably not even very complicated). I would want to have a stab at this, but it's enough of a priority for me or my customer to actually free up time for this, I'm afraid.

So, ultimately, it seems we're left with the requirement to send the flash address with the command (unless I'm missing something). It would be possible to write a script which uses dfu-util list to parse the flash addresses first, but that sounds like overkill and error-prone.

Agreed.

This comment has been minimized.

Copy link
@matthijskooijman

matthijskooijman Nov 7, 2019

Contributor

If you want to have stab at implementing the auto address selection in dfu-util, let me know and I'll point you to some things I found while reading the source yesterday :-)

This comment has been minimized.

Copy link
@fpistm

fpistm Nov 7, 2019

Member

@matthijskooijman this could be interesting to know 😃

jgillick and others added 3 commits Oct 23, 2019
Co-Authored-By: Frederic Pillon <frederic.pillon@st.com>
Co-Authored-By: Frederic Pillon <frederic.pillon@st.com>
@jgillick

This comment has been minimized.

Copy link
Contributor Author

jgillick commented Oct 23, 2019

The automatic reset in BL mode PR looks cool. Let me know if there's anything I can do to help with that, too.

jgillick added 6 commits Oct 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.