Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1606291

Re: [PATCH v5 1/2] usb: host: plat: Enable xhci plat runtime PM

From Felipe Balbi <balbi@kernel.org>
Newsgroups linux.kernel
Subject Re: [PATCH v5 1/2] usb: host: plat: Enable xhci plat runtime PM
Date 2017-03-22 10:10 +0100
Message-ID <tnKrg-8tF-29@gated-at.bofh.it> (permalink)
References (5 earlier) <tcXLc-6gN-9@gated-at.bofh.it> <td83U-4q0-3@gated-at.bofh.it> <tnmRY-fw-9@gated-at.bofh.it> <tnn1E-xY-19@gated-at.bofh.it> <tnGGZ-5HX-1@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


[Multipart message — attachments visible in raw view] - view raw

Hi,

Baolin Wang <baolin.wang@linaro.org> writes:
>>>>> I don't yet understand why we can't just keep runtime pm disabled as a
>>>>> default for xhci platform devices.
>>>>> It could be enabled by whatever creates the platform device by setting some
>>>>> device property
>>>>> (or equivalent), which would be checked in xhci_plat_probe() before enabling
>>>>> runtime pm. It
>>>>> could then optionally be set in sysfs via power/control entry.
>>>>
>>>> I think runtime pm is not one hardware property, is it suitable if we
>>>> introduce one device property to enable/disable runtime pm?
>>>
>>> As I said, runtime pm is not one hardware property, I think it is not
>>> suitable if we introduce one device property to enable/disable runtime
>>> pm.
>>
>> we already this functionality exposed on sysfs.
>
> From my understanding, Mathias suggested me to add one device property
> (name like "usb-host-runtimePM") by platform_device_add_properties()
> to enable/disable runtime PM when creating platform device, like
> usb3_lpm_capable:
>
> if (dwc->usb3_lpm_capable)
>       props[prop_idx++].name = "usb3-lpm-capable";
>
> ret = platform_device_add_properties(xhci, props);
> if (ret) {
>       dev_err(dwc->dev, "failed to add properties to xHCI\n");
>       goto err1;
> }
>
> What I think It is not suitable to introduce one device property like
> above to enable/disable runtime PM, it is not one hardware attribute.

yeah, that's silly. We already have means for doing that:

	my_probe()
        {
		[...]

		pm_runtime_enable(dev);
                pm_runtime_forbid(dev);

		[...]

		return 0;
	}

>>> Secondly, we only can resume the xhci platform device by getting the
>>> xhci usage counter from gadget driver, since the cable plug in/out
>>> events only can be notified to glue layer of gadget driver(like dwc3
>>> glue layer). That means if we want to suspend xhci platform device, we
>>
>> this is a problem with the glue layer, IMO. It should be something like
>> so:
>>
>> static irqreturn_t dwc3_foobar_wakeup(int irq, void *_glue)
>> {
>>         struct dwc3_foobar_glue *glue = _glue;
>>         u32 reg;
>>
>>         reg = real(glue->base, OFFSET);
>>         if (reg & CONNECT)
>>                 pm_runtime_resume(&glue->dwc3);
>>
>>         return IRQ_HANDLED;
>> }
>>
>> then dwc3's ->runtime_resume() should check if the event is supposed to
>> be handled by host or peripheral by checking which mode it was before
>> suspend and making the assumption that we don't change modes while
>> suspended. Something like:
>>
>> static int dwc3_runtime_resume(struct device *dev)
>> {
>>         struct dwc3 *dwc = dev_get_drvdata(dev);
>>
>>         [...]
>>
>>         if (dwc->is_host)
>>                 pm_runtime_resume(dwc->xhci.dev);
>>
>>         [...]
>>
>>         return 0;
>> }
>
> Yeah, if we don't need to get xhci usage counter in xhci_plat_probe()
> to avoid affecting other controller's runtime PM, we can do like this
> and do not need to get/put counter.

why do you need to get xhci's usage counter in xhci_plat_probe() ?

And why would one xhci affect the other? They are different struct
device instances and, thus, have different pm usage counter. How would
one xhci's pm_runtime affect another?

PM runtime is not per driver, it's per device.

-- 
balbi

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

Re: [PATCH v5 1/2] usb: host: plat: Enable xhci plat runtime PM Baolin Wang <baolin.wang@linaro.org> - 2017-03-21 09:00 +0100
  Re: [PATCH v5 1/2] usb: host: plat: Enable xhci plat runtime PM Felipe Balbi <balbi@kernel.org> - 2017-03-21 09:10 +0100
    Re: [PATCH v5 1/2] usb: host: plat: Enable xhci plat runtime PM Baolin Wang <baolin.wang@linaro.org> - 2017-03-22 06:10 +0100
      Re: [PATCH v5 1/2] usb: host: plat: Enable xhci plat runtime PM Felipe Balbi <balbi@kernel.org> - 2017-03-22 10:10 +0100
        Re: [PATCH v5 1/2] usb: host: plat: Enable xhci plat runtime PM Baolin Wang <baolin.wang@linaro.org> - 2017-03-22 11:50 +0100
          Re: [PATCH v5 1/2] usb: host: plat: Enable xhci plat runtime PM Mathias Nyman <mathias.nyman@linux.intel.com> - 2017-03-22 13:50 +0100
            Re: [PATCH v5 1/2] usb: host: plat: Enable xhci plat runtime PM Baolin Wang <baolin.wang@linaro.org> - 2017-03-23 03:20 +0100

csiph-web