All of lore.kernel.org
 help / color / mirror / Atom feed
From: Bjorn Helgaas <helgaas@kernel.org>
To: Raphael Norwitz <raphael.norwitz@nutanix.com>
Cc: "bhelgaas@google.com" <bhelgaas@google.com>,
	"linux-pci@vger.kernel.org" <linux-pci@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"alex.williamson@redhat.com" <alex.williamson@redhat.com>,
	"ameynarkhede03@gmail.com" <ameynarkhede03@gmail.com>
Subject: Re: [PATCH v2] PCI: merge slot and bus reset implementations
Date: Mon, 24 May 2021 17:31:10 -0500	[thread overview]
Message-ID: <20210524223110.GA1127348@bjorn-Precision-5520> (raw)
In-Reply-To: <20210408182328.12323-1-raphael.norwitz@nutanix.com>

On Thu, Apr 08, 2021 at 06:23:40PM +0000, Raphael Norwitz wrote:
> Slot resets are bus resets with additional logic to prevent a device
> from being removed during the reset. Currently slot and bus resets have
> separate implementations in pci.c, complicating higher level logic. As
> discussed on the mailing list, they should be combined into a generic
> function which performs an SBR. This change adds a function,
> pci_reset_bus_function(), which first attempts a slot reset and then
> attempts a bus reset if -ENOTTY is returned, such that there is now a
> single device agnostic function to perform an SBR.
>
> This new function is also needed to add SBR reset quirks and therefore
> is exposed in pci.h.

I dropped this exposure in pci.h because there's no user yet.  We can
expose it when we add the user.

> Link: https://lkml.org/lkml/2021/3/23/911
>
> Suggested-by: Alex Williamson <alex.williamson@redhat.com>
> Signed-off-by: Amey Narkhede <ameynarkhede03@gmail.com>
> Signed-off-by: Raphael Norwitz <raphael.norwitz@nutanix.com>

Applied to pci/reset for v5.14 with subject
"PCI: Add pci_reset_bus_function() Secondary Bus Reset interface"

I attached the patch as applied.

> ---
>  drivers/pci/pci.c   | 19 +++++++++++--------
>  include/linux/pci.h |  1 +
>  2 files changed, 12 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 16a17215f633..a8f8dd588d15 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -4982,6 +4982,15 @@ static int pci_dev_reset_slot_function(struct pci_dev *dev, int probe)
>  	return pci_reset_hotplug_slot(dev->slot->hotplug, probe);
>  }
>  
> +int pci_reset_bus_function(struct pci_dev *dev, int probe)
> +{
> +	int rc = pci_dev_reset_slot_function(dev, probe);
> +
> +	if (rc != -ENOTTY)
> +		return rc;
> +	return pci_parent_bus_reset(dev, probe);
> +}
> +
>  static void pci_dev_lock(struct pci_dev *dev)
>  {
>  	pci_cfg_access_lock(dev);
> @@ -5102,10 +5111,7 @@ int __pci_reset_function_locked(struct pci_dev *dev)
>  	rc = pci_pm_reset(dev, 0);
>  	if (rc != -ENOTTY)
>  		return rc;
> -	rc = pci_dev_reset_slot_function(dev, 0);
> -	if (rc != -ENOTTY)
> -		return rc;
> -	return pci_parent_bus_reset(dev, 0);
> +	return pci_reset_bus_function(dev, 0);
>  }
>  EXPORT_SYMBOL_GPL(__pci_reset_function_locked);
>  
> @@ -5135,13 +5141,10 @@ int pci_probe_reset_function(struct pci_dev *dev)
>  	if (rc != -ENOTTY)
>  		return rc;
>  	rc = pci_pm_reset(dev, 1);
> -	if (rc != -ENOTTY)
> -		return rc;
> -	rc = pci_dev_reset_slot_function(dev, 1);
>  	if (rc != -ENOTTY)
>  		return rc;
>  
> -	return pci_parent_bus_reset(dev, 1);
> +	return pci_reset_bus_function(dev, 1);
>  }
>  
>  /**
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 86c799c97b77..979d54335ac1 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -1228,6 +1228,7 @@ int pci_probe_reset_bus(struct pci_bus *bus);
>  int pci_reset_bus(struct pci_dev *dev);
>  void pci_reset_secondary_bus(struct pci_dev *dev);
>  void pcibios_reset_secondary_bus(struct pci_dev *dev);
> +int pci_reset_bus_function(struct pci_dev *dev, int probe);
>  void pci_update_resource(struct pci_dev *dev, int resno);
>  int __must_check pci_assign_resource(struct pci_dev *dev, int i);
>  int __must_check pci_reassign_resource(struct pci_dev *dev, int i, resource_size_t add_size, resource_size_t align);
> -- 
> 2.20.1

commit 0dad3ce523c2 ("PCI: Add pci_reset_bus_function() Secondary Bus Reset interface")
Author: Raphael Norwitz <raphael.norwitz@nutanix.com>
Date:   Thu Apr 8 18:23:40 2021 +0000

    PCI: Add pci_reset_bus_function() Secondary Bus Reset interface
    
    pci_parent_bus_reset() resets a device by performing a Secondary Bus Reset
    on a PCI-to-PCI bridge leading to the device.
    
    pci_dev_reset_slot_function() does the same, except that it uses a hotplug
    driver to keep the reset from looking like a hot-remove followed by a
    hot-add.
    
    Add a pci_reset_bus_function() wrapper, which attempts the hotplug driver
    slot reset and falls back to the parent bus reset if that fails.  This
    provides a single interface for performing a Secondary Bus Reset.
    
    [bhelgaas: commit log, don't expose yet]
    Suggested-by: Alex Williamson <alex.williamson@redhat.com>
    Link: https://lore.kernel.org/r/20210323100625.0021a943@omen.home.shazbot.org/
    Link: https://lore.kernel.org/r/20210408182328.12323-1-raphael.norwitz@nutanix.com
    Signed-off-by: Amey Narkhede <ameynarkhede03@gmail.com>
    Signed-off-by: Raphael Norwitz <raphael.norwitz@nutanix.com>
    Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
    Reviewed-by: Leon Romanovsky <leonro@nvidia.com>

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index b717680377a9..452351025a09 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -5020,6 +5020,16 @@ static int pci_dev_reset_slot_function(struct pci_dev *dev, int probe)
 	return pci_reset_hotplug_slot(dev->slot->hotplug, probe);
 }
 
+static int pci_reset_bus_function(struct pci_dev *dev, int probe)
+{
+	int rc;
+
+	rc = pci_dev_reset_slot_function(dev, probe);
+	if (rc != -ENOTTY)
+		return rc;
+	return pci_parent_bus_reset(dev, probe);
+}
+
 static void pci_dev_lock(struct pci_dev *dev)
 {
 	pci_cfg_access_lock(dev);
@@ -5140,10 +5150,7 @@ int __pci_reset_function_locked(struct pci_dev *dev)
 	rc = pci_pm_reset(dev, 0);
 	if (rc != -ENOTTY)
 		return rc;
-	rc = pci_dev_reset_slot_function(dev, 0);
-	if (rc != -ENOTTY)
-		return rc;
-	return pci_parent_bus_reset(dev, 0);
+	return pci_reset_bus_function(dev, 0);
 }
 EXPORT_SYMBOL_GPL(__pci_reset_function_locked);
 
@@ -5173,13 +5180,10 @@ int pci_probe_reset_function(struct pci_dev *dev)
 	if (rc != -ENOTTY)
 		return rc;
 	rc = pci_pm_reset(dev, 1);
-	if (rc != -ENOTTY)
-		return rc;
-	rc = pci_dev_reset_slot_function(dev, 1);
 	if (rc != -ENOTTY)
 		return rc;
 
-	return pci_parent_bus_reset(dev, 1);
+	return pci_reset_bus_function(dev, 1);
 }
 
 /**

      parent reply	other threads:[~2021-05-24 22:31 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-08 18:23 [PATCH v2] PCI: merge slot and bus reset implementations Raphael Norwitz
2021-04-11  9:31 ` Leon Romanovsky
2021-05-24 22:31 ` Bjorn Helgaas [this message]

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=20210524223110.GA1127348@bjorn-Precision-5520 \
    --to=helgaas@kernel.org \
    --cc=alex.williamson@redhat.com \
    --cc=ameynarkhede03@gmail.com \
    --cc=bhelgaas@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=raphael.norwitz@nutanix.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.