diff mbox series

[v4,19/20] sdcard: add a 'uhs' property, update the OCR register ACCEPT_SWITCH_1V8 bit

Message ID 20180215221325.7611-20-f4bug@amsat.org
State New
Headers show
Series SDCard: bugfixes, support UHS-I (part 5) | expand

Commit Message

Philippe Mathieu-Daudé Feb. 15, 2018, 10:13 p.m. UTC 
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/sd/sd.c | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Alistair Francis Feb. 15, 2018, 10:55 p.m. UTC | #1 
On Thu, Feb 15, 2018 at 2:13 PM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>

Alistair

> ---
>  hw/sd/sd.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index ada96f5574..b9429b06ca 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -93,6 +93,7 @@ struct SDState {
>      /* Configurable properties */
>      BlockBackend *blk;
>      bool spi;
> +    uint8_t uhs_supported;
>
>      uint32_t mode;    /* current card mode, one of SDCardModes */
>      int32_t state;    /* current card state, one of SDCardStates */
> @@ -292,6 +293,8 @@ static void sd_set_ocr(SDState *sd)
>  {
>      /* All voltages OK */
>      sd->ocr = R_OCR_VDD_VOLTAGE_WIN_HI_MASK;
> +
> +    sd->ocr = FIELD_DP32(sd->ocr, OCR, ACCEPT_SWITCH_1V8, !!sd->uhs_supported);
>  }
>
>  static void sd_ocr_powerup(void *opaque)
> @@ -2189,6 +2192,7 @@ static Property sd_properties[] = {
>       * board to ensure that ssi transfers only occur when the chip select
>       * is asserted.  */
>      DEFINE_PROP_BOOL("spi", SDState, spi, false),
> +    DEFINE_PROP_UINT8("uhs", SDState, uhs_supported, UHS_NOT_SUPPORTED),
>      DEFINE_PROP_END_OF_LIST()
>  };
>
> --
> 2.16.1
>
>
Peter Maydell Feb. 22, 2018, 2:25 p.m. UTC | #2 
On 15 February 2018 at 22:13, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/sd/sd.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index ada96f5574..b9429b06ca 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -93,6 +93,7 @@ struct SDState {
>      /* Configurable properties */
>      BlockBackend *blk;
>      bool spi;
> +    uint8_t uhs_supported;
>
>      uint32_t mode;    /* current card mode, one of SDCardModes */
>      int32_t state;    /* current card state, one of SDCardStates */
> @@ -292,6 +293,8 @@ static void sd_set_ocr(SDState *sd)
>  {
>      /* All voltages OK */
>      sd->ocr = R_OCR_VDD_VOLTAGE_WIN_HI_MASK;
> +
> +    sd->ocr = FIELD_DP32(sd->ocr, OCR, ACCEPT_SWITCH_1V8, !!sd->uhs_supported);
>  }
>
>  static void sd_ocr_powerup(void *opaque)
> @@ -2189,6 +2192,7 @@ static Property sd_properties[] = {
>       * board to ensure that ssi transfers only occur when the chip select
>       * is asserted.  */
>      DEFINE_PROP_BOOL("spi", SDState, spi, false),
> +    DEFINE_PROP_UINT8("uhs", SDState, uhs_supported, UHS_NOT_SUPPORTED),
>      DEFINE_PROP_END_OF_LIST()
>  };

The field name and the use of the value suggest that this is just
a boolean flag, so why define it as a PROP_UINT8 rather than PROP_BOOL ?

thanks
-- PMM
Philippe Mathieu-Daudé March 3, 2018, 11:46 a.m. UTC | #3 
Hi Peter,

On 02/22/2018 11:25 AM, Peter Maydell wrote:
> On 15 February 2018 at 22:13, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>>  hw/sd/sd.c | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
>> index ada96f5574..b9429b06ca 100644
>> --- a/hw/sd/sd.c
>> +++ b/hw/sd/sd.c
>> @@ -93,6 +93,7 @@ struct SDState {
>>      /* Configurable properties */
>>      BlockBackend *blk;
>>      bool spi;
>> +    uint8_t uhs_supported;
>>
>>      uint32_t mode;    /* current card mode, one of SDCardModes */
>>      int32_t state;    /* current card state, one of SDCardStates */
>> @@ -292,6 +293,8 @@ static void sd_set_ocr(SDState *sd)
>>  {
>>      /* All voltages OK */
>>      sd->ocr = R_OCR_VDD_VOLTAGE_WIN_HI_MASK;
>> +
>> +    sd->ocr = FIELD_DP32(sd->ocr, OCR, ACCEPT_SWITCH_1V8, !!sd->uhs_supported);
>>  }
>>
>>  static void sd_ocr_powerup(void *opaque)
>> @@ -2189,6 +2192,7 @@ static Property sd_properties[] = {
>>       * board to ensure that ssi transfers only occur when the chip select
>>       * is asserted.  */
>>      DEFINE_PROP_BOOL("spi", SDState, spi, false),
>> +    DEFINE_PROP_UINT8("uhs", SDState, uhs_supported, UHS_NOT_SUPPORTED),
>>      DEFINE_PROP_END_OF_LIST()
>>  };
> 
> The field name and the use of the value suggest that this is just
> a boolean flag, so why define it as a PROP_UINT8 rather than PROP_BOOL ?

Here is the full enum:

typedef enum  {
    UHS_NOT_SUPPORTED   = 0,
    UHS_I               = 1,
    UHS_II              = 2,    /* currently not supported */
    UHS_III             = 3,    /* currently not supported */
} sd_uhs_mode_t;

I'll rename it "uhs_mode" to reflect this is not a boolean.

Some controllers modeled support UHS-II, I guess we'll need to implement
it at some point during the year.

Regards,

Phil.
diff mbox series

Patch

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index ada96f5574..b9429b06ca 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -93,6 +93,7 @@  struct SDState {
     /* Configurable properties */
     BlockBackend *blk;
     bool spi;
+    uint8_t uhs_supported;
 
     uint32_t mode;    /* current card mode, one of SDCardModes */
     int32_t state;    /* current card state, one of SDCardStates */
@@ -292,6 +293,8 @@  static void sd_set_ocr(SDState *sd)
 {
     /* All voltages OK */
     sd->ocr = R_OCR_VDD_VOLTAGE_WIN_HI_MASK;
+
+    sd->ocr = FIELD_DP32(sd->ocr, OCR, ACCEPT_SWITCH_1V8, !!sd->uhs_supported);
 }
 
 static void sd_ocr_powerup(void *opaque)
@@ -2189,6 +2192,7 @@  static Property sd_properties[] = {
      * board to ensure that ssi transfers only occur when the chip select
      * is asserted.  */
     DEFINE_PROP_BOOL("spi", SDState, spi, false),
+    DEFINE_PROP_UINT8("uhs", SDState, uhs_supported, UHS_NOT_SUPPORTED),
     DEFINE_PROP_END_OF_LIST()
 };