Re: [Qemu-devel] [PATCH v9 00/16] SDHCI: clean v1/v2 Specs (part 2)

[Alistair Francis]

On Wed, Jan 31, 2018 at 6:09 AM, Philippe Mathieu-Daudé <address@hidden> wrote:
> ping?
>
> Patches missing review: 2 (PCI qtesting),11,13,15

Sorry about the slowness. Besides 2 they should be all reviewed now,
let me know if anything else needs to be done.

As for 2 I don't have much experience with Qtest, I'll try to go over
it, but ideally someone who knows Qtest should look at it.

Thanks,
Alistair

>
> On 01/22/2018 11:08 PM, Philippe Mathieu-Daudé wrote:
>> Since v8:
>> - we keep the 'capareg' property, this simplify a lot the series
>> - the Zynq 7000 uses the datasheet CAPAREG
>> - reset R-b/A-b
>>
>> Since v7:
>> - use error_propagate()
>>
>> Since v6:
>> - rebased on upstream to use DEFINE_SDHCI_COMMON_PROPERTIES
>> - included qtests back
>> - add PCI qtests
>> - series was getting big, splitting again; next one is "implement Spec v3"
>> - following Alistair advice, new properties default to current
>>   SDHC_CAPAB_REG_DEFAULT (probably 'default' for historical reason, being
>>   the first capabilities set). This leads to weird properties imho, i.e.
>>   defaulting 'max-frequency' to 52 MHz, the Exynos SoC has to set this
>>   property to 0.
>> - addressed Alistair reviews
>> - confirmed the Exynos4210 specs after chatting on IRC with Krzysztof
>>   Kozlowski who kindly verified in the datasheet.
>>
>> Since v5:
>> - addressed Alistair reviews
>> - dropped "abstract generic-sdhci"
>> - dropped Linux Device Tree names
>> - split qtests in another series
>> - change the bcm2835 minimum blocksize to 1KB (Andrew Baumann)
>> - added Alistair R-b
>> - based on Alistair work:
>>   - add SD tunning sequence via Host Control 2 to use UHS-I cards
>>   - add CMD/DAT[] fields in the Present State (used in next series
>>     to switch card voltage)
>>
>> based on Alistair work:
>> - add SD tunning sequence via Host Control 2 to use UHS-I cards
>> - add CMD/DAT[] fields in the Present State (used in next series
>>   to switch card voltage)
>>
>> Since v4 ("SDHCI: add qtests and fix few issues"):
>> - spec_version default to v2 (current behaviour)
>> - addressed Alistair review (no v1, tell user about valid version)
>>
>> Since v3:
>> - no change, but split back in 2 series, 1st part is "SDHCI: housekeeping 
>> v5",
>>
>> 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
>>
>> 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 with v8
>> 001/16:[----] [--] 'sdhci: use error_propagate(local_err) in realize()'
>> 002/16:[0004] [FC] 'sdhci: add qtest to check the SD capabilities register'
>> 003/16:[----] [--] 'sdhci: add check_capab_readonly() qtest'
>> 004/16:[----] [--] 'sdhci: add a check_capab_baseclock() qtest'
>> 005/16:[----] [--] 'sdhci: add a check_capab_sdma() qtest'
>> 006/16:[----] [--] 'sdhci: add qtest to check the SD Spec version'
>> 007/16:[0022] [FC] 'sdhci: add a 'spec_version property' (default to v2)'
>> 008/16:[down] 'sdhci: use a numeric value for the default CAPAB register'
>> 009/16:[down] 'sdhci: simplify sdhci_get_fifolen()'
>> 010/16:[down] 'sdhci: check the Spec v1 capabilities correctness'
>> 011/16:[----] [--] 'sdhci: add BLOCK_SIZE_MASK for DMA'
>> 012/16:[----] [--] 'sdhci: Fix 64-bit ADMA2'
>> 013/16:[down] 'sdhci: check Spec v2 capabilities (DMA and 64-bit bus)'
>> 014/16:[down] 'hw/arm/exynos4210: access the 64-bit capareg with 
>> qdev_prop_set_uint64()'
>> 015/16:[down] 'hw/arm/exynos4210: add a comment about a very similar SDHCI 
>> (Spec. v2)'
>> 016/16:[down] 'hw/arm/xilinx_zynq: fix the capabilities register to match 
>> the datasheet'
>>
>> Philippe Mathieu-Daudé (15):
>>   sdhci: use error_propagate(local_err) in realize()
>>   sdhci: add qtest to check the SD capabilities register
>>   sdhci: add check_capab_readonly() qtest
>>   sdhci: add a check_capab_baseclock() qtest
>>   sdhci: add a check_capab_sdma() qtest
>>   sdhci: add qtest to check the SD Spec version
>>   sdhci: add a 'spec_version property' (default to v2)
>>   sdhci: use a numeric value for the default CAPAB register
>>   sdhci: simplify sdhci_get_fifolen()
>>   sdhci: check the Spec v1 capabilities correctness
>>   sdhci: add BLOCK_SIZE_MASK for DMA
>>   sdhci: check Spec v2 capabilities (DMA and 64-bit bus)
>>   hw/arm/exynos4210: access the 64-bit capareg with qdev_prop_set_uint64()
>>   hw/arm/exynos4210: add a comment about a very similar SDHCI (Spec. v2)
>>   hw/arm/xilinx_zynq: fix the capabilities register to match the datasheet
>>
>> Sai Pavan Boddu (1):
>>   sdhci: Fix 64-bit ADMA2
>>
>>  include/hw/sd/sdhci.h  |   2 +
>>  hw/sd/sdhci-internal.h |  43 ++++++---
>>  hw/arm/exynos4210.c    |  14 ++-
>>  hw/arm/xilinx_zynq.c   |  53 +++++-----
>>  hw/sd/sdhci.c          | 257 
>> ++++++++++++++++++++++++++++++++-----------------
>>  tests/sdhci-test.c     | 216 +++++++++++++++++++++++++++++++++++++++++
>>  hw/sd/trace-events     |   1 +
>>  tests/Makefile.include |   3 +
>>  8 files changed, 464 insertions(+), 125 deletions(-)
>>  create mode 100644 tests/sdhci-test.c
>>
>