Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH v2] drivers: thermal: tsens: respect thermal_device_mode
 in threshold irq reporting
To:     Amit Kucheria <amitk@kernel.org>
Cc:     Andy Gross <agross@kernel.org>,
        Bjorn Andersson <bjorn.andersson@linaro.org>,
        "Rafael J. Wysocki" <rafael@kernel.org>,
        Daniel Lezcano <daniel.lezcano@linaro.org>,
        Zhang Rui <rui.zhang@intel.com>,
        Linux PM list <linux-pm@vger.kernel.org>,
        linux-arm-msm <linux-arm-msm@vger.kernel.org>,
        LKML <linux-kernel@vger.kernel.org>
References: <20220114031730.25621-1-benl@squareup.com>
 <CAHLCerNYXxrW=K6hQ38mXd+3V-u=5_NFXKBoaOx+yUaYW5Zu7A@mail.gmail.com>
From:   Benjamin Li <benl@squareup.com>
Message-ID: <30c38867-78b0-d3a1-ffcf-9612a7befc3a@squareup.com>
Date:   Thu, 20 Jan 2022 11:32:01 -0800
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:78.0)
 Gecko/20100101 Thunderbird/78.14.0
MIME-Version: 1.0
In-Reply-To: <CAHLCerNYXxrW=K6hQ38mXd+3V-u=5_NFXKBoaOx+yUaYW5Zu7A@mail.gmail.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 1/19/22 4:33 PM, Bjorn Andersson <bjorn.andersson@linaro.org> wrote:
> Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>

Thanks!

On 1/20/22 3:40 AM, Amit Kucheria wrote:
>> +                               dev_dbg(priv->dev, "[%u] %s: TZ update trigger (%d mC)\n",
>> +                                       hw_id, __func__, temp);
>> +                               thermal_zone_device_update(s->tzd, THERMAL_EVENT_UNSPECIFIED);
>> +                       } else {
>> +                               dev_dbg(priv->dev, "[%u] %s: TZ update trigger (%d mC) skipped as zone disabled\n",
> 
> Hmm. I don't like the fact that these messages won't be visible to
> users in dmesg unless they're debugging. This change puts the SoC in a
> potentially unsafe state. Perhaps we should print a ratelimited
> message in the logs that we're operating outside safety limits?

That seems fine, I'll change to dev_info_ratelimited and make the message
a bit scarier.

> 
>> +                                       hw_id, __func__, temp);
>> +                       }
>>                 } else {
>> -                       dev_dbg(priv->dev, "[%u] %s: no violation:  %d\n",
>> -                               hw_id, __func__, temp);
>> +                       dev_dbg(priv->dev, "[%u] %s: no violation:  %d\n", hw_id, __func__, temp);
> 
> Get rid of this hunk, it is unrelated to the above change.

Will do.


> 
>>                 }
>>
>>                 if (tsens_version(priv) < VER_0_1) {
>> --
>> 2.17.1
>>