* Re: [PATCH v11 6/9] libata: handle power transition of ODD
From: Aaron Lu @ 2013-01-08 9:09 UTC (permalink / raw)
To: Tejun Heo
Cc: Jeff Garzik, James Bottomley, Rafael J. Wysocki, Alan Stern,
Aaron Lu, Jeff Wu, linux-ide, linux-pm, linux-scsi, linux-acpi
In-Reply-To: <20130107184245.GS3926@htj.dyndns.org>
On 01/08/2013 02:42 AM, Tejun Heo wrote:
> On Sun, Jan 06, 2013 at 10:48:26AM +0800, Aaron Lu wrote:
>> +bool zpodd_zpready(struct ata_device *dev)
>> +{
>> + struct zpodd *zpodd = dev->zpodd;
>> + return zpodd->zp_ready;
>> +}
>> +
>> +void zpodd_pre_poweroff(struct ata_device *dev)
>> +{
>> + struct zpodd *zpodd = dev->zpodd;
>> +
>> + zpodd->powered_off = true;
>> + device_set_run_wake(&dev->sdev->sdev_gendev, true);
>> + acpi_pm_device_run_wake(&dev->sdev->sdev_gendev, true);
>> +}
>> +
>> +void zpodd_pre_poweron(struct ata_device *dev)
>> +{
>> + struct zpodd *zpodd = dev->zpodd;
>> +
>> + if (zpodd->powered_off) {
>> + acpi_pm_device_run_wake(&dev->sdev->sdev_gendev, false);
>> + device_set_run_wake(&dev->sdev->sdev_gendev, false);
>> + }
>> +}
>> +
>> +void zpodd_post_resume(struct ata_device *dev)
>> +{
>> + struct zpodd *zpodd = dev->zpodd;
>> +
>> + if (!zpodd->powered_off)
>> + return;
>> +
>> + zpodd->powered_off = false;
>> +
>> + if (zpodd->from_notify) {
>> + zpodd->from_notify = false;
>> + if (zpodd->drawer)
>> + eject_tray(dev);
>> + }
>> +
>> + zpodd->last_ready = 0;
>> + zpodd->zp_ready = false;
>> +}
>
> I would really appreciate some comments at least on functions visible
> outside zpodd.c. Please add proper function comments explaining what
> they're doing to achieve what.
Will add them in next version, thanks.
-Aaron
^ permalink raw reply
* Re: [PATCH 2/3] cpufreq: add imx6q-cpufreq driver
From: Viresh Kumar @ 2013-01-08 9:16 UTC (permalink / raw)
To: Shawn Guo
Cc: Rafael J. Wysocki, Sascha Hauer, linux-arm-kernel, cpufreq,
linux-pm
In-Reply-To: <1357628043-19256-3-git-send-email-shawn.guo@linaro.org>
Hi Shawn,
Apart from Sascha's comments, please see few comment from my side:
On Tue, Jan 8, 2013 at 12:24 PM, Shawn Guo <shawn.guo@linaro.org> wrote:
> diff --git a/drivers/cpufreq/imx6q-cpufreq.c b/drivers/cpufreq/imx6q-cpufreq.c
> +static struct regulator *arm_reg;
> +static struct regulator *pu_reg;
> +static struct regulator *soc_reg;
Can move to a single line, if you like.
> +static struct clk *arm_clk;
> +static struct clk *pll1_sys_clk;
> +static struct clk *pll1_sw_clk;
> +static struct clk *step_clk;
> +static struct clk *pll2_pfd2_396m_clk;
same.
> +static int imx6q_set_target(struct cpufreq_policy *policy,
> + unsigned int target_freq, unsigned int relation)
> +{
> + struct cpufreq_freqs freqs;
> + struct opp *opp;
> + unsigned long freq_hz, volt, volt_old;
> + unsigned int index, cpu;
> + int ret;
> +
> + ret = cpufreq_frequency_table_target(policy, freq_table, target_freq,
> + relation, &index);
> + if (ret) {
> + pr_err("failed to match target freqency %d: %d\n",
s/freqency/frequency
> + target_freq, ret);
> + return ret;
> + }
> +
> + freq_hz = freq_table[index].frequency * 1000;
> + freqs.new = freq_hz / 1000;
could be written as:
freqs.new = freq_table[index].frequency;
freq_hz = freqs.new * 1000;
> + freqs.old = clk_get_rate(arm_clk) / 1000;
> +
> + if (freqs.old == freqs.new)
> + return 0;
> +
> + for_each_online_cpu(cpu) {
> + freqs.cpu = cpu;
> + cpufreq_notify_transition(&freqs, CPUFREQ_PRECHANGE);
> + }
> +
> + opp = opp_find_freq_ceil(cpu_dev, &freq_hz);
> + if (IS_ERR(opp)) {
> + pr_err("failed to find OPP for %ld\n", freq_hz);
> + return PTR_ERR(opp);
> + }
> +
> + volt = opp_get_voltage(opp);
> + volt_old = regulator_get_voltage(arm_reg);
> +
> + pr_debug("%u MHz, %ld mV --> %u MHz, %ld mV\n",
> + freqs.old / 1000, volt_old / 1000,
> + freqs.new / 1000, volt / 1000);
> +
> + /* scaling up? scale voltage before frequency */
> + if (freqs.new > freqs.old) {
> + ret = regulator_set_voltage_tol(arm_reg, volt, 0);
> + if (ret) {
> + pr_err("failed to scale voltage up: %d\n", ret);
> + freqs.new = freqs.old;
is this useful?
> + return ret;
> + }
> +
> + /*
> + * Need to increase vddpu and vddsoc for safety
> + * if we are about to run at 1.2 GHz.
> + */
> + if (freqs.new == FREQ_1P2_GHZ / 1000) {
> + regulator_set_voltage_tol(pu_reg,
> + PU_SOC_VOLTAGE_HIGH, 0);
> + regulator_set_voltage_tol(soc_reg,
> + PU_SOC_VOLTAGE_HIGH, 0);
> + }
> + }
> +
> + /*
> + * The setpoints are selected per PLL/PDF frequencies, so we need to
> + * reprogram PLL for frequency scaling. The procedure of reprogramming
> + * PLL1 is as blow.
> + *
> + * - Enable pll2_pfd2_396m_clk and reparent pll1_sw_clk to it
> + * - Disable pll1_sys_clk and reprogram it
> + * - Enable pll1_sys_clk and reparent pll1_sw_clk back to it
> + * - Disable pll2_pfd2_396m_clk
> + */
> + clk_prepare_enable(pll2_pfd2_396m_clk);
> + clk_set_parent(step_clk, pll2_pfd2_396m_clk);
> + clk_set_parent(pll1_sw_clk, step_clk);
> + clk_prepare_enable(pll1_sys_clk);
> + if (freq_hz > clk_get_rate(pll2_pfd2_396m_clk)) {
> + clk_disable_unprepare(pll1_sys_clk);
> + clk_set_rate(pll1_sys_clk, freqs.new * 1000);
> + clk_prepare_enable(pll1_sys_clk);
> + clk_set_parent(pll1_sw_clk, pll1_sys_clk);
> + clk_disable_unprepare(pll2_pfd2_396m_clk);
> + } else {
> + /*
> + * Disable pll1_sys_clk if pll2_pfd2_396m_clk is sufficient
> + * to provide the frequency.
> + */
> + clk_disable_unprepare(pll1_sys_clk);
Are you doing anything in clk_[un]prepare ? If not, do clk_[un]prepare
at init and exit and do enable/disable here.
> + }
> +
> + /* Ensure the arm clock divider is what we expect */
> + ret = clk_set_rate(arm_clk, freqs.new * 1000);
> + if (ret) {
> + pr_err("failed to set clock rate: %d\n", ret);
> + regulator_set_voltage_tol(arm_reg, volt_old, 0);
> + return ret;
> + }
> +
> + /* scaling down? scale voltage after frequency */
> + if (freqs.new < freqs.old) {
> + ret = regulator_set_voltage_tol(arm_reg, volt, 0);
> + if (ret)
> + pr_warn("failed to scale voltage down: %d\n", ret);
> +
> + if (freqs.old == FREQ_1P2_GHZ / 1000) {
> + regulator_set_voltage_tol(pu_reg,
> + PU_SOC_VOLTAGE_NORMAL, 0);
> + regulator_set_voltage_tol(soc_reg,
> + PU_SOC_VOLTAGE_NORMAL, 0);
> + }
> + }
> +
> + for_each_online_cpu(cpu) {
> + freqs.cpu = cpu;
> + cpufreq_notify_transition(&freqs, CPUFREQ_POSTCHANGE);
> + }
> +
> + return 0;
> +}
> +
> +static int imx6q_cpufreq_init(struct cpufreq_policy *policy)
> +{
> + int ret;
> +
> + if (policy->cpu != 0)
> + return -EINVAL;
Why? The problem here is when you hot-unplug cpu0, init would be called for
cpu1 and it would fail.
> +
> + ret = cpufreq_frequency_table_cpuinfo(policy, freq_table);
> + if (ret) {
> + pr_err("invalid frequency table: %d\n", ret);
> + return ret;
> + }
> +
> + policy->cpuinfo.transition_latency = transition_latency;
> + policy->cur = clk_get_rate(arm_clk) / 1000;
> + policy->shared_type = CPUFREQ_SHARED_TYPE_ANY;
> + cpumask_setall(policy->cpus);
> + cpufreq_frequency_table_get_attr(freq_table, policy->cpu);
> +
> + return 0;
> +}
> +
> +static int imx6q_cpufreq_exit(struct cpufreq_policy *policy)
> +{
> + cpufreq_frequency_table_put_attr(policy->cpu);
> + return 0;
> +}
> +
> +static struct freq_attr *imx6q_cpufreq_attr[] = {
> + &cpufreq_freq_attr_scaling_available_freqs,
> + NULL,
> +};
> +
> +static struct cpufreq_driver imx6q_cpufreq_driver = {
> + .verify = imx6q_verify_speed,
> + .target = imx6q_set_target,
> + .get = imx6q_get_speed,
> + .init = imx6q_cpufreq_init,
> + .exit = imx6q_cpufreq_exit,
> + .name = "imx6q-cpufreq",
> + .attr = imx6q_cpufreq_attr,
> +};
> +
> +static int imx6q_cpufreq_probe(struct platform_device *pdev)
> +{
> + struct device_node *np;
> + int ret;
> +
> + np = of_find_node_by_path("/cpus/cpu@0");
> + if (!np) {
> + pr_err("failed to find cpu0 node\n");
> + return -ENOENT;
> + }
> +
> + cpu_dev = get_cpu_device(0);
> + if (!cpu_dev) {
> + pr_err("failed to get cpu0 device\n");
> + ret = -ENODEV;
> + goto put_node;
> + }
> +
> + cpu_dev->of_node = np;
> +
> + arm_clk = clk_get(cpu_dev, "arm");
> + pll1_sys_clk = clk_get(cpu_dev, "pll1_sys");
> + pll1_sw_clk = clk_get(cpu_dev, "pll1_sw");
> + step_clk = clk_get(cpu_dev, "step");
> + pll2_pfd2_396m_clk = clk_get(cpu_dev, "pll2_pfd2_396m");
> + if (IS_ERR(arm_clk) || IS_ERR(pll1_sys_clk) || IS_ERR(pll1_sw_clk) ||
> + IS_ERR(step_clk) || IS_ERR(pll2_pfd2_396m_clk)) {
> + pr_err("failed to get clocks\n");
> + ret = -ENOENT;
> + goto put_node;
That looks to be wrong. If clk failed for the last clk_get, you must
free all other
clks too.
> + }
> +
> + arm_reg = regulator_get(cpu_dev, "arm");
> + pu_reg = regulator_get(cpu_dev, "pu");
> + soc_reg = regulator_get(cpu_dev, "soc");
> + if (!arm_reg || !pu_reg || !soc_reg) {
> + pr_err("failed to get regulators\n");
> + ret = -ENOENT;
> + goto put_clk;
ditto
> + }
> +
> + /* We expect an OPP table supplied by platform */
> + ret = opp_get_opp_count(cpu_dev);
> + if (ret < 0) {
> + pr_err("no OPP table is found: %d\n", ret);
> + goto put_reg;
> + }
> +
> + ret = opp_init_cpufreq_table(cpu_dev, &freq_table);
> + if (ret) {
> + pr_err("failed to init cpufreq table: %d\n", ret);
> + goto put_reg;
> + }
> +
> + if (of_property_read_u32(np, "clock-latency", &transition_latency))
> + transition_latency = CPUFREQ_ETERNAL;
> +
> + ret = cpufreq_register_driver(&imx6q_cpufreq_driver);
> + if (ret) {
> + pr_err("failed register driver: %d\n", ret);
> + goto free_freq_table;
> + }
> +
> + of_node_put(np);
> + return 0;
> +
> +free_freq_table:
> + opp_free_cpufreq_table(cpu_dev, &freq_table);
> +put_reg:
> + regulator_put(soc_reg);
> + regulator_put(pu_reg);
> + regulator_put(arm_reg);
> +put_clk:
> + clk_put(pll2_pfd2_396m_clk);
> + clk_put(step_clk);
> + clk_put(pll1_sw_clk);
> + clk_put(pll1_sys_clk);
> + clk_put(arm_clk);
> +put_node:
> + of_node_put(np);
> + return ret;
> +}
> +
> +static int imx6q_cpufreq_remove(struct platform_device *pdev)
> +{
> + cpufreq_unregister_driver(&imx6q_cpufreq_driver);
> + opp_free_cpufreq_table(cpu_dev, &freq_table);
> + regulator_put(soc_reg);
> + regulator_put(pu_reg);
> + regulator_put(arm_reg);
> + clk_put(pll2_pfd2_396m_clk);
> + clk_put(step_clk);
> + clk_put(pll1_sw_clk);
> + clk_put(pll1_sys_clk);
> + clk_put(arm_clk);
> +
> + return 0;
> +}
> +
> +static struct platform_driver imx6q_cpufreq_platdrv = {
> + .driver = {
> + .name = "imx6q_cpufreq",
> + .owner = THIS_MODULE,
> + },
> + .probe = imx6q_cpufreq_probe,
> + .remove = imx6q_cpufreq_remove,
> +};
> +module_platform_driver(imx6q_cpufreq_platdrv);
> +
> +MODULE_AUTHOR("Shawn Guo <shawn.guo@linaro.org>");
> +MODULE_DESCRIPTION("Freescale i.MX6Q cpufreq driver");
> +MODULE_LICENSE("GPL");
> --
> 1.7.9.5
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 2/3] cpufreq: add imx6q-cpufreq driver
From: Shawn Guo @ 2013-01-08 11:28 UTC (permalink / raw)
To: Sascha Hauer; +Cc: cpufreq, linux-pm, linux-arm-kernel, Rafael J. Wysocki
In-Reply-To: <20130108085756.GM1906@pengutronix.de>
On Tue, Jan 08, 2013 at 09:57:56AM +0100, Sascha Hauer wrote:
...
> > + /* scaling up? scale voltage before frequency */
> > + if (freqs.new > freqs.old) {
> > + ret = regulator_set_voltage_tol(arm_reg, volt, 0);
> > + if (ret) {
> > + pr_err("failed to scale voltage up: %d\n", ret);
> > + freqs.new = freqs.old;
> > + return ret;
> > + }
>
> So this regulator_set_voltage_tol can fail, ...
>
> > +
> > + /*
> > + * Need to increase vddpu and vddsoc for safety
> > + * if we are about to run at 1.2 GHz.
> > + */
> > + if (freqs.new == FREQ_1P2_GHZ / 1000) {
> > + regulator_set_voltage_tol(pu_reg,
> > + PU_SOC_VOLTAGE_HIGH, 0);
> > + regulator_set_voltage_tol(soc_reg,
> > + PU_SOC_VOLTAGE_HIGH, 0);
>
> ... but these can't?
>
I initially have return of every single clk and regulator API call
checked and try to recover everything for error cases. The driver
becomes messy and hard to read with error message and recovering
code all over the places. And I think it's a little bit over
engineering, so end up choosing to check on selected and critical
part to make sure clk and regulator subsystem work good.
> > + }
> > + }
> > +
> > + /*
> > + * The setpoints are selected per PLL/PDF frequencies, so we need to
> > + * reprogram PLL for frequency scaling. The procedure of reprogramming
> > + * PLL1 is as blow.
>
> s/blow/below/
>
Thanks.
> > + *
> > + * - Enable pll2_pfd2_396m_clk and reparent pll1_sw_clk to it
> > + * - Disable pll1_sys_clk and reprogram it
> > + * - Enable pll1_sys_clk and reparent pll1_sw_clk back to it
> > + * - Disable pll2_pfd2_396m_clk
> > + */
>
> [...]
>
> > +
> > +static struct cpufreq_driver imx6q_cpufreq_driver = {
> > + .verify = imx6q_verify_speed,
> > + .target = imx6q_set_target,
> > + .get = imx6q_get_speed,
> > + .init = imx6q_cpufreq_init,
> > + .exit = imx6q_cpufreq_exit,
> > + .name = "imx6q-cpufreq",
> > + .attr = imx6q_cpufreq_attr,
> > +};
> > +
> > +static int imx6q_cpufreq_probe(struct platform_device *pdev)
> > +{
> > + struct device_node *np;
> > + int ret;
> > +
> > + np = of_find_node_by_path("/cpus/cpu@0");
> > + if (!np) {
> > + pr_err("failed to find cpu0 node\n");
> > + return -ENOENT;
> > + }
> > +
> > + cpu_dev = get_cpu_device(0);
> > + if (!cpu_dev) {
> > + pr_err("failed to get cpu0 device\n");
>
> Since you have a struct device * you should use it throughout the driver
> to give the messages a bit more context.
>
In that case, the context will be "cpu cpu0: ...", while we have the
context be "imx6q-cpufreq: ..." right now, since we have the following
line at the very beginning of the code.
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
I think we have a better context with pr_* than dev_* in this case.
> > + ret = -ENODEV;
> > + goto put_node;
> > + }
> > +
> > + cpu_dev->of_node = np;
> > +
> > + arm_clk = clk_get(cpu_dev, "arm");
> > + pll1_sys_clk = clk_get(cpu_dev, "pll1_sys");
> > + pll1_sw_clk = clk_get(cpu_dev, "pll1_sw");
> > + step_clk = clk_get(cpu_dev, "step");
> > + pll2_pfd2_396m_clk = clk_get(cpu_dev, "pll2_pfd2_396m");
>
> devm_*?
>
That what I tried to do at the first time. But the dev of cpu is
special, and managed functions do not work as we expect with it.
Shawn
^ permalink raw reply
* [PATCH] clk: remove unreachable code
From: Rajagopal Venkat @ 2013-01-08 13:03 UTC (permalink / raw)
To: mturquette; +Cc: patches, linaro-dev, linux-pm, linux-kernel, Rajagopal Venkat
while reparenting a clock, NULL check is done for clock in
consideration and its new parent. So re-check is not required.
If done, else part becomes unreachable.
Signed-off-by: Rajagopal Venkat <rajagopal.venkat@linaro.org>
---
drivers/clk/clk.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 251e45d..f896584 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -1048,10 +1048,7 @@ void __clk_reparent(struct clk *clk, struct clk *new_parent)
hlist_del(&clk->child_node);
- if (new_parent)
- hlist_add_head(&clk->child_node, &new_parent->children);
- else
- hlist_add_head(&clk->child_node, &clk_orphan_list);
+ hlist_add_head(&clk->child_node, &new_parent->children);
#ifdef CONFIG_COMMON_CLK_DEBUG
if (!inited)
--
1.7.10.4
^ permalink raw reply related
* Re: [PATCH 2/3] cpufreq: add imx6q-cpufreq driver
From: Shawn Guo @ 2013-01-08 13:10 UTC (permalink / raw)
To: Sascha Hauer; +Cc: cpufreq, linux-pm, linux-arm-kernel, Rafael J. Wysocki
In-Reply-To: <20130108112856.GA19534@S2101-09.ap.freescale.net>
On Tue, Jan 08, 2013 at 07:28:59PM +0800, Shawn Guo wrote:
> > > + cpu_dev->of_node = np;
> > > +
> > > + arm_clk = clk_get(cpu_dev, "arm");
> > > + pll1_sys_clk = clk_get(cpu_dev, "pll1_sys");
> > > + pll1_sw_clk = clk_get(cpu_dev, "pll1_sw");
> > > + step_clk = clk_get(cpu_dev, "step");
> > > + pll2_pfd2_396m_clk = clk_get(cpu_dev, "pll2_pfd2_396m");
> >
> > devm_*?
> >
> That what I tried to do at the first time. But the dev of cpu is
> special, and managed functions do not work as we expect with it.
>
Inspired by the comment, I think I should attach /cpus/cpu@0 node to
pdev->dev instead of the one retrieved from get_cpu_device(0), so that
both managed functions and dev_* print can work.
Shawn
^ permalink raw reply
* Re: [PATCH 2/3] cpufreq: add imx6q-cpufreq driver
From: Shawn Guo @ 2013-01-08 14:35 UTC (permalink / raw)
To: Viresh Kumar
Cc: cpufreq, linux-pm, linux-arm-kernel, Rafael J. Wysocki,
Sascha Hauer
In-Reply-To: <CAOh2x=mv9_iWt+y9cP0oA4WKs86Ne9nRP6CgKy_1COnPm57YOA@mail.gmail.com>
On Tue, Jan 08, 2013 at 02:46:31PM +0530, Viresh Kumar wrote:
> Hi Shawn,
>
> Apart from Sascha's comments, please see few comment from my side:
>
> On Tue, Jan 8, 2013 at 12:24 PM, Shawn Guo <shawn.guo@linaro.org> wrote:
> > diff --git a/drivers/cpufreq/imx6q-cpufreq.c b/drivers/cpufreq/imx6q-cpufreq.c
> > +static struct regulator *arm_reg;
> > +static struct regulator *pu_reg;
> > +static struct regulator *soc_reg;
>
> Can move to a single line, if you like.
>
> > +static struct clk *arm_clk;
> > +static struct clk *pll1_sys_clk;
> > +static struct clk *pll1_sw_clk;
> > +static struct clk *step_clk;
> > +static struct clk *pll2_pfd2_396m_clk;
>
> same.
>
My personal taste is to have them on separate lines, so that it's
a little easier for reading and editing.
> > +static int imx6q_set_target(struct cpufreq_policy *policy,
> > + unsigned int target_freq, unsigned int relation)
> > +{
> > + struct cpufreq_freqs freqs;
> > + struct opp *opp;
> > + unsigned long freq_hz, volt, volt_old;
> > + unsigned int index, cpu;
> > + int ret;
> > +
> > + ret = cpufreq_frequency_table_target(policy, freq_table, target_freq,
> > + relation, &index);
> > + if (ret) {
> > + pr_err("failed to match target freqency %d: %d\n",
>
> s/freqency/frequency
>
Thanks.
> > + target_freq, ret);
> > + return ret;
> > + }
> > +
> > + freq_hz = freq_table[index].frequency * 1000;
> > + freqs.new = freq_hz / 1000;
>
> could be written as:
>
> freqs.new = freq_table[index].frequency;
> freq_hz = freqs.new * 1000;
>
Yeah, it looks better.
>
> > + freqs.old = clk_get_rate(arm_clk) / 1000;
> > +
> > + if (freqs.old == freqs.new)
> > + return 0;
> > +
> > + for_each_online_cpu(cpu) {
> > + freqs.cpu = cpu;
> > + cpufreq_notify_transition(&freqs, CPUFREQ_PRECHANGE);
> > + }
> > +
> > + opp = opp_find_freq_ceil(cpu_dev, &freq_hz);
> > + if (IS_ERR(opp)) {
> > + pr_err("failed to find OPP for %ld\n", freq_hz);
> > + return PTR_ERR(opp);
> > + }
> > +
> > + volt = opp_get_voltage(opp);
> > + volt_old = regulator_get_voltage(arm_reg);
> > +
> > + pr_debug("%u MHz, %ld mV --> %u MHz, %ld mV\n",
> > + freqs.old / 1000, volt_old / 1000,
> > + freqs.new / 1000, volt / 1000);
> > +
> > + /* scaling up? scale voltage before frequency */
> > + if (freqs.new > freqs.old) {
> > + ret = regulator_set_voltage_tol(arm_reg, volt, 0);
> > + if (ret) {
> > + pr_err("failed to scale voltage up: %d\n", ret);
> > + freqs.new = freqs.old;
>
> is this useful?
>
No, it's not. Will remove it.
> > + return ret;
> > + }
> > +
> > + /*
> > + * Need to increase vddpu and vddsoc for safety
> > + * if we are about to run at 1.2 GHz.
> > + */
> > + if (freqs.new == FREQ_1P2_GHZ / 1000) {
> > + regulator_set_voltage_tol(pu_reg,
> > + PU_SOC_VOLTAGE_HIGH, 0);
> > + regulator_set_voltage_tol(soc_reg,
> > + PU_SOC_VOLTAGE_HIGH, 0);
> > + }
> > + }
> > +
> > + /*
> > + * The setpoints are selected per PLL/PDF frequencies, so we need to
> > + * reprogram PLL for frequency scaling. The procedure of reprogramming
> > + * PLL1 is as blow.
> > + *
> > + * - Enable pll2_pfd2_396m_clk and reparent pll1_sw_clk to it
> > + * - Disable pll1_sys_clk and reprogram it
> > + * - Enable pll1_sys_clk and reparent pll1_sw_clk back to it
> > + * - Disable pll2_pfd2_396m_clk
> > + */
> > + clk_prepare_enable(pll2_pfd2_396m_clk);
> > + clk_set_parent(step_clk, pll2_pfd2_396m_clk);
> > + clk_set_parent(pll1_sw_clk, step_clk);
> > + clk_prepare_enable(pll1_sys_clk);
> > + if (freq_hz > clk_get_rate(pll2_pfd2_396m_clk)) {
> > + clk_disable_unprepare(pll1_sys_clk);
> > + clk_set_rate(pll1_sys_clk, freqs.new * 1000);
> > + clk_prepare_enable(pll1_sys_clk);
> > + clk_set_parent(pll1_sw_clk, pll1_sys_clk);
> > + clk_disable_unprepare(pll2_pfd2_396m_clk);
> > + } else {
> > + /*
> > + * Disable pll1_sys_clk if pll2_pfd2_396m_clk is sufficient
> > + * to provide the frequency.
> > + */
> > + clk_disable_unprepare(pll1_sys_clk);
>
> Are you doing anything in clk_[un]prepare ? If not, do clk_[un]prepare
> at init and exit and do enable/disable here.
>
Yes, we do power on/off PLL in clk_[un]prepare.
> > + }
> > +
> > + /* Ensure the arm clock divider is what we expect */
> > + ret = clk_set_rate(arm_clk, freqs.new * 1000);
> > + if (ret) {
> > + pr_err("failed to set clock rate: %d\n", ret);
> > + regulator_set_voltage_tol(arm_reg, volt_old, 0);
> > + return ret;
> > + }
> > +
> > + /* scaling down? scale voltage after frequency */
> > + if (freqs.new < freqs.old) {
> > + ret = regulator_set_voltage_tol(arm_reg, volt, 0);
> > + if (ret)
> > + pr_warn("failed to scale voltage down: %d\n", ret);
> > +
> > + if (freqs.old == FREQ_1P2_GHZ / 1000) {
> > + regulator_set_voltage_tol(pu_reg,
> > + PU_SOC_VOLTAGE_NORMAL, 0);
> > + regulator_set_voltage_tol(soc_reg,
> > + PU_SOC_VOLTAGE_NORMAL, 0);
> > + }
> > + }
> > +
> > + for_each_online_cpu(cpu) {
> > + freqs.cpu = cpu;
> > + cpufreq_notify_transition(&freqs, CPUFREQ_POSTCHANGE);
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int imx6q_cpufreq_init(struct cpufreq_policy *policy)
> > +{
> > + int ret;
> > +
> > + if (policy->cpu != 0)
> > + return -EINVAL;
>
> Why? The problem here is when you hot-unplug cpu0, init would be called for
> cpu1 and it would fail.
>
On imx6q, we can never hot-unplug cpu0. And all the cores of imx6q
share the clock and voltage, so have to scale together. That's why
we only need to do this init for cpu0.
> > +
> > + ret = cpufreq_frequency_table_cpuinfo(policy, freq_table);
> > + if (ret) {
> > + pr_err("invalid frequency table: %d\n", ret);
> > + return ret;
> > + }
> > +
> > + policy->cpuinfo.transition_latency = transition_latency;
> > + policy->cur = clk_get_rate(arm_clk) / 1000;
> > + policy->shared_type = CPUFREQ_SHARED_TYPE_ANY;
> > + cpumask_setall(policy->cpus);
> > + cpufreq_frequency_table_get_attr(freq_table, policy->cpu);
> > +
> > + return 0;
> > +}
> > +
> > +static int imx6q_cpufreq_exit(struct cpufreq_policy *policy)
> > +{
> > + cpufreq_frequency_table_put_attr(policy->cpu);
> > + return 0;
> > +}
> > +
> > +static struct freq_attr *imx6q_cpufreq_attr[] = {
> > + &cpufreq_freq_attr_scaling_available_freqs,
> > + NULL,
> > +};
> > +
> > +static struct cpufreq_driver imx6q_cpufreq_driver = {
> > + .verify = imx6q_verify_speed,
> > + .target = imx6q_set_target,
> > + .get = imx6q_get_speed,
> > + .init = imx6q_cpufreq_init,
> > + .exit = imx6q_cpufreq_exit,
> > + .name = "imx6q-cpufreq",
> > + .attr = imx6q_cpufreq_attr,
> > +};
> > +
> > +static int imx6q_cpufreq_probe(struct platform_device *pdev)
> > +{
> > + struct device_node *np;
> > + int ret;
> > +
> > + np = of_find_node_by_path("/cpus/cpu@0");
> > + if (!np) {
> > + pr_err("failed to find cpu0 node\n");
> > + return -ENOENT;
> > + }
> > +
> > + cpu_dev = get_cpu_device(0);
> > + if (!cpu_dev) {
> > + pr_err("failed to get cpu0 device\n");
> > + ret = -ENODEV;
> > + goto put_node;
> > + }
> > +
> > + cpu_dev->of_node = np;
> > +
> > + arm_clk = clk_get(cpu_dev, "arm");
> > + pll1_sys_clk = clk_get(cpu_dev, "pll1_sys");
> > + pll1_sw_clk = clk_get(cpu_dev, "pll1_sw");
> > + step_clk = clk_get(cpu_dev, "step");
> > + pll2_pfd2_396m_clk = clk_get(cpu_dev, "pll2_pfd2_396m");
> > + if (IS_ERR(arm_clk) || IS_ERR(pll1_sys_clk) || IS_ERR(pll1_sw_clk) ||
> > + IS_ERR(step_clk) || IS_ERR(pll2_pfd2_396m_clk)) {
> > + pr_err("failed to get clocks\n");
> > + ret = -ENOENT;
> > + goto put_node;
>
> That looks to be wrong. If clk failed for the last clk_get, you must
> free all other
> clks too.
Yes, you are right. I just figured out a way to use managed functions,
so it will not be a problem any more.
Shawn
>
> > + }
> > +
> > + arm_reg = regulator_get(cpu_dev, "arm");
> > + pu_reg = regulator_get(cpu_dev, "pu");
> > + soc_reg = regulator_get(cpu_dev, "soc");
> > + if (!arm_reg || !pu_reg || !soc_reg) {
> > + pr_err("failed to get regulators\n");
> > + ret = -ENOENT;
> > + goto put_clk;
>
> ditto
^ permalink raw reply
* Re: [PATCH 2/3] cpufreq: add imx6q-cpufreq driver
From: Viresh Kumar @ 2013-01-08 14:41 UTC (permalink / raw)
To: Shawn Guo
Cc: Rafael J. Wysocki, Sascha Hauer, linux-arm-kernel, cpufreq,
linux-pm
In-Reply-To: <20130108143554.GC19534@S2101-09.ap.freescale.net>
On 8 January 2013 20:05, Shawn Guo <shawn.guo@linaro.org> wrote:
>> > +static int imx6q_cpufreq_init(struct cpufreq_policy *policy)
>> > +{
>> > + int ret;
>> > +
>> > + if (policy->cpu != 0)
>> > + return -EINVAL;
>>
>> Why? The problem here is when you hot-unplug cpu0, init would be called for
>> cpu1 and it would fail.
>>
>
> On imx6q, we can never hot-unplug cpu0. And all the cores of imx6q
> share the clock and voltage, so have to scale together. That's why
> we only need to do this init for cpu0.
Ok, in that case too you don't need this code. init() wouldn't be
called for any other
cpu if policy->cpus is set correctly. So, that turns out to be a junk code.
^ permalink raw reply
* Re: [PATCH 1/3] PM / OPP: Export more symbols for module usage
From: Nishanth Menon @ 2013-01-08 14:47 UTC (permalink / raw)
To: Shawn Guo
Cc: cpufreq, linux-pm, linux-arm-kernel, Rafael J. Wysocki,
Sascha Hauer, Mark Langsdorf
In-Reply-To: <1357628043-19256-2-git-send-email-shawn.guo@linaro.org>
On 14:54-20130108, Shawn Guo wrote:
> Export opp_init_cpufreq_table and opp_free_cpufreq_table for module
> usage.
>
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> ---
> drivers/base/power/opp.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c
> index 50b2831..d16db9a 100644
> --- a/drivers/base/power/opp.c
> +++ b/drivers/base/power/opp.c
> @@ -661,6 +661,7 @@ int opp_init_cpufreq_table(struct device *dev,
>
> return 0;
> }
> +EXPORT_SYMBOL(opp_init_cpufreq_table);
>
> /**
> * opp_free_cpufreq_table() - free the cpufreq table
> @@ -678,6 +679,7 @@ void opp_free_cpufreq_table(struct device *dev,
> kfree(*table);
> *table = NULL;
> }
> +EXPORT_SYMBOL(opp_free_cpufreq_table);
> #endif /* CONFIG_CPU_FREQ */
Is'nt this already covered in
https://patchwork.kernel.org/patch/1847261/
?
--
Regards,
Nishanth Menon
^ permalink raw reply
* Re: [PATCH 2/3] cpufreq: add imx6q-cpufreq driver
From: Shawn Guo @ 2013-01-08 15:04 UTC (permalink / raw)
To: Viresh Kumar
Cc: cpufreq, linux-pm, linux-arm-kernel, Rafael J. Wysocki,
Sascha Hauer
In-Reply-To: <CAKohpomQuoUgPVp6e9_UrK0sk+z26cG36DoxH91LWSmBHD36BQ@mail.gmail.com>
On Tue, Jan 08, 2013 at 08:11:37PM +0530, Viresh Kumar wrote:
> On 8 January 2013 20:05, Shawn Guo <shawn.guo@linaro.org> wrote:
> >> > +static int imx6q_cpufreq_init(struct cpufreq_policy *policy)
> >> > +{
> >> > + int ret;
> >> > +
> >> > + if (policy->cpu != 0)
> >> > + return -EINVAL;
> >>
> >> Why? The problem here is when you hot-unplug cpu0, init would be called for
> >> cpu1 and it would fail.
> >>
> >
> > On imx6q, we can never hot-unplug cpu0. And all the cores of imx6q
> > share the clock and voltage, so have to scale together. That's why
> > we only need to do this init for cpu0.
>
> Ok, in that case too you don't need this code. init() wouldn't be
> called for any other
> cpu if policy->cpus is set correctly. So, that turns out to be a junk code.
I'm seeing that the .init() will be called on cpu1 when removing the
module.
Shawn
^ permalink raw reply
* Re: [PATCH 1/3] PM / OPP: Export more symbols for module usage
From: Shawn Guo @ 2013-01-08 15:06 UTC (permalink / raw)
To: Nishanth Menon
Cc: cpufreq, linux-pm, linux-arm-kernel, Rafael J. Wysocki,
Sascha Hauer, Mark Langsdorf
In-Reply-To: <20130108144720.GA6062@kahuna>
On Tue, Jan 08, 2013 at 08:47:21AM -0600, Nishanth Menon wrote:
> On 14:54-20130108, Shawn Guo wrote:
> > Export opp_init_cpufreq_table and opp_free_cpufreq_table for module
> > usage.
> >
> > Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> > ---
> > drivers/base/power/opp.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c
> > index 50b2831..d16db9a 100644
> > --- a/drivers/base/power/opp.c
> > +++ b/drivers/base/power/opp.c
> > @@ -661,6 +661,7 @@ int opp_init_cpufreq_table(struct device *dev,
> >
> > return 0;
> > }
> > +EXPORT_SYMBOL(opp_init_cpufreq_table);
> >
> > /**
> > * opp_free_cpufreq_table() - free the cpufreq table
> > @@ -678,6 +679,7 @@ void opp_free_cpufreq_table(struct device *dev,
> > kfree(*table);
> > *table = NULL;
> > }
> > +EXPORT_SYMBOL(opp_free_cpufreq_table);
> > #endif /* CONFIG_CPU_FREQ */
>
> Is'nt this already covered in
> https://patchwork.kernel.org/patch/1847261/
> ?
Sorry, I'm working against v3.8-rc and missed that. Will drop it.
Shawn
^ permalink raw reply
* Re: [PATCH 2/3] cpufreq: add imx6q-cpufreq driver
From: Viresh Kumar @ 2013-01-08 15:19 UTC (permalink / raw)
To: Shawn Guo
Cc: cpufreq, linux-pm, linux-arm-kernel, Rafael J. Wysocki,
Sascha Hauer
In-Reply-To: <20130108150357.GD19534@S2101-09.ap.freescale.net>
On 8 January 2013 20:34, Shawn Guo <shawn.guo@linaro.org> wrote:
> I'm seeing that the .init() will be called on cpu1 when removing the
> module.
Hmm... Can you give me sometime to think about how to fix that?
You can go ahead with keeping this change in v2.
^ permalink raw reply
* Re: [PATCH v6 3/4] block: implement runtime pm strategy
From: Alan Stern @ 2013-01-08 15:22 UTC (permalink / raw)
To: Aaron Lu
Cc: Jens Axboe, Rafael J. Wysocki, James Bottomley, linux-pm,
linux-scsi, linux-kernel, Aaron Lu, Shane Huang
In-Reply-To: <50EBCC93.5000203@intel.com>
On Tue, 8 Jan 2013, Aaron Lu wrote:
> >> --- a/include/linux/blkdev.h
> >> +++ b/include/linux/blkdev.h
> >> @@ -974,6 +974,40 @@ extern int blk_pre_runtime_suspend(struct request_queue *q);
> >> extern void blk_post_runtime_suspend(struct request_queue *q, int err);
> >> extern void blk_pre_runtime_resume(struct request_queue *q);
> >> extern void blk_post_runtime_resume(struct request_queue *q, int err);
> >> +
> >> +static inline void blk_pm_put_request(struct request *rq)
> >> +{
> >> + if (!(rq->cmd_flags & REQ_PM) && !--rq->q->nr_pending) {
> >> + pm_runtime_mark_last_busy(rq->q->dev);
> >> + pm_runtime_autosuspend(rq->q->dev);
> >> + }
> >> +}
> >> +
> >> +static inline struct request *blk_pm_peek_request(
> >> + struct request_queue *q, struct request *rq)
> >> +{
> >> + if (q->rpm_status == RPM_SUSPENDED ||
> >> + (q->rpm_status != RPM_ACTIVE && !(rq->cmd_flags & REQ_PM)))
> >> + return NULL;
> >> + else
> >> + return rq;
> >> +}
> >> +
> >> +static inline void blk_pm_requeue_request(struct request *rq)
> >> +{
> >> + if (!(rq->cmd_flags & REQ_PM))
> >> + rq->q->nr_pending--;
> >> +}
> >> +
> >> +static inline void blk_pm_add_request(struct request_queue *q,
> >> + struct request *rq)
> >> +{
> >> + if (!(rq->cmd_flags & REQ_PM) &&
> >> + q->nr_pending++ == 0 &&
> >> + (q->rpm_status == RPM_SUSPENDED ||
> >> + q->rpm_status == RPM_SUSPENDING))
> >> + pm_request_resume(q->dev);
> >> +}
> >
> > These routines also don't belong in include/linux. And they don't need
> > to be marked inline.
>
> OK, will move them.
>
> What about create a new file blk-pm.c for all these block pm related
> code?
Sure, go ahead if Jens doesn't mind. Alternatively, you could put each
of these functions in the file where it gets used.
Just as importantly, all of the public routines added in patch 2/4 to
blk-core.c should have kerneldoc explaining how and where to use them.
In particular, the kerneldoc for blk_pm_runtime_init() has to mention
that the block runtime PM implementation works only for drivers that
use request structures for their I/O; it doesn't work for drivers that
use bio's directly.
Alan Stern
^ permalink raw reply
* Re: [PATCH v6 0/4] block layer runtime pm
From: Alan Stern @ 2013-01-08 15:27 UTC (permalink / raw)
To: Aaron Lu
Cc: Jens Axboe, Rafael J. Wysocki, James Bottomley, linux-pm,
linux-scsi, linux-kernel, Aaron Lu, Shane Huang
In-Reply-To: <50EBCBC3.4070606@intel.com>
On Tue, 8 Jan 2013, Aaron Lu wrote:
> So this also reminds me that as long as CONFIG_PM_RUNTIME is selected,
> the blk_pm_add/put/peek_request functions will be in the block IO path.
> Shall we introduce a new config option to selectively build block
> runtime PM functionality? something like CONFIG_BLK_PM_RUNTIME perhaps?
>
> Just some condition checks in those functions, not sure if it is worth a
> new config though. Please suggest, thanks.
I don't think it is needed. The new routines will be very quick when
blk_pm_runtime_init() hasn't been called, once you add back the checks
for the queue's device.
Alan Stern
^ permalink raw reply
* Re: [PATCH v11 3/9] libata: identify and init ZPODD devices
From: Tejun Heo @ 2013-01-08 17:52 UTC (permalink / raw)
To: Aaron Lu
Cc: Jeff Garzik, James Bottomley, Rafael J. Wysocki, Alan Stern,
Aaron Lu, Jeff Wu, linux-ide, linux-pm, linux-scsi, linux-acpi
In-Reply-To: <50EBE1E2.5080406@intel.com>
Hello, Aaron.
On Tue, Jan 08, 2013 at 05:07:46PM +0800, Aaron Lu wrote:
> >> +struct zpodd {
> >> + bool slot:1;
> >> + bool drawer:1;
> >> +
> >> + struct ata_device *dev;
> >> +};
> >
> > Field names are usually indented. It would be nice to have a comment
>
> checkscript.sh doesn't seem like this if I put a tab around the ':'
>
> ERROR: spaces prohibited around that ':' (ctx:VxW)
> #222: FILE: include/uapi/linux/cdrom.h:915:
> + __u8 reserved1: 2;
> ^
>
> Which style should I follow?
struct zpodd {
bool slot:1;
bool drawer:1;
struct ata_device *dev;
};
> > explaining synchronization. Bitfields w/ their implicit RMW ops tend
> > to make people wonder about what the access rules are.
>
> The slot and drawer bit field is assigned only once during ata probe
> time in EH context, and accessed later in PM's callback context.
> Not sure what access rule should I describe...
/* init during probe, RO afterwards */ should do but I'd prefer if you
stay away from bitfields altogether. There are cases where bitfields
are okay but when you're working across multiple locking domains, it
usually is a bad idea because the code which accesses those fields
look completely independent while still being able to interact with
each other. They look properly synchronized until you realized they
live on the same machine word.
> >> +/*
> >> + * Per the spec, only slot type and drawer type ODD can be supported
> >> + *
> >> + * Return 0 for slot type, 1 for drawer, -ENODEV for other types or on error.
> >> + */
> >
> > Maybe bool odd_has_drawer() is better?
>
> There are other types of ODD other than slot and drawer, and both slot
> and drawer type ODDs can be supported for ZPODD. So a bool can't convey
> such information :-)
Then please make it a proper ATA enum and use it in struct zpodd too.
Thanks!
--
tejun
^ permalink raw reply
* [PATCH] Thermal: Fix to use read critical temperature when required
From: Amit Daniel Kachhap @ 2013-01-09 0:18 UTC (permalink / raw)
To: linux-pm, Zhang Rui; +Cc: linux-kernel, durgadoss.r
This patch modifies the code to use get_crit_temp instead of
the normal get_trip_temp when critical threshold point is crossed
or queried about.
Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
---
drivers/thermal/thermal_sys.c | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
index ecdfc7d..0dc6403 100644
--- a/drivers/thermal/thermal_sys.c
+++ b/drivers/thermal/thermal_sys.c
@@ -345,7 +345,10 @@ static void handle_critical_trips(struct thermal_zone_device *tz,
{
long trip_temp;
- tz->ops->get_trip_temp(tz, trip, &trip_temp);
+ if (tz->ops->get_crit_temp)
+ tz->ops->get_crit_temp(tz, &trip_temp);
+ else
+ tz->ops->get_trip_temp(tz, trip, &trip_temp);
/* If we have not crossed the trip_temp, we do not care. */
if (tz->temperature < trip_temp)
@@ -550,6 +553,7 @@ trip_point_temp_show(struct device *dev, struct device_attribute *attr,
struct thermal_zone_device *tz = to_thermal_zone(dev);
int trip, ret;
long temperature;
+ enum thermal_trip_type type;
if (!tz->ops->get_trip_temp)
return -EPERM;
@@ -557,7 +561,14 @@ trip_point_temp_show(struct device *dev, struct device_attribute *attr,
if (!sscanf(attr->attr.name, "trip_point_%d_temp", &trip))
return -EINVAL;
- ret = tz->ops->get_trip_temp(tz, trip, &temperature);
+ ret = tz->ops->get_trip_type(tz, trip, &type);
+ if (ret)
+ return ret;
+
+ if (type == THERMAL_TRIP_CRITICAL && tz->ops->get_crit_temp)
+ ret = tz->ops->get_crit_temp(tz, &temperature);
+ else
+ ret = tz->ops->get_trip_temp(tz, trip, &temperature);
if (ret)
return ret;
--
1.7.10.4
^ permalink raw reply related
* Re: [PATCH v11 3/9] libata: identify and init ZPODD devices
From: Aaron Lu @ 2013-01-09 3:20 UTC (permalink / raw)
To: Tejun Heo
Cc: Jeff Garzik, James Bottomley, Rafael J. Wysocki, Alan Stern,
Aaron Lu, Jeff Wu, linux-ide, linux-pm, linux-scsi, linux-acpi
In-Reply-To: <20130108175202.GZ3926@htj.dyndns.org>
Hi Tejun,
On 01/09/2013 01:52 AM, Tejun Heo wrote:
> Hello, Aaron.
>
> On Tue, Jan 08, 2013 at 05:07:46PM +0800, Aaron Lu wrote:
>>>> +struct zpodd {
>>>> + bool slot:1;
>>>> + bool drawer:1;
>>>> +
>>>> + struct ata_device *dev;
>>>> +};
>>>
>>> Field names are usually indented. It would be nice to have a comment
>>
>> checkscript.sh doesn't seem like this if I put a tab around the ':'
>>
>> ERROR: spaces prohibited around that ':' (ctx:VxW)
>> #222: FILE: include/uapi/linux/cdrom.h:915:
>> + __u8 reserved1: 2;
>> ^
>>
>> Which style should I follow?
>
> struct zpodd {
> bool slot:1;
> bool drawer:1;
>
> struct ata_device *dev;
> };
Oh, got it :-)
>
>>> explaining synchronization. Bitfields w/ their implicit RMW ops tend
>>> to make people wonder about what the access rules are.
>>
>> The slot and drawer bit field is assigned only once during ata probe
>> time in EH context, and accessed later in PM's callback context.
>> Not sure what access rule should I describe...
>
> /* init during probe, RO afterwards */ should do but I'd prefer if you
> stay away from bitfields altogether. There are cases where bitfields
> are okay but when you're working across multiple locking domains, it
> usually is a bad idea because the code which accesses those fields
> look completely independent while still being able to interact with
> each other. They look properly synchronized until you realized they
> live on the same machine word.
Then I will not use bitfields, I was just thinking to save some space
and didn't realize it would bring such nasty problems...
>
>>>> +/*
>>>> + * Per the spec, only slot type and drawer type ODD can be supported
>>>> + *
>>>> + * Return 0 for slot type, 1 for drawer, -ENODEV for other types or on error.
>>>> + */
>>>
>>> Maybe bool odd_has_drawer() is better?
>>
>> There are other types of ODD other than slot and drawer, and both slot
>> and drawer type ODDs can be supported for ZPODD. So a bool can't convey
>> such information :-)
>
> Then please make it a proper ATA enum and use it in struct zpodd too.
Sure, and thanks for all the suggestions!
-Aaron
^ permalink raw reply
* Re: [PATCH 2/5 RESEND] thermal: exynos: Miscellaneous fixes to support falling threshold interrupt
From: amit kachhap @ 2013-01-09 4:30 UTC (permalink / raw)
To: Joe Perches
Cc: linux-pm, linux-pm, Zhang Rui, linux-samsung-soc, linux-kernel
In-Reply-To: <1357516522.4940.8.camel@joe-AO722>
Hi Joe,
Thanks for the review. Will re-post with your suggestion,
On Sun, Jan 6, 2013 at 3:55 PM, Joe Perches <joe@perches.com> wrote:
> On Sun, 2013-01-06 at 15:50 -0800, Amit Daniel Kachhap wrote:
>> Below fixes are done to support falling threshold interrupt,
>> * Falling interrupt status macro corrected according to exynos5 data sheet.
>> * The get trend function modified to calculate trip temperature correctly.
>> * The clearing of interrupt status in the isr is now done after handling
>> the event that caused the interrupt.
> []
>> diff --git a/drivers/thermal/exynos_thermal.c b/drivers/thermal/exynos_thermal.c
> []
>> @@ -373,12 +373,19 @@ static int exynos_get_temp(struct thermal_zone_device *thermal,
>> static int exynos_get_trend(struct thermal_zone_device *thermal,
>> int trip, enum thermal_trend *trend)
>> {
>> - if (thermal->temperature >= trip)
>> + int ret = 0;
Yes agreed. Will modify it.
>
> Unnecessary initialization
>
>> + unsigned long trip_temp;
>> +
>> + ret = exynos_get_trip_temp(thermal, trip, &trip_temp);
>> + if (ret < 0)
>> + return ret;
>> +
>> + if (thermal->temperature >= trip_temp)
>> *trend = THERMAL_TREND_RAISING;
>> else
>> *trend = THERMAL_TREND_DROPPING;
>
> THERMAL_TREND_STABLE ?
Only 2 trend is sufficient. It is stable for some time as the falling
threshold interrupt is some units below the trip temp.
>
>>
>> - return 0;
>> + return ret;
Ok agreed
>
> return 0 is clearer.
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v11 7/9] libata: expose pm qos flags for ata device
From: Aaron Lu @ 2013-01-09 5:11 UTC (permalink / raw)
To: Tejun Heo
Cc: Jeff Garzik, James Bottomley, Rafael J. Wysocki, Alan Stern,
Aaron Lu, Jeff Wu, linux-ide, linux-pm, linux-scsi, linux-acpi
In-Reply-To: <20130107184358.GT3926@htj.dyndns.org>
On 01/08/2013 02:43 AM, Tejun Heo wrote:
> On Sun, Jan 06, 2013 at 10:48:27AM +0800, Aaron Lu wrote:
>> Expose pm qos flags to user space so that user has a chance to disable
>> pm features like power off, if he/she has a broken platform or devices
>> or simply does not like this pm feature.
>>
>> This flag is exposed to user space only for ata device or atapi device
>> that is zero power capable. For normal atapi device, it will never be
>> powered off.
>>
>> Signed-off-by: Aaron Lu <aaron.lu@intel.com>
>> ---
>> drivers/ata/libata-acpi.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/ata/libata-acpi.c b/drivers/ata/libata-acpi.c
>> index e832bf6..5b67be3 100644
>> --- a/drivers/ata/libata-acpi.c
>> +++ b/drivers/ata/libata-acpi.c
>> @@ -17,6 +17,7 @@
>> #include <linux/pci.h>
>> #include <linux/slab.h>
>> #include <linux/pm_runtime.h>
>> +#include <linux/pm_qos.h>
>> #include <scsi/scsi_device.h>
>> #include "libata.h"
>>
>> @@ -1022,6 +1023,8 @@ static void ata_acpi_unregister_power_resource(struct ata_device *dev)
>> void ata_acpi_bind(struct ata_device *dev)
>> {
>> ata_acpi_register_power_resource(dev);
>> + if (dev->class == ATA_DEV_ATA || zpodd_dev_enabled(dev))
>> + dev_pm_qos_expose_flags(&dev->sdev->sdev_gendev, 0);
>> }
>
> Why from ata_acpi_bind()?
I think I should add a check to see if platform can power off the device
before export this qos flag. The check is done by ACPI, and the qos flag
is for the scsi device. So looks like this is the proper place?
Thanks,
Aaron
^ permalink raw reply
* Re: [PATCH] clk: remove unreachable code
From: Tushar Behera @ 2013-01-09 5:50 UTC (permalink / raw)
To: Rajagopal Venkat; +Cc: mturquette, patches, linaro-dev, linux-pm, linux-kernel
In-Reply-To: <1357650191-30842-1-git-send-email-rajagopal.venkat@linaro.org>
On 01/08/2013 06:33 PM, Rajagopal Venkat wrote:
> while reparenting a clock, NULL check is done for clock in
> consideration and its new parent. So re-check is not required.
> If done, else part becomes unreachable.
>
> Signed-off-by: Rajagopal Venkat <rajagopal.venkat@linaro.org>
> ---
> drivers/clk/clk.c | 5 +----
> 1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index 251e45d..f896584 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -1048,10 +1048,7 @@ void __clk_reparent(struct clk *clk, struct clk *new_parent)
>
> hlist_del(&clk->child_node);
>
> - if (new_parent)
> - hlist_add_head(&clk->child_node, &new_parent->children);
> - else
> - hlist_add_head(&clk->child_node, &clk_orphan_list);
> + hlist_add_head(&clk->child_node, &new_parent->children);
>
> #ifdef CONFIG_COMMON_CLK_DEBUG
> if (!inited)
>
The same logic holds good for following piece of code too.
1060 |-------if (new_parent)
1061 |-------|-------new_parent_d = new_parent->dentry;
1062 |-------else
1063 |-------|-------new_parent_d = orphandir;
--
Tushar Behera
^ permalink raw reply
* Re: [PATCH] clk: remove unreachable code
From: Rajagopal Venkat @ 2013-01-09 5:58 UTC (permalink / raw)
To: Tushar Behera
Cc: linaro-dev-cunTk1MwBs8s++Sfvej+rw,
linux-pm-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
patches-QSEj5FYQhm4dnm+yROfE0A
In-Reply-To: <50ED0517.5090907-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
On 9 January 2013 11:20, Tushar Behera <tushar.behera-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> On 01/08/2013 06:33 PM, Rajagopal Venkat wrote:
>> while reparenting a clock, NULL check is done for clock in
>> consideration and its new parent. So re-check is not required.
>> If done, else part becomes unreachable.
>>
>> Signed-off-by: Rajagopal Venkat <rajagopal.venkat-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>> ---
>> drivers/clk/clk.c | 5 +----
>> 1 file changed, 1 insertion(+), 4 deletions(-)
>>
>> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
>> index 251e45d..f896584 100644
>> --- a/drivers/clk/clk.c
>> +++ b/drivers/clk/clk.c
>> @@ -1048,10 +1048,7 @@ void __clk_reparent(struct clk *clk, struct clk *new_parent)
>>
>> hlist_del(&clk->child_node);
>>
>> - if (new_parent)
>> - hlist_add_head(&clk->child_node, &new_parent->children);
>> - else
>> - hlist_add_head(&clk->child_node, &clk_orphan_list);
>> + hlist_add_head(&clk->child_node, &new_parent->children);
>>
>> #ifdef CONFIG_COMMON_CLK_DEBUG
>> if (!inited)
>>
>
> The same logic holds good for following piece of code too.
>
> 1060 |-------if (new_parent)
> 1061 |-------|-------new_parent_d = new_parent->dentry;
> 1062 |-------else
> 1063 |-------|-------new_parent_d = orphandir;
Yes. Thanks for pointing out.
>
>
> --
> Tushar Behera
--
Regards,
Rajagopal
^ permalink raw reply
* [PATCH] clk: remove unreachable code
From: Rajagopal Venkat @ 2013-01-09 6:29 UTC (permalink / raw)
To: mturquette; +Cc: patches, linaro-dev, linux-pm, linux-kernel, Rajagopal Venkat
In-Reply-To: <1357650191-30842-1-git-send-email-rajagopal.venkat@linaro.org>
while reparenting a clock, NULL check is done for clock in
consideration and its new parent. So re-check is not required.
If done, else part becomes unreachable.
Signed-off-by: Rajagopal Venkat <rajagopal.venkat@linaro.org>
---
drivers/clk/clk.c | 13 ++-----------
1 file changed, 2 insertions(+), 11 deletions(-)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 251e45d..1c4097c 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -1040,7 +1040,6 @@ void __clk_reparent(struct clk *clk, struct clk *new_parent)
{
#ifdef CONFIG_COMMON_CLK_DEBUG
struct dentry *d;
- struct dentry *new_parent_d;
#endif
if (!clk || !new_parent)
@@ -1048,22 +1047,14 @@ void __clk_reparent(struct clk *clk, struct clk *new_parent)
hlist_del(&clk->child_node);
- if (new_parent)
- hlist_add_head(&clk->child_node, &new_parent->children);
- else
- hlist_add_head(&clk->child_node, &clk_orphan_list);
+ hlist_add_head(&clk->child_node, &new_parent->children);
#ifdef CONFIG_COMMON_CLK_DEBUG
if (!inited)
goto out;
- if (new_parent)
- new_parent_d = new_parent->dentry;
- else
- new_parent_d = orphandir;
-
d = debugfs_rename(clk->dentry->d_parent, clk->dentry,
- new_parent_d, clk->name);
+ new_parent->dentry, clk->name);
if (d)
clk->dentry = d;
else
--
1.7.10.4
^ permalink raw reply related
* Re: [PATCH] clk: remove unreachable code
From: Tushar Behera @ 2013-01-09 6:35 UTC (permalink / raw)
To: Rajagopal Venkat; +Cc: mturquette, patches, linaro-dev, linux-pm, linux-kernel
In-Reply-To: <1357712988-22317-1-git-send-email-rajagopal.venkat@linaro.org>
On 01/09/2013 11:59 AM, Rajagopal Venkat wrote:
> while reparenting a clock, NULL check is done for clock in
> consideration and its new parent. So re-check is not required.
> If done, else part becomes unreachable.
>
> Signed-off-by: Rajagopal Venkat <rajagopal.venkat@linaro.org>
> ---
It is good to have revision history of the patches (version number and
changelog).
> drivers/clk/clk.c | 13 ++-----------
> 1 file changed, 2 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index 251e45d..1c4097c 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -1040,7 +1040,6 @@ void __clk_reparent(struct clk *clk, struct clk *new_parent)
> {
> #ifdef CONFIG_COMMON_CLK_DEBUG
> struct dentry *d;
> - struct dentry *new_parent_d;
> #endif
>
> if (!clk || !new_parent)
> @@ -1048,22 +1047,14 @@ void __clk_reparent(struct clk *clk, struct clk *new_parent)
>
> hlist_del(&clk->child_node);
>
> - if (new_parent)
> - hlist_add_head(&clk->child_node, &new_parent->children);
> - else
> - hlist_add_head(&clk->child_node, &clk_orphan_list);
> + hlist_add_head(&clk->child_node, &new_parent->children);
>
> #ifdef CONFIG_COMMON_CLK_DEBUG
> if (!inited)
> goto out;
>
> - if (new_parent)
> - new_parent_d = new_parent->dentry;
> - else
> - new_parent_d = orphandir;
> -
> d = debugfs_rename(clk->dentry->d_parent, clk->dentry,
> - new_parent_d, clk->name);
> + new_parent->dentry, clk->name);
> if (d)
> clk->dentry = d;
> else
>
--
Tushar Behera
^ permalink raw reply
* RE: [PATCH v11 0/9] ZPODD Patches
From: Wu, Jeff @ 2013-01-09 7:55 UTC (permalink / raw)
To: Aaron Lu, Jeff Garzik, James Bottomley, Rafael J. Wysocki,
Alan Stern, Tejun Heo
Cc: Aaron Lu, linux-ide@vger.kernel.org, linux-pm@vger.kernel.org,
linux-scsi@vger.kernel.org, linux-acpi@vger.kernel.org
In-Reply-To: <1357440509-28108-1-git-send-email-aaron.lu@intel.com>
> -----Original Message-----
> From: linux-scsi-owner@vger.kernel.org [mailto:linux-scsi-
> owner@vger.kernel.org] On Behalf Of Aaron Lu
> Sent: Sunday, January 06, 2013 10:48 AM
> To: Jeff Garzik; James Bottomley; Rafael J. Wysocki; Alan Stern; Tejun Heo
> Cc: Aaron Lu; Wu, Jeff; linux-ide@vger.kernel.org; linux-pm@vger.kernel.org;
> linux-scsi@vger.kernel.org; linux-acpi@vger.kernel.org; Aaron Lu
> Subject: [PATCH v11 0/9] ZPODD Patches
>
> v11:
> Introduce event_driven flag in scsi_device to silence the media event poll
> after ODD is powered off; Removed ata layer PM QOS control, instead,
> simply limit ACPI state to D3_HOT when choosing state; Make the power off
> delay a module param named zpodd_poweroff_delay, defaults to 30
> seconds.
>
1. Tray type ZPODD , test pass;
2. Slot type ZPODD , test pass;
Tested-by: Jeff Wu <jeff.wu@amd.com>
> v10:
> Introduce PM_QOS_NO_POLL flag to skip calling disk's events_check callback;
> Do not use zero power ready hint information from event poll; Check
> attached device in port's runtime idle callback to decide if suspend is desired;
> Address various comments from Tejun Heo.
>
> v9:
> Build ZPODD as part of libata instead of another standalone module as it is
> tightly related to other libata files.
> Identify and init ZPODD during probe time instead of after SCSI device is
> created as suggested by Tejun Heo.
> Make use of pm qos flag to give ACPI hint when choosing ACPI state.
> Expose qos flag to give user control of whether power off is allowed.
>
> This patchset used Rafael's pm-qos work:
> git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git pm-qos
>
> v8:
> This version is a redesign, it doesn't have much to do with previous versions.
> The ZPODD implementation is done almost entirely in ATA layer now, except
> 2 helper functions from SCSI sr driver to block disk events.
>
> The basic idea is that, when ata port is runtime suspended, it will check if the
> ODD is ready to be powered off. And if yes, events is blocked and power
> omitted; if not, ODD's power supply remains unchanged by keeping ACPI
> state at D0.
>
> Some background knowledge about ZPODD is added below v1 history log.
>
> v7:
> Re work of runtime pm of sr driver, based on ideas of Alan Stern and Oliver
> Neukum.
>
> Jeff, due to the ready_to_power_off flag added, there is a small change in
> [PATCH v7 6/6] libata: acpi: respect may_power_off flag, please check if I can
> still get your ack, thanks.
>
> v6:
> When user changes may_power_off flag through sysfs entry and if device is
> already runtime suspended, resume resume it so that it can respect this flag
> next time it is runtime suspended as suggested by Alan Stern.
> Call scsi_autopm_get/put_device once in sr_check_events as suggested by
> Alan Stern.
>
> v5:
> Add may_power_off flag to scsi device.
> Alan Stern suggested that I should not mess runtime suspend with runtime
> power off, but the current zpodd implementation made it not easy to
> seperate. So I re-wrote the zpodd implementation, the end result is, normal
> ODD can also enter runtime suspended state, but their power won't be
> removed.
>
> v4:
> Rebase on top of Linus' tree, due to this, the problem of a missing flag in v3 is
> gone; Add a new function scsi_autopm_put_device_autosuspend to first
> mark last busy for the device and then put autosuspend it as suggested by
> Oliver Neukum.
> Typo fix as pointed by Sergei Shtylyov.
> Check can_power_off flag before any runtime pm operations in sr.
>
> v3:
> Rebase on top of scsi-misc tree;
> Add the sr related patches previously in Jeff's libata tree; Re-organize the sr
> patches.
> A problem for now: for patch
> scsi: sr: support zero power ODD(ZPODD)
> I can't set a flag in libata-acpi.c since a related function is missing in scsi-misc
> tree. Will fix this when 3.6-rc1 released.
>
> v2:
> Bug fix for v1;
> Use scsi_autopm_* in sr driver instead of pm_runtime_*;
>
> v1:
> Here are some patches to make ZPODD easier to use for end users and a fix
> for using ZPODD with system suspend.
>
> Some background knowledge about ZPODD:
> ODD means Optical Disc Drive.
> ZPODD means Zero Power ODD, it is a mechanism to place the ODD into zero
> power state when the system is running at S0 system state without user's
> awareness.
> It achieved this by ACPI and SATA device attention pin. For power off, normal
> ACPI control method is used to place the device into D3 cold ACPI device
> state, aka. device power supply omitted. For power on, when user press the
> eject button of a drawer type ODD or when user inserts an ODD into a slot
> type ODD, the device attention pin will trigger. In the current x86
> implementation, this pin will connect to a GPE, and the GPE will trigger an
> ACPI interrupt. With our pre-registered ACPI notification code, the device
> can be runtime resumed, and we place the device back to full power state by
> setting its ACPI state to D0. The whole process is transparent to the end user.
>
> Aaron Lu (9):
> scsi: sr: support runtime pm
> libata: Add CONFIG_SATA_ZPODD
> libata: identify and init ZPODD devices
> libata: move acpi notification code to zpodd
> libata: check zero power ready status for ZPODD
> libata: handle power transition of ODD
> libata: expose pm qos flags for ata device
> libata: no poll when ODD is powered off
> libata: do not suspend port if normal ODD is attached
>
> drivers/ata/Kconfig | 13 +++
> drivers/ata/Makefile | 1 +
> drivers/ata/libata-acpi.c | 111 +++++------------- drivers/ata/libata-core.c |
> 23 +++-
> drivers/ata/libata-eh.c | 12 +-
> drivers/ata/libata-zpodd.c | 281
> +++++++++++++++++++++++++++++++++++++++++++++
> drivers/ata/libata.h | 27 +++++
> drivers/scsi/sr.c | 36 +++++-
> include/linux/libata.h | 3 +
> include/scsi/scsi_device.h | 1 +
> include/uapi/linux/cdrom.h | 34 ++++++
> 11 files changed, 452 insertions(+), 90 deletions(-) create mode 100644
> drivers/ata/libata-zpodd.c
>
> --
> 1.7.11.7
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the
> body of a message to majordomo@vger.kernel.org More majordomo info at
> http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v11 0/9] ZPODD Patches
From: Aaron Lu @ 2013-01-09 9:07 UTC (permalink / raw)
To: Wu, Jeff
Cc: Jeff Garzik, James Bottomley, Rafael J. Wysocki, Alan Stern,
Tejun Heo, Aaron Lu, linux-ide@vger.kernel.org,
linux-pm@vger.kernel.org, linux-scsi@vger.kernel.org,
linux-acpi@vger.kernel.org
In-Reply-To: <5180B254989D2842B6BD8114CBC703AC0AEA7F@SCYBEXDAG03.amd.com>
On 01/09/2013 03:55 PM, Wu, Jeff wrote:
>
>
>> -----Original Message-----
>> From: linux-scsi-owner@vger.kernel.org [mailto:linux-scsi-
>> owner@vger.kernel.org] On Behalf Of Aaron Lu
>> Sent: Sunday, January 06, 2013 10:48 AM
>> To: Jeff Garzik; James Bottomley; Rafael J. Wysocki; Alan Stern; Tejun Heo
>> Cc: Aaron Lu; Wu, Jeff; linux-ide@vger.kernel.org; linux-pm@vger.kernel.org;
>> linux-scsi@vger.kernel.org; linux-acpi@vger.kernel.org; Aaron Lu
>> Subject: [PATCH v11 0/9] ZPODD Patches
>>
>> v11:
>> Introduce event_driven flag in scsi_device to silence the media event poll
>> after ODD is powered off; Removed ata layer PM QOS control, instead,
>> simply limit ACPI state to D3_HOT when choosing state; Make the power off
>> delay a module param named zpodd_poweroff_delay, defaults to 30
>> seconds.
>>
>
> 1. Tray type ZPODD , test pass;
> 2. Slot type ZPODD , test pass;
>
> Tested-by: Jeff Wu <jeff.wu@amd.com>
Cool! Thanks a lot for your test.
-Aaron
>
>
>> v10:
>> Introduce PM_QOS_NO_POLL flag to skip calling disk's events_check callback;
>> Do not use zero power ready hint information from event poll; Check
>> attached device in port's runtime idle callback to decide if suspend is desired;
>> Address various comments from Tejun Heo.
>>
>> v9:
>> Build ZPODD as part of libata instead of another standalone module as it is
>> tightly related to other libata files.
>> Identify and init ZPODD during probe time instead of after SCSI device is
>> created as suggested by Tejun Heo.
>> Make use of pm qos flag to give ACPI hint when choosing ACPI state.
>> Expose qos flag to give user control of whether power off is allowed.
>>
>> This patchset used Rafael's pm-qos work:
>> git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git pm-qos
>>
>> v8:
>> This version is a redesign, it doesn't have much to do with previous versions.
>> The ZPODD implementation is done almost entirely in ATA layer now, except
>> 2 helper functions from SCSI sr driver to block disk events.
>>
>> The basic idea is that, when ata port is runtime suspended, it will check if the
>> ODD is ready to be powered off. And if yes, events is blocked and power
>> omitted; if not, ODD's power supply remains unchanged by keeping ACPI
>> state at D0.
>>
>> Some background knowledge about ZPODD is added below v1 history log.
>>
>> v7:
>> Re work of runtime pm of sr driver, based on ideas of Alan Stern and Oliver
>> Neukum.
>>
>> Jeff, due to the ready_to_power_off flag added, there is a small change in
>> [PATCH v7 6/6] libata: acpi: respect may_power_off flag, please check if I can
>> still get your ack, thanks.
>>
>> v6:
>> When user changes may_power_off flag through sysfs entry and if device is
>> already runtime suspended, resume resume it so that it can respect this flag
>> next time it is runtime suspended as suggested by Alan Stern.
>> Call scsi_autopm_get/put_device once in sr_check_events as suggested by
>> Alan Stern.
>>
>> v5:
>> Add may_power_off flag to scsi device.
>> Alan Stern suggested that I should not mess runtime suspend with runtime
>> power off, but the current zpodd implementation made it not easy to
>> seperate. So I re-wrote the zpodd implementation, the end result is, normal
>> ODD can also enter runtime suspended state, but their power won't be
>> removed.
>>
>> v4:
>> Rebase on top of Linus' tree, due to this, the problem of a missing flag in v3 is
>> gone; Add a new function scsi_autopm_put_device_autosuspend to first
>> mark last busy for the device and then put autosuspend it as suggested by
>> Oliver Neukum.
>> Typo fix as pointed by Sergei Shtylyov.
>> Check can_power_off flag before any runtime pm operations in sr.
>>
>> v3:
>> Rebase on top of scsi-misc tree;
>> Add the sr related patches previously in Jeff's libata tree; Re-organize the sr
>> patches.
>> A problem for now: for patch
>> scsi: sr: support zero power ODD(ZPODD)
>> I can't set a flag in libata-acpi.c since a related function is missing in scsi-misc
>> tree. Will fix this when 3.6-rc1 released.
>>
>> v2:
>> Bug fix for v1;
>> Use scsi_autopm_* in sr driver instead of pm_runtime_*;
>>
>> v1:
>> Here are some patches to make ZPODD easier to use for end users and a fix
>> for using ZPODD with system suspend.
>>
>> Some background knowledge about ZPODD:
>> ODD means Optical Disc Drive.
>> ZPODD means Zero Power ODD, it is a mechanism to place the ODD into zero
>> power state when the system is running at S0 system state without user's
>> awareness.
>> It achieved this by ACPI and SATA device attention pin. For power off, normal
>> ACPI control method is used to place the device into D3 cold ACPI device
>> state, aka. device power supply omitted. For power on, when user press the
>> eject button of a drawer type ODD or when user inserts an ODD into a slot
>> type ODD, the device attention pin will trigger. In the current x86
>> implementation, this pin will connect to a GPE, and the GPE will trigger an
>> ACPI interrupt. With our pre-registered ACPI notification code, the device
>> can be runtime resumed, and we place the device back to full power state by
>> setting its ACPI state to D0. The whole process is transparent to the end user.
>>
>> Aaron Lu (9):
>> scsi: sr: support runtime pm
>> libata: Add CONFIG_SATA_ZPODD
>> libata: identify and init ZPODD devices
>> libata: move acpi notification code to zpodd
>> libata: check zero power ready status for ZPODD
>> libata: handle power transition of ODD
>> libata: expose pm qos flags for ata device
>> libata: no poll when ODD is powered off
>> libata: do not suspend port if normal ODD is attached
>>
>> drivers/ata/Kconfig | 13 +++
>> drivers/ata/Makefile | 1 +
>> drivers/ata/libata-acpi.c | 111 +++++------------- drivers/ata/libata-core.c |
>> 23 +++-
>> drivers/ata/libata-eh.c | 12 +-
>> drivers/ata/libata-zpodd.c | 281
>> +++++++++++++++++++++++++++++++++++++++++++++
>> drivers/ata/libata.h | 27 +++++
>> drivers/scsi/sr.c | 36 +++++-
>> include/linux/libata.h | 3 +
>> include/scsi/scsi_device.h | 1 +
>> include/uapi/linux/cdrom.h | 34 ++++++
>> 11 files changed, 452 insertions(+), 90 deletions(-) create mode 100644
>> drivers/ata/libata-zpodd.c
>>
>> --
>> 1.7.11.7
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the
>> body of a message to majordomo@vger.kernel.org More majordomo info at
>> http://vger.kernel.org/majordomo-info.html
>
>
^ permalink raw reply
* RE: [PATCH 7/9] Thermal: Make PER_ZONE values configurable
From: R, Durgadoss @ 2013-01-09 9:12 UTC (permalink / raw)
To: Greg KH
Cc: Zhang, Rui, linux-pm@vger.kernel.org,
linux-kernel@vger.kernel.org, eduardo.valentin@ti.com,
hongbo.zhang@linaro.org, wni@nvidia.com
In-Reply-To: <20130107192414.GC4465@kroah.com>
Hi Greg,
> -----Original Message-----
> From: Greg KH [mailto:gregkh@linuxfoundation.org]
> Sent: Tuesday, January 08, 2013 12:54 AM
> To: R, Durgadoss
> Cc: Zhang, Rui; linux-pm@vger.kernel.org; linux-kernel@vger.kernel.org;
> eduardo.valentin@ti.com; hongbo.zhang@linaro.org; wni@nvidia.com
> Subject: Re: [PATCH 7/9] Thermal: Make PER_ZONE values configurable
>
> On Mon, Jan 07, 2013 at 12:43:24PM +0530, Durgadoss R wrote:
> > This patch makes MAX_SENSORS_PER_ZONE and
> > MAX_CDEVS_PER_ZONE values configurable. The
> > default value is 1, and range is 1-12.
>
> Why would we ever want to change this? Why make this configurable at
> all, how is a distro supposed to set this value?
>
> Shouldn't it be specified from the driver itself?
These are platform level parameters, that can differ for various platforms.
(Mostly due to board design and thermistor layouts). Stand-alone thermal
sensor drivers are not (need not be) aware of these values.
That's why these values are made configurable.
Thanks,
Durga
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox