lkml.org 
[lkml]   [2018]   [Apr]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRe: [PATCH 02/14] thermal: exynos: Propagate error value from tmu_read()
    Date
    On Monday, April 16, 2018 02:41:48 PM Daniel Lezcano wrote:
    > On 16/04/2018 14:35, Bartlomiej Zolnierkiewicz wrote:
    > > On Monday, April 16, 2018 02:16:56 PM Daniel Lezcano wrote:
    > >> On 16/04/2018 12:11, Bartlomiej Zolnierkiewicz wrote:
    > >>> From: Marek Szyprowski <m.szyprowski@samsung.com>
    > >>>
    > >>> tmu_read() in case of Exynos4210 might return error for out of bound
    > >>> values. Current code ignores such value, what leads to reporting critical
    > >>> temperature value. Add proper error code propagation to exynos_get_temp()
    > >>> function.
    > >>
    > >> For me the comment in the function exynos4210_tmu_read
    > >>
    > >> /* "temp_code" should range between 75 and 175 */
    > >>
    > >> ... is strange. I would double check this assertion before dealing with
    > >> the error value.
    > >
    > > static int exynos4210_tmu_read(struct exynos_tmu_data *data)
    > > {
    > > int ret = readb(data->base + EXYNOS_TMU_REG_CURRENT_TEMP);
    > >
    > > /* "temp_code" should range between 75 and 175 */
    > > return (ret < 75 || ret > 175) ? -ENODATA : ret;
    > > }
    > >
    >
    > But I don't get why it *should* ?

    Because of hardware design.

    > Shouldn't be the same with the 4412, it seems having the same sensor, no?

    Probably same limitations apply to all SoCs (Exynos4412 has very similar
    sensor) but the driver currently lacks the needed checks for them (it is
    on TODO but other things have higher priority).

    > > The value returned by Exynos4210 hardware should be > 75 && < 175 and
    > > it is later used as the "temp_code" parameter for code_to_temp():
    > >
    > > static int code_to_temp(struct exynos_tmu_data *data, u16 temp_code)
    > > {
    > > if (data->cal_type == TYPE_ONE_POINT_TRIMMING)
    > > return temp_code - data->temp_error1 + EXYNOS_FIRST_POINT_TRIM;
    > >
    > > return (temp_code - data->temp_error1) *
    > > (EXYNOS_SECOND_POINT_TRIM - EXYNOS_FIRST_POINT_TRIM) /
    > > (data->temp_error2 - data->temp_error1) +
    > > EXYNOS_FIRST_POINT_TRIM;
    > > }
    > >
    > > so after the current fix the code finally matches the comment.
    > >
    > >>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
    > >>> CC: stable@vger.kernel.org # v4.6+
    > >>> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
    > >>> ---
    > >>> drivers/thermal/samsung/exynos_tmu.c | 9 +++++++--
    > >>> 1 file changed, 7 insertions(+), 2 deletions(-)
    > >>>
    > >>> diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
    > >>> index 986cbd0..ac83f72 100644
    > >>> --- a/drivers/thermal/samsung/exynos_tmu.c
    > >>> +++ b/drivers/thermal/samsung/exynos_tmu.c
    > >>> @@ -892,6 +892,7 @@ static void exynos7_tmu_control(struct platform_device *pdev, bool on)
    > >>> static int exynos_get_temp(void *p, int *temp)
    > >>> {
    > >>> struct exynos_tmu_data *data = p;
    > >>> + int value, ret = 0;
    > >>>
    > >>> if (!data || !data->tmu_read || !data->enabled)
    > >>> return -EINVAL;
    > >>> @@ -899,12 +900,16 @@ static int exynos_get_temp(void *p, int *temp)
    > >>> mutex_lock(&data->lock);
    > >>> clk_enable(data->clk);
    > >>>
    > >>> - *temp = code_to_temp(data, data->tmu_read(data)) * MCELSIUS;
    > >>> + value = data->tmu_read(data);
    > >>> + if (value < 0)
    > >>> + ret = value;
    > >>> + else
    > >>> + *temp = code_to_temp(data, value) * MCELSIUS;
    > >>>
    > >>> clk_disable(data->clk);
    > >>> mutex_unlock(&data->lock);
    > >>>
    > >>> - return 0;
    > >>> + return ret;
    > >>> }
    > >>>
    > >>> #ifdef CONFIG_THERMAL_EMULATION

    Best regards,
    --
    Bartlomiej Zolnierkiewicz
    Samsung R&D Institute Poland
    Samsung Electronics

    \
     
     \ /
      Last update: 2018-04-16 14:50    [W:4.753 / U:1.124 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site