[Qemu-devel] [PATCH v2 0/3] sdhci patches to enable Raspberry Pi

[Qemu-devel] [PATCH v2 0/3] sdhci patches to enable Raspberry Pi

[Andrew Baumann]

This is a series of three tweaks needed to enable the generic sdhci
controller to emulate Raspberry Pi (bcm2835/2836), and boot Linux and
Windows.

There was some discussion of these changes in the following thread:
https://lists.gnu.org/archive/html/qemu-devel/2015-12/msg01401.html

v2 is functionally equivalent to v1; the only changes are to commit
messages and review metadata.

Cheers,
Andrew

Andrew Baumann (2):
  sdhci: don't raise a command index error for an unexpected response
  sdhci: add optional quirk property to disable card insertion/removal
    interrupts

Peter Crosthwaite (1):
  sd: sdhci: Delete over-zealous power check

 hw/sd/sdhci.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

-- 
2.5.3

[Qemu-devel] [PATCH v2 1/3] sd: sdhci: Delete over-zealous power check

[Andrew Baumann]

From: Peter Crosthwaite <crosthwaitepeter@gmail.com>

This check was conditionalising SD card operation on the card being
powered by the SDHCI host controller. It is however possible
(particularly in embedded systems) for the power control of the SD card
to be managed outside of SDHCI. This can be as trivial as hard-wiring
the SD slot VCC to a constant power-rail.

This means the guest SDHCI can validly opt-out of the SDHCI power
control feature while still using the card. So delete this check to
allow operation of the card with SDHCI power control.

This is needed for at least Xilinx Zynq and Raspberry Pi, and
also makes Freescale i.MX25 work for me. The digilent Zybo board
has a public schematic which shows SD VCC hardwiring:

http://digilentinc.com/Data/Products/ZYBO/ZYBO_sch_VB.3.pdf
bottom of page 3.

Signed-off-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
Reviewed-by: Sai Pavan Boddu <saipava@xilinx.com>
[AB: Add Pi to list of devices fixed in commit message]
Signed-off-by: Andrew Baumann <Andrew.Baumann@microsoft.com>
---
 hw/sd/sdhci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 8612760..bc39d48 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -831,7 +831,7 @@ static void sdhci_data_transfer(void *opaque)
 
 static bool sdhci_can_issue_command(SDHCIState *s)
 {
-    if (!SDHC_CLOCK_IS_ON(s->clkcon) || !(s->pwrcon & SDHC_POWER_ON) ||
+    if (!SDHC_CLOCK_IS_ON(s->clkcon) ||
         (((s->prnsts & SDHC_DATA_INHIBIT) || s->stopped_state) &&
         ((s->cmdreg & SDHC_CMD_DATA_PRESENT) ||
         ((s->cmdreg & SDHC_CMD_RESPONSE) == SDHC_CMD_RSP_WITH_BUSY &&
-- 
2.5.3

[Qemu-devel] [PATCH v2 2/3] sdhci: don't raise a command index error for an unexpected response

[Andrew Baumann]

This deletes a block of code that raised a command index error if a
command returned response data, but the guest did not set the
appropriate bits in the response register to handle such a response. I
cannot find any documentation that suggests the controller should
behave in this way, the error code doesn't make sense (command index
error is defined for the case where the index in a response does not
match that of the issued command), and in at least one case (CMD23
issued by UEFI on Raspberry Pi 2), actual hardware does not do this.

Signed-off-by: Andrew Baumann <Andrew.Baumann@microsoft.com>
Reviewed-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
---
 hw/sd/sdhci.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index bc39d48..dd83e89 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -243,9 +243,6 @@ static void sdhci_send_command(SDHCIState *s)
             (s->cmdreg & SDHC_CMD_RESPONSE) == SDHC_CMD_RSP_WITH_BUSY) {
             s->norintsts |= SDHC_NIS_TRSCMP;
         }
-    } else if (rlen != 0 && (s->errintstsen & SDHC_EISEN_CMDIDX)) {
-        s->errintsts |= SDHC_EIS_CMDIDX;
-        s->norintsts |= SDHC_NIS_ERR;
     }
 
     if (s->norintstsen & SDHC_NISEN_CMDCMP) {
-- 
2.5.3

[Qemu-devel] [PATCH v2 3/3] sdhci: add optional quirk property to disable card insertion/removal interrupts

[Andrew Baumann]

This is needed for a quirk of the Raspberry Pi (bcm2835/6) MMC
controller, where the card insert bit is documented as unimplemented
(always reads zero, doesn't generate interrupts) but is in fact
observed on hardware as set at power on, but is cleared (and remains
clear) on subsequent controller resets.

Signed-off-by: Andrew Baumann <Andrew.Baumann@microsoft.com>
Reviewed-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
---
 hw/sd/sdhci.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index dd83e89..61f919b 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -193,7 +193,9 @@ static void sdhci_reset(SDHCIState *s)
      * initialization */
     memset(&s->sdmasysad, 0, (uintptr_t)&s->capareg - (uintptr_t)&s->sdmasysad);
 
-    sd_set_cb(s->card, s->ro_cb, s->eject_cb);
+    if (!s->noeject_quirk) {
+        sd_set_cb(s->card, s->ro_cb, s->eject_cb);
+    }
     s->data_count = 0;
     s->stopped_state = sdhc_not_stopped;
 }
@@ -1208,6 +1210,7 @@ const VMStateDescription sdhci_vmstate = {
         VMSTATE_UINT16(data_count, SDHCIState),
         VMSTATE_UINT64(admasysaddr, SDHCIState),
         VMSTATE_UINT8(stopped_state, SDHCIState),
+        VMSTATE_BOOL(noeject_quirk, SDHCIState),
         VMSTATE_VBUFFER_UINT32(fifo_buffer, SDHCIState, 1, NULL, 0, buf_maxsz),
         VMSTATE_TIMER_PTR(insert_timer, SDHCIState),
         VMSTATE_TIMER_PTR(transfer_timer, SDHCIState),
@@ -1276,6 +1279,7 @@ static Property sdhci_sysbus_properties[] = {
     DEFINE_PROP_UINT32("capareg", SDHCIState, capareg,
             SDHC_CAPAB_REG_DEFAULT),
     DEFINE_PROP_UINT32("maxcurr", SDHCIState, maxcurr, 0),
+    DEFINE_PROP_BOOL("noeject-quirk", SDHCIState, noeject_quirk, false),
     DEFINE_PROP_END_OF_LIST(),
 };
 
-- 
2.5.3

Re: [Qemu-devel] [PATCH v2 3/3] sdhci: add optional quirk property to disable card insertion/removal interrupts

[Peter Crosthwaite]

On Mon, Dec 21, 2015 at 1:31 PM, Andrew Baumann
<Andrew.Baumann@microsoft.com> wrote:
> This is needed for a quirk of the Raspberry Pi (bcm2835/6) MMC
> controller, where the card insert bit is documented as unimplemented
> (always reads zero, doesn't generate interrupts) but is in fact
> observed on hardware as set at power on, but is cleared (and remains
> clear) on subsequent controller resets.
>
> Signed-off-by: Andrew Baumann <Andrew.Baumann@microsoft.com>
> Reviewed-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
> ---
>  hw/sd/sdhci.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
> index dd83e89..61f919b 100644
> --- a/hw/sd/sdhci.c
> +++ b/hw/sd/sdhci.c
> @@ -193,7 +193,9 @@ static void sdhci_reset(SDHCIState *s)
>       * initialization */
>      memset(&s->sdmasysad, 0, (uintptr_t)&s->capareg - (uintptr_t)&s->sdmasysad);
>
> -    sd_set_cb(s->card, s->ro_cb, s->eject_cb);
> +    if (!s->noeject_quirk) {
> +        sd_set_cb(s->card, s->ro_cb, s->eject_cb);
> +    }
>      s->data_count = 0;
>      s->stopped_state = sdhc_not_stopped;
>  }
> @@ -1208,6 +1210,7 @@ const VMStateDescription sdhci_vmstate = {
>          VMSTATE_UINT16(data_count, SDHCIState),
>          VMSTATE_UINT64(admasysaddr, SDHCIState),
>          VMSTATE_UINT8(stopped_state, SDHCIState),
> +        VMSTATE_BOOL(noeject_quirk, SDHCIState),

Sorry, one small thing I missed, static props should not be in the
VMSD. I think you just need to drop the VMSTATE_ addition here.
Otherwise you would need a VMSD version bump.

Is the patch missing the corresponding header change to add the new field?

Regards,
Peter

>          VMSTATE_VBUFFER_UINT32(fifo_buffer, SDHCIState, 1, NULL, 0, buf_maxsz),
>          VMSTATE_TIMER_PTR(insert_timer, SDHCIState),
>          VMSTATE_TIMER_PTR(transfer_timer, SDHCIState),
> @@ -1276,6 +1279,7 @@ static Property sdhci_sysbus_properties[] = {
>      DEFINE_PROP_UINT32("capareg", SDHCIState, capareg,
>              SDHC_CAPAB_REG_DEFAULT),
>      DEFINE_PROP_UINT32("maxcurr", SDHCIState, maxcurr, 0),
> +    DEFINE_PROP_BOOL("noeject-quirk", SDHCIState, noeject_quirk, false),
>      DEFINE_PROP_END_OF_LIST(),
>  };
>
> --
> 2.5.3
>

Re: [Qemu-devel] [PATCH v2 3/3] sdhci: add optional quirk property to disable card insertion/removal interrupts

[Andrew Baumann]

> From: Peter Crosthwaite [mailto:crosthwaitepeter@gmail.com]
> Sent: Monday, 21 December 2015 13:42
> On Mon, Dec 21, 2015 at 1:31 PM, Andrew Baumann
> <Andrew.Baumann@microsoft.com> wrote:
> > This is needed for a quirk of the Raspberry Pi (bcm2835/6) MMC
> > controller, where the card insert bit is documented as unimplemented
> > (always reads zero, doesn't generate interrupts) but is in fact
> > observed on hardware as set at power on, but is cleared (and remains
> > clear) on subsequent controller resets.
> > @@ -1208,6 +1210,7 @@ const VMStateDescription sdhci_vmstate = {
> >          VMSTATE_UINT16(data_count, SDHCIState),
> >          VMSTATE_UINT64(admasysaddr, SDHCIState),
> >          VMSTATE_UINT8(stopped_state, SDHCIState),
> > +        VMSTATE_BOOL(noeject_quirk, SDHCIState),
> 
> Sorry, one small thing I missed, static props should not be in the
> VMSD. I think you just need to drop the VMSTATE_ addition here.
> Otherwise you would need a VMSD version bump.

Yes we can drop the VMSTATE addition here.

> Is the patch missing the corresponding header change to add the new field?

Aargh. You're right, sorry, my mistake -- I've been breaking up all the changes into different branches to get them submitted, and missed the header file here. Will resend.

Andrew