linux-kernel - Re: [PATCH v7 6/7] thermal/drivers/exynos: Handle temperature threshold IRQs with SoC-specific mapping

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: <20250819131704.19780-1-m.majewski2@samsung.com>
Date: Tue, 19 Aug 2025 15:17:04 +0200
From: Mateusz Majewski <m.majewski2@...sung.com>
To: linux.amoon@...il.com
Cc: alim.akhtar@...sung.com, bzolnier@...il.com, daniel.lezcano@...aro.org,
	justinstitt@...gle.com, krzk@...nel.org,
	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
	linux-pm@...r.kernel.org, linux-samsung-soc@...r.kernel.org,
	llvm@...ts.linux.dev, lukasz.luba@....com, m.majewski2@...sung.com,
	morbo@...gle.com, nathan@...nel.org, nick.desaulniers+lkml@...il.com,
	rafael@...nel.org, rui.zhang@...el.com
Subject: Re: [PATCH v7 6/7] thermal/drivers/exynos: Handle temperature
 threshold IRQs with SoC-specific mapping

Hello :)

> +/* Map Rise and Falling edges for IRQ Clean */
> +struct tmu_irq_map {
> +	u32 fall[3];
> +	u32 rise[3];
> +};

Hmm, we can probably get away with less interrupts. We actually only
enable one fall interrupt in tmu_set_low_temp and one rise interrupt in
tmu_set_high_temp.

Regarding tmu_set_crit_temp, on SoCs that have hardware thermal tripping
there is nothing to clear. On others we will reboot immediately anyway,
though maybe there is nothing wrong with clearing the interrupt
beforehand? Regardless of this, there is only a rise critical
temperature interrupt, we never set a matching fall interrupt.

Maybe it would also be good to add a bool to this struct containing
information about whether a fall interrupt is in use, and reuse
the same logic for 4210?

(Nitpick: I am not a native speaker of English, but I think "clean" and
"clear" have slightly different meanings, and the rest of the code
consistently uses "clear", so it would be worthwhile to also use "clear"
here.)

> +	/* Set SoC-specific interrupt bit mappings */
> +	switch (data->soc) {
> +	case SOC_ARCH_EXYNOS3250:
> +	case SOC_ARCH_EXYNOS4412:
> +	case SOC_ARCH_EXYNOS5250:
> +	case SOC_ARCH_EXYNOS5260:
> +		irq_map.fall[2] = BIT(20);
> +		irq_map.fall[1] = BIT(16);
> +		irq_map.fall[0] = BIT(12);
> +		irq_map.rise[2] = BIT(8);
> +		irq_map.rise[1] = BIT(4);
> +		irq_map.rise[0] = BIT(0);
> +		break;
> +	case SOC_ARCH_EXYNOS5420:
> +	case SOC_ARCH_EXYNOS5420_TRIMINFO:
> +		irq_map.fall[2] = BIT(24);
> +		irq_map.fall[1] = BIT(20);
> +		irq_map.fall[0] = BIT(16);
> +		irq_map.rise[2] = BIT(8);
> +		irq_map.rise[1] = BIT(4);
> +		irq_map.rise[0] = BIT(0);
> +		break;
> +	case SOC_ARCH_EXYNOS5433:
> +	case SOC_ARCH_EXYNOS7:
> +		irq_map.fall[2] = BIT(23);
> +		irq_map.fall[1] = BIT(17);
> +		irq_map.fall[0] = BIT(16);
> +		irq_map.rise[2] = BIT(7);
> +		irq_map.rise[1] = BIT(1);
> +		irq_map.rise[0] = BIT(0);
> +		break;
> +	default:
> +		pr_warn("exynos-tmu: Unknown SoC type %d, using fallback IRQ mapping\n", soc);
> +		break;

Maybe put irq_map inside exynos_tmu_data? exynos_map_dt_data has a
switch block that is quite similar, in that it also matches on the SoC
type. This way also there is no need to have a fallback.

Kind regards,
Mateusz Majewski