On 04/17/2018 11:39 AM, Paolo Bonzini wrote:
> Now that end_transfer_func is a tail call in ahci_start_transfer,
> formalize the fact that the callback (of which ahci_start_transfer is
> the sole implementation) takes care of the transfer too: rename it to
> pio_transfer and, if it is present, call the end_transfer_func as soon
> as it returns.
>
> Signed-off-by: Paolo Bonzini <pbonz...@redhat.com>
> ---
> hw/ide/ahci.c | 15 +++++++--------
> hw/ide/core.c | 8 +++++---
> hw/ide/trace-events | 2 +-
> include/hw/ide/internal.h | 2 +-
> 4 files changed, 14 insertions(+), 13 deletions(-)
>
> diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
> index 45ce195fb8..10bcc65308 100644
> --- a/hw/ide/ahci.c
> +++ b/hw/ide/ahci.c
> @@ -1280,8 +1280,8 @@ out:
> return 0;
> }
>
> -/* DMA dev <-> ram */
> -static void ahci_start_transfer(IDEDMA *dma)
> +/* Transfer PIO data between RAM and device */
> +static void ahci_pio_transfer(IDEDMA *dma)
> {
> AHCIDevice *ad = DO_UPCAST(AHCIDevice, dma, dma);
> IDEState *s = &ad->port.ifs[0];
> @@ -1304,9 +1304,9 @@ static void ahci_start_transfer(IDEDMA *dma)
> has_sglist = 1;
> }
>
> - trace_ahci_start_transfer(ad->hba, ad->port_no, is_write ? "writ" :
> "read",
> - size, is_atapi ? "atapi" : "ata",
> - has_sglist ? "" : "o");
> + trace_ahci_pio_transfer(ad->hba, ad->port_no, is_write ? "writ" : "read",
> + size, is_atapi ? "atapi" : "ata",
> + has_sglist ? "" : "o");
>
> if (has_sglist && size) {
> if (is_write) {
> @@ -1319,12 +1319,11 @@ static void ahci_start_transfer(IDEDMA *dma)
> /* Update number of transferred bytes, destroy sglist */
> dma_buf_commit(s, size);
> fis_len = le32_to_cpu(ad->cur_cmd->status);
> +
> out:
> /* declare that we processed everything */
> s->data_ptr = s->data_end;
> ahci_write_fis_pio(ad, fis_len);
> -
> - s->end_transfer_func(s);
> }
>
> static void ahci_start_dma(IDEDMA *dma, IDEState *s,
> @@ -1440,7 +1439,7 @@ static const IDEDMAOps ahci_dma_ops = {
> .start_dma = ahci_start_dma,
> .restart = ahci_restart,
> .restart_dma = ahci_restart_dma,
> - .start_transfer = ahci_start_transfer,
> + .pio_transfer = ahci_pio_transfer,
> .prepare_buf = ahci_dma_prepare_buf,
> .commit_buf = ahci_commit_buf,
> .rw_buf = ahci_dma_rw_buf,
> diff --git a/hw/ide/core.c b/hw/ide/core.c
> index 866c659498..7932b7c069 100644
> --- a/hw/ide/core.c
> +++ b/hw/ide/core.c
> @@ -527,16 +527,18 @@ static void ide_clear_retry(IDEState *s)
> void ide_transfer_start(IDEState *s, uint8_t *buf, int size,
> EndTransferFunc *end_transfer_func)
> {
> - s->end_transfer_func = end_transfer_func;
> s->data_ptr = buf;
> s->data_end = buf + size;
> ide_set_retry(s);
> if (!(s->status & ERR_STAT)) {
> s->status |= DRQ_STAT;
> }
> - if (s->bus->dma->ops->start_transfer) {
> - s->bus->dma->ops->start_transfer(s->bus->dma);
> + if (!s->bus->dma->ops->pio_transfer) {
> + s->end_transfer_func = end_transfer_func;
> + return;
> }
> + s->bus->dma->ops->pio_transfer(s->bus->dma);
> + end_transfer_func(s);
Does not setting s->end_transfer_func mess with some of our dumb hacks
in e.g. ide_restart_bh or ide_is_pio_out?
