Re: [PATCH v13 9/9] PCI: Change the type of probe argument in reset functions

From: Bjorn Helgaas <helgaas@kernel.org>
To: Amey Narkhede <ameynarkhede03@gmail.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>,
	alex.williamson@redhat.com,
	Raphael Norwitz <raphael.norwitz@nutanix.com>,
	linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org,
	kw@linux.com, Shanker Donthineni <sdonthineni@nvidia.com>,
	Sinan Kaya <okaya@kernel.org>, Len Brown <lenb@kernel.org>,
	"Rafael J . Wysocki" <rjw@rjwysocki.net>
Subject: Re: [PATCH v13 9/9] PCI: Change the type of probe argument in reset functions
Date: Mon, 2 Aug 2021 17:59:11 -0500	[thread overview]
Message-ID: <20210802225911.GA1473016@bjorn-Precision-5520> (raw)
In-Reply-To: <20210801142518.1224-10-ameynarkhede03@gmail.com>

On Sun, Aug 01, 2021 at 07:55:18PM +0530, Amey Narkhede wrote:
> Change the type of probe argument in functions which implement reset
> methods from int to bool to make the context and intent clear.
> 
> Suggested-by: Alex Williamson <alex.williamson@redhat.com>
> Suggested-by: Krzysztof Wilczyński <kw@linux.com>
> Signed-off-by: Amey Narkhede <ameynarkhede03@gmail.com>
> ---
>  drivers/crypto/cavium/nitrox/nitrox_main.c    |  2 +-
>  .../ethernet/cavium/liquidio/lio_vf_main.c    |  2 +-
>  drivers/pci/hotplug/pciehp.h                  |  2 +-
>  drivers/pci/hotplug/pciehp_hpc.c              |  4 +-
>  drivers/pci/hotplug/pnv_php.c                 |  4 +-
>  drivers/pci/pci-acpi.c                        |  7 +-
>  drivers/pci/pci.c                             | 69 ++++++++++---------
>  drivers/pci/pci.h                             | 12 ++--
>  drivers/pci/pcie/aer.c                        |  2 +-
>  drivers/pci/quirks.c                          | 34 ++++-----
>  include/linux/pci.h                           |  5 +-
>  include/linux/pci_hotplug.h                   |  2 +-
>  12 files changed, 75 insertions(+), 70 deletions(-)
> 
> diff --git a/drivers/crypto/cavium/nitrox/nitrox_main.c b/drivers/crypto/cavium/nitrox/nitrox_main.c
> index 15d6c8452807..f97fa8e997b5 100644
> --- a/drivers/crypto/cavium/nitrox/nitrox_main.c
> +++ b/drivers/crypto/cavium/nitrox/nitrox_main.c
> @@ -306,7 +306,7 @@ static int nitrox_device_flr(struct pci_dev *pdev)
>  		return -ENOMEM;
>  	}
>  
> -	pcie_reset_flr(pdev, 0);
> +	pcie_reset_flr(pdev, PCI_RESET_DO_RESET);
>  
>  	pci_restore_state(pdev);
>  
> diff --git a/drivers/net/ethernet/cavium/liquidio/lio_vf_main.c b/drivers/net/ethernet/cavium/liquidio/lio_vf_main.c
> index 336d149ee2e2..6e666be6907a 100644
> --- a/drivers/net/ethernet/cavium/liquidio/lio_vf_main.c
> +++ b/drivers/net/ethernet/cavium/liquidio/lio_vf_main.c
> @@ -526,7 +526,7 @@ static void octeon_destroy_resources(struct octeon_device *oct)
>  			oct->irq_name_storage = NULL;
>  		}
>  		/* Soft reset the octeon device before exiting */
> -		if (!pcie_reset_flr(oct->pci_dev, 1))
> +		if (!pcie_reset_flr(oct->pci_dev, PCI_RESET_PROBE))
>  			octeon_pci_flr(oct);
>  		else
>  			cn23xx_vf_ask_pf_to_do_flr(oct);
> diff --git a/drivers/pci/hotplug/pciehp.h b/drivers/pci/hotplug/pciehp.h
> index 4fd200d8b0a9..f12e1ef9f183 100644
> --- a/drivers/pci/hotplug/pciehp.h
> +++ b/drivers/pci/hotplug/pciehp.h
> @@ -181,7 +181,7 @@ void pciehp_release_ctrl(struct controller *ctrl);
>  
>  int pciehp_sysfs_enable_slot(struct hotplug_slot *hotplug_slot);
>  int pciehp_sysfs_disable_slot(struct hotplug_slot *hotplug_slot);
> -int pciehp_reset_slot(struct hotplug_slot *hotplug_slot, int probe);
> +int pciehp_reset_slot(struct hotplug_slot *hotplug_slot, bool mode);

Changing this to bool is ok, but I think changing the name from
"probe" to "mode" makes it worse.  It makes sense that "probe == true"
means do a probe instead of a reset, but "mode == true" tells me
nothing.

Bjorn