On 20/03/2018 22:46, John Snow wrote:
>> }
>> - if (s->bus->dma->ops->start_transfer) {
>> - s->bus->dma->ops->start_transfer(s->bus->dma);
>> + if (!s->bus->dma->ops->start_transfer) {
>> + s->end_transfer_func = end_transfer_func;
>> + return;
>> }
>> + s->bus->dma->ops->start_transfer(s->bus->dma);
>> + end_transfer_func(s);
> wow, this makes me really uneasy to look at. The assumption now is: "If
> you have a start_transfer method, that method if successful implies that
> the transfer is *COMPLETED*."
>
> It's implicit here, but I think anyone but the two of us would probably
> not understand that implication. (Or me in three months.) What can we do
> about that? Since AHCI is the only interface that uses this callback, I
> wonder if we can't just rename it to something more obvious.
You are completely right, it should be renamed to pio_transfer.
> Under normal circumstances this function really does just "start" a
> transfer and it's not obvious from context that some adapters have
> synchronous methods to finish the transfer without further PIO pingpong
> with the guest.
Yeah, though it is already doing it---the patch is only unhiding the
mutual recursion by pulling it one level up.
Paolo
