netdev - Re: [PATCH 16/17] mfd: Add support for LAN966x PCI device

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20240514145518.3e989b83@bootlin.com>
Date: Tue, 14 May 2024 14:55:18 +0200
From: Herve Codina <herve.codina@...tlin.com>
To: <Steen.Hegelund@...rochip.com>
Cc: <tglx@...utronix.de>, <robh@...nel.org>, <krzk+dt@...nel.org>,
 <conor+dt@...nel.org>, <davem@...emloft.net>, <edumazet@...gle.com>,
 <kuba@...nel.org>, <pabeni@...hat.com>, <lee@...nel.org>, <arnd@...db.de>,
 <Horatiu.Vultur@...rochip.com>, <UNGLinuxDriver@...rochip.com>,
 <andrew@...n.ch>, <hkallweit1@...il.com>, <linux@...linux.org.uk>,
 <saravanak@...gle.com>, <bhelgaas@...gle.com>, <p.zabel@...gutronix.de>,
 <Lars.Povlsen@...rochip.com>, <Daniel.Machon@...rochip.com>,
 <alexandre.belloni@...tlin.com>, <linux-kernel@...r.kernel.org>,
 <devicetree@...r.kernel.org>, <netdev@...r.kernel.org>,
 <linux-pci@...r.kernel.org>, <linux-arm-kernel@...ts.infradead.org>,
 <Allan.Nielsen@...rochip.com>, <luca.ceresoli@...tlin.com>,
 <thomas.petazzoni@...tlin.com>
Subject: Re: [PATCH 16/17] mfd: Add support for LAN966x PCI device

Hi Steen,

On Wed, 8 May 2024 08:20:04 +0000
<Steen.Hegelund@...rochip.com> 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é