Re: [PATCH v3 1/3] devfreq: Core updates to support devices which can idle
Prepare devfreq core framework to support devices which can idle. When device idleness is detected perhaps through runtime-pm, need some mechanism to suspend devfreq load monitoring and resume back when device is online. Present code continues monitoring unless device is removed from devfreq core. This patch introduces following design changes, - use per device work instead of global work to monitor device load. This enables suspend/resume of device devfreq and reduces monitoring code complexity. - decouple delayed work based load monitoring logic from core by introducing helpers functions to be used by governors. This provides flexibility for governors either to use delayed work based monitoring functions or to implement their own mechanism. - devfreq core interacts with governors via events to perform specific actions. These events include start/stop devfreq. This sets ground for adding suspend/resume events. The devfreq apis are not modified and are kept intact. Signed-off-by: Rajagopal Venkat rajagopal.ven...@linaro.org Hello, I'll do more through review tomorrow (sorry, I was occuppied by something other than Linux tasks for a while again); however, there are two concerns in this patch. 1. (minor but may bothersome in some rare but not-ignorable cases) Serialization issue between suspend/resume functions; this may happen with some failure or interrupts while entering STR or unexpected usage of the API at drivers. For example, if devfreq_monitor_suspend() and devfreq_montir_resume() are called almost simultaneously, we may execute 1) locked part of suspend, 2) locked part of resume, 3) cancel_delayed_work_sync of suspend. Then, we may have stop_polling = false w/ cancel_delayed_work_sync() in effect. Let's not assume that suspend() and resume() may called almost simultaneously, especially in subsystem core code. 2. What if polling_ms = 0 w/ active governors (such as ondemand)? Users may declare the initial polling_ms = 0 w/ simple-ondemand in order to pause sampling at boot-time and start sampling at run-time some time later. It appears that this patch will start forcibly at boot-time in such a case. --- Documentation/ABI/testing/sysfs-class-devfreq | 8 - drivers/devfreq/devfreq.c | 431 +++--- drivers/devfreq/governor.h| 11 + drivers/devfreq/governor_performance.c| 16 +- drivers/devfreq/governor_powersave.c | 16 +- drivers/devfreq/governor_simpleondemand.c | 24 ++ drivers/devfreq/governor_userspace.c | 23 +- include/linux/devfreq.h | 34 +- 8 files changed, 267 insertions(+), 296 deletions(-) diff --git a/Documentation/ABI/testing/sysfs-class-devfreq b/Documentation/ABI/testing/sysfs-class-devfreq index 23d78b5..89283b1 100644 --- a/Documentation/ABI/testing/sysfs-class-devfreq +++ b/Documentation/ABI/testing/sysfs-class-devfreq @@ -21,14 +21,6 @@ Description: The /sys/class/devfreq/.../cur_freq shows the current frequency of the corresponding devfreq object. -What:/sys/class/devfreq/.../central_polling -Date:September 2011 -Contact: MyungJoo Ham myungjoo@samsung.com -Description: - The /sys/class/devfreq/.../central_polling shows whether - the devfreq ojbect is using devfreq-provided central - polling mechanism or not. - What:/sys/class/devfreq/.../polling_interval Date:September 2011 Contact: MyungJoo Ham myungjoo@samsung.com diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c index b146d76..8e9b5aa 100644 --- a/drivers/devfreq/devfreq.c +++ b/drivers/devfreq/devfreq.c @@ -30,17 +30,11 @@ struct class *devfreq_class; /* - * devfreq_work periodically monitors every registered device. - * The minimum polling interval is one jiffy. The polling interval is - * determined by the minimum polling period among all polling devfreq - * devices. The resolution of polling interval is one jiffy. + * devfreq core provides delayed work based load monitoring helper + * functions. Governors can use these or can implement their own + * monitoring mechanism. */ -static bool polling; static struct workqueue_struct *devfreq_wq; -static struct delayed_work devfreq_work; - -/* wait removing if this is to be removed */ -static struct devfreq *wait_remove_device; /* The list of all device-devfreq */ static LIST_HEAD(devfreq_list); @@ -72,6 +66,8 @@ static struct devfreq *find_device_devfreq(struct device *dev) return ERR_PTR(-ENODEV); } +/* Load monitoring helper functions for governors use */ + /** * update_devfreq() - Reevaluate the device and configure frequency. * @devfreq: the devfreq instance. @@ -121,6 +117,140 @@ int update_devfreq(struct devfreq *devfreq) } /** +
[PATCH V2 1/3] sched: Create sched_select_cpu() to give preferred CPU for power saving
In order to save power, it would be useful to schedule work onto non-IDLE cpus instead of waking up an IDLE one. To achieve this, we need scheduler to guide kernel frameworks (like: timers workqueues) on which is the most preferred CPU that must be used for these tasks. This routine returns the preferred cpu which is non-idle. It accepts a bitwise OR of SD_* flags present in linux/sched.h. If the local CPU isn't idle, it is returned back. If it is idle, then we must look for another CPU which have all the flags passed as argument as set. Also, as this activity is part of load balancing only, SD_LOAD_BALANCE must also be set for selected domain. This patch reuses the code from get_nohz_timer_target() routine, which had similar implementation. get_nohz_timer_target() is also modified to use sched_select_cpu() now. Signed-off-by: Viresh Kumar viresh.ku...@linaro.org --- include/linux/sched.h | 16 ++-- kernel/sched/core.c | 69 +++ 2 files changed, 56 insertions(+), 29 deletions(-) diff --git a/include/linux/sched.h b/include/linux/sched.h index 0059212..d2722cf 100644 --- a/include/linux/sched.h +++ b/include/linux/sched.h @@ -272,14 +272,26 @@ extern void init_idle_bootup_task(struct task_struct *idle); extern int runqueue_is_locked(int cpu); -#if defined(CONFIG_SMP) defined(CONFIG_NO_HZ) +#ifdef CONFIG_SMP +extern int sched_select_cpu(unsigned int sd_flags); + +#ifdef CONFIG_NO_HZ extern void select_nohz_load_balancer(int stop_tick); extern void set_cpu_sd_state_idle(void); -extern int get_nohz_timer_target(void); +/* + * In the semi idle case, use the nearest busy cpu for migrating timers + * from an idle cpu. This is good for power-savings. + * + * We don't do similar optimization for completely idle system, as + * selecting an idle cpu will add more delays to the timers than intended + * (as that cpu's timer base may not be uptodate wrt jiffies etc). + */ +#define get_nohz_timer_target() sched_select_cpu(0) #else static inline void select_nohz_load_balancer(int stop_tick) { } static inline void set_cpu_sd_state_idle(void) { } #endif +#endif /* CONFIG_SMP */ /* * Only dump TASK_* tasks. (0 for all tasks) diff --git a/kernel/sched/core.c b/kernel/sched/core.c index de97083..d573183 100644 --- a/kernel/sched/core.c +++ b/kernel/sched/core.c @@ -542,33 +542,6 @@ void resched_cpu(int cpu) #ifdef CONFIG_NO_HZ /* - * In the semi idle case, use the nearest busy cpu for migrating timers - * from an idle cpu. This is good for power-savings. - * - * We don't do similar optimization for completely idle system, as - * selecting an idle cpu will add more delays to the timers than intended - * (as that cpu's timer base may not be uptodate wrt jiffies etc). - */ -int get_nohz_timer_target(void) -{ - int cpu = smp_processor_id(); - int i; - struct sched_domain *sd; - - rcu_read_lock(); - for_each_domain(cpu, sd) { - for_each_cpu(i, sched_domain_span(sd)) { - if (!idle_cpu(i)) { - cpu = i; - goto unlock; - } - } - } -unlock: - rcu_read_unlock(); - return cpu; -} -/* * When add_timer_on() enqueues a timer into the timer wheel of an * idle CPU then this timer might expire before the next timer event * which is scheduled to wake up that CPU. In case of a completely @@ -639,6 +612,48 @@ void sched_avg_update(struct rq *rq) } } +/* + * This routine returns the preferred cpu which is non-idle. It accepts a + * bitwise OR of SD_* flags present in linux/sched.h. If the local CPU isn't + * idle, it is returned back. If it is idle, then we must look for another CPU + * which have all the flags passed as argument as set. Also, as this activity is + * part of load balancing only, SD_LOAD_BALANCE must also be set for selected + * domain. + */ +int sched_select_cpu(unsigned int sd_flags) +{ + struct sched_domain *sd; + int cpu = smp_processor_id(); + int i; + + /* If Current cpu isn't idle, don't migrate anything */ + if (!idle_cpu(cpu)) + return cpu; + + /* Add SD_LOAD_BALANCE to flags */ + sd_flags |= SD_LOAD_BALANCE; + + rcu_read_lock(); + for_each_domain(cpu, sd) { + /* +* If sd doesnt' have both sd_flags and SD_LOAD_BALANCE set, +* skip sd. +*/ + if ((sd-flags sd_flags) != sd_flags) + continue; + + for_each_cpu(i, sched_domain_span(sd)) { + if (!idle_cpu(i)) { + cpu = i; + goto unlock; + } + } + } +unlock: + rcu_read_unlock(); + return cpu; +} + #else /* !CONFIG_SMP */ void resched_task(struct task_struct *p) { --
[PATCH V2 2/3] timer: hrtimer: Don't check idle_cpu() before calling get_nohz_timer_target()
Check for current cpu's idleness already done in implementation of sched_select_cpu() which is called by get_nohz_timer_target(). So, no need to call idle_cpu() twice, once from sched_select_cpu() and once from timer and hrtimer before calling get_nohz_timer_target(). This patch removes calls to idle_cpu() from timer and hrtimer. Signed-off-by: Viresh Kumar viresh.ku...@linaro.org --- kernel/hrtimer.c | 2 +- kernel/timer.c | 9 + 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c index 6db7a5e..74bdaf6 100644 --- a/kernel/hrtimer.c +++ b/kernel/hrtimer.c @@ -159,7 +159,7 @@ struct hrtimer_clock_base *lock_hrtimer_base(const struct hrtimer *timer, static int hrtimer_get_target(int this_cpu, int pinned) { #ifdef CONFIG_NO_HZ - if (!pinned get_sysctl_timer_migration() idle_cpu(this_cpu)) + if (!pinned get_sysctl_timer_migration()) return get_nohz_timer_target(); #endif return this_cpu; diff --git a/kernel/timer.c b/kernel/timer.c index d5de1b2..8409d9d 100644 --- a/kernel/timer.c +++ b/kernel/timer.c @@ -720,7 +720,7 @@ __mod_timer(struct timer_list *timer, unsigned long expires, { struct tvec_base *base, *new_base; unsigned long flags; - int ret = 0 , cpu; + int ret = 0 , cpu = 0; timer_stats_timer_set_start_info(timer); BUG_ON(!timer-function); @@ -733,12 +733,13 @@ __mod_timer(struct timer_list *timer, unsigned long expires, debug_activate(timer, expires); - cpu = smp_processor_id(); - #if defined(CONFIG_NO_HZ) defined(CONFIG_SMP) - if (!pinned get_sysctl_timer_migration() idle_cpu(cpu)) + if (!pinned get_sysctl_timer_migration()) cpu = get_nohz_timer_target(); #endif + if (!cpu) + cpu = smp_processor_id(); + new_base = per_cpu(tvec_bases, cpu); if (base != new_base) { -- 1.7.12.rc2.18.g61b472e ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
[PATCH V2 0/3] Create sched_select_cpu() and use it in workqueues
Hi All, This is V2 of my sched_select_cpu() work. In order to save power, it would be useful to schedule work onto non-IDLE cpus instead of waking up an IDLE one. To achieve this, we need scheduler to guide kernel frameworks (like: timers workqueues) on which is the most preferred CPU that must be used for these tasks. This patchset is about implementing this concept. - The first patch adds sched_select_cpu() routine which returns the preferred cpu which is non-idle. - Second patch removes idle_cpu() calls from timer hrtimer. - Third patch is about adapting this change in workqueue framework. Earlier discussions over v1 can be found here: http://www.mail-archive.com/linaro-dev@lists.linaro.org/msg13342.html Earlier discussions over this concept were done at last LPC: http://summit.linuxplumbersconf.org/lpc-2012/meeting/90/lpc2012-sched-timer-workqueue/ Module created for testing this behavior is present here: http://git.linaro.org/gitweb?p=people/vireshk/module.git;a=summary Following are the steps followed in test module: 1. Run single work on each cpu 2. This work will start a timer after x (tested with 10) jiffies of delay 3. Timer routine queues a work... (This may be called from idle or non-idle cpu) and starts the same timer again STEP 3 is done for n number of times (i.e. queuing n works, one after other) 4. All works will call a single routine, which will count following per cpu: - Total works processed by a CPU - Total works processed by a CPU, which are queued from it - Total works processed by a CPU, which aren't queued from it Setup: - - ARM Vexpress TC2 - big.LITTLE CPU - Core 0-1: A15, 2-4: A7 - rootfs: linaro-ubuntu-nano Results: --- Without Workqueue Modification, i.e. PATCH 3/3: [ 2493.022335] Workqueue Analyser: works processsed by CPU0, Total: 1000, Own: 0, migrated: 0 [ 2493.047789] Workqueue Analyser: works processsed by CPU1, Total: 1000, Own: 0, migrated: 0 [ 2493.072918] Workqueue Analyser: works processsed by CPU2, Total: 1000, Own: 0, migrated: 0 [ 2493.098576] Workqueue Analyser: works processsed by CPU3, Total: 1000, Own: 0, migrated: 0 [ 2493.123702] Workqueue Analyser: works processsed by CPU4, Total: 1000, Own: 0, migrated: 0 With Workqueue Modification, i.e. PATCH 3/3: [ 2493.022335] Workqueue Analyser: works processsed by CPU0, Total: 1002, Own: 999, migrated: 3 [ 2493.047789] Workqueue Analyser: works processsed by CPU1, Total: 998, Own: 997, migrated: 1 [ 2493.072918] Workqueue Analyser: works processsed by CPU2, Total: 1013, Own: 996, migrated: 17 [ 2493.098576] Workqueue Analyser: works processsed by CPU3, Total: 998, Own: 993, migrated: 5 [ 2493.123702] Workqueue Analyser: works processsed by CPU4, Total: 989, Own: 987, migrated: 2 V1-V2 - - New SD_* macros removed now and earlier ones used - sched_select_cpu() rewritten and it includes the check on current cpu's idleness. - cpu_idle() calls from timer and hrtimer removed now. - Patch 2/3 from V1, removed as it doesn't apply to latest workqueue branch from tejun. - CONFIG_MIGRATE_WQ removed and so is wq_select_cpu() - sched_select_cpu() called only from __queue_work() - got tejun/for-3.7 branch in my tree, before making workqueue changes. Viresh Kumar (3): sched: Create sched_select_cpu() to give preferred CPU for power saving timer: hrtimer: Don't check idle_cpu() before calling get_nohz_timer_target() workqueue: Schedule work on non-idle cpu instead of current one include/linux/sched.h | 16 ++-- kernel/hrtimer.c | 2 +- kernel/sched/core.c | 69 +++ kernel/timer.c| 9 --- kernel/workqueue.c| 2 +- 5 files changed, 63 insertions(+), 35 deletions(-) -- 1.7.12.rc2.18.g61b472e ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
[PATCH V2 3/3] workqueue: Schedule work on non-idle cpu instead of current one
Workqueues queues work on current cpu, if the caller haven't passed a preferred cpu. This may wake up an idle CPU, which is actually not required. This work can be processed by any CPU and so we must select a non-idle CPU here. This patch adds in support in workqueue framework to get preferred CPU details from the scheduler, instead of using current CPU. Most of the time when a work is queued, the current cpu isn't idle and so we will choose it only. There are cases when a cpu is idle when it queues some work. For example, consider following scenario: - A cpu has programmed a timer and is IDLE now. - CPU gets into interrupt handler due to timer and queues a work. As the CPU is currently IDLE, we can queue this work to some other CPU. Signed-off-by: Viresh Kumar viresh.ku...@linaro.org --- kernel/workqueue.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kernel/workqueue.c b/kernel/workqueue.c index 143fd8c..5fa4ba4 100644 --- a/kernel/workqueue.c +++ b/kernel/workqueue.c @@ -1238,7 +1238,7 @@ static void __queue_work(unsigned int cpu, struct workqueue_struct *wq, struct global_cwq *last_gcwq; if (cpu == WORK_CPU_UNBOUND) - cpu = raw_smp_processor_id(); + cpu = sched_select_cpu(0); /* * It's multi cpu. If @work was previously on a different -- 1.7.12.rc2.18.g61b472e ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [RFC PATCH v8] media: add v4l2 subdev driver for S5K4ECGX sensor
Hi Sylwester, On Thursday 27 September 2012 00:00:20 Sylwester Nawrocki wrote: On 09/26/2012 10:32 PM, Mauro Carvalho Chehab wrote: Em Thu, 13 Sep 2012 12:02:14 +0100 Sangwook Lee escreveu: This patch adds driver for S5K4ECGX sensor with embedded ISP SoC, S5K4ECGX, which is a 5M CMOS Image sensor from Samsung The driver implements preview mode of the S5K4ECGX sensor. capture (snapshot) operation, face detection are missing now. Following controls are supported: contrast/saturation/brightness/sharpness Signed-off-by: Sangwook Leesangwook@linaro.org Reviewed-by: Sylwester Nawrockis.nawro...@samsung.com Cc: Francesco Lavrafrancescolavra...@gmail.com Cc: Scott Bambroughscott.bambro...@linaro.org Cc: Homin Leesuap...@insignal.co.kr ... +static int s5k4ecgx_load_firmware(struct v4l2_subdev *sd) +{ + const struct firmware *fw; + const u8 *ptr; + int err, i, regs_num; + struct i2c_client *client = v4l2_get_subdevdata(sd); + u16 val; + u32 addr, crc, crc_file, addr_inc = 0; + + err = request_firmware(fw, S5K4ECGX_FIRMWARE, sd-v4l2_dev-dev); The patch looks correct on my eyes... Yet, calling request_firmware() might not be the right thing to do. The thing is that newer versions of udev refuse to load firmware synchronously during probe/init time. As this function is actually called by s_power, maybe this driver doesn't suffer from that new udev behavior, so, I'll be merging it as-is. However, I suggest you to take a deeper review on that and, if possible, test it with the latest udev. True, it's indeed a bit tricky. The host interface driver this sensor driver has been tested with calls s_power on a subdev only in response to a video device open(). During probe only subdev's .registered() callback is called. And there is no request_firmware() there, as it is not needed to boot the sensor's MCU. The firmware is really just a set of settings that are normally needed only before streaming needs to be started. That triggers a red flag warning :-) What kind of settings do you have in the firmware ? We have analysed this issue and hopefully there should be no request_firmware() calls from within probe(), as long as the host interface driver doesn't call s_power from it's probe() callback. I'm afraid it's not the case for all bridge drivers though. If needed we could probably try and use request_firmware_nowait(), but it seems better to me to make sure the bridge drivers don't call s_power from within their probe() callback. -- Regards, Laurent Pinchart ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH V2 2/3] timer: hrtimer: Don't check idle_cpu() before calling get_nohz_timer_target()
On 27 September 2012 14:34, Viresh Kumar viresh.ku...@linaro.org wrote: #if defined(CONFIG_NO_HZ) defined(CONFIG_SMP) - if (!pinned get_sysctl_timer_migration() idle_cpu(cpu)) + if (!pinned get_sysctl_timer_migration()) cpu = get_nohz_timer_target(); #endif + if (!cpu) + cpu = smp_processor_id(); Ahh... Bad change... cpu can be returned as zero by get_nohz_timer_target(). Please consider below patch here: 8- Subject: [PATCH] timer: hrtimer: Don't check idle_cpu() before calling get_nohz_timer_target() Check for current cpu's idleness already done in implementation of sched_select_cpu() which is called by get_nohz_timer_target(). So, no need to call idle_cpu() twice, once from sched_select_cpu() and once from timer and hrtimer before calling get_nohz_timer_target(). This patch removes calls to idle_cpu() from timer and hrtimer. This also reduces an extra call to smp_processor_id() when get_nohz_timer_target() is called. Signed-off-by: Viresh Kumar viresh.ku...@linaro.org --- kernel/hrtimer.c | 2 +- kernel/timer.c | 8 +--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c index 6db7a5e..74bdaf6 100644 --- a/kernel/hrtimer.c +++ b/kernel/hrtimer.c @@ -159,7 +159,7 @@ struct hrtimer_clock_base *lock_hrtimer_base(const struct hrtimer *timer, static int hrtimer_get_target(int this_cpu, int pinned) { #ifdef CONFIG_NO_HZ - if (!pinned get_sysctl_timer_migration() idle_cpu(this_cpu)) + if (!pinned get_sysctl_timer_migration()) return get_nohz_timer_target(); #endif return this_cpu; diff --git a/kernel/timer.c b/kernel/timer.c index d5de1b2..db57606 100644 --- a/kernel/timer.c +++ b/kernel/timer.c @@ -733,11 +733,13 @@ __mod_timer(struct timer_list *timer, unsigned long expires, debug_activate(timer, expires); - cpu = smp_processor_id(); - #if defined(CONFIG_NO_HZ) defined(CONFIG_SMP) - if (!pinned get_sysctl_timer_migration() idle_cpu(cpu)) + if (!pinned get_sysctl_timer_migration()) cpu = get_nohz_timer_target(); + else + cpu = smp_processor_id(); +#else + cpu = smp_processor_id(); #endif new_base = per_cpu(tvec_bases, cpu); ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH v3 1/3] devfreq: Core updates to support devices which can idle
On 27 September 2012 13:50, MyungJoo Ham myungjoo@samsung.com wrote: Prepare devfreq core framework to support devices which can idle. When device idleness is detected perhaps through runtime-pm, need some mechanism to suspend devfreq load monitoring and resume back when device is online. Present code continues monitoring unless device is removed from devfreq core. This patch introduces following design changes, - use per device work instead of global work to monitor device load. This enables suspend/resume of device devfreq and reduces monitoring code complexity. - decouple delayed work based load monitoring logic from core by introducing helpers functions to be used by governors. This provides flexibility for governors either to use delayed work based monitoring functions or to implement their own mechanism. - devfreq core interacts with governors via events to perform specific actions. These events include start/stop devfreq. This sets ground for adding suspend/resume events. The devfreq apis are not modified and are kept intact. Signed-off-by: Rajagopal Venkat rajagopal.ven...@linaro.org Hello, I'll do more through review tomorrow (sorry, I was occuppied by something other than Linux tasks for a while again); however, there are two concerns in this patch. 1. (minor but may bothersome in some rare but not-ignorable cases) Serialization issue between suspend/resume functions; this may happen with some failure or interrupts while entering STR or unexpected usage of the API at drivers. Regarding the invalid usage of suspend/resume apis, we can have additional checks something like, void devfreq_monitor_suspend(struct devfreq *devfreq) { . if (devfreq-stop_polling) return; .. } void devfreq_monitor_resume(struct devfreq *devfreq) { . if (!devfreq-stop_polling) return; .. } For example, if devfreq_monitor_suspend() and devfreq_montir_resume() are called almost simultaneously, we may execute 1) locked part of suspend, 2) locked part of resume, 3) cancel_delayed_work_sync of suspend. Then, we may have stop_polling = false w/ cancel_delayed_work_sync() in effect. Let's not assume that suspend() and resume() may called almost simultaneously, especially in subsystem core code. These devfreq_monitor_suspend() and devfreq_monitor_resume() functions are executed when device idleness is detected. Perhaps, - using runtime-pm: the runtime_suspend() and runtime_resume() are mutually exclusive and is guaranteed not to run in parallel. - driver may have its own mechanism: in my opinion, driver should ensure suspend/resume are sequential even for it to know its devfreq status. Assuming even if above sequence occurs, I don't see any problem having stop_polling = false w/ cancel_delayed_work_sync() in effect. Since the suspend is the last one to complete, monitoring will not continue. 2. What if polling_ms = 0 w/ active governors (such as ondemand)? Users may declare the initial polling_ms = 0 w/ simple-ondemand in order to pause sampling at boot-time and start sampling at run-time some time later. It appears that this patch will start forcibly at boot-time in such a case. Yes. This is a valid case which can be handled by void devfreq_monitor_start(struct devfreq *devfreq) { INIT_DELAYED_WORK_DEFERRABLE(devfreq-work, devfreq_monitor); + if (devfreq-profile-polling_ms) queue_delayed_work(devfreq_wq, devfreq-work, msecs_to_jiffies(devfreq-profile-polling_ms)); } I wait for your thorough review before sending next version with this change. --- Documentation/ABI/testing/sysfs-class-devfreq | 8 - drivers/devfreq/devfreq.c | 431 +++--- drivers/devfreq/governor.h| 11 + drivers/devfreq/governor_performance.c| 16 +- drivers/devfreq/governor_powersave.c | 16 +- drivers/devfreq/governor_simpleondemand.c | 24 ++ drivers/devfreq/governor_userspace.c | 23 +- include/linux/devfreq.h | 34 +- 8 files changed, 267 insertions(+), 296 deletions(-) diff --git a/Documentation/ABI/testing/sysfs-class-devfreq b/Documentation/ABI/testing/sysfs-class-devfreq index 23d78b5..89283b1 100644 --- a/Documentation/ABI/testing/sysfs-class-devfreq +++ b/Documentation/ABI/testing/sysfs-class-devfreq @@ -21,14 +21,6 @@ Description: The /sys/class/devfreq/.../cur_freq shows the current frequency of the corresponding devfreq object. -What:/sys/class/devfreq/.../central_polling -Date:September 2011 -Contact: MyungJoo Ham myungjoo@samsung.com -Description: - The /sys/class/devfreq/.../central_polling shows whether - the devfreq ojbect is using devfreq-provided central -
Re: [Powertop] [PATCH v2 2/2] Add stubs to support Android platform
On (09/27/12 00:09), Magnus Fromreide wrote: That they fail to throw exceptions from new is no reason to disable set_new_handler, the newhandler is called by the runtime on out of memory and is intended to allow the user to try fixing the issue. This is true for the noexcept version as well. Is this yet another incompatibility? right. the funny thing is, guess what, gcc actually has #ifdef __EXCEPTIONS within operator new() 44 _GLIBCXX_WEAK_DEFINITION void * 45 operator new (std::size_t sz) _GLIBCXX_THROW (std::bad_alloc) 46 { 47 void *p; 48 49 /* malloc (0) is unpredictable; avoid it. */ 50 if (sz == 0) 51 sz = 1; 52 p = (void *) malloc (sz); 53 while (p == 0) 54 { 55 new_handler handler = __new_handler; 56 if (! handler) 57 #ifdef __EXCEPTIONS 58 throw bad_alloc(); 59 #else 60 std::abort(); 61 #endif 62 handler (); 63 p = (void *) malloc (sz); 64 } 65 66 return p; 67 } It tourned out, that __EXCEPTIONS with us (for operator new and STL) since 2001 2001-02-15 Benjamin Kosnik b...@redhat.com So, I guess this is how Google has came up with the idea of C++ w/o exceptions. Wow. -ss +/* define stubs for C++ exception handling */ +#define tryif (true) +#define catch(x) if (false) If you should do this then I think it should be spelled #define try if (true) #define catch else if (false) in order to not break if (condition) try { } catch(type variable) { } but it still breaks the syntax for it which can be shown by simply adding an else clause to the if statement. Oh, and furthermore I consider that Android needs a C++ compiler. + +/* Define __NR_perf_event_open if not already defined */ +#if __arm__ +#ifndef __NR_perf_event_open +#define __NR_perf_event_open364 +#endif +#endif + +/* + * bionic libc mbstowcs version returns zero when max parameter + * is zero, resulting infinite loops in powertop source. Add + * mbstowcs wrapper to fix it. + */ +namespace pandroid { + extern C inline size_t mbstowcs(wchar_t *dst, + const char *src, size_t len) + { + return ::mbstowcs(dst, src, ::strlen(src)); + } +} + +#define mbstowcs(dst, src, len)pandroid::mbstowcs(dst, src, len) Still broken. If dst isn't a NULL pointer then len is the limit on the length of the destination buffer. In throwing away this you open up for stack smashing attacks. ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [RFC PATCH v8] media: add v4l2 subdev driver for S5K4ECGX sensor
On 09/26/2012 10:32 PM, Mauro Carvalho Chehab wrote: Em Thu, 13 Sep 2012 12:02:14 +0100 Sangwook Leesangwook@linaro.org escreveu: This patch adds driver for S5K4ECGX sensor with embedded ISP SoC, S5K4ECGX, which is a 5M CMOS Image sensor from Samsung The driver implements preview mode of the S5K4ECGX sensor. capture (snapshot) operation, face detection are missing now. Following controls are supported: contrast/saturation/brightness/sharpness Signed-off-by: Sangwook Leesangwook@linaro.org Reviewed-by: Sylwester Nawrockis.nawro...@samsung.com Cc: Francesco Lavrafrancescolavra...@gmail.com Cc: Scott Bambroughscott.bambro...@linaro.org Cc: Homin Leesuap...@insignal.co.kr ... +static int s5k4ecgx_load_firmware(struct v4l2_subdev *sd) +{ +const struct firmware *fw; +const u8 *ptr; +int err, i, regs_num; +struct i2c_client *client = v4l2_get_subdevdata(sd); +u16 val; +u32 addr, crc, crc_file, addr_inc = 0; + +err = request_firmware(fw, S5K4ECGX_FIRMWARE, sd-v4l2_dev-dev); The patch looks correct on my eyes... Yet, calling request_firmware() might not be the right thing to do. The thing is that newer versions of udev refuse to load firmware synchronously during probe/init time. As this function is actually called by s_power, maybe this driver doesn't suffer from that new udev behavior, so, I'll be merging it as-is. However, I suggest you to take a deeper review on that and, if possible, test it with the latest udev. True, it's indeed a bit tricky. The host interface driver this sensor driver has been tested with calls s_power on a subdev only in response to a video device open(). During probe only subdev's .registered() callback is called. And there is no request_firmware() there, as it is not needed to boot the sensor's MCU. The firmware is really just a set of settings that are normally needed only before streaming needs to be started. We have analysed this issue and hopefully there should be no request_firmware() calls from within probe(), as long as the host interface driver doesn't call s_power from it's probe() callback. I'm afraid it's not the case for all bridge drivers though. If needed we could probably try and use request_firmware_nowait(), but it seems better to me to make sure the bridge drivers don't call s_power from within their probe() callback. -- Thanks, Sylwester ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [Powertop] [PATCH v2 2/2] Add stubs to support Android platform
On Mon, Sep 24, 2012 at 06:58:04PM +0530, Rajagopal Venkat wrote: This patch adds stubs for features that are not supported by Andriod. An header file which defines all stubs is included only for Android builds. Signed-off-by: Rajagopal Venkat rajagopal.ven...@linaro.org diff --git a/src/android_stubs.h b/src/android_stubs.h new file mode 100644 index 000..ed37c0e --- /dev/null +++ b/src/android_stubs.h + +/* Android C++ new operator does not throw exception on failure */ +#define set_new_handler(x) That they fail to throw exceptions from new is no reason to disable set_new_handler, the newhandler is called by the runtime on out of memory and is intended to allow the user to try fixing the issue. This is true for the noexcept version as well. Is this yet another incompatibility? +/* define stubs for C++ exception handling */ +#define try if (true) +#define catch(x) if (false) If you should do this then I think it should be spelled #define try if (true) #define catch else if (false) in order to not break if (condition) try { } catch(type variable) { } but it still breaks the syntax for it which can be shown by simply adding an else clause to the if statement. Oh, and furthermore I consider that Android needs a C++ compiler. + +/* Define __NR_perf_event_open if not already defined */ +#if __arm__ +#ifndef __NR_perf_event_open +#define __NR_perf_event_open364 +#endif +#endif + +/* + * bionic libc mbstowcs version returns zero when max parameter + * is zero, resulting infinite loops in powertop source. Add + * mbstowcs wrapper to fix it. + */ +namespace pandroid { + extern C inline size_t mbstowcs(wchar_t *dst, + const char *src, size_t len) + { + return ::mbstowcs(dst, src, ::strlen(src)); + } +} + +#define mbstowcs(dst, src, len) pandroid::mbstowcs(dst, src, len) Still broken. If dst isn't a NULL pointer then len is the limit on the length of the destination buffer. In throwing away this you open up for stack smashing attacks. /MF ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH v2 3/3] bootwrapper: Delay switch to Hyp mode until kernel entry
On 26 September 2012 18:08, Jon Medhurst (Tixy) t...@linaro.org wrote: On Wed, 2012-09-26 at 16:01 +0100, Peter Maydell wrote: The patch has added an 'enter_hyp' call into the chunk of code which is relocated to some random address, which means the code is now too long and we only relocate half of it. So when the primary CPU wakes up the secondaries they just run off into the weeds. Solution: move the enter_hyp to before the secondary-CPU pen code rather than after it. That analysis sounds reasonable to me and I have successfully tested this modified solution on an A15x4 model. And also on A15x4-A7x4 after first applying the CCI hack to get bit.LITTLE working. Thanks for the testing -- I have now pushed this patchset to git://git.linaro.org/arm/models/boot-wrapper.git master . -- PMM ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
[PATCH 2/3] kernel-arch.bblass: add AArch64 support
Signed-off-by: Marcin Juszkiewicz marcin.juszkiew...@linaro.org --- meta/classes/kernel-arch.bbclass |3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/meta/classes/kernel-arch.bbclass b/meta/classes/kernel-arch.bbclass index 6446504..b3b78b6 100644 --- a/meta/classes/kernel-arch.bbclass +++ b/meta/classes/kernel-arch.bbclass @@ -8,7 +8,7 @@ valid_archs = alpha cris ia64 \ i386 x86 \ m68knommu m68k ppc powerpc powerpc64 ppc64 \ sparc sparc64 \ - arm \ + arm aarch64 \ m32r mips \ sh sh64 um h8300 \ parisc s390 v850 \ @@ -22,6 +22,7 @@ def map_kernel_arch(a, d): if re.match('(i.86|athlon|x86.64)$', a): return 'x86' elif re.match('armeb$', a): return 'arm' +elif re.match('aarch64$', a): return 'arm64' elif re.match('mips(el|64|64el)$', a): return 'mips' elif re.match('p(pc|owerpc)(|64)', a): return 'powerpc' elif re.match('sh(3|4)$', a): return 'sh' -- 1.7.10.4 ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Add AArch64 support into OE-Core
This patchset adds support for AArch64 in all required classes: - insane - kernel-arch - siteinfo Everything is from public available information (binutils, linux). ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
[PATCH 1/3] siteinfo.bbclass: add AArch64 support
Signed-off-by: Marcin Juszkiewicz marcin.juszkiew...@linaro.org --- meta/classes/siteinfo.bbclass |2 ++ 1 file changed, 2 insertions(+) diff --git a/meta/classes/siteinfo.bbclass b/meta/classes/siteinfo.bbclass index 8c256ce..aab0867 100644 --- a/meta/classes/siteinfo.bbclass +++ b/meta/classes/siteinfo.bbclass @@ -18,6 +18,7 @@ def siteinfo_data(d): archinfo = { allarch: endian-little bit-32, # bogus, but better than special-casing the checks below for allarch +aarch64: endian-little bit-64 arm-common, arm: endian-little bit-32 arm-common, armeb: endian-big bit-32 arm-common, avr32: endian-big bit-32 avr32-common, @@ -60,6 +61,7 @@ def siteinfo_data(d): mingw32: common-mingw, } targetinfo = { +aarch64-linux-gnu: aarch64-linux, arm-linux-gnueabi: arm-linux, arm-linux-uclibceabi: arm-linux-uclibc, armeb-linux-gnueabi: armeb-linux, -- 1.7.10.4 ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
[PATCH 3/3] insane.bbclass: add AArch64 support
Signed-off-by: Marcin Juszkiewicz marcin.juszkiew...@linaro.org --- meta/classes/insane.bbclass |1 + 1 file changed, 1 insertion(+) diff --git a/meta/classes/insane.bbclass b/meta/classes/insane.bbclass index 1fb8970..b390242 100644 --- a/meta/classes/insane.bbclass +++ b/meta/classes/insane.bbclass @@ -44,6 +44,7 @@ def package_qa_get_machine_dict(): arm : (40, 0,0, True, 32), }, linux : { +aarch64 : (183,0,0, True, 64), arm : (40,97,0, True, 32), armeb: (40,97,0, False, 32), powerpc:(20, 0,0, False, 32), -- 1.7.10.4 ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [RFC PATCH v8] media: add v4l2 subdev driver for S5K4ECGX sensor
Hi Laurent, On 09/27/2012 12:29 PM, Laurent Pinchart wrote: On Thursday 27 September 2012 00:00:20 Sylwester Nawrocki wrote: On 09/26/2012 10:32 PM, Mauro Carvalho Chehab wrote: Em Thu, 13 Sep 2012 12:02:14 +0100 Sangwook Lee escreveu: This patch adds driver for S5K4ECGX sensor with embedded ISP SoC, S5K4ECGX, which is a 5M CMOS Image sensor from Samsung The driver implements preview mode of the S5K4ECGX sensor. capture (snapshot) operation, face detection are missing now. Following controls are supported: contrast/saturation/brightness/sharpness Signed-off-by: Sangwook Leesangwook@linaro.org Reviewed-by: Sylwester Nawrockis.nawro...@samsung.com Cc: Francesco Lavrafrancescolavra...@gmail.com Cc: Scott Bambroughscott.bambro...@linaro.org Cc: Homin Leesuap...@insignal.co.kr ... +static int s5k4ecgx_load_firmware(struct v4l2_subdev *sd) +{ + const struct firmware *fw; + const u8 *ptr; + int err, i, regs_num; + struct i2c_client *client = v4l2_get_subdevdata(sd); + u16 val; + u32 addr, crc, crc_file, addr_inc = 0; + + err = request_firmware(fw, S5K4ECGX_FIRMWARE, sd-v4l2_dev-dev); The patch looks correct on my eyes... Yet, calling request_firmware() might not be the right thing to do. The thing is that newer versions of udev refuse to load firmware synchronously during probe/init time. As this function is actually called by s_power, maybe this driver doesn't suffer from that new udev behavior, so, I'll be merging it as-is. However, I suggest you to take a deeper review on that and, if possible, test it with the latest udev. True, it's indeed a bit tricky. The host interface driver this sensor driver has been tested with calls s_power on a subdev only in response to a video device open(). During probe only subdev's .registered() callback is called. And there is no request_firmware() there, as it is not needed to boot the sensor's MCU. The firmware is really just a set of settings that are normally needed only before streaming needs to be started. That triggers a red flag warning :-) What kind of settings do you have in the firmware ? What's in this binary file is about 3 000 register values that are needed to be written to a sensor after it is powered on and before it can be used. I think this is some sort of a patch file that redefines initial state of the sensor, default register values or something. Perhaps some tuning values prepared after the sensor was manufactured. Since it is undocumented I could only guess. I don't think it is really unusual to load such a patch file by means of the firmware API. Some ISP drivers do this as well, to customize to a particular image sensor connected to them. Besides of requiring a proper firmware file. -- Regards, Sylwester ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
[PATCH 0/3] bootwrapper: Add support for big.LITTLE models
This mostly boils down to initialising the Cache Coherent Interconnect (CCI). We do this by looking in the device-tree for a CCI node, that way the same semihosting bootwrapper binary can be used on both the big.LITTLE models and on the A15 models which don't have a CCI. [PATCH 1/3] bootwrapper: Allow for multiple clusters in boot CPU [PATCH 2/3] bootwrapper: Factor out parsing of fdt #address-cells [PATCH 3/3] bootwrapper: Initialise CCI device if found in the fdt boot.S|2 +- semi_loader.c | 104 +--- 2 files changed, 84 insertions(+), 22 deletions(-) ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
[PATCH 3/3] bootwrapper: Initialise CCI device if found in the fdt
From: Jon Medhurst t...@linaro.org The A15xA7 models simulate a Cache Coherent Interconnect (CCI) and this needs to be initialised correctly for Linux to boot. To perform this initiation we add the new function configure_from_fdt() which will look in the fdt for devices to initialise. In this first case we look for the CCI node and if found then setup this device. Signed-off-by: Jon Medhurst t...@linaro.org --- semi_loader.c | 50 ++ 1 file changed, 50 insertions(+) diff --git a/semi_loader.c b/semi_loader.c index c9750be..ca70633 100644 --- a/semi_loader.c +++ b/semi_loader.c @@ -242,6 +242,54 @@ libfdt_error: fatal(libfdt: , fdt_strerror(e), , while updating device tree\n); } +/* For accessing 32-bit device ports */ +#define io32(p) (*(volatile uint32_t *)(p)) + +static void init_cci(unsigned cci) +{ + info(Initialising CCI\n); + + /* +* Ideally, the CCI device tree binding would include suitable +* information so we can correctly configure the CCI, but for +* now we'll just hard-code settings for the present A15xA7 +* models. +*/ + + /* Turn on CCI snoops and DVM messages */ + io32(cci+0x4000) = 0x3; /* A15 cluster */ + io32(cci+0x5000) = 0x3; /* A7 cluster */ + + /* Wait while change pending bit of status register is set */ + while(io32(cci+0xc) 0x1) + {} +} + +static void configure_from_fdt(struct loader_info *info) +{ + void *fdt = (void *)info-fdt_start; + uint32_t const *p; + int addrcells, sizecells; + int offset, len; + + if(!fdt) + return; + + _fdt_address_and_size_cells(fdt, addrcells, sizecells); + + /* See if there is a CCI device to initialise */ + offset = fdt_node_offset_by_compatible(fdt, 0, arm,cci); + if (offset = 0) { + p = fdt_getprop(fdt, offset, reg, len); + if(len != (addrcells + sizecells) * 4) + info(Failed parsing device-tree node for CCI\n); + else + init_cci(fdt32_to_cpu(p[addrcells - 1])); + } + + return; +} + static int is_space(char c) { return c == ' '; @@ -598,4 +646,6 @@ args_done: atag_append(atagp, ATAG_NONE, 0, 0); update_fdt(phys, info); + + configure_from_fdt(info); } -- 1.7.10.4 ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
[PATCH 2/3] bootwrapper: Factor out parsing of fdt #address-cells and #size-cells
From: Jon Medhurst t...@linaro.org We will be reusing this functionality later. Signed-off-by: Jon Medhurst t...@linaro.org --- semi_loader.c | 56 ++-- 1 file changed, 34 insertions(+), 22 deletions(-) diff --git a/semi_loader.c b/semi_loader.c index cbe911c..c9750be 100644 --- a/semi_loader.c +++ b/semi_loader.c @@ -97,6 +97,39 @@ static int _fdt_make_node(void *fdt, int parentoffset, const char *name) return fdt_add_subnode(fdt, parentoffset, name); } +static void _fdt_address_and_size_cells(void *fdt, int* addrcells, int* sizecells) +{ + int e; + uint32_t const *p; + + if(!(p = fdt_getprop(fdt, 0, #address-cells, e))) + goto libfdt_error; + else if(e != 4) + goto size_error; + *addrcells = fdt32_to_cpu(*p); + if(!(p = fdt_getprop(fdt, 0, #size-cells, e))) + goto libfdt_error; + else if(e != 4) + goto size_error; + *sizecells = fdt32_to_cpu(*p); + + /* +* Sanity-check address sizes, since addresses and sizes which do +* not take up exactly 4 or 8 bytes are not supported. +*/ + if ((*addrcells != 1 *addrcells != 2) || + (*sizecells != 1 *sizecells != 2)) + goto size_error; + + return; + +libfdt_error: + fatal(libfdt: , fdt_strerror(e), , while looking for #address-cells/#size-cells\n); + +size_error: + fatal(Unexpected/invalid #address-cells/#size-cells in device tree\n); +} + static void update_fdt(void **dest, struct loader_info *info) { int e; @@ -112,25 +145,7 @@ static void update_fdt(void **dest, struct loader_info *info) if((e = fdt_open_into((void *)info-fdt_start, fdt, FDT_SIZE_MAX)) 0) goto libfdt_error; - /* -* Sanity-check address sizes, since addresses and sizes which do -* not take up exactly 4 or 8 bytes are not supported. -*/ - { - if(!(p = fdt_getprop(fdt, 0, #address-cells, e))) - goto libfdt_error; - else if(e != 4) - goto size_error; - addrcells = fdt32_to_cpu(*p); - if(!(p = fdt_getprop(fdt, 0, #size-cells, e))) - goto libfdt_error; - else if(e != 4) - goto size_error; - sizecells = fdt32_to_cpu(*p); - if ((addrcells != 1 addrcells != 2) || - (sizecells != 1 sizecells != 2)) - goto size_error; - } + _fdt_address_and_size_cells(fdt, addrcells, sizecells); /* * Add a memory node, but only if there isn't one already. If @@ -225,9 +240,6 @@ no_add_memory: libfdt_error: fatal(libfdt: , fdt_strerror(e), , while updating device tree\n); - -size_error: - fatal(Unexpected/invalid #address-cells/#size-cells in device tree\n); } static int is_space(char c) -- 1.7.10.4 ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Work around for building Linaro Android 12.08
For people trying to build Linaro Android 12.08 there's an issue with an unknown reference in powertop2.0. Here's what to do: cd android edit .repo/manifest.xml Find: project groups=path:external/powertop,name:tools/powertop-2.0 name=tools/powertop-2.0 path=exter\ nal/powertop remote=linaro-other revision=c80f55d9d1b823b7f6ff447eefc8a6ba04b939c4/ ...and remove the line. Then cp .repo/manifest.xml ../ cd .. rm -rf android/ edit linaro_android_build_cmds.sh and change curl -k http://snapshots.linaro.org/android/~linaro-android/vexpress-jb-gcc47-armlt-tracking-open-12.08-release/1//pinned-manifest.xml .repo/manifest.xml to cp ../manifest.xml .repo/ and rerun linaro_android_build_cmds.sh You can also take a tip build which is not susceptible to this problem. Tip builds that have been booted are listed at: https://docs.google.com/a/linaro.org/spreadsheet/ccc?key=0AnpUtxWjZbP9dGg0dW5pdDl2T1lOUFk1aEktOFh0ZXc#gid=0 If you still get errors after this, then its a general git timeout error. Just delete the android/ directory and start again or cd android/; repo ./sync -- Zach Pfeffer Android Platform Team Lead, Linaro Platform Teams Linaro.org | Open source software for ARM SoCs Follow Linaro: http://www.facebook.com/pages/Linaro http://twitter.com/#!/linaroorg - http://www.linaro.org/linaro-blog ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev