Messages in this thread | | | Subject | Re: [PATCH v2 1/2] PM / devfreq: Clean up the devfreq instance name in sysfs attr | From | Chanwoo Choi <> | Date | Fri, 24 Jul 2020 10:42:02 +0900 |
| |
On 7/13/20 5:31 PM, Chanwoo Choi wrote: > The sysfs attr interface used eithere 'df' or 'devfreq' for devfreq instance > name. In order to keep the consistency and to improve the readabilty, > unify the instance name as 'df'. Add add the missing conditional statement > to prevent the fault. > > Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com> > --- > drivers/devfreq/devfreq.c | 94 +++++++++++++++++++++++++-------------- > 1 file changed, 60 insertions(+), 34 deletions(-) > > diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c > index 5320c3b37f35..286957f760f1 100644 > --- a/drivers/devfreq/devfreq.c > +++ b/drivers/devfreq/devfreq.c > @@ -1280,18 +1280,20 @@ EXPORT_SYMBOL(devfreq_remove_governor); > static ssize_t name_show(struct device *dev, > struct device_attribute *attr, char *buf) > { > - struct devfreq *devfreq = to_devfreq(dev); > - return sprintf(buf, "%s\n", dev_name(devfreq->dev.parent)); > + struct devfreq *df = to_devfreq(dev); > + return sprintf(buf, "%s\n", dev_name(df->dev.parent)); > } > static DEVICE_ATTR_RO(name); > > static ssize_t governor_show(struct device *dev, > struct device_attribute *attr, char *buf) > { > - if (!to_devfreq(dev)->governor) > + struct devfreq *df = to_devfreq(dev); > + > + if (!df->governor) > return -EINVAL; > > - return sprintf(buf, "%s\n", to_devfreq(dev)->governor->name); > + return sprintf(buf, "%s\n", df->governor->name); > } > > static ssize_t governor_store(struct device *dev, struct device_attribute *attr, > @@ -1302,6 +1304,9 @@ static ssize_t governor_store(struct device *dev, struct device_attribute *attr, > char str_governor[DEVFREQ_NAME_LEN + 1]; > const struct devfreq_governor *governor, *prev_governor; > > + if (!df->governor) > + return -EINVAL; > + > ret = sscanf(buf, "%" __stringify(DEVFREQ_NAME_LEN) "s", str_governor); > if (ret != 1) > return -EINVAL; > @@ -1315,20 +1320,18 @@ static ssize_t governor_store(struct device *dev, struct device_attribute *attr, > if (df->governor == governor) { > ret = 0; > goto out; > - } else if ((df->governor && df->governor->immutable) || > - governor->immutable) { > + } else if (df->governor->immutable || governor->immutable) { > ret = -EINVAL; > goto out; > } > > - if (df->governor) { > - ret = df->governor->event_handler(df, DEVFREQ_GOV_STOP, NULL); > - if (ret) { > - dev_warn(dev, "%s: Governor %s not stopped(%d)\n", > - __func__, df->governor->name, ret); > - goto out; > - } > + ret = df->governor->event_handler(df, DEVFREQ_GOV_STOP, NULL); > + if (ret) { > + dev_warn(dev, "%s: Governor %s not stopped(%d)\n", > + __func__, df->governor->name, ret); > + goto out; > } > + > prev_governor = df->governor; > df->governor = governor; > strncpy(df->governor_name, governor->name, DEVFREQ_NAME_LEN); > @@ -1363,13 +1366,16 @@ static ssize_t available_governors_show(struct device *d, > struct devfreq *df = to_devfreq(d); > ssize_t count = 0; > > + if (!df->governor) > + return -EINVAL; > + > mutex_lock(&devfreq_list_lock); > > /* > * The devfreq with immutable governor (e.g., passive) shows > * only own governor. > */ > - if (df->governor && df->governor->immutable) { > + if (df->governor->immutable) { > count = scnprintf(&buf[count], DEVFREQ_NAME_LEN, > "%s ", df->governor_name); > /* > @@ -1403,27 +1409,37 @@ static ssize_t cur_freq_show(struct device *dev, struct device_attribute *attr, > char *buf) > { > unsigned long freq; > - struct devfreq *devfreq = to_devfreq(dev); > + struct devfreq *df = to_devfreq(dev); > > - if (devfreq->profile->get_cur_freq && > - !devfreq->profile->get_cur_freq(devfreq->dev.parent, &freq)) > + if (!df->profile) > + return -EINVAL; > + > + if (df->profile->get_cur_freq && > + !df->profile->get_cur_freq(df->dev.parent, &freq)) > return sprintf(buf, "%lu\n", freq); > > - return sprintf(buf, "%lu\n", devfreq->previous_freq); > + return sprintf(buf, "%lu\n", df->previous_freq); > } > static DEVICE_ATTR_RO(cur_freq); > > static ssize_t target_freq_show(struct device *dev, > struct device_attribute *attr, char *buf) > { > - return sprintf(buf, "%lu\n", to_devfreq(dev)->previous_freq); > + struct devfreq *df = to_devfreq(dev); > + > + return sprintf(buf, "%lu\n", df->previous_freq); > } > static DEVICE_ATTR_RO(target_freq); > > static ssize_t polling_interval_show(struct device *dev, > struct device_attribute *attr, char *buf) > { > - return sprintf(buf, "%d\n", to_devfreq(dev)->profile->polling_ms); > + struct devfreq *df = to_devfreq(dev); > + > + if (!df->profile) > + return -EINVAL; > + > + return sprintf(buf, "%d\n", df->profile->polling_ms); > } > > static ssize_t polling_interval_store(struct device *dev, > @@ -1551,6 +1567,9 @@ static ssize_t available_frequencies_show(struct device *d, > ssize_t count = 0; > int i; > > + if (!df->profile) > + return -EINVAL; > + > mutex_lock(&df->lock); > > for (i = 0; i < df->profile->max_state; i++) > @@ -1571,49 +1590,53 @@ static DEVICE_ATTR_RO(available_frequencies); > static ssize_t trans_stat_show(struct device *dev, > struct device_attribute *attr, char *buf) > { > - struct devfreq *devfreq = to_devfreq(dev); > + struct devfreq *df = to_devfreq(dev); > ssize_t len; > int i, j; > - unsigned int max_state = devfreq->profile->max_state; > + unsigned int max_state; > + > + if (!df->profile) > + return -EINVAL; > + max_state = df->profile->max_state; > > if (max_state == 0) > return sprintf(buf, "Not Supported.\n"); > > - mutex_lock(&devfreq->lock); > - if (!devfreq->stop_polling && > - devfreq_update_status(devfreq, devfreq->previous_freq)) { > - mutex_unlock(&devfreq->lock); > + mutex_lock(&df->lock); > + if (!df->stop_polling && > + devfreq_update_status(df, df->previous_freq)) { > + mutex_unlock(&df->lock); > return 0; > } > - mutex_unlock(&devfreq->lock); > + mutex_unlock(&df->lock); > > len = sprintf(buf, " From : To\n"); > len += sprintf(buf + len, " :"); > for (i = 0; i < max_state; i++) > len += sprintf(buf + len, "%10lu", > - devfreq->profile->freq_table[i]); > + df->profile->freq_table[i]); > > len += sprintf(buf + len, " time(ms)\n"); > > for (i = 0; i < max_state; i++) { > - if (devfreq->profile->freq_table[i] > - == devfreq->previous_freq) { > + if (df->profile->freq_table[i] > + == df->previous_freq) { > len += sprintf(buf + len, "*"); > } else { > len += sprintf(buf + len, " "); > } > len += sprintf(buf + len, "%10lu:", > - devfreq->profile->freq_table[i]); > + df->profile->freq_table[i]); > for (j = 0; j < max_state; j++) > len += sprintf(buf + len, "%10u", > - devfreq->stats.trans_table[(i * max_state) + j]); > + df->stats.trans_table[(i * max_state) + j]); > > len += sprintf(buf + len, "%10llu\n", (u64) > - jiffies64_to_msecs(devfreq->stats.time_in_state[i])); > + jiffies64_to_msecs(df->stats.time_in_state[i])); > } > > len += sprintf(buf + len, "Total transition : %u\n", > - devfreq->stats.total_trans); > + df->stats.total_trans); > return len; > } > > @@ -1624,6 +1647,9 @@ static ssize_t trans_stat_store(struct device *dev, > struct devfreq *df = to_devfreq(dev); > int err, value; > > + if (!df->profile) > + return -EINVAL; > + > if (df->profile->max_state == 0) > return count; > >
Applied it. It is just clean-up patch for patch2. patch2 needs more discussion. So, only apply patch1.
-- Best Regards, Chanwoo Choi Samsung Electronics
| |