Re: [PATCH v3 11/12] thermal/of: Use thermal trips stored in the thermal zone

[Zhang Rui]

On Sun, 2022-07-03 at 20:30 +0200, Daniel Lezcano wrote:
> Now that we have the thermal trip stored in the thermal zone in a
> generic way, we can rely on them and remove one indirection we found
> in the thermal_of code and do one more step forward the removal of
> the
> duplicated structures.
> 
> Cc: Alexandre Bailon <abailon@xxxxxxxxxxxx>
> Cc: Kevin Hilman <khilman@xxxxxxxxxxxx>
> Cc; Eduardo Valentin <eduval@xxxxxxxxxx>
> Signed-off-by: Daniel Lezcano <daniel.lezcano@xxxxxxxxxx>
> ---
>  drivers/thermal/thermal_of.c | 53 +++++++++++-----------------------
> --
>  1 file changed, 16 insertions(+), 37 deletions(-)
> 
> diff --git a/drivers/thermal/thermal_of.c
> b/drivers/thermal/thermal_of.c
> index 16b6b90a2390..bc885729bf23 100644
> --- a/drivers/thermal/thermal_of.c
> +++ b/drivers/thermal/thermal_of.c
> @@ -118,12 +118,7 @@ static int of_thermal_set_trips(struct
> thermal_zone_device *tz,
>   */
>  int of_thermal_get_ntrips(struct thermal_zone_device *tz)
>  {
> -	struct __thermal_zone *data = tz->devdata;
> -
> -	if (!data || IS_ERR(data))
> -		return -ENODEV;
> -
> -	return data->ntrips;
> +	return tz->ntrips;
>  }
>  EXPORT_SYMBOL_GPL(of_thermal_get_ntrips);
>  
> @@ -139,9 +134,7 @@ EXPORT_SYMBOL_GPL(of_thermal_get_ntrips);
>   */
>  bool of_thermal_is_trip_valid(struct thermal_zone_device *tz, int
> trip)
>  {
> -	struct __thermal_zone *data = tz->devdata;
> -
> -	if (!data || trip >= data->ntrips || trip < 0)
> +	if (trip >= tz->ntrips || trip < 0)
>  		return false;
>  
>  	return true;
> @@ -161,12 +154,7 @@ EXPORT_SYMBOL_GPL(of_thermal_is_trip_valid);
>  const struct thermal_trip *
>  of_thermal_get_trip_points(struct thermal_zone_device *tz)
>  {
> -	struct __thermal_zone *data = tz->devdata;
> -
> -	if (!data)
> -		return NULL;
> -
> -	return data->trips;
> +	return tz->trips;
>  }
>  EXPORT_SYMBOL_GPL(of_thermal_get_trip_points);

what is the difference between
of_thermal_get_ntrips/of_thermal_get_trip_points and
thermal_zone_get_ntrips/thermal_zone_get_trips as introduced in this
patch series?

we need to remove the duplications.

thanks,
rui
>  
> @@ -281,12 +269,10 @@ static int of_thermal_unbind(struct
> thermal_zone_device *thermal,
>  static int of_thermal_get_trip_type(struct thermal_zone_device *tz,
> int trip,
>  				    enum thermal_trip_type *type)
>  {
> -	struct __thermal_zone *data = tz->devdata;
> -
> -	if (trip >= data->ntrips || trip < 0)
> +	if (trip >= tz->ntrips || trip < 0)
>  		return -EDOM;
>  
> -	*type = data->trips[trip].type;
> +	*type = tz->trips[trip].type;
>  
>  	return 0;
>  }
> @@ -294,12 +280,10 @@ static int of_thermal_get_trip_type(struct
> thermal_zone_device *tz, int trip,
>  static int of_thermal_get_trip_temp(struct thermal_zone_device *tz,
> int trip,
>  				    int *temp)
>  {
> -	struct __thermal_zone *data = tz->devdata;
> -
> -	if (trip >= data->ntrips || trip < 0)
> +	if (trip >= tz->ntrips || trip < 0)
>  		return -EDOM;
>  
> -	*temp = data->trips[trip].temperature;
> +	*temp = tz->trips[trip].temperature;
>  
>  	return 0;
>  }
> @@ -309,7 +293,7 @@ static int of_thermal_set_trip_temp(struct
> thermal_zone_device *tz, int trip,
>  {
>  	struct __thermal_zone *data = tz->devdata;
>  
> -	if (trip >= data->ntrips || trip < 0)
> +	if (trip >= tz->ntrips || trip < 0)
>  		return -EDOM;
>  
>  	if (data->ops && data->ops->set_trip_temp) {
> @@ -321,7 +305,7 @@ static int of_thermal_set_trip_temp(struct
> thermal_zone_device *tz, int trip,
>  	}
>  
>  	/* thermal framework should take care of data->mask & (1 <<
> trip) */
> -	data->trips[trip].temperature = temp;
> +	tz->trips[trip].temperature = temp;
>  
>  	return 0;
>  }
> @@ -329,12 +313,10 @@ static int of_thermal_set_trip_temp(struct
> thermal_zone_device *tz, int trip,
>  static int of_thermal_get_trip_hyst(struct thermal_zone_device *tz,
> int trip,
>  				    int *hyst)
>  {
> -	struct __thermal_zone *data = tz->devdata;
> -
> -	if (trip >= data->ntrips || trip < 0)
> +	if (trip >= tz->ntrips || trip < 0)
>  		return -EDOM;
>  
> -	*hyst = data->trips[trip].hysteresis;
> +	*hyst = tz->trips[trip].hysteresis;
>  
>  	return 0;
>  }
> @@ -342,13 +324,11 @@ static int of_thermal_get_trip_hyst(struct
> thermal_zone_device *tz, int trip,
>  static int of_thermal_set_trip_hyst(struct thermal_zone_device *tz,
> int trip,
>  				    int hyst)
>  {
> -	struct __thermal_zone *data = tz->devdata;
> -
> -	if (trip >= data->ntrips || trip < 0)
> +	if (trip >= tz->ntrips || trip < 0)
>  		return -EDOM;
>  
>  	/* thermal framework should take care of data->mask & (1 <<
> trip) */
> -	data->trips[trip].hysteresis = hyst;
> +	tz->trips[trip].hysteresis = hyst;
>  
>  	return 0;
>  }
> @@ -356,12 +336,11 @@ static int of_thermal_set_trip_hyst(struct
> thermal_zone_device *tz, int trip,
>  static int of_thermal_get_crit_temp(struct thermal_zone_device *tz,
>  				    int *temp)
>  {
> -	struct __thermal_zone *data = tz->devdata;
>  	int i;
>  
> -	for (i = 0; i < data->ntrips; i++)
> -		if (data->trips[i].type == THERMAL_TRIP_CRITICAL) {
> -			*temp = data->trips[i].temperature;
> +	for (i = 0; i < tz->ntrips; i++)
> +		if (tz->trips[i].type == THERMAL_TRIP_CRITICAL) {
> +			*temp = tz->trips[i].temperature;
>  			return 0;
>  		}
>