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

Philippe Mathieu-Daudé posted 42 patches 6 years, 4 months ago
Only 39 patches received!
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
[Qemu-devel] [PATCH v3 00/42] SDHCI: housekeeping, add a qtest and fix few issues
Posted by Philippe Mathieu-Daudé 6 years, 4 months ago
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'

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


Re: [Qemu-devel] [PATCH v3 00/42] SDHCI: housekeeping, add a qtest and fix few issues
Posted by Alistair Francis 6 years, 4 months ago
On Fri, Dec 29, 2017 at 9:48 AM, Philippe Mathieu-Daudé <f4bug@amsat.org> 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
>
>

Re: [Qemu-devel] [PATCH v3 00/42] SDHCI: housekeeping, add a qtest and fix few issues
Posted by Philippe Mathieu-Daudé 6 years, 4 months ago
On 01/02/2018 09:15 PM, Alistair Francis wrote:
> 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.

Andreas or Eduardo could you help reviewing patches 4-7 please? These
are QOM-related changes.

Thanks!

Phil.