Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1363919 > unrolled thread
| Started by | Jisheng Zhang <jszhang@marvell.com> |
|---|---|
| First post | 2016-03-24 06:20 +0100 |
| Last post | 2016-03-25 13:00 +0100 |
| Articles | 13 — 3 participants |
Back to article view | Back to linux.kernel
[PATCH 0/2] ARM: cpuidle: bug fix and a trivial improvement Jisheng Zhang <jszhang@marvell.com> - 2016-03-24 06:20 +0100
[PATCH 1/2] ARM: cpuidle: fix !cpuidle_ops[cpu].init case during init Jisheng Zhang <jszhang@marvell.com> - 2016-03-24 06:20 +0100
Re: [PATCH 1/2] ARM: cpuidle: fix !cpuidle_ops[cpu].init case during init Daniel Lezcano <daniel.lezcano@linaro.org> - 2016-03-25 12:50 +0100
Re: [PATCH 1/2] ARM: cpuidle: fix !cpuidle_ops[cpu].init case during init Jisheng Zhang <jszhang@marvell.com> - 2016-03-30 09:30 +0200
Re: [PATCH 1/2] ARM: cpuidle: fix !cpuidle_ops[cpu].init case during init Daniel Lezcano <daniel.lezcano@linaro.org> - 2016-03-30 10:20 +0200
Re: [PATCH 1/2] ARM: cpuidle: fix !cpuidle_ops[cpu].init case during init Jisheng Zhang <jszhang@marvell.com> - 2016-03-30 10:30 +0200
Re: [PATCH 1/2] ARM: cpuidle: fix !cpuidle_ops[cpu].init case during init Jisheng Zhang <jszhang@marvell.com> - 2016-03-30 10:50 +0200
Re: [PATCH 1/2] ARM: cpuidle: fix !cpuidle_ops[cpu].init case during init Daniel Lezcano <daniel.lezcano@linaro.org> - 2016-03-30 11:40 +0200
Re: [PATCH 1/2] ARM: cpuidle: fix !cpuidle_ops[cpu].init case during init Jisheng Zhang <jszhang@marvell.com> - 2016-03-30 11:50 +0200
Re: [PATCH 1/2] ARM: cpuidle: fix !cpuidle_ops[cpu].init case during init Daniel Lezcano <daniel.lezcano@linaro.org> - 2016-03-30 10:50 +0200
Re: [PATCH 1/2] ARM: cpuidle: fix !cpuidle_ops[cpu].init case during init Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> - 2016-03-30 12:40 +0200
[PATCH 2/2] ARM: cpuidle: make arm_cpuidle_suspend() a bit more efficient Jisheng Zhang <jszhang@marvell.com> - 2016-03-24 06:20 +0100
Re: [PATCH 2/2] ARM: cpuidle: make arm_cpuidle_suspend() a bit more efficient Daniel Lezcano <daniel.lezcano@linaro.org> - 2016-03-25 13:00 +0100
| From | Jisheng Zhang <jszhang@marvell.com> |
|---|---|
| Date | 2016-03-24 06:20 +0100 |
| Subject | [PATCH 0/2] ARM: cpuidle: bug fix and a trivial improvement |
| Message-ID | <rg5QC-6d1-1@gated-at.bofh.it> |
There's one corner case need to be fixed: !cpuidle_ops[cpu].init. patch1 tries to address this corner case. patch2 tries to improve arm_cpuidle_suspend() a bit by moving .suspend check into arm_cpuidle_init(). Jisheng Zhang (2): ARM: cpuidle: fix !cpuidle_ops[cpu].init case during init ARM: cpuidle: make arm_cpuidle_suspend() a bit more efficient arch/arm/kernel/cpuidle.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) -- 2.8.0.rc3
[toc] | [next] | [standalone]
| From | Jisheng Zhang <jszhang@marvell.com> |
|---|---|
| Date | 2016-03-24 06:20 +0100 |
| Subject | [PATCH 1/2] ARM: cpuidle: fix !cpuidle_ops[cpu].init case during init |
| Message-ID | <rg5QC-6d1-9@gated-at.bofh.it> |
| In reply to | #1363919 |
Let's assume cpuidle_ops exists but it doesn't implement the according
init member, current arm_cpuidle_init() will return success to its
caller, but in fact it should return -EOPNOTSUPP.
Signed-off-by: Jisheng Zhang <jszhang@marvell.com>
---
arch/arm/kernel/cpuidle.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/arch/arm/kernel/cpuidle.c b/arch/arm/kernel/cpuidle.c
index 703926e..f108d8f 100644
--- a/arch/arm/kernel/cpuidle.c
+++ b/arch/arm/kernel/cpuidle.c
@@ -143,8 +143,12 @@ int __init arm_cpuidle_init(int cpu)
return -ENODEV;
ret = arm_cpuidle_read_ops(cpu_node, cpu);
- if (!ret && cpuidle_ops[cpu].init)
- ret = cpuidle_ops[cpu].init(cpu_node, cpu);
+ if (!ret) {
+ if (cpuidle_ops[cpu].init)
+ ret = cpuidle_ops[cpu].init(cpu_node, cpu);
+ else
+ ret = -EOPNOTSUPP;
+ }
of_node_put(cpu_node);
--
2.8.0.rc3
[toc] | [prev] | [next] | [standalone]
| From | Daniel Lezcano <daniel.lezcano@linaro.org> |
|---|---|
| Date | 2016-03-25 12:50 +0100 |
| Subject | Re: [PATCH 1/2] ARM: cpuidle: fix !cpuidle_ops[cpu].init case during init |
| Message-ID | <rgypA-14r-7@gated-at.bofh.it> |
| In reply to | #1363922 |
On 03/24/2016 06:11 AM, Jisheng Zhang wrote:
> Let's assume cpuidle_ops exists but it doesn't implement the according
> init member, current arm_cpuidle_init() will return success to its
> caller, but in fact it should return -EOPNOTSUPP.
>
> Signed-off-by: Jisheng Zhang <jszhang@marvell.com>
> ---
> arch/arm/kernel/cpuidle.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/kernel/cpuidle.c b/arch/arm/kernel/cpuidle.c
> index 703926e..f108d8f 100644
> --- a/arch/arm/kernel/cpuidle.c
> +++ b/arch/arm/kernel/cpuidle.c
> @@ -143,8 +143,12 @@ int __init arm_cpuidle_init(int cpu)
> return -ENODEV;
>
> ret = arm_cpuidle_read_ops(cpu_node, cpu);
> - if (!ret && cpuidle_ops[cpu].init)
> - ret = cpuidle_ops[cpu].init(cpu_node, cpu);
> + if (!ret) {
> + if (cpuidle_ops[cpu].init)
> + ret = cpuidle_ops[cpu].init(cpu_node, cpu);
> + else
> + ret = -EOPNOTSUPP;
> + }
Hi Jisheng,
this should be handled in the arm_cpuidle_read_ops function.
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
[toc] | [prev] | [next] | [standalone]
| From | Jisheng Zhang <jszhang@marvell.com> |
|---|---|
| Date | 2016-03-30 09:30 +0200 |
| Subject | Re: [PATCH 1/2] ARM: cpuidle: fix !cpuidle_ops[cpu].init case during init |
| Message-ID | <riiJH-215-3@gated-at.bofh.it> |
| In reply to | #1364642 |
Hi Daniel,
On Fri, 25 Mar 2016 12:46:10 +0100 Daniel Lezcano wrote:
> On 03/24/2016 06:11 AM, Jisheng Zhang wrote:
> > Let's assume cpuidle_ops exists but it doesn't implement the according
> > init member, current arm_cpuidle_init() will return success to its
> > caller, but in fact it should return -EOPNOTSUPP.
> >
> > Signed-off-by: Jisheng Zhang <jszhang@marvell.com>
> > ---
> > arch/arm/kernel/cpuidle.c | 8 ++++++--
> > 1 file changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/arm/kernel/cpuidle.c b/arch/arm/kernel/cpuidle.c
> > index 703926e..f108d8f 100644
> > --- a/arch/arm/kernel/cpuidle.c
> > +++ b/arch/arm/kernel/cpuidle.c
> > @@ -143,8 +143,12 @@ int __init arm_cpuidle_init(int cpu)
> > return -ENODEV;
> >
> > ret = arm_cpuidle_read_ops(cpu_node, cpu);
> > - if (!ret && cpuidle_ops[cpu].init)
> > - ret = cpuidle_ops[cpu].init(cpu_node, cpu);
> > + if (!ret) {
> > + if (cpuidle_ops[cpu].init)
> > + ret = cpuidle_ops[cpu].init(cpu_node, cpu);
> > + else
> > + ret = -EOPNOTSUPP;
> > + }
>
> Hi Jisheng,
>
> this should be handled in the arm_cpuidle_read_ops function.
>
Thanks for reviewing. After some consideration, I think this patch isn't correct
There may be platforms which doesn't need the init member at all, although
currently I don't see such platforms in mainline, So I'll drop this patch
and send out one v2 only does the optimization.
Thanks a lot,
Jisheng
[toc] | [prev] | [next] | [standalone]
| From | Daniel Lezcano <daniel.lezcano@linaro.org> |
|---|---|
| Date | 2016-03-30 10:20 +0200 |
| Subject | Re: [PATCH 1/2] ARM: cpuidle: fix !cpuidle_ops[cpu].init case during init |
| Message-ID | <rijw6-2y5-31@gated-at.bofh.it> |
| In reply to | #1366953 |
On 03/30/2016 09:16 AM, Jisheng Zhang wrote: > Hi Daniel, [ ... ] Added Lorenzo and Catalin. >> Hi Jisheng, >> >> this should be handled in the arm_cpuidle_read_ops function. >> > > Thanks for reviewing. After some consideration, I think this patch isn't correct > There may be platforms which doesn't need the init member at all, although > currently I don't see such platforms in mainline, So I'll drop this patch > and send out one v2 only does the optimization. There is an inconsistency between ARM and ARM64. The 'cpu_get_ops', the arm_cpuidle_read_ops from the ARM64 side, returns -EOPNOTSUPP when the init function is not there for cpuidle. I don't think it is a problem, but as ARM/ARM64 are sharing the same cpuidle-arm.c driver it would make sense to unify the behavior between both archs. -- <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | <http://twitter.com/#!/linaroorg> Twitter | <http://www.linaro.org/linaro-blog/> Blog
[toc] | [prev] | [next] | [standalone]
| From | Jisheng Zhang <jszhang@marvell.com> |
|---|---|
| Date | 2016-03-30 10:30 +0200 |
| Subject | Re: [PATCH 1/2] ARM: cpuidle: fix !cpuidle_ops[cpu].init case during init |
| Message-ID | <rijFN-2CP-17@gated-at.bofh.it> |
| In reply to | #1366993 |
On Wed, 30 Mar 2016 10:09:12 +0200 Daniel Lezcano wrote: > On 03/30/2016 09:16 AM, Jisheng Zhang wrote: > > Hi Daniel, > > [ ... ] > > Added Lorenzo and Catalin. > > >> Hi Jisheng, > >> > >> this should be handled in the arm_cpuidle_read_ops function. > >> > > > > Thanks for reviewing. After some consideration, I think this patch isn't correct > > There may be platforms which doesn't need the init member at all, although > > currently I don't see such platforms in mainline, So I'll drop this patch > > and send out one v2 only does the optimization. > > There is an inconsistency between ARM and ARM64. The 'cpu_get_ops', the > arm_cpuidle_read_ops from the ARM64 side, returns -EOPNOTSUPP when the > init function is not there for cpuidle. yes. arm64's arm_cpuidle_init() returns -EOPNOTSUPP if init callback isn't defined > > I don't think it is a problem, but as ARM/ARM64 are sharing the same > cpuidle-arm.c driver it would make sense to unify the behavior between > both archs. yes, agree with you. From "unify" point of view, could I move back the suspend callback check and init callback check into arm_cpuidle_init() for arm as V1 does? Thanks for reviewing, Jisheng
[toc] | [prev] | [next] | [standalone]
| From | Jisheng Zhang <jszhang@marvell.com> |
|---|---|
| Date | 2016-03-30 10:50 +0200 |
| Subject | Re: [PATCH 1/2] ARM: cpuidle: fix !cpuidle_ops[cpu].init case during init |
| Message-ID | <rijZa-2KA-47@gated-at.bofh.it> |
| In reply to | #1367001 |
On Wed, 30 Mar 2016 10:41:09 +0200 Daniel Lezcano wrote: > On 03/30/2016 10:17 AM, Jisheng Zhang wrote: > > On Wed, 30 Mar 2016 10:09:12 +0200 Daniel Lezcano wrote: > > > >> On 03/30/2016 09:16 AM, Jisheng Zhang wrote: > >>> Hi Daniel, > >> > >> [ ... ] > >> > >> Added Lorenzo and Catalin. > >> > >>>> Hi Jisheng, > >>>> > >>>> this should be handled in the arm_cpuidle_read_ops function. > >>>> > >>> > >>> Thanks for reviewing. After some consideration, I think this patch isn't correct > >>> There may be platforms which doesn't need the init member at all, although > >>> currently I don't see such platforms in mainline, So I'll drop this patch > >>> and send out one v2 only does the optimization. > >> > >> There is an inconsistency between ARM and ARM64. The 'cpu_get_ops', the > >> arm_cpuidle_read_ops from the ARM64 side, returns -EOPNOTSUPP when the > >> init function is not there for cpuidle. > > > > yes. > > arm64's arm_cpuidle_init() returns -EOPNOTSUPP if init callback isn't defined > > > >> > >> I don't think it is a problem, but as ARM/ARM64 are sharing the same > >> cpuidle-arm.c driver it would make sense to unify the behavior between > >> both archs. > > > > yes, agree with you. From "unify" point of view, could I move back the suspend > > callback check and init callback check into arm_cpuidle_init() for arm as V1 does? > > Why ? To be consistent with ARM64 ? Yes, that's my intention.
[toc] | [prev] | [next] | [standalone]
| From | Daniel Lezcano <daniel.lezcano@linaro.org> |
|---|---|
| Date | 2016-03-30 11:40 +0200 |
| Subject | Re: [PATCH 1/2] ARM: cpuidle: fix !cpuidle_ops[cpu].init case during init |
| Message-ID | <rikLx-3iV-19@gated-at.bofh.it> |
| In reply to | #1367017 |
On 03/30/2016 10:43 AM, Jisheng Zhang wrote: > On Wed, 30 Mar 2016 10:41:09 +0200 Daniel Lezcano wrote: > >> On 03/30/2016 10:17 AM, Jisheng Zhang wrote: >>> On Wed, 30 Mar 2016 10:09:12 +0200 Daniel Lezcano wrote: >>> >>>> On 03/30/2016 09:16 AM, Jisheng Zhang wrote: >>>>> Hi Daniel, >>>> >>>> [ ... ] >>>> >>>> Added Lorenzo and Catalin. >>>> >>>>>> Hi Jisheng, >>>>>> >>>>>> this should be handled in the arm_cpuidle_read_ops function. >>>>>> >>>>> >>>>> Thanks for reviewing. After some consideration, I think this patch isn't correct >>>>> There may be platforms which doesn't need the init member at all, although >>>>> currently I don't see such platforms in mainline, So I'll drop this patch >>>>> and send out one v2 only does the optimization. >>>> >>>> There is an inconsistency between ARM and ARM64. The 'cpu_get_ops', the >>>> arm_cpuidle_read_ops from the ARM64 side, returns -EOPNOTSUPP when the >>>> init function is not there for cpuidle. >>> >>> yes. >>> arm64's arm_cpuidle_init() returns -EOPNOTSUPP if init callback isn't defined >>> >>>> >>>> I don't think it is a problem, but as ARM/ARM64 are sharing the same >>>> cpuidle-arm.c driver it would make sense to unify the behavior between >>>> both archs. >>> >>> yes, agree with you. From "unify" point of view, could I move back the suspend >>> callback check and init callback check into arm_cpuidle_init() for arm as V1 does? >> >> Why ? To be consistent with ARM64 ? > > Yes, that's my intention. Well, I don't have a strong opinion on that. ARM64 cpu_ops is slightly different from cpuidle_ops as the cpu boot / hotplug operations are placed in a different place and that explains why on ARM64 we can have an successful 'get_ops' because we use the partially filled structure. On ARM, it is cpuidle_ops only, so we can gracefully fail if the ops are not defined. IMO, it still make sense to keep the checks in arm_cpuidle_read_ops for ARM. -- <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | <http://twitter.com/#!/linaroorg> Twitter | <http://www.linaro.org/linaro-blog/> Blog
[toc] | [prev] | [next] | [standalone]
| From | Jisheng Zhang <jszhang@marvell.com> |
|---|---|
| Date | 2016-03-30 11:50 +0200 |
| Subject | Re: [PATCH 1/2] ARM: cpuidle: fix !cpuidle_ops[cpu].init case during init |
| Message-ID | <rikVc-3my-11@gated-at.bofh.it> |
| In reply to | #1367044 |
Hi Daniel, On Wed, 30 Mar 2016 11:31:39 +0200 Daniel Lezcano wrote: > On 03/30/2016 10:43 AM, Jisheng Zhang wrote: > > On Wed, 30 Mar 2016 10:41:09 +0200 Daniel Lezcano wrote: > > > >> On 03/30/2016 10:17 AM, Jisheng Zhang wrote: > >>> On Wed, 30 Mar 2016 10:09:12 +0200 Daniel Lezcano wrote: > >>> > >>>> On 03/30/2016 09:16 AM, Jisheng Zhang wrote: > >>>>> Hi Daniel, > >>>> > >>>> [ ... ] > >>>> > >>>> Added Lorenzo and Catalin. > >>>> > >>>>>> Hi Jisheng, > >>>>>> > >>>>>> this should be handled in the arm_cpuidle_read_ops function. > >>>>>> > >>>>> > >>>>> Thanks for reviewing. After some consideration, I think this patch isn't correct > >>>>> There may be platforms which doesn't need the init member at all, although > >>>>> currently I don't see such platforms in mainline, So I'll drop this patch > >>>>> and send out one v2 only does the optimization. > >>>> > >>>> There is an inconsistency between ARM and ARM64. The 'cpu_get_ops', the > >>>> arm_cpuidle_read_ops from the ARM64 side, returns -EOPNOTSUPP when the > >>>> init function is not there for cpuidle. > >>> > >>> yes. > >>> arm64's arm_cpuidle_init() returns -EOPNOTSUPP if init callback isn't defined > >>> > >>>> > >>>> I don't think it is a problem, but as ARM/ARM64 are sharing the same > >>>> cpuidle-arm.c driver it would make sense to unify the behavior between > >>>> both archs. > >>> > >>> yes, agree with you. From "unify" point of view, could I move back the suspend > >>> callback check and init callback check into arm_cpuidle_init() for arm as V1 does? > >> > >> Why ? To be consistent with ARM64 ? > > > > Yes, that's my intention. > > Well, I don't have a strong opinion on that. ARM64 cpu_ops is slightly > different from cpuidle_ops as the cpu boot / hotplug operations are > placed in a different place and that explains why on ARM64 we can have > an successful 'get_ops' because we use the partially filled structure. > On ARM, it is cpuidle_ops only, so we can gracefully fail if the ops are > not defined. > > IMO, it still make sense to keep the checks in arm_cpuidle_read_ops for ARM. > Got your points. I'll send a v3 to add init check. These checks will be in arm_cpuidle_read_ops. Thanks, Jisheng
[toc] | [prev] | [next] | [standalone]
| From | Daniel Lezcano <daniel.lezcano@linaro.org> |
|---|---|
| Date | 2016-03-30 10:50 +0200 |
| Subject | Re: [PATCH 1/2] ARM: cpuidle: fix !cpuidle_ops[cpu].init case during init |
| Message-ID | <rijZa-2KA-49@gated-at.bofh.it> |
| In reply to | #1367001 |
On 03/30/2016 10:17 AM, Jisheng Zhang wrote: > On Wed, 30 Mar 2016 10:09:12 +0200 Daniel Lezcano wrote: > >> On 03/30/2016 09:16 AM, Jisheng Zhang wrote: >>> Hi Daniel, >> >> [ ... ] >> >> Added Lorenzo and Catalin. >> >>>> Hi Jisheng, >>>> >>>> this should be handled in the arm_cpuidle_read_ops function. >>>> >>> >>> Thanks for reviewing. After some consideration, I think this patch isn't correct >>> There may be platforms which doesn't need the init member at all, although >>> currently I don't see such platforms in mainline, So I'll drop this patch >>> and send out one v2 only does the optimization. >> >> There is an inconsistency between ARM and ARM64. The 'cpu_get_ops', the >> arm_cpuidle_read_ops from the ARM64 side, returns -EOPNOTSUPP when the >> init function is not there for cpuidle. > > yes. > arm64's arm_cpuidle_init() returns -EOPNOTSUPP if init callback isn't defined > >> >> I don't think it is a problem, but as ARM/ARM64 are sharing the same >> cpuidle-arm.c driver it would make sense to unify the behavior between >> both archs. > > yes, agree with you. From "unify" point of view, could I move back the suspend > callback check and init callback check into arm_cpuidle_init() for arm as V1 does? Why ? To be consistent with ARM64 ? -- <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | <http://twitter.com/#!/linaroorg> Twitter | <http://www.linaro.org/linaro-blog/> Blog
[toc] | [prev] | [next] | [standalone]
| From | Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> |
|---|---|
| Date | 2016-03-30 12:40 +0200 |
| Subject | Re: [PATCH 1/2] ARM: cpuidle: fix !cpuidle_ops[cpu].init case during init |
| Message-ID | <rilHA-3WZ-37@gated-at.bofh.it> |
| In reply to | #1366993 |
On Wed, Mar 30, 2016 at 10:09:12AM +0200, Daniel Lezcano wrote: > On 03/30/2016 09:16 AM, Jisheng Zhang wrote: > >Hi Daniel, > > [ ... ] > > Added Lorenzo and Catalin. > > >>Hi Jisheng, > >> > >>this should be handled in the arm_cpuidle_read_ops function. > >> > > > >Thanks for reviewing. After some consideration, I think this patch isn't correct > >There may be platforms which doesn't need the init member at all, although > >currently I don't see such platforms in mainline, So I'll drop this patch > >and send out one v2 only does the optimization. > > There is an inconsistency between ARM and ARM64. The 'cpu_get_ops', > the arm_cpuidle_read_ops from the ARM64 side, returns -EOPNOTSUPP > when the init function is not there for cpuidle. > > I don't think it is a problem, but as ARM/ARM64 are sharing the same > cpuidle-arm.c driver it would make sense to unify the behavior > between both archs. I agree and I think it makes sense to have an arm back-end that fails if there is no cpuidle_ops.init function registered, I doubt any usage of the cpuidle_ops.suspend is reasonable if it was not initialized by a corresponding cpuidle_ops.init at boot, at least that's how I see it working, I am open to other point of views. Thanks, Lorenzo
[toc] | [prev] | [next] | [standalone]
| From | Jisheng Zhang <jszhang@marvell.com> |
|---|---|
| Date | 2016-03-24 06:20 +0100 |
| Subject | [PATCH 2/2] ARM: cpuidle: make arm_cpuidle_suspend() a bit more efficient |
| Message-ID | <rg5QD-6d1-25@gated-at.bofh.it> |
| In reply to | #1363919 |
Currently, we check cpuidle_ops.suspend every time when entering a
low-power idle state. But this check could be avoided in this hot path
by moving it into arm_cpuidle_init() to reduce arm_cpuidle_suspend()
overhead a bit.
Signed-off-by: Jisheng Zhang <jszhang@marvell.com>
---
arch/arm/kernel/cpuidle.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/arch/arm/kernel/cpuidle.c b/arch/arm/kernel/cpuidle.c
index f108d8f..bf68d49 100644
--- a/arch/arm/kernel/cpuidle.c
+++ b/arch/arm/kernel/cpuidle.c
@@ -52,13 +52,9 @@ int arm_cpuidle_simple_enter(struct cpuidle_device *dev,
*/
int arm_cpuidle_suspend(int index)
{
- int ret = -EOPNOTSUPP;
int cpu = smp_processor_id();
- if (cpuidle_ops[cpu].suspend)
- ret = cpuidle_ops[cpu].suspend(index);
-
- return ret;
+ return cpuidle_ops[cpu].suspend(index);
}
/**
@@ -144,7 +140,7 @@ int __init arm_cpuidle_init(int cpu)
ret = arm_cpuidle_read_ops(cpu_node, cpu);
if (!ret) {
- if (cpuidle_ops[cpu].init)
+ if (cpuidle_ops[cpu].init && cpuidle_ops[cpu].suspend)
ret = cpuidle_ops[cpu].init(cpu_node, cpu);
else
ret = -EOPNOTSUPP;
--
2.8.0.rc3
[toc] | [prev] | [next] | [standalone]
| From | Daniel Lezcano <daniel.lezcano@linaro.org> |
|---|---|
| Date | 2016-03-25 13:00 +0100 |
| Subject | Re: [PATCH 2/2] ARM: cpuidle: make arm_cpuidle_suspend() a bit more efficient |
| Message-ID | <rgyzg-18a-5@gated-at.bofh.it> |
| In reply to | #1363928 |
On 03/24/2016 06:11 AM, Jisheng Zhang wrote:
> Currently, we check cpuidle_ops.suspend every time when entering a
> low-power idle state. But this check could be avoided in this hot path
> by moving it into arm_cpuidle_init() to reduce arm_cpuidle_suspend()
> overhead a bit.
>
> Signed-off-by: Jisheng Zhang <jszhang@marvell.com>
> ---
> arch/arm/kernel/cpuidle.c | 8 ++------
> 1 file changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/arch/arm/kernel/cpuidle.c b/arch/arm/kernel/cpuidle.c
> index f108d8f..bf68d49 100644
> --- a/arch/arm/kernel/cpuidle.c
> +++ b/arch/arm/kernel/cpuidle.c
> @@ -52,13 +52,9 @@ int arm_cpuidle_simple_enter(struct cpuidle_device *dev,
> */
> int arm_cpuidle_suspend(int index)
> {
> - int ret = -EOPNOTSUPP;
> int cpu = smp_processor_id();
>
> - if (cpuidle_ops[cpu].suspend)
> - ret = cpuidle_ops[cpu].suspend(index);
> -
> - return ret;
> + return cpuidle_ops[cpu].suspend(index);
> }
I agree with the optimization but, same comment than the previous patch,
it should be handled in arm_cpuidle_read_ops.
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web