Re: A question about the patch: [PATCH] PCI/PM: Keep runtime PM enabled for unbound PCI devices

[mike <qiudayu@linux.vnet.ibm.com>]

On 11/14/2013 01:59 PM, Huang Ying wrote:
> On Thu, 2013-11-14 at 11:23 +0800, mike wrote:
>> On 11/14/2013 03:20 AM, Alan Stern wrote:
>>> On Wed, 13 Nov 2013, Bjorn Helgaas wrote:
>>>
>>>> [+cc Rafael, linux-pm]
>>>>
>>>> On Wed, Nov 13, 2013 at 6:09 AM, mike<qiudayu@linux.vnet.ibm.com>  wrote:
>>>>> Hi Huang Ying,
>>>>>
>>>>> I see you are the author of this patch, commit id is:
>>>>> 967577b062417b4e4b8e27b711220f4124f5153a
>>>>>
>>>>> I have a question while I try to understand this patch,
>>>>> So I would very grateful if you or others can give me some reply.....
>>>>>
>>>>> ............
>>>>> -       rc = ddi->drv->probe(ddi->dev, ddi->id);
>>>>> +       pm_runtime_get_sync(dev);
>>>>> +       pci_dev->driver = pci_drv;
>>>>> ^^^^^^^^^^^^^^^^^^^^^^^^^^
>>>>> I see here you make the driver to initialize before probe,
>>>>> But I have no idea of why you do this change.....
>>>>>
>>>>> and I look inside the code, it may be pm_runtime relate??
>>> Yes, it is related to runtime PM.  In the PCI subsystem, runtime PM
>>> doesn't do anything unless pci_dev->driver is set.  You can see this at
>>> the start of pci_pm_runtime_suspend().
>>>
>>> Since we want the driver's probe routine to be able to carry out
>>> runtime PM operations, we have to set pci_dev->driver before the probe
>>> routine runs.
>> Is there any situations , like in  probe state,  pci_dev->driver
>> has been set. the  pci_pm_runtime_xxx() has passed
>> pci_dev->driver NULL check, but at this point, probe fail
>> occurs, and  pci_dev->driver to be set to NULL.
>>
>> What will happen ? Or this situation will never happen?
>> I'm confuse about this.
> I think that will never happen.  Before ->probe(), pm_runtime_get_sync()
> is called, so pci_pm_runtime_xxx() will not be called until
> pm_runtime_put_noidle() is called in ->probe().  And
>   should be done as one of the latest actions in
> ->probe(), after the normal probe actions succeeded.
OK, just as your description, it seems OK.
But this is really a issue as I explained in last email.

So I want to know if there are any side-effect of changing the code
in pci_pm_runtime_xxx()

  if (!pci_dev->driver)
         return 0;
  to

  if (!dev->driver)
         return 0;

I check the code and find out that, the case  dev->driver is
set but pci_dev->driver is not set just in remove state
(__device_release_driver()), and after that state, all be set
to NULL.

My understand is:
dev->driver = NULL, also means unbound,

am I right?

Thanks
Mike

> Best Regards,
> Huang Ying
>
>