linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] PCI: Remove pci_try_set_mwi
@ 2020-12-09  8:31 Heiner Kallweit
  2020-12-09  8:40 ` Kalle Valo
                   ` (5 more replies)
  0 siblings, 6 replies; 12+ messages in thread
From: Heiner Kallweit @ 2020-12-09  8:31 UTC (permalink / raw)
  To: Bjorn Helgaas, Jonathan Corbet, Jens Axboe, Viresh Kumar,
	Andy Shevchenko, Dan Williams, Vinod Koul, David Miller,
	Lee Jones, Ion Badulescu, Jakub Kicinski, Lino Sanfilippo,
	Christian Lamparter, Kalle Valo, Luis Chamberlain, Adam Radford,
	James E.J. Bottomley, Martin K. Petersen, James Smart,
	Dick Kennedy, Nilesh Javali, GR-QLogic-Storage-Upstream,
	Greg Kroah-Hartman, Jiri Slaby, Peter Chen, Felipe Balbi
  Cc: linux-pci, linux-doc, Linux Kernel Mailing List, linux-ide,
	dmaengine, netdev, linux-parisc, linux-wireless,
	SCSI development list, linux-serial, Linux USB Mailing List

pci_set_mwi() and pci_try_set_mwi() do exactly the same, just that the
former one is declared as __must_check. However also some callers of
pci_set_mwi() have a comment that it's an optional feature. I don't
think there's much sense in this separation and the use of
__must_check. Therefore remove pci_try_set_mwi() and remove the
__must_check attribute from pci_set_mwi().
I don't expect either function to be used in new code anyway.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
patch applies on top of pci/misc for v5.11
---
 Documentation/PCI/pci.rst                     |  5 +----
 drivers/ata/pata_cs5530.c                     |  2 +-
 drivers/ata/sata_mv.c                         |  2 +-
 drivers/dma/dw/pci.c                          |  2 +-
 drivers/dma/hsu/pci.c                         |  2 +-
 drivers/ide/cs5530.c                          |  2 +-
 drivers/mfd/intel-lpss-pci.c                  |  2 +-
 drivers/net/ethernet/adaptec/starfire.c       |  2 +-
 drivers/net/ethernet/alacritech/slicoss.c     |  2 +-
 drivers/net/ethernet/dec/tulip/tulip_core.c   |  5 +----
 drivers/net/ethernet/sun/cassini.c            |  4 ++--
 drivers/net/wireless/intersil/p54/p54pci.c    |  2 +-
 .../intersil/prism54/islpci_hotplug.c         |  3 +--
 .../wireless/realtek/rtl818x/rtl8180/dev.c    |  2 +-
 drivers/pci/pci.c                             | 19 -------------------
 drivers/scsi/3w-9xxx.c                        |  4 ++--
 drivers/scsi/3w-sas.c                         |  4 ++--
 drivers/scsi/csiostor/csio_init.c             |  2 +-
 drivers/scsi/lpfc/lpfc_init.c                 |  2 +-
 drivers/scsi/qla2xxx/qla_init.c               |  8 ++++----
 drivers/scsi/qla2xxx/qla_mr.c                 |  2 +-
 drivers/tty/serial/8250/8250_lpss.c           |  2 +-
 drivers/usb/chipidea/ci_hdrc_pci.c            |  2 +-
 drivers/usb/gadget/udc/amd5536udc_pci.c       |  2 +-
 drivers/usb/gadget/udc/net2280.c              |  2 +-
 drivers/usb/gadget/udc/pch_udc.c              |  2 +-
 include/linux/pci.h                           |  5 ++---
 27 files changed, 33 insertions(+), 60 deletions(-)

diff --git a/Documentation/PCI/pci.rst b/Documentation/PCI/pci.rst
index 814b40f83..120362cc9 100644
--- a/Documentation/PCI/pci.rst
+++ b/Documentation/PCI/pci.rst
@@ -226,10 +226,7 @@ If the PCI device can use the PCI Memory-Write-Invalidate transaction,
 call pci_set_mwi().  This enables the PCI_COMMAND bit for Mem-Wr-Inval
 and also ensures that the cache line size register is set correctly.
 Check the return value of pci_set_mwi() as not all architectures
-or chip-sets may support Memory-Write-Invalidate.  Alternatively,
-if Mem-Wr-Inval would be nice to have but is not required, call
-pci_try_set_mwi() to have the system do its best effort at enabling
-Mem-Wr-Inval.
+or chip-sets may support Memory-Write-Invalidate.
 
 
 Request MMIO/IOP resources
diff --git a/drivers/ata/pata_cs5530.c b/drivers/ata/pata_cs5530.c
index ad75d02b6..8654b3ae1 100644
--- a/drivers/ata/pata_cs5530.c
+++ b/drivers/ata/pata_cs5530.c
@@ -214,7 +214,7 @@ static int cs5530_init_chip(void)
 	}
 
 	pci_set_master(cs5530_0);
-	pci_try_set_mwi(cs5530_0);
+	pci_set_mwi(cs5530_0);
 
 	/*
 	 * Set PCI CacheLineSize to 16-bytes:
diff --git a/drivers/ata/sata_mv.c b/drivers/ata/sata_mv.c
index 664ef658a..ee37755ea 100644
--- a/drivers/ata/sata_mv.c
+++ b/drivers/ata/sata_mv.c
@@ -4432,7 +4432,7 @@ static int mv_pci_init_one(struct pci_dev *pdev,
 	mv_print_info(host);
 
 	pci_set_master(pdev);
-	pci_try_set_mwi(pdev);
+	pci_set_mwi(pdev);
 	return ata_host_activate(host, pdev->irq, mv_interrupt, IRQF_SHARED,
 				 IS_GEN_I(hpriv) ? &mv5_sht : &mv6_sht);
 }
diff --git a/drivers/dma/dw/pci.c b/drivers/dma/dw/pci.c
index 1142aa6f8..1c20b7485 100644
--- a/drivers/dma/dw/pci.c
+++ b/drivers/dma/dw/pci.c
@@ -30,7 +30,7 @@ static int dw_pci_probe(struct pci_dev *pdev, const struct pci_device_id *pid)
 	}
 
 	pci_set_master(pdev);
-	pci_try_set_mwi(pdev);
+	pci_set_mwi(pdev);
 
 	ret = pci_set_dma_mask(pdev, DMA_BIT_MASK(32));
 	if (ret)
diff --git a/drivers/dma/hsu/pci.c b/drivers/dma/hsu/pci.c
index 07cc7320a..420dd3706 100644
--- a/drivers/dma/hsu/pci.c
+++ b/drivers/dma/hsu/pci.c
@@ -73,7 +73,7 @@ static int hsu_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	}
 
 	pci_set_master(pdev);
-	pci_try_set_mwi(pdev);
+	pci_set_mwi(pdev);
 
 	ret = pci_set_dma_mask(pdev, DMA_BIT_MASK(32));
 	if (ret)
diff --git a/drivers/ide/cs5530.c b/drivers/ide/cs5530.c
index 5bb46e713..5d2c421ab 100644
--- a/drivers/ide/cs5530.c
+++ b/drivers/ide/cs5530.c
@@ -168,7 +168,7 @@ static int init_chipset_cs5530(struct pci_dev *dev)
 	 */
 
 	pci_set_master(cs5530_0);
-	pci_try_set_mwi(cs5530_0);
+	pci_set_mwi(cs5530_0);
 
 	/*
 	 * Set PCI CacheLineSize to 16-bytes:
diff --git a/drivers/mfd/intel-lpss-pci.c b/drivers/mfd/intel-lpss-pci.c
index 2d7c588ef..a0c3be750 100644
--- a/drivers/mfd/intel-lpss-pci.c
+++ b/drivers/mfd/intel-lpss-pci.c
@@ -39,7 +39,7 @@ static int intel_lpss_pci_probe(struct pci_dev *pdev,
 
 	/* Probably it is enough to set this for iDMA capable devices only */
 	pci_set_master(pdev);
-	pci_try_set_mwi(pdev);
+	pci_set_mwi(pdev);
 
 	ret = intel_lpss_probe(&pdev->dev, info);
 	if (ret)
diff --git a/drivers/net/ethernet/adaptec/starfire.c b/drivers/net/ethernet/adaptec/starfire.c
index 555299737..1dbff34c4 100644
--- a/drivers/net/ethernet/adaptec/starfire.c
+++ b/drivers/net/ethernet/adaptec/starfire.c
@@ -679,7 +679,7 @@ static int starfire_init_one(struct pci_dev *pdev,
 	pci_set_master(pdev);
 
 	/* enable MWI -- it vastly improves Rx performance on sparc64 */
-	pci_try_set_mwi(pdev);
+	pci_set_mwi(pdev);
 
 #ifdef ZEROCOPY
 	/* Starfire can do TCP/UDP checksumming */
diff --git a/drivers/net/ethernet/alacritech/slicoss.c b/drivers/net/ethernet/alacritech/slicoss.c
index 696517eae..544510f57 100644
--- a/drivers/net/ethernet/alacritech/slicoss.c
+++ b/drivers/net/ethernet/alacritech/slicoss.c
@@ -1749,7 +1749,7 @@ static int slic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	}
 
 	pci_set_master(pdev);
-	pci_try_set_mwi(pdev);
+	pci_set_mwi(pdev);
 
 	slic_configure_pci(pdev);
 
diff --git a/drivers/net/ethernet/dec/tulip/tulip_core.c b/drivers/net/ethernet/dec/tulip/tulip_core.c
index c1dcd6ca1..4f6debb24 100644
--- a/drivers/net/ethernet/dec/tulip/tulip_core.c
+++ b/drivers/net/ethernet/dec/tulip/tulip_core.c
@@ -1193,10 +1193,7 @@ static void tulip_mwi_config(struct pci_dev *pdev, struct net_device *dev)
 	/* if we have any cache line size at all, we can do MRM and MWI */
 	csr0 |= MRM | MWI;
 
-	/* Enable MWI in the standard PCI command bit.
-	 * Check for the case where MWI is desired but not available
-	 */
-	pci_try_set_mwi(pdev);
+	pci_set_mwi(pdev);
 
 	/* read result from hardware (in case bit refused to enable) */
 	pci_read_config_word(pdev, PCI_COMMAND, &pci_command);
diff --git a/drivers/net/ethernet/sun/cassini.c b/drivers/net/ethernet/sun/cassini.c
index 9ff894ba8..9a95ec989 100644
--- a/drivers/net/ethernet/sun/cassini.c
+++ b/drivers/net/ethernet/sun/cassini.c
@@ -4933,14 +4933,14 @@ static int cas_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	pci_cmd &= ~PCI_COMMAND_SERR;
 	pci_cmd |= PCI_COMMAND_PARITY;
 	pci_write_config_word(pdev, PCI_COMMAND, pci_cmd);
-	if (pci_try_set_mwi(pdev))
+	if (pci_set_mwi(pdev))
 		pr_warn("Could not enable MWI for %s\n", pci_name(pdev));
 
 	cas_program_bridge(pdev);
 
 	/*
 	 * On some architectures, the default cache line size set
-	 * by pci_try_set_mwi reduces perforamnce.  We have to increase
+	 * by pci_set_mwi reduces performance.  We have to increase
 	 * it for this case.  To start, we'll print some configuration
 	 * data.
 	 */
diff --git a/drivers/net/wireless/intersil/p54/p54pci.c b/drivers/net/wireless/intersil/p54/p54pci.c
index e97ee547b..c76326d1e 100644
--- a/drivers/net/wireless/intersil/p54/p54pci.c
+++ b/drivers/net/wireless/intersil/p54/p54pci.c
@@ -583,7 +583,7 @@ static int p54p_probe(struct pci_dev *pdev,
 	}
 
 	pci_set_master(pdev);
-	pci_try_set_mwi(pdev);
+	pci_set_mwi(pdev);
 
 	pci_write_config_byte(pdev, 0x40, 0);
 	pci_write_config_byte(pdev, 0x41, 0);
diff --git a/drivers/net/wireless/intersil/prism54/islpci_hotplug.c b/drivers/net/wireless/intersil/prism54/islpci_hotplug.c
index 31a1e6132..e8087d9a5 100644
--- a/drivers/net/wireless/intersil/prism54/islpci_hotplug.c
+++ b/drivers/net/wireless/intersil/prism54/islpci_hotplug.c
@@ -153,8 +153,7 @@ prism54_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	DEBUG(SHOW_TRACING, "%s: pci_set_master(pdev)\n", DRV_NAME);
 	pci_set_master(pdev);
 
-	/* enable MWI */
-	pci_try_set_mwi(pdev);
+	pci_set_mwi(pdev);
 
 	/* setup the network device interface and its structure */
 	if (!(ndev = islpci_setup(pdev))) {
diff --git a/drivers/net/wireless/realtek/rtl818x/rtl8180/dev.c b/drivers/net/wireless/realtek/rtl818x/rtl8180/dev.c
index 2477e18c7..b259b0b58 100644
--- a/drivers/net/wireless/realtek/rtl818x/rtl8180/dev.c
+++ b/drivers/net/wireless/realtek/rtl818x/rtl8180/dev.c
@@ -1871,7 +1871,7 @@ static int rtl8180_probe(struct pci_dev *pdev,
 
 	if (priv->chip_family != RTL818X_CHIP_FAMILY_RTL8180) {
 		priv->band.n_bitrates = ARRAY_SIZE(rtl818x_rates);
-		pci_try_set_mwi(pdev);
+		pci_set_mwi(pdev);
 	}
 
 	if (priv->chip_family != RTL818X_CHIP_FAMILY_RTL8180)
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 9a5500287..f0ab432d2 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -4389,25 +4389,6 @@ int pcim_set_mwi(struct pci_dev *dev)
 }
 EXPORT_SYMBOL(pcim_set_mwi);
 
-/**
- * pci_try_set_mwi - enables memory-write-invalidate PCI transaction
- * @dev: the PCI device for which MWI is enabled
- *
- * Enables the Memory-Write-Invalidate transaction in %PCI_COMMAND.
- * Callers are not required to check the return value.
- *
- * RETURNS: An appropriate -ERRNO error value on error, or zero for success.
- */
-int pci_try_set_mwi(struct pci_dev *dev)
-{
-#ifdef PCI_DISABLE_MWI
-	return 0;
-#else
-	return pci_set_mwi(dev);
-#endif
-}
-EXPORT_SYMBOL(pci_try_set_mwi);
-
 /**
  * pci_clear_mwi - disables Memory-Write-Invalidate for device dev
  * @dev: the PCI device to disable
diff --git a/drivers/scsi/3w-9xxx.c b/drivers/scsi/3w-9xxx.c
index b4718a1b2..d869485e2 100644
--- a/drivers/scsi/3w-9xxx.c
+++ b/drivers/scsi/3w-9xxx.c
@@ -2018,7 +2018,7 @@ static int twa_probe(struct pci_dev *pdev, const struct pci_device_id *dev_id)
 	}
 
 	pci_set_master(pdev);
-	pci_try_set_mwi(pdev);
+	pci_set_mwi(pdev);
 
 	retval = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
 	if (retval)
@@ -2227,7 +2227,7 @@ static int __maybe_unused twa_resume(struct device *dev)
 
 	printk(KERN_WARNING "3w-9xxx: Resuming host %d.\n", tw_dev->host->host_no);
 
-	pci_try_set_mwi(pdev);
+	pci_set_mwi(pdev);
 
 	retval = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
 	if (retval)
diff --git a/drivers/scsi/3w-sas.c b/drivers/scsi/3w-sas.c
index b8f1848ec..49ca153b8 100644
--- a/drivers/scsi/3w-sas.c
+++ b/drivers/scsi/3w-sas.c
@@ -1571,7 +1571,7 @@ static int twl_probe(struct pci_dev *pdev, const struct pci_device_id *dev_id)
 	}
 
 	pci_set_master(pdev);
-	pci_try_set_mwi(pdev);
+	pci_set_mwi(pdev);
 
 	retval = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
 	if (retval)
@@ -1790,7 +1790,7 @@ static int __maybe_unused twl_resume(struct device *dev)
 	TW_Device_Extension *tw_dev = (TW_Device_Extension *)host->hostdata;
 
 	printk(KERN_WARNING "3w-sas: Resuming host %d.\n", tw_dev->host->host_no);
-	pci_try_set_mwi(pdev);
+	pci_set_mwi(pdev);
 
 	retval = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
 	if (retval)
diff --git a/drivers/scsi/csiostor/csio_init.c b/drivers/scsi/csiostor/csio_init.c
index 390b07bf9..c20bf44c6 100644
--- a/drivers/scsi/csiostor/csio_init.c
+++ b/drivers/scsi/csiostor/csio_init.c
@@ -201,7 +201,7 @@ csio_pci_init(struct pci_dev *pdev, int *bars)
 		goto err_disable_device;
 
 	pci_set_master(pdev);
-	pci_try_set_mwi(pdev);
+	pci_set_mwi(pdev);
 
 	rv = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
 	if (rv)
diff --git a/drivers/scsi/lpfc/lpfc_init.c b/drivers/scsi/lpfc/lpfc_init.c
index ac67f420e..b4833c0f8 100644
--- a/drivers/scsi/lpfc/lpfc_init.c
+++ b/drivers/scsi/lpfc/lpfc_init.c
@@ -6119,7 +6119,7 @@ lpfc_enable_pci_dev(struct lpfc_hba *phba)
 		goto out_disable_device;
 	/* Set up device as PCI master and save state for EEH */
 	pci_set_master(pdev);
-	pci_try_set_mwi(pdev);
+	pci_set_mwi(pdev);
 	pci_save_state(pdev);
 
 	/* PCIe EEH recovery on powerpc platforms needs fundamental reset */
diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c
index 5626e9b69..76019dc2e 100644
--- a/drivers/scsi/qla2xxx/qla_init.c
+++ b/drivers/scsi/qla2xxx/qla_init.c
@@ -2356,7 +2356,7 @@ qla2100_pci_config(scsi_qla_host_t *vha)
 	struct device_reg_2xxx __iomem *reg = &ha->iobase->isp;
 
 	pci_set_master(ha->pdev);
-	pci_try_set_mwi(ha->pdev);
+	pci_set_mwi(ha->pdev);
 
 	pci_read_config_word(ha->pdev, PCI_COMMAND, &w);
 	w |= (PCI_COMMAND_PARITY | PCI_COMMAND_SERR);
@@ -2388,7 +2388,7 @@ qla2300_pci_config(scsi_qla_host_t *vha)
 	struct device_reg_2xxx __iomem *reg = &ha->iobase->isp;
 
 	pci_set_master(ha->pdev);
-	pci_try_set_mwi(ha->pdev);
+	pci_set_mwi(ha->pdev);
 
 	pci_read_config_word(ha->pdev, PCI_COMMAND, &w);
 	w |= (PCI_COMMAND_PARITY | PCI_COMMAND_SERR);
@@ -2469,7 +2469,7 @@ qla24xx_pci_config(scsi_qla_host_t *vha)
 	struct device_reg_24xx __iomem *reg = &ha->iobase->isp24;
 
 	pci_set_master(ha->pdev);
-	pci_try_set_mwi(ha->pdev);
+	pci_set_mwi(ha->pdev);
 
 	pci_read_config_word(ha->pdev, PCI_COMMAND, &w);
 	w |= (PCI_COMMAND_PARITY | PCI_COMMAND_SERR);
@@ -2511,7 +2511,7 @@ qla25xx_pci_config(scsi_qla_host_t *vha)
 	struct qla_hw_data *ha = vha->hw;
 
 	pci_set_master(ha->pdev);
-	pci_try_set_mwi(ha->pdev);
+	pci_set_mwi(ha->pdev);
 
 	pci_read_config_word(ha->pdev, PCI_COMMAND, &w);
 	w |= (PCI_COMMAND_PARITY | PCI_COMMAND_SERR);
diff --git a/drivers/scsi/qla2xxx/qla_mr.c b/drivers/scsi/qla2xxx/qla_mr.c
index ca7306685..e9763d8e3 100644
--- a/drivers/scsi/qla2xxx/qla_mr.c
+++ b/drivers/scsi/qla2xxx/qla_mr.c
@@ -499,7 +499,7 @@ qlafx00_pci_config(scsi_qla_host_t *vha)
 	struct qla_hw_data *ha = vha->hw;
 
 	pci_set_master(ha->pdev);
-	pci_try_set_mwi(ha->pdev);
+	pci_set_mwi(ha->pdev);
 
 	pci_read_config_word(ha->pdev, PCI_COMMAND, &w);
 	w |= (PCI_COMMAND_PARITY | PCI_COMMAND_SERR);
diff --git a/drivers/tty/serial/8250/8250_lpss.c b/drivers/tty/serial/8250/8250_lpss.c
index 4dee8a9e0..8acc1e5c9 100644
--- a/drivers/tty/serial/8250/8250_lpss.c
+++ b/drivers/tty/serial/8250/8250_lpss.c
@@ -193,7 +193,7 @@ static void qrk_serial_setup_dma(struct lpss8250 *lpss, struct uart_port *port)
 	if (ret)
 		return;
 
-	pci_try_set_mwi(pdev);
+	pci_set_mwi(pdev);
 
 	/* Special DMA address for UART */
 	dma->rx_dma_addr = 0xfffff000;
diff --git a/drivers/usb/chipidea/ci_hdrc_pci.c b/drivers/usb/chipidea/ci_hdrc_pci.c
index d63479e1a..d412fa910 100644
--- a/drivers/usb/chipidea/ci_hdrc_pci.c
+++ b/drivers/usb/chipidea/ci_hdrc_pci.c
@@ -78,7 +78,7 @@ static int ci_hdrc_pci_probe(struct pci_dev *pdev,
 	}
 
 	pci_set_master(pdev);
-	pci_try_set_mwi(pdev);
+	pci_set_mwi(pdev);
 
 	/* register a nop PHY */
 	ci->phy = usb_phy_generic_register();
diff --git a/drivers/usb/gadget/udc/amd5536udc_pci.c b/drivers/usb/gadget/udc/amd5536udc_pci.c
index 8d387e0e4..9630ce8d3 100644
--- a/drivers/usb/gadget/udc/amd5536udc_pci.c
+++ b/drivers/usb/gadget/udc/amd5536udc_pci.c
@@ -151,7 +151,7 @@ static int udc_pci_probe(
 	dev->chiprev = pdev->revision;
 
 	pci_set_master(pdev);
-	pci_try_set_mwi(pdev);
+	pci_set_mwi(pdev);
 
 	/* init dma pools */
 	if (use_dma) {
diff --git a/drivers/usb/gadget/udc/net2280.c b/drivers/usb/gadget/udc/net2280.c
index fc9f99fe7..e15520698 100644
--- a/drivers/usb/gadget/udc/net2280.c
+++ b/drivers/usb/gadget/udc/net2280.c
@@ -3761,7 +3761,7 @@ static int net2280_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 			&dev->pci->pcimstctl);
 	/* erratum 0115 shouldn't appear: Linux inits PCI_LATENCY_TIMER */
 	pci_set_master(pdev);
-	pci_try_set_mwi(pdev);
+	pci_set_mwi(pdev);
 
 	/* ... also flushes any posted pci writes */
 	dev->chiprev = get_idx_reg(dev->regs, REG_CHIPREV) & 0xffff;
diff --git a/drivers/usb/gadget/udc/pch_udc.c b/drivers/usb/gadget/udc/pch_udc.c
index a3c1fc924..4a0484a0c 100644
--- a/drivers/usb/gadget/udc/pch_udc.c
+++ b/drivers/usb/gadget/udc/pch_udc.c
@@ -3100,7 +3100,7 @@ static int pch_udc_probe(struct pci_dev *pdev,
 	}
 
 	pci_set_master(pdev);
-	pci_try_set_mwi(pdev);
+	pci_set_mwi(pdev);
 
 	/* device struct setup */
 	spin_lock_init(&dev->lock);
diff --git a/include/linux/pci.h b/include/linux/pci.h
index de75f6a4d..c590f616d 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1191,9 +1191,8 @@ void pci_clear_master(struct pci_dev *dev);
 
 int pci_set_pcie_reset_state(struct pci_dev *dev, enum pcie_reset_state state);
 int pci_set_cacheline_size(struct pci_dev *dev);
-int __must_check pci_set_mwi(struct pci_dev *dev);
-int __must_check pcim_set_mwi(struct pci_dev *dev);
-int pci_try_set_mwi(struct pci_dev *dev);
+int pci_set_mwi(struct pci_dev *dev);
+int pcim_set_mwi(struct pci_dev *dev);
 void pci_clear_mwi(struct pci_dev *dev);
 void pci_intx(struct pci_dev *dev, int enable);
 bool pci_check_and_mask_intx(struct pci_dev *dev);
-- 
2.29.2


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] PCI: Remove pci_try_set_mwi
  2020-12-09  8:31 [PATCH] PCI: Remove pci_try_set_mwi Heiner Kallweit
@ 2020-12-09  8:40 ` Kalle Valo
  2020-12-09  9:03 ` Vinod Koul
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 12+ messages in thread
From: Kalle Valo @ 2020-12-09  8:40 UTC (permalink / raw)
  To: Heiner Kallweit
  Cc: Bjorn Helgaas, Jonathan Corbet, Jens Axboe, Viresh Kumar,
	Andy Shevchenko, Dan Williams, Vinod Koul, David Miller,
	Lee Jones, Ion Badulescu, Jakub Kicinski, Lino Sanfilippo,
	Christian Lamparter, Luis Chamberlain, Adam Radford,
	James E.J. Bottomley, Martin K. Petersen, James Smart,
	Dick Kennedy, Nilesh Javali, GR-QLogic-Storage-Upstream,
	Greg Kroah-Hartman, Jiri Slaby, Peter Chen, Felipe Balbi,
	lin ux-pci@vger.kernel.org, linux-doc, Linux Kernel Mailing List,
	linux-ide, dmaengine, netdev, linux-parisc, linux-wireless,
	SCSI development list, linux-serial, Linux USB Mailing List

Heiner Kallweit <hkallweit1@gmail.com> writes:

> pci_set_mwi() and pci_try_set_mwi() do exactly the same, just that the
> former one is declared as __must_check. However also some callers of
> pci_set_mwi() have a comment that it's an optional feature. I don't
> think there's much sense in this separation and the use of
> __must_check. Therefore remove pci_try_set_mwi() and remove the
> __must_check attribute from pci_set_mwi().
> I don't expect either function to be used in new code anyway.
>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
> patch applies on top of pci/misc for v5.11

>  drivers/net/wireless/intersil/p54/p54pci.c    |  2 +-
>  .../intersil/prism54/islpci_hotplug.c         |  3 +--
>  .../wireless/realtek/rtl818x/rtl8180/dev.c    |  2 +-

For drivers/wireless:

Acked-by: Kalle Valo <kvalo@codeaurora.org>

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] PCI: Remove pci_try_set_mwi
  2020-12-09  8:31 [PATCH] PCI: Remove pci_try_set_mwi Heiner Kallweit
  2020-12-09  8:40 ` Kalle Valo
@ 2020-12-09  9:03 ` Vinod Koul
  2020-12-09  9:20 ` Peter Chen
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 12+ messages in thread
From: Vinod Koul @ 2020-12-09  9:03 UTC (permalink / raw)
  To: Heiner Kallweit
  Cc: Bjorn Helgaas, Jonathan Corbet, Jens Axboe, Viresh Kumar,
	Andy Shevchenko, Dan Williams, David Miller, Lee Jones,
	Ion Badulescu, Jakub Kicinski, Lino Sanfilippo,
	Christian Lamparter, Kalle Valo, Luis Chamberlain, Adam Radford,
	James E.J. Bottomley, Martin K. Petersen, James Smart,
	Dick Kennedy, Nilesh Javali, GR-QLogic-Storage-Upstream,
	Greg Kroah-Hartman, Jiri Slaby, Peter Chen, Felipe Balbi,
	linux-pci, linux-doc, Linux Kernel Mailing List, linux-ide,
	dmaengine, netdev, linux-parisc, linux-wireless,
	SCSI development list, linux-serial, Linux USB Mailing List

On 09-12-20, 09:31, Heiner Kallweit wrote:
> pci_set_mwi() and pci_try_set_mwi() do exactly the same, just that the
> former one is declared as __must_check. However also some callers of
> pci_set_mwi() have a comment that it's an optional feature. I don't
> think there's much sense in this separation and the use of
> __must_check. Therefore remove pci_try_set_mwi() and remove the
> __must_check attribute from pci_set_mwi().
> I don't expect either function to be used in new code anyway.
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
> patch applies on top of pci/misc for v5.11
> ---
>  drivers/dma/dw/pci.c                          |  2 +-
>  drivers/dma/hsu/pci.c                         |  2 +-

Acked-By: Vinod Koul <vkoul@kernel.org>

-- 
~Vinod

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] PCI: Remove pci_try_set_mwi
  2020-12-09  8:31 [PATCH] PCI: Remove pci_try_set_mwi Heiner Kallweit
  2020-12-09  8:40 ` Kalle Valo
  2020-12-09  9:03 ` Vinod Koul
@ 2020-12-09  9:20 ` Peter Chen
  2020-12-09  9:22 ` Lee Jones
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 12+ messages in thread
From: Peter Chen @ 2020-12-09  9:20 UTC (permalink / raw)
  To: Heiner Kallweit
  Cc: Bjorn Helgaas, Jonathan Corbet, Jens Axboe, Viresh Kumar,
	Andy Shevchenko, Dan Williams, Vinod Koul, David Miller,
	Lee Jones, Ion Badulescu, Jakub Kicinski, Lino Sanfilippo,
	Christian Lamparter, Kalle Valo, Luis Chamberlain, Adam Radford,
	James E.J. Bottomley, Martin K. Petersen, James Smart,
	Dick Kennedy, Nilesh Javali, GR-QLogic-Storage-Upstream,
	Greg Kroah-Hartman, Jiri Slaby, Felipe Balbi, linux-pci,
	linux-doc, Linux Kernel Mailing List, linux-ide, dmaengine,
	netdev, linux-parisc, linux-wireless, SCSI development list,
	linux-serial, Linux USB Mailing List

On 20-12-09 09:31:21, Heiner Kallweit wrote:
>  drivers/usb/chipidea/ci_hdrc_pci.c            |  2 +-

For chipidea changes:

Acked-by: Peter Chen <peter.chen@nxp.com>

Peter


>  drivers/usb/gadget/udc/amd5536udc_pci.c       |  2 +-
>  drivers/usb/gadget/udc/net2280.c              |  2 +-
>  drivers/usb/gadget/udc/pch_udc.c              |  2 +-
>  include/linux/pci.h                           |  5 ++---
>  27 files changed, 33 insertions(+), 60 deletions(-)
> 
> diff --git a/Documentation/PCI/pci.rst b/Documentation/PCI/pci.rst
> index 814b40f83..120362cc9 100644
> --- a/Documentation/PCI/pci.rst
> +++ b/Documentation/PCI/pci.rst
> @@ -226,10 +226,7 @@ If the PCI device can use the PCI Memory-Write-Invalidate transaction,
>  call pci_set_mwi().  This enables the PCI_COMMAND bit for Mem-Wr-Inval
>  and also ensures that the cache line size register is set correctly.
>  Check the return value of pci_set_mwi() as not all architectures
> -or chip-sets may support Memory-Write-Invalidate.  Alternatively,
> -if Mem-Wr-Inval would be nice to have but is not required, call
> -pci_try_set_mwi() to have the system do its best effort at enabling
> -Mem-Wr-Inval.
> +or chip-sets may support Memory-Write-Invalidate.
>  
>  
>  Request MMIO/IOP resources
> diff --git a/drivers/ata/pata_cs5530.c b/drivers/ata/pata_cs5530.c
> index ad75d02b6..8654b3ae1 100644
> --- a/drivers/ata/pata_cs5530.c
> +++ b/drivers/ata/pata_cs5530.c
> @@ -214,7 +214,7 @@ static int cs5530_init_chip(void)
>  	}
>  
>  	pci_set_master(cs5530_0);
> -	pci_try_set_mwi(cs5530_0);
> +	pci_set_mwi(cs5530_0);
>  
>  	/*
>  	 * Set PCI CacheLineSize to 16-bytes:
> diff --git a/drivers/ata/sata_mv.c b/drivers/ata/sata_mv.c
> index 664ef658a..ee37755ea 100644
> --- a/drivers/ata/sata_mv.c
> +++ b/drivers/ata/sata_mv.c
> @@ -4432,7 +4432,7 @@ static int mv_pci_init_one(struct pci_dev *pdev,
>  	mv_print_info(host);
>  
>  	pci_set_master(pdev);
> -	pci_try_set_mwi(pdev);
> +	pci_set_mwi(pdev);
>  	return ata_host_activate(host, pdev->irq, mv_interrupt, IRQF_SHARED,
>  				 IS_GEN_I(hpriv) ? &mv5_sht : &mv6_sht);
>  }
> diff --git a/drivers/dma/dw/pci.c b/drivers/dma/dw/pci.c
> index 1142aa6f8..1c20b7485 100644
> --- a/drivers/dma/dw/pci.c
> +++ b/drivers/dma/dw/pci.c
> @@ -30,7 +30,7 @@ static int dw_pci_probe(struct pci_dev *pdev, const struct pci_device_id *pid)
>  	}
>  
>  	pci_set_master(pdev);
> -	pci_try_set_mwi(pdev);
> +	pci_set_mwi(pdev);
>  
>  	ret = pci_set_dma_mask(pdev, DMA_BIT_MASK(32));
>  	if (ret)
> diff --git a/drivers/dma/hsu/pci.c b/drivers/dma/hsu/pci.c
> index 07cc7320a..420dd3706 100644
> --- a/drivers/dma/hsu/pci.c
> +++ b/drivers/dma/hsu/pci.c
> @@ -73,7 +73,7 @@ static int hsu_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>  	}
>  
>  	pci_set_master(pdev);
> -	pci_try_set_mwi(pdev);
> +	pci_set_mwi(pdev);
>  
>  	ret = pci_set_dma_mask(pdev, DMA_BIT_MASK(32));
>  	if (ret)
> diff --git a/drivers/ide/cs5530.c b/drivers/ide/cs5530.c
> index 5bb46e713..5d2c421ab 100644
> --- a/drivers/ide/cs5530.c
> +++ b/drivers/ide/cs5530.c
> @@ -168,7 +168,7 @@ static int init_chipset_cs5530(struct pci_dev *dev)
>  	 */
>  
>  	pci_set_master(cs5530_0);
> -	pci_try_set_mwi(cs5530_0);
> +	pci_set_mwi(cs5530_0);
>  
>  	/*
>  	 * Set PCI CacheLineSize to 16-bytes:
> diff --git a/drivers/mfd/intel-lpss-pci.c b/drivers/mfd/intel-lpss-pci.c
> index 2d7c588ef..a0c3be750 100644
> --- a/drivers/mfd/intel-lpss-pci.c
> +++ b/drivers/mfd/intel-lpss-pci.c
> @@ -39,7 +39,7 @@ static int intel_lpss_pci_probe(struct pci_dev *pdev,
>  
>  	/* Probably it is enough to set this for iDMA capable devices only */
>  	pci_set_master(pdev);
> -	pci_try_set_mwi(pdev);
> +	pci_set_mwi(pdev);
>  
>  	ret = intel_lpss_probe(&pdev->dev, info);
>  	if (ret)
> diff --git a/drivers/net/ethernet/adaptec/starfire.c b/drivers/net/ethernet/adaptec/starfire.c
> index 555299737..1dbff34c4 100644
> --- a/drivers/net/ethernet/adaptec/starfire.c
> +++ b/drivers/net/ethernet/adaptec/starfire.c
> @@ -679,7 +679,7 @@ static int starfire_init_one(struct pci_dev *pdev,
>  	pci_set_master(pdev);
>  
>  	/* enable MWI -- it vastly improves Rx performance on sparc64 */
> -	pci_try_set_mwi(pdev);
> +	pci_set_mwi(pdev);
>  
>  #ifdef ZEROCOPY
>  	/* Starfire can do TCP/UDP checksumming */
> diff --git a/drivers/net/ethernet/alacritech/slicoss.c b/drivers/net/ethernet/alacritech/slicoss.c
> index 696517eae..544510f57 100644
> --- a/drivers/net/ethernet/alacritech/slicoss.c
> +++ b/drivers/net/ethernet/alacritech/slicoss.c
> @@ -1749,7 +1749,7 @@ static int slic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>  	}
>  
>  	pci_set_master(pdev);
> -	pci_try_set_mwi(pdev);
> +	pci_set_mwi(pdev);
>  
>  	slic_configure_pci(pdev);
>  
> diff --git a/drivers/net/ethernet/dec/tulip/tulip_core.c b/drivers/net/ethernet/dec/tulip/tulip_core.c
> index c1dcd6ca1..4f6debb24 100644
> --- a/drivers/net/ethernet/dec/tulip/tulip_core.c
> +++ b/drivers/net/ethernet/dec/tulip/tulip_core.c
> @@ -1193,10 +1193,7 @@ static void tulip_mwi_config(struct pci_dev *pdev, struct net_device *dev)
>  	/* if we have any cache line size at all, we can do MRM and MWI */
>  	csr0 |= MRM | MWI;
>  
> -	/* Enable MWI in the standard PCI command bit.
> -	 * Check for the case where MWI is desired but not available
> -	 */
> -	pci_try_set_mwi(pdev);
> +	pci_set_mwi(pdev);
>  
>  	/* read result from hardware (in case bit refused to enable) */
>  	pci_read_config_word(pdev, PCI_COMMAND, &pci_command);
> diff --git a/drivers/net/ethernet/sun/cassini.c b/drivers/net/ethernet/sun/cassini.c
> index 9ff894ba8..9a95ec989 100644
> --- a/drivers/net/ethernet/sun/cassini.c
> +++ b/drivers/net/ethernet/sun/cassini.c
> @@ -4933,14 +4933,14 @@ static int cas_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>  	pci_cmd &= ~PCI_COMMAND_SERR;
>  	pci_cmd |= PCI_COMMAND_PARITY;
>  	pci_write_config_word(pdev, PCI_COMMAND, pci_cmd);
> -	if (pci_try_set_mwi(pdev))
> +	if (pci_set_mwi(pdev))
>  		pr_warn("Could not enable MWI for %s\n", pci_name(pdev));
>  
>  	cas_program_bridge(pdev);
>  
>  	/*
>  	 * On some architectures, the default cache line size set
> -	 * by pci_try_set_mwi reduces perforamnce.  We have to increase
> +	 * by pci_set_mwi reduces performance.  We have to increase
>  	 * it for this case.  To start, we'll print some configuration
>  	 * data.
>  	 */
> diff --git a/drivers/net/wireless/intersil/p54/p54pci.c b/drivers/net/wireless/intersil/p54/p54pci.c
> index e97ee547b..c76326d1e 100644
> --- a/drivers/net/wireless/intersil/p54/p54pci.c
> +++ b/drivers/net/wireless/intersil/p54/p54pci.c
> @@ -583,7 +583,7 @@ static int p54p_probe(struct pci_dev *pdev,
>  	}
>  
>  	pci_set_master(pdev);
> -	pci_try_set_mwi(pdev);
> +	pci_set_mwi(pdev);
>  
>  	pci_write_config_byte(pdev, 0x40, 0);
>  	pci_write_config_byte(pdev, 0x41, 0);
> diff --git a/drivers/net/wireless/intersil/prism54/islpci_hotplug.c b/drivers/net/wireless/intersil/prism54/islpci_hotplug.c
> index 31a1e6132..e8087d9a5 100644
> --- a/drivers/net/wireless/intersil/prism54/islpci_hotplug.c
> +++ b/drivers/net/wireless/intersil/prism54/islpci_hotplug.c
> @@ -153,8 +153,7 @@ prism54_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>  	DEBUG(SHOW_TRACING, "%s: pci_set_master(pdev)\n", DRV_NAME);
>  	pci_set_master(pdev);
>  
> -	/* enable MWI */
> -	pci_try_set_mwi(pdev);
> +	pci_set_mwi(pdev);
>  
>  	/* setup the network device interface and its structure */
>  	if (!(ndev = islpci_setup(pdev))) {
> diff --git a/drivers/net/wireless/realtek/rtl818x/rtl8180/dev.c b/drivers/net/wireless/realtek/rtl818x/rtl8180/dev.c
> index 2477e18c7..b259b0b58 100644
> --- a/drivers/net/wireless/realtek/rtl818x/rtl8180/dev.c
> +++ b/drivers/net/wireless/realtek/rtl818x/rtl8180/dev.c
> @@ -1871,7 +1871,7 @@ static int rtl8180_probe(struct pci_dev *pdev,
>  
>  	if (priv->chip_family != RTL818X_CHIP_FAMILY_RTL8180) {
>  		priv->band.n_bitrates = ARRAY_SIZE(rtl818x_rates);
> -		pci_try_set_mwi(pdev);
> +		pci_set_mwi(pdev);
>  	}
>  
>  	if (priv->chip_family != RTL818X_CHIP_FAMILY_RTL8180)
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 9a5500287..f0ab432d2 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -4389,25 +4389,6 @@ int pcim_set_mwi(struct pci_dev *dev)
>  }
>  EXPORT_SYMBOL(pcim_set_mwi);
>  
> -/**
> - * pci_try_set_mwi - enables memory-write-invalidate PCI transaction
> - * @dev: the PCI device for which MWI is enabled
> - *
> - * Enables the Memory-Write-Invalidate transaction in %PCI_COMMAND.
> - * Callers are not required to check the return value.
> - *
> - * RETURNS: An appropriate -ERRNO error value on error, or zero for success.
> - */
> -int pci_try_set_mwi(struct pci_dev *dev)
> -{
> -#ifdef PCI_DISABLE_MWI
> -	return 0;
> -#else
> -	return pci_set_mwi(dev);
> -#endif
> -}
> -EXPORT_SYMBOL(pci_try_set_mwi);
> -
>  /**
>   * pci_clear_mwi - disables Memory-Write-Invalidate for device dev
>   * @dev: the PCI device to disable
> diff --git a/drivers/scsi/3w-9xxx.c b/drivers/scsi/3w-9xxx.c
> index b4718a1b2..d869485e2 100644
> --- a/drivers/scsi/3w-9xxx.c
> +++ b/drivers/scsi/3w-9xxx.c
> @@ -2018,7 +2018,7 @@ static int twa_probe(struct pci_dev *pdev, const struct pci_device_id *dev_id)
>  	}
>  
>  	pci_set_master(pdev);
> -	pci_try_set_mwi(pdev);
> +	pci_set_mwi(pdev);
>  
>  	retval = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
>  	if (retval)
> @@ -2227,7 +2227,7 @@ static int __maybe_unused twa_resume(struct device *dev)
>  
>  	printk(KERN_WARNING "3w-9xxx: Resuming host %d.\n", tw_dev->host->host_no);
>  
> -	pci_try_set_mwi(pdev);
> +	pci_set_mwi(pdev);
>  
>  	retval = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
>  	if (retval)
> diff --git a/drivers/scsi/3w-sas.c b/drivers/scsi/3w-sas.c
> index b8f1848ec..49ca153b8 100644
> --- a/drivers/scsi/3w-sas.c
> +++ b/drivers/scsi/3w-sas.c
> @@ -1571,7 +1571,7 @@ static int twl_probe(struct pci_dev *pdev, const struct pci_device_id *dev_id)
>  	}
>  
>  	pci_set_master(pdev);
> -	pci_try_set_mwi(pdev);
> +	pci_set_mwi(pdev);
>  
>  	retval = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
>  	if (retval)
> @@ -1790,7 +1790,7 @@ static int __maybe_unused twl_resume(struct device *dev)
>  	TW_Device_Extension *tw_dev = (TW_Device_Extension *)host->hostdata;
>  
>  	printk(KERN_WARNING "3w-sas: Resuming host %d.\n", tw_dev->host->host_no);
> -	pci_try_set_mwi(pdev);
> +	pci_set_mwi(pdev);
>  
>  	retval = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
>  	if (retval)
> diff --git a/drivers/scsi/csiostor/csio_init.c b/drivers/scsi/csiostor/csio_init.c
> index 390b07bf9..c20bf44c6 100644
> --- a/drivers/scsi/csiostor/csio_init.c
> +++ b/drivers/scsi/csiostor/csio_init.c
> @@ -201,7 +201,7 @@ csio_pci_init(struct pci_dev *pdev, int *bars)
>  		goto err_disable_device;
>  
>  	pci_set_master(pdev);
> -	pci_try_set_mwi(pdev);
> +	pci_set_mwi(pdev);
>  
>  	rv = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
>  	if (rv)
> diff --git a/drivers/scsi/lpfc/lpfc_init.c b/drivers/scsi/lpfc/lpfc_init.c
> index ac67f420e..b4833c0f8 100644
> --- a/drivers/scsi/lpfc/lpfc_init.c
> +++ b/drivers/scsi/lpfc/lpfc_init.c
> @@ -6119,7 +6119,7 @@ lpfc_enable_pci_dev(struct lpfc_hba *phba)
>  		goto out_disable_device;
>  	/* Set up device as PCI master and save state for EEH */
>  	pci_set_master(pdev);
> -	pci_try_set_mwi(pdev);
> +	pci_set_mwi(pdev);
>  	pci_save_state(pdev);
>  
>  	/* PCIe EEH recovery on powerpc platforms needs fundamental reset */
> diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c
> index 5626e9b69..76019dc2e 100644
> --- a/drivers/scsi/qla2xxx/qla_init.c
> +++ b/drivers/scsi/qla2xxx/qla_init.c
> @@ -2356,7 +2356,7 @@ qla2100_pci_config(scsi_qla_host_t *vha)
>  	struct device_reg_2xxx __iomem *reg = &ha->iobase->isp;
>  
>  	pci_set_master(ha->pdev);
> -	pci_try_set_mwi(ha->pdev);
> +	pci_set_mwi(ha->pdev);
>  
>  	pci_read_config_word(ha->pdev, PCI_COMMAND, &w);
>  	w |= (PCI_COMMAND_PARITY | PCI_COMMAND_SERR);
> @@ -2388,7 +2388,7 @@ qla2300_pci_config(scsi_qla_host_t *vha)
>  	struct device_reg_2xxx __iomem *reg = &ha->iobase->isp;
>  
>  	pci_set_master(ha->pdev);
> -	pci_try_set_mwi(ha->pdev);
> +	pci_set_mwi(ha->pdev);
>  
>  	pci_read_config_word(ha->pdev, PCI_COMMAND, &w);
>  	w |= (PCI_COMMAND_PARITY | PCI_COMMAND_SERR);
> @@ -2469,7 +2469,7 @@ qla24xx_pci_config(scsi_qla_host_t *vha)
>  	struct device_reg_24xx __iomem *reg = &ha->iobase->isp24;
>  
>  	pci_set_master(ha->pdev);
> -	pci_try_set_mwi(ha->pdev);
> +	pci_set_mwi(ha->pdev);
>  
>  	pci_read_config_word(ha->pdev, PCI_COMMAND, &w);
>  	w |= (PCI_COMMAND_PARITY | PCI_COMMAND_SERR);
> @@ -2511,7 +2511,7 @@ qla25xx_pci_config(scsi_qla_host_t *vha)
>  	struct qla_hw_data *ha = vha->hw;
>  
>  	pci_set_master(ha->pdev);
> -	pci_try_set_mwi(ha->pdev);
> +	pci_set_mwi(ha->pdev);
>  
>  	pci_read_config_word(ha->pdev, PCI_COMMAND, &w);
>  	w |= (PCI_COMMAND_PARITY | PCI_COMMAND_SERR);
> diff --git a/drivers/scsi/qla2xxx/qla_mr.c b/drivers/scsi/qla2xxx/qla_mr.c
> index ca7306685..e9763d8e3 100644
> --- a/drivers/scsi/qla2xxx/qla_mr.c
> +++ b/drivers/scsi/qla2xxx/qla_mr.c
> @@ -499,7 +499,7 @@ qlafx00_pci_config(scsi_qla_host_t *vha)
>  	struct qla_hw_data *ha = vha->hw;
>  
>  	pci_set_master(ha->pdev);
> -	pci_try_set_mwi(ha->pdev);
> +	pci_set_mwi(ha->pdev);
>  
>  	pci_read_config_word(ha->pdev, PCI_COMMAND, &w);
>  	w |= (PCI_COMMAND_PARITY | PCI_COMMAND_SERR);
> diff --git a/drivers/tty/serial/8250/8250_lpss.c b/drivers/tty/serial/8250/8250_lpss.c
> index 4dee8a9e0..8acc1e5c9 100644
> --- a/drivers/tty/serial/8250/8250_lpss.c
> +++ b/drivers/tty/serial/8250/8250_lpss.c
> @@ -193,7 +193,7 @@ static void qrk_serial_setup_dma(struct lpss8250 *lpss, struct uart_port *port)
>  	if (ret)
>  		return;
>  
> -	pci_try_set_mwi(pdev);
> +	pci_set_mwi(pdev);
>  
>  	/* Special DMA address for UART */
>  	dma->rx_dma_addr = 0xfffff000;
> diff --git a/drivers/usb/chipidea/ci_hdrc_pci.c b/drivers/usb/chipidea/ci_hdrc_pci.c
> index d63479e1a..d412fa910 100644
> --- a/drivers/usb/chipidea/ci_hdrc_pci.c
> +++ b/drivers/usb/chipidea/ci_hdrc_pci.c
> @@ -78,7 +78,7 @@ static int ci_hdrc_pci_probe(struct pci_dev *pdev,
>  	}
>  
>  	pci_set_master(pdev);
> -	pci_try_set_mwi(pdev);
> +	pci_set_mwi(pdev);
>  
>  	/* register a nop PHY */
>  	ci->phy = usb_phy_generic_register();
> diff --git a/drivers/usb/gadget/udc/amd5536udc_pci.c b/drivers/usb/gadget/udc/amd5536udc_pci.c
> index 8d387e0e4..9630ce8d3 100644
> --- a/drivers/usb/gadget/udc/amd5536udc_pci.c
> +++ b/drivers/usb/gadget/udc/amd5536udc_pci.c
> @@ -151,7 +151,7 @@ static int udc_pci_probe(
>  	dev->chiprev = pdev->revision;
>  
>  	pci_set_master(pdev);
> -	pci_try_set_mwi(pdev);
> +	pci_set_mwi(pdev);
>  
>  	/* init dma pools */
>  	if (use_dma) {
> diff --git a/drivers/usb/gadget/udc/net2280.c b/drivers/usb/gadget/udc/net2280.c
> index fc9f99fe7..e15520698 100644
> --- a/drivers/usb/gadget/udc/net2280.c
> +++ b/drivers/usb/gadget/udc/net2280.c
> @@ -3761,7 +3761,7 @@ static int net2280_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>  			&dev->pci->pcimstctl);
>  	/* erratum 0115 shouldn't appear: Linux inits PCI_LATENCY_TIMER */
>  	pci_set_master(pdev);
> -	pci_try_set_mwi(pdev);
> +	pci_set_mwi(pdev);
>  
>  	/* ... also flushes any posted pci writes */
>  	dev->chiprev = get_idx_reg(dev->regs, REG_CHIPREV) & 0xffff;
> diff --git a/drivers/usb/gadget/udc/pch_udc.c b/drivers/usb/gadget/udc/pch_udc.c
> index a3c1fc924..4a0484a0c 100644
> --- a/drivers/usb/gadget/udc/pch_udc.c
> +++ b/drivers/usb/gadget/udc/pch_udc.c
> @@ -3100,7 +3100,7 @@ static int pch_udc_probe(struct pci_dev *pdev,
>  	}
>  
>  	pci_set_master(pdev);
> -	pci_try_set_mwi(pdev);
> +	pci_set_mwi(pdev);
>  
>  	/* device struct setup */
>  	spin_lock_init(&dev->lock);
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index de75f6a4d..c590f616d 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -1191,9 +1191,8 @@ void pci_clear_master(struct pci_dev *dev);
>  
>  int pci_set_pcie_reset_state(struct pci_dev *dev, enum pcie_reset_state state);
>  int pci_set_cacheline_size(struct pci_dev *dev);
> -int __must_check pci_set_mwi(struct pci_dev *dev);
> -int __must_check pcim_set_mwi(struct pci_dev *dev);
> -int pci_try_set_mwi(struct pci_dev *dev);
> +int pci_set_mwi(struct pci_dev *dev);
> +int pcim_set_mwi(struct pci_dev *dev);
>  void pci_clear_mwi(struct pci_dev *dev);
>  void pci_intx(struct pci_dev *dev, int enable);
>  bool pci_check_and_mask_intx(struct pci_dev *dev);
> -- 
> 2.29.2
> 

-- 

Thanks,
Peter Chen

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] PCI: Remove pci_try_set_mwi
  2020-12-09  8:31 [PATCH] PCI: Remove pci_try_set_mwi Heiner Kallweit
                   ` (2 preceding siblings ...)
  2020-12-09  9:20 ` Peter Chen
@ 2020-12-09  9:22 ` Lee Jones
  2020-12-09 10:59 ` Andy Shevchenko
  2021-03-26 21:26 ` Bjorn Helgaas
  5 siblings, 0 replies; 12+ messages in thread
From: Lee Jones @ 2020-12-09  9:22 UTC (permalink / raw)
  To: Heiner Kallweit
  Cc: Bjorn Helgaas, Jonathan Corbet, Jens Axboe, Viresh Kumar,
	Andy Shevchenko, Dan Williams, Vinod Koul, David Miller,
	Ion Badulescu, Jakub Kicinski, Lino Sanfilippo,
	Christian Lamparter, Kalle Valo, Luis Chamberlain, Adam Radford,
	James E.J. Bottomley, Martin K. Petersen, James Smart,
	Dick Kennedy, Nilesh Javali, GR-QLogic-Storage-Upstream,
	Greg Kroah-Hartman, Jiri Slaby, Peter Chen, Felipe Balbi,
	linux-pci, linux-doc, Linux Kernel Mailing List, linux-ide,
	dmaengine, netdev, linux-parisc, linux-wireless,
	SCSI development list, linux-serial, Linux USB Mailing List

On Wed, 09 Dec 2020, Heiner Kallweit wrote:

> pci_set_mwi() and pci_try_set_mwi() do exactly the same, just that the
> former one is declared as __must_check. However also some callers of
> pci_set_mwi() have a comment that it's an optional feature. I don't
> think there's much sense in this separation and the use of
> __must_check. Therefore remove pci_try_set_mwi() and remove the
> __must_check attribute from pci_set_mwi().
> I don't expect either function to be used in new code anyway.
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
> patch applies on top of pci/misc for v5.11
> ---
>  Documentation/PCI/pci.rst                     |  5 +----
>  drivers/ata/pata_cs5530.c                     |  2 +-
>  drivers/ata/sata_mv.c                         |  2 +-
>  drivers/dma/dw/pci.c                          |  2 +-
>  drivers/dma/hsu/pci.c                         |  2 +-
>  drivers/ide/cs5530.c                          |  2 +-

>  drivers/mfd/intel-lpss-pci.c                  |  2 +-

Acked-by: Lee Jones <lee.jones@linaro.org>

>  drivers/net/ethernet/adaptec/starfire.c       |  2 +-
>  drivers/net/ethernet/alacritech/slicoss.c     |  2 +-
>  drivers/net/ethernet/dec/tulip/tulip_core.c   |  5 +----
>  drivers/net/ethernet/sun/cassini.c            |  4 ++--
>  drivers/net/wireless/intersil/p54/p54pci.c    |  2 +-
>  .../intersil/prism54/islpci_hotplug.c         |  3 +--
>  .../wireless/realtek/rtl818x/rtl8180/dev.c    |  2 +-
>  drivers/pci/pci.c                             | 19 -------------------
>  drivers/scsi/3w-9xxx.c                        |  4 ++--
>  drivers/scsi/3w-sas.c                         |  4 ++--
>  drivers/scsi/csiostor/csio_init.c             |  2 +-
>  drivers/scsi/lpfc/lpfc_init.c                 |  2 +-
>  drivers/scsi/qla2xxx/qla_init.c               |  8 ++++----
>  drivers/scsi/qla2xxx/qla_mr.c                 |  2 +-
>  drivers/tty/serial/8250/8250_lpss.c           |  2 +-
>  drivers/usb/chipidea/ci_hdrc_pci.c            |  2 +-
>  drivers/usb/gadget/udc/amd5536udc_pci.c       |  2 +-
>  drivers/usb/gadget/udc/net2280.c              |  2 +-
>  drivers/usb/gadget/udc/pch_udc.c              |  2 +-
>  include/linux/pci.h                           |  5 ++---
>  27 files changed, 33 insertions(+), 60 deletions(-)

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] PCI: Remove pci_try_set_mwi
  2020-12-09  8:31 [PATCH] PCI: Remove pci_try_set_mwi Heiner Kallweit
                   ` (3 preceding siblings ...)
  2020-12-09  9:22 ` Lee Jones
@ 2020-12-09 10:59 ` Andy Shevchenko
  2020-12-09 11:02   ` Andy Shevchenko
  2021-03-26 21:26 ` Bjorn Helgaas
  5 siblings, 1 reply; 12+ messages in thread
From: Andy Shevchenko @ 2020-12-09 10:59 UTC (permalink / raw)
  To: Heiner Kallweit
  Cc: Bjorn Helgaas, Jonathan Corbet, Jens Axboe, Viresh Kumar,
	Andy Shevchenko, Dan Williams, Vinod Koul, David Miller,
	Lee Jones, Ion Badulescu, Jakub Kicinski, Lino Sanfilippo,
	Christian Lamparter, Kalle Valo, Luis Chamberlain, Adam Radford,
	James E.J. Bottomley, Martin K. Petersen, James Smart,
	Dick Kennedy, Nilesh Javali, GR-QLogic-Storage-Upstream,
	Greg Kroah-Hartman, Jiri Slaby, Peter Chen, Felipe Balbi,
	linux-pci, Linux Documentation List, Linux Kernel Mailing List,
	linux-ide, dmaengine, netdev, linux-parisc, linux-wireless,
	SCSI development list, open list:SERIAL DRIVERS,
	Linux USB Mailing List

On Wed, Dec 9, 2020 at 10:35 AM Heiner Kallweit <hkallweit1@gmail.com> wrote:
> pci_set_mwi() and pci_try_set_mwi() do exactly the same, just that the
> former one is declared as __must_check. However also some callers of

However also -> However

> pci_set_mwi() have a comment that it's an optional feature. I don't
> think there's much sense in this separation and the use of
> __must_check. Therefore remove pci_try_set_mwi() and remove the
> __must_check attribute from pci_set_mwi().


> I don't expect either function to be used in new code anyway.

You probably want to elaborate here that the feature is specific to
PCI and isn't present on PCIe.

Besides that one comment below.
After addressing, have my
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
for the files left in this message.

...

>  drivers/dma/dw/pci.c                          |  2 +-
>  drivers/dma/hsu/pci.c                         |  2 +-

>  drivers/mfd/intel-lpss-pci.c                  |  2 +-

>  drivers/pci/pci.c                             | 19 -------------------

>  drivers/tty/serial/8250/8250_lpss.c           |  2 +-
>  drivers/usb/chipidea/ci_hdrc_pci.c            |  2 +-

>  drivers/usb/gadget/udc/pch_udc.c              |  2 +-
>  include/linux/pci.h                           |  5 ++---

> diff --git a/Documentation/PCI/pci.rst b/Documentation/PCI/pci.rst
> index 814b40f83..120362cc9 100644
> --- a/Documentation/PCI/pci.rst
> +++ b/Documentation/PCI/pci.rst
> @@ -226,10 +226,7 @@ If the PCI device can use the PCI Memory-Write-Invalidate transaction,
>  call pci_set_mwi().  This enables the PCI_COMMAND bit for Mem-Wr-Inval
>  and also ensures that the cache line size register is set correctly.
>  Check the return value of pci_set_mwi() as not all architectures
> -or chip-sets may support Memory-Write-Invalidate.  Alternatively,
> -if Mem-Wr-Inval would be nice to have but is not required, call
> -pci_try_set_mwi() to have the system do its best effort at enabling
> -Mem-Wr-Inval.
> +or chip-sets may support Memory-Write-Invalidate.

...

> diff --git a/drivers/dma/dw/pci.c b/drivers/dma/dw/pci.c
> index 1142aa6f8..1c20b7485 100644
> --- a/drivers/dma/dw/pci.c
> +++ b/drivers/dma/dw/pci.c
> @@ -30,7 +30,7 @@ static int dw_pci_probe(struct pci_dev *pdev, const struct pci_device_id *pid)
>         }
>
>         pci_set_master(pdev);
> -       pci_try_set_mwi(pdev);
> +       pci_set_mwi(pdev);
>
>         ret = pci_set_dma_mask(pdev, DMA_BIT_MASK(32));
>         if (ret)
> diff --git a/drivers/dma/hsu/pci.c b/drivers/dma/hsu/pci.c
> index 07cc7320a..420dd3706 100644
> --- a/drivers/dma/hsu/pci.c
> +++ b/drivers/dma/hsu/pci.c
> @@ -73,7 +73,7 @@ static int hsu_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>         }
>
>         pci_set_master(pdev);
> -       pci_try_set_mwi(pdev);
> +       pci_set_mwi(pdev);
>
>         ret = pci_set_dma_mask(pdev, DMA_BIT_MASK(32));
>         if (ret)

...

> diff --git a/drivers/mfd/intel-lpss-pci.c b/drivers/mfd/intel-lpss-pci.c
> index 2d7c588ef..a0c3be750 100644
> --- a/drivers/mfd/intel-lpss-pci.c
> +++ b/drivers/mfd/intel-lpss-pci.c
> @@ -39,7 +39,7 @@ static int intel_lpss_pci_probe(struct pci_dev *pdev,
>
>         /* Probably it is enough to set this for iDMA capable devices only */
>         pci_set_master(pdev);
> -       pci_try_set_mwi(pdev);
> +       pci_set_mwi(pdev);
>
>         ret = intel_lpss_probe(&pdev->dev, info);
>         if (ret)

...

> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 9a5500287..f0ab432d2 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -4389,25 +4389,6 @@ int pcim_set_mwi(struct pci_dev *dev)
>  }
>  EXPORT_SYMBOL(pcim_set_mwi);
>
> -/**
> - * pci_try_set_mwi - enables memory-write-invalidate PCI transaction
> - * @dev: the PCI device for which MWI is enabled
> - *
> - * Enables the Memory-Write-Invalidate transaction in %PCI_COMMAND.
> - * Callers are not required to check the return value.
> - *
> - * RETURNS: An appropriate -ERRNO error value on error, or zero for success.
> - */
> -int pci_try_set_mwi(struct pci_dev *dev)
> -{

> -#ifdef PCI_DISABLE_MWI
> -       return 0;
> -#else
> -       return pci_set_mwi(dev);
> -#endif

This seems still valid case for PowerPC and SH.

> -}
> -EXPORT_SYMBOL(pci_try_set_mwi);

...

> diff --git a/drivers/tty/serial/8250/8250_lpss.c b/drivers/tty/serial/8250/8250_lpss.c
> index 4dee8a9e0..8acc1e5c9 100644
> --- a/drivers/tty/serial/8250/8250_lpss.c
> +++ b/drivers/tty/serial/8250/8250_lpss.c
> @@ -193,7 +193,7 @@ static void qrk_serial_setup_dma(struct lpss8250 *lpss, struct uart_port *port)
>         if (ret)
>                 return;
>
> -       pci_try_set_mwi(pdev);
> +       pci_set_mwi(pdev);
>
>         /* Special DMA address for UART */
>         dma->rx_dma_addr = 0xfffff000;
> diff --git a/drivers/usb/chipidea/ci_hdrc_pci.c b/drivers/usb/chipidea/ci_hdrc_pci.c
> index d63479e1a..d412fa910 100644
> --- a/drivers/usb/chipidea/ci_hdrc_pci.c
> +++ b/drivers/usb/chipidea/ci_hdrc_pci.c
> @@ -78,7 +78,7 @@ static int ci_hdrc_pci_probe(struct pci_dev *pdev,
>         }
>
>         pci_set_master(pdev);
> -       pci_try_set_mwi(pdev);
> +       pci_set_mwi(pdev);
>
>         /* register a nop PHY */
>         ci->phy = usb_phy_generic_register();

...

> diff --git a/drivers/usb/gadget/udc/pch_udc.c b/drivers/usb/gadget/udc/pch_udc.c
> index a3c1fc924..4a0484a0c 100644
> --- a/drivers/usb/gadget/udc/pch_udc.c
> +++ b/drivers/usb/gadget/udc/pch_udc.c
> @@ -3100,7 +3100,7 @@ static int pch_udc_probe(struct pci_dev *pdev,
>         }
>
>         pci_set_master(pdev);
> -       pci_try_set_mwi(pdev);
> +       pci_set_mwi(pdev);
>
>         /* device struct setup */
>         spin_lock_init(&dev->lock);
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index de75f6a4d..c590f616d 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -1191,9 +1191,8 @@ void pci_clear_master(struct pci_dev *dev);
>
>  int pci_set_pcie_reset_state(struct pci_dev *dev, enum pcie_reset_state state);
>  int pci_set_cacheline_size(struct pci_dev *dev);
> -int __must_check pci_set_mwi(struct pci_dev *dev);
> -int __must_check pcim_set_mwi(struct pci_dev *dev);
> -int pci_try_set_mwi(struct pci_dev *dev);
> +int pci_set_mwi(struct pci_dev *dev);
> +int pcim_set_mwi(struct pci_dev *dev);
>  void pci_clear_mwi(struct pci_dev *dev);
>  void pci_intx(struct pci_dev *dev, int enable);
>  bool pci_check_and_mask_intx(struct pci_dev *dev);


-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] PCI: Remove pci_try_set_mwi
  2020-12-09 10:59 ` Andy Shevchenko
@ 2020-12-09 11:02   ` Andy Shevchenko
  0 siblings, 0 replies; 12+ messages in thread
From: Andy Shevchenko @ 2020-12-09 11:02 UTC (permalink / raw)
  To: Heiner Kallweit
  Cc: Bjorn Helgaas, Jonathan Corbet, Jens Axboe, Viresh Kumar,
	Andy Shevchenko, Dan Williams, Vinod Koul, David Miller,
	Lee Jones, Ion Badulescu, Jakub Kicinski, Lino Sanfilippo,
	Christian Lamparter, Kalle Valo, Luis Chamberlain, Adam Radford,
	James E.J. Bottomley, Martin K. Petersen, James Smart,
	Dick Kennedy, Nilesh Javali, GR-QLogic-Storage-Upstream,
	Greg Kroah-Hartman, Jiri Slaby, Peter Chen, Felipe Balbi,
	linux-pci, Linux Documentation List, Linux Kernel Mailing List,
	linux-ide, dmaengine, netdev, linux-parisc, linux-wireless,
	SCSI development list, open list:SERIAL DRIVERS,
	Linux USB Mailing List

On Wed, Dec 9, 2020 at 12:59 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Wed, Dec 9, 2020 at 10:35 AM Heiner Kallweit <hkallweit1@gmail.com> wrote:

...

> > -int pci_try_set_mwi(struct pci_dev *dev)
> > -{
>
> > -#ifdef PCI_DISABLE_MWI
> > -       return 0;
> > -#else
> > -       return pci_set_mwi(dev);
> > -#endif
>
> This seems still valid case for PowerPC and SH.

I see that pci_set_mwi() also has the ifdeffery (I thought it's only
here), so it's fine.

> > -}
> > -EXPORT_SYMBOL(pci_try_set_mwi);

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] PCI: Remove pci_try_set_mwi
  2020-12-09  8:31 [PATCH] PCI: Remove pci_try_set_mwi Heiner Kallweit
                   ` (4 preceding siblings ...)
  2020-12-09 10:59 ` Andy Shevchenko
@ 2021-03-26 21:26 ` Bjorn Helgaas
  2021-03-26 21:42   ` Andy Shevchenko
  2021-03-27 23:04   ` Heiner Kallweit
  5 siblings, 2 replies; 12+ messages in thread
From: Bjorn Helgaas @ 2021-03-26 21:26 UTC (permalink / raw)
  To: Heiner Kallweit
  Cc: Bjorn Helgaas, Jonathan Corbet, Jens Axboe, Viresh Kumar,
	Andy Shevchenko, Dan Williams, Vinod Koul, David Miller,
	Lee Jones, Ion Badulescu, Jakub Kicinski, Lino Sanfilippo,
	Christian Lamparter, Kalle Valo, Luis Chamberlain, Adam Radford,
	James E.J. Bottomley, Martin K. Petersen, James Smart,
	Dick Kennedy, Nilesh Javali, GR-QLogic-Storage-Upstream,
	Greg Kroah-Hartman, Jiri Slaby, Peter Chen, Felipe Balbi,
	linux-pci, linux-doc, Linux Kernel Mailing List, linux-ide,
	dmaengine, netdev, linux-parisc, linux-wireless,
	SCSI development list, linux-serial, Linux USB Mailing List,
	Randy Dunlap, Andrew Morton

[+cc Randy, Andrew (though I'm sure you have zero interest in this
ancient question :))]

On Wed, Dec 09, 2020 at 09:31:21AM +0100, Heiner Kallweit wrote:
> pci_set_mwi() and pci_try_set_mwi() do exactly the same, just that the
> former one is declared as __must_check. However also some callers of
> pci_set_mwi() have a comment that it's an optional feature. I don't
> think there's much sense in this separation and the use of
> __must_check. Therefore remove pci_try_set_mwi() and remove the
> __must_check attribute from pci_set_mwi().
> I don't expect either function to be used in new code anyway.

There's not much I like better than removing things.  But some
significant thought went into adding pci_try_set_mwi() in the first
place, so I need a little more convincing about why it's safe to
remove it.

The argument should cite the discussion about adding it.  I think one
of the earliest conversations is here:
https://lore.kernel.org/linux-ide/20070404213704.224128ec.randy.dunlap@oracle.com/

> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
> patch applies on top of pci/misc for v5.11
> ---
>  Documentation/PCI/pci.rst                     |  5 +----
>  drivers/ata/pata_cs5530.c                     |  2 +-
>  drivers/ata/sata_mv.c                         |  2 +-
>  drivers/dma/dw/pci.c                          |  2 +-
>  drivers/dma/hsu/pci.c                         |  2 +-
>  drivers/ide/cs5530.c                          |  2 +-
>  drivers/mfd/intel-lpss-pci.c                  |  2 +-
>  drivers/net/ethernet/adaptec/starfire.c       |  2 +-
>  drivers/net/ethernet/alacritech/slicoss.c     |  2 +-
>  drivers/net/ethernet/dec/tulip/tulip_core.c   |  5 +----
>  drivers/net/ethernet/sun/cassini.c            |  4 ++--
>  drivers/net/wireless/intersil/p54/p54pci.c    |  2 +-
>  .../intersil/prism54/islpci_hotplug.c         |  3 +--
>  .../wireless/realtek/rtl818x/rtl8180/dev.c    |  2 +-
>  drivers/pci/pci.c                             | 19 -------------------
>  drivers/scsi/3w-9xxx.c                        |  4 ++--
>  drivers/scsi/3w-sas.c                         |  4 ++--
>  drivers/scsi/csiostor/csio_init.c             |  2 +-
>  drivers/scsi/lpfc/lpfc_init.c                 |  2 +-
>  drivers/scsi/qla2xxx/qla_init.c               |  8 ++++----
>  drivers/scsi/qla2xxx/qla_mr.c                 |  2 +-
>  drivers/tty/serial/8250/8250_lpss.c           |  2 +-
>  drivers/usb/chipidea/ci_hdrc_pci.c            |  2 +-
>  drivers/usb/gadget/udc/amd5536udc_pci.c       |  2 +-
>  drivers/usb/gadget/udc/net2280.c              |  2 +-
>  drivers/usb/gadget/udc/pch_udc.c              |  2 +-
>  include/linux/pci.h                           |  5 ++---
>  27 files changed, 33 insertions(+), 60 deletions(-)
> 
> diff --git a/Documentation/PCI/pci.rst b/Documentation/PCI/pci.rst
> index 814b40f83..120362cc9 100644
> --- a/Documentation/PCI/pci.rst
> +++ b/Documentation/PCI/pci.rst
> @@ -226,10 +226,7 @@ If the PCI device can use the PCI Memory-Write-Invalidate transaction,
>  call pci_set_mwi().  This enables the PCI_COMMAND bit for Mem-Wr-Inval
>  and also ensures that the cache line size register is set correctly.
>  Check the return value of pci_set_mwi() as not all architectures
> -or chip-sets may support Memory-Write-Invalidate.  Alternatively,
> -if Mem-Wr-Inval would be nice to have but is not required, call
> -pci_try_set_mwi() to have the system do its best effort at enabling
> -Mem-Wr-Inval.
> +or chip-sets may support Memory-Write-Invalidate.
>  
>  
>  Request MMIO/IOP resources
> diff --git a/drivers/ata/pata_cs5530.c b/drivers/ata/pata_cs5530.c
> index ad75d02b6..8654b3ae1 100644
> --- a/drivers/ata/pata_cs5530.c
> +++ b/drivers/ata/pata_cs5530.c
> @@ -214,7 +214,7 @@ static int cs5530_init_chip(void)
>  	}
>  
>  	pci_set_master(cs5530_0);
> -	pci_try_set_mwi(cs5530_0);
> +	pci_set_mwi(cs5530_0);
>  
>  	/*
>  	 * Set PCI CacheLineSize to 16-bytes:
> diff --git a/drivers/ata/sata_mv.c b/drivers/ata/sata_mv.c
> index 664ef658a..ee37755ea 100644
> --- a/drivers/ata/sata_mv.c
> +++ b/drivers/ata/sata_mv.c
> @@ -4432,7 +4432,7 @@ static int mv_pci_init_one(struct pci_dev *pdev,
>  	mv_print_info(host);
>  
>  	pci_set_master(pdev);
> -	pci_try_set_mwi(pdev);
> +	pci_set_mwi(pdev);
>  	return ata_host_activate(host, pdev->irq, mv_interrupt, IRQF_SHARED,
>  				 IS_GEN_I(hpriv) ? &mv5_sht : &mv6_sht);
>  }
> diff --git a/drivers/dma/dw/pci.c b/drivers/dma/dw/pci.c
> index 1142aa6f8..1c20b7485 100644
> --- a/drivers/dma/dw/pci.c
> +++ b/drivers/dma/dw/pci.c
> @@ -30,7 +30,7 @@ static int dw_pci_probe(struct pci_dev *pdev, const struct pci_device_id *pid)
>  	}
>  
>  	pci_set_master(pdev);
> -	pci_try_set_mwi(pdev);
> +	pci_set_mwi(pdev);
>  
>  	ret = pci_set_dma_mask(pdev, DMA_BIT_MASK(32));
>  	if (ret)
> diff --git a/drivers/dma/hsu/pci.c b/drivers/dma/hsu/pci.c
> index 07cc7320a..420dd3706 100644
> --- a/drivers/dma/hsu/pci.c
> +++ b/drivers/dma/hsu/pci.c
> @@ -73,7 +73,7 @@ static int hsu_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>  	}
>  
>  	pci_set_master(pdev);
> -	pci_try_set_mwi(pdev);
> +	pci_set_mwi(pdev);
>  
>  	ret = pci_set_dma_mask(pdev, DMA_BIT_MASK(32));
>  	if (ret)
> diff --git a/drivers/ide/cs5530.c b/drivers/ide/cs5530.c
> index 5bb46e713..5d2c421ab 100644
> --- a/drivers/ide/cs5530.c
> +++ b/drivers/ide/cs5530.c
> @@ -168,7 +168,7 @@ static int init_chipset_cs5530(struct pci_dev *dev)
>  	 */
>  
>  	pci_set_master(cs5530_0);
> -	pci_try_set_mwi(cs5530_0);
> +	pci_set_mwi(cs5530_0);
>  
>  	/*
>  	 * Set PCI CacheLineSize to 16-bytes:
> diff --git a/drivers/mfd/intel-lpss-pci.c b/drivers/mfd/intel-lpss-pci.c
> index 2d7c588ef..a0c3be750 100644
> --- a/drivers/mfd/intel-lpss-pci.c
> +++ b/drivers/mfd/intel-lpss-pci.c
> @@ -39,7 +39,7 @@ static int intel_lpss_pci_probe(struct pci_dev *pdev,
>  
>  	/* Probably it is enough to set this for iDMA capable devices only */
>  	pci_set_master(pdev);
> -	pci_try_set_mwi(pdev);
> +	pci_set_mwi(pdev);
>  
>  	ret = intel_lpss_probe(&pdev->dev, info);
>  	if (ret)
> diff --git a/drivers/net/ethernet/adaptec/starfire.c b/drivers/net/ethernet/adaptec/starfire.c
> index 555299737..1dbff34c4 100644
> --- a/drivers/net/ethernet/adaptec/starfire.c
> +++ b/drivers/net/ethernet/adaptec/starfire.c
> @@ -679,7 +679,7 @@ static int starfire_init_one(struct pci_dev *pdev,
>  	pci_set_master(pdev);
>  
>  	/* enable MWI -- it vastly improves Rx performance on sparc64 */
> -	pci_try_set_mwi(pdev);
> +	pci_set_mwi(pdev);
>  
>  #ifdef ZEROCOPY
>  	/* Starfire can do TCP/UDP checksumming */
> diff --git a/drivers/net/ethernet/alacritech/slicoss.c b/drivers/net/ethernet/alacritech/slicoss.c
> index 696517eae..544510f57 100644
> --- a/drivers/net/ethernet/alacritech/slicoss.c
> +++ b/drivers/net/ethernet/alacritech/slicoss.c
> @@ -1749,7 +1749,7 @@ static int slic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>  	}
>  
>  	pci_set_master(pdev);
> -	pci_try_set_mwi(pdev);
> +	pci_set_mwi(pdev);
>  
>  	slic_configure_pci(pdev);
>  
> diff --git a/drivers/net/ethernet/dec/tulip/tulip_core.c b/drivers/net/ethernet/dec/tulip/tulip_core.c
> index c1dcd6ca1..4f6debb24 100644
> --- a/drivers/net/ethernet/dec/tulip/tulip_core.c
> +++ b/drivers/net/ethernet/dec/tulip/tulip_core.c
> @@ -1193,10 +1193,7 @@ static void tulip_mwi_config(struct pci_dev *pdev, struct net_device *dev)
>  	/* if we have any cache line size at all, we can do MRM and MWI */
>  	csr0 |= MRM | MWI;
>  
> -	/* Enable MWI in the standard PCI command bit.
> -	 * Check for the case where MWI is desired but not available
> -	 */
> -	pci_try_set_mwi(pdev);
> +	pci_set_mwi(pdev);
>  
>  	/* read result from hardware (in case bit refused to enable) */
>  	pci_read_config_word(pdev, PCI_COMMAND, &pci_command);
> diff --git a/drivers/net/ethernet/sun/cassini.c b/drivers/net/ethernet/sun/cassini.c
> index 9ff894ba8..9a95ec989 100644
> --- a/drivers/net/ethernet/sun/cassini.c
> +++ b/drivers/net/ethernet/sun/cassini.c
> @@ -4933,14 +4933,14 @@ static int cas_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>  	pci_cmd &= ~PCI_COMMAND_SERR;
>  	pci_cmd |= PCI_COMMAND_PARITY;
>  	pci_write_config_word(pdev, PCI_COMMAND, pci_cmd);
> -	if (pci_try_set_mwi(pdev))
> +	if (pci_set_mwi(pdev))
>  		pr_warn("Could not enable MWI for %s\n", pci_name(pdev));
>  
>  	cas_program_bridge(pdev);
>  
>  	/*
>  	 * On some architectures, the default cache line size set
> -	 * by pci_try_set_mwi reduces perforamnce.  We have to increase
> +	 * by pci_set_mwi reduces performance.  We have to increase
>  	 * it for this case.  To start, we'll print some configuration
>  	 * data.
>  	 */
> diff --git a/drivers/net/wireless/intersil/p54/p54pci.c b/drivers/net/wireless/intersil/p54/p54pci.c
> index e97ee547b..c76326d1e 100644
> --- a/drivers/net/wireless/intersil/p54/p54pci.c
> +++ b/drivers/net/wireless/intersil/p54/p54pci.c
> @@ -583,7 +583,7 @@ static int p54p_probe(struct pci_dev *pdev,
>  	}
>  
>  	pci_set_master(pdev);
> -	pci_try_set_mwi(pdev);
> +	pci_set_mwi(pdev);
>  
>  	pci_write_config_byte(pdev, 0x40, 0);
>  	pci_write_config_byte(pdev, 0x41, 0);
> diff --git a/drivers/net/wireless/intersil/prism54/islpci_hotplug.c b/drivers/net/wireless/intersil/prism54/islpci_hotplug.c
> index 31a1e6132..e8087d9a5 100644
> --- a/drivers/net/wireless/intersil/prism54/islpci_hotplug.c
> +++ b/drivers/net/wireless/intersil/prism54/islpci_hotplug.c
> @@ -153,8 +153,7 @@ prism54_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>  	DEBUG(SHOW_TRACING, "%s: pci_set_master(pdev)\n", DRV_NAME);
>  	pci_set_master(pdev);
>  
> -	/* enable MWI */
> -	pci_try_set_mwi(pdev);
> +	pci_set_mwi(pdev);
>  
>  	/* setup the network device interface and its structure */
>  	if (!(ndev = islpci_setup(pdev))) {
> diff --git a/drivers/net/wireless/realtek/rtl818x/rtl8180/dev.c b/drivers/net/wireless/realtek/rtl818x/rtl8180/dev.c
> index 2477e18c7..b259b0b58 100644
> --- a/drivers/net/wireless/realtek/rtl818x/rtl8180/dev.c
> +++ b/drivers/net/wireless/realtek/rtl818x/rtl8180/dev.c
> @@ -1871,7 +1871,7 @@ static int rtl8180_probe(struct pci_dev *pdev,
>  
>  	if (priv->chip_family != RTL818X_CHIP_FAMILY_RTL8180) {
>  		priv->band.n_bitrates = ARRAY_SIZE(rtl818x_rates);
> -		pci_try_set_mwi(pdev);
> +		pci_set_mwi(pdev);
>  	}
>  
>  	if (priv->chip_family != RTL818X_CHIP_FAMILY_RTL8180)
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 9a5500287..f0ab432d2 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -4389,25 +4389,6 @@ int pcim_set_mwi(struct pci_dev *dev)
>  }
>  EXPORT_SYMBOL(pcim_set_mwi);
>  
> -/**
> - * pci_try_set_mwi - enables memory-write-invalidate PCI transaction
> - * @dev: the PCI device for which MWI is enabled
> - *
> - * Enables the Memory-Write-Invalidate transaction in %PCI_COMMAND.
> - * Callers are not required to check the return value.
> - *
> - * RETURNS: An appropriate -ERRNO error value on error, or zero for success.
> - */
> -int pci_try_set_mwi(struct pci_dev *dev)
> -{
> -#ifdef PCI_DISABLE_MWI
> -	return 0;
> -#else
> -	return pci_set_mwi(dev);
> -#endif
> -}
> -EXPORT_SYMBOL(pci_try_set_mwi);
> -
>  /**
>   * pci_clear_mwi - disables Memory-Write-Invalidate for device dev
>   * @dev: the PCI device to disable
> diff --git a/drivers/scsi/3w-9xxx.c b/drivers/scsi/3w-9xxx.c
> index b4718a1b2..d869485e2 100644
> --- a/drivers/scsi/3w-9xxx.c
> +++ b/drivers/scsi/3w-9xxx.c
> @@ -2018,7 +2018,7 @@ static int twa_probe(struct pci_dev *pdev, const struct pci_device_id *dev_id)
>  	}
>  
>  	pci_set_master(pdev);
> -	pci_try_set_mwi(pdev);
> +	pci_set_mwi(pdev);
>  
>  	retval = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
>  	if (retval)
> @@ -2227,7 +2227,7 @@ static int __maybe_unused twa_resume(struct device *dev)
>  
>  	printk(KERN_WARNING "3w-9xxx: Resuming host %d.\n", tw_dev->host->host_no);
>  
> -	pci_try_set_mwi(pdev);
> +	pci_set_mwi(pdev);
>  
>  	retval = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
>  	if (retval)
> diff --git a/drivers/scsi/3w-sas.c b/drivers/scsi/3w-sas.c
> index b8f1848ec..49ca153b8 100644
> --- a/drivers/scsi/3w-sas.c
> +++ b/drivers/scsi/3w-sas.c
> @@ -1571,7 +1571,7 @@ static int twl_probe(struct pci_dev *pdev, const struct pci_device_id *dev_id)
>  	}
>  
>  	pci_set_master(pdev);
> -	pci_try_set_mwi(pdev);
> +	pci_set_mwi(pdev);
>  
>  	retval = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
>  	if (retval)
> @@ -1790,7 +1790,7 @@ static int __maybe_unused twl_resume(struct device *dev)
>  	TW_Device_Extension *tw_dev = (TW_Device_Extension *)host->hostdata;
>  
>  	printk(KERN_WARNING "3w-sas: Resuming host %d.\n", tw_dev->host->host_no);
> -	pci_try_set_mwi(pdev);
> +	pci_set_mwi(pdev);
>  
>  	retval = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
>  	if (retval)
> diff --git a/drivers/scsi/csiostor/csio_init.c b/drivers/scsi/csiostor/csio_init.c
> index 390b07bf9..c20bf44c6 100644
> --- a/drivers/scsi/csiostor/csio_init.c
> +++ b/drivers/scsi/csiostor/csio_init.c
> @@ -201,7 +201,7 @@ csio_pci_init(struct pci_dev *pdev, int *bars)
>  		goto err_disable_device;
>  
>  	pci_set_master(pdev);
> -	pci_try_set_mwi(pdev);
> +	pci_set_mwi(pdev);
>  
>  	rv = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
>  	if (rv)
> diff --git a/drivers/scsi/lpfc/lpfc_init.c b/drivers/scsi/lpfc/lpfc_init.c
> index ac67f420e..b4833c0f8 100644
> --- a/drivers/scsi/lpfc/lpfc_init.c
> +++ b/drivers/scsi/lpfc/lpfc_init.c
> @@ -6119,7 +6119,7 @@ lpfc_enable_pci_dev(struct lpfc_hba *phba)
>  		goto out_disable_device;
>  	/* Set up device as PCI master and save state for EEH */
>  	pci_set_master(pdev);
> -	pci_try_set_mwi(pdev);
> +	pci_set_mwi(pdev);
>  	pci_save_state(pdev);
>  
>  	/* PCIe EEH recovery on powerpc platforms needs fundamental reset */
> diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c
> index 5626e9b69..76019dc2e 100644
> --- a/drivers/scsi/qla2xxx/qla_init.c
> +++ b/drivers/scsi/qla2xxx/qla_init.c
> @@ -2356,7 +2356,7 @@ qla2100_pci_config(scsi_qla_host_t *vha)
>  	struct device_reg_2xxx __iomem *reg = &ha->iobase->isp;
>  
>  	pci_set_master(ha->pdev);
> -	pci_try_set_mwi(ha->pdev);
> +	pci_set_mwi(ha->pdev);
>  
>  	pci_read_config_word(ha->pdev, PCI_COMMAND, &w);
>  	w |= (PCI_COMMAND_PARITY | PCI_COMMAND_SERR);
> @@ -2388,7 +2388,7 @@ qla2300_pci_config(scsi_qla_host_t *vha)
>  	struct device_reg_2xxx __iomem *reg = &ha->iobase->isp;
>  
>  	pci_set_master(ha->pdev);
> -	pci_try_set_mwi(ha->pdev);
> +	pci_set_mwi(ha->pdev);
>  
>  	pci_read_config_word(ha->pdev, PCI_COMMAND, &w);
>  	w |= (PCI_COMMAND_PARITY | PCI_COMMAND_SERR);
> @@ -2469,7 +2469,7 @@ qla24xx_pci_config(scsi_qla_host_t *vha)
>  	struct device_reg_24xx __iomem *reg = &ha->iobase->isp24;
>  
>  	pci_set_master(ha->pdev);
> -	pci_try_set_mwi(ha->pdev);
> +	pci_set_mwi(ha->pdev);
>  
>  	pci_read_config_word(ha->pdev, PCI_COMMAND, &w);
>  	w |= (PCI_COMMAND_PARITY | PCI_COMMAND_SERR);
> @@ -2511,7 +2511,7 @@ qla25xx_pci_config(scsi_qla_host_t *vha)
>  	struct qla_hw_data *ha = vha->hw;
>  
>  	pci_set_master(ha->pdev);
> -	pci_try_set_mwi(ha->pdev);
> +	pci_set_mwi(ha->pdev);
>  
>  	pci_read_config_word(ha->pdev, PCI_COMMAND, &w);
>  	w |= (PCI_COMMAND_PARITY | PCI_COMMAND_SERR);
> diff --git a/drivers/scsi/qla2xxx/qla_mr.c b/drivers/scsi/qla2xxx/qla_mr.c
> index ca7306685..e9763d8e3 100644
> --- a/drivers/scsi/qla2xxx/qla_mr.c
> +++ b/drivers/scsi/qla2xxx/qla_mr.c
> @@ -499,7 +499,7 @@ qlafx00_pci_config(scsi_qla_host_t *vha)
>  	struct qla_hw_data *ha = vha->hw;
>  
>  	pci_set_master(ha->pdev);
> -	pci_try_set_mwi(ha->pdev);
> +	pci_set_mwi(ha->pdev);
>  
>  	pci_read_config_word(ha->pdev, PCI_COMMAND, &w);
>  	w |= (PCI_COMMAND_PARITY | PCI_COMMAND_SERR);
> diff --git a/drivers/tty/serial/8250/8250_lpss.c b/drivers/tty/serial/8250/8250_lpss.c
> index 4dee8a9e0..8acc1e5c9 100644
> --- a/drivers/tty/serial/8250/8250_lpss.c
> +++ b/drivers/tty/serial/8250/8250_lpss.c
> @@ -193,7 +193,7 @@ static void qrk_serial_setup_dma(struct lpss8250 *lpss, struct uart_port *port)
>  	if (ret)
>  		return;
>  
> -	pci_try_set_mwi(pdev);
> +	pci_set_mwi(pdev);
>  
>  	/* Special DMA address for UART */
>  	dma->rx_dma_addr = 0xfffff000;
> diff --git a/drivers/usb/chipidea/ci_hdrc_pci.c b/drivers/usb/chipidea/ci_hdrc_pci.c
> index d63479e1a..d412fa910 100644
> --- a/drivers/usb/chipidea/ci_hdrc_pci.c
> +++ b/drivers/usb/chipidea/ci_hdrc_pci.c
> @@ -78,7 +78,7 @@ static int ci_hdrc_pci_probe(struct pci_dev *pdev,
>  	}
>  
>  	pci_set_master(pdev);
> -	pci_try_set_mwi(pdev);
> +	pci_set_mwi(pdev);
>  
>  	/* register a nop PHY */
>  	ci->phy = usb_phy_generic_register();
> diff --git a/drivers/usb/gadget/udc/amd5536udc_pci.c b/drivers/usb/gadget/udc/amd5536udc_pci.c
> index 8d387e0e4..9630ce8d3 100644
> --- a/drivers/usb/gadget/udc/amd5536udc_pci.c
> +++ b/drivers/usb/gadget/udc/amd5536udc_pci.c
> @@ -151,7 +151,7 @@ static int udc_pci_probe(
>  	dev->chiprev = pdev->revision;
>  
>  	pci_set_master(pdev);
> -	pci_try_set_mwi(pdev);
> +	pci_set_mwi(pdev);
>  
>  	/* init dma pools */
>  	if (use_dma) {
> diff --git a/drivers/usb/gadget/udc/net2280.c b/drivers/usb/gadget/udc/net2280.c
> index fc9f99fe7..e15520698 100644
> --- a/drivers/usb/gadget/udc/net2280.c
> +++ b/drivers/usb/gadget/udc/net2280.c
> @@ -3761,7 +3761,7 @@ static int net2280_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>  			&dev->pci->pcimstctl);
>  	/* erratum 0115 shouldn't appear: Linux inits PCI_LATENCY_TIMER */
>  	pci_set_master(pdev);
> -	pci_try_set_mwi(pdev);
> +	pci_set_mwi(pdev);
>  
>  	/* ... also flushes any posted pci writes */
>  	dev->chiprev = get_idx_reg(dev->regs, REG_CHIPREV) & 0xffff;
> diff --git a/drivers/usb/gadget/udc/pch_udc.c b/drivers/usb/gadget/udc/pch_udc.c
> index a3c1fc924..4a0484a0c 100644
> --- a/drivers/usb/gadget/udc/pch_udc.c
> +++ b/drivers/usb/gadget/udc/pch_udc.c
> @@ -3100,7 +3100,7 @@ static int pch_udc_probe(struct pci_dev *pdev,
>  	}
>  
>  	pci_set_master(pdev);
> -	pci_try_set_mwi(pdev);
> +	pci_set_mwi(pdev);
>  
>  	/* device struct setup */
>  	spin_lock_init(&dev->lock);
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index de75f6a4d..c590f616d 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -1191,9 +1191,8 @@ void pci_clear_master(struct pci_dev *dev);
>  
>  int pci_set_pcie_reset_state(struct pci_dev *dev, enum pcie_reset_state state);
>  int pci_set_cacheline_size(struct pci_dev *dev);
> -int __must_check pci_set_mwi(struct pci_dev *dev);
> -int __must_check pcim_set_mwi(struct pci_dev *dev);
> -int pci_try_set_mwi(struct pci_dev *dev);
> +int pci_set_mwi(struct pci_dev *dev);
> +int pcim_set_mwi(struct pci_dev *dev);
>  void pci_clear_mwi(struct pci_dev *dev);
>  void pci_intx(struct pci_dev *dev, int enable);
>  bool pci_check_and_mask_intx(struct pci_dev *dev);
> -- 
> 2.29.2
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] PCI: Remove pci_try_set_mwi
  2021-03-26 21:26 ` Bjorn Helgaas
@ 2021-03-26 21:42   ` Andy Shevchenko
  2021-03-26 21:55     ` Bjorn Helgaas
  2021-03-27 23:04   ` Heiner Kallweit
  1 sibling, 1 reply; 12+ messages in thread
From: Andy Shevchenko @ 2021-03-26 21:42 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Heiner Kallweit, Bjorn Helgaas, Jonathan Corbet, Jens Axboe,
	Viresh Kumar, Dan Williams, Vinod Koul, David Miller, Lee Jones,
	Ion Badulescu, Jakub Kicinski, Lino Sanfilippo,
	Christian Lamparter, Kalle Valo, Luis Chamberlain, Adam Radford,
	James E.J. Bottomley, Martin K. Petersen, James Smart,
	Dick Kennedy, Nilesh Javali, GR-QLogic-Storage-Upstream,
	Greg Kroah-Hartman, Jiri Slaby, Peter Chen, Felipe Balbi,
	linux-pci, linux-doc, Linux Kernel Mailing List, linux-ide,
	dmaengine, netdev, linux-parisc, linux-wireless,
	SCSI development list, linux-serial, Linux USB Mailing List,
	Randy Dunlap, Andrew Morton

On Fri, Mar 26, 2021 at 04:26:55PM -0500, Bjorn Helgaas wrote:
> [+cc Randy, Andrew (though I'm sure you have zero interest in this
> ancient question :))]
> 
> On Wed, Dec 09, 2020 at 09:31:21AM +0100, Heiner Kallweit wrote:
> > pci_set_mwi() and pci_try_set_mwi() do exactly the same, just that the
> > former one is declared as __must_check. However also some callers of
> > pci_set_mwi() have a comment that it's an optional feature. I don't
> > think there's much sense in this separation and the use of
> > __must_check. Therefore remove pci_try_set_mwi() and remove the
> > __must_check attribute from pci_set_mwi().
> > I don't expect either function to be used in new code anyway.
> 
> There's not much I like better than removing things.  But some
> significant thought went into adding pci_try_set_mwi() in the first
> place, so I need a little more convincing about why it's safe to
> remove it.
> 
> The argument should cite the discussion about adding it.  I think one
> of the earliest conversations is here:
> https://lore.kernel.org/linux-ide/20070404213704.224128ec.randy.dunlap@oracle.com/

It's solely PCI feature which is absent on PCIe.

So, if there is a guarantee that the driver never services a device connected
to old PCI bus, it's okay to remove the call (it's no-op on PCIe anyway).

OTOH, PCI core may try MWI itself for every device (but this is an opposite,
what should we do on broken devices that do change their state based on that
bit while violating specification).

In any case

Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

for DesignWare DMA case. I have added that and I never saw that IP connected
to the old PCI.

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] PCI: Remove pci_try_set_mwi
  2021-03-26 21:42   ` Andy Shevchenko
@ 2021-03-26 21:55     ` Bjorn Helgaas
  0 siblings, 0 replies; 12+ messages in thread
From: Bjorn Helgaas @ 2021-03-26 21:55 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Heiner Kallweit, Bjorn Helgaas, Jonathan Corbet, Jens Axboe,
	Viresh Kumar, Dan Williams, Vinod Koul, David Miller, Lee Jones,
	Ion Badulescu, Jakub Kicinski, Lino Sanfilippo,
	Christian Lamparter, Kalle Valo, Luis Chamberlain, Adam Radford,
	James E.J. Bottomley, Martin K. Petersen, James Smart,
	Dick Kennedy, Nilesh Javali, GR-QLogic-Storage-Upstream,
	Greg Kroah-Hartman, Jiri Slaby, Peter Chen, Felipe Balbi,
	linux-pci, linux-doc, Linux Kernel Mailing List, linux-ide,
	dmaengine, netdev, linux-parisc, linux-wireless,
	SCSI development list, linux-serial, Linux USB Mailing List,
	Randy Dunlap, Andrew Morton

On Fri, Mar 26, 2021 at 11:42:46PM +0200, Andy Shevchenko wrote:
> On Fri, Mar 26, 2021 at 04:26:55PM -0500, Bjorn Helgaas wrote:
> > [+cc Randy, Andrew (though I'm sure you have zero interest in this
> > ancient question :))]
> > 
> > On Wed, Dec 09, 2020 at 09:31:21AM +0100, Heiner Kallweit wrote:
> > > pci_set_mwi() and pci_try_set_mwi() do exactly the same, just that the
> > > former one is declared as __must_check. However also some callers of
> > > pci_set_mwi() have a comment that it's an optional feature. I don't
> > > think there's much sense in this separation and the use of
> > > __must_check. Therefore remove pci_try_set_mwi() and remove the
> > > __must_check attribute from pci_set_mwi().
> > > I don't expect either function to be used in new code anyway.
> > 
> > There's not much I like better than removing things.  But some
> > significant thought went into adding pci_try_set_mwi() in the first
> > place, so I need a little more convincing about why it's safe to
> > remove it.
> > 
> > The argument should cite the discussion about adding it.  I think one
> > of the earliest conversations is here:
> > https://lore.kernel.org/linux-ide/20070404213704.224128ec.randy.dunlap@oracle.com/
> 
> It's solely PCI feature which is absent on PCIe.
>
> So, if there is a guarantee that the driver never services a device connected
> to old PCI bus, it's okay to remove the call (it's no-op on PCIe anyway).

Yes, I'm aware that MWI is a no-op on PCIe.  If we want to argue that
we don't need to support Conventional PCI devices, that should be
explicit, and we could remove pci_set_mwi() completely.  But I don't
think we're ready to drop Conventional PCI support.

> OTOH, PCI core may try MWI itself for every device (but this is an opposite,
> what should we do on broken devices that do change their state based on that
> bit while violating specification).
> 
> In any case
> 
> Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Thanks!

Bjorn

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] PCI: Remove pci_try_set_mwi
  2021-03-26 21:26 ` Bjorn Helgaas
  2021-03-26 21:42   ` Andy Shevchenko
@ 2021-03-27 23:04   ` Heiner Kallweit
  2021-03-28 19:58     ` Bjorn Helgaas
  1 sibling, 1 reply; 12+ messages in thread
From: Heiner Kallweit @ 2021-03-27 23:04 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Bjorn Helgaas, Jonathan Corbet, Jens Axboe, Viresh Kumar,
	Andy Shevchenko, Dan Williams, Vinod Koul, David Miller,
	Lee Jones, Ion Badulescu, Jakub Kicinski, Lino Sanfilippo,
	Christian Lamparter, Kalle Valo, Luis Chamberlain, Adam Radford,
	James E.J. Bottomley, Martin K. Petersen, James Smart,
	Dick Kennedy, Nilesh Javali, GR-QLogic-Storage-Upstream,
	Greg Kroah-Hartman, Jiri Slaby, Peter Chen, Felipe Balbi,
	linux-pci, linux-doc, Linux Kernel Mailing List, linux-ide,
	dmaengine, netdev, linux-parisc, linux-wireless,
	SCSI development list, linux-serial, Linux USB Mailing List,
	Randy Dunlap, Andrew Morton

On 26.03.2021 22:26, Bjorn Helgaas wrote:
> [+cc Randy, Andrew (though I'm sure you have zero interest in this
> ancient question :))]
> 
> On Wed, Dec 09, 2020 at 09:31:21AM +0100, Heiner Kallweit wrote:
>> pci_set_mwi() and pci_try_set_mwi() do exactly the same, just that the
>> former one is declared as __must_check. However also some callers of
>> pci_set_mwi() have a comment that it's an optional feature. I don't
>> think there's much sense in this separation and the use of
>> __must_check. Therefore remove pci_try_set_mwi() and remove the
>> __must_check attribute from pci_set_mwi().
>> I don't expect either function to be used in new code anyway.
> 
> There's not much I like better than removing things.  But some
> significant thought went into adding pci_try_set_mwi() in the first
> place, so I need a little more convincing about why it's safe to
> remove it.
> 

Thanks for the link to the 13 yrs old discussion. Unfortunately it
doesn't mention any real argument for the __must_check, just:

"And one of the reasons for adding the __must_check annotation is to
weed out design errors."
And the very next response in the discussion calls this a "non-argument".
Plus not mentioning what the other reasons could be.

Currently we have three ancient drivers that bail out if the call fails.
Most callers of pci_set_mwi() use the return code only to emit an
error message, but they proceed normally. Majority of users calls
pci_try_set_mwi(). And as stated in the commit message I don't expect
any new usage of pci_set_mwi().

> The argument should cite the discussion about adding it.  I think one
> of the earliest conversations is here:
> https://lore.kernel.org/linux-ide/20070404213704.224128ec.randy.dunlap@oracle.com/
> 
>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>> ---
>> patch applies on top of pci/misc for v5.11
>> ---
>>  Documentation/PCI/pci.rst                     |  5 +----
>>  drivers/ata/pata_cs5530.c                     |  2 +-
>>  drivers/ata/sata_mv.c                         |  2 +-
>>  drivers/dma/dw/pci.c                          |  2 +-
>>  drivers/dma/hsu/pci.c                         |  2 +-
>>  drivers/ide/cs5530.c                          |  2 +-
>>  drivers/mfd/intel-lpss-pci.c                  |  2 +-
>>  drivers/net/ethernet/adaptec/starfire.c       |  2 +-
>>  drivers/net/ethernet/alacritech/slicoss.c     |  2 +-
>>  drivers/net/ethernet/dec/tulip/tulip_core.c   |  5 +----
>>  drivers/net/ethernet/sun/cassini.c            |  4 ++--
>>  drivers/net/wireless/intersil/p54/p54pci.c    |  2 +-
>>  .../intersil/prism54/islpci_hotplug.c         |  3 +--
>>  .../wireless/realtek/rtl818x/rtl8180/dev.c    |  2 +-
>>  drivers/pci/pci.c                             | 19 -------------------
>>  drivers/scsi/3w-9xxx.c                        |  4 ++--
>>  drivers/scsi/3w-sas.c                         |  4 ++--
>>  drivers/scsi/csiostor/csio_init.c             |  2 +-
>>  drivers/scsi/lpfc/lpfc_init.c                 |  2 +-
>>  drivers/scsi/qla2xxx/qla_init.c               |  8 ++++----
>>  drivers/scsi/qla2xxx/qla_mr.c                 |  2 +-
>>  drivers/tty/serial/8250/8250_lpss.c           |  2 +-
>>  drivers/usb/chipidea/ci_hdrc_pci.c            |  2 +-
>>  drivers/usb/gadget/udc/amd5536udc_pci.c       |  2 +-
>>  drivers/usb/gadget/udc/net2280.c              |  2 +-
>>  drivers/usb/gadget/udc/pch_udc.c              |  2 +-
>>  include/linux/pci.h                           |  5 ++---
>>  27 files changed, 33 insertions(+), 60 deletions(-)
>>
>> diff --git a/Documentation/PCI/pci.rst b/Documentation/PCI/pci.rst
>> index 814b40f83..120362cc9 100644
>> --- a/Documentation/PCI/pci.rst
>> +++ b/Documentation/PCI/pci.rst
>> @@ -226,10 +226,7 @@ If the PCI device can use the PCI Memory-Write-Invalidate transaction,
>>  call pci_set_mwi().  This enables the PCI_COMMAND bit for Mem-Wr-Inval
>>  and also ensures that the cache line size register is set correctly.
>>  Check the return value of pci_set_mwi() as not all architectures
>> -or chip-sets may support Memory-Write-Invalidate.  Alternatively,
>> -if Mem-Wr-Inval would be nice to have but is not required, call
>> -pci_try_set_mwi() to have the system do its best effort at enabling
>> -Mem-Wr-Inval.
>> +or chip-sets may support Memory-Write-Invalidate.
>>  
>>  
>>  Request MMIO/IOP resources
>> diff --git a/drivers/ata/pata_cs5530.c b/drivers/ata/pata_cs5530.c
>> index ad75d02b6..8654b3ae1 100644
>> --- a/drivers/ata/pata_cs5530.c
>> +++ b/drivers/ata/pata_cs5530.c
>> @@ -214,7 +214,7 @@ static int cs5530_init_chip(void)
>>  	}
>>  
>>  	pci_set_master(cs5530_0);
>> -	pci_try_set_mwi(cs5530_0);
>> +	pci_set_mwi(cs5530_0);
>>  
>>  	/*
>>  	 * Set PCI CacheLineSize to 16-bytes:
>> diff --git a/drivers/ata/sata_mv.c b/drivers/ata/sata_mv.c
>> index 664ef658a..ee37755ea 100644
>> --- a/drivers/ata/sata_mv.c
>> +++ b/drivers/ata/sata_mv.c
>> @@ -4432,7 +4432,7 @@ static int mv_pci_init_one(struct pci_dev *pdev,
>>  	mv_print_info(host);
>>  
>>  	pci_set_master(pdev);
>> -	pci_try_set_mwi(pdev);
>> +	pci_set_mwi(pdev);
>>  	return ata_host_activate(host, pdev->irq, mv_interrupt, IRQF_SHARED,
>>  				 IS_GEN_I(hpriv) ? &mv5_sht : &mv6_sht);
>>  }
>> diff --git a/drivers/dma/dw/pci.c b/drivers/dma/dw/pci.c
>> index 1142aa6f8..1c20b7485 100644
>> --- a/drivers/dma/dw/pci.c
>> +++ b/drivers/dma/dw/pci.c
>> @@ -30,7 +30,7 @@ static int dw_pci_probe(struct pci_dev *pdev, const struct pci_device_id *pid)
>>  	}
>>  
>>  	pci_set_master(pdev);
>> -	pci_try_set_mwi(pdev);
>> +	pci_set_mwi(pdev);
>>  
>>  	ret = pci_set_dma_mask(pdev, DMA_BIT_MASK(32));
>>  	if (ret)
>> diff --git a/drivers/dma/hsu/pci.c b/drivers/dma/hsu/pci.c
>> index 07cc7320a..420dd3706 100644
>> --- a/drivers/dma/hsu/pci.c
>> +++ b/drivers/dma/hsu/pci.c
>> @@ -73,7 +73,7 @@ static int hsu_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>>  	}
>>  
>>  	pci_set_master(pdev);
>> -	pci_try_set_mwi(pdev);
>> +	pci_set_mwi(pdev);
>>  
>>  	ret = pci_set_dma_mask(pdev, DMA_BIT_MASK(32));
>>  	if (ret)
>> diff --git a/drivers/ide/cs5530.c b/drivers/ide/cs5530.c
>> index 5bb46e713..5d2c421ab 100644
>> --- a/drivers/ide/cs5530.c
>> +++ b/drivers/ide/cs5530.c
>> @@ -168,7 +168,7 @@ static int init_chipset_cs5530(struct pci_dev *dev)
>>  	 */
>>  
>>  	pci_set_master(cs5530_0);
>> -	pci_try_set_mwi(cs5530_0);
>> +	pci_set_mwi(cs5530_0);
>>  
>>  	/*
>>  	 * Set PCI CacheLineSize to 16-bytes:
>> diff --git a/drivers/mfd/intel-lpss-pci.c b/drivers/mfd/intel-lpss-pci.c
>> index 2d7c588ef..a0c3be750 100644
>> --- a/drivers/mfd/intel-lpss-pci.c
>> +++ b/drivers/mfd/intel-lpss-pci.c
>> @@ -39,7 +39,7 @@ static int intel_lpss_pci_probe(struct pci_dev *pdev,
>>  
>>  	/* Probably it is enough to set this for iDMA capable devices only */
>>  	pci_set_master(pdev);
>> -	pci_try_set_mwi(pdev);
>> +	pci_set_mwi(pdev);
>>  
>>  	ret = intel_lpss_probe(&pdev->dev, info);
>>  	if (ret)
>> diff --git a/drivers/net/ethernet/adaptec/starfire.c b/drivers/net/ethernet/adaptec/starfire.c
>> index 555299737..1dbff34c4 100644
>> --- a/drivers/net/ethernet/adaptec/starfire.c
>> +++ b/drivers/net/ethernet/adaptec/starfire.c
>> @@ -679,7 +679,7 @@ static int starfire_init_one(struct pci_dev *pdev,
>>  	pci_set_master(pdev);
>>  
>>  	/* enable MWI -- it vastly improves Rx performance on sparc64 */
>> -	pci_try_set_mwi(pdev);
>> +	pci_set_mwi(pdev);
>>  
>>  #ifdef ZEROCOPY
>>  	/* Starfire can do TCP/UDP checksumming */
>> diff --git a/drivers/net/ethernet/alacritech/slicoss.c b/drivers/net/ethernet/alacritech/slicoss.c
>> index 696517eae..544510f57 100644
>> --- a/drivers/net/ethernet/alacritech/slicoss.c
>> +++ b/drivers/net/ethernet/alacritech/slicoss.c
>> @@ -1749,7 +1749,7 @@ static int slic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>>  	}
>>  
>>  	pci_set_master(pdev);
>> -	pci_try_set_mwi(pdev);
>> +	pci_set_mwi(pdev);
>>  
>>  	slic_configure_pci(pdev);
>>  
>> diff --git a/drivers/net/ethernet/dec/tulip/tulip_core.c b/drivers/net/ethernet/dec/tulip/tulip_core.c
>> index c1dcd6ca1..4f6debb24 100644
>> --- a/drivers/net/ethernet/dec/tulip/tulip_core.c
>> +++ b/drivers/net/ethernet/dec/tulip/tulip_core.c
>> @@ -1193,10 +1193,7 @@ static void tulip_mwi_config(struct pci_dev *pdev, struct net_device *dev)
>>  	/* if we have any cache line size at all, we can do MRM and MWI */
>>  	csr0 |= MRM | MWI;
>>  
>> -	/* Enable MWI in the standard PCI command bit.
>> -	 * Check for the case where MWI is desired but not available
>> -	 */
>> -	pci_try_set_mwi(pdev);
>> +	pci_set_mwi(pdev);
>>  
>>  	/* read result from hardware (in case bit refused to enable) */
>>  	pci_read_config_word(pdev, PCI_COMMAND, &pci_command);
>> diff --git a/drivers/net/ethernet/sun/cassini.c b/drivers/net/ethernet/sun/cassini.c
>> index 9ff894ba8..9a95ec989 100644
>> --- a/drivers/net/ethernet/sun/cassini.c
>> +++ b/drivers/net/ethernet/sun/cassini.c
>> @@ -4933,14 +4933,14 @@ static int cas_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>>  	pci_cmd &= ~PCI_COMMAND_SERR;
>>  	pci_cmd |= PCI_COMMAND_PARITY;
>>  	pci_write_config_word(pdev, PCI_COMMAND, pci_cmd);
>> -	if (pci_try_set_mwi(pdev))
>> +	if (pci_set_mwi(pdev))
>>  		pr_warn("Could not enable MWI for %s\n", pci_name(pdev));
>>  
>>  	cas_program_bridge(pdev);
>>  
>>  	/*
>>  	 * On some architectures, the default cache line size set
>> -	 * by pci_try_set_mwi reduces perforamnce.  We have to increase
>> +	 * by pci_set_mwi reduces performance.  We have to increase
>>  	 * it for this case.  To start, we'll print some configuration
>>  	 * data.
>>  	 */
>> diff --git a/drivers/net/wireless/intersil/p54/p54pci.c b/drivers/net/wireless/intersil/p54/p54pci.c
>> index e97ee547b..c76326d1e 100644
>> --- a/drivers/net/wireless/intersil/p54/p54pci.c
>> +++ b/drivers/net/wireless/intersil/p54/p54pci.c
>> @@ -583,7 +583,7 @@ static int p54p_probe(struct pci_dev *pdev,
>>  	}
>>  
>>  	pci_set_master(pdev);
>> -	pci_try_set_mwi(pdev);
>> +	pci_set_mwi(pdev);
>>  
>>  	pci_write_config_byte(pdev, 0x40, 0);
>>  	pci_write_config_byte(pdev, 0x41, 0);
>> diff --git a/drivers/net/wireless/intersil/prism54/islpci_hotplug.c b/drivers/net/wireless/intersil/prism54/islpci_hotplug.c
>> index 31a1e6132..e8087d9a5 100644
>> --- a/drivers/net/wireless/intersil/prism54/islpci_hotplug.c
>> +++ b/drivers/net/wireless/intersil/prism54/islpci_hotplug.c
>> @@ -153,8 +153,7 @@ prism54_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>>  	DEBUG(SHOW_TRACING, "%s: pci_set_master(pdev)\n", DRV_NAME);
>>  	pci_set_master(pdev);
>>  
>> -	/* enable MWI */
>> -	pci_try_set_mwi(pdev);
>> +	pci_set_mwi(pdev);
>>  
>>  	/* setup the network device interface and its structure */
>>  	if (!(ndev = islpci_setup(pdev))) {
>> diff --git a/drivers/net/wireless/realtek/rtl818x/rtl8180/dev.c b/drivers/net/wireless/realtek/rtl818x/rtl8180/dev.c
>> index 2477e18c7..b259b0b58 100644
>> --- a/drivers/net/wireless/realtek/rtl818x/rtl8180/dev.c
>> +++ b/drivers/net/wireless/realtek/rtl818x/rtl8180/dev.c
>> @@ -1871,7 +1871,7 @@ static int rtl8180_probe(struct pci_dev *pdev,
>>  
>>  	if (priv->chip_family != RTL818X_CHIP_FAMILY_RTL8180) {
>>  		priv->band.n_bitrates = ARRAY_SIZE(rtl818x_rates);
>> -		pci_try_set_mwi(pdev);
>> +		pci_set_mwi(pdev);
>>  	}
>>  
>>  	if (priv->chip_family != RTL818X_CHIP_FAMILY_RTL8180)
>> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
>> index 9a5500287..f0ab432d2 100644
>> --- a/drivers/pci/pci.c
>> +++ b/drivers/pci/pci.c
>> @@ -4389,25 +4389,6 @@ int pcim_set_mwi(struct pci_dev *dev)
>>  }
>>  EXPORT_SYMBOL(pcim_set_mwi);
>>  
>> -/**
>> - * pci_try_set_mwi - enables memory-write-invalidate PCI transaction
>> - * @dev: the PCI device for which MWI is enabled
>> - *
>> - * Enables the Memory-Write-Invalidate transaction in %PCI_COMMAND.
>> - * Callers are not required to check the return value.
>> - *
>> - * RETURNS: An appropriate -ERRNO error value on error, or zero for success.
>> - */
>> -int pci_try_set_mwi(struct pci_dev *dev)
>> -{
>> -#ifdef PCI_DISABLE_MWI
>> -	return 0;
>> -#else
>> -	return pci_set_mwi(dev);
>> -#endif
>> -}
>> -EXPORT_SYMBOL(pci_try_set_mwi);
>> -
>>  /**
>>   * pci_clear_mwi - disables Memory-Write-Invalidate for device dev
>>   * @dev: the PCI device to disable
>> diff --git a/drivers/scsi/3w-9xxx.c b/drivers/scsi/3w-9xxx.c
>> index b4718a1b2..d869485e2 100644
>> --- a/drivers/scsi/3w-9xxx.c
>> +++ b/drivers/scsi/3w-9xxx.c
>> @@ -2018,7 +2018,7 @@ static int twa_probe(struct pci_dev *pdev, const struct pci_device_id *dev_id)
>>  	}
>>  
>>  	pci_set_master(pdev);
>> -	pci_try_set_mwi(pdev);
>> +	pci_set_mwi(pdev);
>>  
>>  	retval = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
>>  	if (retval)
>> @@ -2227,7 +2227,7 @@ static int __maybe_unused twa_resume(struct device *dev)
>>  
>>  	printk(KERN_WARNING "3w-9xxx: Resuming host %d.\n", tw_dev->host->host_no);
>>  
>> -	pci_try_set_mwi(pdev);
>> +	pci_set_mwi(pdev);
>>  
>>  	retval = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
>>  	if (retval)
>> diff --git a/drivers/scsi/3w-sas.c b/drivers/scsi/3w-sas.c
>> index b8f1848ec..49ca153b8 100644
>> --- a/drivers/scsi/3w-sas.c
>> +++ b/drivers/scsi/3w-sas.c
>> @@ -1571,7 +1571,7 @@ static int twl_probe(struct pci_dev *pdev, const struct pci_device_id *dev_id)
>>  	}
>>  
>>  	pci_set_master(pdev);
>> -	pci_try_set_mwi(pdev);
>> +	pci_set_mwi(pdev);
>>  
>>  	retval = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
>>  	if (retval)
>> @@ -1790,7 +1790,7 @@ static int __maybe_unused twl_resume(struct device *dev)
>>  	TW_Device_Extension *tw_dev = (TW_Device_Extension *)host->hostdata;
>>  
>>  	printk(KERN_WARNING "3w-sas: Resuming host %d.\n", tw_dev->host->host_no);
>> -	pci_try_set_mwi(pdev);
>> +	pci_set_mwi(pdev);
>>  
>>  	retval = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
>>  	if (retval)
>> diff --git a/drivers/scsi/csiostor/csio_init.c b/drivers/scsi/csiostor/csio_init.c
>> index 390b07bf9..c20bf44c6 100644
>> --- a/drivers/scsi/csiostor/csio_init.c
>> +++ b/drivers/scsi/csiostor/csio_init.c
>> @@ -201,7 +201,7 @@ csio_pci_init(struct pci_dev *pdev, int *bars)
>>  		goto err_disable_device;
>>  
>>  	pci_set_master(pdev);
>> -	pci_try_set_mwi(pdev);
>> +	pci_set_mwi(pdev);
>>  
>>  	rv = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
>>  	if (rv)
>> diff --git a/drivers/scsi/lpfc/lpfc_init.c b/drivers/scsi/lpfc/lpfc_init.c
>> index ac67f420e..b4833c0f8 100644
>> --- a/drivers/scsi/lpfc/lpfc_init.c
>> +++ b/drivers/scsi/lpfc/lpfc_init.c
>> @@ -6119,7 +6119,7 @@ lpfc_enable_pci_dev(struct lpfc_hba *phba)
>>  		goto out_disable_device;
>>  	/* Set up device as PCI master and save state for EEH */
>>  	pci_set_master(pdev);
>> -	pci_try_set_mwi(pdev);
>> +	pci_set_mwi(pdev);
>>  	pci_save_state(pdev);
>>  
>>  	/* PCIe EEH recovery on powerpc platforms needs fundamental reset */
>> diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c
>> index 5626e9b69..76019dc2e 100644
>> --- a/drivers/scsi/qla2xxx/qla_init.c
>> +++ b/drivers/scsi/qla2xxx/qla_init.c
>> @@ -2356,7 +2356,7 @@ qla2100_pci_config(scsi_qla_host_t *vha)
>>  	struct device_reg_2xxx __iomem *reg = &ha->iobase->isp;
>>  
>>  	pci_set_master(ha->pdev);
>> -	pci_try_set_mwi(ha->pdev);
>> +	pci_set_mwi(ha->pdev);
>>  
>>  	pci_read_config_word(ha->pdev, PCI_COMMAND, &w);
>>  	w |= (PCI_COMMAND_PARITY | PCI_COMMAND_SERR);
>> @@ -2388,7 +2388,7 @@ qla2300_pci_config(scsi_qla_host_t *vha)
>>  	struct device_reg_2xxx __iomem *reg = &ha->iobase->isp;
>>  
>>  	pci_set_master(ha->pdev);
>> -	pci_try_set_mwi(ha->pdev);
>> +	pci_set_mwi(ha->pdev);
>>  
>>  	pci_read_config_word(ha->pdev, PCI_COMMAND, &w);
>>  	w |= (PCI_COMMAND_PARITY | PCI_COMMAND_SERR);
>> @@ -2469,7 +2469,7 @@ qla24xx_pci_config(scsi_qla_host_t *vha)
>>  	struct device_reg_24xx __iomem *reg = &ha->iobase->isp24;
>>  
>>  	pci_set_master(ha->pdev);
>> -	pci_try_set_mwi(ha->pdev);
>> +	pci_set_mwi(ha->pdev);
>>  
>>  	pci_read_config_word(ha->pdev, PCI_COMMAND, &w);
>>  	w |= (PCI_COMMAND_PARITY | PCI_COMMAND_SERR);
>> @@ -2511,7 +2511,7 @@ qla25xx_pci_config(scsi_qla_host_t *vha)
>>  	struct qla_hw_data *ha = vha->hw;
>>  
>>  	pci_set_master(ha->pdev);
>> -	pci_try_set_mwi(ha->pdev);
>> +	pci_set_mwi(ha->pdev);
>>  
>>  	pci_read_config_word(ha->pdev, PCI_COMMAND, &w);
>>  	w |= (PCI_COMMAND_PARITY | PCI_COMMAND_SERR);
>> diff --git a/drivers/scsi/qla2xxx/qla_mr.c b/drivers/scsi/qla2xxx/qla_mr.c
>> index ca7306685..e9763d8e3 100644
>> --- a/drivers/scsi/qla2xxx/qla_mr.c
>> +++ b/drivers/scsi/qla2xxx/qla_mr.c
>> @@ -499,7 +499,7 @@ qlafx00_pci_config(scsi_qla_host_t *vha)
>>  	struct qla_hw_data *ha = vha->hw;
>>  
>>  	pci_set_master(ha->pdev);
>> -	pci_try_set_mwi(ha->pdev);
>> +	pci_set_mwi(ha->pdev);
>>  
>>  	pci_read_config_word(ha->pdev, PCI_COMMAND, &w);
>>  	w |= (PCI_COMMAND_PARITY | PCI_COMMAND_SERR);
>> diff --git a/drivers/tty/serial/8250/8250_lpss.c b/drivers/tty/serial/8250/8250_lpss.c
>> index 4dee8a9e0..8acc1e5c9 100644
>> --- a/drivers/tty/serial/8250/8250_lpss.c
>> +++ b/drivers/tty/serial/8250/8250_lpss.c
>> @@ -193,7 +193,7 @@ static void qrk_serial_setup_dma(struct lpss8250 *lpss, struct uart_port *port)
>>  	if (ret)
>>  		return;
>>  
>> -	pci_try_set_mwi(pdev);
>> +	pci_set_mwi(pdev);
>>  
>>  	/* Special DMA address for UART */
>>  	dma->rx_dma_addr = 0xfffff000;
>> diff --git a/drivers/usb/chipidea/ci_hdrc_pci.c b/drivers/usb/chipidea/ci_hdrc_pci.c
>> index d63479e1a..d412fa910 100644
>> --- a/drivers/usb/chipidea/ci_hdrc_pci.c
>> +++ b/drivers/usb/chipidea/ci_hdrc_pci.c
>> @@ -78,7 +78,7 @@ static int ci_hdrc_pci_probe(struct pci_dev *pdev,
>>  	}
>>  
>>  	pci_set_master(pdev);
>> -	pci_try_set_mwi(pdev);
>> +	pci_set_mwi(pdev);
>>  
>>  	/* register a nop PHY */
>>  	ci->phy = usb_phy_generic_register();
>> diff --git a/drivers/usb/gadget/udc/amd5536udc_pci.c b/drivers/usb/gadget/udc/amd5536udc_pci.c
>> index 8d387e0e4..9630ce8d3 100644
>> --- a/drivers/usb/gadget/udc/amd5536udc_pci.c
>> +++ b/drivers/usb/gadget/udc/amd5536udc_pci.c
>> @@ -151,7 +151,7 @@ static int udc_pci_probe(
>>  	dev->chiprev = pdev->revision;
>>  
>>  	pci_set_master(pdev);
>> -	pci_try_set_mwi(pdev);
>> +	pci_set_mwi(pdev);
>>  
>>  	/* init dma pools */
>>  	if (use_dma) {
>> diff --git a/drivers/usb/gadget/udc/net2280.c b/drivers/usb/gadget/udc/net2280.c
>> index fc9f99fe7..e15520698 100644
>> --- a/drivers/usb/gadget/udc/net2280.c
>> +++ b/drivers/usb/gadget/udc/net2280.c
>> @@ -3761,7 +3761,7 @@ static int net2280_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>>  			&dev->pci->pcimstctl);
>>  	/* erratum 0115 shouldn't appear: Linux inits PCI_LATENCY_TIMER */
>>  	pci_set_master(pdev);
>> -	pci_try_set_mwi(pdev);
>> +	pci_set_mwi(pdev);
>>  
>>  	/* ... also flushes any posted pci writes */
>>  	dev->chiprev = get_idx_reg(dev->regs, REG_CHIPREV) & 0xffff;
>> diff --git a/drivers/usb/gadget/udc/pch_udc.c b/drivers/usb/gadget/udc/pch_udc.c
>> index a3c1fc924..4a0484a0c 100644
>> --- a/drivers/usb/gadget/udc/pch_udc.c
>> +++ b/drivers/usb/gadget/udc/pch_udc.c
>> @@ -3100,7 +3100,7 @@ static int pch_udc_probe(struct pci_dev *pdev,
>>  	}
>>  
>>  	pci_set_master(pdev);
>> -	pci_try_set_mwi(pdev);
>> +	pci_set_mwi(pdev);
>>  
>>  	/* device struct setup */
>>  	spin_lock_init(&dev->lock);
>> diff --git a/include/linux/pci.h b/include/linux/pci.h
>> index de75f6a4d..c590f616d 100644
>> --- a/include/linux/pci.h
>> +++ b/include/linux/pci.h
>> @@ -1191,9 +1191,8 @@ void pci_clear_master(struct pci_dev *dev);
>>  
>>  int pci_set_pcie_reset_state(struct pci_dev *dev, enum pcie_reset_state state);
>>  int pci_set_cacheline_size(struct pci_dev *dev);
>> -int __must_check pci_set_mwi(struct pci_dev *dev);
>> -int __must_check pcim_set_mwi(struct pci_dev *dev);
>> -int pci_try_set_mwi(struct pci_dev *dev);
>> +int pci_set_mwi(struct pci_dev *dev);
>> +int pcim_set_mwi(struct pci_dev *dev);
>>  void pci_clear_mwi(struct pci_dev *dev);
>>  void pci_intx(struct pci_dev *dev, int enable);
>>  bool pci_check_and_mask_intx(struct pci_dev *dev);
>> -- 
>> 2.29.2
>>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] PCI: Remove pci_try_set_mwi
  2021-03-27 23:04   ` Heiner Kallweit
@ 2021-03-28 19:58     ` Bjorn Helgaas
  0 siblings, 0 replies; 12+ messages in thread
From: Bjorn Helgaas @ 2021-03-28 19:58 UTC (permalink / raw)
  To: Heiner Kallweit
  Cc: Bjorn Helgaas, Jonathan Corbet, Jens Axboe, Viresh Kumar,
	Andy Shevchenko, Dan Williams, Vinod Koul, David Miller,
	Lee Jones, Ion Badulescu, Jakub Kicinski, Lino Sanfilippo,
	Christian Lamparter, Kalle Valo, Luis Chamberlain, Adam Radford,
	James E.J. Bottomley, Martin K. Petersen, James Smart,
	Dick Kennedy, Nilesh Javali, GR-QLogic-Storage-Upstream,
	Greg Kroah-Hartman, Jiri Slaby, Peter Chen, Felipe Balbi,
	linux-pci, linux-doc, Linux Kernel Mailing List, linux-ide,
	dmaengine, netdev, linux-parisc, linux-wireless,
	SCSI development list, linux-serial, Linux USB Mailing List,
	Randy Dunlap, Andrew Morton

On Sun, Mar 28, 2021 at 12:04:35AM +0100, Heiner Kallweit wrote:
> On 26.03.2021 22:26, Bjorn Helgaas wrote:
> > [+cc Randy, Andrew (though I'm sure you have zero interest in this
> > ancient question :))]
> > 
> > On Wed, Dec 09, 2020 at 09:31:21AM +0100, Heiner Kallweit wrote:
> >> pci_set_mwi() and pci_try_set_mwi() do exactly the same, just that the
> >> former one is declared as __must_check. However also some callers of
> >> pci_set_mwi() have a comment that it's an optional feature. I don't
> >> think there's much sense in this separation and the use of
> >> __must_check. Therefore remove pci_try_set_mwi() and remove the
> >> __must_check attribute from pci_set_mwi().
> >> I don't expect either function to be used in new code anyway.
> > 
> > There's not much I like better than removing things.  But some
> > significant thought went into adding pci_try_set_mwi() in the first
> > place, so I need a little more convincing about why it's safe to
> > remove it.
> > 
> 
> Thanks for the link to the 13 yrs old discussion. Unfortunately it
> doesn't mention any real argument for the __must_check, just:
> 
> "And one of the reasons for adding the __must_check annotation is to
> weed out design errors."
> And the very next response in the discussion calls this a "non-argument".
> Plus not mentioning what the other reasons could be.

I think you're referring to Alan's response [1]:

  akpm> And we *need* to be excessively anal in the PCI setup code.
  akpm> We have metric shitloads of bugs due to problems in that area,
  akpm> and the more formality and error handling and error reporting
  akpm> we can get in there the better off we will be.

  ac> No argument there

So Alan is actually *agreeing* that "we need to be excessively anal in
the PCI setup code,"  not saying that "weeding out design errors is
not an argument for __must_check."

> Currently we have three ancient drivers that bail out if the call fails.
> Most callers of pci_set_mwi() use the return code only to emit an
> error message, but they proceed normally. Majority of users calls
> pci_try_set_mwi(). And as stated in the commit message I don't expect
> any new usage of pci_set_mwi().

I would love to merge this patch.  We just need to clarify the commit
log.  Right now the only justification is "I don't think there's much
sense in the __must_check annotation," which may well be true but
could use some support.

If MWI is purely an optimization and there's never a functional
problem if pci_set_mwi() fails, we should say that (and maybe
update any drivers that bail out on failure).

Andrew and Alan both seem to agree that MSI *is* purely advisory:

  akpm> pci_set_mwi() is an advisory thing, and on certain platforms
  akpm> it might fail to set the cacheline size to the desired number.
  akpm> This is not a fatal error and the driver can successfully run
  akpm> at a lesser performance level.

  ac> Correct.

But even after that, Andrew proposed adding pci_try_set_mwi().  So it
makes sense to really understand what was going on there so we don't
break something in the name of cleaning it up.

[1] https://lore.kernel.org/linux-ide/20070405211609.5263d627@the-village.bc.nu/

> > The argument should cite the discussion about adding it.  I think one
> > of the earliest conversations is here:
> > https://lore.kernel.org/linux-ide/20070404213704.224128ec.randy.dunlap@oracle.com/

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2021-03-28 19:59 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-09  8:31 [PATCH] PCI: Remove pci_try_set_mwi Heiner Kallweit
2020-12-09  8:40 ` Kalle Valo
2020-12-09  9:03 ` Vinod Koul
2020-12-09  9:20 ` Peter Chen
2020-12-09  9:22 ` Lee Jones
2020-12-09 10:59 ` Andy Shevchenko
2020-12-09 11:02   ` Andy Shevchenko
2021-03-26 21:26 ` Bjorn Helgaas
2021-03-26 21:42   ` Andy Shevchenko
2021-03-26 21:55     ` Bjorn Helgaas
2021-03-27 23:04   ` Heiner Kallweit
2021-03-28 19:58     ` Bjorn Helgaas

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).