2012/2/6 Jaehoon Chung <jh80.chung@xxxxxxxxxxx>: > On 02/06/2012 03:17 PM, Namjae Jeon wrote: > >> 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. > > Remove? it's sysfs node value..what do you use to show the value if removed that? I found this usage in mmc_cmd23_show. sorry for the noise. > >>> >>> 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 >> > > > -- > 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