Re: [Qemu-devel] [PATCH v3 00/42] SDHCI: housekeeping, add a qtest and fix few issues

[Alistair Francis]

On Fri, Dec 29, 2017 at 9:48 AM, Philippe Mathieu-Daudé <address@hidden> wrote:
> Since v2:
> - more detailed 'capabilities', all boards converted to use these properties
> - since all qtests pass, removed the previous 'capareg' property
> - added Stefan/Alistair R-b
> - corrected 'access' LED behavior (Alistair's review)
> - more uses of the registerfields API
> - remove some dead code
> - cosmetix:
>   - added more comments
>   - renamed a pair of registers
>   - reordered few struct members
>
> Note, the bcm2835 seems to have 1KB minimum blocksize, however the current
> model is implemented with 512B. I didn't change the current value.
>
> Since v1:
> - addressed Alistair Francis review comments, added some R-b
> - only move register defines to "sd-internal.h"
> - fixed deposit64() arguments
> - dropped unuseful s->fifo_buffer = NULL
> - use a qemu_irq for the LED, restrict the logging to ON/OFF
> - fixed a trace format string error
> - included Andrey Smirnov ACMD12ERRSTS write patch
> - dropped few unuseful patches, and separate the Python polemical ones for 
> later
>
> From the "SDHCI housekeeping" series:
> - 1: we restrict part of "sd/sd.h" into local "sd-internal.h",
> - 2,3: we somehow beautiful the code, no logical changes,
> - 4-7: we refactor the common sysbus/pci qdev code,
> - 8-10: we add plenty of trace events which will result useful later,
> - 11: we finally expose a "dma-memory" property.
> From the "SDHCI: add a qtest and fix few issues" series:
> - 12,13: fix registers
> - 14,15: boards can specify which SDHCI Spec to use (v2 and v3 so far)
> - 15-20: HCI qtest
>
> Regards,
>
> Phil.
>
> $ git backport-diff
> [----] : patches are identical
> [####] : number of functional differences between upstream/downstream patch
> [down] : patch is downstream-only
> The flags [FC] indicate (F)unctional and (C)ontextual differences, 
> respectively
>
> 001/42:[0003] [FC] 'sdhci: clean up includes'
> 002/42:[down] 'sdhci: sort registers comments'
> 003/42:[down] 'sdhci: remove dead code'
> 004/42:[----] [--] 'sdhci: refactor same sysbus/pci properties'
> 005/42:[----] [--] 'sdhci: refactor common sysbus/pci class_init()'
> 006/42:[----] [--] 'sdhci: refactor common sysbus/pci realize()'
> 007/42:[0001] [FC] 'sdhci: refactor common sysbus/pci unrealize()'
> 008/42:[----] [--] 'sdhci: use qemu_log_mask(UNIMP) instead of fprintf()'
> 009/42:[----] [--] 'sdhci: convert the DPRINT() calls into trace events'
> 010/42:[down] 'sdhci: add a GPIO for the 'access control' LED'
> 011/42:[----] [--] 'sdhci: move MASK_TRNMOD with other SDHC_TRN* defines'
> 012/42:[down] 'sdhci: use FIELD_DP32() macro for the WRITE_PROTECT flag'
> 013/42:[down] 'sdhci: rename the SDHC_CAPAB register'
> 014/42:[0008] [FC] 'sdhci: fix CAPAB/MAXCURR registers, 64bit and read-only'
> 015/42:[----] [-C] 'sdhci: Implement write method of ACMD12ERRSTS register'
> 016/42:[down] 'sdhci: use deposit64() on admasysaddr'
> 017/42:[----] [-C] 'sdhci: add a "dma-memory" property'
> 018/42:[down] 'sdhci: add a spec_version property'
> 019/42:[down] 'sdhci: add basic Spec v1 capabilities'
> 020/42:[down] 'sdhci: add max-block-length capability (Spec v1)'
> 021/42:[down] 'sdhci: add clock capabilities (Spec v1)'
> 022/42:[down] 'sdhci: add DMA and 64-bit capabilities (Spec v2)'
> 023/42:[down] 'sdhci: default to Spec v2'
> 024/42:[down] 'sdhci: add a 'dma' shortcut property'
> 025/42:[down] 'sdhci: add BLOCK_SIZE_MASK for DMA'
> 026/42:[down] 'sdhci: Fix 64-bit ADMA2'
> 027/42:[down] 'hw/arm/exynos4210: implement SDHCI Spec v2'
> 028/42:[down] 'hw/arm/xilinx_zynq: implement SDHCI Spec v2'
> 029/42:[0022] [FC] 'sdhci: add qtest to check the SD Spec version'
> 030/42:[down] 'sdhci: check Spec v2 capabilities qtest'
> 031/42:[down] 'sdhci: add v3 capabilities'
> 032/42:[down] 'sdhci: rename the hostctl1 register'
> 033/42:[down] 'hw/arm/bcm2835_peripherals: implement SDHCI Spec v3'
> 034/42:[down] 'hw/arm/fsl-imx6: implement SDHCI Spec v3'
> 035/42:[down] 'hw/arm/xilinx_zynqmp: implement SDHCI Spec v3'
> 036/42:[down] 'sdhci: check Spec v3 capabilities qtest'
> 037/42:[down] 'sdhci: remove the deprecated 'capareg' property'
> 038/42:[0008] [FC] 'sdhci: add check_capab_readonly() qtest'
> 039/42:[0009] [FC] 'sdhci: add a check_capab_baseclock() qtest'
> 040/42:[0011] [FC] 'sdhci: add a check_capab_sdma() qtest'
> 041/42:[----] [-C] 'sdhci: add a check_capab_v3() qtest'
> 042/42:[down] 'sdhci: add Spec v4.2 register definitions'

We are making progress here. Do you think it would be worth splitting
out the earlier patches that have been reviewed so far so they can be
merged? A 42 patch series it pretty daunting and most of the first 20
are pretty straightforward and have been reviewed.

Alistair

>
> Andrey Smirnov (1):
>   sdhci: Implement write method of ACMD12ERRSTS register
>
> Philippe Mathieu-Daudé (40):
>   sdhci: clean up includes
>   sdhci: sort registers comments
>   sdhci: remove dead code
>   sdhci: refactor same sysbus/pci properties into a common one
>   sdhci: refactor common sysbus/pci class_init() into sdhci_class_init()
>   sdhci: refactor common sysbus/pci realize() into sdhci_realizefn()
>   sdhci: refactor common sysbus/pci unrealize() into sdhci_unrealizefn()
>   sdhci: use qemu_log_mask(UNIMP) instead of fprintf()
>   sdhci: convert the DPRINT() calls into trace events
>   sdhci: add a GPIO for the 'access control' LED
>   sdhci: move MASK_TRNMOD with other SDHC_TRN* defines in "sd-internal.h"
>   sdhci: use FIELD_DP32() macro for the WRITE_PROTECT flag
>   sdhci: rename the SDHC_CAPAB register
>   sdhci: fix CAPAB/MAXCURR registers, both are 64bit and read-only
>   sdhci: use deposit64() on admasysaddr
>   sdhci: add a "dma-memory" property
>   sdhci: add a spec_version property
>   sdhci: add basic Spec v1 capabilities
>   sdhci: add max-block-length capability (Spec v1)
>   sdhci: add clock capabilities (Spec v1)
>   sdhci: add DMA and 64-bit capabilities (Spec v2)
>   sdhci: default to Spec v2
>   sdhci: add a 'dma' shortcut property
>   sdhci: add BLOCK_SIZE_MASK for DMA
>   hw/arm/exynos4210: implement SDHCI Spec v2
>   hw/arm/xilinx_zynq: implement SDHCI Spec v2
>   sdhci: add qtest to check the SD Spec version
>   sdhci: check Spec v2 capabilities qtest
>   sdhci: add v3 capabilities
>   sdhci: rename the hostctl1 register
>   hw/arm/bcm2835_peripherals: implement SDHCI Spec v3
>   hw/arm/fsl-imx6: implement SDHCI Spec v3
>   hw/arm/xilinx_zynqmp: implement SDHCI Spec v3
>   sdhci: check Spec v3 capabilities qtest
>   sdhci: remove the deprecated 'capareg' property
>   sdhci: add check_capab_readonly() qtest
>   sdhci: add a check_capab_baseclock() qtest
>   sdhci: add a check_capab_sdma() qtest
>   sdhci: add a check_capab_v3() qtest
>   sdhci: add Spec v4.2 register definitions
>
> Sai Pavan Boddu (1):
>   sdhci: Fix 64-bit ADMA2
>
>  include/hw/sd/sdhci.h        |  59 ++++-
>  hw/sd/sdhci-internal.h       |  81 +++++--
>  hw/arm/bcm2835_peripherals.c |  35 ++-
>  hw/arm/exynos4210.c          |  13 +-
>  hw/arm/fsl-imx6.c            |  12 +
>  hw/arm/xilinx_zynq.c         |  64 ++++--
>  hw/arm/xlnx-zynqmp.c         |  38 +++-
>  hw/sd/sdhci.c                | 513 
> +++++++++++++++++++++++++------------------
>  tests/sdhci-test.c           | 177 +++++++++++++++
>  hw/sd/trace-events           |  15 ++
>  tests/Makefile.include       |   3 +
>  11 files changed, 718 insertions(+), 292 deletions(-)
>  create mode 100644 tests/sdhci-test.c
>
> --
> 2.15.1
>
>