Re: [PATCH] pci: drop link_reset

[Doug Ledford]

On Wed, 2017-01-18 at 23:39 +0200, Michael S. Tsirkin wrote:
> No hardware seems to actually call link_reset, and
> no driver implements it as more than a nop stub.
> 
> This drops the mentions of the callback from everywhere.
> It's dropped from the documentation as well, but
> the doc really needs to be updated to reflect
> reality better (e.g. on pcie slot reset is the link reset).
> 
> This will be done in a later patch.
> 
> Signed-off-by: Michael S. Tsirkin <mst@xxxxxxxxxx>

This is going to conflict with the two patches I have in my for-next
branch related to this same thing (it drops the stubs from qib and
hfi1). ÂIt would be easiest if I just added this to my for-next and
fixed up the conflicts prior to submission.

> ---
> ÂDocumentation/PCI/pci-error-recovery.txt | 24 +++-------------------
> --
> Âdrivers/infiniband/hw/hfi1/pcie.cÂÂÂÂÂÂÂÂ| 10 ----------
> Âdrivers/infiniband/hw/qib/qib_pcie.cÂÂÂÂÂ|ÂÂ8 --------
> Âdrivers/media/pci/ngene/ngene-cards.cÂÂÂÂ|ÂÂ7 -------
> Âinclude/linux/pci.hÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ|ÂÂ3 ---
> Â5 files changed, 3 insertions(+), 49 deletions(-)
> 
> diff --git a/Documentation/PCI/pci-error-recovery.txt
> b/Documentation/PCI/pci-error-recovery.txt
> index ac26869..da3b217 100644
> --- a/Documentation/PCI/pci-error-recovery.txt
> +++ b/Documentation/PCI/pci-error-recovery.txt
> @@ -78,7 +78,6 @@ struct pci_error_handlers
> Â{
> Â	int (*error_detected)(struct pci_dev *dev, enum
> pci_channel_state);
> Â	int (*mmio_enabled)(struct pci_dev *dev);
> -	int (*link_reset)(struct pci_dev *dev);
> Â	int (*slot_reset)(struct pci_dev *dev);
> Â	void (*resume)(struct pci_dev *dev);
> Â};
> @@ -104,8 +103,7 @@ if it implements any, it must implement
> error_detected(). If a callback
> Âis not implemented, the corresponding feature is considered
> unsupported.
> ÂFor example, if mmio_enabled() and resume() aren't there, then it
> Âis assumed that the driver is not doing any direct recovery and
> requires
> -a slot reset. If link_reset() is not implemented, the card is
> assumed to
> -not care about link resets. Typically a driver will want to know
> about
> +a slot reset.ÂÂTypically a driver will want to know about
> Âa slot_reset().
> Â
> ÂThe actual steps taken by a platform to recover from a PCI error
> @@ -232,25 +230,9 @@ proceeds to STEP 4 (Slot Reset)
> Â
> ÂSTEP 3: Link Reset
> Â------------------
> -The platform resets the link, and then calls the link_reset()
> callback
> -on all affected device drivers.ÂÂThis is a PCI-Express specific
> state
> +The platform resets the link.ÂÂThis is a PCI-Express specific step
> Âand is done whenever a non-fatal error has been detected that can be
> -"solved" by resetting the link. This call informs the driver of the
> -reset and the driver should check to see if the device appears to be
> -in working condition.
> -
> -The driver is not supposed to restart normal driver I/O operations
> -at this point.ÂÂIt should limit itself to "probing" the device to
> -check its recoverability status. If all is right, then the platform
> -will call resume() once all drivers have ack'd link_reset().
> -
> -	Result codes:
> -		(identical to STEP 3 (MMIO Enabled)
> -
> -The platform then proceeds to either STEP 4 (Slot Reset) or STEP 5
> -(Resume Operations).
> -
> ->>> The current powerpc implementation does not implement this
> callback.
> +"solved" by resetting the link.
> Â
> ÂSTEP 4: Slot Reset
> Â------------------
> diff --git a/drivers/infiniband/hw/hfi1/pcie.c
> b/drivers/infiniband/hw/hfi1/pcie.c
> index 4ac8f33..ebd941f 100644
> --- a/drivers/infiniband/hw/hfi1/pcie.c
> +++ b/drivers/infiniband/hw/hfi1/pcie.c
> @@ -598,15 +598,6 @@ pci_slot_reset(struct pci_dev *pdev)
> Â	return PCI_ERS_RESULT_CAN_RECOVER;
> Â}
> Â
> -static pci_ers_result_t
> -pci_link_reset(struct pci_dev *pdev)
> -{
> -	struct hfi1_devdata *dd = pci_get_drvdata(pdev);
> -
> -	dd_dev_info(dd, "HFI1 link_reset function called,
> ignored\n");
> -	return PCI_ERS_RESULT_CAN_RECOVER;
> -}
> -
> Âstatic void
> Âpci_resume(struct pci_dev *pdev)
> Â{
> @@ -625,7 +616,6 @@ pci_resume(struct pci_dev *pdev)
> Âconst struct pci_error_handlers hfi1_pci_err_handler = {
> Â	.error_detected = pci_error_detected,
> Â	.mmio_enabled = pci_mmio_enabled,
> -	.link_reset = pci_link_reset,
> Â	.slot_reset = pci_slot_reset,
> Â	.resume = pci_resume,
> Â};
> diff --git a/drivers/infiniband/hw/qib/qib_pcie.c
> b/drivers/infiniband/hw/qib/qib_pcie.c
> index 6abe1c6..c379b83 100644
> --- a/drivers/infiniband/hw/qib/qib_pcie.c
> +++ b/drivers/infiniband/hw/qib/qib_pcie.c
> @@ -682,13 +682,6 @@ qib_pci_slot_reset(struct pci_dev *pdev)
> Â	return PCI_ERS_RESULT_CAN_RECOVER;
> Â}
> Â
> -static pci_ers_result_t
> -qib_pci_link_reset(struct pci_dev *pdev)
> -{
> -	qib_devinfo(pdev, "QIB link_reset function called,
> ignored\n");
> -	return PCI_ERS_RESULT_CAN_RECOVER;
> -}
> -
> Âstatic void
> Âqib_pci_resume(struct pci_dev *pdev)
> Â{
> @@ -707,7 +700,6 @@ qib_pci_resume(struct pci_dev *pdev)
> Âconst struct pci_error_handlers qib_pci_err_handler = {
> Â	.error_detected = qib_pci_error_detected,
> Â	.mmio_enabled = qib_pci_mmio_enabled,
> -	.link_reset = qib_pci_link_reset,
> Â	.slot_reset = qib_pci_slot_reset,
> Â	.resume = qib_pci_resume,
> Â};
> diff --git a/drivers/media/pci/ngene/ngene-cards.c
> b/drivers/media/pci/ngene/ngene-cards.c
> index 423e8c8..8438c1c 100644
> --- a/drivers/media/pci/ngene/ngene-cards.c
> +++ b/drivers/media/pci/ngene/ngene-cards.c
> @@ -781,12 +781,6 @@ static pci_ers_result_t
> ngene_error_detected(struct pci_dev *dev,
> Â	return PCI_ERS_RESULT_CAN_RECOVER;
> Â}
> Â
> -static pci_ers_result_t ngene_link_reset(struct pci_dev *dev)
> -{
> -	printk(KERN_INFO DEVICE_NAME ": link reset\n");
> -	return 0;
> -}
> -
> Âstatic pci_ers_result_t ngene_slot_reset(struct pci_dev *dev)
> Â{
> Â	printk(KERN_INFO DEVICE_NAME ": slot reset\n");
> @@ -800,7 +794,6 @@ static void ngene_resume(struct pci_dev *dev)
> Â
> Âstatic const struct pci_error_handlers ngene_errors = {
> Â	.error_detected = ngene_error_detected,
> -	.link_reset = ngene_link_reset,
> Â	.slot_reset = ngene_slot_reset,
> Â	.resume = ngene_resume,
> Â};
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 30d6c16..316379c 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -661,9 +661,6 @@ struct pci_error_handlers {
> Â	/* MMIO has been re-enabled, but not DMA */
> Â	pci_ers_result_t (*mmio_enabled)(struct pci_dev *dev);
> Â
> -	/* PCI Express link has been reset */
> -	pci_ers_result_t (*link_reset)(struct pci_dev *dev);
> -
> Â	/* PCI slot has been reset */
> Â	pci_ers_result_t (*slot_reset)(struct pci_dev *dev);
> Â
-- 
Doug Ledford <dledford@xxxxxxxxxx>
  GPG KeyID: B826A3330E572FDD
 Â
Key fingerprint = AE6B 1BDA 122B 23B4 265B Â1274 B826 A333 0E57 2FDD

Attachment: signature.asc
Description: This is a digitally signed message part