Re: [PATCH v1 4/7] thermal: introduce the Power Allocator governor

From: Eduardo Valentin
Date: Tue Feb 03 2015 - 14:20:06 EST


On Tue, Feb 03, 2015 at 10:32:11AM -0700, Lina Iyer wrote:

<big cut>

> >
> >Well, I am not convinced drivers really need to be aware of these trip
> >types. Which kind of drivers are we talking? Thermal zone drivers?
> >cooling device drivers?
>
> I am sorry, I am missing the point here. The Tz driver is requested to
> return the type of an enum and if the enum is not shared, then how is
> the driver expected to know what to return.

Yeah, however, the only requirement I see for this governor is to have
two passive trip points.

> >
> >Lina, do you have an existing driver (it can be yet to be posted) that
> >would required using these types? To my understanding, these are simply
> >for the governor internal control, drivers do not really need to understand
> >the difference from one to another.
>
> I am currently prototyping Javi's patches on QCOM existing solution. So its all WIP.

I see.

> >
> >The purpose of the .bind_to_tz callback is exactly to verify if the
> >driver has added the required info into the thermal zone. Including the
> >trip setup.
>
> I may be completely off-base here and my understanding of trips is
> questionable. But it seems like even with this callback, the driver
> expected to know what the particular governor defines and needs from the
> TZ. The definitions may overlap and may mean different things in
> different governros. To me a TZ driver should not care what the
> governors are, but define some common parameters that any governor can
> work with. Am I making sense here?


And that's is what I want to keep across the framework. Thermal zone
driver should not really be aware of governor details. Ideally, the
driver should provide a thermal zone that can be usable by different
governors. That is the ideal setup.

If we allow this new governor to have its own trip types, we will be
segregating thermal zones. And that should be avoid if possible.


>
> >
> >Besides, the existing exposed trip types are sufficient, given that this
> >series is in a working state. Unless we have a valid use case to change
> >/ add trip types and expose them to driver, I would prefer to keep this
> >simple.
> >
> >Side note, drivers/thermal/thermal_core.h has symbols that are not
> >exported. As drivers can be built as separated modules from thermal
> >core, I would not recommend include things in that header. The symbols
> >that are EXPORT_SYMBOL'ed are in thermal.h under include directory.
> >
> >>
> >> Cheers,
> >> Javi
> >>
> >> > I dont think anymore, this should be a enum thermal_trip_type, but it has to be
> >> > generic across governors.
> >> >
> >> >
> >> > Thanks,
> >> > Lina
>
>

Attachment: signature.asc
Description: Digital signature