drivers: usb: dwc2: Add support for Synaptics SR100 SoCs #100527
Conversation
|
[EDIT: Ok I see it's for the Synaptics Sr100 from this PR #100172 all good] |
drivers/usb/udc/udc_dwc2.c
Outdated
| #endif | ||
|
|
||
| #define UDC_DWC2_CLOCK_DEFINE(n) \ | ||
| COND_CODE_1(DT_INST_NODE_HAS_PROP(n, clocks), \ |
There was a problem hiding this comment.
You would need to document the clocks property. Currently the PHY clock enable is effectively handled by vendor quirks.
There was a problem hiding this comment.
Good point; should I add a vendor quirk similar to "stm32f4" instead? I'm not sure if the generic clock_control support added in my PR would break the clock handling in the vendor quirks (see, e.g., samples/subsys/usb/cdc_acm/nucleo_f413zh_dwc2.overlay).
The "clocks" phandle is already documented in "base.yaml".
There was a problem hiding this comment.
It really depends on how "typical" a hardware is when it comes to clock control. This generic handling should not break others because others currently do not use clocks property.
The question is also about what clock this is all about. Peripheral clock? PHY clock? Some other dependent clock that hardware needs to have running but is neither peripheral clock nor PHY clock?
There was a problem hiding this comment.
Another important question: is it safe to disable the clock when hibernated?
How should we describe all this? Currently vendor quirks are the method of describing this sort of fine details. Because it can vary quite much between different implementations, it is not easy to determine what should consist of a vendor quirk and what should be achieved via some Zephyr standard mechanism.
There was a problem hiding this comment.
Given these arguments, I'm now more inclined to re-write this PR and implement a vendor quirk for the SR100 platform.
Furthermore, the "clocks" property is used by the stm32f4 quirk (a build with the overlay "samples/subsys/usb/cdc_acm/nucleo_f413zh_dwc2.overlay" would fail), and, as you stated, we don't know if the clock can be disabled. What do you think?
There was a problem hiding this comment.
Using quirks would keep up the current status-quo. Keeping status-quo can potentially make future rework harder, but I just don't really see a way to easily answer all these questions with devicetree entries without over-complicating it.
Add USB device driver support for Synaptics SR100 devices. Signed-off-by: Andreas Weissel <[email protected]>
aweissel
force-pushed
the
dwc2_clock_control
branch
from
7af701a to
f7b9375
Compare
aweissel
changed the title
|
5 participants
Add USB device driver support for Synaptics SR100 devices (clock control and caps) as a vendor quirk for the dwc2 USB-UDC driver.