On 2023-02-19 18:07:36 +0100, Daniel Lezcano wrote:
> 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() ?
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
