Subject: Re: [PATCH] PM / devfreq: Lock devfreq in trans_stat_show

From: Matthias Kaehlcke <[email-masked]>
Date: Tue, 24 Sep 2019 08:44:24 -0700

On Tue, Sep 24, 2019 at 07:44:16AM +0000, Leonard Crestez wrote:
> On 2019-09-24 5:07 AM, Chanwoo Choi wrote:

> >> @@ -1507,16 +1508,22 @@ static ssize_t trans_stat_show(struct device *dev,
> >>   	struct devfreq *devfreq = to_devfreq(dev);
> >>   	ssize_t len;
> >>   	int i, j;
> >>   	unsigned int max_state = devfreq->profile->max_state;
> >>   
> >> -	if (!devfreq->stop_polling &&
> >> -			devfreq_update_status(devfreq, devfreq->previous_freq))
> >> -		return 0;
> >>   	if (max_state == 0)
> >>   		return sprintf(buf, "Not Supported.\n");
> >>   
> >> +	/* lock and update */
> > 
> > It is not necessary. Anyone can know that this code is related to mutex lock/unlock.
> 
> OK. You're the second person to mention this but it's quite strange to 
> see objections raised against comments.

Comments are great if they add value, in this case the comment is
stating the obvious, which IMO just adds noise to the code.

The coding style guidelines also briefly touch this topic:

  8) Commenting
  -------------

  Comments are good, but there is also a danger of over-commenting.

Documentation/process/coding-style.rst

_______________________________________________
linux-arm-kernel mailing list
[email-masked]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel