On 2/22/19 9:10 AM, Hans de Goede wrote: > Hi, > > On 2/22/19 5:03 PM, Miquel Raynal wrote: >> Hi Hans, >> >> Hans de Goede <hdegoede@xxxxxxxxxx> wrote on Fri, 22 Feb 2019 16:52:55 >> +0100: >> >>> Hi, >>> >>> On 2/22/19 4:31 PM, Miquel Raynal wrote: >>>> Hi Hans, >>>> >>>> Hans de Goede <hdegoede@xxxxxxxxxx> wrote on Fri, 22 Feb 2019 16:26:01 >>>> +0100: >>>> >>>>> Hi, >>>>> >>>>> On 2/22/19 3:53 PM, Miquel Raynal wrote: >>>>>> Right now the ATA core only allows IPs to use a single interrupt. Some >>>>>> of them (for instance the Armada-CP110 one) actually has one interrupt >>>>>> per port. Add some logic to support such situation. >>>>>> >>>>>> We consider that either there is one single interrupt declared in the >>>>>> main IP node, or there are per-port interrupts, each of them being >>>>>> declared in the port sub-nodes. >>>>>> >>>>>> Signed-off-by: Miquel Raynal <miquel.raynal@xxxxxxxxxxx> >>>>>> --- >>>>>> drivers/ata/acard-ahci.c | 2 +- >>>>>> drivers/ata/ahci.c | 2 +- >>>>>> drivers/ata/ahci.h | 3 +- >>>>>> drivers/ata/libahci.c | 2 +- >>>>>> drivers/ata/libahci_platform.c | 66 ++++++++++++++++++++++++++++------ >>>>>> drivers/ata/sata_highbank.c | 2 +- >>>>>> 6 files changed, 61 insertions(+), 16 deletions(-) >>>>>> >>>>>> diff --git a/drivers/ata/acard-ahci.c b/drivers/ata/acard-ahci.c >>>>>> index 583e366be7e2..9414b81e994c 100644 >>>>>> --- a/drivers/ata/acard-ahci.c >>>>>> +++ b/drivers/ata/acard-ahci.c >>>>>> @@ -434,7 +434,7 @@ static int acard_ahci_init_one(struct pci_dev *pdev, const struct pci_device_id >>>>>> if (!hpriv) >>>>>> return -ENOMEM; >>>>>> > - hpriv->irq = pdev->irq; >>>>>> + hpriv->irqs[0] = pdev->irq; >>>>>> hpriv->flags |= (unsigned long)pi.private_data; >>>>>> >> What code-path is going to alloc hpriv->irqs for drivers using this code-path >>>>> which are not using libahci_platform .c ? >>>> >>>> I don't understand the question (or the remark behind the question), >>>> can you explain a little bit more what you have in mind? >>> >>> Sorry I got the code context wrong I meant to put that comment below this chunk: >>> >>> > diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c >>> > index 021ce46e2e57..18bce556d85f 100644 >>> > --- a/drivers/ata/ahci.c >>> > +++ b/drivers/ata/ahci.c >>> > @@ -1817,7 +1817,7 @@ static int ahci_init_one(struct pci_dev *pdev, const struct pci_device_id *ent) >>> > /* legacy intx interrupts */ >>> > pci_intx(pdev, 1); >>> > } >>> > - hpriv->irq = pci_irq_vector(pdev, 0); >>> > + hpriv->irqs[0] = pci_irq_vector(pdev, 0); >>> > >>> > if (!(hpriv->cap & HOST_CAP_SSS) || ahci_ignore_sss) >>> > host->flags |= ATA_HOST_PARALLEL_SCAN; >>> >>> >>> Which AFAIK is a common code-path also used by ahci drivers not using >>> libahci_platform, and in that case hpriv->irqs will be NULL as nothing >>> initializes it. >> >> Oh I see. What do you prefer: >> 1/ >> * I add "irqs" besides "irq" in the structure >> * copy the value of irq in irqs[0] >> * use irqs instead of irq in the libahci_platform ? >> or >> 2/ >> * Allocated one irq there if there is none ? > > I don't really have a preference, Jens what is your take on this? Single array would be the cleanest, don't add an irqs[] beside the irq. -- Jens Axboe