lkml.org 
[lkml]   [2023]   [Oct]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH v3 7/8] thermal: exynos: split initialization of TMU and the thermal zone
From


On 10/23/23 14:33, Marek Szyprowski wrote:
> On 23.10.2023 14:59, Lukasz Luba wrote:
>> On 10/3/23 12:16, Mateusz Majewski wrote:
>>> This will be needed in the future, as the thermal zone subsystem might
>>> call our callbacks right after devm_thermal_of_zone_register. Currently
>>> we just make get_temp return EAGAIN in such case, but this will not be
>>> possible with state-modifying callbacks, for instance set_trips.
>>>
>>> Signed-off-by: Mateusz Majewski <m.majewski2@samsung.com>
>>> ---
>>> v1 -> v2: We take clocks into account; exynos_tmu_initialize needs both
>>>    clocks, as tmu_initialize might use the base_second registers.
>>> However,
>>>    exynos_thermal_zone_configure only needs clk.
>>>
>>>   drivers/thermal/samsung/exynos_tmu.c | 104 +++++++++++++++------------
>>>   1 file changed, 60 insertions(+), 44 deletions(-)
>>>
>>> diff --git a/drivers/thermal/samsung/exynos_tmu.c
>>> b/drivers/thermal/samsung/exynos_tmu.c
>>> index 7138e001fa5a..343e27c61528 100644
>>> --- a/drivers/thermal/samsung/exynos_tmu.c
>>> +++ b/drivers/thermal/samsung/exynos_tmu.c
>>> @@ -251,25 +251,8 @@ static void sanitize_temp_error(struct
>>> exynos_tmu_data *data, u32 trim_info)
>>>   static int exynos_tmu_initialize(struct platform_device *pdev)
>>>   {
>>>       struct exynos_tmu_data *data = platform_get_drvdata(pdev);
>>> -    struct thermal_zone_device *tzd = data->tzd;
>>> -    int num_trips = thermal_zone_get_num_trips(tzd);
>>>       unsigned int status;
>>> -    int ret = 0, temp;
>>> -
>>> -    ret = thermal_zone_get_crit_temp(tzd, &temp);
>>> -    if (ret && data->soc != SOC_ARCH_EXYNOS5433) { /* FIXME */
>>> -        dev_err(&pdev->dev,
>>> -            "No CRITICAL trip point defined in device tree!\n");
>>> -        goto out;
>>> -    }
>>> -
>>> -    if (num_trips > data->ntrip) {
>>> -        dev_info(&pdev->dev,
>>> -             "More trip points than supported by this TMU.\n");
>>> -        dev_info(&pdev->dev,
>>> -             "%d trip points should be configured in polling mode.\n",
>>> -             num_trips - data->ntrip);
>>> -    }
>>> +    int ret = 0;
>>>         mutex_lock(&data->lock);
>>>       clk_enable(data->clk);
>>> @@ -280,32 +263,63 @@ static int exynos_tmu_initialize(struct
>>> platform_device *pdev)
>>>       if (!status) {
>>>           ret = -EBUSY;
>>>       } else {
>>> -        int i, ntrips =
>>> -            min_t(int, num_trips, data->ntrip);
>>> -
>>>           data->tmu_initialize(pdev);
>>> -
>>> -        /* Write temperature code for rising and falling threshold */
>>> -        for (i = 0; i < ntrips; i++) {
>>> -
>>> -            struct thermal_trip trip;
>>> -
>>> -            ret = thermal_zone_get_trip(tzd, i, &trip);
>>> -            if (ret)
>>> -                goto err;
>>> -
>>> -            data->tmu_set_trip_temp(data, i, trip.temperature /
>>> MCELSIUS);
>>> -            data->tmu_set_trip_hyst(data, i, trip.temperature /
>>> MCELSIUS,
>>> -                        trip.hysteresis / MCELSIUS);
>>> -        }
>>> -
>>>           data->tmu_clear_irqs(data);
>>>       }
>>> +
>>> +    mutex_unlock(&data->lock);
>>> +    clk_disable(data->clk);
>>> +    if (!IS_ERR(data->clk_sec))
>>> +        clk_disable(data->clk_sec);
>>
>> In all other places the clock is strictly protected inside the critical
>> section, but not here. In this code in theory on SMP (especially with
>> big.LITTLE system with different speeds of CPUs) this could lead to
>> disabling the clocks just after other CPU acquired the mutex and enabled
>> them (in order to use the HW regs).
>
>
> Clocks have internal atomic counters, so it is safe to disable them
> after leaving critical section. The clock would still be enabled in the
> mentioned case.

Fair enough. So I would just put them there for code cleanup and
aliment with all other places (otherwise it looks odd).


\
 
 \ /
  Last update: 2023-10-23 15:44    [W:0.090 / U:0.012 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site