| Message ID | 20200713083113.5595-2-cw00.choi@samsung.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Chanwoo Choi |
| Headers | show |
| Series | PM / devfreq: Add governor flags | expand |
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.
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;
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(-)