Re: [PATCH V2 3/6] thermal: Add generic cpuhotplug cooling implementation
On 19 March 2012 17:15, Srivatsa S. Bhat
srivatsa.b...@linux.vnet.ibm.com wrote:
On 03/19/2012 11:47 AM, Amit Daniel Kachhap wrote:
This patch adds support for generic cpu thermal cooling low level
implementations using cpuhotplug based on the thermal level requested
from user. Different cpu related cooling devices can be registered by the
user and the binding of these cooling devices to the corresponding
trip points can be easily done as the registration APIs return the
cooling device pointer. The user of these APIs are responsible for
passing the cpumask.
I am really not aware of the cpu thermal cooling stuff, but since this patch
deals with CPU Hotplug (which I am interested in), I have some questions
below..
Signed-off-by: Amit Daniel Kachhap amit.kach...@linaro.org
+
+static int cpuhotplug_get_cur_state(struct thermal_cooling_device *cdev,
+ unsigned long *state)
+{
+ int ret = -EINVAL;
+ struct hotplug_cooling_device *hotplug_dev;
+
+ mutex_lock(cooling_cpuhotplug_lock);
+ list_for_each_entry(hotplug_dev, cooling_cpuhotplug_list, node) {
+ if (hotplug_dev hotplug_dev-cool_dev == cdev) {
+ *state = hotplug_dev-hotplug_state;
+ ret = 0;
+ break;
+ }
+ }
+ mutex_unlock(cooling_cpuhotplug_lock);
+ return ret;
+}
+
+/*This cooling may be as ACTIVE type*/
+static int cpuhotplug_set_cur_state(struct thermal_cooling_device *cdev,
+ unsigned long state)
+{
+ int cpuid, this_cpu = smp_processor_id();
What prevents this task from being migrated to another CPU?
IOW, what ensures that 'this_cpu' remains valid throughout this function?
I see that you are acquiring mutex locks below.. So this is definitely not
a preempt disabled section.. so I guess my question above is valid..
Or is this code bound to a particular cpu?
No this thread is not bound to a cpu. Your comment is valid and some
synchronization is needed for preemption. Thanks for pointing this
out.
+ struct hotplug_cooling_device *hotplug_dev;
+
+ mutex_lock(cooling_cpuhotplug_lock);
+ list_for_each_entry(hotplug_dev, cooling_cpuhotplug_list, node)
+ if (hotplug_dev hotplug_dev-cool_dev == cdev)
+ break;
+
+ mutex_unlock(cooling_cpuhotplug_lock);
+ if (!hotplug_dev || hotplug_dev-cool_dev != cdev)
+ return -EINVAL;
+
+ if (hotplug_dev-hotplug_state == state)
+ return 0;
+
+ /*
+ * This cooling device may be of type ACTIVE, so state field can
+ * be 0 or 1
+ */
+ if (state == 1) {
+ for_each_cpu(cpuid, hotplug_dev-allowed_cpus) {
+ if (cpu_online(cpuid) (cpuid != this_cpu))
What prevents the cpu numbered cpuid from being taken down right at this
moment?
Don't you need explicit synchronization with CPU Hotplug using something like
get_online_cpus()/put_online_cpus() here?
+ cpu_down(cpuid);
+ }
+ } else if (state == 0) {
+ for_each_cpu(cpuid, hotplug_dev-allowed_cpus) {
+ if (!cpu_online(cpuid) (cpuid != this_cpu))
Same here.
+ cpu_up(cpuid);
+ }
+ } else {
+ return -EINVAL;
+ }
+
+ hotplug_dev-hotplug_state = state;
+
+ return 0;
+}
+/* bind hotplug callbacks to cpu hotplug cooling device */
+static struct thermal_cooling_device_ops cpuhotplug_cooling_ops = {
+ .get_max_state = cpuhotplug_get_max_state,
+ .get_cur_state = cpuhotplug_get_cur_state,
+ .set_cur_state = cpuhotplug_set_cur_state,
+};
+
___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: LAVA Downtime Planned
Just a reminder to everyone that I will start decommissioning the toolchain server and lava cloud tomorrow morning, and then LAVA will go offline on Thursday morning. If all goes according to plan then a limited LAVA service should be available late on Friday, and I will work across the weekend to get everything else back up and online. We will be doing our best to keep the downtime to an absolute minimum, but the best laid plans can go awry. The major part that would push the availability out would be if our ISP do not keep to their commitment to get us connectivity by Friday. I will keep you informed as to progress during the move as and when possible. Many thanks for your patience, Dave Dave Pigott Validation Engineer T: +44 1223 45 00 24 | M +44 7940 45 93 44 Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog On 16 Mar 2012, at 02:50, Paul Larson wrote: This is just an early notification that the Linaro validation farm will be physically moving to a new site next week. Unfortunately, the network cables won't stretch that far, so it will mean some downtime. :) Here's the tentative plan: Wednesday, March 21st - the rack with the toolchain server will be shut down, crated and sent off Thursday, March 22nd - pretty much everything else will go offline (main LAVA server, development boards) Friday, March 23rd - physical move will happen, and if all goes well (what could possibly go wrong?!) the main site and as many of the boards as possible will go back online in their new home Saturday, March 24th to Sunday, March 25th - finish configuring and bringing up cloud services, toolchain server, etc. There's another bit of good news here, that the lab will be getting a much needed upgrade in the internet connection. A partial (but significant one) at first, and a bigger one in about a month or so with redundant connections. Bear in mind that this is just the tentative plan, Dave or I will send out a reminder and update as the date gets closer, or if something changes. Thanks, Paul Larson ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
ci.linaro.org planned downtime.
Hi, It is my intention to take ci.linaro.org offline for about 15 minutes at 13:45 UTC (http://www.worldtimeserver.com/current_time_in_UTC.aspx) so we can duplicate the server and start moving it to a larger EC2 instance. Users will know that ci.linaro.org has been running very slowly for a while and we hope that this move will make it usable while we continue to debug the performance problems that we are seeing. If the downtime will cause you significant problems, please get in touch and we will arrange a better time. Thanks, -- James Tunnicliffe ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Snowball V8 USB OTG broken
Hi, I started working on snowball v8 development board to investigate usb otg. My initial effort didn't gave me any result. I started investigating otg driver it seems that it starts in idle mode and then update its status to peripheral mode based on line status. Which in this case should go into host mode as per cable Mini-A. I manually put the driver into host mode and using debug messages in driver. It started to detect memory stick being plugged in and plugged out but it wasn't appearing as block device. As AB8500 interface chip documentation for usb is not in public domain makes it nearly impossible to debug the driver further. As usb otg is bus power so only low power device can be used. Thanks Adnan ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH v7 2/3] clk: introduce the common clock framework
On Thu, Mar 15, 2012 at 11:11:19PM -0700, Mike Turquette wrote:
...
+struct clk_ops {
+ int (*prepare)(struct clk_hw *hw);
+ void(*unprepare)(struct clk_hw *hw);
+ int (*enable)(struct clk_hw *hw);
+ void(*disable)(struct clk_hw *hw);
+ int (*is_enabled)(struct clk_hw *hw);
+ unsigned long (*recalc_rate)(struct clk_hw *hw,
+ unsigned long parent_rate);
I believe I have heard people love the interface with parent_rate
passed in. I love that too. But I would like to ask the same thing
on .round_rate and .set_rate as well for the same reason why we have
it for .recalc_rate.
+ long(*round_rate)(struct clk_hw *hw, unsigned long,
+ unsigned long *);
Yes, we already have parent_rate passed in .round_rate with an pointer.
But I think it'd be better to have it passed in no matter flag
CLK_SET_RATE_PARENT is set or not. Something like:
8---
@@ -584,7 +584,7 @@ EXPORT_SYMBOL_GPL(clk_get_rate);
*/
unsigned long __clk_round_rate(struct clk *clk, unsigned long rate)
{
- unsigned long unused;
+ unsigned long parent_rate = 0;
if (!clk)
return -EINVAL;
@@ -592,10 +592,10 @@ unsigned long __clk_round_rate(struct clk *clk, unsigned
long rate)
if (!clk-ops-round_rate)
return clk-rate;
- if (clk-flags CLK_SET_RATE_PARENT)
- return clk-ops-round_rate(clk-hw, rate, unused);
- else
- return clk-ops-round_rate(clk-hw, rate, NULL);
+ if (clk-parent)
+ parent_rate = clk-parent-rate;
+
+ return clk-ops-round_rate(clk-hw, rate, parent_rate);
}
/**
@@ -765,9 +765,12 @@ static void clk_calc_subtree(struct clk *clk, unsigned
long new_rate)
static struct clk *clk_calc_new_rates(struct clk *clk, unsigned long rate)
{
struct clk *top = clk;
- unsigned long best_parent_rate = clk-parent-rate;
+ unsigned long best_parent_rate = 0;
unsigned long new_rate;
+ if (clk-parent)
+ best_parent_rate = clk-parent-rate;
+
if (!clk-ops-round_rate !(clk-flags CLK_SET_RATE_PARENT)) {
clk-new_rate = clk-rate;
return NULL;
@@ -780,10 +783,7 @@ static struct clk *clk_calc_new_rates(struct clk *clk,
unsigned long rate)
goto out;
}
- if (clk-flags CLK_SET_RATE_PARENT)
- new_rate = clk-ops-round_rate(clk-hw, rate,
best_parent_rate);
- else
- new_rate = clk-ops-round_rate(clk-hw, rate, NULL);
+ new_rate = clk-ops-round_rate(clk-hw, rate, best_parent_rate);
if (best_parent_rate != clk-parent-rate) {
top = clk_calc_new_rates(clk-parent, best_parent_rate);
---8
The reason behind the change is that any clk implements .round_rate will
mostly need to get the parent rate, no matter flag CLK_SET_RATE_PARENT
is set or not. Instead of expecting every .round_rate implementation
to get the parent rate in the way beloe
parent_rate = __clk_get_rate(__clk_get_parent(hw-clk));
we can just pass the parent rate into .round_rate.
+ int (*set_parent)(struct clk_hw *hw, u8 index);
+ u8 (*get_parent)(struct clk_hw *hw);
+ int (*set_rate)(struct clk_hw *hw, unsigned long);
For the same reason, I would change .set_rate interface like below.
8---
diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c
index d5ac6a7..6bd8037 100644
--- a/drivers/clk/clk-divider.c
+++ b/drivers/clk/clk-divider.c
@@ -119,7 +119,8 @@ static long clk_divider_round_rate(struct clk_hw *hw,
unsigned long rate,
}
EXPORT_SYMBOL_GPL(clk_divider_round_rate);
-static int clk_divider_set_rate(struct clk_hw *hw, unsigned long rate)
+static int clk_divider_set_rate(struct clk_hw *hw, unsigned long rate,
+ unsigned long parent_rate)
{
struct clk_divider *divider = to_clk_divider(hw);
unsigned int div;
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index dbc310a..d74ac4b 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -833,17 +833,18 @@ static struct clk *clk_propagate_rate_change(struct clk
*clk, unsigned long even
static void clk_change_rate(struct clk *clk)
{
struct clk *child;
- unsigned long old_rate;
+ unsigned long old_rate, parent_rate = 0;
struct hlist_node *tmp;
old_rate = clk-rate;
+ if (clk-parent)
+ parent_rate = clk-parent-rate;
if (clk-ops-set_rate)
- clk-ops-set_rate(clk-hw, clk-new_rate);
+ clk-ops-set_rate(clk-hw, clk-new_rate, parent_rate);
if (clk-ops-recalc_rate)
- clk-rate = clk-ops-recalc_rate(clk-hw,
- clk-parent-rate);
+ clk-rate = clk-ops-recalc_rate(clk-hw, parent_rate);
Re: [PATCH v7 2/3] clk: introduce the common clock framework
On Tue, Mar 20, 2012 at 7:02 AM, Shawn Guo shawn@linaro.org wrote:
On Thu, Mar 15, 2012 at 11:11:19PM -0700, Mike Turquette wrote:
...
+struct clk_ops {
+ int (*prepare)(struct clk_hw *hw);
+ void (*unprepare)(struct clk_hw *hw);
+ int (*enable)(struct clk_hw *hw);
+ void (*disable)(struct clk_hw *hw);
+ int (*is_enabled)(struct clk_hw *hw);
+ unsigned long (*recalc_rate)(struct clk_hw *hw,
+ unsigned long parent_rate);
I believe I have heard people love the interface with parent_rate
passed in. I love that too. But I would like to ask the same thing
on .round_rate and .set_rate as well for the same reason why we have
it for .recalc_rate.
This is something that was discussed on the list but not taken in due
to rapid flux in code. I always liked the idea however.
+ long (*round_rate)(struct clk_hw *hw, unsigned long,
+ unsigned long *);
Yes, we already have parent_rate passed in .round_rate with an pointer.
But I think it'd be better to have it passed in no matter flag
CLK_SET_RATE_PARENT is set or not. Something like:
This places the burden of checking the flags onto the .round_rate
implementation with __clk_get_flags, but that's not a problem really.
8---
@@ -584,7 +584,7 @@ EXPORT_SYMBOL_GPL(clk_get_rate);
*/
unsigned long __clk_round_rate(struct clk *clk, unsigned long rate)
{
- unsigned long unused;
+ unsigned long parent_rate = 0;
if (!clk)
return -EINVAL;
@@ -592,10 +592,10 @@ unsigned long __clk_round_rate(struct clk *clk,
unsigned long rate)
if (!clk-ops-round_rate)
return clk-rate;
- if (clk-flags CLK_SET_RATE_PARENT)
- return clk-ops-round_rate(clk-hw, rate, unused);
- else
- return clk-ops-round_rate(clk-hw, rate, NULL);
+ if (clk-parent)
+ parent_rate = clk-parent-rate;
+
+ return clk-ops-round_rate(clk-hw, rate, parent_rate);
}
/**
@@ -765,9 +765,12 @@ static void clk_calc_subtree(struct clk *clk, unsigned
long new_rate)
static struct clk *clk_calc_new_rates(struct clk *clk, unsigned long rate)
{
struct clk *top = clk;
- unsigned long best_parent_rate = clk-parent-rate;
+ unsigned long best_parent_rate = 0;
unsigned long new_rate;
+ if (clk-parent)
+ best_parent_rate = clk-parent-rate;
+
if (!clk-ops-round_rate !(clk-flags CLK_SET_RATE_PARENT)) {
clk-new_rate = clk-rate;
return NULL;
@@ -780,10 +783,7 @@ static struct clk *clk_calc_new_rates(struct clk *clk,
unsigned long rate)
goto out;
}
- if (clk-flags CLK_SET_RATE_PARENT)
- new_rate = clk-ops-round_rate(clk-hw, rate,
best_parent_rate);
- else
- new_rate = clk-ops-round_rate(clk-hw, rate, NULL);
+ new_rate = clk-ops-round_rate(clk-hw, rate, best_parent_rate);
if (best_parent_rate != clk-parent-rate) {
top = clk_calc_new_rates(clk-parent, best_parent_rate);
---8
ACK
The reason behind the change is that any clk implements .round_rate will
mostly need to get the parent rate, no matter flag CLK_SET_RATE_PARENT
is set or not. Instead of expecting every .round_rate implementation
to get the parent rate in the way beloe
parent_rate = __clk_get_rate(__clk_get_parent(hw-clk));
we can just pass the parent rate into .round_rate.
+ int (*set_parent)(struct clk_hw *hw, u8 index);
+ u8 (*get_parent)(struct clk_hw *hw);
+ int (*set_rate)(struct clk_hw *hw, unsigned long);
For the same reason, I would change .set_rate interface like below.
8---
diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c
index d5ac6a7..6bd8037 100644
--- a/drivers/clk/clk-divider.c
+++ b/drivers/clk/clk-divider.c
@@ -119,7 +119,8 @@ static long clk_divider_round_rate(struct clk_hw *hw,
unsigned long rate,
}
EXPORT_SYMBOL_GPL(clk_divider_round_rate);
-static int clk_divider_set_rate(struct clk_hw *hw, unsigned long rate)
+static int clk_divider_set_rate(struct clk_hw *hw, unsigned long rate,
+ unsigned long parent_rate)
{
struct clk_divider *divider = to_clk_divider(hw);
unsigned int div;
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index dbc310a..d74ac4b 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -833,17 +833,18 @@ static struct clk *clk_propagate_rate_change(struct clk
*clk, unsigned long even
static void clk_change_rate(struct clk *clk)
{
struct clk *child;
- unsigned long old_rate;
+ unsigned long old_rate, parent_rate = 0;
struct hlist_node *tmp;
old_rate = clk-rate;
+ if (clk-parent)
Re: [PATCH v7 2/3] clk: introduce the common clock framework
On Tue, Mar 20, 2012 at 10:46 AM, Saravana Kannan
skan...@codeaurora.org wrote:
On Tue, March 20, 2012 7:02 am, Shawn Guo wrote:
On Thu, Mar 15, 2012 at 11:11:19PM -0700, Mike Turquette wrote:
...
+struct clk_ops {
+ int (*prepare)(struct clk_hw *hw);
+ void (*unprepare)(struct clk_hw *hw);
+ int (*enable)(struct clk_hw *hw);
+ void (*disable)(struct clk_hw *hw);
+ int (*is_enabled)(struct clk_hw *hw);
+ unsigned long (*recalc_rate)(struct clk_hw *hw,
+ unsigned long parent_rate);
I believe I have heard people love the interface with parent_rate
passed in. I love that too. But I would like to ask the same thing
on .round_rate and .set_rate as well for the same reason why we have
it for .recalc_rate.
In my case, for most clocks, set rate involves reparenting. So, what does
passing parent_rate for these even mean? Passing parent_rate seems more
apt for recalc_rate since it's called when the parent rate changes -- so,
the actual parent itself is not expected to change.
From my conversations with folks across many platforms, I think that
the way your clock tree expects to change rates is the exception, not
the rule. As such you should just ignore the parent_rate parameter as
it useless to you.
I could ignore the parameter, but just wondering how many of the others
see value in this. And if we do add this parameter, it shouldn't be made
mandatory for the platform driver to use it (due to other assumptions the
clock framework might make).
From my rough census of folks that actually need .set_rate support, I
think that everyone except MSM could benefit from this. Your concept
of clk_set_rate is everyone else's clk_set_parent.
Ignoring the new parameter should cause you no harm. It does make me
wonder if it would be a good idea to pass in the parent rate for
.set_parent, which is analogous to .set_rate in many ways.
Regards,
Mike
-Saravana
___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
Fwd: Re: Linux 2.6.35.3 Kernel for ARM and SATA problems
Forwarding a post from debian-arm. I think I may be affected by this bug. Could anyone help me check if this still applies to 3.1.1-1400-linaro-lt-mx5. Best regards ZK -- Wiadomość oryginalna -- Temat: Re: Linux 2.6.35.3 Kernel for ARM and SATA problems Odesłano-Data: Tue, 20 Mar 2012 01:25:53 + (UTC) Odesłano-Od:debian-...@lists.debian.org Data: Mon, 19 Mar 2012 18:25:03 -0700 Nadawca:Mike Thompson mpthomp...@gmail.com Adresat:Phil Endecott spam_from_debian_...@chezphil.org Kopia: debian-...@lists.debian.org On Sun, Mar 18, 2012 at 1:38 PM, Phil Endecott spam_from_debian_...@chezphil.org mailto:spam_from_debian_...@chezphil.org wrote: It works for me. I seem to recall a small amount of agro getting the kernel config right, but u-boot was always able to recognise the device. I suggest that you find (borrow) another SATA device to test with. Good luck! I was able to borrow another SATA drive and that works, thank heaven. I thought I was going crazy for a while -- don't ask how many times I re-compiled the kernel. I did trace the problems I having to the ahci code in the kernel not properly handling an ahci CONINIT event generated by my WD5000BEVT drive. Seems this drive has extra SATA features implemented so that it can be used in hot-plug arrays and these features aren't recognized by the kernel driver so it just seems to shut down the drive and ignore it. The other SATA drive that I do have working with the kernel doesn't implement the extra features so the kernel is happy. Presumably these problems were fixed in later kernels and the patches didn't make it into Freescales 2.6.35.3 branch. On the other hand, the kernel might be fine and the firmware in the drive isn't conforming to the ahci specs, but I think that wold cause problems with the drive on other systems. I'm going to keep looking into this as I do want to get my 500GB SATA drive working with the iMX53 Quick Start. Mike ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH v7 1/3] Documentation: common clk API
On Tue, 20 Mar 2012, Paul Walmsley wrote: We need to indicate in some way that the existing code and API is very likely to change in ways that could involve quite a bit of work for adopters. [...] Anyway. It is okay if we want to have some starter common clock framework in mainline; this is why deferring the merge hasn't been proposed. But the point is that anyone who bases their code or platform on the common clock framework needs to be aware that, to satisfy one of its major use-cases, the behavior and/or API of the common clock code may need to change significantly. Paul, While I understand your concern, please don't forget that the perfect is the enemy of the good. This common clk API has been under development for over *two* years already, with several attempts to merge it. And each previous merge attempt aborted because someone came along at the last minute to do exactly what you are doing i.e. underline all the flaws and call for a redesign. This is becoming a bad joke. We finally have something that the majority of reviewers are happy with and which should cover the majority of existing cases out there. Let's give it a chance, shall we? Otherwise one might ask where were you during those development years if you claim that the behavior and/or API of the common clock code still need to change significantly? Explicitly stating this is not only simple courtesy to its users, many of whom won't have been privy to its development. It also is intended to make the code easier to change once it reaches mainline. The code will be easier to change once it is in mainline, simply due to the fact that you can also change all its users at once. And it is well possible that most users won't have to deal with the same magnitude of complexity as yours, again reducing the scope for resistance to changes. Let's see how things go with the current code and improve it incrementally. Otherwise no one will get involved with this API which is almost just as bad as not having the code merged at all. So, until the API is well-defined and does all that it needs to do for its major users, [...] No, the API simply can't ever be well defined if people don't actually start using it to eventually refine it further. Major users were converted to it, and in most cases The API does all that it needs to do already. Otherwise you'll be stuck in a catch22 situation forever. And APIs in the Linux kernel do change all the time. There is no stable API in the kernel. Extensions will come about eventually, and existing users (simple ones by definition if the current API already meets their needs) should be converted over easily. Nicolas ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH v7 2/3] clk: introduce the common clock framework
On 21 March 2012 07:46, Turquette, Mike mturque...@ti.com wrote: ... As mentioned above, you'll still need to check for CLK_SET_RATE_PARENT in your .round_rate implementation with __clk_get_flags(hw-clk). For my particular case, the clk is PLL with fixed rate clk (oscillator) as parent. It's known that flag CLK_SET_RATE_PARENT will never be set for this type of clks. Did you want to send a formal patch or just have me absorb this into the fixes I'm brewing already? I've already fixed the potential null ptr dereferences in clk_calc_new_rates, but that's no big deal. The code was attached for better discussion, and I would leave the formal patch to you. Regards, Shawn ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
