* Re: Linux 3.7-rc3
From: Daniel Vetter @ 2012-11-02 21:07 UTC (permalink / raw)
To: Linus Torvalds
Cc: Rafael J. Wysocki, Linux Kernel Mailing List, Linux PM list,
Greg Kroah-Hartman, David Airlie
In-Reply-To: <CA+55aFxm0D77c0RYJuAiq2EDJxn+hGKTEGYJnHXNM8XkVAYOTA@mail.gmail.com>
On Fri, Nov 2, 2012 at 9:25 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Mon, Oct 29, 2012 at 5:10 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
>>
>> Unfortunately, s2disk is broken with this one and previous -rc. In the
>> majority of cases it just hangs the machine during hibernation, although
>> sometimes it returns to user space reporting freezing problems, suspicions
>> RCU usage and similar stuff, pretty much without any useful debug information.
>
> Ugh. I was hoping that we'd get more reports on this, right now
> there's not enough information to even make a guess about what's up.
>
> I also note that no actual i915 people were cc'd, despite your
> graphics driver suspect. Adding Daniel and Dave (one of the millions)
> to the cc since they probably do have lkml but probably don't react
> unless something gets pointed out.
>
> Daniel/Dave, do you have any reports of hibernation failing (or VT
> switching problems) with i915 after 3.6?
Not that I've heard of. We did though rewrite the entire modeset
sequence driving code in i915 for 3.7, removing massive amounts of
hacks and stupid "let's disable/enable things harder" code simply
because the old code couldn't keep track of the hw state well enough.
So I wouldn't be surprised at all if that unearthed a hidden bug
somewhere, but we've also added ridiculous amounts of self-checks. And
thus far they caught all issues due to that rework (and some ancient
bugs on top) when either the i915 driver or the bios did something
nefarious ...
I've rechecked the git log and otherwise there's nothing else which
pops up that would fit VT-switching/hibernate going bad but the
modeset rework.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply
* Re: Linux 3.7-rc3
From: Rafael J. Wysocki @ 2012-11-02 20:35 UTC (permalink / raw)
To: Linus Torvalds
Cc: Linux Kernel Mailing List, Linux PM list, Greg Kroah-Hartman,
Daniel Vetter, David Airlie
In-Reply-To: <CA+55aFxm0D77c0RYJuAiq2EDJxn+hGKTEGYJnHXNM8XkVAYOTA@mail.gmail.com>
On Friday, November 02, 2012 01:25:37 PM Linus Torvalds wrote:
> On Mon, Oct 29, 2012 at 5:10 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> >
> > Unfortunately, s2disk is broken with this one and previous -rc. In the
> > majority of cases it just hangs the machine during hibernation, although
> > sometimes it returns to user space reporting freezing problems, suspicions
> > RCU usage and similar stuff, pretty much without any useful debug information.
>
> Ugh. I was hoping that we'd get more reports on this, right now
> there's not enough information to even make a guess about what's up.
>
> I also note that no actual i915 people were cc'd, despite your
> graphics driver suspect. Adding Daniel and Dave (one of the millions)
> to the cc since they probably do have lkml but probably don't react
> unless something gets pointed out.
>
> Daniel/Dave, do you have any reports of hibernation failing (or VT
> switching problems) with i915 after 3.6?
>
> Rafael, do you have logs of the suspicious RCU usage etc reports?
Not at the moment, unfortunately. I'll try to collect some tomorrow,
but I'm leaving for LinuxCon Europre on Sunday morning, so I'm a bit
time constrained now. :-)
Rafael
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
^ permalink raw reply
* Re: [BUGFIX 1/2] PCI/PM: Fix deadlock when unbind device if its parent in D3cold
From: Rafael J. Wysocki @ 2012-11-02 20:27 UTC (permalink / raw)
To: Bjorn Helgaas; +Cc: Huang Ying, linux-kernel, linux-pci, linux-pm, Zhang Yanmin
In-Reply-To: <CAErSpo7hiFQOC+OPZretkyp5EcL84zEWCAiQgEE5Ewq--gUziQ@mail.gmail.com>
On Friday, November 02, 2012 10:50:50 AM Bjorn Helgaas wrote:
> On Wed, Oct 24, 2012 at 12:54 AM, Huang Ying <ying.huang@intel.com> wrote:
> > If a PCI device and its parents are put into D3cold, unbinding the
> > device will trigger deadlock as follow:
> >
> > - driver_unbind
> > - device_release_driver
> > - device_lock(dev) <--- previous lock here
> > - __device_release_driver
> > - pm_runtime_get_sync
> > ...
> > - rpm_resume(dev)
> > - rpm_resume(dev->parent)
> > ...
> > - pci_pm_runtime_resume
> > ...
> > - pci_set_power_state
> > - __pci_start_power_transition
> > - pci_wakeup_bus(dev->parent->subordinate)
> > - pci_walk_bus
> > - device_lock(dev) <--- dead lock here
> >
> >
> > If we do not do device_lock in pci_walk_bus, we can avoid dead lock.
> > Device_lock in pci_walk_bus is introduced in commit:
> > d71374dafbba7ec3f67371d3b7e9f6310a588808, corresponding email thread
> > is: https://lkml.org/lkml/2006/5/26/38. The patch author Zhang Yanmin
> > said device_lock is added to pci_walk_bus because:
> >
> > Some error handling functions call pci_walk_bus. For example, PCIe
> > aer. Here we lock the device, so the driver wouldn't detach from the
> > device, as the cb might call driver's callback function.
> >
> > So I fixed the dead lock as follow:
> >
> > - remove device_lock from pci_walk_bus
> > - add device_lock into callback if callback will call driver's callback
> >
> > I checked pci_walk_bus users one by one, and found only PCIe aer needs
> > device lock.
>
> Is there a problem report or bugzilla for this issue?
>
> > Signed-off-by: Huang Ying <ying.huang@intel.com>
> > Cc: Zhang Yanmin <yanmin.zhang@intel.com>
>
> Should this go to stable as well? The D3cold support appeared in
> v3.6, so my guess is that this fix could go to v3.6.x.
That's correct.
Thanks,
Rafael
> > ---
> > drivers/pci/bus.c | 3 ---
> > drivers/pci/pcie/aer/aerdrv_core.c | 20 ++++++++++++++++----
> > 2 files changed, 16 insertions(+), 7 deletions(-)
> >
> > --- a/drivers/pci/bus.c
> > +++ b/drivers/pci/bus.c
> > @@ -320,10 +320,7 @@ void pci_walk_bus(struct pci_bus *top, i
> > } else
> > next = dev->bus_list.next;
> >
> > - /* Run device routines with the device locked */
> > - device_lock(&dev->dev);
> > retval = cb(dev, userdata);
> > - device_unlock(&dev->dev);
> > if (retval)
> > break;
> > }
> > --- a/drivers/pci/pcie/aer/aerdrv_core.c
> > +++ b/drivers/pci/pcie/aer/aerdrv_core.c
> > @@ -213,6 +213,7 @@ static int report_error_detected(struct
> > struct aer_broadcast_data *result_data;
> > result_data = (struct aer_broadcast_data *) data;
> >
> > + device_lock(&dev->dev);
> > dev->error_state = result_data->state;
> >
> > if (!dev->driver ||
> > @@ -231,12 +232,14 @@ static int report_error_detected(struct
> > dev->driver ?
> > "no AER-aware driver" : "no driver");
> > }
> > - return 0;
> > + goto out;
> > }
> >
> > err_handler = dev->driver->err_handler;
> > vote = err_handler->error_detected(dev, result_data->state);
> > result_data->result = merge_result(result_data->result, vote);
> > +out:
> > + device_unlock(&dev->dev);
> > return 0;
> > }
> >
> > @@ -247,14 +250,17 @@ static int report_mmio_enabled(struct pc
> > struct aer_broadcast_data *result_data;
> > result_data = (struct aer_broadcast_data *) data;
> >
> > + device_lock(&dev->dev);
> > if (!dev->driver ||
> > !dev->driver->err_handler ||
> > !dev->driver->err_handler->mmio_enabled)
> > - return 0;
> > + goto out;
> >
> > err_handler = dev->driver->err_handler;
> > vote = err_handler->mmio_enabled(dev);
> > result_data->result = merge_result(result_data->result, vote);
> > +out:
> > + device_unlock(&dev->dev);
> > return 0;
> > }
> >
> > @@ -265,14 +271,17 @@ static int report_slot_reset(struct pci_
> > struct aer_broadcast_data *result_data;
> > result_data = (struct aer_broadcast_data *) data;
> >
> > + device_lock(&dev->dev);
> > if (!dev->driver ||
> > !dev->driver->err_handler ||
> > !dev->driver->err_handler->slot_reset)
> > - return 0;
> > + goto out;
> >
> > err_handler = dev->driver->err_handler;
> > vote = err_handler->slot_reset(dev);
> > result_data->result = merge_result(result_data->result, vote);
> > +out:
> > + device_unlock(&dev->dev);
> > return 0;
> > }
> >
> > @@ -280,15 +289,18 @@ static int report_resume(struct pci_dev
> > {
> > const struct pci_error_handlers *err_handler;
> >
> > + device_lock(&dev->dev);
> > dev->error_state = pci_channel_io_normal;
> >
> > if (!dev->driver ||
> > !dev->driver->err_handler ||
> > !dev->driver->err_handler->resume)
> > - return 0;
> > + goto out;
> >
> > err_handler = dev->driver->err_handler;
> > err_handler->resume(dev);
> > +out:
> > + device_unlock(&dev->dev);
> > return 0;
> > }
> >
>
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
^ permalink raw reply
* Re: [BUGFIX 2/2] PCI/PM: Resume device before shutdown
From: Rafael J. Wysocki @ 2012-11-02 20:26 UTC (permalink / raw)
To: Bjorn Helgaas; +Cc: Huang Ying, linux-kernel, linux-pci, linux-pm
In-Reply-To: <CAErSpo5bKDrfOC86Cxv+0yFiyj1bMgWF=6XuY+DWFjaMBTQC0A@mail.gmail.com>
On Friday, November 02, 2012 10:52:45 AM Bjorn Helgaas wrote:
> On Wed, Oct 24, 2012 at 12:54 AM, Huang Ying <ying.huang@intel.com> wrote:
> > Some actions during shutdown need device to be in D0 state, such as
> > MSI shutdown etc, so resume device before shutdown.
>
> Is there a problem report or bugzilla for this issue? What are the
> symptoms by which a user could figure out that he needs this fix?
>
> Should this be put in the stable tree as well? If so, for v3.6 only?
Yes, it should be -stable for v3.6.y.
Thanks,
Rafael
> > Signed-off-by: Huang Ying <ying.huang@intel.com>
> > ---
> > drivers/pci/pci-driver.c | 12 ++----------
> > 1 file changed, 2 insertions(+), 10 deletions(-)
> >
> > --- a/drivers/pci/pci-driver.c
> > +++ b/drivers/pci/pci-driver.c
> > @@ -398,6 +398,8 @@ static void pci_device_shutdown(struct d
> > struct pci_dev *pci_dev = to_pci_dev(dev);
> > struct pci_driver *drv = pci_dev->driver;
> >
> > + pm_runtime_resume(dev);
> > +
> > if (drv && drv->shutdown)
> > drv->shutdown(pci_dev);
> > pci_msi_shutdown(pci_dev);
> > @@ -408,16 +410,6 @@ static void pci_device_shutdown(struct d
> > * continue to do DMA
> > */
> > pci_disable_device(pci_dev);
> > -
> > - /*
> > - * Devices may be enabled to wake up by runtime PM, but they need not
> > - * be supposed to wake up the system from its "power off" state (e.g.
> > - * ACPI S5). Therefore disable wakeup for all devices that aren't
> > - * supposed to wake up the system at this point. The state argument
> > - * will be ignored by pci_enable_wake().
> > - */
> > - if (!device_may_wakeup(dev))
> > - pci_enable_wake(pci_dev, PCI_UNKNOWN, false);
> > }
> >
> > #ifdef CONFIG_PM
>
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
^ permalink raw reply
* Re: Linux 3.7-rc3
From: Linus Torvalds @ 2012-11-02 20:25 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Linux Kernel Mailing List, Linux PM list, Greg Kroah-Hartman,
Daniel Vetter, David Airlie
In-Reply-To: <26004880.qboZkUVCy9@vostro.rjw.lan>
On Mon, Oct 29, 2012 at 5:10 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
>
> Unfortunately, s2disk is broken with this one and previous -rc. In the
> majority of cases it just hangs the machine during hibernation, although
> sometimes it returns to user space reporting freezing problems, suspicions
> RCU usage and similar stuff, pretty much without any useful debug information.
Ugh. I was hoping that we'd get more reports on this, right now
there's not enough information to even make a guess about what's up.
I also note that no actual i915 people were cc'd, despite your
graphics driver suspect. Adding Daniel and Dave (one of the millions)
to the cc since they probably do have lkml but probably don't react
unless something gets pointed out.
Daniel/Dave, do you have any reports of hibernation failing (or VT
switching problems) with i915 after 3.6?
Rafael, do you have logs of the suspicious RCU usage etc reports?
Linus
^ permalink raw reply
* Re: [BUGFIX] PCI/PM: Fix proc config reg access for D3cold and bridge suspending
From: Rafael J. Wysocki @ 2012-11-02 20:25 UTC (permalink / raw)
To: Bjorn Helgaas; +Cc: Huang Ying, linux-kernel, linux-pci, linux-pm, stable
In-Reply-To: <CAErSpo6Q9DGwBJNCY5g3gGVPMCngv6pfLXB6TfiFu3gyLiNp7Q@mail.gmail.com>
On Friday, November 02, 2012 10:38:43 AM Bjorn Helgaas wrote:
> On Wed, Oct 24, 2012 at 7:36 PM, Huang Ying <ying.huang@intel.com> wrote:
> > In
> >
> > https://bugzilla.kernel.org/show_bug.cgi?id=48981
> >
> > Peter reported that /proc/bus/pci/??/??.? does not works for 3.6.
> > This is This is because the device configuration space registers will
> > be not accessible if the corresponding parent bridge is suspended or
> > the device is put into D3cold state.
> >
> > This is the same as /sys/bus/pci/devices/0000:??:??.?/config access
> > issue. So the function used to solve sysfs issue is used to solve
> > this issue.
> >
> > Cc: stable@vger.kernel.org
> > Reported-by: Peter <lekensteyn@gmail.com>
>
> Is this bug the same as the one originally reported by Forrest Loomis
> (original reporter of bug 48981)? And
> https://bugzilla.kernel.org/show_bug.cgi?id=49031, reported by Micael
> Dias (Rafael marked 49031 as a duplicate of 48981)?
>
> If so, I'll mention Forrest and Micael and bug 49031 here as well.
No, it's not. That one turned out to be a duplicate of bko#48981.
Thanks,
Rafael
> > Signed-off-by: Huang Ying <ying.huang@intel.com>
> > ---
> > drivers/pci/pci-sysfs.c | 34 ----------------------------------
> > drivers/pci/pci.c | 32 ++++++++++++++++++++++++++++++++
> > drivers/pci/pci.h | 2 ++
> > drivers/pci/proc.c | 8 ++++++++
> > 4 files changed, 42 insertions(+), 34 deletions(-)
> >
> > --- a/drivers/pci/pci-sysfs.c
> > +++ b/drivers/pci/pci-sysfs.c
> > @@ -458,40 +458,6 @@ boot_vga_show(struct device *dev, struct
> > }
> > struct device_attribute vga_attr = __ATTR_RO(boot_vga);
> >
> > -static void
> > -pci_config_pm_runtime_get(struct pci_dev *pdev)
> > -{
> > - struct device *dev = &pdev->dev;
> > - struct device *parent = dev->parent;
> > -
> > - if (parent)
> > - pm_runtime_get_sync(parent);
> > - pm_runtime_get_noresume(dev);
> > - /*
> > - * pdev->current_state is set to PCI_D3cold during suspending,
> > - * so wait until suspending completes
> > - */
> > - pm_runtime_barrier(dev);
> > - /*
> > - * Only need to resume devices in D3cold, because config
> > - * registers are still accessible for devices suspended but
> > - * not in D3cold.
> > - */
> > - if (pdev->current_state == PCI_D3cold)
> > - pm_runtime_resume(dev);
> > -}
> > -
> > -static void
> > -pci_config_pm_runtime_put(struct pci_dev *pdev)
> > -{
> > - struct device *dev = &pdev->dev;
> > - struct device *parent = dev->parent;
> > -
> > - pm_runtime_put(dev);
> > - if (parent)
> > - pm_runtime_put_sync(parent);
> > -}
> > -
> > static ssize_t
> > pci_read_config(struct file *filp, struct kobject *kobj,
> > struct bin_attribute *bin_attr,
> > --- a/drivers/pci/pci.c
> > +++ b/drivers/pci/pci.c
> > @@ -1858,6 +1858,38 @@ bool pci_dev_run_wake(struct pci_dev *de
> > }
> > EXPORT_SYMBOL_GPL(pci_dev_run_wake);
> >
> > +void pci_config_pm_runtime_get(struct pci_dev *pdev)
> > +{
> > + struct device *dev = &pdev->dev;
> > + struct device *parent = dev->parent;
> > +
> > + if (parent)
> > + pm_runtime_get_sync(parent);
> > + pm_runtime_get_noresume(dev);
> > + /*
> > + * pdev->current_state is set to PCI_D3cold during suspending,
> > + * so wait until suspending completes
> > + */
> > + pm_runtime_barrier(dev);
> > + /*
> > + * Only need to resume devices in D3cold, because config
> > + * registers are still accessible for devices suspended but
> > + * not in D3cold.
> > + */
> > + if (pdev->current_state == PCI_D3cold)
> > + pm_runtime_resume(dev);
> > +}
> > +
> > +void pci_config_pm_runtime_put(struct pci_dev *pdev)
> > +{
> > + struct device *dev = &pdev->dev;
> > + struct device *parent = dev->parent;
> > +
> > + pm_runtime_put(dev);
> > + if (parent)
> > + pm_runtime_put_sync(parent);
> > +}
> > +
> > /**
> > * pci_pm_init - Initialize PM functions of given PCI device
> > * @dev: PCI device to handle.
> > --- a/drivers/pci/pci.h
> > +++ b/drivers/pci/pci.h
> > @@ -72,6 +72,8 @@ extern void pci_disable_enabled_device(s
> > extern int pci_finish_runtime_suspend(struct pci_dev *dev);
> > extern int __pci_pme_wakeup(struct pci_dev *dev, void *ign);
> > extern void pci_wakeup_bus(struct pci_bus *bus);
> > +extern void pci_config_pm_runtime_get(struct pci_dev *dev);
> > +extern void pci_config_pm_runtime_put(struct pci_dev *dev);
> > extern void pci_pm_init(struct pci_dev *dev);
> > extern void platform_pci_wakeup_init(struct pci_dev *dev);
> > extern void pci_allocate_cap_save_buffers(struct pci_dev *dev);
> > --- a/drivers/pci/proc.c
> > +++ b/drivers/pci/proc.c
> > @@ -76,6 +76,8 @@ proc_bus_pci_read(struct file *file, cha
> > if (!access_ok(VERIFY_WRITE, buf, cnt))
> > return -EINVAL;
> >
> > + pci_config_pm_runtime_get(dev);
> > +
> > if ((pos & 1) && cnt) {
> > unsigned char val;
> > pci_user_read_config_byte(dev, pos, &val);
> > @@ -121,6 +123,8 @@ proc_bus_pci_read(struct file *file, cha
> > cnt--;
> > }
> >
> > + pci_config_pm_runtime_put(dev);
> > +
> > *ppos = pos;
> > return nbytes;
> > }
> > @@ -146,6 +150,8 @@ proc_bus_pci_write(struct file *file, co
> > if (!access_ok(VERIFY_READ, buf, cnt))
> > return -EINVAL;
> >
> > + pci_config_pm_runtime_get(dev);
> > +
> > if ((pos & 1) && cnt) {
> > unsigned char val;
> > __get_user(val, buf);
> > @@ -191,6 +197,8 @@ proc_bus_pci_write(struct file *file, co
> > cnt--;
> > }
> >
> > + pci_config_pm_runtime_put(dev);
> > +
> > *ppos = pos;
> > i_size_write(ino, dp->size);
> > return nbytes;
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
^ permalink raw reply
* Re: [PATCH] PM / Qos: Ensure device not in PRM_SUSPENDED when pm qos flags request functions are invoked in the pm core.
From: Rafael J. Wysocki @ 2012-11-02 20:11 UTC (permalink / raw)
To: Lan Tianyu; +Cc: stern, linux-pm, linux-acpi
In-Reply-To: <5093F1F3.5020004@intel.com>
On Saturday, November 03, 2012 12:16:51 AM Lan Tianyu wrote:
> On 2012/11/2 19:17, Rafael J. Wysocki wrote:
> > On Friday, November 02, 2012 04:03:50 PM Lan Tianyu wrote:
> >> Since dev_pm_qos_add_request(), dev_pm_qos_update_request() and
> >> dev_pm_qos_remove_request() for pm qos flags should not be invoked
> >> when device in RPM_SUSPENDED. Add pm_runtime_get_sync() and pm_runtime_put()
> >> around these functions in the pm core to ensure device not in RPM_SUSPENDED.
> >>
> >> Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
> >> ---
> >> drivers/base/power/qos.c | 10 ++++++++--
> >> drivers/base/power/sysfs.c | 4 ++++
> >> 2 files changed, 12 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/base/power/qos.c b/drivers/base/power/qos.c
> >> index 31d3f48..b50ba72 100644
> >> --- a/drivers/base/power/qos.c
> >> +++ b/drivers/base/power/qos.c
> >> @@ -624,15 +624,19 @@ int dev_pm_qos_expose_flags(struct device *dev, s32 val)
> >> if (!req)
> >> return -ENOMEM;
> >>
> >> + pm_runtime_get_sync(dev);
> >> ret = dev_pm_qos_add_request(dev, req, DEV_PM_QOS_FLAGS, val);
> >> + pm_runtime_put(dev);
> >
> > I would drop this one ...
> >
> >> if (ret < 0)
> >> return ret;
> >>
> >> dev->power.qos->flags_req = req;
> >> ret = pm_qos_sysfs_add_flags(dev);
> >> - if (ret)
> >> + if (ret) {
> >> + pm_runtime_get_sync(dev);
> >> __dev_pm_qos_drop_user_request(dev, DEV_PM_QOS_FLAGS);
> >> -
> >> + pm_runtime_put(dev);
> >
> > ... and move this one before the return statement (plus a label for
> > goto from the ret < 0 check after adding the request).
> >
> What you mean likes following?
>
> + pm_runtime_get_sync(dev);
> ret = dev_pm_qos_add_request(dev, req, DEV_PM_QOS_FLAGS, val);
> if (ret < 0)
> - return ret;
> + goto fail;
>
> dev->power.qos->flags_req = req;
> ret = pm_qos_sysfs_add_flags(dev);
> if (ret)
> __dev_pm_qos_drop_user_request(dev, DEV_PM_QOS_FLAGS);
>
> +fail:
> + pm_runtime_put(dev);
> return ret;
> }
>
Yes, it does.
> >> + }
> >> return ret;
> >> }
> >> EXPORT_SYMBOL_GPL(dev_pm_qos_expose_flags);
> >> @@ -645,7 +649,9 @@ void dev_pm_qos_hide_flags(struct device *dev)
> >> {
> >> if (dev->power.qos && dev->power.qos->flags_req) {
> >> pm_qos_sysfs_remove_flags(dev);
> >> + pm_runtime_get_sync(dev);
> >> __dev_pm_qos_drop_user_request(dev, DEV_PM_QOS_FLAGS);
> >> + pm_runtime_put(dev);
> >
> > I'm not sure if these two are necessary. If we remove a request,
> > then what happens worst case is that some flags will be cleared
> > effectively which means fewer restrictions on the next sleep state.
> > Then, it shouldn't hurt that the current sleep state is more
> > restricted.
>
> But this mean the device can be put into lower power state(power off).
> So why not do that? that can save more power, right?
Correct. On the other hand, though, if the device already is in the
deepest low-power state available, we will resume it unnecessarily this
way. Which may not be a big deal, however, and since we do that in other
cases, we may as well do it here.
Thanks,
Rafael
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
^ permalink raw reply
* [PATCH 5/6 v2] power: export opp cpufreq functions
From: Mark Langsdorf @ 2012-11-02 18:51 UTC (permalink / raw)
To: linux-kernel; +Cc: cpufreq, Mark Langsdorf, linux-pm
In-Reply-To: <1351882309-733-1-git-send-email-mark.langsdorf@calxeda.com>
These functions are needed to make the cpufreq-core0 and highbank-cpufreq
drivers loadable as modules.
Signed-off-by: Mark Langsdorf <mark.langsdorf@calxeda.com>
Acked-by: Nishanth Menon <nm@ti.com>
Cc: linux-pm@vger.kernel.org
Changes from v1:
Added Nishanth Menon's ack.
Clarified the purpose of the change in the commit message.
---
drivers/base/power/opp.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c
index d946864..37dc5f4 100644
--- a/drivers/base/power/opp.c
+++ b/drivers/base/power/opp.c
@@ -23,6 +23,7 @@
#include <linux/rcupdate.h>
#include <linux/opp.h>
#include <linux/of.h>
+#include <linux/module.h>
/*
* Internal data structure organization with the OPP layer library is as
@@ -643,6 +644,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
@@ -660,6 +662,7 @@ void opp_free_cpufreq_table(struct device *dev,
kfree(*table);
*table = NULL;
}
+EXPORT_SYMBOL(opp_free_cpufreq_table);
#endif /* CONFIG_CPU_FREQ */
/**
@@ -720,4 +723,5 @@ int of_init_opp_table(struct device *dev)
return 0;
}
+EXPORT_SYMBOL(of_init_opp_table);
#endif
--
1.7.11.7
^ permalink raw reply related
* Re: [BUGFIX 2/2] PCI/PM: Resume device before shutdown
From: Bjorn Helgaas @ 2012-11-02 16:52 UTC (permalink / raw)
To: Huang Ying; +Cc: linux-kernel, linux-pci, linux-pm, Rafael J. Wysocki
In-Reply-To: <1351061654-8339-2-git-send-email-ying.huang@intel.com>
On Wed, Oct 24, 2012 at 12:54 AM, Huang Ying <ying.huang@intel.com> wrote:
> Some actions during shutdown need device to be in D0 state, such as
> MSI shutdown etc, so resume device before shutdown.
Is there a problem report or bugzilla for this issue? What are the
symptoms by which a user could figure out that he needs this fix?
Should this be put in the stable tree as well? If so, for v3.6 only?
> Signed-off-by: Huang Ying <ying.huang@intel.com>
> ---
> drivers/pci/pci-driver.c | 12 ++----------
> 1 file changed, 2 insertions(+), 10 deletions(-)
>
> --- a/drivers/pci/pci-driver.c
> +++ b/drivers/pci/pci-driver.c
> @@ -398,6 +398,8 @@ static void pci_device_shutdown(struct d
> struct pci_dev *pci_dev = to_pci_dev(dev);
> struct pci_driver *drv = pci_dev->driver;
>
> + pm_runtime_resume(dev);
> +
> if (drv && drv->shutdown)
> drv->shutdown(pci_dev);
> pci_msi_shutdown(pci_dev);
> @@ -408,16 +410,6 @@ static void pci_device_shutdown(struct d
> * continue to do DMA
> */
> pci_disable_device(pci_dev);
> -
> - /*
> - * Devices may be enabled to wake up by runtime PM, but they need not
> - * be supposed to wake up the system from its "power off" state (e.g.
> - * ACPI S5). Therefore disable wakeup for all devices that aren't
> - * supposed to wake up the system at this point. The state argument
> - * will be ignored by pci_enable_wake().
> - */
> - if (!device_may_wakeup(dev))
> - pci_enable_wake(pci_dev, PCI_UNKNOWN, false);
> }
>
> #ifdef CONFIG_PM
^ permalink raw reply
* Re: [BUGFIX 1/2] PCI/PM: Fix deadlock when unbind device if its parent in D3cold
From: Bjorn Helgaas @ 2012-11-02 16:50 UTC (permalink / raw)
To: Huang Ying
Cc: linux-kernel, linux-pci, linux-pm, Rafael J. Wysocki,
Zhang Yanmin
In-Reply-To: <1351061654-8339-1-git-send-email-ying.huang@intel.com>
On Wed, Oct 24, 2012 at 12:54 AM, Huang Ying <ying.huang@intel.com> wrote:
> If a PCI device and its parents are put into D3cold, unbinding the
> device will trigger deadlock as follow:
>
> - driver_unbind
> - device_release_driver
> - device_lock(dev) <--- previous lock here
> - __device_release_driver
> - pm_runtime_get_sync
> ...
> - rpm_resume(dev)
> - rpm_resume(dev->parent)
> ...
> - pci_pm_runtime_resume
> ...
> - pci_set_power_state
> - __pci_start_power_transition
> - pci_wakeup_bus(dev->parent->subordinate)
> - pci_walk_bus
> - device_lock(dev) <--- dead lock here
>
>
> If we do not do device_lock in pci_walk_bus, we can avoid dead lock.
> Device_lock in pci_walk_bus is introduced in commit:
> d71374dafbba7ec3f67371d3b7e9f6310a588808, corresponding email thread
> is: https://lkml.org/lkml/2006/5/26/38. The patch author Zhang Yanmin
> said device_lock is added to pci_walk_bus because:
>
> Some error handling functions call pci_walk_bus. For example, PCIe
> aer. Here we lock the device, so the driver wouldn't detach from the
> device, as the cb might call driver's callback function.
>
> So I fixed the dead lock as follow:
>
> - remove device_lock from pci_walk_bus
> - add device_lock into callback if callback will call driver's callback
>
> I checked pci_walk_bus users one by one, and found only PCIe aer needs
> device lock.
Is there a problem report or bugzilla for this issue?
> Signed-off-by: Huang Ying <ying.huang@intel.com>
> Cc: Zhang Yanmin <yanmin.zhang@intel.com>
Should this go to stable as well? The D3cold support appeared in
v3.6, so my guess is that this fix could go to v3.6.x.
> ---
> drivers/pci/bus.c | 3 ---
> drivers/pci/pcie/aer/aerdrv_core.c | 20 ++++++++++++++++----
> 2 files changed, 16 insertions(+), 7 deletions(-)
>
> --- a/drivers/pci/bus.c
> +++ b/drivers/pci/bus.c
> @@ -320,10 +320,7 @@ void pci_walk_bus(struct pci_bus *top, i
> } else
> next = dev->bus_list.next;
>
> - /* Run device routines with the device locked */
> - device_lock(&dev->dev);
> retval = cb(dev, userdata);
> - device_unlock(&dev->dev);
> if (retval)
> break;
> }
> --- a/drivers/pci/pcie/aer/aerdrv_core.c
> +++ b/drivers/pci/pcie/aer/aerdrv_core.c
> @@ -213,6 +213,7 @@ static int report_error_detected(struct
> struct aer_broadcast_data *result_data;
> result_data = (struct aer_broadcast_data *) data;
>
> + device_lock(&dev->dev);
> dev->error_state = result_data->state;
>
> if (!dev->driver ||
> @@ -231,12 +232,14 @@ static int report_error_detected(struct
> dev->driver ?
> "no AER-aware driver" : "no driver");
> }
> - return 0;
> + goto out;
> }
>
> err_handler = dev->driver->err_handler;
> vote = err_handler->error_detected(dev, result_data->state);
> result_data->result = merge_result(result_data->result, vote);
> +out:
> + device_unlock(&dev->dev);
> return 0;
> }
>
> @@ -247,14 +250,17 @@ static int report_mmio_enabled(struct pc
> struct aer_broadcast_data *result_data;
> result_data = (struct aer_broadcast_data *) data;
>
> + device_lock(&dev->dev);
> if (!dev->driver ||
> !dev->driver->err_handler ||
> !dev->driver->err_handler->mmio_enabled)
> - return 0;
> + goto out;
>
> err_handler = dev->driver->err_handler;
> vote = err_handler->mmio_enabled(dev);
> result_data->result = merge_result(result_data->result, vote);
> +out:
> + device_unlock(&dev->dev);
> return 0;
> }
>
> @@ -265,14 +271,17 @@ static int report_slot_reset(struct pci_
> struct aer_broadcast_data *result_data;
> result_data = (struct aer_broadcast_data *) data;
>
> + device_lock(&dev->dev);
> if (!dev->driver ||
> !dev->driver->err_handler ||
> !dev->driver->err_handler->slot_reset)
> - return 0;
> + goto out;
>
> err_handler = dev->driver->err_handler;
> vote = err_handler->slot_reset(dev);
> result_data->result = merge_result(result_data->result, vote);
> +out:
> + device_unlock(&dev->dev);
> return 0;
> }
>
> @@ -280,15 +289,18 @@ static int report_resume(struct pci_dev
> {
> const struct pci_error_handlers *err_handler;
>
> + device_lock(&dev->dev);
> dev->error_state = pci_channel_io_normal;
>
> if (!dev->driver ||
> !dev->driver->err_handler ||
> !dev->driver->err_handler->resume)
> - return 0;
> + goto out;
>
> err_handler = dev->driver->err_handler;
> err_handler->resume(dev);
> +out:
> + device_unlock(&dev->dev);
> return 0;
> }
>
^ permalink raw reply
* Re: [BUGFIX] PCI/PM: Fix proc config reg access for D3cold and bridge suspending
From: Bjorn Helgaas @ 2012-11-02 16:38 UTC (permalink / raw)
To: Huang Ying; +Cc: linux-kernel, linux-pci, linux-pm, Rafael J. Wysocki, stable
In-Reply-To: <1351128963-10347-1-git-send-email-ying.huang@intel.com>
On Wed, Oct 24, 2012 at 7:36 PM, Huang Ying <ying.huang@intel.com> wrote:
> In
>
> https://bugzilla.kernel.org/show_bug.cgi?id=48981
>
> Peter reported that /proc/bus/pci/??/??.? does not works for 3.6.
> This is This is because the device configuration space registers will
> be not accessible if the corresponding parent bridge is suspended or
> the device is put into D3cold state.
>
> This is the same as /sys/bus/pci/devices/0000:??:??.?/config access
> issue. So the function used to solve sysfs issue is used to solve
> this issue.
>
> Cc: stable@vger.kernel.org
> Reported-by: Peter <lekensteyn@gmail.com>
Is this bug the same as the one originally reported by Forrest Loomis
(original reporter of bug 48981)? And
https://bugzilla.kernel.org/show_bug.cgi?id=49031, reported by Micael
Dias (Rafael marked 49031 as a duplicate of 48981)?
If so, I'll mention Forrest and Micael and bug 49031 here as well.
> Signed-off-by: Huang Ying <ying.huang@intel.com>
> ---
> drivers/pci/pci-sysfs.c | 34 ----------------------------------
> drivers/pci/pci.c | 32 ++++++++++++++++++++++++++++++++
> drivers/pci/pci.h | 2 ++
> drivers/pci/proc.c | 8 ++++++++
> 4 files changed, 42 insertions(+), 34 deletions(-)
>
> --- a/drivers/pci/pci-sysfs.c
> +++ b/drivers/pci/pci-sysfs.c
> @@ -458,40 +458,6 @@ boot_vga_show(struct device *dev, struct
> }
> struct device_attribute vga_attr = __ATTR_RO(boot_vga);
>
> -static void
> -pci_config_pm_runtime_get(struct pci_dev *pdev)
> -{
> - struct device *dev = &pdev->dev;
> - struct device *parent = dev->parent;
> -
> - if (parent)
> - pm_runtime_get_sync(parent);
> - pm_runtime_get_noresume(dev);
> - /*
> - * pdev->current_state is set to PCI_D3cold during suspending,
> - * so wait until suspending completes
> - */
> - pm_runtime_barrier(dev);
> - /*
> - * Only need to resume devices in D3cold, because config
> - * registers are still accessible for devices suspended but
> - * not in D3cold.
> - */
> - if (pdev->current_state == PCI_D3cold)
> - pm_runtime_resume(dev);
> -}
> -
> -static void
> -pci_config_pm_runtime_put(struct pci_dev *pdev)
> -{
> - struct device *dev = &pdev->dev;
> - struct device *parent = dev->parent;
> -
> - pm_runtime_put(dev);
> - if (parent)
> - pm_runtime_put_sync(parent);
> -}
> -
> static ssize_t
> pci_read_config(struct file *filp, struct kobject *kobj,
> struct bin_attribute *bin_attr,
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -1858,6 +1858,38 @@ bool pci_dev_run_wake(struct pci_dev *de
> }
> EXPORT_SYMBOL_GPL(pci_dev_run_wake);
>
> +void pci_config_pm_runtime_get(struct pci_dev *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct device *parent = dev->parent;
> +
> + if (parent)
> + pm_runtime_get_sync(parent);
> + pm_runtime_get_noresume(dev);
> + /*
> + * pdev->current_state is set to PCI_D3cold during suspending,
> + * so wait until suspending completes
> + */
> + pm_runtime_barrier(dev);
> + /*
> + * Only need to resume devices in D3cold, because config
> + * registers are still accessible for devices suspended but
> + * not in D3cold.
> + */
> + if (pdev->current_state == PCI_D3cold)
> + pm_runtime_resume(dev);
> +}
> +
> +void pci_config_pm_runtime_put(struct pci_dev *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct device *parent = dev->parent;
> +
> + pm_runtime_put(dev);
> + if (parent)
> + pm_runtime_put_sync(parent);
> +}
> +
> /**
> * pci_pm_init - Initialize PM functions of given PCI device
> * @dev: PCI device to handle.
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -72,6 +72,8 @@ extern void pci_disable_enabled_device(s
> extern int pci_finish_runtime_suspend(struct pci_dev *dev);
> extern int __pci_pme_wakeup(struct pci_dev *dev, void *ign);
> extern void pci_wakeup_bus(struct pci_bus *bus);
> +extern void pci_config_pm_runtime_get(struct pci_dev *dev);
> +extern void pci_config_pm_runtime_put(struct pci_dev *dev);
> extern void pci_pm_init(struct pci_dev *dev);
> extern void platform_pci_wakeup_init(struct pci_dev *dev);
> extern void pci_allocate_cap_save_buffers(struct pci_dev *dev);
> --- a/drivers/pci/proc.c
> +++ b/drivers/pci/proc.c
> @@ -76,6 +76,8 @@ proc_bus_pci_read(struct file *file, cha
> if (!access_ok(VERIFY_WRITE, buf, cnt))
> return -EINVAL;
>
> + pci_config_pm_runtime_get(dev);
> +
> if ((pos & 1) && cnt) {
> unsigned char val;
> pci_user_read_config_byte(dev, pos, &val);
> @@ -121,6 +123,8 @@ proc_bus_pci_read(struct file *file, cha
> cnt--;
> }
>
> + pci_config_pm_runtime_put(dev);
> +
> *ppos = pos;
> return nbytes;
> }
> @@ -146,6 +150,8 @@ proc_bus_pci_write(struct file *file, co
> if (!access_ok(VERIFY_READ, buf, cnt))
> return -EINVAL;
>
> + pci_config_pm_runtime_get(dev);
> +
> if ((pos & 1) && cnt) {
> unsigned char val;
> __get_user(val, buf);
> @@ -191,6 +197,8 @@ proc_bus_pci_write(struct file *file, co
> cnt--;
> }
>
> + pci_config_pm_runtime_put(dev);
> +
> *ppos = pos;
> i_size_write(ino, dp->size);
> return nbytes;
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" 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] PM / Qos: Ensure device not in PRM_SUSPENDED when pm qos flags request functions are invoked in the pm core.
From: Lan Tianyu @ 2012-11-02 16:16 UTC (permalink / raw)
To: Rafael J. Wysocki; +Cc: stern, linux-pm, linux-acpi
In-Reply-To: <1888394.d23n1PsP9K@vostro.rjw.lan>
On 2012/11/2 19:17, Rafael J. Wysocki wrote:
> On Friday, November 02, 2012 04:03:50 PM Lan Tianyu wrote:
>> Since dev_pm_qos_add_request(), dev_pm_qos_update_request() and
>> dev_pm_qos_remove_request() for pm qos flags should not be invoked
>> when device in RPM_SUSPENDED. Add pm_runtime_get_sync() and pm_runtime_put()
>> around these functions in the pm core to ensure device not in RPM_SUSPENDED.
>>
>> Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
>> ---
>> drivers/base/power/qos.c | 10 ++++++++--
>> drivers/base/power/sysfs.c | 4 ++++
>> 2 files changed, 12 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/base/power/qos.c b/drivers/base/power/qos.c
>> index 31d3f48..b50ba72 100644
>> --- a/drivers/base/power/qos.c
>> +++ b/drivers/base/power/qos.c
>> @@ -624,15 +624,19 @@ int dev_pm_qos_expose_flags(struct device *dev, s32 val)
>> if (!req)
>> return -ENOMEM;
>>
>> + pm_runtime_get_sync(dev);
>> ret = dev_pm_qos_add_request(dev, req, DEV_PM_QOS_FLAGS, val);
>> + pm_runtime_put(dev);
>
> I would drop this one ...
>
>> if (ret < 0)
>> return ret;
>>
>> dev->power.qos->flags_req = req;
>> ret = pm_qos_sysfs_add_flags(dev);
>> - if (ret)
>> + if (ret) {
>> + pm_runtime_get_sync(dev);
>> __dev_pm_qos_drop_user_request(dev, DEV_PM_QOS_FLAGS);
>> -
>> + pm_runtime_put(dev);
>
> ... and move this one before the return statement (plus a label for
> goto from the ret < 0 check after adding the request).
>
What you mean likes following?
+ pm_runtime_get_sync(dev);
ret = dev_pm_qos_add_request(dev, req, DEV_PM_QOS_FLAGS, val);
if (ret < 0)
- return ret;
+ goto fail;
dev->power.qos->flags_req = req;
ret = pm_qos_sysfs_add_flags(dev);
if (ret)
__dev_pm_qos_drop_user_request(dev, DEV_PM_QOS_FLAGS);
+fail:
+ pm_runtime_put(dev);
return ret;
}
>> + }
>> return ret;
>> }
>> EXPORT_SYMBOL_GPL(dev_pm_qos_expose_flags);
>> @@ -645,7 +649,9 @@ void dev_pm_qos_hide_flags(struct device *dev)
>> {
>> if (dev->power.qos && dev->power.qos->flags_req) {
>> pm_qos_sysfs_remove_flags(dev);
>> + pm_runtime_get_sync(dev);
>> __dev_pm_qos_drop_user_request(dev, DEV_PM_QOS_FLAGS);
>> + pm_runtime_put(dev);
>
> I'm not sure if these two are necessary. If we remove a request,
> then what happens worst case is that some flags will be cleared
> effectively which means fewer restrictions on the next sleep state.
> Then, it shouldn't hurt that the current sleep state is more
> restricted.
But this mean the device can be put into lower power state(power off).
So why not do that? that can save more power, right?
>
> Hmm. Perhaps a comment would suffice?
>
>> }
>> }
>> EXPORT_SYMBOL_GPL(dev_pm_qos_hide_flags);
>> diff --git a/drivers/base/power/sysfs.c b/drivers/base/power/sysfs.c
>> index 50d16e3..240bfaa 100644
>> --- a/drivers/base/power/sysfs.c
>> +++ b/drivers/base/power/sysfs.c
>> @@ -264,7 +264,9 @@ static ssize_t pm_qos_no_power_off_store(struct device *dev,
>> if (ret != 0 && ret != 1)
>> return -EINVAL;
>>
>> + pm_runtime_get_sync(dev);
>> ret = dev_pm_qos_update_flags(dev, PM_QOS_FLAG_NO_POWER_OFF, ret);
>> + pm_runtime_put(dev);
>> return ret < 0 ? ret : n;
>> }
>
> Well, you haven't noticed that dev_pm_qos_update_flags() already does
> pm_runtime_get_sync()/pm_runtime_put(). :-)
Oh. Yeah. I ignore it. :)
>
>> @@ -291,7 +293,9 @@ static ssize_t pm_qos_remote_wakeup_store(struct device *dev,
>> if (ret != 0 && ret != 1)
>> return -EINVAL;
>>
>> + pm_runtime_get_sync(dev);
>> ret = dev_pm_qos_update_flags(dev, PM_QOS_FLAG_REMOTE_WAKEUP, ret);
>> + pm_runtime_put(dev);
>> return ret < 0 ? ret : n;
>> }
>
> Thanks,
> Rafael
>
>
--
Best Regards
Tianyu Lan
linux kernel enabling team
^ permalink raw reply
* Re: [PATCH V3 0/4] cpuidle : multiple drivers support
From: Lorenzo Pieralisi @ 2012-11-02 13:19 UTC (permalink / raw)
To: Daniel Lezcano
Cc: rjw@sisk.pl, linux-pm@vger.kernel.org, pdeschrijver@nvidia.com,
linaro-dev@lists.linaro.org
In-Reply-To: <1351701888-19963-1-git-send-email-daniel.lezcano@linaro.org>
On Wed, Oct 31, 2012 at 04:44:44PM +0000, Daniel Lezcano wrote:
> The discussion about having different cpus on the system with
> different latencies bring us to a first attemp by adding a
> pointer in the cpuidle_device to the states array.
>
> But as Rafael suggested, it would make more sense to create a
> driver per cpu [1].
>
> This patch adds support for multiple cpuidle drivers.
>
> It creates a per cpu cpuidle driver pointer.
>
> In order to not break the different drivers, the function cpuidle_register_driver
> assign for each cpu, the driver.
>
> The multiple driver support is optional and if it is not set, the cpuide driver
> core code remains the same (except some code reorganisation).
>
> I did the following tests compiled, booted, tested without/with CONFIG_CPU_IDLE,
> with/without CONFIG_CPU_IDLE_MULTIPLE_DRIVERS.
>
> Tested on Core2 Duo T9500 with acpi_idle [and intel_idle]
> Tested on ARM Dual Cortex-A9 U8500 (aka Snowball)
>
> V1 tested on Tegra3 and Vexpress TC2
V3 tested on TC2, hence, on the whole series
Tested-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
>
> [1] http://www.spinics.net/lists/linux-acpi/msg37921.html
>
> Changelog:
>
> V2:
> * fixed sysfs output : /sys/devices/system/cpu/cpu[0-9]/driver/name
> * fixed ifdefs in driver.c
> * fixed register_driver function loop when unregistering
> * removed WARN under spinlock
> * fixed changelog for patch [2/4]
> * changed cpuidle_get_cpu_driver function parameter
> * removed cpuidle_for_each_driver function
> * replaced smp_processor_id() by get_cpu/put_cpu
>
> V3:
> * refreshed patchset
>
> Daniel Lezcano (4):
> cpuidle: move driver's refcount to cpuidle
> cpuidle: move driver checking within the lock section
> cpuidle: prepare the driver core to be multi drivers aware
> cpuidle: support multiple drivers
>
> drivers/cpuidle/Kconfig | 9 ++
> drivers/cpuidle/cpuidle.c | 36 +++++---
> drivers/cpuidle/cpuidle.h | 4 +-
> drivers/cpuidle/driver.c | 209 ++++++++++++++++++++++++++++++++++++++------
> drivers/cpuidle/sysfs.c | 174 ++++++++++++++++++++++++++++++++++++--
> include/linux/cpuidle.h | 8 ++-
> 6 files changed, 388 insertions(+), 52 deletions(-)
>
> --
> 1.7.5.4
>
>
^ permalink raw reply
* Re: [PATCH] cpufreq: remove the unnecessary initialization of a local variable
From: Rafael J. Wysocki @ 2012-11-02 12:54 UTC (permalink / raw)
To: Jingoo Han; +Cc: 'Rafael J. Wysocki', cpufreq, linux-pm, linux-kernel
In-Reply-To: <000901cdb72b$74a6af10$5df40d30$%han@samsung.com>
On Wednesday, October 31, 2012 02:49:13 PM Jingoo Han wrote:
> This patch removes unnecessary initializer for the 'ret' variable.
Applied to the linux-next branch of the linux-pm.git tree as v3.8 material.
Thanks,
Rafael
> Signed-off-by: Jingoo Han <jg1.han@samsung.com>
> ---
> drivers/cpufreq/cpufreq.c | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index 261ef65..4e9fcc5 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -404,7 +404,7 @@ static int __cpufreq_set_policy(struct cpufreq_policy *data,
> static ssize_t store_##file_name \
> (struct cpufreq_policy *policy, const char *buf, size_t count) \
> { \
> - unsigned int ret = -EINVAL; \
> + unsigned int ret; \
> struct cpufreq_policy new_policy; \
> \
> ret = cpufreq_get_policy(&new_policy, policy->cpu); \
> @@ -459,7 +459,7 @@ static ssize_t show_scaling_governor(struct cpufreq_policy *policy, char *buf)
> static ssize_t store_scaling_governor(struct cpufreq_policy *policy,
> const char *buf, size_t count)
> {
> - unsigned int ret = -EINVAL;
> + unsigned int ret;
> char str_governor[16];
> struct cpufreq_policy new_policy;
>
>
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
^ permalink raw reply
* Re: [PATCH V3 0/4] cpuidle : multiple drivers support
From: Rafael J. Wysocki @ 2012-11-02 12:44 UTC (permalink / raw)
To: Daniel Lezcano; +Cc: linux-pm, pdeschrijver, lorenzo.pieralisi, linaro-dev
In-Reply-To: <1351701888-19963-1-git-send-email-daniel.lezcano@linaro.org>
On Wednesday, October 31, 2012 05:44:44 PM Daniel Lezcano wrote:
> The discussion about having different cpus on the system with
> different latencies bring us to a first attemp by adding a
> pointer in the cpuidle_device to the states array.
>
> But as Rafael suggested, it would make more sense to create a
> driver per cpu [1].
>
> This patch adds support for multiple cpuidle drivers.
>
> It creates a per cpu cpuidle driver pointer.
>
> In order to not break the different drivers, the function cpuidle_register_driver
> assign for each cpu, the driver.
>
> The multiple driver support is optional and if it is not set, the cpuide driver
> core code remains the same (except some code reorganisation).
>
> I did the following tests compiled, booted, tested without/with CONFIG_CPU_IDLE,
> with/without CONFIG_CPU_IDLE_MULTIPLE_DRIVERS.
>
> Tested on Core2 Duo T9500 with acpi_idle [and intel_idle]
> Tested on ARM Dual Cortex-A9 U8500 (aka Snowball)
>
> V1 tested on Tegra3 and Vexpress TC2
>
> [1] http://www.spinics.net/lists/linux-acpi/msg37921.html
>
> Changelog:
>
> V2:
> * fixed sysfs output : /sys/devices/system/cpu/cpu[0-9]/driver/name
> * fixed ifdefs in driver.c
> * fixed register_driver function loop when unregistering
> * removed WARN under spinlock
> * fixed changelog for patch [2/4]
> * changed cpuidle_get_cpu_driver function parameter
> * removed cpuidle_for_each_driver function
> * replaced smp_processor_id() by get_cpu/put_cpu
>
> V3:
> * refreshed patchset
>
> Daniel Lezcano (4):
> cpuidle: move driver's refcount to cpuidle
> cpuidle: move driver checking within the lock section
> cpuidle: prepare the driver core to be multi drivers aware
> cpuidle: support multiple drivers
>
> drivers/cpuidle/Kconfig | 9 ++
> drivers/cpuidle/cpuidle.c | 36 +++++---
> drivers/cpuidle/cpuidle.h | 4 +-
> drivers/cpuidle/driver.c | 209 ++++++++++++++++++++++++++++++++++++++------
> drivers/cpuidle/sysfs.c | 174 ++++++++++++++++++++++++++++++++++++--
> include/linux/cpuidle.h | 8 ++-
> 6 files changed, 388 insertions(+), 52 deletions(-)
All patches in the series applied to the linux-next branch of the linux-pm.git
tree as v3.8 material.
Thanks,
Rafael
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
^ permalink raw reply
* Re: [PATCH 5/7] ACPI / PM: Provide device PM functions operating on struct acpi_device
From: Rafael J. Wysocki @ 2012-11-02 11:19 UTC (permalink / raw)
To: Aaron Lu
Cc: Linux PM list, ACPI Devel Maling List, Huang Ying, LKML,
Len Brown, Lv Zheng, Adrian Hunter
In-Reply-To: <50935756.4090405@intel.com>
On Friday, November 02, 2012 01:17:10 PM Aaron Lu wrote:
> On 10/30/2012 11:20 PM, Rafael J. Wysocki wrote:
> > On Tuesday, October 30, 2012 03:28:45 PM Aaron Lu wrote:
> >> On Mon, Oct 29, 2012 at 10:11:20AM +0100, Rafael J. Wysocki wrote:
> >>> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >>>
> >>> If the caller of acpi_bus_set_power() already has a pointer to the
> >>> struct acpi_device object corresponding to the device in question, it
> >>> doesn't make sense for it to go through acpi_bus_get_device(), which
> >>> may be costly, because it involves acquiring the global ACPI
> >>> namespace mutex.
> >>>
> >>> For this reason, export the function operating on struct acpi_device
> >>> objects used internally by acpi_bus_set_power(), so that it may be
> >>> called instead of acpi_bus_set_power() in the above case, and change
> >>> its name to acpi_device_set_power().
> >>>
> >>> Additionally, introduce two inline wrappers for checking ACPI PM
> >>> capabilities of devices represented by struct acpi_device objects.
> >>
> >> What about adding yet another wrapper to check power off capability of
> >> the device? If device has _PS3 or _PRx, it means the device can be
> >> powered off from ACPI's perspective. This is useful for ZPODD when
> >> deciding if platform has the required ability to support it.
> >
> > Sure, no problem with that. Perhaps you can cut a patch for that
> > on top of this series?
>
> Do you think it is reasonable to add a new field to acpi_state.flags to
> represent if we, as OSPM, have a way to put the device into a ACPI
> device state? This field can be set once in acpi_bus_get_power_flags and
> used afterwards.
>
> The valid field of acpi_state.flags is what we have today, and it means
> whether this ACPI device state is valid for the device, but not that if
> OSPM can actually put the device into that power state.
Yes, I think that adding such a new flag would make sense.
Thanks,
Rafael
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
^ permalink raw reply
* Re: [PATCH] PM / Qos: Ensure device not in PRM_SUSPENDED when pm qos flags request functions are invoked in the pm core.
From: Rafael J. Wysocki @ 2012-11-02 11:17 UTC (permalink / raw)
To: Lan Tianyu; +Cc: stern, linux-pm, linux-acpi
In-Reply-To: <1351843430-8023-1-git-send-email-tianyu.lan@intel.com>
On Friday, November 02, 2012 04:03:50 PM Lan Tianyu wrote:
> Since dev_pm_qos_add_request(), dev_pm_qos_update_request() and
> dev_pm_qos_remove_request() for pm qos flags should not be invoked
> when device in RPM_SUSPENDED. Add pm_runtime_get_sync() and pm_runtime_put()
> around these functions in the pm core to ensure device not in RPM_SUSPENDED.
>
> Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
> ---
> drivers/base/power/qos.c | 10 ++++++++--
> drivers/base/power/sysfs.c | 4 ++++
> 2 files changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/base/power/qos.c b/drivers/base/power/qos.c
> index 31d3f48..b50ba72 100644
> --- a/drivers/base/power/qos.c
> +++ b/drivers/base/power/qos.c
> @@ -624,15 +624,19 @@ int dev_pm_qos_expose_flags(struct device *dev, s32 val)
> if (!req)
> return -ENOMEM;
>
> + pm_runtime_get_sync(dev);
> ret = dev_pm_qos_add_request(dev, req, DEV_PM_QOS_FLAGS, val);
> + pm_runtime_put(dev);
I would drop this one ...
> if (ret < 0)
> return ret;
>
> dev->power.qos->flags_req = req;
> ret = pm_qos_sysfs_add_flags(dev);
> - if (ret)
> + if (ret) {
> + pm_runtime_get_sync(dev);
> __dev_pm_qos_drop_user_request(dev, DEV_PM_QOS_FLAGS);
> -
> + pm_runtime_put(dev);
... and move this one before the return statement (plus a label for
goto from the ret < 0 check after adding the request).
> + }
> return ret;
> }
> EXPORT_SYMBOL_GPL(dev_pm_qos_expose_flags);
> @@ -645,7 +649,9 @@ void dev_pm_qos_hide_flags(struct device *dev)
> {
> if (dev->power.qos && dev->power.qos->flags_req) {
> pm_qos_sysfs_remove_flags(dev);
> + pm_runtime_get_sync(dev);
> __dev_pm_qos_drop_user_request(dev, DEV_PM_QOS_FLAGS);
> + pm_runtime_put(dev);
I'm not sure if these two are necessary. If we remove a request,
then what happens worst case is that some flags will be cleared
effectively which means fewer restrictions on the next sleep state.
Then, it shouldn't hurt that the current sleep state is more
restricted.
Hmm. Perhaps a comment would suffice?
> }
> }
> EXPORT_SYMBOL_GPL(dev_pm_qos_hide_flags);
> diff --git a/drivers/base/power/sysfs.c b/drivers/base/power/sysfs.c
> index 50d16e3..240bfaa 100644
> --- a/drivers/base/power/sysfs.c
> +++ b/drivers/base/power/sysfs.c
> @@ -264,7 +264,9 @@ static ssize_t pm_qos_no_power_off_store(struct device *dev,
> if (ret != 0 && ret != 1)
> return -EINVAL;
>
> + pm_runtime_get_sync(dev);
> ret = dev_pm_qos_update_flags(dev, PM_QOS_FLAG_NO_POWER_OFF, ret);
> + pm_runtime_put(dev);
> return ret < 0 ? ret : n;
> }
Well, you haven't noticed that dev_pm_qos_update_flags() already does
pm_runtime_get_sync()/pm_runtime_put(). :-)
> @@ -291,7 +293,9 @@ static ssize_t pm_qos_remote_wakeup_store(struct device *dev,
> if (ret != 0 && ret != 1)
> return -EINVAL;
>
> + pm_runtime_get_sync(dev);
> ret = dev_pm_qos_update_flags(dev, PM_QOS_FLAG_REMOTE_WAKEUP, ret);
> + pm_runtime_put(dev);
> return ret < 0 ? ret : n;
> }
Thanks,
Rafael
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
^ permalink raw reply
* [PATCH] Devfreq: Add debugfs node for representing frequency transition information
From: Jonghwa Lee @ 2012-11-02 9:47 UTC (permalink / raw)
To: linux-pm
Cc: linux-kernel, Kyungmin park, MyungJoo Ham, Rafael J. Wysocki,
Mike Turquette, Jonghwa Lee
This patch adds debugfs node to measure transition of frequency on runtime.
It will be creted under '/sys/kernel/debugfs/devfreq/'dev name'/' as the name
of 'trans_table'. It contains number of transition of each frequency level,
time spent on each level, and also total transition count.
It inspired by CPUFREQ's cpufreq_stats method.
<Example of table>
=================================================================================
<freq level #1> <freq level #2> <freq level #3> time spent
* <freq level #1> n12 n13 t1
<freq level #2> n21 n23 t2
<freq level #3> n31 n32 t3
Total transition : N (= n12 + n13 + n21 + n23 + n31 + n32)
==================================================================================
(n12 : Number of transition from level 1 to level 2)
('*' : The last changed frequency when you inspect the node.)
This patch also includes documentation.
Signed-off-by: Jonghwa Lee <jonghwa3.lee@samsung.com>
---
Documentation/devfreq/transition_status_table | 265 +++++++++++++++++++++++++
drivers/devfreq/devfreq.c | 121 +++++++++++
include/linux/devfreq.h | 23 +++
3 files changed, 409 insertions(+), 0 deletions(-)
create mode 100644 Documentation/devfreq/transition_status_table
diff --git a/Documentation/devfreq/transition_status_table b/Documentation/devfreq/transition_status_table
new file mode 100644
index 0000000..8b92391
--- /dev/null
+++ b/Documentation/devfreq/transition_status_table
@@ -0,0 +1,265 @@
+DEVFREQ Transition status table
+=========================================
+
+Copyright (C) 2012 Samsung Electronics
+
+Written by Jonghwa Lee <jonghwa3.lee@samsung.com>
+
+1. Description
+----------------
+
+DEVFREQ Transition status table represents all of transition information up to now. Transition information
+includes number of transition which has been occured and time spent on the each of frequencies.
+This implementation is ispired by CPUFREQ's cpufreq_stats method.
+
+<Example of table>
+============================================================================================================
+ <freq level #1> <freq level #2> <freq level #3> time spent
+
+* <freq level #1> Number of transition Number of transition t1
+ from freq1 to freq2 from freq1 to freq3
+
+ <freq level #2> Number of transition Number of transition t2
+ from freq2 to freq1 from freq2 to freq3
+
+ <freq level #3> Number of transition Number of transition t3
+ from freq3 to freq1 from freq3 to freq2
+
+Total transition : N
+=============================================================================================================
+('*' : The last changed frequency when you inspect the node.)
+
+It may be used for measuring performance or debug use.
+
+
+2. Requirement to use
+-----------------------
+
+There is no kernel configuration option for creating this node at this moment. Only CONFIG_DEBUG_FS is required.
+It will be created automatically when devfreq device created successfully.
+(under /sys/kernel/debug/devfreq/'each devfreq device name'/ by the name of 'trans_table'.)
+
+By the way, it's going to show nothing unless you go through the below steps.
+To show appropriate information about frequency transition, each devfreq device must hand over their own list of
+available frequency and total count of frequency levels to the framework. Because at the devfreq core side,
+system doesn't know how many freqeuncy levels are available and what value of each level exactly is.
+In devfreq_dev_profile structure, freq_list and freq_levs will play the role carrying the those requirement data.
+
+To guarantee trans_table working, following two steps are required.
+(1) Initialize the freq_list of devfreq_dev_profile with all available freqeuncy data
+(2) Initialize the freq_levs of devfreq_dev_profile with total number of available freqeuncy level.
+
+3. Guage performance
+----------------------
+
+This program is user space program that measures the frequency transition during the certain interval you set.
+When the program starts, it parses the trans_table contents and stores it for later comparison at the end.
+And when you stop the gauging, the program will calculate transition information happened during the interval given.
+The result is also formed like transition status table. However it can be used for more specific purpose and usefully
+than trans_table itself. By analyzing its data, we can get an indicator of performance of various situations or
+performance of governor and others also.
+
+HOW TO USE
+-----------
+1. Set the device name correctly in the code to the variable 'dev_name[]' before the compile.
+ This will be used to get the path of debugfs node. (Default is 'exynos4412-busfreq').
+2. Run the program, and you'll see comments of '### Press any key to start, 'q' to exit >>' or error massage.
+3. Input any key then it starts measurement with comments 'Gauging start...' and '### Press any key....'.
+4. Press any key again. It shows the result of measurement directly.
+5. Program will repeat step 2.
+
+<guage_devfreq.c>
+------------------
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <fcntl.h>
+#include <errno.h>
+#include <string.h>
+#include <termios.h>
+
+#define EXTRA_LINE 3
+#define SEC_TO_MSEC 1000
+
+/* FIXME :
+ * Before compile this program,
+ * you shoud fix dev_name[] with device name
+ * which will be tested.
+ */
+/* for exynos4412 */
+char dev_name[] = "exynos4412-busfreq";
+
+struct trans_info {
+ unsigned int max_state;
+ unsigned int *freq;
+ unsigned int *trans_table;
+ unsigned long *time_in_state;
+};
+
+int parse_status(char *fpath, struct trans_info *info)
+{
+ int i, j;
+ char buf[256];
+ unsigned int ntrans;
+ unsigned long time;
+ int max_state = info->max_state;
+ FILE *fp;
+
+ fp = fopen(fpath, "r");
+ if (fp == NULL) {
+ printf("ERR: Coudn't open the file. \n");
+ return -EINVAL;
+ }
+ if(!info->freq[0]) {
+ /* Init freq array with presented frequency */
+ fgets(buf, 256, fp);
+ fgets(buf, 11, fp);
+
+ for (i = 0; i < max_state; i++)
+ fscanf(fp, "%8d",&info->freq[i]);
+
+ fgets(buf, 13, fp);
+ } else {
+ /* Already initialized freq array. Ignore header */
+ fgets(buf, 256, fp);
+ fgets(buf, 256, fp);
+ }
+ for (i = 0; i < max_state; i++) {
+ fgets(buf, 11, fp);
+ for(j = 0; j < max_state; j++) {
+ fscanf(fp, "%8u",&ntrans);
+ info->trans_table[(max_state * i) + j] =
+ ntrans - info->trans_table[(max_state * i) + j];
+ }
+ fscanf(fp, "%10lu", &time);
+ info->time_in_state[i] = time - info->time_in_state[i];
+ fgetc(fp);
+ }
+
+ return 0;
+}
+
+void show_table(struct trans_info *info)
+{
+ int i, j;
+ unsigned long total_period = 0;
+ int max_state = info->max_state;
+
+ printf("\n\n");
+ for (i = 0; i < (info->max_state / 2) + 1; i++)
+ printf("\t");
+ printf("<Guaging result>\n");
+
+ /* Print table bar */
+ printf(" From : To\n");
+ printf(" :");
+ for (i = 0; i < max_state; i++)
+ printf("%8u", info->freq[i]);
+
+ printf(" time(ms)\n");
+
+ /* Print table entry */
+ for (i = 0; i < max_state; i++) {
+
+ printf("%8u:", info->freq[i]);
+
+ for (j = 0; j < max_state; j++)
+ printf("%8u", info->trans_table[(i *
+ max_state) + j]);
+
+ printf("%10lu\n", info->time_in_state[i]);
+ total_period += info->time_in_state[i];
+ }
+
+
+ printf("Total period : %lu\n", total_period / SEC_TO_MSEC);
+ return;
+}
+
+struct trans_info *trans_info_init(char *fpath)
+{
+ struct trans_info *info;
+ FILE *fp;
+ char buf[256];
+ int cnt = 0;
+
+ fp = fopen(fpath, "r");
+ if (fp == NULL) {
+ fprintf(stderr, "ERROR: Coudn't open the file. \n");
+ exit(errno);
+ }
+
+ /* Count number of lines in table */
+ while (fgets(buf, 256, fp)) {
+ cnt++;
+ }
+
+ info = malloc(sizeof(struct trans_info));
+ info->max_state = cnt - EXTRA_LINE;
+ info->freq = malloc(sizeof(unsigned int) * info->max_state);
+ info->trans_table = malloc(sizeof(unsigned int) * info->max_state
+ * info->max_state);
+ info->time_in_state = malloc(sizeof(unsigned long) * info->max_state);
+
+ fclose(fp);
+
+ return info;
+}
+
+int getch()
+{
+ struct termios oldt, newt;
+ int ch;
+
+ tcgetattr(STDIN_FILENO, &oldt);
+ newt = oldt;
+ newt.c_lflag &= ~(ICANON | ECHO);
+ tcsetattr(STDIN_FILENO, TCSANOW, &newt);
+ ch = getchar();
+ tcsetattr(STDIN_FILENO, TCSANOW, &oldt);
+ return ch;
+}
+
+int main(int argc, char **argv) {
+
+ FILE *fp;
+ struct trans_info *info;
+ char ch, fpath[256];
+ int res;
+
+ strcpy(fpath, "/sys/class/devfreq/");
+ strcat(fpath, dev_name);
+ strcat(fpath, "/trans_stat");
+
+ info = trans_info_init(fpath);
+ if (info == NULL) {
+ fprintf(stderr, "ERROR: Initialization failed\n");
+ exit(errno);
+ }
+
+ printf("### Press any key to start, 'q' to exit >>");
+ ch = getch();
+ if (ch == 'q') {
+ printf("\n\nFinish the guaging....\n");
+ return 0;
+ } else {
+ printf("\nGuaging start...\n");
+ printf("### Press any key to stop guaging >>");
+
+ /* Parse trans_stat */
+ res = parse_status(fpath, info);
+ if (res)
+ return res;
+
+ ch = getch();
+ /* Re-Parse trans_stat after duration */
+ res = parse_status(fpath, info);
+ if (res)
+ return res;
+
+ show_table(info);
+ }
+
+ return 0;
+}
diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index c071ea0..1ad733b 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -27,6 +27,8 @@
#include <linux/hrtimer.h>
#include "governor.h"
+#include <linux/debugfs.h>
+
static struct class *devfreq_class;
/*
@@ -40,6 +42,8 @@ static struct workqueue_struct *devfreq_wq;
static LIST_HEAD(devfreq_list);
static DEFINE_MUTEX(devfreq_list_lock);
+static struct dentry *devfreq_debugfs;
+
/**
* find_device_devfreq() - find devfreq struct using device pointer
* @dev: device pointer used to lookup device devfreq.
@@ -68,6 +72,40 @@ static struct devfreq *find_device_devfreq(struct device *dev)
/* Load monitoring helper functions for governors use */
+int devfreq_get_freq_level(struct devfreq *devfreq, unsigned long freq)
+{
+ int lev;
+
+ for (lev = 0; lev < devfreq->profile->freq_levs; lev++)
+ if (freq == devfreq->profile->freq_list[lev])
+ return lev;
+
+ return -EINVAL;
+}
+
+int devfreq_update_status(struct devfreq *devfreq, unsigned long freq)
+{
+ int lev, prev_lev;
+ unsigned long cur_time;
+
+ prev_lev = devfreq_get_freq_level(devfreq, devfreq->previous_freq);
+ if (prev_lev < 0)
+ return lev;
+
+ cur_time = jiffies;
+ devfreq->time_in_state[prev_lev] +=
+ cur_time - devfreq->last_trinfo_updated;
+ if (freq != devfreq->previous_freq) {
+ lev = devfreq_get_freq_level(devfreq, freq);
+ devfreq->trans_table[(prev_lev *
+ devfreq->profile->freq_levs) + lev]++;
+ devfreq->total_trans++;
+ }
+ devfreq->last_trinfo_updated = cur_time;
+
+ return 0;
+}
+
/**
* update_devfreq() - Reevaluate the device and configure frequency.
* @devfreq: the devfreq instance.
@@ -112,6 +150,11 @@ int update_devfreq(struct devfreq *devfreq)
if (err)
return err;
+ if (devfreq->profile->freq_list)
+ if (devfreq_update_status(devfreq, freq))
+ dev_err(&devfreq->dev,
+ "Couldn't update frequency transition information.\n");
+
devfreq->previous_freq = freq;
return err;
}
@@ -302,6 +345,8 @@ static void _remove_devfreq(struct devfreq *devfreq, bool skip)
devfreq->governor->event_handler(devfreq, DEVFREQ_GOV_STOP, NULL);
+ debugfs_remove_recursive(devfreq->debugfs);
+
if (devfreq->profile->exit)
devfreq->profile->exit(devfreq->dev.parent);
@@ -329,6 +374,59 @@ static void devfreq_dev_release(struct device *dev)
_remove_devfreq(devfreq, true);
}
+static ssize_t show_trans_table(struct file *file, char __user *user_buf,
+ size_t count, loff_t *ppos)
+{
+ char *buf = kmalloc(PAGE_SIZE, GFP_KERNEL);
+ struct devfreq *devfreq = file->private_data;
+ ssize_t len = 0;
+ int i, j, err;
+ unsigned int freq_levs = devfreq->profile->freq_levs;
+
+ err = devfreq_update_status(devfreq, devfreq->previous_freq);
+ if (err)
+ return 0;
+
+ len = snprintf(buf, PAGE_SIZE, " From : To\n");
+ len += snprintf(buf + len, PAGE_SIZE - len, " :");
+ for (i = 0; i < freq_levs; i++)
+ len += snprintf(buf + len, PAGE_SIZE - len, "%8u",
+ devfreq->profile->freq_list[i]);
+
+ len += snprintf(buf + len, PAGE_SIZE - len, " time(ms)\n");
+
+ for (i = 0; i < freq_levs; i++) {
+ if (devfreq->profile->freq_list[i]
+ == devfreq->previous_freq) {
+ len += snprintf(buf + len, PAGE_SIZE - len, "*");
+ } else {
+ len += snprintf(buf + len, PAGE_SIZE - len, " ");
+ }
+ len += snprintf(buf + len, PAGE_SIZE - len, "%8u:",
+ devfreq->profile->freq_list[i]);
+ for (j = 0; j < freq_levs; j++)
+ len += snprintf(buf + len, PAGE_SIZE - len, "%8u",
+ devfreq->trans_table[(i * freq_levs) + j]);
+ len += snprintf(buf + len, PAGE_SIZE - len, "%10u\n",
+ jiffies_to_msecs(devfreq->time_in_state[i]));
+ }
+
+ len += snprintf(buf + len, PAGE_SIZE - len, "Total transition : %u\n",
+ devfreq->total_trans);
+
+ len = simple_read_from_buffer(user_buf, count, ppos, buf, len);
+
+ kfree(buf);
+
+ return len;
+}
+
+static const struct file_operations devfreq_trans_table_fops = {
+ .open = simple_open,
+ .read = show_trans_table,
+ .llseek = default_llseek,
+};
+
/**
* devfreq_add_device() - Add devfreq feature to the device
* @dev: the device to add devfreq feature.
@@ -378,6 +476,15 @@ struct devfreq *devfreq_add_device(struct device *dev,
devfreq->data = data;
devfreq->nb.notifier_call = devfreq_notifier_call;
+ devfreq->trans_table = devm_kzalloc(dev, sizeof(unsigned int) *
+ devfreq->profile->freq_levs *
+ devfreq->profile->freq_levs,
+ GFP_KERNEL);
+ devfreq->time_in_state = devm_kzalloc(dev, sizeof(unsigned int) *
+ devfreq->profile->freq_levs,
+ GFP_KERNEL);
+ devfreq->last_trinfo_updated = jiffies;
+
dev_set_name(&devfreq->dev, dev_name(dev));
err = device_register(&devfreq->dev);
if (err) {
@@ -392,6 +499,15 @@ struct devfreq *devfreq_add_device(struct device *dev,
list_add(&devfreq->node, &devfreq_list);
mutex_unlock(&devfreq_list_lock);
+ mutex_lock(&devfreq->lock);
+ if (!IS_ERR(devfreq_debugfs)) {
+ devfreq->debugfs = debugfs_create_dir(dev_name(dev),
+ devfreq_debugfs);
+ debugfs_create_file("trans_table", S_IRUGO, devfreq->debugfs,
+ devfreq, &devfreq_trans_table_fops);
+ }
+ mutex_unlock(&devfreq->lock);
+
err = devfreq->governor->event_handler(devfreq,
DEVFREQ_GOV_START, NULL);
if (err) {
@@ -629,12 +745,17 @@ static int __init devfreq_init(void)
}
devfreq_class->dev_attrs = devfreq_attrs;
+ devfreq_debugfs = debugfs_create_dir("devfreq", NULL);
+ if (IS_ERR(devfreq_debugfs))
+ pr_err("%s: couldn't create debugfs\n", __FILE__);
+
return 0;
}
subsys_initcall(devfreq_init);
static void __exit devfreq_exit(void)
{
+ debugfs_remove(devfreq_debugfs);
class_destroy(devfreq_class);
destroy_workqueue(devfreq_wq);
}
diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
index 1461fb2..093660e 100644
--- a/include/linux/devfreq.h
+++ b/include/linux/devfreq.h
@@ -57,6 +57,10 @@ struct devfreq_dev_status {
* @initial_freq: The operating frequency when devfreq_add_device() is
* called.
* @polling_ms: The polling interval in ms. 0 disables polling.
+ *
+ * @freq_list List for all availble frequencies in this DEVFREQ.
+ * @freq_levs Total number of frequncy levels.
+ *
* @target: The device should set its operating frequency at
* freq or lowest-upper-than-freq value. If freq is
* higher than any operable frequency, set maximum.
@@ -73,11 +77,16 @@ struct devfreq_dev_status {
* from devfreq_remove_device() call. If the user
* has registered devfreq->nb at a notifier-head,
* this is the time to unregister it.
+ *
+ * @
*/
struct devfreq_dev_profile {
unsigned long initial_freq;
unsigned int polling_ms;
+ unsigned int *freq_list;
+ unsigned int freq_levs;
+
int (*target)(struct device *dev, unsigned long *freq, u32 flags);
int (*get_dev_status)(struct device *dev,
struct devfreq_dev_status *stat);
@@ -128,6 +137,12 @@ struct devfreq_governor {
* @max_freq: Limit maximum frequency requested by user (0: none)
* @stop_polling: devfreq polling status of a device.
*
+ * @total_trans Total trasition number.
+ * @trans_table Transition count of each frequency level.
+ * @time_in_state Spent time of each freqeuncy level.
+ * @last_trinfo_updated Time mark of last transition information updating.
+ *
+ * @debgufs
* This structure stores the devfreq information for a give device.
*
* Note that when a governor accesses entries in struct devfreq in its
@@ -153,6 +168,14 @@ struct devfreq {
unsigned long min_freq;
unsigned long max_freq;
bool stop_polling;
+
+ /* information for device freqeuncy transition */
+ unsigned int total_trans;
+ unsigned int *trans_table;
+ unsigned long *time_in_state;
+ unsigned long last_trinfo_updated;
+
+ struct dentry *debugfs;
};
#if defined(CONFIG_PM_DEVFREQ)
--
1.7.4.1
^ permalink raw reply related
* [PATCH] Devfreq: Add debugfs node for representing frequency transition information
From: Jonghwa Lee @ 2012-11-02 8:56 UTC (permalink / raw)
To: linux-pm
Cc: linux-kernel, Kyungmin park, MyungJoo Ham, Rafael J. Wysocki,
Mike Turquette, Jonghwa Lee
This patch adds debugfs node to measure transition of frequency on runtime.
It will be creted under '/sys/kernel/debugfs/devfreq/'dev name'/' as the name
of 'trans_table'. It contains number of transition of each frequency level,
time spent on each level, and also total transition count.
It inspired by CPUFREQ's cpufreq_stats method.
<Example of table>
=================================================================================
<freq level #1> <freq level #2> <freq level #3> time spent
* <freq level #1> n12 n13 t1
<freq level #2> n21 n23 t2
<freq level #3> n31 n32 t3
Total transition : N (= n12 + n13 + n21 + n23 + n31 + n32)
==================================================================================
(n12 : Number of transition from level 1 to level 2)
('*' : The last changed frequency when you inspect the node.)
This patch also includes documentation.
Signed-off-by: Jonghwa Lee <jonghwa3.lee@samsung.com>
---
Documentation/devfreq/transition_status_table | 265 +++++++++++++++++++++++++
drivers/devfreq/devfreq.c | 123 ++++++++++++
include/linux/devfreq.h | 23 +++
3 files changed, 411 insertions(+), 0 deletions(-)
create mode 100644 Documentation/devfreq/transition_status_table
diff --git a/Documentation/devfreq/transition_status_table b/Documentation/devfreq/transition_status_table
new file mode 100644
index 0000000..8b92391
--- /dev/null
+++ b/Documentation/devfreq/transition_status_table
@@ -0,0 +1,265 @@
+DEVFREQ Transition status table
+=========================================
+
+Copyright (C) 2012 Samsung Electronics
+
+Written by Jonghwa Lee <jonghwa3.lee@samsung.com>
+
+1. Description
+----------------
+
+DEVFREQ Transition status table represents all of transition information up to now. Transition information
+includes number of transition which has been occured and time spent on the each of frequencies.
+This implementation is ispired by CPUFREQ's cpufreq_stats method.
+
+<Example of table>
+============================================================================================================
+ <freq level #1> <freq level #2> <freq level #3> time spent
+
+* <freq level #1> Number of transition Number of transition t1
+ from freq1 to freq2 from freq1 to freq3
+
+ <freq level #2> Number of transition Number of transition t2
+ from freq2 to freq1 from freq2 to freq3
+
+ <freq level #3> Number of transition Number of transition t3
+ from freq3 to freq1 from freq3 to freq2
+
+Total transition : N
+=============================================================================================================
+('*' : The last changed frequency when you inspect the node.)
+
+It may be used for measuring performance or debug use.
+
+
+2. Requirement to use
+-----------------------
+
+There is no kernel configuration option for creating this node at this moment. Only CONFIG_DEBUG_FS is required.
+It will be created automatically when devfreq device created successfully.
+(under /sys/kernel/debug/devfreq/'each devfreq device name'/ by the name of 'trans_table'.)
+
+By the way, it's going to show nothing unless you go through the below steps.
+To show appropriate information about frequency transition, each devfreq device must hand over their own list of
+available frequency and total count of frequency levels to the framework. Because at the devfreq core side,
+system doesn't know how many freqeuncy levels are available and what value of each level exactly is.
+In devfreq_dev_profile structure, freq_list and freq_levs will play the role carrying the those requirement data.
+
+To guarantee trans_table working, following two steps are required.
+(1) Initialize the freq_list of devfreq_dev_profile with all available freqeuncy data
+(2) Initialize the freq_levs of devfreq_dev_profile with total number of available freqeuncy level.
+
+3. Guage performance
+----------------------
+
+This program is user space program that measures the frequency transition during the certain interval you set.
+When the program starts, it parses the trans_table contents and stores it for later comparison at the end.
+And when you stop the gauging, the program will calculate transition information happened during the interval given.
+The result is also formed like transition status table. However it can be used for more specific purpose and usefully
+than trans_table itself. By analyzing its data, we can get an indicator of performance of various situations or
+performance of governor and others also.
+
+HOW TO USE
+-----------
+1. Set the device name correctly in the code to the variable 'dev_name[]' before the compile.
+ This will be used to get the path of debugfs node. (Default is 'exynos4412-busfreq').
+2. Run the program, and you'll see comments of '### Press any key to start, 'q' to exit >>' or error massage.
+3. Input any key then it starts measurement with comments 'Gauging start...' and '### Press any key....'.
+4. Press any key again. It shows the result of measurement directly.
+5. Program will repeat step 2.
+
+<guage_devfreq.c>
+------------------
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <fcntl.h>
+#include <errno.h>
+#include <string.h>
+#include <termios.h>
+
+#define EXTRA_LINE 3
+#define SEC_TO_MSEC 1000
+
+/* FIXME :
+ * Before compile this program,
+ * you shoud fix dev_name[] with device name
+ * which will be tested.
+ */
+/* for exynos4412 */
+char dev_name[] = "exynos4412-busfreq";
+
+struct trans_info {
+ unsigned int max_state;
+ unsigned int *freq;
+ unsigned int *trans_table;
+ unsigned long *time_in_state;
+};
+
+int parse_status(char *fpath, struct trans_info *info)
+{
+ int i, j;
+ char buf[256];
+ unsigned int ntrans;
+ unsigned long time;
+ int max_state = info->max_state;
+ FILE *fp;
+
+ fp = fopen(fpath, "r");
+ if (fp == NULL) {
+ printf("ERR: Coudn't open the file. \n");
+ return -EINVAL;
+ }
+ if(!info->freq[0]) {
+ /* Init freq array with presented frequency */
+ fgets(buf, 256, fp);
+ fgets(buf, 11, fp);
+
+ for (i = 0; i < max_state; i++)
+ fscanf(fp, "%8d",&info->freq[i]);
+
+ fgets(buf, 13, fp);
+ } else {
+ /* Already initialized freq array. Ignore header */
+ fgets(buf, 256, fp);
+ fgets(buf, 256, fp);
+ }
+ for (i = 0; i < max_state; i++) {
+ fgets(buf, 11, fp);
+ for(j = 0; j < max_state; j++) {
+ fscanf(fp, "%8u",&ntrans);
+ info->trans_table[(max_state * i) + j] =
+ ntrans - info->trans_table[(max_state * i) + j];
+ }
+ fscanf(fp, "%10lu", &time);
+ info->time_in_state[i] = time - info->time_in_state[i];
+ fgetc(fp);
+ }
+
+ return 0;
+}
+
+void show_table(struct trans_info *info)
+{
+ int i, j;
+ unsigned long total_period = 0;
+ int max_state = info->max_state;
+
+ printf("\n\n");
+ for (i = 0; i < (info->max_state / 2) + 1; i++)
+ printf("\t");
+ printf("<Guaging result>\n");
+
+ /* Print table bar */
+ printf(" From : To\n");
+ printf(" :");
+ for (i = 0; i < max_state; i++)
+ printf("%8u", info->freq[i]);
+
+ printf(" time(ms)\n");
+
+ /* Print table entry */
+ for (i = 0; i < max_state; i++) {
+
+ printf("%8u:", info->freq[i]);
+
+ for (j = 0; j < max_state; j++)
+ printf("%8u", info->trans_table[(i *
+ max_state) + j]);
+
+ printf("%10lu\n", info->time_in_state[i]);
+ total_period += info->time_in_state[i];
+ }
+
+
+ printf("Total period : %lu\n", total_period / SEC_TO_MSEC);
+ return;
+}
+
+struct trans_info *trans_info_init(char *fpath)
+{
+ struct trans_info *info;
+ FILE *fp;
+ char buf[256];
+ int cnt = 0;
+
+ fp = fopen(fpath, "r");
+ if (fp == NULL) {
+ fprintf(stderr, "ERROR: Coudn't open the file. \n");
+ exit(errno);
+ }
+
+ /* Count number of lines in table */
+ while (fgets(buf, 256, fp)) {
+ cnt++;
+ }
+
+ info = malloc(sizeof(struct trans_info));
+ info->max_state = cnt - EXTRA_LINE;
+ info->freq = malloc(sizeof(unsigned int) * info->max_state);
+ info->trans_table = malloc(sizeof(unsigned int) * info->max_state
+ * info->max_state);
+ info->time_in_state = malloc(sizeof(unsigned long) * info->max_state);
+
+ fclose(fp);
+
+ return info;
+}
+
+int getch()
+{
+ struct termios oldt, newt;
+ int ch;
+
+ tcgetattr(STDIN_FILENO, &oldt);
+ newt = oldt;
+ newt.c_lflag &= ~(ICANON | ECHO);
+ tcsetattr(STDIN_FILENO, TCSANOW, &newt);
+ ch = getchar();
+ tcsetattr(STDIN_FILENO, TCSANOW, &oldt);
+ return ch;
+}
+
+int main(int argc, char **argv) {
+
+ FILE *fp;
+ struct trans_info *info;
+ char ch, fpath[256];
+ int res;
+
+ strcpy(fpath, "/sys/class/devfreq/");
+ strcat(fpath, dev_name);
+ strcat(fpath, "/trans_stat");
+
+ info = trans_info_init(fpath);
+ if (info == NULL) {
+ fprintf(stderr, "ERROR: Initialization failed\n");
+ exit(errno);
+ }
+
+ printf("### Press any key to start, 'q' to exit >>");
+ ch = getch();
+ if (ch == 'q') {
+ printf("\n\nFinish the guaging....\n");
+ return 0;
+ } else {
+ printf("\nGuaging start...\n");
+ printf("### Press any key to stop guaging >>");
+
+ /* Parse trans_stat */
+ res = parse_status(fpath, info);
+ if (res)
+ return res;
+
+ ch = getch();
+ /* Re-Parse trans_stat after duration */
+ res = parse_status(fpath, info);
+ if (res)
+ return res;
+
+ show_table(info);
+ }
+
+ return 0;
+}
diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index b146d76..6012b09 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -27,6 +27,8 @@
#include <linux/hrtimer.h>
#include "governor.h"
+#include <linux/debugfs.h>
+
struct class *devfreq_class;
/*
@@ -46,6 +48,8 @@ static struct devfreq *wait_remove_device;
static LIST_HEAD(devfreq_list);
static DEFINE_MUTEX(devfreq_list_lock);
+static struct dentry *devfreq_debugfs;
+
/**
* find_device_devfreq() - find devfreq struct using device pointer
* @dev: device pointer used to lookup device devfreq.
@@ -72,6 +76,40 @@ static struct devfreq *find_device_devfreq(struct device *dev)
return ERR_PTR(-ENODEV);
}
+int devfreq_get_freq_level(struct devfreq *devfreq, unsigned long freq)
+{
+ int lev;
+
+ for (lev = 0; lev < devfreq->profile->freq_levs; lev++)
+ if (freq == devfreq->profile->freq_list[lev])
+ return lev;
+
+ return -EINVAL;
+}
+
+int devfreq_update_status(struct devfreq *devfreq, unsigned long freq)
+{
+ int lev, prev_lev;
+ unsigned long cur_time;
+
+ prev_lev = devfreq_get_freq_level(devfreq, devfreq->previous_freq);
+ if (prev_lev < 0)
+ return lev;
+
+ cur_time = jiffies;
+ devfreq->time_in_state[prev_lev] +=
+ cur_time - devfreq->last_trinfo_updated;
+ if (freq != devfreq->previous_freq) {
+ lev = devfreq_get_freq_level(devfreq, freq);
+ devfreq->trans_table[(prev_lev *
+ devfreq->profile->freq_levs) + lev]++;
+ devfreq->total_trans++;
+ }
+ devfreq->last_trinfo_updated = cur_time;
+
+ return 0;
+}
+
/**
* update_devfreq() - Reevaluate the device and configure frequency.
* @devfreq: the devfreq instance.
@@ -116,6 +154,11 @@ int update_devfreq(struct devfreq *devfreq)
if (err)
return err;
+ if (devfreq->profile->freq_list)
+ if (devfreq_update_status(devfreq, freq))
+ dev_err(&devfreq->dev,
+ "Couldn't update frequency transition information.\n");
+
devfreq->previous_freq = freq;
return err;
}
@@ -179,6 +222,8 @@ static void _remove_devfreq(struct devfreq *devfreq, bool skip)
devfreq->being_removed = true;
+ debugfs_remove_recursive(devfreq->debugfs);
+
if (devfreq->profile->exit)
devfreq->profile->exit(devfreq->dev.parent);
@@ -336,6 +381,59 @@ static void devfreq_monitor(struct work_struct *work)
}
}
+static ssize_t show_trans_table(struct file *file, char __user *user_buf,
+ size_t count, loff_t *ppos)
+{
+ char *buf = kmalloc(PAGE_SIZE, GFP_KERNEL);
+ struct devfreq *devfreq = file->private_data;
+ ssize_t len = 0;
+ int i, j, err;
+ unsigned int freq_levs = devfreq->profile->freq_levs;
+
+ err = devfreq_update_status(devfreq, devfreq->previous_freq);
+ if (err)
+ return 0;
+
+ len = snprintf(buf, PAGE_SIZE, " From : To\n");
+ len += snprintf(buf + len, PAGE_SIZE - len, " :");
+ for (i = 0; i < freq_levs; i++)
+ len += snprintf(buf + len, PAGE_SIZE - len, "%8u",
+ devfreq->profile->freq_list[i]);
+
+ len += snprintf(buf + len, PAGE_SIZE - len, " time(ms)\n");
+
+ for (i = 0; i < freq_levs; i++) {
+ if (devfreq->profile->freq_list[i]
+ == devfreq->previous_freq) {
+ len += snprintf(buf + len, PAGE_SIZE - len, "*");
+ } else {
+ len += snprintf(buf + len, PAGE_SIZE - len, " ");
+ }
+ len += snprintf(buf + len, PAGE_SIZE - len, "%8u:",
+ devfreq->profile->freq_list[i]);
+ for (j = 0; j < freq_levs; j++)
+ len += snprintf(buf + len, PAGE_SIZE - len, "%8u",
+ devfreq->trans_table[(i * freq_levs) + j]);
+ len += snprintf(buf + len, PAGE_SIZE - len, "%10u\n",
+ jiffies_to_msecs(devfreq->time_in_state[i]));
+ }
+
+ len += snprintf(buf + len, PAGE_SIZE - len, "Total transition : %u\n",
+ devfreq->total_trans);
+
+ len = simple_read_from_buffer(user_buf, count, ppos, buf, len);
+
+ kfree(buf);
+
+ return len;
+}
+
+static const struct file_operations devfreq_trans_table_fops = {
+ .open = simple_open,
+ .read = show_trans_table,
+ .llseek = default_llseek,
+};
+
/**
* devfreq_add_device() - Add devfreq feature to the device
* @dev: the device to add devfreq feature.
@@ -390,6 +488,15 @@ struct devfreq *devfreq_add_device(struct device *dev,
= msecs_to_jiffies(devfreq->profile->polling_ms);
devfreq->nb.notifier_call = devfreq_notifier_call;
+ devfreq->trans_table = devm_kzalloc(dev, sizeof(unsigned int) *
+ devfreq->profile->freq_levs *
+ devfreq->profile->freq_levs,
+ GFP_KERNEL);
+ devfreq->time_in_state = devm_kzalloc(dev, sizeof(unsigned int) *
+ devfreq->profile->freq_levs,
+ GFP_KERNEL);
+ devfreq->last_trinfo_updated = jiffies;
+
dev_set_name(&devfreq->dev, dev_name(dev));
err = device_register(&devfreq->dev);
if (err) {
@@ -417,6 +524,16 @@ struct devfreq *devfreq_add_device(struct device *dev,
devfreq->next_polling);
}
mutex_unlock(&devfreq_list_lock);
+
+ mutex_lock(&devfreq->lock);
+ if (!IS_ERR(devfreq_debugfs)) {
+ devfreq->debugfs = debugfs_create_dir(dev_name(dev),
+ devfreq_debugfs);
+ debugfs_create_file("trans_table", S_IRUGO, devfreq->debugfs,
+ devfreq, &devfreq_trans_table_fops);
+ }
+ mutex_unlock(&devfreq->lock);
+
out:
return devfreq;
@@ -623,12 +740,18 @@ static int __init devfreq_init(void)
return PTR_ERR(devfreq_class);
}
devfreq_class->dev_attrs = devfreq_attrs;
+
+ devfreq_debugfs = debugfs_create_dir("devfreq", NULL);
+ if (IS_ERR(devfreq_debugfs))
+ pr_err("%s: couldn't create debugfs\n", __FILE__);
+
return 0;
}
subsys_initcall(devfreq_init);
static void __exit devfreq_exit(void)
{
+ debugfs_remove(devfreq_debugfs);
class_destroy(devfreq_class);
}
module_exit(devfreq_exit);
diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
index 281c72a..d9cce45 100644
--- a/include/linux/devfreq.h
+++ b/include/linux/devfreq.h
@@ -57,6 +57,10 @@ struct devfreq_dev_status {
* @initial_freq The operating frequency when devfreq_add_device() is
* called.
* @polling_ms The polling interval in ms. 0 disables polling.
+ *
+ * @freq_list List for all availble frequencies in this DEVFREQ.
+ * @freq_levs Total number of frequncy levels.
+ *
* @target The device should set its operating frequency at
* freq or lowest-upper-than-freq value. If freq is
* higher than any operable frequency, set maximum.
@@ -71,11 +75,16 @@ struct devfreq_dev_status {
* from devfreq_remove_device() call. If the user
* has registered devfreq->nb at a notifier-head,
* this is the time to unregister it.
+ *
+ * @
*/
struct devfreq_dev_profile {
unsigned long initial_freq;
unsigned int polling_ms;
+ unsigned int *freq_list;
+ unsigned int freq_levs;
+
int (*target)(struct device *dev, unsigned long *freq, u32 flags);
int (*get_dev_status)(struct device *dev,
struct devfreq_dev_status *stat);
@@ -137,6 +146,12 @@ struct devfreq_governor {
* @min_freq Limit minimum frequency requested by user (0: none)
* @max_freq Limit maximum frequency requested by user (0: none)
*
+ * @total_trans Total trasition number.
+ * @trans_table Transition count of each frequency level.
+ * @time_in_state Spent time of each freqeuncy level.
+ * @last_trinfo_updated Time mark of last transition information updating.
+ *
+ * @debgufs
* This structure stores the devfreq information for a give device.
*
* Note that when a governor accesses entries in struct devfreq in its
@@ -164,6 +179,14 @@ struct devfreq {
unsigned long min_freq;
unsigned long max_freq;
+
+ /* information for device freqeuncy transition */
+ unsigned int total_trans;
+ unsigned int *trans_table;
+ unsigned long *time_in_state;
+ unsigned long last_trinfo_updated;
+
+ struct dentry *debugfs;
};
#if defined(CONFIG_PM_DEVFREQ)
--
1.7.4.1
^ permalink raw reply related
* [PATCH] PM / Qos: Ensure device not in PRM_SUSPENDED when pm qos flags request functions are invoked in the pm core.
From: Lan Tianyu @ 2012-11-02 8:03 UTC (permalink / raw)
To: rjw; +Cc: Lan Tianyu, stern, linux-pm, linux-acpi
Since dev_pm_qos_add_request(), dev_pm_qos_update_request() and
dev_pm_qos_remove_request() for pm qos flags should not be invoked
when device in RPM_SUSPENDED. Add pm_runtime_get_sync() and pm_runtime_put()
around these functions in the pm core to ensure device not in RPM_SUSPENDED.
Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
---
drivers/base/power/qos.c | 10 ++++++++--
drivers/base/power/sysfs.c | 4 ++++
2 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/drivers/base/power/qos.c b/drivers/base/power/qos.c
index 31d3f48..b50ba72 100644
--- a/drivers/base/power/qos.c
+++ b/drivers/base/power/qos.c
@@ -624,15 +624,19 @@ int dev_pm_qos_expose_flags(struct device *dev, s32 val)
if (!req)
return -ENOMEM;
+ pm_runtime_get_sync(dev);
ret = dev_pm_qos_add_request(dev, req, DEV_PM_QOS_FLAGS, val);
+ pm_runtime_put(dev);
if (ret < 0)
return ret;
dev->power.qos->flags_req = req;
ret = pm_qos_sysfs_add_flags(dev);
- if (ret)
+ if (ret) {
+ pm_runtime_get_sync(dev);
__dev_pm_qos_drop_user_request(dev, DEV_PM_QOS_FLAGS);
-
+ pm_runtime_put(dev);
+ }
return ret;
}
EXPORT_SYMBOL_GPL(dev_pm_qos_expose_flags);
@@ -645,7 +649,9 @@ void dev_pm_qos_hide_flags(struct device *dev)
{
if (dev->power.qos && dev->power.qos->flags_req) {
pm_qos_sysfs_remove_flags(dev);
+ pm_runtime_get_sync(dev);
__dev_pm_qos_drop_user_request(dev, DEV_PM_QOS_FLAGS);
+ pm_runtime_put(dev);
}
}
EXPORT_SYMBOL_GPL(dev_pm_qos_hide_flags);
diff --git a/drivers/base/power/sysfs.c b/drivers/base/power/sysfs.c
index 50d16e3..240bfaa 100644
--- a/drivers/base/power/sysfs.c
+++ b/drivers/base/power/sysfs.c
@@ -264,7 +264,9 @@ static ssize_t pm_qos_no_power_off_store(struct device *dev,
if (ret != 0 && ret != 1)
return -EINVAL;
+ pm_runtime_get_sync(dev);
ret = dev_pm_qos_update_flags(dev, PM_QOS_FLAG_NO_POWER_OFF, ret);
+ pm_runtime_put(dev);
return ret < 0 ? ret : n;
}
@@ -291,7 +293,9 @@ static ssize_t pm_qos_remote_wakeup_store(struct device *dev,
if (ret != 0 && ret != 1)
return -EINVAL;
+ pm_runtime_get_sync(dev);
ret = dev_pm_qos_update_flags(dev, PM_QOS_FLAG_REMOTE_WAKEUP, ret);
+ pm_runtime_put(dev);
return ret < 0 ? ret : n;
}
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH 5/7] ACPI / PM: Provide device PM functions operating on struct acpi_device
From: Aaron Lu @ 2012-11-02 5:17 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Linux PM list, ACPI Devel Maling List, Huang Ying, LKML,
Len Brown, Lv Zheng, Adrian Hunter
In-Reply-To: <2204038.IsGMuMD7u4@vostro.rjw.lan>
On 10/30/2012 11:20 PM, Rafael J. Wysocki wrote:
> On Tuesday, October 30, 2012 03:28:45 PM Aaron Lu wrote:
>> On Mon, Oct 29, 2012 at 10:11:20AM +0100, Rafael J. Wysocki wrote:
>>> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>>>
>>> If the caller of acpi_bus_set_power() already has a pointer to the
>>> struct acpi_device object corresponding to the device in question, it
>>> doesn't make sense for it to go through acpi_bus_get_device(), which
>>> may be costly, because it involves acquiring the global ACPI
>>> namespace mutex.
>>>
>>> For this reason, export the function operating on struct acpi_device
>>> objects used internally by acpi_bus_set_power(), so that it may be
>>> called instead of acpi_bus_set_power() in the above case, and change
>>> its name to acpi_device_set_power().
>>>
>>> Additionally, introduce two inline wrappers for checking ACPI PM
>>> capabilities of devices represented by struct acpi_device objects.
>>
>> What about adding yet another wrapper to check power off capability of
>> the device? If device has _PS3 or _PRx, it means the device can be
>> powered off from ACPI's perspective. This is useful for ZPODD when
>> deciding if platform has the required ability to support it.
>
> Sure, no problem with that. Perhaps you can cut a patch for that
> on top of this series?
Do you think it is reasonable to add a new field to acpi_state.flags to
represent if we, as OSPM, have a way to put the device into a ACPI
device state? This field can be set once in acpi_bus_get_power_flags and
used afterwards.
The valid field of acpi_state.flags is what we have today, and it means
whether this ACPI device state is valid for the device, but not that if
OSPM can actually put the device into that power state.
Thanks,
Aaron
>
> Rafael
>
>
>>> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>>> ---
>>> drivers/acpi/bus.c | 15 ++++++++++++---
>>> include/acpi/acpi_bus.h | 11 +++++++++++
>>> 2 files changed, 23 insertions(+), 3 deletions(-)
>>>
>>> Index: linux/drivers/acpi/bus.c
>>> ===================================================================
>>> --- linux.orig/drivers/acpi/bus.c
>>> +++ linux/drivers/acpi/bus.c
>>> @@ -257,7 +257,15 @@ static int __acpi_bus_get_power(struct a
>>> }
>>>
>>>
>>> -static int __acpi_bus_set_power(struct acpi_device *device, int state)
>>> +/**
>>> + * acpi_device_set_power - Set power state of an ACPI device.
>>> + * @device: Device to set the power state of.
>>> + * @state: New power state to set.
>>> + *
>>> + * Callers must ensure that the device is power manageable before using this
>>> + * function.
>>> + */
>>> +int acpi_device_set_power(struct acpi_device *device, int state)
>>> {
>>> int result = 0;
>>> acpi_status status = AE_OK;
>>> @@ -341,6 +349,7 @@ static int __acpi_bus_set_power(struct a
>>>
>>> return result;
>>> }
>>> +EXPORT_SYMBOL(acpi_device_set_power);
>>>
>>>
>>> int acpi_bus_set_power(acpi_handle handle, int state)
>>> @@ -359,7 +368,7 @@ int acpi_bus_set_power(acpi_handle handl
>>> return -ENODEV;
>>> }
>>>
>>> - return __acpi_bus_set_power(device, state);
>>> + return acpi_device_set_power(device, state);
>>> }
>>> EXPORT_SYMBOL(acpi_bus_set_power);
>>>
>>> @@ -402,7 +411,7 @@ int acpi_bus_update_power(acpi_handle ha
>>> if (result)
>>> return result;
>>>
>>> - result = __acpi_bus_set_power(device, state);
>>> + result = acpi_device_set_power(device, state);
>>> if (!result && state_p)
>>> *state_p = state;
>>>
>>> Index: linux/include/acpi/acpi_bus.h
>>> ===================================================================
>>> --- linux.orig/include/acpi/acpi_bus.h
>>> +++ linux/include/acpi/acpi_bus.h
>>> @@ -338,6 +338,7 @@ acpi_status acpi_bus_get_status_handle(a
>>> unsigned long long *sta);
>>> int acpi_bus_get_status(struct acpi_device *device);
>>> int acpi_bus_set_power(acpi_handle handle, int state);
>>> +int acpi_device_set_power(struct acpi_device *device, int state);
>>> int acpi_bus_update_power(acpi_handle handle, int *state_p);
>>> bool acpi_bus_power_manageable(acpi_handle handle);
>>> bool acpi_bus_can_wakeup(acpi_handle handle);
>>> @@ -482,6 +483,16 @@ static inline int acpi_pm_device_sleep_w
>>> }
>>> #endif
>>>
>>> +static inline bool acpi_device_power_manageable(struct acpi_device *adev)
>>> +{
>>> + return adev->flags.power_manageable;
>>> +}
>>> +
>>> +static inline bool acpi_device_can_wakeup(struct acpi_device *adev)
>>> +{
>>> + return adev->wakeup.flags.valid;
>>> +}
>>> +
>>> #else /* CONFIG_ACPI */
>>>
>>> static inline int register_acpi_bus_type(void *bus) { return 0; }
>>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-acpi" 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 v4] Thermal: exynos: Add sysfs node supporting exynos's emulation mode.
From: R, Durgadoss @ 2012-11-02 5:13 UTC (permalink / raw)
To: Jonghwa Lee, linux-pm@vger.kernel.org
Cc: linux-kernel@vger.kernel.org, Brown, Len, Rafael J. Wysocki,
Amit Dinel Kachhap, MyungJoo Ham, Kyungmin Park, Zhang, Rui
In-Reply-To: <1351823091-2540-1-git-send-email-jonghwa3.lee@samsung.com>
Hi Lee,
> -----Original Message-----
> From: Jonghwa Lee [mailto:jonghwa3.lee@samsung.com]
> Sent: Friday, November 02, 2012 7:55 AM
> To: linux-pm@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org; Brown, Len; R, Durgadoss; Rafael J.
> Wysocki; Amit Dinel Kachhap; MyungJoo Ham; Kyungmin Park; Jonghwa Lee
> Subject: [PATCH v4] Thermal: exynos: Add sysfs node supporting exynos's
> emulation mode.
>
> This patch supports exynos's emulation mode with newly created sysfs node.
> Exynos 4x12 (4212, 4412) and 5 series provide emulation mode for thermal
> management unit. Thermal emulation mode supports software debug for
> TMU's
> operation. User can set temperature manually with software code and TMU
> will read current temperature from user value not from sensor's value.
> This patch includes also documentary placed under
> Documentation/thermal/.
>
Thanks for fixing the comments.
Please CC linux-acpi, when you submit thermal patches, going forward.
I am CCing Rui for now, for him to review/merge this patch.
Reviewed-by: Durgadoss R <durgadoss.r@intel.com>
Thanks,
Durga
> Signed-off-by: Jonghwa Lee <jonghwa3.lee@samsung.com>
> ---
> v4
> - Fix Typo.
> - Remove unnecessary codes.
> - Add comments about feature of exynos emulation operation to the
> document.
>
> v3
> - Remove unnecessay variables.
> - Do some code clean in exynos_tmu_emulation_store().
> - Make wrapping function of sysfs node creation function to use
> #ifdefs in minimum.
>
> v2
> exynos_thermal.c
> - Fix build error occured by wrong emulation control register name.
> - Remove exynos5410 dependent codes.
> exynos_thermal_emulation
> - Align indentation.
>
> Documentation/thermal/exynos_thermal_emulation | 56
> +++++++++++++++
> drivers/thermal/Kconfig | 9 +++
> drivers/thermal/exynos_thermal.c | 91
> ++++++++++++++++++++++++
> 3 files changed, 156 insertions(+), 0 deletions(-)
> create mode 100644 Documentation/thermal/exynos_thermal_emulation
>
> diff --git a/Documentation/thermal/exynos_thermal_emulation
> b/Documentation/thermal/exynos_thermal_emulation
> new file mode 100644
> index 0000000..a6ea06f
> --- /dev/null
> +++ b/Documentation/thermal/exynos_thermal_emulation
> @@ -0,0 +1,56 @@
> +EXYNOS EMULATION MODE
> +========================
> +
> +Copyright (C) 2012 Samsung Electronics
> +
> +Written by Jonghwa Lee <jonghwa3.lee@samsung.com>
> +
> +Description
> +-----------
> +
> +Exynos 4x12 (4212, 4412) and 5 series provide emulation mode for thermal
> management unit.
> +Thermal emulation mode supports software debug for TMU's operation.
> User can set temperature
> +manually with software code and TMU will read current temperature from
> user value not from
> +sensor's value.
> +
> +Enabling CONFIG_EXYNOS_THERMAL_EMUL option will make this support
> in available.
> +When it's enabled, sysfs node will be created under
> +/sys/bus/platform/devices/'exynos device name'/ with name of
> 'emulation'.
> +
> +The sysfs node, 'emulation', will contain value 0 for the initial state. When
> you input any
> +temperature you want to update to sysfs node, it automatically enable
> emulation mode and
> +current temperature will be changed into it.
> +(Exynos also supports user changable delay time which would be used to
> delay of
> + changing temperature. However, this node only uses same delay of real
> sensing time, 938us.)
> +
> +Exynos emulation mode requires synchronous of value changing and
> enabling. It means when you
> +want to update the any value of delay or next temperature, then you have
> to enable emulation
> +mode at the same time. (Or you have to keep the mode enabling.) If you
> don't, it fails to
> +change the value to updated one and just use last succeessful value
> repeatedly. That's why
> +this node gives users the right to change termerpature only. Just one
> interface makes it more
> +simply to use.
> +
> +Disabling emulation mode only requires writing value 0 to sysfs node.
> +
> +
> +TEMP 120 |
> + |
> + 100 |
> + |
> + 80 |
> + | +-----------
> + 60 | | |
> + | +-------------| |
> + 40 | | | |
> + | | | |
> + 20 | | | +----------
> + | | | | |
> + 0
> |______________|_____________|__________|__________|_______
> __
> + A A A A TIME
> + |<----->| |<----->| |<----->| |
> + | 938us | | | | | |
> +emulation : 0 50 | 70 | 20 | 0
> +current temp : sensor 50 70 20 sensor
> +
> +
> +
> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> index e1cb6bd..c02a66c 100644
> --- a/drivers/thermal/Kconfig
> +++ b/drivers/thermal/Kconfig
> @@ -55,3 +55,12 @@ config EXYNOS_THERMAL
> help
> If you say yes here you get support for TMU (Thermal Managment
> Unit) on SAMSUNG EXYNOS series of SoC.
> +
> +config EXYNOS_THERMAL_EMUL
> + bool "EXYNOS TMU emulation mode support"
> + depends on !CPU_EXYNOS4210 && EXYNOS_THERMAL
> + help
> + Exynos 4412 and 4414 and 5 series has emulation mode on TMU.
> + Enable this option will be make sysfs node in exynos thermal
> platform
> + device directory to support emulation mode. With emulation mode
> sysfs
> + node, you can manually input temperature to TMU for simulation
> purpose.
> diff --git a/drivers/thermal/exynos_thermal.c
> b/drivers/thermal/exynos_thermal.c
> index fd03e85..eebd4e5 100644
> --- a/drivers/thermal/exynos_thermal.c
> +++ b/drivers/thermal/exynos_thermal.c
> @@ -99,6 +99,14 @@
> #define IDLE_INTERVAL 10000
> #define MCELSIUS 1000
>
> +#ifdef CONFIG_EXYNOS_THERMAL_EMUL
> +#define EXYNOS_EMUL_TIME 0x57F0
> +#define EXYNOS_EMUL_TIME_SHIFT 16
> +#define EXYNOS_EMUL_DATA_SHIFT 8
> +#define EXYNOS_EMUL_DATA_MASK 0xFF
> +#define EXYNOS_EMUL_ENABLE 0x1
> +#endif /* CONFIG_EXYNOS_THERMAL_EMUL */
> +
> /* CPU Zone information */
> #define PANIC_ZONE 4
> #define WARN_ZONE 3
> @@ -832,6 +840,82 @@ static inline struct exynos_tmu_platform_data
> *exynos_get_driver_data(
> return (struct exynos_tmu_platform_data *)
> platform_get_device_id(pdev)->driver_data;
> }
> +
> +#ifdef CONFIG_EXYNOS_THERMAL_EMUL
> +static ssize_t exynos_tmu_emulation_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct platform_device *pdev = container_of(dev,
> + struct platform_device, dev);
> + struct exynos_tmu_data *data = platform_get_drvdata(pdev);
> + unsigned int reg;
> + u8 temp_code;
> + int temp = 0;
> +
> + mutex_lock(&data->lock);
> + clk_enable(data->clk);
> + reg = readl(data->base + EXYNOS_EMUL_CON);
> + clk_disable(data->clk);
> + mutex_unlock(&data->lock);
> +
> + if (reg & EXYNOS_EMUL_ENABLE) {
> + reg >>= EXYNOS_EMUL_DATA_SHIFT;
> + temp_code = reg & EXYNOS_EMUL_DATA_MASK;
> + temp = code_to_temp(data, temp_code);
> + }
> +
> + return sprintf(buf, "%d\n", temp);
> +}
> +
> +static ssize_t exynos_tmu_emulation_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct platform_device *pdev = container_of(dev,
> + struct platform_device, dev);
> + struct exynos_tmu_data *data = platform_get_drvdata(pdev);
> + unsigned int reg;
> + int temp;
> +
> + if (!sscanf(buf, "%d\n", &temp) || temp < 0)
> + return -EINVAL;
> +
> + mutex_lock(&data->lock);
> + clk_enable(data->clk);
> +
> + reg = readl(data->base + EXYNOS_EMUL_CON);
> +
> + if (temp)
> + reg = (EXYNOS_EMUL_TIME << EXYNOS_EMUL_TIME_SHIFT)
> |
> + (temp_to_code(data, temp) <<
> EXYNOS_EMUL_DATA_SHIFT) |
> + EXYNOS_EMUL_ENABLE;
> + else
> + reg &= ~EXYNOS_EMUL_ENABLE;
> +
> + writel(reg, data->base + EXYNOS_EMUL_CON);
> +
> + clk_disable(data->clk);
> + mutex_unlock(&data->lock);
> +
> + return count;
> +}
> +
> +static DEVICE_ATTR(emulation, 0644, exynos_tmu_emulation_show,
> + exynos_tmu_emulation_store);
> +static int create_emulation_sysfs(struct device *dev)
> +{
> + return device_create_file(dev, &dev_attr_emulation);
> +}
> +static void remove_emulation_sysfs(struct device *dev)
> +{
> + device_remove_file(dev, &dev_attr_emulation);
> +}
> +#else
> +static inline int create_emulation_sysfs(struct device *dev) {return 0;}
> +static inline void remove_emulation_sysfs(struct device *dev){}
> +#endif
> +
> static int __devinit exynos_tmu_probe(struct platform_device *pdev)
> {
> struct exynos_tmu_data *data;
> @@ -930,6 +1014,11 @@ static int __devinit exynos_tmu_probe(struct
> platform_device *pdev)
> dev_err(&pdev->dev, "Failed to register thermal
> interface\n");
> goto err_clk;
> }
> +
> + ret = create_emulation_sysfs(&pdev->dev);
> + if (ret)
> + dev_err(&pdev->dev, "Failed to create emulation mode
> sysfs node\n");
> +
> return 0;
> err_clk:
> platform_set_drvdata(pdev, NULL);
> @@ -941,6 +1030,8 @@ static int __devexit exynos_tmu_remove(struct
> platform_device *pdev)
> {
> struct exynos_tmu_data *data = platform_get_drvdata(pdev);
>
> + remove_emulation_sysfs(&pdev->dev);
> +
> exynos_tmu_control(pdev, false);
>
> exynos_unregister_thermal();
> --
> 1.7.4.1
^ permalink raw reply
* drivers/cpuidle/driver.c:81:1: sparse: symbol '__pcpu_scope_cpuidle_drivers' was not declared. Should it be static?
From: kbuild test robot @ 2012-11-02 2:52 UTC (permalink / raw)
To: Daniel Lezcano; +Cc: linux-pm, Rafael J. Wysocki
[-- Attachment #1: Type: text/plain, Size: 573 bytes --]
tree: git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
head: e709b61f9efd3b4ac899e5c4f7b881fdc694f343
commit: 4e6eebfe11765c746aef06f7c6aa0a6d3ff34a1c cpuidle: support multiple drivers
date: 5 hours ago
sparse warnings:
+ drivers/cpuidle/driver.c:81:1: sparse: symbol '__pcpu_scope_cpuidle_drivers' was not declared. Should it be static?
Please consider folding the attached diff :-)
---
0-DAY kernel build testing backend Open Source Technology Center
Fengguang Wu, Yuanhan Liu Intel Corporation
[-- Attachment #2: make-it-static-4e6eebf.diff --]
[-- Type: text/x-diff, Size: 502 bytes --]
diff --git a/drivers/cpuidle/driver.c b/drivers/cpuidle/driver.c
index 465e256..3af841f 100644
--- a/drivers/cpuidle/driver.c
+++ b/drivers/cpuidle/driver.c
@@ -78,7 +78,7 @@ static void __cpuidle_unregister_driver(struct cpuidle_driver *drv, int cpu)
#ifdef CONFIG_CPU_IDLE_MULTIPLE_DRIVERS
-DEFINE_PER_CPU(struct cpuidle_driver *, cpuidle_drivers);
+static DEFINE_PER_CPU(struct cpuidle_driver *, cpuidle_drivers);
static void __cpuidle_set_cpu_driver(struct cpuidle_driver *drv, int cpu)
{
^ permalink raw reply related
* [PATCH v4] Thermal: exynos: Add sysfs node supporting exynos's emulation mode.
From: Jonghwa Lee @ 2012-11-02 2:24 UTC (permalink / raw)
To: linux-pm
Cc: linux-kernel, Len Brown, Durgadoss R, Rafael J. Wysocki,
Amit Dinel Kachhap, MyungJoo Ham, Kyungmin Park, Jonghwa Lee
This patch supports exynos's emulation mode with newly created sysfs node.
Exynos 4x12 (4212, 4412) and 5 series provide emulation mode for thermal
management unit. Thermal emulation mode supports software debug for TMU's
operation. User can set temperature manually with software code and TMU
will read current temperature from user value not from sensor's value.
This patch includes also documentary placed under Documentation/thermal/.
Signed-off-by: Jonghwa Lee <jonghwa3.lee@samsung.com>
---
v4
- Fix Typo.
- Remove unnecessary codes.
- Add comments about feature of exynos emulation operation to the document.
v3
- Remove unnecessay variables.
- Do some code clean in exynos_tmu_emulation_store().
- Make wrapping function of sysfs node creation function to use
#ifdefs in minimum.
v2
exynos_thermal.c
- Fix build error occured by wrong emulation control register name.
- Remove exynos5410 dependent codes.
exynos_thermal_emulation
- Align indentation.
Documentation/thermal/exynos_thermal_emulation | 56 +++++++++++++++
drivers/thermal/Kconfig | 9 +++
drivers/thermal/exynos_thermal.c | 91 ++++++++++++++++++++++++
3 files changed, 156 insertions(+), 0 deletions(-)
create mode 100644 Documentation/thermal/exynos_thermal_emulation
diff --git a/Documentation/thermal/exynos_thermal_emulation b/Documentation/thermal/exynos_thermal_emulation
new file mode 100644
index 0000000..a6ea06f
--- /dev/null
+++ b/Documentation/thermal/exynos_thermal_emulation
@@ -0,0 +1,56 @@
+EXYNOS EMULATION MODE
+========================
+
+Copyright (C) 2012 Samsung Electronics
+
+Written by Jonghwa Lee <jonghwa3.lee@samsung.com>
+
+Description
+-----------
+
+Exynos 4x12 (4212, 4412) and 5 series provide emulation mode for thermal management unit.
+Thermal emulation mode supports software debug for TMU's operation. User can set temperature
+manually with software code and TMU will read current temperature from user value not from
+sensor's value.
+
+Enabling CONFIG_EXYNOS_THERMAL_EMUL option will make this support in available.
+When it's enabled, sysfs node will be created under
+/sys/bus/platform/devices/'exynos device name'/ with name of 'emulation'.
+
+The sysfs node, 'emulation', will contain value 0 for the initial state. When you input any
+temperature you want to update to sysfs node, it automatically enable emulation mode and
+current temperature will be changed into it.
+(Exynos also supports user changable delay time which would be used to delay of
+ changing temperature. However, this node only uses same delay of real sensing time, 938us.)
+
+Exynos emulation mode requires synchronous of value changing and enabling. It means when you
+want to update the any value of delay or next temperature, then you have to enable emulation
+mode at the same time. (Or you have to keep the mode enabling.) If you don't, it fails to
+change the value to updated one and just use last succeessful value repeatedly. That's why
+this node gives users the right to change termerpature only. Just one interface makes it more
+simply to use.
+
+Disabling emulation mode only requires writing value 0 to sysfs node.
+
+
+TEMP 120 |
+ |
+ 100 |
+ |
+ 80 |
+ | +-----------
+ 60 | | |
+ | +-------------| |
+ 40 | | | |
+ | | | |
+ 20 | | | +----------
+ | | | | |
+ 0 |______________|_____________|__________|__________|_________
+ A A A A TIME
+ |<----->| |<----->| |<----->| |
+ | 938us | | | | | |
+emulation : 0 50 | 70 | 20 | 0
+current temp : sensor 50 70 20 sensor
+
+
+
diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
index e1cb6bd..c02a66c 100644
--- a/drivers/thermal/Kconfig
+++ b/drivers/thermal/Kconfig
@@ -55,3 +55,12 @@ config EXYNOS_THERMAL
help
If you say yes here you get support for TMU (Thermal Managment
Unit) on SAMSUNG EXYNOS series of SoC.
+
+config EXYNOS_THERMAL_EMUL
+ bool "EXYNOS TMU emulation mode support"
+ depends on !CPU_EXYNOS4210 && EXYNOS_THERMAL
+ help
+ Exynos 4412 and 4414 and 5 series has emulation mode on TMU.
+ Enable this option will be make sysfs node in exynos thermal platform
+ device directory to support emulation mode. With emulation mode sysfs
+ node, you can manually input temperature to TMU for simulation purpose.
diff --git a/drivers/thermal/exynos_thermal.c b/drivers/thermal/exynos_thermal.c
index fd03e85..eebd4e5 100644
--- a/drivers/thermal/exynos_thermal.c
+++ b/drivers/thermal/exynos_thermal.c
@@ -99,6 +99,14 @@
#define IDLE_INTERVAL 10000
#define MCELSIUS 1000
+#ifdef CONFIG_EXYNOS_THERMAL_EMUL
+#define EXYNOS_EMUL_TIME 0x57F0
+#define EXYNOS_EMUL_TIME_SHIFT 16
+#define EXYNOS_EMUL_DATA_SHIFT 8
+#define EXYNOS_EMUL_DATA_MASK 0xFF
+#define EXYNOS_EMUL_ENABLE 0x1
+#endif /* CONFIG_EXYNOS_THERMAL_EMUL */
+
/* CPU Zone information */
#define PANIC_ZONE 4
#define WARN_ZONE 3
@@ -832,6 +840,82 @@ static inline struct exynos_tmu_platform_data *exynos_get_driver_data(
return (struct exynos_tmu_platform_data *)
platform_get_device_id(pdev)->driver_data;
}
+
+#ifdef CONFIG_EXYNOS_THERMAL_EMUL
+static ssize_t exynos_tmu_emulation_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct platform_device *pdev = container_of(dev,
+ struct platform_device, dev);
+ struct exynos_tmu_data *data = platform_get_drvdata(pdev);
+ unsigned int reg;
+ u8 temp_code;
+ int temp = 0;
+
+ mutex_lock(&data->lock);
+ clk_enable(data->clk);
+ reg = readl(data->base + EXYNOS_EMUL_CON);
+ clk_disable(data->clk);
+ mutex_unlock(&data->lock);
+
+ if (reg & EXYNOS_EMUL_ENABLE) {
+ reg >>= EXYNOS_EMUL_DATA_SHIFT;
+ temp_code = reg & EXYNOS_EMUL_DATA_MASK;
+ temp = code_to_temp(data, temp_code);
+ }
+
+ return sprintf(buf, "%d\n", temp);
+}
+
+static ssize_t exynos_tmu_emulation_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct platform_device *pdev = container_of(dev,
+ struct platform_device, dev);
+ struct exynos_tmu_data *data = platform_get_drvdata(pdev);
+ unsigned int reg;
+ int temp;
+
+ if (!sscanf(buf, "%d\n", &temp) || temp < 0)
+ return -EINVAL;
+
+ mutex_lock(&data->lock);
+ clk_enable(data->clk);
+
+ reg = readl(data->base + EXYNOS_EMUL_CON);
+
+ if (temp)
+ reg = (EXYNOS_EMUL_TIME << EXYNOS_EMUL_TIME_SHIFT) |
+ (temp_to_code(data, temp) << EXYNOS_EMUL_DATA_SHIFT) |
+ EXYNOS_EMUL_ENABLE;
+ else
+ reg &= ~EXYNOS_EMUL_ENABLE;
+
+ writel(reg, data->base + EXYNOS_EMUL_CON);
+
+ clk_disable(data->clk);
+ mutex_unlock(&data->lock);
+
+ return count;
+}
+
+static DEVICE_ATTR(emulation, 0644, exynos_tmu_emulation_show,
+ exynos_tmu_emulation_store);
+static int create_emulation_sysfs(struct device *dev)
+{
+ return device_create_file(dev, &dev_attr_emulation);
+}
+static void remove_emulation_sysfs(struct device *dev)
+{
+ device_remove_file(dev, &dev_attr_emulation);
+}
+#else
+static inline int create_emulation_sysfs(struct device *dev) {return 0;}
+static inline void remove_emulation_sysfs(struct device *dev){}
+#endif
+
static int __devinit exynos_tmu_probe(struct platform_device *pdev)
{
struct exynos_tmu_data *data;
@@ -930,6 +1014,11 @@ static int __devinit exynos_tmu_probe(struct platform_device *pdev)
dev_err(&pdev->dev, "Failed to register thermal interface\n");
goto err_clk;
}
+
+ ret = create_emulation_sysfs(&pdev->dev);
+ if (ret)
+ dev_err(&pdev->dev, "Failed to create emulation mode sysfs node\n");
+
return 0;
err_clk:
platform_set_drvdata(pdev, NULL);
@@ -941,6 +1030,8 @@ static int __devexit exynos_tmu_remove(struct platform_device *pdev)
{
struct exynos_tmu_data *data = platform_get_drvdata(pdev);
+ remove_emulation_sysfs(&pdev->dev);
+
exynos_tmu_control(pdev, false);
exynos_unregister_thermal();
--
1.7.4.1
^ permalink raw reply related
* Re: [PATCH v8 05/11] libata-eh: allow defer in ata_exec_internal
From: Aaron Lu @ 2012-11-02 0:43 UTC (permalink / raw)
To: Tejun Heo
Cc: Jeff Garzik, Rafael J. Wysocki, James Bottomley, Alan Stern,
Oliver Neukum, Jeff Wu, Aaron Lu, Shane Huang, linux-ide,
linux-pm, linux-scsi, linux-acpi
In-Reply-To: <20121101160300.GB9169@htj.dyndns.org>
On 11/02/2012 12:03 AM, Tejun Heo wrote:
> Hello, Aaron.
>
> On Thu, Nov 01, 2012 at 10:35:10AM +0800, Aaron Lu wrote:
>>> You can always add some fields. :)
>>
>> OK. My concern is that, such information is only useful to ZPODD
>> capable device+platforms, so checking this loading mechanism thing for
>> all ATAPI devices during probe time doesn't seem a good idea.
>
> Hmmm.. but it's not like querying acpi is high cost or anything.
> Maybe I'm missing something but if it can be simpler that way, please
> do so by all means. I don't care whether you add some extra fields or
> some processing overhead during probing. It doesn't really matter.
OK, thanks for the suggestion.
>
>>> Hmm... I see. Which ACPI binding is it? The ATA ACPI binding happens
>>> during probing. It's a different one, I presume?
>>
>> Since commit 6b66d95895c149cbc04d4fac5a2f5477c543a8ae:
>> libata: bind the Linux device tree to the ACPI device tree
>> ACPI binding happens when SCSI devices are added to the device tree. The
>> ata port/device software structure does not have a acpi_handle field
>> anymore.
>
> Please bear with me. I haven't paid much attention to zpodd, so it
> probably is my ignorance but at least the ATA <-> ACPI association
> happens during probing by calling ata_acpi_on_devcfg() from
> ata_dev_configure(), and it pretty much has to happen then because
> _SDD/_GTF should be executed after hardreset which happens during
> probing. So, yeah, I'm confused.
Oh, yes, ACPI handle can be retrieved any time by quering some ACPI
method. And during probe time, it is done that way, while the binding
happens much later... But the ACPI handle can indeed be fetched at that
time :-)
So all the conditions I need to test ZPODD support is ready during probe
time and I'll move the check loading mechanism code there, thanks for
your suggestion.
Thanks,
Aaron
^ 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