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. Jon -- nvpublic
