On Thu, Mar 24, 2022 at 02:05:39PM +0000, Jonathan Cameron wrote:
> Hi Ira,
>
> > Here is the code to be more clear...
> >
> >
> > drivers/cxl/pci.c:
> >
> > int cxl_pci_create_doe_devices(struct pci_dev *pdev)
> > {
> > struct device *dev = &pdev->dev;
> > bool use_irq = true;
> > int irqs = 0;
> > u16 off = 0;
> > int rc;
> >
> > pci_doe_for_each_off(pdev, off)
> > irqs++;
> > pci_info(pdev, "Found %d DOE mailbox's\n", irqs);
> >
> > /*
> > * Allocate enough vectors for the DOE's
> > */
> > rc = pci_alloc_irq_vectors(pdev, irqs, irqs, PCI_IRQ_MSI |
> > PCI_IRQ_MSIX);
> > if (rc != irqs) {
> > pci_err(pdev, "Not enough interrupts for all the DOEs; use polling\n");
> > use_irq = false;
> > /* Some got allocated; clean them up */
> > if (rc > 0)
> > cxl_pci_free_irq_vectors(pdev);
> > } else {
> > /*
> > * Enabling bus mastering is require for MSI/MSIx. It could be
> > * done later within the DOE initialization, but as it
> > * potentially has other impacts keep it here when setting up
> > * the IRQ's.
> > */
> > pci_set_master(pdev);
> > rc = devm_add_action_or_reset(dev,
> > cxl_pci_free_irq_vectors,
> > pdev);
> > if (rc)
> > return rc;
> > }
> >
> > pci_doe_for_each_off(pdev, off) {
> > ...
> > /* Create each auxiliary device which internally calls */
> > pci_doe_create_mb(pdev, off, use_irq);
> > ...
> > }
> > ...
> > }
> >
> >
> > drivers/pci/pci-doe.c:
> >
> > static irqreturn_t pci_doe_irq_handler(int irq, void *data)
> > {
> > ...
> > }
> >
> > static int pci_doe_request_irq(struct pci_doe_mb *doe_mb)
> > {
> > struct pci_dev *pdev = doe_mb->pdev;
> > int offset = doe_mb->cap_offset;
> > int doe_irq, rc;
> > u32 val;
> >
> > pci_read_config_dword(pdev, offset + PCI_DOE_CAP, &val);
> >
> > if (!FIELD_GET(PCI_DOE_CAP_INT, val))
> > return -ENOTSUPP;
> >
> > doe_irq = FIELD_GET(PCI_DOE_CAP_IRQ, val);
> > rc = pci_request_irq(pdev, doe_irq, pci_doe_irq_handler,
> > NULL, doe_mb,
> > "DOE[%d:%s]", doe_irq, pci_name(pdev));
> > if (rc)
> > return rc;
> >
> > doe_mb->irq = doe_irq;
> > pci_write_config_dword(pdev, offset + PCI_DOE_CTRL,
> > PCI_DOE_CTRL_INT_EN);
> > return 0;
> > }
> >
> > struct pci_doe_mb *pci_doe_create_mb(struct pci_dev *pdev, u16 cap_offset,
> > bool use_irq)
> > {
> > ...
> > if (use_irq) {
> > rc = pci_doe_request_irq(doe_mb);
> > if (rc)
> > pci_err(pdev, "DOE request irq failed for mailbox @ %u : %d\n",
> > cap_offset, rc);
> > }
> > ...
> > }
> >
> >
> > Does this look reasonable?
>
> I'm a little nervous about how we are going to make DOEs on switches work.
> Guess I'll do an experiment once your next version is out and check we
> can do that reasonably cleanly. For switches we'll probably have to
> check for DOEs on all such ports and end up with infrastructure to
> map to all protocols we might see on a switch.
Are the switches not represented as PCI devices in linux?
If my vision of switches is correct I think that problem is independent of what
I'm solving here. In other words the relationship between a port on a switch
and a DOE capability on that switch will have to be established somehow and
nothing I'm doing precludes doing that, but at the same time nothing I'm doing
helps that either.
Ira
>
> Jonathan
>
> >
