Re: [PATCH 1/2] cpufreq: return early from __cpufreq_driver_getavg()
Re-sending, as it bounced from the lists :(
When i reply to mail from my Samsung S2, it replies in HTML format.
Don't know how to fix it :)
On 20 October 2012 10:12, Viresh Kumar wrote:
>
> On Oct 20, 2012 3:37 AM, "Rafael J. Wysocki" wrote:
>>
>> On Saturday 20 of October 2012 01:42:05 Viresh Kumar wrote:
>> > There is no need to do cpufreq_get_cpu() and cpufreq_put_cpu() for
>> > drivers that
>> > don't support getavg() routine.
>> >
>> > Signed-off-by: Viresh Kumar
>>
>> The patch doesn't seem to follow the changelog or the other way around.
>
> Sorry if my log isn't clear enough.
> But i could still see it matching the code :)
>
> I have moved the check for drivers capabilities at the top
> of routine, so that there is no need to call mentioned routines.
>
>>
>> Thanks,
>> Rafael
>>
>>
>> > ---
>> > drivers/cpufreq/cpufreq.c | 6 --
>> > 1 file changed, 4 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
>> > index 85df538..f552d5f 100644
>> > --- a/drivers/cpufreq/cpufreq.c
>> > +++ b/drivers/cpufreq/cpufreq.c
>> > @@ -1511,12 +1511,14 @@ int __cpufreq_driver_getavg(struct
>> > cpufreq_policy *policy, unsigned int cpu)
>> > {
>> > int ret = 0;
>> >
>> > + if (!(cpu_online(cpu) && cpufreq_driver->getavg))
>> > + return 0;
>> > +
>> > policy = cpufreq_cpu_get(policy->cpu);
>> > if (!policy)
>> > return -EINVAL;
>> >
>> > - if (cpu_online(cpu) && cpufreq_driver->getavg)
>> > - ret = cpufreq_driver->getavg(policy, cpu);
>> > + ret = cpufreq_driver->getavg(policy, cpu);
>> >
>> > cpufreq_cpu_put(policy);
>> > return ret;
>> >
>> --
>> I speak only for myself.
>> Rafael J. Wysocki, Intel Open Source Technology Center.
___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH] mfd: ab8500: add devicetree support for fuelgauge
Hi Rajanikanth,
On 10/16/2012 05:36 AM, Rajanikanth H.V wrote:
> From: "Rajanikanth H.V"
>
> - This patch adds device tree support for fuelgauge driver
> - optimize bm devices platform_data usage and of_probe(...)
> Note: of_probe() routine for battery managed devices is made
> common across all bm drivers.
> - test status:
> - interrupt numbers assigned differs between legacy and FDT mode.
>
> Legacy platform_data Mode:
> root@ME:/ cat /proc/interrupts
>CPU0 CPU1
> 483: 0 0ab8500 ab8500-ponkey-dbf
> 484: 0 0ab8500 ab8500-ponkey-dbr
> 485: 0 0ab8500 BATT_OVV
> 494: 0 1ab8500
> 495: 0 0ab8500 ab8500-rtc
> 501: 0 13ab8500 NCONV_ACCU
> 503: 7 22ab8500 CCEOC
> 504: 0 1ab8500 CC_INT_CALIB
> 505: 0 0ab8500 LOW_BAT_F
> 516: 0 34ab8500 ab8500-gpadc
> 556: 0 0ab8500 usb-link-status
>
> FDT Mode:
> root@ME:/ cat /proc/interrupts
>CPU0 CPU1
> 6: 0 0ab8500 ab8500-ponkey-dbf
> 7: 0 0ab8500 ab8500-ponkey-dbr
> 8: 0 0ab8500 BATT_OVV
> 162: 0 7ab8500 ab8500-gpadc
> 163: 0 1ab8500
> 164: 0 0ab8500 ab8500-rtc
> 484: 0 0ab8500 usb-link-status
> 499: 0 4ab8500 NCONV_ACCU
> 500: 0 0ab8500 LOW_BAT_F
> 502: 0 1ab8500 CC_INT_CALIB
> 503: 0 6ab8500 CCEOC
>
> Signed-off-by: Rajanikanth H.V
[...]
> +int __devinit
> +bmdevs_of_probe(struct device *dev,
> + struct device_node *np,
> + struct abx500_bm_data **battery)
> +{
> + struct abx500_battery_type *btype;
> + struct device_node *np_bat_supply;
> + struct abx500_bm_data *bat;
> + int i, thermistor;
> + char*bat_tech = "UNKNOWN";
This initialization is useless.
> +
> + *battery = devm_kzalloc(dev, sizeof(*bat), GFP_KERNEL);
> + if (!*battery) {
> + dev_err(dev, "%s no mem for plat_data\n", __func__);
> + return -ENOMEM;
> + }
> + *battery = &ab8500_bm_data;
Looks like dynamic allocation here is not what you need, since you are
overwriting the pointer to the allocated data.
> +
> + /* get phandle to 'battery-info' node */
> + np_bat_supply = of_parse_phandle(np, "battery", 0);
> + if (!np_bat_supply) {
> + dev_err(dev, "missing property battery\n");
> + return -EINVAL;
> + }
> + if (of_property_read_bool(np_bat_supply,
> + "thermistor-on-batctrl"))
> + thermistor = NTC_INTERNAL;
> + else
> + thermistor = NTC_EXTERNAL;
> +
> + bat = *battery;
> + if (thermistor == NTC_EXTERNAL) {
> + bat->n_btypes = 4;
> + bat->bat_type = bat_type_ext_thermistor;
> + bat->adc_therm = ABx500_ADC_THERM_BATTEMP;
> + }
> + bat_tech = (char *)of_get_property(
> + np_bat_supply, "stericsson,battery-type", NULL);
No need to cast a void * to a char *.
> + if (!bat_tech) {
> + dev_warn(dev, "missing property battery-name/type\n");
> + strncpy(bat_tech, "UNKNOWN", 7);
You are writing at a NULL pointer here...
> + }
> + of_node_put(np_bat_supply);
You can't call of_node_put here, because you are going to use bat_tech
later on. You have to call it at the end of this function, after you are
done with bat_tech.
> +
> + if (strncmp(bat_tech, "LION", 4) == 0) {
> + bat->no_maintenance = true;
> + bat->chg_unknown_bat = true;
> + bat->bat_type[BATTERY_UNKNOWN].charge_full_design = 2600;
> + bat->bat_type[BATTERY_UNKNOWN].termination_vol= 4150;
> + bat->bat_type[BATTERY_UNKNOWN].recharge_vol = 4130;
> + bat->bat_type[BATTERY_UNKNOWN].normal_cur_lvl = 520;
> + bat->bat_type[BATTERY_UNKNOWN].normal_vol_lvl = 4200;
> + }
> + /* select the battery resolution table */
> + for (i = 0; i < bat->n_btypes; ++i) {
> + btype = (bat->bat_type + i);
> + if (thermistor == NTC_EXTERNAL) {
> + btype->batres_tbl =
> + temp_to_batres_tbl_ext_thermistor;
> + } else if (strncmp(bat_tech, "LION", 4) == 0) {
> + btype->batres_tbl =
> + temp_to_batres_tbl_9100;
> + } else {
> + btype->batres_tbl =
> + temp_to_batres_tbl_thermistor;
> + }
> + }
> + return 0;
> +}
[...]
> diff --git a/drivers/power/ab8500_fg.c b/drivers/power/ab8500_fg.c
> index bf02225..ff64dd4 100644
>
Re: [PATCH] acpi/cpuidle: reinitialize power_usage values when adding/removing C-states
On 10/19/2012 11:50 PM, Julius Werner wrote:
> When cpuidle drivers do not supply explicit power_usage values,
> cpuidle/driver.c inserts dummy values instead. When a running processor
> dynamically gains new C-states (e.g. after ACPI events), the power_usage
> values of those states will stay uninitialized, and cpuidle governors
> will never choose to enter them.
>
> This patch ensures that the ACPI cpuidle driver sets those dummy power
> values itself whenever it (re-)initializes its idle states.
> Tested and verified on an Acer AC700 Chromebook, which supports the C3
> state only when it switches from AC to battery power.
>
> Signed-off-by: Julius Werner
> ---
I am not against this patch but I am wondering if it is not time to do
some cleanup around this.
The flag 'power_specified' is never used in any driver.
And the field 'power_usage' is used only in the tegra3 driver where
logically as power_specified is not set, it will be overwritten at the
init (could someone could check the
/sys/devices/system/cpu/cpu0/cpuidle/state1/power is different from 600
on tegra3 ?)
The drivers define their states in a power consumption descendant order
making de facto the same ordering than the 'set_power_state' function in
driver.c
The governor looks at the power_usage (which is always filled by
'set_power_state').
static int menu_select(struct cpuidle_driver *drv, struct cpuidle_device
*dev)
[ ... ]
if (s->power_usage < power_usage) {
power_usage = s->power_usage;
data->last_state_idx = i;
data->exit_us = s->exit_latency;
}
[ ... ]
Could we just say this is always true because state[i+1] consumes less
than state[i] ?
And then just remove the 'set_power_state' function, and the field
'driver->power_specified' ?
That will cleanup the code and fix this problem, no ?
Thanks
-- Daniel
> drivers/acpi/processor_idle.c |3 +++
> 1 files changed, 3 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
> index e8086c7..078e22f 100644
> --- a/drivers/acpi/processor_idle.c
> +++ b/drivers/acpi/processor_idle.c
> @@ -1090,6 +1090,9 @@ static int acpi_processor_setup_cpuidle_states(struct
> acpi_processor *pr)
> state->exit_latency = cx->latency;
> state->target_residency = cx->latency * latency_factor;
>
> + /* reinitialize this in case new states are added after boot */
> + state->power_usage = -1 - count;
> +
> state->flags = 0;
> switch (cx->type) {
> case ACPI_STATE_C1:
--
Facebook |
Twitter |
