* Re: [PATCH v9 06/10] ata: zpodd: check zero power ready status
From: Jeff Garzik @ 2012-12-03 18:56 UTC (permalink / raw)
To: Tejun Heo
Cc: James Bottomley, Aaron Lu, Rafael J. Wysocki, linux-pm,
Alan Stern, Jeff Wu, Aaron Lu, linux-ide, linux-scsi, linux-acpi
In-Reply-To: <20121203162323.GB19802@htj.dyndns.org>
On 12/03/2012 11:23 AM, Tejun Heo wrote:
> Hello, James.
>
> On Mon, Dec 03, 2012 at 08:25:43AM +0000, James Bottomley wrote:
>>> diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
>>> index e65c62e..1756151 100644
>>> --- a/include/scsi/scsi_device.h
>>> +++ b/include/scsi/scsi_device.h
>>> @@ -160,6 +160,7 @@ struct scsi_device {
>>> unsigned can_power_off:1; /* Device supports runtime power off */
>>> unsigned wce_default_on:1; /* Cache is ON by default */
>>> unsigned no_dif:1; /* T10 PI (DIF) should be disabled */
>>> + unsigned event_driven:1; /* No need to poll the device */
>>>
>>> DECLARE_BITMAP(supported_events, SDEV_EVT_MAXBITS); /* supported events */
>>> struct list_head event_list; /* asserted events */
>>
>> Yes, but if we can get away with doing that, it should be in genhd
>> because it's completely generic.
>>
>> I was imagining we'd have to fake the reply to the test unit ready or
>> some other commands, which is why it would need to be in sr.c
>>
>> The check events code is Tejun's baby, if he's OK with it then just do
>> it in genhd.c
>
> The problem here is there's no easy to reach genhd from libata (or the
> other way around) without going through sr. I think we're gonna have
> to have something in sr one way or the other.
...which is precisely as I said when v1 of this ZPODD patchset appeared.
sr modifications cannot be avoided.
Jeff
^ permalink raw reply
* Re: [RFC PATCH 4/5] arm: omap2: support port power on lan95xx devices
From: Alan Stern @ 2012-12-03 17:09 UTC (permalink / raw)
To: Andy Green
Cc: Ming Lei, Greg Kroah-Hartman, Lan Tianyu, Sarah Sharp,
Rafael J. Wysocki, linux-pm, Oliver Neukum, linux-omap, linux-usb,
Roger Quadros, Felipe Balbi
In-Reply-To: <50BC3003.80608@linaro.org>
On Mon, 3 Dec 2012, Andy Green wrote:
> Unless someone NAKs it for sure already (if you're already sure you're
> going to, please do so to avoid wasting time), I'll issue a try#2 of my
> code later which demonstrates what I mean. At least I guess it's useful
> for comparative purposes.
Before you go writing a whole lot more code, we should discuss the
basics a bit more clearly. There are several unsettled issues here:
1. Should the LAN95xx stuff be associated with the ehci-omap.0's
driver or with the hub port? The port would be more flexible,
offering the ability to turn the power off and on while the
system is running without affecting anything else. But the
port code is currently in flux, which could cause this new
addition to be delayed.
(On the other hand, since the LAN95xx is the only thing
connected to the root hub, it could be powered off and on by
unbinding the ehci-omap.0 device from its driver and rebinding
it.)
2. If we do choose the port, do we want to identify it by matching
against a device name string or by matching a sequence of port
numbers (in this case, a length-1 sequence)? The port numbers
are fixed by the board design, whereas the device name strings
might get changed in the future. On the other hand, the port
numbers apply only to USB whereas device names can be used by
any subsystem.
3. Should the matching mechanism go into the device core or into
the USB port code? (This is related to the previous question.)
4. Should this be implemented simply as a regulator (or a pair of
regulators)? Or should it be generalized to some sort of PM
domain thing? The generic_pm_domain structure defined in
include/linux/pm_domain.h seems like overkill, but maybe it's
the most appropriate thing to use.
Until we decide on the answers to these questions, there's no point
writing detailed patches.
Alan Stern
^ permalink raw reply
* Re: [PATCH v9 06/10] ata: zpodd: check zero power ready status
From: Tejun Heo @ 2012-12-03 16:23 UTC (permalink / raw)
To: James Bottomley
Cc: Aaron Lu, Rafael J. Wysocki, linux-pm, Jeff Garzik, Alan Stern,
Jeff Wu, Aaron Lu, linux-ide, linux-scsi, linux-acpi
In-Reply-To: <1354523143.2307.2.camel@dabdike.int.hansenpartnership.com>
Hello, James.
On Mon, Dec 03, 2012 at 08:25:43AM +0000, James Bottomley wrote:
> > diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
> > index e65c62e..1756151 100644
> > --- a/include/scsi/scsi_device.h
> > +++ b/include/scsi/scsi_device.h
> > @@ -160,6 +160,7 @@ struct scsi_device {
> > unsigned can_power_off:1; /* Device supports runtime power off */
> > unsigned wce_default_on:1; /* Cache is ON by default */
> > unsigned no_dif:1; /* T10 PI (DIF) should be disabled */
> > + unsigned event_driven:1; /* No need to poll the device */
> >
> > DECLARE_BITMAP(supported_events, SDEV_EVT_MAXBITS); /* supported events */
> > struct list_head event_list; /* asserted events */
>
> Yes, but if we can get away with doing that, it should be in genhd
> because it's completely generic.
>
> I was imagining we'd have to fake the reply to the test unit ready or
> some other commands, which is why it would need to be in sr.c
>
> The check events code is Tejun's baby, if he's OK with it then just do
> it in genhd.c
The problem here is there's no easy to reach genhd from libata (or the
other way around) without going through sr. I think we're gonna have
to have something in sr one way or the other.
Thanks.
--
tejun
^ permalink raw reply
* RE: [RFC PATCH 2/7] Thermal: Create zone level APIs
From: R, Durgadoss @ 2012-12-03 13:12 UTC (permalink / raw)
To: Hongbo Zhang
Cc: Zhang, Rui, linux-pm@vger.kernel.org, wni@nvidia.com,
eduardo.valentin@ti.com, amit.kachhap@linaro.org,
sachin.kamat@linaro.org
In-Reply-To: <CAJLyvQwaix91RjAoc2NdMHq4LOxc9iOLGCqa6R6iOMT2kBQbkA@mail.gmail.com>
Hi,
[big cut.]
> >> >> > +struct thermal_zone {
> >> >> > + char name[THERMAL_NAME_LENGTH];
> >> >> > + int id;
> >> >> > + void *devdata;
> >> >> > + struct thermal_zone *ops;
> >> >> What is this, thermal_zone_device_ops? and how to initialize this ops?
> >> >
> >> > oh, Not required, for now. Will remove it..
> >> > Good catch :-)
> >>
> >> I am still confused a bit.
> >> Is this thermal_zone going to take the place of the old
> >> thermal_zone_device?
> >
> > Yes, it will replace the older one.
> >
> >> If yes, which functions/callbacks are used to access
> >> thermal_zone->trip[] like the old get_trip_*?
> >
> > We will not have call backs associated with 'zone'.
> > A zone is kind of a virtual entity, which can have one or
> > more sensors associated with it.
> >
> > Trip points are attached to 'a sensor' which can be in
> > any zone. So, thermal_sensor_ops functions will help you
> > access the trip values.
> But I didn't find such functions in the current thermal_sensor_ops for
> access trip points, only functions for thresholds now.
> Will you add these functions when you update the governors?
The idea is the governors need these values to calculate the trend
of the thermal zone. And these are embedded inside thermal zone
structure. So, when required a zone can 'know' about its sensors and
their trip points through these pointers.
Having said that, when we modify governors, if a need arises to
implement these functions, we will implement it.
Thanks,
Durga
^ permalink raw reply
* Re: [RFC PATCH 2/7] Thermal: Create zone level APIs
From: Hongbo Zhang @ 2012-12-03 11:50 UTC (permalink / raw)
To: R, Durgadoss
Cc: Zhang, Rui, linux-pm@vger.kernel.org, wni@nvidia.com,
eduardo.valentin@ti.com, amit.kachhap@linaro.org,
sachin.kamat@linaro.org
In-Reply-To: <4D68720C2E767A4AA6A8796D42C8EB59230FA6@BGSMSX101.gar.corp.intel.com>
On 3 December 2012 17:51, R, Durgadoss <durgadoss.r@intel.com> wrote:
>
>
>> -----Original Message-----
>> From: Hongbo Zhang [mailto:hongbo.zhang@linaro.org]
>> Sent: Monday, December 03, 2012 1:51 PM
>> To: R, Durgadoss
>> Cc: Zhang, Rui; linux-pm@vger.kernel.org; wni@nvidia.com;
>> eduardo.valentin@ti.com; amit.kachhap@linaro.org;
>> sachin.kamat@linaro.org
>> Subject: Re: [RFC PATCH 2/7] Thermal: Create zone level APIs
>>
>> On 3 December 2012 15:47, R, Durgadoss <durgadoss.r@intel.com> wrote:
>> >
>> >
>> >> -----Original Message-----
>> >> From: Hongbo Zhang [mailto:hongbo.zhang@linaro.org]
>> >> Sent: Monday, December 03, 2012 1:13 PM
>> >> To: R, Durgadoss
>> >> Cc: Zhang, Rui; linux-pm@vger.kernel.org; wni@nvidia.com;
>> >> eduardo.valentin@ti.com; amit.kachhap@linaro.org;
>> >> sachin.kamat@linaro.org
>> >> Subject: Re: [RFC PATCH 2/7] Thermal: Create zone level APIs
>> >>
>> >> On 17 November 2012 18:45, Durgadoss R <durgadoss.r@intel.com>
>> wrote:
>> >> > This patch adds a new thermal_zone structure to
>> >> > thermal.h. Also, adds zone level APIs to the thermal
>> >> > framework.
>> >> >
>> >> > Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
>> >> > ---
>> >> > drivers/thermal/thermal_sys.c | 206
>> >> +++++++++++++++++++++++++++++++++++++++++
>> >> > include/linux/thermal.h | 25 +++++
>> >> > 2 files changed, 231 insertions(+)
>> >> >
>> >> > diff --git a/drivers/thermal/thermal_sys.c
>> b/drivers/thermal/thermal_sys.c
>> >> > index e726c8b..9d386d7 100644
>> >> > --- a/drivers/thermal/thermal_sys.c
>> >> > +++ b/drivers/thermal/thermal_sys.c
>> >> > @@ -44,19 +44,28 @@ MODULE_DESCRIPTION("Generic thermal
>> >> management sysfs support");
>> >> > MODULE_LICENSE("GPL");
>> >> >
>> >> > static DEFINE_IDR(thermal_tz_idr);
>> >> > +static DEFINE_IDR(thermal_zone_idr);
>> >> > static DEFINE_IDR(thermal_cdev_idr);
>> >> > static DEFINE_IDR(thermal_sensor_idr);
>> >> > static DEFINE_MUTEX(thermal_idr_lock);
>> >> >
>> >> > static LIST_HEAD(thermal_tz_list);
>> >> > static LIST_HEAD(thermal_sensor_list);
>> >> > +static LIST_HEAD(thermal_zone_list);
>> >> > static LIST_HEAD(thermal_cdev_list);
>> >> > static LIST_HEAD(thermal_governor_list);
>> >> >
>> >> > static DEFINE_MUTEX(thermal_list_lock);
>> >> > static DEFINE_MUTEX(ts_list_lock);
>> >> > +static DEFINE_MUTEX(tz_list_lock);
>> >> > static DEFINE_MUTEX(thermal_governor_lock);
>> >> >
>> >> > +#define for_each_thermal_sensor(pos) \
>> >> > + list_for_each_entry(pos, &thermal_sensor_list, node)
>> >> > +
>> >> > +#define for_each_thermal_zone(pos) \
>> >> > + list_for_each_entry(pos, &thermal_zone_list, node)
>> >> > +
>> >> > static struct thermal_governor *__find_governor(const char *name)
>> >> > {
>> >> > struct thermal_governor *pos;
>> >> > @@ -419,15 +428,62 @@ static void thermal_zone_device_check(struct
>> >> work_struct *work)
>> >> > thermal_zone_device_update(tz);
>> >> > }
>> >> >
>> >> > +static int get_sensor_indx(struct thermal_zone *tz, struct
>> thermal_sensor
>> >> *ts)
>> >> > +{
>> >> > + int i, indx = -EINVAL;
>> >> > +
>> >> > + if (!tz || !ts)
>> >> > + return -EINVAL;
>> >> > +
>> >> > + mutex_lock(&ts_list_lock);
>> >> > + for (i = 0; i < tz->sensor_indx; i++) {
>> >> > + if (tz->sensors[i] == ts) {
>> >> > + indx = i;
>> >> > + break;
>> >> > + }
>> >> > + }
>> >> > + mutex_unlock(&ts_list_lock);
>> >> > + return indx;
>> >> > +}
>> >> > +
>> >> > +static void remove_sensor_from_zone(struct thermal_zone *tz,
>> >> > + struct thermal_sensor *ts)
>> >> > +{
>> >> > + int indx, j;
>> >> > +
>> >> > + indx = get_sensor_indx(tz, ts);
>> >> > + if (indx < 0)
>> >> > + return;
>> >> > +
>> >> > + sysfs_remove_link(&tz->device.kobj, kobject_name(&ts-
>> >> >device.kobj));
>> >> > +
>> >> > + /* Shift the entries in the tz->sensors array */
>> >> > + for (j = indx; j < MAX_SENSORS_PER_ZONE - 1; j++)
>> >> > + tz->sensors[j] = tz->sensors[j + 1];
>> >> > +
>> >> > + tz->sensor_indx--;
>> >> > +}
>> >> > +
>> >> > /* sys I/F for thermal zone */
>> >> >
>> >> > #define to_thermal_zone(_dev) \
>> >> > container_of(_dev, struct thermal_zone_device, device)
>> >> >
>> >> > +#define to_zone(_dev) \
>> >> > + container_of(_dev, struct thermal_zone, device)
>> >> > +
>> >> > #define to_thermal_sensor(_dev) \
>> >> > container_of(_dev, struct thermal_sensor, device)
>> >> >
>> >> > static ssize_t
>> >> > +zone_name_show(struct device *dev, struct device_attribute *attr,
>> char
>> >> *buf)
>> >> > +{
>> >> > + struct thermal_zone *tz = to_zone(dev);
>> >> > +
>> >> > + return sprintf(buf, "%s\n", tz->name);
>> >> > +}
>> >> > +
>> >> > +static ssize_t
>> >> > sensor_name_show(struct device *dev, struct device_attribute *attr,
>> char
>> >> *buf)
>> >> > {
>> >> > struct thermal_sensor *ts = to_thermal_sensor(dev);
>> >> > @@ -772,6 +828,8 @@ static DEVICE_ATTR(policy, S_IRUGO | S_IWUSR,
>> >> policy_show, policy_store);
>> >> > static DEVICE_ATTR(sensor_name, 0444, sensor_name_show, NULL);
>> >> > static DEVICE_ATTR(temp_input, 0444, sensor_temp_show, NULL);
>> >> >
>> >> > +static DEVICE_ATTR(zone_name, 0444, zone_name_show, NULL);
>> >> > +
>> >> > /* sys I/F for cooling device */
>> >> > #define to_cooling_device(_dev) \
>> >> > container_of(_dev, struct thermal_cooling_device, device)
>> >> > @@ -1557,6 +1615,146 @@ static void remove_trip_attrs(struct
>> >> thermal_zone_device *tz)
>> >> > kfree(tz->trip_hyst_attrs);
>> >> > }
>> >> >
>> >> > +struct thermal_zone *create_thermal_zone(const char *name, void
>> >> *devdata)
>> >> > +{
>> >> > + struct thermal_zone *tz;
>> >> > + int ret;
>> >> > +
>> >> > + if (!name || (name && strlen(name) >=
>> THERMAL_NAME_LENGTH))
>> >> > + return ERR_PTR(-EINVAL);
>> >> > +
>> >> > + tz = kzalloc(sizeof(*tz), GFP_KERNEL);
>> >> > + if (!tz)
>> >> > + return ERR_PTR(-ENOMEM);
>> >> > +
>> >> > + idr_init(&tz->idr);
>> >> > + ret = get_idr(&thermal_zone_idr, &thermal_idr_lock, &tz->id);
>> >> > + if (ret)
>> >> > + goto exit_free;
>> >> > +
>> >> > + strcpy(tz->name, name);
>> >> > + tz->devdata = devdata;
>> >> > + tz->device.class = &thermal_class;
>> >> > +
>> >> > + dev_set_name(&tz->device, "zone%d", tz->id);
>> >> > + ret = device_register(&tz->device);
>> >> > + if (ret)
>> >> > + goto exit_idr;
>> >> > +
>> >> > + ret = device_create_file(&tz->device, &dev_attr_zone_name);
>> >> > + if (ret)
>> >> > + goto exit_unregister;
>> >> > +
>> >> > + /* Add this zone to the global list of thermal zones */
>> >> > + mutex_lock(&tz_list_lock);
>> >> > + list_add_tail(&tz->node, &thermal_zone_list);
>> >> > + mutex_unlock(&tz_list_lock);
>> >> > + return tz;
>> >> > +
>> >> > +exit_unregister:
>> >> > + device_unregister(&tz->device);
>> >> > +exit_idr:
>> >> > + release_idr(&thermal_zone_idr, &thermal_idr_lock, tz->id);
>> >> > +exit_free:
>> >> > + kfree(tz);
>> >> > + return ERR_PTR(ret);
>> >> > +}
>> >> > +EXPORT_SYMBOL(create_thermal_zone);
>> >> > +
>> >> > +void remove_thermal_zone(struct thermal_zone *tz)
>> >> > +{
>> >> > + struct thermal_zone *pos, *next;
>> >> > + bool found = false;
>> >> > +
>> >> > + if (!tz)
>> >> > + return;
>> >> > +
>> >> > + mutex_lock(&tz_list_lock);
>> >> > +
>> >> > + list_for_each_entry_safe(pos, next, &thermal_zone_list, node) {
>> >> > + if (pos == tz) {
>> >> > + list_del(&tz->node);
>> >> > + found = true;
>> >> > + break;
>> >> > + }
>> >> > + }
>> >> > +
>> >> > + if (!found)
>> >> > + goto exit;
>> >> > +
>> >> > + device_remove_file(&tz->device, &dev_attr_zone_name);
>> >> > +
>> >> > + release_idr(&thermal_zone_idr, &thermal_idr_lock, tz->id);
>> >> > + idr_destroy(&tz->idr);
>> >> > +
>> >> > + device_unregister(&tz->device);
>> >> > + kfree(tz);
>> >> > +exit:
>> >> > + mutex_unlock(&tz_list_lock);
>> >> > + return;
>> >> > +}
>> >> > +EXPORT_SYMBOL(remove_thermal_zone);
>> >> > +
>> >> > +struct thermal_sensor *get_sensor_by_name(const char *name)
>> >> > +{
>> >> > + struct thermal_sensor *pos;
>> >> > + struct thermal_sensor *ts = NULL;
>> >> > +
>> >> > + mutex_lock(&ts_list_lock);
>> >> > + for_each_thermal_sensor(pos) {
>> >> > + if (!strnicmp(pos->name, name, THERMAL_NAME_LENGTH)) {
>> >> > + ts = pos;
>> >> > + break;
>> >> > + }
>> >> > + }
>> >> > + mutex_unlock(&ts_list_lock);
>> >> > + return ts;
>> >> > +}
>> >> > +EXPORT_SYMBOL(get_sensor_by_name);
>> >> > +
>> >> > +int add_sensor_to_zone_by_name(struct thermal_zone *tz, const
>> char
>> >> *name)
>> >> > +{
>> >> > + int ret;
>> >> > + struct thermal_sensor *ts = get_sensor_by_name(name);
>> >> > +
>> >> > + if (!tz || !ts)
>> >> > + return -EINVAL;
>> >> > +
>> >> > + mutex_lock(&tz_list_lock);
>> >> > +
>> >> > + /* Ensure we are not adding the same sensor again!! */
>> >> > + ret = get_sensor_indx(tz, ts);
>> >> > + if (ret >= 0) {
>> >> > + ret = -EEXIST;
>> >> > + goto exit_zone;
>> >> > + }
>> >> > +
>> >> > + mutex_lock(&ts_list_lock);
>> >> > +
>> >> > + ret = sysfs_create_link(&tz->device.kobj, &ts->device.kobj,
>> >> > + kobject_name(&ts->device.kobj));
>> >> > + if (ret)
>> >> > + goto exit_sensor;
>> >> > +
>> >> > + tz->sensors[tz->sensor_indx++] = ts;
>> >> > +
>> >> > +exit_sensor:
>> >> > + mutex_unlock(&ts_list_lock);
>> >> > +exit_zone:
>> >> > + mutex_unlock(&tz_list_lock);
>> >> > + return ret;
>> >> > +}
>> >> > +EXPORT_SYMBOL(add_sensor_to_zone_by_name);
>> >> > +
>> >> > +int add_sensor_to_zone(struct thermal_zone *tz, struct
>> thermal_sensor
>> >> *ts)
>> >> > +{
>> >> > + if (!tz || !ts)
>> >> > + return -EINVAL;
>> >> > +
>> >> > + return add_sensor_to_zone_by_name(tz, ts->name);
>> >> > +}
>> >> > +EXPORT_SYMBOL(add_sensor_to_zone);
>> >> > +
>> >> > /**
>> >> > * thermal_sensor_register - register a new thermal sensor
>> >> > * @name: name of the thermal sensor
>> >> > @@ -1624,6 +1822,7 @@ EXPORT_SYMBOL(thermal_sensor_register);
>> >> > void thermal_sensor_unregister(struct thermal_sensor *ts)
>> >> > {
>> >> > int i;
>> >> > + struct thermal_zone *tz;
>> >> > struct thermal_sensor *pos, *next;
>> >> > bool found = false;
>> >> >
>> >> > @@ -1643,6 +1842,13 @@ void thermal_sensor_unregister(struct
>> >> thermal_sensor *ts)
>> >> > if (!found)
>> >> > return;
>> >> >
>> >> > + mutex_lock(&tz_list_lock);
>> >> > +
>> >> > + for_each_thermal_zone(tz)
>> >> > + remove_sensor_from_zone(tz, ts);
>> >> > +
>> >> > + mutex_unlock(&tz_list_lock);
>> >> > +
>> >> > for (i = 0; i < ts->thresholds; i++)
>> >> > device_remove_file(&ts->device, &ts->thresh_attrs[i].attr);
>> >> >
>> >> > diff --git a/include/linux/thermal.h b/include/linux/thermal.h
>> >> > index e2f85ec..38438be 100644
>> >> > --- a/include/linux/thermal.h
>> >> > +++ b/include/linux/thermal.h
>> >> > @@ -49,6 +49,11 @@
>> >> > /* Default Thermal Governor: Does Linear Throttling */
>> >> > #define DEFAULT_THERMAL_GOVERNOR "step_wise"
>> >> >
>> >> > +/* Maximum number of sensors, allowed in a thermal zone
>> >> > + * We will start with 5, and increase if needed.
>> >> > + */
>> >> > +#define MAX_SENSORS_PER_ZONE 5
>> >> > +
>> >> > struct thermal_sensor;
>> >> > struct thermal_zone_device;
>> >> > struct thermal_cooling_device;
>> >> > @@ -191,6 +196,21 @@ struct thermal_zone_device {
>> >> > struct delayed_work poll_queue;
>> >> > };
>> >> >
>> >> > +struct thermal_zone {
>> >> > + char name[THERMAL_NAME_LENGTH];
>> >> > + int id;
>> >> > + void *devdata;
>> >> > + struct thermal_zone *ops;
>> >> What is this, thermal_zone_device_ops? and how to initialize this ops?
>> >
>> > oh, Not required, for now. Will remove it..
>> > Good catch :-)
>>
>> I am still confused a bit.
>> Is this thermal_zone going to take the place of the old
>> thermal_zone_device?
>
> Yes, it will replace the older one.
>
>> If yes, which functions/callbacks are used to access
>> thermal_zone->trip[] like the old get_trip_*?
>
> We will not have call backs associated with 'zone'.
> A zone is kind of a virtual entity, which can have one or
> more sensors associated with it.
>
> Trip points are attached to 'a sensor' which can be in
> any zone. So, thermal_sensor_ops functions will help you
> access the trip values.
But I didn't find such functions in the current thermal_sensor_ops for
access trip points, only functions for thresholds now.
Will you add these functions when you update the governors?
>
> Thanks,
> Durga
>
>>
>> >
>> > Thanks,
>> > Durga
>> >>
>> >> > + struct thermal_governor *governor;
>> >> > + struct idr idr;
>> >> > + struct device device;
>> >> > + struct list_head node;
>> >> > +
>> >> > + /* Sensor level information */
>> >> > + int sensor_indx; /* index into 'sensors' array */
>> >> > + struct thermal_sensor *sensors[MAX_SENSORS_PER_ZONE];
>> >> > +};
>> >> > +
>> >> > /* Structure that holds thermal governor information */
>> >> > struct thermal_governor {
>> >> > char name[THERMAL_NAME_LENGTH];
>> >> > @@ -264,6 +284,11 @@ struct thermal_sensor
>> >> *thermal_sensor_register(const char *,
>> >> > void thermal_sensor_unregister(struct thermal_sensor *);
>> >> > int enable_sensor_thresholds(struct thermal_sensor *, int);
>> >> >
>> >> > +struct thermal_zone *create_thermal_zone(const char *, void *);
>> >> > +void remove_thermal_zone(struct thermal_zone *);
>> >> > +int add_sensor_to_zone(struct thermal_zone *, struct thermal_sensor
>> >> *);
>> >> > +int add_sensor_to_zone_by_name(struct thermal_zone *, const char
>> *);
>> >> > +
>> >> > #ifdef CONFIG_NET
>> >> > extern int thermal_generate_netlink_event(u32 orig, enum events
>> >> event);
>> >> > #else
>> >> > --
>> >> > 1.7.9.5
>> >> >
^ permalink raw reply
* RE: [RFC PATCH 0/7] Support for Multiple sensors per zone
From: R, Durgadoss @ 2012-12-03 9:56 UTC (permalink / raw)
To: Hongbo Zhang
Cc: Zhang, Rui, linux-pm@vger.kernel.org, wni@nvidia.com,
eduardo.valentin@ti.com, amit.kachhap@linaro.org,
sachin.kamat@linaro.org
In-Reply-To: <CAJLyvQxn_cfYeJZ+XFaM8HZBq6mNRh5R-VtX2vN92Dp+S_9Mgw@mail.gmail.com>
> -----Original Message-----
> From: Hongbo Zhang [mailto:hongbo.zhang@linaro.org]
> Sent: Monday, December 03, 2012 2:31 PM
> To: R, Durgadoss
> Cc: Zhang, Rui; linux-pm@vger.kernel.org; wni@nvidia.com;
> eduardo.valentin@ti.com; amit.kachhap@linaro.org;
> sachin.kamat@linaro.org
> Subject: Re: [RFC PATCH 0/7] Support for Multiple sensors per zone
>
> On 17 November 2012 18:45, Durgadoss R <durgadoss.r@intel.com> wrote:
> > This patch series attempts to add support for multiple
> > sensors per zone. The work is based on the Thermal discussion
> > happened in plumbers conference 2012, here:
> > http://www.linuxplumbersconf.org/2012/schedule/
> > Title: "Enhancing the Thermal Management Infrastructure in Linux"
> >
> > The intention is to make it easy for generic sensor drivers
> > to register with the framework, and let them participate in
> > platform thermal management. Another goal is to expose the
> > binding information in a consistent way so that user space
> > can consume the information and potentially manage platform thermals.
> >
> > This series contains 7 patches:
> > Patch 1/7: Creates new sensor level APIs
> > Patch 2/7: Creates new zone level APIs. The existing tzd structure is
> > kept as such for clarity and compatibility purposes.
> > Patch 3/7: Creates functions to add/remove a cdev to/from a zone. The
> > existing tcd structure need not be modified.
> > Patch 4/7: Adds a thermal_trip sysfs node, which exposes various trip
> > points for all sensors present in a zone.
> > Patch 5/7: Adds a thermal_map sysfs node. It is a compact representation
> > of the binding relationship between a sensor and a cdev,
> > within a zone.
> > Patch 6/7: Creates Documentation for the new APIs. A new file is
> > created for clarity. Final goal is to merge with the existing
> > file or refactor the files, as whatever seems appropriate.
> > Patch 7/7: A dummy driver that can be used for testing. This is not for
> merge.
> >
> > Next steps:
> Do you have milestone?
Good question :-)
I would like these new APIs to get merged for 3.9 release.
Then we can work on migrating the old drivers + governors to
use this new API.
But, We should together ask for Rui's help here :-)
> Because new thermal driver writers are wondering if to use the old
> framework or can wait the new one.
We should move towards the new APIs.
Thanks for your review.
Thanks,
Durga
> > 1. Move all the existing drivers to the new implementation model.
> > Help welcomed from individual driver authors/maintainers for this.
> > 2. Make the thermal governors work with this new model.
> > 3. Remove old/unused code from thermal_sys.c.
> > 4. Add more detailed documentation
> >
> > I didn't want to submit patches for all these in one-go, since it
> > might end-up being difficult to comprehend, besides delaying the
> > review process. The other obvious reason being I cannot test all
> > the changes on various drivers for 1.
> >
> > All these patches have been tested on a Core-i5 desktop running
> > ubuntu 12.04 and an atom notebook running ubuntu 11.10.
> > Kindly help review.
> >
> > Durgadoss R (7):
> > Thermal: Create sensor level APIs
> > Thermal: Create zone level APIs
> > Thermal: Add APIs to bind cdev to new zone structure
> > Thermal: Add Thermal_trip sysfs node
> > Thermal: Add 'thermal_map' sysfs node
> > Thermal: Add Documentation to new APIs
> > Thermal: Dummy driver used for testing
> >
> > Documentation/thermal/sysfs-api2.txt | 213 ++++++++
> > drivers/thermal/Kconfig | 5 +
> > drivers/thermal/Makefile | 3 +
> > drivers/thermal/thermal_sys.c | 915
> ++++++++++++++++++++++++++++++++++
> > drivers/thermal/thermal_test.c | 321 ++++++++++++
> > include/linux/thermal.h | 118 +++++
> > 6 files changed, 1575 insertions(+)
> > create mode 100644 Documentation/thermal/sysfs-api2.txt
> > create mode 100644 drivers/thermal/thermal_test.c
> >
> > --
> > 1.7.9.5
> >
^ permalink raw reply
* RE: [RFC PATCH 2/7] Thermal: Create zone level APIs
From: R, Durgadoss @ 2012-12-03 9:51 UTC (permalink / raw)
To: Hongbo Zhang
Cc: Zhang, Rui, linux-pm@vger.kernel.org, wni@nvidia.com,
eduardo.valentin@ti.com, amit.kachhap@linaro.org,
sachin.kamat@linaro.org
In-Reply-To: <CAJLyvQxW3fLrpyBhaKSpDoq2K4b4zOd8qLzAv1XsqEtNVgk8Sw@mail.gmail.com>
> -----Original Message-----
> From: Hongbo Zhang [mailto:hongbo.zhang@linaro.org]
> Sent: Monday, December 03, 2012 1:51 PM
> To: R, Durgadoss
> Cc: Zhang, Rui; linux-pm@vger.kernel.org; wni@nvidia.com;
> eduardo.valentin@ti.com; amit.kachhap@linaro.org;
> sachin.kamat@linaro.org
> Subject: Re: [RFC PATCH 2/7] Thermal: Create zone level APIs
>
> On 3 December 2012 15:47, R, Durgadoss <durgadoss.r@intel.com> wrote:
> >
> >
> >> -----Original Message-----
> >> From: Hongbo Zhang [mailto:hongbo.zhang@linaro.org]
> >> Sent: Monday, December 03, 2012 1:13 PM
> >> To: R, Durgadoss
> >> Cc: Zhang, Rui; linux-pm@vger.kernel.org; wni@nvidia.com;
> >> eduardo.valentin@ti.com; amit.kachhap@linaro.org;
> >> sachin.kamat@linaro.org
> >> Subject: Re: [RFC PATCH 2/7] Thermal: Create zone level APIs
> >>
> >> On 17 November 2012 18:45, Durgadoss R <durgadoss.r@intel.com>
> wrote:
> >> > This patch adds a new thermal_zone structure to
> >> > thermal.h. Also, adds zone level APIs to the thermal
> >> > framework.
> >> >
> >> > Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
> >> > ---
> >> > drivers/thermal/thermal_sys.c | 206
> >> +++++++++++++++++++++++++++++++++++++++++
> >> > include/linux/thermal.h | 25 +++++
> >> > 2 files changed, 231 insertions(+)
> >> >
> >> > diff --git a/drivers/thermal/thermal_sys.c
> b/drivers/thermal/thermal_sys.c
> >> > index e726c8b..9d386d7 100644
> >> > --- a/drivers/thermal/thermal_sys.c
> >> > +++ b/drivers/thermal/thermal_sys.c
> >> > @@ -44,19 +44,28 @@ MODULE_DESCRIPTION("Generic thermal
> >> management sysfs support");
> >> > MODULE_LICENSE("GPL");
> >> >
> >> > static DEFINE_IDR(thermal_tz_idr);
> >> > +static DEFINE_IDR(thermal_zone_idr);
> >> > static DEFINE_IDR(thermal_cdev_idr);
> >> > static DEFINE_IDR(thermal_sensor_idr);
> >> > static DEFINE_MUTEX(thermal_idr_lock);
> >> >
> >> > static LIST_HEAD(thermal_tz_list);
> >> > static LIST_HEAD(thermal_sensor_list);
> >> > +static LIST_HEAD(thermal_zone_list);
> >> > static LIST_HEAD(thermal_cdev_list);
> >> > static LIST_HEAD(thermal_governor_list);
> >> >
> >> > static DEFINE_MUTEX(thermal_list_lock);
> >> > static DEFINE_MUTEX(ts_list_lock);
> >> > +static DEFINE_MUTEX(tz_list_lock);
> >> > static DEFINE_MUTEX(thermal_governor_lock);
> >> >
> >> > +#define for_each_thermal_sensor(pos) \
> >> > + list_for_each_entry(pos, &thermal_sensor_list, node)
> >> > +
> >> > +#define for_each_thermal_zone(pos) \
> >> > + list_for_each_entry(pos, &thermal_zone_list, node)
> >> > +
> >> > static struct thermal_governor *__find_governor(const char *name)
> >> > {
> >> > struct thermal_governor *pos;
> >> > @@ -419,15 +428,62 @@ static void thermal_zone_device_check(struct
> >> work_struct *work)
> >> > thermal_zone_device_update(tz);
> >> > }
> >> >
> >> > +static int get_sensor_indx(struct thermal_zone *tz, struct
> thermal_sensor
> >> *ts)
> >> > +{
> >> > + int i, indx = -EINVAL;
> >> > +
> >> > + if (!tz || !ts)
> >> > + return -EINVAL;
> >> > +
> >> > + mutex_lock(&ts_list_lock);
> >> > + for (i = 0; i < tz->sensor_indx; i++) {
> >> > + if (tz->sensors[i] == ts) {
> >> > + indx = i;
> >> > + break;
> >> > + }
> >> > + }
> >> > + mutex_unlock(&ts_list_lock);
> >> > + return indx;
> >> > +}
> >> > +
> >> > +static void remove_sensor_from_zone(struct thermal_zone *tz,
> >> > + struct thermal_sensor *ts)
> >> > +{
> >> > + int indx, j;
> >> > +
> >> > + indx = get_sensor_indx(tz, ts);
> >> > + if (indx < 0)
> >> > + return;
> >> > +
> >> > + sysfs_remove_link(&tz->device.kobj, kobject_name(&ts-
> >> >device.kobj));
> >> > +
> >> > + /* Shift the entries in the tz->sensors array */
> >> > + for (j = indx; j < MAX_SENSORS_PER_ZONE - 1; j++)
> >> > + tz->sensors[j] = tz->sensors[j + 1];
> >> > +
> >> > + tz->sensor_indx--;
> >> > +}
> >> > +
> >> > /* sys I/F for thermal zone */
> >> >
> >> > #define to_thermal_zone(_dev) \
> >> > container_of(_dev, struct thermal_zone_device, device)
> >> >
> >> > +#define to_zone(_dev) \
> >> > + container_of(_dev, struct thermal_zone, device)
> >> > +
> >> > #define to_thermal_sensor(_dev) \
> >> > container_of(_dev, struct thermal_sensor, device)
> >> >
> >> > static ssize_t
> >> > +zone_name_show(struct device *dev, struct device_attribute *attr,
> char
> >> *buf)
> >> > +{
> >> > + struct thermal_zone *tz = to_zone(dev);
> >> > +
> >> > + return sprintf(buf, "%s\n", tz->name);
> >> > +}
> >> > +
> >> > +static ssize_t
> >> > sensor_name_show(struct device *dev, struct device_attribute *attr,
> char
> >> *buf)
> >> > {
> >> > struct thermal_sensor *ts = to_thermal_sensor(dev);
> >> > @@ -772,6 +828,8 @@ static DEVICE_ATTR(policy, S_IRUGO | S_IWUSR,
> >> policy_show, policy_store);
> >> > static DEVICE_ATTR(sensor_name, 0444, sensor_name_show, NULL);
> >> > static DEVICE_ATTR(temp_input, 0444, sensor_temp_show, NULL);
> >> >
> >> > +static DEVICE_ATTR(zone_name, 0444, zone_name_show, NULL);
> >> > +
> >> > /* sys I/F for cooling device */
> >> > #define to_cooling_device(_dev) \
> >> > container_of(_dev, struct thermal_cooling_device, device)
> >> > @@ -1557,6 +1615,146 @@ static void remove_trip_attrs(struct
> >> thermal_zone_device *tz)
> >> > kfree(tz->trip_hyst_attrs);
> >> > }
> >> >
> >> > +struct thermal_zone *create_thermal_zone(const char *name, void
> >> *devdata)
> >> > +{
> >> > + struct thermal_zone *tz;
> >> > + int ret;
> >> > +
> >> > + if (!name || (name && strlen(name) >=
> THERMAL_NAME_LENGTH))
> >> > + return ERR_PTR(-EINVAL);
> >> > +
> >> > + tz = kzalloc(sizeof(*tz), GFP_KERNEL);
> >> > + if (!tz)
> >> > + return ERR_PTR(-ENOMEM);
> >> > +
> >> > + idr_init(&tz->idr);
> >> > + ret = get_idr(&thermal_zone_idr, &thermal_idr_lock, &tz->id);
> >> > + if (ret)
> >> > + goto exit_free;
> >> > +
> >> > + strcpy(tz->name, name);
> >> > + tz->devdata = devdata;
> >> > + tz->device.class = &thermal_class;
> >> > +
> >> > + dev_set_name(&tz->device, "zone%d", tz->id);
> >> > + ret = device_register(&tz->device);
> >> > + if (ret)
> >> > + goto exit_idr;
> >> > +
> >> > + ret = device_create_file(&tz->device, &dev_attr_zone_name);
> >> > + if (ret)
> >> > + goto exit_unregister;
> >> > +
> >> > + /* Add this zone to the global list of thermal zones */
> >> > + mutex_lock(&tz_list_lock);
> >> > + list_add_tail(&tz->node, &thermal_zone_list);
> >> > + mutex_unlock(&tz_list_lock);
> >> > + return tz;
> >> > +
> >> > +exit_unregister:
> >> > + device_unregister(&tz->device);
> >> > +exit_idr:
> >> > + release_idr(&thermal_zone_idr, &thermal_idr_lock, tz->id);
> >> > +exit_free:
> >> > + kfree(tz);
> >> > + return ERR_PTR(ret);
> >> > +}
> >> > +EXPORT_SYMBOL(create_thermal_zone);
> >> > +
> >> > +void remove_thermal_zone(struct thermal_zone *tz)
> >> > +{
> >> > + struct thermal_zone *pos, *next;
> >> > + bool found = false;
> >> > +
> >> > + if (!tz)
> >> > + return;
> >> > +
> >> > + mutex_lock(&tz_list_lock);
> >> > +
> >> > + list_for_each_entry_safe(pos, next, &thermal_zone_list, node) {
> >> > + if (pos == tz) {
> >> > + list_del(&tz->node);
> >> > + found = true;
> >> > + break;
> >> > + }
> >> > + }
> >> > +
> >> > + if (!found)
> >> > + goto exit;
> >> > +
> >> > + device_remove_file(&tz->device, &dev_attr_zone_name);
> >> > +
> >> > + release_idr(&thermal_zone_idr, &thermal_idr_lock, tz->id);
> >> > + idr_destroy(&tz->idr);
> >> > +
> >> > + device_unregister(&tz->device);
> >> > + kfree(tz);
> >> > +exit:
> >> > + mutex_unlock(&tz_list_lock);
> >> > + return;
> >> > +}
> >> > +EXPORT_SYMBOL(remove_thermal_zone);
> >> > +
> >> > +struct thermal_sensor *get_sensor_by_name(const char *name)
> >> > +{
> >> > + struct thermal_sensor *pos;
> >> > + struct thermal_sensor *ts = NULL;
> >> > +
> >> > + mutex_lock(&ts_list_lock);
> >> > + for_each_thermal_sensor(pos) {
> >> > + if (!strnicmp(pos->name, name, THERMAL_NAME_LENGTH)) {
> >> > + ts = pos;
> >> > + break;
> >> > + }
> >> > + }
> >> > + mutex_unlock(&ts_list_lock);
> >> > + return ts;
> >> > +}
> >> > +EXPORT_SYMBOL(get_sensor_by_name);
> >> > +
> >> > +int add_sensor_to_zone_by_name(struct thermal_zone *tz, const
> char
> >> *name)
> >> > +{
> >> > + int ret;
> >> > + struct thermal_sensor *ts = get_sensor_by_name(name);
> >> > +
> >> > + if (!tz || !ts)
> >> > + return -EINVAL;
> >> > +
> >> > + mutex_lock(&tz_list_lock);
> >> > +
> >> > + /* Ensure we are not adding the same sensor again!! */
> >> > + ret = get_sensor_indx(tz, ts);
> >> > + if (ret >= 0) {
> >> > + ret = -EEXIST;
> >> > + goto exit_zone;
> >> > + }
> >> > +
> >> > + mutex_lock(&ts_list_lock);
> >> > +
> >> > + ret = sysfs_create_link(&tz->device.kobj, &ts->device.kobj,
> >> > + kobject_name(&ts->device.kobj));
> >> > + if (ret)
> >> > + goto exit_sensor;
> >> > +
> >> > + tz->sensors[tz->sensor_indx++] = ts;
> >> > +
> >> > +exit_sensor:
> >> > + mutex_unlock(&ts_list_lock);
> >> > +exit_zone:
> >> > + mutex_unlock(&tz_list_lock);
> >> > + return ret;
> >> > +}
> >> > +EXPORT_SYMBOL(add_sensor_to_zone_by_name);
> >> > +
> >> > +int add_sensor_to_zone(struct thermal_zone *tz, struct
> thermal_sensor
> >> *ts)
> >> > +{
> >> > + if (!tz || !ts)
> >> > + return -EINVAL;
> >> > +
> >> > + return add_sensor_to_zone_by_name(tz, ts->name);
> >> > +}
> >> > +EXPORT_SYMBOL(add_sensor_to_zone);
> >> > +
> >> > /**
> >> > * thermal_sensor_register - register a new thermal sensor
> >> > * @name: name of the thermal sensor
> >> > @@ -1624,6 +1822,7 @@ EXPORT_SYMBOL(thermal_sensor_register);
> >> > void thermal_sensor_unregister(struct thermal_sensor *ts)
> >> > {
> >> > int i;
> >> > + struct thermal_zone *tz;
> >> > struct thermal_sensor *pos, *next;
> >> > bool found = false;
> >> >
> >> > @@ -1643,6 +1842,13 @@ void thermal_sensor_unregister(struct
> >> thermal_sensor *ts)
> >> > if (!found)
> >> > return;
> >> >
> >> > + mutex_lock(&tz_list_lock);
> >> > +
> >> > + for_each_thermal_zone(tz)
> >> > + remove_sensor_from_zone(tz, ts);
> >> > +
> >> > + mutex_unlock(&tz_list_lock);
> >> > +
> >> > for (i = 0; i < ts->thresholds; i++)
> >> > device_remove_file(&ts->device, &ts->thresh_attrs[i].attr);
> >> >
> >> > diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> >> > index e2f85ec..38438be 100644
> >> > --- a/include/linux/thermal.h
> >> > +++ b/include/linux/thermal.h
> >> > @@ -49,6 +49,11 @@
> >> > /* Default Thermal Governor: Does Linear Throttling */
> >> > #define DEFAULT_THERMAL_GOVERNOR "step_wise"
> >> >
> >> > +/* Maximum number of sensors, allowed in a thermal zone
> >> > + * We will start with 5, and increase if needed.
> >> > + */
> >> > +#define MAX_SENSORS_PER_ZONE 5
> >> > +
> >> > struct thermal_sensor;
> >> > struct thermal_zone_device;
> >> > struct thermal_cooling_device;
> >> > @@ -191,6 +196,21 @@ struct thermal_zone_device {
> >> > struct delayed_work poll_queue;
> >> > };
> >> >
> >> > +struct thermal_zone {
> >> > + char name[THERMAL_NAME_LENGTH];
> >> > + int id;
> >> > + void *devdata;
> >> > + struct thermal_zone *ops;
> >> What is this, thermal_zone_device_ops? and how to initialize this ops?
> >
> > oh, Not required, for now. Will remove it..
> > Good catch :-)
>
> I am still confused a bit.
> Is this thermal_zone going to take the place of the old
> thermal_zone_device?
Yes, it will replace the older one.
> If yes, which functions/callbacks are used to access
> thermal_zone->trip[] like the old get_trip_*?
We will not have call backs associated with 'zone'.
A zone is kind of a virtual entity, which can have one or
more sensors associated with it.
Trip points are attached to 'a sensor' which can be in
any zone. So, thermal_sensor_ops functions will help you
access the trip values.
Thanks,
Durga
>
> >
> > Thanks,
> > Durga
> >>
> >> > + struct thermal_governor *governor;
> >> > + struct idr idr;
> >> > + struct device device;
> >> > + struct list_head node;
> >> > +
> >> > + /* Sensor level information */
> >> > + int sensor_indx; /* index into 'sensors' array */
> >> > + struct thermal_sensor *sensors[MAX_SENSORS_PER_ZONE];
> >> > +};
> >> > +
> >> > /* Structure that holds thermal governor information */
> >> > struct thermal_governor {
> >> > char name[THERMAL_NAME_LENGTH];
> >> > @@ -264,6 +284,11 @@ struct thermal_sensor
> >> *thermal_sensor_register(const char *,
> >> > void thermal_sensor_unregister(struct thermal_sensor *);
> >> > int enable_sensor_thresholds(struct thermal_sensor *, int);
> >> >
> >> > +struct thermal_zone *create_thermal_zone(const char *, void *);
> >> > +void remove_thermal_zone(struct thermal_zone *);
> >> > +int add_sensor_to_zone(struct thermal_zone *, struct thermal_sensor
> >> *);
> >> > +int add_sensor_to_zone_by_name(struct thermal_zone *, const char
> *);
> >> > +
> >> > #ifdef CONFIG_NET
> >> > extern int thermal_generate_netlink_event(u32 orig, enum events
> >> event);
> >> > #else
> >> > --
> >> > 1.7.9.5
> >> >
^ permalink raw reply
* Re: [RFC PATCH 0/7] Support for Multiple sensors per zone
From: Hongbo Zhang @ 2012-12-03 9:01 UTC (permalink / raw)
To: Durgadoss R
Cc: rui.zhang, linux-pm, wni, eduardo.valentin, amit.kachhap,
sachin.kamat
In-Reply-To: <1353149158-19102-1-git-send-email-durgadoss.r@intel.com>
On 17 November 2012 18:45, Durgadoss R <durgadoss.r@intel.com> wrote:
> This patch series attempts to add support for multiple
> sensors per zone. The work is based on the Thermal discussion
> happened in plumbers conference 2012, here:
> http://www.linuxplumbersconf.org/2012/schedule/
> Title: "Enhancing the Thermal Management Infrastructure in Linux"
>
> The intention is to make it easy for generic sensor drivers
> to register with the framework, and let them participate in
> platform thermal management. Another goal is to expose the
> binding information in a consistent way so that user space
> can consume the information and potentially manage platform thermals.
>
> This series contains 7 patches:
> Patch 1/7: Creates new sensor level APIs
> Patch 2/7: Creates new zone level APIs. The existing tzd structure is
> kept as such for clarity and compatibility purposes.
> Patch 3/7: Creates functions to add/remove a cdev to/from a zone. The
> existing tcd structure need not be modified.
> Patch 4/7: Adds a thermal_trip sysfs node, which exposes various trip
> points for all sensors present in a zone.
> Patch 5/7: Adds a thermal_map sysfs node. It is a compact representation
> of the binding relationship between a sensor and a cdev,
> within a zone.
> Patch 6/7: Creates Documentation for the new APIs. A new file is
> created for clarity. Final goal is to merge with the existing
> file or refactor the files, as whatever seems appropriate.
> Patch 7/7: A dummy driver that can be used for testing. This is not for merge.
>
> Next steps:
Do you have milestone?
Because new thermal driver writers are wondering if to use the old
framework or can wait the new one.
> 1. Move all the existing drivers to the new implementation model.
> Help welcomed from individual driver authors/maintainers for this.
> 2. Make the thermal governors work with this new model.
> 3. Remove old/unused code from thermal_sys.c.
> 4. Add more detailed documentation
>
> I didn't want to submit patches for all these in one-go, since it
> might end-up being difficult to comprehend, besides delaying the
> review process. The other obvious reason being I cannot test all
> the changes on various drivers for 1.
>
> All these patches have been tested on a Core-i5 desktop running
> ubuntu 12.04 and an atom notebook running ubuntu 11.10.
> Kindly help review.
>
> Durgadoss R (7):
> Thermal: Create sensor level APIs
> Thermal: Create zone level APIs
> Thermal: Add APIs to bind cdev to new zone structure
> Thermal: Add Thermal_trip sysfs node
> Thermal: Add 'thermal_map' sysfs node
> Thermal: Add Documentation to new APIs
> Thermal: Dummy driver used for testing
>
> Documentation/thermal/sysfs-api2.txt | 213 ++++++++
> drivers/thermal/Kconfig | 5 +
> drivers/thermal/Makefile | 3 +
> drivers/thermal/thermal_sys.c | 915 ++++++++++++++++++++++++++++++++++
> drivers/thermal/thermal_test.c | 321 ++++++++++++
> include/linux/thermal.h | 118 +++++
> 6 files changed, 1575 insertions(+)
> create mode 100644 Documentation/thermal/sysfs-api2.txt
> create mode 100644 drivers/thermal/thermal_test.c
>
> --
> 1.7.9.5
>
^ permalink raw reply
* Re: [PATCH v9 06/10] ata: zpodd: check zero power ready status
From: Aaron Lu @ 2012-12-03 8:59 UTC (permalink / raw)
To: James Bottomley, Tejun Heo
Cc: Rafael J. Wysocki, linux-pm, Jeff Garzik, Alan Stern, Jeff Wu,
Aaron Lu, linux-ide, linux-scsi, linux-acpi
In-Reply-To: <1354523143.2307.2.camel@dabdike.int.hansenpartnership.com>
On Mon, Dec 03, 2012 at 08:25:43AM +0000, James Bottomley wrote:
> On Mon, 2012-12-03 at 16:13 +0800, Aaron Lu wrote:
> > On Wed, Nov 28, 2012 at 08:56:09AM +0000, James Bottomley wrote:
> > > On Tue, 2012-11-27 at 17:39 -0800, Tejun Heo wrote:
> > > > Hey, Rafael.
> > > >
> > > > On Wed, Nov 28, 2012 at 01:51:00AM +0100, Rafael J. Wysocki wrote:
> > > > > Having considered that a bit more I'm now thinking that in fact the power state
> > > > > the device is in at the moment doesn't really matter, so the polling code need
> > > > > not really know what PM is doing. What it needs to know is that the device
> > > > > will generate a hardware event when something interesting happens, so it is not
> > > > > necessary to poll it.
> > > > >
> > > > > In this particular case it is related to power management (apparently, hardware
> > > > > events will only be generated after the device has been put into ACPI D3cold,
> > > > > or so Aaron seems to be claiming), but it need not be in general, at least in
> > > > > principle.
> > > > >
> > > > > It looks like we need an "event driven" flag that the can be set in the lower
> > > > > layers and read by the upper layers. I suppose this means it would need to be
> > > > > in struct device, but not necessarily in the PM-specific part of it.
> > > >
> > > > We already have that. That's what gendisk->async_events is for (as
> > > > opposed to gendisk->events). If all events are async_events, block
> > > > won't poll for events, but I'm not sure that's the golden bullet.
> > > >
> > > > * None implements async_events yet and an API is missing -
> > > > disk_check_events() - which is trivial to add, but it's the same
> > > > story. We'll need a mechanism to shoot up notification from libata
> > > > to block layer. It's admittedly easier to justify routing through
> > > > SCSI tho. So, we're mostly shifting the problem. Given that async
> > > > events is nice to have, so it isn't a bad idea.
> > >
> > > Could we drive it in the polling code? As in, if we set a flag to say
> > > we're event driven and please don't bother us, we could just respond to
> > > the poll with the last known state (this would probably have to be in
> > > SCSI somewhere since most polls are Test Unit Readys). That way ZPODD
> > > sets this flag when the device powers down and unsets it when it powers
> > > up.
> >
> > Does it mean I can do something like this:
> >
> > diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
> > index 5fc97d2..219820c 100644
> > --- a/drivers/scsi/sr.c
> > +++ b/drivers/scsi/sr.c
> > @@ -578,7 +578,10 @@ static unsigned int sr_block_check_events(struct gendisk *disk,
> > unsigned int clearing)
> > {
> > struct scsi_cd *cd = scsi_cd(disk);
> > - return cdrom_check_events(&cd->cdi, clearing);
> > + if (!cd->device->event_driven)
> > + return cdrom_check_events(&cd->cdi, clearing);
> > + else
> > + return 0;
> > }
> >
> > static int sr_block_revalidate_disk(struct gendisk *disk)
> > diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
> > index e65c62e..1756151 100644
> > --- a/include/scsi/scsi_device.h
> > +++ b/include/scsi/scsi_device.h
> > @@ -160,6 +160,7 @@ struct scsi_device {
> > unsigned can_power_off:1; /* Device supports runtime power off */
> > unsigned wce_default_on:1; /* Cache is ON by default */
> > unsigned no_dif:1; /* T10 PI (DIF) should be disabled */
> > + unsigned event_driven:1; /* No need to poll the device */
> >
> > DECLARE_BITMAP(supported_events, SDEV_EVT_MAXBITS); /* supported events */
> > struct list_head event_list; /* asserted events */
>
> Yes, but if we can get away with doing that, it should be in genhd
> because it's completely generic.
>
> I was imagining we'd have to fake the reply to the test unit ready or
> some other commands, which is why it would need to be in sr.c
>
> The check events code is Tejun's baby, if he's OK with it then just do
> it in genhd.c
I agree that it better be done in genhd, the only concern is, in that
case, this flag will need to be in struct device. I'm not sure if this
is acceptible though, since the whole events checking thing is for block
based devices only.
Ideally, it should be in struct disk_events, but I don't see a way for
libata to access that structure... so any suggestion of doing this? or
it is OK to add such a field to struct device?
Thanks,
Aaron
>
> > Then when ZPODD is powered off, it will set this flag; and unset it when
> > it is powered up.
>
> Right.
>
> James
>
>
^ permalink raw reply
* Re: [PATCH v9 06/10] ata: zpodd: check zero power ready status
From: James Bottomley @ 2012-12-03 8:25 UTC (permalink / raw)
To: Aaron Lu
Cc: Tejun Heo, Rafael J. Wysocki, linux-pm, Jeff Garzik, Alan Stern,
Jeff Wu, Aaron Lu, linux-ide, linux-scsi, linux-acpi
In-Reply-To: <20121203081321.GA9990@mint-spring.sh.intel.com>
On Mon, 2012-12-03 at 16:13 +0800, Aaron Lu wrote:
> On Wed, Nov 28, 2012 at 08:56:09AM +0000, James Bottomley wrote:
> > On Tue, 2012-11-27 at 17:39 -0800, Tejun Heo wrote:
> > > Hey, Rafael.
> > >
> > > On Wed, Nov 28, 2012 at 01:51:00AM +0100, Rafael J. Wysocki wrote:
> > > > Having considered that a bit more I'm now thinking that in fact the power state
> > > > the device is in at the moment doesn't really matter, so the polling code need
> > > > not really know what PM is doing. What it needs to know is that the device
> > > > will generate a hardware event when something interesting happens, so it is not
> > > > necessary to poll it.
> > > >
> > > > In this particular case it is related to power management (apparently, hardware
> > > > events will only be generated after the device has been put into ACPI D3cold,
> > > > or so Aaron seems to be claiming), but it need not be in general, at least in
> > > > principle.
> > > >
> > > > It looks like we need an "event driven" flag that the can be set in the lower
> > > > layers and read by the upper layers. I suppose this means it would need to be
> > > > in struct device, but not necessarily in the PM-specific part of it.
> > >
> > > We already have that. That's what gendisk->async_events is for (as
> > > opposed to gendisk->events). If all events are async_events, block
> > > won't poll for events, but I'm not sure that's the golden bullet.
> > >
> > > * None implements async_events yet and an API is missing -
> > > disk_check_events() - which is trivial to add, but it's the same
> > > story. We'll need a mechanism to shoot up notification from libata
> > > to block layer. It's admittedly easier to justify routing through
> > > SCSI tho. So, we're mostly shifting the problem. Given that async
> > > events is nice to have, so it isn't a bad idea.
> >
> > Could we drive it in the polling code? As in, if we set a flag to say
> > we're event driven and please don't bother us, we could just respond to
> > the poll with the last known state (this would probably have to be in
> > SCSI somewhere since most polls are Test Unit Readys). That way ZPODD
> > sets this flag when the device powers down and unsets it when it powers
> > up.
>
> Does it mean I can do something like this:
>
> diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
> index 5fc97d2..219820c 100644
> --- a/drivers/scsi/sr.c
> +++ b/drivers/scsi/sr.c
> @@ -578,7 +578,10 @@ static unsigned int sr_block_check_events(struct gendisk *disk,
> unsigned int clearing)
> {
> struct scsi_cd *cd = scsi_cd(disk);
> - return cdrom_check_events(&cd->cdi, clearing);
> + if (!cd->device->event_driven)
> + return cdrom_check_events(&cd->cdi, clearing);
> + else
> + return 0;
> }
>
> static int sr_block_revalidate_disk(struct gendisk *disk)
> diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
> index e65c62e..1756151 100644
> --- a/include/scsi/scsi_device.h
> +++ b/include/scsi/scsi_device.h
> @@ -160,6 +160,7 @@ struct scsi_device {
> unsigned can_power_off:1; /* Device supports runtime power off */
> unsigned wce_default_on:1; /* Cache is ON by default */
> unsigned no_dif:1; /* T10 PI (DIF) should be disabled */
> + unsigned event_driven:1; /* No need to poll the device */
>
> DECLARE_BITMAP(supported_events, SDEV_EVT_MAXBITS); /* supported events */
> struct list_head event_list; /* asserted events */
Yes, but if we can get away with doing that, it should be in genhd
because it's completely generic.
I was imagining we'd have to fake the reply to the test unit ready or
some other commands, which is why it would need to be in sr.c
The check events code is Tejun's baby, if he's OK with it then just do
it in genhd.c
> Then when ZPODD is powered off, it will set this flag; and unset it when
> it is powered up.
Right.
James
^ permalink raw reply
* Re: [RFC PATCH 2/7] Thermal: Create zone level APIs
From: Hongbo Zhang @ 2012-12-03 8:21 UTC (permalink / raw)
To: R, Durgadoss
Cc: Zhang, Rui, linux-pm@vger.kernel.org, wni@nvidia.com,
eduardo.valentin@ti.com, amit.kachhap@linaro.org,
sachin.kamat@linaro.org
In-Reply-To: <4D68720C2E767A4AA6A8796D42C8EB59230E20@BGSMSX101.gar.corp.intel.com>
On 3 December 2012 15:47, R, Durgadoss <durgadoss.r@intel.com> wrote:
>
>
>> -----Original Message-----
>> From: Hongbo Zhang [mailto:hongbo.zhang@linaro.org]
>> Sent: Monday, December 03, 2012 1:13 PM
>> To: R, Durgadoss
>> Cc: Zhang, Rui; linux-pm@vger.kernel.org; wni@nvidia.com;
>> eduardo.valentin@ti.com; amit.kachhap@linaro.org;
>> sachin.kamat@linaro.org
>> Subject: Re: [RFC PATCH 2/7] Thermal: Create zone level APIs
>>
>> On 17 November 2012 18:45, Durgadoss R <durgadoss.r@intel.com> wrote:
>> > This patch adds a new thermal_zone structure to
>> > thermal.h. Also, adds zone level APIs to the thermal
>> > framework.
>> >
>> > Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
>> > ---
>> > drivers/thermal/thermal_sys.c | 206
>> +++++++++++++++++++++++++++++++++++++++++
>> > include/linux/thermal.h | 25 +++++
>> > 2 files changed, 231 insertions(+)
>> >
>> > diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
>> > index e726c8b..9d386d7 100644
>> > --- a/drivers/thermal/thermal_sys.c
>> > +++ b/drivers/thermal/thermal_sys.c
>> > @@ -44,19 +44,28 @@ MODULE_DESCRIPTION("Generic thermal
>> management sysfs support");
>> > MODULE_LICENSE("GPL");
>> >
>> > static DEFINE_IDR(thermal_tz_idr);
>> > +static DEFINE_IDR(thermal_zone_idr);
>> > static DEFINE_IDR(thermal_cdev_idr);
>> > static DEFINE_IDR(thermal_sensor_idr);
>> > static DEFINE_MUTEX(thermal_idr_lock);
>> >
>> > static LIST_HEAD(thermal_tz_list);
>> > static LIST_HEAD(thermal_sensor_list);
>> > +static LIST_HEAD(thermal_zone_list);
>> > static LIST_HEAD(thermal_cdev_list);
>> > static LIST_HEAD(thermal_governor_list);
>> >
>> > static DEFINE_MUTEX(thermal_list_lock);
>> > static DEFINE_MUTEX(ts_list_lock);
>> > +static DEFINE_MUTEX(tz_list_lock);
>> > static DEFINE_MUTEX(thermal_governor_lock);
>> >
>> > +#define for_each_thermal_sensor(pos) \
>> > + list_for_each_entry(pos, &thermal_sensor_list, node)
>> > +
>> > +#define for_each_thermal_zone(pos) \
>> > + list_for_each_entry(pos, &thermal_zone_list, node)
>> > +
>> > static struct thermal_governor *__find_governor(const char *name)
>> > {
>> > struct thermal_governor *pos;
>> > @@ -419,15 +428,62 @@ static void thermal_zone_device_check(struct
>> work_struct *work)
>> > thermal_zone_device_update(tz);
>> > }
>> >
>> > +static int get_sensor_indx(struct thermal_zone *tz, struct thermal_sensor
>> *ts)
>> > +{
>> > + int i, indx = -EINVAL;
>> > +
>> > + if (!tz || !ts)
>> > + return -EINVAL;
>> > +
>> > + mutex_lock(&ts_list_lock);
>> > + for (i = 0; i < tz->sensor_indx; i++) {
>> > + if (tz->sensors[i] == ts) {
>> > + indx = i;
>> > + break;
>> > + }
>> > + }
>> > + mutex_unlock(&ts_list_lock);
>> > + return indx;
>> > +}
>> > +
>> > +static void remove_sensor_from_zone(struct thermal_zone *tz,
>> > + struct thermal_sensor *ts)
>> > +{
>> > + int indx, j;
>> > +
>> > + indx = get_sensor_indx(tz, ts);
>> > + if (indx < 0)
>> > + return;
>> > +
>> > + sysfs_remove_link(&tz->device.kobj, kobject_name(&ts-
>> >device.kobj));
>> > +
>> > + /* Shift the entries in the tz->sensors array */
>> > + for (j = indx; j < MAX_SENSORS_PER_ZONE - 1; j++)
>> > + tz->sensors[j] = tz->sensors[j + 1];
>> > +
>> > + tz->sensor_indx--;
>> > +}
>> > +
>> > /* sys I/F for thermal zone */
>> >
>> > #define to_thermal_zone(_dev) \
>> > container_of(_dev, struct thermal_zone_device, device)
>> >
>> > +#define to_zone(_dev) \
>> > + container_of(_dev, struct thermal_zone, device)
>> > +
>> > #define to_thermal_sensor(_dev) \
>> > container_of(_dev, struct thermal_sensor, device)
>> >
>> > static ssize_t
>> > +zone_name_show(struct device *dev, struct device_attribute *attr, char
>> *buf)
>> > +{
>> > + struct thermal_zone *tz = to_zone(dev);
>> > +
>> > + return sprintf(buf, "%s\n", tz->name);
>> > +}
>> > +
>> > +static ssize_t
>> > sensor_name_show(struct device *dev, struct device_attribute *attr, char
>> *buf)
>> > {
>> > struct thermal_sensor *ts = to_thermal_sensor(dev);
>> > @@ -772,6 +828,8 @@ static DEVICE_ATTR(policy, S_IRUGO | S_IWUSR,
>> policy_show, policy_store);
>> > static DEVICE_ATTR(sensor_name, 0444, sensor_name_show, NULL);
>> > static DEVICE_ATTR(temp_input, 0444, sensor_temp_show, NULL);
>> >
>> > +static DEVICE_ATTR(zone_name, 0444, zone_name_show, NULL);
>> > +
>> > /* sys I/F for cooling device */
>> > #define to_cooling_device(_dev) \
>> > container_of(_dev, struct thermal_cooling_device, device)
>> > @@ -1557,6 +1615,146 @@ static void remove_trip_attrs(struct
>> thermal_zone_device *tz)
>> > kfree(tz->trip_hyst_attrs);
>> > }
>> >
>> > +struct thermal_zone *create_thermal_zone(const char *name, void
>> *devdata)
>> > +{
>> > + struct thermal_zone *tz;
>> > + int ret;
>> > +
>> > + if (!name || (name && strlen(name) >= THERMAL_NAME_LENGTH))
>> > + return ERR_PTR(-EINVAL);
>> > +
>> > + tz = kzalloc(sizeof(*tz), GFP_KERNEL);
>> > + if (!tz)
>> > + return ERR_PTR(-ENOMEM);
>> > +
>> > + idr_init(&tz->idr);
>> > + ret = get_idr(&thermal_zone_idr, &thermal_idr_lock, &tz->id);
>> > + if (ret)
>> > + goto exit_free;
>> > +
>> > + strcpy(tz->name, name);
>> > + tz->devdata = devdata;
>> > + tz->device.class = &thermal_class;
>> > +
>> > + dev_set_name(&tz->device, "zone%d", tz->id);
>> > + ret = device_register(&tz->device);
>> > + if (ret)
>> > + goto exit_idr;
>> > +
>> > + ret = device_create_file(&tz->device, &dev_attr_zone_name);
>> > + if (ret)
>> > + goto exit_unregister;
>> > +
>> > + /* Add this zone to the global list of thermal zones */
>> > + mutex_lock(&tz_list_lock);
>> > + list_add_tail(&tz->node, &thermal_zone_list);
>> > + mutex_unlock(&tz_list_lock);
>> > + return tz;
>> > +
>> > +exit_unregister:
>> > + device_unregister(&tz->device);
>> > +exit_idr:
>> > + release_idr(&thermal_zone_idr, &thermal_idr_lock, tz->id);
>> > +exit_free:
>> > + kfree(tz);
>> > + return ERR_PTR(ret);
>> > +}
>> > +EXPORT_SYMBOL(create_thermal_zone);
>> > +
>> > +void remove_thermal_zone(struct thermal_zone *tz)
>> > +{
>> > + struct thermal_zone *pos, *next;
>> > + bool found = false;
>> > +
>> > + if (!tz)
>> > + return;
>> > +
>> > + mutex_lock(&tz_list_lock);
>> > +
>> > + list_for_each_entry_safe(pos, next, &thermal_zone_list, node) {
>> > + if (pos == tz) {
>> > + list_del(&tz->node);
>> > + found = true;
>> > + break;
>> > + }
>> > + }
>> > +
>> > + if (!found)
>> > + goto exit;
>> > +
>> > + device_remove_file(&tz->device, &dev_attr_zone_name);
>> > +
>> > + release_idr(&thermal_zone_idr, &thermal_idr_lock, tz->id);
>> > + idr_destroy(&tz->idr);
>> > +
>> > + device_unregister(&tz->device);
>> > + kfree(tz);
>> > +exit:
>> > + mutex_unlock(&tz_list_lock);
>> > + return;
>> > +}
>> > +EXPORT_SYMBOL(remove_thermal_zone);
>> > +
>> > +struct thermal_sensor *get_sensor_by_name(const char *name)
>> > +{
>> > + struct thermal_sensor *pos;
>> > + struct thermal_sensor *ts = NULL;
>> > +
>> > + mutex_lock(&ts_list_lock);
>> > + for_each_thermal_sensor(pos) {
>> > + if (!strnicmp(pos->name, name, THERMAL_NAME_LENGTH)) {
>> > + ts = pos;
>> > + break;
>> > + }
>> > + }
>> > + mutex_unlock(&ts_list_lock);
>> > + return ts;
>> > +}
>> > +EXPORT_SYMBOL(get_sensor_by_name);
>> > +
>> > +int add_sensor_to_zone_by_name(struct thermal_zone *tz, const char
>> *name)
>> > +{
>> > + int ret;
>> > + struct thermal_sensor *ts = get_sensor_by_name(name);
>> > +
>> > + if (!tz || !ts)
>> > + return -EINVAL;
>> > +
>> > + mutex_lock(&tz_list_lock);
>> > +
>> > + /* Ensure we are not adding the same sensor again!! */
>> > + ret = get_sensor_indx(tz, ts);
>> > + if (ret >= 0) {
>> > + ret = -EEXIST;
>> > + goto exit_zone;
>> > + }
>> > +
>> > + mutex_lock(&ts_list_lock);
>> > +
>> > + ret = sysfs_create_link(&tz->device.kobj, &ts->device.kobj,
>> > + kobject_name(&ts->device.kobj));
>> > + if (ret)
>> > + goto exit_sensor;
>> > +
>> > + tz->sensors[tz->sensor_indx++] = ts;
>> > +
>> > +exit_sensor:
>> > + mutex_unlock(&ts_list_lock);
>> > +exit_zone:
>> > + mutex_unlock(&tz_list_lock);
>> > + return ret;
>> > +}
>> > +EXPORT_SYMBOL(add_sensor_to_zone_by_name);
>> > +
>> > +int add_sensor_to_zone(struct thermal_zone *tz, struct thermal_sensor
>> *ts)
>> > +{
>> > + if (!tz || !ts)
>> > + return -EINVAL;
>> > +
>> > + return add_sensor_to_zone_by_name(tz, ts->name);
>> > +}
>> > +EXPORT_SYMBOL(add_sensor_to_zone);
>> > +
>> > /**
>> > * thermal_sensor_register - register a new thermal sensor
>> > * @name: name of the thermal sensor
>> > @@ -1624,6 +1822,7 @@ EXPORT_SYMBOL(thermal_sensor_register);
>> > void thermal_sensor_unregister(struct thermal_sensor *ts)
>> > {
>> > int i;
>> > + struct thermal_zone *tz;
>> > struct thermal_sensor *pos, *next;
>> > bool found = false;
>> >
>> > @@ -1643,6 +1842,13 @@ void thermal_sensor_unregister(struct
>> thermal_sensor *ts)
>> > if (!found)
>> > return;
>> >
>> > + mutex_lock(&tz_list_lock);
>> > +
>> > + for_each_thermal_zone(tz)
>> > + remove_sensor_from_zone(tz, ts);
>> > +
>> > + mutex_unlock(&tz_list_lock);
>> > +
>> > for (i = 0; i < ts->thresholds; i++)
>> > device_remove_file(&ts->device, &ts->thresh_attrs[i].attr);
>> >
>> > diff --git a/include/linux/thermal.h b/include/linux/thermal.h
>> > index e2f85ec..38438be 100644
>> > --- a/include/linux/thermal.h
>> > +++ b/include/linux/thermal.h
>> > @@ -49,6 +49,11 @@
>> > /* Default Thermal Governor: Does Linear Throttling */
>> > #define DEFAULT_THERMAL_GOVERNOR "step_wise"
>> >
>> > +/* Maximum number of sensors, allowed in a thermal zone
>> > + * We will start with 5, and increase if needed.
>> > + */
>> > +#define MAX_SENSORS_PER_ZONE 5
>> > +
>> > struct thermal_sensor;
>> > struct thermal_zone_device;
>> > struct thermal_cooling_device;
>> > @@ -191,6 +196,21 @@ struct thermal_zone_device {
>> > struct delayed_work poll_queue;
>> > };
>> >
>> > +struct thermal_zone {
>> > + char name[THERMAL_NAME_LENGTH];
>> > + int id;
>> > + void *devdata;
>> > + struct thermal_zone *ops;
>> What is this, thermal_zone_device_ops? and how to initialize this ops?
>
> oh, Not required, for now. Will remove it..
> Good catch :-)
I am still confused a bit.
Is this thermal_zone going to take the place of the old thermal_zone_device?
If yes, which functions/callbacks are used to access
thermal_zone->trip[] like the old get_trip_*?
>
> Thanks,
> Durga
>>
>> > + struct thermal_governor *governor;
>> > + struct idr idr;
>> > + struct device device;
>> > + struct list_head node;
>> > +
>> > + /* Sensor level information */
>> > + int sensor_indx; /* index into 'sensors' array */
>> > + struct thermal_sensor *sensors[MAX_SENSORS_PER_ZONE];
>> > +};
>> > +
>> > /* Structure that holds thermal governor information */
>> > struct thermal_governor {
>> > char name[THERMAL_NAME_LENGTH];
>> > @@ -264,6 +284,11 @@ struct thermal_sensor
>> *thermal_sensor_register(const char *,
>> > void thermal_sensor_unregister(struct thermal_sensor *);
>> > int enable_sensor_thresholds(struct thermal_sensor *, int);
>> >
>> > +struct thermal_zone *create_thermal_zone(const char *, void *);
>> > +void remove_thermal_zone(struct thermal_zone *);
>> > +int add_sensor_to_zone(struct thermal_zone *, struct thermal_sensor
>> *);
>> > +int add_sensor_to_zone_by_name(struct thermal_zone *, const char *);
>> > +
>> > #ifdef CONFIG_NET
>> > extern int thermal_generate_netlink_event(u32 orig, enum events
>> event);
>> > #else
>> > --
>> > 1.7.9.5
>> >
^ permalink raw reply
* Re: [PATCH v9 06/10] ata: zpodd: check zero power ready status
From: Aaron Lu @ 2012-12-03 8:13 UTC (permalink / raw)
To: James Bottomley
Cc: Tejun Heo, Rafael J. Wysocki, linux-pm, Jeff Garzik, Alan Stern,
Jeff Wu, Aaron Lu, linux-ide, linux-scsi, linux-acpi
In-Reply-To: <1354092969.2276.49.camel@dabdike>
On Wed, Nov 28, 2012 at 08:56:09AM +0000, James Bottomley wrote:
> On Tue, 2012-11-27 at 17:39 -0800, Tejun Heo wrote:
> > Hey, Rafael.
> >
> > On Wed, Nov 28, 2012 at 01:51:00AM +0100, Rafael J. Wysocki wrote:
> > > Having considered that a bit more I'm now thinking that in fact the power state
> > > the device is in at the moment doesn't really matter, so the polling code need
> > > not really know what PM is doing. What it needs to know is that the device
> > > will generate a hardware event when something interesting happens, so it is not
> > > necessary to poll it.
> > >
> > > In this particular case it is related to power management (apparently, hardware
> > > events will only be generated after the device has been put into ACPI D3cold,
> > > or so Aaron seems to be claiming), but it need not be in general, at least in
> > > principle.
> > >
> > > It looks like we need an "event driven" flag that the can be set in the lower
> > > layers and read by the upper layers. I suppose this means it would need to be
> > > in struct device, but not necessarily in the PM-specific part of it.
> >
> > We already have that. That's what gendisk->async_events is for (as
> > opposed to gendisk->events). If all events are async_events, block
> > won't poll for events, but I'm not sure that's the golden bullet.
> >
> > * None implements async_events yet and an API is missing -
> > disk_check_events() - which is trivial to add, but it's the same
> > story. We'll need a mechanism to shoot up notification from libata
> > to block layer. It's admittedly easier to justify routing through
> > SCSI tho. So, we're mostly shifting the problem. Given that async
> > events is nice to have, so it isn't a bad idea.
>
> Could we drive it in the polling code? As in, if we set a flag to say
> we're event driven and please don't bother us, we could just respond to
> the poll with the last known state (this would probably have to be in
> SCSI somewhere since most polls are Test Unit Readys). That way ZPODD
> sets this flag when the device powers down and unsets it when it powers
> up.
Does it mean I can do something like this:
diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
index 5fc97d2..219820c 100644
--- a/drivers/scsi/sr.c
+++ b/drivers/scsi/sr.c
@@ -578,7 +578,10 @@ static unsigned int sr_block_check_events(struct gendisk *disk,
unsigned int clearing)
{
struct scsi_cd *cd = scsi_cd(disk);
- return cdrom_check_events(&cd->cdi, clearing);
+ if (!cd->device->event_driven)
+ return cdrom_check_events(&cd->cdi, clearing);
+ else
+ return 0;
}
static int sr_block_revalidate_disk(struct gendisk *disk)
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index e65c62e..1756151 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -160,6 +160,7 @@ struct scsi_device {
unsigned can_power_off:1; /* Device supports runtime power off */
unsigned wce_default_on:1; /* Cache is ON by default */
unsigned no_dif:1; /* T10 PI (DIF) should be disabled */
+ unsigned event_driven:1; /* No need to poll the device */
DECLARE_BITMAP(supported_events, SDEV_EVT_MAXBITS); /* supported events */
struct list_head event_list; /* asserted events */
Then when ZPODD is powered off, it will set this flag; and unset it when
it is powered up.
Thanks,
Aaron
>
> James
>
> > * Still dunno much about zpodd but IIUC the notification from
> > zero-power is via ACPI. To advertise that the device doesn't need
> > polling, it should also be able to do async notification while
> > powered up, which isn't covered by zpodd but ATA async notification.
> > So, ummm... that's another obstacle. If zpodd requires the device
> > to support ATA async notification, it might not be too bad tho.
>
>
>
^ permalink raw reply related
* Re: [RFC PATCH 2/7] Thermal: Create zone level APIs
From: Hongbo Zhang @ 2012-12-03 7:42 UTC (permalink / raw)
To: Durgadoss R
Cc: rui.zhang, linux-pm, wni, eduardo.valentin, amit.kachhap,
sachin.kamat
In-Reply-To: <1353149158-19102-3-git-send-email-durgadoss.r@intel.com>
On 17 November 2012 18:45, Durgadoss R <durgadoss.r@intel.com> wrote:
> This patch adds a new thermal_zone structure to
> thermal.h. Also, adds zone level APIs to the thermal
> framework.
>
> Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
> ---
> drivers/thermal/thermal_sys.c | 206 +++++++++++++++++++++++++++++++++++++++++
> include/linux/thermal.h | 25 +++++
> 2 files changed, 231 insertions(+)
>
> diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
> index e726c8b..9d386d7 100644
> --- a/drivers/thermal/thermal_sys.c
> +++ b/drivers/thermal/thermal_sys.c
> @@ -44,19 +44,28 @@ MODULE_DESCRIPTION("Generic thermal management sysfs support");
> MODULE_LICENSE("GPL");
>
> static DEFINE_IDR(thermal_tz_idr);
> +static DEFINE_IDR(thermal_zone_idr);
> static DEFINE_IDR(thermal_cdev_idr);
> static DEFINE_IDR(thermal_sensor_idr);
> static DEFINE_MUTEX(thermal_idr_lock);
>
> static LIST_HEAD(thermal_tz_list);
> static LIST_HEAD(thermal_sensor_list);
> +static LIST_HEAD(thermal_zone_list);
> static LIST_HEAD(thermal_cdev_list);
> static LIST_HEAD(thermal_governor_list);
>
> static DEFINE_MUTEX(thermal_list_lock);
> static DEFINE_MUTEX(ts_list_lock);
> +static DEFINE_MUTEX(tz_list_lock);
> static DEFINE_MUTEX(thermal_governor_lock);
>
> +#define for_each_thermal_sensor(pos) \
> + list_for_each_entry(pos, &thermal_sensor_list, node)
> +
> +#define for_each_thermal_zone(pos) \
> + list_for_each_entry(pos, &thermal_zone_list, node)
> +
> static struct thermal_governor *__find_governor(const char *name)
> {
> struct thermal_governor *pos;
> @@ -419,15 +428,62 @@ static void thermal_zone_device_check(struct work_struct *work)
> thermal_zone_device_update(tz);
> }
>
> +static int get_sensor_indx(struct thermal_zone *tz, struct thermal_sensor *ts)
> +{
> + int i, indx = -EINVAL;
> +
> + if (!tz || !ts)
> + return -EINVAL;
> +
> + mutex_lock(&ts_list_lock);
> + for (i = 0; i < tz->sensor_indx; i++) {
> + if (tz->sensors[i] == ts) {
> + indx = i;
> + break;
> + }
> + }
> + mutex_unlock(&ts_list_lock);
> + return indx;
> +}
> +
> +static void remove_sensor_from_zone(struct thermal_zone *tz,
> + struct thermal_sensor *ts)
> +{
> + int indx, j;
> +
> + indx = get_sensor_indx(tz, ts);
> + if (indx < 0)
> + return;
> +
> + sysfs_remove_link(&tz->device.kobj, kobject_name(&ts->device.kobj));
> +
> + /* Shift the entries in the tz->sensors array */
> + for (j = indx; j < MAX_SENSORS_PER_ZONE - 1; j++)
> + tz->sensors[j] = tz->sensors[j + 1];
> +
> + tz->sensor_indx--;
> +}
> +
> /* sys I/F for thermal zone */
>
> #define to_thermal_zone(_dev) \
> container_of(_dev, struct thermal_zone_device, device)
>
> +#define to_zone(_dev) \
> + container_of(_dev, struct thermal_zone, device)
> +
> #define to_thermal_sensor(_dev) \
> container_of(_dev, struct thermal_sensor, device)
>
> static ssize_t
> +zone_name_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> + struct thermal_zone *tz = to_zone(dev);
> +
> + return sprintf(buf, "%s\n", tz->name);
> +}
> +
> +static ssize_t
> sensor_name_show(struct device *dev, struct device_attribute *attr, char *buf)
> {
> struct thermal_sensor *ts = to_thermal_sensor(dev);
> @@ -772,6 +828,8 @@ static DEVICE_ATTR(policy, S_IRUGO | S_IWUSR, policy_show, policy_store);
> static DEVICE_ATTR(sensor_name, 0444, sensor_name_show, NULL);
> static DEVICE_ATTR(temp_input, 0444, sensor_temp_show, NULL);
>
> +static DEVICE_ATTR(zone_name, 0444, zone_name_show, NULL);
> +
> /* sys I/F for cooling device */
> #define to_cooling_device(_dev) \
> container_of(_dev, struct thermal_cooling_device, device)
> @@ -1557,6 +1615,146 @@ static void remove_trip_attrs(struct thermal_zone_device *tz)
> kfree(tz->trip_hyst_attrs);
> }
>
> +struct thermal_zone *create_thermal_zone(const char *name, void *devdata)
> +{
> + struct thermal_zone *tz;
> + int ret;
> +
> + if (!name || (name && strlen(name) >= THERMAL_NAME_LENGTH))
> + return ERR_PTR(-EINVAL);
> +
> + tz = kzalloc(sizeof(*tz), GFP_KERNEL);
> + if (!tz)
> + return ERR_PTR(-ENOMEM);
> +
> + idr_init(&tz->idr);
> + ret = get_idr(&thermal_zone_idr, &thermal_idr_lock, &tz->id);
> + if (ret)
> + goto exit_free;
> +
> + strcpy(tz->name, name);
> + tz->devdata = devdata;
> + tz->device.class = &thermal_class;
> +
> + dev_set_name(&tz->device, "zone%d", tz->id);
> + ret = device_register(&tz->device);
> + if (ret)
> + goto exit_idr;
> +
> + ret = device_create_file(&tz->device, &dev_attr_zone_name);
> + if (ret)
> + goto exit_unregister;
> +
> + /* Add this zone to the global list of thermal zones */
> + mutex_lock(&tz_list_lock);
> + list_add_tail(&tz->node, &thermal_zone_list);
> + mutex_unlock(&tz_list_lock);
> + return tz;
> +
> +exit_unregister:
> + device_unregister(&tz->device);
> +exit_idr:
> + release_idr(&thermal_zone_idr, &thermal_idr_lock, tz->id);
> +exit_free:
> + kfree(tz);
> + return ERR_PTR(ret);
> +}
> +EXPORT_SYMBOL(create_thermal_zone);
> +
> +void remove_thermal_zone(struct thermal_zone *tz)
> +{
> + struct thermal_zone *pos, *next;
> + bool found = false;
> +
> + if (!tz)
> + return;
> +
> + mutex_lock(&tz_list_lock);
> +
> + list_for_each_entry_safe(pos, next, &thermal_zone_list, node) {
> + if (pos == tz) {
> + list_del(&tz->node);
> + found = true;
> + break;
> + }
> + }
> +
> + if (!found)
> + goto exit;
> +
> + device_remove_file(&tz->device, &dev_attr_zone_name);
> +
> + release_idr(&thermal_zone_idr, &thermal_idr_lock, tz->id);
> + idr_destroy(&tz->idr);
> +
> + device_unregister(&tz->device);
> + kfree(tz);
> +exit:
> + mutex_unlock(&tz_list_lock);
> + return;
> +}
> +EXPORT_SYMBOL(remove_thermal_zone);
> +
> +struct thermal_sensor *get_sensor_by_name(const char *name)
> +{
> + struct thermal_sensor *pos;
> + struct thermal_sensor *ts = NULL;
> +
> + mutex_lock(&ts_list_lock);
> + for_each_thermal_sensor(pos) {
> + if (!strnicmp(pos->name, name, THERMAL_NAME_LENGTH)) {
> + ts = pos;
> + break;
> + }
> + }
> + mutex_unlock(&ts_list_lock);
> + return ts;
> +}
> +EXPORT_SYMBOL(get_sensor_by_name);
> +
> +int add_sensor_to_zone_by_name(struct thermal_zone *tz, const char *name)
> +{
> + int ret;
> + struct thermal_sensor *ts = get_sensor_by_name(name);
> +
> + if (!tz || !ts)
> + return -EINVAL;
> +
> + mutex_lock(&tz_list_lock);
> +
> + /* Ensure we are not adding the same sensor again!! */
> + ret = get_sensor_indx(tz, ts);
> + if (ret >= 0) {
> + ret = -EEXIST;
> + goto exit_zone;
> + }
> +
> + mutex_lock(&ts_list_lock);
> +
> + ret = sysfs_create_link(&tz->device.kobj, &ts->device.kobj,
> + kobject_name(&ts->device.kobj));
> + if (ret)
> + goto exit_sensor;
> +
> + tz->sensors[tz->sensor_indx++] = ts;
> +
> +exit_sensor:
> + mutex_unlock(&ts_list_lock);
> +exit_zone:
> + mutex_unlock(&tz_list_lock);
> + return ret;
> +}
> +EXPORT_SYMBOL(add_sensor_to_zone_by_name);
> +
> +int add_sensor_to_zone(struct thermal_zone *tz, struct thermal_sensor *ts)
> +{
> + if (!tz || !ts)
> + return -EINVAL;
> +
> + return add_sensor_to_zone_by_name(tz, ts->name);
> +}
> +EXPORT_SYMBOL(add_sensor_to_zone);
> +
> /**
> * thermal_sensor_register - register a new thermal sensor
> * @name: name of the thermal sensor
> @@ -1624,6 +1822,7 @@ EXPORT_SYMBOL(thermal_sensor_register);
> void thermal_sensor_unregister(struct thermal_sensor *ts)
> {
> int i;
> + struct thermal_zone *tz;
> struct thermal_sensor *pos, *next;
> bool found = false;
>
> @@ -1643,6 +1842,13 @@ void thermal_sensor_unregister(struct thermal_sensor *ts)
> if (!found)
> return;
>
> + mutex_lock(&tz_list_lock);
> +
> + for_each_thermal_zone(tz)
> + remove_sensor_from_zone(tz, ts);
> +
> + mutex_unlock(&tz_list_lock);
> +
> for (i = 0; i < ts->thresholds; i++)
> device_remove_file(&ts->device, &ts->thresh_attrs[i].attr);
>
> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> index e2f85ec..38438be 100644
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h
> @@ -49,6 +49,11 @@
> /* Default Thermal Governor: Does Linear Throttling */
> #define DEFAULT_THERMAL_GOVERNOR "step_wise"
>
> +/* Maximum number of sensors, allowed in a thermal zone
> + * We will start with 5, and increase if needed.
> + */
> +#define MAX_SENSORS_PER_ZONE 5
> +
> struct thermal_sensor;
> struct thermal_zone_device;
> struct thermal_cooling_device;
> @@ -191,6 +196,21 @@ struct thermal_zone_device {
> struct delayed_work poll_queue;
> };
>
> +struct thermal_zone {
> + char name[THERMAL_NAME_LENGTH];
> + int id;
> + void *devdata;
> + struct thermal_zone *ops;
What is this, thermal_zone_device_ops? and how to initialize this ops?
> + struct thermal_governor *governor;
> + struct idr idr;
> + struct device device;
> + struct list_head node;
> +
> + /* Sensor level information */
> + int sensor_indx; /* index into 'sensors' array */
> + struct thermal_sensor *sensors[MAX_SENSORS_PER_ZONE];
> +};
> +
> /* Structure that holds thermal governor information */
> struct thermal_governor {
> char name[THERMAL_NAME_LENGTH];
> @@ -264,6 +284,11 @@ struct thermal_sensor *thermal_sensor_register(const char *,
> void thermal_sensor_unregister(struct thermal_sensor *);
> int enable_sensor_thresholds(struct thermal_sensor *, int);
>
> +struct thermal_zone *create_thermal_zone(const char *, void *);
> +void remove_thermal_zone(struct thermal_zone *);
> +int add_sensor_to_zone(struct thermal_zone *, struct thermal_sensor *);
> +int add_sensor_to_zone_by_name(struct thermal_zone *, const char *);
> +
> #ifdef CONFIG_NET
> extern int thermal_generate_netlink_event(u32 orig, enum events event);
> #else
> --
> 1.7.9.5
>
^ permalink raw reply
* RE: [RFC PATCH 2/7] Thermal: Create zone level APIs
From: R, Durgadoss @ 2012-12-03 7:47 UTC (permalink / raw)
To: Hongbo Zhang
Cc: Zhang, Rui, linux-pm@vger.kernel.org, wni@nvidia.com,
eduardo.valentin@ti.com, amit.kachhap@linaro.org,
sachin.kamat@linaro.org
In-Reply-To: <CAJLyvQzptLcWPiJUquO2c2PegiMRt00SR+V=8kHA2wkZG4EFNQ@mail.gmail.com>
> -----Original Message-----
> From: Hongbo Zhang [mailto:hongbo.zhang@linaro.org]
> Sent: Monday, December 03, 2012 1:13 PM
> To: R, Durgadoss
> Cc: Zhang, Rui; linux-pm@vger.kernel.org; wni@nvidia.com;
> eduardo.valentin@ti.com; amit.kachhap@linaro.org;
> sachin.kamat@linaro.org
> Subject: Re: [RFC PATCH 2/7] Thermal: Create zone level APIs
>
> On 17 November 2012 18:45, Durgadoss R <durgadoss.r@intel.com> wrote:
> > This patch adds a new thermal_zone structure to
> > thermal.h. Also, adds zone level APIs to the thermal
> > framework.
> >
> > Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
> > ---
> > drivers/thermal/thermal_sys.c | 206
> +++++++++++++++++++++++++++++++++++++++++
> > include/linux/thermal.h | 25 +++++
> > 2 files changed, 231 insertions(+)
> >
> > diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
> > index e726c8b..9d386d7 100644
> > --- a/drivers/thermal/thermal_sys.c
> > +++ b/drivers/thermal/thermal_sys.c
> > @@ -44,19 +44,28 @@ MODULE_DESCRIPTION("Generic thermal
> management sysfs support");
> > MODULE_LICENSE("GPL");
> >
> > static DEFINE_IDR(thermal_tz_idr);
> > +static DEFINE_IDR(thermal_zone_idr);
> > static DEFINE_IDR(thermal_cdev_idr);
> > static DEFINE_IDR(thermal_sensor_idr);
> > static DEFINE_MUTEX(thermal_idr_lock);
> >
> > static LIST_HEAD(thermal_tz_list);
> > static LIST_HEAD(thermal_sensor_list);
> > +static LIST_HEAD(thermal_zone_list);
> > static LIST_HEAD(thermal_cdev_list);
> > static LIST_HEAD(thermal_governor_list);
> >
> > static DEFINE_MUTEX(thermal_list_lock);
> > static DEFINE_MUTEX(ts_list_lock);
> > +static DEFINE_MUTEX(tz_list_lock);
> > static DEFINE_MUTEX(thermal_governor_lock);
> >
> > +#define for_each_thermal_sensor(pos) \
> > + list_for_each_entry(pos, &thermal_sensor_list, node)
> > +
> > +#define for_each_thermal_zone(pos) \
> > + list_for_each_entry(pos, &thermal_zone_list, node)
> > +
> > static struct thermal_governor *__find_governor(const char *name)
> > {
> > struct thermal_governor *pos;
> > @@ -419,15 +428,62 @@ static void thermal_zone_device_check(struct
> work_struct *work)
> > thermal_zone_device_update(tz);
> > }
> >
> > +static int get_sensor_indx(struct thermal_zone *tz, struct thermal_sensor
> *ts)
> > +{
> > + int i, indx = -EINVAL;
> > +
> > + if (!tz || !ts)
> > + return -EINVAL;
> > +
> > + mutex_lock(&ts_list_lock);
> > + for (i = 0; i < tz->sensor_indx; i++) {
> > + if (tz->sensors[i] == ts) {
> > + indx = i;
> > + break;
> > + }
> > + }
> > + mutex_unlock(&ts_list_lock);
> > + return indx;
> > +}
> > +
> > +static void remove_sensor_from_zone(struct thermal_zone *tz,
> > + struct thermal_sensor *ts)
> > +{
> > + int indx, j;
> > +
> > + indx = get_sensor_indx(tz, ts);
> > + if (indx < 0)
> > + return;
> > +
> > + sysfs_remove_link(&tz->device.kobj, kobject_name(&ts-
> >device.kobj));
> > +
> > + /* Shift the entries in the tz->sensors array */
> > + for (j = indx; j < MAX_SENSORS_PER_ZONE - 1; j++)
> > + tz->sensors[j] = tz->sensors[j + 1];
> > +
> > + tz->sensor_indx--;
> > +}
> > +
> > /* sys I/F for thermal zone */
> >
> > #define to_thermal_zone(_dev) \
> > container_of(_dev, struct thermal_zone_device, device)
> >
> > +#define to_zone(_dev) \
> > + container_of(_dev, struct thermal_zone, device)
> > +
> > #define to_thermal_sensor(_dev) \
> > container_of(_dev, struct thermal_sensor, device)
> >
> > static ssize_t
> > +zone_name_show(struct device *dev, struct device_attribute *attr, char
> *buf)
> > +{
> > + struct thermal_zone *tz = to_zone(dev);
> > +
> > + return sprintf(buf, "%s\n", tz->name);
> > +}
> > +
> > +static ssize_t
> > sensor_name_show(struct device *dev, struct device_attribute *attr, char
> *buf)
> > {
> > struct thermal_sensor *ts = to_thermal_sensor(dev);
> > @@ -772,6 +828,8 @@ static DEVICE_ATTR(policy, S_IRUGO | S_IWUSR,
> policy_show, policy_store);
> > static DEVICE_ATTR(sensor_name, 0444, sensor_name_show, NULL);
> > static DEVICE_ATTR(temp_input, 0444, sensor_temp_show, NULL);
> >
> > +static DEVICE_ATTR(zone_name, 0444, zone_name_show, NULL);
> > +
> > /* sys I/F for cooling device */
> > #define to_cooling_device(_dev) \
> > container_of(_dev, struct thermal_cooling_device, device)
> > @@ -1557,6 +1615,146 @@ static void remove_trip_attrs(struct
> thermal_zone_device *tz)
> > kfree(tz->trip_hyst_attrs);
> > }
> >
> > +struct thermal_zone *create_thermal_zone(const char *name, void
> *devdata)
> > +{
> > + struct thermal_zone *tz;
> > + int ret;
> > +
> > + if (!name || (name && strlen(name) >= THERMAL_NAME_LENGTH))
> > + return ERR_PTR(-EINVAL);
> > +
> > + tz = kzalloc(sizeof(*tz), GFP_KERNEL);
> > + if (!tz)
> > + return ERR_PTR(-ENOMEM);
> > +
> > + idr_init(&tz->idr);
> > + ret = get_idr(&thermal_zone_idr, &thermal_idr_lock, &tz->id);
> > + if (ret)
> > + goto exit_free;
> > +
> > + strcpy(tz->name, name);
> > + tz->devdata = devdata;
> > + tz->device.class = &thermal_class;
> > +
> > + dev_set_name(&tz->device, "zone%d", tz->id);
> > + ret = device_register(&tz->device);
> > + if (ret)
> > + goto exit_idr;
> > +
> > + ret = device_create_file(&tz->device, &dev_attr_zone_name);
> > + if (ret)
> > + goto exit_unregister;
> > +
> > + /* Add this zone to the global list of thermal zones */
> > + mutex_lock(&tz_list_lock);
> > + list_add_tail(&tz->node, &thermal_zone_list);
> > + mutex_unlock(&tz_list_lock);
> > + return tz;
> > +
> > +exit_unregister:
> > + device_unregister(&tz->device);
> > +exit_idr:
> > + release_idr(&thermal_zone_idr, &thermal_idr_lock, tz->id);
> > +exit_free:
> > + kfree(tz);
> > + return ERR_PTR(ret);
> > +}
> > +EXPORT_SYMBOL(create_thermal_zone);
> > +
> > +void remove_thermal_zone(struct thermal_zone *tz)
> > +{
> > + struct thermal_zone *pos, *next;
> > + bool found = false;
> > +
> > + if (!tz)
> > + return;
> > +
> > + mutex_lock(&tz_list_lock);
> > +
> > + list_for_each_entry_safe(pos, next, &thermal_zone_list, node) {
> > + if (pos == tz) {
> > + list_del(&tz->node);
> > + found = true;
> > + break;
> > + }
> > + }
> > +
> > + if (!found)
> > + goto exit;
> > +
> > + device_remove_file(&tz->device, &dev_attr_zone_name);
> > +
> > + release_idr(&thermal_zone_idr, &thermal_idr_lock, tz->id);
> > + idr_destroy(&tz->idr);
> > +
> > + device_unregister(&tz->device);
> > + kfree(tz);
> > +exit:
> > + mutex_unlock(&tz_list_lock);
> > + return;
> > +}
> > +EXPORT_SYMBOL(remove_thermal_zone);
> > +
> > +struct thermal_sensor *get_sensor_by_name(const char *name)
> > +{
> > + struct thermal_sensor *pos;
> > + struct thermal_sensor *ts = NULL;
> > +
> > + mutex_lock(&ts_list_lock);
> > + for_each_thermal_sensor(pos) {
> > + if (!strnicmp(pos->name, name, THERMAL_NAME_LENGTH)) {
> > + ts = pos;
> > + break;
> > + }
> > + }
> > + mutex_unlock(&ts_list_lock);
> > + return ts;
> > +}
> > +EXPORT_SYMBOL(get_sensor_by_name);
> > +
> > +int add_sensor_to_zone_by_name(struct thermal_zone *tz, const char
> *name)
> > +{
> > + int ret;
> > + struct thermal_sensor *ts = get_sensor_by_name(name);
> > +
> > + if (!tz || !ts)
> > + return -EINVAL;
> > +
> > + mutex_lock(&tz_list_lock);
> > +
> > + /* Ensure we are not adding the same sensor again!! */
> > + ret = get_sensor_indx(tz, ts);
> > + if (ret >= 0) {
> > + ret = -EEXIST;
> > + goto exit_zone;
> > + }
> > +
> > + mutex_lock(&ts_list_lock);
> > +
> > + ret = sysfs_create_link(&tz->device.kobj, &ts->device.kobj,
> > + kobject_name(&ts->device.kobj));
> > + if (ret)
> > + goto exit_sensor;
> > +
> > + tz->sensors[tz->sensor_indx++] = ts;
> > +
> > +exit_sensor:
> > + mutex_unlock(&ts_list_lock);
> > +exit_zone:
> > + mutex_unlock(&tz_list_lock);
> > + return ret;
> > +}
> > +EXPORT_SYMBOL(add_sensor_to_zone_by_name);
> > +
> > +int add_sensor_to_zone(struct thermal_zone *tz, struct thermal_sensor
> *ts)
> > +{
> > + if (!tz || !ts)
> > + return -EINVAL;
> > +
> > + return add_sensor_to_zone_by_name(tz, ts->name);
> > +}
> > +EXPORT_SYMBOL(add_sensor_to_zone);
> > +
> > /**
> > * thermal_sensor_register - register a new thermal sensor
> > * @name: name of the thermal sensor
> > @@ -1624,6 +1822,7 @@ EXPORT_SYMBOL(thermal_sensor_register);
> > void thermal_sensor_unregister(struct thermal_sensor *ts)
> > {
> > int i;
> > + struct thermal_zone *tz;
> > struct thermal_sensor *pos, *next;
> > bool found = false;
> >
> > @@ -1643,6 +1842,13 @@ void thermal_sensor_unregister(struct
> thermal_sensor *ts)
> > if (!found)
> > return;
> >
> > + mutex_lock(&tz_list_lock);
> > +
> > + for_each_thermal_zone(tz)
> > + remove_sensor_from_zone(tz, ts);
> > +
> > + mutex_unlock(&tz_list_lock);
> > +
> > for (i = 0; i < ts->thresholds; i++)
> > device_remove_file(&ts->device, &ts->thresh_attrs[i].attr);
> >
> > diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> > index e2f85ec..38438be 100644
> > --- a/include/linux/thermal.h
> > +++ b/include/linux/thermal.h
> > @@ -49,6 +49,11 @@
> > /* Default Thermal Governor: Does Linear Throttling */
> > #define DEFAULT_THERMAL_GOVERNOR "step_wise"
> >
> > +/* Maximum number of sensors, allowed in a thermal zone
> > + * We will start with 5, and increase if needed.
> > + */
> > +#define MAX_SENSORS_PER_ZONE 5
> > +
> > struct thermal_sensor;
> > struct thermal_zone_device;
> > struct thermal_cooling_device;
> > @@ -191,6 +196,21 @@ struct thermal_zone_device {
> > struct delayed_work poll_queue;
> > };
> >
> > +struct thermal_zone {
> > + char name[THERMAL_NAME_LENGTH];
> > + int id;
> > + void *devdata;
> > + struct thermal_zone *ops;
> What is this, thermal_zone_device_ops? and how to initialize this ops?
oh, Not required, for now. Will remove it..
Good catch :-)
Thanks,
Durga
>
> > + struct thermal_governor *governor;
> > + struct idr idr;
> > + struct device device;
> > + struct list_head node;
> > +
> > + /* Sensor level information */
> > + int sensor_indx; /* index into 'sensors' array */
> > + struct thermal_sensor *sensors[MAX_SENSORS_PER_ZONE];
> > +};
> > +
> > /* Structure that holds thermal governor information */
> > struct thermal_governor {
> > char name[THERMAL_NAME_LENGTH];
> > @@ -264,6 +284,11 @@ struct thermal_sensor
> *thermal_sensor_register(const char *,
> > void thermal_sensor_unregister(struct thermal_sensor *);
> > int enable_sensor_thresholds(struct thermal_sensor *, int);
> >
> > +struct thermal_zone *create_thermal_zone(const char *, void *);
> > +void remove_thermal_zone(struct thermal_zone *);
> > +int add_sensor_to_zone(struct thermal_zone *, struct thermal_sensor
> *);
> > +int add_sensor_to_zone_by_name(struct thermal_zone *, const char *);
> > +
> > #ifdef CONFIG_NET
> > extern int thermal_generate_netlink_event(u32 orig, enum events
> event);
> > #else
> > --
> > 1.7.9.5
> >
^ permalink raw reply
* RE: [RFC PATCH 6/7] Thermal: Add Documentation to new APIs
From: R, Durgadoss @ 2012-12-03 7:44 UTC (permalink / raw)
To: Hongbo Zhang
Cc: Zhang, Rui, linux-pm@vger.kernel.org, wni@nvidia.com,
eduardo.valentin@ti.com, amit.kachhap@linaro.org,
sachin.kamat@linaro.org
In-Reply-To: <CAJLyvQxOm8qh4a+7X+ttCJ4p1068m=YZeAtNXyq0FsOQ6MY2iQ@mail.gmail.com>
Yes, it's a typo.
Will fix it in the next submission.
Thanks,
Durga
> -----Original Message-----
> From: Hongbo Zhang [mailto:hongbo.zhang@linaro.org]
> Sent: Monday, December 03, 2012 12:50 PM
> To: R, Durgadoss
> Cc: Zhang, Rui; linux-pm@vger.kernel.org; wni@nvidia.com;
> eduardo.valentin@ti.com; amit.kachhap@linaro.org;
> sachin.kamat@linaro.org
> Subject: Re: [RFC PATCH 6/7] Thermal: Add Documentation to new APIs
>
> On 17 November 2012 18:45, Durgadoss R <durgadoss.r@intel.com> wrote:
> > This patch adds Documentation for the new APIs
> > introduced in this patch set. The documentation
> > also has a model sysfs structure for reference.
> >
> > Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
> > ---
> > Documentation/thermal/sysfs-api2.txt | 213
> ++++++++++++++++++++++++++++++++++
> > 1 file changed, 213 insertions(+)
> > create mode 100644 Documentation/thermal/sysfs-api2.txt
> >
> > diff --git a/Documentation/thermal/sysfs-api2.txt
> b/Documentation/thermal/sysfs-api2.txt
> > new file mode 100644
> > index 0000000..be67125
> > --- /dev/null
> > +++ b/Documentation/thermal/sysfs-api2.txt
> > @@ -0,0 +1,213 @@
> > +Thermal Framework
> > +-----------------
> > +
> > +Written by Durgadoss R <durgadoss.r@intel.com>
> > +Copyright (c) 2012 Intel Corporation
> > +
> > +Created on: 4 November 2012
> > +
> > +0. Introduction
> > +---------------
> > +The Linux thermal framework provides a set of interfaces for thermal
> > +sensors and thermal cooling devices (fan, processor...) to register
> > +with the thermal management solution and to be a part of it.
> > +
> > +This document focuses on how to enable new thermal sensors and
> cooling
> > +devices to participate in thermal management. This solution is intended
> > +to be 'light-weight' and platform/architecture independent. Any thermal
> > +sensor/cooling device should be able to use the infrastructure easily.
> > +
> > +The goal of thermal framework is to expose the thermal sensor/zone and
> > +cooling device attributes in a consistent way. This will help the
> > +thermal governors to make use of the information to manage platform
> > +thermals efficiently.
> > +
> > +1. Terminology
> > +--------------
> > +This section describes the terminology used in the rest of this
> > +document as well as the thermal framework code.
> > +
> > +thermal_sensor: Hardware that can report temperature of a particular
> > + spot in the platform, where it is placed. The temperature
> > + reported by the sensor is the 'real' temperature reported
> > + by the hardware.
> > +thermal_zone: A virtual area on the device, that gets heated up. It may
> > + have one or more thermal sensors attached to it.
> > +cooling_device: Any component that can help in reducing the
> temperature of
> > + a 'hot spot' either by reducing its performance (passive
> > + cooling) or by other means(Active cooling E.g. Fan)
> > +
> > +trip_points: Various temperature levels for each sensor. As of now, we
> > + have four levels namely active, passive, hot and critical.
> > + Hot and critical trip point support only one value whereas
> > + active and passive can have any number of values. These
> > + temperature values can come from platform data, and are
> > + exposed through sysfs in a consistent manner. Stand-alone
> > + thermal sensor drivers are not expected to know these values.
> > + These values are RO.
> > +thresholds: These are programmable temperature limits, on reaching
> which
> > + the thermal sensor generates an interrupt. The framework is
> > + notified about this interrupt to take appropriate action.
> > + There can be as many number of thresholds as that of the
> > + hardware supports. These values are RW.
> > +
> > +thermal_map: This provides the mapping (aka binding) information
> between
> > + various sensors and cooling devices in a particular zone.
> > + Typically, this also comes from platform data; Stand-alone
> > + sensor drivers or cooling device drivers are not expected
> > + to know these mapping information.
> > +
> > +2. Thermal framework APIs
> > +-------------------------
> > +2.1: For Thermal Sensors
> > +2.1.1 thermal_sensor_register:
> > + This function creates a new sensor directory under /sys/class/thermal/
> > + as sensor[0-*]. This API is expected to be called by thermal sensor
> > + drivers. These drivers may or may not be in thermal subsystem. This
> > + function returns a thermal_sensor structure on success and
> appropriate
> > + error on failure.
> > +
> > + name: Name of the sensor
> > + devdata: Device private data
> > + ops: Thermal sensor callbacks
> > + .get_temp: obtain the current temperature of the sensor
> > + .get_trend: obtain the trend of the sensor
> > + .get_threshold: get a particular threshold temperature
> > + .set_threshold: set a particular threshold temperature
> > +
> > +2.1.2 thermal_sensor_unregister:
> > + This function deletes the sensor directory under /sys/class/thermal/
> > + for the given sensor. Thermal sensor drivers may call this API
> > + during the driver's 'exit' routine.
> > +
> > + ts: Thermal sensor that has to be unregistered
> > +
> > +2.1.3 enable_sensor_thresholds:
> > + This function creates 'threshold[0-*]' attributes under a particular
> > + sensorX directory. These values are RW. This function is called by
> > + the sensr driver only if the sensor supports interrupt mechanism.
> > +
> > + ts: Thermal sensor for which thresholds have to be enabled
> > + num_thresholds: Number of thresholds supported by the sensor
> > +
> > +2.2: For Cooling Devices
> > +2.2.1 thermal_cooling_device_register:
> > + This function adds a new thermal cooling device (fan/processor/...)
> > + to /sys/class/thermal/ folder as cooling_device[0-*]. This function
> > + is expected to be called by cooling device drivers that may be
> > + present in other subsystems also.
> > +
> > + name: the cooling device name
> > + devdata: device private data
> > + ops: thermal cooling devices callbacks
> > + .get_max_state: get the Maximum throttle state of the cooling device
> > + .get_cur_state: get the Current throttle state of the cooling device
> > + .set_cur_state: set the Current throttle state of the cooling device
> > +
> > +2.2.2 thermal_cooling_device_unregister:
> > + This function deletes the given cdev entry form /sys/class/thermal;
> > + and also cleans all the symlinks referred from various zones.
> > +
> > + cdev: Cooling device to be unregistered
> > +
> > +2.3: For Thermal Zones
> > +2.3.1 create_thermal_zone:
> > + This function adds a new 'zone' under /sys/class/thermal/
> > + directory as zone[0-*]. This zone has at least one thermal
> > + sensor and at most MAX_SENSORS_PER_ZONE number of sensors
> > + attached to it. Similarly, this zone has at least one cdev
> > + and at most MAX_CDEVS_PER_ZONE number of cdevs attached to it.
> > + Both the MAX_*_PER_ZONE values are configurable, through
> > + Kconfig option(during 'menuconfig').
> > +
> > + name: Name of the thermal zone
> > + devdata: Device private data
> > +
> > +2.3.2 add_sensor_to_zone
> > + This function adds a 'sensorX' entry under /sys/class/thermal/
> > + zoneY/ directory. This 'sensorX' is a symlink to the actual
> > + sensor entry under /sys/class/thermal/. Correspondingly, the
> > + method remove_sensor_from_zone deletes the symlink.
> > +
> > + tz: thermal zone structure
> > + ts: thermal sensor structure
> > +
> > +2.3.3 add_cdev_to_zone
> > + This function adds a 'cdevX' entry under /sys/class/thermal/
> > + zoneY/ directory. This 'cdevX' is a symlink to the actual
> > + cdev entry under /sys/class/thermal/. Correspondingly, the
> > + method remove_cdev_from_zone deletes the symlink.
> > +
> > + tz: thermal zone structure
> > + cdev: thermal cooling device structure
> > +
> > +2.4 For Thermal Trip
> > +2.4.1 add_sensor_trip_info
> > + This function adds trip point information for the given sensor,
> > + (under a given zone) under /sys/class/thermal/zoneX/thermal_trip/
> > + This API creates 4 sysfs attributes namely active, passive, hot,
> > + and critical. Each of these hold one or more trip point temperature
> > + values, as provided from platform data.
> > +
> > + tz: thermal zone structure
> > + ts: thermal sensor to which the trip points are attached
> > + trip: trip point structure. Usually obtained from platform data
> > +
> > +2.5 For Thermal Map
> > +2.5.1 add_map_entry
> > + This function adds a 'map[0-*]' sysfs attribute under
> > + /sys/class/thermal/zoneX/thermal_map/. Each map holds a space
> > + separated list of values, that specify the binding relationship
> > + between a sensor and a cdev in the given zone. The map structure
> > + is typically obtained as platform data. For example, through
> > + ACPI tables, SFI tables, Device tree etc.
> > +
> > + tz: thermal zone to which a 'map' is being added
> > + map: thermal_map structure
> > +
> > +3. Sysfs attributes structure
> > +-----------------------------
> > +Thermal sysfs attributes will be represented under /sys/class/thermal.
> > +
> > +3.1: For Thermal Sensors
> > + /sys/class/thermal/thermal_zone[0-*]:
> Should be /sys/class/thermal/sensor[0-*] ?
>
> > + |---type: Name of the thermal sensor
> > + |---temp_input: Current temperature in mC
> > + |---threshold[0-*]: Threshold temperature
> > +
> > +3.2: For Thermal Cooling Devices
> > + /sys/class/thermal/cooling_device[0-*]:
> > + |---type: Type of the cooling device
> > + |---max_state: Maximum throttle state of the cdev
> > + |---cur_state: Current throttle state of the cdev
> > +
> > +3.3: For Thermal Zones
> > + /sys/class/thermal/zone[0-*]:
> > + |---name: Name of the thermal
> > + |---sensorX: Symlink to ../sensorX
> > + |---cdevY: Symlink to ../cdevY
> > + |---thermal_trip: trip point values for sensors
> > + |---thermal_map: mapping info between sensors and cdevs
> > +
> > +3.4: For Thermal Trip
> > + This attribute represents the trip point values for all sensors
> > + present in the thermal zone. All values are in mC.
> > + /sys/class/thermal/zoneX/thermal_trip/sensorY:
> > + |---hot: hot trip point value
> > + |---critical: critical trip point value
> > + |---passive: list of passive trip point values
> > + |---active: list of active trip point values
> > +
> > +3.5: For Thermal Map
> > + Each attribute represents the mapping/binding information between
> > + a sensor and a cdev, together with a trip type.
> > + /sys/class/thermal/zoneX/thermal_map/:
> > + |---mapX: mapping information
> > + A typical map entry is like below:
> > +
> > + trip_type sensor cdev trip_mask weight(s)
> > + passive cpu proc 0x03 50 30
> > + active cpu fan0 0x03 50 70
> > +
> > + The trip mask is a bit string; if 'n' th bit is set, then for
> > + trip point 'n' this cdev is throttled with the given weight[n].
> > --
> > 1.7.9.5
> >
^ permalink raw reply
* Re: [RFC PATCH 6/7] Thermal: Add Documentation to new APIs
From: Hongbo Zhang @ 2012-12-03 7:19 UTC (permalink / raw)
To: Durgadoss R
Cc: rui.zhang, linux-pm, wni, eduardo.valentin, amit.kachhap,
sachin.kamat
In-Reply-To: <1353149158-19102-7-git-send-email-durgadoss.r@intel.com>
On 17 November 2012 18:45, Durgadoss R <durgadoss.r@intel.com> wrote:
> This patch adds Documentation for the new APIs
> introduced in this patch set. The documentation
> also has a model sysfs structure for reference.
>
> Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
> ---
> Documentation/thermal/sysfs-api2.txt | 213 ++++++++++++++++++++++++++++++++++
> 1 file changed, 213 insertions(+)
> create mode 100644 Documentation/thermal/sysfs-api2.txt
>
> diff --git a/Documentation/thermal/sysfs-api2.txt b/Documentation/thermal/sysfs-api2.txt
> new file mode 100644
> index 0000000..be67125
> --- /dev/null
> +++ b/Documentation/thermal/sysfs-api2.txt
> @@ -0,0 +1,213 @@
> +Thermal Framework
> +-----------------
> +
> +Written by Durgadoss R <durgadoss.r@intel.com>
> +Copyright (c) 2012 Intel Corporation
> +
> +Created on: 4 November 2012
> +
> +0. Introduction
> +---------------
> +The Linux thermal framework provides a set of interfaces for thermal
> +sensors and thermal cooling devices (fan, processor...) to register
> +with the thermal management solution and to be a part of it.
> +
> +This document focuses on how to enable new thermal sensors and cooling
> +devices to participate in thermal management. This solution is intended
> +to be 'light-weight' and platform/architecture independent. Any thermal
> +sensor/cooling device should be able to use the infrastructure easily.
> +
> +The goal of thermal framework is to expose the thermal sensor/zone and
> +cooling device attributes in a consistent way. This will help the
> +thermal governors to make use of the information to manage platform
> +thermals efficiently.
> +
> +1. Terminology
> +--------------
> +This section describes the terminology used in the rest of this
> +document as well as the thermal framework code.
> +
> +thermal_sensor: Hardware that can report temperature of a particular
> + spot in the platform, where it is placed. The temperature
> + reported by the sensor is the 'real' temperature reported
> + by the hardware.
> +thermal_zone: A virtual area on the device, that gets heated up. It may
> + have one or more thermal sensors attached to it.
> +cooling_device: Any component that can help in reducing the temperature of
> + a 'hot spot' either by reducing its performance (passive
> + cooling) or by other means(Active cooling E.g. Fan)
> +
> +trip_points: Various temperature levels for each sensor. As of now, we
> + have four levels namely active, passive, hot and critical.
> + Hot and critical trip point support only one value whereas
> + active and passive can have any number of values. These
> + temperature values can come from platform data, and are
> + exposed through sysfs in a consistent manner. Stand-alone
> + thermal sensor drivers are not expected to know these values.
> + These values are RO.
> +thresholds: These are programmable temperature limits, on reaching which
> + the thermal sensor generates an interrupt. The framework is
> + notified about this interrupt to take appropriate action.
> + There can be as many number of thresholds as that of the
> + hardware supports. These values are RW.
> +
> +thermal_map: This provides the mapping (aka binding) information between
> + various sensors and cooling devices in a particular zone.
> + Typically, this also comes from platform data; Stand-alone
> + sensor drivers or cooling device drivers are not expected
> + to know these mapping information.
> +
> +2. Thermal framework APIs
> +-------------------------
> +2.1: For Thermal Sensors
> +2.1.1 thermal_sensor_register:
> + This function creates a new sensor directory under /sys/class/thermal/
> + as sensor[0-*]. This API is expected to be called by thermal sensor
> + drivers. These drivers may or may not be in thermal subsystem. This
> + function returns a thermal_sensor structure on success and appropriate
> + error on failure.
> +
> + name: Name of the sensor
> + devdata: Device private data
> + ops: Thermal sensor callbacks
> + .get_temp: obtain the current temperature of the sensor
> + .get_trend: obtain the trend of the sensor
> + .get_threshold: get a particular threshold temperature
> + .set_threshold: set a particular threshold temperature
> +
> +2.1.2 thermal_sensor_unregister:
> + This function deletes the sensor directory under /sys/class/thermal/
> + for the given sensor. Thermal sensor drivers may call this API
> + during the driver's 'exit' routine.
> +
> + ts: Thermal sensor that has to be unregistered
> +
> +2.1.3 enable_sensor_thresholds:
> + This function creates 'threshold[0-*]' attributes under a particular
> + sensorX directory. These values are RW. This function is called by
> + the sensr driver only if the sensor supports interrupt mechanism.
> +
> + ts: Thermal sensor for which thresholds have to be enabled
> + num_thresholds: Number of thresholds supported by the sensor
> +
> +2.2: For Cooling Devices
> +2.2.1 thermal_cooling_device_register:
> + This function adds a new thermal cooling device (fan/processor/...)
> + to /sys/class/thermal/ folder as cooling_device[0-*]. This function
> + is expected to be called by cooling device drivers that may be
> + present in other subsystems also.
> +
> + name: the cooling device name
> + devdata: device private data
> + ops: thermal cooling devices callbacks
> + .get_max_state: get the Maximum throttle state of the cooling device
> + .get_cur_state: get the Current throttle state of the cooling device
> + .set_cur_state: set the Current throttle state of the cooling device
> +
> +2.2.2 thermal_cooling_device_unregister:
> + This function deletes the given cdev entry form /sys/class/thermal;
> + and also cleans all the symlinks referred from various zones.
> +
> + cdev: Cooling device to be unregistered
> +
> +2.3: For Thermal Zones
> +2.3.1 create_thermal_zone:
> + This function adds a new 'zone' under /sys/class/thermal/
> + directory as zone[0-*]. This zone has at least one thermal
> + sensor and at most MAX_SENSORS_PER_ZONE number of sensors
> + attached to it. Similarly, this zone has at least one cdev
> + and at most MAX_CDEVS_PER_ZONE number of cdevs attached to it.
> + Both the MAX_*_PER_ZONE values are configurable, through
> + Kconfig option(during 'menuconfig').
> +
> + name: Name of the thermal zone
> + devdata: Device private data
> +
> +2.3.2 add_sensor_to_zone
> + This function adds a 'sensorX' entry under /sys/class/thermal/
> + zoneY/ directory. This 'sensorX' is a symlink to the actual
> + sensor entry under /sys/class/thermal/. Correspondingly, the
> + method remove_sensor_from_zone deletes the symlink.
> +
> + tz: thermal zone structure
> + ts: thermal sensor structure
> +
> +2.3.3 add_cdev_to_zone
> + This function adds a 'cdevX' entry under /sys/class/thermal/
> + zoneY/ directory. This 'cdevX' is a symlink to the actual
> + cdev entry under /sys/class/thermal/. Correspondingly, the
> + method remove_cdev_from_zone deletes the symlink.
> +
> + tz: thermal zone structure
> + cdev: thermal cooling device structure
> +
> +2.4 For Thermal Trip
> +2.4.1 add_sensor_trip_info
> + This function adds trip point information for the given sensor,
> + (under a given zone) under /sys/class/thermal/zoneX/thermal_trip/
> + This API creates 4 sysfs attributes namely active, passive, hot,
> + and critical. Each of these hold one or more trip point temperature
> + values, as provided from platform data.
> +
> + tz: thermal zone structure
> + ts: thermal sensor to which the trip points are attached
> + trip: trip point structure. Usually obtained from platform data
> +
> +2.5 For Thermal Map
> +2.5.1 add_map_entry
> + This function adds a 'map[0-*]' sysfs attribute under
> + /sys/class/thermal/zoneX/thermal_map/. Each map holds a space
> + separated list of values, that specify the binding relationship
> + between a sensor and a cdev in the given zone. The map structure
> + is typically obtained as platform data. For example, through
> + ACPI tables, SFI tables, Device tree etc.
> +
> + tz: thermal zone to which a 'map' is being added
> + map: thermal_map structure
> +
> +3. Sysfs attributes structure
> +-----------------------------
> +Thermal sysfs attributes will be represented under /sys/class/thermal.
> +
> +3.1: For Thermal Sensors
> + /sys/class/thermal/thermal_zone[0-*]:
Should be /sys/class/thermal/sensor[0-*] ?
> + |---type: Name of the thermal sensor
> + |---temp_input: Current temperature in mC
> + |---threshold[0-*]: Threshold temperature
> +
> +3.2: For Thermal Cooling Devices
> + /sys/class/thermal/cooling_device[0-*]:
> + |---type: Type of the cooling device
> + |---max_state: Maximum throttle state of the cdev
> + |---cur_state: Current throttle state of the cdev
> +
> +3.3: For Thermal Zones
> + /sys/class/thermal/zone[0-*]:
> + |---name: Name of the thermal
> + |---sensorX: Symlink to ../sensorX
> + |---cdevY: Symlink to ../cdevY
> + |---thermal_trip: trip point values for sensors
> + |---thermal_map: mapping info between sensors and cdevs
> +
> +3.4: For Thermal Trip
> + This attribute represents the trip point values for all sensors
> + present in the thermal zone. All values are in mC.
> + /sys/class/thermal/zoneX/thermal_trip/sensorY:
> + |---hot: hot trip point value
> + |---critical: critical trip point value
> + |---passive: list of passive trip point values
> + |---active: list of active trip point values
> +
> +3.5: For Thermal Map
> + Each attribute represents the mapping/binding information between
> + a sensor and a cdev, together with a trip type.
> + /sys/class/thermal/zoneX/thermal_map/:
> + |---mapX: mapping information
> + A typical map entry is like below:
> +
> + trip_type sensor cdev trip_mask weight(s)
> + passive cpu proc 0x03 50 30
> + active cpu fan0 0x03 50 70
> +
> + The trip mask is a bit string; if 'n' th bit is set, then for
> + trip point 'n' this cdev is throttled with the given weight[n].
> --
> 1.7.9.5
>
^ permalink raw reply
* Re: [RFC PATCH 4/5] arm: omap2: support port power on lan95xx devices
From: Andy Green @ 2012-12-03 4:52 UTC (permalink / raw)
To: Ming Lei
Cc: Alan Stern, Greg Kroah-Hartman, Lan Tianyu, Sarah Sharp,
Rafael J. Wysocki, linux-pm, Oliver Neukum, linux-omap, linux-usb,
Roger Quadros, Felipe Balbi
In-Reply-To: <CACVXFVNvNJJHccN=jL+N9UorwRjeC0JBMXgsB1Th_jNBeqihAw@mail.gmail.com>
On 03/12/12 12:11, the mail apparently from Ming Lei included:
> On Mon, Dec 3, 2012 at 12:37 AM, Andy Green <andy.green@linaro.org> wrote:
>> On 02/12/12 23:01, the mail apparently from Ming Lei included:
>>
>> Hi -
>>
>>
>>> This patch defines power controller for powering on/off LAN95xx
>>> USB hub and USB ethernet devices, and implements one match function
>>> to associate the power controller with related USB port device.
>>> The big problem of this approach is that it depends on the global
>>> device ADD/DEL notifier.
>>>
>>> Another idea of associating power controller with port device
>>> is by introducing usb port driver, and move all this port power
>>> control stuff from platform code to the port driver, which is just
>>> what I think of and looks doable. The problem of the idea is that
>>> port driver is per board, so maybe cause lots of platform sort of
>>> code to be put under drivers/usb/port/, but this approach can avoid
>>> global device ADD/DEL notifier.
>>>
>>> I'd like to get some feedback about which one is better or other choice,
>>> then I may do it in next cycle.
>>>
>>> Cc: Andy Green <andy.green@linaro.org>
>>> Cc: Roger Quadros <rogerq@ti.com>
>>> Cc: Alan Stern <stern@rowland.harvard.edu>
>>> Cc: Felipe Balbi <balbi@ti.com>
>>> Signed-off-by: Ming Lei <tom.leiming@gmail.com>
>>> ---
>>> arch/arm/mach-omap2/board-omap4panda.c | 99
>>> +++++++++++++++++++++++++++++++-
>>> 1 file changed, 96 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-omap2/board-omap4panda.c
>>> b/arch/arm/mach-omap2/board-omap4panda.c
>>> index 5c8e9ce..3183832 100644
>>> --- a/arch/arm/mach-omap2/board-omap4panda.c
>>> +++ b/arch/arm/mach-omap2/board-omap4panda.c
>>> @@ -32,6 +32,8 @@
>>> #include <linux/usb/musb.h>
>>> #include <linux/wl12xx.h>
>>> #include <linux/platform_data/omap-abe-twl6040.h>
>>> +#include <linux/power_controller.h>
>>> +#include <linux/usb/port.h>
>>>
>>> #include <asm/hardware/gic.h>
>>> #include <asm/mach-types.h>
>>> @@ -154,6 +156,99 @@ static struct gpio panda_ehci_gpios[] __initdata = {
>>> { GPIO_HUB_NRESET, GPIOF_OUT_INIT_LOW, "hub_nreset" },
>>> };
>>>
>>> +static void ehci_hub_power_on(struct power_controller *pc, struct device
>>> *dev)
>>> +{
>>> + gpio_set_value(GPIO_HUB_NRESET, 1);
>>> + gpio_set_value(GPIO_HUB_POWER, 1);
>>> +}
>>
>>
>> You should wait a bit after applying power to the smsc chip before letting
>> go of nReset too. In the regulator-based implementation I sent it's handled
>> by delays encoded in the regulator structs.
>
> It isn't a big thing about the discussion. If these code is only platform code,
> we can use gpio or regulator or other thing.
Well, you need a delay there FYI. It's just a nit.
>>> +static void ehci_hub_power_off(struct power_controller *pc, struct device
>>> *dev)
>>> +{
>>> + gpio_set_value(GPIO_HUB_NRESET, 0);
>>> + gpio_set_value(GPIO_HUB_POWER, 0);
>>> +}
>>> +
>>> +static struct usb_port_power_switch_data root_hub_port_data = {
>>> + .hub_tier = 0,
>>> + .port_number = 1,
>>> + .type = USB_PORT_CONNECT_TYPE_HARD_WIRED,
>>> +};
>>> +
>>> +static struct usb_port_power_switch_data smsc_hub_port_data = {
>>> + .hub_tier = 1,
>>> + .port_number = 1,
>>> + .type = USB_PORT_CONNECT_TYPE_HARD_WIRED,
>>> +};
>>> +
>>> +static struct power_controller pc = {
>>> + .name = "omap_hub_eth_pc",
>>> + .count = ATOMIC_INIT(0),
>>> + .power_on = ehci_hub_power_on,
>>> + .power_off = ehci_hub_power_off,
>>> +};
>>> +
>>> +static inline int omap_ehci_hub_port(struct device *dev)
>>> +{
>>> + /* we expect dev->parent points to ehcd controller */
>>> + if (dev->parent && !strcmp(dev_name(dev->parent), "ehci-omap.0"))
>>> + return 1;
>>> + return 0;
>>> +}
>>> +
>>> +static inline int dev_pc_match(struct device *dev)
>>> +{
>>> + struct device *anc;
>>> + int ret = 0;
>>> +
>>> + if (likely(strcmp(dev_name(dev), "port1")))
>>> + goto exit;
>>> +
>>> + if (dev->parent && (anc = dev->parent->parent)) {
>>> + if (omap_ehci_hub_port(anc)) {
>>> + ret = 1;
>>> + goto exit;
>>> + }
>>> +
>>> + /* is it port of lan95xx hub? */
>>> + if ((anc = anc->parent) && omap_ehci_hub_port(anc)) {
>>> + ret = 2;
>>> + goto exit;
>>> + }
>>> + }
>>> +exit:
>>> + return ret;
>>> +}
>>> +
>>> +/*
>>> + * Notifications of device registration
>>> + */
>>> +static int device_notify(struct notifier_block *nb, unsigned long action,
>>> void *data)
>>> +{
>>> + struct device *dev = data;
>>> + int ret;
>>> +
>>> + switch (action) {
>>> + case DEV_NOTIFY_ADD_DEVICE:
>>> + ret = dev_pc_match(dev);
>>> + if (likely(!ret))
>>> + goto exit;
>>> + if (ret == 1)
>>> + dev_pc_bind(&pc, dev, &root_hub_port_data,
>>> sizeof(root_hub_port_data));
>>> + else
>>> + dev_pc_bind(&pc, dev, &smsc_hub_port_data,
>>> sizeof(smsc_hub_port_data));
>>> + break;
>>> +
>>> + case DEV_NOTIFY_DEL_DEVICE:
>>> + break;
>>> + }
>>> +exit:
>>> + return 0;
>>> +}
>>> +
>>> +static struct notifier_block usb_port_nb = {
>>> + .notifier_call = device_notify,
>>> +};
>>> +
>>
>>
>> Some thoughts on trying to make this functionality specific to power only
>> and ehci hub port only:
>>
>> - Quite a few boards have smsc95xx... they're all going to carry these
>> additions in the board file? Surely you'll have to generalize the pieces
>
> All things are board dependent because we are discussing peripheral
> device(not builtin SoC devices), so it is proper to put it in the board file.
> If some boards want to share it, we can put it in a single .c file and
> let board file include it.
Where would the .c file go... SMSC is not platform, or even arch
specific chip (eg, iMX or MIPS or even x86 boards can have it), so not
arch/arm/mach-xxxx or arch/arm/plat-xxx or arch/arm. I guess it would
go in drivers/usb or drivers/net.
Push in ARM-Land is towards single kernels which support many platforms,
a good case in point is omap2plus_defconfg which wants to allow to
support all OMAP2/3/4/5 platforms in one kernel. If you "include" that
C file over and over in board files, it's not very nice for that. They
anyway want to eliminate board files for the single kernel binary
reason, and just have one load of config come in by dtb.
So it guides you towards having static helper code once in drivers/ for
the scenarios you support... if that's where you end up smsc is less
good a target for a helper than to have helpers for classes of object
like regulator and clk, that you can combine and reuse on all sorts of
target devices, which is device_asset approach.
It also guides you to having the special platform sauce be something
that can go into the dtb: per-board code can't. That's why device_asset
stuff only places asset structs in the board file and is removing code
from there.
>> that perform device_path business out of the omap4panda board file at least.
>> At that point the path matching code becomes generic end-of-the-device-path
>> matching code.
>
> Looks Alan has mentioned, there might be no generic way, and any device's
> name change in the path may make the way fragile.
What you have provided is no less fragile if you allow "port1" and the
ehci-omap.0 to be set from the outside.
Unless someone NAKs it for sure already (if you're already sure you're
going to, please do so to avoid wasting time), I'll issue a try#2 of my
code later which demonstrates what I mean. At least I guess it's useful
for comparative purposes.
>> - How could these literals like "port1" etc be nicely provided by Device
>> Tree? In ARM-land there's pressure to eventually eliminate board files
>> completely and pass in everything from dtb. device_asset can neatly grow DT
>> bindings in a generic way, since the footprint in the board file is some
>
> IMO, it isn't necessary to expose these assets to device or users, from the
> view of device or user, which only cares two actions(poweron and poweroff)
> about the discussed problem. Also it should be better to put these assets
> into device resource list, instead of introducing them in 'struct device'.
From the point of view of allowing it to be reused on different boards
/ platforms / arches, you are going to have to do something better than
have literals in the code.
>> regulators that already have dt bindings and some device_asset structs.
>> Similarly there's pressure for magic code to service a board (rather than
>> SoC) to go elsewhere than the board file.
>
> Looks you associate these assets with ehci-omap device, which mightn't be
> enough, because we need to control port's power for supporting port
> power off mechanism. Do you have generic way to associate these assets
> with usb port device and let port use it generally?
Yes, you need a parent device pointer (ehci host controller in this
case) and the path rhs, but only stuff that is defined by usb stack
code. Needing a parent pointer is OK because this stuff only has
meaning for hardwired assets on the platform, so the parent device will
always be known as a platform_device at boot time anyway.
The code I'll provide will work the same in sdio or other bus case, just
use mmc host controller pointer and the sdio device name the same way.
>> - Shouldn't this take care of enabling and disabling the ULPI PHY clock on
>> Panda too? There's no purpose leaving it running if the one thing the ULPI
>> PHY is connected to is depowered, and when you do power it, on Panda you
>> will reset the ULPI PHY at the same time anyway (smsc reset and ULPI PHY
>> reset are connected together on Panda). Then you can eliminate
>> omap4_ehci_init() in the board file.
>
> OK, we can include the ULPI PHY clock things easily in ->power_on() and
> ->power_off() of 'power controller'
Yes if the ARM people will accept establishing more code in board files
that doesn't have a DT story.
-Andy
--
Andy Green | TI Landing Team Leader
Linaro.org │ Open source software for ARM SoCs | Follow Linaro
http://facebook.com/pages/Linaro/155974581091106 -
http://twitter.com/#!/linaroorg - http://linaro.org/linaro-blog
^ permalink raw reply
* Re: [RFC PATCH 4/5] arm: omap2: support port power on lan95xx devices
From: Ming Lei @ 2012-12-03 4:11 UTC (permalink / raw)
To: Andy Green
Cc: Alan Stern, Greg Kroah-Hartman, Lan Tianyu, Sarah Sharp,
Rafael J. Wysocki, linux-pm, Oliver Neukum, linux-omap, linux-usb,
Roger Quadros, Felipe Balbi
In-Reply-To: <50BB83E1.7020408@linaro.org>
On Mon, Dec 3, 2012 at 12:37 AM, Andy Green <andy.green@linaro.org> wrote:
> On 02/12/12 23:01, the mail apparently from Ming Lei included:
>
> Hi -
>
>
>> This patch defines power controller for powering on/off LAN95xx
>> USB hub and USB ethernet devices, and implements one match function
>> to associate the power controller with related USB port device.
>> The big problem of this approach is that it depends on the global
>> device ADD/DEL notifier.
>>
>> Another idea of associating power controller with port device
>> is by introducing usb port driver, and move all this port power
>> control stuff from platform code to the port driver, which is just
>> what I think of and looks doable. The problem of the idea is that
>> port driver is per board, so maybe cause lots of platform sort of
>> code to be put under drivers/usb/port/, but this approach can avoid
>> global device ADD/DEL notifier.
>>
>> I'd like to get some feedback about which one is better or other choice,
>> then I may do it in next cycle.
>>
>> Cc: Andy Green <andy.green@linaro.org>
>> Cc: Roger Quadros <rogerq@ti.com>
>> Cc: Alan Stern <stern@rowland.harvard.edu>
>> Cc: Felipe Balbi <balbi@ti.com>
>> Signed-off-by: Ming Lei <tom.leiming@gmail.com>
>> ---
>> arch/arm/mach-omap2/board-omap4panda.c | 99
>> +++++++++++++++++++++++++++++++-
>> 1 file changed, 96 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/board-omap4panda.c
>> b/arch/arm/mach-omap2/board-omap4panda.c
>> index 5c8e9ce..3183832 100644
>> --- a/arch/arm/mach-omap2/board-omap4panda.c
>> +++ b/arch/arm/mach-omap2/board-omap4panda.c
>> @@ -32,6 +32,8 @@
>> #include <linux/usb/musb.h>
>> #include <linux/wl12xx.h>
>> #include <linux/platform_data/omap-abe-twl6040.h>
>> +#include <linux/power_controller.h>
>> +#include <linux/usb/port.h>
>>
>> #include <asm/hardware/gic.h>
>> #include <asm/mach-types.h>
>> @@ -154,6 +156,99 @@ static struct gpio panda_ehci_gpios[] __initdata = {
>> { GPIO_HUB_NRESET, GPIOF_OUT_INIT_LOW, "hub_nreset" },
>> };
>>
>> +static void ehci_hub_power_on(struct power_controller *pc, struct device
>> *dev)
>> +{
>> + gpio_set_value(GPIO_HUB_NRESET, 1);
>> + gpio_set_value(GPIO_HUB_POWER, 1);
>> +}
>
>
> You should wait a bit after applying power to the smsc chip before letting
> go of nReset too. In the regulator-based implementation I sent it's handled
> by delays encoded in the regulator structs.
It isn't a big thing about the discussion. If these code is only platform code,
we can use gpio or regulator or other thing.
>
>
>> +static void ehci_hub_power_off(struct power_controller *pc, struct device
>> *dev)
>> +{
>> + gpio_set_value(GPIO_HUB_NRESET, 0);
>> + gpio_set_value(GPIO_HUB_POWER, 0);
>> +}
>> +
>> +static struct usb_port_power_switch_data root_hub_port_data = {
>> + .hub_tier = 0,
>> + .port_number = 1,
>> + .type = USB_PORT_CONNECT_TYPE_HARD_WIRED,
>> +};
>> +
>> +static struct usb_port_power_switch_data smsc_hub_port_data = {
>> + .hub_tier = 1,
>> + .port_number = 1,
>> + .type = USB_PORT_CONNECT_TYPE_HARD_WIRED,
>> +};
>> +
>> +static struct power_controller pc = {
>> + .name = "omap_hub_eth_pc",
>> + .count = ATOMIC_INIT(0),
>> + .power_on = ehci_hub_power_on,
>> + .power_off = ehci_hub_power_off,
>> +};
>> +
>> +static inline int omap_ehci_hub_port(struct device *dev)
>> +{
>> + /* we expect dev->parent points to ehcd controller */
>> + if (dev->parent && !strcmp(dev_name(dev->parent), "ehci-omap.0"))
>> + return 1;
>> + return 0;
>> +}
>> +
>> +static inline int dev_pc_match(struct device *dev)
>> +{
>> + struct device *anc;
>> + int ret = 0;
>> +
>> + if (likely(strcmp(dev_name(dev), "port1")))
>> + goto exit;
>> +
>> + if (dev->parent && (anc = dev->parent->parent)) {
>> + if (omap_ehci_hub_port(anc)) {
>> + ret = 1;
>> + goto exit;
>> + }
>> +
>> + /* is it port of lan95xx hub? */
>> + if ((anc = anc->parent) && omap_ehci_hub_port(anc)) {
>> + ret = 2;
>> + goto exit;
>> + }
>> + }
>> +exit:
>> + return ret;
>> +}
>> +
>> +/*
>> + * Notifications of device registration
>> + */
>> +static int device_notify(struct notifier_block *nb, unsigned long action,
>> void *data)
>> +{
>> + struct device *dev = data;
>> + int ret;
>> +
>> + switch (action) {
>> + case DEV_NOTIFY_ADD_DEVICE:
>> + ret = dev_pc_match(dev);
>> + if (likely(!ret))
>> + goto exit;
>> + if (ret == 1)
>> + dev_pc_bind(&pc, dev, &root_hub_port_data,
>> sizeof(root_hub_port_data));
>> + else
>> + dev_pc_bind(&pc, dev, &smsc_hub_port_data,
>> sizeof(smsc_hub_port_data));
>> + break;
>> +
>> + case DEV_NOTIFY_DEL_DEVICE:
>> + break;
>> + }
>> +exit:
>> + return 0;
>> +}
>> +
>> +static struct notifier_block usb_port_nb = {
>> + .notifier_call = device_notify,
>> +};
>> +
>
>
> Some thoughts on trying to make this functionality specific to power only
> and ehci hub port only:
>
> - Quite a few boards have smsc95xx... they're all going to carry these
> additions in the board file? Surely you'll have to generalize the pieces
All things are board dependent because we are discussing peripheral
device(not builtin SoC devices), so it is proper to put it in the board file.
If some boards want to share it, we can put it in a single .c file and
let board file include it.
> that perform device_path business out of the omap4panda board file at least.
> At that point the path matching code becomes generic end-of-the-device-path
> matching code.
Looks Alan has mentioned, there might be no generic way, and any device's
name change in the path may make the way fragile.
>
> - How could these literals like "port1" etc be nicely provided by Device
> Tree? In ARM-land there's pressure to eventually eliminate board files
> completely and pass in everything from dtb. device_asset can neatly grow DT
> bindings in a generic way, since the footprint in the board file is some
IMO, it isn't necessary to expose these assets to device or users, from the
view of device or user, which only cares two actions(poweron and poweroff)
about the discussed problem. Also it should be better to put these assets
into device resource list, instead of introducing them in 'struct device'.
> regulators that already have dt bindings and some device_asset structs.
> Similarly there's pressure for magic code to service a board (rather than
> SoC) to go elsewhere than the board file.
Looks you associate these assets with ehci-omap device, which mightn't be
enough, because we need to control port's power for supporting port
power off mechanism. Do you have generic way to associate these assets
with usb port device and let port use it generally?
>
> - Shouldn't this take care of enabling and disabling the ULPI PHY clock on
> Panda too? There's no purpose leaving it running if the one thing the ULPI
> PHY is connected to is depowered, and when you do power it, on Panda you
> will reset the ULPI PHY at the same time anyway (smsc reset and ULPI PHY
> reset are connected together on Panda). Then you can eliminate
> omap4_ehci_init() in the board file.
OK, we can include the ULPI PHY clock things easily in ->power_on() and
->power_off() of 'power controller'
Thanks,
--
Ming Lei
^ permalink raw reply
* Re: [RFC PATCH 2/5] driver core: introduce global device ADD/DEL notifier
From: Ming Lei @ 2012-12-03 3:13 UTC (permalink / raw)
To: Andy Green
Cc: Alan Stern, Greg Kroah-Hartman, Lan Tianyu, Sarah Sharp,
Rafael J. Wysocki, linux-pm, Oliver Neukum, linux-omap, linux-usb,
Roger Quadros, Felipe Balbi
In-Reply-To: <50BB7E21.1080200@linaro.org>
On Mon, Dec 3, 2012 at 12:13 AM, Andy Green <andy.green@linaro.org> wrote:
> On 02/12/12 23:01, the mail apparently from Ming Lei included:
>
>> The global device ADD/DEL notifier is introduced so that
>> some platform code can bind some device resource to the
>> device. When this platform code runs, there is no any bus
>> information about the device, so have to resort to the
>> global notifier.
>>
>> Cc: Andy Green <andy.green@linaro.org>
>> Cc: Roger Quadros <rogerq@ti.com>
>> Cc: Alan Stern <stern@rowland.harvard.edu>
>> Cc: Felipe Balbi <balbi@ti.com>
>> Signed-off-by: Ming Lei <tom.leiming@gmail.com>
>> ---
>> drivers/base/core.c | 21 +++++++++++++++++++++
>> include/linux/device.h | 13 +++++++++++++
>> 2 files changed, 34 insertions(+)
>>
>> diff --git a/drivers/base/core.c b/drivers/base/core.c
>> index a235085..37f11ff 100644
>> --- a/drivers/base/core.c
>> +++ b/drivers/base/core.c
>> @@ -43,6 +43,9 @@ static __init int sysfs_deprecated_setup(char *arg)
>> early_param("sysfs.deprecated", sysfs_deprecated_setup);
>> #endif
>>
>> +/* global device nofifier */
>> +struct blocking_notifier_head dev_notifier;
>> +
>> int (*platform_notify)(struct device *dev) = NULL;
>> int (*platform_notify_remove)(struct device *dev) = NULL;
>> static struct kobject *dev_kobj;
>> @@ -1041,6 +1044,8 @@ int device_add(struct device *dev)
>> if (platform_notify)
>> platform_notify(dev);
>>
>> + blocking_notifier_call_chain(&dev_notifier, DEV_NOTIFY_ADD_DEVICE,
>> dev);
>> +
>> error = device_create_file(dev, &uevent_attr);
>> if (error)
>> goto attrError;
>> @@ -1228,6 +1233,7 @@ void device_del(struct device *dev)
>> device_pm_remove(dev);
>> driver_deferred_probe_del(dev);
>>
>> + blocking_notifier_call_chain(&dev_notifier, DEV_NOTIFY_DEL_DEVICE,
>> dev);
>> /* Notify the platform of the removal, in case they
>> * need to do anything...
>> */
>> @@ -1376,6 +1382,8 @@ struct device *device_find_child(struct device
>> *parent, void *data,
>>
>> int __init devices_init(void)
>> {
>> + BLOCKING_INIT_NOTIFIER_HEAD(&dev_notifier);
>> +
>> devices_kset = kset_create_and_add("devices", &device_uevent_ops,
>> NULL);
>> if (!devices_kset)
>> return -ENOMEM;
>> @@ -1995,6 +2003,19 @@ int dev_printk(const char *level, const struct
>> device *dev,
>> }
>> EXPORT_SYMBOL(dev_printk);
>>
>> +/* The notifier should be avoided as far as possible */
>> +int dev_register_notifier(struct notifier_block *nb)
>> +{
>> + return blocking_notifier_chain_register(&dev_notifier, nb);
>> +}
>> +EXPORT_SYMBOL_GPL(dev_register_notifier);
>> +
>> +int dev_unregister_notifier(struct notifier_block *nb)
>> +{
>> + return blocking_notifier_chain_unregister(&dev_notifier, nb);
>> +}
>> +EXPORT_SYMBOL_GPL(dev_unregister_notifier);
>> +
>> #define define_dev_printk_level(func, kern_level) \
>> int func(const struct device *dev, const char *fmt, ...) \
>> { \
>> diff --git a/include/linux/device.h b/include/linux/device.h
>> index 43dcda9..aeb54f6 100644
>> --- a/include/linux/device.h
>> +++ b/include/linux/device.h
>> @@ -179,6 +179,19 @@ extern int bus_unregister_notifier(struct bus_type
>> *bus,
>> #define BUS_NOTIFY_UNBOUND_DRIVER 0x00000006 /* driver is unbound
>> from the device */
>>
>> +/* All 2 notifers below get called with the target struct device *
>> + * as an argument. Note that those functions are likely to be called
>> + * with the device lock held in the core, so be careful.
>> + */
>> +#define DEV_NOTIFY_ADD_DEVICE 0x00000001 /* device added */
>> +#define DEV_NOTIFY_DEL_DEVICE 0x00000002 /* device removed */
>> +extern int dev_register_notifier(struct notifier_block *nb);
>> +extern int dev_unregister_notifier(struct notifier_block *nb);
>> +
>> +extern struct kset *bus_get_kset(struct bus_type *bus);
>> +extern struct klist *bus_get_device_klist(struct bus_type *bus);
>> +
>> +
>> extern struct kset *bus_get_kset(struct bus_type *bus);
>> extern struct klist *bus_get_device_klist(struct bus_type *bus);
>
>
> Device de/registraton time is not necessarily a good choice for the
> notification point. At boot time, platform_devices may be being registered
> with no sign of driver and anything getting enabled in the notifier without
> further filtering (at each notifier...) will then always be enabled the
> whole session whether in use or not.
>
> probe() and remove() are more interesting because at that point the driver
> is present and we're definitely instantiating the thing or finished with its
> instantiation, and it's valid for non-usb devices too.
>
> In the one case you're servicing in the series, usb hub port, the device is
> very unusual in that it has no driver. However if you're going to add a
> global notifier in generic device code, you should have it do the right
> thing for normal device case so other people can use it... how I solved this
> for hub port was to simply normalize hub port by introducing a stub driver
> for it, so you get a probe / remove like anything else.
I also mentioned doing such things in usb port driver in 4/5, and I'd
like to get some feedback about which one is better.
The problem is that you have to move all platform code into drivers/usb
because usb port is not a platform device, and there is no easy way to
obtain some platform dependent info.(You can ague we can get it from
hcd driver, but someone may object it).
Another problem is that smsc95xx chip are used in more than one board,
and with different power on/off approach, such as beagle vs. panda, so
the single omap port driver has to consider board difference things, I am
wondering if it is good to do in port driver and if USB guys can agree on it.
Sp I post these patches just for making the discussion moving on.
Thanks,
--
Ming Lei
^ permalink raw reply
* Re: [RFC PATCH 1/5] Device Power: introduce power controller
From: Ming Lei @ 2012-12-03 3:00 UTC (permalink / raw)
To: Andy Green
Cc: Alan Stern, Greg Kroah-Hartman, Lan Tianyu, Sarah Sharp,
Rafael J. Wysocki, linux-pm, Oliver Neukum, linux-omap, linux-usb,
Roger Quadros, Felipe Balbi
In-Reply-To: <50BB7BA9.4090902@linaro.org>
On Mon, Dec 3, 2012 at 12:02 AM, Andy Green <andy.green@linaro.org> wrote:
> On 02/12/12 23:01, the mail apparently from Ming Lei included:
>
>> Power controller is an abstract on simple power on/off switch.
>>
>> One power controller can bind to more than one device, which
>> provides power logically, for example, we can think one usb port
>> in hub provides power to the usb device attached to the port, even
>> though the power is supplied actually by other ways, eg. the usb
>> hub is a self-power device. From hardware view, more than one
>> device can share one power domain, and power controller can power
>> on if one of these devices need to provide power, and power off if
>> all these devices don't need to provide power.
>
>
> What stops us using struct regulator here? If you have child regulators
> supplied by a parent supply, isn't that the right semantic already without
> introducing a whole new thing? Apologies if I missed the point.
There are two purposes:
One is to hide the implementation details of the power controller because
the user doesn't care how it is implemented, maybe clock, regulator, gpio
and other platform dependent stuffs involved, so the patch simplify the usage
from the view of users.
Another is that several users may share one power controller, and the
introduced power controller can help users to use it.
Also the power controller is stored as device resource, not any new
stuff added into 'struct device', and all users of the power controller
needn't write code to operate device resource things too.
Thanks,
--
Ming Lei
^ permalink raw reply
* [PATCH v2] thermal: rcar: add .get_trip_type/temp and .notify support
From: Kuninori Morimoto, Kuninori Morimoto @ 2012-12-03 2:48 UTC (permalink / raw)
To: Zhang Rui; +Cc: Simon, Magnus, linux-pm, Kuninori Morimoto
In-Reply-To: <87hao3rh9t.wl%kuninori.morimoto.gx@renesas.com>
This patch adds .get_trip_type(), .get_trip_temp(), and .notify()
on rcar_thermal_zone_ops.
Driver will try platform power OFF if it reached to
critical temperature.
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
v1 -> v2
- fixup (90 - 1) -> (90)
- fixup comment /* +90 <= temp <= +135 */ -> /* +90 <= temp */
drivers/thermal/rcar_thermal.c | 68 ++++++++++++++++++++++++++++++++++++++--
1 file changed, 65 insertions(+), 3 deletions(-)
diff --git a/drivers/thermal/rcar_thermal.c b/drivers/thermal/rcar_thermal.c
index 90db951..89979ff 100644
--- a/drivers/thermal/rcar_thermal.c
+++ b/drivers/thermal/rcar_thermal.c
@@ -22,10 +22,13 @@
#include <linux/io.h>
#include <linux/module.h>
#include <linux/platform_device.h>
+#include <linux/reboot.h>
#include <linux/slab.h>
#include <linux/spinlock.h>
#include <linux/thermal.h>
+#define IDLE_INTERVAL 5000
+
#define THSCR 0x2c
#define THSSR 0x30
@@ -176,8 +179,66 @@ static int rcar_thermal_get_temp(struct thermal_zone_device *zone,
return 0;
}
+static int rcar_thermal_get_trip_type(struct thermal_zone_device *zone,
+ int trip, enum thermal_trip_type *type)
+{
+ struct rcar_thermal_priv *priv = rcar_zone_to_priv(zone);
+
+ /* see rcar_thermal_get_temp() */
+ switch (trip) {
+ case 0: /* +90 <= temp */
+ *type = THERMAL_TRIP_CRITICAL;
+ break;
+ default:
+ dev_err(priv->dev, "rcar driver trip error\n");
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int rcar_thermal_get_trip_temp(struct thermal_zone_device *zone,
+ int trip, unsigned long *temp)
+{
+ struct rcar_thermal_priv *priv = rcar_zone_to_priv(zone);
+
+ /* see rcar_thermal_get_temp() */
+ switch (trip) {
+ case 0: /* +90 <= temp */
+ *temp = MCELSIUS(90);
+ break;
+ default:
+ dev_err(priv->dev, "rcar driver trip error\n");
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int rcar_thermal_notify(struct thermal_zone_device *zone,
+ int trip, enum thermal_trip_type type)
+{
+ struct rcar_thermal_priv *priv = rcar_zone_to_priv(zone);
+
+ switch (type) {
+ case THERMAL_TRIP_CRITICAL:
+ /* FIXME */
+ dev_warn(priv->dev,
+ "Thermal reached to critical temperature\n");
+ machine_power_off();
+ break;
+ default:
+ break;
+ }
+
+ return 0;
+}
+
static struct thermal_zone_device_ops rcar_thermal_zone_ops = {
- .get_temp = rcar_thermal_get_temp,
+ .get_temp = rcar_thermal_get_temp,
+ .get_trip_type = rcar_thermal_get_trip_type,
+ .get_trip_temp = rcar_thermal_get_trip_temp,
+ .notify = rcar_thermal_notify,
};
/*
@@ -211,8 +272,9 @@ static int rcar_thermal_probe(struct platform_device *pdev)
return -ENOMEM;
}
- zone = thermal_zone_device_register("rcar_thermal", 0, 0, priv,
- &rcar_thermal_zone_ops, NULL, 0, 0);
+ zone = thermal_zone_device_register("rcar_thermal", 1, 0, priv,
+ &rcar_thermal_zone_ops, NULL, 0,
+ IDLE_INTERVAL);
if (IS_ERR(zone)) {
dev_err(&pdev->dev, "thermal zone device is NULL\n");
return PTR_ERR(zone);
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH 3/3] thermal: rcar: add .get_trip_type/temp and .notify support
From: Kuninori Morimoto @ 2012-12-03 2:39 UTC (permalink / raw)
To: Zhang Rui; +Cc: Simon, Magnus, linux-pm, Kuninori Morimoto
In-Reply-To: <1354501553.1936.38.camel@rzhang1-mobl4>
Hi Zhang
> > > > +static int rcar_thermal_get_trip_temp(struct thermal_zone_device *zone,
> > > > + int trip, unsigned long *temp)
> > > > +{
> > > > + struct rcar_thermal_priv *priv = rcar_zone_to_priv(zone);
> > > > +
> > > > + /* see rcar_thermal_get_temp() */
> > > > + switch (trip) {
> > > > + case 0: /* +90 <= temp < +135 */
> BTW, the thing I do not understand is,
> what does "135" in the comment mean?
135 is maximum temperature on this chip,
but I understand your point.
/* +90 <= temp */ is enough.
I will fix this comment in v2 too.
Best regards
---
Kuninori Morimoto
^ permalink raw reply
* Re: [PATCH 3/3] thermal: rcar: add .get_trip_type/temp and .notify support
From: Zhang Rui @ 2012-12-03 2:25 UTC (permalink / raw)
To: Kuninori Morimoto; +Cc: Simon, Magnus, linux-pm, Kuninori Morimoto
In-Reply-To: <87ip8jri4f.wl%kuninori.morimoto.gx@renesas.com>
On Sun, 2012-12-02 at 18:20 -0800, Kuninori Morimoto wrote:
> Hi Zhang
>
> > > +static int rcar_thermal_get_trip_temp(struct thermal_zone_device *zone,
> > > + int trip, unsigned long *temp)
> > > +{
> > > + struct rcar_thermal_priv *priv = rcar_zone_to_priv(zone);
> > > +
> > > + /* see rcar_thermal_get_temp() */
> > > + switch (trip) {
> > > + case 0: /* +90 <= temp < +135 */
BTW, the thing I do not understand is,
what does "135" in the comment mean?
thanks,
rui
> > > + *temp = MCELSIUS(90 - 1);
> >
> > what does the comment above mean?
> > the system is supposed to run from 90C to 135C? but you're setting the
> > critical trip point to 89C.
>
> Oops, sorry my original patch used old kernel for test.
> Then it needed this -1.
> But now it was already solved by this patch.
>
> 29321357ac6db54eeb8574da1f6c3e0ce8cfbb60
> (thermal: fix off-by-1 error in trip point trigger condition)
>
> I can fix this, but which is best for you ?
> v2 patch ? or additional patch ?
>
> Best regards
> ---
> Kuninori Morimoto
^ permalink raw reply
* Re: [PATCH 3/3] thermal: rcar: add .get_trip_type/temp and .notify support
From: Zhang Rui @ 2012-12-03 2:22 UTC (permalink / raw)
To: Kuninori Morimoto; +Cc: Simon, Magnus, linux-pm, Kuninori Morimoto
In-Reply-To: <87ip8jri4f.wl%kuninori.morimoto.gx@renesas.com>
On Sun, 2012-12-02 at 18:20 -0800, Kuninori Morimoto wrote:
> Hi Zhang
>
> > > +static int rcar_thermal_get_trip_temp(struct thermal_zone_device *zone,
> > > + int trip, unsigned long *temp)
> > > +{
> > > + struct rcar_thermal_priv *priv = rcar_zone_to_priv(zone);
> > > +
> > > + /* see rcar_thermal_get_temp() */
> > > + switch (trip) {
> > > + case 0: /* +90 <= temp < +135 */
> > > + *temp = MCELSIUS(90 - 1);
> >
> > what does the comment above mean?
> > the system is supposed to run from 90C to 135C? but you're setting the
> > critical trip point to 89C.
>
> Oops, sorry my original patch used old kernel for test.
> Then it needed this -1.
> But now it was already solved by this patch.
>
> 29321357ac6db54eeb8574da1f6c3e0ce8cfbb60
> (thermal: fix off-by-1 error in trip point trigger condition)
>
> I can fix this, but which is best for you ?
> v2 patch ? or additional patch ?
>
please send V2.
thanks,
rui
^ permalink raw reply
* Re: [PATCH 3/3] thermal: rcar: add .get_trip_type/temp and .notify support
From: Kuninori Morimoto @ 2012-12-03 2:20 UTC (permalink / raw)
To: Zhang Rui; +Cc: Simon, Magnus, linux-pm, Kuninori Morimoto
In-Reply-To: <1354496556.1936.7.camel@rzhang1-mobl4>
Hi Zhang
> > +static int rcar_thermal_get_trip_temp(struct thermal_zone_device *zone,
> > + int trip, unsigned long *temp)
> > +{
> > + struct rcar_thermal_priv *priv = rcar_zone_to_priv(zone);
> > +
> > + /* see rcar_thermal_get_temp() */
> > + switch (trip) {
> > + case 0: /* +90 <= temp < +135 */
> > + *temp = MCELSIUS(90 - 1);
>
> what does the comment above mean?
> the system is supposed to run from 90C to 135C? but you're setting the
> critical trip point to 89C.
Oops, sorry my original patch used old kernel for test.
Then it needed this -1.
But now it was already solved by this patch.
29321357ac6db54eeb8574da1f6c3e0ce8cfbb60
(thermal: fix off-by-1 error in trip point trigger condition)
I can fix this, but which is best for you ?
v2 patch ? or additional patch ?
Best regards
---
Kuninori Morimoto
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox