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

On 2023-02-19 18:07:36 +0100, Daniel Lezcano wrote:

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

quoted

Hi Daniel,

Thanks for your work.

On 2023-02-19 15:36:41 +0100, Daniel Lezcano wrote:

quoted

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

...

quoted

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

For R-Car,

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

...

quoted

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() ?

Looks good to me.

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

-- 
Kind Regards,
Niklas Söderlund