Hi Steen,
On Wed, 8 May 2024 08:20:04 +0000
<Steen.Hegelund@xxxxxxxxxxxxx> wrote:
...
> > +
> > +static irqreturn_t pci_dev_irq_handler(int irq, void *data)
> > +{
> > + struct pci_dev_intr_ctrl *intr_ctrl = data;
> > + int ret;
> > +
> > + ret = generic_handle_domain_irq(intr_ctrl->irq_domain, 0);
> > + return ret ? IRQ_NONE : IRQ_HANDLED;
> > +}
> > +
> > +static struct pci_dev_intr_ctrl *pci_dev_create_intr_ctrl(struct pci_dev *pdev)
> > +{
> > + struct pci_dev_intr_ctrl *intr_ctrl;
> > + struct fwnode_handle *fwnode;
> > + int ret;
> > +
> > + if (!pdev->irq)
> > + return ERR_PTR(-EOPNOTSUPP);
> > +
> > + fwnode = dev_fwnode(&pdev->dev);
> > + if (!fwnode)
> > + return ERR_PTR(-ENODEV);
> > +
> > + intr_ctrl = kmalloc(sizeof(*intr_ctrl), GFP_KERNEL);
> > + if (!intr_ctrl)
> > + return ERR_PTR(-ENOMEM);
> > +
> > + intr_ctrl->pci_dev = pdev;
> > +
> > + intr_ctrl->irq_domain = irq_domain_create_linear(fwnode, 1, &pci_dev_irq_domain_ops,
> > + intr_ctrl);
> > + if (!intr_ctrl->irq_domain) {
> > + pci_err(pdev, "Failed to create irqdomain\n");
> > + ret = -ENOMEM;
> > + goto err_free_intr_ctrl;
> > + }
> > +
> > + ret = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_LEGACY);
> > + if (ret < 0) {
> > + pci_err(pdev, "Unable alloc irq vector (%d)\n", ret);
> > + goto err_remove_domain;
> > + }
> > + intr_ctrl->irq = pci_irq_vector(pdev, 0);
> > + ret = request_irq(intr_ctrl->irq, pci_dev_irq_handler, IRQF_SHARED,
> > + dev_name(&pdev->dev), intr_ctrl);
> > + if (ret) {
> > + pci_err(pdev, "Unable to request irq %d (%d)\n", intr_ctrl->irq, ret);
> > + goto err_free_irq_vector;
> > + }
> > +
> > + return intr_ctrl;
> > +
> > +err_free_irq_vector:
> > + pci_free_irq_vectors(pdev);
> > +err_remove_domain:
> > + irq_domain_remove(intr_ctrl->irq_domain);
> > +err_free_intr_ctrl:
> > + kfree(intr_ctrl);
> > + return ERR_PTR(ret);
> > +}
> > +
> > +static void pci_dev_remove_intr_ctrl(struct pci_dev_intr_ctrl *intr_ctrl)
> > +{
> > + free_irq(intr_ctrl->irq, intr_ctrl);
> > + pci_free_irq_vectors(intr_ctrl->pci_dev);
> > + irq_dispose_mapping(irq_find_mapping(intr_ctrl->irq_domain, 0));
> > + irq_domain_remove(intr_ctrl->irq_domain);
> > + kfree(intr_ctrl);
> > +}
> > +
>
> It looks like the two functions below (and their helper functions) are so
> generic that they could be part of the pci driver core support.
> Any plans for that?
Indeed, I tried to write them in a generic way.
Right now, at least for the next iteration of this series, I don't plan to
move them as part of the PCI code.
This piece of code did not get any feedback and I would prefer to keep them
here for the moment.
Of course, they could be move out of the LAN966x PCI driver later.
>
> > +static void devm_pci_dev_remove_intr_ctrl(void *data)
> > +{
> > + struct pci_dev_intr_ctrl *intr_ctrl = data;
> > +
> > + pci_dev_remove_intr_ctrl(intr_ctrl);
> > +}
> > +
> > +static int devm_pci_dev_create_intr_ctrl(struct pci_dev *pdev)
> > +{
> > + struct pci_dev_intr_ctrl *intr_ctrl;
> > +
> > + intr_ctrl = pci_dev_create_intr_ctrl(pdev);
> > +
> > + if (IS_ERR(intr_ctrl))
> > + return PTR_ERR(intr_ctrl);
> > +
> > + return devm_add_action_or_reset(&pdev->dev, devm_pci_dev_remove_intr_ctrl, intr_ctrl);
> > +}
> > +
>
Best regards,
Hervé
