* Re: [PATCH v3 1/2] thermal: exynos: Fix wrong bit to control tmu core
From: Zhang Rui @ 2012-11-20 5:53 UTC (permalink / raw)
To: Kyungmin Park
Cc: Jonghwan Choi, jonghwa3.lee, open list, Amit Daniel Kachhap,
Sachin Kamat, Linux PM list, dg77.kim
In-Reply-To: <CAH9JG2UYsQ6VFMO4COyEr9fBzCL03+37bXTN=AgjKbD7NjR1MA@mail.gmail.com>
On Tue, 2012-11-20 at 10:39 +0900, Kyungmin Park wrote:
> On 11/20/12, Jonghwan Choi <jhbird.choi@samsung.com> wrote:
> > [0]bit is used to enable/disable tmu core. [1] bit is a reserved bit.
> >
> > Signed-off-by: Jonghwan Choi <jhbird.choi@samsung.com>
> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
Amit and Donggeun Kim,
any comments on this patch?
thanks,
rui
> > ---
> > drivers/thermal/exynos_thermal.c | 16 ++++++++++++----
> > 1 files changed, 12 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/thermal/exynos_thermal.c
> > b/drivers/thermal/exynos_thermal.c
> > index 6dd29e4..129e827 100644
> > --- a/drivers/thermal/exynos_thermal.c
> > +++ b/drivers/thermal/exynos_thermal.c
> > @@ -52,9 +52,12 @@
> >
> > #define EXYNOS_TMU_TRIM_TEMP_MASK 0xff
> > #define EXYNOS_TMU_GAIN_SHIFT 8
> > +#define EXYNOS_TMU_GAIN_MASK (0xF << EXYNOS_TMU_GAIN_SHIFT)
> > #define EXYNOS_TMU_REF_VOLTAGE_SHIFT 24
> > -#define EXYNOS_TMU_CORE_ON 3
> > -#define EXYNOS_TMU_CORE_OFF 2
> > +#define EXYNOS_TMU_REF_VOLTAGE_MASK (0x1F <<
> > EXYNOS_TMU_REF_VOLTAGE_SHIFT)
> > +#define EXYNOS_TMU_CORE_ON BIT(0)
> > +#define EXYNOS_TMU_CORE_ON_SHIFT 0
> > +#define EXYNOS_TMU_CORE_ON_MASK (0x1 <<
> > EXYNOS_TMU_CORE_ON_SHIFT)
> > #define EXYNOS_TMU_DEF_CODE_TO_TEMP_OFFSET 50
> >
> > /* Exynos4210 specific registers */
> > @@ -85,7 +88,9 @@
> > #define EXYNOS_TMU_CLEAR_FALL_INT (0x111 << 16)
> > #define EXYNOS_MUX_ADDR_VALUE 6
> > #define EXYNOS_MUX_ADDR_SHIFT 20
> > +#define EXYNOS_MUX_ADDR_MASK (0x7 << EXYNOS_MUX_ADDR_SHIFT)
> > #define EXYNOS_TMU_TRIP_MODE_SHIFT 13
> > +#define EXYNOS_TMU_TRIP_MODE_MASK (0x7 << EXYNOS_TMU_TRIP_MODE_SHIFT)
> >
> > #define EFUSE_MIN_VALUE 40
> > #define EFUSE_MAX_VALUE 100
> > @@ -650,10 +655,14 @@ static void exynos_tmu_control(struct platform_device
> > *pdev, bool on)
> > mutex_lock(&data->lock);
> > clk_enable(data->clk);
> >
> > - con = pdata->reference_voltage << EXYNOS_TMU_REF_VOLTAGE_SHIFT |
> > + con = readl(data->base + EXYNOS_TMU_REG_CONTROL);
> > + con &= ~(EXYNOS_TMU_REF_VOLTAGE_MASK | EXYNOS_TMU_GAIN_MASK |
> > + EXYNOS_TMU_CORE_ON_MASK);
> > + con |= pdata->reference_voltage << EXYNOS_TMU_REF_VOLTAGE_SHIFT |
> > pdata->gain << EXYNOS_TMU_GAIN_SHIFT;
> >
> > if (data->soc == SOC_ARCH_EXYNOS) {
> > + con &= ~(EXYNOS_TMU_TRIP_MODE_MASK | EXYNOS_MUX_ADDR_MASK);
> > con |= pdata->noise_cancel_mode <<
> > EXYNOS_TMU_TRIP_MODE_SHIFT;
> > con |= (EXYNOS_MUX_ADDR_VALUE << EXYNOS_MUX_ADDR_SHIFT);
> > }
> > @@ -665,7 +674,6 @@ static void exynos_tmu_control(struct platform_device
> > *pdev, bool on)
> > pdata->trigger_level1_en << 4 |
> > pdata->trigger_level0_en;
> > } else {
> > - con |= EXYNOS_TMU_CORE_OFF;
> > interrupt_en = 0; /* Disable all interrupts */
> > }
> > writel(interrupt_en, data->base + EXYNOS_TMU_REG_INTEN);
> > --
> > 1.7.4.1
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at http://www.tux.org/lkml/
> >
^ permalink raw reply
* RE: [PATCH v3 0/4] ARM: EXYNOS: Power domain DT support extension
From: Kukjin Kim @ 2012-11-20 4:53 UTC (permalink / raw)
To: 'Tomasz Figa', linux-samsung-soc
Cc: linux-pm, linux-arm-kernel, kyungmin.park, thomas.abraham, rjw,
m.szyprowski, tomasz.figa
In-Reply-To: <1552856.8ocEO5iISn@amdc1227>
Tomasz Figa wrote:
>
> Hi,
>
> On Tuesday 13 of November 2012 13:51:51 Tomasz Figa wrote:
> > This patch series fixes two issues with existing DT support for Exynos
> > power domains and extends it with the ability of binding devices to
> > domains, basically making it possible to use power domains with DT.
> >
> > Based on for-next branch of Kgene's tree.
> >
> > Changes since v2:
> > - Rebased to for-next of Kgene's tree.
> > Changes since v1:
> > - Added "samsung," prefix to "power-domain" property.
> > - Added power domain nodes to Exynos4 device tree sources.
> >
> > Tomasz Figa (4):
> > ARM: EXYNOS: pm_domain: Detect domain state on registration from DT
> > ARM: EXYNOS: pm_domain: Fix power domain name initialization
> > ARM: EXYNOS: pm_domain: Bind devices to power domains using DT
> > ARM: dts: exynos4: Set up power domains
> >
> > .../bindings/arm/exynos/power_domain.txt | 15 +++-
> > arch/arm/boot/dts/exynos4.dtsi | 30 +++++++
> > arch/arm/boot/dts/exynos4210.dtsi | 5 ++
> > arch/arm/mach-exynos/pm_domains.c | 93
> > +++++++++++++++++++++- 4 files changed, 135 insertions(+), 8
> > deletions(-)
>
> Sorry, please disregard this series, as I have sent wrong version by
> mistake.
>
> I will send correct one later.
>
OK.
Thanks.
Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.
^ permalink raw reply
* Re: [RFC] PCI/PM: Keep runtime PM enabled for unbound PCI devices
From: Huang Ying @ 2012-11-20 0:52 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Alan Stern, Bjorn Helgaas, linux-kernel, linux-pci, linux-pm,
Rafael J. Wysocki
In-Reply-To: <1521498.PJusLS7B2o@vostro.rjw.lan>
On Mon, 2012-11-19 at 23:00 +0100, Rafael J. Wysocki wrote:
> On Monday, November 19, 2012 11:31:01 AM Alan Stern wrote:
> > On Mon, 19 Nov 2012, Huang Ying wrote:
> >
> > > For unbound PCI devices, what we need is:
> > >
> > > - Always in D0 state, because some devices does not work again after
> > > being put into D3 by the PCI bus.
> > >
> > > - In SUSPENDED state if allowed, so that the parent devices can still
> > > be put into low power state.
> > >
> > > To satisfy these requirement, the runtime PM for the unbound PCI
> > > devices are disabled and set to SUSPENDED state. One issue of this
> > > solution is that the PCI devices will be put into SUSPENDED state even
> > > if the SUSPENDED state is forbidden via the sysfs interface
> > > (.../power/control) of the device. This is not an issue for most
> > > devices, because most PCI devices are not used at all if unbounded.
> > > But there are exceptions. For example, unbound VGA card can be used
> > > for display, but suspend its parents make it stop working.
> > >
> > > To fix the issue, we keep the runtime PM enabled when the PCI devices
> > > are unbound. But the runtime PM callbacks will do nothing if the PCI
> > > devices are unbound. This way, we can put the PCI devices into
> > > SUSPENDED state without put the PCI devices into D3 state.
> > >
> > > Known issue: after some changing, pci_dev->driver is used to indicate
> > > whether the PCI devices are bound and whether the runtime PM callbacks
> > > should do nothing. Maybe it is better to use a dedicated flag such as
> > > .skip_rtpm_callbacks. That may improve code readability.
> >
> > I think it's okay like this, especially if you add a comment in
> > pci_runtime_suspend, pci_runtime_resume, and pci_runtime_idle
> > explaining that when pci_dev->driver isn't set, the device should
> > always remain in D0 regardless of the runtime status.
>
> Yes, I agree with Alan. Please add comments as Alan's suggesting and it
> should be fine.
Sure. Will do that.
Best Regards,
Huang Ying
^ permalink raw reply
* Re: [RFC] PCI/PM: Keep runtime PM enabled for unbound PCI devices
From: Rafael J. Wysocki @ 2012-11-19 22:00 UTC (permalink / raw)
To: Alan Stern, Huang Ying
Cc: Bjorn Helgaas, linux-kernel, linux-pci, linux-pm,
Rafael J. Wysocki
In-Reply-To: <Pine.LNX.4.44L0.1211191129060.1598-100000@iolanthe.rowland.org>
On Monday, November 19, 2012 11:31:01 AM Alan Stern wrote:
> On Mon, 19 Nov 2012, Huang Ying wrote:
>
> > For unbound PCI devices, what we need is:
> >
> > - Always in D0 state, because some devices does not work again after
> > being put into D3 by the PCI bus.
> >
> > - In SUSPENDED state if allowed, so that the parent devices can still
> > be put into low power state.
> >
> > To satisfy these requirement, the runtime PM for the unbound PCI
> > devices are disabled and set to SUSPENDED state. One issue of this
> > solution is that the PCI devices will be put into SUSPENDED state even
> > if the SUSPENDED state is forbidden via the sysfs interface
> > (.../power/control) of the device. This is not an issue for most
> > devices, because most PCI devices are not used at all if unbounded.
> > But there are exceptions. For example, unbound VGA card can be used
> > for display, but suspend its parents make it stop working.
> >
> > To fix the issue, we keep the runtime PM enabled when the PCI devices
> > are unbound. But the runtime PM callbacks will do nothing if the PCI
> > devices are unbound. This way, we can put the PCI devices into
> > SUSPENDED state without put the PCI devices into D3 state.
> >
> > Known issue: after some changing, pci_dev->driver is used to indicate
> > whether the PCI devices are bound and whether the runtime PM callbacks
> > should do nothing. Maybe it is better to use a dedicated flag such as
> > .skip_rtpm_callbacks. That may improve code readability.
>
> I think it's okay like this, especially if you add a comment in
> pci_runtime_suspend, pci_runtime_resume, and pci_runtime_idle
> explaining that when pci_dev->driver isn't set, the device should
> always remain in D0 regardless of the runtime status.
Yes, I agree with Alan. Please add comments as Alan's suggesting and it
should be fine.
Thanks,
Rafael
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
^ permalink raw reply
* [PATCH 052/493] cpufreq: remove use of __devexit_p
From: Bill Pemberton @ 2012-11-19 18:20 UTC (permalink / raw)
To: gregkh; +Cc: Rafael J. Wysocki, cpufreq, linux-pm
In-Reply-To: <1353349642-3677-1-git-send-email-wfp5p@virginia.edu>
CONFIG_HOTPLUG is going away as an option so __devexit_p is no longer
needed.
Signed-off-by: Bill Pemberton <wfp5p@virginia.edu>
Cc: "Rafael J. Wysocki" <rjw@sisk.pl>
Cc: cpufreq@vger.kernel.org
Cc: linux-pm@vger.kernel.org
---
drivers/cpufreq/longhaul.c | 2 +-
drivers/cpufreq/powernow-k8.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/cpufreq/longhaul.c b/drivers/cpufreq/longhaul.c
index 53ddbc7..8d7ebb2 100644
--- a/drivers/cpufreq/longhaul.c
+++ b/drivers/cpufreq/longhaul.c
@@ -946,7 +946,7 @@ static struct cpufreq_driver longhaul_driver = {
.target = longhaul_target,
.get = longhaul_get,
.init = longhaul_cpu_init,
- .exit = __devexit_p(longhaul_cpu_exit),
+ .exit = longhaul_cpu_exit,
.name = "longhaul",
.owner = THIS_MODULE,
.attr = longhaul_attr,
diff --git a/drivers/cpufreq/powernow-k8.c b/drivers/cpufreq/powernow-k8.c
index e3ebb4f..1fd0c8a 100644
--- a/drivers/cpufreq/powernow-k8.c
+++ b/drivers/cpufreq/powernow-k8.c
@@ -1242,7 +1242,7 @@ static struct cpufreq_driver cpufreq_amd64_driver = {
.target = powernowk8_target,
.bios_limit = acpi_processor_get_bios_limit,
.init = powernowk8_cpu_init,
- .exit = __devexit_p(powernowk8_cpu_exit),
+ .exit = powernowk8_cpu_exit,
.get = powernowk8_get,
.name = "powernow-k8",
.owner = THIS_MODULE,
--
1.8.0
^ permalink raw reply related
* [PATCH 396/493] cpufreq: remove use of __devexit
From: Bill Pemberton @ 2012-11-19 18:25 UTC (permalink / raw)
To: gregkh; +Cc: Rafael J. Wysocki, cpufreq, linux-pm
In-Reply-To: <1353349642-3677-1-git-send-email-wfp5p@virginia.edu>
CONFIG_HOTPLUG is going away as an option so __devexit is no
longer needed.
Signed-off-by: Bill Pemberton <wfp5p@virginia.edu>
Cc: "Rafael J. Wysocki" <rjw@sisk.pl>
Cc: cpufreq@vger.kernel.org
Cc: linux-pm@vger.kernel.org
---
drivers/cpufreq/longhaul.c | 2 +-
drivers/cpufreq/powernow-k8.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/cpufreq/longhaul.c b/drivers/cpufreq/longhaul.c
index 8d7ebb2..f1fa500 100644
--- a/drivers/cpufreq/longhaul.c
+++ b/drivers/cpufreq/longhaul.c
@@ -930,7 +930,7 @@ static int __cpuinit longhaul_cpu_init(struct cpufreq_policy *policy)
return 0;
}
-static int __devexit longhaul_cpu_exit(struct cpufreq_policy *policy)
+static int longhaul_cpu_exit(struct cpufreq_policy *policy)
{
cpufreq_frequency_table_put_attr(policy->cpu);
return 0;
diff --git a/drivers/cpufreq/powernow-k8.c b/drivers/cpufreq/powernow-k8.c
index 1fd0c8a..056faf6 100644
--- a/drivers/cpufreq/powernow-k8.c
+++ b/drivers/cpufreq/powernow-k8.c
@@ -1186,7 +1186,7 @@ err_out:
return -ENODEV;
}
-static int __devexit powernowk8_cpu_exit(struct cpufreq_policy *pol)
+static int powernowk8_cpu_exit(struct cpufreq_policy *pol)
{
struct powernow_k8_data *data = per_cpu(powernow_data, pol->cpu);
--
1.8.0
^ permalink raw reply related
* [PATCH 250/493] cpufreq: remove use of __devinit
From: Bill Pemberton @ 2012-11-19 18:23 UTC (permalink / raw)
To: gregkh; +Cc: Rafael J. Wysocki, cpufreq, linux-pm
In-Reply-To: <1353349642-3677-1-git-send-email-wfp5p@virginia.edu>
CONFIG_HOTPLUG is going away as an option so __devinit is no longer
needed.
Signed-off-by: Bill Pemberton <wfp5p@virginia.edu>
Cc: "Rafael J. Wysocki" <rjw@sisk.pl>
Cc: cpufreq@vger.kernel.org
Cc: linux-pm@vger.kernel.org
---
drivers/cpufreq/cpufreq-cpu0.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/cpufreq/cpufreq-cpu0.c b/drivers/cpufreq/cpufreq-cpu0.c
index e915827..52bf36d 100644
--- a/drivers/cpufreq/cpufreq-cpu0.c
+++ b/drivers/cpufreq/cpufreq-cpu0.c
@@ -174,7 +174,7 @@ static struct cpufreq_driver cpu0_cpufreq_driver = {
.attr = cpu0_cpufreq_attr,
};
-static int __devinit cpu0_cpufreq_driver_init(void)
+static int cpu0_cpufreq_driver_init(void)
{
struct device_node *np;
int ret;
--
1.8.0
^ permalink raw reply related
* Re: [RFC] PCI/PM: Keep runtime PM enabled for unbound PCI devices
From: Alan Stern @ 2012-11-19 16:31 UTC (permalink / raw)
To: Huang Ying
Cc: Bjorn Helgaas, linux-kernel, linux-pci, linux-pm,
Rafael J. Wysocki, Rafael J. Wysocki
In-Reply-To: <1353294849-22588-1-git-send-email-ying.huang@intel.com>
On Mon, 19 Nov 2012, Huang Ying wrote:
> For unbound PCI devices, what we need is:
>
> - Always in D0 state, because some devices does not work again after
> being put into D3 by the PCI bus.
>
> - In SUSPENDED state if allowed, so that the parent devices can still
> be put into low power state.
>
> To satisfy these requirement, the runtime PM for the unbound PCI
> devices are disabled and set to SUSPENDED state. One issue of this
> solution is that the PCI devices will be put into SUSPENDED state even
> if the SUSPENDED state is forbidden via the sysfs interface
> (.../power/control) of the device. This is not an issue for most
> devices, because most PCI devices are not used at all if unbounded.
> But there are exceptions. For example, unbound VGA card can be used
> for display, but suspend its parents make it stop working.
>
> To fix the issue, we keep the runtime PM enabled when the PCI devices
> are unbound. But the runtime PM callbacks will do nothing if the PCI
> devices are unbound. This way, we can put the PCI devices into
> SUSPENDED state without put the PCI devices into D3 state.
>
> Known issue: after some changing, pci_dev->driver is used to indicate
> whether the PCI devices are bound and whether the runtime PM callbacks
> should do nothing. Maybe it is better to use a dedicated flag such as
> .skip_rtpm_callbacks. That may improve code readability.
I think it's okay like this, especially if you add a comment in
pci_runtime_suspend, pci_runtime_resume, and pci_runtime_idle
explaining that when pci_dev->driver isn't set, the device should
always remain in D0 regardless of the runtime status.
Alan Stern
^ permalink raw reply
* Re: [PATCH v9 06/10] ata: zpodd: check zero power ready status
From: James Bottomley @ 2012-11-19 15:06 UTC (permalink / raw)
To: Tejun Heo
Cc: Aaron Lu, Jeff Garzik, Rafael J. Wysocki, Alan Stern, Jeff Wu,
Aaron Lu, linux-ide, linux-pm, linux-scsi, linux-acpi
In-Reply-To: <20121119145605.GC15971@htj.dyndns.org>
On Mon, 2012-11-19 at 06:56 -0800, Tejun Heo wrote:
> Hey, Aaron.
>
> On Mon, Nov 19, 2012 at 11:09:40AM +0800, Aaron Lu wrote:
> > > What I'm confused about is what autopm does for devices w/o zpodd.
> > > What happens then? Is it gonna leave power on for the device and,
> > > say, go on to suspend the controller? But, how would that work for,
> > > say, future devices with async notification for media events?
> >
> > Maybe we shouldn't allow autopm for such devices?
>
> Yeah, maybe. It would be nice to be able to automatically power off
> disks and ports which aren't being used tho.
>
> > > That said, I can't say the snooping is pretty. It's a rather nasty
> > > thing to do. So, libata now wants information from the event polling
> > > in block layer, but reaching for block_device from ata_devices is
> > > nasty too. Hmmm... but aren't you already doing that to block polling
> > > on a powered down device?
> >
> > I was feeling brain damaged by this for some time :-)
> >
> > Basically, only ATA layer is aware of the power off thing, and sr knows
> > nothing about this(or it is not supposed to know this, at least, this is
> > what SCSI people think) and once powered off, I do not want the poll to
> > disturb the device, so I need to block the poll. I can't come up with
> > another way to achieve this except this nasty way.
> >
> > James suggests me to keep the poll, but emulate the command. The problem
> > with this is, the autopm for resume will kick in on each poll, so I'll
> > need to decide if power up the ODD for this time's resume is needed in
> > port's runtime resume callback. This made things complex and it also put
> > too much logic in the resume callback, which is not desired. And even if
> > I keep the ODD in powered off state by emulating this poll command, its
> > ancestor devices will still be resumed, and I may need to do some trick
> > in their resume callback to avoid needless power/state transitions. This
> > doesn't feel like an elegant way to solve this either.
> >
> > So yes, I'm still using this _nasty_ way to make the ODD stay in powered
> > off state as long as possible. But if there is other elegant ways to
> > solve this, I would appreciate it and happily using it. Personally, I
> > hope we can make sr aware of ZPODD, that would make the pain gone.
>
> I really think we need a way for (auto)pm and event polling to talk to
> each other so that autopm can tell event poll to sod off while pm is
> in effect. Trying to solve this from inside libata doesn't seem
> right. The problem, again, seems to be figuring out which hardware
> device maps to which block device. Hmmm... Any good ideas?
I've asked the PM people several times about this, because it's a real
problem for almost everything: PM needs some type of top to bottom
stack view, which the layering isolation we have within storage really
doesn't cope with well. No real suggestion has been forthcoming.
The reason I think it should be emulated (in the acpi layer, not libata,
but as long as it's not in SCSI, I'm not so fussed where it ends up) is
because ZPODD is the software equivalent of ZPREADY, which will be done
in hardware and will be effectively invisible to autopm in the same way
that SCSI (and ATA) power management is mostly invisible. If we
currently do ZPREADY emulation in the low layer (i.e. ZPODD has exact
ZPREADY emulation), we won't care (except for flipping the sofware bit)
whether the device support ZPODD or ZPREADY and it will all just
work(tm). The industry expectation is that ZPODD is just a transition
state between current power management and ZPREADY.
James
^ permalink raw reply
* Re: [PATCH v9 06/10] ata: zpodd: check zero power ready status
From: Tejun Heo @ 2012-11-19 14:56 UTC (permalink / raw)
To: Aaron Lu
Cc: Jeff Garzik, James Bottomley, Rafael J. Wysocki, Alan Stern,
Jeff Wu, Aaron Lu, linux-ide, linux-pm, linux-scsi, linux-acpi
In-Reply-To: <50A9A2F4.3020006@intel.com>
Hey, Aaron.
On Mon, Nov 19, 2012 at 11:09:40AM +0800, Aaron Lu wrote:
> > What I'm confused about is what autopm does for devices w/o zpodd.
> > What happens then? Is it gonna leave power on for the device and,
> > say, go on to suspend the controller? But, how would that work for,
> > say, future devices with async notification for media events?
>
> Maybe we shouldn't allow autopm for such devices?
Yeah, maybe. It would be nice to be able to automatically power off
disks and ports which aren't being used tho.
> > That said, I can't say the snooping is pretty. It's a rather nasty
> > thing to do. So, libata now wants information from the event polling
> > in block layer, but reaching for block_device from ata_devices is
> > nasty too. Hmmm... but aren't you already doing that to block polling
> > on a powered down device?
>
> I was feeling brain damaged by this for some time :-)
>
> Basically, only ATA layer is aware of the power off thing, and sr knows
> nothing about this(or it is not supposed to know this, at least, this is
> what SCSI people think) and once powered off, I do not want the poll to
> disturb the device, so I need to block the poll. I can't come up with
> another way to achieve this except this nasty way.
>
> James suggests me to keep the poll, but emulate the command. The problem
> with this is, the autopm for resume will kick in on each poll, so I'll
> need to decide if power up the ODD for this time's resume is needed in
> port's runtime resume callback. This made things complex and it also put
> too much logic in the resume callback, which is not desired. And even if
> I keep the ODD in powered off state by emulating this poll command, its
> ancestor devices will still be resumed, and I may need to do some trick
> in their resume callback to avoid needless power/state transitions. This
> doesn't feel like an elegant way to solve this either.
>
> So yes, I'm still using this _nasty_ way to make the ODD stay in powered
> off state as long as possible. But if there is other elegant ways to
> solve this, I would appreciate it and happily using it. Personally, I
> hope we can make sr aware of ZPODD, that would make the pain gone.
I really think we need a way for (auto)pm and event polling to talk to
each other so that autopm can tell event poll to sod off while pm is
in effect. Trying to solve this from inside libata doesn't seem
right. The problem, again, seems to be figuring out which hardware
device maps to which block device. Hmmm... Any good ideas?
Thanks.
--
tejun
^ permalink raw reply
* Re: [PATCH v9 05/10] libata: separate ATAPI code
From: Tejun Heo @ 2012-11-19 14:51 UTC (permalink / raw)
To: Aaron Lu
Cc: Jeff Garzik, James Bottomley, Rafael J. Wysocki, Alan Stern,
Jeff Wu, Aaron Lu, linux-ide, linux-pm, linux-scsi, linux-acpi
In-Reply-To: <50A9979B.1040309@intel.com>
On Mon, Nov 19, 2012 at 10:21:15AM +0800, Aaron Lu wrote:
> My thought is that, the 2 functions are for ATAPI and can be used by
> libata-eh, libata-zpodd and maybe others in the future, so it deserves
> a separate place. But if this is regarded as unnecessary, I will drop
> this patch.
Yeah, please drop. There are a lot of function in libata-core and eh
which are used all over libata. No need to move them because they're
gonna be used somewhere else.
Thanks.
--
tejun
^ permalink raw reply
* Re: [PATCH v9 07/10] block: add a new interface to block events
From: Tejun Heo @ 2012-11-19 14:50 UTC (permalink / raw)
To: Alan Stern
Cc: Aaron Lu, Jeff Garzik, James Bottomley, Rafael J. Wysocki,
Jeff Wu, Aaron Lu, linux-ide, linux-pm, linux-scsi, linux-acpi
In-Reply-To: <Pine.LNX.4.44L0.1211182058450.10879-100000@netrider.rowland.org>
On Sun, Nov 18, 2012 at 09:07:22PM -0500, Alan Stern wrote:
> Hey...
Hey, again. :P
> > So, at least for SATA, I think what autopm can do is...
> >
> > * Trigger zpodd if possible.
> >
> > * Trigger suspend iff polling isn't happening on the device.
>
> That sums it up nicely. Of course, the PM core is unaware of details
> such as media polling. What we can do is have the SATA runtime-idle
> method return -EBUSY if the device isn't a ZPODD and if polling is
> enabled. Unfortunately I don't think there's any way currently to
> trigger autopm when the user turns off polling. Maybe something could
> be added to the appropriate sysfs handler.
Given that genhd event polling and PM are likely to interact with each
other, I think it would be a good idea to implement a proper way for
the two to communicate. Dunno what it should be tho. The tricky part
would be mapping the block device to the underlying hardware one.
Thanks.
--
tejun
^ permalink raw reply
* RE: [PATCH 2/2] Thermal: Introduce THERMAL_TREND_RAISE/DROP_FULL support for step_wise governor
From: R, Durgadoss @ 2012-11-19 11:21 UTC (permalink / raw)
To: Zhang, Rui, Linux PM list; +Cc: Amit Kachhap
In-Reply-To: <1353313293.6468.4.camel@rzhang1-mobl4>
> -----Original Message-----
> From: Zhang, Rui
> Sent: Monday, November 19, 2012 1:52 PM
> To: Linux PM list
> Cc: Zhang, Rui; Amit Kachhap; R, Durgadoss
> Subject: [PATCH 2/2] Thermal: Introduce
> THERMAL_TREND_RAISE/DROP_FULL support for step_wise governor
>
> From fcc7e0a36388f468c25526290e2fb2beebaae99c Mon Sep 17 00:00:00
> 2001
> From: Zhang Rui <rui.zhang@intel.com>
> Date: Mon, 19 Nov 2012 16:10:20 +0800
> Subject: [PATCH 2/2] Introduce THERMAL_TREND_RAISE/DROP_FULL
> support for
> step_wise governor
>
> step_wise governor should set the device cooling state to
> upper/lower limit directly when THERMAL_TREND_RAISE/DROP_FULL.
>
Patch looks good..
Reviewed-by: Durgadoss R <durgadoss.r@intel.com>
Thanks,
Durga
> Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> ---
> drivers/thermal/step_wise.c | 19 +++++++++++++++++--
> 1 file changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/thermal/step_wise.c b/drivers/thermal/step_wise.c
> index 1242cff..6273f7d 100644
> --- a/drivers/thermal/step_wise.c
> +++ b/drivers/thermal/step_wise.c
> @@ -35,6 +35,10 @@
> * state for this trip point
> * b. if the trend is THERMAL_TREND_DROPPING, use lower cooling
> * state for this trip point
> + * c. if the trend is THERMAL_TREND_RAISE_FULL, use upper limit
> + * for this trip point
> + * d. if the trend is THERMAL_TREND_DROP_FULL, use lower limit
> + * for this trip point
> */
> static unsigned long get_target_state(struct thermal_instance *instance,
> enum thermal_trend trend)
> @@ -44,12 +48,23 @@ static unsigned long get_target_state(struct
> thermal_instance *instance,
>
> cdev->ops->get_cur_state(cdev, &cur_state);
>
> - if (trend == THERMAL_TREND_RAISING) {
> + switch (trend) {
> + case THERMAL_TREND_RAISING:
> cur_state = cur_state < instance->upper ?
> (cur_state + 1) : instance->upper;
> - } else if (trend == THERMAL_TREND_DROPPING) {
> + break;
> + case THERMAL_TREND_DROPPING:
> cur_state = cur_state > instance->lower ?
> (cur_state - 1) : instance->lower;
> + break;
> + case THERMAL_TREND_RAISE_FULL:
> + cur_state = instance->upper;
> + break;
> + case THERMAL_TREND_DROP_FULL:
> + cur_state = instance->lower;
> + break;
> + default:
> + break;
> }
>
> return cur_state;
> --
> 1.7.9.5
>
>
^ permalink raw reply
* RE: [PATCH 1/2] Thermal: Introduce THERMAL_TREND_RAISE_FULL and and THERMAL_TREND_DROP_FULL
From: R, Durgadoss @ 2012-11-19 11:19 UTC (permalink / raw)
To: Zhang, Rui, Linux PM list; +Cc: Amit Kachhap
In-Reply-To: <1353313272.6468.3.camel@rzhang1-mobl4>
Hi Rui/Amit,
This patch looks fine and gives what we need in a simple way :-)
> -----Original Message-----
> From: Zhang, Rui
> Sent: Monday, November 19, 2012 1:51 PM
> To: Linux PM list
> Cc: Zhang, Rui; Amit Kachhap; R, Durgadoss
> Subject: [PATCH 1/2] Thermal: Introduce THERMAL_TREND_RAISE_FULL and
> and THERMAL_TREND_DROP_FULL
>
> From cd05abc4929c21275e3674fb303ca6007f8415a0 Mon Sep 17 00:00:00 2001
> From: Zhang Rui <rui.zhang@intel.com>
> Date: Mon, 19 Nov 2012 15:33:51 +0800
> Subject: [PATCH 1/2] Introduce THERMAL_TREND_RAISE_FULL and
> THERMAL_TREND_DROP_FULL
>
> These two new thermal_trend types are used to tell the governor
> that the temeprature is raising/dropping quickly.
typo 'temperature'.
>
> Thermal cooling governors should handle this situation and make
> proper decisions, e.g. set cooling state to upper/lower limit directly
> instead of one step each time for step_wise governor.
>
Reviewed-by: Durgadoss R <durgadoss.r@intel.com>
Thanks,
Durga
> Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> ---
> include/linux/thermal.h | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> index 807f214..dcaa400 100644
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h
> @@ -68,6 +68,8 @@ enum thermal_trend {
> THERMAL_TREND_STABLE, /* temperature is stable */
> THERMAL_TREND_RAISING, /* temperature is raising */
> THERMAL_TREND_DROPPING, /* temperature is dropping */
> + THERMAL_TREND_RAISE_FULL, /* apply highest cooling action */
> + THERMAL_TREND_DROP_FULL, /* apply lowest cooling action */
> };
>
> /* Events supported by Thermal Netlink */
> --
> 1.7.9.5
>
>
^ permalink raw reply
* [PATCH 2/2] Thermal: Introduce THERMAL_TREND_RAISE/DROP_FULL support for step_wise governor
From: Zhang Rui @ 2012-11-19 8:21 UTC (permalink / raw)
To: Linux PM list; +Cc: Zhang, Rui, Amit Kachhap, durga
>From fcc7e0a36388f468c25526290e2fb2beebaae99c Mon Sep 17 00:00:00 2001
From: Zhang Rui <rui.zhang@intel.com>
Date: Mon, 19 Nov 2012 16:10:20 +0800
Subject: [PATCH 2/2] Introduce THERMAL_TREND_RAISE/DROP_FULL support for
step_wise governor
step_wise governor should set the device cooling state to
upper/lower limit directly when THERMAL_TREND_RAISE/DROP_FULL.
Signed-off-by: Zhang Rui <rui.zhang@intel.com>
---
drivers/thermal/step_wise.c | 19 +++++++++++++++++--
1 file changed, 17 insertions(+), 2 deletions(-)
diff --git a/drivers/thermal/step_wise.c b/drivers/thermal/step_wise.c
index 1242cff..6273f7d 100644
--- a/drivers/thermal/step_wise.c
+++ b/drivers/thermal/step_wise.c
@@ -35,6 +35,10 @@
* state for this trip point
* b. if the trend is THERMAL_TREND_DROPPING, use lower cooling
* state for this trip point
+ * c. if the trend is THERMAL_TREND_RAISE_FULL, use upper limit
+ * for this trip point
+ * d. if the trend is THERMAL_TREND_DROP_FULL, use lower limit
+ * for this trip point
*/
static unsigned long get_target_state(struct thermal_instance *instance,
enum thermal_trend trend)
@@ -44,12 +48,23 @@ static unsigned long get_target_state(struct thermal_instance *instance,
cdev->ops->get_cur_state(cdev, &cur_state);
- if (trend == THERMAL_TREND_RAISING) {
+ switch (trend) {
+ case THERMAL_TREND_RAISING:
cur_state = cur_state < instance->upper ?
(cur_state + 1) : instance->upper;
- } else if (trend == THERMAL_TREND_DROPPING) {
+ break;
+ case THERMAL_TREND_DROPPING:
cur_state = cur_state > instance->lower ?
(cur_state - 1) : instance->lower;
+ break;
+ case THERMAL_TREND_RAISE_FULL:
+ cur_state = instance->upper;
+ break;
+ case THERMAL_TREND_DROP_FULL:
+ cur_state = instance->lower;
+ break;
+ default:
+ break;
}
return cur_state;
--
1.7.9.5
^ permalink raw reply related
* [PATCH 1/2] Thermal: Introduce THERMAL_TREND_RAISE_FULL and and THERMAL_TREND_DROP_FULL
From: Zhang Rui @ 2012-11-19 8:21 UTC (permalink / raw)
To: Linux PM list; +Cc: Zhang, Rui, Amit Kachhap, durga
>From cd05abc4929c21275e3674fb303ca6007f8415a0 Mon Sep 17 00:00:00 2001
From: Zhang Rui <rui.zhang@intel.com>
Date: Mon, 19 Nov 2012 15:33:51 +0800
Subject: [PATCH 1/2] Introduce THERMAL_TREND_RAISE_FULL and
THERMAL_TREND_DROP_FULL
These two new thermal_trend types are used to tell the governor
that the temeprature is raising/dropping quickly.
Thermal cooling governors should handle this situation and make
proper decisions, e.g. set cooling state to upper/lower limit directly
instead of one step each time for step_wise governor.
Signed-off-by: Zhang Rui <rui.zhang@intel.com>
---
include/linux/thermal.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index 807f214..dcaa400 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -68,6 +68,8 @@ enum thermal_trend {
THERMAL_TREND_STABLE, /* temperature is stable */
THERMAL_TREND_RAISING, /* temperature is raising */
THERMAL_TREND_DROPPING, /* temperature is dropping */
+ THERMAL_TREND_RAISE_FULL, /* apply highest cooling action */
+ THERMAL_TREND_DROP_FULL, /* apply lowest cooling action */
};
/* Events supported by Thermal Netlink */
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH v9 07/10] block: add a new interface to block events
From: Aaron Lu @ 2012-11-19 3:21 UTC (permalink / raw)
To: Alan Stern, Tejun Heo
Cc: Jeff Garzik, James Bottomley, Rafael J. Wysocki, Jeff Wu,
Aaron Lu, linux-ide, linux-pm, linux-scsi, linux-acpi
In-Reply-To: <Pine.LNX.4.44L0.1211182058450.10879-100000@netrider.rowland.org>
On 11/19/2012 10:07 AM, Alan Stern wrote:
> On Sun, 18 Nov 2012, Tejun Heo wrote:
>
>> Hey, Alan.
>
> Hey...
>
>> It's controlled by /sys/class/scsi_host/hostX/link_power_management.
>> Once enabled, the power saving is completedly handled by hardware. If
>> link stays idle longer than certain duration, the hardware initiates
>> low power state and leaves it when something needs to happen.
>>
>>>> So, this whole autopm thing doesn't sound like a good idea to me.
>>>
>>> No doubt it's better suited to some devices than to others.
>>
>> Yeah, SATA seems to need a different approach than USB.
>
> Based on your description, I agree.
>
>>> That may be true for SATA. For USB optical drives, it does make sense
>>> to power-down the host controller when the drive isn't in use. USB
>>> suspend/resume takes on the order of 50-100 ms or so.
>>
>> I see. For SATA too, the controller / link bring up doesn't take too
>> long. The crappy part is what the attached, especially ATAPI, devices
>> would do after such events as those events will be visible to them
>> basically as random link resets.
>>
>> So, at least for SATA, I think what autopm can do is...
>>
>> * Trigger zpodd if possible.
>>
>> * Trigger suspend iff polling isn't happening on the device.
>
> That sums it up nicely. Of course, the PM core is unaware of details
> such as media polling. What we can do is have the SATA runtime-idle
> method return -EBUSY if the device isn't a ZPODD and if polling is
> enabled. Unfortunately I don't think there's any way currently to
> trigger autopm when the user turns off polling. Maybe something could
> be added to the appropriate sysfs handler.
OK, thanks for your(both of you) suggestions.
But probably for ZPODD devices first, as it has a clear use case and
should already have productions. For async notification capable ODDs,
I'll leave it for someone else or maybe at a later time if I can get
such a device to test on.
To conclude, the ata port's runtime idle callback will return 0 when:
1 It attached a ZPODD capable ODD;
2 Or it attached a hard disk.
For all other cases, it will return -EBUSY.
Does this look correct?
Thanks,
Aaron
^ permalink raw reply
* [RFC] PCI/PM: Keep runtime PM enabled for unbound PCI devices
From: Huang Ying @ 2012-11-19 3:14 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: linux-kernel, linux-pci, linux-pm, Rafael J. Wysocki, Huang Ying,
Rafael J. Wysocki, Alan Stern
For unbound PCI devices, what we need is:
- Always in D0 state, because some devices does not work again after
being put into D3 by the PCI bus.
- In SUSPENDED state if allowed, so that the parent devices can still
be put into low power state.
To satisfy these requirement, the runtime PM for the unbound PCI
devices are disabled and set to SUSPENDED state. One issue of this
solution is that the PCI devices will be put into SUSPENDED state even
if the SUSPENDED state is forbidden via the sysfs interface
(.../power/control) of the device. This is not an issue for most
devices, because most PCI devices are not used at all if unbounded.
But there are exceptions. For example, unbound VGA card can be used
for display, but suspend its parents make it stop working.
To fix the issue, we keep the runtime PM enabled when the PCI devices
are unbound. But the runtime PM callbacks will do nothing if the PCI
devices are unbound. This way, we can put the PCI devices into
SUSPENDED state without put the PCI devices into D3 state.
Known issue: after some changing, pci_dev->driver is used to indicate
whether the PCI devices are bound and whether the runtime PM callbacks
should do nothing. Maybe it is better to use a dedicated flag such as
.skip_rtpm_callbacks. That may improve code readability.
Signed-off-by: Huang Ying <ying.huang@intel.com>
CC: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
CC: Alan Stern <stern@rowland.harvard.edu>
CC: stable@vger.kernel.org # v3.6+
---
drivers/pci/pci-driver.c | 57 +++++++++++++++++++++++------------------------
drivers/pci/pci.c | 2 +
2 files changed, 31 insertions(+), 28 deletions(-)
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -1900,6 +1900,8 @@ void pci_pm_init(struct pci_dev *dev)
u16 pmc;
pm_runtime_forbid(&dev->dev);
+ pm_runtime_set_active(&dev->dev);
+ pm_runtime_enable(&dev->dev);
device_enable_async_suspend(&dev->dev);
dev->wakeup_prepared = false;
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -256,31 +256,26 @@ struct drv_dev_and_id {
static long local_pci_probe(void *_ddi)
{
struct drv_dev_and_id *ddi = _ddi;
- struct device *dev = &ddi->dev->dev;
- struct device *parent = dev->parent;
+ struct pci_dev *pci_dev = ddi->dev;
+ struct pci_driver *pci_drv = ddi->drv;
+ struct device *dev = &pci_dev->dev;
int rc;
- /* The parent bridge must be in active state when probing */
- if (parent)
- pm_runtime_get_sync(parent);
- /* Unbound PCI devices are always set to disabled and suspended.
- * During probe, the device is set to enabled and active and the
- * usage count is incremented. If the driver supports runtime PM,
- * it should call pm_runtime_put_noidle() in its probe routine and
- * pm_runtime_get_noresume() in its remove routine.
- */
- pm_runtime_get_noresume(dev);
- pm_runtime_set_active(dev);
- pm_runtime_enable(dev);
-
- rc = ddi->drv->probe(ddi->dev, ddi->id);
+ /*
+ * Unbound PCI devices are always set to suspended and put in
+ * D0. During probe, the device is set to active and the
+ * usage count is incremented. If the driver supports runtime
+ * PM, it should call pm_runtime_put_noidle() in its probe
+ * routine and pm_runtime_get_noresume() in its remove
+ * routine.
+ */
+ pm_runtime_get_sync(dev);
+ pci_dev->driver = pci_drv;
+ rc = pci_drv->probe(pci_dev, ddi->id);
if (rc) {
- pm_runtime_disable(dev);
- pm_runtime_set_suspended(dev);
- pm_runtime_put_noidle(dev);
+ pci_dev->driver = NULL;
+ pm_runtime_put_sync(dev);
}
- if (parent)
- pm_runtime_put(parent);
return rc;
}
@@ -330,10 +325,8 @@ __pci_device_probe(struct pci_driver *dr
id = pci_match_device(drv, pci_dev);
if (id)
error = pci_call_probe(drv, pci_dev, id);
- if (error >= 0) {
- pci_dev->driver = drv;
+ if (error >= 0)
error = 0;
- }
}
return error;
}
@@ -369,9 +362,7 @@ static int pci_device_remove(struct devi
}
/* Undo the runtime PM settings in local_pci_probe() */
- pm_runtime_disable(dev);
- pm_runtime_set_suspended(dev);
- pm_runtime_put_noidle(dev);
+ pm_runtime_put_sync(dev);
/*
* If the device is still on, set the power state as "unknown",
@@ -994,6 +985,9 @@ static int pci_pm_runtime_suspend(struct
pci_power_t prev = pci_dev->current_state;
int error;
+ if (!pci_dev->driver)
+ return 0;
+
if (!pm || !pm->runtime_suspend)
return -ENOSYS;
@@ -1029,6 +1023,9 @@ static int pci_pm_runtime_resume(struct
struct pci_dev *pci_dev = to_pci_dev(dev);
const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
+ if (!pci_dev->driver)
+ return 0;
+
if (!pm || !pm->runtime_resume)
return -ENOSYS;
@@ -1046,8 +1043,12 @@ static int pci_pm_runtime_resume(struct
static int pci_pm_runtime_idle(struct device *dev)
{
+ struct pci_dev *pci_dev = to_pci_dev(dev);
const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
+ if (!pci_dev->driver)
+ goto out;
+
if (!pm)
return -ENOSYS;
@@ -1057,8 +1058,8 @@ static int pci_pm_runtime_idle(struct de
return ret;
}
+out:
pm_runtime_suspend(dev);
-
return 0;
}
^ permalink raw reply
* Re: [PATCH v9 06/10] ata: zpodd: check zero power ready status
From: Aaron Lu @ 2012-11-19 3:09 UTC (permalink / raw)
To: Tejun Heo
Cc: Jeff Garzik, James Bottomley, Rafael J. Wysocki, Alan Stern,
Jeff Wu, Aaron Lu, linux-ide, linux-pm, linux-scsi, linux-acpi
In-Reply-To: <20121118150004.GJ7306@mtj.dyndns.org>
On 11/18/2012 11:00 PM, Tejun Heo wrote:
> Hello, Aaron.
Hi,
>
> On Wed, Nov 14, 2012 at 10:18:23AM +0800, Aaron Lu wrote:
>> On 11/13/2012 03:13 AM, Tejun Heo wrote:
>>> Hello,
>>>
>>> On Fri, Nov 09, 2012 at 02:51:58PM +0800, Aaron Lu wrote:
>>>> +#define POWEROFF_DELAY (30 * 1000) /* 30 seconds for power off delay */
>>>> +
>>>> struct zpodd {
>>>> bool slot:1;
>>>> bool drawer:1;
>>>> bool from_notify:1; /* resumed as a result of acpi notification */
>>>> + bool status_ready:1; /* ready status derived from media event poll,
>>>> + it is not accurate, but serves as a hint */
>>>> + bool zp_ready:1; /* zero power ready state */
>>>> +
>>>> + unsigned long last_ready; /* last zero power ready timestamp */
>>>>
>>>> struct ata_device *dev;
>>>> };
>>>
>>> How are accesses to the bit fields synchronized?
>>
>> They are synchronized by PM core.
>> PM core ensures that no two suspend or resume callback run concurrently.
>> And when ODD is executing a command, it is in active state, no PM
>> callback will run.
>
> Care to add short comment for that? Flag and bitfield updates aren't
> atomic to each other, so I find it usually helpful to clearly state
> the synchronization rules for them. More so if locking is inherited
> from upprer layer and not immediately obvious.
OK.
>
>>> Hmmm... so, the "full" check only happens when autopm kicks in, right?
>>> Is it really worth avoiding an extra TUR on autopm events? That's not
>>> really a hot path. It seems a bit over-engineered to me.
>>
>> A little more information about this:
>> When there is disc inside and no program mounted the drive, the ODD will
>> be runtime suspended/resumed every 2 seconds along with the event poll.
>
> Is that a desirable behavior? I haven't been following autopm and am
> a bit fuzzy about how autopm works and what it does.
>
> If there isn't any user of the device autopm kicks in. If zpodd is
> enabled and there's no media, the device goes off power. If the user
> initiates an event which may change media status, the driver is
> notified via acpi and autopm backs out restoring power to the device.
> Am I understanding it correctly?
Yes.
>
> What I'm confused about is what autopm does for devices w/o zpodd.
> What happens then? Is it gonna leave power on for the device and,
> say, go on to suspend the controller? But, how would that work for,
> say, future devices with async notification for media events?
Maybe we shouldn't allow autopm for such devices?
>
> Also, if autopm is enabled, an optical device would go in and out of
> suspend every two seconds?
>
>> I'm not sure if the above situation can happen often. Normal desktop
>> environment should automatically mount the ODD once they got uevent, and
>> for console users, they will probably manually mount the drive after
>> they have inserted a disc. The way I did it this way is to deal with the
>> worst possible case. But if this is deemed as not necessary, I think I
>> can remove the snoop hint thing and use TUR directly.
>
> The problem with issuing TUR regularly is that some ODDs lock up after
> getting hit by frequent TURs. That's the reason why sr event check
> routine is being careful with TUR and only issue
> GET_EVENT_STATUS_NOTIFICATION. Windows does about the same thing and
> some vendors somehow screwed up TUR.
>
> That said, I can't say the snooping is pretty. It's a rather nasty
> thing to do. So, libata now wants information from the event polling
> in block layer, but reaching for block_device from ata_devices is
> nasty too. Hmmm... but aren't you already doing that to block polling
> on a powered down device?
I was feeling brain damaged by this for some time :-)
Basically, only ATA layer is aware of the power off thing, and sr knows
nothing about this(or it is not supposed to know this, at least, this is
what SCSI people think) and once powered off, I do not want the poll to
disturb the device, so I need to block the poll. I can't come up with
another way to achieve this except this nasty way.
James suggests me to keep the poll, but emulate the command. The problem
with this is, the autopm for resume will kick in on each poll, so I'll
need to decide if power up the ODD for this time's resume is needed in
port's runtime resume callback. This made things complex and it also put
too much logic in the resume callback, which is not desired. And even if
I keep the ODD in powered off state by emulating this poll command, its
ancestor devices will still be resumed, and I may need to do some trick
in their resume callback to avoid needless power/state transitions. This
doesn't feel like an elegant way to solve this either.
So yes, I'm still using this _nasty_ way to make the ODD stay in powered
off state as long as possible. But if there is other elegant ways to
solve this, I would appreciate it and happily using it. Personally, I
hope we can make sr aware of ZPODD, that would make the pain gone.
Thanks,
Aaron
^ permalink raw reply
* Re: [PATCHv9 1/3] Runtime Interpreted Power Sequences
From: Anton Vorontsov @ 2012-11-19 2:32 UTC (permalink / raw)
To: Alex Courbot
Cc: Stephen Warren, Thierry Reding, Mark Zhang, Grant Likely,
Rob Herring, Mark Brown, David Woodhouse, Arnd Bergmann,
linux-tegra@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, linux-fbdev@vger.kernel.org,
devicetree-discuss@lists.ozlabs.org, linux-pm@vger.kernel.org,
Alexandre Courbot
In-Reply-To: <8058793.mIXgB85Mso@percival>
On Mon, Nov 19, 2012 at 11:29:05AM +0900, Alex Courbot wrote:
> On Saturday 17 November 2012 19:38:50 Anton Vorontsov wrote:
> > > +++ b/drivers/power/power_seq/Kconfig
> > > @@ -0,0 +1,2 @@
> > > +config POWER_SEQ
> > > + tristate
> >
> > This really needs a proper Kconfig description and a help text, shortly
> > describing the purpose of the subsystem.
>
> Does it? The current state makes power seqs automatically selected by drivers
> that need it, and they do not appear in the configuration menu. If we add a
> description they will then become a visible item, so the logic would be to
> make it user-selectable.
Ah. OK, then yes, there is no need for the short description, but the help
text would a good idea anyway.
Thanks,
Anton.
^ permalink raw reply
* Re: [PATCHv9 1/3] Runtime Interpreted Power Sequences
From: Alex Courbot @ 2012-11-19 2:29 UTC (permalink / raw)
To: Anton Vorontsov
Cc: Stephen Warren, Thierry Reding, Mark Zhang, Grant Likely,
Rob Herring, Mark Brown, David Woodhouse, Arnd Bergmann,
linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org,
linux-pm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Alexandre Courbot
In-Reply-To: <20121117113849.GA5228@lizard>
On Saturday 17 November 2012 19:38:50 Anton Vorontsov wrote:
> > +++ b/drivers/power/power_seq/Kconfig
> > @@ -0,0 +1,2 @@
> > +config POWER_SEQ
> > + tristate
>
> This really needs a proper Kconfig description and a help text, shortly
> describing the purpose of the subsystem.
Does it? The current state makes power seqs automatically selected by drivers
that need it, and they do not appear in the configuration menu. If we add a
description they will then become a visible item, so the logic would be to
make it user-selectable.
If that approach is preferred, I will also have to change pwm-backlight so
that it compiles without power sequences. Not that I mind, but I liked the
idea of not adding yet-another-option to the config menu.
Fixed the code according to all your other comments, thanks!
Alex.
^ permalink raw reply
* Re: [PATCH v9 05/10] libata: separate ATAPI code
From: Aaron Lu @ 2012-11-19 2:21 UTC (permalink / raw)
To: Tejun Heo
Cc: Jeff Garzik, James Bottomley, Rafael J. Wysocki, Alan Stern,
Jeff Wu, Aaron Lu, linux-ide, linux-pm, linux-scsi, linux-acpi
In-Reply-To: <20121118150117.GK7306@mtj.dyndns.org>
On 11/18/2012 11:01 PM, Tejun Heo wrote:
> On Tue, Nov 13, 2012 at 08:49:24PM +0800, Aaron Lu wrote:
>> On Mon, Nov 12, 2012 at 10:57:10AM -0800, Tejun Heo wrote:
>>> On Fri, Nov 09, 2012 at 02:51:57PM +0800, Aaron Lu wrote:
>>>> The atapi_eh_tur and atapi_eh_request_sense can be reused by ZPODD
>>>> code, so separate them out to a file named libata-atapi.c, and the
>>>> Makefile is modified accordingly. No functional changes should result
>>>> from this commit.
>>>
>>> Why is this change necessary? This doesn't seem to help anything to
>>> me.
>>
>> Function zpready introduced in patch 6 used these 2 functions to see if
>> an ODD is in a zero power ready state.
>
> And why zpready can't use the functions if they're in a different file?
Oh, sure, they can be used.
My thought is that, the 2 functions are for ATAPI and can be used by
libata-eh, libata-zpodd and maybe others in the future, so it deserves
a separate place. But if this is regarded as unnecessary, I will drop
this patch.
Thanks,
Aaron
^ permalink raw reply
* Re: [PATCH v9 03/10] ata: zpodd: identify and init ZPODD devices
From: Aaron Lu @ 2012-11-19 2:15 UTC (permalink / raw)
To: Tejun Heo
Cc: Jeff Garzik, James Bottomley, Rafael J. Wysocki, Alan Stern,
Jeff Wu, Aaron Lu, linux-ide, linux-pm, linux-scsi, linux-acpi
In-Reply-To: <20121118143800.GI7306@mtj.dyndns.org>
On 11/18/2012 10:38 PM, Tejun Heo wrote:
> Hello, Aaron.
Hi,
>
> On Wed, Nov 14, 2012 at 09:32:27AM +0800, Aaron Lu wrote:
>>> I don't think you're supposed to use dev->private_data from libata
>>> core layer. Just add a new field. Nobody cares about adding 8 more
>>> bytes to struct ata_device and spending 8 more bytes is way better
>>> than muddying the ownership of ->private_data.
>>
>> OK.
>> And just out of curiosity, who's supposed to use device's private_data?
>> I didn't find any user for ata_device's private_data in libata.
>
> All the ->private_data fields are to be used by low level drivers
> (ahci, ata_piix, pata_via...). Given the twisted nature of ATA
> devices, it's a bit surprising that no driver yet found a need for
> ata_dev->private_data. For most SATA controllers, port to device is
> one to one so maybe ap->private_data is enough.
>
>>> And this gets completely wrong. What if the device supports DA and
>>> low level driver makes use of ->private_data?
>>
>> I didn't find any user of ata_device's private_data, so I used it for
>> ZPODD. But if this is dangerous, I'll use a new field.
>
> As there currently is no other user, it won't break anything but yeah
> please add a properly typed and named field.
OK, and thanks for the suggestion.
-Aaron
>
> Thanks.
>
^ permalink raw reply
* Re: [PATCH v9 07/10] block: add a new interface to block events
From: Alan Stern @ 2012-11-19 2:07 UTC (permalink / raw)
To: Tejun Heo
Cc: Aaron Lu, Jeff Garzik, James Bottomley, Rafael J. Wysocki,
Jeff Wu, Aaron Lu, linux-ide, linux-pm, linux-scsi, linux-acpi
In-Reply-To: <20121118233531.GC25790@mtj.dyndns.org>
On Sun, 18 Nov 2012, Tejun Heo wrote:
> Hey, Alan.
Hey...
> It's controlled by /sys/class/scsi_host/hostX/link_power_management.
> Once enabled, the power saving is completedly handled by hardware. If
> link stays idle longer than certain duration, the hardware initiates
> low power state and leaves it when something needs to happen.
>
> > > So, this whole autopm thing doesn't sound like a good idea to me.
> >
> > No doubt it's better suited to some devices than to others.
>
> Yeah, SATA seems to need a different approach than USB.
Based on your description, I agree.
> > That may be true for SATA. For USB optical drives, it does make sense
> > to power-down the host controller when the drive isn't in use. USB
> > suspend/resume takes on the order of 50-100 ms or so.
>
> I see. For SATA too, the controller / link bring up doesn't take too
> long. The crappy part is what the attached, especially ATAPI, devices
> would do after such events as those events will be visible to them
> basically as random link resets.
>
> So, at least for SATA, I think what autopm can do is...
>
> * Trigger zpodd if possible.
>
> * Trigger suspend iff polling isn't happening on the device.
That sums it up nicely. Of course, the PM core is unaware of details
such as media polling. What we can do is have the SATA runtime-idle
method return -EBUSY if the device isn't a ZPODD and if polling is
enabled. Unfortunately I don't think there's any way currently to
trigger autopm when the user turns off polling. Maybe something could
be added to the appropriate sysfs handler.
Alan Stern
^ permalink raw reply
* Re: [PATCH V6 1/2] Thermal: Add ST-Ericsson DB8500 thermal driver.
From: Zhang Rui @ 2012-11-19 0:42 UTC (permalink / raw)
To: Francesco Lavra
Cc: hongbo.zhang, linaro-kernel, linaro-dev, patches, linux-pm,
linux-kernel, amit.kachhap, STEricsson_nomadik_linux, kernel,
hongbo.zhang
In-Reply-To: <50A8F0DF.8040509@gmail.com>
On Sun, 2012-11-18 at 15:29 +0100, Francesco Lavra wrote:
> On 11/15/2012 01:51 PM, Zhang Rui wrote:
> > On Thu, 2012-11-15 at 18:56 +0800, hongbo.zhang wrote:
> >> From: "hongbo.zhang" <hongbo.zhang@linaro.com>
> >>
> >> This driver is based on the thermal management framework in thermal_sys.c. A
> >> thermal zone device is created with the trip points to which cooling devices
> >> can be bound, the current cooling device is cpufreq, e.g. CPU frequency is
> >> clipped down to cool the CPU, and other cooling devices can be added and bound
> >> to the trip points dynamically. The platform specific PRCMU interrupts are
> >> used to active thermal update when trip points are reached.
> >>
> >> Signed-off-by: hongbo.zhang <hongbo.zhang@linaro.com>
> >> Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>
> >> Reviewed-by: Francesco Lavra <francescolavra.fl@gmail.com>
> >
> > Patch is refreshed and applied to thermal next.
> > refreshed patch attached.
> [...]
> > diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> > index 99b6587..d96da07 100644
> > --- a/drivers/thermal/Kconfig
> > +++ b/drivers/thermal/Kconfig
> > @@ -101,5 +101,25 @@ config EXYNOS_THERMAL
> > If you say yes here you get support for TMU (Thermal Managment
> > Unit) on SAMSUNG EXYNOS series of SoC.
> >
> > +config DB8500_THERMAL
> > + bool "DB8500 thermal management"
> > + depends on ARCH_U8500
>
> Shouldn't it depend on THERMAL as well, as in Hongbo's original patch?
>
all of these options are available only if CONFIG_THERMAL is set.
please refer to commit 72e198978223f2020f7f59a6e2520f2b7d005e72 in the
thermal next tree.
thanks,
rui
> > + default y
> > + help
> > + Adds DB8500 thermal management implementation according to the thermal
> > + management framework. A thermal zone with several trip points will be
> > + created. Cooling devices can be bound to the trip points to cool this
> > + thermal zone if trip points reached.
> > +
> > +config DB8500_CPUFREQ_COOLING
> > + tristate "DB8500 cpufreq cooling"
> > + depends on ARCH_U8500
> > + depends on CPU_THERMAL
> > + default y
> > + help
> > + Adds DB8500 cpufreq cooling devices, and these cooling devices can be
> > + bound to thermal zone trip points. When a trip point reached, the
> > + bound cpufreq cooling device turns active to set CPU frequency low to
> > + cool down the CPU.
> >
> > endif
>
> --
> Francesco
^ 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