On Fri, Jan 12, 2018 at 2:03 PM, Peter Maydell <peter.mayd...@linaro.org> wrote: > On 11 January 2018 at 19:30, Philippe Mathieu-Daudé <f4...@amsat.org> wrote: >> Add a dma property allowing machine creation to provide the address-space >> sdhci dma operates on. >> >> [based on a patch from Alistair Francis <alistair.fran...@xilinx.com> >> from qemu/xilinx tag xilinx-v2016.1] >> Signed-off-by: Philippe Mathieu-Daudé <f4...@amsat.org> >> Reviewed-by: Alistair Francis <alistair.fran...@xilinx.com> >> --- >> include/hw/sd/sdhci.h | 2 ++ >> hw/sd/sdhci.c | 36 +++++++++++++++++++++++------------- >> 2 files changed, 25 insertions(+), 13 deletions(-) > > This looks OK for the sysbus sdhci controller, but not for the > PCI one: > (1) the link property is meaningless for PCI, and we shouldn't > expose a meaningless thing to the end user > (2) the PCI device should surely be doing its DMA via the AS > returned by pci_get_address_space() ? (compare pci_dma_read &c > in include/hw/pci/pci.h) > > 2 is a bugfix to the existing code, of course, but if we're > going to tidy this up then the right answer to 2 may affect > how you want to handle 1.
Ok. I'll respin, thanks for your review :)