| Message ID | 20201104155237.77772-4-sven.auhagen@voleatech.de |
|---|---|
| State | New |
| Headers | show |
| Series | Armada8k enable per-port SATA interrupts and drop a hack in the IRQ subsystem | expand |
Hi, On 11/4/20 4:52 PM, sven.auhagen@voleatech.de wrote: > From: Sven Auhagen <sven.auhagen@voleatech.de> > > Disable the platform irq init in ahci init platform host > if it was initiated by a custom function. > To check for it I am using the AHCI_HFLAG_MULTI_MSI flag. > > Suggested-by: Hans de Goede <hdegoede@redhat.com> > Signed-off-by: Sven Auhagen <sven.auhagen@voleatech.de> The code is fine here. But I'm not entirely happy with the commit message how about: """ ata: libahci_platform: Do not try to get an IRQ when AHCI_HFLAG_MULTI_MSI is set When the ahci-host AHCI_HFLAG_MULTI_MSI flag is set then the driver must provide a get_irq_vector callback and take care of getting the IRQs itself. So in this case ahci_platform_init_host() should not try to get an IRQ itself. """ With the commit message updated to the above (or something similar) you may add my: Reviewed-by: Hans de Goede <hdegoede@redhat.com> I've also just given you my Reviewed-by for patch 4 and 5, and the result already has a Reviewed-by. So if you can sendout a v3 of this series with the commit message for this patch fixed, then it is ready for merging from my pov. Regards, Hans > --- > drivers/ata/libahci_platform.c | 16 +++++++++------- > 1 file changed, 9 insertions(+), 7 deletions(-) > > diff --git a/drivers/ata/libahci_platform.c b/drivers/ata/libahci_platform.c > index de638dafce21..f6f2a111d226 100644 > --- a/drivers/ata/libahci_platform.c > +++ b/drivers/ata/libahci_platform.c > @@ -581,14 +581,16 @@ int ahci_platform_init_host(struct platform_device *pdev, > struct ata_host *host; > int i, irq, n_ports, rc; > > - irq = platform_get_irq(pdev, 0); > - if (irq <= 0) { > - if (irq != -EPROBE_DEFER) > - dev_err(dev, "no irq\n"); > - return irq; > - } > + if (!(hpriv->flags & AHCI_HFLAG_MULTI_MSI)) { > + irq = platform_get_irq(pdev, 0); > + if (irq <= 0) { > + if (irq != -EPROBE_DEFER) > + dev_err(dev, "no irq\n"); > + return irq; > + } > > - hpriv->irq = irq; > + hpriv->irq = irq; > + } > > /* prepare host */ > pi.private_data = (void *)(unsigned long)hpriv->flags; >
On Mon, Nov 09, 2020 at 03:00:58PM +0100, Hans de Goede wrote: > Hi, > > On 11/4/20 4:52 PM, sven.auhagen@voleatech.de wrote: > > From: Sven Auhagen <sven.auhagen@voleatech.de> > > > > Disable the platform irq init in ahci init platform host > > if it was initiated by a custom function. > > To check for it I am using the AHCI_HFLAG_MULTI_MSI flag. > > > > Suggested-by: Hans de Goede <hdegoede@redhat.com> > > Signed-off-by: Sven Auhagen <sven.auhagen@voleatech.de> > > The code is fine here. But I'm not entirely happy with > the commit message how about: > > """ > ata: libahci_platform: Do not try to get an IRQ when AHCI_HFLAG_MULTI_MSI is set > > When the ahci-host AHCI_HFLAG_MULTI_MSI flag is set then the driver must provide > a get_irq_vector callback and take care of getting the IRQs itself. So in this > case ahci_platform_init_host() should not try to get an IRQ itself. > """ > > With the commit message updated to the above (or something similar) you > may add my: > > Reviewed-by: Hans de Goede <hdegoede@redhat.com> > > I've also just given you my Reviewed-by for patch 4 and 5, and the > result already has a Reviewed-by. > > So if you can sendout a v3 of this series with the commit message for > this patch fixed, then it is ready for merging from my pov. Thank you, I will change the commit message and send a v3. Best Sven > > Regards, > > Hans > > > > --- > > drivers/ata/libahci_platform.c | 16 +++++++++------- > > 1 file changed, 9 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/ata/libahci_platform.c b/drivers/ata/libahci_platform.c > > index de638dafce21..f6f2a111d226 100644 > > --- a/drivers/ata/libahci_platform.c > > +++ b/drivers/ata/libahci_platform.c > > @@ -581,14 +581,16 @@ int ahci_platform_init_host(struct platform_device *pdev, > > struct ata_host *host; > > int i, irq, n_ports, rc; > > > > - irq = platform_get_irq(pdev, 0); > > - if (irq <= 0) { > > - if (irq != -EPROBE_DEFER) > > - dev_err(dev, "no irq\n"); > > - return irq; > > - } > > + if (!(hpriv->flags & AHCI_HFLAG_MULTI_MSI)) { > > + irq = platform_get_irq(pdev, 0); > > + if (irq <= 0) { > > + if (irq != -EPROBE_DEFER) > > + dev_err(dev, "no irq\n"); > > + return irq; > > + } > > > > - hpriv->irq = irq; > > + hpriv->irq = irq; > > + } > > > > /* prepare host */ > > pi.private_data = (void *)(unsigned long)hpriv->flags; > > >
diff --git a/drivers/ata/libahci_platform.c b/drivers/ata/libahci_platform.c index de638dafce21..f6f2a111d226 100644 --- a/drivers/ata/libahci_platform.c +++ b/drivers/ata/libahci_platform.c @@ -581,14 +581,16 @@ int ahci_platform_init_host(struct platform_device *pdev, struct ata_host *host; int i, irq, n_ports, rc; - irq = platform_get_irq(pdev, 0); - if (irq <= 0) { - if (irq != -EPROBE_DEFER) - dev_err(dev, "no irq\n"); - return irq; - } + if (!(hpriv->flags & AHCI_HFLAG_MULTI_MSI)) { + irq = platform_get_irq(pdev, 0); + if (irq <= 0) { + if (irq != -EPROBE_DEFER) + dev_err(dev, "no irq\n"); + return irq; + } - hpriv->irq = irq; + hpriv->irq = irq; + } /* prepare host */ pi.private_data = (void *)(unsigned long)hpriv->flags;