On 23/08/2023 01:58, Elson Serrao wrote: > > > On 8/19/2023 2:35 AM, Krzysztof Kozlowski wrote: >> On 18/08/2023 21:16, Elson Serrao wrote: >>> >>> >>> On 8/15/2023 10:44 PM, Krzysztof Kozlowski wrote: >>>> On 14/08/2023 20:50, Elson Roy Serrao wrote: >>>>> This property allows dwc3 runtime suspend when bus suspend interrupt >>>>> is received even with cable connected. This would allow the dwc3 >>>>> controller to enter low power mode during bus suspend scenario. >>>>> >>>>> This property would particularly benefit dwc3 IPs where hibernation is >>>>> not enabled and the dwc3 low power mode entry/exit is handled by the >>>>> glue driver. The assumption here is that the platform using this dt >>>>> property is capable of detecting resume events to bring the controller >>>>> out of suspend. >>>>> >>>>> Signed-off-by: Elson Roy Serrao <quic_eserrao@xxxxxxxxxxx> >>>>> --- >>>>> Documentation/devicetree/bindings/usb/snps,dwc3.yaml | 5 +++++ >>>>> 1 file changed, 5 insertions(+) >>>>> >>>>> diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml >>>>> index a696f23730d3..e19a60d06d2b 100644 >>>>> --- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml >>>>> +++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml >>>>> @@ -403,6 +403,11 @@ properties: >>>>> description: >>>>> Enable USB remote wakeup. >>>>> >>>>> + snps,runtime-suspend-on-usb-suspend: >>>>> + description: >>>>> + If True then dwc3 runtime suspend is allowed during bus suspend >>>>> + case even with the USB cable connected. >>>> >>>> This was no tested... but anyway, this is no a DT property but OS >>>> policy. There is no such thing as "runtime suspend" in the hardware, >>>> because you describe one particular OS. >>>> >>>> Sorry, no a DT property, drop the change entirely. >>>> >>>> >>> Hi Krzysztof >>> >>> Sorry my local dt checker had some issue and it did not catch these >>> errors. I have rectified it now. >>> >>> This dt property is mainly for skipping dwc3 controller halt when a USB >>> suspend interrupt is received with usb cable connected, so that we dont >>> trigger a DISCONNECT event. Perhaps a better name would reflect the true >>> usage of this? >>> >>> Something like snps,skip-dwc3-halt-on-usb-suspend. dwc3 cores where >>> hibernation feature is not enabled/supported can use this property >> >> So this is specific to DWC3 core, thus should be just implied by compatible. >> > > Hi Krzysztof > > Apologies for not being clear. Below is the reasoning behind this dt entry. > > When bus suspend interrupt is received and if usb cable is connected, > dwc3 driver does not suspend. The aim of this series is to handle this > interrupt when USB cable is connected to achieve power savings. OEMs > might have their own implementation in their glue driver to turn off > clocks and other resources when USB is not in use, thus saving power. > But since glue layer has dependency on dwc3 driver (parent-child > relationship) we need to allow dwc3 driver to NOT ignore the bus suspend > interrupt and let the dwc3 driver suspend (so that glue driver can > suspend as well) All this describes current OS implementation so has nothing to do with bindings. > > Now it is the responsibility of glue driver to detect USB wakeup signal > from the host during resume (since dwc3 driver is suspended at this > point and cannot handle interrupts). Every OEM may not have the > capability to detect wakeup signal. Again, driver architecture. > The goal of this dt property is for > the dwc3 driver to allow handling of the bus suspend interrupt when such DT properties are not "for the drivers". > a capability exists on a given HW platform. When this property is Each platform has this capability. If not, then it is compatible-related, as I said before which you did not address. > defined dwc3 driver knows that the low power mode entry/exit is > controlled by the glue driver and thus it can allow the suspend > operation when bus suspend interrupt is received. > > For example on Qualcomm platforms there is a phy sideband signalling > which detects the wakeup signal when resume is initiated by the host. So compatible-specific. > Thus qcom platforms can benefit from this feature by defining this dt > property. (in a parallel discussion with Thinh N to come up with a > better name for this dt entry). Thanks, with quite a long message you at the end admitted this is compatible-specific. Exactly what I wrote it one sentence previously. Best regards, Krzysztof
