lkml.org 
[lkml]   [2023]   [Jan]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: [PATCH 1/3] thermal/drivers/intel: Use generic trip points for quark_dts
    From
    On 31/01/2023 20:11, Rafael J. Wysocki wrote:
    > On Tue, Jan 31, 2023 at 5:41 PM Daniel Lezcano
    > <daniel.lezcano@linaro.org> wrote:
    >>
    >> On 26/01/2023 15:15, Rafael J. Wysocki wrote:
    >>> On Wed, Jan 18, 2023 at 7:16 PM Daniel Lezcano
    >>> <daniel.lezcano@linaro.org> wrote:
    >>>>
    >>>> The thermal framework gives the possibility to register the trip
    >>>> points with the thermal zone. When that is done, no get_trip_* ops are
    >>>> needed and they can be removed.
    >>>>
    >>>> Convert ops content logic into generic trip points and register them with the
    >>>> thermal zone.
    >>>>
    >>>> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
    >>>> ---
    >>
    >> [ ... ]
    >>
    >>>> - aux_entry->tzone = thermal_zone_device_register("quark_dts",
    >>>> - QRK_MAX_DTS_TRIPS,
    >>>> - wr_mask,
    >>>> - aux_entry, &tzone_ops, NULL, 0, polling_delay);
    >>>> + err = get_trip_temp(QRK_DTS_ID_TP_CRITICAL, &temperature);
    >>>> + if (err)
    >>>> + goto err_ret;
    >>>> +
    >>>> + aux_entry->trips[QRK_DTS_ID_TP_CRITICAL].temperature = temperature;
    >>>> + aux_entry->trips[QRK_DTS_ID_TP_CRITICAL].type = THERMAL_TRIP_CRITICAL;
    >>>> +
    >>>> + err = get_trip_temp(QRK_DTS_ID_TP_HOT, &temperature);
    >>>> + if (err)
    >>>> + goto err_ret;
    >>>
    >>> If I'm not mistaken, this won't even try to register the thermal zone
    >>> if at least one trip cannot be initialized, but previously it was
    >>> registered in that case, but the trips that failed to respond were
    >>> disabled.
    >>>
    >>> This is a change in behavior that would at least need to be documented
    >>> in the changelog, but it isn't.
    >>>
    >>> I'm not sure if it is safe to make even, however.
    >>
    >> Thanks for catching this.
    >>
    >> Two solutions:
    >>
    >> 1. Set the temperature to THERMAL_TEMP_INVALID and change
    >> get_thermal_trip() to return -EINVAL or -ERANGE if the temperature is
    >> THERMAL_TEMP_INVALID
    >>
    >> 2. Register only the valid trip points.
    >>
    >> What would be the preferable way ?
    >
    > I think that the trip points that are registered currently need to
    > still be registered after the change.
    >
    > Does registering a trip point with the temperature set to
    > THERMAL_TEMP_INVALID cause it to be effectively disabled?

    No but if we have thermal_zone_get_trip() returning -EINVAL if
    THERMAL_TEMP_INVALID is set for the specified trip id. Then the
    registering will set the disabled flag.

    https://git.kernel.org/pub/scm/linux/kernel/git/thermal/linux.git/tree/drivers/thermal/thermal_core.c?h=thermal/bleeding-edge#n1395




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

    \
     
     \ /
      Last update: 2023-03-27 00:04    [W:2.995 / U:0.012 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site