Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20230410205305.1649678-1-daniel.lezcano@linaro.org>
 <20230410205305.1649678-3-daniel.lezcano@linaro.org> <CAJZ5v0jysxvCZ2-dXfqfiJfpZGOYwgMwk1kEuiGOQjV3LK4gMw@mail.gmail.com>
 <c2581726-3e02-104c-f2ac-55268470d2c8@linaro.org>
In-Reply-To: <c2581726-3e02-104c-f2ac-55268470d2c8@linaro.org>
From:   "Rafael J. Wysocki" <rafael@kernel.org>
Date:   Wed, 12 Apr 2023 20:56:36 +0200
Message-ID: <CAJZ5v0i65XivddWvP9OSuXePwbeWYsgtOr_GB=BBN_jwsWarjQ@mail.gmail.com>
Subject: Re: [PATCH v2 2/7] thermal/core: Encapsulate tz->device field
To:     Daniel Lezcano <daniel.lezcano@linaro.org>
Cc:     "Rafael J. Wysocki" <rafael@kernel.org>, rui.zhang@intel.com,
        linux-kernel@vger.kernel.org, linux-pm@vger.kernel.org,
        Amit Kucheria <amitk@kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 8BIT
Precedence: bulk

On Tue, Apr 11, 2023 at 10:20 PM Daniel Lezcano
<daniel.lezcano@linaro.org> wrote:
>
> On 11/04/2023 20:19, Rafael J. Wysocki wrote:
> > On Mon, Apr 10, 2023 at 10:53 PM Daniel Lezcano
> > <daniel.lezcano@linaro.org> wrote:
> >>
> >> There are still some drivers needing to play with the thermal zone
> >> device internals. That is not the best but until we can figure out if
> >> the information is really needed, let's encapsulate the field used in
> >> the thermal zone device structure, so we can move forward relocating
> >> the thermal zone device structure definition in the thermal framework
> >> private headers.
> >
> > I'm not really sure why this is needed, so please explain.
>
> Some drivers are accessing tz->device, that implies they have the
> knowledge of the thermal_zone_device structure but we want to
> self-encapsulate this structure and reduce the scope of the structure to
> the thermal core only.
>
> The ACPI and the Menlon drivers are the drivers accessing tz->device.
>
> By adding this wrapper, these drivers do no longer need the thermal zone
> device structure definition.

So you want to move the definition of struct thermal_zone_device from
include/linux/thermal.h into a local header in drivers/thermal/ and
make it entirely local to the thermal core IIUC.

Which would be forcing the callers of
thermal_zone_device_register_with_trips() (and friends) to use
pointers to a data type that's not completely defined (from their
perspective), but they would still have access to the trips array
passed to that function.

That doesn't sound particularly consistent and what's the purpose of doing it?

> >> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> >> ---
> >>   drivers/thermal/thermal_core.c | 6 ++++++
> >>   include/linux/thermal.h        | 1 +
> >>   2 files changed, 7 insertions(+)
> >>
> >> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> >> index c5025aca22ee..842f678c1c3e 100644
> >> --- a/drivers/thermal/thermal_core.c
> >> +++ b/drivers/thermal/thermal_core.c
> >> @@ -1398,6 +1398,12 @@ int thermal_zone_device_id(struct thermal_zone_device *tzd)
> >>   }
> >>   EXPORT_SYMBOL_GPL(thermal_zone_device_id);
> >>
> >> +struct device *thermal_zone_device(struct thermal_zone_device *tzd)
> >> +{
> >> +       return &tzd->device;
> >> +}
> >> +EXPORT_SYMBOL_GPL(thermal_zone_device);
> >> +
> >>   /**
> >>    * thermal_zone_device_unregister - removes the registered thermal zone device
> >>    * @tz: the thermal zone device to remove
> >> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> >> index 82ddb32f9876..87837094d549 100644
> >> --- a/include/linux/thermal.h
> >> +++ b/include/linux/thermal.h
> >> @@ -313,6 +313,7 @@ thermal_zone_device_register_with_trips(const char *, struct thermal_trip *, int
> >>   void *thermal_zone_device_priv(struct thermal_zone_device *tzd);
> >>   const char *thermal_zone_device_type(struct thermal_zone_device *tzd);
> >>   int thermal_zone_device_id(struct thermal_zone_device *tzd);
> >> +struct device *thermal_zone_device(struct thermal_zone_device *tzd);
> >>
> >>   int thermal_zone_bind_cooling_device(struct thermal_zone_device *, int,
> >>                                       struct thermal_cooling_device *,
> >> --
> >> 2.34.1
> >>
>
> --
> <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
>
> Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
> <http://twitter.com/#!/linaroorg> Twitter |
> <http://www.linaro.org/linaro-blog/> Blog
>