[RFC,v15,04/11] ARM: cpuidle: Register cpuidle_ops for QCOM cpus

Message ID 1425914206-22295-5-git-send-email-lina.iyer@linaro.org
State New
Headers show

Commit Message

Lina Iyer March 9, 2015, 3:16 p.m.
Register cpuidle ops for Krait Processor Sub-System.
Supports v1 and v2 versions of KPSS.

Cc: Stephen Boyd <sboyd@codeaurora.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Kevin Hilman <khilman@linaro.org>
Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
---
 arch/arm/include/asm/cpuidle_ops.h | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Daniel Lezcano March 12, 2015, 12:34 p.m. | #1
On 03/11/2015 07:16 PM, Stephen Boyd wrote:
> On 03/09/15 08:16, Lina Iyer wrote:
>> Register cpuidle ops for Krait Processor Sub-System.
>> Supports v1 and v2 versions of KPSS.
>>
>> Cc: Stephen Boyd <sboyd@codeaurora.org>
>> Cc: Arnd Bergmann <arnd@arndb.de>
>> Cc: Kevin Hilman <khilman@linaro.org>
>> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
>> Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
>> ---
>>   arch/arm/include/asm/cpuidle_ops.h | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/arch/arm/include/asm/cpuidle_ops.h b/arch/arm/include/asm/cpuidle_ops.h
>> index be0a612..40cb262 100644
>> --- a/arch/arm/include/asm/cpuidle_ops.h
>> +++ b/arch/arm/include/asm/cpuidle_ops.h
>> @@ -1,3 +1,8 @@
>>   /*
>>    * List of cpuidle operations
>>    */
>> +
>> +#if IS_ENABLED(CONFIG_QCOM_PM)
>
> Why not #ifdef CONFIG_QCOM_PM? I thought IS_ENABLED was for C code where
> you want to put it in an if () statement.

This is right. But the IS_ENABLED is also valid.

It seems the current trend is to replace #ifdef with #if IS_ENABLED. It 
is supposed to prevent to create or'ed conditional with the module support.

eg:

#if defined(CONFIG_FOO) || defined(CONFIG_FOO_MODULE)

is replaced by:

#if IS_ENABLED(CONFIG_FOO)

Lina: By the way, it would be preferable to use the 
CONFIG_ARM_QCOM_CPUIDLE where IS_ENABLED makes more sense even the 
drivers are not modules (who knows if they are converted as module again 
hence #ifdef will be invalid).



>> +CPUIDLE_OPS(qcom_kpss_v1)
>> +CPUIDLE_OPS(qcom_kpss_v2)
>> +#endif
>
>
Lina Iyer March 12, 2015, 3:06 p.m. | #2
On Thu, Mar 12 2015 at 06:34 -0600, Daniel Lezcano wrote:
>On 03/11/2015 07:16 PM, Stephen Boyd wrote:
>>On 03/09/15 08:16, Lina Iyer wrote:

[...]

>>>+#if IS_ENABLED(CONFIG_QCOM_PM)
>>
>>Why not #ifdef CONFIG_QCOM_PM? I thought IS_ENABLED was for C code where
>>you want to put it in an if () statement.
>
>This is right. But the IS_ENABLED is also valid.
>
>It seems the current trend is to replace #ifdef with #if IS_ENABLED. 
>It is supposed to prevent to create or'ed conditional with the module 
>support.
>
>eg:
>
>#if defined(CONFIG_FOO) || defined(CONFIG_FOO_MODULE)
>
>is replaced by:
>
>#if IS_ENABLED(CONFIG_FOO)
>
>Lina: By the way, it would be preferable to use the 
>CONFIG_ARM_QCOM_CPUIDLE where IS_ENABLED makes more sense even the 

Daniel: I dont have CONFIG_ARM_QCOM_CPUIDLE defined anymore. By moving
to generic ARM cpuidle, i removed all changes to drivers/cpuidle/. So I
used the config item that defineds these items.

Also, having two indpendent CONFIG_ items provides a chance that the
declaration is not matched with the definition or vice versa.

>drivers are not modules (who knows if they are converted as module 
>again hence #ifdef will be invalid).
>
>
>
>>>+CPUIDLE_OPS(qcom_kpss_v1)
>>>+CPUIDLE_OPS(qcom_kpss_v2)
>>>+#endif

--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Daniel Lezcano March 13, 2015, 9:57 a.m. | #3
On 03/12/2015 04:06 PM, Lina Iyer wrote:
> On Thu, Mar 12 2015 at 06:34 -0600, Daniel Lezcano wrote:
>> On 03/11/2015 07:16 PM, Stephen Boyd wrote:
>>> On 03/09/15 08:16, Lina Iyer wrote:
>
> [...]
>
>>>> +#if IS_ENABLED(CONFIG_QCOM_PM)
>>>
>>> Why not #ifdef CONFIG_QCOM_PM? I thought IS_ENABLED was for C code where
>>> you want to put it in an if () statement.
>>
>> This is right. But the IS_ENABLED is also valid.
>>
>> It seems the current trend is to replace #ifdef with #if IS_ENABLED.
>> It is supposed to prevent to create or'ed conditional with the module
>> support.
>>
>> eg:
>>
>> #if defined(CONFIG_FOO) || defined(CONFIG_FOO_MODULE)
>>
>> is replaced by:
>>
>> #if IS_ENABLED(CONFIG_FOO)
>>
>> Lina: By the way, it would be preferable to use the
>> CONFIG_ARM_QCOM_CPUIDLE where IS_ENABLED makes more sense even the
>
> Daniel: I dont have CONFIG_ARM_QCOM_CPUIDLE defined anymore. By moving
> to generic ARM cpuidle, i removed all changes to drivers/cpuidle/. So I
> used the config item that defineds these items.
>
> Also, having two indpendent CONFIG_ items provides a chance that the
> declaration is not matched with the definition or vice versa.

I think CONFIG_ARM_QCOM_CPUIDLE should be defined as a bool in the 
Kconfig.arm in drivers/cpuidle.

We may want to disable the different cpuidle drivers even if they rely 
on the generic cpuidle driver.

>> drivers are not modules (who knows if they are converted as module
>> again hence #ifdef will be invalid).
>>
>>
>>
>>>> +CPUIDLE_OPS(qcom_kpss_v1)
>>>> +CPUIDLE_OPS(qcom_kpss_v2)
>>>> +#endif
>

Patch

diff --git a/arch/arm/include/asm/cpuidle_ops.h b/arch/arm/include/asm/cpuidle_ops.h
index be0a612..40cb262 100644
--- a/arch/arm/include/asm/cpuidle_ops.h
+++ b/arch/arm/include/asm/cpuidle_ops.h
@@ -1,3 +1,8 @@ 
 /*
  * List of cpuidle operations
  */
+
+#if IS_ENABLED(CONFIG_QCOM_PM)
+CPUIDLE_OPS(qcom_kpss_v1)
+CPUIDLE_OPS(qcom_kpss_v2)
+#endif