Hello Javi,
On Fri, May 23, 2014 at 10:35:52AM +0100, Javi Merino wrote:
> On Thu, May 22, 2014 at 04:27:30PM +0100, Eduardo Valentin wrote:
> > Hello Javi,
> >
> > On Fri, May 16, 2014 at 12:16:08PM +0100, Javi Merino wrote:
> >
> > > Document struct struct thermal_zone_device and struct thermal_governor
> > > fields and their use by the thermal framework code.
> > >
> > > Cc: Zhang Rui <rui.zh...@intel.com>
> > > Cc: Eduardo Valentin <eduardo.valen...@ti.com>
> > > Signed-off-by: Javi Merino <javi.mer...@arm.com>
> > >
> > > ---
> > >
> > > Hi linux-pm,
> > >
> > > I have some patches that add new fields to these structures but I
> > > don't have a good place to describe those fields as these structs are
> > > mostly undocumented so I thought I'd document them.
> > >
> > > I'm unsure about some of the descriptions, specially for passive and
> > > forced_passive so please review them.
> > >
> > > include/linux/thermal.h | 44
> > > ++++++++++++++++++++++++++++++++++++++++++--
> > > 1 file changed, 42 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> > > index f7e11c7ea7d9..af928c667dba 100644
> > > --- a/include/linux/thermal.h
> > > +++ b/include/linux/thermal.h
> > > @@ -158,6 +158,40 @@ struct thermal_attr {
> > > char name[THERMAL_NAME_LENGTH];
> > > };
> > >
> > > +/**
> > > + * struct thermal_zone_device - structure for a thermal zone
> > > + * @id: unique id number for each thermal zone
> > > + * @type: the thermal zone device type
> > > + * @device: struct device for this thermal zone
> > > + * @trip_temp_attrs: attributes for trip points for sysfs: trip
> > > temperature
> > > + * @trip_type_attrs: attributes for trip points for sysfs: trip type
> > > + * @trip_hyst_attrs: attributes for trip points for sysfs: trip
> > > hysteresis
> > > + * @devdata: private pointer for device private data
> > > + * @trips: number of trip points the thermal zone supports
> > > + * @passive_delay: number of milliseconds to wait between polls
> > > when
> > > + * performing passive cooling. Only used by the
> > > step-wise
> >
> > I don't think this parameter is specific to step-wise.
>
> It's only used by step-wise, all the other governors always poll at
> polling_delay.
Agreed, but the fact we have only one governor using it, it does not
imply it is a specific concept for that governor. The concept can and
should be reused by other governors. Idea is simple, you have different
temporal / thermal evolution when idle, when monitoring passive cooling
actions. And this fact won't change, no matter the policy you apply.
>
> While we are at it, can we change the name so that it's more generic and
> can be reused by other governors? Something like idle_poll and
> active_poll? I'm open to suggestions for a better name.
>
active_poll means when cooling devices are active? which kind of cooling
devices?
> > > + * governor
> > > + * @polling_delay: number of milliseconds to wait between polls
> > > when
> > > + * checking whether trip points have been crossed
> > > (0 for
> > > + * interrupt driven systems)
> > > + * @temperature: current temperature. This is only for core code,
> > > + * drivers should use thermal_zone_get_temp() to
> > > get the
> > > + * current temperature
> > > + * @last_temperature: previous temperature read
> > > + * @emul_temperature: emulated temperature when using
> > > CONFIG_THERMAL_EMULATION
> > > + * @passive: step-wise specific parameter. 1 if you've crossed a
> > > passive
> > > + * trip point, 0 otherwise
> >
> > ditto.
>
> ditto, no other governor changes passive and it can't be changed from
> sysfs.
>
> > > + * @forced_passive: step-wise specific parameter. If > 0,
> > > temperature at
> > > + * which to switch on all cpufreq cooling devices.
> >
> > ditto.
>
> Again, this is step-wise specific. You can change it from sysfs, but
> the other governors (userspace and fair-share) will happily ignore
> you.
>
> > Also, governors are not aware of specific cooling devices.
>
> I didn't say that governors are aware of specific cooling devices. I
> said that if forced_passive is greater than 0, then only cpufreq
> cooling devices are switched on. It's wrong, it's not cpufreq cooling
> devices, it's ACPI Processor cooling devices.
>
I mean, we don't have only cpufreq cooling devices.
> Cheers,
> Javi
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
