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

drivers: usb_dc_sam: free endpoint memory on End of Reset event #25243

Merged
merged 2 commits into from
May 13, 2020

Conversation

jfischer-no
Copy link
Collaborator

Free endpoint memory on End of Reset event (EORST).
Fixes: #24626

I also tweaked logging a little to see what happens during configuration, but I can remove commit f689a5a if not desired.

Tweak logging for enable, disable, configure.

Signed-off-by: Johann Fischer <j.fischer@phytec.de>
Free endpoint memory on End of Reset event (EORST).

Fixes: zephyrproject-rtos#24626

Signed-off-by: Johann Fischer <j.fischer@phytec.de>
@jfischer-no jfischer-no added bug The issue is a bug, or the PR is fixing a bug area: Drivers area: USB Universal Serial Bus labels May 12, 2020
@jfischer-no jfischer-no requested a review from finikorg as a code owner May 12, 2020 10:28
@jfischer-no jfischer-no added this to the v2.3.0 milestone May 12, 2020
Copy link
Member

@carlescufi carlescufi left a comment

Choose a reason for hiding this comment

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

LGTM

@carlescufi carlescufi requested a review from stephanosio May 12, 2020 15:12
Copy link
Collaborator

@aurel32 aurel32 left a comment

Choose a reason for hiding this comment

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

According to the datasheet, this should not be needed as the endpoint is supposed to be disabled and its memory de-allocated upon receiving a USB reset. However I confirm this is not what happens on the hardware and that your changes actually fix the issue. Thanks!

Copy link
Member

@nandojve nandojve 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 thank you for the PR.

@@ -469,7 +469,7 @@ int usb_dc_ep_configure(const struct usb_dc_ep_cfg_data *const cfg)
return -EBUSY;
}

LOG_DBG("ep %x, mps %d, type %d", cfg->ep_addr, cfg->ep_mps,
LOG_INF("Configure ep %x, mps %d, type %d", cfg->ep_addr, cfg->ep_mps,
Copy link
Member

Choose a reason for hiding this comment

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

On my view we can keep logs as LOG_DBG.

Copy link
Member

@stephanosio stephanosio left a comment

Choose a reason for hiding this comment

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

Although the reason why EP 1-3 do not get disabled after reset, as stated in the datasheet, may require further investigation, this patch does fix the problem described in #24626.

[00:00:09.270,000] <inf> usb_cdc_acm: USB device configured
>>from EORST ISR<<
EP 0: enabled
EP 1: enabled
EP 2: enabled
EP 3: enabled
EP 4: disabled
EP 5: disabled
EP 6: disabled
EP 7: disabled
EP 8: disabled
EP 9: disabled

@stephanosio stephanosio added the platform: Microchip SAM Microchip SAM Platform (formerly Atmel SAM) label May 13, 2020
@carlescufi carlescufi merged commit 5a3d442 into zephyrproject-rtos:master May 13, 2020
@jfischer-no jfischer-no deleted the pr-fix-24626-sam branch May 13, 2020 10:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Drivers area: USB Universal Serial Bus bug The issue is a bug, or the PR is fixing a bug platform: Microchip SAM Microchip SAM Platform (formerly Atmel SAM)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

USB re-connection fails on SAM E70
5 participants