2022-11-11 14:30:46

by Thomas Gleixner

[permalink] [raw]
Subject: [patch 30/39] PCI/MSI: Move pci_msi_restore_state() to api.c

From: Ahmed S. Darwish <[email protected]>

To distangle the maze in msi.c, all exported device-driver MSI APIs are
now to be grouped in one file, api.c.

Move pci_msi_enabled() and add kernel-doc for the function.

Signed-off-by: Ahmed S. Darwish <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>

diff --git a/drivers/pci/msi/api.c b/drivers/pci/msi/api.c
index ee9ed5ccd94d..8d1cf6db9bd7 100644
--- a/drivers/pci/msi/api.c
+++ b/drivers/pci/msi/api.c
@@ -308,6 +308,21 @@ void pci_free_irq_vectors(struct pci_dev *dev)
}
EXPORT_SYMBOL(pci_free_irq_vectors);

+/**
+ * pci_restore_msi_state() - Restore cached MSI(-X) state on device
+ * @dev: the PCI device to operate on
+ *
+ * Write the Linux-cached MSI(-X) state back on device. This is
+ * typically useful upon system resume, or after an error-recovery PCI
+ * adapter reset.
+ */
+void pci_restore_msi_state(struct pci_dev *dev)
+{
+ __pci_restore_msi_state(dev);
+ __pci_restore_msix_state(dev);
+}
+EXPORT_SYMBOL_GPL(pci_restore_msi_state);
+
/**
* pci_msi_enabled() - Are MSI(-X) interrupts enabled system-wide?
*
diff --git a/drivers/pci/msi/msi.c b/drivers/pci/msi/msi.c
index 59c33bc7fe81..a5d168c823ff 100644
--- a/drivers/pci/msi/msi.c
+++ b/drivers/pci/msi/msi.c
@@ -199,7 +199,7 @@ bool __weak arch_restore_msi_irqs(struct pci_dev *dev)
return true;
}

-static void __pci_restore_msi_state(struct pci_dev *dev)
+void __pci_restore_msi_state(struct pci_dev *dev)
{
struct msi_desc *entry;
u16 control;
@@ -231,7 +231,7 @@ static void pci_msix_clear_and_set_ctrl(struct pci_dev *dev, u16 clear, u16 set)
pci_write_config_word(dev, dev->msix_cap + PCI_MSIX_FLAGS, ctrl);
}

-static void __pci_restore_msix_state(struct pci_dev *dev)
+void __pci_restore_msix_state(struct pci_dev *dev)
{
struct msi_desc *entry;
bool write_msg;
@@ -257,13 +257,6 @@ static void __pci_restore_msix_state(struct pci_dev *dev)
pci_msix_clear_and_set_ctrl(dev, PCI_MSIX_FLAGS_MASKALL, 0);
}

-void pci_restore_msi_state(struct pci_dev *dev)
-{
- __pci_restore_msi_state(dev);
- __pci_restore_msix_state(dev);
-}
-EXPORT_SYMBOL_GPL(pci_restore_msi_state);
-
static void pcim_msi_release(void *pcidev)
{
struct pci_dev *dev = pcidev;
diff --git a/drivers/pci/msi/msi.h b/drivers/pci/msi/msi.h
index f3f4ede53171..8170ef2c5ad0 100644
--- a/drivers/pci/msi/msi.h
+++ b/drivers/pci/msi/msi.h
@@ -94,6 +94,8 @@ void pci_free_msi_irqs(struct pci_dev *dev);
int __pci_enable_msi_range(struct pci_dev *dev, int minvec, int maxvec, struct irq_affinity *affd);
int __pci_enable_msix_range(struct pci_dev *dev, struct msix_entry *entries, int minvec,
int maxvec, struct irq_affinity *affd, int flags);
+void __pci_restore_msi_state(struct pci_dev *dev);
+void __pci_restore_msix_state(struct pci_dev *dev);

/* Legacy (!IRQDOMAIN) fallbacks */




2022-11-16 16:53:23

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [patch 30/39] PCI/MSI: Move pci_msi_restore_state() to api.c

On Fri, Nov 11, 2022 at 02:55:03PM +0100, Thomas Gleixner wrote:
> From: Ahmed S. Darwish <[email protected]>
>
> To distangle the maze in msi.c, all exported device-driver MSI APIs are
> now to be grouped in one file, api.c.
>
> Move pci_msi_enabled() and add kernel-doc for the function.
>
> Signed-off-by: Ahmed S. Darwish <[email protected]>
> Signed-off-by: Thomas Gleixner <[email protected]>

Acked-by: Bjorn Helgaas <[email protected]>

> diff --git a/drivers/pci/msi/api.c b/drivers/pci/msi/api.c
> index ee9ed5ccd94d..8d1cf6db9bd7 100644
> --- a/drivers/pci/msi/api.c
> +++ b/drivers/pci/msi/api.c
> @@ -308,6 +308,21 @@ void pci_free_irq_vectors(struct pci_dev *dev)
> }
> EXPORT_SYMBOL(pci_free_irq_vectors);
>
> +/**
> + * pci_restore_msi_state() - Restore cached MSI(-X) state on device
> + * @dev: the PCI device to operate on
> + *
> + * Write the Linux-cached MSI(-X) state back on device. This is
> + * typically useful upon system resume, or after an error-recovery PCI
> + * adapter reset.
> + */
> +void pci_restore_msi_state(struct pci_dev *dev)
> +{
> + __pci_restore_msi_state(dev);
> + __pci_restore_msix_state(dev);
> +}
> +EXPORT_SYMBOL_GPL(pci_restore_msi_state);
> +
> /**
> * pci_msi_enabled() - Are MSI(-X) interrupts enabled system-wide?
> *
> diff --git a/drivers/pci/msi/msi.c b/drivers/pci/msi/msi.c
> index 59c33bc7fe81..a5d168c823ff 100644
> --- a/drivers/pci/msi/msi.c
> +++ b/drivers/pci/msi/msi.c
> @@ -199,7 +199,7 @@ bool __weak arch_restore_msi_irqs(struct pci_dev *dev)
> return true;
> }
>
> -static void __pci_restore_msi_state(struct pci_dev *dev)
> +void __pci_restore_msi_state(struct pci_dev *dev)
> {
> struct msi_desc *entry;
> u16 control;
> @@ -231,7 +231,7 @@ static void pci_msix_clear_and_set_ctrl(struct pci_dev *dev, u16 clear, u16 set)
> pci_write_config_word(dev, dev->msix_cap + PCI_MSIX_FLAGS, ctrl);
> }
>
> -static void __pci_restore_msix_state(struct pci_dev *dev)
> +void __pci_restore_msix_state(struct pci_dev *dev)
> {
> struct msi_desc *entry;
> bool write_msg;
> @@ -257,13 +257,6 @@ static void __pci_restore_msix_state(struct pci_dev *dev)
> pci_msix_clear_and_set_ctrl(dev, PCI_MSIX_FLAGS_MASKALL, 0);
> }
>
> -void pci_restore_msi_state(struct pci_dev *dev)
> -{
> - __pci_restore_msi_state(dev);
> - __pci_restore_msix_state(dev);
> -}
> -EXPORT_SYMBOL_GPL(pci_restore_msi_state);
> -
> static void pcim_msi_release(void *pcidev)
> {
> struct pci_dev *dev = pcidev;
> diff --git a/drivers/pci/msi/msi.h b/drivers/pci/msi/msi.h
> index f3f4ede53171..8170ef2c5ad0 100644
> --- a/drivers/pci/msi/msi.h
> +++ b/drivers/pci/msi/msi.h
> @@ -94,6 +94,8 @@ void pci_free_msi_irqs(struct pci_dev *dev);
> int __pci_enable_msi_range(struct pci_dev *dev, int minvec, int maxvec, struct irq_affinity *affd);
> int __pci_enable_msix_range(struct pci_dev *dev, struct msix_entry *entries, int minvec,
> int maxvec, struct irq_affinity *affd, int flags);
> +void __pci_restore_msi_state(struct pci_dev *dev);
> +void __pci_restore_msix_state(struct pci_dev *dev);
>
> /* Legacy (!IRQDOMAIN) fallbacks */
>
>

2022-11-16 18:49:12

by Jason Gunthorpe

[permalink] [raw]
Subject: Re: [patch 30/39] PCI/MSI: Move pci_msi_restore_state() to api.c

On Fri, Nov 11, 2022 at 02:55:03PM +0100, Thomas Gleixner wrote:
> From: Ahmed S. Darwish <[email protected]>
>
> To distangle the maze in msi.c, all exported device-driver MSI APIs are
> now to be grouped in one file, api.c.
>
> Move pci_msi_enabled() and add kernel-doc for the function.
>
> Signed-off-by: Ahmed S. Darwish <[email protected]>
> Signed-off-by: Thomas Gleixner <[email protected]>
>
> diff --git a/drivers/pci/msi/api.c b/drivers/pci/msi/api.c
> index ee9ed5ccd94d..8d1cf6db9bd7 100644
> --- a/drivers/pci/msi/api.c
> +++ b/drivers/pci/msi/api.c
> @@ -308,6 +308,21 @@ void pci_free_irq_vectors(struct pci_dev *dev)
> }
> EXPORT_SYMBOL(pci_free_irq_vectors);
>
> +/**
> + * pci_restore_msi_state() - Restore cached MSI(-X) state on device
> + * @dev: the PCI device to operate on
> + *
> + * Write the Linux-cached MSI(-X) state back on device. This is
> + * typically useful upon system resume, or after an error-recovery PCI
> + * adapter reset.
> + */
> +void pci_restore_msi_state(struct pci_dev *dev)
> +{
> + __pci_restore_msi_state(dev);
> + __pci_restore_msix_state(dev);
> +}
> +EXPORT_SYMBOL_GPL(pci_restore_msi_state);

This leaves behind two functions that are only called from here.

I think they should be moved into this file as well.

Or perhaps more broadly, it would make sense if the functions
were split up into 'for irq chip implementors' 'for drivers' and
'shared utilities'.

There are several other examples, like
__pci_enable_msix_range()/__pci_enable_msi_range() that are only used
by api.c and could reasonably move as well, plus their tree of
single-use support functions.

And if you think about it that way - things have ended up so that
api.c is really all about vector allocation (as that is the only thing
the drivers actually do) so it would be tempting to call it
allocator.c and consolidate everything in that topic.

Jason

Subject: [tip: irq/core] PCI/MSI: Move pci_msi_restore_state() to api.c

The following commit has been merged into the irq/core branch of tip:

Commit-ID: 57127da98bc87688324cc2d29927b340d7754701
Gitweb: https://git.kernel.org/tip/57127da98bc87688324cc2d29927b340d7754701
Author: Ahmed S. Darwish <[email protected]>
AuthorDate: Fri, 11 Nov 2022 14:55:03 +01:00
Committer: Thomas Gleixner <[email protected]>
CommitterDate: Thu, 17 Nov 2022 15:15:21 +01:00

PCI/MSI: Move pci_msi_restore_state() to api.c

To disentangle the maze in msi.c, all exported device-driver MSI APIs are
now to be grouped in one file, api.c.

Move pci_msi_enabled() and add kernel-doc for the function.

Signed-off-by: Ahmed S. Darwish <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
Acked-by: Bjorn Helgaas <[email protected]>
Link: https://lore.kernel.org/r/[email protected]


---
drivers/pci/msi/api.c | 15 +++++++++++++++
drivers/pci/msi/msi.c | 11 ++---------
drivers/pci/msi/msi.h | 2 ++
3 files changed, 19 insertions(+), 9 deletions(-)

diff --git a/drivers/pci/msi/api.c b/drivers/pci/msi/api.c
index 49ae3a3..6c3ad48 100644
--- a/drivers/pci/msi/api.c
+++ b/drivers/pci/msi/api.c
@@ -309,6 +309,21 @@ void pci_free_irq_vectors(struct pci_dev *dev)
EXPORT_SYMBOL(pci_free_irq_vectors);

/**
+ * pci_restore_msi_state() - Restore cached MSI(-X) state on device
+ * @dev: the PCI device to operate on
+ *
+ * Write the Linux-cached MSI(-X) state back on device. This is
+ * typically useful upon system resume, or after an error-recovery PCI
+ * adapter reset.
+ */
+void pci_restore_msi_state(struct pci_dev *dev)
+{
+ __pci_restore_msi_state(dev);
+ __pci_restore_msix_state(dev);
+}
+EXPORT_SYMBOL_GPL(pci_restore_msi_state);
+
+/**
* pci_msi_enabled() - Are MSI(-X) interrupts enabled system-wide?
*
* Return: true if MSI has not been globally disabled through ACPI FADT,
diff --git a/drivers/pci/msi/msi.c b/drivers/pci/msi/msi.c
index 59c33bc..a5d168c 100644
--- a/drivers/pci/msi/msi.c
+++ b/drivers/pci/msi/msi.c
@@ -199,7 +199,7 @@ bool __weak arch_restore_msi_irqs(struct pci_dev *dev)
return true;
}

-static void __pci_restore_msi_state(struct pci_dev *dev)
+void __pci_restore_msi_state(struct pci_dev *dev)
{
struct msi_desc *entry;
u16 control;
@@ -231,7 +231,7 @@ static void pci_msix_clear_and_set_ctrl(struct pci_dev *dev, u16 clear, u16 set)
pci_write_config_word(dev, dev->msix_cap + PCI_MSIX_FLAGS, ctrl);
}

-static void __pci_restore_msix_state(struct pci_dev *dev)
+void __pci_restore_msix_state(struct pci_dev *dev)
{
struct msi_desc *entry;
bool write_msg;
@@ -257,13 +257,6 @@ static void __pci_restore_msix_state(struct pci_dev *dev)
pci_msix_clear_and_set_ctrl(dev, PCI_MSIX_FLAGS_MASKALL, 0);
}

-void pci_restore_msi_state(struct pci_dev *dev)
-{
- __pci_restore_msi_state(dev);
- __pci_restore_msix_state(dev);
-}
-EXPORT_SYMBOL_GPL(pci_restore_msi_state);
-
static void pcim_msi_release(void *pcidev)
{
struct pci_dev *dev = pcidev;
diff --git a/drivers/pci/msi/msi.h b/drivers/pci/msi/msi.h
index f3f4ede..8170ef2 100644
--- a/drivers/pci/msi/msi.h
+++ b/drivers/pci/msi/msi.h
@@ -94,6 +94,8 @@ void pci_free_msi_irqs(struct pci_dev *dev);
int __pci_enable_msi_range(struct pci_dev *dev, int minvec, int maxvec, struct irq_affinity *affd);
int __pci_enable_msix_range(struct pci_dev *dev, struct msix_entry *entries, int minvec,
int maxvec, struct irq_affinity *affd, int flags);
+void __pci_restore_msi_state(struct pci_dev *dev);
+void __pci_restore_msix_state(struct pci_dev *dev);

/* Legacy (!IRQDOMAIN) fallbacks */