Skip to content

USB DFU fixes #11027

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

Merged
merged 5 commits into from
Feb 4, 2019
Merged

USB DFU fixes #11027

merged 5 commits into from
Feb 4, 2019

Conversation

aurel32
Copy link
Collaborator

@aurel32 aurel32 commented Nov 2, 2018

This patchset fixes the DFU firmware upload function and the support for DFU in composite mode. It also fixes the upload code in view of unifying the USB legacy and composite code (issue #10503).

Fixes #12813

@aurel32 aurel32 added bug The issue is a bug, or the PR is fixing a bug area: USB Universal Serial Bus labels Nov 2, 2018
@nashif
Copy link
Member

nashif commented Nov 2, 2018

no need for "subsys: " prefix in commits :)

@codecov-io
Copy link

codecov-io commented Nov 2, 2018

Codecov Report

Merging #11027 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #11027   +/-   ##
=======================================
  Coverage   48.63%   48.63%           
=======================================
  Files         313      313           
  Lines       46435    46435           
  Branches    10710    10710           
=======================================
  Hits        22585    22585           
  Misses      19394    19394           
  Partials     4456     4456

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 19155c3...6b30baf. Read the comment docs.

@aurel32
Copy link
Collaborator Author

aurel32 commented Nov 2, 2018

no need for "subsys: " prefix in commits :)

Ok, I just pushed a new version without it.

@aurel32
Copy link
Collaborator Author

aurel32 commented Nov 5, 2018

recheck

@nashif
Copy link
Member

nashif commented Nov 8, 2018

@finikorg @jfischer-phytec-iot please review

@carlescufi
Copy link
Member

@aurel32 @jfischer-phytec-iot what remains to be done here?

@teburd
Copy link
Collaborator

teburd commented Jan 23, 2019

Would love to have this working in 1.14

@aurel32
Copy link
Collaborator Author

aurel32 commented Jan 23, 2019

@aurel32 @jfischer-phytec-iot what remains to be done here?

We would need the help from @galak to understand the DT part.

@galak
Copy link
Collaborator

galak commented Jan 23, 2019

We would need the help from @galak to understand the DT part.

what do you need help to understand? Ping me on slack if that's easier.

@aurel32 aurel32 added this to the v1.14.0 milestone Jan 30, 2019
@carlescufi
Copy link
Member

@aurel32 do you have plans to fix this for 1.14?

@aurel32
Copy link
Collaborator Author

aurel32 commented Feb 4, 2019

@aurel32 do you have plans to fix this for 1.14?

I think there is nothing to fix, it just needs a rebase. I have done one locally, but I still need to test it.

@aurel32
Copy link
Collaborator Author

aurel32 commented Feb 4, 2019

@jfischer-phytec-iot I have just rebased the PR.

@jfischer-no
Copy link
Collaborator

@aurel32 would you apply @nashif suggestion from #12813?

@aurel32
Copy link
Collaborator Author

aurel32 commented Feb 4, 2019

@aurel32 would you apply @nashif suggestion from #12813?

I am just discovering that issue now. I'll add that patch in the PR.

With an SPI based flash, CONFIG_FLASH_BASE_ADDRESS is not defined. In
that case it is safe to assume that the base address is 0.

Signed-off-by: Aurelien Jarno <[email protected]>
The flash_read() function takes an offset from beginning of the flash.
This patch subtract FLASH_BASE_ADDRESS from the absolute address. This
fixes DFU firmware upload.

Signed-off-by: Aurelien Jarno <[email protected]>
The upload code assumes the buffer to use is the DFU provide one, and
not the one provided by the request handler, which is different in
composite mode.

This is only a theoretical issue, as this function is only executed
once the device has been re-enumerated in legacy mode, but that is a
required step before unifying legacy and composite code.

Signed-off-by: Aurelien Jarno <[email protected]>
In composite mode the request handler buffer is common to all functions,
and its size is defined by CONFIG_USB_COMPOSITE_BUFFER_SIZE.

Given the device is enumerated as composite in runtime mode and
as legacy in download mode, we need to define wTransferSize as
the minimum of the two.

Signed-off-by: Aurelien Jarno <[email protected]>
Given the device can be enumerated as both legacy and composite, we
always need to provide the payload_data buffer.

Signed-off-by: Aurelien Jarno <[email protected]>
@aurel32
Copy link
Collaborator Author

aurel32 commented Feb 4, 2019

@aurel32 would you apply @nashif suggestion from #12813?

I am just discovering that issue now. I'll add that patch in the PR.

Done.

@nashif nashif merged commit 192fe2a into zephyrproject-rtos:master Feb 4, 2019
@aurel32 aurel32 deleted the dfu-fixes branch February 5, 2019 13:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: USB Universal Serial Bus bug The issue is a bug, or the PR is fixing a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DTS: CONFIG_FLASH_BASE_ADDRESS not being generated for SPI based Flash chip
7 participants