Re: [PATCH v18 02/20] PCI: Rename PCI_EPC_IRQ_LEGACY to PCI_EPC_IRQ_INTX
flat view
From: Manivannan Sadhasivam <mani@kernel.org>
Date: 2023-07-30 04:58:48
Also in:
linux-pci, linux-renesas-soc
On Sat, Jul 29, 2023 at 10:32:01AM -0500, Bjorn Helgaas wrote:
On Sat, Jul 29, 2023 at 10:58:46AM +0900, Damien Le Moal wrote:quoted
On 7/29/23 10:55, Damien Le Moal wrote:quoted
On 7/29/23 10:35, Serge Semin wrote:quoted
On Mon, Jul 24, 2023 at 01:02:11PM +0530, Manivannan Sadhasivam wrote:quoted
On Fri, Jul 21, 2023 at 05:10:27PM +0900, Damien Le Moal wrote:quoted
On 7/21/23 16:44, Yoshihiro Shimoda wrote:quoted
Using "INTx" instead of "legacy" is more specific. So, rename PCI_EPC_IRQ_LEGACY to PCI_EPC_IRQ_INTX. Suggested-by: Bjorn Helgaas <helgaas@kernel.org> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> Reviewed-by: Manivannan Sadhasivam <mani@kernel.org> Acked-by: Jesper Nilsson <jesper.nilsson@axis.com> # ARTPEC Reviewed-by: Serge Semin <redacted>quoted
quoted
I would rather drop completely the PCI_EPC_IRQ_XXX enum and simply use the PCI_IRQ_XXX macros used everywhere. Less definitions :) See attached patch that I have in my queue (about to send that).It looks better! This patch should be dropped.Back then Bjorn specifically asked to change the names suffix in a preparation patch before adding the INTx support to the DW PCIe core driver (see the Sb tag in the patch log). Damien, seeing you cleanup the names anyway what about fixing the macro suffix too: INTx instead of LEGACY)?Sure, I can do that. That is going to be a gigantic patch though given that PCI_IRQ_LEGACY is used well beyond the ep/pcie controller drivers. While I agree it would be nice to do, not sure it is worth such code churn.quoted
Mani, Damien, what do you suggest to Yoshihiro to do with the LEGACY/INTx names in the following up patches of this series?If everyone is OK with the patch I proposed (the PCI_IRQ_LEGACY -> PCI_IRQ_INTx change can go on top), then I can rebase it and send it next week (the remaining of my EP cleanup series needs some more testing & rebasing). Yoshihiro can either include it in his series or rebase on it if the patch is added to pci-next quickly.Note that we could start by simply defining an alias: #define PCI_IRQ_INTx PCI_IRQ_LEGACY and gradually convert all drivers using it until we can get rid of PCI_IRQ_LEGACY.I try to catch additions of "legacy," e.g., in new drivers, but I agree this patch looks like it might be more churn than it's worth. But I like your idea of an alias, Damien. Maybe something like the below to make it more obvious that the preferred usage is the "INTX" form.
Looks good to me. I'd prefer to merge these (Damien's) patches first and let Yoshihiro rebase on top of pci-next instead of clubbing everything in a single series. - Mani
quoted hunk ↗ jump to hunk
diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h index 5cb694031072..6c0bb4c5d12e 100644 --- a/include/linux/pci-epc.h +++ b/include/linux/pci-epc.h@@ -21,11 +21,13 @@ enum pci_epc_interface_type { enum pci_epc_irq_type { PCI_EPC_IRQ_UNKNOWN, - PCI_EPC_IRQ_LEGACY, + PCI_EPC_IRQ_INTX, PCI_EPC_IRQ_MSI, PCI_EPC_IRQ_MSIX, }; +#define PCI_EPC_IRQ_LEGACY PCI_EPC_IRQ_INTX + static inline const char * pci_epc_interface_string(enum pci_epc_interface_type type) {diff --git a/include/linux/pci.h b/include/linux/pci.h index c69a2cc1f412..6638e0cd487f 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h@@ -1048,11 +1048,13 @@ enum { PCI_SCAN_ALL_PCIE_DEVS = 0x00000040, /* Scan all, not just dev 0 */ }; -#define PCI_IRQ_LEGACY (1 << 0) /* Allow legacy interrupts */ +#define PCI_IRQ_INTX (1 << 0) /* Allow INTx interrupts */ #define PCI_IRQ_MSI (1 << 1) /* Allow MSI interrupts */ #define PCI_IRQ_MSIX (1 << 2) /* Allow MSI-X interrupts */ #define PCI_IRQ_AFFINITY (1 << 3) /* Auto-assign affinity */ +#define PCI_IRQ_LEGACY PCI_IRQ_INTX /* prefer PCI_IRQ_INTX */ + /* These external functions are only available when PCI support is enabled */ #ifdef CONFIG_PCI
-- மணிவண்ணன் சதாசிவம்