| Message ID | 1524743493-28113-7-git-send-email-b.zolnierkie@samsung.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Eduardo Valentin |
| Headers | show |
On Thu, Apr 26, 2018 at 01:51:21PM +0200, Bartlomiej Zolnierkiewicz wrote: > of_thermal_get_ntrips() may return value bigger than supported > by a given SoC (i.e. on Exynos5422/5800) Can you elaborate a bit ? > so fix the code to not > iterate the loop for i values >= data->ntrip. > > There should be no functional changes caused by this patch. > > Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> > --- > drivers/thermal/samsung/exynos_tmu.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c > index 5a64879..58cd68e 100644 > --- a/drivers/thermal/samsung/exynos_tmu.c > +++ b/drivers/thermal/samsung/exynos_tmu.c > @@ -311,9 +311,9 @@ static u32 get_th_reg(struct exynos_tmu_data *data, u32 threshold, bool falling) > const struct thermal_trip * const trips = > of_thermal_get_trip_points(tz); > unsigned long temp; > - int i; > + int i, ntrips = min_t(int, of_thermal_get_ntrips(tz), data->ntrip); > > - for (i = 0; i < of_thermal_get_ntrips(tz); i++) { > + for (i = 0; i < ntrips; i++) { > if (trips[i].type == THERMAL_TRIP_CRITICAL) > continue; > > -- > 1.9.1 >
On Monday, April 30, 2018 05:34:31 PM Daniel Lezcano wrote: > On Thu, Apr 26, 2018 at 01:51:21PM +0200, Bartlomiej Zolnierkiewicz wrote: > > of_thermal_get_ntrips() may return value bigger than supported > > by a given SoC (i.e. on Exynos5422/5800) > > Can you elaborate a bit ? Odroid-XU3 DTS file [1] define 6 thermal trip points (2 passive ones) while data->ntrip is 4, the current code works fine by accident as the threshold values for trip points 5 & 6 don't fit into 32-bits threshold value (however since they are passive ones this is okay). Of course the code for handling passive trip points still needs to be fixed to properly handle all odd cases (which are not present in current DTS files). [1] arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi > > so fix the code to not > > iterate the loop for i values >= data->ntrip. > > > > There should be no functional changes caused by this patch. > > > > Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> > > --- > > drivers/thermal/samsung/exynos_tmu.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c > > index 5a64879..58cd68e 100644 > > --- a/drivers/thermal/samsung/exynos_tmu.c > > +++ b/drivers/thermal/samsung/exynos_tmu.c > > @@ -311,9 +311,9 @@ static u32 get_th_reg(struct exynos_tmu_data *data, u32 threshold, bool falling) > > const struct thermal_trip * const trips = > > of_thermal_get_trip_points(tz); > > unsigned long temp; > > - int i; > > + int i, ntrips = min_t(int, of_thermal_get_ntrips(tz), data->ntrip); > > > > - for (i = 0; i < of_thermal_get_ntrips(tz); i++) { > > + for (i = 0; i < ntrips; i++) { > > if (trips[i].type == THERMAL_TRIP_CRITICAL) > > continue; Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics
On Mon, Apr 30, 2018 at 05:48:29PM +0200, Bartlomiej Zolnierkiewicz wrote: > On Monday, April 30, 2018 05:34:31 PM Daniel Lezcano wrote: > > On Thu, Apr 26, 2018 at 01:51:21PM +0200, Bartlomiej Zolnierkiewicz wrote: > > > of_thermal_get_ntrips() may return value bigger than supported > > > by a given SoC (i.e. on Exynos5422/5800) > > > > Can you elaborate a bit ? > > Odroid-XU3 DTS file [1] define 6 thermal trip points (2 passive ones) > while data->ntrip is 4, the current code works fine by accident as > the threshold values for trip points 5 & 6 don't fit into 32-bits > threshold value (however since they are passive ones this is okay). Ah, I see. data->ntrip is the SoC specific tmu max value capping what is defined in the DT, right ? > Of course the code for handling passive trip points still needs to > be fixed to properly handle all odd cases (which are not present in > current DTS files). > > [1] arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi Yeah, it is definitively a good idea of cleaning up this driver :/ Reviewed-by: Daniel Lezcano <daniel.lezcano@linaro.org>
On Tuesday, May 01, 2018 11:39:14 AM Daniel Lezcano wrote: > On Mon, Apr 30, 2018 at 05:48:29PM +0200, Bartlomiej Zolnierkiewicz wrote: > > On Monday, April 30, 2018 05:34:31 PM Daniel Lezcano wrote: > > > On Thu, Apr 26, 2018 at 01:51:21PM +0200, Bartlomiej Zolnierkiewicz wrote: > > > > of_thermal_get_ntrips() may return value bigger than supported > > > > by a given SoC (i.e. on Exynos5422/5800) > > > > > > Can you elaborate a bit ? > > > > Odroid-XU3 DTS file [1] define 6 thermal trip points (2 passive ones) > > while data->ntrip is 4, the current code works fine by accident as > > the threshold values for trip points 5 & 6 don't fit into 32-bits > > threshold value (however since they are passive ones this is okay). > > Ah, I see. data->ntrip is the SoC specific tmu max value capping what is > defined in the DT, right ? It is SoC specific max value for active + critical trips points. Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics
diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c index 5a64879..58cd68e 100644 --- a/drivers/thermal/samsung/exynos_tmu.c +++ b/drivers/thermal/samsung/exynos_tmu.c @@ -311,9 +311,9 @@ static u32 get_th_reg(struct exynos_tmu_data *data, u32 threshold, bool falling) const struct thermal_trip * const trips = of_thermal_get_trip_points(tz); unsigned long temp; - int i; + int i, ntrips = min_t(int, of_thermal_get_ntrips(tz), data->ntrip); - for (i = 0; i < of_thermal_get_ntrips(tz); i++) { + for (i = 0; i < ntrips; i++) { if (trips[i].type == THERMAL_TRIP_CRITICAL) continue;
of_thermal_get_ntrips() may return value bigger than supported by a given SoC (i.e. on Exynos5422/5800) so fix the code to not iterate the loop for i values >= data->ntrip. There should be no functional changes caused by this patch. Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> --- drivers/thermal/samsung/exynos_tmu.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)