On 02/01/2023 07:24, Jung Daehwan wrote:
> On Thu, Dec 29, 2022 at 11:25:58AM +0100, Krzysztof Kozlowski wrote:
>> On 29/12/2022 10:57, Daehwan Jung wrote:
>>> Currently, dwc3 invokes just xhci platform driver without any data.
>>> We add xhci node as child of dwc3 node in order to get data from
>>> device tree. It populates "xhci" child by name during initialization
>>> of host. This patch only effects if dwc3 node has a child named "xhci"
>>> not to disturb original path.
>>>
>>> We add "samsung,exynos-xhci" compatible in xhci platform driver
>>
>> Where? It is not documented.
>
> I submitted the patch of dt bindings on same patchset.
> Is there any missing documentation?
This is your first patch in the series and in this patch there is no
such bindings. Re-order the patches to have proper order.
>
>>
>>> to support Exynos SOCs.
>>
>> That's so not true. You do nothing to support Exynos SoC here. Please
>> stop pasting incorrect and misleading commit msgs.
>
> I agree misleading commit msgs. I will modify it.
>
>>
>>> We introduce roothub wakeup, which uses roothub
>>> as system wakeup source. It needs xhci platform driver to override
>>> roothub ops.
>>
>> You did not explain why you introduced wakelocks...
>>
>
> I'm sorry I didn't write description enough.
> I add it below.
>
>>
>> (...)
>>
>>> if (shared_hcd) {
>>> usb_remove_hcd(shared_hcd);
>>> xhci->shared_hcd = NULL;
>>> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
>>> index 79d7931c048a..693495054001 100644
>>> --- a/drivers/usb/host/xhci.c
>>> +++ b/drivers/usb/host/xhci.c
>>> @@ -5502,6 +5502,10 @@ void xhci_init_driver(struct hc_driver *drv,
>>> drv->check_bandwidth = over->check_bandwidth;
>>> if (over->reset_bandwidth)
>>> drv->reset_bandwidth = over->reset_bandwidth;
>>> + if (over->bus_suspend)
>>> + drv->bus_suspend = over->bus_suspend;
>>> + if (over->bus_resume)
>>> + drv->bus_resume = over->bus_resume;
>>> }
>>> }
>>> EXPORT_SYMBOL_GPL(xhci_init_driver);
>>> diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
>>> index c9f06c5e4e9d..cb9c54a6a22c 100644
>>> --- a/drivers/usb/host/xhci.h
>>> +++ b/drivers/usb/host/xhci.h
>>> @@ -1752,6 +1752,8 @@ struct xhci_hub {
>>> struct xhci_hcd {
>>> struct usb_hcd *main_hcd;
>>> struct usb_hcd *shared_hcd;
>>> + struct wakeup_source *main_wakelock;
>>> + struct wakeup_source *shared_wakelock;
>>
>> Drop wakelocks. This is not related to USB and not needed here. Do you
>> see anywhere else in core kernel code usage of the wakelocks?
>>
>> You got this comment already, didn't you? So why you do not address it?
>>
>
> I want to add a new feature in xhci platform driver. I want to make it
> possible to enter system sleep while usb host connected like USB Mouse.
> It gets system enter sleep only if there's no usb transaction at all.
> Deciding if there's tranaction or not is in root hub because it's parent
> of all child usb devices.
I have USB mouse connected to my system and the system enters suspend,
thus I don't think this patch solves this particular issue.
Best regards,
Krzysztof
