Re: [PATCH v4 2/3] dt-bindings: usb: snps,dwc3: Add runtime-suspend-on-usb-suspend property

From: Elson Serrao
Date: Thu Sep 21 2023 - 17:16:19 EST




On 8/30/2023 11:29 PM, Krzysztof Kozlowski wrote:
On 31/08/2023 05:01, Thinh Nguyen wrote:
On Wed, Aug 30, 2023, Krzysztof Kozlowski wrote:
On 30/08/2023 06:31, Elson Serrao wrote:


On 8/29/2023 6:37 PM, Thinh Nguyen wrote:
Just want to clarify, there are dwc3 properties and there are dt binding
properties. Often the case that dt binding matches 1-to-1 with dwc3
driver property. Now, we need to enhance the checkers so that the dwc3
driver property to match cases where it is platform specific and through
compatible string.


Thank you for the clarification Thinh.
To confirm, we would need to modify the driver to parse a new compatible
string (say "snps,dwc3-ext-wakeup") and add .data field so that the
driver is aware that this particular platform supports external wakeup
detection.Right ?

No, it's not then platform specific. You said it depends on each
platform. Platform is Qualcomm SM8450 for example.


Hi Elson,

Use the compatible string of your platform.

e.g.
if (dev->of_node) {
struct device_node *parent = of_get_parent(dev->of_node);

dwc->no_disconnect_on_usb_suspend =
of_device_is_compatible(parent, "qcom,your-compatible-string") ||
of_device_is_compatible(parent, "some-other-platform");
}

You need to enhance dwc3_get_properties(). This may get big as dwc3 adds
more properties. Perhaps you can help come up with ideas to keep this
clean. Perhaps we can separate this out of dwc3 core.c?

HI Thinh

Apologies for the delayed response.
Series https://patchwork.kernel.org/project/linux-usb/cover/1655094654-24052-1-git-send-email-quic_kriskura@xxxxxxxxxxx/ from Krishna K, introduced a dt property 'wakeup-source' which indicates a platforms capability to handle wakeup interrupts. Based on this property, glue drivers can inform dwc3 core that the device is wakeup capable through device_init_wakeup(). For example dwc3-qcom driver informs it like below as per the implementation done in the above series

wakeup_source = of_property_read_bool(dev->of_node, "wakeup-source");
device_init_wakeup(&pdev->dev, wakeup_source);
device_init_wakeup(&qcom->dwc3->dev, wakeup_source);

The dwc3 core now can access this info through device_may_wakeup(dwc->dev) while checking for bus suspend scenario to know whether the platform is capable of detecting wakeup.

Please let me know your thoughts on this approach.

Thanks
Elson