On Fri, Jan 12, 2018 at 9:08 PM, Alistair Francis
<alistair.fran...@xilinx.com> wrote:
> On Thu, Jan 11, 2018 at 12:56 PM, Philippe Mathieu-Daudé
> <f4...@amsat.org> wrote:
>> Signed-off-by: Philippe Mathieu-Daudé <f4...@amsat.org>
>> ---
>> hw/sd/sdhci-internal.h | 21 +++++++++++++++++++++
>> include/hw/sd/sdhci.h | 2 ++
>> hw/sd/sdhci.c | 22 ++++++++++++++++++++--
>> 3 files changed, 43 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/sd/sdhci-internal.h b/hw/sd/sdhci-internal.h
>> index 4ed9727ec3..ac4704eb61 100644
>> --- a/hw/sd/sdhci-internal.h
>> +++ b/hw/sd/sdhci-internal.h
>> @@ -43,6 +43,7 @@
>> #define SDHC_TRNS_DMA 0x0001
>> #define SDHC_TRNS_BLK_CNT_EN 0x0002
>> #define SDHC_TRNS_ACMD12 0x0004
>> +#define SDHC_TRNS_ACMD23 0x0008 /* since v3 */
>> #define SDHC_TRNS_READ 0x0010
>> #define SDHC_TRNS_MULTI 0x0020
>> #define SDHC_TRNMOD_MASK 0x0037
>> @@ -183,12 +184,23 @@ FIELD(SDHC_ACMD12ERRSTS, TIMEOUT_ERR, 1, 1);
>> FIELD(SDHC_ACMD12ERRSTS, CRC_ERR, 2, 1);
>> FIELD(SDHC_ACMD12ERRSTS, INDEX_ERR, 4, 1);
>>
>> +/* Host Control Register 2 (since v3) */
>> +#define SDHC_HOSTCTL2 0x3E
>
> Why not use the REG() macro here?
Because this is a 16-bit register, and I don't want to confuse with REG32()
Although the A_ address is correct, the R_ index only works if all
REG() are from the same size.
We should add some alignment check, I was thinking about using
_Static_assert() in the macros.
>
> Besides that the patch looks good.
>
> Reviewed-by: Alistair Francis <alistair.fran...@xilinx.com>
Thanks!
>
> Alistair
>
>
>> +FIELD(SDHC_HOSTCTL2, UHS_MODE_SEL, 0, 3);
>> +FIELD(SDHC_HOSTCTL2, V18_ENA, 3, 1); /* UHS-I only */
>> +FIELD(SDHC_HOSTCTL2, DRIVER_STRENGTH, 4, 2); /* UHS-I only */
>> +FIELD(SDHC_HOSTCTL2, EXECUTE_TUNING, 6, 1); /* UHS-I only */
>> +FIELD(SDHC_HOSTCTL2, SAMPLING_CLKSEL, 7, 1); /* UHS-I only */
>> +FIELD(SDHC_HOSTCTL2, ASYNC_INT, 14, 1);
>> +FIELD(SDHC_HOSTCTL2, PRESET_ENA, 15, 1);
>> +
>> /* HWInit Capabilities Register 0x05E80080 */
>> #define SDHC_CAPAB 0x40
>> FIELD(SDHC_CAPAB, TOCLKFREQ, 0, 6);
>> FIELD(SDHC_CAPAB, TOUNIT, 7, 1);
>> FIELD(SDHC_CAPAB, BASECLKFREQ, 8, 8);
>> FIELD(SDHC_CAPAB, MAXBLOCKLENGTH, 16, 2);
>> +FIELD(SDHC_CAPAB, EMBEDDED_8BIT, 18, 1); /* since v3 */
>> FIELD(SDHC_CAPAB, ADMA2, 19, 1); /* since v2 */
>> FIELD(SDHC_CAPAB, ADMA1, 20, 1); /* v1 only? */
>> FIELD(SDHC_CAPAB, HIGHSPEED, 21, 1);
>> @@ -198,6 +210,15 @@ FIELD(SDHC_CAPAB, V33, 24, 1);
>> FIELD(SDHC_CAPAB, V30, 25, 1);
>> FIELD(SDHC_CAPAB, V18, 26, 1);
>> FIELD(SDHC_CAPAB, BUS64BIT, 28, 1); /* since v2 */
>> +FIELD(SDHC_CAPAB, ASYNC_INT, 29, 1); /* since v3 */
>> +FIELD(SDHC_CAPAB, SLOT_TYPE, 30, 2); /* since v3 */
>> +FIELD(SDHC_CAPAB, BUS_SPEED, 32, 3); /* since v3 */
>> +FIELD(SDHC_CAPAB, DRIVER_STRENGTH, 36, 3); /* since v3 */
>> +FIELD(SDHC_CAPAB, DRIVER_TYPE_A, 36, 1); /* since v3 */
>> +FIELD(SDHC_CAPAB, DRIVER_TYPE_C, 37, 1); /* since v3 */
>> +FIELD(SDHC_CAPAB, DRIVER_TYPE_D, 38, 1); /* since v3 */
>> +FIELD(SDHC_CAPAB, TIMER_RETUNNING, 40, 4); /* since v3 */
>> +FIELD(SDHC_CAPAB, SDR50_TUNNING, 45, 1); /* since v3 */
>>
>> /* HWInit Maximum Current Capabilities Register 0x0 */
>> #define SDHC_MAXCURR 0x48
>> diff --git a/include/hw/sd/sdhci.h b/include/hw/sd/sdhci.h
>> index 26b50583af..f45e911065 100644
>> --- a/include/hw/sd/sdhci.h
>> +++ b/include/hw/sd/sdhci.h
>> @@ -104,6 +104,8 @@ typedef struct SDHCIState {
>> /* v2 */
>> bool adma1, adma2;
>> bool bus64;
>> + /* v3 */
>> + uint8_t slot_type, sdr, strength;
>> } cap;
>> } SDHCIState;
>>
>> diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
>> index ab4ffeca1d..773eb68fd6 100644
>> --- a/hw/sd/sdhci.c
>> +++ b/hw/sd/sdhci.c
>> @@ -63,6 +63,18 @@ static void sdhci_init_capareg(SDHCIState *s, Error
>> **errp)
>> uint32_t val;
>>
>> switch (s->spec_version) {
>> + case 3:
>> + val = FIELD_EX64(capareg, SDHC_CAPAB, SLOT_TYPE);
>> + if (val) {
>> + error_setg(errp, "slot-type not supported");
>> + return;
>> + }
>> + capareg = FIELD_DP64(capareg, SDHC_CAPAB, SLOT_TYPE, val);
>> + capareg = FIELD_DP64(capareg, SDHC_CAPAB, BUS_SPEED, s->cap.sdr);
>> + capareg = FIELD_DP64(capareg, SDHC_CAPAB, DRIVER_STRENGTH,
>> + s->cap.strength);
>> +
>> + /* fallback */
>> case 2: /* default version */
>> capareg = FIELD_DP64(capareg, SDHC_CAPAB, ADMA1, s->cap.adma1);
>> capareg = FIELD_DP64(capareg, SDHC_CAPAB, ADMA2, s->cap.adma2);
>> @@ -1169,8 +1181,11 @@ static inline unsigned int
>> sdhci_get_fifolen(SDHCIState *s)
>>
>> static void sdhci_init_readonly_registers(SDHCIState *s, Error **errp)
>> {
>> - if (s->spec_version != 2) {
>> - error_setg(errp, "Only Spec v2 is supported");
>> + switch (s->spec_version) {
>> + case 2 ... 3:
>> + break;
>> + default:
>> + error_setg(errp, "Only Spec v2/v3 are supported");
>> return;
>> }
>> s->version = (SDHC_HCVER_VENDOR << 8) | (s->spec_version - 1);
>> @@ -1319,6 +1334,9 @@ static Property sdhci_properties[] = {
>> DEFINE_PROP_BOOL("1v8", SDHCIState, cap.v18, false),
>>
>> DEFINE_PROP_BOOL("64bit", SDHCIState, cap.bus64, false),
>> + DEFINE_PROP_UINT8("slot-type", SDHCIState, cap.slot_type, 0),
>> + DEFINE_PROP_UINT8("bus-speed", SDHCIState, cap.sdr, 0),
>> + DEFINE_PROP_UINT8("driver-strength", SDHCIState, cap.strength, 0),
>>
>> /* capareg: deprecated */
>> DEFINE_PROP_UINT64("capareg", SDHCIState, capareg, UINT64_MAX),
>> --
>> 2.15.1
>>
>>
