On Fri, Jun 26, 2015 at 01:36:23PM -0400, John Snow wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA256
>
>
>
> On 06/26/2015 12:11 PM, Stefan Hajnoczi wrote:
> > On Mon, Jun 22, 2015 at 08:21:15PM -0400, John Snow wrote:
> >> @@ -682,19 +680,22 @@ static void ahci_write_fis_sdb(AHCIState
> >> *s, int port, uint32_t finished)
> >>
> >> sdb_fis->type = SATA_FIS_TYPE_SDB; /* Interrupt pending &
> >> Notification bit */ - sdb_fis->flags =
> >> (ad->hba->control_regs.irqstatus ? (1 << 6) : 0); +
> >> sdb_fis->flags = 0x40; /* Interrupt bit, always 1 for NCQ */
> > ...
> >> - ahci_trigger_irq(s, ad, PORT_IRQ_SDB_FIS); + if
> >> (sdb_fis->flags & 0x40) { + ahci_trigger_irq(s, ad,
> >> PORT_IRQ_SDB_FIS); + }
> >
> > This if statement is always true.
> >
>
> Yes. I did that on purpose. Maybe that was too weird.
>
> The idea was to emphasize that the IRQ is definitely only triggered
> *IF* the interrupt bit is set. It just so happens that we currently
> always set it.
>
> The generation and trigger mechanics are supposed to be separate, but
> we currently have no use case for them being separate (because we are
> an omniscient HBA-HDD amalgamation), so they're mushed together here.
>
> This is kind of like a documentation "NB."
>
> I can replace it with:
>
> /* Trigger IRQ if interrupt bit is set, which currently it always is: */
> ahci_trigger_irq(...);Good idea, that makes it clear.
pgpjdlTxwot0M.pgp
Description: PGP signature
