On 24.03.2016 14:23, Paolo Bonzini wrote:
On 24/03/2016 10:24, Pavel Butsykin wrote:
For extra points, make ide_handle_rw_error convert IDE_DMA_* to
IDE_RETRY_* so that the callers only need to pass in IDE_RETRY_DMA
You mean to do something like that:
ide_handle_rw_error(s, -ret, s->dma_cmd | IDE_RETRY_DMA)
Just ide_handle_rw_error(s, -ret, IDE_RETRY_DMA)
and in ide_handle_rw_error
if (op == IDE_RETRY_DMA) {
if (s->dma_cmd == IDE_DMA_READ)
op |= IDE_RETRY_READ;
else if (s->dma_cmd == IDE_DMA_TRIM)
op |= IDE_RETRY_TRIM;
}
But I'm not sure anymore this is a good idea.
Look at the ide_sector_read_cb function. The ->dma_cmd field is used
only for the DMA, we can't use the ide_cmd_is_read macro for PIO or
ATAPI. In fact, the ide_handle_rw_error looks like a generic function
and I do not see any good reason to move the handling of the dma_cmd
field(especially in the context of bug fixes).
However, if it works, using dma_cmd would be better than using the dma_cb.
We can't use the dma_cmd field to build the ATAPI ->error_status. Your
proposal is quite logical, but please look at the 3rd patch, there to
build ->error_status we can use the same dma_cb, it is very convenient.
Paolo
