Hi,

My fix was very conservative (I don't have a good understanding of MMC,
and clearly outdated documentations).

I've tried your patch Raphael, and it's working as expected (and it's so
much cleaner).

OpenBSD runs impressively well on my BeagleBone, the only drawback I can
see is that the system is slowed down by poor MMC performances (due to 
missing DMA support in ommmc driver I suppose ?).

Thanks,

Cédric

Sunday 21 Sep 2014, 14:09:57 (+0200), Raphael Graf:
> On Sat, September 20, 2014 7:45 pm, Jonathan Gray wrote:
> > On Sat, Sep 20, 2014 at 06:01:51PM +0200, Cédric Tessier wrote:
> >> Hi,
> >>
> >> I've bought a BeagleBone Black rev. C board, and I was trying to install
> >> OpenBSD on it, but the internal eMMC was causing errors.
> >>
> >>  sdmmc1: unknown CARD_TYPE 0x17
> >>  scsibus1 at sdmmc1: 2 targets, initiator 0
> >>  sd1 at scsibus1 targ 1 lun 0: <SD/MMC, Drive #01, > SCSI2 0/direct fixed
> >>  sd1: 1024MB, 512 bytes/sector, 2097152 sectors
> >>
> >> Card type and sectors count were wrong, and accessing the device was
> >> causing I/O errors.
> >>
> >> I've investigated the problem, and it looks like the support of
> >> High Capacity eMMC (> 2GB) is missing.
> >>
> >> I've written a quick and dirty patch (tested on 5.5 and snapshot) which fix
> >> all my issues.
> >>
> >> Modifications:
> >> - mask reserved bits for card type value
> >
> > These bits do not appear to be reserved.  In your case bit 4
> > seems to indicate HS200/200 MHz clock capable.
> >
> > Bit 5 is HS400/400 MHz clock capable.
> >
> >> - read sectors count from EXT_CSD
> >> - fix sectors count and enable SDHC if High Capacity eMMC is detected
> >
> > Is the old method of reading the block length still supported
> > with emmc and csd ver > 2?  That wasn't the case for normal sd card.
> >
> > It seems the emmc situation is a bit different as the capacity
> > stored in a seperate place.
> >
> >> +
> >> +          if (ext_csd[EXT_CSD_REV] >= 2) {
> >> +                  sectors =   ext_csd[EXT_CSD_SEC_COUNT + 0] << 0  |
> >> +                                          ext_csd[EXT_CSD_SEC_COUNT + 1] 
> >> << 8  |
> >> +                                          ext_csd[EXT_CSD_SEC_COUNT + 2] 
> >> << 16 |
> >> +                                          ext_csd[EXT_CSD_SEC_COUNT + 3] 
> >> << 24;
> >> +                  /*
> >> +                   * High capacity MMC seems to report a "magic" 4096 * 
> >> 512 bytes
> >> +                   * capacity in csd, but ext_csd contains the real 
> >> sectors count
> >> +                   */
> >> +                  if ((sf->csd.capacity == (4096 * 512)) &&
> >> +                          (sectors > (2u * 1024 * 1024 * 1024) / 512)) {
> >> +                          sf->flags |= SFF_SDHC;
> >> +                          sf->csd.capacity = sectors;
> >> +                  }
> >
> > I think this should change to
> >
> >                     if (sectors > (2u * 1024 * 1024 * 1024) / 512)
> >                             sf->flags |= SFF_SDHC;
> >                     sf->csd.capacity = sectors;
> >
> > All csd rev > 2 cards should report valid sectors in the extended space.
> >
> 
> This seems not to be the case, the EXT_CSD_SEC_COUNT is only valid for
> capacities over 2G. I'd use this field only if it's not zero, like freebsd 
> does.
> 
> Instead of masking the EXT_CSD_CARD_TYPE field, it may be better to just check
> for the relevant bits as in the diff below.
> 
> Does this still work for you, Cédric?
> 
> 
> Index: sys/dev/sdmmc/sdmmc_mem.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/sdmmc/sdmmc_mem.c,v
> retrieving revision 1.19
> diff -u -p -r1.19 sdmmc_mem.c
> --- sys/dev/sdmmc/sdmmc_mem.c 12 Jul 2014 18:48:52 -0000      1.19
> +++ sys/dev/sdmmc/sdmmc_mem.c 21 Sep 2014 12:02:35 -0000
> @@ -428,6 +428,7 @@ sdmmc_mem_mmc_init(struct sdmmc_softc *s
>       u_int8_t ext_csd[512];
>       int speed = 0;
>       int hs_timing = 0;
> +     u_int32_t sectors = 0;
> 
>       if (sf->csd.mmcver >= MMC_CSD_MMCVER_4_0) {
>               /* read EXT_CSD */
> @@ -439,18 +440,12 @@ sdmmc_mem_mmc_init(struct sdmmc_softc *s
>                       return error;
>               }
> 
> -             switch (ext_csd[EXT_CSD_CARD_TYPE]) {
> -             case EXT_CSD_CARD_TYPE_26M:
> -                     speed = 26000;
> -                     break;
> -             case EXT_CSD_CARD_TYPE_52M:
> -             case EXT_CSD_CARD_TYPE_52M_V18:
> -             case EXT_CSD_CARD_TYPE_52M_V12:
> -             case EXT_CSD_CARD_TYPE_52M_V12_18:
> +             if (ext_csd[EXT_CSD_CARD_TYPE] & EXT_CSD_CARD_TYPE_52M) {
>                       speed = 52000;
>                       hs_timing = 1;
> -                     break;
> -             default:
> +             } else if (ext_csd[EXT_CSD_CARD_TYPE] & EXT_CSD_CARD_TYPE_26M) {
> +                     speed = 26000;
> +             } else {
>                       printf("%s: unknown CARD_TYPE 0x%x\n", DEVNAME(sc),
>                           ext_csd[EXT_CSD_CARD_TYPE]);
>               }
> @@ -487,6 +482,16 @@ sdmmc_mem_mmc_init(struct sdmmc_softc *s
>                               printf("%s, HS_TIMING set failed\n", 
> DEVNAME(sc));
>                               return EINVAL;
>                       }
> +             }
> +
> +             sectors = ext_csd[EXT_CSD_SEC_COUNT + 0] << 0 |
> +                 ext_csd[EXT_CSD_SEC_COUNT + 1] << 8  |
> +                 ext_csd[EXT_CSD_SEC_COUNT + 2] << 16 |
> +                 ext_csd[EXT_CSD_SEC_COUNT + 3] << 24;
> +
> +             if (sectors != 0) {
> +                     sf->flags |= SFF_SDHC;
> +                     sf->csd.capacity = sectors;
>               }
>       }
> 
> Index: sys/dev/sdmmc/sdmmcreg.h
> ===================================================================
> RCS file: /cvs/src/sys/dev/sdmmc/sdmmcreg.h,v
> retrieving revision 1.5
> diff -u -p -r1.5 sdmmcreg.h
> --- sys/dev/sdmmc/sdmmcreg.h  12 Sep 2013 11:54:04 -0000      1.5
> +++ sys/dev/sdmmc/sdmmcreg.h  21 Sep 2014 12:02:35 -0000
> @@ -95,6 +95,7 @@
>  #define EXT_CSD_REV                  192     /* RO */
>  #define EXT_CSD_STRUCTURE            194     /* RO */
>  #define EXT_CSD_CARD_TYPE            196     /* RO */
> +#define EXT_CSD_SEC_COUNT            212     /* RO */
> 
>  /* EXT_CSD field definitions */
>  #define EXT_CSD_CMD_SET_NORMAL               (1U << 0)
> 
> 

Reply via email to