Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:2 as permitted sender) client-ip=2620:137:e000::3:2;
MIME-Version: 1.0
References: <20231012102700.2858952-1-daniel.lezcano@linaro.org>
 <a6b51de7-4f56-4db9-a7dd-60555ac6c37f@arm.com> <d9f3bd7b-a5db-4d37-bb1f-f97e40c8a63a@linaro.org>
In-Reply-To: <d9f3bd7b-a5db-4d37-bb1f-f97e40c8a63a@linaro.org>
From:   "Rafael J. Wysocki" <rafael@kernel.org>
Date:   Thu, 12 Oct 2023 19:44:01 +0200
Message-ID: <CAJZ5v0gC4+Jam0a4KpEr7onydn8Sp8MkN2yzVxm0W9qDpmEoDw@mail.gmail.com>
Subject: Re: [PATCH 1/2] thermal/core: Hardening the self-encapsulation
To:     Daniel Lezcano <daniel.lezcano@linaro.org>
Cc:     Lukasz Luba <lukasz.luba@arm.com>,
        Thierry Reding <thierry.reding@gmail.com>, rafael@kernel.org,
        Amit Kucheria <amitk@kernel.org>,
        Zhang Rui <rui.zhang@intel.com>,
        "open list:THERMAL" <linux-pm@vger.kernel.org>,
        open list <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 8BIT
Precedence: bulk

On Thu, Oct 12, 2023 at 3:14 PM Daniel Lezcano
<daniel.lezcano@linaro.org> wrote:
>
>
> Hi Lukasz,
>
> On 12/10/2023 14:01, Lukasz Luba wrote:
> > Hi Daniel,
> >
> > On 10/12/23 11:26, Daniel Lezcano wrote:
> >> The thermal private header has leaked all around the drivers which
> >> interacted with the core internals. The thermal zone structure which
> >> was part of the exported header led also to a leakage of the fields
> >> into the different drivers, making very difficult to improve the core
> >> code without having to change the drivers.
> >>
> >> Now we mostly fixed how the thermal drivers were interacting with the
> >> thermal zones (actually fixed how they should not interact). The
> >> thermal zone structure will be moved to the private thermal core
> >> header. This header has been removed from the different drivers and
> >> must belong to the core code only. In order to prevent this private
> >> header to be included again in the drivers, make explicit only the
> >> core code can include this header by defining a THERMAL_CORE_SUBSYS
> >> macro. The private header will contain a check against this macro.
> >>
> >> The Tegra SoCtherm driver needs to access thermal_core.h to have the
> >> get_thermal_instance() function definition. It is the only one
> >> remaining driver which need to access the thermal_core.h header, so
> >> the check will emit a warning at compilation time.
> >>
> >> Thierry Reding is reworking the driver to get rid of this function [1]
> >> and thus when the changes will be merged, the compilation warning will
> >> be converted to a compilation error, closing definitively the door to
> >> the drivers willing to play with the thermal zone device internals.
> >
> > That looks like a good idea. Although, shouldn't we avoid the
> > compilation warnings and just first merge the fixes for drivers?
>
> Yes, we should but there is the series for nvidia (pointed in the
> changelog) which need a slight refresh for the bindings AFAIR. That
> series is since March 2023 and Thierry seems busy [1]. I'm holding the
> hardening since then.
>
> So I don't know how to make progress on this? I was assuming we can
> merge this series and let the compiler recall what has to be fixed.
>
> [1] https://lore.kernel.org/all/ZK14edZUih1kH_sZ@orome/
>
> and as soon as it is fixed, we convert the WARNING to ERROR :P

To be honest, I'm not sure if anything needs to be done along the
lines of this patch right now or even at all.

The only concern here would be that some new drivers would include
thermal_core.h while we were waiting for the remaining existing
abusers to be fixed, but since this hasn't happened for the last 6
months, I'm not worried.

It would be good to add a notice to thermal_core.h that this file is
for internal use in the thermal core only, though.