[PATCH v2 00/22] eMMC support

[PATCH v2 00/22] eMMC support

[Sai Pavan Boddu]

Hi,

This patch series add support for eMMC cards. This work was previosly
submitted by Vincent, rebased few changes on top.

Cedric & Joel has helped to added boot partition access support. I
expect them to make a follow-up series to use it with aspeed machines.

Present series adds eMMC support to Versal SOC.

Initial patch series version is RFC
Changes for V2:
	Split Patch 1
	Add comments for eMMC Erase commands
	Added documentation about eMMC and Versal-virt board.
	Make eMMC optional for xlnx-versal-virt machines

Regards,
Sai Pavan

Cédric Le Goater (2):
  sd: sdmmc-internal: Add command string for SEND_OP_CMD
  sd: emmc: Add Extended CSD register definitions

Joel Stanley (2):
  sd: emmc: Support boot area in emmc image
  sd: emmc: Subtract bootarea size from blk

Sai Pavan Boddu (13):
  sd: sd: Remove usage of tabs in the file
  sd: emmc: Dont not update CARD_CAPACITY for eMMC cards
  sd: emmc: Update CMD1 definition for eMMC
  sd: emmc: support idle state in CMD2
  sd: emmc: Add mmc switch function support
  sd: emmc: add CMD21 tuning sequence
  sd: emmc: Make ACMD41 illegal for mmc
  sd: emmc: Add support for emmc erase
  sd: emmc: Update CID structure for eMMC
  sd: sdhci: Support eMMC devices
  arm: xlnx-versal: Add emmc to versal
  docs: devel: emmc: Add a doc for emmc card emulation
  docs: arm: xlnx-versal-virt: Add eMMC support documentation

Vincent Palatin (5):
  block: add eMMC block device type
  sd: emmc: Add support for eMMC cards
  sd: emmc: update OCR fields for eMMC
  sd: emmc: Add support for EXT_CSD & CSD for eMMC
  sd: emmc: Update CMD8 to send EXT_CSD register

 docs/devel/emmc.txt                  |  16 +
 docs/system/arm/xlnx-versal-virt.rst |  10 +
 hw/sd/sdmmc-internal.h               |  97 +++++++
 include/hw/arm/xlnx-versal.h         |   1 +
 include/sysemu/blockdev.h            |   1 +
 blockdev.c                           |   1 +
 hw/arm/xlnx-versal-virt.c            |  30 +-
 hw/arm/xlnx-versal.c                 |  13 +-
 hw/sd/sd.c                           | 545 ++++++++++++++++++++++++++---------
 hw/sd/sdhci.c                        |   4 -
 hw/sd/sdmmc-internal.c               |   2 +-
 11 files changed, 571 insertions(+), 149 deletions(-)
 create mode 100644 docs/devel/emmc.txt

-- 
2.7.4

[PATCH v2 01/22] block: add eMMC block device type

[Sai Pavan Boddu]

From: Vincent Palatin <vpalatin@chromium.org>

Add new block device type.

Signed-off-by: Vincent Palatin <vpalatin@chromium.org>
[SPB: Rebased over 5.1 version]
Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
Signed-off-by: Joel Stanley <joel@jms.id.au>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
---
 include/sysemu/blockdev.h | 1 +
 blockdev.c                | 1 +
 2 files changed, 2 insertions(+)

diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h
index 3b5fcda..eefae9f 100644
--- a/include/sysemu/blockdev.h
+++ b/include/sysemu/blockdev.h
@@ -24,6 +24,7 @@ typedef enum {
      */
     IF_NONE = 0,
     IF_IDE, IF_SCSI, IF_FLOPPY, IF_PFLASH, IF_MTD, IF_SD, IF_VIRTIO, IF_XEN,
+    IF_EMMC,
     IF_COUNT
 } BlockInterfaceType;
 
diff --git a/blockdev.c b/blockdev.c
index cd438e6..390d43c 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -83,6 +83,7 @@ static const char *const if_name[IF_COUNT] = {
     [IF_SD] = "sd",
     [IF_VIRTIO] = "virtio",
     [IF_XEN] = "xen",
+    [IF_EMMC] = "emmc",
 };
 
 static int if_max_devs[IF_COUNT] = {
-- 
2.7.4

[PATCH v2 02/22] sd: sd: Remove usage of tabs in the file

[Sai Pavan Boddu]

Set tabstop as 4 and used expandtabs

Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
---
 hw/sd/sd.c | 190 ++++++++++++++++++++++++++++++-------------------------------
 1 file changed, 95 insertions(+), 95 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 8517dbc..74b9162 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -332,39 +332,39 @@ static void sd_set_scr(SDState *sd)
     sd->scr[7] = 0x00;
 }
 
-#define MID	0xaa
-#define OID	"XY"
-#define PNM	"QEMU!"
-#define PRV	0x01
-#define MDT_YR	2006
-#define MDT_MON	2
+#define MID 0xaa
+#define OID "XY"
+#define PNM "QEMU!"
+#define PRV 0x01
+#define MDT_YR  2006
+#define MDT_MON 2
 
 static void sd_set_cid(SDState *sd)
 {
-    sd->cid[0] = MID;		/* Fake card manufacturer ID (MID) */
-    sd->cid[1] = OID[0];	/* OEM/Application ID (OID) */
+    sd->cid[0] = MID;       /* Fake card manufacturer ID (MID) */
+    sd->cid[1] = OID[0];    /* OEM/Application ID (OID) */
     sd->cid[2] = OID[1];
-    sd->cid[3] = PNM[0];	/* Fake product name (PNM) */
+    sd->cid[3] = PNM[0];    /* Fake product name (PNM) */
     sd->cid[4] = PNM[1];
     sd->cid[5] = PNM[2];
     sd->cid[6] = PNM[3];
     sd->cid[7] = PNM[4];
-    sd->cid[8] = PRV;		/* Fake product revision (PRV) */
-    sd->cid[9] = 0xde;		/* Fake serial number (PSN) */
+    sd->cid[8] = PRV;       /* Fake product revision (PRV) */
+    sd->cid[9] = 0xde;      /* Fake serial number (PSN) */
     sd->cid[10] = 0xad;
     sd->cid[11] = 0xbe;
     sd->cid[12] = 0xef;
-    sd->cid[13] = 0x00 |	/* Manufacture date (MDT) */
+    sd->cid[13] = 0x00 |    /* Manufacture date (MDT) */
         ((MDT_YR - 2000) / 10);
     sd->cid[14] = ((MDT_YR % 10) << 4) | MDT_MON;
     sd->cid[15] = (sd_crc7(sd->cid, 15) << 1) | 1;
 }
 
-#define HWBLOCK_SHIFT	9			/* 512 bytes */
-#define SECTOR_SHIFT	5			/* 16 kilobytes */
-#define WPGROUP_SHIFT	7			/* 2 megs */
-#define CMULT_SHIFT	9			/* 512 times HWBLOCK_SIZE */
-#define WPGROUP_SIZE	(1 << (HWBLOCK_SHIFT + SECTOR_SHIFT + WPGROUP_SHIFT))
+#define HWBLOCK_SHIFT   9           /* 512 bytes */
+#define SECTOR_SHIFT    5           /* 16 kilobytes */
+#define WPGROUP_SHIFT   7           /* 2 megs */
+#define CMULT_SHIFT 9           /* 512 times HWBLOCK_SIZE */
+#define WPGROUP_SIZE    (1 << (HWBLOCK_SHIFT + SECTOR_SHIFT + WPGROUP_SHIFT))
 
 static const uint8_t sd_csd_rw_mask[16] = {
     0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
@@ -385,31 +385,31 @@ static void sd_set_csd(SDState *sd, uint64_t size)
     csize = (size >> (CMULT_SHIFT + hwblock_shift)) - 1;
 
     if (size <= SDSC_MAX_CAPACITY) { /* Standard Capacity SD */
-        sd->csd[0] = 0x00;	/* CSD structure */
-        sd->csd[1] = 0x26;	/* Data read access-time-1 */
-        sd->csd[2] = 0x00;	/* Data read access-time-2 */
+        sd->csd[0] = 0x00;  /* CSD structure */
+        sd->csd[1] = 0x26;  /* Data read access-time-1 */
+        sd->csd[2] = 0x00;  /* Data read access-time-2 */
         sd->csd[3] = 0x32;      /* Max. data transfer rate: 25 MHz */
-        sd->csd[4] = 0x5f;	/* Card Command Classes */
-        sd->csd[5] = 0x50 |	/* Max. read data block length */
+        sd->csd[4] = 0x5f;  /* Card Command Classes */
+        sd->csd[5] = 0x50 | /* Max. read data block length */
             hwblock_shift;
-        sd->csd[6] = 0xe0 |	/* Partial block for read allowed */
+        sd->csd[6] = 0xe0 | /* Partial block for read allowed */
             ((csize >> 10) & 0x03);
-        sd->csd[7] = 0x00 |	/* Device size */
+        sd->csd[7] = 0x00 | /* Device size */
             ((csize >> 2) & 0xff);
-        sd->csd[8] = 0x3f |	/* Max. read current */
+        sd->csd[8] = 0x3f | /* Max. read current */
             ((csize << 6) & 0xc0);
-        sd->csd[9] = 0xfc |	/* Max. write current */
+        sd->csd[9] = 0xfc | /* Max. write current */
             ((CMULT_SHIFT - 2) >> 1);
-        sd->csd[10] = 0x40 |	/* Erase sector size */
+        sd->csd[10] = 0x40 |    /* Erase sector size */
             (((CMULT_SHIFT - 2) << 7) & 0x80) | (sectsize >> 1);
-        sd->csd[11] = 0x00 |	/* Write protect group size */
+        sd->csd[11] = 0x00 |    /* Write protect group size */
             ((sectsize << 7) & 0x80) | wpsize;
-        sd->csd[12] = 0x90 |	/* Write speed factor */
+        sd->csd[12] = 0x90 |    /* Write speed factor */
             (hwblock_shift >> 2);
-        sd->csd[13] = 0x20 |	/* Max. write data block length */
+        sd->csd[13] = 0x20 |    /* Max. write data block length */
             ((hwblock_shift << 6) & 0xc0);
-        sd->csd[14] = 0x00;	/* File format group */
-    } else {			/* SDHC */
+        sd->csd[14] = 0x00; /* File format group */
+    } else {            /* SDHC */
         size /= 512 * KiB;
         size -= 1;
         sd->csd[0] = 0x40;
@@ -503,7 +503,7 @@ static int sd_req_crc_validate(SDRequest *req)
     buffer[0] = 0x40 | req->cmd;
     stl_be_p(&buffer[1], req->arg);
     return 0;
-    return sd_crc7(buffer, 5) != req->crc;	/* TODO */
+    return sd_crc7(buffer, 5) != req->crc;  /* TODO */
 }
 
 static void sd_response_r1_make(SDState *sd, uint8_t *response)
@@ -803,19 +803,19 @@ static void sd_function_switch(SDState *sd, uint32_t arg)
     int i, mode, new_func;
     mode = !!(arg & 0x80000000);
 
-    sd->data[0] = 0x00;		/* Maximum current consumption */
+    sd->data[0] = 0x00;     /* Maximum current consumption */
     sd->data[1] = 0x01;
-    sd->data[2] = 0x80;		/* Supported group 6 functions */
+    sd->data[2] = 0x80;     /* Supported group 6 functions */
     sd->data[3] = 0x01;
-    sd->data[4] = 0x80;		/* Supported group 5 functions */
+    sd->data[4] = 0x80;     /* Supported group 5 functions */
     sd->data[5] = 0x01;
-    sd->data[6] = 0x80;		/* Supported group 4 functions */
+    sd->data[6] = 0x80;     /* Supported group 4 functions */
     sd->data[7] = 0x01;
-    sd->data[8] = 0x80;		/* Supported group 3 functions */
+    sd->data[8] = 0x80;     /* Supported group 3 functions */
     sd->data[9] = 0x01;
-    sd->data[10] = 0x80;	/* Supported group 2 functions */
+    sd->data[10] = 0x80;    /* Supported group 2 functions */
     sd->data[11] = 0x43;
-    sd->data[12] = 0x80;	/* Supported group 1 functions */
+    sd->data[12] = 0x80;    /* Supported group 1 functions */
     sd->data[13] = 0x03;
 
     memset(&sd->data[14], 0, 3);
@@ -940,7 +940,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
 
     switch (req.cmd) {
     /* Basic commands (Class 0 and Class 1) */
-    case 0:	/* CMD0:   GO_IDLE_STATE */
+    case 0: /* CMD0:   GO_IDLE_STATE */
         switch (sd->state) {
         case sd_inactive_state:
             return sd->spi ? sd_r1 : sd_r0;
@@ -952,14 +952,14 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
         }
         break;
 
-    case 1:	/* CMD1:   SEND_OP_CMD */
+    case 1: /* CMD1:   SEND_OP_CMD */
         if (!sd->spi)
             goto bad_cmd;
 
         sd->state = sd_transfer_state;
         return sd_r1;
 
-    case 2:	/* CMD2:   ALL_SEND_CID */
+    case 2: /* CMD2:   ALL_SEND_CID */
         if (sd->spi)
             goto bad_cmd;
         switch (sd->state) {
@@ -972,7 +972,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
         }
         break;
 
-    case 3:	/* CMD3:   SEND_RELATIVE_ADDR */
+    case 3: /* CMD3:   SEND_RELATIVE_ADDR */
         if (sd->spi)
             goto bad_cmd;
         switch (sd->state) {
@@ -987,7 +987,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
         }
         break;
 
-    case 4:	/* CMD4:   SEND_DSR */
+    case 4: /* CMD4:   SEND_DSR */
         if (sd->spi)
             goto bad_cmd;
         switch (sd->state) {
@@ -1002,7 +1002,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
     case 5: /* CMD5: reserved for SDIO cards */
         return sd_illegal;
 
-    case 6:	/* CMD6:   SWITCH_FUNCTION */
+    case 6: /* CMD6:   SWITCH_FUNCTION */
         switch (sd->mode) {
         case sd_data_transfer_mode:
             sd_function_switch(sd, req.arg);
@@ -1016,7 +1016,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
         }
         break;
 
-    case 7:	/* CMD7:   SELECT/DESELECT_CARD */
+    case 7: /* CMD7:   SELECT/DESELECT_CARD */
         if (sd->spi)
             goto bad_cmd;
         switch (sd->state) {
@@ -1054,7 +1054,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
         }
         break;
 
-    case 8:	/* CMD8:   SEND_IF_COND */
+    case 8: /* CMD8:   SEND_IF_COND */
         if (sd->spec_version < SD_PHY_SPECv2_00_VERS) {
             break;
         }
@@ -1072,7 +1072,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
         sd->vhs = req.arg;
         return sd_r7;
 
-    case 9:	/* CMD9:   SEND_CSD */
+    case 9: /* CMD9:   SEND_CSD */
         switch (sd->state) {
         case sd_standby_state:
             if (sd->rca != rca)
@@ -1094,7 +1094,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
         }
         break;
 
-    case 10:	/* CMD10:  SEND_CID */
+    case 10:    /* CMD10:  SEND_CID */
         switch (sd->state) {
         case sd_standby_state:
             if (sd->rca != rca)
@@ -1116,7 +1116,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
         }
         break;
 
-    case 12:	/* CMD12:  STOP_TRANSMISSION */
+    case 12:    /* CMD12:  STOP_TRANSMISSION */
         switch (sd->state) {
         case sd_sendingdata_state:
             sd->state = sd_transfer_state;
@@ -1133,7 +1133,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
         }
         break;
 
-    case 13:	/* CMD13:  SEND_STATUS */
+    case 13:    /* CMD13:  SEND_STATUS */
         switch (sd->mode) {
         case sd_data_transfer_mode:
             if (sd->rca != rca)
@@ -1146,7 +1146,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
         }
         break;
 
-    case 15:	/* CMD15:  GO_INACTIVE_STATE */
+    case 15:    /* CMD15:  GO_INACTIVE_STATE */
         if (sd->spi)
             goto bad_cmd;
         switch (sd->mode) {
@@ -1163,7 +1163,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
         break;
 
     /* Block read commands (Classs 2) */
-    case 16:	/* CMD16:  SET_BLOCKLEN */
+    case 16:    /* CMD16:  SET_BLOCKLEN */
         switch (sd->state) {
         case sd_transfer_state:
             if (req.arg > (1 << HWBLOCK_SHIFT)) {
@@ -1180,7 +1180,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
         }
         break;
 
-    case 17:	/* CMD17:  READ_SINGLE_BLOCK */
+    case 17:    /* CMD17:  READ_SINGLE_BLOCK */
         switch (sd->state) {
         case sd_transfer_state:
 
@@ -1199,7 +1199,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
         }
         break;
 
-    case 18:	/* CMD18:  READ_MULTIPLE_BLOCK */
+    case 18:    /* CMD18:  READ_MULTIPLE_BLOCK */
         switch (sd->state) {
         case sd_transfer_state:
 
@@ -1244,7 +1244,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
         break;
 
     /* Block write commands (Class 4) */
-    case 24:	/* CMD24:  WRITE_SINGLE_BLOCK */
+    case 24:    /* CMD24:  WRITE_SINGLE_BLOCK */
         switch (sd->state) {
         case sd_transfer_state:
             /* Writing in SPI mode not implemented.  */
@@ -1274,7 +1274,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
         }
         break;
 
-    case 25:	/* CMD25:  WRITE_MULTIPLE_BLOCK */
+    case 25:    /* CMD25:  WRITE_MULTIPLE_BLOCK */
         switch (sd->state) {
         case sd_transfer_state:
             /* Writing in SPI mode not implemented.  */
@@ -1304,7 +1304,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
         }
         break;
 
-    case 26:	/* CMD26:  PROGRAM_CID */
+    case 26:    /* CMD26:  PROGRAM_CID */
         if (sd->spi)
             goto bad_cmd;
         switch (sd->state) {
@@ -1319,7 +1319,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
         }
         break;
 
-    case 27:	/* CMD27:  PROGRAM_CSD */
+    case 27:    /* CMD27:  PROGRAM_CSD */
         switch (sd->state) {
         case sd_transfer_state:
             sd->state = sd_receivingdata_state;
@@ -1333,7 +1333,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
         break;
 
     /* Write protection (Class 6) */
-    case 28:	/* CMD28:  SET_WRITE_PROT */
+    case 28:    /* CMD28:  SET_WRITE_PROT */
         switch (sd->state) {
         case sd_transfer_state:
             if (addr >= sd->size) {
@@ -1352,7 +1352,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
         }
         break;
 
-    case 29:	/* CMD29:  CLR_WRITE_PROT */
+    case 29:    /* CMD29:  CLR_WRITE_PROT */
         switch (sd->state) {
         case sd_transfer_state:
             if (addr >= sd->size) {
@@ -1371,7 +1371,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
         }
         break;
 
-    case 30:	/* CMD30:  SEND_WRITE_PROT */
+    case 30:    /* CMD30:  SEND_WRITE_PROT */
         switch (sd->state) {
         case sd_transfer_state:
             sd->state = sd_sendingdata_state;
@@ -1386,7 +1386,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
         break;
 
     /* Erase commands (Class 5) */
-    case 32:	/* CMD32:  ERASE_WR_BLK_START */
+    case 32:    /* CMD32:  ERASE_WR_BLK_START */
         switch (sd->state) {
         case sd_transfer_state:
             sd->erase_start = req.arg;
@@ -1397,7 +1397,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
         }
         break;
 
-    case 33:	/* CMD33:  ERASE_WR_BLK_END */
+    case 33:    /* CMD33:  ERASE_WR_BLK_END */
         switch (sd->state) {
         case sd_transfer_state:
             sd->erase_end = req.arg;
@@ -1408,7 +1408,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
         }
         break;
 
-    case 38:	/* CMD38:  ERASE */
+    case 38:    /* CMD38:  ERASE */
         switch (sd->state) {
         case sd_transfer_state:
             if (sd->csd[14] & 0x30) {
@@ -1428,7 +1428,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
         break;
 
     /* Lock card commands (Class 7) */
-    case 42:	/* CMD42:  LOCK_UNLOCK */
+    case 42:    /* CMD42:  LOCK_UNLOCK */
         switch (sd->state) {
         case sd_transfer_state:
             sd->state = sd_receivingdata_state;
@@ -1451,7 +1451,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
         return sd_illegal;
 
     /* Application specific commands (Class 8) */
-    case 55:	/* CMD55:  APP_CMD */
+    case 55:    /* CMD55:  APP_CMD */
         switch (sd->state) {
         case sd_ready_state:
         case sd_identification_state:
@@ -1474,7 +1474,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
         sd->card_status |= APP_CMD;
         return sd_r1;
 
-    case 56:	/* CMD56:  GEN_CMD */
+    case 56:    /* CMD56:  GEN_CMD */
         switch (sd->state) {
         case sd_transfer_state:
             sd->data_offset = 0;
@@ -1518,7 +1518,7 @@ static sd_rsp_type_t sd_app_command(SDState *sd,
                              req.cmd, req.arg, sd_state_name(sd->state));
     sd->card_status |= APP_CMD;
     switch (req.cmd) {
-    case 6:	/* ACMD6:  SET_BUS_WIDTH */
+    case 6: /* ACMD6:  SET_BUS_WIDTH */
         if (sd->spi) {
             goto unimplemented_spi_cmd;
         }
@@ -1533,7 +1533,7 @@ static sd_rsp_type_t sd_app_command(SDState *sd,
         }
         break;
 
-    case 13:	/* ACMD13: SD_STATUS */
+    case 13:    /* ACMD13: SD_STATUS */
         switch (sd->state) {
         case sd_transfer_state:
             sd->state = sd_sendingdata_state;
@@ -1546,7 +1546,7 @@ static sd_rsp_type_t sd_app_command(SDState *sd,
         }
         break;
 
-    case 22:	/* ACMD22: SEND_NUM_WR_BLOCKS */
+    case 22:    /* ACMD22: SEND_NUM_WR_BLOCKS */
         switch (sd->state) {
         case sd_transfer_state:
             *(uint32_t *) sd->data = sd->blk_written;
@@ -1561,7 +1561,7 @@ static sd_rsp_type_t sd_app_command(SDState *sd,
         }
         break;
 
-    case 23:	/* ACMD23: SET_WR_BLK_ERASE_COUNT */
+    case 23:    /* ACMD23: SET_WR_BLK_ERASE_COUNT */
         switch (sd->state) {
         case sd_transfer_state:
             return sd_r1;
@@ -1571,7 +1571,7 @@ static sd_rsp_type_t sd_app_command(SDState *sd,
         }
         break;
 
-    case 41:	/* ACMD41: SD_APP_OP_COND */
+    case 41:    /* ACMD41: SD_APP_OP_COND */
         if (sd->spi) {
             /* SEND_OP_CMD */
             sd->state = sd_transfer_state;
@@ -1613,7 +1613,7 @@ static sd_rsp_type_t sd_app_command(SDState *sd,
 
         return sd_r3;
 
-    case 42:	/* ACMD42: SET_CLR_CARD_DETECT */
+    case 42:    /* ACMD42: SET_CLR_CARD_DETECT */
         switch (sd->state) {
         case sd_transfer_state:
             /* Bringing in the 50KOhm pull-up resistor... Done.  */
@@ -1624,7 +1624,7 @@ static sd_rsp_type_t sd_app_command(SDState *sd,
         }
         break;
 
-    case 51:	/* ACMD51: SEND_SCR */
+    case 51:    /* ACMD51: SEND_SCR */
         switch (sd->state) {
         case sd_transfer_state:
             sd->state = sd_sendingdata_state;
@@ -1808,9 +1808,9 @@ static void sd_blk_write(SDState *sd, uint64_t addr, uint32_t len)
     }
 }
 
-#define BLK_READ_BLOCK(a, len)	sd_blk_read(sd, a, len)
-#define BLK_WRITE_BLOCK(a, len)	sd_blk_write(sd, a, len)
-#define APP_READ_BLOCK(a, len)	memset(sd->data, 0xec, len)
+#define BLK_READ_BLOCK(a, len)  sd_blk_read(sd, a, len)
+#define BLK_WRITE_BLOCK(a, len) sd_blk_write(sd, a, len)
+#define APP_READ_BLOCK(a, len)  memset(sd->data, 0xec, len)
 #define APP_WRITE_BLOCK(a, len)
 
 void sd_write_byte(SDState *sd, uint8_t value)
@@ -1833,7 +1833,7 @@ void sd_write_byte(SDState *sd, uint8_t value)
                             sd_acmd_name(sd->current_cmd),
                             sd->current_cmd, value);
     switch (sd->current_cmd) {
-    case 24:	/* CMD24:  WRITE_SINGLE_BLOCK */
+    case 24:    /* CMD24:  WRITE_SINGLE_BLOCK */
         sd->data[sd->data_offset ++] = value;
         if (sd->data_offset >= sd->blk_len) {
             /* TODO: Check CRC before committing */
@@ -1846,7 +1846,7 @@ void sd_write_byte(SDState *sd, uint8_t value)
         }
         break;
 
-    case 25:	/* CMD25:  WRITE_MULTIPLE_BLOCK */
+    case 25:    /* CMD25:  WRITE_MULTIPLE_BLOCK */
         if (sd->data_offset == 0) {
             /* Start of the block - let's check the address is valid */
             if (sd->data_start + sd->blk_len > sd->size) {
@@ -1881,7 +1881,7 @@ void sd_write_byte(SDState *sd, uint8_t value)
         }
         break;
 
-    case 26:	/* CMD26:  PROGRAM_CID */
+    case 26:    /* CMD26:  PROGRAM_CID */
         sd->data[sd->data_offset ++] = value;
         if (sd->data_offset >= sizeof(sd->cid)) {
             /* TODO: Check CRC before committing */
@@ -1900,7 +1900,7 @@ void sd_write_byte(SDState *sd, uint8_t value)
         }
         break;
 
-    case 27:	/* CMD27:  PROGRAM_CSD */
+    case 27:    /* CMD27:  PROGRAM_CSD */
         sd->data[sd->data_offset ++] = value;
         if (sd->data_offset >= sizeof(sd->csd)) {
             /* TODO: Check CRC before committing */
@@ -1924,7 +1924,7 @@ void sd_write_byte(SDState *sd, uint8_t value)
         }
         break;
 
-    case 42:	/* CMD42:  LOCK_UNLOCK */
+    case 42:    /* CMD42:  LOCK_UNLOCK */
         sd->data[sd->data_offset ++] = value;
         if (sd->data_offset >= sd->blk_len) {
             /* TODO: Check CRC before committing */
@@ -1935,7 +1935,7 @@ void sd_write_byte(SDState *sd, uint8_t value)
         }
         break;
 
-    case 56:	/* CMD56:  GEN_CMD */
+    case 56:    /* CMD56:  GEN_CMD */
         sd->data[sd->data_offset ++] = value;
         if (sd->data_offset >= sd->blk_len) {
             APP_WRITE_BLOCK(sd->data_start, sd->data_offset);
@@ -1987,29 +1987,29 @@ uint8_t sd_read_byte(SDState *sd)
                            sd_acmd_name(sd->current_cmd),
                            sd->current_cmd, io_len);
     switch (sd->current_cmd) {
-    case 6:	/* CMD6:   SWITCH_FUNCTION */
+    case 6: /* CMD6:   SWITCH_FUNCTION */
         ret = sd->data[sd->data_offset ++];
 
         if (sd->data_offset >= 64)
             sd->state = sd_transfer_state;
         break;
 
-    case 9:	/* CMD9:   SEND_CSD */
-    case 10:	/* CMD10:  SEND_CID */
+    case 9:     /* CMD9:   SEND_CSD */
+    case 10:    /* CMD10:  SEND_CID */
         ret = sd->data[sd->data_offset ++];
 
         if (sd->data_offset >= 16)
             sd->state = sd_transfer_state;
         break;
 
-    case 13:	/* ACMD13: SD_STATUS */
+    case 13:    /* ACMD13: SD_STATUS */
         ret = sd->sd_status[sd->data_offset ++];
 
         if (sd->data_offset >= sizeof(sd->sd_status))
             sd->state = sd_transfer_state;
         break;
 
-    case 17:	/* CMD17:  READ_SINGLE_BLOCK */
+    case 17:    /* CMD17:  READ_SINGLE_BLOCK */
         if (sd->data_offset == 0)
             BLK_READ_BLOCK(sd->data_start, io_len);
         ret = sd->data[sd->data_offset ++];
@@ -2018,7 +2018,7 @@ uint8_t sd_read_byte(SDState *sd)
             sd->state = sd_transfer_state;
         break;
 
-    case 18:	/* CMD18:  READ_MULTIPLE_BLOCK */
+    case 18:    /* CMD18:  READ_MULTIPLE_BLOCK */
         if (sd->data_offset == 0) {
             if (sd->data_start + io_len > sd->size) {
                 sd->card_status |= ADDRESS_ERROR;
@@ -2049,28 +2049,28 @@ uint8_t sd_read_byte(SDState *sd)
         ret = sd_tuning_block_pattern[sd->data_offset++];
         break;
 
-    case 22:	/* ACMD22: SEND_NUM_WR_BLOCKS */
+    case 22:    /* ACMD22: SEND_NUM_WR_BLOCKS */
         ret = sd->data[sd->data_offset ++];
 
         if (sd->data_offset >= 4)
             sd->state = sd_transfer_state;
         break;
 
-    case 30:	/* CMD30:  SEND_WRITE_PROT */
+    case 30:    /* CMD30:  SEND_WRITE_PROT */
         ret = sd->data[sd->data_offset ++];
 
         if (sd->data_offset >= 4)
             sd->state = sd_transfer_state;
         break;
 
-    case 51:	/* ACMD51: SEND_SCR */
+    case 51:    /* ACMD51: SEND_SCR */
         ret = sd->scr[sd->data_offset ++];
 
         if (sd->data_offset >= sizeof(sd->scr))
             sd->state = sd_transfer_state;
         break;
 
-    case 56:	/* CMD56:  GEN_CMD */
+    case 56:    /* CMD56:  GEN_CMD */
         if (sd->data_offset == 0)
             APP_READ_BLOCK(sd->data_start, sd->blk_len);
         ret = sd->data[sd->data_offset ++];
-- 
2.7.4

[PATCH v2 03/22] sd: emmc: Add support for eMMC cards

[Sai Pavan Boddu]

From: Vincent Palatin <vpalatin@chromium.org>

This patch adds support for eMMC cards, change SET_RELATIVE_ADDR
command to assign relative address as requested by user.

Signed-off-by: Vincent Palatin <vpalatin@chromium.org>
Signed-off-by: Joel Stanley <joel@jms.id.au>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
[spb: Split original patch series]
Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
---
 hw/sd/sd.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 74b9162..42ee49c 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -108,6 +108,7 @@ struct SDState {
     uint8_t spec_version;
     BlockBackend *blk;
     bool spi;
+    bool emmc;
 
     /* Runtime changeables */
 
@@ -431,9 +432,13 @@ static void sd_set_csd(SDState *sd, uint64_t size)
     sd->csd[15] = (sd_crc7(sd->csd, 15) << 1) | 1;
 }
 
-static void sd_set_rca(SDState *sd)
+static void sd_set_rca(SDState *sd, uint16_t value)
 {
-    sd->rca += 0x4567;
+    if (sd->emmc) {
+        sd->rca = value;
+    } else {
+        sd->rca += 0x4567;
+    }
 }
 
 FIELD(CSR, AKE_SEQ_ERROR,               3,  1)
@@ -979,8 +984,8 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
         case sd_identification_state:
         case sd_standby_state:
             sd->state = sd_standby_state;
-            sd_set_rca(sd);
-            return sd_r6;
+            sd_set_rca(sd, req.arg >> 16);
+            return sd->emmc ? sd_r1 : sd_r6;
 
         default:
             break;
@@ -2176,6 +2181,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_BOOL("emmc", SDState, emmc, false),
     DEFINE_PROP_END_OF_LIST()
 };
 
-- 
2.7.4

[PATCH v2 04/22] sd: emmc: update OCR fields for eMMC

[Sai Pavan Boddu]

From: Vincent Palatin <vpalatin@chromium.org>

eMMC OCR register doesn't has UHS-II field and voltage fields are
different.

Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
---
 hw/sd/sd.c | 27 ++++++++++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 42ee49c..430bea5 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -283,6 +283,15 @@ FIELD(OCR, UHS_II_CARD,                29,  1) /* Only UHS-II */
 FIELD(OCR, CARD_CAPACITY,              30,  1) /* 0:SDSC, 1:SDHC/SDXC */
 FIELD(OCR, CARD_POWER_UP,              31,  1)
 
+/*
+ * eMMC OCR register
+ */
+FIELD(EMMC_OCR, VDD_WINDOW_0,  7, 1)
+FIELD(EMMC_OCR, VDD_WINDOW_1,  8, 7)
+FIELD(EMMC_OCR, VDD_WINDOW_2, 15, 9)
+FIELD(EMMC_OCR, ACCESS_MODE,  29, 2)
+FIELD(EMMC_OCR, POWER_UP,     31, 1)
+
 #define ACMD41_ENQUIRY_MASK     0x00ffffff
 #define ACMD41_R3_MASK          (R_OCR_VDD_VOLTAGE_WIN_HI_MASK \
                                | R_OCR_ACCEPT_SWITCH_1V8_MASK \
@@ -292,8 +301,16 @@ FIELD(OCR, CARD_POWER_UP,              31,  1)
 
 static void sd_set_ocr(SDState *sd)
 {
-    /* All voltages OK */
-    sd->ocr = R_OCR_VDD_VOLTAGE_WIN_HI_MASK;
+    if (sd->emmc) {
+        /*
+         * Dual Voltage eMMC card
+         */
+        sd->ocr = R_EMMC_OCR_VDD_WINDOW_0_MASK |
+                  R_EMMC_OCR_VDD_WINDOW_2_MASK;
+    } else {
+        /* All voltages OK */
+        sd->ocr = R_OCR_VDD_VOLTAGE_WIN_HI_MASK;
+    }
 }
 
 static void sd_ocr_powerup(void *opaque)
@@ -521,7 +538,11 @@ static void sd_response_r1_make(SDState *sd, uint8_t *response)
 
 static void sd_response_r3_make(SDState *sd, uint8_t *response)
 {
-    stl_be_p(response, sd->ocr & ACMD41_R3_MASK);
+    if (sd->emmc) {
+        stl_be_p(response, sd->ocr);
+    } else {
+        stl_be_p(response, sd->ocr & ACMD41_R3_MASK);
+    }
 }
 
 static void sd_response_r6_make(SDState *sd, uint8_t *response)
-- 
2.7.4

[PATCH v2 05/22] sd: emmc: Add support for EXT_CSD & CSD for eMMC

[Sai Pavan Boddu]

From: Vincent Palatin <vpalatin@chromium.org>

eMMC CSD is similar to SD with an option to refer EXT_CSD for larger
devices.

Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
---
 hw/sd/sd.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 55 insertions(+), 2 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 430bea5..4c211ba 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -135,6 +135,7 @@ struct SDState {
     uint64_t data_start;
     uint32_t data_offset;
     uint8_t data[512];
+    uint8_t ext_csd[512];
     qemu_irq readonly_cb;
     qemu_irq inserted_cb;
     QEMUTimer *ocr_power_timer;
@@ -389,6 +390,51 @@ static const uint8_t sd_csd_rw_mask[16] = {
     0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xfc, 0xfe,
 };
 
+static void mmc_set_ext_csd(SDState *sd, uint64_t size)
+{
+    uint32_t sectcount = size >> HWBLOCK_SHIFT;
+
+    memset(sd->ext_csd, 0, sizeof(sd->ext_csd));
+    sd->ext_csd[504] = 0x1; /* supported command sets */
+    sd->ext_csd[503] = 0x1; /* HPI features  */
+    sd->ext_csd[502] = 0x1; /* Background operations support */
+    sd->ext_csd[241] = 0xA; /* 1st initialization time after partitioning */
+    sd->ext_csd[232] = 0x1; /* Trim multiplier */
+    sd->ext_csd[231] = 0x15; /* Secure feature support */
+    sd->ext_csd[230] = 0x96; /* Secure erase support */
+    sd->ext_csd[229] = 0x96; /* Secure TRIM multiplier */
+    sd->ext_csd[228] = 0x7; /* Boot information */
+    sd->ext_csd[226] = 0x8; /* Boot partition size */
+    sd->ext_csd[225] = 0x6; /* Access size */
+    sd->ext_csd[224] = 0x4; /* HC Erase unit size */
+    sd->ext_csd[223] = 0x1; /* HC erase timeout */
+    sd->ext_csd[222] = 0x1; /* Reliable write sector count */
+    sd->ext_csd[221] = 0x4; /* HC write protect group size */
+    sd->ext_csd[220] = 0x8; /* Sleep current VCC  */
+    sd->ext_csd[219] = 0x7; /* Sleep current VCCQ */
+    sd->ext_csd[217] = 0x11; /* Sleep/Awake timeout */
+    sd->ext_csd[215] = (sectcount >> 24) & 0xff; /* Sector count */
+    sd->ext_csd[214] = (sectcount >> 16) & 0xff; /* ... */
+    sd->ext_csd[213] = (sectcount >> 8) & 0xff;  /* ... */
+    sd->ext_csd[212] = (sectcount & 0xff);       /* ... */
+    sd->ext_csd[210] = 0xa; /* Min write perf for 8bit@52Mhz */
+    sd->ext_csd[209] = 0xa; /* Min read perf for 8bit@52Mhz  */
+    sd->ext_csd[208] = 0xa; /* Min write perf for 4bit@52Mhz */
+    sd->ext_csd[207] = 0xa; /* Min read perf for 4bit@52Mhz */
+    sd->ext_csd[206] = 0xa; /* Min write perf for 4bit@26Mhz */
+    sd->ext_csd[205] = 0xa; /* Min read perf for 4bit@26Mhz */
+    sd->ext_csd[199] = 0x1; /* Partition switching timing */
+    sd->ext_csd[198] = 0x1; /* Out-of-interrupt busy timing */
+    sd->ext_csd[196] = 0xFF; /* Card type */
+    sd->ext_csd[194] = 0x2; /* CSD Structure version */
+    sd->ext_csd[192] = 0x5; /* Extended CSD revision */
+    sd->ext_csd[168] = 0x1; /* RPMB size */
+    sd->ext_csd[160] = 0x3; /* Partinioning support */
+    sd->ext_csd[159] = 0x00; /* Max enhanced area size */
+    sd->ext_csd[158] = 0x00; /* ... */
+    sd->ext_csd[157] = 0xEC; /* ... */
+}
+
 static void sd_set_csd(SDState *sd, uint64_t size)
 {
     int hwblock_shift = HWBLOCK_SHIFT;
@@ -402,8 +448,11 @@ static void sd_set_csd(SDState *sd, uint64_t size)
     }
     csize = (size >> (CMULT_SHIFT + hwblock_shift)) - 1;
 
-    if (size <= SDSC_MAX_CAPACITY) { /* Standard Capacity SD */
-        sd->csd[0] = 0x00;  /* CSD structure */
+    if (size <= SDSC_MAX_CAPACITY || sd->emmc) { /* Standard Capacity SD */
+        if (sd->emmc && size >= SDSC_MAX_CAPACITY) {
+            csize = 0xfff;
+        }
+        sd->csd[0] = sd->emmc ? 0xd0 : 0x00;  /* CSD structure */
         sd->csd[1] = 0x26;  /* Data read access-time-1 */
         sd->csd[2] = 0x00;  /* Data read access-time-2 */
         sd->csd[3] = 0x32;      /* Max. data transfer rate: 25 MHz */
@@ -447,6 +496,10 @@ static void sd_set_csd(SDState *sd, uint64_t size)
         sd->csd[14] = 0x00;
     }
     sd->csd[15] = (sd_crc7(sd->csd, 15) << 1) | 1;
+
+    if (sd->emmc) {
+        mmc_set_ext_csd(sd, size);
+    }
 }
 
 static void sd_set_rca(SDState *sd, uint16_t value)
-- 
2.7.4

[PATCH v2 06/22] sd: emmc: Update CMD8 to send EXT_CSD register

[Sai Pavan Boddu]

From: Vincent Palatin <vpalatin@chromium.org>

Sends the EXT_CSD register as response to CMD8.

Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
---
 hw/sd/sd.c | 52 ++++++++++++++++++++++++++++++++++++----------------
 1 file changed, 36 insertions(+), 16 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 4c211ba..a4f93b5 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1133,24 +1133,37 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
         }
         break;
 
-    case 8: /* CMD8:   SEND_IF_COND */
-        if (sd->spec_version < SD_PHY_SPECv2_00_VERS) {
-            break;
-        }
-        if (sd->state != sd_idle_state) {
-            break;
-        }
-        sd->vhs = 0;
-
-        /* No response if not exactly one VHS bit is set.  */
-        if (!(req.arg >> 8) || (req.arg >> (ctz32(req.arg & ~0xff) + 1))) {
-            return sd->spi ? sd_r7 : sd_r0;
-        }
+    case 8:    /* CMD8:   SEND_IF_COND / SEND_EXT_CSD */
+        if (sd->emmc) {
+            switch (sd->state) {
+            case sd_transfer_state:
+                /* MMC : Sends the EXT_CSD register as a Block of data */
+                sd->state = sd_sendingdata_state;
+                memcpy(sd->data, sd->ext_csd, sizeof(sd->ext_csd));
+                sd->data_start = addr;
+                sd->data_offset = 0;
+                return sd_r1;
+            default:
+                break;
+            }
+        } else {
+            if (sd->spec_version < SD_PHY_SPECv2_00_VERS) {
+                break;
+            }
+            if (sd->state != sd_idle_state) {
+                break;
+            }
+            sd->vhs = 0;
 
-        /* Accept.  */
-        sd->vhs = req.arg;
-        return sd_r7;
+            /* No response if not exactly one VHS bit is set.  */
+            if (!(req.arg >> 8) || (req.arg >> (ctz32(req.arg & ~0xff) + 1))) {
+                return sd->spi ? sd_r7 : sd_r0;
+            }
 
+            /* Accept.  */
+            sd->vhs = req.arg;
+            return sd_r7;
+        }
     case 9: /* CMD9:   SEND_CSD */
         switch (sd->state) {
         case sd_standby_state:
@@ -2073,6 +2086,13 @@ uint8_t sd_read_byte(SDState *sd)
             sd->state = sd_transfer_state;
         break;
 
+    case 8:     /* CMD8: SEND_EXT_CSD on MMC */
+        ret = sd->data[sd->data_offset++];
+        if (sd->data_offset >= sizeof(sd->ext_csd)) {
+            sd->state = sd_transfer_state;
+        }
+        break;
+
     case 9:     /* CMD9:   SEND_CSD */
     case 10:    /* CMD10:  SEND_CID */
         ret = sd->data[sd->data_offset ++];
-- 
2.7.4

[PATCH v2 07/22] sd: sdmmc-internal: Add command string for SEND_OP_CMD

[Sai Pavan Boddu]

From: Cédric Le Goater <clg@kaod.org>

This adds extra info to trace log.

Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
---
 hw/sd/sdmmc-internal.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/sd/sdmmc-internal.c b/hw/sd/sdmmc-internal.c
index 2053def..8648a78 100644
--- a/hw/sd/sdmmc-internal.c
+++ b/hw/sd/sdmmc-internal.c
@@ -14,7 +14,7 @@
 const char *sd_cmd_name(uint8_t cmd)
 {
     static const char *cmd_abbrev[SDMMC_CMD_MAX] = {
-         [0]    = "GO_IDLE_STATE",
+         [0]    = "GO_IDLE_STATE",           [1]    = "SEND_OP_CMD",
          [2]    = "ALL_SEND_CID",            [3]    = "SEND_RELATIVE_ADDR",
          [4]    = "SET_DSR",                 [5]    = "IO_SEND_OP_COND",
          [6]    = "SWITCH_FUNC",             [7]    = "SELECT/DESELECT_CARD",
-- 
2.7.4

[PATCH v2 08/22] sd: emmc: Dont not update CARD_CAPACITY for eMMC cards

[Sai Pavan Boddu]

OCR.CARD_CAPACITY field is only valid for sd cards, So skip it for eMMC.

Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
---
 hw/sd/sd.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index a4f93b5..9835f52 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -324,7 +324,8 @@ static void sd_ocr_powerup(void *opaque)
     /* card power-up OK */
     sd->ocr = FIELD_DP32(sd->ocr, OCR, CARD_POWER_UP, 1);
 
-    if (sd->size > SDSC_MAX_CAPACITY) {
+    /* eMMC supports only Byte mode */
+    if (!sd->emmc && sd->size > SDSC_MAX_CAPACITY) {
         sd->ocr = FIELD_DP32(sd->ocr, OCR, CARD_CAPACITY, 1);
     }
 }
-- 
2.7.4

[PATCH v2 09/22] sd: emmc: Update CMD1 definition for eMMC

[Sai Pavan Boddu]

Add support to Power up the card and send response r3 in case of eMMC.

Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
---
 hw/sd/sd.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 9835f52..8bc8d5d 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1033,8 +1033,16 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
         break;
 
     case 1: /* CMD1:   SEND_OP_CMD */
-        if (!sd->spi)
+        /* MMC: Powerup & send r3
+         * SD: send r1 in spi mode
+         */
+        if (sd->emmc) {
+            sd_ocr_powerup(sd);
+            return sd->state == sd_idle_state ?
+                   sd_r3 : sd_r0;
+        } else if (!sd->spi) {
             goto bad_cmd;
+        }
 
         sd->state = sd_transfer_state;
         return sd_r1;
-- 
2.7.4

[PATCH v2 10/22] sd: emmc: support idle state in CMD2

[Sai Pavan Boddu]

eMMC is expected to be in idle-state post CMD1. Ready state is an
intermediate stage which we don't come across in Device identification
mode.

Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
---
 hw/sd/sd.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 8bc8d5d..ae5c5e8 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1051,6 +1051,10 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
         if (sd->spi)
             goto bad_cmd;
         switch (sd->state) {
+        case sd_idle_state:
+            if (!sd->emmc) {
+                break;
+            }
         case sd_ready_state:
             sd->state = sd_identification_state;
             return sd_r2_i;
-- 
2.7.4

[PATCH v2 11/22] sd: emmc: Add mmc switch function support

[Sai Pavan Boddu]

switch operation in eMMC card updates the ext_csd register to
request changes in card operations. Here we implement similar
sequence but requests are mostly dummy and make no change.

Implement SWITCH_ERROR if the write operation extends goes beyond length
of ext_csd.

Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 hw/sd/sd.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 50 insertions(+), 6 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index ae5c5e8..e50d40b 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -515,6 +515,7 @@ static void sd_set_rca(SDState *sd, uint16_t value)
 FIELD(CSR, AKE_SEQ_ERROR,               3,  1)
 FIELD(CSR, APP_CMD,                     5,  1)
 FIELD(CSR, FX_EVENT,                    6,  1)
+FIELD(CSR, SWITCH_ERROR,                7,  1)
 FIELD(CSR, READY_FOR_DATA,              8,  1)
 FIELD(CSR, CURRENT_STATE,               9,  4)
 FIELD(CSR, ERASE_RESET,                13,  1)
@@ -878,6 +879,43 @@ static uint32_t sd_wpbits(SDState *sd, uint64_t addr)
     return ret;
 }
 
+enum {
+    MMC_CMD6_ACCESS_COMMAND_SET = 0,
+    MMC_CMD6_ACCESS_SET_BITS,
+    MMC_CMD6_ACCESS_CLEAR_BITS,
+    MMC_CMD6_ACCESS_WRITE_BYTE,
+};
+
+static void mmc_function_switch(SDState *sd, uint32_t arg)
+{
+    uint32_t access = extract32(arg, 24, 2);
+    uint32_t index = extract32(arg, 16, 8);
+    uint32_t value = extract32(arg, 8, 8);
+    uint8_t b = sd->ext_csd[index];
+
+    switch (access) {
+    case MMC_CMD6_ACCESS_COMMAND_SET:
+        qemu_log_mask(LOG_UNIMP, "MMC Command set switching not supported\n");
+        return;
+    case MMC_CMD6_ACCESS_SET_BITS:
+        b |= value;
+        break;
+    case MMC_CMD6_ACCESS_CLEAR_BITS:
+        b &= ~value;
+        break;
+    case MMC_CMD6_ACCESS_WRITE_BYTE:
+        b = value;
+        break;
+    }
+
+    if (index >= 192) {
+        sd->card_status |= R_CSR_SWITCH_ERROR_MASK;
+        return;
+    }
+
+    sd->ext_csd[index] = b;
+}
+
 static void sd_function_switch(SDState *sd, uint32_t arg)
 {
     int i, mode, new_func;
@@ -1097,12 +1135,18 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
     case 6: /* CMD6:   SWITCH_FUNCTION */
         switch (sd->mode) {
         case sd_data_transfer_mode:
-            sd_function_switch(sd, req.arg);
-            sd->state = sd_sendingdata_state;
-            sd->data_start = 0;
-            sd->data_offset = 0;
-            return sd_r1;
-
+            if (sd->emmc) {
+                sd->state = sd_programming_state;
+                mmc_function_switch(sd, req.arg);
+                sd->state = sd_transfer_state;
+                return sd_r1b;
+            } else {
+                sd_function_switch(sd, req.arg);
+                sd->state = sd_sendingdata_state;
+                sd->data_start = 0;
+                sd->data_offset = 0;
+                return sd_r1;
+            }
         default:
             break;
         }
-- 
2.7.4

[PATCH v2 12/22] sd: emmc: add CMD21 tuning sequence

[Sai Pavan Boddu]

eMMC cards support tuning sequence for entering HS200 mode.

Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 hw/sd/sd.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index e50d40b..d702027 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1378,6 +1378,14 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
         }
         break;
 
+    case 21:    /* CMD21: mmc SEND TUNING_BLOCK */
+        if (sd->emmc && (sd->state == sd_transfer_state)) {
+            sd->state = sd_sendingdata_state;
+            sd->data_offset = 0;
+            return sd_r1;
+        }
+        break;
+
     case 23:    /* CMD23: SET_BLOCK_COUNT */
         if (sd->spec_version < SD_PHY_SPECv3_01_VERS) {
             break;
@@ -2112,6 +2120,30 @@ static const uint8_t sd_tuning_block_pattern[SD_TUNING_BLOCK_SIZE] = {
     0xbb, 0xff, 0xf7, 0xff,         0xf7, 0x7f, 0x7b, 0xde,
 };
 
+#define EXCSD_BUS_WIDTH_OFFSET 183
+#define BUS_WIDTH_8_MASK    0x4
+#define BUS_WIDTH_4_MASK    0x2
+#define MMC_TUNING_BLOCK_SIZE   128
+
+static const uint8_t mmc_tunning_block_pattern[128] = {
+       0xff, 0xff, 0x00, 0xff, 0xff, 0xff, 0x00, 0x00,
+       0xff, 0xff, 0xcc, 0xcc, 0xcc, 0x33, 0xcc, 0xcc,
+       0xcc, 0x33, 0x33, 0xcc, 0xcc, 0xcc, 0xff, 0xff,
+       0xff, 0xee, 0xff, 0xff, 0xff, 0xee, 0xee, 0xff,
+       0xff, 0xff, 0xdd, 0xff, 0xff, 0xff, 0xdd, 0xdd,
+       0xff, 0xff, 0xff, 0xbb, 0xff, 0xff, 0xff, 0xbb,
+       0xbb, 0xff, 0xff, 0xff, 0x77, 0xff, 0xff, 0xff,
+       0x77, 0x77, 0xff, 0x77, 0xbb, 0xdd, 0xee, 0xff,
+       0xff, 0xff, 0xff, 0x00, 0xff, 0xff, 0xff, 0x00,
+       0x00, 0xff, 0xff, 0xcc, 0xcc, 0xcc, 0x33, 0xcc,
+       0xcc, 0xcc, 0x33, 0x33, 0xcc, 0xcc, 0xcc, 0xff,
+       0xff, 0xff, 0xee, 0xff, 0xff, 0xff, 0xee, 0xee,
+       0xff, 0xff, 0xff, 0xdd, 0xff, 0xff, 0xff, 0xdd,
+       0xdd, 0xff, 0xff, 0xff, 0xbb, 0xff, 0xff, 0xff,
+       0xbb, 0xbb, 0xff, 0xff, 0xff, 0x77, 0xff, 0xff,
+       0xff, 0x77, 0x77, 0xff, 0x77, 0xbb, 0xdd, 0xee,
+};
+
 uint8_t sd_read_byte(SDState *sd)
 {
     /* TODO: Append CRCs */
@@ -2205,6 +2237,21 @@ uint8_t sd_read_byte(SDState *sd)
         ret = sd_tuning_block_pattern[sd->data_offset++];
         break;
 
+    case 21:    /* CMD21: SEND_TUNNING_BLOCK (MMC) */
+        if (sd->data_offset >= MMC_TUNING_BLOCK_SIZE - 1) {
+            sd->state = sd_transfer_state;
+        }
+        if (sd->ext_csd[EXCSD_BUS_WIDTH_OFFSET] & BUS_WIDTH_8_MASK) {
+            ret = mmc_tunning_block_pattern[sd->data_offset++];
+        } else {
+            /* Return LSB Nibbles of two byte from the 8bit tuning block
+             * for 4bit mode
+             */
+            ret = mmc_tunning_block_pattern[sd->data_offset++] & 0x0F;
+            ret |= (mmc_tunning_block_pattern[sd->data_offset++] & 0x0F) << 4;
+        }
+        break;
+
     case 22:    /* ACMD22: SEND_NUM_WR_BLOCKS */
         ret = sd->data[sd->data_offset ++];
 
-- 
2.7.4

[PATCH v2 13/22] sd: emmc: Make ACMD41 illegal for mmc

[Sai Pavan Boddu]

ACMD41 is not applicable for eMMC.

Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 hw/sd/sd.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index d702027..df82b61 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1729,6 +1729,9 @@ static sd_rsp_type_t sd_app_command(SDState *sd,
         break;
 
     case 41:    /* ACMD41: SD_APP_OP_COND */
+        if (sd->emmc) {
+            break;
+        }
         if (sd->spi) {
             /* SEND_OP_CMD */
             sd->state = sd_transfer_state;
-- 
2.7.4

[PATCH v2 14/22] sd: emmc: Add support for emmc erase

[Sai Pavan Boddu]

Add CMD35 and CMD36 which sets the erase start and end.

Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 hw/sd/sd.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index df82b61..6d2ef2b 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1544,6 +1544,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
 
     /* Erase commands (Class 5) */
     case 32:    /* CMD32:  ERASE_WR_BLK_START */
+    case 35:    /* CMD35:  ERASE_GROUP_START */
         switch (sd->state) {
         case sd_transfer_state:
             sd->erase_start = req.arg;
@@ -1555,6 +1556,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
         break;
 
     case 33:    /* CMD33:  ERASE_WR_BLK_END */
+    case 36:    /* CMD36:  ERASE_GROUP_END */
         switch (sd->state) {
         case sd_transfer_state:
             sd->erase_end = req.arg;
-- 
2.7.4

[PATCH v2 15/22] sd: emmc: Update CID structure for eMMC

[Sai Pavan Boddu]

CID structure is little different for eMMC, w.r.t to product name and
manufacturing date.

Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 hw/sd/sd.c | 47 ++++++++++++++++++++++++++++++-----------------
 1 file changed, 30 insertions(+), 17 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 6d2ef2b..3c24810 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -361,23 +361,36 @@ static void sd_set_scr(SDState *sd)
 
 static void sd_set_cid(SDState *sd)
 {
-    sd->cid[0] = MID;       /* Fake card manufacturer ID (MID) */
-    sd->cid[1] = OID[0];    /* OEM/Application ID (OID) */
-    sd->cid[2] = OID[1];
-    sd->cid[3] = PNM[0];    /* Fake product name (PNM) */
-    sd->cid[4] = PNM[1];
-    sd->cid[5] = PNM[2];
-    sd->cid[6] = PNM[3];
-    sd->cid[7] = PNM[4];
-    sd->cid[8] = PRV;       /* Fake product revision (PRV) */
-    sd->cid[9] = 0xde;      /* Fake serial number (PSN) */
-    sd->cid[10] = 0xad;
-    sd->cid[11] = 0xbe;
-    sd->cid[12] = 0xef;
-    sd->cid[13] = 0x00 |    /* Manufacture date (MDT) */
-        ((MDT_YR - 2000) / 10);
-    sd->cid[14] = ((MDT_YR % 10) << 4) | MDT_MON;
-    sd->cid[15] = (sd_crc7(sd->cid, 15) << 1) | 1;
+    if (sd->emmc) {
+        sd->cid[0] = MID;
+        sd->cid[1] = 0x1;       /* CBX */
+        sd->cid[2] = OID[0];    /* OEM/Application ID (OID) */
+        sd->cid[8] = 0x0;
+        sd->cid[9] = PRV;        /* Fake product revision (PRV) */
+        sd->cid[10] = 0xde;      /* Fake serial number (PSN) */
+        sd->cid[11] = 0xad;
+        sd->cid[12] = 0xbe;
+        sd->cid[13] = 0xef;
+        sd->cid[14] = ((MDT_YR - 1997) % 0x10); /* MDT */
+    } else {
+        sd->cid[0] = MID;       /* Fake card manufacturer ID (MID) */
+        sd->cid[1] = OID[0];    /* OEM/Application ID (OID) */
+        sd->cid[2] = OID[1];
+        sd->cid[8] = PRV;       /* Fake product revision (PRV) */
+        sd->cid[9] = 0xde;      /* Fake serial number (PSN) */
+        sd->cid[10] = 0xad;
+        sd->cid[11] = 0xbe;
+        sd->cid[12] = 0xef;
+        sd->cid[13] = 0x00 |    /* Manufacture date (MDT) */
+            ((MDT_YR - 2000) / 10);
+        sd->cid[14] = ((MDT_YR % 10) << 4) | MDT_MON;
+   }
+   sd->cid[3] = PNM[0];    /* Fake product name (PNM) 48bit */
+   sd->cid[4] = PNM[1];
+   sd->cid[5] = PNM[2];
+   sd->cid[6] = PNM[3];
+   sd->cid[7] = PNM[4];
+   sd->cid[15] = (sd_crc7(sd->cid, 15) << 1) | 1;
 }
 
 #define HWBLOCK_SHIFT   9           /* 512 bytes */
-- 
2.7.4

[PATCH v2 16/22] sd: emmc: Add Extended CSD register definitions

[Sai Pavan Boddu]

From: Cédric Le Goater <clg@kaod.org>

Add user friendly macros for EXT_CSD register.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
[spb: Rebased over versal emmc series,
      updated commit message]
Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
---
 hw/sd/sdmmc-internal.h | 97 ++++++++++++++++++++++++++++++++++++++++++++++++++
 hw/sd/sd.c             | 54 +++++++++++++++-------------
 2 files changed, 126 insertions(+), 25 deletions(-)

diff --git a/hw/sd/sdmmc-internal.h b/hw/sd/sdmmc-internal.h
index d8bf17d..7ab7b4d 100644
--- a/hw/sd/sdmmc-internal.h
+++ b/hw/sd/sdmmc-internal.h
@@ -37,4 +37,101 @@ const char *sd_cmd_name(uint8_t cmd);
  */
 const char *sd_acmd_name(uint8_t cmd);
 
+/*
+ * EXT_CSD fields
+ */
+
+#define EXT_CSD_CMDQ_MODE_EN    15  /* R/W */
+#define EXT_CSD_FLUSH_CACHE   32      /* W */
+#define EXT_CSD_CACHE_CTRL    33      /* R/W */
+#define EXT_CSD_POWER_OFF_NOTIFICATION  34  /* R/W */
+#define EXT_CSD_PACKED_FAILURE_INDEX  35  /* RO */
+#define EXT_CSD_PACKED_CMD_STATUS 36  /* RO */
+#define EXT_CSD_EXP_EVENTS_STATUS 54  /* RO, 2 bytes */
+#define EXT_CSD_EXP_EVENTS_CTRL   56  /* R/W, 2 bytes */
+#define EXT_CSD_DATA_SECTOR_SIZE  61  /* R */
+#define EXT_CSD_GP_SIZE_MULT    143 /* R/W */
+#define EXT_CSD_PARTITION_SETTING_COMPLETED 155 /* R/W */
+#define EXT_CSD_PARTITION_ATTRIBUTE 156 /* R/W */
+#define EXT_CSD_PARTITION_SUPPORT 160 /* RO */
+#define EXT_CSD_HPI_MGMT    161 /* R/W */
+#define EXT_CSD_RST_N_FUNCTION    162 /* R/W */
+#define EXT_CSD_BKOPS_EN    163 /* R/W */
+#define EXT_CSD_BKOPS_START   164 /* W */
+#define EXT_CSD_SANITIZE_START    165     /* W */
+#define EXT_CSD_WR_REL_PARAM    166 /* RO */
+#define EXT_CSD_RPMB_MULT   168 /* RO */
+#define EXT_CSD_FW_CONFIG   169 /* R/W */
+#define EXT_CSD_BOOT_WP     173 /* R/W */
+#define EXT_CSD_ERASE_GROUP_DEF   175 /* R/W */
+#define EXT_CSD_PART_CONFIG   179 /* R/W */
+#define EXT_CSD_ERASED_MEM_CONT   181 /* RO */
+#define EXT_CSD_BUS_WIDTH   183 /* R/W */
+#define EXT_CSD_STROBE_SUPPORT    184 /* RO */
+#define EXT_CSD_HS_TIMING   185 /* R/W */
+#define EXT_CSD_POWER_CLASS   187 /* R/W */
+#define EXT_CSD_REV     192 /* RO */
+#define EXT_CSD_STRUCTURE   194 /* RO */
+#define EXT_CSD_CARD_TYPE   196 /* RO */
+#define EXT_CSD_DRIVER_STRENGTH   197 /* RO */
+#define EXT_CSD_OUT_OF_INTERRUPT_TIME 198 /* RO */
+#define EXT_CSD_PART_SWITCH_TIME        199     /* RO */
+#define EXT_CSD_PWR_CL_52_195   200 /* RO */
+#define EXT_CSD_PWR_CL_26_195   201 /* RO */
+#define EXT_CSD_PWR_CL_52_360   202 /* RO */
+#define EXT_CSD_PWR_CL_26_360   203 /* RO */
+#define EXT_CSD_SEC_CNT     212 /* RO, 4 bytes */
+#define EXT_CSD_S_A_TIMEOUT   217 /* RO */
+#define EXT_CSD_S_C_VCCQ          219     /* RO */
+#define EXT_CSD_S_C_VCC                 220     /* RO */
+#define EXT_CSD_REL_WR_SEC_C    222 /* RO */
+#define EXT_CSD_HC_WP_GRP_SIZE    221 /* RO */
+#define EXT_CSD_ERASE_TIMEOUT_MULT  223 /* RO */
+#define EXT_CSD_HC_ERASE_GRP_SIZE 224 /* RO */
+#define EXT_CSD_ACC_SIZE    225 /* RO */
+#define EXT_CSD_BOOT_MULT   226 /* RO */
+#define EXT_CSD_BOOT_INFO   228 /* RO */
+#define EXT_CSD_SEC_TRIM_MULT   229 /* RO */
+#define EXT_CSD_SEC_ERASE_MULT    230 /* RO */
+#define EXT_CSD_SEC_FEATURE_SUPPORT 231 /* RO */
+#define EXT_CSD_TRIM_MULT   232 /* RO */
+#define EXT_CSD_PWR_CL_200_195    236 /* RO */
+#define EXT_CSD_PWR_CL_200_360    237 /* RO */
+#define EXT_CSD_PWR_CL_DDR_52_195 238 /* RO */
+#define EXT_CSD_PWR_CL_DDR_52_360 239 /* RO */
+#define EXT_CSD_BKOPS_STATUS    246 /* RO */
+#define EXT_CSD_POWER_OFF_LONG_TIME 247 /* RO */
+#define EXT_CSD_GENERIC_CMD6_TIME 248 /* RO */
+#define EXT_CSD_CACHE_SIZE    249 /* RO, 4 bytes */
+#define EXT_CSD_PWR_CL_DDR_200_360  253 /* RO */
+#define EXT_CSD_FIRMWARE_VERSION  254 /* RO, 8 bytes */
+#define EXT_CSD_PRE_EOL_INFO    267 /* RO */
+#define EXT_CSD_DEVICE_LIFE_TIME_EST_TYP_A  268 /* RO */
+#define EXT_CSD_DEVICE_LIFE_TIME_EST_TYP_B  269 /* RO */
+#define EXT_CSD_CMDQ_DEPTH    307 /* RO */
+#define EXT_CSD_CMDQ_SUPPORT    308 /* RO */
+#define EXT_CSD_SUPPORTED_MODE    493 /* RO */
+#define EXT_CSD_TAG_UNIT_SIZE   498 /* RO */
+#define EXT_CSD_DATA_TAG_SUPPORT  499 /* RO */
+#define EXT_CSD_MAX_PACKED_WRITES 500 /* RO */
+#define EXT_CSD_MAX_PACKED_READS  501 /* RO */
+#define EXT_CSD_BKOPS_SUPPORT   502 /* RO */
+#define EXT_CSD_HPI_FEATURES    503 /* RO */
+#define EXT_CSD_S_CMD_SET   504 /* RO */
+
+/*
+ * EXT_CSD field definitions
+ */
+
+#define EXT_CSD_WR_REL_PARAM_EN   (1 << 2)
+#define EXT_CSD_WR_REL_PARAM_EN_RPMB_REL_WR (1 << 4)
+
+#define EXT_CSD_PART_CONFIG_ACC_MASK  (0x7)
+#define EXT_CSD_PART_CONFIG_ACC_DEFAULT (0x0)
+#define EXT_CSD_PART_CONFIG_ACC_BOOT0 (0x1)
+
+#define EXT_CSD_PART_CONFIG_EN_MASK (0x7 << 3)
+#define EXT_CSD_PART_CONFIG_EN_BOOT0  (0x1 << 3)
+#define EXT_CSD_PART_CONFIG_EN_USER (0x7 << 3)
+
 #endif
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 3c24810..60799aa 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -409,41 +409,45 @@ static void mmc_set_ext_csd(SDState *sd, uint64_t size)
     uint32_t sectcount = size >> HWBLOCK_SHIFT;
 
     memset(sd->ext_csd, 0, sizeof(sd->ext_csd));
-    sd->ext_csd[504] = 0x1; /* supported command sets */
-    sd->ext_csd[503] = 0x1; /* HPI features  */
-    sd->ext_csd[502] = 0x1; /* Background operations support */
+    sd->ext_csd[EXT_CSD_S_CMD_SET] = 0x1; /* supported command sets */
+    sd->ext_csd[EXT_CSD_HPI_FEATURES] = 0x3; /* HPI features  */
+    sd->ext_csd[EXT_CSD_BKOPS_SUPPORT] = 0x1; /* Background operations
+                                                 support */
     sd->ext_csd[241] = 0xA; /* 1st initialization time after partitioning */
-    sd->ext_csd[232] = 0x1; /* Trim multiplier */
-    sd->ext_csd[231] = 0x15; /* Secure feature support */
-    sd->ext_csd[230] = 0x96; /* Secure erase support */
-    sd->ext_csd[229] = 0x96; /* Secure TRIM multiplier */
-    sd->ext_csd[228] = 0x7; /* Boot information */
-    sd->ext_csd[226] = 0x8; /* Boot partition size */
-    sd->ext_csd[225] = 0x6; /* Access size */
-    sd->ext_csd[224] = 0x4; /* HC Erase unit size */
-    sd->ext_csd[223] = 0x1; /* HC erase timeout */
-    sd->ext_csd[222] = 0x1; /* Reliable write sector count */
-    sd->ext_csd[221] = 0x4; /* HC write protect group size */
-    sd->ext_csd[220] = 0x8; /* Sleep current VCC  */
-    sd->ext_csd[219] = 0x7; /* Sleep current VCCQ */
-    sd->ext_csd[217] = 0x11; /* Sleep/Awake timeout */
+    sd->ext_csd[EXT_CSD_TRIM_MULT] = 0x1; /* Trim multiplier */
+    sd->ext_csd[EXT_CSD_SEC_FEATURE_SUPPORT] = 0x15; /* Secure feature
+                                                        support */
+    sd->ext_csd[EXT_CSD_SEC_ERASE_MULT] = 0x96; /* Secure erase support */
+    sd->ext_csd[EXT_CSD_SEC_TRIM_MULT] = 0x96; /* Secure TRIM multiplier */
+    sd->ext_csd[EXT_CSD_BOOT_INFO] = 0x7; /* Boot information */
+    sd->ext_csd[EXT_CSD_BOOT_MULT] = 0x8; /* Boot partition size. 128KB unit */
+    sd->ext_csd[EXT_CSD_ACC_SIZE] = 0x6; /* Access size */
+    sd->ext_csd[EXT_CSD_HC_ERASE_GRP_SIZE] = 0x4; /* HC Erase unit size */
+    sd->ext_csd[EXT_CSD_ERASE_TIMEOUT_MULT] = 0x1; /* HC erase timeout */
+    sd->ext_csd[EXT_CSD_REL_WR_SEC_C] = 0x1; /* Reliable write sector count */
+    sd->ext_csd[EXT_CSD_HC_WP_GRP_SIZE] = 0x4; /* HC write protect group size */
+    sd->ext_csd[EXT_CSD_S_C_VCC] = 0x8; /* Sleep current VCC  */
+    sd->ext_csd[EXT_CSD_S_C_VCCQ] = 0x7; /* Sleep current VCCQ */
+    sd->ext_csd[EXT_CSD_S_A_TIMEOUT] = 0x11; /* Sleep/Awake timeout */
     sd->ext_csd[215] = (sectcount >> 24) & 0xff; /* Sector count */
     sd->ext_csd[214] = (sectcount >> 16) & 0xff; /* ... */
     sd->ext_csd[213] = (sectcount >> 8) & 0xff;  /* ... */
-    sd->ext_csd[212] = (sectcount & 0xff);       /* ... */
+    sd->ext_csd[EXT_CSD_SEC_CNT] = (sectcount & 0xff);       /* ... */
     sd->ext_csd[210] = 0xa; /* Min write perf for 8bit@52Mhz */
     sd->ext_csd[209] = 0xa; /* Min read perf for 8bit@52Mhz  */
     sd->ext_csd[208] = 0xa; /* Min write perf for 4bit@52Mhz */
     sd->ext_csd[207] = 0xa; /* Min read perf for 4bit@52Mhz */
     sd->ext_csd[206] = 0xa; /* Min write perf for 4bit@26Mhz */
     sd->ext_csd[205] = 0xa; /* Min read perf for 4bit@26Mhz */
-    sd->ext_csd[199] = 0x1; /* Partition switching timing */
-    sd->ext_csd[198] = 0x1; /* Out-of-interrupt busy timing */
-    sd->ext_csd[196] = 0xFF; /* Card type */
-    sd->ext_csd[194] = 0x2; /* CSD Structure version */
-    sd->ext_csd[192] = 0x5; /* Extended CSD revision */
-    sd->ext_csd[168] = 0x1; /* RPMB size */
-    sd->ext_csd[160] = 0x3; /* Partinioning support */
+    sd->ext_csd[EXT_CSD_PART_SWITCH_TIME] = 0x1; /* Partition switching
+                                                    timing */
+    sd->ext_csd[EXT_CSD_OUT_OF_INTERRUPT_TIME] = 0x1; /* Out-of-interrupt busy
+                                                         timing */
+    sd->ext_csd[EXT_CSD_CARD_TYPE] = 0xFF; /* Card type */
+    sd->ext_csd[EXT_CSD_STRUCTURE] = 0x2; /* CSD Structure version */
+    sd->ext_csd[EXT_CSD_REV] = 0x5; /* Extended CSD revision */
+    sd->ext_csd[EXT_CSD_RPMB_MULT] = 0x1; /* RPMB size */
+    sd->ext_csd[EXT_CSD_PARTITION_SUPPORT] = 0x3; /* Partinioning support */
     sd->ext_csd[159] = 0x00; /* Max enhanced area size */
     sd->ext_csd[158] = 0x00; /* ... */
     sd->ext_csd[157] = 0xEC; /* ... */
-- 
2.7.4

[PATCH v2 17/22] sd: emmc: Support boot area in emmc image

[Sai Pavan Boddu]

From: Joel Stanley <joel@jms.id.au>

This assumes a specially constructued image:

  dd if=/dev/zero of=mmc-bootarea.img count=2 bs=1M
  dd if=u-boot-spl.bin of=mmc-bootarea.img conv=notrunc
  dd if=u-boot.bin of=mmc-bootarea.img conv=notrunc count=64 bs=1K
  cat mmc-bootarea.img obmc-phosphor-image.wic > mmc.img
  truncate --size 16GB mmc.img
  truncate --size 128MB mmc-bootarea.img

Signed-off-by: Joel Stanley <joel@jms.id.au>
[clg: - changes on the definition names ]
Signed-off-by: Cédric Le Goater <clg@kaod.org>
[spb: use data_start property to access right emmc partition,
      Clean up PARTITION_ENABLE support as incomplete,
      Fix commit message to be generic.]
Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
---
 hw/sd/sd.c | 40 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 60799aa..ab29e54 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1040,6 +1040,34 @@ static void sd_lock_command(SDState *sd)
         sd->card_status &= ~CARD_IS_LOCKED;
 }
 
+/*
+ * This requires a disk image that has two boot partitions inserted at the
+ * beginning of it. The size of the boot partitions are configured in the
+ * ext_csd structure, which is hardcoded in qemu. They are currently set to
+ * 1MB each.
+ */
+static uint32_t sd_bootpart_offset(SDState *sd)
+{
+    unsigned int access = sd->ext_csd[EXT_CSD_PART_CONFIG] &
+        EXT_CSD_PART_CONFIG_ACC_MASK;
+    unsigned int boot_capacity = sd->ext_csd[EXT_CSD_BOOT_MULT] << 17;
+
+    if (!sd->emmc) {
+        return 0;
+    }
+
+    switch (access) {
+    case EXT_CSD_PART_CONFIG_ACC_DEFAULT:
+        return boot_capacity * 2;
+    case EXT_CSD_PART_CONFIG_ACC_BOOT0:
+        return 0;
+    case EXT_CSD_PART_CONFIG_ACC_BOOT0 + 1:
+        return boot_capacity * 1;
+    default:
+         g_assert_not_reached();
+    }
+}
+
 static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
 {
     uint32_t rca = 0x0000;
@@ -1355,6 +1383,9 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
                 return sd_r1;
             }
 
+            if (sd->emmc) {
+                addr += sd_bootpart_offset(sd);
+            }
             sd->state = sd_sendingdata_state;
             sd->data_start = addr;
             sd->data_offset = 0;
@@ -1374,6 +1405,9 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
                 return sd_r1;
             }
 
+            if (sd->emmc) {
+                addr += sd_bootpart_offset(sd);
+            }
             sd->state = sd_sendingdata_state;
             sd->data_start = addr;
             sd->data_offset = 0;
@@ -1430,6 +1464,9 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
                 return sd_r1;
             }
 
+            if (sd->emmc) {
+                addr += sd_bootpart_offset(sd);
+            }
             sd->state = sd_receivingdata_state;
             sd->data_start = addr;
             sd->data_offset = 0;
@@ -1460,6 +1497,9 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
                 return sd_r1;
             }
 
+            if (sd->emmc) {
+                addr += sd_bootpart_offset(sd);
+            }
             sd->state = sd_receivingdata_state;
             sd->data_start = addr;
             sd->data_offset = 0;
-- 
2.7.4

[PATCH v2 18/22] sd: emmc: Subtract bootarea size from blk

[Sai Pavan Boddu]

From: Joel Stanley <joel@jms.id.au>

The userdata size is derived from the file the user passes on the
command line, but we must take into account the boot areas.

Signed-off-by: Joel Stanley <joel@jms.id.au>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/sd/sd.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index ab29e54..a0b4507 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -653,6 +653,11 @@ static void sd_reset(DeviceState *dev)
     }
     size = sect << 9;
 
+    if (sd->emmc) {
+        unsigned int boot_capacity = sd->ext_csd[EXT_CSD_BOOT_MULT] << 17;
+        size -= boot_capacity * 2;
+    }
+
     sect = sd_addr_to_wpnum(size) + 1;
 
     sd->state = sd_idle_state;
-- 
2.7.4

[PATCH v2 19/22] sd: sdhci: Support eMMC devices

[Sai Pavan Boddu]

Embedded device slots should be allowed as support of eMMC is available.

Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
---
 hw/sd/sdhci.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 8ffa539..771212a 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -99,10 +99,6 @@ static void sdhci_check_capareg(SDHCIState *s, Error **errp)
         msk = FIELD_DP64(msk, SDHC_CAPAB, ASYNC_INT, 0);
 
         val = FIELD_EX64(s->capareg, SDHC_CAPAB, SLOT_TYPE);
-        if (val) {
-            error_setg(errp, "slot-type not supported");
-            return;
-        }
         trace_sdhci_capareg("slot type", val);
         msk = FIELD_DP64(msk, SDHC_CAPAB, SLOT_TYPE, 0);
 
-- 
2.7.4

[PATCH v2 20/22] arm: xlnx-versal: Add emmc to versal

[Sai Pavan Boddu]

Configuring SDHCI-0 to act as eMMC controller.

Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
---
 include/hw/arm/xlnx-versal.h |  1 +
 hw/arm/xlnx-versal-virt.c    | 30 +++++++++++++++++++++++++-----
 hw/arm/xlnx-versal.c         | 13 +++++++++++--
 3 files changed, 37 insertions(+), 7 deletions(-)

diff --git a/include/hw/arm/xlnx-versal.h b/include/hw/arm/xlnx-versal.h
index 2b76885..440f3b4 100644
--- a/include/hw/arm/xlnx-versal.h
+++ b/include/hw/arm/xlnx-versal.h
@@ -76,6 +76,7 @@ struct Versal {
     struct {
         MemoryRegion *mr_ddr;
         uint32_t psci_conduit;
+        bool has_emmc;
     } cfg;
 };
 
diff --git a/hw/arm/xlnx-versal-virt.c b/hw/arm/xlnx-versal-virt.c
index 8482cd6..273e0c7 100644
--- a/hw/arm/xlnx-versal-virt.c
+++ b/hw/arm/xlnx-versal-virt.c
@@ -46,6 +46,7 @@ struct VersalVirt {
 
     struct {
         bool secure;
+        bool has_emmc;
     } cfg;
 };
 
@@ -333,6 +334,13 @@ static void fdt_add_sd_nodes(VersalVirt *s)
         qemu_fdt_setprop_sized_cells(s->fdt, name, "reg",
                                      2, addr, 2, MM_PMC_SD0_SIZE);
         qemu_fdt_setprop(s->fdt, name, "compatible", compat, sizeof(compat));
+        /*
+         * eMMC specific properties
+         */
+        if (s->cfg.has_emmc && i == 0) {
+            qemu_fdt_setprop(s->fdt, name, "non-removable", NULL, 0);
+            qemu_fdt_setprop_sized_cells(s->fdt, name, "bus-width", 1, 8);
+        }
         g_free(name);
     }
 }
@@ -512,7 +520,7 @@ static void create_virtio_regions(VersalVirt *s)
     }
 }
 
-static void sd_plugin_card(SDHCIState *sd, DriveInfo *di)
+static void sd_plugin_card(SDHCIState *sd, DriveInfo *di, bool emmc)
 {
     BlockBackend *blk = di ? blk_by_legacy_dinfo(di) : NULL;
     DeviceState *card;
@@ -520,15 +528,22 @@ static void sd_plugin_card(SDHCIState *sd, DriveInfo *di)
     card = qdev_new(TYPE_SD_CARD);
     object_property_add_child(OBJECT(sd), "card[*]", OBJECT(card));
     qdev_prop_set_drive_err(card, "drive", blk, &error_fatal);
+    object_property_set_bool(OBJECT(card), "emmc", emmc, &error_fatal);
     qdev_realize_and_unref(card, qdev_get_child_bus(DEVICE(sd), "sd-bus"),
                            &error_fatal);
 }
 
+static void versal_virt_set_emmc(Object *obj, bool value, Error **errp)
+{
+    VersalVirt *s = XLNX_VERSAL_VIRT_MACHINE(obj);
+
+    s->cfg.has_emmc = value;
+}
+
 static void versal_virt_init(MachineState *machine)
 {
     VersalVirt *s = XLNX_VERSAL_VIRT_MACHINE(machine);
     int psci_conduit = QEMU_PSCI_CONDUIT_DISABLED;
-    int i;
 
     /*
      * If the user provides an Operating System to be loaded, we expect them
@@ -560,6 +575,8 @@ static void versal_virt_init(MachineState *machine)
                              &error_abort);
     object_property_set_int(OBJECT(&s->soc), "psci-conduit", psci_conduit,
                             &error_abort);
+    object_property_set_bool(OBJECT(&s->soc), "has-emmc", s->cfg.has_emmc,
+                             &error_abort);
     sysbus_realize(SYS_BUS_DEVICE(&s->soc), &error_fatal);
 
     fdt_create(s);
@@ -581,10 +598,10 @@ static void versal_virt_init(MachineState *machine)
     memory_region_add_subregion_overlap(get_system_memory(),
                                         0, &s->soc.fpd.apu.mr, 0);
 
+    sd_plugin_card(&s->soc.pmc.iou.sd[0],
+        drive_get_next(s->cfg.has_emmc ? IF_EMMC : IF_SD), s->cfg.has_emmc);
     /* Plugin SD cards.  */
-    for (i = 0; i < ARRAY_SIZE(s->soc.pmc.iou.sd); i++) {
-        sd_plugin_card(&s->soc.pmc.iou.sd[i], drive_get_next(IF_SD));
-    }
+    sd_plugin_card(&s->soc.pmc.iou.sd[1], drive_get_next(IF_SD), false);
 
     s->binfo.ram_size = machine->ram_size;
     s->binfo.loader_start = 0x0;
@@ -621,6 +638,9 @@ static void versal_virt_machine_class_init(ObjectClass *oc, void *data)
     mc->default_cpus = XLNX_VERSAL_NR_ACPUS;
     mc->no_cdrom = true;
     mc->default_ram_id = "ddr";
+    object_class_property_add_bool(oc, "emmc",
+        NULL, versal_virt_set_emmc);
+
 }
 
 static const TypeInfo versal_virt_machine_init_typeinfo = {
diff --git a/hw/arm/xlnx-versal.c b/hw/arm/xlnx-versal.c
index 628e77e..67ed1af 100644
--- a/hw/arm/xlnx-versal.c
+++ b/hw/arm/xlnx-versal.c
@@ -230,6 +230,9 @@ static void versal_create_admas(Versal *s, qemu_irq *pic)
 }
 
 #define SDHCI_CAPABILITIES  0x280737ec6481 /* Same as on ZynqMP.  */
+#define SDHCI_EMMC_CAPS ((SDHCI_CAPABILITIES & ~(3 << 30)) | \
+                     (1 << 30))
+
 static void versal_create_sds(Versal *s, qemu_irq *pic)
 {
     int i;
@@ -244,9 +247,14 @@ static void versal_create_sds(Versal *s, qemu_irq *pic)
 
         object_property_set_uint(OBJECT(dev), "sd-spec-version", 3,
                                  &error_fatal);
-        object_property_set_uint(OBJECT(dev), "capareg", SDHCI_CAPABILITIES,
+        object_property_set_uint(OBJECT(dev), "capareg", SDHCI_EMMC_CAPS,
                                  &error_fatal);
-        object_property_set_uint(OBJECT(dev), "uhs", UHS_I, &error_fatal);
+        /*
+         * UHS is not applicable for eMMC
+         */
+        if (!s->cfg.has_emmc || i == 1) {
+            object_property_set_uint(OBJECT(dev), "uhs", UHS_I, &error_fatal);
+        }
         sysbus_realize(SYS_BUS_DEVICE(dev), &error_fatal);
 
         mr = sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 0);
@@ -386,6 +394,7 @@ static Property versal_properties[] = {
     DEFINE_PROP_LINK("ddr", Versal, cfg.mr_ddr, TYPE_MEMORY_REGION,
                      MemoryRegion *),
     DEFINE_PROP_UINT32("psci-conduit", Versal, cfg.psci_conduit, 0),
+    DEFINE_PROP_BOOL("has-emmc", Versal, cfg.has_emmc, false),
     DEFINE_PROP_END_OF_LIST()
 };
 
-- 
2.7.4

[PATCH v2 21/22] docs: devel: emmc: Add a doc for emmc card emulation

[Sai Pavan Boddu]

Add few simple steps to create emmc card with boot and user data
partitions.

Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
---
 docs/devel/emmc.txt | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)
 create mode 100644 docs/devel/emmc.txt

diff --git a/docs/devel/emmc.txt b/docs/devel/emmc.txt
new file mode 100644
index 0000000..2d098fe
--- /dev/null
+++ b/docs/devel/emmc.txt
@@ -0,0 +1,16 @@
+====================
+eMMC block emulation
+====================
+
+Any eMMC devices has 3 kinds of partitions Boot, RPMB and User data. We
+are supporting Boot and User data partitions. Boot area partitions are
+expected to be 1MB size as hard coded in EXT_CSD register.
+
+Below is the example of combining two 1MB bootarea partition and
+user data partitions.
+
+      cat mmc-bootarea0.bin mmc-bootarea1.bin  image.wic > mmc.img
+      qemu-img resize mmc.img 4G
+
+Note: mmc-bootarea0/1 are just raw paritions. User data can have
+partition tables.
-- 
2.7.4

[PATCH v2 22/22] docs: arm: xlnx-versal-virt: Add eMMC support documentation

[Sai Pavan Boddu]

Add details of eMMC specific machine property.

Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
---
 docs/system/arm/xlnx-versal-virt.rst | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/docs/system/arm/xlnx-versal-virt.rst b/docs/system/arm/xlnx-versal-virt.rst
index 2602d0f..a48a88d 100644
--- a/docs/system/arm/xlnx-versal-virt.rst
+++ b/docs/system/arm/xlnx-versal-virt.rst
@@ -29,6 +29,7 @@ Implemented devices:
 - 2 GEMs (Cadence MACB Ethernet MACs)
 - 8 ADMA (Xilinx zDMA) channels
 - 2 SD Controllers
+    * SDHCI0 can be configured as eMMC
 - OCM (256KB of On Chip Memory)
 - DDR memory
 
@@ -43,6 +44,15 @@ Other differences between the hardware and the QEMU model:
 - QEMU provides 8 virtio-mmio virtio transports; these start at
   address ``0xa0000000`` and have IRQs from 111 and upwards.
 
+Enabling eMMC
+"""""""""""""
+In order to enable eMMC pass the following machine property "emmc=on".
+ex:
+    "-M xlnx-versal-virt,emmc=on"
+
+Above switch would configure SDHCI0 to accept eMMC card. More details on eMMC
+emulation can be found in docs/devel/emmc.txt.
+
 Running
 """""""
 If the user provides an Operating System to be loaded, we expect users
-- 
2.7.4

Re: [PATCH v2 04/22] sd: emmc: update OCR fields for eMMC

[Cédric Le Goater]

On 2/22/21 9:20 AM, Sai Pavan Boddu wrote:
> From: Vincent Palatin <vpalatin@chromium.org>
> 
> eMMC OCR register doesn't has UHS-II field and voltage fields are
> different.

Can a patch be "From" a person without a "Signed-off-by" of the same 
person ? 

C.
 
> Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
>
> ---
>  hw/sd/sd.c | 27 ++++++++++++++++++++++++---
>  1 file changed, 24 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index 42ee49c..430bea5 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -283,6 +283,15 @@ FIELD(OCR, UHS_II_CARD,                29,  1) /* Only UHS-II */
>  FIELD(OCR, CARD_CAPACITY,              30,  1) /* 0:SDSC, 1:SDHC/SDXC */
>  FIELD(OCR, CARD_POWER_UP,              31,  1)
>  
> +/*
> + * eMMC OCR register
> + */
> +FIELD(EMMC_OCR, VDD_WINDOW_0,  7, 1)
> +FIELD(EMMC_OCR, VDD_WINDOW_1,  8, 7)
> +FIELD(EMMC_OCR, VDD_WINDOW_2, 15, 9)
> +FIELD(EMMC_OCR, ACCESS_MODE,  29, 2)
> +FIELD(EMMC_OCR, POWER_UP,     31, 1)
> +
>  #define ACMD41_ENQUIRY_MASK     0x00ffffff
>  #define ACMD41_R3_MASK          (R_OCR_VDD_VOLTAGE_WIN_HI_MASK \
>                                 | R_OCR_ACCEPT_SWITCH_1V8_MASK \
> @@ -292,8 +301,16 @@ FIELD(OCR, CARD_POWER_UP,              31,  1)
>  
>  static void sd_set_ocr(SDState *sd)
>  {
> -    /* All voltages OK */
> -    sd->ocr = R_OCR_VDD_VOLTAGE_WIN_HI_MASK;
> +    if (sd->emmc) {
> +        /*
> +         * Dual Voltage eMMC card
> +         */
> +        sd->ocr = R_EMMC_OCR_VDD_WINDOW_0_MASK |
> +                  R_EMMC_OCR_VDD_WINDOW_2_MASK;
> +    } else {
> +        /* All voltages OK */
> +        sd->ocr = R_OCR_VDD_VOLTAGE_WIN_HI_MASK;
> +    }
>  }
>  
>  static void sd_ocr_powerup(void *opaque)
> @@ -521,7 +538,11 @@ static void sd_response_r1_make(SDState *sd, uint8_t *response)
>  
>  static void sd_response_r3_make(SDState *sd, uint8_t *response)
>  {
> -    stl_be_p(response, sd->ocr & ACMD41_R3_MASK);
> +    if (sd->emmc) {
> +        stl_be_p(response, sd->ocr);
> +    } else {
> +        stl_be_p(response, sd->ocr & ACMD41_R3_MASK);
> +    }
>  }
>  
>  static void sd_response_r6_make(SDState *sd, uint8_t *response)
> 

Re: [PATCH v2 16/22] sd: emmc: Add Extended CSD register definitions

[Cédric Le Goater]

On 2/22/21 9:20 AM, Sai Pavan Boddu wrote:
> From: Cédric Le Goater <clg@kaod.org>
> 
> Add user friendly macros for EXT_CSD register.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> [spb: Rebased over versal emmc series,
>       updated commit message]
> Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>


You can merge this patch in patch 05 "sd: emmc: Add support for 
EXT_CSD & CSD for eMMC" if you wish.

C.

> ---
>  hw/sd/sdmmc-internal.h | 97 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  hw/sd/sd.c             | 54 +++++++++++++++-------------
>  2 files changed, 126 insertions(+), 25 deletions(-)
> 
> diff --git a/hw/sd/sdmmc-internal.h b/hw/sd/sdmmc-internal.h
> index d8bf17d..7ab7b4d 100644
> --- a/hw/sd/sdmmc-internal.h
> +++ b/hw/sd/sdmmc-internal.h
> @@ -37,4 +37,101 @@ const char *sd_cmd_name(uint8_t cmd);
>   */
>  const char *sd_acmd_name(uint8_t cmd);
>  
> +/*
> + * EXT_CSD fields
> + */
> +
> +#define EXT_CSD_CMDQ_MODE_EN    15  /* R/W */
> +#define EXT_CSD_FLUSH_CACHE   32      /* W */
> +#define EXT_CSD_CACHE_CTRL    33      /* R/W */
> +#define EXT_CSD_POWER_OFF_NOTIFICATION  34  /* R/W */
> +#define EXT_CSD_PACKED_FAILURE_INDEX  35  /* RO */
> +#define EXT_CSD_PACKED_CMD_STATUS 36  /* RO */
> +#define EXT_CSD_EXP_EVENTS_STATUS 54  /* RO, 2 bytes */
> +#define EXT_CSD_EXP_EVENTS_CTRL   56  /* R/W, 2 bytes */
> +#define EXT_CSD_DATA_SECTOR_SIZE  61  /* R */
> +#define EXT_CSD_GP_SIZE_MULT    143 /* R/W */
> +#define EXT_CSD_PARTITION_SETTING_COMPLETED 155 /* R/W */
> +#define EXT_CSD_PARTITION_ATTRIBUTE 156 /* R/W */
> +#define EXT_CSD_PARTITION_SUPPORT 160 /* RO */
> +#define EXT_CSD_HPI_MGMT    161 /* R/W */
> +#define EXT_CSD_RST_N_FUNCTION    162 /* R/W */
> +#define EXT_CSD_BKOPS_EN    163 /* R/W */
> +#define EXT_CSD_BKOPS_START   164 /* W */
> +#define EXT_CSD_SANITIZE_START    165     /* W */
> +#define EXT_CSD_WR_REL_PARAM    166 /* RO */
> +#define EXT_CSD_RPMB_MULT   168 /* RO */
> +#define EXT_CSD_FW_CONFIG   169 /* R/W */
> +#define EXT_CSD_BOOT_WP     173 /* R/W */
> +#define EXT_CSD_ERASE_GROUP_DEF   175 /* R/W */
> +#define EXT_CSD_PART_CONFIG   179 /* R/W */
> +#define EXT_CSD_ERASED_MEM_CONT   181 /* RO */
> +#define EXT_CSD_BUS_WIDTH   183 /* R/W */
> +#define EXT_CSD_STROBE_SUPPORT    184 /* RO */
> +#define EXT_CSD_HS_TIMING   185 /* R/W */
> +#define EXT_CSD_POWER_CLASS   187 /* R/W */
> +#define EXT_CSD_REV     192 /* RO */
> +#define EXT_CSD_STRUCTURE   194 /* RO */
> +#define EXT_CSD_CARD_TYPE   196 /* RO */
> +#define EXT_CSD_DRIVER_STRENGTH   197 /* RO */
> +#define EXT_CSD_OUT_OF_INTERRUPT_TIME 198 /* RO */
> +#define EXT_CSD_PART_SWITCH_TIME        199     /* RO */
> +#define EXT_CSD_PWR_CL_52_195   200 /* RO */
> +#define EXT_CSD_PWR_CL_26_195   201 /* RO */
> +#define EXT_CSD_PWR_CL_52_360   202 /* RO */
> +#define EXT_CSD_PWR_CL_26_360   203 /* RO */
> +#define EXT_CSD_SEC_CNT     212 /* RO, 4 bytes */
> +#define EXT_CSD_S_A_TIMEOUT   217 /* RO */
> +#define EXT_CSD_S_C_VCCQ          219     /* RO */
> +#define EXT_CSD_S_C_VCC                 220     /* RO */
> +#define EXT_CSD_REL_WR_SEC_C    222 /* RO */
> +#define EXT_CSD_HC_WP_GRP_SIZE    221 /* RO */
> +#define EXT_CSD_ERASE_TIMEOUT_MULT  223 /* RO */
> +#define EXT_CSD_HC_ERASE_GRP_SIZE 224 /* RO */
> +#define EXT_CSD_ACC_SIZE    225 /* RO */
> +#define EXT_CSD_BOOT_MULT   226 /* RO */
> +#define EXT_CSD_BOOT_INFO   228 /* RO */
> +#define EXT_CSD_SEC_TRIM_MULT   229 /* RO */
> +#define EXT_CSD_SEC_ERASE_MULT    230 /* RO */
> +#define EXT_CSD_SEC_FEATURE_SUPPORT 231 /* RO */
> +#define EXT_CSD_TRIM_MULT   232 /* RO */
> +#define EXT_CSD_PWR_CL_200_195    236 /* RO */
> +#define EXT_CSD_PWR_CL_200_360    237 /* RO */
> +#define EXT_CSD_PWR_CL_DDR_52_195 238 /* RO */
> +#define EXT_CSD_PWR_CL_DDR_52_360 239 /* RO */
> +#define EXT_CSD_BKOPS_STATUS    246 /* RO */
> +#define EXT_CSD_POWER_OFF_LONG_TIME 247 /* RO */
> +#define EXT_CSD_GENERIC_CMD6_TIME 248 /* RO */
> +#define EXT_CSD_CACHE_SIZE    249 /* RO, 4 bytes */
> +#define EXT_CSD_PWR_CL_DDR_200_360  253 /* RO */
> +#define EXT_CSD_FIRMWARE_VERSION  254 /* RO, 8 bytes */
> +#define EXT_CSD_PRE_EOL_INFO    267 /* RO */
> +#define EXT_CSD_DEVICE_LIFE_TIME_EST_TYP_A  268 /* RO */
> +#define EXT_CSD_DEVICE_LIFE_TIME_EST_TYP_B  269 /* RO */
> +#define EXT_CSD_CMDQ_DEPTH    307 /* RO */
> +#define EXT_CSD_CMDQ_SUPPORT    308 /* RO */
> +#define EXT_CSD_SUPPORTED_MODE    493 /* RO */
> +#define EXT_CSD_TAG_UNIT_SIZE   498 /* RO */
> +#define EXT_CSD_DATA_TAG_SUPPORT  499 /* RO */
> +#define EXT_CSD_MAX_PACKED_WRITES 500 /* RO */
> +#define EXT_CSD_MAX_PACKED_READS  501 /* RO */
> +#define EXT_CSD_BKOPS_SUPPORT   502 /* RO */
> +#define EXT_CSD_HPI_FEATURES    503 /* RO */
> +#define EXT_CSD_S_CMD_SET   504 /* RO */
> +
> +/*
> + * EXT_CSD field definitions
> + */
> +
> +#define EXT_CSD_WR_REL_PARAM_EN   (1 << 2)
> +#define EXT_CSD_WR_REL_PARAM_EN_RPMB_REL_WR (1 << 4)
> +
> +#define EXT_CSD_PART_CONFIG_ACC_MASK  (0x7)
> +#define EXT_CSD_PART_CONFIG_ACC_DEFAULT (0x0)
> +#define EXT_CSD_PART_CONFIG_ACC_BOOT0 (0x1)
> +
> +#define EXT_CSD_PART_CONFIG_EN_MASK (0x7 << 3)
> +#define EXT_CSD_PART_CONFIG_EN_BOOT0  (0x1 << 3)
> +#define EXT_CSD_PART_CONFIG_EN_USER (0x7 << 3)
> +
>  #endif
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index 3c24810..60799aa 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -409,41 +409,45 @@ static void mmc_set_ext_csd(SDState *sd, uint64_t size)
>      uint32_t sectcount = size >> HWBLOCK_SHIFT;
>  
>      memset(sd->ext_csd, 0, sizeof(sd->ext_csd));
> -    sd->ext_csd[504] = 0x1; /* supported command sets */
> -    sd->ext_csd[503] = 0x1; /* HPI features  */
> -    sd->ext_csd[502] = 0x1; /* Background operations support */
> +    sd->ext_csd[EXT_CSD_S_CMD_SET] = 0x1; /* supported command sets */
> +    sd->ext_csd[EXT_CSD_HPI_FEATURES] = 0x3; /* HPI features  */
> +    sd->ext_csd[EXT_CSD_BKOPS_SUPPORT] = 0x1; /* Background operations
> +                                                 support */
>      sd->ext_csd[241] = 0xA; /* 1st initialization time after partitioning */
> -    sd->ext_csd[232] = 0x1; /* Trim multiplier */
> -    sd->ext_csd[231] = 0x15; /* Secure feature support */
> -    sd->ext_csd[230] = 0x96; /* Secure erase support */
> -    sd->ext_csd[229] = 0x96; /* Secure TRIM multiplier */
> -    sd->ext_csd[228] = 0x7; /* Boot information */
> -    sd->ext_csd[226] = 0x8; /* Boot partition size */
> -    sd->ext_csd[225] = 0x6; /* Access size */
> -    sd->ext_csd[224] = 0x4; /* HC Erase unit size */
> -    sd->ext_csd[223] = 0x1; /* HC erase timeout */
> -    sd->ext_csd[222] = 0x1; /* Reliable write sector count */
> -    sd->ext_csd[221] = 0x4; /* HC write protect group size */
> -    sd->ext_csd[220] = 0x8; /* Sleep current VCC  */
> -    sd->ext_csd[219] = 0x7; /* Sleep current VCCQ */
> -    sd->ext_csd[217] = 0x11; /* Sleep/Awake timeout */
> +    sd->ext_csd[EXT_CSD_TRIM_MULT] = 0x1; /* Trim multiplier */
> +    sd->ext_csd[EXT_CSD_SEC_FEATURE_SUPPORT] = 0x15; /* Secure feature
> +                                                        support */
> +    sd->ext_csd[EXT_CSD_SEC_ERASE_MULT] = 0x96; /* Secure erase support */
> +    sd->ext_csd[EXT_CSD_SEC_TRIM_MULT] = 0x96; /* Secure TRIM multiplier */
> +    sd->ext_csd[EXT_CSD_BOOT_INFO] = 0x7; /* Boot information */
> +    sd->ext_csd[EXT_CSD_BOOT_MULT] = 0x8; /* Boot partition size. 128KB unit */
> +    sd->ext_csd[EXT_CSD_ACC_SIZE] = 0x6; /* Access size */
> +    sd->ext_csd[EXT_CSD_HC_ERASE_GRP_SIZE] = 0x4; /* HC Erase unit size */
> +    sd->ext_csd[EXT_CSD_ERASE_TIMEOUT_MULT] = 0x1; /* HC erase timeout */
> +    sd->ext_csd[EXT_CSD_REL_WR_SEC_C] = 0x1; /* Reliable write sector count */
> +    sd->ext_csd[EXT_CSD_HC_WP_GRP_SIZE] = 0x4; /* HC write protect group size */
> +    sd->ext_csd[EXT_CSD_S_C_VCC] = 0x8; /* Sleep current VCC  */
> +    sd->ext_csd[EXT_CSD_S_C_VCCQ] = 0x7; /* Sleep current VCCQ */
> +    sd->ext_csd[EXT_CSD_S_A_TIMEOUT] = 0x11; /* Sleep/Awake timeout */
>      sd->ext_csd[215] = (sectcount >> 24) & 0xff; /* Sector count */
>      sd->ext_csd[214] = (sectcount >> 16) & 0xff; /* ... */
>      sd->ext_csd[213] = (sectcount >> 8) & 0xff;  /* ... */
> -    sd->ext_csd[212] = (sectcount & 0xff);       /* ... */
> +    sd->ext_csd[EXT_CSD_SEC_CNT] = (sectcount & 0xff);       /* ... */
>      sd->ext_csd[210] = 0xa; /* Min write perf for 8bit@52Mhz */
>      sd->ext_csd[209] = 0xa; /* Min read perf for 8bit@52Mhz  */
>      sd->ext_csd[208] = 0xa; /* Min write perf for 4bit@52Mhz */
>      sd->ext_csd[207] = 0xa; /* Min read perf for 4bit@52Mhz */
>      sd->ext_csd[206] = 0xa; /* Min write perf for 4bit@26Mhz */
>      sd->ext_csd[205] = 0xa; /* Min read perf for 4bit@26Mhz */
> -    sd->ext_csd[199] = 0x1; /* Partition switching timing */
> -    sd->ext_csd[198] = 0x1; /* Out-of-interrupt busy timing */
> -    sd->ext_csd[196] = 0xFF; /* Card type */
> -    sd->ext_csd[194] = 0x2; /* CSD Structure version */
> -    sd->ext_csd[192] = 0x5; /* Extended CSD revision */
> -    sd->ext_csd[168] = 0x1; /* RPMB size */
> -    sd->ext_csd[160] = 0x3; /* Partinioning support */
> +    sd->ext_csd[EXT_CSD_PART_SWITCH_TIME] = 0x1; /* Partition switching
> +                                                    timing */
> +    sd->ext_csd[EXT_CSD_OUT_OF_INTERRUPT_TIME] = 0x1; /* Out-of-interrupt busy
> +                                                         timing */
> +    sd->ext_csd[EXT_CSD_CARD_TYPE] = 0xFF; /* Card type */
> +    sd->ext_csd[EXT_CSD_STRUCTURE] = 0x2; /* CSD Structure version */
> +    sd->ext_csd[EXT_CSD_REV] = 0x5; /* Extended CSD revision */
> +    sd->ext_csd[EXT_CSD_RPMB_MULT] = 0x1; /* RPMB size */
> +    sd->ext_csd[EXT_CSD_PARTITION_SUPPORT] = 0x3; /* Partinioning support */
>      sd->ext_csd[159] = 0x00; /* Max enhanced area size */
>      sd->ext_csd[158] = 0x00; /* ... */
>      sd->ext_csd[157] = 0xEC; /* ... */
> 

RE: [PATCH v2 04/22] sd: emmc: update OCR fields for eMMC

[Sai Pavan Boddu]

Hi Cedric

> -----Original Message-----
> From: Cédric Le Goater <clg@kaod.org>
> Sent: Monday, February 22, 2021 3:22 PM
> To: Sai Pavan Boddu <saipava@xilinx.com>; Markus Armbruster
> <armbru@redhat.com>; Kevin Wolf <kwolf@redhat.com>; Max Reitz
> <mreitz@redhat.com>; Vladimir Sementsov-Ogievskiy
> <vsementsov@virtuozzo.com>; Eric Blake <eblake@redhat.com>; Joel Stanley
> <joel@jms.id.au>; Vincent Palatin <vpalatin@chromium.org>; Dr. David Alan
> Gilbert <dgilbert@redhat.com>; Thomas Huth <thuth@redhat.com>; Stefan
> Hajnoczi <stefanha@redhat.com>; Peter Maydell <peter.maydell@linaro.org>;
> Alistair Francis <alistair.francis@wdc.com>; Edgar Iglesias <edgari@xilinx.com>;
> Luc Michel <luc.michel@greensocs.com>; Paolo Bonzini
> <pbonzini@redhat.com>
> Cc: qemu-block@nongnu.org; qemu-devel@nongnu.org; Sai Pavan Boddu
> <saipava@xilinx.com>
> Subject: Re: [PATCH v2 04/22] sd: emmc: update OCR fields for eMMC
> 
> On 2/22/21 9:20 AM, Sai Pavan Boddu wrote:
> > From: Vincent Palatin <vpalatin@chromium.org>
> >
> > eMMC OCR register doesn't has UHS-II field and voltage fields are
> > different.
> 
> Can a patch be "From" a person without a "Signed-off-by" of the same person ?
[Sai Pavan Boddu] No I should not be like this. My mistake, I would respin the series with corrections. Few other patches may have this after the split I did.

Regards,
Sai Pavan
> 
> C.
> 
> > Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
> >
> > ---
> >  hw/sd/sd.c | 27 ++++++++++++++++++++++++---
> >  1 file changed, 24 insertions(+), 3 deletions(-)
> >
> > diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> > index 42ee49c..430bea5 100644
> > --- a/hw/sd/sd.c
> > +++ b/hw/sd/sd.c
> > @@ -283,6 +283,15 @@ FIELD(OCR, UHS_II_CARD,                29,  1) /* Only
> UHS-II */
> >  FIELD(OCR, CARD_CAPACITY,              30,  1) /* 0:SDSC, 1:SDHC/SDXC */
> >  FIELD(OCR, CARD_POWER_UP,              31,  1)
> >
> > +/*
> > + * eMMC OCR register
> > + */
> > +FIELD(EMMC_OCR, VDD_WINDOW_0,  7, 1)
> > +FIELD(EMMC_OCR, VDD_WINDOW_1,  8, 7)
> > +FIELD(EMMC_OCR, VDD_WINDOW_2, 15, 9)
> > +FIELD(EMMC_OCR, ACCESS_MODE,  29, 2)
> > +FIELD(EMMC_OCR, POWER_UP,     31, 1)
> > +
> >  #define ACMD41_ENQUIRY_MASK     0x00ffffff
> >  #define ACMD41_R3_MASK          (R_OCR_VDD_VOLTAGE_WIN_HI_MASK \
> >                                 | R_OCR_ACCEPT_SWITCH_1V8_MASK \
> > @@ -292,8 +301,16 @@ FIELD(OCR, CARD_POWER_UP,              31,  1)
> >
> >  static void sd_set_ocr(SDState *sd)
> >  {
> > -    /* All voltages OK */
> > -    sd->ocr = R_OCR_VDD_VOLTAGE_WIN_HI_MASK;
> > +    if (sd->emmc) {
> > +        /*
> > +         * Dual Voltage eMMC card
> > +         */
> > +        sd->ocr = R_EMMC_OCR_VDD_WINDOW_0_MASK |
> > +                  R_EMMC_OCR_VDD_WINDOW_2_MASK;
> > +    } else {
> > +        /* All voltages OK */
> > +        sd->ocr = R_OCR_VDD_VOLTAGE_WIN_HI_MASK;
> > +    }
> >  }
> >
> >  static void sd_ocr_powerup(void *opaque) @@ -521,7 +538,11 @@ static
> > void sd_response_r1_make(SDState *sd, uint8_t *response)
> >
> >  static void sd_response_r3_make(SDState *sd, uint8_t *response)  {
> > -    stl_be_p(response, sd->ocr & ACMD41_R3_MASK);
> > +    if (sd->emmc) {
> > +        stl_be_p(response, sd->ocr);
> > +    } else {
> > +        stl_be_p(response, sd->ocr & ACMD41_R3_MASK);
> > +    }
> >  }
> >
> >  static void sd_response_r6_make(SDState *sd, uint8_t *response)
> >

Re: [PATCH v2 04/22] sd: emmc: update OCR fields for eMMC

[Philippe Mathieu-Daudé]

On 2/22/21 10:55 AM, Sai Pavan Boddu wrote:
> Hi Cedric
> 
>> -----Original Message-----
>> From: Cédric Le Goater <clg@kaod.org>
>> Sent: Monday, February 22, 2021 3:22 PM
>> To: Sai Pavan Boddu <saipava@xilinx.com>; Markus Armbruster
>> <armbru@redhat.com>; Kevin Wolf <kwolf@redhat.com>; Max Reitz
>> <mreitz@redhat.com>; Vladimir Sementsov-Ogievskiy
>> <vsementsov@virtuozzo.com>; Eric Blake <eblake@redhat.com>; Joel Stanley
>> <joel@jms.id.au>; Vincent Palatin <vpalatin@chromium.org>; Dr. David Alan
>> Gilbert <dgilbert@redhat.com>; Thomas Huth <thuth@redhat.com>; Stefan
>> Hajnoczi <stefanha@redhat.com>; Peter Maydell <peter.maydell@linaro.org>;
>> Alistair Francis <alistair.francis@wdc.com>; Edgar Iglesias <edgari@xilinx.com>;
>> Luc Michel <luc.michel@greensocs.com>; Paolo Bonzini
>> <pbonzini@redhat.com>
>> Cc: qemu-block@nongnu.org; qemu-devel@nongnu.org; Sai Pavan Boddu
>> <saipava@xilinx.com>
>> Subject: Re: [PATCH v2 04/22] sd: emmc: update OCR fields for eMMC
>>
>> On 2/22/21 9:20 AM, Sai Pavan Boddu wrote:
>>> From: Vincent Palatin <vpalatin@chromium.org>
>>>
>>> eMMC OCR register doesn't has UHS-II field and voltage fields are
>>> different.
>>
>> Can a patch be "From" a person without a "Signed-off-by" of the same person ?
> [Sai Pavan Boddu] No I should not be like this. My mistake, I would respin the series with corrections. Few other patches may have this after the split I did.

Please wait for more review before respining for this detail.

Re: [PATCH v2 01/22] block: add eMMC block device type

[Philippe Mathieu-Daudé]

On 2/22/21 9:20 AM, Sai Pavan Boddu wrote:
> From: Vincent Palatin <vpalatin@chromium.org>
> 
> Add new block device type.
> 
> Signed-off-by: Vincent Palatin <vpalatin@chromium.org>
> [SPB: Rebased over 5.1 version]
> Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>  include/sysemu/blockdev.h | 1 +
>  blockdev.c                | 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h
> index 3b5fcda..eefae9f 100644
> --- a/include/sysemu/blockdev.h
> +++ b/include/sysemu/blockdev.h
> @@ -24,6 +24,7 @@ typedef enum {
>       */
>      IF_NONE = 0,
>      IF_IDE, IF_SCSI, IF_FLOPPY, IF_PFLASH, IF_MTD, IF_SD, IF_VIRTIO, IF_XEN,
> +    IF_EMMC,
>      IF_COUNT
>  } BlockInterfaceType;
>  
> diff --git a/blockdev.c b/blockdev.c
> index cd438e6..390d43c 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -83,6 +83,7 @@ static const char *const if_name[IF_COUNT] = {
>      [IF_SD] = "sd",
>      [IF_VIRTIO] = "virtio",
>      [IF_XEN] = "xen",
> +    [IF_EMMC] = "emmc",
>  };

We don't need to introduce support for the legacy -drive magic.

-device should be enough for this device, right?

Re: [PATCH v2 01/22] block: add eMMC block device type

[Markus Armbruster]

Philippe Mathieu-Daudé <philmd@redhat.com> writes:

> On 2/22/21 9:20 AM, Sai Pavan Boddu wrote:
>> From: Vincent Palatin <vpalatin@chromium.org>
>> 
>> Add new block device type.
>> 
>> Signed-off-by: Vincent Palatin <vpalatin@chromium.org>
>> [SPB: Rebased over 5.1 version]
>> Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
>> Signed-off-by: Joel Stanley <joel@jms.id.au>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
>> ---
>>  include/sysemu/blockdev.h | 1 +
>>  blockdev.c                | 1 +
>>  2 files changed, 2 insertions(+)
>> 
>> diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h
>> index 3b5fcda..eefae9f 100644
>> --- a/include/sysemu/blockdev.h
>> +++ b/include/sysemu/blockdev.h
>> @@ -24,6 +24,7 @@ typedef enum {
>>       */
>>      IF_NONE = 0,
>>      IF_IDE, IF_SCSI, IF_FLOPPY, IF_PFLASH, IF_MTD, IF_SD, IF_VIRTIO, IF_XEN,
>> +    IF_EMMC,
>>      IF_COUNT
>>  } BlockInterfaceType;
>>  
>> diff --git a/blockdev.c b/blockdev.c
>> index cd438e6..390d43c 100644
>> --- a/blockdev.c
>> +++ b/blockdev.c
>> @@ -83,6 +83,7 @@ static const char *const if_name[IF_COUNT] = {
>>      [IF_SD] = "sd",
>>      [IF_VIRTIO] = "virtio",
>>      [IF_XEN] = "xen",
>> +    [IF_EMMC] = "emmc",
>>  };
>
> We don't need to introduce support for the legacy -drive magic.
>
> -device should be enough for this device, right?

External interface extensions need rationale: why do we want / need it?
The commit message neglects to provide one.

Even more so when the interface in question is in a state like -drive
is.

Re: [PATCH v2 01/22] block: add eMMC block device type

[Dr. David Alan Gilbert]

* Markus Armbruster (armbru@redhat.com) wrote:
> Philippe Mathieu-Daudé <philmd@redhat.com> writes:
> 
> > On 2/22/21 9:20 AM, Sai Pavan Boddu wrote:
> >> From: Vincent Palatin <vpalatin@chromium.org>
> >> 
> >> Add new block device type.
> >> 
> >> Signed-off-by: Vincent Palatin <vpalatin@chromium.org>
> >> [SPB: Rebased over 5.1 version]
> >> Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
> >> Signed-off-by: Joel Stanley <joel@jms.id.au>
> >> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> >> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> >> ---
> >>  include/sysemu/blockdev.h | 1 +
> >>  blockdev.c                | 1 +
> >>  2 files changed, 2 insertions(+)
> >> 
> >> diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h
> >> index 3b5fcda..eefae9f 100644
> >> --- a/include/sysemu/blockdev.h
> >> +++ b/include/sysemu/blockdev.h
> >> @@ -24,6 +24,7 @@ typedef enum {
> >>       */
> >>      IF_NONE = 0,
> >>      IF_IDE, IF_SCSI, IF_FLOPPY, IF_PFLASH, IF_MTD, IF_SD, IF_VIRTIO, IF_XEN,
> >> +    IF_EMMC,
> >>      IF_COUNT
> >>  } BlockInterfaceType;
> >>  
> >> diff --git a/blockdev.c b/blockdev.c
> >> index cd438e6..390d43c 100644
> >> --- a/blockdev.c
> >> +++ b/blockdev.c
> >> @@ -83,6 +83,7 @@ static const char *const if_name[IF_COUNT] = {
> >>      [IF_SD] = "sd",
> >>      [IF_VIRTIO] = "virtio",
> >>      [IF_XEN] = "xen",
> >> +    [IF_EMMC] = "emmc",
> >>  };
> >
> > We don't need to introduce support for the legacy -drive magic.
> >
> > -device should be enough for this device, right?
> 
> External interface extensions need rationale: why do we want / need it?
> The commit message neglects to provide one.
> 
> Even more so when the interface in question is in a state like -drive
> is.

I wouldn't be too nasty about -drive;  for me I still find it the
easiest way to start a VM.

Dave

-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [PATCH v2 01/22] block: add eMMC block device type

[Philippe Mathieu-Daudé]

On 2/22/21 2:16 PM, Dr. David Alan Gilbert wrote:
> * Markus Armbruster (armbru@redhat.com) wrote:
>> Philippe Mathieu-Daudé <philmd@redhat.com> writes:
>>
>>> On 2/22/21 9:20 AM, Sai Pavan Boddu wrote:
>>>> From: Vincent Palatin <vpalatin@chromium.org>
>>>>
>>>> Add new block device type.
>>>>
>>>> Signed-off-by: Vincent Palatin <vpalatin@chromium.org>
>>>> [SPB: Rebased over 5.1 version]
>>>> Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
>>>> Signed-off-by: Joel Stanley <joel@jms.id.au>
>>>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>>>> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
>>>> ---
>>>>  include/sysemu/blockdev.h | 1 +
>>>>  blockdev.c                | 1 +
>>>>  2 files changed, 2 insertions(+)
>>>>
>>>> diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h
>>>> index 3b5fcda..eefae9f 100644
>>>> --- a/include/sysemu/blockdev.h
>>>> +++ b/include/sysemu/blockdev.h
>>>> @@ -24,6 +24,7 @@ typedef enum {
>>>>       */
>>>>      IF_NONE = 0,
>>>>      IF_IDE, IF_SCSI, IF_FLOPPY, IF_PFLASH, IF_MTD, IF_SD, IF_VIRTIO, IF_XEN,
>>>> +    IF_EMMC,
>>>>      IF_COUNT
>>>>  } BlockInterfaceType;
>>>>  
>>>> diff --git a/blockdev.c b/blockdev.c
>>>> index cd438e6..390d43c 100644
>>>> --- a/blockdev.c
>>>> +++ b/blockdev.c
>>>> @@ -83,6 +83,7 @@ static const char *const if_name[IF_COUNT] = {
>>>>      [IF_SD] = "sd",
>>>>      [IF_VIRTIO] = "virtio",
>>>>      [IF_XEN] = "xen",
>>>> +    [IF_EMMC] = "emmc",
>>>>  };
>>>
>>> We don't need to introduce support for the legacy -drive magic.
>>>
>>> -device should be enough for this device, right?
>>
>> External interface extensions need rationale: why do we want / need it?
>> The commit message neglects to provide one.
>>
>> Even more so when the interface in question is in a state like -drive
>> is.
> 
> I wouldn't be too nasty about -drive;  for me I still find it the
> easiest way to start a VM.

But eMMC isn't a bus where you can plug drives, it is soldered
on-board and is mmio mapped to a fixed address. I don't see the
point of having a drive interface for it...

RE: [PATCH v2 01/22] block: add eMMC block device type

[Sai Pavan Boddu]

Hi Philippe,

> -----Original Message-----
> From: Philippe Mathieu-Daudé <philmd@redhat.com>
> Sent: Monday, February 22, 2021 6:54 PM
> To: Dr. David Alan Gilbert <dgilbert@redhat.com>; Markus Armbruster
> <armbru@redhat.com>
> Cc: Sai Pavan Boddu <saipava@xilinx.com>; Kevin Wolf <kwolf@redhat.com>;
> Max Reitz <mreitz@redhat.com>; Vladimir Sementsov-Ogievskiy
> <vsementsov@virtuozzo.com>; Eric Blake <eblake@redhat.com>; Joel Stanley
> <joel@jms.id.au>; Cédric Le Goater <clg@kaod.org>; Vincent Palatin
> <vpalatin@chromium.org>; Thomas Huth <thuth@redhat.com>; Stefan
> Hajnoczi <stefanha@redhat.com>; Peter Maydell <peter.maydell@linaro.org>;
> Alistair Francis <alistair.francis@wdc.com>; Edgar Iglesias <edgari@xilinx.com>;
> Luc Michel <luc.michel@greensocs.com>; Paolo Bonzini
> <pbonzini@redhat.com>; Sai Pavan Boddu <saipava@xilinx.com>; qemu-
> devel@nongnu.org; qemu-block@nongnu.org
> Subject: Re: [PATCH v2 01/22] block: add eMMC block device type
> 
> On 2/22/21 2:16 PM, Dr. David Alan Gilbert wrote:
> > * Markus Armbruster (armbru@redhat.com) wrote:
> >> Philippe Mathieu-Daudé <philmd@redhat.com> writes:
> >>
> >>> On 2/22/21 9:20 AM, Sai Pavan Boddu wrote:
> >>>> From: Vincent Palatin <vpalatin@chromium.org>
> >>>>
> >>>> Add new block device type.
> >>>>
> >>>> Signed-off-by: Vincent Palatin <vpalatin@chromium.org>
> >>>> [SPB: Rebased over 5.1 version]
> >>>> Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
> >>>> Signed-off-by: Joel Stanley <joel@jms.id.au>
> >>>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> >>>> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> >>>> ---
> >>>>  include/sysemu/blockdev.h | 1 +
> >>>>  blockdev.c                | 1 +
> >>>>  2 files changed, 2 insertions(+)
> >>>>
> >>>> diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h
> >>>> index 3b5fcda..eefae9f 100644
> >>>> --- a/include/sysemu/blockdev.h
> >>>> +++ b/include/sysemu/blockdev.h
> >>>> @@ -24,6 +24,7 @@ typedef enum {
> >>>>       */
> >>>>      IF_NONE = 0,
> >>>>      IF_IDE, IF_SCSI, IF_FLOPPY, IF_PFLASH, IF_MTD, IF_SD,
> >>>> IF_VIRTIO, IF_XEN,
> >>>> +    IF_EMMC,
> >>>>      IF_COUNT
> >>>>  } BlockInterfaceType;
> >>>>
> >>>> diff --git a/blockdev.c b/blockdev.c index cd438e6..390d43c 100644
> >>>> --- a/blockdev.c
> >>>> +++ b/blockdev.c
> >>>> @@ -83,6 +83,7 @@ static const char *const if_name[IF_COUNT] = {
> >>>>      [IF_SD] = "sd",
> >>>>      [IF_VIRTIO] = "virtio",
> >>>>      [IF_XEN] = "xen",
> >>>> +    [IF_EMMC] = "emmc",
> >>>>  };
> >>>
> >>> We don't need to introduce support for the legacy -drive magic.
> >>>
> >>> -device should be enough for this device, right?
> >>
> >> External interface extensions need rationale: why do we want / need it?
> >> The commit message neglects to provide one.
> >>
> >> Even more so when the interface in question is in a state like -drive
> >> is.
> >
> > I wouldn't be too nasty about -drive;  for me I still find it the
> > easiest way to start a VM.
> 
> But eMMC isn't a bus where you can plug drives, it is soldered on-board and is
> mmio mapped to a fixed address. I don't see the point of having a drive
> interface for it...
[Sai Pavan Boddu] Yeah, this makes sense but having a drive would be a simple implementation without disturbing much in the sd card emulation code. And its just easy to use, just as how sd cards are inserted.

I need to see, how easy it would be with -device.

Thanks,
Sai Pavan

Re: [PATCH v2 01/22] block: add eMMC block device type

[Dr. David Alan Gilbert]

* Sai Pavan Boddu (saipava@xilinx.com) wrote:
> Hi Philippe,
> 
> > -----Original Message-----
> > From: Philippe Mathieu-Daudé <philmd@redhat.com>
> > Sent: Monday, February 22, 2021 6:54 PM
> > To: Dr. David Alan Gilbert <dgilbert@redhat.com>; Markus Armbruster
> > <armbru@redhat.com>
> > Cc: Sai Pavan Boddu <saipava@xilinx.com>; Kevin Wolf <kwolf@redhat.com>;
> > Max Reitz <mreitz@redhat.com>; Vladimir Sementsov-Ogievskiy
> > <vsementsov@virtuozzo.com>; Eric Blake <eblake@redhat.com>; Joel Stanley
> > <joel@jms.id.au>; Cédric Le Goater <clg@kaod.org>; Vincent Palatin
> > <vpalatin@chromium.org>; Thomas Huth <thuth@redhat.com>; Stefan
> > Hajnoczi <stefanha@redhat.com>; Peter Maydell <peter.maydell@linaro.org>;
> > Alistair Francis <alistair.francis@wdc.com>; Edgar Iglesias <edgari@xilinx.com>;
> > Luc Michel <luc.michel@greensocs.com>; Paolo Bonzini
> > <pbonzini@redhat.com>; Sai Pavan Boddu <saipava@xilinx.com>; qemu-
> > devel@nongnu.org; qemu-block@nongnu.org
> > Subject: Re: [PATCH v2 01/22] block: add eMMC block device type
> > 
> > On 2/22/21 2:16 PM, Dr. David Alan Gilbert wrote:
> > > * Markus Armbruster (armbru@redhat.com) wrote:
> > >> Philippe Mathieu-Daudé <philmd@redhat.com> writes:
> > >>
> > >>> On 2/22/21 9:20 AM, Sai Pavan Boddu wrote:
> > >>>> From: Vincent Palatin <vpalatin@chromium.org>
> > >>>>
> > >>>> Add new block device type.
> > >>>>
> > >>>> Signed-off-by: Vincent Palatin <vpalatin@chromium.org>
> > >>>> [SPB: Rebased over 5.1 version]
> > >>>> Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
> > >>>> Signed-off-by: Joel Stanley <joel@jms.id.au>
> > >>>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> > >>>> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> > >>>> ---
> > >>>>  include/sysemu/blockdev.h | 1 +
> > >>>>  blockdev.c                | 1 +
> > >>>>  2 files changed, 2 insertions(+)
> > >>>>
> > >>>> diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h
> > >>>> index 3b5fcda..eefae9f 100644
> > >>>> --- a/include/sysemu/blockdev.h
> > >>>> +++ b/include/sysemu/blockdev.h
> > >>>> @@ -24,6 +24,7 @@ typedef enum {
> > >>>>       */
> > >>>>      IF_NONE = 0,
> > >>>>      IF_IDE, IF_SCSI, IF_FLOPPY, IF_PFLASH, IF_MTD, IF_SD,
> > >>>> IF_VIRTIO, IF_XEN,
> > >>>> +    IF_EMMC,
> > >>>>      IF_COUNT
> > >>>>  } BlockInterfaceType;
> > >>>>
> > >>>> diff --git a/blockdev.c b/blockdev.c index cd438e6..390d43c 100644
> > >>>> --- a/blockdev.c
> > >>>> +++ b/blockdev.c
> > >>>> @@ -83,6 +83,7 @@ static const char *const if_name[IF_COUNT] = {
> > >>>>      [IF_SD] = "sd",
> > >>>>      [IF_VIRTIO] = "virtio",
> > >>>>      [IF_XEN] = "xen",
> > >>>> +    [IF_EMMC] = "emmc",
> > >>>>  };
> > >>>
> > >>> We don't need to introduce support for the legacy -drive magic.
> > >>>
> > >>> -device should be enough for this device, right?
> > >>
> > >> External interface extensions need rationale: why do we want / need it?
> > >> The commit message neglects to provide one.
> > >>
> > >> Even more so when the interface in question is in a state like -drive
> > >> is.
> > >
> > > I wouldn't be too nasty about -drive;  for me I still find it the
> > > easiest way to start a VM.
> > 
> > But eMMC isn't a bus where you can plug drives, it is soldered on-board and is
> > mmio mapped to a fixed address. I don't see the point of having a drive
> > interface for it...
> [Sai Pavan Boddu] Yeah, this makes sense but having a drive would be a simple implementation without disturbing much in the sd card emulation code. And its just easy to use, just as how sd cards are inserted.
> 
> I need to see, how easy it would be with -device.


Lets see what your command line looks like for starting it with emmc.

Dave

> Thanks,
> Sai Pavan
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

RE: [PATCH v2 01/22] block: add eMMC block device type

[Sai Pavan Boddu]

Hi Philippe,

> -----Original Message-----
> From: Philippe Mathieu-Daudé <philmd@redhat.com>
> Sent: Monday, February 22, 2021 5:34 PM
> To: Sai Pavan Boddu <saipava@xilinx.com>; Markus Armbruster
> <armbru@redhat.com>; Kevin Wolf <kwolf@redhat.com>; Max Reitz
> <mreitz@redhat.com>; Vladimir Sementsov-Ogievskiy
> <vsementsov@virtuozzo.com>; Eric Blake <eblake@redhat.com>; Joel Stanley
> <joel@jms.id.au>; Cédric Le Goater <clg@kaod.org>; Vincent Palatin
> <vpalatin@chromium.org>; Dr. David Alan Gilbert <dgilbert@redhat.com>;
> Thomas Huth <thuth@redhat.com>; Stefan Hajnoczi <stefanha@redhat.com>;
> Peter Maydell <peter.maydell@linaro.org>; Alistair Francis
> <alistair.francis@wdc.com>; Edgar Iglesias <edgari@xilinx.com>; Luc Michel
> <luc.michel@greensocs.com>; Paolo Bonzini <pbonzini@redhat.com>
> Cc: Sai Pavan Boddu <saipava@xilinx.com>; qemu-devel@nongnu.org; qemu-
> block@nongnu.org
> Subject: Re: [PATCH v2 01/22] block: add eMMC block device type
> 
> On 2/22/21 9:20 AM, Sai Pavan Boddu wrote:
> > From: Vincent Palatin <vpalatin@chromium.org>
> >
> > Add new block device type.
> >
> > Signed-off-by: Vincent Palatin <vpalatin@chromium.org>
> > [SPB: Rebased over 5.1 version]
> > Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
> > Signed-off-by: Joel Stanley <joel@jms.id.au>
> > Signed-off-by: Cédric Le Goater <clg@kaod.org>
> > Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> > ---
> >  include/sysemu/blockdev.h | 1 +
> >  blockdev.c                | 1 +
> >  2 files changed, 2 insertions(+)
> >
> > diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h
> > index 3b5fcda..eefae9f 100644
> > --- a/include/sysemu/blockdev.h
> > +++ b/include/sysemu/blockdev.h
> > @@ -24,6 +24,7 @@ typedef enum {
> >       */
> >      IF_NONE = 0,
> >      IF_IDE, IF_SCSI, IF_FLOPPY, IF_PFLASH, IF_MTD, IF_SD, IF_VIRTIO,
> > IF_XEN,
> > +    IF_EMMC,
> >      IF_COUNT
> >  } BlockInterfaceType;
> >
> > diff --git a/blockdev.c b/blockdev.c
> > index cd438e6..390d43c 100644
> > --- a/blockdev.c
> > +++ b/blockdev.c
> > @@ -83,6 +83,7 @@ static const char *const if_name[IF_COUNT] = {
> >      [IF_SD] = "sd",
> >      [IF_VIRTIO] = "virtio",
> >      [IF_XEN] = "xen",
> > +    [IF_EMMC] = "emmc",
> >  };
> 
> We don't need to introduce support for the legacy -drive magic.
> 
> -device should be enough for this device, right?
[Sai Pavan Boddu] I was seeing to use -device for emmc. But I see we anyway need blockdev support for this, which would require us the use -drive.

Can you give some pointers, how to approach this ?

Regards,
Sai Pavan

Re: [PATCH v2 01/22] block: add eMMC block device type

[Stefan Hajnoczi]

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

On Tue, Feb 23, 2021 at 05:35:20PM +0000, Sai Pavan Boddu wrote:
> Hi Philippe,
> 
> > -----Original Message-----
> > From: Philippe Mathieu-Daudé <philmd@redhat.com>
> > Sent: Monday, February 22, 2021 5:34 PM
> > To: Sai Pavan Boddu <saipava@xilinx.com>; Markus Armbruster
> > <armbru@redhat.com>; Kevin Wolf <kwolf@redhat.com>; Max Reitz
> > <mreitz@redhat.com>; Vladimir Sementsov-Ogievskiy
> > <vsementsov@virtuozzo.com>; Eric Blake <eblake@redhat.com>; Joel Stanley
> > <joel@jms.id.au>; Cédric Le Goater <clg@kaod.org>; Vincent Palatin
> > <vpalatin@chromium.org>; Dr. David Alan Gilbert <dgilbert@redhat.com>;
> > Thomas Huth <thuth@redhat.com>; Stefan Hajnoczi <stefanha@redhat.com>;
> > Peter Maydell <peter.maydell@linaro.org>; Alistair Francis
> > <alistair.francis@wdc.com>; Edgar Iglesias <edgari@xilinx.com>; Luc Michel
> > <luc.michel@greensocs.com>; Paolo Bonzini <pbonzini@redhat.com>
> > Cc: Sai Pavan Boddu <saipava@xilinx.com>; qemu-devel@nongnu.org; qemu-
> > block@nongnu.org
> > Subject: Re: [PATCH v2 01/22] block: add eMMC block device type
> > 
> > On 2/22/21 9:20 AM, Sai Pavan Boddu wrote:
> > > From: Vincent Palatin <vpalatin@chromium.org>
> > >
> > > Add new block device type.
> > >
> > > Signed-off-by: Vincent Palatin <vpalatin@chromium.org>
> > > [SPB: Rebased over 5.1 version]
> > > Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
> > > Signed-off-by: Joel Stanley <joel@jms.id.au>
> > > Signed-off-by: Cédric Le Goater <clg@kaod.org>
> > > Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> > > ---
> > >  include/sysemu/blockdev.h | 1 +
> > >  blockdev.c                | 1 +
> > >  2 files changed, 2 insertions(+)
> > >
> > > diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h
> > > index 3b5fcda..eefae9f 100644
> > > --- a/include/sysemu/blockdev.h
> > > +++ b/include/sysemu/blockdev.h
> > > @@ -24,6 +24,7 @@ typedef enum {
> > >       */
> > >      IF_NONE = 0,
> > >      IF_IDE, IF_SCSI, IF_FLOPPY, IF_PFLASH, IF_MTD, IF_SD, IF_VIRTIO,
> > > IF_XEN,
> > > +    IF_EMMC,
> > >      IF_COUNT
> > >  } BlockInterfaceType;
> > >
> > > diff --git a/blockdev.c b/blockdev.c
> > > index cd438e6..390d43c 100644
> > > --- a/blockdev.c
> > > +++ b/blockdev.c
> > > @@ -83,6 +83,7 @@ static const char *const if_name[IF_COUNT] = {
> > >      [IF_SD] = "sd",
> > >      [IF_VIRTIO] = "virtio",
> > >      [IF_XEN] = "xen",
> > > +    [IF_EMMC] = "emmc",
> > >  };
> > 
> > We don't need to introduce support for the legacy -drive magic.
> > 
> > -device should be enough for this device, right?
> [Sai Pavan Boddu] I was seeing to use -device for emmc. But I see we anyway need blockdev support for this, which would require us the use -drive.
> 
> Can you give some pointers, how to approach this ?

It is probably not necessary to add a new IF_ constant. Would this work:

  -drive if=none,id=emmc0,file=test.img,format=raw
  -device emmc,...,drive=emmc0

Or the more modern:

  -blockdev node-name=emmc0,driver=file,filename=test.img
  -device emmc,...,drive=emmc0

?

(The syntax might need small tweaks but is shows the general idea.)

Stefan

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

Re: [PATCH v2 01/22] block: add eMMC block device type

[Cédric Le Goater]

On 2/24/21 12:40 PM, Stefan Hajnoczi wrote:
> On Tue, Feb 23, 2021 at 05:35:20PM +0000, Sai Pavan Boddu wrote:
>> Hi Philippe,
>>
>>> -----Original Message-----
>>> From: Philippe Mathieu-Daudé <philmd@redhat.com>
>>> Sent: Monday, February 22, 2021 5:34 PM
>>> To: Sai Pavan Boddu <saipava@xilinx.com>; Markus Armbruster
>>> <armbru@redhat.com>; Kevin Wolf <kwolf@redhat.com>; Max Reitz
>>> <mreitz@redhat.com>; Vladimir Sementsov-Ogievskiy
>>> <vsementsov@virtuozzo.com>; Eric Blake <eblake@redhat.com>; Joel Stanley
>>> <joel@jms.id.au>; Cédric Le Goater <clg@kaod.org>; Vincent Palatin
>>> <vpalatin@chromium.org>; Dr. David Alan Gilbert <dgilbert@redhat.com>;
>>> Thomas Huth <thuth@redhat.com>; Stefan Hajnoczi <stefanha@redhat.com>;
>>> Peter Maydell <peter.maydell@linaro.org>; Alistair Francis
>>> <alistair.francis@wdc.com>; Edgar Iglesias <edgari@xilinx.com>; Luc Michel
>>> <luc.michel@greensocs.com>; Paolo Bonzini <pbonzini@redhat.com>
>>> Cc: Sai Pavan Boddu <saipava@xilinx.com>; qemu-devel@nongnu.org; qemu-
>>> block@nongnu.org
>>> Subject: Re: [PATCH v2 01/22] block: add eMMC block device type
>>>
>>> On 2/22/21 9:20 AM, Sai Pavan Boddu wrote:
>>>> From: Vincent Palatin <vpalatin@chromium.org>
>>>>
>>>> Add new block device type.
>>>>
>>>> Signed-off-by: Vincent Palatin <vpalatin@chromium.org>
>>>> [SPB: Rebased over 5.1 version]
>>>> Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
>>>> Signed-off-by: Joel Stanley <joel@jms.id.au>
>>>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>>>> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
>>>> ---
>>>>  include/sysemu/blockdev.h | 1 +
>>>>  blockdev.c                | 1 +
>>>>  2 files changed, 2 insertions(+)
>>>>
>>>> diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h
>>>> index 3b5fcda..eefae9f 100644
>>>> --- a/include/sysemu/blockdev.h
>>>> +++ b/include/sysemu/blockdev.h
>>>> @@ -24,6 +24,7 @@ typedef enum {
>>>>       */
>>>>      IF_NONE = 0,
>>>>      IF_IDE, IF_SCSI, IF_FLOPPY, IF_PFLASH, IF_MTD, IF_SD, IF_VIRTIO,
>>>> IF_XEN,
>>>> +    IF_EMMC,
>>>>      IF_COUNT
>>>>  } BlockInterfaceType;
>>>>
>>>> diff --git a/blockdev.c b/blockdev.c
>>>> index cd438e6..390d43c 100644
>>>> --- a/blockdev.c
>>>> +++ b/blockdev.c
>>>> @@ -83,6 +83,7 @@ static const char *const if_name[IF_COUNT] = {
>>>>      [IF_SD] = "sd",
>>>>      [IF_VIRTIO] = "virtio",
>>>>      [IF_XEN] = "xen",
>>>> +    [IF_EMMC] = "emmc",
>>>>  };
>>>
>>> We don't need to introduce support for the legacy -drive magic.
>>>
>>> -device should be enough for this device, right?
>> [Sai Pavan Boddu] I was seeing to use -device for emmc. But I see we anyway need blockdev support for this, which would require us the use -drive.
>>
>> Can you give some pointers, how to approach this ?
> 
> It is probably not necessary to add a new IF_ constant. Would this work:
> 
>   -drive if=none,id=emmc0,file=test.img,format=raw
>   -device emmc,...,drive=emmc0
> 
> Or the more modern:
> 
>   -blockdev node-name=emmc0,driver=file,filename=test.img
>   -device emmc,...,drive=emmc0
> 
> ?
> 
> (The syntax might need small tweaks but is shows the general idea.)

Yes. This is better. 

We could have an "emmc" device inheriting from "sd-card". The "emmc" 
property would not be necessary anymore and may be, we could cleanup 
up some parts doing : 

    if (sd->emmc) { /* eMMC */
        ...
    } else {

    }

with SDCardClass handlers. the SWITCH_FUNCTION command is a good 
candidate, CMD8 also.

C.

RE: [PATCH v2 01/22] block: add eMMC block device type

[Sai Pavan Boddu]

Hi Stefan

> -----Original Message-----
> From: Stefan Hajnoczi <stefanha@redhat.com>
> Sent: Wednesday, February 24, 2021 5:10 PM
> To: Sai Pavan Boddu <saipava@xilinx.com>
> Cc: Philippe Mathieu-Daudé <philmd@redhat.com>; Markus Armbruster
> <armbru@redhat.com>; Kevin Wolf <kwolf@redhat.com>; Max Reitz
> <mreitz@redhat.com>; Vladimir Sementsov-Ogievskiy
> <vsementsov@virtuozzo.com>; Eric Blake <eblake@redhat.com>; Joel Stanley
> <joel@jms.id.au>; Cédric Le Goater <clg@kaod.org>; Vincent Palatin
> <vpalatin@chromium.org>; Dr. David Alan Gilbert <dgilbert@redhat.com>;
> Thomas Huth <thuth@redhat.com>; Peter Maydell
> <peter.maydell@linaro.org>; Alistair Francis <alistair.francis@wdc.com>; Edgar
> Iglesias <edgari@xilinx.com>; Luc Michel <luc.michel@greensocs.com>; Paolo
> Bonzini <pbonzini@redhat.com>; qemu-devel@nongnu.org; qemu-
> block@nongnu.org
> Subject: Re: [PATCH v2 01/22] block: add eMMC block device type
> 
> On Tue, Feb 23, 2021 at 05:35:20PM +0000, Sai Pavan Boddu wrote:
> > Hi Philippe,
> >
> > > -----Original Message-----
> > > From: Philippe Mathieu-Daudé <philmd@redhat.com>
> > > Sent: Monday, February 22, 2021 5:34 PM
> > > To: Sai Pavan Boddu <saipava@xilinx.com>; Markus Armbruster
> > > <armbru@redhat.com>; Kevin Wolf <kwolf@redhat.com>; Max Reitz
> > > <mreitz@redhat.com>; Vladimir Sementsov-Ogievskiy
> > > <vsementsov@virtuozzo.com>; Eric Blake <eblake@redhat.com>; Joel
> > > Stanley <joel@jms.id.au>; Cédric Le Goater <clg@kaod.org>; Vincent
> > > Palatin <vpalatin@chromium.org>; Dr. David Alan Gilbert
> > > <dgilbert@redhat.com>; Thomas Huth <thuth@redhat.com>; Stefan
> > > Hajnoczi <stefanha@redhat.com>; Peter Maydell
> > > <peter.maydell@linaro.org>; Alistair Francis
> > > <alistair.francis@wdc.com>; Edgar Iglesias <edgari@xilinx.com>; Luc
> > > Michel <luc.michel@greensocs.com>; Paolo Bonzini
> > > <pbonzini@redhat.com>
> > > Cc: Sai Pavan Boddu <saipava@xilinx.com>; qemu-devel@nongnu.org;
> > > qemu- block@nongnu.org
> > > Subject: Re: [PATCH v2 01/22] block: add eMMC block device type
> > >
> > > On 2/22/21 9:20 AM, Sai Pavan Boddu wrote:
> > > > From: Vincent Palatin <vpalatin@chromium.org>
> > > >
> > > > Add new block device type.
> > > >
> > > > Signed-off-by: Vincent Palatin <vpalatin@chromium.org>
> > > > [SPB: Rebased over 5.1 version]
> > > > Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
> > > > Signed-off-by: Joel Stanley <joel@jms.id.au>
> > > > Signed-off-by: Cédric Le Goater <clg@kaod.org>
> > > > Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> > > > ---
> > > >  include/sysemu/blockdev.h | 1 +
> > > >  blockdev.c                | 1 +
> > > >  2 files changed, 2 insertions(+)
> > > >
> > > > diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h
> > > > index 3b5fcda..eefae9f 100644
> > > > --- a/include/sysemu/blockdev.h
> > > > +++ b/include/sysemu/blockdev.h
> > > > @@ -24,6 +24,7 @@ typedef enum {
> > > >       */
> > > >      IF_NONE = 0,
> > > >      IF_IDE, IF_SCSI, IF_FLOPPY, IF_PFLASH, IF_MTD, IF_SD,
> > > > IF_VIRTIO, IF_XEN,
> > > > +    IF_EMMC,
> > > >      IF_COUNT
> > > >  } BlockInterfaceType;
> > > >
> > > > diff --git a/blockdev.c b/blockdev.c index cd438e6..390d43c 100644
> > > > --- a/blockdev.c
> > > > +++ b/blockdev.c
> > > > @@ -83,6 +83,7 @@ static const char *const if_name[IF_COUNT] = {
> > > >      [IF_SD] = "sd",
> > > >      [IF_VIRTIO] = "virtio",
> > > >      [IF_XEN] = "xen",
> > > > +    [IF_EMMC] = "emmc",
> > > >  };
> > >
> > > We don't need to introduce support for the legacy -drive magic.
> > >
> > > -device should be enough for this device, right?
> > [Sai Pavan Boddu] I was seeing to use -device for emmc. But I see we anyway
> need blockdev support for this, which would require us the use -drive.
> >
> > Can you give some pointers, how to approach this ?
> 
> It is probably not necessary to add a new IF_ constant. Would this work:
> 
>   -drive if=none,id=emmc0,file=test.img,format=raw
>   -device emmc,...,drive=emmc0
[Sai Pavan Boddu] Great, this works for me.
> 
> Or the more modern:
> 
>   -blockdev node-name=emmc0,driver=file,filename=test.img
>   -device emmc,...,drive=emmc0
[Sai Pavan Boddu] Thanks, I would try to follow the modern approach then!

Regards,
Sai Pavan
> 
> ?
> 
> (The syntax might need small tweaks but is shows the general idea.)
> 
> Stefan

RE: [PATCH v2 01/22] block: add eMMC block device type

[Sai Pavan Boddu]

Hi Cedric,


> -----Original Message-----
> From: Cédric Le Goater <clg@kaod.org>
> Sent: Wednesday, February 24, 2021 7:25 PM
> To: Stefan Hajnoczi <stefanha@redhat.com>; Sai Pavan Boddu
> <saipava@xilinx.com>
> Cc: Philippe Mathieu-Daudé <philmd@redhat.com>; Markus Armbruster
> <armbru@redhat.com>; Kevin Wolf <kwolf@redhat.com>; Max Reitz
> <mreitz@redhat.com>; Vladimir Sementsov-Ogievskiy
> <vsementsov@virtuozzo.com>; Eric Blake <eblake@redhat.com>; Joel Stanley
> <joel@jms.id.au>; Vincent Palatin <vpalatin@chromium.org>; Dr. David Alan
> Gilbert <dgilbert@redhat.com>; Thomas Huth <thuth@redhat.com>; Peter
> Maydell <peter.maydell@linaro.org>; Alistair Francis
> <alistair.francis@wdc.com>; Edgar Iglesias <edgari@xilinx.com>; Luc Michel
> <luc.michel@greensocs.com>; Paolo Bonzini <pbonzini@redhat.com>; qemu-
> devel@nongnu.org; qemu-block@nongnu.org
> Subject: Re: [PATCH v2 01/22] block: add eMMC block device type
> 
> On 2/24/21 12:40 PM, Stefan Hajnoczi wrote:
> > On Tue, Feb 23, 2021 at 05:35:20PM +0000, Sai Pavan Boddu wrote:
> >> Hi Philippe,
> >>
> >>> -----Original Message-----
> >>> From: Philippe Mathieu-Daudé <philmd@redhat.com>
> >>> Sent: Monday, February 22, 2021 5:34 PM
> >>> To: Sai Pavan Boddu <saipava@xilinx.com>; Markus Armbruster
> >>> <armbru@redhat.com>; Kevin Wolf <kwolf@redhat.com>; Max Reitz
> >>> <mreitz@redhat.com>; Vladimir Sementsov-Ogievskiy
> >>> <vsementsov@virtuozzo.com>; Eric Blake <eblake@redhat.com>; Joel
> >>> Stanley <joel@jms.id.au>; Cédric Le Goater <clg@kaod.org>; Vincent
> >>> Palatin <vpalatin@chromium.org>; Dr. David Alan Gilbert
> >>> <dgilbert@redhat.com>; Thomas Huth <thuth@redhat.com>; Stefan
> >>> Hajnoczi <stefanha@redhat.com>; Peter Maydell
> >>> <peter.maydell@linaro.org>; Alistair Francis
> >>> <alistair.francis@wdc.com>; Edgar Iglesias <edgari@xilinx.com>; Luc
> >>> Michel <luc.michel@greensocs.com>; Paolo Bonzini
> >>> <pbonzini@redhat.com>
> >>> Cc: Sai Pavan Boddu <saipava@xilinx.com>; qemu-devel@nongnu.org;
> >>> qemu- block@nongnu.org
> >>> Subject: Re: [PATCH v2 01/22] block: add eMMC block device type
> >>>
> >>> On 2/22/21 9:20 AM, Sai Pavan Boddu wrote:
> >>>> From: Vincent Palatin <vpalatin@chromium.org>
> >>>>
> >>>> Add new block device type.
> >>>>
> >>>> Signed-off-by: Vincent Palatin <vpalatin@chromium.org>
> >>>> [SPB: Rebased over 5.1 version]
> >>>> Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
> >>>> Signed-off-by: Joel Stanley <joel@jms.id.au>
> >>>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> >>>> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> >>>> ---
> >>>>  include/sysemu/blockdev.h | 1 +
> >>>>  blockdev.c                | 1 +
> >>>>  2 files changed, 2 insertions(+)
> >>>>
> >>>> diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h
> >>>> index 3b5fcda..eefae9f 100644
> >>>> --- a/include/sysemu/blockdev.h
> >>>> +++ b/include/sysemu/blockdev.h
> >>>> @@ -24,6 +24,7 @@ typedef enum {
> >>>>       */
> >>>>      IF_NONE = 0,
> >>>>      IF_IDE, IF_SCSI, IF_FLOPPY, IF_PFLASH, IF_MTD, IF_SD,
> >>>> IF_VIRTIO, IF_XEN,
> >>>> +    IF_EMMC,
> >>>>      IF_COUNT
> >>>>  } BlockInterfaceType;
> >>>>
> >>>> diff --git a/blockdev.c b/blockdev.c index cd438e6..390d43c 100644
> >>>> --- a/blockdev.c
> >>>> +++ b/blockdev.c
> >>>> @@ -83,6 +83,7 @@ static const char *const if_name[IF_COUNT] = {
> >>>>      [IF_SD] = "sd",
> >>>>      [IF_VIRTIO] = "virtio",
> >>>>      [IF_XEN] = "xen",
> >>>> +    [IF_EMMC] = "emmc",
> >>>>  };
> >>>
> >>> We don't need to introduce support for the legacy -drive magic.
> >>>
> >>> -device should be enough for this device, right?
> >> [Sai Pavan Boddu] I was seeing to use -device for emmc. But I see we
> anyway need blockdev support for this, which would require us the use -drive.
> >>
> >> Can you give some pointers, how to approach this ?
> >
> > It is probably not necessary to add a new IF_ constant. Would this work:
> >
> >   -drive if=none,id=emmc0,file=test.img,format=raw
> >   -device emmc,...,drive=emmc0
> >
> > Or the more modern:
> >
> >   -blockdev node-name=emmc0,driver=file,filename=test.img
> >   -device emmc,...,drive=emmc0
> >
> > ?
> >
> > (The syntax might need small tweaks but is shows the general idea.)
> 
> Yes. This is better.
> 
> We could have an "emmc" device inheriting from "sd-card". The "emmc"
> property would not be necessary anymore and may be, we could cleanup up
> some parts doing :
> 
>     if (sd->emmc) { /* eMMC */
>         ...
>     } else {
> 
>     }
> 
> with SDCardClass handlers. the SWITCH_FUNCTION command is a good
> candidate, CMD8 also.
[Sai Pavan Boddu] Nice, this approach looks clean.
But we still may be depending on emmc property. Not sure!

I would get back with v3, your review on those patches would be great.

Thanks & Regards,
Sai Pavan
> 
> C.

Re: [PATCH v2 01/22] block: add eMMC block device type

[Cédric Le Goater]

On 2/24/21 8:13 PM, Sai Pavan Boddu wrote:
> Hi Cedric,
> 
> 
>> -----Original Message-----
>> From: Cédric Le Goater <clg@kaod.org>
>> Sent: Wednesday, February 24, 2021 7:25 PM
>> To: Stefan Hajnoczi <stefanha@redhat.com>; Sai Pavan Boddu
>> <saipava@xilinx.com>
>> Cc: Philippe Mathieu-Daudé <philmd@redhat.com>; Markus Armbruster
>> <armbru@redhat.com>; Kevin Wolf <kwolf@redhat.com>; Max Reitz
>> <mreitz@redhat.com>; Vladimir Sementsov-Ogievskiy
>> <vsementsov@virtuozzo.com>; Eric Blake <eblake@redhat.com>; Joel Stanley
>> <joel@jms.id.au>; Vincent Palatin <vpalatin@chromium.org>; Dr. David Alan
>> Gilbert <dgilbert@redhat.com>; Thomas Huth <thuth@redhat.com>; Peter
>> Maydell <peter.maydell@linaro.org>; Alistair Francis
>> <alistair.francis@wdc.com>; Edgar Iglesias <edgari@xilinx.com>; Luc Michel
>> <luc.michel@greensocs.com>; Paolo Bonzini <pbonzini@redhat.com>; qemu-
>> devel@nongnu.org; qemu-block@nongnu.org
>> Subject: Re: [PATCH v2 01/22] block: add eMMC block device type
>>
>> On 2/24/21 12:40 PM, Stefan Hajnoczi wrote:
>>> On Tue, Feb 23, 2021 at 05:35:20PM +0000, Sai Pavan Boddu wrote:
>>>> Hi Philippe,
>>>>
>>>>> -----Original Message-----
>>>>> From: Philippe Mathieu-Daudé <philmd@redhat.com>
>>>>> Sent: Monday, February 22, 2021 5:34 PM
>>>>> To: Sai Pavan Boddu <saipava@xilinx.com>; Markus Armbruster
>>>>> <armbru@redhat.com>; Kevin Wolf <kwolf@redhat.com>; Max Reitz
>>>>> <mreitz@redhat.com>; Vladimir Sementsov-Ogievskiy
>>>>> <vsementsov@virtuozzo.com>; Eric Blake <eblake@redhat.com>; Joel
>>>>> Stanley <joel@jms.id.au>; Cédric Le Goater <clg@kaod.org>; Vincent
>>>>> Palatin <vpalatin@chromium.org>; Dr. David Alan Gilbert
>>>>> <dgilbert@redhat.com>; Thomas Huth <thuth@redhat.com>; Stefan
>>>>> Hajnoczi <stefanha@redhat.com>; Peter Maydell
>>>>> <peter.maydell@linaro.org>; Alistair Francis
>>>>> <alistair.francis@wdc.com>; Edgar Iglesias <edgari@xilinx.com>; Luc
>>>>> Michel <luc.michel@greensocs.com>; Paolo Bonzini
>>>>> <pbonzini@redhat.com>
>>>>> Cc: Sai Pavan Boddu <saipava@xilinx.com>; qemu-devel@nongnu.org;
>>>>> qemu- block@nongnu.org
>>>>> Subject: Re: [PATCH v2 01/22] block: add eMMC block device type
>>>>>
>>>>> On 2/22/21 9:20 AM, Sai Pavan Boddu wrote:
>>>>>> From: Vincent Palatin <vpalatin@chromium.org>
>>>>>>
>>>>>> Add new block device type.
>>>>>>
>>>>>> Signed-off-by: Vincent Palatin <vpalatin@chromium.org>
>>>>>> [SPB: Rebased over 5.1 version]
>>>>>> Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
>>>>>> Signed-off-by: Joel Stanley <joel@jms.id.au>
>>>>>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>>>>>> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
>>>>>> ---
>>>>>>  include/sysemu/blockdev.h | 1 +
>>>>>>  blockdev.c                | 1 +
>>>>>>  2 files changed, 2 insertions(+)
>>>>>>
>>>>>> diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h
>>>>>> index 3b5fcda..eefae9f 100644
>>>>>> --- a/include/sysemu/blockdev.h
>>>>>> +++ b/include/sysemu/blockdev.h
>>>>>> @@ -24,6 +24,7 @@ typedef enum {
>>>>>>       */
>>>>>>      IF_NONE = 0,
>>>>>>      IF_IDE, IF_SCSI, IF_FLOPPY, IF_PFLASH, IF_MTD, IF_SD,
>>>>>> IF_VIRTIO, IF_XEN,
>>>>>> +    IF_EMMC,
>>>>>>      IF_COUNT
>>>>>>  } BlockInterfaceType;
>>>>>>
>>>>>> diff --git a/blockdev.c b/blockdev.c index cd438e6..390d43c 100644
>>>>>> --- a/blockdev.c
>>>>>> +++ b/blockdev.c
>>>>>> @@ -83,6 +83,7 @@ static const char *const if_name[IF_COUNT] = {
>>>>>>      [IF_SD] = "sd",
>>>>>>      [IF_VIRTIO] = "virtio",
>>>>>>      [IF_XEN] = "xen",
>>>>>> +    [IF_EMMC] = "emmc",
>>>>>>  };
>>>>>
>>>>> We don't need to introduce support for the legacy -drive magic.
>>>>>
>>>>> -device should be enough for this device, right?
>>>> [Sai Pavan Boddu] I was seeing to use -device for emmc. But I see we
>> anyway need blockdev support for this, which would require us the use -drive.
>>>>
>>>> Can you give some pointers, how to approach this ?
>>>
>>> It is probably not necessary to add a new IF_ constant. Would this work:
>>>
>>>   -drive if=none,id=emmc0,file=test.img,format=raw
>>>   -device emmc,...,drive=emmc0
>>>
>>> Or the more modern:
>>>
>>>   -blockdev node-name=emmc0,driver=file,filename=test.img
>>>   -device emmc,...,drive=emmc0
>>>
>>> ?
>>>
>>> (The syntax might need small tweaks but is shows the general idea.)
>>
>> Yes. This is better.
>>
>> We could have an "emmc" device inheriting from "sd-card". The "emmc"
>> property would not be necessary anymore and may be, we could cleanup up
>> some parts doing :
>>
>>     if (sd->emmc) { /* eMMC */
>>         ...
>>     } else {
>>
>>     }
>>
>> with SDCardClass handlers. the SWITCH_FUNCTION command is a good
>> candidate, CMD8 also.
> [Sai Pavan Boddu] Nice, this approach looks clean.
> But we still may be depending on emmc property. Not sure!

It could be a SDCardClass attribute instead since it's a constant for
the emmc device.

C. 

> 
> I would get back with v3, your review on those patches would be great.
> 
> Thanks & Regards,
> Sai Pavan
>>
>> C.