Messages in this thread | | | Subject | Re: [PATCH v3 3/4] thermal/core: Build ascending ordered indexes for the trip points | From | Zhang Rui <> | Date | Tue, 19 Jul 2022 09:35:21 +0800 |
| |
On Tue, 2022-07-19 at 09:14 +0800, Zhang Rui wrote: > On Mon, 2022-07-18 at 15:21 +0200, Daniel Lezcano wrote: > > > > Hi Zhang, > > > > thanks for the review > > > > On 18/07/2022 07:28, Zhang Rui wrote: > > > On Fri, 2022-07-15 at 23:09 +0200, Daniel Lezcano wrote: > > > > [ ... ] > > > > > > Instead of taking the risk of breaking the existing platforms, > > > > use an > > > > array of temperature ordered trip identifiers and make it > > > > available > > > > for the code needing to browse the trip points in an ordered > > > > way. > > > > > > > > Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org> > > > > --- > > > > [ ... ] > > > > > > +static void sort_trips_indexes(struct thermal_zone_device *tz) > > > > +{ > > > > + int i, j; > > > > + > > > > + for (i = 0; i < tz->trips; i++) > > > > + tz->trips_indexes[i] = i; > > > > + > > > > + for (i = 0; i < tz->trips; i++) { > > > > + for (j = i + 1; j < tz->trips; j++) { > > > > + int t1, t2; > > > > + > > > > + tz->ops->get_trip_temp(tz, tz- > > > > > trips_indexes[i], &t1); > > > > > > This line can be moved to the upper loop. > > > > Right, thanks! > > > > > > + tz->ops->get_trip_temp(tz, tz- > > > > > trips_indexes[j], &t2); > > > > + > > > > > > what about the disabled trip points? > > > > > > we should ignore those trip points and check the return value to > > > make > > > sure we're comparing the valid trip_temp values. > > > > We don't have to care about, whatever the position, the > > corresponding > > trip id will be disabled by the trip init function before calling > > this > > one and ignored in the handle_thermal_trip() function > > hah, I missed this one and replied to your latest reply directly. > > The thing I'm concerning is that if we don't check the return value, > for a disabled trip point, the trip_temp (t1/t2) returned is some > random value, it all depends on the previous value set by last > successful .get_trip_temp(),
or uninitialized value from the stack, but still random value every time we invoke .get_trip_temp() for the same trip point.
-rui
> and this may screw up the sorting. > > thanks, > rui > > > > >
| |