linux-kernel - Re: thermal/drivers/tegra: Getting rid of the get_thermal

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Y9J4WAFyXyV/nqlG@orome>
Date:   Thu, 26 Jan 2023 13:55:52 +0100
From:   Thierry Reding <thierry.reding@...il.com>
To:     Daniel Lezcano <daniel.lezcano@...aro.org>
Cc:     Wei Ni <wni@...dia.com>, "Rafael J. Wysocki" <rafael@...nel.org>,
        Jon Hunter <jonathanh@...dia.com>,
        Johan Hovold <johan@...nel.org>,
        Philipp Zabel <p.zabel@...gutronix.de>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        "linux-tegra@...r.kernel.org" <linux-tegra@...r.kernel.org>,
        Linux PM mailing list <linux-pm@...r.kernel.org>
Subject: Re: thermal/drivers/tegra: Getting rid of the get_thermal_instance()
 usage

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

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)