| Message ID | 20211206210438.688216619@linutronix.de (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | genirq/msi, PCI/MSI: Spring cleaning - Part 2 | expand |
On Mon, Dec 06, 2021 at 11:39:26PM +0100, Thomas Gleixner wrote: > Store the properties which are interesting for various places so the MSI > descriptor fiddling can be removed. > > Signed-off-by: Thomas Gleixner <tglx@linutronix.de> Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
On Mon, Dec 06, 2021 at 11:39:26PM +0100, Thomas Gleixner wrote: > Store the properties which are interesting for various places so the MSI > descriptor fiddling can be removed. > > Signed-off-by: Thomas Gleixner <tglx@linutronix.de> Acked-by: Bjorn Helgaas <bhelgaas@google.com> > --- > V2: Use the setter function > --- > drivers/pci/msi/msi.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > --- a/drivers/pci/msi/msi.c > +++ b/drivers/pci/msi/msi.c > @@ -244,6 +244,8 @@ static void free_msi_irqs(struct pci_dev > iounmap(dev->msix_base); > dev->msix_base = NULL; > } > + > + msi_device_set_properties(&dev->dev, 0); > } > > static void pci_intx_for_msi(struct pci_dev *dev, int enable) > @@ -341,6 +343,7 @@ msi_setup_entry(struct pci_dev *dev, int > { > struct irq_affinity_desc *masks = NULL; > struct msi_desc *entry; > + unsigned long prop; > u16 control; > > if (affd) > @@ -372,6 +375,10 @@ msi_setup_entry(struct pci_dev *dev, int > if (entry->pci.msi_attrib.can_mask) > pci_read_config_dword(dev, entry->pci.mask_pos, &entry->pci.msi_mask); > > + prop = MSI_PROP_PCI_MSI; > + if (entry->pci.msi_attrib.is_64) > + prop |= MSI_PROP_64BIT; > + msi_device_set_properties(&dev->dev, prop); > out: > kfree(masks); > return entry; > @@ -514,6 +521,7 @@ static int msix_setup_entries(struct pci > if (masks) > curmsk++; > } > + msi_device_set_properties(&dev->dev, MSI_PROP_PCI_MSIX | MSI_PROP_64BIT); > ret = 0; > out: > kfree(masks); >
On Mon, Dec 06, 2021 at 11:39:26PM +0100, Thomas Gleixner wrote: > Store the properties which are interesting for various places so the MSI > descriptor fiddling can be removed. > > Signed-off-by: Thomas Gleixner <tglx@linutronix.de> > --- > V2: Use the setter function > --- > drivers/pci/msi/msi.c | 8 ++++++++ > 1 file changed, 8 insertions(+) I took more time to look at this, to summarize my remarks on the other patches I think we don't need properties. The info in the msi_desc can come from the pci_dev which we have easy access to. This seems overall clearer The notable one is the sysfs, but that is probably better handled by storing a const char *sysfs_label in the dev->msi and emitting that instead of computing it. Jason
On Wed, Dec 08 2021 at 11:58, Jason Gunthorpe wrote: > On Mon, Dec 06, 2021 at 11:39:26PM +0100, Thomas Gleixner wrote: >> Store the properties which are interesting for various places so the MSI >> descriptor fiddling can be removed. >> >> Signed-off-by: Thomas Gleixner <tglx@linutronix.de> >> --- >> V2: Use the setter function >> --- >> drivers/pci/msi/msi.c | 8 ++++++++ >> 1 file changed, 8 insertions(+) > > I took more time to look at this, to summarize my remarks on the other > patches > > I think we don't need properties. The info in the msi_desc can come > from the pci_dev which we have easy access to. This seems overall > clearer I fixed that now. > The notable one is the sysfs, but that is probably better handled by > storing a > > const char *sysfs_label > > in the dev->msi and emitting that instead of computing it. I just compute is for now via is_pci_dev() and to_pci_dev()->msi_enabled. We are still debating to remove the whole thing completely. Thanks, tglx
On Thu, Dec 09 2021 at 18:53, Thomas Gleixner wrote: > On Wed, Dec 08 2021 at 11:58, Jason Gunthorpe wrote: >> On Mon, Dec 06, 2021 at 11:39:26PM +0100, Thomas Gleixner wrote: >>> Store the properties which are interesting for various places so the MSI >>> descriptor fiddling can be removed. >>> >>> Signed-off-by: Thomas Gleixner <tglx@linutronix.de> >>> --- >>> V2: Use the setter function >>> --- >>> drivers/pci/msi/msi.c | 8 ++++++++ >>> 1 file changed, 8 insertions(+) >> >> I took more time to look at this, to summarize my remarks on the other >> patches >> >> I think we don't need properties. The info in the msi_desc can come >> from the pci_dev which we have easy access to. This seems overall >> clearer > > I fixed that now. So much for the theory. dev->msi[x]_enabled are set after everything is set up. Some of the places are part of the setup... /me goes back to stare
--- a/drivers/pci/msi/msi.c +++ b/drivers/pci/msi/msi.c @@ -244,6 +244,8 @@ static void free_msi_irqs(struct pci_dev iounmap(dev->msix_base); dev->msix_base = NULL; } + + msi_device_set_properties(&dev->dev, 0); } static void pci_intx_for_msi(struct pci_dev *dev, int enable) @@ -341,6 +343,7 @@ msi_setup_entry(struct pci_dev *dev, int { struct irq_affinity_desc *masks = NULL; struct msi_desc *entry; + unsigned long prop; u16 control; if (affd) @@ -372,6 +375,10 @@ msi_setup_entry(struct pci_dev *dev, int if (entry->pci.msi_attrib.can_mask) pci_read_config_dword(dev, entry->pci.mask_pos, &entry->pci.msi_mask); + prop = MSI_PROP_PCI_MSI; + if (entry->pci.msi_attrib.is_64) + prop |= MSI_PROP_64BIT; + msi_device_set_properties(&dev->dev, prop); out: kfree(masks); return entry; @@ -514,6 +521,7 @@ static int msix_setup_entries(struct pci if (masks) curmsk++; } + msi_device_set_properties(&dev->dev, MSI_PROP_PCI_MSIX | MSI_PROP_64BIT); ret = 0; out: kfree(masks);
Store the properties which are interesting for various places so the MSI descriptor fiddling can be removed. Signed-off-by: Thomas Gleixner <tglx@linutronix.de> --- V2: Use the setter function --- drivers/pci/msi/msi.c | 8 ++++++++ 1 file changed, 8 insertions(+)