The MSI message API has gone through some churn, that I think results
in an inconsistent interface. We now have this:
void read_msi_msg(struct msi_desc *entry, struct msi_msg *msg);
void get_cached_msi_msg(struct msi_desc *entry, struct msi_msg *msg);
void __write_msi_msg(struct msi_desc *entry, struct msi_msg *msg);
void write_msi_msg(unsigned int irq, struct msi_msg *msg);
write_msi_msg() takes an irq arg, but read_msi_msg() takes an
msi_desc. Presumably write_msi_msg() was not converted because it
has a much larger user base, but this sort of inconsitency results
in a poor API.
This series reverts a selection of commits to return us to:
void __read_msi_msg(struct msi_desc *entry, struct msi_msg *msg);
void __get_cached_msi_msg(struct msi_desc *entry, struct msi_msg *msg);
void __write_msi_msg(struct msi_desc *entry, struct msi_msg *msg);
void get_cached_msi_msg(unsigned int irq, struct msi_msg *msg);
void write_msi_msg(unsigned int irq, struct msi_msg *msg);
I've left the removal of read_msi_msg() since it has no users, but
restored get_cached_msi_msg() since it has an imminent user. This
will also cleanup the upcoming merge conflicts in next. Thanks,
Alex
---
Alex Williamson (3):
Revert "PCI/MSI: Remove unused get_cached_msi_msg()"
Revert "PCI/MSI: Rename __get_cached_msi_msg() to get_cached_msi_msg()"
Revert "PCI/MSI: Rename __read_msi_msg() to read_msi_msg()"
arch/ia64/kernel/msi_ia64.c | 2 +-
arch/ia64/sn/kernel/msi_sn.c | 2 +-
arch/powerpc/platforms/pseries/msi.c | 2 +-
arch/x86/kernel/apic/io_apic.c | 2 +-
arch/x86/pci/xen.c | 2 +-
drivers/pci/msi.c | 11 +++++++++--
include/linux/msi.h | 5 +++--
7 files changed, 17 insertions(+), 9 deletions(-)
This is another case of making the MSI msg API inconsistent, for
seemingly no gain. Continue to use __get_cached_msi_msg() with an
msi_desc and leave get_cached_msi_msg() for an irq interfaces.
This reverts 18ef822c59f60d52c8534b295086d38e9e8e0f7d
Signed-off-by: Alex Williamson <[email protected]>
---
arch/ia64/kernel/msi_ia64.c | 2 +-
arch/ia64/sn/kernel/msi_sn.c | 2 +-
arch/x86/kernel/apic/io_apic.c | 2 +-
drivers/pci/msi.c | 2 +-
include/linux/msi.h | 2 +-
5 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/arch/ia64/kernel/msi_ia64.c b/arch/ia64/kernel/msi_ia64.c
index 4efe748..8c3730c 100644
--- a/arch/ia64/kernel/msi_ia64.c
+++ b/arch/ia64/kernel/msi_ia64.c
@@ -23,7 +23,7 @@ static int ia64_set_msi_irq_affinity(struct irq_data *idata,
if (irq_prepare_move(irq, cpu))
return -1;
- get_cached_msi_msg(idata->msi_desc, &msg);
+ __get_cached_msi_msg(idata->msi_desc, &msg);
addr = msg.address_lo;
addr &= MSI_ADDR_DEST_ID_MASK;
diff --git a/arch/ia64/sn/kernel/msi_sn.c b/arch/ia64/sn/kernel/msi_sn.c
index 8bec242..446e779 100644
--- a/arch/ia64/sn/kernel/msi_sn.c
+++ b/arch/ia64/sn/kernel/msi_sn.c
@@ -175,7 +175,7 @@ static int sn_set_msi_irq_affinity(struct irq_data *data,
* Release XIO resources for the old MSI PCI address
*/
- get_cached_msi_msg(data->msi_desc, &msg);
+ __get_cached_msi_msg(data->msi_desc, &msg);
sn_pdev = (struct pcidev_info *)sn_irq_info->irq_pciioinfo;
pdev = sn_pdev->pdi_linux_pcidev;
provider = SN_PCIDEV_BUSPROVIDER(pdev);
diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index e877cfb..29290f5 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -3145,7 +3145,7 @@ msi_set_affinity(struct irq_data *data, const struct cpumask *mask, bool force)
if (ret)
return ret;
- get_cached_msi_msg(data->msi_desc, &msg);
+ __get_cached_msi_msg(data->msi_desc, &msg);
msg.data &= ~MSI_DATA_VECTOR_MASK;
msg.data |= MSI_DATA_VECTOR(cfg->vector);
diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index 12d0f29..2bd0fbe 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -279,7 +279,7 @@ void __read_msi_msg(struct msi_desc *entry, struct msi_msg *msg)
}
}
-void get_cached_msi_msg(struct msi_desc *entry, struct msi_msg *msg)
+void __get_cached_msi_msg(struct msi_desc *entry, struct msi_msg *msg)
{
/* Assert that the cache is valid, assuming that
* valid messages are not all-zeroes. */
diff --git a/include/linux/msi.h b/include/linux/msi.h
index 0bb302f..d0e0cfe 100644
--- a/include/linux/msi.h
+++ b/include/linux/msi.h
@@ -16,7 +16,7 @@ struct msi_desc;
void mask_msi_irq(struct irq_data *data);
void unmask_msi_irq(struct irq_data *data);
void __read_msi_msg(struct msi_desc *entry, struct msi_msg *msg);
-void get_cached_msi_msg(struct msi_desc *entry, struct msi_msg *msg);
+void __get_cached_msi_msg(struct msi_desc *entry, struct msi_msg *msg);
void __write_msi_msg(struct msi_desc *entry, struct msi_msg *msg);
void write_msi_msg(unsigned int irq, struct msi_msg *msg);
We have an upcoming merge collision with code that actually does use
this, b8f02af096b1fc9fd46680cbe55214e477eb76d3. Restore this function
in advance of the merge.
This reverts a160fe94cb533a438258642d8d5b2b3795497341
Signed-off-by: Alex Williamson <[email protected]>
---
drivers/pci/msi.c | 7 +++++++
include/linux/msi.h | 1 +
2 files changed, 8 insertions(+)
diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index 2bd0fbe..12fdb27 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -289,6 +289,13 @@ void __get_cached_msi_msg(struct msi_desc *entry, struct msi_msg *msg)
*msg = entry->msg;
}
+void get_cached_msi_msg(unsigned int irq, struct msi_msg *msg)
+{
+ struct msi_desc *entry = irq_get_msi_desc(irq);
+
+ __get_cached_msi_msg(entry, msg);
+}
+
void __write_msi_msg(struct msi_desc *entry, struct msi_msg *msg)
{
if (entry->dev->current_state != PCI_D0) {
diff --git a/include/linux/msi.h b/include/linux/msi.h
index d0e0cfe..fb35a71 100644
--- a/include/linux/msi.h
+++ b/include/linux/msi.h
@@ -18,6 +18,7 @@ void unmask_msi_irq(struct irq_data *data);
void __read_msi_msg(struct msi_desc *entry, struct msi_msg *msg);
void __get_cached_msi_msg(struct msi_desc *entry, struct msi_msg *msg);
void __write_msi_msg(struct msi_desc *entry, struct msi_msg *msg);
+void get_cached_msi_msg(unsigned int irq, struct msi_msg *msg);
void write_msi_msg(unsigned int irq, struct msi_msg *msg);
struct msi_desc {
This makes for an inconsistent MSI API, read_msi_msg() takes an
msi_desc arg, but write_msi_msg() takes an irq. Let's keep the
API consistent that the __read/write interfaces take an msi_desc
and the standard interfaces take an irq.
This reverts 09d4ecd3f067e307211882412f8acd09daaacca2
Signed-off-by: Alex Williamson <[email protected]>
---
arch/powerpc/platforms/pseries/msi.c | 2 +-
arch/x86/pci/xen.c | 2 +-
drivers/pci/msi.c | 2 +-
include/linux/msi.h | 2 +-
4 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/msi.c b/arch/powerpc/platforms/pseries/msi.c
index 88f14a6..8ab5add 100644
--- a/arch/powerpc/platforms/pseries/msi.c
+++ b/arch/powerpc/platforms/pseries/msi.c
@@ -476,7 +476,7 @@ again:
irq_set_msi_desc(virq, entry);
/* Read config space back so we can restore after reset */
- read_msi_msg(entry, &msg);
+ __read_msi_msg(entry, &msg);
entry->msg = msg;
}
diff --git a/arch/x86/pci/xen.c b/arch/x86/pci/xen.c
index ad03739..093f5f4 100644
--- a/arch/x86/pci/xen.c
+++ b/arch/x86/pci/xen.c
@@ -229,7 +229,7 @@ static int xen_hvm_setup_msi_irqs(struct pci_dev *dev, int nvec, int type)
return 1;
list_for_each_entry(msidesc, &dev->msi_list, list) {
- read_msi_msg(msidesc, &msg);
+ __read_msi_msg(msidesc, &msg);
pirq = MSI_ADDR_EXT_DEST_ID(msg.address_hi) |
((msg.address_lo >> MSI_ADDR_DEST_ID_SHIFT) & 0xff);
if (msg.data != XEN_PIRQ_MSI_DATA ||
diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index b6565ea..12d0f29 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -249,7 +249,7 @@ void default_restore_msi_irqs(struct pci_dev *dev)
}
}
-void read_msi_msg(struct msi_desc *entry, struct msi_msg *msg)
+void __read_msi_msg(struct msi_desc *entry, struct msi_msg *msg)
{
BUG_ON(entry->dev->current_state != PCI_D0);
diff --git a/include/linux/msi.h b/include/linux/msi.h
index 36c63cf..0bb302f 100644
--- a/include/linux/msi.h
+++ b/include/linux/msi.h
@@ -15,7 +15,7 @@ struct irq_data;
struct msi_desc;
void mask_msi_irq(struct irq_data *data);
void unmask_msi_irq(struct irq_data *data);
-void read_msi_msg(struct msi_desc *entry, struct msi_msg *msg);
+void __read_msi_msg(struct msi_desc *entry, struct msi_msg *msg);
void get_cached_msi_msg(struct msi_desc *entry, struct msi_msg *msg);
void __write_msi_msg(struct msi_desc *entry, struct msi_msg *msg);
void write_msi_msg(unsigned int irq, struct msi_msg *msg);
On Wed, Oct 01, 2014 at 10:48:16AM -0600, Alex Williamson wrote:
> The MSI message API has gone through some churn, that I think results
> in an inconsistent interface. We now have this:
>
> void read_msi_msg(struct msi_desc *entry, struct msi_msg *msg);
> void get_cached_msi_msg(struct msi_desc *entry, struct msi_msg *msg);
> void __write_msi_msg(struct msi_desc *entry, struct msi_msg *msg);
> void write_msi_msg(unsigned int irq, struct msi_msg *msg);
>
> write_msi_msg() takes an irq arg, but read_msi_msg() takes an
> msi_desc. Presumably write_msi_msg() was not converted because it
> has a much larger user base, but this sort of inconsitency results
> in a poor API.
Yeah, that isn't good, thanks for noticing.
I rebuilt the pci/msi branch and dropped the commits you mentioned.
The ones that are left are these:
56b72b409579 PCI/MSI: Use __write_msi_msg() instead of write_msi_msg()
1e8f4cc82ede MSI/powerpc: Use __read_msi_msg() instead of read_msi_msg()
2b260085e466 PCI/MSI: Use __get_cached_msi_msg() instead of get_cached_msi_msg()
which I think are OK.
> This series reverts a selection of commits to return us to:
>
> void __read_msi_msg(struct msi_desc *entry, struct msi_msg *msg);
> void __get_cached_msi_msg(struct msi_desc *entry, struct msi_msg *msg);
> void __write_msi_msg(struct msi_desc *entry, struct msi_msg *msg);
> void get_cached_msi_msg(unsigned int irq, struct msi_msg *msg);
> void write_msi_msg(unsigned int irq, struct msi_msg *msg);
>
> I've left the removal of read_msi_msg() since it has no users, but
> restored get_cached_msi_msg() since it has an imminent user. This
> will also cleanup the upcoming merge conflicts in next. Thanks,
>
> Alex
>
> ---
>
> Alex Williamson (3):
> Revert "PCI/MSI: Remove unused get_cached_msi_msg()"
> Revert "PCI/MSI: Rename __get_cached_msi_msg() to get_cached_msi_msg()"
> Revert "PCI/MSI: Rename __read_msi_msg() to read_msi_msg()"
>
>
> arch/ia64/kernel/msi_ia64.c | 2 +-
> arch/ia64/sn/kernel/msi_sn.c | 2 +-
> arch/powerpc/platforms/pseries/msi.c | 2 +-
> arch/x86/kernel/apic/io_apic.c | 2 +-
> arch/x86/pci/xen.c | 2 +-
> drivers/pci/msi.c | 11 +++++++++--
> include/linux/msi.h | 5 +++--
> 7 files changed, 17 insertions(+), 9 deletions(-)