Message-ID: <20240129230334.GA480827@bhelgaas>
Date: Mon, 29 Jan 2024 17:03:34 -0600
From: Bjorn Helgaas <helgaas@...nel.org>
To: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
Cc: Bjorn Helgaas <bhelgaas@...gle.com>, linux-kernel@...r.kernel.org,
	linux-pci@...r.kernel.org
Subject: Re: [PATCH 1/1] PCI: Place interrupt related code into irq.c

On Mon, Jan 29, 2024 at 01:36:54PM +0200, Ilpo Järvinen wrote:
> Interrupt related code is spread into irq.c, pci.c, and setup-irq.c.
> Group them into pre-existing irq.c.
> 
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>

Applied to pci/enumeration for v6.9, thanks.

> ---
>  drivers/pci/Makefile     |   2 +-
>  drivers/pci/irq.c        | 204 +++++++++++++++++++++++++++++++++++++++
>  drivers/pci/pci-driver.c |   9 --
>  drivers/pci/pci.c        | 144 ---------------------------
>  drivers/pci/setup-irq.c  |  64 ------------
>  5 files changed, 205 insertions(+), 218 deletions(-)
>  delete mode 100644 drivers/pci/setup-irq.c
> 
> diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile
> index cc8b4e01e29d..54a7adf0bb88 100644
> --- a/drivers/pci/Makefile
> +++ b/drivers/pci/Makefile
> @@ -5,7 +5,7 @@
>  obj-$(CONFIG_PCI)		+= access.o bus.o probe.o host-bridge.o \
>  				   remove.o pci.o pci-driver.o search.o \
>  				   pci-sysfs.o rom.o setup-res.o irq.o vpd.o \
> -				   setup-bus.o vc.o mmap.o setup-irq.o
> +				   setup-bus.o vc.o mmap.o
>  
>  obj-$(CONFIG_PCI)		+= msi/
>  obj-$(CONFIG_PCI)		+= pcie/
> diff --git a/drivers/pci/irq.c b/drivers/pci/irq.c
> index 0050e8f6814e..4555630be9ec 100644
> --- a/drivers/pci/irq.c
> +++ b/drivers/pci/irq.c
> @@ -8,9 +8,13 @@
>  
>  #include <linux/device.h>
>  #include <linux/kernel.h>
> +#include <linux/errno.h>
>  #include <linux/export.h>
> +#include <linux/interrupt.h>
>  #include <linux/pci.h>
>  
> +#include "pci.h"
> +
>  /**
>   * pci_request_irq - allocate an interrupt line for a PCI device
>   * @dev:	PCI device to operate on
> @@ -74,3 +78,203 @@ void pci_free_irq(struct pci_dev *dev, unsigned int nr, void *dev_id)
>  	kfree(free_irq(pci_irq_vector(dev, nr), dev_id));
>  }
>  EXPORT_SYMBOL(pci_free_irq);
> +
> +/**
> + * pci_swizzle_interrupt_pin - swizzle INTx for device behind bridge
> + * @dev: the PCI device
> + * @pin: the INTx pin (1=INTA, 2=INTB, 3=INTC, 4=INTD)
> + *
> + * Perform INTx swizzling for a device behind one level of bridge.  This is
> + * required by section 9.1 of the PCI-to-PCI bridge specification for devices
> + * behind bridges on add-in cards.  For devices with ARI enabled, the slot
> + * number is always 0 (see the Implementation Note in section 2.2.8.1 of
> + * the PCI Express Base Specification, Revision 2.1)
> + */
> +u8 pci_swizzle_interrupt_pin(const struct pci_dev *dev, u8 pin)
> +{
> +	int slot;
> +
> +	if (pci_ari_enabled(dev->bus))
> +		slot = 0;
> +	else
> +		slot = PCI_SLOT(dev->devfn);
> +
> +	return (((pin - 1) + slot) % 4) + 1;
> +}
> +
> +int pci_get_interrupt_pin(struct pci_dev *dev, struct pci_dev **bridge)
> +{
> +	u8 pin;
> +
> +	pin = dev->pin;
> +	if (!pin)
> +		return -1;
> +
> +	while (!pci_is_root_bus(dev->bus)) {
> +		pin = pci_swizzle_interrupt_pin(dev, pin);
> +		dev = dev->bus->self;
> +	}
> +	*bridge = dev;
> +	return pin;
> +}
> +
> +/**
> + * pci_common_swizzle - swizzle INTx all the way to root bridge
> + * @dev: the PCI device
> + * @pinp: pointer to the INTx pin value (1=INTA, 2=INTB, 3=INTD, 4=INTD)
> + *
> + * Perform INTx swizzling for a device.  This traverses through all PCI-to-PCI
> + * bridges all the way up to a PCI root bus.
> + */
> +u8 pci_common_swizzle(struct pci_dev *dev, u8 *pinp)
> +{
> +	u8 pin = *pinp;
> +
> +	while (!pci_is_root_bus(dev->bus)) {
> +		pin = pci_swizzle_interrupt_pin(dev, pin);
> +		dev = dev->bus->self;
> +	}
> +	*pinp = pin;
> +	return PCI_SLOT(dev->devfn);
> +}
> +EXPORT_SYMBOL_GPL(pci_common_swizzle);
> +
> +void pci_assign_irq(struct pci_dev *dev)
> +{
> +	u8 pin;
> +	u8 slot = -1;
> +	int irq = 0;
> +	struct pci_host_bridge *hbrg = pci_find_host_bridge(dev->bus);
> +
> +	if (!(hbrg->map_irq)) {
> +		pci_dbg(dev, "runtime IRQ mapping not provided by arch\n");
> +		return;
> +	}
> +
> +	/*
> +	 * If this device is not on the primary bus, we need to figure out
> +	 * which interrupt pin it will come in on. We know which slot it
> +	 * will come in on because that slot is where the bridge is. Each
> +	 * time the interrupt line passes through a PCI-PCI bridge we must
> +	 * apply the swizzle function.
> +	 */
> +	pci_read_config_byte(dev, PCI_INTERRUPT_PIN, &pin);
> +	/* Cope with illegal. */
> +	if (pin > 4)
> +		pin = 1;
> +
> +	if (pin) {
> +		/* Follow the chain of bridges, swizzling as we go. */
> +		if (hbrg->swizzle_irq)
> +			slot = (*(hbrg->swizzle_irq))(dev, &pin);
> +
> +		/*
> +		 * If a swizzling function is not used, map_irq() must
> +		 * ignore slot.
> +		 */
> +		irq = (*(hbrg->map_irq))(dev, slot, pin);
> +		if (irq == -1)
> +			irq = 0;
> +	}
> +	dev->irq = irq;
> +
> +	pci_dbg(dev, "assign IRQ: got %d\n", dev->irq);
> +
> +	/*
> +	 * Always tell the device, so the driver knows what is the real IRQ
> +	 * to use; the device does not use it.
> +	 */
> +	pci_write_config_byte(dev, PCI_INTERRUPT_LINE, irq);
> +}
> +
> +static bool pci_check_and_set_intx_mask(struct pci_dev *dev, bool mask)
> +{
> +	struct pci_bus *bus = dev->bus;
> +	bool mask_updated = true;
> +	u32 cmd_status_dword;
> +	u16 origcmd, newcmd;
> +	unsigned long flags;
> +	bool irq_pending;
> +
> +	/*
> +	 * We do a single dword read to retrieve both command and status.
> +	 * Document assumptions that make this possible.
> +	 */
> +	BUILD_BUG_ON(PCI_COMMAND % 4);
> +	BUILD_BUG_ON(PCI_COMMAND + 2 != PCI_STATUS);
> +
> +	raw_spin_lock_irqsave(&pci_lock, flags);
> +
> +	bus->ops->read(bus, dev->devfn, PCI_COMMAND, 4, &cmd_status_dword);
> +
> +	irq_pending = (cmd_status_dword >> 16) & PCI_STATUS_INTERRUPT;
> +
> +	/*
> +	 * Check interrupt status register to see whether our device
> +	 * triggered the interrupt (when masking) or the next IRQ is
> +	 * already pending (when unmasking).
> +	 */
> +	if (mask != irq_pending) {
> +		mask_updated = false;
> +		goto done;
> +	}
> +
> +	origcmd = cmd_status_dword;
> +	newcmd = origcmd & ~PCI_COMMAND_INTX_DISABLE;
> +	if (mask)
> +		newcmd |= PCI_COMMAND_INTX_DISABLE;
> +	if (newcmd != origcmd)
> +		bus->ops->write(bus, dev->devfn, PCI_COMMAND, 2, newcmd);
> +
> +done:
> +	raw_spin_unlock_irqrestore(&pci_lock, flags);
> +
> +	return mask_updated;
> +}
> +
> +/**
> + * pci_check_and_mask_intx - mask INTx on pending interrupt
> + * @dev: the PCI device to operate on
> + *
> + * Check if the device dev has its INTx line asserted, mask it and return
> + * true in that case. False is returned if no interrupt was pending.
> + */
> +bool pci_check_and_mask_intx(struct pci_dev *dev)
> +{
> +	return pci_check_and_set_intx_mask(dev, true);
> +}
> +EXPORT_SYMBOL_GPL(pci_check_and_mask_intx);
> +
> +/**
> + * pci_check_and_unmask_intx - unmask INTx if no interrupt is pending
> + * @dev: the PCI device to operate on
> + *
> + * Check if the device dev has its INTx line asserted, unmask it if not and
> + * return true. False is returned and the mask remains active if there was
> + * still an interrupt pending.
> + */
> +bool pci_check_and_unmask_intx(struct pci_dev *dev)
> +{
> +	return pci_check_and_set_intx_mask(dev, false);
> +}
> +EXPORT_SYMBOL_GPL(pci_check_and_unmask_intx);
> +
> +/**
> + * pcibios_penalize_isa_irq - penalize an ISA IRQ
> + * @irq: ISA IRQ to penalize
> + * @active: IRQ active or not
> + *
> + * Permits the platform to provide architecture-specific functionality when
> + * penalizing ISA IRQs. This is the default implementation. Architecture
> + * implementations can override this.
> + */
> +void __weak pcibios_penalize_isa_irq(int irq, int active) {}
> +
> +int __weak pcibios_alloc_irq(struct pci_dev *dev)
> +{
> +	return 0;
> +}
> +
> +void __weak pcibios_free_irq(struct pci_dev *dev)
> +{
> +}
> diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
> index 51ec9e7e784f..ec838f2e892e 100644
> --- a/drivers/pci/pci-driver.c
> +++ b/drivers/pci/pci-driver.c
> @@ -419,15 +419,6 @@ static int __pci_device_probe(struct pci_driver *drv, struct pci_dev *pci_dev)
>  	return error;
>  }
>  
> -int __weak pcibios_alloc_irq(struct pci_dev *dev)
> -{
> -	return 0;
> -}
> -
> -void __weak pcibios_free_irq(struct pci_dev *dev)
> -{
> -}
> -
>  #ifdef CONFIG_PCI_IOV
>  static inline bool pci_device_can_probe(struct pci_dev *pdev)
>  {
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index d8f11a078924..75388584e60d 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -24,7 +24,6 @@
>  #include <linux/log2.h>
>  #include <linux/logic_pio.h>
>  #include <linux/pm_wakeup.h>
> -#include <linux/interrupt.h>
>  #include <linux/device.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/pci_hotplug.h>
> @@ -2292,17 +2291,6 @@ void __weak pcibios_release_device(struct pci_dev *dev) {}
>   */
>  void __weak pcibios_disable_device(struct pci_dev *dev) {}
>  
> -/**
> - * pcibios_penalize_isa_irq - penalize an ISA IRQ
> - * @irq: ISA IRQ to penalize
> - * @active: IRQ active or not
> - *
> - * Permits the platform to provide architecture-specific functionality when
> - * penalizing ISA IRQs. This is the default implementation. Architecture
> - * implementations can override this.
> - */
> -void __weak pcibios_penalize_isa_irq(int irq, int active) {}
> -
>  static void do_pci_disable_device(struct pci_dev *dev)
>  {
>  	u16 pci_command;
> @@ -3964,66 +3952,6 @@ int pci_enable_atomic_ops_to_root(struct pci_dev *dev, u32 cap_mask)
>  }
>  EXPORT_SYMBOL(pci_enable_atomic_ops_to_root);
>  
> -/**
> - * pci_swizzle_interrupt_pin - swizzle INTx for device behind bridge
> - * @dev: the PCI device
> - * @pin: the INTx pin (1=INTA, 2=INTB, 3=INTC, 4=INTD)
> - *
> - * Perform INTx swizzling for a device behind one level of bridge.  This is
> - * required by section 9.1 of the PCI-to-PCI bridge specification for devices
> - * behind bridges on add-in cards.  For devices with ARI enabled, the slot
> - * number is always 0 (see the Implementation Note in section 2.2.8.1 of
> - * the PCI Express Base Specification, Revision 2.1)
> - */
> -u8 pci_swizzle_interrupt_pin(const struct pci_dev *dev, u8 pin)
> -{
> -	int slot;
> -
> -	if (pci_ari_enabled(dev->bus))
> -		slot = 0;
> -	else
> -		slot = PCI_SLOT(dev->devfn);
> -
> -	return (((pin - 1) + slot) % 4) + 1;
> -}
> -
> -int pci_get_interrupt_pin(struct pci_dev *dev, struct pci_dev **bridge)
> -{
> -	u8 pin;
> -
> -	pin = dev->pin;
> -	if (!pin)
> -		return -1;
> -
> -	while (!pci_is_root_bus(dev->bus)) {
> -		pin = pci_swizzle_interrupt_pin(dev, pin);
> -		dev = dev->bus->self;
> -	}
> -	*bridge = dev;
> -	return pin;
> -}
> -
> -/**
> - * pci_common_swizzle - swizzle INTx all the way to root bridge
> - * @dev: the PCI device
> - * @pinp: pointer to the INTx pin value (1=INTA, 2=INTB, 3=INTD, 4=INTD)
> - *
> - * Perform INTx swizzling for a device.  This traverses through all PCI-to-PCI
> - * bridges all the way up to a PCI root bus.
> - */
> -u8 pci_common_swizzle(struct pci_dev *dev, u8 *pinp)
> -{
> -	u8 pin = *pinp;
> -
> -	while (!pci_is_root_bus(dev->bus)) {
> -		pin = pci_swizzle_interrupt_pin(dev, pin);
> -		dev = dev->bus->self;
> -	}
> -	*pinp = pin;
> -	return PCI_SLOT(dev->devfn);
> -}
> -EXPORT_SYMBOL_GPL(pci_common_swizzle);
> -
>  /**
>   * pci_release_region - Release a PCI bar
>   * @pdev: PCI device whose resources were previously reserved by
> @@ -4737,78 +4665,6 @@ void pci_intx(struct pci_dev *pdev, int enable)
>  }
>  EXPORT_SYMBOL_GPL(pci_intx);
>  
> -static bool pci_check_and_set_intx_mask(struct pci_dev *dev, bool mask)
> -{
> -	struct pci_bus *bus = dev->bus;
> -	bool mask_updated = true;
> -	u32 cmd_status_dword;
> -	u16 origcmd, newcmd;
> -	unsigned long flags;
> -	bool irq_pending;
> -
> -	/*
> -	 * We do a single dword read to retrieve both command and status.
> -	 * Document assumptions that make this possible.
> -	 */
> -	BUILD_BUG_ON(PCI_COMMAND % 4);
> -	BUILD_BUG_ON(PCI_COMMAND + 2 != PCI_STATUS);
> -
> -	raw_spin_lock_irqsave(&pci_lock, flags);
> -
> -	bus->ops->read(bus, dev->devfn, PCI_COMMAND, 4, &cmd_status_dword);
> -
> -	irq_pending = (cmd_status_dword >> 16) & PCI_STATUS_INTERRUPT;
> -
> -	/*
> -	 * Check interrupt status register to see whether our device
> -	 * triggered the interrupt (when masking) or the next IRQ is
> -	 * already pending (when unmasking).
> -	 */
> -	if (mask != irq_pending) {
> -		mask_updated = false;
> -		goto done;
> -	}
> -
> -	origcmd = cmd_status_dword;
> -	newcmd = origcmd & ~PCI_COMMAND_INTX_DISABLE;
> -	if (mask)
> -		newcmd |= PCI_COMMAND_INTX_DISABLE;
> -	if (newcmd != origcmd)
> -		bus->ops->write(bus, dev->devfn, PCI_COMMAND, 2, newcmd);
> -
> -done:
> -	raw_spin_unlock_irqrestore(&pci_lock, flags);
> -
> -	return mask_updated;
> -}
> -
> -/**
> - * pci_check_and_mask_intx - mask INTx on pending interrupt
> - * @dev: the PCI device to operate on
> - *
> - * Check if the device dev has its INTx line asserted, mask it and return
> - * true in that case. False is returned if no interrupt was pending.
> - */
> -bool pci_check_and_mask_intx(struct pci_dev *dev)
> -{
> -	return pci_check_and_set_intx_mask(dev, true);
> -}
> -EXPORT_SYMBOL_GPL(pci_check_and_mask_intx);
> -
> -/**
> - * pci_check_and_unmask_intx - unmask INTx if no interrupt is pending
> - * @dev: the PCI device to operate on
> - *
> - * Check if the device dev has its INTx line asserted, unmask it if not and
> - * return true. False is returned and the mask remains active if there was
> - * still an interrupt pending.
> - */
> -bool pci_check_and_unmask_intx(struct pci_dev *dev)
> -{
> -	return pci_check_and_set_intx_mask(dev, false);
> -}
> -EXPORT_SYMBOL_GPL(pci_check_and_unmask_intx);
> -
>  /**
>   * pci_wait_for_pending_transaction - wait for pending transaction
>   * @dev: the PCI device to operate on
> diff --git a/drivers/pci/setup-irq.c b/drivers/pci/setup-irq.c
> deleted file mode 100644
> index cc7d26b015f3..000000000000
> --- a/drivers/pci/setup-irq.c
> +++ /dev/null
> @@ -1,64 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0
> -/*
> - * Support routines for initializing a PCI subsystem
> - *
> - * Extruded from code written by
> - *      Dave Rusling (david.rusling@....mts.dec.com)
> - *      David Mosberger (davidm@...arizona.edu)
> - *	David Miller (davem@...hat.com)
> - */
> -
> -#include <linux/kernel.h>
> -#include <linux/pci.h>
> -#include <linux/errno.h>
> -#include <linux/ioport.h>
> -#include <linux/cache.h>
> -#include "pci.h"
> -
> -void pci_assign_irq(struct pci_dev *dev)
> -{
> -	u8 pin;
> -	u8 slot = -1;
> -	int irq = 0;
> -	struct pci_host_bridge *hbrg = pci_find_host_bridge(dev->bus);
> -
> -	if (!(hbrg->map_irq)) {
> -		pci_dbg(dev, "runtime IRQ mapping not provided by arch\n");
> -		return;
> -	}
> -
> -	/*
> -	 * If this device is not on the primary bus, we need to figure out
> -	 * which interrupt pin it will come in on. We know which slot it
> -	 * will come in on because that slot is where the bridge is. Each
> -	 * time the interrupt line passes through a PCI-PCI bridge we must
> -	 * apply the swizzle function.
> -	 */
> -	pci_read_config_byte(dev, PCI_INTERRUPT_PIN, &pin);
> -	/* Cope with illegal. */
> -	if (pin > 4)
> -		pin = 1;
> -
> -	if (pin) {
> -		/* Follow the chain of bridges, swizzling as we go. */
> -		if (hbrg->swizzle_irq)
> -			slot = (*(hbrg->swizzle_irq))(dev, &pin);
> -
> -		/*
> -		 * If a swizzling function is not used, map_irq() must
> -		 * ignore slot.
> -		 */
> -		irq = (*(hbrg->map_irq))(dev, slot, pin);
> -		if (irq == -1)
> -			irq = 0;
> -	}
> -	dev->irq = irq;
> -
> -	pci_dbg(dev, "assign IRQ: got %d\n", dev->irq);
> -
> -	/*
> -	 * Always tell the device, so the driver knows what is the real IRQ
> -	 * to use; the device does not use it.
> -	 */
> -	pci_write_config_byte(dev, PCI_INTERRUPT_LINE, irq);
> -}
> -- 
> 2.39.2
> 

Powered by blists - more mailing lists