09.07.2020 12:51, Jon Hunter пишет: > > > On 09/07/2020 10:48, Dmitry Osipenko wrote: >> 09.07.2020 12:06, Jon Hunter пишет: >>> >>> 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? >> >> Could you please clarify what do you mean by "ignore any error"? Do you >> mean the error message? > > Yes exactly. We would skip that, which seems a bit odd. Yeah.. well, I was a bit doubtful about the error message, maybe arm_cpuidle_simple_enter() could have its own message? :) But since you're asking about it now, then perhaps it should be better and more clear to everyone not to skip the error message. Alright, I'll prepare the v3. Thank you for the review!
