Re: [PATCH v2] cpuidle: tegra: Correctly handle result of arm_cpuidle_simple_enter()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 08/07/2020 15:32, Dmitry Osipenko wrote:
> 08.07.2020 15:34, Jon Hunter пишет:
>>
>> On 02/07/2020 01:13, Dmitry Osipenko wrote:
>>> The enter() callback of CPUIDLE drivers returns index of the entered idle
>>> state on success or a negative value on failure. The negative value could
>>> any negative value, i.e. it doesn't necessarily needs to be a error code.
>>> That's because CPUIDLE core only cares about the fact of failure and not
>>> about the reason of the enter() failure.
>>>
>>> Like every other enter() callback, the arm_cpuidle_simple_enter() returns
>>> the entered idle-index on success. Unlike some of other drivers, it never
>>> fails. It happened that TEGRA_C1=index=err=0 in the code of cpuidle-tegra
>>> driver, and thus, there is no problem for the cpuidle-tegra driver created
>>> by the typo in the code which assumes that the arm_cpuidle_simple_enter()
>>> returns a error code.
>>>
>>> The arm_cpuidle_simple_enter() also may return a -ENODEV error if CPU_IDLE
>>> is disabled in a kernel's config, but all CPUIDLE drivers are disabled if
>>> CPU_IDLE is disabled, including the cpuidle-tegra driver. So we can't ever
>>> see the error code from arm_cpuidle_simple_enter() today.
>>>
>>> Of course the code may get some changes in the future and then the typo
>>> may transform into a real bug, so let's correct the typo in the code by
>>> making tegra_cpuidle_enter() to directly return the index returned by the
>>> arm_cpuidle_simple_enter().
>>
>> Are you suggesting that arm_cpuidle_simple_enter() could be updated to
>> actually return an error? Sorry it is not clear to me what you are implying.
> 
> Hello, Jon!
> 
> Yes, I'm saying that *potentially* arm_cpuidle_simple_enter() could be
> updated to actually return error.


OK, then I am confused, because after your change, we would now ignore
any error that could be returned in the future. Yes the current code
does not set the variable 'index' correctly, but before we set the value
of 'index' shouldn't we check that the value being returned is not a
negative error code first?

Jon

-- 
nvpublic




[Index of Archives]     [ARM Kernel]     [Linux ARM]     [Linux ARM MSM]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux