Date: Tue, 22 Mar 2016 14:35:50 +0000
From: Javi Merino <javi.merino@arm.com>
To: Andy Champ <andycham@amazon.com>
Cc: edubezval@gmail.com, corbet@lwn.net, durgadoss.r@intel.com,
        leo.yan@linaro.org, kapileshwar.singh@arm.com, wni@nvidia.com,
        linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] Syntactic and factual errors in the API document
Message-ID: <20160322143529.GB3087@e104805>
References: <1458650245-24303-1-git-send-email-andycham@amazon.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <1458650245-24303-1-git-send-email-andycham@amazon.com>
User-Agent: Mutt/1.5.21 (2010-09-15)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 6445
Lines: 122

Hi Andy,

Thanks for improving the documentation!  One minor nit below.  Other
than that, you can add my reviewed-by.

On Tue, Mar 22, 2016 at 12:37:25PM +0000, Andy Champ wrote:
> There are several places where the English in the document is syntactically
> invalid, or unclear. There are also one or two factual errors.
> 
> ---
>  Documentation/thermal/sysfs-api.txt | 44 ++++++++++++++++++-------------------
>  1 file changed, 22 insertions(+), 22 deletions(-)
> 
> diff --git a/Documentation/thermal/sysfs-api.txt b/Documentation/thermal/sysfs-api.txt
> index 8c745c8..5bc73ef 100644
> --- a/Documentation/thermal/sysfs-api.txt
> +++ b/Documentation/thermal/sysfs-api.txt
> @@ -69,8 +69,8 @@ temperature) and throttle appropriate devices.
>  1.1.2 void thermal_zone_device_unregister(struct thermal_zone_device *tz)
>  
>      This interface function removes the thermal zone device.
> -    It deletes the corresponding entry form /sys/class/thermal folder and
> -    unbind all the thermal cooling devices it uses.
> +    It deletes the corresponding entry from /sys/class/thermal folder and
> +    unbinds all the thermal cooling devices it uses.
>  
>  1.2 thermal cooling device interface
>  1.2.1 struct thermal_cooling_device *thermal_cooling_device_register(char *name,
> @@ -78,32 +78,32 @@ temperature) and throttle appropriate devices.
>  
>      This interface function adds a new thermal cooling device (fan/processor/...)
>      to /sys/class/thermal/ folder as cooling_device[0-*]. It tries to bind itself
> -    to all the thermal zone devices register at the same time.
> +    to all the thermal zone devices registered at the same time.
>      name: the cooling device name.
>      devdata: device private data.
>      ops: thermal cooling devices call-backs.
>  	.get_max_state: get the Maximum throttle state of the cooling device.
> -	.get_cur_state: get the Current throttle state of the cooling device.
> +	.get_cur_state: get the Currently requested throttle state of the cooling device.

Not sure about this one.  It's good that it uses the same language as
set_cur_state.  What's the problem with "Current throttle state"?

>  	.set_cur_state: set the Current throttle state of the cooling device.
>  
>  1.2.2 void thermal_cooling_device_unregister(struct thermal_cooling_device *cdev)
>  
> -    This interface function remove the thermal cooling device.
> -    It deletes the corresponding entry form /sys/class/thermal folder and
> -    unbind itself from all the thermal zone devices using it.
> +    This interface function removes the thermal cooling device.
> +    It deletes the corresponding entry from /sys/class/thermal folder and
> +    unbinds itself from all the thermal zone devices using it.
>  
>  1.3 interface for binding a thermal zone device with a thermal cooling device
>  1.3.1 int thermal_zone_bind_cooling_device(struct thermal_zone_device *tz,
>  	int trip, struct thermal_cooling_device *cdev,
>  	unsigned long upper, unsigned long lower, unsigned int weight);
>  
> -    This interface function bind a thermal cooling device to the certain trip
> +    This interface function binds a thermal cooling device to a particular trip
>      point of a thermal zone device.
>      This function is usually called in the thermal zone device .bind callback.
>      tz: the thermal zone device
>      cdev: thermal cooling device
> -    trip: indicates which trip point the cooling devices is associated with
> -	  in this thermal zone.
> +    trip: indicates which trip point in this thermal zone the cooling device
> +          is associated with.
>      upper:the Maximum cooling state for this trip point.
>            THERMAL_NO_LIMIT means no upper limit,
>  	  and the cooling device can be in max_state.
> @@ -116,13 +116,13 @@ temperature) and throttle appropriate devices.
>  1.3.2 int thermal_zone_unbind_cooling_device(struct thermal_zone_device *tz,
>  		int trip, struct thermal_cooling_device *cdev);
>  
> -    This interface function unbind a thermal cooling device from the certain
> +    This interface function unbinds a thermal cooling device from a particular
>      trip point of a thermal zone device. This function is usually called in
>      the thermal zone device .unbind callback.
>      tz: the thermal zone device
>      cdev: thermal cooling device
> -    trip: indicates which trip point the cooling devices is associated with
> -	  in this thermal zone.
> +    trip: indicates which trip point in this thermal zone the cooling device
> +          is associated with.
>  
>  1.4 Thermal Zone Parameters
>  1.4.1 struct thermal_bind_params
> @@ -142,13 +142,13 @@ temperature) and throttle appropriate devices.
>                 this thermal zone and cdev, for a particular trip point.
>                 If nth bit is set, then the cdev and thermal zone are bound
>                 for trip point n.
> -    .limits: This is an array of cooling state limits. Must have exactly
> -         2 * thermal_zone.number_of_trip_points. It is an array consisting
> -         of tuples <lower-state upper-state> of state limits. Each trip
> -         will be associated with one state limit tuple when binding.
> -         A NULL pointer means <THERMAL_NO_LIMITS THERMAL_NO_LIMITS>
> -         on all trips. These limits are used when binding a cdev to a
> -         trip point.
> +    .binding_limits: This is an array of cooling state limits. Must have
> +                     exactly 2 * thermal_zone.number_of_trip_points. It is an
> +                     array consisting of tuples <lower-state upper-state> of
> +                     state limits. Each trip will be associated with one state
> +                     limit tuple when binding. A NULL pointer means
> +                     <THERMAL_NO_LIMITS THERMAL_NO_LIMITS> on all trips.
> +                     These limits are used when binding a cdev to a trip point.
>      .match: This call back returns success(0) if the 'tz and cdev' need to
>  	    be bound, as per platform data.
>  1.4.2 struct thermal_zone_params
> @@ -283,8 +283,8 @@ cdev[0-*]
>  	RO, Optional
>  
>  cdev[0-*]_trip_point
> -	The trip point with which cdev[0-*] is associated in this thermal
> -	zone; -1 means the cooling device is not associated with any trip
> +	The trip point in this thermal zone which cdev[0-*] is associated
> +	with; -1 means the cooling device is not associated with any trip
>  	point.
>  	RO, Optional
>