Hi Bartlomiej, On 11/15/19 3:01 AM, Bartlomiej Zolnierkiewicz wrote: > > Hi Chanwoo, > > On 11/14/19 2:52 AM, Chanwoo Choi wrote: >> Hi Kamil, >> >> The 'freq_table' and 'max_state' in the devfreq_dev_profile >> were used in the ARM Mali device driver[1][2][3]. Although ARM Mali >> device driver was posted to mainline kernel, they used >> them for a long time. It means that this patch break >> the compatibility. The ARM Mali drivers are very >> important devfreq device driver. > > This argument is not a a technical one and the official upstream > kernel policy is to not depend on out-of-tree drivers. > > Besides the ARM Mali drivers are full of code like: > > #if LINUX_VERSION_CODE >= KERNEL_VERSION(x, y, z) > ... > #else > ... > #endif > > so few more instances of similar code won't do any harm.. ;-) > >> [1] https://developer.arm.com/tools-and-software/graphics-and-gaming/mali-drivers/bifrost-kernel# >> [2] https://developer.arm.com/tools-and-software/graphics-and-gaming/mali-drivers/midgard-kernel >> [3] https://developer.arm.com/tools-and-software/graphics-and-gaming/mali-drivers/utgard-kernel > > I took a look at ARM Mali drivers source code anyway and I fail to > see a rationale behind their behavior of doing 'freq_table' and > 'max_state' initialization in the driver itself (instead of leaving > it up to the devfreq core code, like all in-kernel drivers are doing > already). > > Could you please explain rationale behind ARM Mali drivers' special > needs? > > [ Both ARM Mali and devfreq core code are using generic PM OPP code > these days to do 'freq_table' and 'max_state' initialization, the > only difference seems to be that ARM Mali creates the frequency > table in the descending order (but there also seems to be no real > need for it). ] > > Maybe this is an opportunity to simplify also the ARM Mali driver? OK. I agree to simplify them on this time. For touching the 'freq_table' and 'max_state', need to fix the descending order of freq_table. The partition_enable_ops() in the drivers/thermal/devfreq_cooling.c requires the descending order of freq_table. Have to change it by using the ascending time or support both ascending and descending order for freq_table. 1. Move freq_table, max_state of devfreq_dev_profile to struct devfreq 2. Edit partition_enable_ops() in the drivers/thermal/devfreq_cooling.c by using ascending order instead of descending order. > >> Also, the devfreq device driver specifies their own >> information and data into devfreq_dev_profile structure >> before registering the devfreq device with devfreq_add_device(). >> This patch breaks the basic usage rule of devfreq_dev_profile structure. > > Well, 'struct devfreq_stats *stats' can be trivially moved out of > 'struct devfreq_profile' to 'struct devfreq' if you prefer it that > way.. > > Best regards, > -- > Bartlomiej Zolnierkiewicz > Samsung R&D Institute Poland > Samsung Electronics > >> So, I can't agree this patch. Not ack. >> >> Regards, >> Chanwoo Choi >> >> On 11/13/19 6:13 PM, Kamil Konieczny wrote: >>> Count time and transitions between devfreq frequencies in separate struct >>> for improved code readability and maintenance. >>> >>> Signed-off-by: Kamil Konieczny <k.konieczny@xxxxxxxxxxx> >>> --- >>> drivers/devfreq/devfreq.c | 156 ++++++++++++++++------------- >>> drivers/devfreq/exynos-bus.c | 6 +- >>> drivers/devfreq/governor_passive.c | 26 +++-- >>> include/linux/devfreq.h | 43 ++++---- >>> 4 files changed, 129 insertions(+), 102 deletions(-) >>> >>> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c >>> index d79412b0de59..d85867a91230 100644 >>> --- a/drivers/devfreq/devfreq.c >>> +++ b/drivers/devfreq/devfreq.c >>> @@ -105,10 +105,11 @@ static unsigned long find_available_max_freq(struct devfreq *devfreq) >>> */ >>> static int devfreq_get_freq_level(struct devfreq *devfreq, unsigned long freq) >>> { >>> + struct devfreq_stats *stats = devfreq->profile->stats; >>> int lev; >>> >>> - for (lev = 0; lev < devfreq->profile->max_state; lev++) >>> - if (freq == devfreq->profile->freq_table[lev]) >>> + for (lev = 0; lev < stats->max_state; lev++) >>> + if (freq == stats->freq_table[lev]) >>> return lev; >>> >>> return -EINVAL; >>> @@ -117,56 +118,64 @@ static int devfreq_get_freq_level(struct devfreq *devfreq, unsigned long freq) >>> static int set_freq_table(struct devfreq *devfreq) >>> { >>> struct devfreq_dev_profile *profile = devfreq->profile; >>> + struct devfreq_stats *stats; >>> struct dev_pm_opp *opp; >>> unsigned long freq; >>> - int i, count; >>> + int i, count, err = -ENOMEM; >>> >>> /* Initialize the freq_table from OPP table */ >>> count = dev_pm_opp_get_opp_count(devfreq->dev.parent); >>> if (count <= 0) >>> return -EINVAL; >>> >>> - profile->max_state = count; >>> - profile->freq_table = devm_kcalloc(devfreq->dev.parent, >>> - count, >>> - sizeof(*profile->freq_table), >>> - GFP_KERNEL); >>> - if (!profile->freq_table) { >>> - profile->max_state = 0; >>> + stats = devm_kzalloc(devfreq->dev.parent, >>> + sizeof(struct devfreq_stats), GFP_KERNEL); >>> + if (!stats) >>> return -ENOMEM; >>> - } >>> >>> - for (i = 0, freq = 0; i < profile->max_state; i++, freq++) { >>> + profile->stats = stats; >>> + stats->max_state = count; >>> + stats->freq_table = devm_kcalloc(devfreq->dev.parent, >>> + count, >>> + sizeof(*stats->freq_table), >>> + GFP_KERNEL); >>> + if (!stats->freq_table) >>> + goto err_no_mem; >>> + >>> + for (i = 0, freq = 0; i < count; i++, freq++) { >>> opp = dev_pm_opp_find_freq_ceil(devfreq->dev.parent, &freq); >>> if (IS_ERR(opp)) { >>> - devm_kfree(devfreq->dev.parent, profile->freq_table); >>> - profile->max_state = 0; >>> - return PTR_ERR(opp); >>> + devm_kfree(devfreq->dev.parent, stats->freq_table); >>> + stats->max_state = 0; >>> + err = PTR_ERR(opp); >>> + goto err_no_mem; >>> } >>> dev_pm_opp_put(opp); >>> - profile->freq_table[i] = freq; >>> + stats->freq_table[i] = freq; >>> } >>> >>> - profile->trans_table = devm_kzalloc(devfreq->dev.parent, >>> - array3_size(sizeof(unsigned int), >>> - count, count), >>> - GFP_KERNEL); >>> - if (!profile->trans_table) >>> + stats->trans_table = devm_kzalloc(devfreq->dev.parent, >>> + array3_size(sizeof(unsigned int), >>> + count, count), >>> + GFP_KERNEL); >>> + if (!stats->trans_table) >>> goto err_no_mem; >>> >>> - profile->time_in_state = devm_kcalloc(devfreq->dev.parent, count, >>> - sizeof(*profile->time_in_state), >>> - GFP_KERNEL); >>> - if (!profile->time_in_state) >>> + stats->time_in_state = devm_kcalloc(devfreq->dev.parent, count, >>> + sizeof(*stats->time_in_state), >>> + GFP_KERNEL); >>> + if (!stats->time_in_state) >>> goto err_no_mem; >>> >>> - profile->last_time = get_jiffies_64(); >>> - spin_lock_init(&profile->stats_lock); >>> + stats->last_time = get_jiffies_64(); >>> + spin_lock_init(&stats->stats_lock); >>> >>> return 0; >>> err_no_mem: >>> - profile->max_state = 0; >>> - return -ENOMEM; >>> + stats->max_state = 0; >>> + devm_kfree(devfreq->dev.parent, profile->stats); >>> + profile->stats = NULL; >>> + return err; >>> } >>> >>> /** >>> @@ -176,7 +185,7 @@ static int set_freq_table(struct devfreq *devfreq) >>> */ >>> int devfreq_update_status(struct devfreq *devfreq, unsigned long freq) >>> { >>> - struct devfreq_dev_profile *profile = devfreq->profile; >>> + struct devfreq_stats *stats = devfreq->profile->stats; >>> unsigned long long cur_time; >>> int lev, prev_lev, ret = 0; >>> >>> @@ -184,22 +193,21 @@ int devfreq_update_status(struct devfreq *devfreq, unsigned long freq) >>> >>> /* Immediately exit if previous_freq is not initialized yet. */ >>> if (!devfreq->previous_freq) { >>> - spin_lock(&profile->stats_lock); >>> - profile->last_time = cur_time; >>> - spin_unlock(&profile->stats_lock); >>> + spin_lock(&stats->stats_lock); >>> + stats->last_time = cur_time; >>> + spin_unlock(&stats->stats_lock); >>> return 0; >>> } >>> >>> prev_lev = devfreq_get_freq_level(devfreq, devfreq->previous_freq); >>> >>> - spin_lock(&profile->stats_lock); >>> + spin_lock(&stats->stats_lock); >>> if (prev_lev < 0) { >>> ret = prev_lev; >>> goto out; >>> } >>> >>> - profile->time_in_state[prev_lev] += >>> - cur_time - profile->last_time; >>> + stats->time_in_state[prev_lev] += cur_time - stats->last_time; >>> lev = devfreq_get_freq_level(devfreq, freq); >>> if (lev < 0) { >>> ret = lev; >>> @@ -207,14 +215,14 @@ int devfreq_update_status(struct devfreq *devfreq, unsigned long freq) >>> } >>> >>> if (lev != prev_lev) { >>> - profile->trans_table[(prev_lev * >>> - profile->max_state) + lev]++; >>> - profile->total_trans++; >>> + stats->trans_table[(prev_lev * >>> + stats->max_state) + lev]++; >>> + stats->total_trans++; >>> } >>> >>> out: >>> - profile->last_time = cur_time; >>> - spin_unlock(&profile->stats_lock); >>> + stats->last_time = cur_time; >>> + spin_unlock(&stats->stats_lock); >>> return ret; >>> } >>> EXPORT_SYMBOL(devfreq_update_status); >>> @@ -504,9 +512,9 @@ void devfreq_monitor_resume(struct devfreq *devfreq) >>> queue_delayed_work(devfreq_wq, &devfreq->work, >>> msecs_to_jiffies(profile->polling_ms)); >>> >>> - spin_lock(&profile->stats_lock); >>> - profile->last_time = get_jiffies_64(); >>> - spin_unlock(&profile->stats_lock); >>> + spin_lock(&profile->stats->stats_lock); >>> + profile->stats->last_time = get_jiffies_64(); >>> + spin_unlock(&profile->stats->stats_lock); >>> devfreq->stop_polling = false; >>> >>> if (profile->get_cur_freq && >>> @@ -677,7 +685,7 @@ struct devfreq *devfreq_add_device(struct device *dev, >>> devfreq->data = data; >>> devfreq->nb.notifier_call = devfreq_notifier_call; >>> >>> - if (!profile->max_state && !profile->freq_table) { >>> + if (!profile->stats) { >>> mutex_unlock(&devfreq->lock); >>> err = set_freq_table(devfreq); >>> if (err < 0) >>> @@ -1282,6 +1290,7 @@ static ssize_t min_freq_store(struct device *dev, struct device_attribute *attr, >>> const char *buf, size_t count) >>> { >>> struct devfreq *df = to_devfreq(dev); >>> + struct devfreq_stats *stats = df->profile->stats; >>> unsigned long value; >>> int ret; >>> >>> @@ -1297,13 +1306,13 @@ static ssize_t min_freq_store(struct device *dev, struct device_attribute *attr, >>> goto unlock; >>> } >>> } else { >>> - unsigned long *freq_table = df->profile->freq_table; >>> + unsigned long *freq_table = stats->freq_table; >>> >>> /* Get minimum frequency according to sorting order */ >>> - if (freq_table[0] < freq_table[df->profile->max_state - 1]) >>> + if (freq_table[0] < freq_table[stats->max_state - 1]) >>> value = freq_table[0]; >>> else >>> - value = freq_table[df->profile->max_state - 1]; >>> + value = freq_table[stats->max_state - 1]; >>> } >>> >>> df->min_freq = value; >>> @@ -1326,6 +1335,7 @@ static ssize_t max_freq_store(struct device *dev, struct device_attribute *attr, >>> const char *buf, size_t count) >>> { >>> struct devfreq *df = to_devfreq(dev); >>> + struct devfreq_stats *stats = df->profile->stats; >>> unsigned long value; >>> int ret; >>> >>> @@ -1341,11 +1351,11 @@ static ssize_t max_freq_store(struct device *dev, struct device_attribute *attr, >>> goto unlock; >>> } >>> } else { >>> - unsigned long *freq_table = df->profile->freq_table; >>> + unsigned long *freq_table = stats->freq_table; >>> >>> /* Get maximum frequency according to sorting order */ >>> - if (freq_table[0] < freq_table[df->profile->max_state - 1]) >>> - value = freq_table[df->profile->max_state - 1]; >>> + if (freq_table[0] < freq_table[stats->max_state - 1]) >>> + value = freq_table[stats->max_state - 1]; >>> else >>> value = freq_table[0]; >>> } >>> @@ -1373,14 +1383,15 @@ static ssize_t available_frequencies_show(struct device *d, >>> char *buf) >>> { >>> struct devfreq *df = to_devfreq(d); >>> + struct devfreq_stats *stats = df->profile->stats; >>> ssize_t count = 0; >>> int i; >>> >>> mutex_lock(&df->lock); >>> >>> - for (i = 0; i < df->profile->max_state; i++) >>> + for (i = 0; i < stats->max_state; i++) >>> count += scnprintf(&buf[count], (PAGE_SIZE - count - 2), >>> - "%lu ", df->profile->freq_table[i]); >>> + "%lu ", stats->freq_table[i]); >>> >>> mutex_unlock(&df->lock); >>> /* Truncate the trailing space */ >>> @@ -1398,9 +1409,10 @@ static ssize_t trans_stat_show(struct device *dev, >>> { >>> struct devfreq *devfreq = to_devfreq(dev); >>> struct devfreq_dev_profile *profile = devfreq->profile; >>> + struct devfreq_stats *stats = profile->stats; >>> + unsigned int max_state = stats->max_state; >>> ssize_t len; >>> int i, j; >>> - unsigned int max_state = profile->max_state; >>> >>> if (!devfreq->stop_polling && >>> devfreq_update_status(devfreq, devfreq->previous_freq)) >>> @@ -1411,45 +1423,45 @@ static ssize_t trans_stat_show(struct device *dev, >>> len = sprintf(buf, " From : To\n"); >>> len += sprintf(buf + len, " :"); >>> >>> - spin_lock(&profile->stats_lock); >>> + spin_lock(&stats->stats_lock); >>> for (i = 0; i < max_state; i++) >>> len += sprintf(buf + len, "%10lu", >>> - profile->freq_table[i]); >>> + stats->freq_table[i]); >>> >>> len += sprintf(buf + len, " time(ms)\n"); >>> >>> for (i = 0; i < max_state; i++) { >>> - if (profile->freq_table[i] == devfreq->previous_freq) >>> + if (stats->freq_table[i] == devfreq->previous_freq) >>> len += sprintf(buf + len, "*"); >>> else >>> len += sprintf(buf + len, " "); >>> >>> len += sprintf(buf + len, "%10lu:", >>> - profile->freq_table[i]); >>> + stats->freq_table[i]); >>> for (j = 0; j < max_state; j++) >>> len += sprintf(buf + len, "%10u", >>> - profile->trans_table[(i * max_state) + j]); >>> + stats->trans_table[(i * max_state) + j]); >>> len += sprintf(buf + len, "%10llu\n", (u64) >>> - jiffies64_to_msecs(profile->time_in_state[i])); >>> + jiffies64_to_msecs(stats->time_in_state[i])); >>> } >>> >>> len += sprintf(buf + len, "Total transition : %u\n", >>> - profile->total_trans); >>> - spin_unlock(&profile->stats_lock); >>> + stats->total_trans); >>> + spin_unlock(&stats->stats_lock); >>> return len; >>> } >>> static DEVICE_ATTR_RO(trans_stat); >>> >>> -static void defvreq_stats_clear_table(struct devfreq_dev_profile *profile) >>> +static void defvreq_stats_clear_table(struct devfreq_stats *stats) >>> { >>> - unsigned int count = profile->max_state; >>> - >>> - spin_lock(&profile->stats_lock); >>> - memset(profile->time_in_state, 0, count * sizeof(u64)); >>> - memset(profile->trans_table, 0, count * count * sizeof(int)); >>> - profile->last_time = get_jiffies_64(); >>> - profile->total_trans = 0; >>> - spin_unlock(&profile->stats_lock); >>> + unsigned int count = stats->max_state; >>> + >>> + spin_lock(&stats->stats_lock); >>> + memset(stats->time_in_state, 0, count * sizeof(u64)); >>> + memset(stats->trans_table, 0, count * count * sizeof(int)); >>> + stats->last_time = get_jiffies_64(); >>> + stats->total_trans = 0; >>> + spin_unlock(&stats->stats_lock); >>> } >>> >>> static ssize_t trans_reset_store(struct device *dev, >>> @@ -1459,7 +1471,7 @@ static ssize_t trans_reset_store(struct device *dev, >>> { >>> struct devfreq *devfreq = to_devfreq(dev); >>> >>> - defvreq_stats_clear_table(devfreq->profile); >>> + defvreq_stats_clear_table(devfreq->profile->stats); >>> >>> return count; >>> } >>> diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c >>> index d9f377912c10..b212aae2bb3e 100644 >>> --- a/drivers/devfreq/exynos-bus.c >>> +++ b/drivers/devfreq/exynos-bus.c >>> @@ -496,9 +496,9 @@ static int exynos_bus_probe(struct platform_device *pdev) >>> } >>> >>> out: >>> - max_state = bus->devfreq->profile->max_state; >>> - min_freq = (bus->devfreq->profile->freq_table[0] / 1000); >>> - max_freq = (bus->devfreq->profile->freq_table[max_state - 1] / 1000); >>> + max_state = profile->stats->max_state; >>> + min_freq = (profile->stats->freq_table[0] / 1000); >>> + max_freq = (profile->stats->freq_table[max_state - 1] / 1000); >>> pr_info("exynos-bus: new bus device registered: %s (%6ld KHz ~ %6ld KHz)\n", >>> dev_name(dev), min_freq, max_freq); >>> >>> diff --git a/drivers/devfreq/governor_passive.c b/drivers/devfreq/governor_passive.c >>> index 58308948b863..b2d87a88335c 100644 >>> --- a/drivers/devfreq/governor_passive.c >>> +++ b/drivers/devfreq/governor_passive.c >>> @@ -18,6 +18,8 @@ static int devfreq_passive_get_target_freq(struct devfreq *devfreq, >>> struct devfreq_passive_data *p_data >>> = (struct devfreq_passive_data *)devfreq->data; >>> struct devfreq *parent_devfreq = (struct devfreq *)p_data->parent; >>> + struct devfreq_stats *parent_stats = parent_devfreq->profile->stats; >>> + struct devfreq_stats *stats; >>> unsigned long child_freq = ULONG_MAX; >>> struct dev_pm_opp *opp; >>> int i, count, ret = 0; >>> @@ -47,10 +49,14 @@ static int devfreq_passive_get_target_freq(struct devfreq *devfreq, >>> * device. And then the index is used for getting the suitable >>> * new frequency for passive devfreq device. >>> */ >>> - if (!devfreq->profile || !devfreq->profile->freq_table >>> - || devfreq->profile->max_state <= 0) >>> + if (!devfreq->profile || !devfreq->profile->stats || >>> + devfreq->profile->stats->max_state <= 0 || >>> + !parent_devfreq->profile || !parent_devfreq->profile->stats || >>> + parent_devfreq->profile->stats->max_state <= 0) >>> return -EINVAL; >>> >>> + stats = devfreq->profile->stats; >>> + parent_stats = parent_devfreq->profile->stats; >>> /* >>> * The passive governor have to get the correct frequency from OPP >>> * list of parent device. Because in this case, *freq is temporary >>> @@ -68,21 +74,21 @@ static int devfreq_passive_get_target_freq(struct devfreq *devfreq, >>> * Get the OPP table's index of decided freqeuncy by governor >>> * of parent device. >>> */ >>> - for (i = 0; i < parent_devfreq->profile->max_state; i++) >>> - if (parent_devfreq->profile->freq_table[i] == *freq) >>> + for (i = 0; i < parent_stats->max_state; i++) >>> + if (parent_stats->freq_table[i] == *freq) >>> break; >>> >>> - if (i == parent_devfreq->profile->max_state) { >>> + if (i == parent_stats->max_state) { >>> ret = -EINVAL; >>> goto out; >>> } >>> >>> /* Get the suitable frequency by using index of parent device. */ >>> - if (i < devfreq->profile->max_state) { >>> - child_freq = devfreq->profile->freq_table[i]; >>> + if (i < stats->max_state) { >>> + child_freq = stats->freq_table[i]; >>> } else { >>> - count = devfreq->profile->max_state; >>> - child_freq = devfreq->profile->freq_table[count - 1]; >>> + count = stats->max_state; >>> + child_freq = stats->freq_table[count - 1]; >>> } >>> >>> /* Return the suitable frequency for passive device. */ >>> @@ -109,7 +115,7 @@ static int update_devfreq_passive(struct devfreq *devfreq, unsigned long freq) >>> if (ret < 0) >>> goto out; >>> >>> - if (devfreq->profile->freq_table >>> + if (devfreq->profile->stats >>> && (devfreq_update_status(devfreq, freq))) >>> dev_err(&devfreq->dev, >>> "Couldn't update frequency transition information.\n"); >>> diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h >>> index 4ceb2a517a9c..8459af1a1583 100644 >>> --- a/include/linux/devfreq.h >>> +++ b/include/linux/devfreq.h >>> @@ -64,6 +64,30 @@ struct devfreq_dev_status { >>> */ >>> #define DEVFREQ_FLAG_LEAST_UPPER_BOUND 0x1 >>> >>> +/** >>> + * struct devfreq_stats - Devfreq's transitions stats counters >>> + * @freq_table: Optional list of frequencies to support statistics >>> + * and freq_table must be generated in ascending order. >>> + * @max_state: The size of freq_table. >>> + * @total_trans: Number of devfreq transitions >>> + * @trans_table: Statistics of devfreq transitions >>> + * @time_in_state: Statistics of devfreq states >>> + * @last_time: The last time stats were updated >>> + * @stats_lock: Lock protecting trans_table, time_in_state, >>> + * last_time and total_trans used for statistics >>> + */ >>> +struct devfreq_stats { >>> + unsigned long *freq_table; >>> + unsigned int max_state; >>> + >>> + /* information for device frequency transition */ >>> + unsigned int total_trans; >>> + unsigned int *trans_table; >>> + u64 *time_in_state; >>> + unsigned long long last_time; >>> + spinlock_t stats_lock; >>> +}; >>> + >>> /** >>> * struct devfreq_dev_profile - Devfreq's user device profile >>> * @initial_freq: The operating frequency when devfreq_add_device() is >>> @@ -88,15 +112,7 @@ struct devfreq_dev_status { >>> * from devfreq_remove_device() call. If the user >>> * has registered devfreq->nb at a notifier-head, >>> * this is the time to unregister it. >>> - * @freq_table: Optional list of frequencies to support statistics >>> - * and freq_table must be generated in ascending order. >>> - * @max_state: The size of freq_table. >>> - * @total_trans: Number of devfreq transitions >>> - * @trans_table: Statistics of devfreq transitions >>> - * @time_in_state: Statistics of devfreq states >>> - * @last_time: The last time stats were updated >>> - * @stats_lock: Lock protecting trans_table, time_in_state, >>> - * last_time and total_trans used for statistics >>> + * @stats: Statistics of devfreq states and state transitions >>> */ >>> struct devfreq_dev_profile { >>> unsigned long initial_freq; >>> @@ -108,14 +124,7 @@ struct devfreq_dev_profile { >>> int (*get_cur_freq)(struct device *dev, unsigned long *freq); >>> void (*exit)(struct device *dev); >>> >>> - unsigned long *freq_table; >>> - unsigned int max_state; >>> - /* information for device frequency transition */ >>> - unsigned int total_trans; >>> - unsigned int *trans_table; >>> - u64 *time_in_state; >>> - unsigned long long last_time; >>> - spinlock_t stats_lock; >>> + struct devfreq_stats *stats; >>> }; >>> >>> /** >>> > > -- Best Regards, Chanwoo Choi Samsung Electronics