All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
To: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: "rjw@rjwysocki.net" <rjw@rjwysocki.net>,
	"linux-pm@vger.kernel.org" <linux-pm@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Catalin Marinas <Catalin.Marinas@arm.com>,
	"robherring2@gmail.com" <robherring2@gmail.com>,
	"arnd@arndb.de" <arnd@arndb.de>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	"linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>,
	"lina.iyer@linaro.org" <lina.iyer@linaro.org>
Subject: Re: [PATCH V2 7/8] ARM: cpuidle: Register per cpuidle device
Date: Thu, 19 Mar 2015 11:38:24 +0000	[thread overview]
Message-ID: <20150319113824.GC4449@red-moon> (raw)
In-Reply-To: <550AB298.6040907@linaro.org>

On Thu, Mar 19, 2015 at 11:27:20AM +0000, Daniel Lezcano wrote:

[...]

> >>   	for_each_possible_cpu(cpu) {
> >>   		ret = arm_cpuidle_init(cpu);
> >> +
> >> +		/* This cpu does not support any idle states */
> >> +		if (ret == -ENOSYS)
> >> +			continue;
> >> +
> >>   		if (ret) {
> >>   			pr_err("CPU %d failed to init idle CPU ops\n", cpu);
> >> -			return ret;
> >> +			goto out_fail;
> >> +		}
> >> +
> >> +		dev = kzalloc(sizeof(*dev), GFP_KERNEL);
> >> +		if (!dev) {
> >> +			pr_err("Failed to allocate cpuidle device\n");
> >> +			goto out_fail;
> >> +		}
> >> +
> >> +		dev->cpu = cpu;
> >> +		per_cpu(cpuidle_arm_dev, cpu) = dev;
> >> +
> >> +		ret = cpuidle_register_device(dev);
> >> +		if (ret) {
> >> +			pr_err("Failed to register cpuidle device for CPU %d\n",
> >> +			       cpu);
> >> +			kfree(dev);
> >> +			goto out_fail;
> >>   		}
> >>   	}
> >> +out:
> >> +	return ret;
> >
> > return 0;
> >
> >>
> >> -	return cpuidle_register(drv, NULL);
> >> +out_fail:
> >> +	for (cpu--; cpu <= 0; cpu--) {
> >
> > This loop is wrong.
> 
> Why is it wrong ? We have to initialize at cpu - 1 to unregister the 
> previous registered cpu, not the current one, no ?

Yes, but on cpu>=0 not cpu<=0

while (--cpu >= 0)

?

Lorenzo

WARNING: multiple messages have this Message-ID (diff)
From: Lorenzo Pieralisi <lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org>
To: Daniel Lezcano <daniel.lezcano-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Cc: "rjw-LthD3rsA81gm4RdzfppkhA@public.gmane.org"
	<rjw-LthD3rsA81gm4RdzfppkhA@public.gmane.org>,
	"linux-pm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
	<linux-pm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	"linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
	<linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	Catalin Marinas <Catalin.Marinas-5wv7dgnIgG8@public.gmane.org>,
	"robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org"
	<robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	"arnd-r2nGTMty4D4@public.gmane.org"
	<arnd-r2nGTMty4D4@public.gmane.org>,
	"devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
	<devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	"linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org"
	<linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org>,
	"lina.iyer-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org"
	<lina.iyer-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Subject: Re: [PATCH V2 7/8] ARM: cpuidle: Register per cpuidle device
Date: Thu, 19 Mar 2015 11:38:24 +0000	[thread overview]
Message-ID: <20150319113824.GC4449@red-moon> (raw)
In-Reply-To: <550AB298.6040907-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

On Thu, Mar 19, 2015 at 11:27:20AM +0000, Daniel Lezcano wrote:

[...]

> >>   	for_each_possible_cpu(cpu) {
> >>   		ret = arm_cpuidle_init(cpu);
> >> +
> >> +		/* This cpu does not support any idle states */
> >> +		if (ret == -ENOSYS)
> >> +			continue;
> >> +
> >>   		if (ret) {
> >>   			pr_err("CPU %d failed to init idle CPU ops\n", cpu);
> >> -			return ret;
> >> +			goto out_fail;
> >> +		}
> >> +
> >> +		dev = kzalloc(sizeof(*dev), GFP_KERNEL);
> >> +		if (!dev) {
> >> +			pr_err("Failed to allocate cpuidle device\n");
> >> +			goto out_fail;
> >> +		}
> >> +
> >> +		dev->cpu = cpu;
> >> +		per_cpu(cpuidle_arm_dev, cpu) = dev;
> >> +
> >> +		ret = cpuidle_register_device(dev);
> >> +		if (ret) {
> >> +			pr_err("Failed to register cpuidle device for CPU %d\n",
> >> +			       cpu);
> >> +			kfree(dev);
> >> +			goto out_fail;
> >>   		}
> >>   	}
> >> +out:
> >> +	return ret;
> >
> > return 0;
> >
> >>
> >> -	return cpuidle_register(drv, NULL);
> >> +out_fail:
> >> +	for (cpu--; cpu <= 0; cpu--) {
> >
> > This loop is wrong.
> 
> Why is it wrong ? We have to initialize at cpu - 1 to unregister the 
> previous registered cpu, not the current one, no ?

Yes, but on cpu>=0 not cpu<=0

while (--cpu >= 0)

?

Lorenzo
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

WARNING: multiple messages have this Message-ID (diff)
From: lorenzo.pieralisi@arm.com (Lorenzo Pieralisi)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH V2 7/8] ARM: cpuidle: Register per cpuidle device
Date: Thu, 19 Mar 2015 11:38:24 +0000	[thread overview]
Message-ID: <20150319113824.GC4449@red-moon> (raw)
In-Reply-To: <550AB298.6040907@linaro.org>

On Thu, Mar 19, 2015 at 11:27:20AM +0000, Daniel Lezcano wrote:

[...]

> >>   	for_each_possible_cpu(cpu) {
> >>   		ret = arm_cpuidle_init(cpu);
> >> +
> >> +		/* This cpu does not support any idle states */
> >> +		if (ret == -ENOSYS)
> >> +			continue;
> >> +
> >>   		if (ret) {
> >>   			pr_err("CPU %d failed to init idle CPU ops\n", cpu);
> >> -			return ret;
> >> +			goto out_fail;
> >> +		}
> >> +
> >> +		dev = kzalloc(sizeof(*dev), GFP_KERNEL);
> >> +		if (!dev) {
> >> +			pr_err("Failed to allocate cpuidle device\n");
> >> +			goto out_fail;
> >> +		}
> >> +
> >> +		dev->cpu = cpu;
> >> +		per_cpu(cpuidle_arm_dev, cpu) = dev;
> >> +
> >> +		ret = cpuidle_register_device(dev);
> >> +		if (ret) {
> >> +			pr_err("Failed to register cpuidle device for CPU %d\n",
> >> +			       cpu);
> >> +			kfree(dev);
> >> +			goto out_fail;
> >>   		}
> >>   	}
> >> +out:
> >> +	return ret;
> >
> > return 0;
> >
> >>
> >> -	return cpuidle_register(drv, NULL);
> >> +out_fail:
> >> +	for (cpu--; cpu <= 0; cpu--) {
> >
> > This loop is wrong.
> 
> Why is it wrong ? We have to initialize at cpu - 1 to unregister the 
> previous registered cpu, not the current one, no ?

Yes, but on cpu>=0 not cpu<=0

while (--cpu >= 0)

?

Lorenzo

  reply	other threads:[~2015-03-19 11:38 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-03-18 18:46 [PATCH V2 0/8] ARM: cpuidle: Unify the ARM64/ARM DT approach Daniel Lezcano
2015-03-18 18:46 ` Daniel Lezcano
2015-03-18 18:46 ` [PATCH V2 1/8] ARM: cpuidle: Remove duplicate header inclusion Daniel Lezcano
2015-03-18 18:46   ` Daniel Lezcano
2015-03-18 18:46   ` Daniel Lezcano
2015-03-18 18:46 ` [PATCH V2 2/8] ARM: cpuidle: Add a cpuidle ops structure to be used for DT Daniel Lezcano
2015-03-18 18:46   ` Daniel Lezcano
2015-03-18 18:46 ` [PATCH V2 3/8] ARM64: cpuidle: Replace cpu_suspend by the common ARM/ARM64 function Daniel Lezcano
2015-03-18 18:46   ` Daniel Lezcano
2015-03-21 20:56   ` Lorenzo Pieralisi
2015-03-21 20:56     ` Lorenzo Pieralisi
2015-03-18 18:46 ` [PATCH V2 4/8] ARM64: cpuidle: Rename cpu_init_idle to a common function name Daniel Lezcano
2015-03-18 18:46   ` Daniel Lezcano
2015-03-18 18:46 ` [PATCH V2 5/8] ARM64: cpuidle: Remove arm64 reference Daniel Lezcano
2015-03-18 18:46   ` Daniel Lezcano
2015-03-18 18:46 ` [PATCH V2 6/8] ARM: cpuidle: Enable the ARM64 driver for both ARM32/ARM64 Daniel Lezcano
2015-03-18 18:46   ` Daniel Lezcano
2015-03-18 18:46 ` [PATCH V2 7/8] ARM: cpuidle: Register per cpuidle device Daniel Lezcano
2015-03-18 18:46   ` Daniel Lezcano
2015-03-19 11:08   ` Lorenzo Pieralisi
2015-03-19 11:08     ` Lorenzo Pieralisi
2015-03-19 11:27     ` Daniel Lezcano
2015-03-19 11:27       ` Daniel Lezcano
2015-03-19 11:38       ` Lorenzo Pieralisi [this message]
2015-03-19 11:38         ` Lorenzo Pieralisi
2015-03-19 11:38         ` Lorenzo Pieralisi
2015-03-19 11:44         ` Daniel Lezcano
2015-03-19 11:44           ` Daniel Lezcano
2015-03-19 15:31   ` Lina Iyer
2015-03-19 15:31     ` Lina Iyer
2015-03-19 15:33     ` Daniel Lezcano
2015-03-19 15:33       ` Daniel Lezcano
2015-03-19 15:33       ` Daniel Lezcano
2015-03-19 15:42       ` Lina Iyer
2015-03-19 15:42         ` Lina Iyer
2015-03-19 17:11         ` Daniel Lezcano
2015-03-19 17:11           ` Daniel Lezcano
2015-03-18 18:46 ` [PATCH V2 8/8] ARM: cpuidle: Change function name to be consistent with x86 Daniel Lezcano
2015-03-18 18:46   ` Daniel Lezcano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150319113824.GC4449@red-moon \
    --to=lorenzo.pieralisi@arm.com \
    --cc=Catalin.Marinas@arm.com \
    --cc=arnd@arndb.de \
    --cc=daniel.lezcano@linaro.org \
    --cc=devicetree@vger.kernel.org \
    --cc=lina.iyer@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=rjw@rjwysocki.net \
    --cc=robherring2@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.