Message ID | 20180108154303.6522-5-f4bug@amsat.org |
---|---|
State | New |
Headers | show |
Series | SDHCI: make it abstract, add inherited devices, add qtests | expand |
On Mon, Jan 8, 2018 at 7:42 AM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: > Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > --- > include/hw/sd/sdhci.h | 2 ++ > hw/sd/sdhci.c | 52 +++++++++++++++++++++++++++++++-------------------- > 2 files changed, 34 insertions(+), 20 deletions(-) > > diff --git a/include/hw/sd/sdhci.h b/include/hw/sd/sdhci.h > index 2703da1d5a..c1602becd2 100644 > --- a/include/hw/sd/sdhci.h > +++ b/include/hw/sd/sdhci.h > @@ -103,6 +103,8 @@ typedef struct SDHCIState { > bool pending_insert_quirk; /* Quirk for Raspberry Pi card insert int */ > uint8_t spec_version; > struct { > + uint8_t timeout_clk_freq, base_clk_freq_mhz; > + bool timeout_clk_in_mhz; > uint16_t max_blk_len; > bool suspend; > bool high_speed; > diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c > index c78643fe54..05681c86d6 100644 > --- a/hw/sd/sdhci.c > +++ b/hw/sd/sdhci.c > @@ -46,36 +46,31 @@ > #define SDHC_CAPAB_64BITBUS 0ul /* 64-bit System Bus Support */ > #define SDHC_CAPAB_ADMA1 1ul /* ADMA1 support */ > #define SDHC_CAPAB_ADMA2 1ul /* ADMA2 support */ > -/* Maximum clock frequency for SDclock in MHz > - * value in range 10-63 MHz, 0 - not defined */ > -#define SDHC_CAPAB_BASECLKFREQ 52ul > -#define SDHC_CAPAB_TOUNIT 1ul /* Timeout clock unit 0 - kHz, 1 - MHz */ > -/* Timeout clock frequency 1-63, 0 - not defined */ > -#define SDHC_CAPAB_TOCLKFREQ 52ul > > /* Now check all parameters and calculate CAPABILITIES REGISTER value */ > -#if SDHC_CAPAB_64BITBUS > 1 || SDHC_CAPAB_ADMA2 > 1 || SDHC_CAPAB_ADMA1 > 1 || \ > - SDHC_CAPAB_TOUNIT > 1 > +#if SDHC_CAPAB_64BITBUS > 1 || SDHC_CAPAB_ADMA2 > 1 || SDHC_CAPAB_ADMA1 > 1 > #error Capabilities features can have value 0 or 1 only! > #endif > > -#if (SDHC_CAPAB_BASECLKFREQ > 0 && SDHC_CAPAB_BASECLKFREQ < 10) || \ > - SDHC_CAPAB_BASECLKFREQ > 63 > -#error SDclock frequency can have value in range 0, 10-63 only! > -#endif > - > -#if SDHC_CAPAB_TOCLKFREQ > 63 > -#error Timeout clock frequency can have value in range 0-63 only! > -#endif > - > #define SDHC_CAPAB_REG_DEFAULT \ > ((SDHC_CAPAB_64BITBUS << 28) | (SDHC_CAPAB_ADMA1 << 20) | \ > - (SDHC_CAPAB_ADMA2 << 19) | \ > - (SDHC_CAPAB_BASECLKFREQ << 8) | (SDHC_CAPAB_TOUNIT << 7) | \ > - (SDHC_CAPAB_TOCLKFREQ)) > + (SDHC_CAPAB_ADMA2 << 19)) > > #define MASKED_WRITE(reg, mask, val) (reg = (reg & (mask)) | (val)) > > +static void sdhci_check_capab_freq_range(SDHCIState *s, const char *desc, > + uint8_t freq, Error **errp) > +{ > + switch (freq) { > + case 0: > + case 10 ... 63: > + break; You are missing a default here. Alistair > + error_setg(errp, "SD %s clock frequency can have value" > + "in range 0-63 only", desc); > + return; > + } > +} > + > static void sdhci_init_capareg(SDHCIState *s, Error **errp) > { > uint64_t capareg = 0; > @@ -83,6 +78,16 @@ static void sdhci_init_capareg(SDHCIState *s, Error **errp) > > switch (s->spec_version) { > case 1: > + sdhci_check_capab_freq_range(s, "Timeout", s->cap.timeout_clk_freq, > + errp); > + capareg = FIELD_DP64(capareg, SDHC_CAPAB, TOCLKFREQ, > + s->cap.timeout_clk_freq); > + sdhci_check_capab_freq_range(s, "Base", s->cap.base_clk_freq_mhz, errp); > + capareg = FIELD_DP64(capareg, SDHC_CAPAB, BASECLKFREQ, > + s->cap.base_clk_freq_mhz); > + capareg = FIELD_DP64(capareg, SDHC_CAPAB, TOUNIT, > + s->cap.timeout_clk_in_mhz); > + > val = ctz32(s->cap.max_blk_len >> 9); > if (val >= 0b11) { > error_setg(errp, "block size can be 512, 1024 or 2048 only"); > @@ -1304,6 +1309,13 @@ const VMStateDescription sdhci_vmstate = { > static Property sdhci_properties[] = { > DEFINE_PROP_UINT8("sd-spec-version", SDHCIState, spec_version, 2), > > + /* Timeout clock frequency 1-63, 0 - not defined */ > + DEFINE_PROP_UINT8("timeout-freq", SDHCIState, cap.timeout_clk_freq, 0), > + /* Timeout clock unit 0 - kHz, 1 - MHz */ > + DEFINE_PROP_BOOL("freq-in-mhz", SDHCIState, cap.timeout_clk_in_mhz, true), > + /* Maximum base clock frequency for SD clock in MHz (range 10-63 MHz, 0) */ > + DEFINE_PROP_UINT8("max-frequency", SDHCIState, cap.base_clk_freq_mhz, 0), > + > /* Maximum host controller R/W buffers size > * Possible values: 512, 1024, 2048 bytes */ > DEFINE_PROP_UINT16("max-block-length", SDHCIState, cap.max_blk_len, 512), > -- > 2.15.1 > >
On 01/08/2018 07:22 PM, Alistair Francis wrote: > On Mon, Jan 8, 2018 at 7:42 AM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: >> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> >> --- >> include/hw/sd/sdhci.h | 2 ++ >> hw/sd/sdhci.c | 52 +++++++++++++++++++++++++++++++-------------------- >> 2 files changed, 34 insertions(+), 20 deletions(-) >> >> diff --git a/include/hw/sd/sdhci.h b/include/hw/sd/sdhci.h >> index 2703da1d5a..c1602becd2 100644 >> --- a/include/hw/sd/sdhci.h >> +++ b/include/hw/sd/sdhci.h >> @@ -103,6 +103,8 @@ typedef struct SDHCIState { >> bool pending_insert_quirk; /* Quirk for Raspberry Pi card insert int */ >> uint8_t spec_version; >> struct { >> + uint8_t timeout_clk_freq, base_clk_freq_mhz; >> + bool timeout_clk_in_mhz; >> uint16_t max_blk_len; >> bool suspend; >> bool high_speed; >> diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c >> index c78643fe54..05681c86d6 100644 >> --- a/hw/sd/sdhci.c >> +++ b/hw/sd/sdhci.c >> @@ -46,36 +46,31 @@ >> #define SDHC_CAPAB_64BITBUS 0ul /* 64-bit System Bus Support */ >> #define SDHC_CAPAB_ADMA1 1ul /* ADMA1 support */ >> #define SDHC_CAPAB_ADMA2 1ul /* ADMA2 support */ >> -/* Maximum clock frequency for SDclock in MHz >> - * value in range 10-63 MHz, 0 - not defined */ >> -#define SDHC_CAPAB_BASECLKFREQ 52ul >> -#define SDHC_CAPAB_TOUNIT 1ul /* Timeout clock unit 0 - kHz, 1 - MHz */ >> -/* Timeout clock frequency 1-63, 0 - not defined */ >> -#define SDHC_CAPAB_TOCLKFREQ 52ul >> >> /* Now check all parameters and calculate CAPABILITIES REGISTER value */ >> -#if SDHC_CAPAB_64BITBUS > 1 || SDHC_CAPAB_ADMA2 > 1 || SDHC_CAPAB_ADMA1 > 1 || \ >> - SDHC_CAPAB_TOUNIT > 1 >> +#if SDHC_CAPAB_64BITBUS > 1 || SDHC_CAPAB_ADMA2 > 1 || SDHC_CAPAB_ADMA1 > 1 >> #error Capabilities features can have value 0 or 1 only! >> #endif >> >> -#if (SDHC_CAPAB_BASECLKFREQ > 0 && SDHC_CAPAB_BASECLKFREQ < 10) || \ >> - SDHC_CAPAB_BASECLKFREQ > 63 >> -#error SDclock frequency can have value in range 0, 10-63 only! >> -#endif >> - >> -#if SDHC_CAPAB_TOCLKFREQ > 63 >> -#error Timeout clock frequency can have value in range 0-63 only! >> -#endif >> - >> #define SDHC_CAPAB_REG_DEFAULT \ >> ((SDHC_CAPAB_64BITBUS << 28) | (SDHC_CAPAB_ADMA1 << 20) | \ >> - (SDHC_CAPAB_ADMA2 << 19) | \ >> - (SDHC_CAPAB_BASECLKFREQ << 8) | (SDHC_CAPAB_TOUNIT << 7) | \ >> - (SDHC_CAPAB_TOCLKFREQ)) >> + (SDHC_CAPAB_ADMA2 << 19)) >> >> #define MASKED_WRITE(reg, mask, val) (reg = (reg & (mask)) | (val)) >> >> +static void sdhci_check_capab_freq_range(SDHCIState *s, const char *desc, >> + uint8_t freq, Error **errp) >> +{ >> + switch (freq) { >> + case 0: >> + case 10 ... 63: >> + break; > > You are missing a default here. Thanks, peer review is useful :) I didn't try qtest expected failures yet, I'll see if it's doable. > > Alistair > >> + error_setg(errp, "SD %s clock frequency can have value" >> + "in range 0-63 only", desc); >> + return; >> + } >> +} >> + >> static void sdhci_init_capareg(SDHCIState *s, Error **errp) >> { >> uint64_t capareg = 0; >> @@ -83,6 +78,16 @@ static void sdhci_init_capareg(SDHCIState *s, Error **errp) >> >> switch (s->spec_version) { >> case 1: >> + sdhci_check_capab_freq_range(s, "Timeout", s->cap.timeout_clk_freq, >> + errp); >> + capareg = FIELD_DP64(capareg, SDHC_CAPAB, TOCLKFREQ, >> + s->cap.timeout_clk_freq); >> + sdhci_check_capab_freq_range(s, "Base", s->cap.base_clk_freq_mhz, errp); >> + capareg = FIELD_DP64(capareg, SDHC_CAPAB, BASECLKFREQ, >> + s->cap.base_clk_freq_mhz); >> + capareg = FIELD_DP64(capareg, SDHC_CAPAB, TOUNIT, >> + s->cap.timeout_clk_in_mhz); >> + >> val = ctz32(s->cap.max_blk_len >> 9); >> if (val >= 0b11) { >> error_setg(errp, "block size can be 512, 1024 or 2048 only"); >> @@ -1304,6 +1309,13 @@ const VMStateDescription sdhci_vmstate = { >> static Property sdhci_properties[] = { >> DEFINE_PROP_UINT8("sd-spec-version", SDHCIState, spec_version, 2), >> >> + /* Timeout clock frequency 1-63, 0 - not defined */ >> + DEFINE_PROP_UINT8("timeout-freq", SDHCIState, cap.timeout_clk_freq, 0), >> + /* Timeout clock unit 0 - kHz, 1 - MHz */ >> + DEFINE_PROP_BOOL("freq-in-mhz", SDHCIState, cap.timeout_clk_in_mhz, true), >> + /* Maximum base clock frequency for SD clock in MHz (range 10-63 MHz, 0) */ >> + DEFINE_PROP_UINT8("max-frequency", SDHCIState, cap.base_clk_freq_mhz, 0), >> + >> /* Maximum host controller R/W buffers size >> * Possible values: 512, 1024, 2048 bytes */ >> DEFINE_PROP_UINT16("max-block-length", SDHCIState, cap.max_blk_len, 512), >> -- >> 2.15.1 >> >>
diff --git a/include/hw/sd/sdhci.h b/include/hw/sd/sdhci.h index 2703da1d5a..c1602becd2 100644 --- a/include/hw/sd/sdhci.h +++ b/include/hw/sd/sdhci.h @@ -103,6 +103,8 @@ typedef struct SDHCIState { bool pending_insert_quirk; /* Quirk for Raspberry Pi card insert int */ uint8_t spec_version; struct { + uint8_t timeout_clk_freq, base_clk_freq_mhz; + bool timeout_clk_in_mhz; uint16_t max_blk_len; bool suspend; bool high_speed; diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c index c78643fe54..05681c86d6 100644 --- a/hw/sd/sdhci.c +++ b/hw/sd/sdhci.c @@ -46,36 +46,31 @@ #define SDHC_CAPAB_64BITBUS 0ul /* 64-bit System Bus Support */ #define SDHC_CAPAB_ADMA1 1ul /* ADMA1 support */ #define SDHC_CAPAB_ADMA2 1ul /* ADMA2 support */ -/* Maximum clock frequency for SDclock in MHz - * value in range 10-63 MHz, 0 - not defined */ -#define SDHC_CAPAB_BASECLKFREQ 52ul -#define SDHC_CAPAB_TOUNIT 1ul /* Timeout clock unit 0 - kHz, 1 - MHz */ -/* Timeout clock frequency 1-63, 0 - not defined */ -#define SDHC_CAPAB_TOCLKFREQ 52ul /* Now check all parameters and calculate CAPABILITIES REGISTER value */ -#if SDHC_CAPAB_64BITBUS > 1 || SDHC_CAPAB_ADMA2 > 1 || SDHC_CAPAB_ADMA1 > 1 || \ - SDHC_CAPAB_TOUNIT > 1 +#if SDHC_CAPAB_64BITBUS > 1 || SDHC_CAPAB_ADMA2 > 1 || SDHC_CAPAB_ADMA1 > 1 #error Capabilities features can have value 0 or 1 only! #endif -#if (SDHC_CAPAB_BASECLKFREQ > 0 && SDHC_CAPAB_BASECLKFREQ < 10) || \ - SDHC_CAPAB_BASECLKFREQ > 63 -#error SDclock frequency can have value in range 0, 10-63 only! -#endif - -#if SDHC_CAPAB_TOCLKFREQ > 63 -#error Timeout clock frequency can have value in range 0-63 only! -#endif - #define SDHC_CAPAB_REG_DEFAULT \ ((SDHC_CAPAB_64BITBUS << 28) | (SDHC_CAPAB_ADMA1 << 20) | \ - (SDHC_CAPAB_ADMA2 << 19) | \ - (SDHC_CAPAB_BASECLKFREQ << 8) | (SDHC_CAPAB_TOUNIT << 7) | \ - (SDHC_CAPAB_TOCLKFREQ)) + (SDHC_CAPAB_ADMA2 << 19)) #define MASKED_WRITE(reg, mask, val) (reg = (reg & (mask)) | (val)) +static void sdhci_check_capab_freq_range(SDHCIState *s, const char *desc, + uint8_t freq, Error **errp) +{ + switch (freq) { + case 0: + case 10 ... 63: + break; + error_setg(errp, "SD %s clock frequency can have value" + "in range 0-63 only", desc); + return; + } +} + static void sdhci_init_capareg(SDHCIState *s, Error **errp) { uint64_t capareg = 0; @@ -83,6 +78,16 @@ static void sdhci_init_capareg(SDHCIState *s, Error **errp) switch (s->spec_version) { case 1: + sdhci_check_capab_freq_range(s, "Timeout", s->cap.timeout_clk_freq, + errp); + capareg = FIELD_DP64(capareg, SDHC_CAPAB, TOCLKFREQ, + s->cap.timeout_clk_freq); + sdhci_check_capab_freq_range(s, "Base", s->cap.base_clk_freq_mhz, errp); + capareg = FIELD_DP64(capareg, SDHC_CAPAB, BASECLKFREQ, + s->cap.base_clk_freq_mhz); + capareg = FIELD_DP64(capareg, SDHC_CAPAB, TOUNIT, + s->cap.timeout_clk_in_mhz); + val = ctz32(s->cap.max_blk_len >> 9); if (val >= 0b11) { error_setg(errp, "block size can be 512, 1024 or 2048 only"); @@ -1304,6 +1309,13 @@ const VMStateDescription sdhci_vmstate = { static Property sdhci_properties[] = { DEFINE_PROP_UINT8("sd-spec-version", SDHCIState, spec_version, 2), + /* Timeout clock frequency 1-63, 0 - not defined */ + DEFINE_PROP_UINT8("timeout-freq", SDHCIState, cap.timeout_clk_freq, 0), + /* Timeout clock unit 0 - kHz, 1 - MHz */ + DEFINE_PROP_BOOL("freq-in-mhz", SDHCIState, cap.timeout_clk_in_mhz, true), + /* Maximum base clock frequency for SD clock in MHz (range 10-63 MHz, 0) */ + DEFINE_PROP_UINT8("max-frequency", SDHCIState, cap.base_clk_freq_mhz, 0), + /* Maximum host controller R/W buffers size * Possible values: 512, 1024, 2048 bytes */ DEFINE_PROP_UINT16("max-block-length", SDHCIState, cap.max_blk_len, 512),
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> --- include/hw/sd/sdhci.h | 2 ++ hw/sd/sdhci.c | 52 +++++++++++++++++++++++++++++++-------------------- 2 files changed, 34 insertions(+), 20 deletions(-)