Linux Power Management development
 help / color / mirror / Atom feed
* Re: [PATCH 1/5] driver core / ACPI: Move ACPI support to core device and driver types
From: Rafael J. Wysocki @ 2012-10-31 21:46 UTC (permalink / raw)
  To: Luck, Tony
  Cc: H. Peter Anvin, Greg Kroah-Hartman, LKML, Linux PM list,
	ACPI Devel Maling List, Len Brown, Mathias Nyman, Mika Westerberg,
	Zheng, Lv, Huang, Ying, Andy Shevchenko, x86 list
In-Reply-To: <3908561D78D1C84285E8C5FCA982C28F19D5B9DE@ORSMSX108.amr.corp.intel.com>

On Wednesday, October 31, 2012 09:36:36 PM Luck, Tony wrote:
> > The BIOSes of currently available ia64 systems don't contain ACPI nodes whose
> > IDs will match the IDs of the new devices (ie. the ones that are going to be
> > added to acpi_platform_device_ids[]), so for ia64 it should be sufficient to
> > test that code as is (ie. without any new devices in the system).
> 
> Ok - built cleanly on ia64.  Boots too. Just one new console message:
> 
> ACPI: bus type platform registered
> 
> that seems pretty harmless.
> 
> Acked-by: Tony Luck <tony.luck@intel.com>

Thanks a lot!


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

^ permalink raw reply

* Re: [PATCH v8 09/11] block: add a new interface to block events
From: Tejun Heo @ 2012-10-31 21:51 UTC (permalink / raw)
  To: Aaron Lu
  Cc: James Bottomley, Jeff Garzik, Rafael J. Wysocki, Alan Stern,
	Oliver Neukum, Jeff Wu, Aaron Lu, Shane Huang, linux-ide,
	linux-pm, linux-scsi, linux-acpi
In-Reply-To: <508F7BF1.1040009@intel.com>

Hello,

On Tue, Oct 30, 2012 at 03:04:17PM +0800, Aaron Lu wrote:
> > check_event() can retry.  Just add a per-sr mutex which is try-locked
> > by sr_block_check_events() and grab it when entering zero power.
> 
> Good suggestion. I didn't think about solving it this way.
> 
> Many people suggest me that ZPODD is pure SATA/ACPI stuff, and should
> not pollute sr driver, so I was trying hard not to touch sr while
> preparing these patches, unless there is no other choice(like the
> blocking event interface).
> 
> So I'm not sure if your suggestion is the way to go.
> 
> James, what do you think? Is it OK if I add a mutex into the scsi_cd
> structure to do this? Of course I'll define this only under
> CONFIG_SATA_ZPODD.

I don't think what James' and my suggestions are that different.  Just
silence check_event() while zpodd is kicked in somehow.  There's no
reason to synchronize across multiple subsystems.

Thanks.

-- 
tejun

^ permalink raw reply

* Re: [PATCH v8 05/11] libata-eh: allow defer in ata_exec_internal
From: Tejun Heo @ 2012-10-31 21:52 UTC (permalink / raw)
  To: Aaron Lu
  Cc: Jeff Garzik, Rafael J. Wysocki, James Bottomley, Alan Stern,
	Oliver Neukum, Jeff Wu, Aaron Lu, Shane Huang, linux-ide,
	linux-pm, linux-scsi, linux-acpi
In-Reply-To: <508F44E1.3000801@intel.com>

Hello,

On Tue, Oct 30, 2012 at 11:09:21AM +0800, Aaron Lu wrote:
> I'm not aware of a place to store such ODD specific information when
> probing the device.

You can always add some fields. :)

> I'm currently storing the loading mech type in structure zpodd, which
> gets created after the corresponding SCSI device gets created in
> ata_scsi_scan_host, so at the probe time, the zpodd structure does not
> exist yet. And the reason I create the zpodd sturcture this late is
> that, it is only created when the ODD together the platform is ZPODD
> capable, and to find out if this platform is ZPODD capbale, ACPI binding
> has to occur first, and ACPI binding happens when SCSI device is added
> to the device tree.

Hmm... I see.  Which ACPI binding is it?  The ATA ACPI binding happens
during probing.  It's a different one, I presume?

Thanks.

-- 
tejun

^ permalink raw reply

* drivers/acpi/acpica/nsxfname.c:500 acpi_get_object_info() info: redundant null check on sub calling kfree()
From: kbuild test robot @ 2012-10-31 23:11 UTC (permalink / raw)
  To: Bob Moore; +Cc: linux-pm, Lv Zheng, Rafael J. Wysocki

tree:   git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
head:   741933bb4230bfc0ef9eba19384fe58e528b39a9
commit: 00741297a43870e21dcae6429b5240a3cc7a5312 ACPICA: AcpiGetObjectInfo: Add support for ACPI 5 _SUB method
date:   74 minutes ago


smatch warnings:

  drivers/acpi/acpica/nsxfname.c:494 acpi_get_object_info() info: redundant null check on hid calling kfree()
  drivers/acpi/acpica/nsxfname.c:497 acpi_get_object_info() info: redundant null check on uid calling kfree()
+ drivers/acpi/acpica/nsxfname.c:500 acpi_get_object_info() info: redundant null check on sub calling kfree()
  drivers/acpi/acpica/nsxfname.c:503 acpi_get_object_info() info: redundant null check on cid_list calling kfree()

vim +500 drivers/acpi/acpica/nsxfname.c

15b8dd53 drivers/acpi/acpica/nsxfname.c    Bob Moore      2009-06-29  484  	info->type = type;
15b8dd53 drivers/acpi/acpica/nsxfname.c    Bob Moore      2009-06-29  485  	info->name = name;
15b8dd53 drivers/acpi/acpica/nsxfname.c    Bob Moore      2009-06-29  486  	info->param_count = param_count;
15b8dd53 drivers/acpi/acpica/nsxfname.c    Bob Moore      2009-06-29  487  	info->valid = valid;
15b8dd53 drivers/acpi/acpica/nsxfname.c    Bob Moore      2009-06-29  488  
15b8dd53 drivers/acpi/acpica/nsxfname.c    Bob Moore      2009-06-29  489  	*return_buffer = info;
15b8dd53 drivers/acpi/acpica/nsxfname.c    Bob Moore      2009-06-29  490  	status = AE_OK;
15b8dd53 drivers/acpi/acpica/nsxfname.c    Bob Moore      2009-06-29  491  
4be44fcd drivers/acpi/namespace/nsxfname.c Len Brown      2005-08-05  492        cleanup:
15b8dd53 drivers/acpi/acpica/nsxfname.c    Bob Moore      2009-06-29  493  	if (hid) {
15b8dd53 drivers/acpi/acpica/nsxfname.c    Bob Moore      2009-06-29  494  		ACPI_FREE(hid);
15b8dd53 drivers/acpi/acpica/nsxfname.c    Bob Moore      2009-06-29  495  	}
15b8dd53 drivers/acpi/acpica/nsxfname.c    Bob Moore      2009-06-29  496  	if (uid) {
15b8dd53 drivers/acpi/acpica/nsxfname.c    Bob Moore      2009-06-29  497  		ACPI_FREE(uid);
15b8dd53 drivers/acpi/acpica/nsxfname.c    Bob Moore      2009-06-29  498  	}
00741297 drivers/acpi/acpica/nsxfname.c    Bob Moore      2012-10-31  499  	if (sub) {
00741297 drivers/acpi/acpica/nsxfname.c    Bob Moore      2012-10-31 @500  		ACPI_FREE(sub);
00741297 drivers/acpi/acpica/nsxfname.c    Bob Moore      2012-10-31  501  	}
^1da177e drivers/acpi/namespace/nsxfname.c Linus Torvalds 2005-04-16  502  	if (cid_list) {
8313524a drivers/acpi/namespace/nsxfname.c Bob Moore      2006-10-03  503  		ACPI_FREE(cid_list);
^1da177e drivers/acpi/namespace/nsxfname.c Linus Torvalds 2005-04-16  504  	}
^1da177e drivers/acpi/namespace/nsxfname.c Linus Torvalds 2005-04-16  505  	return (status);
^1da177e drivers/acpi/namespace/nsxfname.c Linus Torvalds 2005-04-16  506  }
^1da177e drivers/acpi/namespace/nsxfname.c Linus Torvalds 2005-04-16  507  
8313524a drivers/acpi/namespace/nsxfname.c Bob Moore      2006-10-03  508  ACPI_EXPORT_SYMBOL(acpi_get_object_info)

---
0-DAY kernel build testing backend         Open Source Technology Center
Fengguang Wu, Yuanhan Liu                              Intel Corporation

^ permalink raw reply

* Re: [PATCH v3 2/6] PM / Runtime: introduce pm_runtime_set[get]_memalloc_noio()
From: Ming Lei @ 2012-10-31 23:18 UTC (permalink / raw)
  To: Alan Stern
  Cc: Oliver Neukum, linux-kernel, Minchan Kim, Greg Kroah-Hartman,
	Rafael J. Wysocki, Jens Axboe, David S. Miller, Andrew Morton,
	netdev, linux-usb, linux-pm, linux-mm
In-Reply-To: <Pine.LNX.4.44L0.1210311139300.1954-100000@iolanthe.rowland.org>

On Wed, Oct 31, 2012 at 11:41 PM, Alan Stern <stern@rowland.harvard.edu> wrote:
>
> Sorry, I misread your message.  You are setting the device's flag, not
> the thread's flag.

Never mind.

>
> This still doesn't help in this case where CONFIG_PM_RUNTIME is
> disabled.  I think it will be simpler to set the noio flag during every
> device reset.

Yes, it's better to set the flag during every device reset now.

Also pppoe or network interface over serial port is a bit difficult to
deal with, as Oliver pointed out.


Thanks,
--
Ming Lei

^ permalink raw reply

* RE: [PATCH V3 4/5] Thermal: Add ST-Ericsson DB8500 thermal driver.
From: Zhang, Rui @ 2012-11-01  1:52 UTC (permalink / raw)
  To: hongbo.zhang, linaro-dev-cunTk1MwBs8s++Sfvej+rw@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-pm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	amit.kachhap-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org
  Cc: STEricsson_nomadik_linux-nkJGhpqTU55BDgjK7y7TUQ@public.gmane.org,
	kernel-vMlcbD5RyM6HZuj8yyL1ah2eb7JE58TQ@public.gmane.org,
	linaro-kernel-cunTk1MwBs8s++Sfvej+rw@public.gmane.org,
	hongbo.zhang, patches-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org
In-Reply-To: <1351615741-24134-5-git-send-email-hongbo.zhang-68IGFXMjmZ7QT0dZR+AlfA@public.gmane.org>



> -----Original Message-----
> From: hongbo.zhang [mailto:hongbo.zhang-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org]
> Sent: Wednesday, October 31, 2012 12:49 AM
> To: linaro-dev-cunTk1MwBs8s++Sfvej+rw@public.gmane.org; linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-
> pm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Zhang, Rui; amit.kachhap-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org
> Cc: patches-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org; linaro-kernel-cunTk1MwBs8s++Sfvej+rw@public.gmane.org;
> STEricsson_nomadik_linux-nkJGhpqTU55BDgjK7y7TUQ@public.gmane.org; kernel-vMlcbD5RyM6HZuj8yyL1ah2eb7JE58TQ@public.gmane.org;
> hongbo.zhang
> Subject: [PATCH V3 4/5] Thermal: Add ST-Ericsson DB8500 thermal driver.
> Importance: High
> 
> From: "hongbo.zhang" <hongbo.zhang-68IGFXMjmZ7QT0dZR+AlfA@public.gmane.org>
> 
> This diver is based on the thermal management framework in
> thermal_sys.c. A thermal zone device is created with the trip points to
> which cooling devices can be bound, the current cooling device is
> cpufreq, e.g. CPU frequency is clipped down to cool the CPU, and other
> cooling devices can be added and bound to the trip points dynamically.
> The platform specific PRCMU interrupts are used to active thermal
> update when trip points are reached.
> 
> Signed-off-by: hongbo.zhang <hongbo.zhang-68IGFXMjmZ7QT0dZR+AlfA@public.gmane.org>
> ---
>  .../devicetree/bindings/thermal/db8500-thermal.txt |  40 ++
>  drivers/thermal/Kconfig                            |  20 +
>  drivers/thermal/Makefile                           |   2 +
>  drivers/thermal/db8500_cpufreq_cooling.c           | 108 +++++
>  drivers/thermal/db8500_thermal.c                   | 531
> +++++++++++++++++++++
>  include/linux/platform_data/db8500_thermal.h       |  38 ++
>  6 files changed, 739 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/thermal/db8500-
> thermal.txt
>  create mode 100644 drivers/thermal/db8500_cpufreq_cooling.c
>  create mode 100644 drivers/thermal/db8500_thermal.c  create mode
> 100644 include/linux/platform_data/db8500_thermal.h
> 
> diff --git a/Documentation/devicetree/bindings/thermal/db8500-
> thermal.txt b/Documentation/devicetree/bindings/thermal/db8500-
> thermal.txt
> new file mode 100644
> index 0000000..cab6916
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/thermal/db8500-thermal.txt
> @@ -0,0 +1,40 @@
> +* ST-Ericsson DB8500 Thermal
> +
> +** Thermal node properties:
> +
> +- compatible : "stericsson,db8500-thermal";
> +- reg : address range of the thermal sensor registers;
> +- interrupts : interrupts generated from PRCMU;
> +- interrupt-names : "IRQ_HOTMON_LOW" and "IRQ_HOTMON_HIGH";
> +- num-trips : number of total trip points;
> +- tripN-temp : temperature of trip point N, should be in ascending
> +order;
> +- tripN-type : type of trip point N, should be one of "active"
> +"passive" "hot" "critical";
> +- tripN-cdev-num : number of the cooling devices which can be bound to
> +trip point N;
> +- tripN-cdev-nameM : name of the No. M cooling device of trip point N;
> +
> +Usually the num-trips and tripN-*** are separated in board related dts
> files.
> +
> +Example:
> +thermal@801573c0 {
> +	compatible = "stericsson,db8500-thermal";
> +	reg = <0x801573c0 0x40>;
> +	interrupts = <21 0x4>, <22 0x4>;
> +	interrupt-names = "IRQ_HOTMON_LOW", "IRQ_HOTMON_HIGH";
> +
> +	num-trips = <3>;
> +
> +	trip0-temp = <70000>;
> +	trip0-type = "active";
> +	trip0-cdev-num = <1>;
> +	trip0-cdev-name0 = "thermal-cpufreq-0";
> +
> +	trip1-temp = <75000>;
> +	trip1-type = "active";
> +	trip1-cdev-num = <2>;
> +	trip1-cdev-name0 = "thermal-cpufreq-0";
> +	trip1-cdev-name1 = "thermal-fan";
> +
> +	trip2-temp = <85000>;
> +	trip2-type = "critical";
> +	trip2-cdev-num = <0>;
> +}
> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig index
> e1cb6bd..54c8fd0 100644
> --- a/drivers/thermal/Kconfig
> +++ b/drivers/thermal/Kconfig
> @@ -31,6 +31,26 @@ config CPU_THERMAL
>  	  and not the ACPI interface.
>  	  If you want this support, you should say Y here.
> 
> +config DB8500_THERMAL
> +	bool "DB8500 thermal management"
> +	depends on THERMAL
> +	default y
> +	help
> +	  Adds DB8500 thermal management implementation according to the
> thermal
> +	  management framework. A thermal zone with several trip points
> will be
> +	  created. Cooling devices can be bound to the trip points to
> cool this
> +	  thermal zone if trip points reached.
> +
> +config DB8500_CPUFREQ_COOLING
> +	tristate "DB8500 cpufreq cooling"
> +	depends on CPU_THERMAL
> +	default y
> +	help
> +	  Adds DB8500 cpufreq cooling devices, and these cooling devices
> can be
> +	  bound to thermal zone trip points. When a trip point reached,
> the
> +	  bound cpufreq cooling device turns active to set CPU frequency
> low to
> +	  cool down the CPU.
> +
>  config SPEAR_THERMAL
>  	bool "SPEAr thermal sensor driver"
>  	depends on THERMAL
> diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile index
> 885550d..c7a8dab 100644
> --- a/drivers/thermal/Makefile
> +++ b/drivers/thermal/Makefile
> @@ -7,3 +7,5 @@ obj-$(CONFIG_CPU_THERMAL)		+= cpu_cooling.o
>  obj-$(CONFIG_SPEAR_THERMAL)		+= spear_thermal.o
>  obj-$(CONFIG_RCAR_THERMAL)	+= rcar_thermal.o
>  obj-$(CONFIG_EXYNOS_THERMAL)		+= exynos_thermal.o
> +obj-$(CONFIG_DB8500_THERMAL)		+= db8500_thermal.o
> +obj-$(CONFIG_DB8500_CPUFREQ_COOLING)	+= db8500_cpufreq_cooling.o
> diff --git a/drivers/thermal/db8500_cpufreq_cooling.c
> b/drivers/thermal/db8500_cpufreq_cooling.c
> new file mode 100644
> index 0000000..4cf8e72
> --- /dev/null
> +++ b/drivers/thermal/db8500_cpufreq_cooling.c
> @@ -0,0 +1,108 @@
> +/*
> + * db8500_cpufreq_cooling.c - DB8500 cpufreq works as cooling device.
> + *
> + * Copyright (C) 2012 ST-Ericsson
> + * Copyright (C) 2012 Linaro Ltd.
> + *
> + * Author: Hongbo Zhang <hongbo.zhang-68IGFXMjmZ7QT0dZR+AlfA@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; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * 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.
> + */
> +
> +#include <linux/cpu_cooling.h>
> +#include <linux/cpufreq.h>
> +#include <linux/err.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +
> +static int db8500_cpufreq_cooling_probe(struct platform_device *pdev)
> {
> +	struct thermal_cooling_device *cdev;
> +	struct cpumask mask_val;
> +
> +	/* make sure cpufreq driver has been initialized */
> +	if (!cpufreq_frequency_get_table(0))
> +		return -EPROBE_DEFER;
> +
> +	cpumask_set_cpu(0, &mask_val);
> +	cdev = cpufreq_cooling_register(&mask_val);
> +
> +	if (IS_ERR_OR_NULL(cdev)) {
> +		dev_err(&pdev->dev, "Failed to register cooling device\n");
> +		return PTR_ERR(cdev);
> +	}
> +
> +	platform_set_drvdata(pdev, cdev);
> +
> +	dev_info(&pdev->dev, "Cooling device registered: %s\n",	cdev-
> >type);
> +
> +	return 0;
> +}
> +
> +static int db8500_cpufreq_cooling_remove(struct platform_device *pdev)
> +{
> +	struct thermal_cooling_device *cdev = platform_get_drvdata(pdev);
> +
> +	cpufreq_cooling_unregister(cdev);
> +
> +	return 0;
> +}
> +
> +static int db8500_cpufreq_cooling_suspend(struct platform_device *pdev,
> +		pm_message_t state)
> +{
> +	return -ENOSYS;
> +}
> +
> +static int db8500_cpufreq_cooling_resume(struct platform_device *pdev)
> +{
> +	return -ENOSYS;
> +}
> +
> +#ifdef CONFIG_OF
> +static const struct of_device_id db8500_cpufreq_cooling_match[] = {
> +	{ .compatible = "stericsson,db8500-cpufreq-cooling" },
> +	{},
> +};
> +#else
> +#define db8500_cpufreq_cooling_match NULL #endif
> +
> +static struct platform_driver db8500_cpufreq_cooling_driver = {
> +	.driver = {
> +		.owner = THIS_MODULE,
> +		.name = "db8500-cpufreq-cooling",
> +		.of_match_table = db8500_cpufreq_cooling_match,
> +	},
> +	.probe = db8500_cpufreq_cooling_probe,
> +	.suspend = db8500_cpufreq_cooling_suspend,
> +	.resume = db8500_cpufreq_cooling_resume,
> +	.remove = db8500_cpufreq_cooling_remove, };
> +
> +static int __init db8500_cpufreq_cooling_init(void) {
> +	return platform_driver_register(&db8500_cpufreq_cooling_driver);
> +}
> +
> +static void __exit db8500_cpufreq_cooling_exit(void) {
> +	platform_driver_unregister(&db8500_cpufreq_cooling_driver);
> +}
> +
> +/* Should be later than db8500_cpufreq_register */
> +late_initcall(db8500_cpufreq_cooling_init);
> +module_exit(db8500_cpufreq_cooling_exit);
> +
> +MODULE_AUTHOR("Hongbo Zhang <hongbo.zhang-0IS4wlFg1OjSUeElwK9/Pw@public.gmane.org>");
> +MODULE_DESCRIPTION("DB8500 cpufreq cooling driver");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/thermal/db8500_thermal.c
> b/drivers/thermal/db8500_thermal.c
> new file mode 100644
> index 0000000..023f7b4
> --- /dev/null
> +++ b/drivers/thermal/db8500_thermal.c
> @@ -0,0 +1,531 @@
> +/*
> + * db8500_thermal.c - DB8500 Thermal Management Implementation
> + *
> + * Copyright (C) 2012 ST-Ericsson
> + * Copyright (C) 2012 Linaro Ltd.
> + *
> + * Author: Hongbo Zhang <hongbo.zhang-68IGFXMjmZ7QT0dZR+AlfA@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; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * 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.
> + */
> +
> +#include <linux/cpu_cooling.h>
> +#include <linux/interrupt.h>
> +#include <linux/mfd/dbx500-prcmu.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_data/db8500_thermal.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +#include <linux/thermal.h>
> +
> +#define PRCMU_DEFAULT_MEASURE_TIME	0xFFF
> +#define PRCMU_DEFAULT_LOW_TEMP		0
> +
> +struct db8500_thermal_zone {
> +	struct thermal_zone_device *therm_dev;
> +	struct mutex th_lock;
> +	struct work_struct therm_work;
> +	struct db8500_thsens_platform_data *trip_tab;
> +	enum thermal_device_mode mode;
> +	enum thermal_trend trend;
> +	unsigned long cur_temp_pseudo;
> +	unsigned int cur_index;
> +};
> +
> +/* Local function to check if thermal zone matches cooling devices */
> +static int db8500_thermal_match_cdev(struct thermal_cooling_device
> *cdev,
> +		struct db8500_trip_point *trip_points) {
> +	int i;
> +	char *cdev_name;
> +
> +	if (!strlen(cdev->type))
> +		return -EINVAL;
> +
> +	for (i = 0; i < COOLING_DEV_MAX; i++) {
> +		cdev_name = trip_points->cdev_name[i];
> +		if (!strcmp(cdev_name, cdev->type))
> +			return 0;
> +	}
> +
> +	return -ENODEV;
> +}
> +
> +/* Callback to bind cooling device to thermal zone */ static int
> +db8500_cdev_bind(struct thermal_zone_device *thermal,
> +		struct thermal_cooling_device *cdev)
> +{
> +	struct db8500_thermal_zone *pzone = thermal->devdata;
> +	struct db8500_thsens_platform_data *ptrips = pzone->trip_tab;
> +	unsigned long max_state, upper, lower;
> +	int i, ret = -EINVAL;
> +
> +	for (i = 0; i < ptrips->num_trips; i++) {
> +		if (db8500_thermal_match_cdev(cdev, &ptrips-
> >trip_points[i]))
> +			continue;
> +
> +		cdev->ops->get_max_state(cdev, &max_state);

Why not moving this line out of the loop
so that you only need to call it once.

> +		lower = upper = (i > max_state) ? max_state : i;
> +

What does this mean?
Say cooling device 0 matches trip point 3, and you want to use cooling state 3 only, for trip point 3?
No matter how many cooling states the cooling device supports?

Thanks,
rui
> +		ret = thermal_zone_bind_cooling_device(thermal, i, cdev,
> +			upper, lower);
> +
> +		dev_info(&cdev->device, "%s bind to %d: %d-%s\n",
> +			cdev->type, i, ret, ret ? "fail" : "succeed");
> +	}
> +
> +	return ret;
> +}
> +
> +/* Callback to unbind cooling device from thermal zone */ static int
> +db8500_cdev_unbind(struct thermal_zone_device *thermal,
> +		struct thermal_cooling_device *cdev)
> +{
> +	struct db8500_thermal_zone *pzone = thermal->devdata;
> +	struct db8500_thsens_platform_data *ptrips = pzone->trip_tab;
> +	int i, ret = -EINVAL;
> +
> +	for (i = 0; i < ptrips->num_trips; i++) {
> +		if (db8500_thermal_match_cdev(cdev, &ptrips-
> >trip_points[i]))
> +			continue;
> +
> +		ret = thermal_zone_unbind_cooling_device(thermal, i, cdev);
> +
> +		dev_info(&cdev->device, "%s unbind: %s\n",
> +			cdev->type, ret ? "fail" : "succeed");
> +	}
> +
> +	return ret;
> +}
> +
> +/* Callback to get current temperature */ static int
> +db8500_sys_get_temp(struct thermal_zone_device *thermal,
> +		unsigned long *temp)
> +{
> +	struct db8500_thermal_zone *pzone = thermal->devdata;
> +
> +	/*
> +	 * TODO: There is no PRCMU interface to get temperature data
> currently,
> +	 * so a pseudo temperature is returned , it works for thermal
> framework
> +	 * and this will be fixed when the PRCMU interface is available.
> +	 */
> +	*temp = pzone->cur_temp_pseudo;
> +
> +	return 0;
> +}
> +
> +/* Callback to get temperature changing trend */ static int
> +db8500_sys_get_trend(struct thermal_zone_device *thermal,
> +		int trip, enum thermal_trend *trend)
> +{
> +	struct db8500_thermal_zone *pzone = thermal->devdata;
> +
> +	*trend = pzone->trend;
> +
> +	return 0;
> +}
> +
> +/* Callback to get thermal zone mode */ static int
> +db8500_sys_get_mode(struct thermal_zone_device *thermal,
> +		enum thermal_device_mode *mode)
> +{
> +	struct db8500_thermal_zone *pzone = thermal->devdata;
> +
> +	mutex_lock(&pzone->th_lock);
> +	*mode = pzone->mode;
> +	mutex_unlock(&pzone->th_lock);
> +
> +	return 0;
> +}
> +
> +/* Callback to set thermal zone mode */ static int
> +db8500_sys_set_mode(struct thermal_zone_device *thermal,
> +		enum thermal_device_mode mode)
> +{
> +	struct db8500_thermal_zone *pzone = thermal->devdata;
> +
> +	mutex_lock(&pzone->th_lock);
> +
> +	pzone->mode = mode;
> +	if (mode == THERMAL_DEVICE_ENABLED)
> +		schedule_work(&pzone->therm_work);
> +
> +	mutex_unlock(&pzone->th_lock);
> +
> +	return 0;
> +}
> +
> +/* Callback to get trip point type */
> +static int db8500_sys_get_trip_type(struct thermal_zone_device
> *thermal,
> +		int trip, enum thermal_trip_type *type) {
> +	struct db8500_thermal_zone *pzone = thermal->devdata;
> +	struct db8500_thsens_platform_data *ptrips = pzone->trip_tab;
> +
> +	if (trip >= ptrips->num_trips)
> +		return -EINVAL;
> +
> +	*type = ptrips->trip_points[trip].type;
> +
> +	return 0;
> +}
> +
> +/* Callback to get trip point temperature */ static int
> +db8500_sys_get_trip_temp(struct thermal_zone_device *thermal,
> +		int trip, unsigned long *temp)
> +{
> +	struct db8500_thermal_zone *pzone = thermal->devdata;
> +	struct db8500_thsens_platform_data *ptrips = pzone->trip_tab;
> +
> +	if (trip >= ptrips->num_trips)
> +		return -EINVAL;
> +
> +	*temp = ptrips->trip_points[trip].temp;
> +
> +	return 0;
> +}
> +
> +/* Callback to get critical trip point temperature */ static int
> +db8500_sys_get_crit_temp(struct thermal_zone_device *thermal,
> +		unsigned long *temp)
> +{
> +	struct db8500_thermal_zone *pzone = thermal->devdata;
> +	struct db8500_thsens_platform_data *ptrips = pzone->trip_tab;
> +	int i;
> +
> +	for (i = ptrips->num_trips - 1; i > 0; i--) {
> +		if (ptrips->trip_points[i].type == THERMAL_TRIP_CRITICAL) {
> +			*temp = ptrips->trip_points[i].temp;
> +			return 0;
> +		}
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static struct thermal_zone_device_ops thdev_ops = {
> +	.bind = db8500_cdev_bind,
> +	.unbind = db8500_cdev_unbind,
> +	.get_temp = db8500_sys_get_temp,
> +	.get_trend = db8500_sys_get_trend,
> +	.get_mode = db8500_sys_get_mode,
> +	.set_mode = db8500_sys_set_mode,
> +	.get_trip_type = db8500_sys_get_trip_type,
> +	.get_trip_temp = db8500_sys_get_trip_temp,
> +	.get_crit_temp = db8500_sys_get_crit_temp, };
> +
> +static void db8500_thermal_update_config(struct db8500_thermal_zone
> *pzone,
> +		unsigned int idx, enum thermal_trend trend,
> +		unsigned long next_low, unsigned long next_high) {
> +	pzone->cur_index = idx;
> +	pzone->cur_temp_pseudo = (next_low + next_high)/2;
> +	pzone->trend = trend;
> +
> +	prcmu_stop_temp_sense();
> +	prcmu_config_hotmon((u8)(next_low/1000), (u8)(next_high/1000));
> +	prcmu_start_temp_sense(PRCMU_DEFAULT_MEASURE_TIME);
> +}
> +
> +static irqreturn_t prcmu_low_irq_handler(int irq, void *irq_data) {
> +	struct db8500_thermal_zone *pzone = irq_data;
> +	struct db8500_thsens_platform_data *ptrips = pzone->trip_tab;
> +	unsigned int idx = pzone->cur_index;
> +	unsigned long next_low, next_high;
> +
> +	if (unlikely(idx == 0))
> +		/* Meaningless for thermal management, ignoring it */
> +		return IRQ_HANDLED;
> +
> +	if (idx == 1) {
> +		next_high = ptrips->trip_points[0].temp;
> +		next_low = PRCMU_DEFAULT_LOW_TEMP;
> +	} else {
> +		next_high = ptrips->trip_points[idx-1].temp;
> +		next_low = ptrips->trip_points[idx-2].temp;
> +	}
> +	idx -= 1;
> +
> +	db8500_thermal_update_config(pzone, idx, THERMAL_TREND_DROPPING,
> +		next_low, next_high);
> +
> +	dev_dbg(&pzone->therm_dev->device,
> +		"PRCMU set max %ld, min %ld\n", next_high, next_low);
> +
> +	schedule_work(&pzone->therm_work);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static irqreturn_t prcmu_high_irq_handler(int irq, void *irq_data) {
> +	struct db8500_thermal_zone *pzone = irq_data;
> +	struct db8500_thsens_platform_data *ptrips = pzone->trip_tab;
> +	unsigned int idx = pzone->cur_index;
> +	unsigned long next_low, next_high;
> +
> +	if (idx < ptrips->num_trips - 1) {
> +		next_high = ptrips->trip_points[idx+1].temp;
> +		next_low = ptrips->trip_points[idx].temp;
> +		idx += 1;
> +
> +		db8500_thermal_update_config(pzone, idx,
> THERMAL_TREND_RAISING,
> +			next_low, next_high);
> +
> +		dev_dbg(&pzone->therm_dev->device,
> +		"PRCMU set max %ld, min %ld\n", next_high, next_low);
> +	} else if (idx == ptrips->num_trips - 1)
> +		pzone->cur_temp_pseudo = ptrips->trip_points[idx].temp + 1;
> +
> +	schedule_work(&pzone->therm_work);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static void db8500_thermal_work(struct work_struct *work) {
> +	enum thermal_device_mode cur_mode;
> +	struct db8500_thermal_zone *pzone;
> +
> +	pzone = container_of(work, struct db8500_thermal_zone,
> therm_work);
> +
> +	mutex_lock(&pzone->th_lock);
> +	cur_mode = pzone->mode;
> +	mutex_unlock(&pzone->th_lock);
> +
> +	if (cur_mode == THERMAL_DEVICE_DISABLED)
> +		return;
> +
> +	thermal_zone_device_update(pzone->therm_dev);
> +	dev_dbg(&pzone->therm_dev->device, "thermal work finished.\n"); }
> +
> +#ifdef CONFIG_OF
> +static struct db8500_thsens_platform_data*
> +		db8500_thermal_parse_dt(struct platform_device *pdev) {
> +	struct db8500_thsens_platform_data *ptrips;
> +	struct device_node *np = pdev->dev.of_node;
> +	char prop_name[32];
> +	const char *tmp_str;
> +	u32 tmp_data;
> +	int i, j;
> +
> +	ptrips = devm_kzalloc(&pdev->dev, sizeof(*ptrips), GFP_KERNEL);
> +	if (!ptrips)
> +		return NULL;
> +
> +	if (of_property_read_u32(np, "num-trips", &tmp_data))
> +		goto err_parse_dt;
> +
> +	if (tmp_data > THERMAL_MAX_TRIPS)
> +		goto err_parse_dt;
> +
> +	ptrips->num_trips = tmp_data;
> +
> +	for (i = 0; i < ptrips->num_trips; i++) {
> +		sprintf(prop_name, "trip%d-temp", i);
> +		if (of_property_read_u32(np, prop_name, &tmp_data))
> +			goto err_parse_dt;
> +
> +		ptrips->trip_points[i].temp = tmp_data;
> +
> +		sprintf(prop_name, "trip%d-type", i);
> +		if (of_property_read_string(np, prop_name, &tmp_str))
> +			goto err_parse_dt;
> +
> +		if (!strcmp(tmp_str, "active"))
> +			ptrips->trip_points[i].type = THERMAL_TRIP_ACTIVE;
> +		else if (!strcmp(tmp_str, "passive"))
> +			ptrips->trip_points[i].type = THERMAL_TRIP_PASSIVE;
> +		else if (!strcmp(tmp_str, "hot"))
> +			ptrips->trip_points[i].type = THERMAL_TRIP_HOT;
> +		else if (!strcmp(tmp_str, "critical"))
> +			ptrips->trip_points[i].type = THERMAL_TRIP_CRITICAL;
> +		else
> +			goto err_parse_dt;
> +
> +		sprintf(prop_name, "trip%d-cdev-num", i);
> +		if (of_property_read_u32(np, prop_name, &tmp_data))
> +			goto err_parse_dt;
> +
> +		if (tmp_data > COOLING_DEV_MAX)
> +			goto err_parse_dt;
> +
> +		for (j = 0; j < tmp_data; j++) {
> +			sprintf(prop_name, "trip%d-cdev-name%d", i, j);
> +			if (of_property_read_string(np, prop_name, &tmp_str))
> +				goto err_parse_dt;
> +
> +			if (strlen(tmp_str) > THERMAL_NAME_LENGTH)
> +				goto err_parse_dt;
> +
> +			strcpy(ptrips->trip_points[i].cdev_name[j], tmp_str);
> +		}
> +	}
> +	return ptrips;
> +
> +err_parse_dt:
> +	dev_err(&pdev->dev, "Parsing device tree data error.\n");
> +	return NULL;
> +}
> +#else
> +static inline struct db8500_thsens_platform_data*
> +		db8500_thermal_parse_dt(struct platform_device *pdev) {
> +	return NULL;
> +}
> +#endif
> +
> +static int db8500_thermal_probe(struct platform_device *pdev) {
> +	struct db8500_thermal_zone *pzone = NULL;
> +	struct db8500_thsens_platform_data *ptrips = NULL;
> +	struct device_node *np = pdev->dev.of_node;
> +	int low_irq, high_irq, ret = 0;
> +	unsigned long dft_low, dft_high;
> +
> +	if (np)
> +		ptrips = db8500_thermal_parse_dt(pdev);
> +	else
> +		ptrips = dev_get_platdata(&pdev->dev);
> +
> +	if (!ptrips)
> +		return -EINVAL;
> +
> +	pzone = devm_kzalloc(&pdev->dev, sizeof(*pzone), GFP_KERNEL);
> +	if (!pzone)
> +		return -ENOMEM;
> +
> +	mutex_init(&pzone->th_lock);
> +	mutex_lock(&pzone->th_lock);
> +
> +	pzone->mode = THERMAL_DEVICE_DISABLED;
> +	pzone->trip_tab = ptrips;
> +
> +	dft_low = PRCMU_DEFAULT_LOW_TEMP;
> +	dft_high = ptrips->trip_points[0].temp;
> +
> +	db8500_thermal_update_config(pzone, 0, THERMAL_TREND_STABLE,
> +		dft_low, dft_high);
> +
> +	INIT_WORK(&pzone->therm_work, db8500_thermal_work);
> +
> +	low_irq = platform_get_irq_byname(pdev, "IRQ_HOTMON_LOW");
> +	if (low_irq < 0) {
> +		dev_err(&pdev->dev, "Get IRQ_HOTMON_LOW failed.\n");
> +		return low_irq;
> +	}
> +
> +	ret = devm_request_threaded_irq(&pdev->dev, low_irq, NULL,
> +		prcmu_low_irq_handler, IRQF_NO_SUSPEND | IRQF_ONESHOT,
> +		"dbx500_temp_low", pzone);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "Failed to allocate temp low irq.\n");
> +		return ret;
> +	}
> +
> +	high_irq = platform_get_irq_byname(pdev, "IRQ_HOTMON_HIGH");
> +	if (high_irq < 0) {
> +		dev_err(&pdev->dev, "Get IRQ_HOTMON_HIGH failed.\n");
> +		return high_irq;
> +	}
> +
> +	ret = devm_request_threaded_irq(&pdev->dev, high_irq, NULL,
> +		prcmu_high_irq_handler, IRQF_NO_SUSPEND | IRQF_ONESHOT,
> +		"dbx500_temp_high", pzone);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "Failed to allocate temp high irq.\n");
> +		return ret;
> +	}
> +
> +	pzone->therm_dev =
> thermal_zone_device_register("db8500_thermal_zone",
> +		ptrips->num_trips, 0, pzone, &thdev_ops, 0, 0);
> +
> +	if (IS_ERR_OR_NULL(pzone->therm_dev)) {
> +		dev_err(&pdev->dev, "Register thermal zone device
> failed.\n");
> +		return PTR_ERR(pzone->therm_dev);
> +	}
> +	dev_info(&pdev->dev, "Thermal zone device registered.\n");
> +
> +	platform_set_drvdata(pdev, pzone);
> +	pzone->mode = THERMAL_DEVICE_ENABLED;
> +	mutex_unlock(&pzone->th_lock);
> +
> +	return 0;
> +}
> +
> +static int db8500_thermal_remove(struct platform_device *pdev) {
> +	struct db8500_thermal_zone *pzone = platform_get_drvdata(pdev);
> +
> +	thermal_zone_device_unregister(pzone->therm_dev);
> +	cancel_work_sync(&pzone->therm_work);
> +	mutex_destroy(&pzone->th_lock);
> +
> +	return 0;
> +}
> +
> +static int db8500_thermal_suspend(struct platform_device *pdev,
> +		pm_message_t state)
> +{
> +	struct db8500_thermal_zone *pzone = platform_get_drvdata(pdev);
> +
> +	flush_work_sync(&pzone->therm_work);
> +	prcmu_stop_temp_sense();
> +
> +	return 0;
> +}
> +
> +static int db8500_thermal_resume(struct platform_device *pdev) {
> +	struct db8500_thermal_zone *pzone = platform_get_drvdata(pdev);
> +	struct db8500_thsens_platform_data *ptrips = pzone->trip_tab;
> +	unsigned long dft_low, dft_high;
> +
> +	dft_low = PRCMU_DEFAULT_LOW_TEMP;
> +	dft_high = ptrips->trip_points[0].temp;
> +
> +	db8500_thermal_update_config(pzone, 0, THERMAL_TREND_STABLE,
> +		dft_low, dft_high);
> +
> +	return 0;
> +}
> +
> +#ifdef CONFIG_OF
> +static const struct of_device_id db8500_thermal_match[] = {
> +	{ .compatible = "stericsson,db8500-thermal" },
> +	{},
> +};
> +#else
> +#define db8500_thermal_match NULL
> +#endif
> +
> +static struct platform_driver db8500_thermal_driver = {
> +	.driver = {
> +		.owner = THIS_MODULE,
> +		.name = "db8500-thermal",
> +		.of_match_table = db8500_thermal_match,
> +	},
> +	.probe = db8500_thermal_probe,
> +	.suspend = db8500_thermal_suspend,
> +	.resume = db8500_thermal_resume,
> +	.remove = db8500_thermal_remove,
> +};
> +
> +module_platform_driver(db8500_thermal_driver);
> +
> +MODULE_AUTHOR("Hongbo Zhang <hongbo.zhang-0IS4wlFg1OjSUeElwK9/Pw@public.gmane.org>");
> +MODULE_DESCRIPTION("DB8500 thermal driver"); MODULE_LICENSE("GPL");
> diff --git a/include/linux/platform_data/db8500_thermal.h
> b/include/linux/platform_data/db8500_thermal.h
> new file mode 100644
> index 0000000..3bf6090
> --- /dev/null
> +++ b/include/linux/platform_data/db8500_thermal.h
> @@ -0,0 +1,38 @@
> +/*
> + * db8500_thermal.h - DB8500 Thermal Management Implementation
> + *
> + * Copyright (C) 2012 ST-Ericsson
> + * Copyright (C) 2012 Linaro Ltd.
> + *
> + * Author: Hongbo Zhang <hongbo.zhang-68IGFXMjmZ7QT0dZR+AlfA@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; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * 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.
> + */
> +
> +#ifndef _DB8500_THERMAL_H_
> +#define _DB8500_THERMAL_H_
> +
> +#include <linux/thermal.h>
> +
> +#define COOLING_DEV_MAX 8
> +
> +struct db8500_trip_point {
> +	unsigned long temp;
> +	enum thermal_trip_type type;
> +	char cdev_name[COOLING_DEV_MAX][THERMAL_NAME_LENGTH];
> +};
> +
> +struct db8500_thsens_platform_data {
> +	struct db8500_trip_point trip_points[THERMAL_MAX_TRIPS];
> +	int num_trips;
> +};
> +
> +#endif /* _DB8500_THERMAL_H_ */
> --
> 1.7.11.3

^ permalink raw reply

* Re: [PATCH v8 05/11] libata-eh: allow defer in ata_exec_internal
From: Aaron Lu @ 2012-11-01  2:35 UTC (permalink / raw)
  To: Tejun Heo
  Cc: Jeff Garzik, Rafael J. Wysocki, James Bottomley, Alan Stern,
	Oliver Neukum, Jeff Wu, Aaron Lu, Shane Huang, linux-ide,
	linux-pm, linux-scsi, linux-acpi
In-Reply-To: <20121031215213.GD2945@htj.dyndns.org>

On 11/01/2012 05:52 AM, Tejun Heo wrote:
> Hello,
> 
> On Tue, Oct 30, 2012 at 11:09:21AM +0800, Aaron Lu wrote:
>> I'm not aware of a place to store such ODD specific information when
>> probing the device.
> 
> You can always add some fields. :)

OK. My concern is that, such information is only useful to ZPODD
capable device+platforms, so checking this loading mechanism thing for
all ATAPI devices during probe time doesn't seem a good idea.

> 
>> I'm currently storing the loading mech type in structure zpodd, which
>> gets created after the corresponding SCSI device gets created in
>> ata_scsi_scan_host, so at the probe time, the zpodd structure does not
>> exist yet. And the reason I create the zpodd sturcture this late is
>> that, it is only created when the ODD together the platform is ZPODD
>> capable, and to find out if this platform is ZPODD capbale, ACPI binding
>> has to occur first, and ACPI binding happens when SCSI device is added
>> to the device tree.
> 
> Hmm... I see.  Which ACPI binding is it?  The ATA ACPI binding happens
> during probing.  It's a different one, I presume?

Since commit 6b66d95895c149cbc04d4fac5a2f5477c543a8ae:
libata: bind the Linux device tree to the ACPI device tree
ACPI binding happens when SCSI devices are added to the device tree. The
ata port/device software structure does not have a acpi_handle field
anymore.

Thanks,
Aaron


^ permalink raw reply

* Re: [PATCH v8 09/11] block: add a new interface to block events
From: Aaron Lu @ 2012-11-01  6:30 UTC (permalink / raw)
  To: Tejun Heo
  Cc: James Bottomley, Jeff Garzik, Rafael J. Wysocki, Alan Stern,
	Oliver Neukum, Jeff Wu, Aaron Lu, Shane Huang, linux-ide,
	linux-pm, linux-scsi, linux-acpi
In-Reply-To: <20121031215121.GC2945@htj.dyndns.org>

Hi,

On 11/01/2012 05:51 AM, Tejun Heo wrote:
> Hello,
> 
> On Tue, Oct 30, 2012 at 03:04:17PM +0800, Aaron Lu wrote:
>>> check_event() can retry.  Just add a per-sr mutex which is try-locked
>>> by sr_block_check_events() and grab it when entering zero power.
>>
>> Good suggestion. I didn't think about solving it this way.
>>
>> Many people suggest me that ZPODD is pure SATA/ACPI stuff, and should
>> not pollute sr driver, so I was trying hard not to touch sr while
>> preparing these patches, unless there is no other choice(like the
>> blocking event interface).
>>
>> So I'm not sure if your suggestion is the way to go.
>>
>> James, what do you think? Is it OK if I add a mutex into the scsi_cd
>> structure to do this? Of course I'll define this only under
>> CONFIG_SATA_ZPODD.
> 
> I don't think what James' and my suggestions are that different.  Just
> silence check_event() while zpodd is kicked in somehow.  There's no

Well, since sr is not supposed to know anything about ZPODD, I don't see
another way to silence check_event() in ATA layer.

What I hope to achieve is when zero power ready status is sensed in ATA
layer, no more sr_check_events should be called, as that function will
runtime resume the ODD.

Thanks,
Aaron

> reason to synchronize across multiple subsystems.
> 
> Thanks.
> 


^ permalink raw reply

* Re: [PATCH 5/5] ACPI: Add support for platform bus type
From: Yinghai Lu @ 2012-11-01  6:34 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: LKML, Linux PM list, Greg Kroah-Hartman, ACPI Devel Maling List,
	Len Brown, Mathias Nyman, Mika Westerberg, Lv Zheng, Huang Ying,
	Andy Shevchenko, H. Peter Anvin, x86 list, Tony Luck
In-Reply-To: <1992450.Ke1VSjeX8X@vostro.rjw.lan>

[-- Attachment #1: Type: text/plain, Size: 14965 bytes --]

On Wed, Oct 31, 2012 at 2:36 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> From: Mika Westerberg <mika.westerberg@linux.intel.com>
>
> With ACPI 5 it is now possible to enumerate traditional SoC
> peripherals, like serial bus controllers and slave devices behind
> them.  These devices are typically based on IP-blocks used in many
> existing SoC platforms and platform drivers for them may already
> be present in the kernel tree.
>
> To make driver "porting" more straightforward, add ACPI support to
> the platform bus type.  Instead of writing ACPI "glue" drivers for
> the existing platform drivers, register the platform bus type with
> ACPI to create platform device objects for the drivers and bind the
> corresponding ACPI handles to those platform devices.
>
> This should allow us to reuse the existing platform drivers for the
> devices in question with the minimum amount of modifications.
>
> This changeset is based on Mika Westerberg's and Mathias Nyman's
> work.
>
> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>  drivers/acpi/Makefile        |    1
>  drivers/acpi/acpi_platform.c |  285 +++++++++++++++++++++++++++++++++++++++++++
>  drivers/acpi/internal.h      |    7 +
>  drivers/acpi/scan.c          |   16 ++
>  drivers/base/platform.c      |    5
>  5 files changed, 313 insertions(+), 1 deletion(-)

this patch is too big, and should be split to at least two or more.

>  create mode 100644 drivers/acpi/acpi_platform.c
>
> Index: linux/drivers/acpi/Makefile
> ===================================================================
> --- linux.orig/drivers/acpi/Makefile
> +++ linux/drivers/acpi/Makefile
> @@ -37,6 +37,7 @@ acpi-y                                += processor_core.o
>  acpi-y                         += ec.o
>  acpi-$(CONFIG_ACPI_DOCK)       += dock.o
>  acpi-y                         += pci_root.o pci_link.o pci_irq.o pci_bind.o
> +acpi-y                         += acpi_platform.o
>  acpi-y                         += power.o
>  acpi-y                         += event.o
>  acpi-y                         += sysfs.o
> Index: linux/drivers/acpi/acpi_platform.c
> ===================================================================
> --- /dev/null
> +++ linux/drivers/acpi/acpi_platform.c
> @@ -0,0 +1,285 @@
> +/*
> + * ACPI support for platform bus type.
> + *
> + * Copyright (C) 2012, Intel Corporation
> + * Authors: Mika Westerberg <mika.westerberg@linux.intel.com>
> + *          Mathias Nyman <mathias.nyman@linux.intel.com>
> + *          Rafael J. Wysocki <rafael.j.wysocki@intel.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.
> + */
> +
> +#include <linux/acpi.h>
> +#include <linux/device.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +
> +ACPI_MODULE_NAME("platform");
> +
> +struct resource_info {
> +       struct device *dev;
> +       struct resource *res;
> +       size_t n, cur;
> +};
> +
> +static acpi_status acpi_platform_count_resources(struct acpi_resource *res,
> +                                                void *data)
> +{
> +       struct acpi_resource_extended_irq *acpi_xirq;
> +       struct resource_info *ri = data;
> +
> +       switch (res->type) {
> +       case ACPI_RESOURCE_TYPE_FIXED_MEMORY32:
> +       case ACPI_RESOURCE_TYPE_IRQ:
> +               ri->n++;
> +               break;
> +       case ACPI_RESOURCE_TYPE_EXTENDED_IRQ:
> +               acpi_xirq = &res->data.extended_irq;
> +               ri->n += acpi_xirq->interrupt_count;
> +               break;
> +       case ACPI_RESOURCE_TYPE_ADDRESS32:
> +               if (res->data.address32.resource_type == ACPI_IO_RANGE)
> +                       ri->n++;
> +               break;
> +       }
> +
> +       return AE_OK;
> +}
> +
> +static acpi_status acpi_platform_add_resources(struct acpi_resource *res,
> +                                              void *data)
> +{
> +       struct acpi_resource_fixed_memory32 *acpi_mem;
> +       struct acpi_resource_address32 *acpi_add32;
> +       struct acpi_resource_extended_irq *acpi_xirq;
> +       struct acpi_resource_irq *acpi_irq;
> +       struct resource_info *ri = data;
> +       struct resource *r;
> +       int irq, i;
> +
> +       switch (res->type) {
> +       case ACPI_RESOURCE_TYPE_FIXED_MEMORY32:
> +               acpi_mem = &res->data.fixed_memory32;
> +               r = &ri->res[ri->cur++];
> +
> +               r->start = acpi_mem->address;
> +               r->end = r->start + acpi_mem->address_length - 1;
> +               r->flags = IORESOURCE_MEM;
> +
> +               dev_dbg(ri->dev, "Memory32Fixed %pR\n", r);
> +               break;
> +
> +       case ACPI_RESOURCE_TYPE_ADDRESS32:
> +               acpi_add32 = &res->data.address32;
> +
> +               if (acpi_add32->resource_type == ACPI_IO_RANGE) {
> +                       r = &ri->res[ri->cur++];
> +                       r->start = acpi_add32->minimum;
> +                       r->end = r->start + acpi_add32->address_length - 1;
> +                       r->flags = IORESOURCE_IO;
> +                       dev_dbg(ri->dev, "Address32 %pR\n", r);
> +               }
> +               break;
> +
> +       case ACPI_RESOURCE_TYPE_IRQ:
> +               acpi_irq = &res->data.irq;
> +               r = &ri->res[ri->cur++];
> +
> +               irq = acpi_register_gsi(ri->dev,
> +                                       acpi_irq->interrupts[0],
> +                                       acpi_irq->triggering,
> +                                       acpi_irq->polarity);
> +
> +               r->start = r->end = irq;
> +               r->flags = IORESOURCE_IRQ;
> +
> +               dev_dbg(ri->dev, "IRQ %pR\n", r);
> +               break;
> +
> +       case ACPI_RESOURCE_TYPE_EXTENDED_IRQ:
> +               acpi_xirq = &res->data.extended_irq;
> +
> +               for (i = 0; i < acpi_xirq->interrupt_count; i++, ri->cur++) {
> +                       r = &ri->res[ri->cur];
> +                       irq = acpi_register_gsi(ri->dev,
> +                                               acpi_xirq->interrupts[i],
> +                                               acpi_xirq->triggering,
> +                                               acpi_xirq->polarity);
> +
> +                       r->start = r->end = irq;
> +                       r->flags = IORESOURCE_IRQ;
> +
> +                       dev_dbg(ri->dev, "Interrupt %pR\n", r);
> +               }
> +               break;
> +       }
> +
> +       return AE_OK;
> +}
> +

could be shared with pci root or pnp devices?

> +static acpi_status acpi_platform_get_device_uid(struct acpi_device *adev,
> +                                               int *uid)
> +{
> +       struct acpi_device_info *info;
> +       acpi_status status;
> +
> +       status = acpi_get_object_info(adev->handle, &info);
> +       if (ACPI_FAILURE(status))
> +               return status;
> +
> +       status = AE_NOT_EXIST;
> +       if ((info->valid & ACPI_VALID_UID) &&
> +            !kstrtoint(info->unique_id.string, 0, uid))
> +               status = AE_OK;
> +
> +       kfree(info);
> +       return status;
> +}
> +
> +/**
> + * acpi_create_platform_device - Create platform device for ACPI device node
> + * @adev: ACPI device node to create a platform device for.
> + *
> + * Check if the given @adev can be represented as a platform device and, if
> + * that's the case, create and register a platform device, populate its common
> + * resources and returns a pointer to it.  Otherwise, return %NULL.
> + *
> + * The platform device's name will be taken from the @adev's _HID and _UID.
> + */
> +struct platform_device *acpi_create_platform_device(struct acpi_device *adev)
> +{
> +       struct platform_device *pdev = NULL;
> +       struct acpi_device *acpi_parent;
> +       struct device *parent = NULL;
> +       struct resource_info ri;
> +       acpi_status status;
> +       int devid;
> +
> +       /* If the ACPI node already has a physical device attached, skip it. */
> +       if (adev->physical_node_count)
> +               return NULL;
> +
> +       /* Use the UID of the device as the new platform device id if found. */
> +       status = acpi_platform_get_device_uid(adev, &devid);
> +       if (ACPI_FAILURE(status))
> +               devid = -1;
> +
> +       memset(&ri, 0, sizeof(ri));
> +
> +       /* First, count the resources. */
> +       status = acpi_walk_resources(adev->handle, METHOD_NAME__CRS,
> +                                    acpi_platform_count_resources, &ri);
> +       if (ACPI_FAILURE(status) || !ri.n)
> +               return NULL;
> +
> +       /* Next, allocate memory for all the resources and populate it. */
> +       ri.dev = &adev->dev;
> +       ri.res = kzalloc(ri.n * sizeof(struct resource), GFP_KERNEL);
> +       if (!ri.res) {
> +               dev_err(&adev->dev,
> +                       "failed to allocate memory for resources\n");
> +               return NULL;
> +       }
> +
> +       status = acpi_walk_resources(adev->handle, METHOD_NAME__CRS,
> +                                    acpi_platform_add_resources, &ri);
> +       if (ACPI_FAILURE(status)) {
> +               dev_err(&adev->dev, "failed to walk resources\n");
> +               goto out;
> +       }
> +
> +       if (WARN_ON(ri.n != ri.cur))
> +               goto out;
> +
> +       /*
> +        * If the ACPI node has a parent and that parent has a physical device
> +        * attached to it, that physical device should be the parent of the
> +        * platform device we are about to create.
> +        */
> +       acpi_parent = adev->parent;
> +       if (acpi_parent) {
> +               struct acpi_device_physical_node *entry;
> +               struct list_head *list;
> +
> +               mutex_lock(&acpi_parent->physical_node_lock);
> +               list = &acpi_parent->physical_node_list;
> +               if (!list_empty(list)) {
> +                       entry = list_first_entry(list,
> +                                       struct acpi_device_physical_node,
> +                                       node);
> +                       parent = entry->dev;
> +               }
> +               mutex_unlock(&acpi_parent->physical_node_lock);
> +       }
> +       pdev = platform_device_register_resndata(parent, acpi_device_hid(adev),
> +                                                devid, ri.res, ri.n, NULL, 0);
> +       if (IS_ERR(pdev)) {
> +               dev_err(&adev->dev, "platform device creation failed: %ld\n",
> +                       PTR_ERR(pdev));
> +               pdev = NULL;
> +       } else {
> +               dev_dbg(&adev->dev, "created platform device %s\n",
> +                       dev_name(&pdev->dev));
> +       }
> +
> + out:
> +       kfree(ri.res);
> +       return pdev;
> +}
> +
> +static acpi_status acpi_platform_match(acpi_handle handle, u32 depth,
> +                                      void *data, void **return_value)
> +{
> +       struct platform_device *pdev = data;
> +       struct acpi_device *adev;
> +       acpi_status status;
> +
> +       status = acpi_bus_get_device(handle, &adev);
> +       if (ACPI_FAILURE(status))
> +               return status;
> +
> +       /* Skip ACPI devices that have physical device attached */
> +       if (adev->physical_node_count)
> +               return AE_OK;
> +
> +       if (!strcmp(pdev->name, acpi_device_hid(adev))) {
> +               int devid;
> +
> +               /* Check that both name and UID match if it exists */
> +               status = acpi_platform_get_device_uid(adev, &devid);
> +               if (ACPI_FAILURE(status))
> +                       devid = -1;
> +
> +               if (pdev->id != devid)
> +                       return AE_OK;
> +
> +               *(acpi_handle *)return_value = handle;
> +               return AE_CTRL_TERMINATE;
> +       }
> +
> +       return AE_OK;
> +}
> +
> +static int acpi_platform_find_device(struct device *dev, acpi_handle *handle)
> +{
> +       struct platform_device *pdev = to_platform_device(dev);
> +
> +       *handle = NULL;
> +       acpi_get_devices(pdev->name, acpi_platform_match, pdev, handle);
> +
> +       return *handle ? 0 : -ENODEV;
> +}
> +
> +static struct acpi_bus_type acpi_platform_bus = {
> +       .bus = &platform_bus_type,
> +       .find_device = acpi_platform_find_device,
> +};
> +
> +static int __init acpi_platform_init(void)
> +{
> +       return register_acpi_bus_type(&acpi_platform_bus);
> +}
> +arch_initcall(acpi_platform_init);
> Index: linux/drivers/acpi/internal.h
> ===================================================================
> --- linux.orig/drivers/acpi/internal.h
> +++ linux/drivers/acpi/internal.h
> @@ -93,4 +93,11 @@ static inline int suspend_nvs_save(void)
>  static inline void suspend_nvs_restore(void) {}
>  #endif
>
> +/*--------------------------------------------------------------------------
> +                               Platform bus support
> +  -------------------------------------------------------------------------- */
> +struct platform_device;
> +
> +struct platform_device *acpi_create_platform_device(struct acpi_device *adev);
> +
>  #endif /* _ACPI_INTERNAL_H_ */
> Index: linux/drivers/acpi/scan.c
> ===================================================================
> --- linux.orig/drivers/acpi/scan.c
> +++ linux/drivers/acpi/scan.c
> @@ -29,6 +29,15 @@ extern struct acpi_device *acpi_root;
>
>  static const char *dummy_hid = "device";
>
> +/*
> + * The following ACPI IDs are known to be suitable for representing as
> + * platform devices.
> + */
> +static const struct acpi_device_id acpi_platform_device_ids[] = {
> +

?

> +       { }
> +};
> +
>  static LIST_HEAD(acpi_device_list);
>  static LIST_HEAD(acpi_bus_id_list);
>  DEFINE_MUTEX(acpi_device_lock);
> @@ -1544,8 +1553,13 @@ static acpi_status acpi_bus_check_add(ac
>          */
>         device = NULL;
>         acpi_bus_get_device(handle, &device);
> -       if (ops->acpi_op_add && !device)
> +       if (ops->acpi_op_add && !device) {
>                 acpi_add_single_object(&device, handle, type, sta, ops);
> +               /* Is the device a known good platform device? */
> +               if (device
> +                   && !acpi_match_device_ids(device, acpi_platform_device_ids))
> +                       acpi_create_platform_device(device);
> +       }

That is ugly! any reason for not using acpi_driver for them.

there is not reason to treat those platform_device different from pci
root device and other pnp device.

something like attached patch. .add of acpi_driver ops could call
acpi_create_platform_device.

Yinghai

[-- Attachment #2: acpi_platform_driver.patch --]
[-- Type: application/octet-stream, Size: 2369 bytes --]

diff --git a/drivers/acpi/acpi_platform.c b/drivers/acpi/acpi_platform.c
index a5a2346..4ed97a7 100644
--- a/drivers/acpi/acpi_platform.c
+++ b/drivers/acpi/acpi_platform.c
@@ -230,6 +230,44 @@ struct platform_device *acpi_create_platform_device(struct acpi_device *adev)
 	return pdev;
 }
 
+/*
+ * The following ACPI IDs are known to be suitable for representing as
+ * platform devices.
+ */
+static const struct acpi_device_id acpi_platform_device_ids[] = {
+
+	{ }
+};
+
+#define ACPI_PLATFORM_CLASS		"acpi_platform"
+static int __devinit acpi_platform_device_add(struct acpi_device *device)
+{
+	acpi_create_platform_device(device);
+}
+static int acpi_platform_device_remove(struct acpi_device *device, int type)
+{
+	/* TODO: remove that platfrom_device and resource */
+}
+
+static struct acpi_driver acpi_platform_driver = {
+	.name = "acpi_platform",
+	.class = ACPI_PLATFORM_CLASS,
+	.ids = acpi_platform_device_ids,
+	.ops = {
+		.add = acpi_platform_device_add,
+		.remove = acpi_platform_device_remove,
+		},
+};
+
+static int __init acpi_platform_init(void)
+{
+	if (acpi_bus_register_driver(&acpi_platform_driver) < 0)
+		return -ENODEV;
+
+	return 0;
+}
+subsys_initcall(acpi_platform_init);
+
 static acpi_status acpi_platform_match(acpi_handle handle, u32 depth,
 				       void *data, void **return_value)
 {
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index e7afba1..73bfa3d 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -29,15 +29,6 @@ extern struct acpi_device *acpi_root;
 
 static const char *dummy_hid = "device";
 
-/*
- * The following ACPI IDs are known to be suitable for representing as
- * platform devices.
- */
-static const struct acpi_device_id acpi_platform_device_ids[] = {
-
-	{ }
-};
-
 static LIST_HEAD(acpi_device_list);
 static LIST_HEAD(acpi_bus_id_list);
 DEFINE_MUTEX(acpi_device_lock);
@@ -1553,13 +1544,8 @@ static acpi_status acpi_bus_check_add(acpi_handle handle, u32 lvl,
 	 */
 	device = NULL;
 	acpi_bus_get_device(handle, &device);
-	if (ops->acpi_op_add && !device) {
+	if (ops->acpi_op_add && !device)
 		acpi_add_single_object(&device, handle, type, sta, ops);
-		/* Is the device a known good platform device? */
-		if (device
-		    && !acpi_match_device_ids(device, acpi_platform_device_ids))
-			acpi_create_platform_device(device);
-	}
 
 	if (!device)
 		return AE_CTRL_DEPTH;

^ permalink raw reply related

* [PATCH] Thermal: exynos: Add support for temperature falling interrupt.
From: Jonghwa Lee @ 2012-11-01  7:47 UTC (permalink / raw)
  To: linux-pm
  Cc: linux-kernel, Len Brown, Durgadoss R, Rafael J. Wysocki,
	Amit Dinel Kachhap, MyungJoo Ham, Kyungmin Park, Jonghwa Lee

This patch introduces using temperature falling interrupt in exynos
thermal driver. Former patch, it only use polling way to check
whether if system themperature is fallen. However, exynos SOC also
provides temperature falling interrupt way to do same things by hw.
This feature is not supported in exynos4210.

Signed-off-by: Jonghwa Lee <jonghwa3.lee@samsung.com>
---
 drivers/thermal/exynos_thermal.c             |   81 +++++++++++++++-----------
 include/linux/platform_data/exynos_thermal.h |    3 +
 2 files changed, 49 insertions(+), 35 deletions(-)

diff --git a/drivers/thermal/exynos_thermal.c b/drivers/thermal/exynos_thermal.c
index 4bdd1eb..f91a699 100644
--- a/drivers/thermal/exynos_thermal.c
+++ b/drivers/thermal/exynos_thermal.c
@@ -94,6 +94,7 @@
 #define SENSOR_NAME_LEN	16
 #define MAX_TRIP_COUNT	8
 #define MAX_COOLING_DEVICE 4
+#define MAX_THRESHOLD_LEVS 4
 
 #define ACTIVE_INTERVAL 500
 #define IDLE_INTERVAL 10000
@@ -133,6 +134,7 @@ struct exynos_tmu_data {
 struct	thermal_trip_point_conf {
 	int trip_val[MAX_TRIP_COUNT];
 	int trip_count;
+	u8 trigger_falling;
 };
 
 struct	thermal_cooling_conf {
@@ -182,7 +184,8 @@ static int exynos_set_mode(struct thermal_zone_device *thermal,
 
 	mutex_lock(&th_zone->therm_dev->lock);
 
-	if (mode == THERMAL_DEVICE_ENABLED)
+	if (mode == THERMAL_DEVICE_ENABLED &&
+		!th_zone->sensor_conf->trip_data.trigger_falling)
 		th_zone->therm_dev->polling_delay = IDLE_INTERVAL;
 	else
 		th_zone->therm_dev->polling_delay = 0;
@@ -421,7 +424,8 @@ static void exynos_report_trigger(void)
 			break;
 	}
 
-	if (th_zone->mode == THERMAL_DEVICE_ENABLED) {
+	if (th_zone->mode == THERMAL_DEVICE_ENABLED &&
+		!th_zone->sensor_conf->trip_data.trigger_falling) {
 		if (i > 0)
 			th_zone->therm_dev->polling_delay = ACTIVE_INTERVAL;
 		else
@@ -460,7 +464,8 @@ static int exynos_register_thermal(struct thermal_sensor_conf *sensor_conf)
 
 	th_zone->therm_dev = thermal_zone_device_register(sensor_conf->name,
 			EXYNOS_ZONE_COUNT, 0, NULL, &exynos_dev_ops, 0,
-			IDLE_INTERVAL);
+			sensor_conf->trip_data.trigger_falling ?
+			0 : IDLE_INTERVAL);
 
 	if (IS_ERR(th_zone->therm_dev)) {
 		pr_err("Failed to register thermal zone device\n");
@@ -567,8 +572,9 @@ static int exynos_tmu_initialize(struct platform_device *pdev)
 {
 	struct exynos_tmu_data *data = platform_get_drvdata(pdev);
 	struct exynos_tmu_platform_data *pdata = data->pdata;
-	unsigned int status, trim_info, rising_threshold;
-	int ret = 0, threshold_code;
+	unsigned int status, trim_info;
+	unsigned int rising_threshold = 0, falling_threshold = 0;
+	int ret = 0, threshold_code, i, trigger_levs = 0;
 
 	mutex_lock(&data->lock);
 	clk_enable(data->clk);
@@ -593,6 +599,11 @@ static int exynos_tmu_initialize(struct platform_device *pdev)
 			(data->temp_error2 != 0))
 		data->temp_error1 = pdata->efuse_value;
 
+	/* Count trigger levels to be enabled */
+	for (i = 0; i < MAX_THRESHOLD_LEVS; i++)
+		if (pdata->trigger_levels[i])
+			trigger_levs++;
+
 	if (data->soc == SOC_ARCH_EXYNOS4210) {
 		/* Write temperature code for threshold */
 		threshold_code = temp_to_code(data, pdata->threshold);
@@ -602,44 +613,38 @@ static int exynos_tmu_initialize(struct platform_device *pdev)
 		}
 		writeb(threshold_code,
 			data->base + EXYNOS4210_TMU_REG_THRESHOLD_TEMP);
-
-		writeb(pdata->trigger_levels[0],
-			data->base + EXYNOS4210_TMU_REG_TRIG_LEVEL0);
-		writeb(pdata->trigger_levels[1],
-			data->base + EXYNOS4210_TMU_REG_TRIG_LEVEL1);
-		writeb(pdata->trigger_levels[2],
-			data->base + EXYNOS4210_TMU_REG_TRIG_LEVEL2);
-		writeb(pdata->trigger_levels[3],
-			data->base + EXYNOS4210_TMU_REG_TRIG_LEVEL3);
+		for (i = 0; i < trigger_levs; i++)
+			writeb(pdata->trigger_levels[i],
+			data->base + EXYNOS4210_TMU_REG_TRIG_LEVEL0 + i * 4);
 
 		writel(EXYNOS4210_TMU_INTCLEAR_VAL,
 			data->base + EXYNOS_TMU_REG_INTCLEAR);
 	} else if (data->soc == SOC_ARCH_EXYNOS) {
-		/* Write temperature code for threshold */
-		threshold_code = temp_to_code(data, pdata->trigger_levels[0]);
-		if (threshold_code < 0) {
-			ret = threshold_code;
-			goto out;
-		}
-		rising_threshold = threshold_code;
-		threshold_code = temp_to_code(data, pdata->trigger_levels[1]);
-		if (threshold_code < 0) {
-			ret = threshold_code;
-			goto out;
-		}
-		rising_threshold |= (threshold_code << 8);
-		threshold_code = temp_to_code(data, pdata->trigger_levels[2]);
-		if (threshold_code < 0) {
-			ret = threshold_code;
-			goto out;
+		/* Write temperature code for rising and falling threshold */
+		for (i = 0; i < trigger_levs; i++) {
+			threshold_code = temp_to_code(data,
+						pdata->trigger_levels[i]);
+			if (threshold_code < 0) {
+				ret = threshold_code;
+				goto out;
+			}
+			rising_threshold |= threshold_code;
+			if (pdata->threshold_falling) {
+				threshold_code = temp_to_code(data,
+						pdata->trigger_levels[i] -
+						pdata->threshold_falling);
+				if (threshold_code > 0)
+					falling_threshold |=
+						threshold_code << 8 * i;
+			}
 		}
-		rising_threshold |= (threshold_code << 16);
 
 		writel(rising_threshold,
 				data->base + EXYNOS_THD_TEMP_RISE);
-		writel(0, data->base + EXYNOS_THD_TEMP_FALL);
+		writel(falling_threshold,
+				data->base + EXYNOS_THD_TEMP_FALL);
 
-		writel(EXYNOS_TMU_CLEAR_RISE_INT|EXYNOS_TMU_CLEAR_FALL_INT,
+		writel(EXYNOS_TMU_CLEAR_RISE_INT | EXYNOS_TMU_CLEAR_FALL_INT,
 				data->base + EXYNOS_TMU_REG_INTCLEAR);
 	}
 out:
@@ -672,6 +677,8 @@ static void exynos_tmu_control(struct platform_device *pdev, bool on)
 			pdata->trigger_level2_en << 8 |
 			pdata->trigger_level1_en << 4 |
 			pdata->trigger_level0_en;
+		if (pdata->threshold_falling)
+			interrupt_en |= interrupt_en << 16;
 	} else {
 		con |= EXYNOS_TMU_CORE_OFF;
 		interrupt_en = 0; /* Disable all interrupts */
@@ -710,7 +717,8 @@ static void exynos_tmu_work(struct work_struct *work)
 
 
 	if (data->soc == SOC_ARCH_EXYNOS)
-		writel(EXYNOS_TMU_CLEAR_RISE_INT,
+		writel(EXYNOS_TMU_CLEAR_RISE_INT |
+				EXYNOS_TMU_CLEAR_FALL_INT,
 				data->base + EXYNOS_TMU_REG_INTCLEAR);
 	else
 		writel(EXYNOS4210_TMU_INTCLEAR_VAL,
@@ -767,6 +775,7 @@ static struct exynos_tmu_platform_data const exynos4210_default_tmu_data = {
 
 #if defined(CONFIG_SOC_EXYNOS5250) || defined(CONFIG_SOC_EXYNOS4412)
 static struct exynos_tmu_platform_data const exynos_default_tmu_data = {
+	.threshold_falling = 10,
 	.trigger_levels[0] = 85,
 	.trigger_levels[1] = 103,
 	.trigger_levels[2] = 110,
@@ -1002,6 +1011,8 @@ static int __devinit exynos_tmu_probe(struct platform_device *pdev)
 		exynos_sensor_conf.trip_data.trip_val[i] =
 			pdata->threshold + pdata->trigger_levels[i];
 
+	exynos_sensor_conf.trip_data.trigger_falling = pdata->threshold_falling;
+
 	exynos_sensor_conf.cooling_data.freq_clip_count =
 						pdata->freq_tab_count;
 	for (i = 0; i < pdata->freq_tab_count; i++) {
diff --git a/include/linux/platform_data/exynos_thermal.h b/include/linux/platform_data/exynos_thermal.h
index a7bdb2f..da7e627 100644
--- a/include/linux/platform_data/exynos_thermal.h
+++ b/include/linux/platform_data/exynos_thermal.h
@@ -53,6 +53,8 @@ struct freq_clip_table {
  * struct exynos_tmu_platform_data
  * @threshold: basic temperature for generating interrupt
  *	       25 <= threshold <= 125 [unit: degree Celsius]
+ * @threshold_falling: differntial value for setting threshold
+ *		       of temperature falling interrupt.
  * @trigger_levels: array for each interrupt levels
  *	[unit: degree Celsius]
  *	0: temperature for trigger_level0 interrupt
@@ -97,6 +99,7 @@ struct freq_clip_table {
  */
 struct exynos_tmu_platform_data {
 	u8 threshold;
+	u8 threshold_falling;
 	u8 trigger_levels[4];
 	bool trigger_level0_en;
 	bool trigger_level1_en;
-- 
1.7.4.1


^ permalink raw reply related

* RE: [PATCH v3] Thermal: exynos: Add sysfs node supporting exynos's emulation mode.
From: R, Durgadoss @ 2012-11-01  7:56 UTC (permalink / raw)
  To: Jonghwa Lee, linux-pm@vger.kernel.org
  Cc: linux-kernel@vger.kernel.org, Brown, Len, Rafael J. Wysocki,
	Amit Dinel Kachhap
In-Reply-To: <1351677237-18240-1-git-send-email-jonghwa3.lee@samsung.com>

Hi,

> +++ b/Documentation/thermal/exynos_thermal_emulation
> @@ -0,0 +1,49 @@
> +EXYNOS EMULATION MODE
> +========================
> +
> +Copyright (C) 2012 Samsung Electronics
> +
> +Writen by Jonghwa Lee <jonghwa3.lee@samsung.com>

s/Writen/Written

> +
> +Description
> +-----------
> +
> +Exynos 4x12 (4212, 4412) and 5 series provide emulation mode for thermal
> management unit.
> +Thermal emulation mode supports software debug for TMU's operation.
> User can set temperature
> +manually with software code and TMU will read current temperature from
> user value not from
> +sensor's value.
> +
> +Enabling CONFIG_EXYNOS_THERMAL_EMUL option will make this support
> in available.
> +When it's enabled, sysfs node will be created under
> +/sys/bus/platform/devices/'exynos device name'/ with name of
> 'emulation'.
> +
> +The sysfs node, 'emulation', will contain value 0 for the initial state. When
> you input any
> +temperature you want to update to sysfs node, it automatically enable
> emulation mode and
> +current temperature will be changed into it.
> +(Exynos also supports user changable delay time which would be used to
> delay of
> + changing temperature. However, this node only uses same delay of real
> sensing time, 938us.)
> +
> +Disabling emulation mode only requires writing value 0 to sysfs node.

This documentation looks fine. As a reply to one of the comments in the
previous version, you gave me an explanation. Could you please add that
explanation here as well ? I think that would help all of us, after this
patch set gets merged..

> +
> +
> +TEMP	120 |
> +	    |
> +	100 |
> +	    |
> +	 80 |
> +	    |		     	 	 +-----------
> +	 60 |      		     	 |	    |
> +	    |	           +-------------|          |
> +	 40 |              |         	 |          |
> +   	    |		   |	     	 |          |
> +	 20 |		   |	     	 |          +----------
> +	    |	 	   |	     	 |          |          |
> +  	  0
> |______________|_____________|__________|__________|_______
> __
> +		   A	    	 A	    A	   	       A     TIME
> +		   |<----->|	 |<----->|  |<----->|	       |
> +		   | 938us |  	 |	 |  |       |          |
> +emulation    :  0 50	   |  	 70      |  20      |          0
> +current temp :   sensor   50		 70         20	      sensor
> +
> +
> +
> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> index e1cb6bd..c02a66c 100644
> --- a/drivers/thermal/Kconfig
> +++ b/drivers/thermal/Kconfig
> @@ -55,3 +55,12 @@ config EXYNOS_THERMAL
>  	help
>  	  If you say yes here you get support for TMU (Thermal Managment
>  	  Unit) on SAMSUNG EXYNOS series of SoC.
> +
> +config EXYNOS_THERMAL_EMUL
> +	bool "EXYNOS TMU emulation mode support"
> +	depends on !CPU_EXYNOS4210 && EXYNOS_THERMAL
> +	help
> +	  Exynos 4412 and 4414 and 5 series has emulation mode on TMU.
> +	  Enable this option will be make sysfs node in exynos thermal
> platform
> +	  device directory to support emulation mode. With emulation mode
> sysfs
> +	  node, you can manually input temperature to TMU for simulation
> purpose.
> diff --git a/drivers/thermal/exynos_thermal.c
> b/drivers/thermal/exynos_thermal.c
> index fd03e85..4bdd1eb 100644
> --- a/drivers/thermal/exynos_thermal.c
> +++ b/drivers/thermal/exynos_thermal.c
> @@ -99,6 +99,14 @@
>  #define IDLE_INTERVAL 10000
>  #define MCELSIUS	1000
> 
> +#ifdef CONFIG_EXYNOS_THERMAL_EMUL
> +#define EXYNOS_EMUL_TIME	0x57F0
> +#define EXYNOS_EMUL_TIME_SHIFT	16
> +#define EXYNOS_EMUL_DATA_SHIFT	8
> +#define EXYNOS_EMUL_DATA_MASK	0xFF
> +#define EXYNOS_EMUL_ENABLE	0x1
> +#endif /* CONFIG_EXYNOS_THERMAL_EMUL */
> +
>  /* CPU Zone information */
>  #define PANIC_ZONE      4
>  #define WARN_ZONE       3
> @@ -832,6 +840,84 @@ static inline struct  exynos_tmu_platform_data
> *exynos_get_driver_data(
>  	return (struct exynos_tmu_platform_data *)
>  			platform_get_device_id(pdev)->driver_data;
>  }
> +
> +#ifdef CONFIG_EXYNOS_THERMAL_EMUL
> +static ssize_t exynos_tmu_emulation_show(struct device *dev,
> +					 struct device_attribute *attr,
> +					 char *buf)
> +{
> +	struct platform_device *pdev = container_of(dev,
> +					struct platform_device, dev);
> +	struct exynos_tmu_data *data = platform_get_drvdata(pdev);
> +	unsigned int reg;
> +	u8 temp_code;
> +	int temp = 0;
> +
> +	mutex_lock(&data->lock);
> +	clk_enable(data->clk);
> +	reg = readl(data->base + EXYNOS_EMUL_CON);
> +	clk_disable(data->clk);
> +	mutex_unlock(&data->lock);
> +
> +	if (reg & EXYNOS_EMUL_ENABLE) {
> +		reg >>= EXYNOS_EMUL_DATA_SHIFT;
> +		temp_code = reg & EXYNOS_EMUL_DATA_MASK;
> +		temp = code_to_temp(data, temp_code);
> +	} else {
> +		temp = 0;
> +	}

Do we still need the else part ?

> +
> +	return sprintf(buf, "%d\n", temp);
> +}
> +
> +static ssize_t exynos_tmu_emulation_store(struct device *dev,
> +					struct device_attribute *attr,
> +					const char *buf, size_t count)
> +{
> +	struct platform_device *pdev = container_of(dev,
> +					struct platform_device, dev);
> +	struct exynos_tmu_data *data = platform_get_drvdata(pdev);
> +	unsigned int reg;
> +	int temp = 0;

Looks like we don't need to assign 0 to temp here..

> +
> +	if (!sscanf(buf, "%d\n", &temp) || temp < 0)
> +		return -EINVAL;
> +
> +	mutex_lock(&data->lock);
> +	clk_enable(data->clk);
> +
> +	reg = readl(data->base + EXYNOS_EMUL_CON);
> +
> +	if (temp)
> +		reg = (EXYNOS_EMUL_TIME << EXYNOS_EMUL_TIME_SHIFT)
> |
> +			(temp_to_code(data, temp) <<
> EXYNOS_EMUL_DATA_SHIFT) |
> +			EXYNOS_EMUL_ENABLE;
> +	else
> +		reg &= ~EXYNOS_EMUL_ENABLE;
> +
> +	writel(reg, data->base + EXYNOS_EMUL_CON);
> +
> +	clk_disable(data->clk);
> +	mutex_unlock(&data->lock);
> +
> +	return count;
> +}
> +
> +static DEVICE_ATTR(emulation, 0644, exynos_tmu_emulation_show,
> +					exynos_tmu_emulation_store);
> +static int create_emulation_sysfs(struct device *dev)
> +{
> +	return device_create_file(dev, &dev_attr_emulation);
> +}
> +static void remove_emulation_sysfs(struct device *dev)
> +{
> +	device_remove_file(dev, &dev_attr_emulation);
> +}
> +#else
> +static inline int create_emulation_sysfs(struct device *dev) {return 0;}
> +static inline void remove_emulation_sysfs(struct device *dev){}
> +#endif
> +
>  static int __devinit exynos_tmu_probe(struct platform_device *pdev)
>  {
>  	struct exynos_tmu_data *data;
> @@ -930,6 +1016,11 @@ static int __devinit exynos_tmu_probe(struct
> platform_device *pdev)
>  		dev_err(&pdev->dev, "Failed to register thermal
> interface\n");
>  		goto err_clk;
>  	}
> +
> +	ret = create_emulation_sysfs(&pdev->dev);
> +	if (ret)
> +		dev_err(&pdev->dev, "Faield to create emulation mode

s/Faield/Failed

> sysfs node\n");
> +
>  	return 0;
>  err_clk:
>  	platform_set_drvdata(pdev, NULL);
> @@ -941,6 +1032,8 @@ static int __devexit exynos_tmu_remove(struct
> platform_device *pdev)
>  {
>  	struct exynos_tmu_data *data = platform_get_drvdata(pdev);
> 
> +	remove_emulation_sysfs(&pdev->dev);
> +

Yes, this makes it cleaner.

>  	exynos_tmu_control(pdev, false);
> 
>  	exynos_unregister_thermal();
> --
> 1.7.4.1


^ permalink raw reply

* Re: [PATCH v3] Thermal: exynos: Add sysfs node supporting exynos's emulation mode.
From: jonghwa3.lee @ 2012-11-01  8:10 UTC (permalink / raw)
  To: R, Durgadoss
  Cc: Jonghwa Lee, linux-pm@vger.kernel.org,
	linux-kernel@vger.kernel.org, Brown, Len, Rafael J. Wysocki,
	Amit Dinel Kachhap
In-Reply-To: <4D68720C2E767A4AA6A8796D42C8EB591FF32C@BGSMSX101.gar.corp.intel.com>

Hi,
On 2012년 11월 01일 16:56, R, Durgadoss wrote:
> Hi,
>
>> +++ b/Documentation/thermal/exynos_thermal_emulation
>> @@ -0,0 +1,49 @@
>> +EXYNOS EMULATION MODE
>> +========================
>> +
>> +Copyright (C) 2012 Samsung Electronics
>> +
>> +Writen by Jonghwa Lee <jonghwa3.lee@samsung.com>
> s/Writen/Written

Oops, Sorry ^^;

>> +
>> +Description
>> +-----------
>> +
>> +Exynos 4x12 (4212, 4412) and 5 series provide emulation mode for thermal
>> management unit.
>> +Thermal emulation mode supports software debug for TMU's operation.
>> User can set temperature
>> +manually with software code and TMU will read current temperature from
>> user value not from
>> +sensor's value.
>> +
>> +Enabling CONFIG_EXYNOS_THERMAL_EMUL option will make this support
>> in available.
>> +When it's enabled, sysfs node will be created under
>> +/sys/bus/platform/devices/'exynos device name'/ with name of
>> 'emulation'.
>> +
>> +The sysfs node, 'emulation', will contain value 0 for the initial state. When
>> you input any
>> +temperature you want to update to sysfs node, it automatically enable
>> emulation mode and
>> +current temperature will be changed into it.
>> +(Exynos also supports user changable delay time which would be used to
>> delay of
>> + changing temperature. However, this node only uses same delay of real
>> sensing time, 938us.)
>> +
>> +Disabling emulation mode only requires writing value 0 to sysfs node.
> This documentation looks fine. As a reply to one of the comments in the
> previous version, you gave me an explanation. Could you please add that
> explanation here as well ? I think that would help all of us, after this
> patch set gets merged..

Okay, I'll add that comments to the next patch.

>> +
>> +
>> +TEMP	120 |
>> +	    |
>> +	100 |
>> +	    |
>> +	 80 |
>> +	    |		     	 	 +-----------
>> +	 60 |      		     	 |	    |
>> +	    |	           +-------------|          |
>> +	 40 |              |         	 |          |
>> +   	    |		   |	     	 |          |
>> +	 20 |		   |	     	 |          +----------
>> +	    |	 	   |	     	 |          |          |
>> +  	  0
>> |______________|_____________|__________|__________|_______
>> __
>> +		   A	    	 A	    A	   	       A     TIME
>> +		   |<----->|	 |<----->|  |<----->|	       |
>> +		   | 938us |  	 |	 |  |       |          |
>> +emulation    :  0 50	   |  	 70      |  20      |          0
>> +current temp :   sensor   50		 70         20	      sensor
>> +
>> +
>> +
>> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
>> index e1cb6bd..c02a66c 100644
>> --- a/drivers/thermal/Kconfig
>> +++ b/drivers/thermal/Kconfig
>> @@ -55,3 +55,12 @@ config EXYNOS_THERMAL
>>  	help
>>  	  If you say yes here you get support for TMU (Thermal Managment
>>  	  Unit) on SAMSUNG EXYNOS series of SoC.
>> +
>> +config EXYNOS_THERMAL_EMUL
>> +	bool "EXYNOS TMU emulation mode support"
>> +	depends on !CPU_EXYNOS4210 && EXYNOS_THERMAL
>> +	help
>> +	  Exynos 4412 and 4414 and 5 series has emulation mode on TMU.
>> +	  Enable this option will be make sysfs node in exynos thermal
>> platform
>> +	  device directory to support emulation mode. With emulation mode
>> sysfs
>> +	  node, you can manually input temperature to TMU for simulation
>> purpose.
>> diff --git a/drivers/thermal/exynos_thermal.c
>> b/drivers/thermal/exynos_thermal.c
>> index fd03e85..4bdd1eb 100644
>> --- a/drivers/thermal/exynos_thermal.c
>> +++ b/drivers/thermal/exynos_thermal.c
>> @@ -99,6 +99,14 @@
>>  #define IDLE_INTERVAL 10000
>>  #define MCELSIUS	1000
>>
>> +#ifdef CONFIG_EXYNOS_THERMAL_EMUL
>> +#define EXYNOS_EMUL_TIME	0x57F0
>> +#define EXYNOS_EMUL_TIME_SHIFT	16
>> +#define EXYNOS_EMUL_DATA_SHIFT	8
>> +#define EXYNOS_EMUL_DATA_MASK	0xFF
>> +#define EXYNOS_EMUL_ENABLE	0x1
>> +#endif /* CONFIG_EXYNOS_THERMAL_EMUL */
>> +
>>  /* CPU Zone information */
>>  #define PANIC_ZONE      4
>>  #define WARN_ZONE       3
>> @@ -832,6 +840,84 @@ static inline struct  exynos_tmu_platform_data
>> *exynos_get_driver_data(
>>  	return (struct exynos_tmu_platform_data *)
>>  			platform_get_device_id(pdev)->driver_data;
>>  }
>> +
>> +#ifdef CONFIG_EXYNOS_THERMAL_EMUL
>> +static ssize_t exynos_tmu_emulation_show(struct device *dev,
>> +					 struct device_attribute *attr,
>> +					 char *buf)
>> +{
>> +	struct platform_device *pdev = container_of(dev,
>> +					struct platform_device, dev);
>> +	struct exynos_tmu_data *data = platform_get_drvdata(pdev);
>> +	unsigned int reg;
>> +	u8 temp_code;
>> +	int temp = 0;
>> +
>> +	mutex_lock(&data->lock);
>> +	clk_enable(data->clk);
>> +	reg = readl(data->base + EXYNOS_EMUL_CON);
>> +	clk_disable(data->clk);
>> +	mutex_unlock(&data->lock);
>> +
>> +	if (reg & EXYNOS_EMUL_ENABLE) {
>> +		reg >>= EXYNOS_EMUL_DATA_SHIFT;
>> +		temp_code = reg & EXYNOS_EMUL_DATA_MASK;
>> +		temp = code_to_temp(data, temp_code);
>> +	} else {
>> +		temp = 0;
>> +	}
> Do we still need the else part ?
>

No, we don't. I'll remove it.

>> +
>> +	return sprintf(buf, "%d\n", temp);
>> +}
>> +
>> +static ssize_t exynos_tmu_emulation_store(struct device *dev,
>> +					struct device_attribute *attr,
>> +					const char *buf, size_t count)
>> +{
>> +	struct platform_device *pdev = container_of(dev,
>> +					struct platform_device, dev);
>> +	struct exynos_tmu_data *data = platform_get_drvdata(pdev);
>> +	unsigned int reg;
>> +	int temp = 0;
> Looks like we don't need to assign 0 to temp here..

Yes, you're right.

>> +
>> +	if (!sscanf(buf, "%d\n", &temp) || temp < 0)
>> +		return -EINVAL;
>> +
>> +	mutex_lock(&data->lock);
>> +	clk_enable(data->clk);
>> +
>> +	reg = readl(data->base + EXYNOS_EMUL_CON);
>> +
>> +	if (temp)
>> +		reg = (EXYNOS_EMUL_TIME << EXYNOS_EMUL_TIME_SHIFT)
>> |
>> +			(temp_to_code(data, temp) <<
>> EXYNOS_EMUL_DATA_SHIFT) |
>> +			EXYNOS_EMUL_ENABLE;
>> +	else
>> +		reg &= ~EXYNOS_EMUL_ENABLE;
>> +
>> +	writel(reg, data->base + EXYNOS_EMUL_CON);
>> +
>> +	clk_disable(data->clk);
>> +	mutex_unlock(&data->lock);
>> +
>> +	return count;
>> +}
>> +
>> +static DEVICE_ATTR(emulation, 0644, exynos_tmu_emulation_show,
>> +					exynos_tmu_emulation_store);
>> +static int create_emulation_sysfs(struct device *dev)
>> +{
>> +	return device_create_file(dev, &dev_attr_emulation);
>> +}
>> +static void remove_emulation_sysfs(struct device *dev)
>> +{
>> +	device_remove_file(dev, &dev_attr_emulation);
>> +}
>> +#else
>> +static inline int create_emulation_sysfs(struct device *dev) {return 0;}
>> +static inline void remove_emulation_sysfs(struct device *dev){}
>> +#endif
>> +
>>  static int __devinit exynos_tmu_probe(struct platform_device *pdev)
>>  {
>>  	struct exynos_tmu_data *data;
>> @@ -930,6 +1016,11 @@ static int __devinit exynos_tmu_probe(struct
>> platform_device *pdev)
>>  		dev_err(&pdev->dev, "Failed to register thermal
>> interface\n");
>>  		goto err_clk;
>>  	}
>> +
>> +	ret = create_emulation_sysfs(&pdev->dev);
>> +	if (ret)
>> +		dev_err(&pdev->dev, "Faield to create emulation mode
> s/Faield/Failed

Typo again -_-;, sorry.

>> sysfs node\n");
>> +
>>  	return 0;
>>  err_clk:
>>  	platform_set_drvdata(pdev, NULL);
>> @@ -941,6 +1032,8 @@ static int __devexit exynos_tmu_remove(struct
>> platform_device *pdev)
>>  {
>>  	struct exynos_tmu_data *data = platform_get_drvdata(pdev);
>>
>> +	remove_emulation_sysfs(&pdev->dev);
>> +
> Yes, this makes it cleaner.
>
>>  	exynos_tmu_control(pdev, false);
>>
>>  	exynos_unregister_thermal();
>> --
>> 1.7.4.1
Thanks for reviewing my codes.
I'll apply your comments and re-post it as soon.

Thanks,
Jonghwa.

> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>


^ permalink raw reply

* [PATCH 2/2] PM / Qos: Add notifier for pm qos flags
From: Lan Tianyu @ 2012-11-01 10:47 UTC (permalink / raw)
  To: rjw, stern; +Cc: Lan Tianyu, linux-pm, linux-acpi
In-Reply-To: <1351766846-24654-1-git-send-email-tianyu.lan@intel.com>

If pm qos flags was changed, device driver would take different
action according flags. E.g NO_POWER_OFF flag, if it was set when
a device had been power off, device driver should power on the device,
resume it and suspend it again without not power off. So device driver
should be notified. This patch is to add notifier for pm qos flags and
notify device when flags is changed.

Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
---
 drivers/base/power/qos.c |   86 +++++++++++++++++++++++++++++++++++++---------
 include/linux/pm_qos.h   |    7 ++--
 2 files changed, 75 insertions(+), 18 deletions(-)

diff --git a/drivers/base/power/qos.c b/drivers/base/power/qos.c
index 31d3f48..c37f52f 100644
--- a/drivers/base/power/qos.c
+++ b/drivers/base/power/qos.c
@@ -149,6 +149,10 @@ static int apply_constraint(struct dev_pm_qos_request *req,
 	case DEV_PM_QOS_FLAGS:
 		ret = pm_qos_update_flags(&qos->flags, &req->data.flr,
 					  action, value);
+		if (ret)
+			blocking_notifier_call_chain(qos->flags.notifiers,
+				(unsigned long)qos->flags.effective_flags,
+				req);
 		break;
 	default:
 		ret = -EINVAL;
@@ -168,26 +172,38 @@ static int dev_pm_qos_constraints_allocate(struct device *dev)
 {
 	struct dev_pm_qos *qos;
 	struct pm_qos_constraints *c;
-	struct blocking_notifier_head *n;
+	struct pm_qos_flags	*flags;
+	struct blocking_notifier_head *constraints_notify;
+	struct blocking_notifier_head *flags_notify;
+	int ret = 0;
 
 	qos = kzalloc(sizeof(*qos), GFP_KERNEL);
 	if (!qos)
 		return -ENOMEM;
 
-	n = kzalloc(sizeof(*n), GFP_KERNEL);
-	if (!n) {
-		kfree(qos);
-		return -ENOMEM;
+	constraints_notify = kzalloc(sizeof(*constraints_notify), GFP_KERNEL);
+	if (!constraints_notify) {
+		ret = -ENOMEM;
+		goto alloc_constraints_fail;
+	}
+	BLOCKING_INIT_NOTIFIER_HEAD(constraints_notify);
+
+	flags_notify = kzalloc(sizeof(*flags_notify), GFP_KERNEL);
+	if (!flags_notify) {
+		ret = -ENOMEM;
+		goto alloc_flags_fail;
 	}
-	BLOCKING_INIT_NOTIFIER_HEAD(n);
+	BLOCKING_INIT_NOTIFIER_HEAD(flags_notify);
 
 	c = &qos->latency;
 	plist_head_init(&c->list);
 	c->target_value = PM_QOS_DEV_LAT_DEFAULT_VALUE;
 	c->default_value = PM_QOS_DEV_LAT_DEFAULT_VALUE;
 	c->type = PM_QOS_MIN;
-	c->notifiers = n;
+	c->notifiers = constraints_notify;
 
+	flags = &qos->flags;
+	flags->notifiers = flags_notify;
 	INIT_LIST_HEAD(&qos->flags.list);
 
 	spin_lock_irq(&dev->power.lock);
@@ -195,6 +211,12 @@ static int dev_pm_qos_constraints_allocate(struct device *dev)
 	spin_unlock_irq(&dev->power.lock);
 
 	return 0;
+
+alloc_flags_fail:
+	kfree(constraints_notify);
+alloc_constraints_fail:
+	kfree(qos);
+	return ret;
 }
 
 /**
@@ -432,6 +454,7 @@ EXPORT_SYMBOL_GPL(dev_pm_qos_remove_request);
  *
  * @dev: target device for the constraint
  * @notifier: notifier block managed by caller.
+ * @type: type of notifier
  *
  * Will register the notifier into a notification chain that gets called
  * upon changes to the target value for the device.
@@ -439,9 +462,11 @@ EXPORT_SYMBOL_GPL(dev_pm_qos_remove_request);
  * If the device's constraints object doesn't exist when this routine is called,
  * it will be created (or error code will be returned if that fails).
  */
-int dev_pm_qos_add_notifier(struct device *dev, struct notifier_block *notifier)
+int dev_pm_qos_add_notifier(struct device *dev, struct notifier_block *notifier,
+		enum dev_pm_qos_req_type type)
 {
 	int ret = 0;
+	struct blocking_notifier_head *notifiers_head = NULL;
 
 	mutex_lock(&dev_pm_qos_mtx);
 
@@ -449,9 +474,22 @@ int dev_pm_qos_add_notifier(struct device *dev, struct notifier_block *notifier)
 		ret = dev->power.power_state.event != PM_EVENT_INVALID ?
 			dev_pm_qos_constraints_allocate(dev) : -ENODEV;
 
-	if (!ret)
-		ret = blocking_notifier_chain_register(
-				dev->power.qos->latency.notifiers, notifier);
+	if (!ret) {
+		switch (type) {
+		case DEV_PM_QOS_LATENCY:
+			notifiers_head = dev->power.qos->latency.notifiers;
+			break;
+		case DEV_PM_QOS_FLAGS:
+			notifiers_head = dev->power.qos->flags.notifiers;
+			break;
+		default:
+			break;
+		}
+
+		if (notifiers_head)
+			ret = blocking_notifier_chain_register(
+					notifiers_head, notifier);
+	}
 
 	mutex_unlock(&dev_pm_qos_mtx);
 	return ret;
@@ -464,22 +502,38 @@ EXPORT_SYMBOL_GPL(dev_pm_qos_add_notifier);
  *
  * @dev: target device for the constraint
  * @notifier: notifier block to be removed.
+ * @type: type of notifier
  *
  * Will remove the notifier from the notification chain that gets called
  * upon changes to the target value.
  */
 int dev_pm_qos_remove_notifier(struct device *dev,
-			       struct notifier_block *notifier)
+		struct notifier_block *notifier, enum dev_pm_qos_req_type type)
 {
 	int retval = 0;
+	struct blocking_notifier_head *notifiers_head = NULL;
+
 
 	mutex_lock(&dev_pm_qos_mtx);
 
 	/* Silently return if the constraints object is not present. */
-	if (dev->power.qos)
-		retval = blocking_notifier_chain_unregister(
-				dev->power.qos->latency.notifiers,
-				notifier);
+	if (dev->power.qos) {
+		switch (type) {
+		case DEV_PM_QOS_LATENCY:
+			notifiers_head = dev->power.qos->latency.notifiers;
+			break;
+		case DEV_PM_QOS_FLAGS:
+			notifiers_head = dev->power.qos->flags.notifiers;
+			break;
+		default:
+			break;
+		}
+
+		if (notifiers_head)
+			retval = blocking_notifier_chain_unregister(
+					notifiers_head,
+					notifier);
+	}
 
 	mutex_unlock(&dev_pm_qos_mtx);
 	return retval;
diff --git a/include/linux/pm_qos.h b/include/linux/pm_qos.h
index 5a95013..479da6c 100644
--- a/include/linux/pm_qos.h
+++ b/include/linux/pm_qos.h
@@ -84,6 +84,7 @@ struct pm_qos_constraints {
 struct pm_qos_flags {
 	struct list_head list;
 	s32 effective_flags;	/* Do not change to 64 bit */
+	struct blocking_notifier_head *notifiers;
 };
 
 struct dev_pm_qos {
@@ -134,9 +135,11 @@ int dev_pm_qos_add_request(struct device *dev, struct dev_pm_qos_request *req,
 int dev_pm_qos_update_request(struct dev_pm_qos_request *req, s32 new_value);
 int dev_pm_qos_remove_request(struct dev_pm_qos_request *req);
 int dev_pm_qos_add_notifier(struct device *dev,
-			    struct notifier_block *notifier);
+			    struct notifier_block *notifier,
+			    enum dev_pm_qos_req_type type);
 int dev_pm_qos_remove_notifier(struct device *dev,
-			       struct notifier_block *notifier);
+				struct notifier_block *notifier,
+			    enum dev_pm_qos_req_type type);
 int dev_pm_qos_add_global_notifier(struct notifier_block *notifier);
 int dev_pm_qos_remove_global_notifier(struct notifier_block *notifier);
 void dev_pm_qos_constraints_init(struct device *dev);
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH 1/2] PM / Qos: Fix a free error in the dev_pm_qos_constraints_destroy()
From: Lan Tianyu @ 2012-11-01 10:47 UTC (permalink / raw)
  To: rjw, stern; +Cc: Lan Tianyu, linux-pm, linux-acpi

Free a wrong point to struct dev_pm_qos->latency which suppose to
be the point to struct dev_pm_qos. The patch is to fix the issue.

Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
---
 drivers/base/power/qos.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/base/power/qos.c b/drivers/base/power/qos.c
index 3a7687a..31d3f48 100644
--- a/drivers/base/power/qos.c
+++ b/drivers/base/power/qos.c
@@ -253,7 +253,7 @@ void dev_pm_qos_constraints_destroy(struct device *dev)
 	spin_unlock_irq(&dev->power.lock);
 
 	kfree(c->notifiers);
-	kfree(c);
+	kfree(qos);
 
  out:
 	mutex_unlock(&dev_pm_qos_mtx);
-- 
1.7.9.5


^ permalink raw reply related

* Re: [PATCH 2/2] PM / Qos: Add notifier for pm qos flags
From: Rafael J. Wysocki @ 2012-11-01 14:26 UTC (permalink / raw)
  To: Lan Tianyu; +Cc: stern, linux-pm, linux-acpi
In-Reply-To: <1351766846-24654-2-git-send-email-tianyu.lan@intel.com>

On Thursday, November 01, 2012 06:47:26 PM Lan Tianyu wrote:
> If pm qos flags was changed, device driver would take different
> action according flags. E.g NO_POWER_OFF flag, if it was set when
> a device had been power off, device driver should power on the device,
> resume it and suspend it again without not power off. So device driver
> should be notified. This patch is to add notifier for pm qos flags and
> notify device when flags is changed.
> 
> Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>

I haven't added notifiers for PM QoS flags on purpose, so I'm not going
to apply this patch.

Instead, it is required that callers of dev_pm_qos_update_request() for
DEV_PM_QOS_FLAGS type of requests ensure that the device is not RPM_SUSPENDED
when calling that routine (please note that the user interface calls
pm_runtime_get_sync() on the device before changing its flags).  Well, I think
I should add that to the kerneldoc comment of dev_pm_qos_update_request().

Moreover, I'm planning to remove the notifiers from the device PM QoS code
entirely and change the locking so that dev_pm_qos_update_request() and
friends may be called from interrupt context (which is necessary for some
use cases).

Thanks,
Rafael


> ---
>  drivers/base/power/qos.c |   86 +++++++++++++++++++++++++++++++++++++---------
>  include/linux/pm_qos.h   |    7 ++--
>  2 files changed, 75 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/base/power/qos.c b/drivers/base/power/qos.c
> index 31d3f48..c37f52f 100644
> --- a/drivers/base/power/qos.c
> +++ b/drivers/base/power/qos.c
> @@ -149,6 +149,10 @@ static int apply_constraint(struct dev_pm_qos_request *req,
>  	case DEV_PM_QOS_FLAGS:
>  		ret = pm_qos_update_flags(&qos->flags, &req->data.flr,
>  					  action, value);
> +		if (ret)
> +			blocking_notifier_call_chain(qos->flags.notifiers,
> +				(unsigned long)qos->flags.effective_flags,
> +				req);
>  		break;
>  	default:
>  		ret = -EINVAL;
> @@ -168,26 +172,38 @@ static int dev_pm_qos_constraints_allocate(struct device *dev)
>  {
>  	struct dev_pm_qos *qos;
>  	struct pm_qos_constraints *c;
> -	struct blocking_notifier_head *n;
> +	struct pm_qos_flags	*flags;
> +	struct blocking_notifier_head *constraints_notify;
> +	struct blocking_notifier_head *flags_notify;
> +	int ret = 0;
>  
>  	qos = kzalloc(sizeof(*qos), GFP_KERNEL);
>  	if (!qos)
>  		return -ENOMEM;
>  
> -	n = kzalloc(sizeof(*n), GFP_KERNEL);
> -	if (!n) {
> -		kfree(qos);
> -		return -ENOMEM;
> +	constraints_notify = kzalloc(sizeof(*constraints_notify), GFP_KERNEL);
> +	if (!constraints_notify) {
> +		ret = -ENOMEM;
> +		goto alloc_constraints_fail;
> +	}
> +	BLOCKING_INIT_NOTIFIER_HEAD(constraints_notify);
> +
> +	flags_notify = kzalloc(sizeof(*flags_notify), GFP_KERNEL);
> +	if (!flags_notify) {
> +		ret = -ENOMEM;
> +		goto alloc_flags_fail;
>  	}
> -	BLOCKING_INIT_NOTIFIER_HEAD(n);
> +	BLOCKING_INIT_NOTIFIER_HEAD(flags_notify);
>  
>  	c = &qos->latency;
>  	plist_head_init(&c->list);
>  	c->target_value = PM_QOS_DEV_LAT_DEFAULT_VALUE;
>  	c->default_value = PM_QOS_DEV_LAT_DEFAULT_VALUE;
>  	c->type = PM_QOS_MIN;
> -	c->notifiers = n;
> +	c->notifiers = constraints_notify;
>  
> +	flags = &qos->flags;
> +	flags->notifiers = flags_notify;
>  	INIT_LIST_HEAD(&qos->flags.list);
>  
>  	spin_lock_irq(&dev->power.lock);
> @@ -195,6 +211,12 @@ static int dev_pm_qos_constraints_allocate(struct device *dev)
>  	spin_unlock_irq(&dev->power.lock);
>  
>  	return 0;
> +
> +alloc_flags_fail:
> +	kfree(constraints_notify);
> +alloc_constraints_fail:
> +	kfree(qos);
> +	return ret;
>  }
>  
>  /**
> @@ -432,6 +454,7 @@ EXPORT_SYMBOL_GPL(dev_pm_qos_remove_request);
>   *
>   * @dev: target device for the constraint
>   * @notifier: notifier block managed by caller.
> + * @type: type of notifier
>   *
>   * Will register the notifier into a notification chain that gets called
>   * upon changes to the target value for the device.
> @@ -439,9 +462,11 @@ EXPORT_SYMBOL_GPL(dev_pm_qos_remove_request);
>   * If the device's constraints object doesn't exist when this routine is called,
>   * it will be created (or error code will be returned if that fails).
>   */
> -int dev_pm_qos_add_notifier(struct device *dev, struct notifier_block *notifier)
> +int dev_pm_qos_add_notifier(struct device *dev, struct notifier_block *notifier,
> +		enum dev_pm_qos_req_type type)
>  {
>  	int ret = 0;
> +	struct blocking_notifier_head *notifiers_head = NULL;
>  
>  	mutex_lock(&dev_pm_qos_mtx);
>  
> @@ -449,9 +474,22 @@ int dev_pm_qos_add_notifier(struct device *dev, struct notifier_block *notifier)
>  		ret = dev->power.power_state.event != PM_EVENT_INVALID ?
>  			dev_pm_qos_constraints_allocate(dev) : -ENODEV;
>  
> -	if (!ret)
> -		ret = blocking_notifier_chain_register(
> -				dev->power.qos->latency.notifiers, notifier);
> +	if (!ret) {
> +		switch (type) {
> +		case DEV_PM_QOS_LATENCY:
> +			notifiers_head = dev->power.qos->latency.notifiers;
> +			break;
> +		case DEV_PM_QOS_FLAGS:
> +			notifiers_head = dev->power.qos->flags.notifiers;
> +			break;
> +		default:
> +			break;
> +		}
> +
> +		if (notifiers_head)
> +			ret = blocking_notifier_chain_register(
> +					notifiers_head, notifier);
> +	}
>  
>  	mutex_unlock(&dev_pm_qos_mtx);
>  	return ret;
> @@ -464,22 +502,38 @@ EXPORT_SYMBOL_GPL(dev_pm_qos_add_notifier);
>   *
>   * @dev: target device for the constraint
>   * @notifier: notifier block to be removed.
> + * @type: type of notifier
>   *
>   * Will remove the notifier from the notification chain that gets called
>   * upon changes to the target value.
>   */
>  int dev_pm_qos_remove_notifier(struct device *dev,
> -			       struct notifier_block *notifier)
> +		struct notifier_block *notifier, enum dev_pm_qos_req_type type)
>  {
>  	int retval = 0;
> +	struct blocking_notifier_head *notifiers_head = NULL;
> +
>  
>  	mutex_lock(&dev_pm_qos_mtx);
>  
>  	/* Silently return if the constraints object is not present. */
> -	if (dev->power.qos)
> -		retval = blocking_notifier_chain_unregister(
> -				dev->power.qos->latency.notifiers,
> -				notifier);
> +	if (dev->power.qos) {
> +		switch (type) {
> +		case DEV_PM_QOS_LATENCY:
> +			notifiers_head = dev->power.qos->latency.notifiers;
> +			break;
> +		case DEV_PM_QOS_FLAGS:
> +			notifiers_head = dev->power.qos->flags.notifiers;
> +			break;
> +		default:
> +			break;
> +		}
> +
> +		if (notifiers_head)
> +			retval = blocking_notifier_chain_unregister(
> +					notifiers_head,
> +					notifier);
> +	}
>  
>  	mutex_unlock(&dev_pm_qos_mtx);
>  	return retval;
> diff --git a/include/linux/pm_qos.h b/include/linux/pm_qos.h
> index 5a95013..479da6c 100644
> --- a/include/linux/pm_qos.h
> +++ b/include/linux/pm_qos.h
> @@ -84,6 +84,7 @@ struct pm_qos_constraints {
>  struct pm_qos_flags {
>  	struct list_head list;
>  	s32 effective_flags;	/* Do not change to 64 bit */
> +	struct blocking_notifier_head *notifiers;
>  };
>  
>  struct dev_pm_qos {
> @@ -134,9 +135,11 @@ int dev_pm_qos_add_request(struct device *dev, struct dev_pm_qos_request *req,
>  int dev_pm_qos_update_request(struct dev_pm_qos_request *req, s32 new_value);
>  int dev_pm_qos_remove_request(struct dev_pm_qos_request *req);
>  int dev_pm_qos_add_notifier(struct device *dev,
> -			    struct notifier_block *notifier);
> +			    struct notifier_block *notifier,
> +			    enum dev_pm_qos_req_type type);
>  int dev_pm_qos_remove_notifier(struct device *dev,
> -			       struct notifier_block *notifier);
> +				struct notifier_block *notifier,
> +			    enum dev_pm_qos_req_type type);
>  int dev_pm_qos_add_global_notifier(struct notifier_block *notifier);
>  int dev_pm_qos_remove_global_notifier(struct notifier_block *notifier);
>  void dev_pm_qos_constraints_init(struct device *dev);
> 
-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

^ permalink raw reply

* Re: [PATCH 1/2] PM / Qos: Fix a free error in the dev_pm_qos_constraints_destroy()
From: Rafael J. Wysocki @ 2012-11-01 14:27 UTC (permalink / raw)
  To: Lan Tianyu; +Cc: stern, linux-pm, linux-acpi
In-Reply-To: <1351766846-24654-1-git-send-email-tianyu.lan@intel.com>

On Thursday, November 01, 2012 06:47:25 PM Lan Tianyu wrote:
> Free a wrong point to struct dev_pm_qos->latency which suppose to
> be the point to struct dev_pm_qos. The patch is to fix the issue.
> 
> Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>

Thanks, will apply.

Rafael


> ---
>  drivers/base/power/qos.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/base/power/qos.c b/drivers/base/power/qos.c
> index 3a7687a..31d3f48 100644
> --- a/drivers/base/power/qos.c
> +++ b/drivers/base/power/qos.c
> @@ -253,7 +253,7 @@ void dev_pm_qos_constraints_destroy(struct device *dev)
>  	spin_unlock_irq(&dev->power.lock);
>  
>  	kfree(c->notifiers);
> -	kfree(c);
> +	kfree(qos);
>  
>   out:
>  	mutex_unlock(&dev_pm_qos_mtx);
> 
-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

^ permalink raw reply

* Re: [PATCH 5/5] ACPI: Add support for platform bus type
From: Rafael J. Wysocki @ 2012-11-01 14:35 UTC (permalink / raw)
  To: Yinghai Lu
  Cc: LKML, Linux PM list, Greg Kroah-Hartman, ACPI Devel Maling List,
	Len Brown, Mathias Nyman, Mika Westerberg, Lv Zheng, Huang Ying,
	Andy Shevchenko, H. Peter Anvin, x86 list, Tony Luck
In-Reply-To: <CAE9FiQURGWOUzHc5j4pHCfcXE9y4a3WaPCEg=RewUpA=u8pSAQ@mail.gmail.com>

On Wednesday, October 31, 2012 11:34:24 PM Yinghai Lu wrote:
> On Wed, Oct 31, 2012 at 2:36 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> > From: Mika Westerberg <mika.westerberg@linux.intel.com>
> >
> > With ACPI 5 it is now possible to enumerate traditional SoC
> > peripherals, like serial bus controllers and slave devices behind
> > them.  These devices are typically based on IP-blocks used in many
> > existing SoC platforms and platform drivers for them may already
> > be present in the kernel tree.
> >
> > To make driver "porting" more straightforward, add ACPI support to
> > the platform bus type.  Instead of writing ACPI "glue" drivers for
> > the existing platform drivers, register the platform bus type with
> > ACPI to create platform device objects for the drivers and bind the
> > corresponding ACPI handles to those platform devices.
> >
> > This should allow us to reuse the existing platform drivers for the
> > devices in question with the minimum amount of modifications.
> >
> > This changeset is based on Mika Westerberg's and Mathias Nyman's
> > work.
> >
> > Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
> > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > ---
> >  drivers/acpi/Makefile        |    1
> >  drivers/acpi/acpi_platform.c |  285 +++++++++++++++++++++++++++++++++++++++++++
> >  drivers/acpi/internal.h      |    7 +
> >  drivers/acpi/scan.c          |   16 ++
> >  drivers/base/platform.c      |    5
> >  5 files changed, 313 insertions(+), 1 deletion(-)
> 
> this patch is too big, and should be split to at least two or more.
> 
> >  create mode 100644 drivers/acpi/acpi_platform.c
> >
> > Index: linux/drivers/acpi/Makefile
> > ===================================================================
> > --- linux.orig/drivers/acpi/Makefile
> > +++ linux/drivers/acpi/Makefile
> > @@ -37,6 +37,7 @@ acpi-y                                += processor_core.o
> >  acpi-y                         += ec.o
> >  acpi-$(CONFIG_ACPI_DOCK)       += dock.o
> >  acpi-y                         += pci_root.o pci_link.o pci_irq.o pci_bind.o
> > +acpi-y                         += acpi_platform.o
> >  acpi-y                         += power.o
> >  acpi-y                         += event.o
> >  acpi-y                         += sysfs.o
> > Index: linux/drivers/acpi/acpi_platform.c
> > ===================================================================
> > --- /dev/null
> > +++ linux/drivers/acpi/acpi_platform.c
> > @@ -0,0 +1,285 @@
> > +/*
> > + * ACPI support for platform bus type.
> > + *
> > + * Copyright (C) 2012, Intel Corporation
> > + * Authors: Mika Westerberg <mika.westerberg@linux.intel.com>
> > + *          Mathias Nyman <mathias.nyman@linux.intel.com>
> > + *          Rafael J. Wysocki <rafael.j.wysocki@intel.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.
> > + */
> > +
> > +#include <linux/acpi.h>
> > +#include <linux/device.h>
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/platform_device.h>
> > +
> > +ACPI_MODULE_NAME("platform");
> > +
> > +struct resource_info {
> > +       struct device *dev;
> > +       struct resource *res;
> > +       size_t n, cur;
> > +};
> > +
> > +static acpi_status acpi_platform_count_resources(struct acpi_resource *res,
> > +                                                void *data)
> > +{
> > +       struct acpi_resource_extended_irq *acpi_xirq;
> > +       struct resource_info *ri = data;
> > +
> > +       switch (res->type) {
> > +       case ACPI_RESOURCE_TYPE_FIXED_MEMORY32:
> > +       case ACPI_RESOURCE_TYPE_IRQ:
> > +               ri->n++;
> > +               break;
> > +       case ACPI_RESOURCE_TYPE_EXTENDED_IRQ:
> > +               acpi_xirq = &res->data.extended_irq;
> > +               ri->n += acpi_xirq->interrupt_count;
> > +               break;
> > +       case ACPI_RESOURCE_TYPE_ADDRESS32:
> > +               if (res->data.address32.resource_type == ACPI_IO_RANGE)
> > +                       ri->n++;
> > +               break;
> > +       }
> > +
> > +       return AE_OK;
> > +}
> > +
> > +static acpi_status acpi_platform_add_resources(struct acpi_resource *res,
> > +                                              void *data)
> > +{
> > +       struct acpi_resource_fixed_memory32 *acpi_mem;
> > +       struct acpi_resource_address32 *acpi_add32;
> > +       struct acpi_resource_extended_irq *acpi_xirq;
> > +       struct acpi_resource_irq *acpi_irq;
> > +       struct resource_info *ri = data;
> > +       struct resource *r;
> > +       int irq, i;
> > +
> > +       switch (res->type) {
> > +       case ACPI_RESOURCE_TYPE_FIXED_MEMORY32:
> > +               acpi_mem = &res->data.fixed_memory32;
> > +               r = &ri->res[ri->cur++];
> > +
> > +               r->start = acpi_mem->address;
> > +               r->end = r->start + acpi_mem->address_length - 1;
> > +               r->flags = IORESOURCE_MEM;
> > +
> > +               dev_dbg(ri->dev, "Memory32Fixed %pR\n", r);
> > +               break;
> > +
> > +       case ACPI_RESOURCE_TYPE_ADDRESS32:
> > +               acpi_add32 = &res->data.address32;
> > +
> > +               if (acpi_add32->resource_type == ACPI_IO_RANGE) {
> > +                       r = &ri->res[ri->cur++];
> > +                       r->start = acpi_add32->minimum;
> > +                       r->end = r->start + acpi_add32->address_length - 1;
> > +                       r->flags = IORESOURCE_IO;
> > +                       dev_dbg(ri->dev, "Address32 %pR\n", r);
> > +               }
> > +               break;
> > +
> > +       case ACPI_RESOURCE_TYPE_IRQ:
> > +               acpi_irq = &res->data.irq;
> > +               r = &ri->res[ri->cur++];
> > +
> > +               irq = acpi_register_gsi(ri->dev,
> > +                                       acpi_irq->interrupts[0],
> > +                                       acpi_irq->triggering,
> > +                                       acpi_irq->polarity);
> > +
> > +               r->start = r->end = irq;
> > +               r->flags = IORESOURCE_IRQ;
> > +
> > +               dev_dbg(ri->dev, "IRQ %pR\n", r);
> > +               break;
> > +
> > +       case ACPI_RESOURCE_TYPE_EXTENDED_IRQ:
> > +               acpi_xirq = &res->data.extended_irq;
> > +
> > +               for (i = 0; i < acpi_xirq->interrupt_count; i++, ri->cur++) {
> > +                       r = &ri->res[ri->cur];
> > +                       irq = acpi_register_gsi(ri->dev,
> > +                                               acpi_xirq->interrupts[i],
> > +                                               acpi_xirq->triggering,
> > +                                               acpi_xirq->polarity);
> > +
> > +                       r->start = r->end = irq;
> > +                       r->flags = IORESOURCE_IRQ;
> > +
> > +                       dev_dbg(ri->dev, "Interrupt %pR\n", r);
> > +               }
> > +               break;
> > +       }
> > +
> > +       return AE_OK;
> > +}
> > +
> 
> could be shared with pci root or pnp devices?

Maybe.

> > +static acpi_status acpi_platform_get_device_uid(struct acpi_device *adev,
> > +                                               int *uid)
> > +{
> > +       struct acpi_device_info *info;
> > +       acpi_status status;
> > +
> > +       status = acpi_get_object_info(adev->handle, &info);
> > +       if (ACPI_FAILURE(status))
> > +               return status;
> > +
> > +       status = AE_NOT_EXIST;
> > +       if ((info->valid & ACPI_VALID_UID) &&
> > +            !kstrtoint(info->unique_id.string, 0, uid))
> > +               status = AE_OK;
> > +
> > +       kfree(info);
> > +       return status;
> > +}
> > +
> > +/**
> > + * acpi_create_platform_device - Create platform device for ACPI device node
> > + * @adev: ACPI device node to create a platform device for.
> > + *
> > + * Check if the given @adev can be represented as a platform device and, if
> > + * that's the case, create and register a platform device, populate its common
> > + * resources and returns a pointer to it.  Otherwise, return %NULL.
> > + *
> > + * The platform device's name will be taken from the @adev's _HID and _UID.
> > + */
> > +struct platform_device *acpi_create_platform_device(struct acpi_device *adev)
> > +{
> > +       struct platform_device *pdev = NULL;
> > +       struct acpi_device *acpi_parent;
> > +       struct device *parent = NULL;
> > +       struct resource_info ri;
> > +       acpi_status status;
> > +       int devid;
> > +
> > +       /* If the ACPI node already has a physical device attached, skip it. */
> > +       if (adev->physical_node_count)
> > +               return NULL;
> > +
> > +       /* Use the UID of the device as the new platform device id if found. */
> > +       status = acpi_platform_get_device_uid(adev, &devid);
> > +       if (ACPI_FAILURE(status))
> > +               devid = -1;
> > +
> > +       memset(&ri, 0, sizeof(ri));
> > +
> > +       /* First, count the resources. */
> > +       status = acpi_walk_resources(adev->handle, METHOD_NAME__CRS,
> > +                                    acpi_platform_count_resources, &ri);
> > +       if (ACPI_FAILURE(status) || !ri.n)
> > +               return NULL;
> > +
> > +       /* Next, allocate memory for all the resources and populate it. */
> > +       ri.dev = &adev->dev;
> > +       ri.res = kzalloc(ri.n * sizeof(struct resource), GFP_KERNEL);
> > +       if (!ri.res) {
> > +               dev_err(&adev->dev,
> > +                       "failed to allocate memory for resources\n");
> > +               return NULL;
> > +       }
> > +
> > +       status = acpi_walk_resources(adev->handle, METHOD_NAME__CRS,
> > +                                    acpi_platform_add_resources, &ri);
> > +       if (ACPI_FAILURE(status)) {
> > +               dev_err(&adev->dev, "failed to walk resources\n");
> > +               goto out;
> > +       }
> > +
> > +       if (WARN_ON(ri.n != ri.cur))
> > +               goto out;
> > +
> > +       /*
> > +        * If the ACPI node has a parent and that parent has a physical device
> > +        * attached to it, that physical device should be the parent of the
> > +        * platform device we are about to create.
> > +        */
> > +       acpi_parent = adev->parent;
> > +       if (acpi_parent) {
> > +               struct acpi_device_physical_node *entry;
> > +               struct list_head *list;
> > +
> > +               mutex_lock(&acpi_parent->physical_node_lock);
> > +               list = &acpi_parent->physical_node_list;
> > +               if (!list_empty(list)) {
> > +                       entry = list_first_entry(list,
> > +                                       struct acpi_device_physical_node,
> > +                                       node);
> > +                       parent = entry->dev;
> > +               }
> > +               mutex_unlock(&acpi_parent->physical_node_lock);
> > +       }
> > +       pdev = platform_device_register_resndata(parent, acpi_device_hid(adev),
> > +                                                devid, ri.res, ri.n, NULL, 0);
> > +       if (IS_ERR(pdev)) {
> > +               dev_err(&adev->dev, "platform device creation failed: %ld\n",
> > +                       PTR_ERR(pdev));
> > +               pdev = NULL;
> > +       } else {
> > +               dev_dbg(&adev->dev, "created platform device %s\n",
> > +                       dev_name(&pdev->dev));
> > +       }
> > +
> > + out:
> > +       kfree(ri.res);
> > +       return pdev;
> > +}
> > +
> > +static acpi_status acpi_platform_match(acpi_handle handle, u32 depth,
> > +                                      void *data, void **return_value)
> > +{
> > +       struct platform_device *pdev = data;
> > +       struct acpi_device *adev;
> > +       acpi_status status;
> > +
> > +       status = acpi_bus_get_device(handle, &adev);
> > +       if (ACPI_FAILURE(status))
> > +               return status;
> > +
> > +       /* Skip ACPI devices that have physical device attached */
> > +       if (adev->physical_node_count)
> > +               return AE_OK;
> > +
> > +       if (!strcmp(pdev->name, acpi_device_hid(adev))) {
> > +               int devid;
> > +
> > +               /* Check that both name and UID match if it exists */
> > +               status = acpi_platform_get_device_uid(adev, &devid);
> > +               if (ACPI_FAILURE(status))
> > +                       devid = -1;
> > +
> > +               if (pdev->id != devid)
> > +                       return AE_OK;
> > +
> > +               *(acpi_handle *)return_value = handle;
> > +               return AE_CTRL_TERMINATE;
> > +       }
> > +
> > +       return AE_OK;
> > +}
> > +
> > +static int acpi_platform_find_device(struct device *dev, acpi_handle *handle)
> > +{
> > +       struct platform_device *pdev = to_platform_device(dev);
> > +
> > +       *handle = NULL;
> > +       acpi_get_devices(pdev->name, acpi_platform_match, pdev, handle);
> > +
> > +       return *handle ? 0 : -ENODEV;
> > +}
> > +
> > +static struct acpi_bus_type acpi_platform_bus = {
> > +       .bus = &platform_bus_type,
> > +       .find_device = acpi_platform_find_device,
> > +};
> > +
> > +static int __init acpi_platform_init(void)
> > +{
> > +       return register_acpi_bus_type(&acpi_platform_bus);
> > +}
> > +arch_initcall(acpi_platform_init);
> > Index: linux/drivers/acpi/internal.h
> > ===================================================================
> > --- linux.orig/drivers/acpi/internal.h
> > +++ linux/drivers/acpi/internal.h
> > @@ -93,4 +93,11 @@ static inline int suspend_nvs_save(void)
> >  static inline void suspend_nvs_restore(void) {}
> >  #endif
> >
> > +/*--------------------------------------------------------------------------
> > +                               Platform bus support
> > +  -------------------------------------------------------------------------- */
> > +struct platform_device;
> > +
> > +struct platform_device *acpi_create_platform_device(struct acpi_device *adev);
> > +
> >  #endif /* _ACPI_INTERNAL_H_ */
> > Index: linux/drivers/acpi/scan.c
> > ===================================================================
> > --- linux.orig/drivers/acpi/scan.c
> > +++ linux/drivers/acpi/scan.c
> > @@ -29,6 +29,15 @@ extern struct acpi_device *acpi_root;
> >
> >  static const char *dummy_hid = "device";
> >
> > +/*
> > + * The following ACPI IDs are known to be suitable for representing as
> > + * platform devices.
> > + */
> > +static const struct acpi_device_id acpi_platform_device_ids[] = {
> > +
> 
> ?

Yes, empty list.  We're going to populate it later.

> > +       { }
> > +};
> > +
> >  static LIST_HEAD(acpi_device_list);
> >  static LIST_HEAD(acpi_bus_id_list);
> >  DEFINE_MUTEX(acpi_device_lock);
> > @@ -1544,8 +1553,13 @@ static acpi_status acpi_bus_check_add(ac
> >          */
> >         device = NULL;
> >         acpi_bus_get_device(handle, &device);
> > -       if (ops->acpi_op_add && !device)
> > +       if (ops->acpi_op_add && !device) {
> >                 acpi_add_single_object(&device, handle, type, sta, ops);
> > +               /* Is the device a known good platform device? */
> > +               if (device
> > +                   && !acpi_match_device_ids(device, acpi_platform_device_ids))
> > +                       acpi_create_platform_device(device);
> > +       }
> 
> That is ugly! any reason for not using acpi_driver for them.

Yes, a couple of reasons.  First off, there are existing platform drivers for
these things already and there's no reason for creating a "glue" layer between
those drivers and struct acpi_device objects.  Second, we're going to get rid
of acpi_driver things entirely going forward (the existing ones will be replaced
by platform drivers or included into the ACPI core).

> there is not reason to treat those platform_device different from pci
> root device and other pnp device.

Well, I agree. :-)

So those things you're talking about we'll be platform devices too in the
future.

> something like attached patch. .add of acpi_driver ops could call
> acpi_create_platform_device.

Been there, decided to do it differently this time.

Thanks,
Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

^ permalink raw reply

* Re: [PATCH V3 4/5] Thermal: Add ST-Ericsson DB8500 thermal driver.
From: Francesco Lavra @ 2012-11-01 14:48 UTC (permalink / raw)
  To: hongbo.zhang
  Cc: linaro-dev, linux-kernel, linux-pm, rui.zhang, amit.kachhap,
	STEricsson_nomadik_linux, kernel, linaro-kernel, hongbo.zhang,
	patches
In-Reply-To: <1351615741-24134-5-git-send-email-hongbo.zhang@linaro.com>

Hi,

On 10/30/2012 05:49 PM, hongbo.zhang wrote:
> From: "hongbo.zhang" <hongbo.zhang@linaro.com>
> 
> This diver is based on the thermal management framework in thermal_sys.c. A
> thermal zone device is created with the trip points to which cooling devices
> can be bound, the current cooling device is cpufreq, e.g. CPU frequency is
> clipped down to cool the CPU, and other cooling devices can be added and bound
> to the trip points dynamically.  The platform specific PRCMU interrupts are
> used to active thermal update when trip points are reached.
> 
> Signed-off-by: hongbo.zhang <hongbo.zhang@linaro.com>

[...]
> +static struct db8500_thsens_platform_data*
> +		db8500_thermal_parse_dt(struct platform_device *pdev)
> +{
> +	struct db8500_thsens_platform_data *ptrips;
> +	struct device_node *np = pdev->dev.of_node;
> +	char prop_name[32];
> +	const char *tmp_str;
> +	u32 tmp_data;
> +	int i, j;
> +
> +	ptrips = devm_kzalloc(&pdev->dev, sizeof(*ptrips), GFP_KERNEL);
> +	if (!ptrips)
> +		return NULL;
> +
> +	if (of_property_read_u32(np, "num-trips", &tmp_data))
> +		goto err_parse_dt;
> +
> +	if (tmp_data > THERMAL_MAX_TRIPS)
> +		goto err_parse_dt;
> +
> +	ptrips->num_trips = tmp_data;
> +
> +	for (i = 0; i < ptrips->num_trips; i++) {
> +		sprintf(prop_name, "trip%d-temp", i);
> +		if (of_property_read_u32(np, prop_name, &tmp_data))
> +			goto err_parse_dt;
> +
> +		ptrips->trip_points[i].temp = tmp_data;
> +
> +		sprintf(prop_name, "trip%d-type", i);
> +		if (of_property_read_string(np, prop_name, &tmp_str))
> +			goto err_parse_dt;
> +
> +		if (!strcmp(tmp_str, "active"))
> +			ptrips->trip_points[i].type = THERMAL_TRIP_ACTIVE;
> +		else if (!strcmp(tmp_str, "passive"))
> +			ptrips->trip_points[i].type = THERMAL_TRIP_PASSIVE;
> +		else if (!strcmp(tmp_str, "hot"))
> +			ptrips->trip_points[i].type = THERMAL_TRIP_HOT;
> +		else if (!strcmp(tmp_str, "critical"))
> +			ptrips->trip_points[i].type = THERMAL_TRIP_CRITICAL;
> +		else
> +			goto err_parse_dt;
> +
> +		sprintf(prop_name, "trip%d-cdev-num", i);
> +		if (of_property_read_u32(np, prop_name, &tmp_data))
> +			goto err_parse_dt;
> +
> +		if (tmp_data > COOLING_DEV_MAX)
> +			goto err_parse_dt;
> +
> +		for (j = 0; j < tmp_data; j++) {
> +			sprintf(prop_name, "trip%d-cdev-name%d", i, j);
> +			if (of_property_read_string(np, prop_name, &tmp_str))
> +				goto err_parse_dt;
> +
> +			if (strlen(tmp_str) > THERMAL_NAME_LENGTH)
> +				goto err_parse_dt;
> +
> +			strcpy(ptrips->trip_points[i].cdev_name[j], tmp_str);

If strlen(tmp_str) equals THERMAL_NAME_LENGTH, strcpy() will go past the
size of the destination array.

After the above is fixed, you can add my:
Reviewed-by: Francesco Lavra <francescolavra.fl@gmail.com>

If you re-send a new version of the patch series, I suggest you do so in
a new thread.

Thanks,
Francesco

^ permalink raw reply

* Re: [PATCH v8 05/11] libata-eh: allow defer in ata_exec_internal
From: Tejun Heo @ 2012-11-01 16:03 UTC (permalink / raw)
  To: Aaron Lu
  Cc: Jeff Garzik, Rafael J. Wysocki, James Bottomley, Alan Stern,
	Oliver Neukum, Jeff Wu, Aaron Lu, Shane Huang, linux-ide,
	linux-pm, linux-scsi, linux-acpi
In-Reply-To: <5091DFDE.1060904@intel.com>

Hello, Aaron.

On Thu, Nov 01, 2012 at 10:35:10AM +0800, Aaron Lu wrote:
> > You can always add some fields. :)
> 
> OK. My concern is that, such information is only useful to ZPODD
> capable device+platforms, so checking this loading mechanism thing for
> all ATAPI devices during probe time doesn't seem a good idea.

Hmmm.. but it's not like querying acpi is high cost or anything.
Maybe I'm missing something but if it can be simpler that way, please
do so by all means.  I don't care whether you add some extra fields or
some processing overhead during probing.  It doesn't really matter.

> > Hmm... I see.  Which ACPI binding is it?  The ATA ACPI binding happens
> > during probing.  It's a different one, I presume?
> 
> Since commit 6b66d95895c149cbc04d4fac5a2f5477c543a8ae:
> libata: bind the Linux device tree to the ACPI device tree
> ACPI binding happens when SCSI devices are added to the device tree. The
> ata port/device software structure does not have a acpi_handle field
> anymore.

Please bear with me.  I haven't paid much attention to zpodd, so it
probably is my ignorance but at least the ATA <-> ACPI association
happens during probing by calling ata_acpi_on_devcfg() from
ata_dev_configure(), and it pretty much has to happen then because
_SDD/_GTF should be executed after hardreset which happens during
probing.  So, yeah, I'm confused.

Thanks.

-- 
tejun

^ permalink raw reply

* Re: [PATCH 5/5] ACPI: Add support for platform bus type
From: Yinghai Lu @ 2012-11-01 16:19 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: LKML, Linux PM list, Greg Kroah-Hartman, ACPI Devel Maling List,
	Len Brown, Mathias Nyman, Mika Westerberg, Lv Zheng, Huang Ying,
	Andy Shevchenko, H. Peter Anvin, x86 list, Tony Luck
In-Reply-To: <14394066.j8A8VXQzU7@vostro.rjw.lan>

On Thu, Nov 1, 2012 at 7:35 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
>> > @@ -1544,8 +1553,13 @@ static acpi_status acpi_bus_check_add(ac
>> >          */
>> >         device = NULL;
>> >         acpi_bus_get_device(handle, &device);
>> > -       if (ops->acpi_op_add && !device)
>> > +       if (ops->acpi_op_add && !device) {
>> >                 acpi_add_single_object(&device, handle, type, sta, ops);
>> > +               /* Is the device a known good platform device? */
>> > +               if (device
>> > +                   && !acpi_match_device_ids(device, acpi_platform_device_ids))
>> > +                       acpi_create_platform_device(device);
>> > +       }
>>
>> That is ugly! any reason for not using acpi_driver for them.
>
> Yes, a couple of reasons.  First off, there are existing platform drivers for
> these things already and there's no reason for creating a "glue" layer between
> those drivers and struct acpi_device objects.  Second, we're going to get rid
> of acpi_driver things entirely going forward (the existing ones will be replaced
> by platform drivers or included into the ACPI core).

that should be glue between acpi_device and platform_device.

how are you going to handle removing path ? aka when acpi_device get
trimed, how those platform get deleted?

>
>> there is not reason to treat those platform_device different from pci
>> root device and other pnp device.
>
> Well, I agree. :-)
>
> So those things you're talking about we'll be platform devices too in the
> future.
>
>> something like attached patch. .add of acpi_driver ops could call
>> acpi_create_platform_device.
>
> Been there, decided to do it differently this time.
>

So you are going to replace acpi_device/acpi_driver with
platform_device/platform_driver ?

Did you ever try to start to the work to see if it is doable? aka do
you have draft version to do that?

Yinghai

^ permalink raw reply

* Re: [PATCH 5/5] ACPI: Add support for platform bus type
From: Rafael J. Wysocki @ 2012-11-01 21:21 UTC (permalink / raw)
  To: Yinghai Lu
  Cc: LKML, Linux PM list, Greg Kroah-Hartman, ACPI Devel Maling List,
	Len Brown, Mathias Nyman, Mika Westerberg, Lv Zheng, Huang Ying,
	Andy Shevchenko, H. Peter Anvin, x86 list, Tony Luck,
	Matthew Garrett, Zhang Rui
In-Reply-To: <CAE9FiQUXk9AcR87B8PyLoW3JJveZMd7SqB9pDmfA8F320SCLng@mail.gmail.com>

On Thursday, November 01, 2012 09:19:59 AM Yinghai Lu wrote:
> On Thu, Nov 1, 2012 at 7:35 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> >> > @@ -1544,8 +1553,13 @@ static acpi_status acpi_bus_check_add(ac
> >> >          */
> >> >         device = NULL;
> >> >         acpi_bus_get_device(handle, &device);
> >> > -       if (ops->acpi_op_add && !device)
> >> > +       if (ops->acpi_op_add && !device) {
> >> >                 acpi_add_single_object(&device, handle, type, sta, ops);
> >> > +               /* Is the device a known good platform device? */
> >> > +               if (device
> >> > +                   && !acpi_match_device_ids(device, acpi_platform_device_ids))
> >> > +                       acpi_create_platform_device(device);
> >> > +       }
> >>
> >> That is ugly! any reason for not using acpi_driver for them.
> >
> > Yes, a couple of reasons.  First off, there are existing platform drivers for
> > these things already and there's no reason for creating a "glue" layer between
> > those drivers and struct acpi_device objects.  Second, we're going to get rid
> > of acpi_driver things entirely going forward (the existing ones will be replaced
> > by platform drivers or included into the ACPI core).
> 
> that should be glue between acpi_device and platform_device.
> 
> how are you going to handle removing path ? aka when acpi_device get
> trimed, how those platform get deleted?

This is a valid point.

Roughly, the idea is to walk the dev's list of physical nodes and call
.remove() for each of them in acpi_bus_remove().  Or to do something
equivalent to that.

However, we're not going to add any IDs of removable devices to
acpi_platform_device_ids[] for now, so we simply don't need to modify that
code path just yet (we'll modify it when we're about to add such devices to
that table).

> >> there is not reason to treat those platform_device different from pci
> >> root device and other pnp device.
> >
> > Well, I agree. :-)
> >
> > So those things you're talking about we'll be platform devices too in the
> > future.
> >
> >> something like attached patch. .add of acpi_driver ops could call
> >> acpi_create_platform_device.
> >
> > Been there, decided to do it differently this time.
> >
> 
> So you are going to replace acpi_device/acpi_driver with
> platform_device/platform_driver ?

Not exactly.  Let me start from the big picture, though. :-)

First off, we need to unify the handling of devices by the ACPI subsystem
regardless of whether they are on a specific bus, like PCI, or they are
busless "platform" devices.

Currently, if a device is on a specific bus *and* there is a device node in the
ACPI namespace corresponding to it, there will be two objects based on
struct device for it eventually, the "physical node", like struct pci_dev, and
the "ACPI node" represented by struct acpi_device.  They are associated with
each other through the code in drivers/acpi/glue.c.  In turn, if the device is
busless and not discoverable natively, we only create the "ACPI node" struct
acpi_device thing for it.  Those ACPI nodes are then *sometimes* bind to
drivers (represented by struct acpi_driver).

The fact that the busless devices are *sometimes* handled by binding drivers
directly to struct acpi_device while the other devices are handled through
glue.c confuses things substantially and causes problems to happen right now
(for example, acpi_driver drivers sometimes attempt to bind to things that have
other native drivers and should really be handled by them).
Furthermore, the situation will only get worse over time if we don't do
anything about that, because we're going to see more and more devices that
won't be discoverable natively and will have corresponding nodes in the ACPI
namespace and we're going to see more buses whose devices will have such
nodes.

Moreover, for many of those devices there are native drivers present in
the kernel tree already, because they will be based on IP blocks used in
the current hardware (for example, we may see ARM-based systems based on
exactly the same hardware with ACPI BIOSes and without them).  That applies
to busless devices as well as to devices on specific buses.

Now, the problem is how the unification is going to be done and I honestly
don't think we have much *choice* here.  Namely, for PCI (and other devices
discoverable natively) we pretty much have to do the glue.c thing (or something
equivalent), because we need to match what we've discovered natively against
the information from the ACPI tables in the BIOS.  This means that for busless
devices we need to create "physical" nodes as well, so that all of them are
handled by drivers binding to the "physical" node rather than to struct
acpi_device.  This also will allow us to reuse the existing drivers with
minimum modifications (well, hopefully).

Pretty much every ACPI developer I have discussed that with so far, including
people like Matthew Garrett and Zhang Rui who have been working on ACPI for
years, seems to agree that this is the way to go.

Thus, in the long run, acpi_driver drivers will be replaced by something else
(platform drivers seem to be a natural choice in many cases), but struct
acpi_device objects won't go away, at least not entirely.  My long haul plan
is to drop the "dev" component of struct acpi_device and rename it to something
like struct acpi_dev_node, to clarify its meaning.

I'm quite confident that this is doable.  The part I'm most concerned about is
the interactions with user space, because we need to pay attention not to break
the existing user space interfaces (that are actually used).  That said, I'm
not expecting this to be a one-shot transition.  Rather, this is going to take
time, like several kernel releases, and I'm sure there are details we need to
be very careful about.

That's one of the reasons why acpi_platform_device_ids[] is there: so that we
can carefully carry out the unification step by step.

> Did you ever try to start to the work to see if it is doable? aka do
> you have draft version to do that?

Unfortunately, I don't have anything I could share with you at the moment.

Thanks,
Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

^ permalink raw reply

* Re: [PATCH 5/5] ACPI: Add support for platform bus type
From: Yinghai Lu @ 2012-11-01 22:38 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: LKML, Linux PM list, Greg Kroah-Hartman, ACPI Devel Maling List,
	Len Brown, Mathias Nyman, Mika Westerberg, Lv Zheng, Huang Ying,
	Andy Shevchenko, H. Peter Anvin, x86 list, Tony Luck,
	Matthew Garrett, Zhang Rui
In-Reply-To: <9809109.C0U00708CM@vostro.rjw.lan>

On Thu, Nov 1, 2012 at 2:21 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
>> So you are going to replace acpi_device/acpi_driver with
>> platform_device/platform_driver ?
>
> Not exactly.  Let me start from the big picture, though. :-)
>
> First off, we need to unify the handling of devices by the ACPI subsystem
> regardless of whether they are on a specific bus, like PCI, or they are
> busless "platform" devices.
>
> Currently, if a device is on a specific bus *and* there is a device node in the
> ACPI namespace corresponding to it, there will be two objects based on
> struct device for it eventually, the "physical node", like struct pci_dev, and
> the "ACPI node" represented by struct acpi_device.  They are associated with
> each other through the code in drivers/acpi/glue.c.  In turn, if the device is
> busless and not discoverable natively, we only create the "ACPI node" struct
> acpi_device thing for it.  Those ACPI nodes are then *sometimes* bind to
> drivers (represented by struct acpi_driver).
>
> The fact that the busless devices are *sometimes* handled by binding drivers
> directly to struct acpi_device while the other devices are handled through
> glue.c confuses things substantially and causes problems to happen right now
> (for example, acpi_driver drivers sometimes attempt to bind to things that have
> other native drivers and should really be handled by them).
> Furthermore, the situation will only get worse over time if we don't do
> anything about that, because we're going to see more and more devices that
> won't be discoverable natively and will have corresponding nodes in the ACPI
> namespace and we're going to see more buses whose devices will have such
> nodes.
>
> Moreover, for many of those devices there are native drivers present in
> the kernel tree already, because they will be based on IP blocks used in
> the current hardware (for example, we may see ARM-based systems based on
> exactly the same hardware with ACPI BIOSes and without them).  That applies
> to busless devices as well as to devices on specific buses.
>
> Now, the problem is how the unification is going to be done and I honestly
> don't think we have much *choice* here.  Namely, for PCI (and other devices
> discoverable natively) we pretty much have to do the glue.c thing (or something
> equivalent), because we need to match what we've discovered natively against
> the information from the ACPI tables in the BIOS.  This means that for busless
> devices we need to create "physical" nodes as well, so that all of them are
> handled by drivers binding to the "physical" node rather than to struct
> acpi_device.  This also will allow us to reuse the existing drivers with
> minimum modifications (well, hopefully).

ok, acpi_driver will be killed at first.

acpi_pci_root_driver will be converted to platform driver or
add acpi_pci_host_bridge to work with pci_host_bridge.

BTW,  the problem for hotadd pci root bus,
the acpi_driver ops.add can pci root bus and create pci dev before all
acpi device get
created still there.
    https://lkml.org/lkml/2012/10/5/569

^ permalink raw reply

* [PATCH] PM / QoS: Document request manipulation requirement for flags
From: Rafael J. Wysocki @ 2012-11-01 23:21 UTC (permalink / raw)
  To: Lan Tianyu, linux-pm; +Cc: stern, linux-acpi
In-Reply-To: <3164424.M2MHcDsd3M@vostro.rjw.lan>

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

In fact, the callers of dev_pm_qos_add_request(),
dev_pm_qos_update_request() and dev_pm_qos_remove_request() for
requests of type DEV_PM_QOS_FLAGS need to ensure that the target
device is not RPM_SUSPENDED before using any of these functions (or
be prepared for the new PM QoS flags to take effect after the device
has been resumed).  Document this in their kerneldoc comments.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/base/power/qos.c |    9 +++++++++
 1 file changed, 9 insertions(+)

Index: linux/drivers/base/power/qos.c
===================================================================
--- linux.orig/drivers/base/power/qos.c
+++ linux/drivers/base/power/qos.c
@@ -277,6 +277,9 @@ void dev_pm_qos_constraints_destroy(stru
  * -EINVAL in case of wrong parameters, -ENOMEM if there's not enough memory
  * to allocate for data structures, -ENODEV if the device has just been removed
  * from the system.
+ *
+ * Callers should ensure that the target device is not RPM_SUSPENDED before
+ * using this function for requests of type DEV_PM_QOS_FLAGS.
  */
 int dev_pm_qos_add_request(struct device *dev, struct dev_pm_qos_request *req,
 			   enum dev_pm_qos_req_type type, s32 value)
@@ -367,6 +370,9 @@ static int __dev_pm_qos_update_request(s
  * 0 if the aggregated constraint value has not changed,
  * -EINVAL in case of wrong parameters, -ENODEV if the device has been
  * removed from the system
+ *
+ * Callers should ensure that the target device is not RPM_SUSPENDED before
+ * using this function for requests of type DEV_PM_QOS_FLAGS.
  */
 int dev_pm_qos_update_request(struct dev_pm_qos_request *req, s32 new_value)
 {
@@ -398,6 +404,9 @@ EXPORT_SYMBOL_GPL(dev_pm_qos_update_requ
  * 0 if the aggregated constraint value has not changed,
  * -EINVAL in case of wrong parameters, -ENODEV if the device has been
  * removed from the system
+ *
+ * Callers should ensure that the target device is not RPM_SUSPENDED before
+ * using this function for requests of type DEV_PM_QOS_FLAGS.
  */
 int dev_pm_qos_remove_request(struct dev_pm_qos_request *req)
 {


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

^ permalink raw reply

* Re: [PATCH 5/5] ACPI: Add support for platform bus type
From: Rafael J. Wysocki @ 2012-11-01 23:17 UTC (permalink / raw)
  To: Yinghai Lu
  Cc: LKML, Linux PM list, Greg Kroah-Hartman, ACPI Devel Maling List,
	Len Brown, Mathias Nyman, Mika Westerberg, Lv Zheng, Huang Ying,
	Andy Shevchenko, H. Peter Anvin, x86 list, Tony Luck,
	Matthew Garrett, Zhang Rui
In-Reply-To: <CAE9FiQWiRcaC3dKQFUHoC0qe0TneW2T+CYNZPEic5At_QcP0rw@mail.gmail.com>

On Thursday, November 01, 2012 03:38:19 PM Yinghai Lu wrote:
> On Thu, Nov 1, 2012 at 2:21 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> >> So you are going to replace acpi_device/acpi_driver with
> >> platform_device/platform_driver ?
> >
> > Not exactly.  Let me start from the big picture, though. :-)
> >
> > First off, we need to unify the handling of devices by the ACPI subsystem
> > regardless of whether they are on a specific bus, like PCI, or they are
> > busless "platform" devices.
> >
> > Currently, if a device is on a specific bus *and* there is a device node in the
> > ACPI namespace corresponding to it, there will be two objects based on
> > struct device for it eventually, the "physical node", like struct pci_dev, and
> > the "ACPI node" represented by struct acpi_device.  They are associated with
> > each other through the code in drivers/acpi/glue.c.  In turn, if the device is
> > busless and not discoverable natively, we only create the "ACPI node" struct
> > acpi_device thing for it.  Those ACPI nodes are then *sometimes* bind to
> > drivers (represented by struct acpi_driver).
> >
> > The fact that the busless devices are *sometimes* handled by binding drivers
> > directly to struct acpi_device while the other devices are handled through
> > glue.c confuses things substantially and causes problems to happen right now
> > (for example, acpi_driver drivers sometimes attempt to bind to things that have
> > other native drivers and should really be handled by them).
> > Furthermore, the situation will only get worse over time if we don't do
> > anything about that, because we're going to see more and more devices that
> > won't be discoverable natively and will have corresponding nodes in the ACPI
> > namespace and we're going to see more buses whose devices will have such
> > nodes.
> >
> > Moreover, for many of those devices there are native drivers present in
> > the kernel tree already, because they will be based on IP blocks used in
> > the current hardware (for example, we may see ARM-based systems based on
> > exactly the same hardware with ACPI BIOSes and without them).  That applies
> > to busless devices as well as to devices on specific buses.
> >
> > Now, the problem is how the unification is going to be done and I honestly
> > don't think we have much *choice* here.  Namely, for PCI (and other devices
> > discoverable natively) we pretty much have to do the glue.c thing (or something
> > equivalent), because we need to match what we've discovered natively against
> > the information from the ACPI tables in the BIOS.  This means that for busless
> > devices we need to create "physical" nodes as well, so that all of them are
> > handled by drivers binding to the "physical" node rather than to struct
> > acpi_device.  This also will allow us to reuse the existing drivers with
> > minimum modifications (well, hopefully).
> 
> ok, acpi_driver will be killed at first.
> 
> acpi_pci_root_driver will be converted to platform driver or
> add acpi_pci_host_bridge to work with pci_host_bridge.

Yup.

> BTW,  the problem for hotadd pci root bus,
> the acpi_driver ops.add can pci root bus and create pci dev before all
> acpi device get
> created still there.
>     https://lkml.org/lkml/2012/10/5/569

Yes, I'm aware of that, it's on my todo list FWIW. :-)

Thanks,
Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

^ permalink raw reply

* Re: [PATCH v8 05/11] libata-eh: allow defer in ata_exec_internal
From: Aaron Lu @ 2012-11-02  0:43 UTC (permalink / raw)
  To: Tejun Heo
  Cc: Jeff Garzik, Rafael J. Wysocki, James Bottomley, Alan Stern,
	Oliver Neukum, Jeff Wu, Aaron Lu, Shane Huang, linux-ide,
	linux-pm, linux-scsi, linux-acpi
In-Reply-To: <20121101160300.GB9169@htj.dyndns.org>

On 11/02/2012 12:03 AM, Tejun Heo wrote:
> Hello, Aaron.
> 
> On Thu, Nov 01, 2012 at 10:35:10AM +0800, Aaron Lu wrote:
>>> You can always add some fields. :)
>>
>> OK. My concern is that, such information is only useful to ZPODD
>> capable device+platforms, so checking this loading mechanism thing for
>> all ATAPI devices during probe time doesn't seem a good idea.
> 
> Hmmm.. but it's not like querying acpi is high cost or anything.
> Maybe I'm missing something but if it can be simpler that way, please
> do so by all means.  I don't care whether you add some extra fields or
> some processing overhead during probing.  It doesn't really matter.

OK, thanks for the suggestion.

> 
>>> Hmm... I see.  Which ACPI binding is it?  The ATA ACPI binding happens
>>> during probing.  It's a different one, I presume?
>>
>> Since commit 6b66d95895c149cbc04d4fac5a2f5477c543a8ae:
>> libata: bind the Linux device tree to the ACPI device tree
>> ACPI binding happens when SCSI devices are added to the device tree. The
>> ata port/device software structure does not have a acpi_handle field
>> anymore.
> 
> Please bear with me.  I haven't paid much attention to zpodd, so it
> probably is my ignorance but at least the ATA <-> ACPI association
> happens during probing by calling ata_acpi_on_devcfg() from
> ata_dev_configure(), and it pretty much has to happen then because
> _SDD/_GTF should be executed after hardreset which happens during
> probing.  So, yeah, I'm confused.

Oh, yes, ACPI handle can be retrieved any time by quering some ACPI
method. And during probe time, it is done that way, while the binding
happens much later... But the ACPI handle can indeed be fetched at that
time :-)
So all the conditions I need to test ZPODD support is ready during probe
time and I'll move the check loading mechanism code there, thanks for
your suggestion.

Thanks,
Aaron


^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox