Received-SPF: pass (google.com: domain of linux-kernel+bounces-22184-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Precedence: bulk
MIME-Version: 1.0
References: <20240106191502.29126-1-quic_manafm@quicinc.com>
 <CAJZ5v0gE6eEpALrfxHvCd5TRqjB+v8pffG4CKLTVXiSvuiWhHg@mail.gmail.com> <d7b82fc8-0ed8-80b8-9eb8-c77f9277178f@quicinc.com>
In-Reply-To: <d7b82fc8-0ed8-80b8-9eb8-c77f9277178f@quicinc.com>
From: "Rafael J. Wysocki" <rafael@kernel.org>
Date: Wed, 10 Jan 2024 13:48:47 +0100
Message-ID: <CAJZ5v0g4hnRqRCseRnTjfEF+-2=ZT8U9=2m9FODqh3G8eDd=Sw@mail.gmail.com>
Subject: Re: [PATCH] thermal/sysfs: Always enable hysteresis write support
To: Manaf Meethalavalappu Pallikunhi <quic_manafm@quicinc.com>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>, Daniel Lezcano <daniel.lezcano@linaro.org>, 
	Zhang Rui <rui.zhang@intel.com>, Lukasz Luba <lukasz.luba@arm.com>, linux-pm@vger.kernel.org, 
	linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Hi Manaf,

On Wed, Jan 10, 2024 at 9:17=E2=80=AFAM Manaf Meethalavalappu Pallikunhi
<quic_manafm@quicinc.com> wrote:
>
> Hi Rafael,
>
> On 1/9/2024 7:12 PM, Rafael J. Wysocki wrote:
>
> On Sat, Jan 6, 2024 at 8:16=E2=80=AFPM Manaf Meethalavalappu Pallikunhi
> <quic_manafm@quicinc.com> wrote:
>
> The commit 2e38a2a981b2("thermal/core: Add a generic
> thermal_zone_set_trip() function") adds the support to update
> trip hysteresis even if set_trip_hyst() operation is not defined.
> But during hysteresis attribute creation, if this operation is
> defined then only it enables hysteresis write access. It leads
> to a case where hysteresis sysfs will be read only for a thermal
> zone when its set_trip_hyst() operation is not defined.
>
> Which is by design.
>
> I think it is regression after recent re-work. If a sensor is registered =
with thermal framework via thermal_of,
>
> sensor driver doesn't need to know the trip configuration and nothing to =
do with set_trip_hyst() in driver.
>
> Without this change, if a sensor needs to be monitored from userspace(tri=
p/hysteresis),

What exactly do you mean by "monitored" here?

> it is enforcing sensor driver to add  dummy set_trip_hyst() operation. Co=
rrect me otherwise

With the current design, whether or not trip properties can be updated
by user space is a thermal zone property expressed by the presence of
the set_trip_* operations, so yes, whoever registers the thermal zone
needs to provide those so that user space can update the trip
properties.

> For some thermal zone types (eg. acpi), updating trip hysteresis via
> sysfs might lead to incorrect behavior.
>
> To address this issue, is it okay to  guard  hysteresis write permission =
under CONFIG_THERMAL_WRITABLE_TRIPS defconfig ?

Not really, because it would affect all of the thermal zones then.

TBH, the exact scenario in which user space needs to update trip
hysteresis is not particularly clear to me, so can you provide some
more details, please?

Thanks!