lkml.org 
[lkml]   [2022]   [Sep]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH v5 02/30] thermal/sysfs: Do not make get_trip_hyst optional
    On Mon, Sep 26, 2022 at 4:06 PM Daniel Lezcano
    <daniel.lezcano@linaro.org> wrote:
    >
    > The get_trip_hyst() ops is optional. It results all around the thermal
    > framework code a check against the ops pointer and different action
    > adding more complexity and making the code less readable and
    > understandable.
    >
    > A zero hysteresis value is perfectly valid, so instead of adding more
    > circumvolutions in the code, create unconditionnaly the hysteresis and
    > use the thermal_zone_get_trip() function which returns a zero
    > hysteresis if the get_trip_hyst() is not defined.
    >
    > Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>

    I would change the subject to something like "thermal/sysfs: Always
    expose hysteresis attributes", because that's what the patch does
    IIUC.

    And in the changelog I would write something like the below:

    "Instead of avoiding to expose the hysteresis attributes of a thermal
    zone when its get_trip_hyst() operation is not defined, which is
    confusing, expose them always and use the default
    thermal_zone_get_trip() function returning 0 hysteresis when that
    operation is not present.

    The hysteresis of 0 is perfectly valid, so this change should not
    introduce any backwards compatibility issues."

    > ---
    > drivers/thermal/thermal_sysfs.c | 25 +++++++++----------------
    > 1 file changed, 9 insertions(+), 16 deletions(-)
    >
    > diff --git a/drivers/thermal/thermal_sysfs.c b/drivers/thermal/thermal_sysfs.c
    > index 75f6b151a58b..18cdd7cd0008 100644
    > --- a/drivers/thermal/thermal_sysfs.c
    > +++ b/drivers/thermal/thermal_sysfs.c
    > @@ -424,23 +424,20 @@ static int create_trip_attrs(struct thermal_zone_device *tz, int mask)
    > return -ENOMEM;
    > }
    >
    > - if (tz->ops->get_trip_hyst) {
    > - tz->trip_hyst_attrs = kcalloc(tz->num_trips,
    > - sizeof(*tz->trip_hyst_attrs),
    > - GFP_KERNEL);
    > - if (!tz->trip_hyst_attrs) {
    > - kfree(tz->trip_type_attrs);
    > - kfree(tz->trip_temp_attrs);
    > - return -ENOMEM;
    > - }
    > + tz->trip_hyst_attrs = kcalloc(tz->num_trips,
    > + sizeof(*tz->trip_hyst_attrs),
    > + GFP_KERNEL);
    > + if (!tz->trip_hyst_attrs) {
    > + kfree(tz->trip_type_attrs);
    > + kfree(tz->trip_temp_attrs);
    > + return -ENOMEM;
    > }
    >
    > attrs = kcalloc(tz->num_trips * 3 + 1, sizeof(*attrs), GFP_KERNEL);
    > if (!attrs) {
    > kfree(tz->trip_type_attrs);
    > kfree(tz->trip_temp_attrs);
    > - if (tz->ops->get_trip_hyst)
    > - kfree(tz->trip_hyst_attrs);
    > + kfree(tz->trip_hyst_attrs);
    > return -ENOMEM;
    > }
    >
    > @@ -473,9 +470,6 @@ static int create_trip_attrs(struct thermal_zone_device *tz, int mask)
    > }
    > attrs[indx + tz->num_trips] = &tz->trip_temp_attrs[indx].attr.attr;
    >
    > - /* create Optional trip hyst attribute */
    > - if (!tz->ops->get_trip_hyst)
    > - continue;
    > snprintf(tz->trip_hyst_attrs[indx].name, THERMAL_NAME_LENGTH,
    > "trip_point_%d_hyst", indx);
    >
    > @@ -512,8 +506,7 @@ static void destroy_trip_attrs(struct thermal_zone_device *tz)
    >
    > kfree(tz->trip_type_attrs);
    > kfree(tz->trip_temp_attrs);
    > - if (tz->ops->get_trip_hyst)
    > - kfree(tz->trip_hyst_attrs);
    > + kfree(tz->trip_hyst_attrs);
    > kfree(tz->trips_attribute_group.attrs);
    > }
    >
    > --
    > 2.34.1
    >

    \
     
     \ /
      Last update: 2022-09-26 21:16    [W:4.105 / U:0.024 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site