2012/2/6 Jaehoon Chung <jh80.chung@xxxxxxxxxxx>: > Hi Mr.Jeon. > > On 02/06/2012 01:19 PM, Namjae Jeon wrote: > >> Hi. Jaehoon. >> >> I have questions. >> >> 1. Would you explain why we need on/off function of this CMD23 at runtime ? > > Now..operation mode is set at compile time..That means if we use the pre-defined mode, > must always use the pre-defined mode. > Packed-cmd, Data-tag, Context-Id..etc..almost features need to use CMD23. > But in vendor side, some card need to use the open-ended in order to update the firmware code. > (or not) > If we use the cmd23, to update the firmware code maybe need to rebuild..it's worthless. > So i added this patch. First, Thanks for your explaination. If possible, you can add this description in patch header so that the other understand. > >> 2. While frequently reading/writing, Have you tested by being >> enabling/disbling CMD23 using this function ? > > Yes..i tested that case..and working well. > >> 3. Why is irq disable needed when calling mmc_change_operation_mode ? >> If are we just using spin_lock ? > > that value is how we process the request, so i think that need spin_lock_irq(). > Well..spin_lock()? i think that.. if it can replace spin_lock, it is better to avoid using irq disble. if surely needed, it should be explained. > >> 4. When do we use host->pre_defined_op value after setting it ? > > Just the setting value..to show the value..any other opinion. > If you have the other approach, i will change. I didn't find this value usage in patch. if it is not using in this patch and other elsewhere, plz remove it. > > Best Regards, > Jaehoon Chung > >> >> Thanks. >> >> 2012/2/6 Jaehoon Chung <jh80.chung@xxxxxxxxxxx>: >>> This patch is support the sysfs for operation mode. >>> >>> There are two operation modes(open-ended/pre-defined). >>> Now, operation mode is selected only one at the compile time. >>> >>> But using this patch, we can change the operation mode with node at runtime. >>> >>> * pre-defined mode >>> echo 1 > /sys/class/mmc_host/mmc0/pre_defined_op >>> * open-ended mode >>> echo 0 > /sys/class/mmc_host/mmc0/pre_defined_op >>> >>> Signed-off-by: Jaehoon Chung <jh80.chung@xxxxxxxxxxx> >>> Signed-off-by: Kyungmin Park <kyungmin.park@xxxxxxxxxxx> >>> --- >>> Changelog V4: >>> - Add the explanation in Documentation/mmc/mmc-dev-attrs.txt >>> Changelog V3: >>> - Add the mmc_change_operation_mode() >>> Changelog v2: >>> - Add the check point in mmc_cmd23_store() >>> (If host controller didn't support CMD23, need not to change the ops-mode.) >>> >>> Documentation/mmc/mmc-dev-attrs.txt | 16 ++++++++++++++ >>> drivers/mmc/card/block.c | 19 +++++++++++++++++ >>> drivers/mmc/core/host.c | 39 +++++++++++++++++++++++++++++++++++ >>> include/linux/mmc/card.h | 1 + >>> include/linux/mmc/host.h | 3 ++ >>> 5 files changed, 78 insertions(+), 0 deletions(-) >>> >>> diff --git a/Documentation/mmc/mmc-dev-attrs.txt b/Documentation/mmc/mmc-dev-attrs.txt >>> index 22ae844..e7ba3b6 100644 >>> --- a/Documentation/mmc/mmc-dev-attrs.txt >>> +++ b/Documentation/mmc/mmc-dev-attrs.txt >>> @@ -74,3 +74,19 @@ This attribute appears only if CONFIG_MMC_CLKGATE is enabled. >>> clkgate_delay Tune the clock gating delay with desired value in milliseconds. >>> >>> echo <desired delay> > /sys/class/mmc_host/mmcX/clkgate_delay >>> + >>> +SD and MMC Operation Mode Attribute >>> +========================================== >>> + >>> +SD and MMC can support the two operation mode. >>> +(Pre-defined(CMD23)/Open-ended operation mode) >>> + >>> +This attribute can change the operation mode at runtime if card is supported the CMD23. >>> + >>> + pre_defined_op Support the pre-defined mode if set 1. >>> + >>> +1) Open-ended mode >>> +echo 0 > /sys/class/mmc_host/mmcX/pre_defined_op >>> + >>> +2) Pre-defined mode >>> +echo 1 > /sys/class/mmc_host/mmcX/pre_defined_op >>> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c >>> index a7c75d8..2c1981c 100644 >>> --- a/drivers/mmc/card/block.c >>> +++ b/drivers/mmc/card/block.c >>> @@ -1766,6 +1766,25 @@ static const struct mmc_fixup blk_fixups[] = >>> END_FIXUP >>> }; >>> >>> +void mmc_change_operation_mode(struct mmc_card *card, int val) >>> +{ >>> + struct mmc_blk_data *md = mmc_get_drvdata(card); >>> + >>> + if (mmc_host_cmd23(card->host)) { >>> + if (mmc_card_mmc(card) || >>> + (mmc_card_sd(card) && >>> + card->scr.cmds & SD_SCR_CMD23_SUPPORT)) { >>> + if (val) { >>> + md->flags |= MMC_BLK_CMD23; >>> + card->host->pre_defined_op = val; >>> + } else { >>> + md->flags &= ~MMC_BLK_CMD23; >>> + card->host->pre_defined_op = val; >>> + } >>> + } >>> + } >>> +} >>> + >>> static int mmc_blk_probe(struct mmc_card *card) >>> { >>> struct mmc_blk_data *md, *part_md; >>> diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c >>> index 30055f2..7dab0dc 100644 >>> --- a/drivers/mmc/core/host.c >>> +++ b/drivers/mmc/core/host.c >>> @@ -293,6 +293,41 @@ static inline void mmc_host_clk_sysfs_init(struct mmc_host *host) >>> >>> #endif >>> >>> +static ssize_t mmc_cmd23_show(struct device *dev, >>> + struct device_attribute *attr, char *buf) >>> +{ >>> + struct mmc_host *host = cls_dev_to_mmc_host(dev); >>> + return snprintf(buf, PAGE_SIZE, "%d\n", host->pre_defined_op); >>> +} >>> + >>> +static ssize_t mmc_cmd23_store(struct device *dev, >>> + struct device_attribute *attr, const char *buf, size_t count) >>> +{ >>> + struct mmc_host *host = cls_dev_to_mmc_host(dev); >>> + int value; >>> + unsigned long flags; >>> + >>> + if (kstrtoint(buf, 0, &value)) >>> + return -EINVAL; >>> + >>> + spin_lock_irqsave(&host->lock, flags); >>> + mmc_change_operation_mode(host->card, value); >>> + spin_unlock_irqrestore(&host->lock, flags); >>> + return count; >>> +} >>> + >>> +static inline void mmc_host_cmd23_sysfs_init(struct mmc_host *host) >>> +{ >>> + host->pre_defined_attr.show = mmc_cmd23_show; >>> + host->pre_defined_attr.store = mmc_cmd23_store; >>> + sysfs_attr_init(&host->pre_defined_attr.attr); >>> + host->pre_defined_attr.attr.name = "pre_defined_op"; >>> + host->pre_defined_attr.attr.mode = S_IRUGO | S_IWUSR; >>> + if (device_create_file(&host->class_dev, &host->pre_defined_attr)) >>> + pr_err("%s: Failed to create pre_defined_op sysfs entry\n", >>> + mmc_hostname(host)); >>> +} >>> + >>> /** >>> * mmc_alloc_host - initialise the per-host structure. >>> * @extra: sizeof private data structure >>> @@ -381,6 +416,10 @@ int mmc_add_host(struct mmc_host *host) >>> #endif >>> mmc_host_clk_sysfs_init(host); >>> >>> + if (host->caps & MMC_CAP_CMD23) >>> + host->pre_defined_op = 1; >>> + mmc_host_cmd23_sysfs_init(host); >>> + >>> mmc_start_host(host); >>> register_pm_notifier(&host->pm_notify); >>> >>> diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h >>> index 1a1ca71..b36d408 100644 >>> --- a/include/linux/mmc/card.h >>> +++ b/include/linux/mmc/card.h >>> @@ -489,5 +489,6 @@ extern void mmc_unregister_driver(struct mmc_driver *); >>> >>> extern void mmc_fixup_device(struct mmc_card *card, >>> const struct mmc_fixup *table); >>> +extern void mmc_change_operation_mode(struct mmc_card *card, int val); >>> >>> #endif /* LINUX_MMC_CARD_H */ >>> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h >>> index 20d7c82..59a30bf 100644 >>> --- a/include/linux/mmc/host.h >>> +++ b/include/linux/mmc/host.h >>> @@ -259,6 +259,9 @@ struct mmc_host { >>> MMC_CAP2_HS200_1_2V_SDR) >>> #define MMC_CAP2_BROKEN_VOLTAGE (1 << 7) /* Use the broken voltage */ >>> >>> + struct device_attribute pre_defined_attr; >>> + int pre_defined_op; /* CMD23 should be use or not */ >>> + >>> mmc_pm_flag_t pm_caps; /* supported pm features */ >>> unsigned int power_notify_type; >>> #define MMC_HOST_PW_NOTIFY_NONE 0 >>> -- >>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in >>> the body of a message to majordomo@xxxxxxxxxxxxxxx >>> More majordomo info at http://vger.kernel.org/majordomo-info.html >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in >> the body of a message to majordomo@xxxxxxxxxxxxxxx >> More majordomo info at http://vger.kernel.org/majordomo-info.html >> > > -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html