All of lore.kernel.org
 help / color / mirror / Atom feed
From: Vidya Sagar <vidyas@nvidia.com>
To: Sinan Kaya <okaya@kernel.org>, Thierry Reding <treding@nvidia.com>
Cc: <bhelgaas@google.com>, <lorenzo.pieralisi@arm.com>,
	<jonathanh@nvidia.com>, <linux-tegra@vger.kernel.org>,
	<linux-pci@vger.kernel.org>, <kthota@nvidia.com>,
	<mmaddireddy@nvidia.com>, <sagar.tv@gmail.com>,
	"Rafael J. Wysocki" <rjw@rjwysocki.net>
Subject: Re: [PATCH] PCI: Add CRS timeout for pci_device_is_present()
Date: Tue, 15 Oct 2019 17:44:47 +0530	[thread overview]
Message-ID: <85267afb-c08e-5625-d3ee-bd32af9ecb12@nvidia.com> (raw)
In-Reply-To: <4953b718-8818-575e-2ec1-8197e6b32593@kernel.org>

On 10/15/2019 4:40 PM, Sinan Kaya wrote:
> +Rafael
> 
> On 10/15/2019 2:30 AM, Thierry Reding wrote:
>> Vidya, can you clarify for which device you're seeing the issues? Sounds
>> like adding a call to pci_pm_reset() (via pci_reset_function()) at some
>> point.
>>
>> Sinan, it looks as if pci_pm_reset() (or any of its callers) is not used
>> very widely. Is that just because most drivers haven't had a need for it
>> yet? Or am I missing some core functionality that calls this for every
>> device anyway?
> 
> pci_pm_reset() is there as an alternative reset path. We are not
> supposed to call this function. Sorry for giving you wrong direction
> here. pci_reset_function() should call it only if there is no other
> suitable reset function is found.
> 
> I think the PCI core should be putting the device back D0 state as one
> of the first actions before enumerating. Wake up could be a combination
> of ACPI and/or PCI wake up depending on where your device sits in the
> topology.
Yup. It is indeed doing it as part of pci_power_up() in pci.c file.
But, what is confusing to me is the order of the calls.
pci_power_up() has following calls in the same order.
	pci_raw_set_power_state(dev, PCI_D0);
	pci_update_current_state(dev, PCI_D0);
But, pci_raw_set_power_state() is accessing config space without calling
pci_device_is_present() whereas pci_update_current_state() which is called
later in the flow is calling pci_device_is_present()...!

> 
> On the other hand, wake up code doesn't perform the CRS wait. CRS
> wait is deferred until the first vendor id read in pci_scan_device().
> I see that it already waits for 60 seconds.
> 
> Going back to the patch...
> 
> I think we need to find the path that actually needs this sleep and
> put pci_dev_wait() there.
Following is the path in resume() flow.
[   36.380726] Call trace:
[   36.383270]  dump_backtrace+0x0/0x158
[   36.386802]  show_stack+0x14/0x20
[   36.389749]  dump_stack+0xb0/0xf8
[   36.393451]  pci_update_current_state+0x58/0xe0
[   36.398178]  pci_power_up+0x60/0x70
[   36.401672]  pci_pm_resume_noirq+0x6c/0x130
[   36.405669]  dpm_run_callback.isra.16+0x20/0x70
[   36.410248]  device_resume_noirq+0x120/0x238
[   36.414364]  async_resume_noirq+0x24/0x58
[   36.418364]  async_run_entry_fn+0x40/0x148
[   36.422418]  process_one_work+0x1e8/0x360
[   36.426525]  worker_thread+0x40/0x488
[   36.430201]  kthread+0x118/0x120
[   36.433843]  ret_from_fork+0x10/0x1c

> 
> +++ b/drivers/pci/pci.c
> @@ -5905,7 +5905,8 @@ bool pci_device_is_present(struct pci_dev *pdev)
> 
>   	if (pci_dev_is_disconnected(pdev))
>   		return false;
> -	return pci_bus_read_dev_vendor_id(pdev->bus, pdev->devfn, &v, 0);
> +	return pci_bus_read_dev_vendor_id(pdev->bus, pdev->devfn, &v,
> +					  PCI_CRS_TIMEOUT);
>   }
> 
> pci_device_is_present() is a too low-level function and it may not
> be allowed to sleep. It uses 0 as timeout value.
> 
> 


  reply	other threads:[~2019-10-15 12:15 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-05 18:21 [PATCH] PCI: Add CRS timeout for pci_device_is_present() Vidya Sagar
2019-10-14  8:20 ` Thierry Reding
2019-10-14 20:21   ` Sinan Kaya
2019-10-15  9:30     ` Thierry Reding
2019-10-15 11:10       ` Sinan Kaya
2019-10-15 12:14         ` Vidya Sagar [this message]
     [not found]           ` <afa16546-e63d-6eba-8be0-8e52339cd100@nvidia.com>
2019-10-25 11:58             ` Vidya Sagar
2019-10-26 13:59               ` Sinan Kaya
2019-11-04 11:43                 ` Vidya Sagar
2019-11-04 16:52                   ` Lorenzo Pieralisi
2019-11-04 17:39           ` Bjorn Helgaas
2019-11-05 10:55             ` Rafael J. Wysocki
2019-11-06 16:41               ` Bjorn Helgaas
2019-11-11  6:01                 ` Vidya Sagar
2019-11-11 22:32                   ` Bjorn Helgaas
2019-11-12 12:59                     ` Thierry Reding
2019-11-12 14:21                       ` Bjorn Helgaas
2019-11-12 17:59                         ` Vidya Sagar
2019-11-12 18:58                           ` Bjorn Helgaas
2019-11-13  5:39                             ` Vidya Sagar
2019-11-13 11:20                             ` Thierry Reding
2019-11-14 18:36                               ` Bjorn Helgaas
2019-11-15 10:04                                 ` Vidya Sagar
2019-11-15 22:36                                   ` Bjorn Helgaas
2019-11-18 15:18                                     ` Vidya Sagar
2019-11-12 17:59                     ` Vidya Sagar
2019-10-15 12:03       ` Vidya Sagar
2019-10-15 11:34     ` Vidya Sagar
2019-10-14 10:45 ` Andrew Murray

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=85267afb-c08e-5625-d3ee-bd32af9ecb12@nvidia.com \
    --to=vidyas@nvidia.com \
    --cc=bhelgaas@google.com \
    --cc=jonathanh@nvidia.com \
    --cc=kthota@nvidia.com \
    --cc=linux-pci@vger.kernel.org \
    --cc=linux-tegra@vger.kernel.org \
    --cc=lorenzo.pieralisi@arm.com \
    --cc=mmaddireddy@nvidia.com \
    --cc=okaya@kernel.org \
    --cc=rjw@rjwysocki.net \
    --cc=sagar.tv@gmail.com \
    --cc=treding@nvidia.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.