Skip to content

Conversation

@roma-jam
Copy link
Contributor

@roma-jam roma-jam commented Nov 26, 2025

Description

During the recent update (#3326)

dcd_deinit(rhport); -> TU_VERIFY(dcd_deinit(rhport));

the assertion fails for the ports, where dcd_deinit() isn't implemented yet.

Changes

This PR introduces a quick fix by changing the default return value from false to true in the WEAK function, allowing the assert to pass.

The same way, as we have it for:

TU_ATTR_WEAK bool dcd_dcache_clean(const void* addr, uint32_t data_size) {
  (void) addr; (void) data_size;
  return true;
}

TU_ATTR_WEAK bool dcd_dcache_invalidate(const void* addr, uint32_t data_size) {
  (void) addr; (void) data_size;
  return true;
}

TU_ATTR_WEAK bool dcd_dcache_clean_invalidate(const void* addr, uint32_t data_size) {
  (void) addr; (void) data_size;
  return true;
}

Context

The real implementation may vary, but for dcd_dwc2 (for example), this might still be not implemented, as we re-configure the controller fully during the following dcd_init().

Copilot AI review requested due to automatic review settings November 26, 2025 22:13
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes an assertion failure in ports where dcd_deinit() is not yet implemented by changing the weak function's default return value from false to true. This aligns with the pattern used by other weak DCD cache functions and allows the TU_VERIFY(dcd_deinit(rhport)) assertion to pass when the function hasn't been overridden.

Key changes:

  • Modified the weak dcd_deinit() function to return true instead of false by default

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@HiFiPhile
Copy link
Collaborator

I think we can take the chance to add dcd_deinit to dwc2, disable interrupt and disconnect may be enough ?

@roma-jam
Copy link
Contributor Author

Hi @HiFiPhile,

I think we can take the chance to add dcd_deinit to dwc2

Sure, that is my next step.

This PR is about another thing: the dcd_deinit() might be optional, as the dcd_init() could have all the necessary code to start from two states: was init done or wasn't init done.
So in this case, if we have an TU_ASSERT() for the optional call, we need to make it pass even if the function isn't implemented.

@HiFiPhile
Copy link
Collaborator

HiFiPhile commented Nov 27, 2025

My concern is maybe other port require additional tweak to proper deinit, returning true will hide the issue. I prefer to make TU_ASSERT(dcd_deinit(rhport)); in usbd.c to be safe and in dwc2:

dcd_deinit(uint8_t rhport) {
  (void) rhport;
  // to be filled later
  return true;
}

@hathach What do you think ?

We also need to add tu_varclr(&_usbd_dev); to tud_deinit

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants