Re: [PATCH v2 06/16] thermal: Don't use 'device' internal thermal zone structure field

From: Daniel Lezcano
Date: Thu Feb 23 2023 - 09:36:12 EST


On 23/02/2023 12:43, Rafael J. Wysocki wrote:
On Thu, Feb 23, 2023 at 10:56 AM Daniel Lezcano
<daniel.lezcano@xxxxxxxxxx> wrote:

On 22/02/2023 21:06, Rafael J. Wysocki wrote:
On Wed, Feb 22, 2023 at 9:00 PM Daniel Lezcano
<daniel.lezcano@xxxxxxxxxx> wrote:

On 22/02/2023 20:43, Rafael J. Wysocki wrote:
On Tue, Feb 21, 2023 at 7:07 PM Daniel Lezcano
<daniel.lezcano@xxxxxxxxxx> wrote:

Some drivers are directly using the thermal zone's 'device' structure
field.

Use the driver device pointer instead of the thermal zone device when
it is available.

Remove the traces when they are duplicate with the traces in the core
code.

Signed-off-by: Daniel Lezcano <daniel.lezcano@xxxxxxxxxx>
Reviewed-by: Balsam CHIHI <bchihi@xxxxxxxxxxxx> #Mediatek LVTS
---

[ ... ]

thermal_zone_device_update(data->ti_thermal, THERMAL_EVENT_UNSPECIFIED);

- dev_dbg(&data->ti_thermal->device, "updated thermal zone %s\n",
+ dev_dbg(data->bgp->dev, "updated thermal zone %s\n",
data->ti_thermal->type);

The code before the change is more consistent, because it refers to
the same object in both instances.

It looks like a type field accessor is needed, eg. thermal_zone_device_type()?

Or move the debug message to thermal_zone_device_update()?

Actually it is done on purpose because the patch 9 replaces the accesses
to 'type' by 'id', the thermal_zone_device_type() accessor won't be needed.

Cool.

However, this is a change in behavior (albeit small) which doesn't
appear to be necessary.

What would be wrong with having a tz->type accessor too?

I can add the 'type' accessor but from my point of view it is not
correct because the information belongs to the thermal framework and it
is used to export the information in the sysfs which is along with the
directory name giving the id of the thermal zone.

I'm not sure what you mean here.

Surely, the 'type' is provided by whoever registers the thermal zone,
so I'm not sure in what way it "belongs" to the framework.

I meant the goal of 'type' is to be exported to sysfs, nothing else.

That is the reason why I used the word 'belongs', because it was introduced to stay in the scope of the thermal framework, but then its usage has been diverted to a name.

Anyway, from my POV having traces in the ops is not a good thing, so I'll propose later to remove them and add a single message in the call sites.

Meanwhile, I'll provide the accessor for 'type' and hopefully we do not end up with a plethora of accessors to be used in the core code.


--
<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