Re: thermal/drivers/tegra: Getting rid of the get_thermal_instance() usage

[Thierry Reding <thierry.reding@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 4341 bytes --]

On Tue, Jan 24, 2023 at 08:57:23PM +0100, Daniel Lezcano wrote:
> 
> Hi,
> 
> does anyone know what is the purpose of the get_thermal_instance() usage in
> this code:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/thermal/linux.git/tree/drivers/thermal/tegra/soctherm.c?h=thermal/linux-next#n623
> 
> The driver is using a function which is reserved for the thermal core. It
> should not.
> 
> Is the following change ok ?
> 
> diff --git a/drivers/thermal/tegra/soctherm.c
> b/drivers/thermal/tegra/soctherm.c
> index 220873298d77..5f552402d987 100644
> --- a/drivers/thermal/tegra/soctherm.c
> +++ b/drivers/thermal/tegra/soctherm.c
> @@ -620,9 +620,8 @@ static int tegra_thermctl_set_trip_temp(struct
> thermal_zone_device *tz, int trip
>  				continue;
> 
>  			cdev = ts->throt_cfgs[i].cdev;
> -			if (get_thermal_instance(tz, cdev, trip_id))
> -				stc = find_throttle_cfg_by_name(ts, cdev->type);
> -			else
> +			stc = find_throttle_cfg_by_name(ts, cdev->type);
> +			if (!stc)
>  				continue;
> 
>  			return throttrip_program(dev, sg, stc, temp);
> @@ -768,9 +767,9 @@ static int tegra_soctherm_set_hwtrips(struct device
> *dev,
>  			continue;
> 
>  		cdev = ts->throt_cfgs[i].cdev;
> -		if (get_thermal_instance(tz, cdev, trip))
> -			stc = find_throttle_cfg_by_name(ts, cdev->type);
> -		else
> +
> +		stc = find_throttle_cfg_by_name(ts, cdev->type);
> +		if (!stc)
>  			continue;
> 
>  		ret = throttrip_program(dev, sg, stc, temperature);

There's a small difference in behavior after applying this patch. Prior
to this I get (on Tegra210):

	[   12.354091] tegra_soctherm 700e2000.thermal-sensor: missing thermtrips, will use critical trips as shut down temp
	[   12.379009] tegra_soctherm 700e2000.thermal-sensor: thermtrip: will shut down when cpu reaches 102500 mC
	[   12.388882] tegra_soctherm 700e2000.thermal-sensor: programming throttle for cpu to 102500
	[   12.401007] tegra_soctherm 700e2000.thermal-sensor: throttrip: will throttle when cpu reaches 102500 mC
	[   12.471041] tegra_soctherm 700e2000.thermal-sensor: thermtrip: will shut down when gpu reaches 103000 mC
	[   12.482852] tegra_soctherm 700e2000.thermal-sensor: programming throttle for gpu to 103000
	[   12.482860] tegra_soctherm 700e2000.thermal-sensor: throttrip: will throttle when gpu reaches 103000 mC
	[   12.485357] tegra_soctherm 700e2000.thermal-sensor: thermtrip: will shut down when pll reaches 103000 mC
	[   12.501774] tegra_soctherm 700e2000.thermal-sensor: thermtrip: will shut down when mem reaches 103000 mC

and after these changes, it turns into:

	[   12.447113] tegra_soctherm 700e2000.thermal-sensor: missing thermtrips, will use critical trips as shut down temp
	[   12.472300] tegra_soctherm 700e2000.thermal-sensor: thermtrip: will shut down when cpu reaches 102500 mC
	[   12.481789] tegra_soctherm 700e2000.thermal-sensor: programming throttle for cpu to 102500
	[   12.495447] tegra_soctherm 700e2000.thermal-sensor: throttrip: will throttle when cpu reaches 102500 mC
	[   12.496514] tegra_soctherm 700e2000.thermal-sensor: thermtrip: will shut down when gpu reaches 103000 mC
	[   12.510353] tegra_soctherm 700e2000.thermal-sensor: programming throttle for gpu to 103000
	[   12.526856] tegra_soctherm 700e2000.thermal-sensor: throttrip: will throttle when gpu reaches 103000 mC
	[   12.528774] tegra_soctherm 700e2000.thermal-sensor: thermtrip: will shut down when pll reaches 103000 mC
	[   12.569352] tegra_soctherm 700e2000.thermal-sensor: programming throttle for pll to 103000
	[   12.577635] tegra_soctherm 700e2000.thermal-sensor: throttrip: will throttle when pll reaches 103000 mC
	[   12.590952] tegra_soctherm 700e2000.thermal-sensor: thermtrip: will shut down when mem reaches 103000 mC
	[   12.600783] tegra_soctherm 700e2000.thermal-sensor: programming throttle for mem to 103000
	[   12.609204] tegra_soctherm 700e2000.thermal-sensor: throttrip: will throttle when mem reaches 103000 mC

The "programming throttle ..." messages are something I've added locally
to trace what gets called. So it looks like for "pll" and "mem" thermal
zones, we now program trip points whereas we previously didn't.

I'll take a closer look to see if we can replace the calls to
get_thermal_instance() by something else.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]