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

Daniel Lezcano
2023-02-19T17:07:51Z

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@linaro.org>
>> ---
>
> ...
>
>> 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>
>
> ...
>
>
>> 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 |