Re: [PATCH v4] mmc: core: select the operation mode with sysfs

[Jaehoon Chung <jh80.chung@samsung.com>]

On 02/06/2012 03:17 PM, Namjae Jeon wrote:

> 2012/2/6 Jaehoon Chung <jh80.chung@samsung.com>:
>> 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?

>>
>> Best Regards,
>> Jaehoon Chung
>>
>>>
>>> Thanks.
>>>
>>> 2012/2/6 Jaehoon Chung <jh80.chung@samsung.com>:
>>>> 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@samsung.com>
>>>> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
>>>> ---
>>>> 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@vger.kernel.org
>>>> 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@vger.kernel.org
>>> 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@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>