Messages in this thread | | | Date | Mon, 28 Apr 2014 06:58:10 +0200 | From | Andreas Mohr <> | Subject | Re: [PATCH 0/4] acerhdf/thermal: adding new models and appropriate governor |
| |
Hi,
On Mon, Apr 28, 2014 at 01:13:19AM +0200, Peter Feuerer wrote: > Hi, > > Andreas Mohr writes: > > > { "Acer", "AOA1....", &aao_reg_map_AOAxxx_Acer_orig_version }, > > > > Of course you then have the indirection of device name <-> specific > > register values (quote: "really ugly and hard to read"?), > > but IMHO that's ok since normally you wouldn't be too focused > > on looking up register values (...right!?). > > > > And if the next interface-breaking config change came along, > > you'd otherwise have to add yet another register index pair... > > (at which point some 100+ char line monsters > > would be breathing down our neck...) > > I think we have been discussing this solution a year ago or something and > seems like it is really time to implement it. As I wrote in the other mail > to Boris, I'd like to just do a minor modification for now and then when > those 4 patches have been applied concentrate on implementing the splitted > structs.
Yup, to get things out of the way now :)
[[BTW your mail environment seems configured to have trailing blanks - might want to "optimize" that away...]]
> > - depends on THERMAL && ACPI > > + depends on THERMAL && ACPI && THERMAL_GOV_BANG_BANG > > Do we actively depend on THERMAL (code-wise, I mean?) Or is it now an > > implicit dependency given that we request THERMAL_GOV_BANG_BANG? If > > implicit, then THERMAL probably ought to be removed. But if we use > > generic thermal APIs (which we probably do), then of course we do have > > that dependency.... > > There's an implicit dependency due to the request of THERMAL_GOV_BANG_BANG, so > yes, we could remove THERMAL here.
ok. Does not seem too risky, and reduces the need for future updates.
> > "used to force thermal" --> misleading ("we used to do this, but it's > > bad so we better do that"). > > > > "intended to"? "established to"? "added to"? or some simpler wording? > > What do you think about this wording: > /* > * this struct is used to instruct thermal layer to use bang_bang instead of > * default governor for acerhdf > */
Nicely detailed.
About the _t typedef: it's said that use of "_t" is undesirable anyway due to being reserved for POSIX. https://en.wikipedia.org/wiki/Typedef http://stackoverflow.com/questions/1186072/naming-scheme-for-typedefs As an alternative using a full "_type" is ok right? (that's what I've been doing...)
Andreas
| |