lkml.org 
[lkml]   [2011]   [Jan]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRE: [PATCH v2 1/1]mmc: implemented eMMC4.4 enhanced area feature
    Hi Chris,
    Thanks for comments. I will fix the typo and line warp errors in the next submissions. :)

    > > MMC_DEV_ATTR(name, "%s\n", card->cid.prod_name);
    > > MMC_DEV_ATTR(oemid, "0x%04x\n", card->cid.oemid);
    > > MMC_DEV_ATTR(serial, "0x%08x\n", card->cid.serial);
    > > +MMC_DEV_ATTR(enhanced_area_offset, "0x%llxB\n",
    > > + card->ext_csd.enhanced_area_offset);
    > > +MMC_DEV_ATTR(enhanced_area_size, "0x%xKB\n",
    > card->ext_csd.enhanced_area_size);
    >
    > I think these should probably be printed in decimal instead of hex.
    >
    > Hm, this way the sysfs files will show up regardless of whether we have
    > an eMMC device or an SD card, which is very confusing. Can you not check
    > whether enhanced_area_en == 1 before create these files?
    >
    SD card will not use this code when initializing. So I think these files only are created for eMMC card. If the eMMC card doesn't support enhanced area, can we use the same way like below scenario, exporting some value like -EINVAL to user? What do you think?

    > >
    > > static struct attribute *mmc_std_attrs[] = {
    > > &dev_attr_cid.attr,
    > > @@ -349,6 +387,8 @@ static struct attribute *mmc_std_attrs[] = {
    > > &dev_attr_name.attr,
    > > &dev_attr_oemid.attr,
    > > &dev_attr_serial.attr,
    > > + &dev_attr_enhanced_area_offset.attr,
    > > + &dev_attr_enhanced_area_size.attr,
    > > NULL,
    > > };
    > >
    > > @@ -484,6 +524,39 @@ static int mmc_init_card(struct mmc_host *host, u32
    > ocr,
    > > }
    > >
    > > /*
    > > + * If enhanced_area_en is TRUE, host need to enable
    > > + * ERASE_GRP_DEF bit.
    > > + * ERASE_GRP_DEF bit will be lost everytime
    > > + * after a reset or power off.
    > > + */
    > > + if (card->ext_csd.enhanced_area_en) {
    > > + err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
    > > + EXT_CSD_ERASE_GROUP_DEF, 1);
    > > +
    > > + if (err && err != -EBADMSG)
    > > + goto free_card;
    > > +
    > > + if (err) {
    > > + err = 0;
    > > + /*
    > > + * Just disable enhanced area off & sz
    > > + * will try to enable ERASE_GROUP_DEF
    > > + * during next time reinit
    > > + */
    > > + card->ext_csd.enhanced_area_offset = 0;
    > > + card->ext_csd.enhanced_area_size = 0;
    >
    > It makes more sense to me to return -EINVAL instead of 0 if we know
    > nothing about the enhanced_area. What do you think?
    Agree. This can let user know the enhanced area offset and size are invalid argument.

    Thanks
    Chuanxiao


    \
     
     \ /
      Last update: 2011-01-20 07:13    [W:0.024 / U:31.332 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site