Skip to content

Conversation

@jfischer-no
Copy link
Contributor

@jfischer-no jfischer-no commented Apr 10, 2019

Set bwPollTimeout for DFU_GETSTATUS request dynamically.
For now, adjust bwPollTimeout only during DNLOAD stage.

Fixes #8734

@jfischer-no jfischer-no added bug The issue is a bug, or the PR is fixing a bug area: USB Universal Serial Bus labels Apr 10, 2019
@jfischer-no jfischer-no added this to the v1.14.0 milestone Apr 10, 2019
@jfischer-no
Copy link
Contributor Author

jfischer-no commented Apr 10, 2019

I tried to touch as little as possible. Side effects maybe possible.

@Qbicz please test (works for me on reel board)
@tormodvolden FYI

Note, I think not that this PR makes #14662 obsolete.

Copy link
Member

@Qbicz Qbicz left a comment

Choose a reason for hiding this comment

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

@jfischer-phytec-iot thanks. I tested this and I was able to perform DFU with standard dfu-util which has 5 second USB operation timeout on a nrf52840_pca10056 which has longer erase time.

Thanks @tormodvolden for spotting this. I will close the issue in dfu-util once this is merged.

Copy link
Member

Choose a reason for hiding this comment

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

Can we add a prompt so it will be user-configurable in case this is needed to be changed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sorry, the prompt should be there, will fix

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Qbicz fixed

Copy link
Member

Choose a reason for hiding this comment

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

Great. For me PR is fine

Set bwPollTimeout for DFU_GETSTATUS request dynamically.
For now, adjust bwPollTimeout only during DNLOAD stage.

Fixes: zephyrproject-rtos#8734

Signed-off-by: Johann Fischer <[email protected]>
Add config overlay for cdc_acm+dfu composite configuration.
Manual configuration over menuconfig may be error-prone for
the user because then FLASH_LOAD_OFFSET and FLASH_LOAD_SIZE
should also to be set to appropriate values.

Signed-off-by: Johann Fischer <[email protected]>
@jfischer-no jfischer-no force-pushed the pr-usbdfu-polltimeout branch from 626f005 to 1f35592 Compare April 12, 2019 12:29
Copy link
Member

Choose a reason for hiding this comment

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

Great. For me PR is fine

@carlescufi
Copy link
Member

@barsok can you test this today?

@Qbicz
Copy link
Member

Qbicz commented Apr 16, 2019

@jfischer-phytec-iot what's the preferred way to use 2 .conf files together? We tried with @barsok and the only way that worked was to create a CMake list with
cmake -GNinja -DBOARD=$B -DCONF_FILE="overlay-composite-cdc-dfu.conf;prj.conf" ..

@nashif nashif merged commit bf71aee into zephyrproject-rtos:master Apr 16, 2019
@jfischer-no jfischer-no deleted the pr-usbdfu-polltimeout branch April 16, 2019 17:21
@jfischer-no
Copy link
Contributor Author

@Qbicz I do it like this, too.

@barsok
Copy link
Contributor

barsok commented Apr 17, 2019

@carlescufi I have tested the code and I have found presumably unwanted behaviour: [reset] -> dfu read, bwPollTimeout==256 -> dfu download -> error during download, bwPollTimeout remains at 10000, -> dfu upload bwPollTimeout==10000. In English, an error during download does not bring bwPollTimeout to default value

@carlescufi
Copy link
Member

@jfischer-phytec-iot can you confirm the report from @barsok?

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.

USB: DFU timeout on nRF52840 due to long flash erase

5 participants