From mboxrd@z Thu Jan 1 00:00:00 1970 From: Hans de Goede Subject: Re: [PATCH 2/5] ata: libahci_platform: Support per-port interrupts Date: Fri, 22 Feb 2019 17:10:02 +0100 Message-ID: <09a58952-ca34-cbfb-d0a9-d6c9f8afcb8b@redhat.com> References: <20190222145356.23072-1-miquel.raynal@bootlin.com> <20190222145356.23072-3-miquel.raynal@bootlin.com> <5040d4cf-7da1-eefa-a8a3-bc4054d528da@redhat.com> <20190222163114.0f4488fe@xps13> <20190222170348.12e45f42@xps13> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20190222170348.12e45f42@xps13> Content-Language: en-US List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=m.gmane.org@lists.infradead.org To: Miquel Raynal Cc: Mark Rutland , Andrew Lunn , Jason Cooper , devicetree@vger.kernel.org, Marc Zyngier , Gregory Clement , Maxime Chevallier , Nadav Haklai , linux-ide@vger.kernel.org, Rob Herring , Antoine Tenart , Jens Axboe , Thomas Petazzoni , Thomas Gleixner , linux-arm-kernel@lists.infradead.org, Sebastian Hesselbarth List-Id: devicetree@vger.kernel.org Hi, On 2/22/19 5:03 PM, Miquel Raynal wrote: > Hi Hans, > > Hans de Goede 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 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 >>>>> --- >>>>> 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? Regards, Hans