linux-kernel - Re: [PATCH v5 2/5] thermal: qcom-spmi-temp-alarm: Add temp alarm data struct based on HW subtype

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: <e171a7fb-ff96-42a4-9a29-37640e99177b@oss.qualcomm.com>
Date: Sat, 21 Jun 2025 11:35:38 +0200
From: Konrad Dybcio <konrad.dybcio@....qualcomm.com>
To: Anjelique Melendez <anjelique.melendez@....qualcomm.com>, amitk@...nel.org,
        thara.gopinath@...il.com, rafael@...nel.org, daniel.lezcano@...aro.org
Cc: rui.zhang@...el.com, lukasz.luba@....com, david.collins@....qualcomm.com,
        stefan.schmidt@...aro.org, quic_tsoni@...cinc.com,
        linux-arm-msm@...r.kernel.org, linux-pm@...r.kernel.org,
        linux-kernel@...r.kernel.org, dmitry.baryshkov@...aro.org,
        dmitry.baryshkov@....qualcomm.com
Subject: Re: [PATCH v5 2/5] thermal: qcom-spmi-temp-alarm: Add temp alarm data
 struct based on HW subtype

On 6/20/25 2:19 AM, Anjelique Melendez wrote:
> Currently multiple if/else statements are used in functions to decipher
> between SPMI temp alarm Gen 1, Gen 2 and Gen 2 Rev 1 functionality. Instead
> refactor the driver so that SPMI temp alarm chips will have reference to a
> spmi_temp_alarm_data struct which defines data and function callbacks
> based on the HW subtype.
> 
> Signed-off-by: Anjelique Melendez <anjelique.melendez@....qualcomm.com>
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
> ---

[...]

> +static int qpnp_tm_gen1_get_temp_stage(struct qpnp_tm_chip *chip)
>  {
>  	int ret;
>  	u8 reg = 0;

this initialization is not necessary, as you override the
value right below (there's more cases of this)

[...]

> @@ -221,10 +235,10 @@ static int qpnp_tm_get_temp(struct thermal_zone_device *tz, int *temp)
>  static int qpnp_tm_update_critical_trip_temp(struct qpnp_tm_chip *chip,
>  					     int temp)
>  {
> -	long stage2_threshold_min = (*chip->temp_map)[THRESH_MIN][1];
> -	long stage2_threshold_max = (*chip->temp_map)[THRESH_MAX][1];
> +	long stage2_threshold_min = (*chip->data->temp_map)[THRESH_MIN][1];
> +	long stage2_threshold_max = (*chip->data->temp_map)[THRESH_MAX][1];

maybe we could go with an `enum overtemp_stage` to get rid of
such magic indexations - not necessarily in this patch, but in
general

lgtm otherwise

Konrad