Re: [PATCH v1 01/17] thermal/core: Add a thermal zone 'devdata' accessor

From: Daniel Lezcano
Date: Sun Feb 19 2023 - 12:07:48 EST


On 19/02/2023 16:07, Niklas Söderlund wrote:
Hi Daniel,

Thanks for your work.

On 2023-02-19 15:36:41 +0100, Daniel Lezcano wrote:
The thermal zone device structure is exposed to the different drivers
and obviously they access the internals while that should be
restricted to the core thermal code.

In order to self-encapsulate the thermal core code, we need to prevent
the drivers accessing directly the thermal zone structure and provide
accessor functions to deal with.

Provide an accessor to the 'devdata' structure and make use of it in
the different drivers.

No functional changes intended.

Signed-off-by: Daniel Lezcano <daniel.lezcano@xxxxxxxxxx>
---

...

drivers/thermal/rcar_gen3_thermal.c | 4 ++--
drivers/thermal/rcar_thermal.c | 3 +--

For R-Car,

Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@xxxxxxxxxxxx>

...


diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index 2bb4bf33f4f3..724b95662da9 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -365,6 +365,8 @@ thermal_zone_device_register_with_trips(const char *, struct thermal_trip *, int
void *, struct thermal_zone_device_ops *,
struct thermal_zone_params *, int, int);
+void *thermal_zone_device_get_data(struct thermal_zone_device *tzd);
+

bikeshedding:

Would it make sens to name this thermal_zone_device_get_priv_data(),
thermal_zone_device_get_priv() or something like that? To make it more
explicitly when reading the driver code this fetches the drivers private
data, and not some data belonging to the zone itself.

In the headers files, there are more occurrences with _name_priv():

# _name_priv()
git grep priv include/linux/ | grep "priv(" | grep -v get | wc -l
52

# _name_private()
git grep priv include/linux/ | grep "private(" | grep -v get | wc -l
33

# _name_get_private()
git grep priv include/linux/ | grep "private(" | grep get | wc -l
12

# _name_get_priv()
git grep priv include/linux/ | grep "priv(" | grep get | wc -l
4


What about thermal_zone_device_priv() ?






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