diff mbox

[06/18] thermal: exynos: fix trips limit checking in get_th_reg()

Message ID 1524743493-28113-7-git-send-email-b.zolnierkie@samsung.com (mailing list archive)
State Not Applicable
Headers show

Commit Message

Bartlomiej Zolnierkiewicz April 26, 2018, 11:51 a.m. UTC
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(-)

Comments

Daniel Lezcano April 30, 2018, 3:34 p.m. UTC | #1
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
>
Bartlomiej Zolnierkiewicz April 30, 2018, 3:48 p.m. UTC | #2
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

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Daniel Lezcano May 1, 2018, 9:39 a.m. UTC | #3
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>
Bartlomiej Zolnierkiewicz May 2, 2018, 9:19 a.m. UTC | #4
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

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

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;