SDHCI housekeeping

[Qemu-devel] [PATCH 13/14] sdhci: add sdhci_init_capareg() to initialize the CAPAB register

Posted by Philippe Mathieu-Daudé 8 years, 1 month ago

following patches modifying CAPAB will be easier to review.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/sd/sdhci.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index dadc4787b2..78bb8e8e89 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -100,6 +100,13 @@
 
 #define MASKED_WRITE(reg, mask, val)  (reg = (reg & (mask)) | (val))
 
+static void sdhci_init_capareg(SDHCIState *s, Error **errp)
+{
+    if (s->capareg == UINT32_MAX) {
+        s->capareg = SDHC_CAPAB_REG_DEFAULT;
+    }
+}
+
 static uint8_t sdhci_slotint(SDHCIState *s)
 {
     return (s->norintsts & s->norintsigen) || (s->errintsts & s->errintsigen) ||
@@ -1158,6 +1165,8 @@ static void sdhci_initfn(SDHCIState *s)
 
 static void sdhci_realizefn(SDHCIState *s, Error **errp)
 {
+    sdhci_init_capareg(s, errp);
+
     s->buf_maxsz = sdhci_get_fifolen(s);
     s->fifo_buffer = g_malloc0(s->buf_maxsz);
 
@@ -1244,8 +1253,7 @@ const VMStateDescription sdhci_vmstate = {
 /* Capabilities registers provide information on supported features of this
  * specific host controller implementation */
 static Property sdhci_properties[] = {
-    DEFINE_PROP_UINT32("capareg", SDHCIState, capareg,
-            SDHC_CAPAB_REG_DEFAULT),
+    DEFINE_PROP_UINT32("capareg", SDHCIState, capareg, UINT32_MAX),
     DEFINE_PROP_UINT32("maxcurr", SDHCIState, maxcurr, 0),
     DEFINE_PROP_BOOL("pending-insert-quirk", SDHCIState, pending_insert_quirk,
                      false),
-- 
2.15.1

Re: [Qemu-devel] [PATCH 13/14] sdhci: add sdhci_init_capareg() to initialize the CAPAB register

Posted by Alistair Francis 8 years, 1 month ago

On Wed, Dec 13, 2017 at 11:58 AM, Philippe Mathieu-Daudé
<f4bug@amsat.org> wrote:
> following patches modifying CAPAB will be easier to review.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/sd/sdhci.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
> index dadc4787b2..78bb8e8e89 100644
> --- a/hw/sd/sdhci.c
> +++ b/hw/sd/sdhci.c
> @@ -100,6 +100,13 @@
>
>  #define MASKED_WRITE(reg, mask, val)  (reg = (reg & (mask)) | (val))
>
> +static void sdhci_init_capareg(SDHCIState *s, Error **errp)
> +{
> +    if (s->capareg == UINT32_MAX) {
> +        s->capareg = SDHC_CAPAB_REG_DEFAULT;
> +    }
> +}
> +
>  static uint8_t sdhci_slotint(SDHCIState *s)
>  {
>      return (s->norintsts & s->norintsigen) || (s->errintsts & s->errintsigen) ||
> @@ -1158,6 +1165,8 @@ static void sdhci_initfn(SDHCIState *s)
>
>  static void sdhci_realizefn(SDHCIState *s, Error **errp)
>  {
> +    sdhci_init_capareg(s, errp);
> +
>      s->buf_maxsz = sdhci_get_fifolen(s);
>      s->fifo_buffer = g_malloc0(s->buf_maxsz);
>
> @@ -1244,8 +1253,7 @@ const VMStateDescription sdhci_vmstate = {
>  /* Capabilities registers provide information on supported features of this
>   * specific host controller implementation */
>  static Property sdhci_properties[] = {
> -    DEFINE_PROP_UINT32("capareg", SDHCIState, capareg,
> -            SDHC_CAPAB_REG_DEFAULT),
> +    DEFINE_PROP_UINT32("capareg", SDHCIState, capareg, UINT32_MAX),

This looks like a step in the wrong direction.

If this is required to do something smarter latter it should probably
be in that series.

Alistair

>      DEFINE_PROP_UINT32("maxcurr", SDHCIState, maxcurr, 0),
>      DEFINE_PROP_BOOL("pending-insert-quirk", SDHCIState, pending_insert_quirk,
>                       false),
> --
> 2.15.1
>
>

Re: [Qemu-devel] [PATCH 13/14] sdhci: add sdhci_init_capareg() to initialize the CAPAB register

Posted by Philippe Mathieu-Daudé 8 years, 1 month ago

On Thu, Dec 14, 2017 at 2:51 PM, Alistair Francis
<alistair.francis@xilinx.com> wrote:
> On Wed, Dec 13, 2017 at 11:58 AM, Philippe Mathieu-Daudé
> <f4bug@amsat.org> wrote:
>> following patches modifying CAPAB will be easier to review.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>>  hw/sd/sdhci.c | 12 ++++++++++--
>>  1 file changed, 10 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
>> index dadc4787b2..78bb8e8e89 100644
>> --- a/hw/sd/sdhci.c
>> +++ b/hw/sd/sdhci.c
>> @@ -100,6 +100,13 @@
>>
>>  #define MASKED_WRITE(reg, mask, val)  (reg = (reg & (mask)) | (val))
>>
>> +static void sdhci_init_capareg(SDHCIState *s, Error **errp)
>> +{
>> +    if (s->capareg == UINT32_MAX) {
>> +        s->capareg = SDHC_CAPAB_REG_DEFAULT;
>> +    }
>> +}
>> +
>>  static uint8_t sdhci_slotint(SDHCIState *s)
>>  {
>>      return (s->norintsts & s->norintsigen) || (s->errintsts & s->errintsigen) ||
>> @@ -1158,6 +1165,8 @@ static void sdhci_initfn(SDHCIState *s)
>>
>>  static void sdhci_realizefn(SDHCIState *s, Error **errp)
>>  {
>> +    sdhci_init_capareg(s, errp);
>> +
>>      s->buf_maxsz = sdhci_get_fifolen(s);
>>      s->fifo_buffer = g_malloc0(s->buf_maxsz);
>>
>> @@ -1244,8 +1253,7 @@ const VMStateDescription sdhci_vmstate = {
>>  /* Capabilities registers provide information on supported features of this
>>   * specific host controller implementation */
>>  static Property sdhci_properties[] = {
>> -    DEFINE_PROP_UINT32("capareg", SDHCIState, capareg,
>> -            SDHC_CAPAB_REG_DEFAULT),
>> +    DEFINE_PROP_UINT32("capareg", SDHCIState, capareg, UINT32_MAX),
>
> This looks like a step in the wrong direction.
>
> If this is required to do something smarter latter it should probably
> be in that series.

You are right, I might have add this patch in the incorrect series.
I'll move it to the one where the CAPAB register is correctly set
regarding the SDHCI supports the v2 or v3 spec.