diff mbox series

[V2,19/36] PCI/MSI: Store properties in device::msi::data

Message ID 20211206210438.688216619@linutronix.de (mailing list archive)
State Superseded
Headers show
Series genirq/msi, PCI/MSI: Spring cleaning - Part 2 | expand

Commit Message

Thomas Gleixner Dec. 6, 2021, 10:39 p.m. UTC
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(+)

Comments

Greg KH Dec. 7, 2021, 7:52 a.m. UTC | #1
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>
Bjorn Helgaas Dec. 7, 2021, 9:05 p.m. UTC | #2
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);
>
Jason Gunthorpe Dec. 8, 2021, 3:58 p.m. UTC | #3
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
Thomas Gleixner Dec. 9, 2021, 5:53 p.m. UTC | #4
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
Thomas Gleixner Dec. 9, 2021, 9:02 p.m. UTC | #5
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
diff mbox series

Patch

--- 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);