Re: [patch] fix sdmmc bug affecting strictly sector-addressable eMMC chips

2015-09-09 Thread kremlin
This message is in response to this previous discussion:

http://marc.info/?l=openbsd-tech=144074914604130

>> Since sector mode is supported by the host software, shouldn't we
>> always set the MMC_OCR_SECTOR_MODE bit and check the returned OCR
>> value to see if the card also supports sector mode addressing?
>>
>> Related thread: http://marc.info/?l=linux-mmc=129419687808618

> That is a good point. I'll fix that.

Now, instead of waiting for 50 failures (what was I thinking?) we simply
provide the MMC_OCR_SECTOR_MODE bit in CMD1's argument (given the card
is not an SD card) and set the SMF_SECTOR_MODE flag if the card's
response indicates it is indeed sector-addressed.

Ian

Index: sdmmc_mem.c
===
RCS file: /cvs/src/sys/dev/sdmmc/sdmmc_mem.c,v
retrieving revision 1.21
diff -u -p -r1.21 sdmmc_mem.c
--- sdmmc_mem.c 22 Apr 2015 04:02:06 -  1.21
+++ sdmmc_mem.c 9 Sep 2015 23:17:58 -
@@ -525,11 +525,14 @@ sdmmc_mem_send_op_cond(struct sdmmc_soft
cmd.c_opcode = SD_APP_OP_COND;
error = sdmmc_app_command(sc, );
} else {
+   cmd.c_arg |= MMC_OCR_SECTOR_MODE;
cmd.c_opcode = MMC_SEND_OP_COND;
error = sdmmc_mmc_command(sc, );
}
if (error != 0)
break;
+   if (ISSET(MMC_R3(cmd.c_resp), MMC_OCR_SECTOR_MODE))
+   sc->sc_flags |= SMF_SECTOR_MODE;
if (ISSET(MMC_R3(cmd.c_resp), MMC_OCR_MEM_READY) ||
ocr == 0)
break;
Index: sdmmcreg.h
===
RCS file: /cvs/src/sys/dev/sdmmc/sdmmcreg.h,v
retrieving revision 1.6
diff -u -p -r1.6 sdmmcreg.h
--- sdmmcreg.h  23 Sep 2014 12:08:13 -  1.6
+++ sdmmcreg.h  9 Sep 2015 23:17:58 -
@@ -48,6 +48,7 @@
 
 /* OCR bits */
 #define MMC_OCR_MEM_READY  (1<<31) /* memory power-up status bit */
+#define MMC_OCR_SECTOR_MODE(1<<30) /* Provided in argument 
following CMD1 */
 #define MMC_OCR_3_5V_3_6V  (1<<23)
 #define MMC_OCR_3_4V_3_5V  (1<<22)
 #define MMC_OCR_3_3V_3_4V  (1<<21)
Index: sdmmcvar.h
===
RCS file: /cvs/src/sys/dev/sdmmc/sdmmcvar.h,v
retrieving revision 1.22
diff -u -p -r1.22 sdmmcvar.h
--- sdmmcvar.h  12 Sep 2013 11:54:04 -  1.22
+++ sdmmcvar.h  9 Sep 2015 23:17:58 -
@@ -164,6 +164,7 @@ struct sdmmc_softc {
 #define SMF_CARD_ATTACHED  0x0020  /* card driver(s) attached */
 #defineSMF_STOP_AFTER_MULTIPLE 0x0040  /* send a stop after a multiple 
cmd */
 #define SMF_CONFIG_PENDING 0x0080  /* config_pending_incr() called */
+#define SMF_SECTOR_MODE 0x0100 /* card is sector-addressed */
 
uint32_t sc_caps;   /* host capability */
 #define SMC_CAPS_AUTO_STOP 0x0001  /* send CMD12 automagically by host */



Re: [patch] fix sdmmc bug affecting strictly sector-addressable eMMC chips

2015-08-28 Thread Uwe Stuehler
On Fri, Aug 28, 2015 at 02:13:44AM -0400, kremlin wrote:
 sdmmc gives up configuring the card after 100 failed CMD1 commands. My
 patch starts asserting the MMC_OCR_SECTOR_MODE bit after 50 failed
 attempts. This fixes the bug on my BBB with Micron chip:

Nice work!  I wonder why you chose to be so conservative (or arbitrary)
about the after 50 failed attempts condition. :)

Since sector mode is supported by the host software, shouldn't we always
set the MMC_OCR_SECTOR_MODE bit and check the returned OCR value to see
if the card also supports sector mode addressing?

Related thread: http://marc.info/?l=linux-mmcm=129419687808618

Cheers,
Uwe



Re: [patch] fix sdmmc bug affecting strictly sector-addressable eMMC chips

2015-08-28 Thread ian kremlin
 Nice work!  I wonder why you chose to be so conservative (or arbitrary)
 about the after 50 failed attempts condition. :)

My reasoning is that after 50 failed CMD1s, either two things are the
case: The card will only initialize with the sector bit set, or it is
a high capacity device taking a long time to become ready. In either
of these cases, the card will be block/sector addressed anyway.

 Since sector mode is supported by the host software, shouldn't we always
 set the MMC_OCR_SECTOR_MODE bit and check the returned OCR value to see
 if the card also supports sector mode addressing?

 Related thread: http://marc.info/?l=linux-mmcm=129419687808618

That is a good point. I'll fix that.