Re: [PATCH 1/3] thermal: of: support writable trips via dt

[Eduardo Valentin]

Hello Navneet

On Wed, Nov 26, 2014 at 05:16:27PM -0800, Navneet Kumar wrote:
> From: navneet kumar <navneetk@xxxxxxxxxx>
> 
> Support writable trip points configuration from the
> device tree. 'OF' reads this configuration and adjusts
> the 'trips' mask accordingly to allow the 'set_trip_xxx'
> calls to be effective.
> 
> Signed-off-by: Diwakar Tundlam <dtundlam@xxxxxxxxxx>

Thanks for sharing your patches!

> ---
>  drivers/thermal/of-thermal.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c
> index 62143ba31001..cf9ee3e82fee 100644
> --- a/drivers/thermal/of-thermal.c
> +++ b/drivers/thermal/of-thermal.c
> @@ -604,7 +604,8 @@ static int thermal_of_get_trip_type(struct device_node *np,
>   * Return: 0 on success, proper error code otherwise
>   */
>  static int thermal_of_populate_trip(struct device_node *np,
> -				    struct __thermal_trip *trip)
> +				    struct __thermal_trip *trip,
> +				    bool *trip_writable)
>  {
>  	int prop;
>  	int ret;
> @@ -629,6 +630,8 @@ static int thermal_of_populate_trip(struct device_node *np,
>  		return ret;
>  	}
>  
> +	*trip_writable = of_property_read_bool(np, "writable");

New DT properties needs to be properly discussed in device tree mainling
list. From what I see here, this property does not describe hardware,
does it? 

A simple git grep writable Documentation/devicetree/bindings/
returns nothing. So, I am a bit skeptic having this property is
allowable.

In any case, can you please send your proposal also copying device tree mailing list?
devicetree@xxxxxxxxxxxxxxx

Besides, you need to document this new property:
Documentation/devicetree/bindings/thermal/thermal.txt 


All the best,

Eduardo Valentin

> +
>  	/* Required for cooling map matching */
>  	trip->np = np;
>  	of_node_get(np);
> @@ -657,6 +660,8 @@ thermal_of_build_thermal_zone(struct device_node *np)
>  	struct __thermal_zone *tz;
>  	int ret, i;
>  	u32 prop;
> +	bool trip_writable;
> +	u64 m = 0;
>  
>  	if (!np) {
>  		pr_err("no thermal zone np\n");
> @@ -700,9 +705,14 @@ thermal_of_build_thermal_zone(struct device_node *np)
>  
>  	i = 0;
>  	for_each_child_of_node(child, gchild) {
> -		ret = thermal_of_populate_trip(gchild, &tz->trips[i++]);
> +		trip_writable = false;
> +		ret = thermal_of_populate_trip(gchild, &tz->trips[i],
> +					       &trip_writable);
>  		if (ret)
>  			goto free_trips;
> +		if (trip_writable)
> +			m |= 1ULL << i;
> +		i++;
>  	}
>  
>  	of_node_put(child);
> -- 
> 1.8.1.5
>

Attachment: signature.asc
Description: Digital signature