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

[Qemu-devel] [PATCH v3 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. v3 fixes two bugs in patch 3/3 (details
there).

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         | 10 +++++-----
 include/hw/sd/sdhci.h |  1 +
 2 files changed, 6 insertions(+), 5 deletions(-)

-- 
2.5.3

[Qemu-devel] [PATCH v3 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 v3 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 v3 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>
---

Notes:
    v3: add missing field in header, drop needless change to vmstate

 hw/sd/sdhci.c         | 5 ++++-
 include/hw/sd/sdhci.h | 1 +
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index dd83e89..7acb4d7 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;
 }
@@ -1276,6 +1278,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(),
 };
 
diff --git a/include/hw/sd/sdhci.h b/include/hw/sd/sdhci.h
index e78d938..ffd1f80 100644
--- a/include/hw/sd/sdhci.h
+++ b/include/hw/sd/sdhci.h
@@ -77,6 +77,7 @@ typedef struct SDHCIState {
     uint32_t buf_maxsz;
     uint16_t data_count;   /* current element in FIFO buffer */
     uint8_t  stopped_state;/* Current SDHC state */
+    bool     noeject_quirk;/* Quirk to disable card insert/remove interrupts */
     /* Buffer Data Port Register - virtual access point to R and W buffers */
     /* Software Reset Register - always reads as 0 */
     /* Force Event Auto CMD12 Error Interrupt Reg - write only */
-- 
2.5.3

Re: [Qemu-devel] [PATCH v3 0/3] sdhci patches to enable Raspberry Pi

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1259 bytes --]

On Mon, Dec 21, 2015 at 02:47:46PM -0800, Andrew Baumann wrote:
> 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. v3 fixes two bugs in patch 3/3 (details
> there).
> 
> 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         | 10 +++++-----
>  include/hw/sd/sdhci.h |  1 +
>  2 files changed, 6 insertions(+), 5 deletions(-)
> 
> -- 
> 2.5.3

I have merged SD Card patches in the past and this looks reasonable.  My
SD Card knowledge is zero, however.  You have Reviewed-by from Peter
Crosthwaithe so let's merge this.

Thanks, applied to my block-next tree:
https://github.com/stefanha/qemu/commits/block-next

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [Qemu-devel] [PATCH v3 0/3] sdhci patches to enable Raspberry Pi

[Andrew Baumann]

> From: Stefan Hajnoczi [mailto:stefanha@gmail.com]
> Sent: Tuesday, 22 December 2015 00:04
> On Mon, Dec 21, 2015 at 02:47:46PM -0800, Andrew Baumann wrote:
> > 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. v3 fixes two bugs in patch 3/3 (details
> > there).
> >
> > 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         | 10 +++++-----
> >  include/hw/sd/sdhci.h |  1 +
> >  2 files changed, 6 insertions(+), 5 deletions(-)
> >
> > --
> > 2.5.3
> 
> I have merged SD Card patches in the past and this looks reasonable.  My
> SD Card knowledge is zero, however.  You have Reviewed-by from Peter
> Crosthwaithe so let's merge this.
> 
> Thanks, applied to my block-next tree:
> https://github.com/stefanha/qemu/commits/block-next

Thanks. Unfortunately I screwed up testing patch 3/3 (the sdhci noeject quirk, committed as 723697551a7e9), and it doesn't work as intended -- in fact, it serves no useful purpose in its present form. I'll send a new series shortly to back it out and replace it with a pi-specific quirk. 

(The other two patches from this series are still good, BTW.)

Andrew