* Re: use of pm_runtime_disable() from driver probe?
From: Rafael J. Wysocki @ 2012-07-08 14:59 UTC (permalink / raw)
To: Alan Stern; +Cc: Kevin Hilman, linux-pm
In-Reply-To: <Pine.LNX.4.44L0.1207072155400.508-100000@netrider.rowland.org>
On Sunday, July 08, 2012, Alan Stern wrote:
> On Sat, 7 Jul 2012, Rafael J. Wysocki wrote:
>
> > > What is the right way to handle the runtime PM enable/disable in a
> > > failed probe attempt?
> >
> > I would recommend using pm_runtime_disable() and then either
> > pm_runtime_set_active() or pm_runtime_set_suspended() depending on what the
> > real power state of the device is at this point.
> >
> > Anyway, you can't force the device into a low-power state using runtime PM
> > after a failing probe, at least in general.
>
> Right. About the best you could do would be to call the appropriate
> runtime_suspend method directly, instead of going through the runtime
> PM core. However, if the probe is failing then there's no reason to
> think the runtime_suspend method will work.
>
> Basically, on the platform bus, if you don't have a driver for a device
> (which you don't, if the probe routine fails) then you can't do runtime
> PM on it.
>
> But what if the device belongs to a PM domain? Putting the entire
> domain back into a low-power state might have the desired effect -- but
> the probe routine wouldn't be able to do this. The platform subsystem
> would have to take care of it somehow.
Yes.
For example, on sh platforms we call pm_gnepd_poweroff_unused() from a
late initcall (drivers/sh/pm_runtime.c), which causes all PM domains that
only contain suspended devices and devices without drivers to be turned off.
Thanks,
Rafael
^ permalink raw reply
* Re: [PATCH v3 4/7] mfd: omap: control: usb-phy: introduce the ctrl-module usb driver
From: ABRAHAM, KISHON VIJAY @ 2012-07-09 9:23 UTC (permalink / raw)
To: eduardo.valentin, Gupta, Ajay Kumar
Cc: Konstantin Baydarov, b-cousson, santosh.shilimkar, tony, paul,
balbi, amit.kucheria, linux-pm, linux-arm-kernel, linux-omap,
amit.kachhap
In-Reply-To: <20120628045821.GD2471@besouro>
+Ajay
On Thu, Jun 28, 2012 at 10:28 AM, Eduardo Valentin
<eduardo.valentin@ti.com> wrote:
> Hello,
>
> On Wed, Jun 27, 2012 at 10:05:00PM +0400, Konstantin Baydarov wrote:
>> Created a new platform driver for the platform device created by the
>> control module mfd core, wrt usb. This driver has API's to power on/off
>> the phy and the API's to write to musb mailbox.
>>
>> Changes since previous version:
>> - Bandgap and usb phy: drivers are now independent from control module driver, they use
>> their own functions to acess scm registers.
>> - Parent SCM platform device IOMEM resources is used to get the base address of SCM window.
>> - SCM Dependency was removed from Kconfig.
>> - Bandgap and usb phy: Added private spinlocks for bandgap and usb drivers.
>>
>> (p.s. the mailbox for musb in omap4 is present in system control
>> module)
>>
>> [kishon@ti.com: wrote the original API's related to USB functions]
>> Signed-off-by: Konstantin Baydarov <kbaidarov@dev.rtsoft.ru>
>> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
>> Signed-off-by: Eduardo Valentin <eduardo.valentin@ti.com>
>> ---
>> drivers/usb/otg/Kconfig | 12 +++
>> drivers/usb/otg/Makefile | 1 +
>> drivers/usb/otg/omap4-usb-phy.c | 170 +++++++++++++++++++++++++++++++++++++
>> include/linux/usb/omap4_usb_phy.h | 53 ++++++++++++
>> 4 files changed, 236 insertions(+), 0 deletions(-)
>> create mode 100644 drivers/usb/otg/omap4-usb-phy.c
>> create mode 100644 include/linux/usb/omap4_usb_phy.h
>>
>> diff --git a/drivers/usb/otg/Kconfig b/drivers/usb/otg/Kconfig
>> index 5c87db0..0ed691b 100644
>> --- a/drivers/usb/otg/Kconfig
>> +++ b/drivers/usb/otg/Kconfig
>> @@ -78,6 +78,18 @@ config TWL6030_USB
>> are hooked to this driver through platform_data structure.
>> The definition of internal PHY APIs are in the mach-omap2 layer.
>>
>> +config OMAP4_USB_PHY
>> + tristate "Texas Instruments OMAP4+ USB pin control driver"
>> + help
>> + If you say yes here you get support for the Texas Instruments
>> + OMAP4+ USB pin control driver. The register set is part of system
>> + control module.
>> +
>> + USB phy in OMAP configures control module register for powering on
>> + the phy, configuring VBUSVALID, AVALID, IDDIG and SESSEND. For
>> + performing the above mentioned configuration, API's are added in
>> + by this children of the control module driver.
>> +
>> config NOP_USB_XCEIV
>> tristate "NOP USB Transceiver Driver"
>> select USB_OTG_UTILS
>> diff --git a/drivers/usb/otg/Makefile b/drivers/usb/otg/Makefile
>> index 41aa509..60c8c83 100644
>> --- a/drivers/usb/otg/Makefile
>> +++ b/drivers/usb/otg/Makefile
>> @@ -13,6 +13,7 @@ obj-$(CONFIG_USB_GPIO_VBUS) += gpio_vbus.o
>> obj-$(CONFIG_ISP1301_OMAP) += isp1301_omap.o
>> obj-$(CONFIG_TWL4030_USB) += twl4030-usb.o
>> obj-$(CONFIG_TWL6030_USB) += twl6030-usb.o
>> +obj-$(CONFIG_OMAP4_USB_PHY) += omap4-usb-phy.o
>> obj-$(CONFIG_NOP_USB_XCEIV) += nop-usb-xceiv.o
>> obj-$(CONFIG_USB_ULPI) += ulpi.o
>> obj-$(CONFIG_USB_ULPI_VIEWPORT) += ulpi_viewport.o
>> diff --git a/drivers/usb/otg/omap4-usb-phy.c b/drivers/usb/otg/omap4-usb-phy.c
>> new file mode 100644
>> index 0000000..cbea2ea
>> --- /dev/null
>> +++ b/drivers/usb/otg/omap4-usb-phy.c
>> @@ -0,0 +1,170 @@
>> +/*
>> + * OMAP4 system control module driver, USB control children
>> + *
>> + * Copyright (C) 2012 Texas Instruments Incorporated - http://www.ti.com/
>> + *
>> + * Contact:
>> + * Kishon Vijay Abraham I <kishon@ti.com>
>> + * Eduardo Valentin <eduardo.valentin@ti.com>
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License
>> + * version 2 as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful, but
>> + * WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
>> + * General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program; if not, write to the Free Software
>> + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
>> + * 02110-1301 USA
>> + *
>> + */
>> +
>> +#include <linux/module.h>
>> +#include <linux/init.h>
>> +#include <linux/gpio.h>
>> +#include <linux/delay.h>
>> +#include <linux/err.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/usb/omap4_usb_phy.h>
>> +
>> +void __iomem *omap_usb_phy_base;
>> +spinlock_t omap_usb_phy_lock;
>> +
>> +static int omap_usb_phy_readl(u32 reg, u32 *val)
>> +{
>> + if (!omap_usb_phy_base)
>> + return -EINVAL;
>> +
>> + *val = __raw_readl(omap_usb_phy_base + reg);
>> + return 0;
>> +}
>> +
>> +/*
>> + * TODO: Get rid from omap_usb_phy_writel() return value -
>> + * It's useless.
>
> if you do that remember to keep a WARN_ON in case the base address is not set.
>
>> + */
>> +static int omap_usb_phy_writel(u32 val, u32 reg)
>> +{
>> + unsigned long flags;
>> +
>> + if (!omap_usb_phy_base)
>> + return -EINVAL;
>> +
>> + spin_lock_irqsave(&omap_usb_phy_lock, flags);
>> + __raw_writel(val, omap_usb_phy_base + reg);
>> + spin_unlock_irqrestore(&omap_usb_phy_lock, flags);
>
> I think it is better to lock per operation than per write.
>
>> + return 0;
>> +}
>> +
>> +/**
>> + * omap4_usb_phy_power - power on/off the phy using control module reg
>> + * @dev: struct device *
>> + * @on: 0 or 1, based on powering on or off the PHY
>> + *
>> + * omap_usb2 can call this API to power on or off the PHY.
>> + */
>> +int omap4_usb_phy_power(struct device *dev, int on)
>> +{
>> + u32 val;
>> + int ret;
>> +
>> + if (on) {
>> + ret = omap_usb_phy_readl(CONTROL_DEV_CONF, &val);
>> + if (!ret && (val & PHY_PD)) {
>> + ret = omap_usb_phy_writel(~PHY_PD,
>> + CONTROL_DEV_CONF);
>> + /* XXX: add proper documentation for this delay */
>> + mdelay(200);
>> + }
>> + } else
>> + ret = omap_usb_phy_writel(PHY_PD, CONTROL_DEV_CONF);
>> +
>> + return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(omap4_usb_phy_power);
>> +
>> +/**
>> + * omap4_usb_phy_mailbox - write to usb otg mailbox
>> + * @dev: struct device *
>> + * @val: the value to be written to the mailbox
>> + *
>> + * On detection of a device (ID pin is grounded), the phy should call this API
>> + * to set AVALID, VBUSVALID and ID pin is grounded.
>> + *
>> + * When OMAP is connected to a host (OMAP in device mode), the phy should call
>> + * this API to set AVALID, VBUSVALID and ID pin in high impedance.
>> + *
>> + * The phy should call this API, if OMAP is disconnected from host or device.
>> + */
>> +int omap4_usb_phy_mailbox(struct device *dev, u32 val)
>> +{
>> + return omap_usb_phy_writel(val, CONTROL_USBOTGHS_CONTROL);
>> +}
>> +EXPORT_SYMBOL_GPL(omap4_usb_phy_mailbox);
>> +
>> +static int __devinit omap_usb_phy_probe(struct platform_device *pdev)
>> +{
>> + struct resource *io_res;
>> + struct platform_device *pparent;
>> +
>> + if (!pdev->dev.parent) {
>> + dev_err(&pdev->dev, "No parent device!\n");
>> + return -ENOMEM;
>> + }
>> +
>> + pparent = to_platform_device(pdev->dev.parent);
>> +
>> + io_res = platform_get_resource(pparent, IORESOURCE_MEM, 0);
>
> Can someone please explain me what do we gain by doing this at children code?
>
>> + if (!io_res)
>> + return -ENOENT;
>> +
>> + omap_usb_phy_base = ioremap(io_res->start, resource_size(io_res));
>> + if (!omap_usb_phy_base)
>> + return -ENOMEM;
>> +
>> + /* Initialize register lock */
>> + spin_lock_init(&omap_usb_phy_lock);
>> +
>> + return 0;
>> +}
>> +
>> +static int __devexit omap_usb_phy_remove(struct platform_device *pdev)
>> +{
>> + return 0;
>> +}
>> +
>> +static const struct of_device_id of_omap_usb_phy_match[] = {
>> + { .compatible = "ti,omap4-usb-phy", },
>> + { },
>> +};
>> +
>> +static struct platform_driver omap_usb_phy_driver = {
>> + .probe = omap_usb_phy_probe,
>> + .remove = __devexit_p(omap_usb_phy_remove),
>> + .driver = {
>> + .name = "omap4-usb-phy",
>> + .owner = THIS_MODULE,
>> + .of_match_table = of_omap_usb_phy_match,
>> + },
>> +};
>> +
>> +static int __init omap_usb_phy_init(void)
>> +{
>> + return platform_driver_register(&omap_usb_phy_driver);
>> +}
>> +postcore_initcall(omap_usb_phy_init);
>> +
>> +static void __exit omap_usb_phy_exit(void)
>> +{
>> + platform_driver_unregister(&omap_usb_phy_driver);
>> +}
>> +module_exit(omap_usb_phy_exit);
>> +
>> +MODULE_DESCRIPTION("OMAP4+ USB-phy driver");
>> +MODULE_LICENSE("GPL");
>> +MODULE_ALIAS("platform: omap4-usb-phy");
>> +MODULE_AUTHOR("Texas Instrument Inc.");
>> diff --git a/include/linux/usb/omap4_usb_phy.h b/include/linux/usb/omap4_usb_phy.h
>> new file mode 100644
>> index 0000000..b6a4701
>> --- /dev/null
>> +++ b/include/linux/usb/omap4_usb_phy.h
>> @@ -0,0 +1,53 @@
>> +/*
>> + * OMAP4 USB-phy
>> + *
>> + * Copyright (C) 2012 Texas Instruments Incorporated - http://www.ti.com/
>> + *
>> + * Contact:
>> + * Kishon Vijay Abraham I <kishon@ti.com>
>> + * Eduardo Valentin <eduardo.valentin@ti.com>
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License
>> + * version 2 as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful, but
>> + * WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
>> + * General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program; if not, write to the Free Software
>> + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
>> + * 02110-1301 USA
>> + *
>> + */
>> +
>> +#ifndef __OMAP4_USB_PHY_H
>> +#define __OMAP4_USB_PHY_H
>> +
>> +#define PHY_PD 0x1
>> +#define AVALID BIT(0)
>> +#define BVALID BIT(1)
>> +#define VBUSVALID BIT(2)
>> +#define SESSEND BIT(3)
>> +#define IDDIG BIT(4)
>> +#define CONTROL_DEV_CONF 0x00000300
>> +#define CONTROL_USBOTGHS_CONTROL 0x0000033C
>> +
>> +/* USB-PHY helpers */
>> +#if (defined(CONFIG_OMAP4_USB_PHY)) || (defined(CONFIG_OMAP4_USB_PHY_MODULE))
>> +extern int omap4_usb_phy_mailbox(struct device *dev, u32 val);
>> +extern int omap4_usb_phy_power(struct device *dev, int on);
>> +#else
>> +static inline int omap4_usb_phy_mailbox(struct device *dev, u32 val)
>> +{
>> + return 0;
>> +}
>> +static inline int omap4_usb_phy_power(struct device *dev, int on)
>> +{
>> + return 0;
>> +}
>> +#endif
>> +
>> +#endif
>> --
>> 1.7.7.6
>>
>>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" 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 v3 4/7] mfd: omap: control: usb-phy: introduce the ctrl-module usb driver
From: Gupta, Ajay Kumar @ 2012-07-09 9:29 UTC (permalink / raw)
To: ABRAHAM, KISHON VIJAY, Valentin, Eduardo
Cc: Konstantin Baydarov, Cousson, Benoit, Shilimkar, Santosh,
tony@atomide.com, paul@pwsan.com, Balbi, Felipe,
amit.kucheria@linaro.org, linux-pm@lists.linux-foundation.org,
linux-arm-kernel@lists.infradead.org, linux-omap@vger.kernel.org,
amit.kachhap@linaro.org, Damodar Santhapuri
In-Reply-To: <CAAe_U6+YBO-9RE6tML3nW6zVdGRoXHkkbGvGaFYAC2Ag=P7s3w@mail.gmail.com>
Hi,
> On Thu, Jun 28, 2012 at 10:28 AM, Eduardo Valentin
> <eduardo.valentin@ti.com> wrote:
> > Hello,
> >
> > On Wed, Jun 27, 2012 at 10:05:00PM +0400, Konstantin Baydarov wrote:
> >> Created a new platform driver for the platform device created by the
> >> control module mfd core, wrt usb. This driver has API's to power
> on/off the phy and the API's to write to musb mailbox.
USB PHY on am335x platform also need to be enabled using phy control
register in system control module register space.
I think we should make this driver generic enough so that it can be
used by am335x like platforms also to avoid duplication of such drivers.
Ajay
> >>
> >> Changes since previous version:
> >> - Bandgap and usb phy: drivers are now independent from control
> module driver, they use
> >> their own functions to acess scm registers.
> >> - Parent SCM platform device IOMEM resources is used to get the base
> address of SCM window.
> >> - SCM Dependency was removed from Kconfig.
> >> - Bandgap and usb phy: Added private spinlocks for bandgap and usb
> drivers.
> >>
> >> (p.s. the mailbox for musb in omap4 is present in system control
> >> module)
> >>
> >> [kishon@ti.com: wrote the original API's related to USB functions]
> >> Signed-off-by: Konstantin Baydarov <kbaidarov@dev.rtsoft.ru>
> >> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> >> Signed-off-by: Eduardo Valentin <eduardo.valentin@ti.com>
> >> ---
> >> drivers/usb/otg/Kconfig | 12 +++
> >> drivers/usb/otg/Makefile | 1 +
> >> drivers/usb/otg/omap4-usb-phy.c | 170
> +++++++++++++++++++++++++++++++++++++
> >> include/linux/usb/omap4_usb_phy.h | 53 ++++++++++++
> >> 4 files changed, 236 insertions(+), 0 deletions(-)
> >> create mode 100644 drivers/usb/otg/omap4-usb-phy.c
> >> create mode 100644 include/linux/usb/omap4_usb_phy.h
> >>
> >> diff --git a/drivers/usb/otg/Kconfig b/drivers/usb/otg/Kconfig
> >> index 5c87db0..0ed691b 100644
> >> --- a/drivers/usb/otg/Kconfig
> >> +++ b/drivers/usb/otg/Kconfig
> >> @@ -78,6 +78,18 @@ config TWL6030_USB
> >> are hooked to this driver through platform_data structure.
> >> The definition of internal PHY APIs are in the mach-omap2
> layer.
> >>
> >> +config OMAP4_USB_PHY
> >> + tristate "Texas Instruments OMAP4+ USB pin control driver"
> >> + help
> >> + If you say yes here you get support for the Texas
> Instruments
> >> + OMAP4+ USB pin control driver. The register set is part of
> system
> >> + control module.
> >> +
> >> + USB phy in OMAP configures control module register for
> powering on
> >> + the phy, configuring VBUSVALID, AVALID, IDDIG and SESSEND.
> For
> >> + performing the above mentioned configuration, API's are
> added in
> >> + by this children of the control module driver.
> >> +
> >> config NOP_USB_XCEIV
> >> tristate "NOP USB Transceiver Driver"
> >> select USB_OTG_UTILS
> >> diff --git a/drivers/usb/otg/Makefile b/drivers/usb/otg/Makefile
> >> index 41aa509..60c8c83 100644
> >> --- a/drivers/usb/otg/Makefile
> >> +++ b/drivers/usb/otg/Makefile
> >> @@ -13,6 +13,7 @@ obj-$(CONFIG_USB_GPIO_VBUS) += gpio_vbus.o
> >> obj-$(CONFIG_ISP1301_OMAP) += isp1301_omap.o
> >> obj-$(CONFIG_TWL4030_USB) += twl4030-usb.o
> >> obj-$(CONFIG_TWL6030_USB) += twl6030-usb.o
> >> +obj-$(CONFIG_OMAP4_USB_PHY) += omap4-usb-phy.o
> >> obj-$(CONFIG_NOP_USB_XCEIV) += nop-usb-xceiv.o
> >> obj-$(CONFIG_USB_ULPI) += ulpi.o
> >> obj-$(CONFIG_USB_ULPI_VIEWPORT) += ulpi_viewport.o
> >> diff --git a/drivers/usb/otg/omap4-usb-phy.c
> b/drivers/usb/otg/omap4-usb-phy.c
> >> new file mode 100644
> >> index 0000000..cbea2ea
> >> --- /dev/null
> >> +++ b/drivers/usb/otg/omap4-usb-phy.c
> >> @@ -0,0 +1,170 @@
> >> +/*
> >> + * OMAP4 system control module driver, USB control children
> >> + *
> >> + * Copyright (C) 2012 Texas Instruments Incorporated -
> http://www.ti.com/
> >> + *
> >> + * Contact:
> >> + * Kishon Vijay Abraham I <kishon@ti.com>
> >> + * Eduardo Valentin <eduardo.valentin@ti.com>
> >> + *
> >> + * This program is free software; you can redistribute it and/or
> >> + * modify it under the terms of the GNU General Public License
> >> + * version 2 as published by the Free Software Foundation.
> >> + *
> >> + * This program is distributed in the hope that it will be useful,
> but
> >> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> GNU
> >> + * General Public License for more details.
> >> + *
> >> + * You should have received a copy of the GNU General Public
> License
> >> + * along with this program; if not, write to the Free Software
> >> + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
> >> + * 02110-1301 USA
> >> + *
> >> + */
> >> +
> >> +#include <linux/module.h>
> >> +#include <linux/init.h>
> >> +#include <linux/gpio.h>
> >> +#include <linux/delay.h>
> >> +#include <linux/err.h>
> >> +#include <linux/platform_device.h>
> >> +#include <linux/usb/omap4_usb_phy.h>
> >> +
> >> +void __iomem *omap_usb_phy_base;
> >> +spinlock_t omap_usb_phy_lock;
> >> +
> >> +static int omap_usb_phy_readl(u32 reg, u32 *val)
> >> +{
> >> + if (!omap_usb_phy_base)
> >> + return -EINVAL;
> >> +
> >> + *val = __raw_readl(omap_usb_phy_base + reg);
> >> + return 0;
> >> +}
> >> +
> >> +/*
> >> + * TODO: Get rid from omap_usb_phy_writel() return value -
> >> + * It's useless.
> >
> > if you do that remember to keep a WARN_ON in case the base address is
> not set.
> >
> >> + */
> >> +static int omap_usb_phy_writel(u32 val, u32 reg)
> >> +{
> >> + unsigned long flags;
> >> +
> >> + if (!omap_usb_phy_base)
> >> + return -EINVAL;
> >> +
> >> + spin_lock_irqsave(&omap_usb_phy_lock, flags);
> >> + __raw_writel(val, omap_usb_phy_base + reg);
> >> + spin_unlock_irqrestore(&omap_usb_phy_lock, flags);
> >
> > I think it is better to lock per operation than per write.
> >
> >> + return 0;
> >> +}
> >> +
> >> +/**
> >> + * omap4_usb_phy_power - power on/off the phy using control module
> reg
> >> + * @dev: struct device *
> >> + * @on: 0 or 1, based on powering on or off the PHY
> >> + *
> >> + * omap_usb2 can call this API to power on or off the PHY.
> >> + */
> >> +int omap4_usb_phy_power(struct device *dev, int on)
> >> +{
> >> + u32 val;
> >> + int ret;
> >> +
> >> + if (on) {
> >> + ret = omap_usb_phy_readl(CONTROL_DEV_CONF, &val);
> >> + if (!ret && (val & PHY_PD)) {
> >> + ret = omap_usb_phy_writel(~PHY_PD,
> >> + CONTROL_DEV_CONF);
> >> + /* XXX: add proper documentation for this
> delay */
> >> + mdelay(200);
> >> + }
> >> + } else
> >> + ret = omap_usb_phy_writel(PHY_PD, CONTROL_DEV_CONF);
> >> +
> >> + return ret;
> >> +}
> >> +EXPORT_SYMBOL_GPL(omap4_usb_phy_power);
> >> +
> >> +/**
> >> + * omap4_usb_phy_mailbox - write to usb otg mailbox
> >> + * @dev: struct device *
> >> + * @val: the value to be written to the mailbox
> >> + *
> >> + * On detection of a device (ID pin is grounded), the phy should
> call this API
> >> + * to set AVALID, VBUSVALID and ID pin is grounded.
> >> + *
> >> + * When OMAP is connected to a host (OMAP in device mode), the phy
> should call
> >> + * this API to set AVALID, VBUSVALID and ID pin in high impedance.
> >> + *
> >> + * The phy should call this API, if OMAP is disconnected from host
> or device.
> >> + */
> >> +int omap4_usb_phy_mailbox(struct device *dev, u32 val)
> >> +{
> >> + return omap_usb_phy_writel(val, CONTROL_USBOTGHS_CONTROL);
> >> +}
> >> +EXPORT_SYMBOL_GPL(omap4_usb_phy_mailbox);
> >> +
> >> +static int __devinit omap_usb_phy_probe(struct platform_device
> *pdev)
> >> +{
> >> + struct resource *io_res;
> >> + struct platform_device *pparent;
> >> +
> >> + if (!pdev->dev.parent) {
> >> + dev_err(&pdev->dev, "No parent device!\n");
> >> + return -ENOMEM;
> >> + }
> >> +
> >> + pparent = to_platform_device(pdev->dev.parent);
> >> +
> >> + io_res = platform_get_resource(pparent, IORESOURCE_MEM, 0);
> >
> > Can someone please explain me what do we gain by doing this at
> children code?
> >
> >> + if (!io_res)
> >> + return -ENOENT;
> >> +
> >> + omap_usb_phy_base = ioremap(io_res->start,
> resource_size(io_res));
> >> + if (!omap_usb_phy_base)
> >> + return -ENOMEM;
> >> +
> >> + /* Initialize register lock */
> >> + spin_lock_init(&omap_usb_phy_lock);
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +static int __devexit omap_usb_phy_remove(struct platform_device
> *pdev)
> >> +{
> >> + return 0;
> >> +}
> >> +
> >> +static const struct of_device_id of_omap_usb_phy_match[] = {
> >> + { .compatible = "ti,omap4-usb-phy", },
> >> + { },
> >> +};
> >> +
> >> +static struct platform_driver omap_usb_phy_driver = {
> >> + .probe = omap_usb_phy_probe,
> >> + .remove = __devexit_p(omap_usb_phy_remove),
> >> + .driver = {
> >> + .name = "omap4-usb-phy",
> >> + .owner = THIS_MODULE,
> >> + .of_match_table = of_omap_usb_phy_match,
> >> + },
> >> +};
> >> +
> >> +static int __init omap_usb_phy_init(void)
> >> +{
> >> + return platform_driver_register(&omap_usb_phy_driver);
> >> +}
> >> +postcore_initcall(omap_usb_phy_init);
> >> +
> >> +static void __exit omap_usb_phy_exit(void)
> >> +{
> >> + platform_driver_unregister(&omap_usb_phy_driver);
> >> +}
> >> +module_exit(omap_usb_phy_exit);
> >> +
> >> +MODULE_DESCRIPTION("OMAP4+ USB-phy driver");
> >> +MODULE_LICENSE("GPL");
> >> +MODULE_ALIAS("platform: omap4-usb-phy");
> >> +MODULE_AUTHOR("Texas Instrument Inc.");
> >> diff --git a/include/linux/usb/omap4_usb_phy.h
> b/include/linux/usb/omap4_usb_phy.h
> >> new file mode 100644
> >> index 0000000..b6a4701
> >> --- /dev/null
> >> +++ b/include/linux/usb/omap4_usb_phy.h
> >> @@ -0,0 +1,53 @@
> >> +/*
> >> + * OMAP4 USB-phy
> >> + *
> >> + * Copyright (C) 2012 Texas Instruments Incorporated -
> http://www.ti.com/
> >> + *
> >> + * Contact:
> >> + * Kishon Vijay Abraham I <kishon@ti.com>
> >> + * Eduardo Valentin <eduardo.valentin@ti.com>
> >> + *
> >> + * This program is free software; you can redistribute it and/or
> >> + * modify it under the terms of the GNU General Public License
> >> + * version 2 as published by the Free Software Foundation.
> >> + *
> >> + * This program is distributed in the hope that it will be useful,
> but
> >> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> GNU
> >> + * General Public License for more details.
> >> + *
> >> + * You should have received a copy of the GNU General Public
> License
> >> + * along with this program; if not, write to the Free Software
> >> + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
> >> + * 02110-1301 USA
> >> + *
> >> + */
> >> +
> >> +#ifndef __OMAP4_USB_PHY_H
> >> +#define __OMAP4_USB_PHY_H
> >> +
> >> +#define PHY_PD 0x1
> >> +#define AVALID BIT(0)
> >> +#define BVALID BIT(1)
> >> +#define VBUSVALID BIT(2)
> >> +#define SESSEND BIT(3)
> >> +#define IDDIG BIT(4)
> >> +#define CONTROL_DEV_CONF 0x00000300
> >> +#define CONTROL_USBOTGHS_CONTROL 0x0000033C
> >> +
> >> +/* USB-PHY helpers */
> >> +#if (defined(CONFIG_OMAP4_USB_PHY)) ||
> (defined(CONFIG_OMAP4_USB_PHY_MODULE))
> >> +extern int omap4_usb_phy_mailbox(struct device *dev, u32 val);
> >> +extern int omap4_usb_phy_power(struct device *dev, int on);
> >> +#else
> >> +static inline int omap4_usb_phy_mailbox(struct device *dev, u32
> val)
> >> +{
> >> + return 0;
> >> +}
> >> +static inline int omap4_usb_phy_power(struct device *dev, int on)
> >> +{
> >> + return 0;
> >> +}
> >> +#endif
> >> +
> >> +#endif
> >> --
> >> 1.7.7.6
> >>
> >>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" 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 1/5] thermal: Add generic cpufreq cooling implementation
From: amit kachhap @ 2012-07-10 4:46 UTC (permalink / raw)
To: Valentin, Eduardo
Cc: Rob Lee, linux-pm, akpm, linux-samsung-soc, durgadoss.r, patches,
linux-kernel, lm-sensors, linux-acpi, khali, linaro-dev,
rui.zhang, guenter.roeck, lenb
In-Reply-To: <CAGF5oy9Dd1YUCbG8BGKVeYEZ7q2UP0HQ3oz+0cnABuFb-sBP6g@mail.gmail.com>
Hi Rob/Eduardo,
As these patches have still not made into mainline. I will rebase the
whole set along with your suggestion and send them shortly.
Sorry for late reply.
Thanks,
Amit Daniel
On Tue, Jun 26, 2012 at 1:12 PM, Valentin, Eduardo
<eduardo.valentin@ti.com> wrote:
> Hey Rob and Amit,
>
> On Tue, Jun 26, 2012 at 6:12 AM, Rob Lee <rob.lee@linaro.org> wrote:
>> Hey Amit,
>>
>> I was just re-organizing the imx thermal driver that uses this cpu
>> cooling interface and noticed a couple of small issues here. If
>
> While rewriting the OMAP BG driver on top of this series I noticed
> similar issues. Apart from those pointed by Rob, I have another minor
> fix.
>
>> these suggestions are agreed upon and if it's too late for these
>> issues be changed with this patchset, I can submit them separately
>> unless you'd prefer to.
>>
>> On Sat, May 12, 2012 at 4:40 AM, Amit Daniel Kachhap
>> <amit.kachhap@linaro.org> wrote:
>>> This patch adds support for generic cpu thermal cooling low level
>>> implementations using frequency scaling up/down based on the registration
>>> parameters. Different cpu related cooling devices can be registered by the
>>> user and the binding of these cooling devices to the corresponding
>>> trip points can be easily done as the registration APIs return the
>>> cooling device pointer. The user of these APIs are responsible for
>>> passing clipping frequency . The drivers can also register to recieve
>>> notification about any cooling action called.
>>>
>>> Signed-off-by: Amit Daniel Kachhap <amit.kachhap@linaro.org>
>>> ---
>>> Documentation/thermal/cpu-cooling-api.txt | 60 ++++
>>> drivers/thermal/Kconfig | 11 +
>>> drivers/thermal/Makefile | 3 +-
>>> drivers/thermal/cpu_cooling.c | 483 +++++++++++++++++++++++++++++
>>> include/linux/cpu_cooling.h | 99 ++++++
>>> 5 files changed, 655 insertions(+), 1 deletions(-)
>>> create mode 100644 Documentation/thermal/cpu-cooling-api.txt
>>> create mode 100644 drivers/thermal/cpu_cooling.c
>>> create mode 100644 include/linux/cpu_cooling.h
>>>
>>> diff --git a/Documentation/thermal/cpu-cooling-api.txt b/Documentation/thermal/cpu-cooling-api.txt
>>> new file mode 100644
>>> index 0000000..557adb8
>>> --- /dev/null
>>> +++ b/Documentation/thermal/cpu-cooling-api.txt
>>> @@ -0,0 +1,60 @@
>>> +CPU cooling APIs How To
>>> +===================================
>>> +
>>> +Written by Amit Daniel Kachhap <amit.kachhap@linaro.org>
>>> +
>>> +Updated: 12 May 2012
>>> +
>>> +Copyright (c) 2012 Samsung Electronics Co., Ltd(http://www.samsung.com)
>>> +
>>> +0. Introduction
>>> +
>>> +The generic cpu cooling(freq clipping, cpuhotplug etc) provides
>>> +registration/unregistration APIs to the caller. The binding of the cooling
>>> +devices to the trip point is left for the user. The registration APIs returns
>>> +the cooling device pointer.
>>> +
>>> +1. cpu cooling APIs
>>> +
>>> +1.1 cpufreq registration/unregistration APIs
>>> +1.1.1 struct thermal_cooling_device *cpufreq_cooling_register(
>>> + struct freq_clip_table *tab_ptr, unsigned int tab_size)
>>> +
>>> + This interface function registers the cpufreq cooling device with the name
>>> + "thermal-cpufreq-%x". This api can support multiple instances of cpufreq
>>> + cooling devices.
>>> +
>>> + tab_ptr: The table containing the maximum value of frequency to be clipped
>>> + for each cooling state.
>>> + .freq_clip_max: Value of frequency to be clipped for each allowed
>>> + cpus.
>>> + .temp_level: Temperature level at which the frequency clamping will
>>> + happen.
>>> + .mask_val: cpumask of the allowed cpu's
>>> + tab_size: the total number of cpufreq cooling states.
>>> +
>>> +1.1.2 void cpufreq_cooling_unregister(struct thermal_cooling_device *cdev)
>>> +
>>> + This interface function unregisters the "thermal-cpufreq-%x" cooling device.
>>> +
>>> + cdev: Cooling device pointer which has to be unregistered.
>>> +
>>> +
>>> +1.2 CPU cooling action notifier register/unregister interface
>>> +1.2.1 int cputherm_register_notifier(struct notifier_block *nb,
>>> + unsigned int list)
>>> +
>>> + This interface registers a driver with cpu cooling layer. The driver will
>>> + be notified when any cpu cooling action is called.
>>> +
>>> + nb: notifier function to register
>>> + list: CPUFREQ_COOLING_START or CPUFREQ_COOLING_STOP
>>> +
>>> +1.2.2 int cputherm_unregister_notifier(struct notifier_block *nb,
>>> + unsigned int list)
>>> +
>>> + This interface registers a driver with cpu cooling layer. The driver will
>>> + be notified when any cpu cooling action is called.
>>> +
>>> + nb: notifier function to register
>>> + list: CPUFREQ_COOLING_START or CPUFREQ_COOLING_STOP
>>> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
>>> index 514a691..d9c529f 100644
>>> --- a/drivers/thermal/Kconfig
>>> +++ b/drivers/thermal/Kconfig
>>> @@ -19,6 +19,17 @@ config THERMAL_HWMON
>>> depends on HWMON=y || HWMON=THERMAL
>>> default y
>>>
>>> +config CPU_THERMAL
>>
>> Perhaps the name CPU_COOLING or CPUFREQ_COOLING would more accurately
>> describe this functionality? I like the latter since now this
>> mechanism only supports cooling by using cpufreq.
>>
>>> + bool "generic cpu cooling support"
>>
>> If we use CPUFREQ_COOLING, then perhaps this could say:
>>
>> bool "cpu cooling through cpufreq frequency limiting"
>>
>>> + depends on THERMAL && CPU_FREQ
>>> + help
>>> + This implements the generic cpu cooling mechanism through frequency
>>> + reduction, cpu hotplug and any other ways of reducing temperature. An
>>> + ACPI version of this already exists(drivers/acpi/processor_thermal.c).
>>> + This will be useful for platforms using the generic thermal interface
>>> + and not the ACPI interface.
>>> + If you want this support, you should say Y or M here.
>>> +
>>
>> Currently, only the "frequency reduction" method exists so perhaps
>> it's best to remove mention of "cpu hotplug and any other ways of
>> reducing temperature"? Here'as a version for cpufreq only:
>>
>> config CPUFREQ_COOLING
>> bool "cpu cooling through cpufreq frequency limiting"
>> depends on THERMAL && CPU_FREQ
>> help
>> This implements a generic cpu cooling mechanism through cpufreq
>> frequency limiting. An ACPI version of this already exists
>> (drivers/acpi/processor_thermal.c). This will be useful for
>> platforms using the generic thermal interface and not the ACPI
>> interface.
>> If you want this support, you should say Y or M here.
>>
>>> config SPEAR_THERMAL
>>> bool "SPEAr thermal sensor driver"
>>> depends on THERMAL
>>> diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
>>> index a9fff0b..30c456c 100644
>>> --- a/drivers/thermal/Makefile
>>> +++ b/drivers/thermal/Makefile
>>> @@ -3,4 +3,5 @@
>>> #
>>>
>>> obj-$(CONFIG_THERMAL) += thermal_sys.o
>>> -obj-$(CONFIG_SPEAR_THERMAL) += spear_thermal.o
>>> \ No newline at end of file
>>> +obj-$(CONFIG_CPU_THERMAL) += cpu_cooling.o
>>> +obj-$(CONFIG_SPEAR_THERMAL) += spear_thermal.o
>>> diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c
>>> new file mode 100644
>>> index 0000000..c40d9a0
>>> --- /dev/null
>>> +++ b/drivers/thermal/cpu_cooling.c
>>> @@ -0,0 +1,483 @@
>>> +/*
>>> + * linux/drivers/thermal/cpu_cooling.c
>>> + *
>>> + * Copyright (C) 2012 Samsung Electronics Co., Ltd(http://www.samsung.com)
>>> + * Copyright (C) 2012 Amit Daniel <amit.kachhap@linaro.org>
>>> + *
>>> + * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>> + * This program is free software; you can redistribute it and/or modify
>>> + * it under the terms of the GNU General Public License as published by
>>> + * the Free Software Foundation; version 2 of the License.
>>> + *
>>> + * This program is distributed in the hope that it will be useful, but
>>> + * WITHOUT ANY WARRANTY; without even the implied warranty of
>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
>>> + * General Public License for more details.
>>> + *
>>> + * You should have received a copy of the GNU General Public License along
>>> + * with this program; if not, write to the Free Software Foundation, Inc.,
>>> + * 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA.
>>> + *
>>> + * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>> + */
>>> +#include <linux/kernel.h>
>>> +#include <linux/module.h>
>>> +#include <linux/thermal.h>
>>> +#include <linux/platform_device.h>
>>> +#include <linux/cpufreq.h>
>>> +#include <linux/err.h>
>>> +#include <linux/slab.h>
>>> +#include <linux/cpu.h>
>>> +#include <linux/cpu_cooling.h>
>>> +
>>> +/**
>>> + * struct cpufreq_cooling_device
>>> + * @id: unique integer value corresponding to each cpufreq_cooling_device
>>> + * registered.
>>> + * @cool_dev: thermal_cooling_device pointer to keep track of the the
>>> + * egistered cooling device.
>>> + * @tab_ptr: freq_clip_table table containing the maximum value of frequency to
>>> + * be set for different cooling state.
>>> + * @tab_size: integer value representing a count of the above table.
>>> + * @cpufreq_state: integer value representing the current state of cpufreq
>>> + * cooling devices.
>>> + * @allowed_cpus: all the cpus involved for this cpufreq_cooling_device.
>>> + * @node: list_head to link all cpufreq_cooling_device together.
>>> + *
>>> + * This structure is required for keeping information of each
>>> + * cpufreq_cooling_device registered as a list whose head is represented by
>>> + * cooling_cpufreq_list. In order to prevent corruption of this list a
>>> + * mutex lock cooling_cpufreq_lock is used.
>>> + */
>>> +struct cpufreq_cooling_device {
>>> + int id;
>>> + struct thermal_cooling_device *cool_dev;
>>> + struct freq_clip_table *tab_ptr;
>>> + unsigned int tab_size;
>>> + unsigned int cpufreq_state;
>>> + struct cpumask allowed_cpus;
>>> + struct list_head node;
>>> +};
>>> +static LIST_HEAD(cooling_cpufreq_list);
>>> +static DEFINE_MUTEX(cooling_cpufreq_lock);
>>> +static DEFINE_IDR(cpufreq_idr);
>>> +
>>> +/*per cpu variable to store the previous max frequency allowed*/
>>> +static DEFINE_PER_CPU(unsigned int, max_policy_freq);
>>> +
>>> +/*notify_table passes value to the CPUFREQ_ADJUST callback function.*/
>>> +#define NOTIFY_INVALID NULL
>>> +static struct freq_clip_table *notify_table;
>>> +
>>> +/*Head of the blocking notifier chain to inform about frequency clamping*/
>>> +static BLOCKING_NOTIFIER_HEAD(cputherm_state_notifier_list);
>>> +
>>> +/**
>>> + * get_idr - function to get a unique id.
>>> + * @idr: struct idr * handle used to create a id.
>>> + * @id: int * value generated by this function.
>>> + */
>>> +static int get_idr(struct idr *idr, int *id)
>>> +{
>>> + int err;
>>> +again:
>>> + if (unlikely(idr_pre_get(idr, GFP_KERNEL) == 0))
>>> + return -ENOMEM;
>>> +
>>> + mutex_lock(&cooling_cpufreq_lock);
>>> + err = idr_get_new(idr, NULL, id);
>>> + mutex_unlock(&cooling_cpufreq_lock);
>>> +
>>> + if (unlikely(err == -EAGAIN))
>>> + goto again;
>>> + else if (unlikely(err))
>>> + return err;
>>> +
>>> + *id = *id & MAX_ID_MASK;
>>> + return 0;
>>> +}
>>> +
>>> +/**
>>> + * release_idr - function to free the unique id.
>>> + * @idr: struct idr * handle used for creating the id.
>>> + * @id: int value representing the unique id.
>>> + */
>>> +static void release_idr(struct idr *idr, int id)
>>> +{
>>> + mutex_lock(&cooling_cpufreq_lock);
>>> + idr_remove(idr, id);
>>> + mutex_unlock(&cooling_cpufreq_lock);
>>> +}
>>> +
>>> +/**
>>> + * cputherm_register_notifier - Register a notifier with cpu cooling interface.
>>> + * @nb: struct notifier_block * with callback info.
>>> + * @list: integer value for which notification is needed. possible values are
>>> + * CPUFREQ_COOLING_START and CPUFREQ_COOLING_STOP.
>>> + *
>>> + * This exported function registers a driver with cpu cooling layer. The driver
>>> + * will be notified when any cpu cooling action is called.
>>> + */
>>> +int cputherm_register_notifier(struct notifier_block *nb, unsigned int list)
>>> +{
>>> + int ret = 0;
>>> +
>>> + switch (list) {
>>> + case CPUFREQ_COOLING_START:
>>> + case CPUFREQ_COOLING_STOP:
>>> + ret = blocking_notifier_chain_register(
>>> + &cputherm_state_notifier_list, nb);
>>> + break;
>>> + default:
>>> + ret = -EINVAL;
>>> + }
>>> + return ret;
>>> +}
>>> +EXPORT_SYMBOL(cputherm_register_notifier);
>>> +
>>> +/**
>>> + * cputherm_unregister_notifier - Un-register a notifier.
>>> + * @nb: struct notifier_block * with callback info.
>>> + * @list: integer value for which notification is needed. values possible are
>>> + * CPUFREQ_COOLING_START or CPUFREQ_COOLING_STOP.
>>> + *
>>> + * This exported function un-registers a driver with cpu cooling layer.
>>> + */
>>> +int cputherm_unregister_notifier(struct notifier_block *nb, unsigned int list)
>>> +{
>>> + int ret = 0;
>>> +
>>> + switch (list) {
>>> + case CPUFREQ_COOLING_START:
>>> + case CPUFREQ_COOLING_STOP:
>>> + ret = blocking_notifier_chain_unregister(
>>> + &cputherm_state_notifier_list, nb);
>>> + break;
>>> + default:
>>> + ret = -EINVAL;
>>> + }
>>> + return ret;
>>> +}
>>> +EXPORT_SYMBOL(cputherm_unregister_notifier);
>>> +
>>> +/*Below codes defines functions to be used for cpufreq as cooling device*/
>>> +
>>> +/**
>>> + * is_cpufreq_valid - function to check if a cpu has frequency transition policy.
>>> + * @cpu: cpu for which check is needed.
>>> + */
>>> +static int is_cpufreq_valid(int cpu)
>>> +{
>>> + struct cpufreq_policy policy;
>>> + return !cpufreq_get_policy(&policy, cpu);
>>> +}
>>> +
>>> +/**
>>> + * cpufreq_apply_cooling - function to apply frequency clipping.
>>> + * @cpufreq_device: cpufreq_cooling_device pointer containing frequency
>>> + * clipping data.
>>> + * @cooling_state: value of the cooling state.
>>> + */
>>> +static int cpufreq_apply_cooling(struct cpufreq_cooling_device *cpufreq_device,
>>> + unsigned long cooling_state)
>>> +{
>>> + unsigned int event, cpuid, state;
>>> + struct freq_clip_table *th_table, *table_ptr;
>>> + const struct cpumask *maskPtr = &cpufreq_device->allowed_cpus;
>>> + struct cpufreq_cooling_device *cpufreq_ptr;
>>> +
>>> + if (cooling_state > cpufreq_device->tab_size)
>>> + return -EINVAL;
>>> +
>>> + /*Check if the old cooling action is same as new cooling action*/
>>> + if (cpufreq_device->cpufreq_state == cooling_state)
>>> + return 0;
>>> +
>>> + /*pass cooling table info to the cpufreq_thermal_notifier callback*/
>>> + notify_table = NOTIFY_INVALID;
>>> +
>>> + if (cooling_state > 0) {
>>> + th_table = &(cpufreq_device->tab_ptr[cooling_state - 1]);
>>> + notify_table = th_table;
>>> + }
>>> +
>>> + /*check if any lower clip frequency active in other cpufreq_device's*/
>>> + list_for_each_entry(cpufreq_ptr, &cooling_cpufreq_list, node) {
>>> +
>>> + state = cpufreq_ptr->cpufreq_state;
>>> + if (state == 0 || cpufreq_ptr == cpufreq_device)
>>> + continue;
>>> +
>>> + if (!cpumask_equal(&cpufreq_ptr->allowed_cpus,
>>> + &cpufreq_device->allowed_cpus))
>>> + continue;
>>> +
>>> + table_ptr = &(cpufreq_ptr->tab_ptr[state - 1]);
>>> + if (notify_table == NULL ||
>>> + (table_ptr->freq_clip_max <
>>> + notify_table->freq_clip_max))
>>> + notify_table = table_ptr;
>>> + }
>>> +
>>> + cpufreq_device->cpufreq_state = cooling_state;
>>> +
>>> + if (notify_table != NOTIFY_INVALID) {
>>> + event = CPUFREQ_COOLING_START;
>>> + maskPtr = notify_table->mask_val;
>>> + } else {
>>> + event = CPUFREQ_COOLING_STOP;
>>> + }
>>> +
>>> + blocking_notifier_call_chain(&cputherm_state_notifier_list,
>>> + event, notify_table);
>>> +
>>> + for_each_cpu(cpuid, maskPtr) {
>>> + if (is_cpufreq_valid(cpuid))
>>> + cpufreq_update_policy(cpuid);
>>> + }
>>> +
>>> + notify_table = NOTIFY_INVALID;
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +/**
>>> + * cpufreq_thermal_notifier - notifier callback for cpufreq policy change.
>>> + * @nb: struct notifier_block * with callback info.
>>> + * @event: value showing cpufreq event for which this function invoked.
>>> + * @data: callback-specific data
>>> + */
>>> +static int cpufreq_thermal_notifier(struct notifier_block *nb,
>>> + unsigned long event, void *data)
>>> +{
>>> + struct cpufreq_policy *policy = data;
>>> + unsigned long max_freq = 0;
>>> +
>>> + if (event != CPUFREQ_ADJUST)
>>> + return 0;
>>> +
>>> + if (notify_table != NOTIFY_INVALID) {
>>> + max_freq = notify_table->freq_clip_max;
>>> +
>>> + if (!per_cpu(max_policy_freq, policy->cpu))
>>> + per_cpu(max_policy_freq, policy->cpu) = policy->max;
>>> + } else {
>>> + if (per_cpu(max_policy_freq, policy->cpu)) {
>>> + max_freq = per_cpu(max_policy_freq, policy->cpu);
>>> + per_cpu(max_policy_freq, policy->cpu) = 0;
>>> + } else {
>>> + max_freq = policy->max;
>>> + }
>>> + }
>>> +
>>> + /* Never exceed user_policy.max*/
>>> + if (max_freq > policy->user_policy.max)
>>> + max_freq = policy->user_policy.max;
>>> +
>>> + if (policy->max != max_freq)
>>> + cpufreq_verify_within_limits(policy, 0, max_freq);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +/*
>>> + * cpufreq cooling device callback functions are defined below
>>> + */
>>> +
>>> +/**
>>> + * cpufreq_get_max_state - callback function to get the max cooling state.
>>> + * @cdev: thermal cooling device pointer.
>>> + * @state: fill this variable with the max cooling state.
>>> + */
>>> +static int cpufreq_get_max_state(struct thermal_cooling_device *cdev,
>>> + unsigned long *state)
>>> +{
>>> + int ret = -EINVAL;
>>> + struct cpufreq_cooling_device *cpufreq_device;
>>> +
>>> + mutex_lock(&cooling_cpufreq_lock);
>>> + list_for_each_entry(cpufreq_device, &cooling_cpufreq_list, node) {
>>> + if (cpufreq_device && cpufreq_device->cool_dev == cdev) {
>>> + *state = cpufreq_device->tab_size;
>>> + ret = 0;
>>> + break;
>>> + }
>>> + }
>>> + mutex_unlock(&cooling_cpufreq_lock);
>>> + return ret;
>>> +}
>>> +
>>> +/**
>>> + * cpufreq_get_cur_state - callback function to get the current cooling state.
>>> + * @cdev: thermal cooling device pointer.
>>> + * @state: fill this variable with the current cooling state.
>>> + */
>>> +static int cpufreq_get_cur_state(struct thermal_cooling_device *cdev,
>>> + unsigned long *state)
>>> +{
>>> + int ret = -EINVAL;
>>> + struct cpufreq_cooling_device *cpufreq_device;
>>> +
>>> + mutex_lock(&cooling_cpufreq_lock);
>>> + list_for_each_entry(cpufreq_device, &cooling_cpufreq_list, node) {
>>> + if (cpufreq_device && cpufreq_device->cool_dev == cdev) {
>>> + *state = cpufreq_device->cpufreq_state;
>>> + ret = 0;
>>> + break;
>>> + }
>>> + }
>>> + mutex_unlock(&cooling_cpufreq_lock);
>>> + return ret;
>>> +}
>>> +
>>> +/**
>>> + * cpufreq_set_cur_state - callback function to set the current cooling state.
>>> + * @cdev: thermal cooling device pointer.
>>> + * @state: set this variable to the current cooling state.
>>> + */
>>> +static int cpufreq_set_cur_state(struct thermal_cooling_device *cdev,
>>> + unsigned long state)
>>> +{
>>> + int ret = -EINVAL;
>>> + struct cpufreq_cooling_device *cpufreq_device;
>>> +
>>> + mutex_lock(&cooling_cpufreq_lock);
>>> + list_for_each_entry(cpufreq_device, &cooling_cpufreq_list, node) {
>>> + if (cpufreq_device && cpufreq_device->cool_dev == cdev) {
>>> + ret = 0;
>>> + break;
>>> + }
>>> + }
>>> + if (!ret)
>>> + ret = cpufreq_apply_cooling(cpufreq_device, state);
>>> +
>>> + mutex_unlock(&cooling_cpufreq_lock);
>>> +
>>> + return ret;
>>> +}
>>> +
>>> +/*Bind cpufreq callbacks to thermal cooling device ops*/
>>> +static struct thermal_cooling_device_ops const cpufreq_cooling_ops = {
>>> + .get_max_state = cpufreq_get_max_state,
>>> + .get_cur_state = cpufreq_get_cur_state,
>>> + .set_cur_state = cpufreq_set_cur_state,
>>> +};
>>> +
>>> +/*Notifier for cpufreq policy change*/
>>> +static struct notifier_block thermal_cpufreq_notifier_block = {
>>> + .notifier_call = cpufreq_thermal_notifier,
>>> +};
>>> +
>>> +/**
>>> + * cpufreq_cooling_register - function to create cpufreq cooling device.
>>> + * @tab_ptr: table ptr containing the maximum value of frequency to be clipped
>>> + * for each cooling state.
>>> + * @tab_size: count of entries in the above table.
>>> + * happen.
>>> + */
>>> +struct thermal_cooling_device *cpufreq_cooling_register(
>>> + struct freq_clip_table *tab_ptr, unsigned int tab_size)
>>> +{
>>> + struct thermal_cooling_device *cool_dev;
>>> + struct cpufreq_cooling_device *cpufreq_dev = NULL;
>>> + struct freq_clip_table *clip_tab;
>>> + unsigned int cpufreq_dev_count = 0;
>>> + char dev_name[THERMAL_NAME_LENGTH];
>>> + int ret = 0, id = 0, i;
>>> +
>>> + if (tab_ptr == NULL || tab_size == 0)
>>> + return ERR_PTR(-EINVAL);
>>> +
>>> + list_for_each_entry(cpufreq_dev, &cooling_cpufreq_list, node)
>>> + cpufreq_dev_count++;
>>> +
>>> + cpufreq_dev = kzalloc(sizeof(struct cpufreq_cooling_device),
>>> + GFP_KERNEL);
>>> + if (!cpufreq_dev)
>>> + return ERR_PTR(-ENOMEM);
>>> +
>>> + /*Verify that all the entries of freq_clip_table are present*/
>>> + for (i = 0; i < tab_size; i++) {
>>> + clip_tab = ((struct freq_clip_table *)&tab_ptr[i]);
>>> + if (!clip_tab->freq_clip_max || !clip_tab->mask_val
>>> + || !clip_tab->temp_level) {
>>> + kfree(cpufreq_dev);
>>> + return ERR_PTR(-EINVAL);
>>> + }
>>> + /*
>>> + *Consolidate all the cpumask for all the individual entries
>>> + *of the trip table. This is useful in resetting all the
>>> + *clipped frequencies to the normal level for each cpufreq
>>> + *cooling device.
>>> + */
>>> + cpumask_or(&cpufreq_dev->allowed_cpus,
>>> + &cpufreq_dev->allowed_cpus, clip_tab->mask_val);
>>> + }
>>> +
>>> + cpufreq_dev->tab_ptr = tab_ptr;
>>> + cpufreq_dev->tab_size = tab_size;
>>> +
>>> + ret = get_idr(&cpufreq_idr, &cpufreq_dev->id);
>>> + if (ret) {
>>> + kfree(cpufreq_dev);
>>> + return ERR_PTR(-EINVAL);
>>> + }
>>> +
>>> + sprintf(dev_name, "thermal-cpufreq-%d", cpufreq_dev->id);
>>> +
>>> + cool_dev = thermal_cooling_device_register(dev_name, cpufreq_dev,
>>> + &cpufreq_cooling_ops);
>>> + if (!cool_dev) {
>>> + release_idr(&cpufreq_idr, cpufreq_dev->id);
>>> + kfree(cpufreq_dev);
>>> + return ERR_PTR(-EINVAL);
>>> + }
>>> + cpufreq_dev->id = id;
>>> + cpufreq_dev->cool_dev = cool_dev;
>>> + mutex_lock(&cooling_cpufreq_lock);
>>> + list_add_tail(&cpufreq_dev->node, &cooling_cpufreq_list);
>>> +
>>> + /*Register the notifier for first cpufreq cooling device*/
>>> + if (cpufreq_dev_count == 0)
>>> + cpufreq_register_notifier(&thermal_cpufreq_notifier_block,
>>> + CPUFREQ_POLICY_NOTIFIER);
>>> +
>>> + mutex_unlock(&cooling_cpufreq_lock);
>>> + return cool_dev;
>>> +}
>>> +EXPORT_SYMBOL(cpufreq_cooling_register);
>>> +
>>> +/**
>>> + * cpufreq_cooling_unregister - function to remove cpufreq cooling device.
>>> + * @cdev: thermal cooling device pointer.
>>> + */
>>> +void cpufreq_cooling_unregister(struct thermal_cooling_device *cdev)
>>> +{
>>> + struct cpufreq_cooling_device *cpufreq_dev = NULL;
>>> + unsigned int cpufreq_dev_count = 0;
>>> +
>>> + mutex_lock(&cooling_cpufreq_lock);
>>> + list_for_each_entry(cpufreq_dev, &cooling_cpufreq_list, node) {
>>> + if (cpufreq_dev && cpufreq_dev->cool_dev == cdev)
>>> + break;
>>> + cpufreq_dev_count++;
>>> + }
>>> +
>>> + if (!cpufreq_dev || cpufreq_dev->cool_dev != cdev) {
>>> + mutex_unlock(&cooling_cpufreq_lock);
>>> + return;
>>> + }
>>> +
>>> + list_del(&cpufreq_dev->node);
>>> +
>>> + /*Unregister the notifier for the last cpufreq cooling device*/
>>> + if (cpufreq_dev_count == 1) {
>>> + cpufreq_unregister_notifier(&thermal_cpufreq_notifier_block,
>>> + CPUFREQ_POLICY_NOTIFIER);
>>> + }
>>> + mutex_unlock(&cooling_cpufreq_lock);
>>> + thermal_cooling_device_unregister(cpufreq_dev->cool_dev);
>>> + release_idr(&cpufreq_idr, cpufreq_dev->id);
>>> + kfree(cpufreq_dev);
>>> +}
>>> +EXPORT_SYMBOL(cpufreq_cooling_unregister);
>>> diff --git a/include/linux/cpu_cooling.h b/include/linux/cpu_cooling.h
>>> new file mode 100644
>>> index 0000000..ed6c096
>>> --- /dev/null
>>> +++ b/include/linux/cpu_cooling.h
>>> @@ -0,0 +1,99 @@
>>> +/*
>>> + * linux/include/linux/cpu_cooling.h
>>> + *
>>> + * Copyright (C) 2012 Samsung Electronics Co., Ltd(http://www.samsung.com)
>>> + * Copyright (C) 2012 Amit Daniel <amit.kachhap@linaro.org>
>>> + *
>>> + * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>> + * This program is free software; you can redistribute it and/or modify
>>> + * it under the terms of the GNU General Public License as published by
>>> + * the Free Software Foundation; version 2 of the License.
>>> + *
>>> + * This program is distributed in the hope that it will be useful, but
>>> + * WITHOUT ANY WARRANTY; without even the implied warranty of
>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
>>> + * General Public License for more details.
>>> + *
>>> + * You should have received a copy of the GNU General Public License along
>>> + * with this program; if not, write to the Free Software Foundation, Inc.,
>>> + * 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA.
>>> + *
>>> + * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>> + */
>>> +
>>> +#ifndef __CPU_COOLING_H__
>>> +#define __CPU_COOLING_H__
>>> +
>>> +#include <linux/thermal.h>
>>> +
>>> +#define CPUFREQ_COOLING_START 0
>>> +#define CPUFREQ_COOLING_STOP 1
>>> +
>>> +/**
>>> + * struct freq_clip_table
>>> + * @freq_clip_max: maximum frequency allowed for this cooling state.
>>> + * @temp_level: Temperature level at which the temperature clipping will
>>> + * happen.
>>> + * @mask_val: cpumask of the allowed cpu's where the clipping will take place.
>>> + *
>>> + * This structure is required to be filled and passed to the
>>> + * cpufreq_cooling_unregister function.
>>> + */
>>> +struct freq_clip_table {
>>> + unsigned int freq_clip_max;
>>> + unsigned int temp_level;
>>> + const struct cpumask *mask_val;
>>> +};
>>> +
>>> +/**
>>> + * cputherm_register_notifier - Register a notifier with cpu cooling interface.
>>> + * @nb: struct notifier_block * with callback info.
>>> + * @list: integer value for which notification is needed. possible values are
>>> + * CPUFREQ_COOLING_TYPE and CPUHOTPLUG_COOLING_TYPE.
>>
>> CPUFREQ_COOLING_TYPE and CPUHOTPLUG_COOLING_TYPE should be changed to
>> CPUFREQ_COOLING_START CPUFREQ_COOLING_STOP, right?
>>
>>> + *
>>> + * This exported function registers a driver with cpu cooling layer. The driver
>>> + * will be notified when any cpu cooling action is called.
>>> + */
>>> +int cputherm_register_notifier(struct notifier_block *nb, unsigned int list);
>>> +
>>> +/**
>>> + * cputherm_unregister_notifier - Un-register a notifier.
>>> + * @nb: struct notifier_block * with callback info.
>>> + * @list: integer value for which notification is needed. values possible are
>>> + * CPUFREQ_COOLING_TYPE.
>>
>> Should be changed to CPUFREQ_COOLING_START, CPUFREQ_COOLING_STOP, right?
>>
>>> + *
>>> + * This exported function un-registers a driver with cpu cooling layer.
>>> + */
>>> +int cputherm_unregister_notifier(struct notifier_block *nb, unsigned int list);
>>> +
>>> +#ifdef CONFIG_CPU_FREQ
>>
>> Consider changing this to "#ifdef CPU_THERMAL" A platform thermal
>> driver may want to always call this function as its cpu cooling
>> mechanism but it should still function without the cpu_cooling being
>> enabled. In this case, the platform thermal driver would only provide
>> temperature readings and critical trip point handling. But if the
>> platform happened may have CONFIG_CPU_FREQ enabled, a build error
>> would occur in the current implementation.
>
> I agree here. It makes more sense to have the check against CPU_THERMAL
>
>>
>> (the CONFIG_CPU_FREQ requirement will be met by CPU_THERMAL being
>> enabled since it also implies CONFIG_CPU_FREQ being enabled per the
>> Kconfig dependency)
>
> True.
>
>>
>>> +/**
>>> + * cpufreq_cooling_register - function to create cpufreq cooling device.
>>> + * @tab_ptr: table ptr containing the maximum value of frequency to be clipped
>>> + * for each cooling state.
>>> + * @tab_size: count of entries in the above table.
>>> + * @mask_val: cpumask containing the allowed cpu's where frequency clipping can
>>> + * happen.
>>> + */
>>> +struct thermal_cooling_device *cpufreq_cooling_register(
>>> + struct freq_clip_table *tab_ptr, unsigned int tab_size);
>>> +
>>> +/**
>>> + * cpufreq_cooling_unregister - function to remove cpufreq cooling device.
>>> + * @cdev: thermal cooling device pointer.
>>> + */
>>> +void cpufreq_cooling_unregister(struct thermal_cooling_device *cdev);
>>> +#else /*!CONFIG_CPU_FREQ*/
>>
>> Modify comment if above suggestion is taken.
>>
>>> +static inline struct thermal_cooling_device *cpufreq_cooling_register(
>>> + struct freq_clip_table *tab_ptr, unsigned int tab_size);
>
> The above semicollon will cause a build error in case this path of the
> ifdef is taken.
>
>>> +{
>>> + return NULL;
>>> +}
>>> +static inline void cpufreq_cooling_unregister(
>>> + struct thermal_cooling_device *cdev)
>>> +{
>>> + return;
>>> +}
>>> +#endif /*CONFIG_CPU_FREQ*/
>>
>> Modify comment if above suggestion is taken.
>>
>>> +
>>> +#endif /* __CPU_COOLING_H__ */
>>> --
>>> 1.7.1
>>>
>>>
>>> _______________________________________________
>>> linaro-dev mailing list
>>> linaro-dev@lists.linaro.org
>>> http://lists.linaro.org/mailman/listinfo/linaro-dev
>> --
>> 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
>
>
>
> --
>
> Eduardo Valentin
> --
> 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: [linux-pm] [PATCH v4 0/5] thermal: exynos: Add kernel thermal support for exynos platform
From: amit daniel kachhap @ 2012-07-10 5:21 UTC (permalink / raw)
To: lenb
Cc: patches, linux-kernel, lm-sensors, linux-acpi, khali, linaro-dev,
guenter.roeck, linux-pm, akpm
In-Reply-To: <1336815645-29625-1-git-send-email-amit.kachhap@linaro.org>
Hi Len,
These set of patches were accepted by you for 3.5 merge window. Can
you again apply them on your for-next branch for 3.6 merge? I have
some arch enablement and clean up patches based on them. It is really
important for me to mainline everything in this merge window.
Thanks,
Amit Daniel
On Sat, May 12, 2012 at 3:10 PM, Amit Daniel Kachhap
<amit.kachhap@linaro.org> wrote:
> Hi Andrew/Rui,
>
> As discussed with Rui Zhang, I dropped the patch for new trip type
> THERMAL_TRIP_STATE_INSTANCE and made the necessary state magnagement changes
> in cpufreq cooling functions. Also I fixed all the review comments suggested
> by Andrew. If any other changes please let me know.
>
> This patchset introduces a new generic cooling device based on cpufreq that
> can be used on non-ACPI platforms. As a proof of concept, we have drivers for
> the following platforms using this mechanism now:
>
> * Samsung Exynos (Exynos4 and Exynos5) in the current patchset.
> * TI OMAP (git://git.linaro.org/people/amitdanielk/linux.git omap4460_thermal)
> * Freescale i.MX (git://git.linaro.org/people/amitdanielk/linux.git imx6q_thermal)
>
> The is a small change in cpufreq cooling registration APIs, so a minor change is
> needed for OMAP and Freescale platforms.
>
> Thanks,
> Amit Daniel
>
> Changes since V3:
> * Dropped the concept of using new trip type THERMAL_TRIP_STATE_INSTANCE as
> discussed with Rui Zhang. This requires adding some state management logic
> in cpufreq cooling implementation.
> * Many review comments suggested by Andrew Morton
> * More documentation added in cpufreq cooling codes.
>
> Changes since V2:
> *Added Exynos5 TMU sensor support by enhancing the exynos4 tmu driver. Exynos5 TMU
> driver was internally developed by SangWook Ju <sw.ju@samsung.com>.
> *Removed cpuhotplug cooling code in this patchset.
> *Rebased the patches against 3.4-rc6 kernel.
>
> Changes since V1:
> *Moved the sensor driver to driver/thermal folder from driver/hwmon folder
> as suggested by Mark Brown and Guenter Roeck
> *Added notifier support to notify the registered drivers of any cpu cooling
> action. The driver can modify the default cooling behaviour(eg set different
> max clip frequency).
> *The percentage based frequency replaced with absolute clipped frequency.
> *Some more conditional checks when setting max frequency.
> *Renamed the new trip type THERMAL_TRIP_STATE_ACTIVE to
> THERMAL_TRIP_STATE_INSTANCE
> *Many review comments from R, Durgadoss <durgadoss.r@intel.com> and
> eduardo.valentin@ti.com implemented.
> *Removed cooling stats through debugfs patch
> *The V1 based can be found here,
> https://lkml.org/lkml/2012/2/22/123
> http://lkml.org/lkml/2012/3/3/32
>
> Changes since RFC:
> *Changed the cpu cooling registration/unregistration API's to instance based
> *Changed the STATE_ACTIVE trip type to pass correct instance id
> *Adding support to restore back the policy->max_freq after doing frequency
> clipping.
> *Moved the trip cooling stats from sysfs node to debugfs node as suggested
> by Greg KH greg@kroah.com
> *Incorporated several review comments from eduardo.valentin@ti.com
> *Moved the Temperature sensor driver from driver/hwmon/ to driver/mfd
> as discussed with Guenter Roeck <guenter.roeck@ericsson.com> and
> Donggeun Kim <dg77.kim@samsung.com> (https://lkml.org/lkml/2012/1/5/7)
> *Some changes according to the changes in common cpu cooling APIs
> *The RFC based patches can be found here,
> https://lkml.org/lkml/2011/12/13/186
> https://lkml.org/lkml/2011/12/21/169
>
>
> Brief Description:
>
> 1) The generic cooling devices code is placed inside driver/thermal/* as
> placing inside acpi folder will need un-necessary enabling of acpi code. This
> codes is architecture independent.
>
> 2) This patchset adds generic cpu cooling low level implementation through
> frequency clipping. In future, other cpu related cooling devices may be added
> here. An ACPI version of this already exists (drivers/acpi/processor_thermal.c)
> . But this will be useful for platforms like ARM using the generic
> thermal interface along with the generic cpu cooling devices. The cooling
> device registration API's return cooling device pointers which can be easily
> binded with the thermal zone trip points. The important APIs exposed are,
> a)struct thermal_cooling_device *cpufreq_cooling_register(
> struct freq_clip_table *tab_ptr, unsigned int tab_size)
> b)void cpufreq_cooling_unregister(struct thermal_cooling_device *cdev)
>
> 3) Samsung exynos platform thermal implementation is done using the generic
> cpu cooling APIs and the new trip type. The temperature sensor driver present
> in the hwmon folder(registered as hwmon driver) is moved to thermal folder
> and registered as a thermal driver.
>
> All this patchset is based on Kernel version 3.4-rc6
>
> A simple data/control flow diagrams is shown below,
>
> Core Linux thermal <-----> Exynos thermal interface <----- Temperature Sensor
> | |
> \|/ |
> Cpufreq cooling device <---------------
>
> TODO:
> *Will send the DT enablement patches later after the driver is merged.
>
>
> Amit Daniel Kachhap (5):
> thermal: Add generic cpufreq cooling implementation
> hwmon: exynos4: Move thermal sensor driver to driver/thermal
> directory
> thermal: exynos5: Add exynos5 thermal sensor driver support
> thermal: exynos: Register the tmu sensor with the kernel thermal
> layer
> ARM: exynos: Add thermal sensor driver platform data support
>
> Documentation/hwmon/exynos4_tmu | 81 ---
> Documentation/thermal/cpu-cooling-api.txt | 60 ++
> Documentation/thermal/exynos_thermal | 52 ++
> drivers/hwmon/Kconfig | 10 -
> drivers/hwmon/Makefile | 1 -
> drivers/hwmon/exynos4_tmu.c | 514 --------------
> drivers/thermal/Kconfig | 20 +
> drivers/thermal/Makefile | 4 +-
> drivers/thermal/cpu_cooling.c | 483 +++++++++++++
> drivers/thermal/exynos_thermal.c | 956 ++++++++++++++++++++++++++
> include/linux/cpu_cooling.h | 99 +++
> include/linux/platform_data/exynos4_tmu.h | 83 ---
> include/linux/platform_data/exynos_thermal.h | 100 +++
> 13 files changed, 1773 insertions(+), 690 deletions(-)
> delete mode 100644 Documentation/hwmon/exynos4_tmu
> create mode 100644 Documentation/thermal/cpu-cooling-api.txt
> create mode 100644 Documentation/thermal/exynos_thermal
> delete mode 100644 drivers/hwmon/exynos4_tmu.c
> create mode 100644 drivers/thermal/cpu_cooling.c
> create mode 100644 drivers/thermal/exynos_thermal.c
> create mode 100644 include/linux/cpu_cooling.h
> delete mode 100644 include/linux/platform_data/exynos4_tmu.h
> create mode 100644 include/linux/platform_data/exynos_thermal.h
>
> _______________________________________________
> linux-pm mailing list
> linux-pm@lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/linux-pm
^ permalink raw reply
* Re: [PATCH v4 1/5] thermal: Add generic cpufreq cooling implementation
From: Hongbo Zhang @ 2012-07-10 7:01 UTC (permalink / raw)
To: Amit Daniel Kachhap
Cc: linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA,
durgadoss.r-ral2JQCrhuEAvxtiuMwx3w,
patches-QSEj5FYQhm4dnm+yROfE0A,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
lm-sensors-GZX6beZjE8VD60Wz+7aTrA,
linux-acpi-u79uwXL29TY76Z2rM5mHXA,
rui.zhang-ral2JQCrhuEAvxtiuMwx3w, khali-PUYAD+kWke1g9hUCZPvPmw,
linaro-dev-cunTk1MwBs8s++Sfvej+rw,
linux-pm-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
lenb-DgEjT+Ai2ygdnm+yROfE0A,
akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
guenter.roeck-IzeFyvvaP7pWk0Htik3J/w
In-Reply-To: <1336815645-29625-2-git-send-email-amit.kachhap-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
[-- Attachment #1.1: Type: text/plain, Size: 28639 bytes --]
On 12 May 2012 17:40, Amit Daniel Kachhap <amit.kachhap-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> This patch adds support for generic cpu thermal cooling low level
> implementations using frequency scaling up/down based on the registration
> parameters. Different cpu related cooling devices can be registered by the
> user and the binding of these cooling devices to the corresponding
> trip points can be easily done as the registration APIs return the
> cooling device pointer. The user of these APIs are responsible for
> passing clipping frequency . The drivers can also register to recieve
> notification about any cooling action called.
>
> Signed-off-by: Amit Daniel Kachhap <amit.kachhap-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
> Documentation/thermal/cpu-cooling-api.txt | 60 ++++
> drivers/thermal/Kconfig | 11 +
> drivers/thermal/Makefile | 3 +-
> drivers/thermal/cpu_cooling.c | 483
> +++++++++++++++++++++++++++++
> include/linux/cpu_cooling.h | 99 ++++++
> 5 files changed, 655 insertions(+), 1 deletions(-)
> create mode 100644 Documentation/thermal/cpu-cooling-api.txt
> create mode 100644 drivers/thermal/cpu_cooling.c
> create mode 100644 include/linux/cpu_cooling.h
>
> diff --git a/Documentation/thermal/cpu-cooling-api.txt
> b/Documentation/thermal/cpu-cooling-api.txt
> new file mode 100644
> index 0000000..557adb8
> --- /dev/null
> +++ b/Documentation/thermal/cpu-cooling-api.txt
> @@ -0,0 +1,60 @@
> +CPU cooling APIs How To
> +===================================
> +
> +Written by Amit Daniel Kachhap <amit.kachhap-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> +
> +Updated: 12 May 2012
> +
> +Copyright (c) 2012 Samsung Electronics Co., Ltd(http://www.samsung.com)
> +
> +0. Introduction
> +
> +The generic cpu cooling(freq clipping, cpuhotplug etc) provides
> +registration/unregistration APIs to the caller. The binding of the cooling
> +devices to the trip point is left for the user. The registration APIs
> returns
> +the cooling device pointer.
> +
> +1. cpu cooling APIs
> +
> +1.1 cpufreq registration/unregistration APIs
> +1.1.1 struct thermal_cooling_device *cpufreq_cooling_register(
> + struct freq_clip_table *tab_ptr, unsigned int tab_size)
> +
> + This interface function registers the cpufreq cooling device with the
> name
> + "thermal-cpufreq-%x". This api can support multiple instances of
> cpufreq
> + cooling devices.
> +
> + tab_ptr: The table containing the maximum value of frequency to be
> clipped
> + for each cooling state.
> + .freq_clip_max: Value of frequency to be clipped for each allowed
> + cpus.
> + .temp_level: Temperature level at which the frequency clamping will
> + happen.
> + .mask_val: cpumask of the allowed cpu's
> + tab_size: the total number of cpufreq cooling states.
> +
> +1.1.2 void cpufreq_cooling_unregister(struct thermal_cooling_device *cdev)
> +
> + This interface function unregisters the "thermal-cpufreq-%x" cooling
> device.
> +
> + cdev: Cooling device pointer which has to be unregistered.
> +
> +
> +1.2 CPU cooling action notifier register/unregister interface
> +1.2.1 int cputherm_register_notifier(struct notifier_block *nb,
> + unsigned int list)
> +
> + This interface registers a driver with cpu cooling layer. The driver
> will
> + be notified when any cpu cooling action is called.
> +
> + nb: notifier function to register
> + list: CPUFREQ_COOLING_START or CPUFREQ_COOLING_STOP
> +
> +1.2.2 int cputherm_unregister_notifier(struct notifier_block *nb,
> + unsigned int list)
> +
> + This interface registers a driver with cpu cooling layer. The driver
> will
> + be notified when any cpu cooling action is called.
> +
> + nb: notifier function to register
> + list: CPUFREQ_COOLING_START or CPUFREQ_COOLING_STOP
> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> index 514a691..d9c529f 100644
> --- a/drivers/thermal/Kconfig
> +++ b/drivers/thermal/Kconfig
> @@ -19,6 +19,17 @@ config THERMAL_HWMON
> depends on HWMON=y || HWMON=THERMAL
> default y
>
> +config CPU_THERMAL
> + bool "generic cpu cooling support"
> + depends on THERMAL && CPU_FREQ
> + help
> + This implements the generic cpu cooling mechanism through
> frequency
> + reduction, cpu hotplug and any other ways of reducing
> temperature. An
> + ACPI version of this already
> exists(drivers/acpi/processor_thermal.c).
> + This will be useful for platforms using the generic thermal
> interface
> + and not the ACPI interface.
> + If you want this support, you should say Y or M here.
> +
> config SPEAR_THERMAL
> bool "SPEAr thermal sensor driver"
> depends on THERMAL
> diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
> index a9fff0b..30c456c 100644
> --- a/drivers/thermal/Makefile
> +++ b/drivers/thermal/Makefile
> @@ -3,4 +3,5 @@
> #
>
> obj-$(CONFIG_THERMAL) += thermal_sys.o
> -obj-$(CONFIG_SPEAR_THERMAL) += spear_thermal.o
> \ No newline at end of file
> +obj-$(CONFIG_CPU_THERMAL) += cpu_cooling.o
> +obj-$(CONFIG_SPEAR_THERMAL) += spear_thermal.o
> diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c
> new file mode 100644
> index 0000000..c40d9a0
> --- /dev/null
> +++ b/drivers/thermal/cpu_cooling.c
> @@ -0,0 +1,483 @@
> +/*
> + * linux/drivers/thermal/cpu_cooling.c
> + *
> + * Copyright (C) 2012 Samsung Electronics Co., Ltd(
> http://www.samsung.com)
> + * Copyright (C) 2012 Amit Daniel <amit.kachhap-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> + *
> + *
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; version 2 of the License.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> along
> + * with this program; if not, write to the Free Software Foundation,
> Inc.,
> + * 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA.
> + *
> + *
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> + */
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/thermal.h>
> +#include <linux/platform_device.h>
> +#include <linux/cpufreq.h>
> +#include <linux/err.h>
> +#include <linux/slab.h>
> +#include <linux/cpu.h>
> +#include <linux/cpu_cooling.h>
> +
> +/**
> + * struct cpufreq_cooling_device
> + * @id: unique integer value corresponding to each cpufreq_cooling_device
> + * registered.
> + * @cool_dev: thermal_cooling_device pointer to keep track of the the
> + * egistered cooling device.
> + * @tab_ptr: freq_clip_table table containing the maximum value of
> frequency to
> + * be set for different cooling state.
> + * @tab_size: integer value representing a count of the above table.
> + * @cpufreq_state: integer value representing the current state of cpufreq
> + * cooling devices.
> + * @allowed_cpus: all the cpus involved for this cpufreq_cooling_device.
> + * @node: list_head to link all cpufreq_cooling_device together.
> + *
> + * This structure is required for keeping information of each
> + * cpufreq_cooling_device registered as a list whose head is represented
> by
> + * cooling_cpufreq_list. In order to prevent corruption of this list a
> + * mutex lock cooling_cpufreq_lock is used.
> + */
> +struct cpufreq_cooling_device {
> + int id;
> + struct thermal_cooling_device *cool_dev;
> + struct freq_clip_table *tab_ptr;
> + unsigned int tab_size;
> + unsigned int cpufreq_state;
> + struct cpumask allowed_cpus;
> + struct list_head node;
> +};
> +static LIST_HEAD(cooling_cpufreq_list);
> +static DEFINE_MUTEX(cooling_cpufreq_lock);
> +static DEFINE_IDR(cpufreq_idr);
> +
> +/*per cpu variable to store the previous max frequency allowed*/
> +static DEFINE_PER_CPU(unsigned int, max_policy_freq);
> +
> +/*notify_table passes value to the CPUFREQ_ADJUST callback function.*/
> +#define NOTIFY_INVALID NULL
> +static struct freq_clip_table *notify_table;
> +
> +/*Head of the blocking notifier chain to inform about frequency clamping*/
> +static BLOCKING_NOTIFIER_HEAD(cputherm_state_notifier_list);
> +
> +/**
> + * get_idr - function to get a unique id.
> + * @idr: struct idr * handle used to create a id.
> + * @id: int * value generated by this function.
> + */
> +static int get_idr(struct idr *idr, int *id)
> +{
> + int err;
> +again:
> + if (unlikely(idr_pre_get(idr, GFP_KERNEL) == 0))
> + return -ENOMEM;
> +
> + mutex_lock(&cooling_cpufreq_lock);
> + err = idr_get_new(idr, NULL, id);
> + mutex_unlock(&cooling_cpufreq_lock);
> +
> + if (unlikely(err == -EAGAIN))
> + goto again;
> + else if (unlikely(err))
> + return err;
> +
> + *id = *id & MAX_ID_MASK;
> + return 0;
> +}
> +
> +/**
> + * release_idr - function to free the unique id.
> + * @idr: struct idr * handle used for creating the id.
> + * @id: int value representing the unique id.
> + */
> +static void release_idr(struct idr *idr, int id)
> +{
> + mutex_lock(&cooling_cpufreq_lock);
> + idr_remove(idr, id);
> + mutex_unlock(&cooling_cpufreq_lock);
> +}
> +
> +/**
> + * cputherm_register_notifier - Register a notifier with cpu cooling
> interface.
> + * @nb: struct notifier_block * with callback info.
> + * @list: integer value for which notification is needed. possible values
> are
> + * CPUFREQ_COOLING_START and CPUFREQ_COOLING_STOP.
> + *
> + * This exported function registers a driver with cpu cooling layer. The
> driver
> + * will be notified when any cpu cooling action is called.
> + */
> +int cputherm_register_notifier(struct notifier_block *nb, unsigned int
> list)
> +{
> + int ret = 0;
> +
> + switch (list) {
> + case CPUFREQ_COOLING_START:
> + case CPUFREQ_COOLING_STOP:
> + ret = blocking_notifier_chain_register(
> + &cputherm_state_notifier_list, nb);
> + break;
> + default:
> + ret = -EINVAL;
> + }
> + return ret;
> +}
> +EXPORT_SYMBOL(cputherm_register_notifier);
> +
> +/**
> + * cputherm_unregister_notifier - Un-register a notifier.
> + * @nb: struct notifier_block * with callback info.
> + * @list: integer value for which notification is needed. values possible
> are
> + * CPUFREQ_COOLING_START or CPUFREQ_COOLING_STOP.
> + *
> + * This exported function un-registers a driver with cpu cooling layer.
> + */
> +int cputherm_unregister_notifier(struct notifier_block *nb, unsigned int
> list)
> +{
> + int ret = 0;
> +
> + switch (list) {
> + case CPUFREQ_COOLING_START:
> + case CPUFREQ_COOLING_STOP:
> + ret = blocking_notifier_chain_unregister(
> + &cputherm_state_notifier_list, nb);
> + break;
> + default:
> + ret = -EINVAL;
> + }
> + return ret;
> +}
> +EXPORT_SYMBOL(cputherm_unregister_notifier);
> +
> +/*Below codes defines functions to be used for cpufreq as cooling device*/
> +
> +/**
> + * is_cpufreq_valid - function to check if a cpu has frequency transition
> policy.
> + * @cpu: cpu for which check is needed.
> + */
> +static int is_cpufreq_valid(int cpu)
> +{
> + struct cpufreq_policy policy;
> + return !cpufreq_get_policy(&policy, cpu);
> +}
> +
> +/**
> + * cpufreq_apply_cooling - function to apply frequency clipping.
> + * @cpufreq_device: cpufreq_cooling_device pointer containing frequency
> + * clipping data.
> + * @cooling_state: value of the cooling state.
> + */
> +static int cpufreq_apply_cooling(struct cpufreq_cooling_device
> *cpufreq_device,
> + unsigned long cooling_state)
> +{
> + unsigned int event, cpuid, state;
> + struct freq_clip_table *th_table, *table_ptr;
> + const struct cpumask *maskPtr = &cpufreq_device->allowed_cpus;
> + struct cpufreq_cooling_device *cpufreq_ptr;
> +
> + if (cooling_state > cpufreq_device->tab_size)
> + return -EINVAL;
> +
> + /*Check if the old cooling action is same as new cooling action*/
> + if (cpufreq_device->cpufreq_state == cooling_state)
> + return 0;
> +
> + /*pass cooling table info to the cpufreq_thermal_notifier
> callback*/
> + notify_table = NOTIFY_INVALID;
> +
> + if (cooling_state > 0) {
> + th_table = &(cpufreq_device->tab_ptr[cooling_state - 1]);
> + notify_table = th_table;
> + }
> +
> + /*check if any lower clip frequency active in other
> cpufreq_device's*/
> + list_for_each_entry(cpufreq_ptr, &cooling_cpufreq_list, node) {
> +
> + state = cpufreq_ptr->cpufreq_state;
> + if (state == 0 || cpufreq_ptr == cpufreq_device)
> + continue;
> +
> + if (!cpumask_equal(&cpufreq_ptr->allowed_cpus,
> + &cpufreq_device->allowed_cpus))
> + continue;
> +
> + table_ptr = &(cpufreq_ptr->tab_ptr[state - 1]);
> + if (notify_table == NULL ||
> + (table_ptr->freq_clip_max <
> + notify_table->freq_clip_max))
> + notify_table = table_ptr;
> + }
> +
> + cpufreq_device->cpufreq_state = cooling_state;
> +
> + if (notify_table != NOTIFY_INVALID) {
> + event = CPUFREQ_COOLING_START;
> + maskPtr = notify_table->mask_val;
> + } else {
> + event = CPUFREQ_COOLING_STOP;
> + }
> +
> + blocking_notifier_call_chain(&cputherm_state_notifier_list,
> + event, notify_table);
> +
> + for_each_cpu(cpuid, maskPtr) {
> + if (is_cpufreq_valid(cpuid))
> + cpufreq_update_policy(cpuid);
> + }
> +
> + notify_table = NOTIFY_INVALID;
> +
> + return 0;
> +}
> +
> +/**
> + * cpufreq_thermal_notifier - notifier callback for cpufreq policy change.
> + * @nb: struct notifier_block * with callback info.
> + * @event: value showing cpufreq event for which this function invoked.
> + * @data: callback-specific data
> + */
> +static int cpufreq_thermal_notifier(struct notifier_block *nb,
> + unsigned long event, void *data)
> +{
> + struct cpufreq_policy *policy = data;
> + unsigned long max_freq = 0;
> +
> + if (event != CPUFREQ_ADJUST)
> + return 0;
> +
> + if (notify_table != NOTIFY_INVALID) {
> + max_freq = notify_table->freq_clip_max;
> +
> + if (!per_cpu(max_policy_freq, policy->cpu))
> + per_cpu(max_policy_freq, policy->cpu) =
> policy->max;
> + } else {
> + if (per_cpu(max_policy_freq, policy->cpu)) {
> + max_freq = per_cpu(max_policy_freq, policy->cpu);
> + per_cpu(max_policy_freq, policy->cpu) = 0;
> + } else {
> + max_freq = policy->max;
> + }
> + }
> +
> + /* Never exceed user_policy.max*/
> + if (max_freq > policy->user_policy.max)
> + max_freq = policy->user_policy.max;
> +
> + if (policy->max != max_freq)
> + cpufreq_verify_within_limits(policy, 0, max_freq);
> +
> + return 0;
> +}
> +
> +/*
> + * cpufreq cooling device callback functions are defined below
> + */
> +
> +/**
> + * cpufreq_get_max_state - callback function to get the max cooling state.
> + * @cdev: thermal cooling device pointer.
> + * @state: fill this variable with the max cooling state.
> + */
> +static int cpufreq_get_max_state(struct thermal_cooling_device *cdev,
> + unsigned long *state)
> +{
> + int ret = -EINVAL;
> + struct cpufreq_cooling_device *cpufreq_device;
> +
> + mutex_lock(&cooling_cpufreq_lock);
> + list_for_each_entry(cpufreq_device, &cooling_cpufreq_list, node) {
> + if (cpufreq_device && cpufreq_device->cool_dev == cdev) {
> + *state = cpufreq_device->tab_size;
> + ret = 0;
> + break;
> + }
> + }
> + mutex_unlock(&cooling_cpufreq_lock);
> + return ret;
> +}
> +
> +/**
> + * cpufreq_get_cur_state - callback function to get the current cooling
> state.
> + * @cdev: thermal cooling device pointer.
> + * @state: fill this variable with the current cooling state.
> + */
> +static int cpufreq_get_cur_state(struct thermal_cooling_device *cdev,
> + unsigned long *state)
> +{
> + int ret = -EINVAL;
> + struct cpufreq_cooling_device *cpufreq_device;
> +
> + mutex_lock(&cooling_cpufreq_lock);
> + list_for_each_entry(cpufreq_device, &cooling_cpufreq_list, node) {
> + if (cpufreq_device && cpufreq_device->cool_dev == cdev) {
> + *state = cpufreq_device->cpufreq_state;
> + ret = 0;
> + break;
> + }
> + }
> + mutex_unlock(&cooling_cpufreq_lock);
> + return ret;
> +}
> +
> +/**
> + * cpufreq_set_cur_state - callback function to set the current cooling
> state.
> + * @cdev: thermal cooling device pointer.
> + * @state: set this variable to the current cooling state.
> + */
> +static int cpufreq_set_cur_state(struct thermal_cooling_device *cdev,
> + unsigned long state)
> +{
> + int ret = -EINVAL;
> + struct cpufreq_cooling_device *cpufreq_device;
> +
> + mutex_lock(&cooling_cpufreq_lock);
> + list_for_each_entry(cpufreq_device, &cooling_cpufreq_list, node) {
> + if (cpufreq_device && cpufreq_device->cool_dev == cdev) {
> + ret = 0;
> + break;
> + }
> + }
> + if (!ret)
> + ret = cpufreq_apply_cooling(cpufreq_device, state);
> +
> + mutex_unlock(&cooling_cpufreq_lock);
> +
> + return ret;
> +}
> +
> +/*Bind cpufreq callbacks to thermal cooling device ops*/
> +static struct thermal_cooling_device_ops const cpufreq_cooling_ops = {
> + .get_max_state = cpufreq_get_max_state,
> + .get_cur_state = cpufreq_get_cur_state,
> + .set_cur_state = cpufreq_set_cur_state,
> +};
> +
> +/*Notifier for cpufreq policy change*/
> +static struct notifier_block thermal_cpufreq_notifier_block = {
> + .notifier_call = cpufreq_thermal_notifier,
> +};
> +
> +/**
> + * cpufreq_cooling_register - function to create cpufreq cooling device.
> + * @tab_ptr: table ptr containing the maximum value of frequency to be
> clipped
> + * for each cooling state.
> + * @tab_size: count of entries in the above table.
> + * happen.
> + */
> +struct thermal_cooling_device *cpufreq_cooling_register(
> + struct freq_clip_table *tab_ptr, unsigned int tab_size)
> +{
> + struct thermal_cooling_device *cool_dev;
> + struct cpufreq_cooling_device *cpufreq_dev = NULL;
> + struct freq_clip_table *clip_tab;
> + unsigned int cpufreq_dev_count = 0;
> + char dev_name[THERMAL_NAME_LENGTH];
> + int ret = 0, id = 0, i;
> +
> + if (tab_ptr == NULL || tab_size == 0)
> + return ERR_PTR(-EINVAL);
> +
> + list_for_each_entry(cpufreq_dev, &cooling_cpufreq_list, node)
> + cpufreq_dev_count++;
> +
> + cpufreq_dev = kzalloc(sizeof(struct cpufreq_cooling_device),
> + GFP_KERNEL);
> + if (!cpufreq_dev)
> + return ERR_PTR(-ENOMEM);
> +
> + /*Verify that all the entries of freq_clip_table are present*/
> + for (i = 0; i < tab_size; i++) {
> + clip_tab = ((struct freq_clip_table *)&tab_ptr[i]);
> + if (!clip_tab->freq_clip_max || !clip_tab->mask_val
> + || !clip_tab->temp_level) {
>
About clip_tab->temp_level:
I'd like to remove this variable checking, because as suggested by Vincent
Guittot, it is better to separate cooling devices
and thermal zone, thus we can add/remove cooling devices
freely/dynamically. In such case, a cooling device doesn't
know any temp_level before binding--the binding condition is name matching,
what's more, the cooling device should not
know much data of other devices.
+ kfree(cpufreq_dev);
> + return ERR_PTR(-EINVAL);
> + }
> + /*
> + *Consolidate all the cpumask for all the individual
> entries
> + *of the trip table. This is useful in resetting all the
> + *clipped frequencies to the normal level for each cpufreq
> + *cooling device.
> + */
> + cpumask_or(&cpufreq_dev->allowed_cpus,
> + &cpufreq_dev->allowed_cpus, clip_tab->mask_val);
> + }
> +
> + cpufreq_dev->tab_ptr = tab_ptr;
> + cpufreq_dev->tab_size = tab_size;
> +
> + ret = get_idr(&cpufreq_idr, &cpufreq_dev->id);
> + if (ret) {
> + kfree(cpufreq_dev);
> + return ERR_PTR(-EINVAL);
> + }
> +
> + sprintf(dev_name, "thermal-cpufreq-%d", cpufreq_dev->id);
> +
> + cool_dev = thermal_cooling_device_register(dev_name, cpufreq_dev,
> + &cpufreq_cooling_ops);
> + if (!cool_dev) {
> + release_idr(&cpufreq_idr, cpufreq_dev->id);
> + kfree(cpufreq_dev);
> + return ERR_PTR(-EINVAL);
> + }
> + cpufreq_dev->id = id;
> + cpufreq_dev->cool_dev = cool_dev;
> + mutex_lock(&cooling_cpufreq_lock);
> + list_add_tail(&cpufreq_dev->node, &cooling_cpufreq_list);
> +
> + /*Register the notifier for first cpufreq cooling device*/
> + if (cpufreq_dev_count == 0)
> + cpufreq_register_notifier(&thermal_cpufreq_notifier_block,
> + CPUFREQ_POLICY_NOTIFIER);
> +
> + mutex_unlock(&cooling_cpufreq_lock);
> + return cool_dev;
> +}
> +EXPORT_SYMBOL(cpufreq_cooling_register);
> +
> +/**
> + * cpufreq_cooling_unregister - function to remove cpufreq cooling device.
> + * @cdev: thermal cooling device pointer.
> + */
> +void cpufreq_cooling_unregister(struct thermal_cooling_device *cdev)
> +{
> + struct cpufreq_cooling_device *cpufreq_dev = NULL;
> + unsigned int cpufreq_dev_count = 0;
> +
> + mutex_lock(&cooling_cpufreq_lock);
> + list_for_each_entry(cpufreq_dev, &cooling_cpufreq_list, node) {
> + if (cpufreq_dev && cpufreq_dev->cool_dev == cdev)
> + break;
> + cpufreq_dev_count++;
> + }
> +
> + if (!cpufreq_dev || cpufreq_dev->cool_dev != cdev) {
> + mutex_unlock(&cooling_cpufreq_lock);
> + return;
> + }
> +
> + list_del(&cpufreq_dev->node);
> +
> + /*Unregister the notifier for the last cpufreq cooling device*/
> + if (cpufreq_dev_count == 1) {
> +
> cpufreq_unregister_notifier(&thermal_cpufreq_notifier_block,
> + CPUFREQ_POLICY_NOTIFIER);
> + }
> + mutex_unlock(&cooling_cpufreq_lock);
> + thermal_cooling_device_unregister(cpufreq_dev->cool_dev);
> + release_idr(&cpufreq_idr, cpufreq_dev->id);
> + kfree(cpufreq_dev);
> +}
> +EXPORT_SYMBOL(cpufreq_cooling_unregister);
> diff --git a/include/linux/cpu_cooling.h b/include/linux/cpu_cooling.h
> new file mode 100644
> index 0000000..ed6c096
> --- /dev/null
> +++ b/include/linux/cpu_cooling.h
> @@ -0,0 +1,99 @@
> +/*
> + * linux/include/linux/cpu_cooling.h
> + *
> + * Copyright (C) 2012 Samsung Electronics Co., Ltd(
> http://www.samsung.com)
> + * Copyright (C) 2012 Amit Daniel <amit.kachhap-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> + *
> + *
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; version 2 of the License.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> along
> + * with this program; if not, write to the Free Software Foundation,
> Inc.,
> + * 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA.
> + *
> + *
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> + */
> +
> +#ifndef __CPU_COOLING_H__
> +#define __CPU_COOLING_H__
> +
> +#include <linux/thermal.h>
> +
> +#define CPUFREQ_COOLING_START 0
> +#define CPUFREQ_COOLING_STOP 1
> +
> +/**
> + * struct freq_clip_table
> + * @freq_clip_max: maximum frequency allowed for this cooling state.
> + * @temp_level: Temperature level at which the temperature clipping will
> + * happen.
> + * @mask_val: cpumask of the allowed cpu's where the clipping will take
> place.
> + *
> + * This structure is required to be filled and passed to the
> + * cpufreq_cooling_unregister function.
> + */
> +struct freq_clip_table {
> + unsigned int freq_clip_max;
> + unsigned int temp_level;
> + const struct cpumask *mask_val;
> +};
> +
> +/**
> + * cputherm_register_notifier - Register a notifier with cpu cooling
> interface.
> + * @nb: struct notifier_block * with callback info.
> + * @list: integer value for which notification is needed. possible values
> are
> + * CPUFREQ_COOLING_TYPE and CPUHOTPLUG_COOLING_TYPE.
> + *
> + * This exported function registers a driver with cpu cooling layer. The
> driver
> + * will be notified when any cpu cooling action is called.
> + */
> +int cputherm_register_notifier(struct notifier_block *nb, unsigned int
> list);
> +
> +/**
> + * cputherm_unregister_notifier - Un-register a notifier.
> + * @nb: struct notifier_block * with callback info.
> + * @list: integer value for which notification is needed. values possible
> are
> + * CPUFREQ_COOLING_TYPE.
> + *
> + * This exported function un-registers a driver with cpu cooling layer.
> + */
> +int cputherm_unregister_notifier(struct notifier_block *nb, unsigned int
> list);
> +
> +#ifdef CONFIG_CPU_FREQ
> +/**
> + * cpufreq_cooling_register - function to create cpufreq cooling device.
> + * @tab_ptr: table ptr containing the maximum value of frequency to be
> clipped
> + * for each cooling state.
> + * @tab_size: count of entries in the above table.
> + * @mask_val: cpumask containing the allowed cpu's where frequency
> clipping can
> + * happen.
> + */
> +struct thermal_cooling_device *cpufreq_cooling_register(
> + struct freq_clip_table *tab_ptr, unsigned int tab_size);
> +
> +/**
> + * cpufreq_cooling_unregister - function to remove cpufreq cooling device.
> + * @cdev: thermal cooling device pointer.
> + */
> +void cpufreq_cooling_unregister(struct thermal_cooling_device *cdev);
> +#else /*!CONFIG_CPU_FREQ*/
> +static inline struct thermal_cooling_device *cpufreq_cooling_register(
> + struct freq_clip_table *tab_ptr, unsigned int tab_size);
> +{
> + return NULL;
> +}
> +static inline void cpufreq_cooling_unregister(
> + struct thermal_cooling_device *cdev)
> +{
> + return;
> +}
> +#endif /*CONFIG_CPU_FREQ*/
> +
> +#endif /* __CPU_COOLING_H__ */
> --
> 1.7.1
>
>
> _______________________________________________
> linaro-dev mailing list
> linaro-dev-cunTk1MwBs8s++Sfvej+rw@public.gmane.org
> http://lists.linaro.org/mailman/listinfo/linaro-dev
>
[-- Attachment #1.2: Type: text/html, Size: 32261 bytes --]
[-- Attachment #2: Type: text/plain, Size: 175 bytes --]
_______________________________________________
linaro-dev mailing list
linaro-dev-cunTk1MwBs8s++Sfvej+rw@public.gmane.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
^ permalink raw reply
* Re: [PATCH 2/4] cpuidle: define the enter function in the driver structure
From: Rajendra Nayak @ 2012-07-10 8:29 UTC (permalink / raw)
To: Daniel Lezcano; +Cc: Rafael J. Wysocki, linux-acpi, linux-pm, linaro-dev, lenb
In-Reply-To: <4FF6C4C1.7030004@linaro.org>
Hi Daniel,
On Friday 06 July 2012 04:28 PM, Daniel Lezcano wrote:
> The main purpose of all these cleanup patches are to move out all
> non-data information from the cpuidle_state structure in order to add a
> new api which could be 'cpuidle_register_cpu_latency(int cpu, struct
> cpuidle_latencies latencies)'.
So are there any technical difficulties in adding such an api with the
non-data information being part of cpuidle_state? Or its just that you
think the already duplicated non-data information (per state) is going
to be duplicated much more (per state per cpu) in most cases.
regards,
Rajendra
^ permalink raw reply
* Re: [PATCH 2/4] cpuidle: define the enter function in the driver structure
From: Daniel Lezcano @ 2012-07-10 11:39 UTC (permalink / raw)
To: Rajendra Nayak; +Cc: linux-acpi, linux-pm, linaro-dev
In-Reply-To: <4FFBE7E3.8020809@linaro.org>
On 07/10/2012 10:29 AM, Rajendra Nayak wrote:
> Hi Daniel,
>
> On Friday 06 July 2012 04:28 PM, Daniel Lezcano wrote:
>> The main purpose of all these cleanup patches are to move out all
>> non-data information from the cpuidle_state structure in order to add a
>> new api which could be 'cpuidle_register_cpu_latency(int cpu, struct
>> cpuidle_latencies latencies)'.
>
> So are there any technical difficulties in adding such an api with the
> non-data information being part of cpuidle_state? Or its just that you
> think the already duplicated non-data information (per state) is going
> to be duplicated much more (per state per cpu) in most cases.
Yes, it is possible. I am already working on an alternate patchset I
will post soon, but I don't like to add more things in a subsystem
before cleaning up the code when it makes sense.
When I looked at the cpuidle code, I noticed this 'enter' field was used
for the same function. With per cpu latency and using the cpuidle_state
structure, that means duplication of this field. For an x86_64
architecture with 16 cores and 4 C-states, that is 512 bytes of memory
instead of 8 bytes.
That is the same for example the 'disable' field which could be stored
in the 'flags' field as a bit instead of the moving it to the 'stats'
structure with "unsigned long long".
As the architectures tend to add more cores [1][2], that means more
memory consumption. In a very near future, we will have 100 cores in a
cpu. Assuming we have 4 C-states, that is 100 * 4 * 8 = 3200 bytes of
memory used for a single function. IMHO, the kernel shouldn't assume the
user must add more memory on the system.
Anyway, I will post a patchset to use the cpuidle_state per cpu.
Thanks
-- Daniel
[1] http://www.engadget.com/2007/02/11/intel-demonstrates-80-core-processor/
[2] http://www.tilera.com/products/processors/TILE-Gx_Family
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
^ permalink raw reply
* Re: [PATCH 2/4] cpuidle: define the enter function in the driver structure
From: Rajendra Nayak @ 2012-07-10 12:38 UTC (permalink / raw)
To: Daniel Lezcano; +Cc: Rafael J. Wysocki, linux-acpi, linux-pm, linaro-dev, lenb
In-Reply-To: <4FFC147C.4050508@linaro.org>
> As the architectures tend to add more cores [1][2], that means more
> memory consumption. In a very near future, we will have 100 cores in a
> cpu. Assuming we have 4 C-states, that is 100 * 4 * 8 = 3200 bytes of
> memory used for a single function. IMHO, the kernel shouldn't assume the
> user must add more memory on the system.
I completely agree.
>
> Anyway, I will post a patchset to use the cpuidle_state per cpu.
Maybe what you were doing wasn't clearly evident with the existing
cpuidle core, but once its scaled to add a per cpu state, it might
look more meaningful. So if you post your patches to add cpuidle
state per cpu, it should certainly give more insight into why this
cleanup makes sense.
>
> Thanks
> -- Daniel
>
> [1] http://www.engadget.com/2007/02/11/intel-demonstrates-80-core-processor/
> [2] http://www.tilera.com/products/processors/TILE-Gx_Family
>
^ permalink raw reply
* Re: use of pm_runtime_disable() from driver probe?
From: Kevin Hilman @ 2012-07-10 18:11 UTC (permalink / raw)
To: Rafael J. Wysocki; +Cc: linux-pm
In-Reply-To: <201207072125.26189.rjw@sisk.pl>
"Rafael J. Wysocki" <rjw@sisk.pl> writes:
> Hi,
>
> On Saturday, July 07, 2012, Kevin Hilman wrote:
>> Hello,
>>
>> I've got a question around the proper use of pm_runtime_disable() in a
>> driver's ->probe method. Basically, using pm_runtime_disable() in
>> ->probe() (e.g because of a error/failure) can result in the device
>> remaning runtime enabled forever.
>
> Er, no.
Right, poor choice of words. What I meant was "...can result in the
device remaining powered on forever."
>> This is because the driver core wraps
>> the probe in _get_noresume()/_put_sync() so any usage of _put_sync() in
>> ->probe itself does not trigger an actual device runtime suspend. If
>> the driver also calls pm_runtime_disable() in probe, that results in the
>> device remaining runtime enabled.
>
> I'm not quite sure what way. pm_runtime_disable() increments
> dev->power.disable_count and if that is not decremented later, the
> device remains runtime disabled.
>
> You probably wanted to say that the device will end up in the full-power
> state in that case, which isn't the same and is intentional.
Correct. I'm glad you understood what I meant instead of what I said.
I wish my kids could do that. ;)
[...]
>> So, the question is: is it right to use pm_runtime_disable() this way?
>
> If your intention is to put the device into a low-power state in case of
> a failing .probe(), this isn't the way to do it. pm_runtime_disable()
> means "don't do any runtime power management on this device any more"
> regardless of the current state of the device (it may resume the device
> in some situations, but it won't suspend it, although it may wait for
> a suspend in progress to complete).
That's correct, but I also think that is what driver writers probably
believe they are doing. IOW, after a probe failure, the driver writer
does indeed mean "don't do runtime PM on this device any more." But
they really mean "don't do runtime PM on this device *after* it's been
powered down."
However, because of the get/put in the driver core, it's not entirely
obvious to driver writers that calling pm_runtime_disable() actually
keeps the device powered up, even though they just called _put_sync().
Another way of stating the problem is that the _put_sync() in the driver
core's driver_probe_device() assumes that runtime PM is enabled for that
device.
> That said, I'm not aware of any other generally reliable way to do that
> either.
That's what I was afraid of. :(
> The problem is, if .probe() is failing we don't have callbacks
> allowing us to power manage the device (there may be bus type callbacks
> knowing how to do that, but not for the platform bus type).
There are callbacks for the PM domains, but they are not called either
if the driver calls pm_runtime_disable().
>> Removing it means the device is left in runtime suspended state even in
>> the error case. However, without the pm_runtime_disable(), if this
>> driver was a module, repeated attempts to load would result in
>> unbalalced calls to pm_runtime_enable.
>>
>> What is the right way to handle the runtime PM enable/disable in a
>> failed probe attempt?
>
> I would recommend using pm_runtime_disable() and then either
> pm_runtime_set_active() or pm_runtime_set_suspended() depending on what the
> real power state of the device is at this point.
That doesn't help because it only changes the runtime PM core's notion
of the power state, not the physical power state because it doesn't call
the subsystem callbacks.
Or, are you suggesting to set the state to suspended, and then use a
late_initcall to compare the runtime status with the physical device
status and power down devices accordingly?
> Anyway, you can't force the device into a low-power state using
> runtime PM after a failing probe, at least in general.
Well, using PM domains, that's exactly what can happen if the driver
doesn't call pm_runtime_disable() because the _put_sync() in the driver
core will trigger the PM domain callbacks.
Kevin
^ permalink raw reply
* Re: use of pm_runtime_disable() from driver probe?
From: Rafael J. Wysocki @ 2012-07-10 18:31 UTC (permalink / raw)
To: Kevin Hilman; +Cc: linux-pm
In-Reply-To: <87r4sjo5a2.fsf@ti.com>
On Tuesday, July 10, 2012, Kevin Hilman wrote:
> "Rafael J. Wysocki" <rjw@sisk.pl> writes:
>
> > Hi,
> >
> > On Saturday, July 07, 2012, Kevin Hilman wrote:
> >> Hello,
> >>
> >> I've got a question around the proper use of pm_runtime_disable() in a
> >> driver's ->probe method. Basically, using pm_runtime_disable() in
> >> ->probe() (e.g because of a error/failure) can result in the device
> >> remaning runtime enabled forever.
> >
> > Er, no.
>
> Right, poor choice of words. What I meant was "...can result in the
> device remaining powered on forever."
>
> >> This is because the driver core wraps
> >> the probe in _get_noresume()/_put_sync() so any usage of _put_sync() in
> >> ->probe itself does not trigger an actual device runtime suspend. If
> >> the driver also calls pm_runtime_disable() in probe, that results in the
> >> device remaining runtime enabled.
> >
> > I'm not quite sure what way. pm_runtime_disable() increments
> > dev->power.disable_count and if that is not decremented later, the
> > device remains runtime disabled.
> >
> > You probably wanted to say that the device will end up in the full-power
> > state in that case, which isn't the same and is intentional.
>
> Correct. I'm glad you understood what I meant instead of what I said.
> I wish my kids could do that. ;)
>
> [...]
>
> >> So, the question is: is it right to use pm_runtime_disable() this way?
> >
> > If your intention is to put the device into a low-power state in case of
> > a failing .probe(), this isn't the way to do it. pm_runtime_disable()
> > means "don't do any runtime power management on this device any more"
> > regardless of the current state of the device (it may resume the device
> > in some situations, but it won't suspend it, although it may wait for
> > a suspend in progress to complete).
>
> That's correct, but I also think that is what driver writers probably
> believe they are doing. IOW, after a probe failure, the driver writer
> does indeed mean "don't do runtime PM on this device any more." But
> they really mean "don't do runtime PM on this device *after* it's been
> powered down."
>
> However, because of the get/put in the driver core, it's not entirely
> obvious to driver writers that calling pm_runtime_disable() actually
> keeps the device powered up, even though they just called _put_sync().
>
> Another way of stating the problem is that the _put_sync() in the driver
> core's driver_probe_device() assumes that runtime PM is enabled for that
> device.
>
> > That said, I'm not aware of any other generally reliable way to do that
> > either.
>
> That's what I was afraid of. :(
>
> > The problem is, if .probe() is failing we don't have callbacks
> > allowing us to power manage the device (there may be bus type callbacks
> > knowing how to do that, but not for the platform bus type).
>
> There are callbacks for the PM domains, but they are not called either
> if the driver calls pm_runtime_disable().
Oh, I think I see what the problem is.
> >> Removing it means the device is left in runtime suspended state even in
> >> the error case. However, without the pm_runtime_disable(), if this
> >> driver was a module, repeated attempts to load would result in
> >> unbalalced calls to pm_runtime_enable.
> >>
> >> What is the right way to handle the runtime PM enable/disable in a
> >> failed probe attempt?
> >
> > I would recommend using pm_runtime_disable() and then either
> > pm_runtime_set_active() or pm_runtime_set_suspended() depending on what the
> > real power state of the device is at this point.
>
> That doesn't help because it only changes the runtime PM core's notion
> of the power state, not the physical power state because it doesn't call
> the subsystem callbacks.
>
> Or, are you suggesting to set the state to suspended, and then use a
> late_initcall to compare the runtime status with the physical device
> status and power down devices accordingly?
>
> > Anyway, you can't force the device into a low-power state using
> > runtime PM after a failing probe, at least in general.
>
> Well, using PM domains, that's exactly what can happen if the driver
> doesn't call pm_runtime_disable() because the _put_sync() in the driver
> core will trigger the PM domain callbacks.
OK, so if you have PM domains, then the case is equivalent to having a bus
type with its own runtime PM callbacks. In that case, if .probe() fails,
it obviously doesn't mean that the device shouldn't be power managed,
so the driver shouldn't call pm_runtime_disable().
Generally, if runtime PM was enabled for a device before .probe() has been
called, the driver shouldn't disable it in .probe() whatever the reason,
because it may not have enough information for deciding whether or not
runtime PM should be disabled.
Thanks,
Rafael
^ permalink raw reply
* Re: use of pm_runtime_disable() from driver probe?
From: Alan Stern @ 2012-07-10 18:47 UTC (permalink / raw)
To: Rafael J. Wysocki; +Cc: Kevin Hilman, linux-pm
In-Reply-To: <201207102031.15579.rjw@sisk.pl>
On Tue, 10 Jul 2012, Rafael J. Wysocki wrote:
> > > Anyway, you can't force the device into a low-power state using
> > > runtime PM after a failing probe, at least in general.
> >
> > Well, using PM domains, that's exactly what can happen if the driver
> > doesn't call pm_runtime_disable() because the _put_sync() in the driver
> > core will trigger the PM domain callbacks.
>
> OK, so if you have PM domains, then the case is equivalent to having a bus
> type with its own runtime PM callbacks. In that case, if .probe() fails,
> it obviously doesn't mean that the device shouldn't be power managed,
> so the driver shouldn't call pm_runtime_disable().
>
> Generally, if runtime PM was enabled for a device before .probe() has been
> called, the driver shouldn't disable it in .probe() whatever the reason,
> because it may not have enough information for deciding whether or not
> runtime PM should be disabled.
So if the PM domain code called pm_runtime_enable() then the domain
code should be responsible for calling pm_runtime_disable() too,
presumably after putting the device back into a low-power state. I'm
not sure when that would occur, however. Immediately after registering
the device, if no driver is bound?
In the case where the probe routine called pm_runtime_enable(), you're
stuck. The probe routine _has_ to call pm_runtime_disable() when a
failure occurs, to keep the disable count balanced.
Alan Stern
^ permalink raw reply
* Re: use of pm_runtime_disable() from driver probe?
From: Rafael J. Wysocki @ 2012-07-10 19:14 UTC (permalink / raw)
To: Alan Stern; +Cc: Kevin Hilman, linux-pm
In-Reply-To: <Pine.LNX.4.44L0.1207101437230.1308-100000@iolanthe.rowland.org>
On Tuesday, July 10, 2012, Alan Stern wrote:
> On Tue, 10 Jul 2012, Rafael J. Wysocki wrote:
>
> > > > Anyway, you can't force the device into a low-power state using
> > > > runtime PM after a failing probe, at least in general.
> > >
> > > Well, using PM domains, that's exactly what can happen if the driver
> > > doesn't call pm_runtime_disable() because the _put_sync() in the driver
> > > core will trigger the PM domain callbacks.
> >
> > OK, so if you have PM domains, then the case is equivalent to having a bus
> > type with its own runtime PM callbacks. In that case, if .probe() fails,
> > it obviously doesn't mean that the device shouldn't be power managed,
> > so the driver shouldn't call pm_runtime_disable().
> >
> > Generally, if runtime PM was enabled for a device before .probe() has been
> > called, the driver shouldn't disable it in .probe() whatever the reason,
> > because it may not have enough information for deciding whether or not
> > runtime PM should be disabled.
>
> So if the PM domain code called pm_runtime_enable() then the domain
> code should be responsible for calling pm_runtime_disable() too,
> presumably after putting the device back into a low-power state. I'm
> not sure when that would occur, however. Immediately after registering
> the device, if no driver is bound?
>
> In the case where the probe routine called pm_runtime_enable(), you're
> stuck. The probe routine _has_ to call pm_runtime_disable() when a
> failure occurs, to keep the disable count balanced.
Yes, I has just been thinking about that.
If .probe() enabled runtime PM and called pm_runtime_get_sync() (or _resume),
it can't clean up properly in case of an error, because its
pm_runtime_put_sync() (or _suspend) won't be effective and you're right that
it has to call pm_runtime_disable().
So, we don't handle this particular case correctly.
I'm not sure what the solution should be, though. We could remove the
runtime PM operations around really_probe(), but then there may be drivers
assuming that the core will call pm_runtime_put_sync() after .probe()
has returned.
Thanks,
Rafael
^ permalink raw reply
* Re: use of pm_runtime_disable() from driver probe?
From: Rafael J. Wysocki @ 2012-07-10 19:41 UTC (permalink / raw)
To: linux-pm; +Cc: Kevin Hilman
In-Reply-To: <201207102114.05001.rjw@sisk.pl>
On Tuesday, July 10, 2012, Rafael J. Wysocki wrote:
> On Tuesday, July 10, 2012, Alan Stern wrote:
> > On Tue, 10 Jul 2012, Rafael J. Wysocki wrote:
> >
> > > > > Anyway, you can't force the device into a low-power state using
> > > > > runtime PM after a failing probe, at least in general.
> > > >
> > > > Well, using PM domains, that's exactly what can happen if the driver
> > > > doesn't call pm_runtime_disable() because the _put_sync() in the driver
> > > > core will trigger the PM domain callbacks.
> > >
> > > OK, so if you have PM domains, then the case is equivalent to having a bus
> > > type with its own runtime PM callbacks. In that case, if .probe() fails,
> > > it obviously doesn't mean that the device shouldn't be power managed,
> > > so the driver shouldn't call pm_runtime_disable().
> > >
> > > Generally, if runtime PM was enabled for a device before .probe() has been
> > > called, the driver shouldn't disable it in .probe() whatever the reason,
> > > because it may not have enough information for deciding whether or not
> > > runtime PM should be disabled.
> >
> > So if the PM domain code called pm_runtime_enable() then the domain
> > code should be responsible for calling pm_runtime_disable() too,
> > presumably after putting the device back into a low-power state. I'm
> > not sure when that would occur, however. Immediately after registering
> > the device, if no driver is bound?
> >
> > In the case where the probe routine called pm_runtime_enable(), you're
> > stuck. The probe routine _has_ to call pm_runtime_disable() when a
> > failure occurs, to keep the disable count balanced.
>
> Yes, I has just been thinking about that.
>
> If .probe() enabled runtime PM and called pm_runtime_get_sync() (or _resume),
> it can't clean up properly in case of an error, because its
> pm_runtime_put_sync() (or _suspend) won't be effective and you're right that
> it has to call pm_runtime_disable().
>
> So, we don't handle this particular case correctly.
>
> I'm not sure what the solution should be, though. We could remove the
> runtime PM operations around really_probe(), but then there may be drivers
> assuming that the core will call pm_runtime_put_sync() after .probe()
> has returned.
I have an idea.
What about the following patch? It shouldn't matter except for the cases when
.probe() attempts to suspend the device by itself.
---
drivers/base/dd.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
Index: linux/drivers/base/dd.c
===================================================================
--- linux.orig/drivers/base/dd.c
+++ linux/drivers/base/dd.c
@@ -356,10 +356,8 @@ int driver_probe_device(struct device_dr
pr_debug("bus: '%s': %s: matched device %s with driver %s\n",
drv->bus->name, __func__, dev_name(dev), drv->name);
- pm_runtime_get_noresume(dev);
- pm_runtime_barrier(dev);
ret = really_probe(dev, drv);
- pm_runtime_put_sync(dev);
+ pm_runtime_idle(dev);
return ret;
}
@@ -406,9 +404,8 @@ int device_attach(struct device *dev)
ret = 0;
}
} else {
- pm_runtime_get_noresume(dev);
ret = bus_for_each_drv(dev->bus, NULL, dev, __device_attach);
- pm_runtime_put_sync(dev);
+ pm_runtime_idle(dev);
}
out_unlock:
device_unlock(dev);
^ permalink raw reply
* Re: use of pm_runtime_disable() from driver probe?
From: Alan Stern @ 2012-07-10 20:17 UTC (permalink / raw)
To: Rafael J. Wysocki; +Cc: Kevin Hilman, linux-pm
In-Reply-To: <201207102141.24775.rjw@sisk.pl>
On Tue, 10 Jul 2012, Rafael J. Wysocki wrote:
> > If .probe() enabled runtime PM and called pm_runtime_get_sync() (or _resume),
> > it can't clean up properly in case of an error, because its
> > pm_runtime_put_sync() (or _suspend) won't be effective and you're right that
> > it has to call pm_runtime_disable().
> >
> > So, we don't handle this particular case correctly.
> >
> > I'm not sure what the solution should be, though. We could remove the
> > runtime PM operations around really_probe(), but then there may be drivers
> > assuming that the core will call pm_runtime_put_sync() after .probe()
> > has returned.
>
> I have an idea.
>
> What about the following patch? It shouldn't matter except for the cases when
> .probe() attempts to suspend the device by itself.
>
> ---
> drivers/base/dd.c | 7 ++-----
> 1 file changed, 2 insertions(+), 5 deletions(-)
>
> Index: linux/drivers/base/dd.c
> ===================================================================
> --- linux.orig/drivers/base/dd.c
> +++ linux/drivers/base/dd.c
> @@ -356,10 +356,8 @@ int driver_probe_device(struct device_dr
> pr_debug("bus: '%s': %s: matched device %s with driver %s\n",
> drv->bus->name, __func__, dev_name(dev), drv->name);
>
> - pm_runtime_get_noresume(dev);
> - pm_runtime_barrier(dev);
> ret = really_probe(dev, drv);
> - pm_runtime_put_sync(dev);
> + pm_runtime_idle(dev);
>
> return ret;
> }
> @@ -406,9 +404,8 @@ int device_attach(struct device *dev)
> ret = 0;
> }
> } else {
> - pm_runtime_get_noresume(dev);
> ret = bus_for_each_drv(dev->bus, NULL, dev, __device_attach);
> - pm_runtime_put_sync(dev);
> + pm_runtime_idle(dev);
> }
> out_unlock:
> device_unlock(dev);
This opens up the possibility of calling probe while a runtime resume
or suspend is in progress. (On the other hand, the existing code
doesn't prevent a concurrent runtime resume.) Maybe it would be best
to leave the pm_runtime_barrier().
Apart from that, I think it would solve the problem.
Alan Stern
^ permalink raw reply
* Re: use of pm_runtime_disable() from driver probe?
From: Kevin Hilman @ 2012-07-10 21:04 UTC (permalink / raw)
To: Alan Stern; +Cc: linux-pm
In-Reply-To: <Pine.LNX.4.44L0.1207101615320.1308-100000@iolanthe.rowland.org>
Alan Stern <stern@rowland.harvard.edu> writes:
> On Tue, 10 Jul 2012, Rafael J. Wysocki wrote:
>
>> > If .probe() enabled runtime PM and called pm_runtime_get_sync() (or _resume),
>> > it can't clean up properly in case of an error, because its
>> > pm_runtime_put_sync() (or _suspend) won't be effective and you're right that
>> > it has to call pm_runtime_disable().
>> >
>> > So, we don't handle this particular case correctly.
Right, this is exactly the case I'm working on now. The driver is
calling pm_runtime_enable() and _disable() in .probe().
>> > I'm not sure what the solution should be, though. We could remove the
>> > runtime PM operations around really_probe(), but then there may be drivers
>> > assuming that the core will call pm_runtime_put_sync() after .probe()
>> > has returned.
>>
>> I have an idea.
>>
>> What about the following patch? It shouldn't matter except for the cases when
>> .probe() attempts to suspend the device by itself.
>>
>> ---
>> drivers/base/dd.c | 7 ++-----
>> 1 file changed, 2 insertions(+), 5 deletions(-)
>>
>> Index: linux/drivers/base/dd.c
>> ===================================================================
>> --- linux.orig/drivers/base/dd.c
>> +++ linux/drivers/base/dd.c
>> @@ -356,10 +356,8 @@ int driver_probe_device(struct device_dr
>> pr_debug("bus: '%s': %s: matched device %s with driver %s\n",
>> drv->bus->name, __func__, dev_name(dev), drv->name);
>>
>> - pm_runtime_get_noresume(dev);
>> - pm_runtime_barrier(dev);
>> ret = really_probe(dev, drv);
>> - pm_runtime_put_sync(dev);
>> + pm_runtime_idle(dev);
>>
>> return ret;
>> }
>> @@ -406,9 +404,8 @@ int device_attach(struct device *dev)
>> ret = 0;
>> }
>> } else {
>> - pm_runtime_get_noresume(dev);
>> ret = bus_for_each_drv(dev->bus, NULL, dev, __device_attach);
>> - pm_runtime_put_sync(dev);
>> + pm_runtime_idle(dev);
>> }
>> out_unlock:
>> device_unlock(dev);
>
>
> This opens up the possibility of calling probe while a runtime resume
> or suspend is in progress. (On the other hand, the existing code
> doesn't prevent a concurrent runtime resume.) Maybe it would be best
> to leave the pm_runtime_barrier().
>
> Apart from that, I think it would solve the problem.
I just tested it and it definitely solves the problem. I forgot exactly
why the get/put was added around probe in the first place, so am not
sure if this opens up other potential problems like Alan mentioned.
In any case, if you go forward with this patch, feel free to add:
Reviewed-by: Kevin Hilman <khilman@ti.com>
Tested-by: Kevin Hilman <khilman@ti.com>
Kevin
^ permalink raw reply
* Re: use of pm_runtime_disable() from driver probe?
From: Rafael J. Wysocki @ 2012-07-10 22:31 UTC (permalink / raw)
To: Alan Stern; +Cc: Kevin Hilman, linux-pm
In-Reply-To: <Pine.LNX.4.44L0.1207101615320.1308-100000@iolanthe.rowland.org>
On Tuesday, July 10, 2012, Alan Stern wrote:
> On Tue, 10 Jul 2012, Rafael J. Wysocki wrote:
>
> > > If .probe() enabled runtime PM and called pm_runtime_get_sync() (or _resume),
> > > it can't clean up properly in case of an error, because its
> > > pm_runtime_put_sync() (or _suspend) won't be effective and you're right that
> > > it has to call pm_runtime_disable().
> > >
> > > So, we don't handle this particular case correctly.
> > >
> > > I'm not sure what the solution should be, though. We could remove the
> > > runtime PM operations around really_probe(), but then there may be drivers
> > > assuming that the core will call pm_runtime_put_sync() after .probe()
> > > has returned.
> >
> > I have an idea.
> >
> > What about the following patch? It shouldn't matter except for the cases when
> > .probe() attempts to suspend the device by itself.
> >
> > ---
> > drivers/base/dd.c | 7 ++-----
> > 1 file changed, 2 insertions(+), 5 deletions(-)
> >
> > Index: linux/drivers/base/dd.c
> > ===================================================================
> > --- linux.orig/drivers/base/dd.c
> > +++ linux/drivers/base/dd.c
> > @@ -356,10 +356,8 @@ int driver_probe_device(struct device_dr
> > pr_debug("bus: '%s': %s: matched device %s with driver %s\n",
> > drv->bus->name, __func__, dev_name(dev), drv->name);
> >
> > - pm_runtime_get_noresume(dev);
> > - pm_runtime_barrier(dev);
> > ret = really_probe(dev, drv);
> > - pm_runtime_put_sync(dev);
> > + pm_runtime_idle(dev);
> >
> > return ret;
> > }
> > @@ -406,9 +404,8 @@ int device_attach(struct device *dev)
> > ret = 0;
> > }
> > } else {
> > - pm_runtime_get_noresume(dev);
> > ret = bus_for_each_drv(dev->bus, NULL, dev, __device_attach);
> > - pm_runtime_put_sync(dev);
> > + pm_runtime_idle(dev);
> > }
> > out_unlock:
> > device_unlock(dev);
>
>
> This opens up the possibility of calling probe while a runtime resume
> or suspend is in progress. (On the other hand, the existing code
> doesn't prevent a concurrent runtime resume.) Maybe it would be best
> to leave the pm_runtime_barrier().
That wouldn't close the race, though, because the suspend/resume may still
be started right after _barrier has returned.
The race is only possible if runtime PM is enabled by a subsystem or PM domain
code before the first eligible driver is registered and if that code is not
careful enough to get ready for driver registration. I'm not sure how likely
it is to happen in practice.
Thanks,
Rafael
^ permalink raw reply
* Re: [linux-pm] cpuidle future and improvements
From: Kevin Hilman @ 2012-07-11 14:00 UTC (permalink / raw)
To: Daniel Lezcano
Cc: Colin Cross, Rob Lee, Lists Linaro-dev, Peter De Schrijver,
Linux Kernel Mailing List, Amit Kucheria, linux-acpi, linux-next,
linux-pm, Linus Torvalds, Andrew Morton
In-Reply-To: <4FE85F6A.80307@linaro.org>
Daniel Lezcano <daniel.lezcano@linaro.org> writes:
> On 06/18/2012 08:15 PM, Colin Cross wrote:
>> On Mon, Jun 18, 2012 at 1:40 AM, Daniel Lezcano
>> <daniel.lezcano@linaro.org> wrote:
>>> I propose to host a cpuidle-next tree where all these modifications will
>>> be and where people can send patches against, preventing last minutes
>>> conflicts and perhaps Lenb will agree to pull from this tree. In the
>>> meantime, the tree will be part of the linux-next, the patches will be
>>> more widely tested and could be fixed earlier.
>>
>> My coupled cpuidle patches were acked and temporarily in Len's
>> next/Linus pull branch, but were later dropped when the first pull
>> request to Linus was rejected. I asked Len to either put the coupled
>> cpuidle patches into his next branch, or let me host them so people
>> could base SoC branches off of them and let Len pull them later, but
>> got no response. If you do start a cpuidle for-next branch, can you
>> pull my coupled-cpuidle branch:
>>
>> The following changes since commit 76e10d158efb6d4516018846f60c2ab5501900bc:
>>
>> Linux 3.4 (2012-05-20 15:29:13 -0700)
>>
>> are available in the git repository at:
>> https://android.googlesource.com/kernel/common.git coupled-cpuidle
>>
>> Colin Cross (4):
>> cpuidle: refactor out cpuidle_enter_state
>> cpuidle: fix error handling in __cpuidle_register_device
>> cpuidle: add support for states that affect multiple cpus
>> cpuidle: coupled: add parallel barrier function
>>
>> drivers/cpuidle/Kconfig | 3 +
>> drivers/cpuidle/Makefile | 1 +
>> drivers/cpuidle/coupled.c | 715 +++++++++++++++++++++++++++++++++++++++++++++
>> drivers/cpuidle/cpuidle.c | 68 ++++-
>> drivers/cpuidle/cpuidle.h | 32 ++
>> include/linux/cpuidle.h | 11 +
>> 6 files changed, 813 insertions(+), 17 deletions(-)
>> create mode 100644 drivers/cpuidle/coupled.c
>
>
> Done.
>
> http://git.linaro.org/gitweb?p=people/dlezcano/cpuidle-next.git;a=shortlog;h=refs/heads/cpuidle-next
Great!
Daniel, thanks for tracking this. Are you planning to submit a pull
request to Rafael so we finally can get this into linux-next and merged
for v3.6?
Looks like there will be a slight problem to sort out though. Len's
'next' branch[1] is already included in linux-next and as some version
of the coupled CPUidle already merged.
I hope we can sort this out in time for v3.6 because this series has
been well reviewed, well tested and ready for merge since before the
v3.5 merge window.
Kevin
[1] git://git.kernel.org/pub/scm/linux/kernel/git/lenb/linux next
^ permalink raw reply
* Re: use of pm_runtime_disable() from driver probe?
From: Alan Stern @ 2012-07-11 14:20 UTC (permalink / raw)
To: Rafael J. Wysocki; +Cc: Kevin Hilman, linux-pm
In-Reply-To: <201207110031.06662.rjw@sisk.pl>
On Wed, 11 Jul 2012, Rafael J. Wysocki wrote:
> > This opens up the possibility of calling probe while a runtime resume
> > or suspend is in progress. (On the other hand, the existing code
> > doesn't prevent a concurrent runtime resume.) Maybe it would be best
> > to leave the pm_runtime_barrier().
>
> That wouldn't close the race, though, because the suspend/resume may still
> be started right after _barrier has returned.
True, but see below.
> The race is only possible if runtime PM is enabled by a subsystem or PM domain
> code before the first eligible driver is registered and if that code is not
> careful enough to get ready for driver registration. I'm not sure how likely
> it is to happen in practice.
It's not just the first eligible driver. Drivers can be bound and
unbound dynamically, and suspends/resume operations can sit on the wait
queue or wait until a timer expires. We don't want an old request
suddenly to take effect in the middle of a probe.
The barrier will get rid of any old requests. New ones would have to
be added after the probe starts, which as you say, is unlikely.
Alan Stern
^ permalink raw reply
* Re: use of pm_runtime_disable() from driver probe?
From: Rafael J. Wysocki @ 2012-07-11 17:36 UTC (permalink / raw)
To: Alan Stern, Kevin Hilman; +Cc: linux-pm
In-Reply-To: <Pine.LNX.4.44L0.1207111017010.1559-100000@iolanthe.rowland.org>
On Wednesday, July 11, 2012, Alan Stern wrote:
> On Wed, 11 Jul 2012, Rafael J. Wysocki wrote:
>
> > > This opens up the possibility of calling probe while a runtime resume
> > > or suspend is in progress. (On the other hand, the existing code
> > > doesn't prevent a concurrent runtime resume.) Maybe it would be best
> > > to leave the pm_runtime_barrier().
> >
> > That wouldn't close the race, though, because the suspend/resume may still
> > be started right after _barrier has returned.
>
> True, but see below.
>
> > The race is only possible if runtime PM is enabled by a subsystem or PM domain
> > code before the first eligible driver is registered and if that code is not
> > careful enough to get ready for driver registration. I'm not sure how likely
> > it is to happen in practice.
>
> It's not just the first eligible driver. Drivers can be bound and
> unbound dynamically, and suspends/resume operations can sit on the wait
> queue or wait until a timer expires. We don't want an old request
> suddenly to take effect in the middle of a probe.
>
> The barrier will get rid of any old requests. New ones would have to
> be added after the probe starts, which as you say, is unlikely.
OK, I'll keep the barrier, then.
Kevin, can you please double check the patch below?
Rafael
---
drivers/base/dd.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
Index: linux/drivers/base/dd.c
===================================================================
--- linux.orig/drivers/base/dd.c
+++ linux/drivers/base/dd.c
@@ -356,10 +356,9 @@ int driver_probe_device(struct device_dr
pr_debug("bus: '%s': %s: matched device %s with driver %s\n",
drv->bus->name, __func__, dev_name(dev), drv->name);
- pm_runtime_get_noresume(dev);
pm_runtime_barrier(dev);
ret = really_probe(dev, drv);
- pm_runtime_put_sync(dev);
+ pm_runtime_idle(dev);
return ret;
}
@@ -406,9 +405,8 @@ int device_attach(struct device *dev)
ret = 0;
}
} else {
- pm_runtime_get_noresume(dev);
ret = bus_for_each_drv(dev->bus, NULL, dev, __device_attach);
- pm_runtime_put_sync(dev);
+ pm_runtime_idle(dev);
}
out_unlock:
device_unlock(dev);
^ permalink raw reply
* Re: use of pm_runtime_disable() from driver probe?
From: Kevin Hilman @ 2012-07-11 23:07 UTC (permalink / raw)
To: Rafael J. Wysocki; +Cc: linux-pm
In-Reply-To: <201207111936.45639.rjw@sisk.pl>
"Rafael J. Wysocki" <rjw@sisk.pl> writes:
> On Wednesday, July 11, 2012, Alan Stern wrote:
>> On Wed, 11 Jul 2012, Rafael J. Wysocki wrote:
>>
>> > > This opens up the possibility of calling probe while a runtime resume
>> > > or suspend is in progress. (On the other hand, the existing code
>> > > doesn't prevent a concurrent runtime resume.) Maybe it would be best
>> > > to leave the pm_runtime_barrier().
>> >
>> > That wouldn't close the race, though, because the suspend/resume may still
>> > be started right after _barrier has returned.
>>
>> True, but see below.
>>
>> > The race is only possible if runtime PM is enabled by a subsystem or PM domain
>> > code before the first eligible driver is registered and if that code is not
>> > careful enough to get ready for driver registration. I'm not sure how likely
>> > it is to happen in practice.
>>
>> It's not just the first eligible driver. Drivers can be bound and
>> unbound dynamically, and suspends/resume operations can sit on the wait
>> queue or wait until a timer expires. We don't want an old request
>> suddenly to take effect in the middle of a probe.
>>
>> The barrier will get rid of any old requests. New ones would have to
>> be added after the probe starts, which as you say, is unlikely.
>
> OK, I'll keep the barrier, then.
>
> Kevin, can you please double check the patch below?
Yup, this verion looks right and I tested it and verified that it still
fixes the problem I'm seeing.
Reviewed-by: Kevin Hilman <khilman@ti.com>
Tested-by: Kevin Hilman <khilman@ti.com>
Thanks!
Kevin
> Rafael
>
> ---
> drivers/base/dd.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> Index: linux/drivers/base/dd.c
> ===================================================================
> --- linux.orig/drivers/base/dd.c
> +++ linux/drivers/base/dd.c
> @@ -356,10 +356,9 @@ int driver_probe_device(struct device_dr
> pr_debug("bus: '%s': %s: matched device %s with driver %s\n",
> drv->bus->name, __func__, dev_name(dev), drv->name);
>
> - pm_runtime_get_noresume(dev);
> pm_runtime_barrier(dev);
> ret = really_probe(dev, drv);
> - pm_runtime_put_sync(dev);
> + pm_runtime_idle(dev);
>
> return ret;
> }
> @@ -406,9 +405,8 @@ int device_attach(struct device *dev)
> ret = 0;
> }
> } else {
> - pm_runtime_get_noresume(dev);
> ret = bus_for_each_drv(dev->bus, NULL, dev, __device_attach);
> - pm_runtime_put_sync(dev);
> + pm_runtime_idle(dev);
> }
> out_unlock:
> device_unlock(dev);
^ permalink raw reply
* Re: use of pm_runtime_disable() from driver probe?
From: Rafael J. Wysocki @ 2012-07-11 23:16 UTC (permalink / raw)
To: Kevin Hilman; +Cc: linux-pm
In-Reply-To: <87ipdthp6b.fsf@ti.com>
On Thursday, July 12, 2012, Kevin Hilman wrote:
> "Rafael J. Wysocki" <rjw@sisk.pl> writes:
>
> > On Wednesday, July 11, 2012, Alan Stern wrote:
> >> On Wed, 11 Jul 2012, Rafael J. Wysocki wrote:
> >>
> >> > > This opens up the possibility of calling probe while a runtime resume
> >> > > or suspend is in progress. (On the other hand, the existing code
> >> > > doesn't prevent a concurrent runtime resume.) Maybe it would be best
> >> > > to leave the pm_runtime_barrier().
> >> >
> >> > That wouldn't close the race, though, because the suspend/resume may still
> >> > be started right after _barrier has returned.
> >>
> >> True, but see below.
> >>
> >> > The race is only possible if runtime PM is enabled by a subsystem or PM domain
> >> > code before the first eligible driver is registered and if that code is not
> >> > careful enough to get ready for driver registration. I'm not sure how likely
> >> > it is to happen in practice.
> >>
> >> It's not just the first eligible driver. Drivers can be bound and
> >> unbound dynamically, and suspends/resume operations can sit on the wait
> >> queue or wait until a timer expires. We don't want an old request
> >> suddenly to take effect in the middle of a probe.
> >>
> >> The barrier will get rid of any old requests. New ones would have to
> >> be added after the probe starts, which as you say, is unlikely.
> >
> > OK, I'll keep the barrier, then.
> >
> > Kevin, can you please double check the patch below?
>
> Yup, this verion looks right and I tested it and verified that it still
> fixes the problem I'm seeing.
>
> Reviewed-by: Kevin Hilman <khilman@ti.com>
> Tested-by: Kevin Hilman <khilman@ti.com>
Cool, thanks!
I'll submit it officially (with a changelog and tags) tomorrow.
Rafael
> > ---
> > drivers/base/dd.c | 6 ++----
> > 1 file changed, 2 insertions(+), 4 deletions(-)
> >
> > Index: linux/drivers/base/dd.c
> > ===================================================================
> > --- linux.orig/drivers/base/dd.c
> > +++ linux/drivers/base/dd.c
> > @@ -356,10 +356,9 @@ int driver_probe_device(struct device_dr
> > pr_debug("bus: '%s': %s: matched device %s with driver %s\n",
> > drv->bus->name, __func__, dev_name(dev), drv->name);
> >
> > - pm_runtime_get_noresume(dev);
> > pm_runtime_barrier(dev);
> > ret = really_probe(dev, drv);
> > - pm_runtime_put_sync(dev);
> > + pm_runtime_idle(dev);
> >
> > return ret;
> > }
> > @@ -406,9 +405,8 @@ int device_attach(struct device *dev)
> > ret = 0;
> > }
> > } else {
> > - pm_runtime_get_noresume(dev);
> > ret = bus_for_each_drv(dev->bus, NULL, dev, __device_attach);
> > - pm_runtime_put_sync(dev);
> > + pm_runtime_idle(dev);
> > }
> > out_unlock:
> > device_unlock(dev);
>
>
^ permalink raw reply
* Re: [PATCH v4 1/5] thermal: Add generic cpufreq cooling implementation
From: amit kachhap @ 2012-07-12 3:20 UTC (permalink / raw)
To: Hongbo Zhang
Cc: linux-samsung-soc, akpm, patches, linux-kernel, lm-sensors,
linux-acpi, khali, linaro-dev, guenter.roeck, linux-pm
In-Reply-To: <CAJLyvQwgpwd3MRJcme1x-9J=Pdr1NTBDT_d2EKcFEjTNs2U7eg@mail.gmail.com>
On Tue, Jul 10, 2012 at 12:31 PM, Hongbo Zhang <hongbo.zhang@linaro.org> wrote:
>
>
> On 12 May 2012 17:40, Amit Daniel Kachhap <amit.kachhap@linaro.org> wrote:
>>
>> This patch adds support for generic cpu thermal cooling low level
>> implementations using frequency scaling up/down based on the registration
>> parameters. Different cpu related cooling devices can be registered by the
>> user and the binding of these cooling devices to the corresponding
>> trip points can be easily done as the registration APIs return the
>> cooling device pointer. The user of these APIs are responsible for
>> passing clipping frequency . The drivers can also register to recieve
>> notification about any cooling action called.
>>
>> Signed-off-by: Amit Daniel Kachhap <amit.kachhap@linaro.org>
>> ---
>> Documentation/thermal/cpu-cooling-api.txt | 60 ++++
>> drivers/thermal/Kconfig | 11 +
>> drivers/thermal/Makefile | 3 +-
>> drivers/thermal/cpu_cooling.c | 483
>> +++++++++++++++++++++++++++++
>> include/linux/cpu_cooling.h | 99 ++++++
>> 5 files changed, 655 insertions(+), 1 deletions(-)
>> create mode 100644 Documentation/thermal/cpu-cooling-api.txt
>> create mode 100644 drivers/thermal/cpu_cooling.c
>> create mode 100644 include/linux/cpu_cooling.h
>>
>> diff --git a/Documentation/thermal/cpu-cooling-api.txt
>> b/Documentation/thermal/cpu-cooling-api.txt
>> new file mode 100644
>> index 0000000..557adb8
>> --- /dev/null
>> +++ b/Documentation/thermal/cpu-cooling-api.txt
>> @@ -0,0 +1,60 @@
>> +CPU cooling APIs How To
>> +===================================
>> +
>> +Written by Amit Daniel Kachhap <amit.kachhap@linaro.org>
>> +
>> +Updated: 12 May 2012
>> +
>> +Copyright (c) 2012 Samsung Electronics Co., Ltd(http://www.samsung.com)
>> +
>> +0. Introduction
>> +
>> +The generic cpu cooling(freq clipping, cpuhotplug etc) provides
>> +registration/unregistration APIs to the caller. The binding of the
>> cooling
>> +devices to the trip point is left for the user. The registration APIs
>> returns
>> +the cooling device pointer.
>> +
>> +1. cpu cooling APIs
>> +
>> +1.1 cpufreq registration/unregistration APIs
>> +1.1.1 struct thermal_cooling_device *cpufreq_cooling_register(
>> + struct freq_clip_table *tab_ptr, unsigned int tab_size)
>> +
>> + This interface function registers the cpufreq cooling device with the
>> name
>> + "thermal-cpufreq-%x". This api can support multiple instances of
>> cpufreq
>> + cooling devices.
>> +
>> + tab_ptr: The table containing the maximum value of frequency to be
>> clipped
>> + for each cooling state.
>> + .freq_clip_max: Value of frequency to be clipped for each allowed
>> + cpus.
>> + .temp_level: Temperature level at which the frequency clamping
>> will
>> + happen.
>> + .mask_val: cpumask of the allowed cpu's
>> + tab_size: the total number of cpufreq cooling states.
>> +
>> +1.1.2 void cpufreq_cooling_unregister(struct thermal_cooling_device
>> *cdev)
>> +
>> + This interface function unregisters the "thermal-cpufreq-%x" cooling
>> device.
>> +
>> + cdev: Cooling device pointer which has to be unregistered.
>> +
>> +
>> +1.2 CPU cooling action notifier register/unregister interface
>> +1.2.1 int cputherm_register_notifier(struct notifier_block *nb,
>> + unsigned int list)
>> +
>> + This interface registers a driver with cpu cooling layer. The driver
>> will
>> + be notified when any cpu cooling action is called.
>> +
>> + nb: notifier function to register
>> + list: CPUFREQ_COOLING_START or CPUFREQ_COOLING_STOP
>> +
>> +1.2.2 int cputherm_unregister_notifier(struct notifier_block *nb,
>> + unsigned int list)
>> +
>> + This interface registers a driver with cpu cooling layer. The driver
>> will
>> + be notified when any cpu cooling action is called.
>> +
>> + nb: notifier function to register
>> + list: CPUFREQ_COOLING_START or CPUFREQ_COOLING_STOP
>> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
>> index 514a691..d9c529f 100644
>> --- a/drivers/thermal/Kconfig
>> +++ b/drivers/thermal/Kconfig
>> @@ -19,6 +19,17 @@ config THERMAL_HWMON
>> depends on HWMON=y || HWMON=THERMAL
>> default y
>>
>> +config CPU_THERMAL
>> + bool "generic cpu cooling support"
>> + depends on THERMAL && CPU_FREQ
>> + help
>> + This implements the generic cpu cooling mechanism through
>> frequency
>> + reduction, cpu hotplug and any other ways of reducing
>> temperature. An
>> + ACPI version of this already
>> exists(drivers/acpi/processor_thermal.c).
>> + This will be useful for platforms using the generic thermal
>> interface
>> + and not the ACPI interface.
>> + If you want this support, you should say Y or M here.
>> +
>> config SPEAR_THERMAL
>> bool "SPEAr thermal sensor driver"
>> depends on THERMAL
>> diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
>> index a9fff0b..30c456c 100644
>> --- a/drivers/thermal/Makefile
>> +++ b/drivers/thermal/Makefile
>> @@ -3,4 +3,5 @@
>> #
>>
>> obj-$(CONFIG_THERMAL) += thermal_sys.o
>> -obj-$(CONFIG_SPEAR_THERMAL) += spear_thermal.o
>> \ No newline at end of file
>> +obj-$(CONFIG_CPU_THERMAL) += cpu_cooling.o
>> +obj-$(CONFIG_SPEAR_THERMAL) += spear_thermal.o
>> diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c
>> new file mode 100644
>> index 0000000..c40d9a0
>> --- /dev/null
>> +++ b/drivers/thermal/cpu_cooling.c
>> @@ -0,0 +1,483 @@
>> +/*
>> + * linux/drivers/thermal/cpu_cooling.c
>> + *
>> + * Copyright (C) 2012 Samsung Electronics Co.,
>> Ltd(http://www.samsung.com)
>> + * Copyright (C) 2012 Amit Daniel <amit.kachhap@linaro.org>
>> + *
>> + *
>> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; version 2 of the License.
>> + *
>> + * This program is distributed in the hope that it will be useful, but
>> + * WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
>> + * General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> along
>> + * with this program; if not, write to the Free Software Foundation,
>> Inc.,
>> + * 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA.
>> + *
>> + *
>> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> + */
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/thermal.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/cpufreq.h>
>> +#include <linux/err.h>
>> +#include <linux/slab.h>
>> +#include <linux/cpu.h>
>> +#include <linux/cpu_cooling.h>
>> +
>> +/**
>> + * struct cpufreq_cooling_device
>> + * @id: unique integer value corresponding to each cpufreq_cooling_device
>> + * registered.
>> + * @cool_dev: thermal_cooling_device pointer to keep track of the the
>> + * egistered cooling device.
>> + * @tab_ptr: freq_clip_table table containing the maximum value of
>> frequency to
>> + * be set for different cooling state.
>> + * @tab_size: integer value representing a count of the above table.
>> + * @cpufreq_state: integer value representing the current state of
>> cpufreq
>> + * cooling devices.
>> + * @allowed_cpus: all the cpus involved for this cpufreq_cooling_device.
>> + * @node: list_head to link all cpufreq_cooling_device together.
>> + *
>> + * This structure is required for keeping information of each
>> + * cpufreq_cooling_device registered as a list whose head is represented
>> by
>> + * cooling_cpufreq_list. In order to prevent corruption of this list a
>> + * mutex lock cooling_cpufreq_lock is used.
>> + */
>> +struct cpufreq_cooling_device {
>> + int id;
>> + struct thermal_cooling_device *cool_dev;
>> + struct freq_clip_table *tab_ptr;
>> + unsigned int tab_size;
>> + unsigned int cpufreq_state;
>> + struct cpumask allowed_cpus;
>> + struct list_head node;
>> +};
>> +static LIST_HEAD(cooling_cpufreq_list);
>> +static DEFINE_MUTEX(cooling_cpufreq_lock);
>> +static DEFINE_IDR(cpufreq_idr);
>> +
>> +/*per cpu variable to store the previous max frequency allowed*/
>> +static DEFINE_PER_CPU(unsigned int, max_policy_freq);
>> +
>> +/*notify_table passes value to the CPUFREQ_ADJUST callback function.*/
>> +#define NOTIFY_INVALID NULL
>> +static struct freq_clip_table *notify_table;
>> +
>> +/*Head of the blocking notifier chain to inform about frequency
>> clamping*/
>> +static BLOCKING_NOTIFIER_HEAD(cputherm_state_notifier_list);
>> +
>> +/**
>> + * get_idr - function to get a unique id.
>> + * @idr: struct idr * handle used to create a id.
>> + * @id: int * value generated by this function.
>> + */
>> +static int get_idr(struct idr *idr, int *id)
>> +{
>> + int err;
>> +again:
>> + if (unlikely(idr_pre_get(idr, GFP_KERNEL) == 0))
>> + return -ENOMEM;
>> +
>> + mutex_lock(&cooling_cpufreq_lock);
>> + err = idr_get_new(idr, NULL, id);
>> + mutex_unlock(&cooling_cpufreq_lock);
>> +
>> + if (unlikely(err == -EAGAIN))
>> + goto again;
>> + else if (unlikely(err))
>> + return err;
>> +
>> + *id = *id & MAX_ID_MASK;
>> + return 0;
>> +}
>> +
>> +/**
>> + * release_idr - function to free the unique id.
>> + * @idr: struct idr * handle used for creating the id.
>> + * @id: int value representing the unique id.
>> + */
>> +static void release_idr(struct idr *idr, int id)
>> +{
>> + mutex_lock(&cooling_cpufreq_lock);
>> + idr_remove(idr, id);
>> + mutex_unlock(&cooling_cpufreq_lock);
>> +}
>> +
>> +/**
>> + * cputherm_register_notifier - Register a notifier with cpu cooling
>> interface.
>> + * @nb: struct notifier_block * with callback info.
>> + * @list: integer value for which notification is needed. possible values
>> are
>> + * CPUFREQ_COOLING_START and CPUFREQ_COOLING_STOP.
>> + *
>> + * This exported function registers a driver with cpu cooling layer. The
>> driver
>> + * will be notified when any cpu cooling action is called.
>> + */
>> +int cputherm_register_notifier(struct notifier_block *nb, unsigned int
>> list)
>> +{
>> + int ret = 0;
>> +
>> + switch (list) {
>> + case CPUFREQ_COOLING_START:
>> + case CPUFREQ_COOLING_STOP:
>> + ret = blocking_notifier_chain_register(
>> + &cputherm_state_notifier_list, nb);
>> + break;
>> + default:
>> + ret = -EINVAL;
>> + }
>> + return ret;
>> +}
>> +EXPORT_SYMBOL(cputherm_register_notifier);
>> +
>> +/**
>> + * cputherm_unregister_notifier - Un-register a notifier.
>> + * @nb: struct notifier_block * with callback info.
>> + * @list: integer value for which notification is needed. values possible
>> are
>> + * CPUFREQ_COOLING_START or CPUFREQ_COOLING_STOP.
>> + *
>> + * This exported function un-registers a driver with cpu cooling layer.
>> + */
>> +int cputherm_unregister_notifier(struct notifier_block *nb, unsigned int
>> list)
>> +{
>> + int ret = 0;
>> +
>> + switch (list) {
>> + case CPUFREQ_COOLING_START:
>> + case CPUFREQ_COOLING_STOP:
>> + ret = blocking_notifier_chain_unregister(
>> + &cputherm_state_notifier_list, nb);
>> + break;
>> + default:
>> + ret = -EINVAL;
>> + }
>> + return ret;
>> +}
>> +EXPORT_SYMBOL(cputherm_unregister_notifier);
>> +
>> +/*Below codes defines functions to be used for cpufreq as cooling
>> device*/
>> +
>> +/**
>> + * is_cpufreq_valid - function to check if a cpu has frequency transition
>> policy.
>> + * @cpu: cpu for which check is needed.
>> + */
>> +static int is_cpufreq_valid(int cpu)
>> +{
>> + struct cpufreq_policy policy;
>> + return !cpufreq_get_policy(&policy, cpu);
>> +}
>> +
>> +/**
>> + * cpufreq_apply_cooling - function to apply frequency clipping.
>> + * @cpufreq_device: cpufreq_cooling_device pointer containing frequency
>> + * clipping data.
>> + * @cooling_state: value of the cooling state.
>> + */
>> +static int cpufreq_apply_cooling(struct cpufreq_cooling_device
>> *cpufreq_device,
>> + unsigned long cooling_state)
>> +{
>> + unsigned int event, cpuid, state;
>> + struct freq_clip_table *th_table, *table_ptr;
>> + const struct cpumask *maskPtr = &cpufreq_device->allowed_cpus;
>> + struct cpufreq_cooling_device *cpufreq_ptr;
>> +
>> + if (cooling_state > cpufreq_device->tab_size)
>> + return -EINVAL;
>> +
>> + /*Check if the old cooling action is same as new cooling action*/
>> + if (cpufreq_device->cpufreq_state == cooling_state)
>> + return 0;
>> +
>> + /*pass cooling table info to the cpufreq_thermal_notifier
>> callback*/
>> + notify_table = NOTIFY_INVALID;
>> +
>> + if (cooling_state > 0) {
>> + th_table = &(cpufreq_device->tab_ptr[cooling_state - 1]);
>> + notify_table = th_table;
>> + }
>> +
>> + /*check if any lower clip frequency active in other
>> cpufreq_device's*/
>> + list_for_each_entry(cpufreq_ptr, &cooling_cpufreq_list, node) {
>> +
>> + state = cpufreq_ptr->cpufreq_state;
>> + if (state == 0 || cpufreq_ptr == cpufreq_device)
>> + continue;
>> +
>> + if (!cpumask_equal(&cpufreq_ptr->allowed_cpus,
>> + &cpufreq_device->allowed_cpus))
>> + continue;
>> +
>> + table_ptr = &(cpufreq_ptr->tab_ptr[state - 1]);
>> + if (notify_table == NULL ||
>> + (table_ptr->freq_clip_max <
>> + notify_table->freq_clip_max))
>> + notify_table = table_ptr;
>> + }
>> +
>> + cpufreq_device->cpufreq_state = cooling_state;
>> +
>> + if (notify_table != NOTIFY_INVALID) {
>> + event = CPUFREQ_COOLING_START;
>> + maskPtr = notify_table->mask_val;
>> + } else {
>> + event = CPUFREQ_COOLING_STOP;
>> + }
>> +
>> + blocking_notifier_call_chain(&cputherm_state_notifier_list,
>> + event, notify_table);
>> +
>> + for_each_cpu(cpuid, maskPtr) {
>> + if (is_cpufreq_valid(cpuid))
>> + cpufreq_update_policy(cpuid);
>> + }
>> +
>> + notify_table = NOTIFY_INVALID;
>> +
>> + return 0;
>> +}
>> +
>> +/**
>> + * cpufreq_thermal_notifier - notifier callback for cpufreq policy
>> change.
>> + * @nb: struct notifier_block * with callback info.
>> + * @event: value showing cpufreq event for which this function invoked.
>> + * @data: callback-specific data
>> + */
>> +static int cpufreq_thermal_notifier(struct notifier_block *nb,
>> + unsigned long event, void *data)
>> +{
>> + struct cpufreq_policy *policy = data;
>> + unsigned long max_freq = 0;
>> +
>> + if (event != CPUFREQ_ADJUST)
>> + return 0;
>> +
>> + if (notify_table != NOTIFY_INVALID) {
>> + max_freq = notify_table->freq_clip_max;
>> +
>> + if (!per_cpu(max_policy_freq, policy->cpu))
>> + per_cpu(max_policy_freq, policy->cpu) =
>> policy->max;
>> + } else {
>> + if (per_cpu(max_policy_freq, policy->cpu)) {
>> + max_freq = per_cpu(max_policy_freq, policy->cpu);
>> + per_cpu(max_policy_freq, policy->cpu) = 0;
>> + } else {
>> + max_freq = policy->max;
>> + }
>> + }
>> +
>> + /* Never exceed user_policy.max*/
>> + if (max_freq > policy->user_policy.max)
>> + max_freq = policy->user_policy.max;
>> +
>> + if (policy->max != max_freq)
>> + cpufreq_verify_within_limits(policy, 0, max_freq);
>> +
>> + return 0;
>> +}
>> +
>> +/*
>> + * cpufreq cooling device callback functions are defined below
>> + */
>> +
>> +/**
>> + * cpufreq_get_max_state - callback function to get the max cooling
>> state.
>> + * @cdev: thermal cooling device pointer.
>> + * @state: fill this variable with the max cooling state.
>> + */
>> +static int cpufreq_get_max_state(struct thermal_cooling_device *cdev,
>> + unsigned long *state)
>> +{
>> + int ret = -EINVAL;
>> + struct cpufreq_cooling_device *cpufreq_device;
>> +
>> + mutex_lock(&cooling_cpufreq_lock);
>> + list_for_each_entry(cpufreq_device, &cooling_cpufreq_list, node) {
>> + if (cpufreq_device && cpufreq_device->cool_dev == cdev) {
>> + *state = cpufreq_device->tab_size;
>> + ret = 0;
>> + break;
>> + }
>> + }
>> + mutex_unlock(&cooling_cpufreq_lock);
>> + return ret;
>> +}
>> +
>> +/**
>> + * cpufreq_get_cur_state - callback function to get the current cooling
>> state.
>> + * @cdev: thermal cooling device pointer.
>> + * @state: fill this variable with the current cooling state.
>> + */
>> +static int cpufreq_get_cur_state(struct thermal_cooling_device *cdev,
>> + unsigned long *state)
>> +{
>> + int ret = -EINVAL;
>> + struct cpufreq_cooling_device *cpufreq_device;
>> +
>> + mutex_lock(&cooling_cpufreq_lock);
>> + list_for_each_entry(cpufreq_device, &cooling_cpufreq_list, node) {
>> + if (cpufreq_device && cpufreq_device->cool_dev == cdev) {
>> + *state = cpufreq_device->cpufreq_state;
>> + ret = 0;
>> + break;
>> + }
>> + }
>> + mutex_unlock(&cooling_cpufreq_lock);
>> + return ret;
>> +}
>> +
>> +/**
>> + * cpufreq_set_cur_state - callback function to set the current cooling
>> state.
>> + * @cdev: thermal cooling device pointer.
>> + * @state: set this variable to the current cooling state.
>> + */
>> +static int cpufreq_set_cur_state(struct thermal_cooling_device *cdev,
>> + unsigned long state)
>> +{
>> + int ret = -EINVAL;
>> + struct cpufreq_cooling_device *cpufreq_device;
>> +
>> + mutex_lock(&cooling_cpufreq_lock);
>> + list_for_each_entry(cpufreq_device, &cooling_cpufreq_list, node) {
>> + if (cpufreq_device && cpufreq_device->cool_dev == cdev) {
>> + ret = 0;
>> + break;
>> + }
>> + }
>> + if (!ret)
>> + ret = cpufreq_apply_cooling(cpufreq_device, state);
>> +
>> + mutex_unlock(&cooling_cpufreq_lock);
>> +
>> + return ret;
>> +}
>> +
>> +/*Bind cpufreq callbacks to thermal cooling device ops*/
>> +static struct thermal_cooling_device_ops const cpufreq_cooling_ops = {
>> + .get_max_state = cpufreq_get_max_state,
>> + .get_cur_state = cpufreq_get_cur_state,
>> + .set_cur_state = cpufreq_set_cur_state,
>> +};
>> +
>> +/*Notifier for cpufreq policy change*/
>> +static struct notifier_block thermal_cpufreq_notifier_block = {
>> + .notifier_call = cpufreq_thermal_notifier,
>> +};
>> +
>> +/**
>> + * cpufreq_cooling_register - function to create cpufreq cooling device.
>> + * @tab_ptr: table ptr containing the maximum value of frequency to be
>> clipped
>> + * for each cooling state.
>> + * @tab_size: count of entries in the above table.
>> + * happen.
>> + */
>> +struct thermal_cooling_device *cpufreq_cooling_register(
>> + struct freq_clip_table *tab_ptr, unsigned int tab_size)
>> +{
>> + struct thermal_cooling_device *cool_dev;
>> + struct cpufreq_cooling_device *cpufreq_dev = NULL;
>> + struct freq_clip_table *clip_tab;
>> + unsigned int cpufreq_dev_count = 0;
>> + char dev_name[THERMAL_NAME_LENGTH];
>> + int ret = 0, id = 0, i;
>> +
>> + if (tab_ptr == NULL || tab_size == 0)
>> + return ERR_PTR(-EINVAL);
>> +
>> + list_for_each_entry(cpufreq_dev, &cooling_cpufreq_list, node)
>> + cpufreq_dev_count++;
>> +
>> + cpufreq_dev = kzalloc(sizeof(struct cpufreq_cooling_device),
>> + GFP_KERNEL);
>> + if (!cpufreq_dev)
>> + return ERR_PTR(-ENOMEM);
>> +
>> + /*Verify that all the entries of freq_clip_table are present*/
>> + for (i = 0; i < tab_size; i++) {
>> + clip_tab = ((struct freq_clip_table *)&tab_ptr[i]);
>> + if (!clip_tab->freq_clip_max || !clip_tab->mask_val
>> + || !clip_tab->temp_level) {
>
> About clip_tab->temp_level:
> I'd like to remove this variable checking, because as suggested by Vincent
> Guittot, it is better to separate cooling devices
> and thermal zone, thus we can add/remove cooling devices freely/dynamically.
> In such case, a cooling device doesn't
> know any temp_level before binding--the binding condition is name matching,
> what's more, the cooling device should not
> know much data of other devices.
Hi Hongbo,
The temp_level is not used anywhere inside the cooling devices. They
are just present to pass the trip temperature information to the
registered notifiers.
Thanks,
Amit D
>
>> + kfree(cpufreq_dev);
>> + return ERR_PTR(-EINVAL);
>> + }
>> + /*
>> + *Consolidate all the cpumask for all the individual
>> entries
>> + *of the trip table. This is useful in resetting all the
>> + *clipped frequencies to the normal level for each cpufreq
>> + *cooling device.
>> + */
>> + cpumask_or(&cpufreq_dev->allowed_cpus,
>> + &cpufreq_dev->allowed_cpus, clip_tab->mask_val);
>> + }
>> +
>> + cpufreq_dev->tab_ptr = tab_ptr;
>> + cpufreq_dev->tab_size = tab_size;
>> +
>> + ret = get_idr(&cpufreq_idr, &cpufreq_dev->id);
>> + if (ret) {
>> + kfree(cpufreq_dev);
>> + return ERR_PTR(-EINVAL);
>> + }
>> +
>> + sprintf(dev_name, "thermal-cpufreq-%d", cpufreq_dev->id);
>> +
>> + cool_dev = thermal_cooling_device_register(dev_name, cpufreq_dev,
>> + &cpufreq_cooling_ops);
>> + if (!cool_dev) {
>> + release_idr(&cpufreq_idr, cpufreq_dev->id);
>> + kfree(cpufreq_dev);
>> + return ERR_PTR(-EINVAL);
>> + }
>> + cpufreq_dev->id = id;
>> + cpufreq_dev->cool_dev = cool_dev;
>> + mutex_lock(&cooling_cpufreq_lock);
>> + list_add_tail(&cpufreq_dev->node, &cooling_cpufreq_list);
>> +
>> + /*Register the notifier for first cpufreq cooling device*/
>> + if (cpufreq_dev_count == 0)
>> + cpufreq_register_notifier(&thermal_cpufreq_notifier_block,
>> + CPUFREQ_POLICY_NOTIFIER);
>> +
>> + mutex_unlock(&cooling_cpufreq_lock);
>> + return cool_dev;
>> +}
>> +EXPORT_SYMBOL(cpufreq_cooling_register);
>> +
>> +/**
>> + * cpufreq_cooling_unregister - function to remove cpufreq cooling
>> device.
>> + * @cdev: thermal cooling device pointer.
>> + */
>> +void cpufreq_cooling_unregister(struct thermal_cooling_device *cdev)
>> +{
>> + struct cpufreq_cooling_device *cpufreq_dev = NULL;
>> + unsigned int cpufreq_dev_count = 0;
>> +
>> + mutex_lock(&cooling_cpufreq_lock);
>> + list_for_each_entry(cpufreq_dev, &cooling_cpufreq_list, node) {
>> + if (cpufreq_dev && cpufreq_dev->cool_dev == cdev)
>> + break;
>> + cpufreq_dev_count++;
>> + }
>> +
>> + if (!cpufreq_dev || cpufreq_dev->cool_dev != cdev) {
>> + mutex_unlock(&cooling_cpufreq_lock);
>> + return;
>> + }
>> +
>> + list_del(&cpufreq_dev->node);
>> +
>> + /*Unregister the notifier for the last cpufreq cooling device*/
>> + if (cpufreq_dev_count == 1) {
>> +
>> cpufreq_unregister_notifier(&thermal_cpufreq_notifier_block,
>> + CPUFREQ_POLICY_NOTIFIER);
>> + }
>> + mutex_unlock(&cooling_cpufreq_lock);
>> + thermal_cooling_device_unregister(cpufreq_dev->cool_dev);
>> + release_idr(&cpufreq_idr, cpufreq_dev->id);
>> + kfree(cpufreq_dev);
>> +}
>> +EXPORT_SYMBOL(cpufreq_cooling_unregister);
>> diff --git a/include/linux/cpu_cooling.h b/include/linux/cpu_cooling.h
>> new file mode 100644
>> index 0000000..ed6c096
>> --- /dev/null
>> +++ b/include/linux/cpu_cooling.h
>> @@ -0,0 +1,99 @@
>> +/*
>> + * linux/include/linux/cpu_cooling.h
>> + *
>> + * Copyright (C) 2012 Samsung Electronics Co.,
>> Ltd(http://www.samsung.com)
>> + * Copyright (C) 2012 Amit Daniel <amit.kachhap@linaro.org>
>> + *
>> + *
>> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; version 2 of the License.
>> + *
>> + * This program is distributed in the hope that it will be useful, but
>> + * WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
>> + * General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> along
>> + * with this program; if not, write to the Free Software Foundation,
>> Inc.,
>> + * 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA.
>> + *
>> + *
>> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> + */
>> +
>> +#ifndef __CPU_COOLING_H__
>> +#define __CPU_COOLING_H__
>> +
>> +#include <linux/thermal.h>
>> +
>> +#define CPUFREQ_COOLING_START 0
>> +#define CPUFREQ_COOLING_STOP 1
>> +
>> +/**
>> + * struct freq_clip_table
>> + * @freq_clip_max: maximum frequency allowed for this cooling state.
>> + * @temp_level: Temperature level at which the temperature clipping will
>> + * happen.
>> + * @mask_val: cpumask of the allowed cpu's where the clipping will take
>> place.
>> + *
>> + * This structure is required to be filled and passed to the
>> + * cpufreq_cooling_unregister function.
>> + */
>> +struct freq_clip_table {
>> + unsigned int freq_clip_max;
>> + unsigned int temp_level;
>> + const struct cpumask *mask_val;
>> +};
>> +
>> +/**
>> + * cputherm_register_notifier - Register a notifier with cpu cooling
>> interface.
>> + * @nb: struct notifier_block * with callback info.
>> + * @list: integer value for which notification is needed. possible values
>> are
>> + * CPUFREQ_COOLING_TYPE and CPUHOTPLUG_COOLING_TYPE.
>> + *
>> + * This exported function registers a driver with cpu cooling layer. The
>> driver
>> + * will be notified when any cpu cooling action is called.
>> + */
>> +int cputherm_register_notifier(struct notifier_block *nb, unsigned int
>> list);
>> +
>> +/**
>> + * cputherm_unregister_notifier - Un-register a notifier.
>> + * @nb: struct notifier_block * with callback info.
>> + * @list: integer value for which notification is needed. values possible
>> are
>> + * CPUFREQ_COOLING_TYPE.
>> + *
>> + * This exported function un-registers a driver with cpu cooling layer.
>> + */
>> +int cputherm_unregister_notifier(struct notifier_block *nb, unsigned int
>> list);
>> +
>> +#ifdef CONFIG_CPU_FREQ
>> +/**
>> + * cpufreq_cooling_register - function to create cpufreq cooling device.
>> + * @tab_ptr: table ptr containing the maximum value of frequency to be
>> clipped
>> + * for each cooling state.
>> + * @tab_size: count of entries in the above table.
>> + * @mask_val: cpumask containing the allowed cpu's where frequency
>> clipping can
>> + * happen.
>> + */
>> +struct thermal_cooling_device *cpufreq_cooling_register(
>> + struct freq_clip_table *tab_ptr, unsigned int tab_size);
>> +
>> +/**
>> + * cpufreq_cooling_unregister - function to remove cpufreq cooling
>> device.
>> + * @cdev: thermal cooling device pointer.
>> + */
>> +void cpufreq_cooling_unregister(struct thermal_cooling_device *cdev);
>> +#else /*!CONFIG_CPU_FREQ*/
>> +static inline struct thermal_cooling_device *cpufreq_cooling_register(
>> + struct freq_clip_table *tab_ptr, unsigned int tab_size);
>> +{
>> + return NULL;
>> +}
>> +static inline void cpufreq_cooling_unregister(
>> + struct thermal_cooling_device *cdev)
>> +{
>> + return;
>> +}
>> +#endif /*CONFIG_CPU_FREQ*/
>> +
>> +#endif /* __CPU_COOLING_H__ */
>> --
>> 1.7.1
>>
>>
>> _______________________________________________
>>
>> linaro-dev mailing list
>> linaro-dev@lists.linaro.org
>> http://lists.linaro.org/mailman/listinfo/linaro-dev
>
>
>
> _______________________________________________
> linaro-dev mailing list
> linaro-dev@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/linaro-dev
>
^ permalink raw reply
* Re: [PATCH v4 1/5] thermal: Add generic cpufreq cooling implementation
From: Hongbo Zhang @ 2012-07-12 5:24 UTC (permalink / raw)
To: amit kachhap
Cc: linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA,
durgadoss.r-ral2JQCrhuEAvxtiuMwx3w,
akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
patches-QSEj5FYQhm4dnm+yROfE0A,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
lm-sensors-GZX6beZjE8VD60Wz+7aTrA,
linux-acpi-u79uwXL29TY76Z2rM5mHXA, khali-PUYAD+kWke1g9hUCZPvPmw,
linaro-dev-cunTk1MwBs8s++Sfvej+rw,
rui.zhang-ral2JQCrhuEAvxtiuMwx3w,
guenter.roeck-IzeFyvvaP7pWk0Htik3J/w,
linux-pm-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
lenb-DgEjT+Ai2ygdnm+yROfE0A
In-Reply-To: <CADGdYn5Enfd1Et3gwVqWjMfMHuyk1LJ9WCScOGpPF0ZLEyk0Fg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
[-- Attachment #1.1: Type: text/plain, Size: 32382 bytes --]
See my codes:
http://git.linaro.org/gitweb?p=people/hongbozhang/kernel.git;a=blob;f=drivers/thermal/db8500_cpufreq_cooling.c;h=2599149c25d381d55c083d19545f11005c05fdc2;hb=ste_thermal_v3.4_rc3
http://git.linaro.org/gitweb?p=people/hongbozhang/kernel.git;a=blob;f=drivers/thermal/db8500_thermal.c;h=2bb9b9a1477732df3f786ba246cf28ebc55f9998;hb=ste_thermal_v3.4_rc3
I create cooling device separately from thermal zone, before binding the
cooling device doesn't know any trip points.
This is good for adding/removing cooling devices freely.
What's more, my driver won't work if this checking still there.
I mean, this checking is optional, not mandatory.
On 12 July 2012 11:20, amit kachhap <amit.kachhap-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> On Tue, Jul 10, 2012 at 12:31 PM, Hongbo Zhang <hongbo.zhang-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> wrote:
> >
> >
> > On 12 May 2012 17:40, Amit Daniel Kachhap <amit.kachhap-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> wrote:
> >>
> >> This patch adds support for generic cpu thermal cooling low level
> >> implementations using frequency scaling up/down based on the
> registration
> >> parameters. Different cpu related cooling devices can be registered by
> the
> >> user and the binding of these cooling devices to the corresponding
> >> trip points can be easily done as the registration APIs return the
> >> cooling device pointer. The user of these APIs are responsible for
> >> passing clipping frequency . The drivers can also register to recieve
> >> notification about any cooling action called.
> >>
> >> Signed-off-by: Amit Daniel Kachhap <amit.kachhap-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> >> ---
> >> Documentation/thermal/cpu-cooling-api.txt | 60 ++++
> >> drivers/thermal/Kconfig | 11 +
> >> drivers/thermal/Makefile | 3 +-
> >> drivers/thermal/cpu_cooling.c | 483
> >> +++++++++++++++++++++++++++++
> >> include/linux/cpu_cooling.h | 99 ++++++
> >> 5 files changed, 655 insertions(+), 1 deletions(-)
> >> create mode 100644 Documentation/thermal/cpu-cooling-api.txt
> >> create mode 100644 drivers/thermal/cpu_cooling.c
> >> create mode 100644 include/linux/cpu_cooling.h
> >>
> >> diff --git a/Documentation/thermal/cpu-cooling-api.txt
> >> b/Documentation/thermal/cpu-cooling-api.txt
> >> new file mode 100644
> >> index 0000000..557adb8
> >> --- /dev/null
> >> +++ b/Documentation/thermal/cpu-cooling-api.txt
> >> @@ -0,0 +1,60 @@
> >> +CPU cooling APIs How To
> >> +===================================
> >> +
> >> +Written by Amit Daniel Kachhap <amit.kachhap-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> >> +
> >> +Updated: 12 May 2012
> >> +
> >> +Copyright (c) 2012 Samsung Electronics Co., Ltd(
> http://www.samsung.com)
> >> +
> >> +0. Introduction
> >> +
> >> +The generic cpu cooling(freq clipping, cpuhotplug etc) provides
> >> +registration/unregistration APIs to the caller. The binding of the
> >> cooling
> >> +devices to the trip point is left for the user. The registration APIs
> >> returns
> >> +the cooling device pointer.
> >> +
> >> +1. cpu cooling APIs
> >> +
> >> +1.1 cpufreq registration/unregistration APIs
> >> +1.1.1 struct thermal_cooling_device *cpufreq_cooling_register(
> >> + struct freq_clip_table *tab_ptr, unsigned int tab_size)
> >> +
> >> + This interface function registers the cpufreq cooling device with
> the
> >> name
> >> + "thermal-cpufreq-%x". This api can support multiple instances of
> >> cpufreq
> >> + cooling devices.
> >> +
> >> + tab_ptr: The table containing the maximum value of frequency to be
> >> clipped
> >> + for each cooling state.
> >> + .freq_clip_max: Value of frequency to be clipped for each
> allowed
> >> + cpus.
> >> + .temp_level: Temperature level at which the frequency clamping
> >> will
> >> + happen.
> >> + .mask_val: cpumask of the allowed cpu's
> >> + tab_size: the total number of cpufreq cooling states.
> >> +
> >> +1.1.2 void cpufreq_cooling_unregister(struct thermal_cooling_device
> >> *cdev)
> >> +
> >> + This interface function unregisters the "thermal-cpufreq-%x"
> cooling
> >> device.
> >> +
> >> + cdev: Cooling device pointer which has to be unregistered.
> >> +
> >> +
> >> +1.2 CPU cooling action notifier register/unregister interface
> >> +1.2.1 int cputherm_register_notifier(struct notifier_block *nb,
> >> + unsigned int list)
> >> +
> >> + This interface registers a driver with cpu cooling layer. The
> driver
> >> will
> >> + be notified when any cpu cooling action is called.
> >> +
> >> + nb: notifier function to register
> >> + list: CPUFREQ_COOLING_START or CPUFREQ_COOLING_STOP
> >> +
> >> +1.2.2 int cputherm_unregister_notifier(struct notifier_block *nb,
> >> + unsigned int list)
> >> +
> >> + This interface registers a driver with cpu cooling layer. The
> driver
> >> will
> >> + be notified when any cpu cooling action is called.
> >> +
> >> + nb: notifier function to register
> >> + list: CPUFREQ_COOLING_START or CPUFREQ_COOLING_STOP
> >> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> >> index 514a691..d9c529f 100644
> >> --- a/drivers/thermal/Kconfig
> >> +++ b/drivers/thermal/Kconfig
> >> @@ -19,6 +19,17 @@ config THERMAL_HWMON
> >> depends on HWMON=y || HWMON=THERMAL
> >> default y
> >>
> >> +config CPU_THERMAL
> >> + bool "generic cpu cooling support"
> >> + depends on THERMAL && CPU_FREQ
> >> + help
> >> + This implements the generic cpu cooling mechanism through
> >> frequency
> >> + reduction, cpu hotplug and any other ways of reducing
> >> temperature. An
> >> + ACPI version of this already
> >> exists(drivers/acpi/processor_thermal.c).
> >> + This will be useful for platforms using the generic thermal
> >> interface
> >> + and not the ACPI interface.
> >> + If you want this support, you should say Y or M here.
> >> +
> >> config SPEAR_THERMAL
> >> bool "SPEAr thermal sensor driver"
> >> depends on THERMAL
> >> diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
> >> index a9fff0b..30c456c 100644
> >> --- a/drivers/thermal/Makefile
> >> +++ b/drivers/thermal/Makefile
> >> @@ -3,4 +3,5 @@
> >> #
> >>
> >> obj-$(CONFIG_THERMAL) += thermal_sys.o
> >> -obj-$(CONFIG_SPEAR_THERMAL) += spear_thermal.o
> >> \ No newline at end of file
> >> +obj-$(CONFIG_CPU_THERMAL) += cpu_cooling.o
> >> +obj-$(CONFIG_SPEAR_THERMAL) += spear_thermal.o
> >> diff --git a/drivers/thermal/cpu_cooling.c
> b/drivers/thermal/cpu_cooling.c
> >> new file mode 100644
> >> index 0000000..c40d9a0
> >> --- /dev/null
> >> +++ b/drivers/thermal/cpu_cooling.c
> >> @@ -0,0 +1,483 @@
> >> +/*
> >> + * linux/drivers/thermal/cpu_cooling.c
> >> + *
> >> + * Copyright (C) 2012 Samsung Electronics Co.,
> >> Ltd(http://www.samsung.com)
> >> + * Copyright (C) 2012 Amit Daniel <amit.kachhap-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> >> + *
> >> + *
> >>
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >> + * This program is free software; you can redistribute it and/or
> modify
> >> + * it under the terms of the GNU General Public License as published
> by
> >> + * the Free Software Foundation; version 2 of the License.
> >> + *
> >> + * This program is distributed in the hope that it will be useful, but
> >> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> >> + * General Public License for more details.
> >> + *
> >> + * You should have received a copy of the GNU General Public License
> >> along
> >> + * with this program; if not, write to the Free Software Foundation,
> >> Inc.,
> >> + * 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA.
> >> + *
> >> + *
> >>
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >> + */
> >> +#include <linux/kernel.h>
> >> +#include <linux/module.h>
> >> +#include <linux/thermal.h>
> >> +#include <linux/platform_device.h>
> >> +#include <linux/cpufreq.h>
> >> +#include <linux/err.h>
> >> +#include <linux/slab.h>
> >> +#include <linux/cpu.h>
> >> +#include <linux/cpu_cooling.h>
> >> +
> >> +/**
> >> + * struct cpufreq_cooling_device
> >> + * @id: unique integer value corresponding to each
> cpufreq_cooling_device
> >> + * registered.
> >> + * @cool_dev: thermal_cooling_device pointer to keep track of the the
> >> + * egistered cooling device.
> >> + * @tab_ptr: freq_clip_table table containing the maximum value of
> >> frequency to
> >> + * be set for different cooling state.
> >> + * @tab_size: integer value representing a count of the above table.
> >> + * @cpufreq_state: integer value representing the current state of
> >> cpufreq
> >> + * cooling devices.
> >> + * @allowed_cpus: all the cpus involved for this
> cpufreq_cooling_device.
> >> + * @node: list_head to link all cpufreq_cooling_device together.
> >> + *
> >> + * This structure is required for keeping information of each
> >> + * cpufreq_cooling_device registered as a list whose head is
> represented
> >> by
> >> + * cooling_cpufreq_list. In order to prevent corruption of this list a
> >> + * mutex lock cooling_cpufreq_lock is used.
> >> + */
> >> +struct cpufreq_cooling_device {
> >> + int id;
> >> + struct thermal_cooling_device *cool_dev;
> >> + struct freq_clip_table *tab_ptr;
> >> + unsigned int tab_size;
> >> + unsigned int cpufreq_state;
> >> + struct cpumask allowed_cpus;
> >> + struct list_head node;
> >> +};
> >> +static LIST_HEAD(cooling_cpufreq_list);
> >> +static DEFINE_MUTEX(cooling_cpufreq_lock);
> >> +static DEFINE_IDR(cpufreq_idr);
> >> +
> >> +/*per cpu variable to store the previous max frequency allowed*/
> >> +static DEFINE_PER_CPU(unsigned int, max_policy_freq);
> >> +
> >> +/*notify_table passes value to the CPUFREQ_ADJUST callback function.*/
> >> +#define NOTIFY_INVALID NULL
> >> +static struct freq_clip_table *notify_table;
> >> +
> >> +/*Head of the blocking notifier chain to inform about frequency
> >> clamping*/
> >> +static BLOCKING_NOTIFIER_HEAD(cputherm_state_notifier_list);
> >> +
> >> +/**
> >> + * get_idr - function to get a unique id.
> >> + * @idr: struct idr * handle used to create a id.
> >> + * @id: int * value generated by this function.
> >> + */
> >> +static int get_idr(struct idr *idr, int *id)
> >> +{
> >> + int err;
> >> +again:
> >> + if (unlikely(idr_pre_get(idr, GFP_KERNEL) == 0))
> >> + return -ENOMEM;
> >> +
> >> + mutex_lock(&cooling_cpufreq_lock);
> >> + err = idr_get_new(idr, NULL, id);
> >> + mutex_unlock(&cooling_cpufreq_lock);
> >> +
> >> + if (unlikely(err == -EAGAIN))
> >> + goto again;
> >> + else if (unlikely(err))
> >> + return err;
> >> +
> >> + *id = *id & MAX_ID_MASK;
> >> + return 0;
> >> +}
> >> +
> >> +/**
> >> + * release_idr - function to free the unique id.
> >> + * @idr: struct idr * handle used for creating the id.
> >> + * @id: int value representing the unique id.
> >> + */
> >> +static void release_idr(struct idr *idr, int id)
> >> +{
> >> + mutex_lock(&cooling_cpufreq_lock);
> >> + idr_remove(idr, id);
> >> + mutex_unlock(&cooling_cpufreq_lock);
> >> +}
> >> +
> >> +/**
> >> + * cputherm_register_notifier - Register a notifier with cpu cooling
> >> interface.
> >> + * @nb: struct notifier_block * with callback info.
> >> + * @list: integer value for which notification is needed. possible
> values
> >> are
> >> + * CPUFREQ_COOLING_START and CPUFREQ_COOLING_STOP.
> >> + *
> >> + * This exported function registers a driver with cpu cooling layer.
> The
> >> driver
> >> + * will be notified when any cpu cooling action is called.
> >> + */
> >> +int cputherm_register_notifier(struct notifier_block *nb, unsigned int
> >> list)
> >> +{
> >> + int ret = 0;
> >> +
> >> + switch (list) {
> >> + case CPUFREQ_COOLING_START:
> >> + case CPUFREQ_COOLING_STOP:
> >> + ret = blocking_notifier_chain_register(
> >> + &cputherm_state_notifier_list, nb);
> >> + break;
> >> + default:
> >> + ret = -EINVAL;
> >> + }
> >> + return ret;
> >> +}
> >> +EXPORT_SYMBOL(cputherm_register_notifier);
> >> +
> >> +/**
> >> + * cputherm_unregister_notifier - Un-register a notifier.
> >> + * @nb: struct notifier_block * with callback info.
> >> + * @list: integer value for which notification is needed. values
> possible
> >> are
> >> + * CPUFREQ_COOLING_START or CPUFREQ_COOLING_STOP.
> >> + *
> >> + * This exported function un-registers a driver with cpu cooling layer.
> >> + */
> >> +int cputherm_unregister_notifier(struct notifier_block *nb, unsigned
> int
> >> list)
> >> +{
> >> + int ret = 0;
> >> +
> >> + switch (list) {
> >> + case CPUFREQ_COOLING_START:
> >> + case CPUFREQ_COOLING_STOP:
> >> + ret = blocking_notifier_chain_unregister(
> >> + &cputherm_state_notifier_list, nb);
> >> + break;
> >> + default:
> >> + ret = -EINVAL;
> >> + }
> >> + return ret;
> >> +}
> >> +EXPORT_SYMBOL(cputherm_unregister_notifier);
> >> +
> >> +/*Below codes defines functions to be used for cpufreq as cooling
> >> device*/
> >> +
> >> +/**
> >> + * is_cpufreq_valid - function to check if a cpu has frequency
> transition
> >> policy.
> >> + * @cpu: cpu for which check is needed.
> >> + */
> >> +static int is_cpufreq_valid(int cpu)
> >> +{
> >> + struct cpufreq_policy policy;
> >> + return !cpufreq_get_policy(&policy, cpu);
> >> +}
> >> +
> >> +/**
> >> + * cpufreq_apply_cooling - function to apply frequency clipping.
> >> + * @cpufreq_device: cpufreq_cooling_device pointer containing frequency
> >> + * clipping data.
> >> + * @cooling_state: value of the cooling state.
> >> + */
> >> +static int cpufreq_apply_cooling(struct cpufreq_cooling_device
> >> *cpufreq_device,
> >> + unsigned long cooling_state)
> >> +{
> >> + unsigned int event, cpuid, state;
> >> + struct freq_clip_table *th_table, *table_ptr;
> >> + const struct cpumask *maskPtr = &cpufreq_device->allowed_cpus;
> >> + struct cpufreq_cooling_device *cpufreq_ptr;
> >> +
> >> + if (cooling_state > cpufreq_device->tab_size)
> >> + return -EINVAL;
> >> +
> >> + /*Check if the old cooling action is same as new cooling
> action*/
> >> + if (cpufreq_device->cpufreq_state == cooling_state)
> >> + return 0;
> >> +
> >> + /*pass cooling table info to the cpufreq_thermal_notifier
> >> callback*/
> >> + notify_table = NOTIFY_INVALID;
> >> +
> >> + if (cooling_state > 0) {
> >> + th_table = &(cpufreq_device->tab_ptr[cooling_state -
> 1]);
> >> + notify_table = th_table;
> >> + }
> >> +
> >> + /*check if any lower clip frequency active in other
> >> cpufreq_device's*/
> >> + list_for_each_entry(cpufreq_ptr, &cooling_cpufreq_list, node) {
> >> +
> >> + state = cpufreq_ptr->cpufreq_state;
> >> + if (state == 0 || cpufreq_ptr == cpufreq_device)
> >> + continue;
> >> +
> >> + if (!cpumask_equal(&cpufreq_ptr->allowed_cpus,
> >> + &cpufreq_device->allowed_cpus))
> >> + continue;
> >> +
> >> + table_ptr = &(cpufreq_ptr->tab_ptr[state - 1]);
> >> + if (notify_table == NULL ||
> >> + (table_ptr->freq_clip_max <
> >> + notify_table->freq_clip_max))
> >> + notify_table = table_ptr;
> >> + }
> >> +
> >> + cpufreq_device->cpufreq_state = cooling_state;
> >> +
> >> + if (notify_table != NOTIFY_INVALID) {
> >> + event = CPUFREQ_COOLING_START;
> >> + maskPtr = notify_table->mask_val;
> >> + } else {
> >> + event = CPUFREQ_COOLING_STOP;
> >> + }
> >> +
> >> + blocking_notifier_call_chain(&cputherm_state_notifier_list,
> >> + event, notify_table);
> >> +
> >> + for_each_cpu(cpuid, maskPtr) {
> >> + if (is_cpufreq_valid(cpuid))
> >> + cpufreq_update_policy(cpuid);
> >> + }
> >> +
> >> + notify_table = NOTIFY_INVALID;
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +/**
> >> + * cpufreq_thermal_notifier - notifier callback for cpufreq policy
> >> change.
> >> + * @nb: struct notifier_block * with callback info.
> >> + * @event: value showing cpufreq event for which this function invoked.
> >> + * @data: callback-specific data
> >> + */
> >> +static int cpufreq_thermal_notifier(struct notifier_block *nb,
> >> + unsigned long event, void *data)
> >> +{
> >> + struct cpufreq_policy *policy = data;
> >> + unsigned long max_freq = 0;
> >> +
> >> + if (event != CPUFREQ_ADJUST)
> >> + return 0;
> >> +
> >> + if (notify_table != NOTIFY_INVALID) {
> >> + max_freq = notify_table->freq_clip_max;
> >> +
> >> + if (!per_cpu(max_policy_freq, policy->cpu))
> >> + per_cpu(max_policy_freq, policy->cpu) =
> >> policy->max;
> >> + } else {
> >> + if (per_cpu(max_policy_freq, policy->cpu)) {
> >> + max_freq = per_cpu(max_policy_freq,
> policy->cpu);
> >> + per_cpu(max_policy_freq, policy->cpu) = 0;
> >> + } else {
> >> + max_freq = policy->max;
> >> + }
> >> + }
> >> +
> >> + /* Never exceed user_policy.max*/
> >> + if (max_freq > policy->user_policy.max)
> >> + max_freq = policy->user_policy.max;
> >> +
> >> + if (policy->max != max_freq)
> >> + cpufreq_verify_within_limits(policy, 0, max_freq);
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +/*
> >> + * cpufreq cooling device callback functions are defined below
> >> + */
> >> +
> >> +/**
> >> + * cpufreq_get_max_state - callback function to get the max cooling
> >> state.
> >> + * @cdev: thermal cooling device pointer.
> >> + * @state: fill this variable with the max cooling state.
> >> + */
> >> +static int cpufreq_get_max_state(struct thermal_cooling_device *cdev,
> >> + unsigned long *state)
> >> +{
> >> + int ret = -EINVAL;
> >> + struct cpufreq_cooling_device *cpufreq_device;
> >> +
> >> + mutex_lock(&cooling_cpufreq_lock);
> >> + list_for_each_entry(cpufreq_device, &cooling_cpufreq_list,
> node) {
> >> + if (cpufreq_device && cpufreq_device->cool_dev == cdev)
> {
> >> + *state = cpufreq_device->tab_size;
> >> + ret = 0;
> >> + break;
> >> + }
> >> + }
> >> + mutex_unlock(&cooling_cpufreq_lock);
> >> + return ret;
> >> +}
> >> +
> >> +/**
> >> + * cpufreq_get_cur_state - callback function to get the current cooling
> >> state.
> >> + * @cdev: thermal cooling device pointer.
> >> + * @state: fill this variable with the current cooling state.
> >> + */
> >> +static int cpufreq_get_cur_state(struct thermal_cooling_device *cdev,
> >> + unsigned long *state)
> >> +{
> >> + int ret = -EINVAL;
> >> + struct cpufreq_cooling_device *cpufreq_device;
> >> +
> >> + mutex_lock(&cooling_cpufreq_lock);
> >> + list_for_each_entry(cpufreq_device, &cooling_cpufreq_list,
> node) {
> >> + if (cpufreq_device && cpufreq_device->cool_dev == cdev)
> {
> >> + *state = cpufreq_device->cpufreq_state;
> >> + ret = 0;
> >> + break;
> >> + }
> >> + }
> >> + mutex_unlock(&cooling_cpufreq_lock);
> >> + return ret;
> >> +}
> >> +
> >> +/**
> >> + * cpufreq_set_cur_state - callback function to set the current cooling
> >> state.
> >> + * @cdev: thermal cooling device pointer.
> >> + * @state: set this variable to the current cooling state.
> >> + */
> >> +static int cpufreq_set_cur_state(struct thermal_cooling_device *cdev,
> >> + unsigned long state)
> >> +{
> >> + int ret = -EINVAL;
> >> + struct cpufreq_cooling_device *cpufreq_device;
> >> +
> >> + mutex_lock(&cooling_cpufreq_lock);
> >> + list_for_each_entry(cpufreq_device, &cooling_cpufreq_list,
> node) {
> >> + if (cpufreq_device && cpufreq_device->cool_dev == cdev)
> {
> >> + ret = 0;
> >> + break;
> >> + }
> >> + }
> >> + if (!ret)
> >> + ret = cpufreq_apply_cooling(cpufreq_device, state);
> >> +
> >> + mutex_unlock(&cooling_cpufreq_lock);
> >> +
> >> + return ret;
> >> +}
> >> +
> >> +/*Bind cpufreq callbacks to thermal cooling device ops*/
> >> +static struct thermal_cooling_device_ops const cpufreq_cooling_ops = {
> >> + .get_max_state = cpufreq_get_max_state,
> >> + .get_cur_state = cpufreq_get_cur_state,
> >> + .set_cur_state = cpufreq_set_cur_state,
> >> +};
> >> +
> >> +/*Notifier for cpufreq policy change*/
> >> +static struct notifier_block thermal_cpufreq_notifier_block = {
> >> + .notifier_call = cpufreq_thermal_notifier,
> >> +};
> >> +
> >> +/**
> >> + * cpufreq_cooling_register - function to create cpufreq cooling
> device.
> >> + * @tab_ptr: table ptr containing the maximum value of frequency to be
> >> clipped
> >> + * for each cooling state.
> >> + * @tab_size: count of entries in the above table.
> >> + * happen.
> >> + */
> >> +struct thermal_cooling_device *cpufreq_cooling_register(
> >> + struct freq_clip_table *tab_ptr, unsigned int tab_size)
> >> +{
> >> + struct thermal_cooling_device *cool_dev;
> >> + struct cpufreq_cooling_device *cpufreq_dev = NULL;
> >> + struct freq_clip_table *clip_tab;
> >> + unsigned int cpufreq_dev_count = 0;
> >> + char dev_name[THERMAL_NAME_LENGTH];
> >> + int ret = 0, id = 0, i;
> >> +
> >> + if (tab_ptr == NULL || tab_size == 0)
> >> + return ERR_PTR(-EINVAL);
> >> +
> >> + list_for_each_entry(cpufreq_dev, &cooling_cpufreq_list, node)
> >> + cpufreq_dev_count++;
> >> +
> >> + cpufreq_dev = kzalloc(sizeof(struct cpufreq_cooling_device),
> >> + GFP_KERNEL);
> >> + if (!cpufreq_dev)
> >> + return ERR_PTR(-ENOMEM);
> >> +
> >> + /*Verify that all the entries of freq_clip_table are present*/
> >> + for (i = 0; i < tab_size; i++) {
> >> + clip_tab = ((struct freq_clip_table *)&tab_ptr[i]);
> >> + if (!clip_tab->freq_clip_max || !clip_tab->mask_val
> >> + || !clip_tab->temp_level) {
> >
> > About clip_tab->temp_level:
> > I'd like to remove this variable checking, because as suggested by
> Vincent
> > Guittot, it is better to separate cooling devices
> > and thermal zone, thus we can add/remove cooling devices
> freely/dynamically.
> > In such case, a cooling device doesn't
> > know any temp_level before binding--the binding condition is name
> matching,
> > what's more, the cooling device should not
> > know much data of other devices.
>
> Hi Hongbo,
>
> The temp_level is not used anywhere inside the cooling devices. They
> are just present to pass the trip temperature information to the
> registered notifiers.
>
> Thanks,
> Amit D
> >
> >> + kfree(cpufreq_dev);
> >> + return ERR_PTR(-EINVAL);
> >> + }
> >> + /*
> >> + *Consolidate all the cpumask for all the individual
> >> entries
> >> + *of the trip table. This is useful in resetting all the
> >> + *clipped frequencies to the normal level for each
> cpufreq
> >> + *cooling device.
> >> + */
> >> + cpumask_or(&cpufreq_dev->allowed_cpus,
> >> + &cpufreq_dev->allowed_cpus, clip_tab->mask_val);
> >> + }
> >> +
> >> + cpufreq_dev->tab_ptr = tab_ptr;
> >> + cpufreq_dev->tab_size = tab_size;
> >> +
> >> + ret = get_idr(&cpufreq_idr, &cpufreq_dev->id);
> >> + if (ret) {
> >> + kfree(cpufreq_dev);
> >> + return ERR_PTR(-EINVAL);
> >> + }
> >> +
> >> + sprintf(dev_name, "thermal-cpufreq-%d", cpufreq_dev->id);
> >> +
> >> + cool_dev = thermal_cooling_device_register(dev_name,
> cpufreq_dev,
> >> + &cpufreq_cooling_ops);
> >> + if (!cool_dev) {
> >> + release_idr(&cpufreq_idr, cpufreq_dev->id);
> >> + kfree(cpufreq_dev);
> >> + return ERR_PTR(-EINVAL);
> >> + }
> >> + cpufreq_dev->id = id;
> >> + cpufreq_dev->cool_dev = cool_dev;
> >> + mutex_lock(&cooling_cpufreq_lock);
> >> + list_add_tail(&cpufreq_dev->node, &cooling_cpufreq_list);
> >> +
> >> + /*Register the notifier for first cpufreq cooling device*/
> >> + if (cpufreq_dev_count == 0)
> >> +
> cpufreq_register_notifier(&thermal_cpufreq_notifier_block,
> >> +
> CPUFREQ_POLICY_NOTIFIER);
> >> +
> >> + mutex_unlock(&cooling_cpufreq_lock);
> >> + return cool_dev;
> >> +}
> >> +EXPORT_SYMBOL(cpufreq_cooling_register);
> >> +
> >> +/**
> >> + * cpufreq_cooling_unregister - function to remove cpufreq cooling
> >> device.
> >> + * @cdev: thermal cooling device pointer.
> >> + */
> >> +void cpufreq_cooling_unregister(struct thermal_cooling_device *cdev)
> >> +{
> >> + struct cpufreq_cooling_device *cpufreq_dev = NULL;
> >> + unsigned int cpufreq_dev_count = 0;
> >> +
> >> + mutex_lock(&cooling_cpufreq_lock);
> >> + list_for_each_entry(cpufreq_dev, &cooling_cpufreq_list, node) {
> >> + if (cpufreq_dev && cpufreq_dev->cool_dev == cdev)
> >> + break;
> >> + cpufreq_dev_count++;
> >> + }
> >> +
> >> + if (!cpufreq_dev || cpufreq_dev->cool_dev != cdev) {
> >> + mutex_unlock(&cooling_cpufreq_lock);
> >> + return;
> >> + }
> >> +
> >> + list_del(&cpufreq_dev->node);
> >> +
> >> + /*Unregister the notifier for the last cpufreq cooling device*/
> >> + if (cpufreq_dev_count == 1) {
> >> +
> >> cpufreq_unregister_notifier(&thermal_cpufreq_notifier_block,
> >> + CPUFREQ_POLICY_NOTIFIER);
> >> + }
> >> + mutex_unlock(&cooling_cpufreq_lock);
> >> + thermal_cooling_device_unregister(cpufreq_dev->cool_dev);
> >> + release_idr(&cpufreq_idr, cpufreq_dev->id);
> >> + kfree(cpufreq_dev);
> >> +}
> >> +EXPORT_SYMBOL(cpufreq_cooling_unregister);
> >> diff --git a/include/linux/cpu_cooling.h b/include/linux/cpu_cooling.h
> >> new file mode 100644
> >> index 0000000..ed6c096
> >> --- /dev/null
> >> +++ b/include/linux/cpu_cooling.h
> >> @@ -0,0 +1,99 @@
> >> +/*
> >> + * linux/include/linux/cpu_cooling.h
> >> + *
> >> + * Copyright (C) 2012 Samsung Electronics Co.,
> >> Ltd(http://www.samsung.com)
> >> + * Copyright (C) 2012 Amit Daniel <amit.kachhap-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> >> + *
> >> + *
> >>
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >> + * This program is free software; you can redistribute it and/or
> modify
> >> + * it under the terms of the GNU General Public License as published
> by
> >> + * the Free Software Foundation; version 2 of the License.
> >> + *
> >> + * This program is distributed in the hope that it will be useful, but
> >> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> >> + * General Public License for more details.
> >> + *
> >> + * You should have received a copy of the GNU General Public License
> >> along
> >> + * with this program; if not, write to the Free Software Foundation,
> >> Inc.,
> >> + * 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA.
> >> + *
> >> + *
> >>
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >> + */
> >> +
> >> +#ifndef __CPU_COOLING_H__
> >> +#define __CPU_COOLING_H__
> >> +
> >> +#include <linux/thermal.h>
> >> +
> >> +#define CPUFREQ_COOLING_START 0
> >> +#define CPUFREQ_COOLING_STOP 1
> >> +
> >> +/**
> >> + * struct freq_clip_table
> >> + * @freq_clip_max: maximum frequency allowed for this cooling state.
> >> + * @temp_level: Temperature level at which the temperature clipping
> will
> >> + * happen.
> >> + * @mask_val: cpumask of the allowed cpu's where the clipping will take
> >> place.
> >> + *
> >> + * This structure is required to be filled and passed to the
> >> + * cpufreq_cooling_unregister function.
> >> + */
> >> +struct freq_clip_table {
> >> + unsigned int freq_clip_max;
> >> + unsigned int temp_level;
> >> + const struct cpumask *mask_val;
> >> +};
> >> +
> >> +/**
> >> + * cputherm_register_notifier - Register a notifier with cpu cooling
> >> interface.
> >> + * @nb: struct notifier_block * with callback info.
> >> + * @list: integer value for which notification is needed. possible
> values
> >> are
> >> + * CPUFREQ_COOLING_TYPE and CPUHOTPLUG_COOLING_TYPE.
> >> + *
> >> + * This exported function registers a driver with cpu cooling layer.
> The
> >> driver
> >> + * will be notified when any cpu cooling action is called.
> >> + */
> >> +int cputherm_register_notifier(struct notifier_block *nb, unsigned int
> >> list);
> >> +
> >> +/**
> >> + * cputherm_unregister_notifier - Un-register a notifier.
> >> + * @nb: struct notifier_block * with callback info.
> >> + * @list: integer value for which notification is needed. values
> possible
> >> are
> >> + * CPUFREQ_COOLING_TYPE.
> >> + *
> >> + * This exported function un-registers a driver with cpu cooling layer.
> >> + */
> >> +int cputherm_unregister_notifier(struct notifier_block *nb, unsigned
> int
> >> list);
> >> +
> >> +#ifdef CONFIG_CPU_FREQ
> >> +/**
> >> + * cpufreq_cooling_register - function to create cpufreq cooling
> device.
> >> + * @tab_ptr: table ptr containing the maximum value of frequency to be
> >> clipped
> >> + * for each cooling state.
> >> + * @tab_size: count of entries in the above table.
> >> + * @mask_val: cpumask containing the allowed cpu's where frequency
> >> clipping can
> >> + * happen.
> >> + */
> >> +struct thermal_cooling_device *cpufreq_cooling_register(
> >> + struct freq_clip_table *tab_ptr, unsigned int tab_size);
> >> +
> >> +/**
> >> + * cpufreq_cooling_unregister - function to remove cpufreq cooling
> >> device.
> >> + * @cdev: thermal cooling device pointer.
> >> + */
> >> +void cpufreq_cooling_unregister(struct thermal_cooling_device *cdev);
> >> +#else /*!CONFIG_CPU_FREQ*/
> >> +static inline struct thermal_cooling_device *cpufreq_cooling_register(
> >> + struct freq_clip_table *tab_ptr, unsigned int tab_size);
> >> +{
> >> + return NULL;
> >> +}
> >> +static inline void cpufreq_cooling_unregister(
> >> + struct thermal_cooling_device *cdev)
> >> +{
> >> + return;
> >> +}
> >> +#endif /*CONFIG_CPU_FREQ*/
> >> +
> >> +#endif /* __CPU_COOLING_H__ */
> >> --
> >> 1.7.1
> >>
> >>
> >> _______________________________________________
> >>
> >> linaro-dev mailing list
> >> linaro-dev-cunTk1MwBs8s++Sfvej+rw@public.gmane.org
> >> http://lists.linaro.org/mailman/listinfo/linaro-dev
> >
> >
> >
> > _______________________________________________
> > linaro-dev mailing list
> > linaro-dev-cunTk1MwBs8s++Sfvej+rw@public.gmane.org
> > http://lists.linaro.org/mailman/listinfo/linaro-dev
> >
>
[-- Attachment #1.2: Type: text/html, Size: 41811 bytes --]
[-- Attachment #2: Type: text/plain, Size: 175 bytes --]
_______________________________________________
linaro-dev mailing list
linaro-dev-cunTk1MwBs8s++Sfvej+rw@public.gmane.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
^ permalink raw reply
* [RESEND PATCH v4 0/5] thermal: Add kernel thermal support for exynos platform
From: Amit Daniel Kachhap @ 2012-07-12 13:41 UTC (permalink / raw)
To: linux-pm, lenb
Cc: linux-samsung-soc, linux-kernel, linux-acpi, lm-sensors,
amit.kachhap, akpm
Hi Len,
This series is a repost of the thermal support for exynos platform.
These set of patches were already accepted for 3.5 merge.
(https://lkml.org/lkml/2012/6/2/7) but somehow could not go through so,
I am resending them again with rebasing against 3.5-rc6. Please apply them
for 3.6 merge.
Thanks,
Amit Daniel
Amit Daniel Kachhap (5):
thermal: add generic cpufreq cooling implementation
hwmon: exynos4: move thermal sensor driver to driver/thermal
directory
thermal: exynos5: add exynos5 thermal sensor driver support
thermal: exynos: register the tmu sensor with the kernel thermal
layer
ARM: exynos: add thermal sensor driver platform data support
Documentation/hwmon/exynos4_tmu | 81 ---
Documentation/thermal/cpu-cooling-api.txt | 60 ++
Documentation/thermal/exynos_thermal | 52 ++
drivers/hwmon/Kconfig | 10 -
drivers/hwmon/Makefile | 1 -
drivers/hwmon/exynos4_tmu.c | 514 --------------
drivers/thermal/Kconfig | 20 +
drivers/thermal/Makefile | 4 +-
drivers/thermal/cpu_cooling.c | 483 +++++++++++++
drivers/thermal/exynos_thermal.c | 956 ++++++++++++++++++++++++++
include/linux/cpu_cooling.h | 99 +++
include/linux/platform_data/exynos4_tmu.h | 83 ---
include/linux/platform_data/exynos_thermal.h | 100 +++
13 files changed, 1773 insertions(+), 690 deletions(-)
delete mode 100644 Documentation/hwmon/exynos4_tmu
create mode 100644 Documentation/thermal/cpu-cooling-api.txt
create mode 100644 Documentation/thermal/exynos_thermal
delete mode 100644 drivers/hwmon/exynos4_tmu.c
create mode 100644 drivers/thermal/cpu_cooling.c
create mode 100644 drivers/thermal/exynos_thermal.c
create mode 100644 include/linux/cpu_cooling.h
delete mode 100644 include/linux/platform_data/exynos4_tmu.h
create mode 100644 include/linux/platform_data/exynos_thermal.h
^ 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