Re: [GIT PULL] Thermal management updates for v4.17-rc1

[Eduardo Valentin]

On Fri, Apr 13, 2018 at 12:27:17PM +0200, Bartlomiej Zolnierkiewicz wrote:
> On Friday, April 13, 2018 03:08:03 AM Eduardo Valentin wrote:
> > On Fri, Apr 13, 2018 at 01:39:05PM +0800, Zhang Rui wrote:
> > > Hi, Eduardo,
> > > 
> > > On 四, 2018-04-12 at 21:08 -0700, Eduardo Valentin wrote:
> > > > Hello,
> > > > 
> > > > On Thu, Apr 12, 2018 at 09:55:19AM -0700, Linus Torvalds wrote:
> > > > > 
> > > > > On Wed, Apr 11, 2018 at 10:08 PM, Zhang Rui 
> > > > > wrote:
> > > > > > 
> > > > > > 
> > > > > > could you please illustrate me what the kconfig & warning is?
> > > > > Just "make allmodconfig" and the warning is about a uninitialized
> > > > > variable.
> > > > > 
> > > > > Line 304 in drivers/thermal/samsung/exynos_tmu.c if my shell
> > > > > history
> > > > > is to be believed.
> > > > > 
> > > > > Linus
> > > > Yeah, this has also passed my local compilation error. Somehow my
> > > > gcc4.9
> > > > is not catching it. Using an older gcc (gcc4.6) does catch it.
> > > > 
> > > > Anyways, given that the conversion functions are written to cover
> > > > for unexpected cal_type, the right way of fixing this is to rewrite
> > > > the conversion functions to allow for returning error codes and
> > > > adjusting the callers as expected.
> > > > 
> > > > Rui, bzolnier, please consider the following fix:
> > > > 
> > > as it is late in this merge window, I'd prefer to
> > > 1. drop all the thermal-soc material in the first pull request which I
> > > will send out soon.
> > > 2. you can prepare another pull request containing the thermal-soc
> > > materials except the exynos fixes
> > > 3. exynos fixes with the problem solved can be queued for -rc2 or
> > > later.
> > > 
> > 
> > Agreed
> 
> What should I do now to help resolve the issue?

Please resend your series with the patches without the warnings..

Thanks,

Eduardo

Re: [GIT PULL] Thermal management updates for v4.17-rc1

[Bartlomiej Zolnierkiewicz]

On Friday, April 13, 2018 01:12:39 PM Bartlomiej Zolnierkiewicz wrote:
> On Friday, April 13, 2018 12:41:18 PM Bartlomiej Zolnierkiewicz wrote:
> > On Friday, April 13, 2018 12:30:04 PM Daniel Lezcano wrote:
> > > On 13/04/2018 11:28, Bartlomiej Zolnierkiewicz wrote:
> > > 
> > > [ ... ]
> > > 
> > > >>> It is okay to return 0 because this code-path (the default one) will 
> > > >>> be
> > > >>> never hit by the driver (probe makes sure of it) - the default case is
> > > >>> here is just to silence compilation errors..
> > > >>
> > > >> The init function is making sure cal_type is one or another. Can you 
> > > >> fix
> > > >> it correctly by replacing the 'switch' by a 'if' instead of adding dead
> > > >> branches to please gcc?
> > > >>
> > > >> if (data->cal_type == TYPE_TWO_POINT_TRIMMING) {
> > > >>return ...;
> > > >> }
> > > >>
> > > >> return ...;
> > > > 
> > > > I'm not the one that added this switch statement (it has been there 
> > > > since
> > > > 2011) and I would be happy to remove it. 
> > > 
> > > Actually the switch statement was fine until the cleanup.
> > 
> > I don't see how it was fine before as the driver has never used the default
> > case (always used TYPE_ONE_POINT_TRIMMING or TYPE_TWO_POINT_TRIMMING).
> > 
> > Could you please explain this more?
> > 
> > > > However could we please defer
> > > > this to v4.17 and merge the current set of Exynos thermal fixes/cleanups
> > > > (they simplify the driver a lot and make ground for future changes)?
> > > 
> > > Regarding the latest comment, this can be fixed properly by 'return' (or
> > > whatever you want which does not get around of gcc warnings).
> > 
> > Do you mean that you want the patch with switch statement removal?
> > 
> > Is incremental fix OK or do you want something else?
> 
> Danial has already posted it, I hope the fix is fine with you.

should have been:

Eduardo: Daniel has already posted it, I hope the fix is fine with you.

(& sorry for the typo)

> Also sorry for the delay with handling issue - I was on holiday last two
> days and for some reason I was under (wrong) impression that the previous
> fix has been in thermal tree (so I was quite surprised today reading this
> mail thread).

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

Re: [GIT PULL] Thermal management updates for v4.17-rc1

[Bartlomiej Zolnierkiewicz]

On Friday, April 13, 2018 12:41:18 PM Bartlomiej Zolnierkiewicz wrote:
> On Friday, April 13, 2018 12:30:04 PM Daniel Lezcano wrote:
> > On 13/04/2018 11:28, Bartlomiej Zolnierkiewicz wrote:
> > 
> > [ ... ]
> > 
> > >>> It is okay to return 0 because this code-path (the default one) will be
> > >>> never hit by the driver (probe makes sure of it) - the default case is
> > >>> here is just to silence compilation errors..
> > >>
> > >> The init function is making sure cal_type is one or another. Can you fix
> > >> it correctly by replacing the 'switch' by a 'if' instead of adding dead
> > >> branches to please gcc?
> > >>
> > >> if (data->cal_type == TYPE_TWO_POINT_TRIMMING) {
> > >>  return ...;
> > >> }
> > >>
> > >> return ...;
> > > 
> > > I'm not the one that added this switch statement (it has been there since
> > > 2011) and I would be happy to remove it. 
> > 
> > Actually the switch statement was fine until the cleanup.
> 
> I don't see how it was fine before as the driver has never used the default
> case (always used TYPE_ONE_POINT_TRIMMING or TYPE_TWO_POINT_TRIMMING).
> 
> Could you please explain this more?
> 
> > > However could we please defer
> > > this to v4.17 and merge the current set of Exynos thermal fixes/cleanups
> > > (they simplify the driver a lot and make ground for future changes)?
> > 
> > Regarding the latest comment, this can be fixed properly by 'return' (or
> > whatever you want which does not get around of gcc warnings).
> 
> Do you mean that you want the patch with switch statement removal?
> 
> Is incremental fix OK or do you want something else?

Danial has already posted it, I hope the fix is fine with you.

Also sorry for the delay with handling issue - I was on holiday last two
days and for some reason I was under (wrong) impression that the previous
fix has been in thermal tree (so I was quite surprised today reading this
mail thread).

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

Re: [GIT PULL] Thermal management updates for v4.17-rc1

[Daniel Lezcano]

On 13/04/2018 12:41, Bartlomiej Zolnierkiewicz wrote:
> On Friday, April 13, 2018 12:30:04 PM Daniel Lezcano wrote:
>> On 13/04/2018 11:28, Bartlomiej Zolnierkiewicz wrote:
>>
>> [ ... ]
>>
> It is okay to return 0 because this code-path (the default one) will be
> never hit by the driver (probe makes sure of it) - the default case is
> here is just to silence compilation errors..

 The init function is making sure cal_type is one or another. Can you fix
 it correctly by replacing the 'switch' by a 'if' instead of adding dead
 branches to please gcc?

 if (data->cal_type == TYPE_TWO_POINT_TRIMMING) {
return ...;
 }

 return ...;
>>>
>>> I'm not the one that added this switch statement (it has been there since
>>> 2011) and I would be happy to remove it. 
>>
>> Actually the switch statement was fine until the cleanup.
> 
> I don't see how it was fine before as the driver has never used the default
> case (always used TYPE_ONE_POINT_TRIMMING or TYPE_TWO_POINT_TRIMMING).
> 
> Could you please explain this more?

>From commit 480b5bfc16e17ef51ca1c

+++ b/drivers/thermal/samsung/exynos_tmu.c
@@ -260,7 +260,7 @@ static int temp_to_code(struct exynos_tmu_data
*data, u8 temp)
temp_code = temp + data->temp_error1 -
pdata->first_point_trim;
break;
default:
-   temp_code = temp + pdata->default_temp_offset;
+   WARN_ON(1);
break;
}

@@ -287,7 +287,7 @@ static int code_to_temp(struct exynos_tmu_data
*data, u16 temp_code)
temp = temp_code - data->temp_error1 +
pdata->first_point_trim;
break;
default:
-   temp = temp_code - pdata->default_temp_offset;
+   WARN_ON(1);
break;
}

I'm not saying the code path was fine but from the compiler point of
view, it was. By removing the defaulting temp value there is a code path
gcc sees the temp variable as not initialized.

Your cleanups are relevant.


> --
> Bartlomiej Zolnierkiewicz
> Samsung R&D Institute Poland
> Samsung Electronics
> 


-- 
  Linaro.org │ Open source software for ARM SoCs

Follow Linaro:   Facebook |
 Twitter |
 Blog

Re: [GIT PULL] Thermal management updates for v4.17-rc1

[Bartlomiej Zolnierkiewicz]

On Friday, April 13, 2018 12:30:04 PM Daniel Lezcano wrote:
> On 13/04/2018 11:28, Bartlomiej Zolnierkiewicz wrote:
> 
> [ ... ]
> 
> >>> It is okay to return 0 because this code-path (the default one) will be
> >>> never hit by the driver (probe makes sure of it) - the default case is
> >>> here is just to silence compilation errors..
> >>
> >> The init function is making sure cal_type is one or another. Can you fix
> >> it correctly by replacing the 'switch' by a 'if' instead of adding dead
> >> branches to please gcc?
> >>
> >> if (data->cal_type == TYPE_TWO_POINT_TRIMMING) {
> >>return ...;
> >> }
> >>
> >> return ...;
> > 
> > I'm not the one that added this switch statement (it has been there since
> > 2011) and I would be happy to remove it. 
> 
> Actually the switch statement was fine until the cleanup.

I don't see how it was fine before as the driver has never used the default
case (always used TYPE_ONE_POINT_TRIMMING or TYPE_TWO_POINT_TRIMMING).

Could you please explain this more?

> > However could we please defer
> > this to v4.17 and merge the current set of Exynos thermal fixes/cleanups
> > (they simplify the driver a lot and make ground for future changes)?
> 
> Regarding the latest comment, this can be fixed properly by 'return' (or
> whatever you want which does not get around of gcc warnings).

Do you mean that you want the patch with switch statement removal?

Is incremental fix OK or do you want something else?

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

Re: [GIT PULL] Thermal management updates for v4.17-rc1

[Daniel Lezcano]

On 13/04/2018 11:28, Bartlomiej Zolnierkiewicz wrote:

[ ... ]

>>> It is okay to return 0 because this code-path (the default one) will be
>>> never hit by the driver (probe makes sure of it) - the default case is
>>> here is just to silence compilation errors..
>>
>> The init function is making sure cal_type is one or another. Can you fix
>> it correctly by replacing the 'switch' by a 'if' instead of adding dead
>> branches to please gcc?
>>
>> if (data->cal_type == TYPE_TWO_POINT_TRIMMING) {
>>  return ...;
>> }
>>
>> return ...;
> 
> I'm not the one that added this switch statement (it has been there since
> 2011) and I would be happy to remove it. 

Actually the switch statement was fine until the cleanup.

> However could we please defer
> this to v4.17 and merge the current set of Exynos thermal fixes/cleanups
> (they simplify the driver a lot and make ground for future changes)?

Regarding the latest comment, this can be fixed properly by 'return' (or
whatever you want which does not get around of gcc warnings).

-- 
  Linaro.org │ Open source software for ARM SoCs

Follow Linaro:   Facebook |
 Twitter |
 Blog

Re: [GIT PULL] Thermal management updates for v4.17-rc1

[Bartlomiej Zolnierkiewicz]

On Friday, April 13, 2018 03:08:03 AM Eduardo Valentin wrote:
> On Fri, Apr 13, 2018 at 01:39:05PM +0800, Zhang Rui wrote:
> > Hi, Eduardo,
> > 
> > On 四, 2018-04-12 at 21:08 -0700, Eduardo Valentin wrote:
> > > Hello,
> > > 
> > > On Thu, Apr 12, 2018 at 09:55:19AM -0700, Linus Torvalds wrote:
> > > > 
> > > > On Wed, Apr 11, 2018 at 10:08 PM, Zhang Rui 
> > > > wrote:
> > > > > 
> > > > > 
> > > > > could you please illustrate me what the kconfig & warning is?
> > > > Just "make allmodconfig" and the warning is about a uninitialized
> > > > variable.
> > > > 
> > > > Line 304 in drivers/thermal/samsung/exynos_tmu.c if my shell
> > > > history
> > > > is to be believed.
> > > > 
> > > > Linus
> > > Yeah, this has also passed my local compilation error. Somehow my
> > > gcc4.9
> > > is not catching it. Using an older gcc (gcc4.6) does catch it.
> > > 
> > > Anyways, given that the conversion functions are written to cover
> > > for unexpected cal_type, the right way of fixing this is to rewrite
> > > the conversion functions to allow for returning error codes and
> > > adjusting the callers as expected.
> > > 
> > > Rui, bzolnier, please consider the following fix:
> > > 
> > as it is late in this merge window, I'd prefer to
> > 1. drop all the thermal-soc material in the first pull request which I
> > will send out soon.
> > 2. you can prepare another pull request containing the thermal-soc
> > materials except the exynos fixes
> > 3. exynos fixes with the problem solved can be queued for -rc2 or
> > later.
> > 
> 
> Agreed

What should I do now to help resolve the issue?

[ There has been no action from you on Arnd's fix for over two weeks and
  also you have not commented on it now.. ]

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

Re: [GIT PULL] Thermal management updates for v4.17-rc1

[Eduardo Valentin]

On Fri, Apr 13, 2018 at 03:08:03AM -0700, Eduardo Valentin wrote:
> On Fri, Apr 13, 2018 at 01:39:05PM +0800, Zhang Rui wrote:
> > Hi, Eduardo,
> > 
> > On 四, 2018-04-12 at 21:08 -0700, Eduardo Valentin wrote:
> > > Hello,
> > > 
> > > On Thu, Apr 12, 2018 at 09:55:19AM -0700, Linus Torvalds wrote:
> > > > 
> > > > On Wed, Apr 11, 2018 at 10:08 PM, Zhang Rui 
> > > > wrote:
> > > > > 
> > > > > 
> > > > > could you please illustrate me what the kconfig & warning is?
> > > > Just "make allmodconfig" and the warning is about a uninitialized
> > > > variable.
> > > > 
> > > > Line 304 in drivers/thermal/samsung/exynos_tmu.c if my shell
> > > > history
> > > > is to be believed.
> > > > 
> > > > Linus
> > > Yeah, this has also passed my local compilation error. Somehow my
> > > gcc4.9
> > > is not catching it. Using an older gcc (gcc4.6) does catch it.
> > > 
> > > Anyways, given that the conversion functions are written to cover
> > > for unexpected cal_type, the right way of fixing this is to rewrite
> > > the conversion functions to allow for returning error codes and
> > > adjusting the callers as expected.
> > > 
> > > Rui, bzolnier, please consider the following fix:
> > > 
> > as it is late in this merge window, I'd prefer to
> > 1. drop all the thermal-soc material in the first pull request which I
> > will send out soon.
> > 2. you can prepare another pull request containing the thermal-soc
> > materials except the exynos fixes

Sent you this
https://marc.info/?l=linux-pm&m=152361492711499&w=2

> > 3. exynos fixes with the problem solved can be queued for -rc2 or
> > later.

I see there is still some discussion around the topic of how to fix
this. So, once we get to a point of agreement, I will send the remaining
with exynos fixes.

> > 
> 
> Agreed
> 

Re: [GIT PULL] Thermal management updates for v4.17-rc1

[Eduardo Valentin]

On Fri, Apr 13, 2018 at 01:39:05PM +0800, Zhang Rui wrote:
> Hi, Eduardo,
> 
> On 四, 2018-04-12 at 21:08 -0700, Eduardo Valentin wrote:
> > Hello,
> > 
> > On Thu, Apr 12, 2018 at 09:55:19AM -0700, Linus Torvalds wrote:
> > > 
> > > On Wed, Apr 11, 2018 at 10:08 PM, Zhang Rui 
> > > wrote:
> > > > 
> > > > 
> > > > could you please illustrate me what the kconfig & warning is?
> > > Just "make allmodconfig" and the warning is about a uninitialized
> > > variable.
> > > 
> > > Line 304 in drivers/thermal/samsung/exynos_tmu.c if my shell
> > > history
> > > is to be believed.
> > > 
> > > Linus
> > Yeah, this has also passed my local compilation error. Somehow my
> > gcc4.9
> > is not catching it. Using an older gcc (gcc4.6) does catch it.
> > 
> > Anyways, given that the conversion functions are written to cover
> > for unexpected cal_type, the right way of fixing this is to rewrite
> > the conversion functions to allow for returning error codes and
> > adjusting the callers as expected.
> > 
> > Rui, bzolnier, please consider the following fix:
> > 
> as it is late in this merge window, I'd prefer to
> 1. drop all the thermal-soc material in the first pull request which I
> will send out soon.
> 2. you can prepare another pull request containing the thermal-soc
> materials except the exynos fixes
> 3. exynos fixes with the problem solved can be queued for -rc2 or
> later.
> 

Agreed

Re: [GIT PULL] Thermal management updates for v4.17-rc1

[Bartlomiej Zolnierkiewicz]

On Friday, April 13, 2018 11:19:40 AM Daniel Lezcano wrote:
> On 13/04/2018 11:08, Bartlomiej Zolnierkiewicz wrote:
> > On Friday, April 13, 2018 11:00:43 AM Daniel Lezcano wrote:
> >> On 13/04/2018 10:55, Bartlomiej Zolnierkiewicz wrote:
> >>> On Friday, April 13, 2018 01:39:05 PM Zhang Rui wrote:
>  Hi, Eduardo,
> 
>  On 四, 2018-04-12 at 21:08 -0700, Eduardo Valentin wrote:
> > Hello,
> >
> > On Thu, Apr 12, 2018 at 09:55:19AM -0700, Linus Torvalds wrote:
> >>
> >> On Wed, Apr 11, 2018 at 10:08 PM, Zhang Rui 
> >> wrote:
> >>>
> >>>
> >>> could you please illustrate me what the kconfig & warning is?
> >> Just "make allmodconfig" and the warning is about a uninitialized
> >> variable.
> >>
> >> Line 304 in drivers/thermal/samsung/exynos_tmu.c if my shell
> >> history
> >> is to be believed.
> >>
> >> Linus
> > Yeah, this has also passed my local compilation error. Somehow my
> > gcc4.9
> > is not catching it. Using an older gcc (gcc4.6) does catch it.
> >
> > Anyways, given that the conversion functions are written to cover
> > for unexpected cal_type, the right way of fixing this is to rewrite
> > the conversion functions to allow for returning error codes and
> > adjusting the callers as expected.
> >
> > Rui, bzolnier, please consider the following fix:
> >
>  as it is late in this merge window, I'd prefer to
>  1. drop all the thermal-soc material in the first pull request which I
>  will send out soon.
>  2. you can prepare another pull request containing the thermal-soc
>  materials except the exynos fixes
>  3. exynos fixes with the problem solved can be queued for -rc2 or
>  later.
> >>>
> >>> Could you please just merge the obvious fix from Arnd instead?
> >>>
> >>> [ it was posted two weeks ago and ACKed by me ]
> >>>
> >>> https://patchwork.kernel.org/patch/10313313/
> >>
> >> I'm not sure these are correct fixes.
> >>
> >> The change 480b5bfc16e1 tells:
> >>
> >> "There should be no functional changes caused by this patch."
> >>
> >> but the fix above returns 0 as a default value instead of '50' or '25'
> >> for the 5440 and that impacts the threshold etc ...
> >>
> >> IMO, the correct fix would be to define a default value '50', override
> >> it at init time to '25' if it is a 5440. And then the variable 'temp'
> >> and 'temp_code' get this value in the default case.
> > 
> > It is okay to return 0 because this code-path (the default one) will be
> > never hit by the driver (probe makes sure of it) - the default case is
> > here is just to silence compilation errors..
> 
> The init function is making sure cal_type is one or another. Can you fix
> it correctly by replacing the 'switch' by a 'if' instead of adding dead
> branches to please gcc?
> 
> if (data->cal_type == TYPE_TWO_POINT_TRIMMING) {
>   return ...;
> }
> 
> return ...;

I'm not the one that added this switch statement (it has been there since
2011) and I would be happy to remove it.  However could we please defer
this to v4.17 and merge the current set of Exynos thermal fixes/cleanups
(they simplify the driver a lot and make ground for future changes)?

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

Re: [GIT PULL] Thermal management updates for v4.17-rc1

[Daniel Lezcano]

On 13/04/2018 11:08, Bartlomiej Zolnierkiewicz wrote:
> On Friday, April 13, 2018 11:00:43 AM Daniel Lezcano wrote:
>> On 13/04/2018 10:55, Bartlomiej Zolnierkiewicz wrote:
>>> On Friday, April 13, 2018 01:39:05 PM Zhang Rui wrote:
 Hi, Eduardo,

 On 四, 2018-04-12 at 21:08 -0700, Eduardo Valentin wrote:
> Hello,
>
> On Thu, Apr 12, 2018 at 09:55:19AM -0700, Linus Torvalds wrote:
>>
>> On Wed, Apr 11, 2018 at 10:08 PM, Zhang Rui 
>> wrote:
>>>
>>>
>>> could you please illustrate me what the kconfig & warning is?
>> Just "make allmodconfig" and the warning is about a uninitialized
>> variable.
>>
>> Line 304 in drivers/thermal/samsung/exynos_tmu.c if my shell
>> history
>> is to be believed.
>>
>> Linus
> Yeah, this has also passed my local compilation error. Somehow my
> gcc4.9
> is not catching it. Using an older gcc (gcc4.6) does catch it.
>
> Anyways, given that the conversion functions are written to cover
> for unexpected cal_type, the right way of fixing this is to rewrite
> the conversion functions to allow for returning error codes and
> adjusting the callers as expected.
>
> Rui, bzolnier, please consider the following fix:
>
 as it is late in this merge window, I'd prefer to
 1. drop all the thermal-soc material in the first pull request which I
 will send out soon.
 2. you can prepare another pull request containing the thermal-soc
 materials except the exynos fixes
 3. exynos fixes with the problem solved can be queued for -rc2 or
 later.
>>>
>>> Could you please just merge the obvious fix from Arnd instead?
>>>
>>> [ it was posted two weeks ago and ACKed by me ]
>>>
>>> https://patchwork.kernel.org/patch/10313313/
>>
>> I'm not sure these are correct fixes.
>>
>> The change 480b5bfc16e1 tells:
>>
>> "There should be no functional changes caused by this patch."
>>
>> but the fix above returns 0 as a default value instead of '50' or '25'
>> for the 5440 and that impacts the threshold etc ...
>>
>> IMO, the correct fix would be to define a default value '50', override
>> it at init time to '25' if it is a 5440. And then the variable 'temp'
>> and 'temp_code' get this value in the default case.
> 
> It is okay to return 0 because this code-path (the default one) will be
> never hit by the driver (probe makes sure of it) - the default case is
> here is just to silence compilation errors..

The init function is making sure cal_type is one or another. Can you fix
it correctly by replacing the 'switch' by a 'if' instead of adding dead
branches to please gcc?

if (data->cal_type == TYPE_TWO_POINT_TRIMMING) {
return ...;
}

return ...;

-- 
  Linaro.org │ Open source software for ARM SoCs

Follow Linaro:   Facebook |
 Twitter |
 Blog

Re: [GIT PULL] Thermal management updates for v4.17-rc1

[Bartlomiej Zolnierkiewicz]

On Friday, April 13, 2018 11:00:43 AM Daniel Lezcano wrote:
> On 13/04/2018 10:55, Bartlomiej Zolnierkiewicz wrote:
> > On Friday, April 13, 2018 01:39:05 PM Zhang Rui wrote:
> >> Hi, Eduardo,
> >>
> >> On 四, 2018-04-12 at 21:08 -0700, Eduardo Valentin wrote:
> >>> Hello,
> >>>
> >>> On Thu, Apr 12, 2018 at 09:55:19AM -0700, Linus Torvalds wrote:
> 
>  On Wed, Apr 11, 2018 at 10:08 PM, Zhang Rui 
>  wrote:
> >
> >
> > could you please illustrate me what the kconfig & warning is?
>  Just "make allmodconfig" and the warning is about a uninitialized
>  variable.
> 
>  Line 304 in drivers/thermal/samsung/exynos_tmu.c if my shell
>  history
>  is to be believed.
> 
>  Linus
> >>> Yeah, this has also passed my local compilation error. Somehow my
> >>> gcc4.9
> >>> is not catching it. Using an older gcc (gcc4.6) does catch it.
> >>>
> >>> Anyways, given that the conversion functions are written to cover
> >>> for unexpected cal_type, the right way of fixing this is to rewrite
> >>> the conversion functions to allow for returning error codes and
> >>> adjusting the callers as expected.
> >>>
> >>> Rui, bzolnier, please consider the following fix:
> >>>
> >> as it is late in this merge window, I'd prefer to
> >> 1. drop all the thermal-soc material in the first pull request which I
> >> will send out soon.
> >> 2. you can prepare another pull request containing the thermal-soc
> >> materials except the exynos fixes
> >> 3. exynos fixes with the problem solved can be queued for -rc2 or
> >> later.
> > 
> > Could you please just merge the obvious fix from Arnd instead?
> > 
> > [ it was posted two weeks ago and ACKed by me ]
> > 
> > https://patchwork.kernel.org/patch/10313313/
> 
> I'm not sure these are correct fixes.
> 
> The change 480b5bfc16e1 tells:
> 
> "There should be no functional changes caused by this patch."
> 
> but the fix above returns 0 as a default value instead of '50' or '25'
> for the 5440 and that impacts the threshold etc ...
> 
> IMO, the correct fix would be to define a default value '50', override
> it at init time to '25' if it is a 5440. And then the variable 'temp'
> and 'temp_code' get this value in the default case.

It is okay to return 0 because this code-path (the default one) will be
never hit by the driver (probe makes sure of it) - the default case is
here is just to silence compilation errors..

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

Re: [GIT PULL] Thermal management updates for v4.17-rc1

[Daniel Lezcano]

On 13/04/2018 10:55, Bartlomiej Zolnierkiewicz wrote:
> On Friday, April 13, 2018 01:39:05 PM Zhang Rui wrote:
>> Hi, Eduardo,
>>
>> On 四, 2018-04-12 at 21:08 -0700, Eduardo Valentin wrote:
>>> Hello,
>>>
>>> On Thu, Apr 12, 2018 at 09:55:19AM -0700, Linus Torvalds wrote:

 On Wed, Apr 11, 2018 at 10:08 PM, Zhang Rui 
 wrote:
>
>
> could you please illustrate me what the kconfig & warning is?
 Just "make allmodconfig" and the warning is about a uninitialized
 variable.

 Line 304 in drivers/thermal/samsung/exynos_tmu.c if my shell
 history
 is to be believed.

 Linus
>>> Yeah, this has also passed my local compilation error. Somehow my
>>> gcc4.9
>>> is not catching it. Using an older gcc (gcc4.6) does catch it.
>>>
>>> Anyways, given that the conversion functions are written to cover
>>> for unexpected cal_type, the right way of fixing this is to rewrite
>>> the conversion functions to allow for returning error codes and
>>> adjusting the callers as expected.
>>>
>>> Rui, bzolnier, please consider the following fix:
>>>
>> as it is late in this merge window, I'd prefer to
>> 1. drop all the thermal-soc material in the first pull request which I
>> will send out soon.
>> 2. you can prepare another pull request containing the thermal-soc
>> materials except the exynos fixes
>> 3. exynos fixes with the problem solved can be queued for -rc2 or
>> later.
> 
> Could you please just merge the obvious fix from Arnd instead?
> 
> [ it was posted two weeks ago and ACKed by me ]
> 
> https://patchwork.kernel.org/patch/10313313/

I'm not sure these are correct fixes.

The change 480b5bfc16e1 tells:

"There should be no functional changes caused by this patch."

but the fix above returns 0 as a default value instead of '50' or '25'
for the 5440 and that impacts the threshold etc ...

IMO, the correct fix would be to define a default value '50', override
it at init time to '25' if it is a 5440. And then the variable 'temp'
and 'temp_code' get this value in the default case.




> Best regards,
> --
> Bartlomiej Zolnierkiewicz
> Samsung R&D Institute Poland
> Samsung Electronics
> 


-- 
  Linaro.org │ Open source software for ARM SoCs

Follow Linaro:   Facebook |
 Twitter |
 Blog

Re: [GIT PULL] Thermal management updates for v4.17-rc1

[Bartlomiej Zolnierkiewicz]

On Friday, April 13, 2018 01:39:05 PM Zhang Rui wrote:
> Hi, Eduardo,
> 
> On 四, 2018-04-12 at 21:08 -0700, Eduardo Valentin wrote:
> > Hello,
> > 
> > On Thu, Apr 12, 2018 at 09:55:19AM -0700, Linus Torvalds wrote:
> > > 
> > > On Wed, Apr 11, 2018 at 10:08 PM, Zhang Rui 
> > > wrote:
> > > > 
> > > > 
> > > > could you please illustrate me what the kconfig & warning is?
> > > Just "make allmodconfig" and the warning is about a uninitialized
> > > variable.
> > > 
> > > Line 304 in drivers/thermal/samsung/exynos_tmu.c if my shell
> > > history
> > > is to be believed.
> > > 
> > > Linus
> > Yeah, this has also passed my local compilation error. Somehow my
> > gcc4.9
> > is not catching it. Using an older gcc (gcc4.6) does catch it.
> > 
> > Anyways, given that the conversion functions are written to cover
> > for unexpected cal_type, the right way of fixing this is to rewrite
> > the conversion functions to allow for returning error codes and
> > adjusting the callers as expected.
> > 
> > Rui, bzolnier, please consider the following fix:
> > 
> as it is late in this merge window, I'd prefer to
> 1. drop all the thermal-soc material in the first pull request which I
> will send out soon.
> 2. you can prepare another pull request containing the thermal-soc
> materials except the exynos fixes
> 3. exynos fixes with the problem solved can be queued for -rc2 or
> later.

Could you please just merge the obvious fix from Arnd instead?

[ it was posted two weeks ago and ACKed by me ]

https://patchwork.kernel.org/patch/10313313/

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

Re: [GIT PULL] Thermal management updates for v4.17-rc1

[Bartlomiej Zolnierkiewicz]

On Thursday, April 12, 2018 09:08:57 PM Eduardo Valentin wrote:
> Hello,

Hi,

> On Thu, Apr 12, 2018 at 09:55:19AM -0700, Linus Torvalds wrote:
> > On Wed, Apr 11, 2018 at 10:08 PM, Zhang Rui  wrote:
> > >
> > > could you please illustrate me what the kconfig & warning is?
> > 
> > Just "make allmodconfig" and the warning is about a uninitialized variable.
> > 
> > Line 304 in drivers/thermal/samsung/exynos_tmu.c if my shell history
> > is to be believed.
> > 
> > Linus
> 
> Yeah, this has also passed my local compilation error. Somehow my gcc4.9
> is not catching it. Using an older gcc (gcc4.6) does catch it.
> 
> Anyways, given that the conversion functions are written to cover
> for unexpected cal_type, the right way of fixing this is to rewrite
> the conversion functions to allow for returning error codes and
> adjusting the callers as expected.
> 
> Rui, bzolnier, please consider the following fix:
> 
> From 2aaf94f80c0021a21b4122c9f4197acff08ea398 Mon Sep 17 00:00:00 2001
> From: Eduardo Valentin 
> Date: Thu, 12 Apr 2018 21:00:48 -0700
> Subject: [PATCH 1/1] thermal: exynos: fix compilation warning around
>  conversion functions
> 
> In order to fix the warns:
> drivers/thermal/samsung/exynos_tmu.c:931:37: warning: 'temp' may be used 
> uninitialized in this function [-Wmaybe-uninitialized]
> drivers/thermal/samsung/exynos_tmu.c:304:9: warning: 'temp_code' may be used 
> uninitialized in this function [-Wmaybe-uninitialized]
> 
> the conversion functions should allow return error codes
> and the not mix the converted value with error code.
> 
> This patch change the conversion functions to return
> error code or success and adjusts the callers accordingly.
> 
> Signed-off-by: Eduardo Valentin 
> ---
>  drivers/thermal/samsung/exynos_tmu.c | 120 
> ---
>  1 file changed, 84 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/thermal/samsung/exynos_tmu.c 
> b/drivers/thermal/samsung/exynos_tmu.c
> index 2ec8548..b3f0704 100644
> --- a/drivers/thermal/samsung/exynos_tmu.c
> +++ b/drivers/thermal/samsung/exynos_tmu.c
> @@ -282,52 +282,54 @@ static void exynos_report_trigger(struct 
> exynos_tmu_data *p)
>   * TMU treats temperature as a mapped temperature code.
>   * The temperature is converted differently depending on the calibration 
> type.
>   */
> -static int temp_to_code(struct exynos_tmu_data *data, u8 temp)
> +static int temp_to_code(struct exynos_tmu_data *data, u8 temp, int 
> *temp_code)
>  {
> - int temp_code;
> + int ret = 0;
>  
>   switch (data->cal_type) {
>   case TYPE_TWO_POINT_TRIMMING:
> - temp_code = (temp - EXYNOS_FIRST_POINT_TRIM) *
> + *temp_code = (temp - EXYNOS_FIRST_POINT_TRIM) *
>   (data->temp_error2 - data->temp_error1) /
>   (EXYNOS_SECOND_POINT_TRIM - EXYNOS_FIRST_POINT_TRIM) +
>   data->temp_error1;
>   break;
>   case TYPE_ONE_POINT_TRIMMING:
> - temp_code = temp + data->temp_error1 - EXYNOS_FIRST_POINT_TRIM;
> + *temp_code = temp + data->temp_error1 - EXYNOS_FIRST_POINT_TRIM;
>   break;
>   default:

Since this condition cannot happen (the driver makes sure of this during
probe) I would prefer much simpler fix from Arnd:

https://patchwork.kernel.org/patch/10313313/

(I've already ACKed it two weeks ago).

>   WARN_ON(1);
> + ret = -EINVAL;
>   break;
>   }
>  
> - return temp_code;
> + return ret;
>  }
>  
>  /*
>   * Calculate a temperature value from a temperature code.
>   * The unit of the temperature is degree Celsius.
>   */
> -static int code_to_temp(struct exynos_tmu_data *data, u16 temp_code)
> +static int code_to_temp(struct exynos_tmu_data *data, u16 temp_code, int 
> *temp)
>  {
> - int temp;
> + int ret = 0;
>  
>   switch (data->cal_type) {
>   case TYPE_TWO_POINT_TRIMMING:
> - temp = (temp_code - data->temp_error1) *
> + *temp = (temp_code - data->temp_error1) *
>   (EXYNOS_SECOND_POINT_TRIM - EXYNOS_FIRST_POINT_TRIM) /
>   (data->temp_error2 - data->temp_error1) +
>   EXYNOS_FIRST_POINT_TRIM;
>   break;
>   case TYPE_ONE_POINT_TRIMMING:
> - temp = temp_code - data->temp_error1 + EXYNOS_FIRST_POINT_TRIM;
> + *temp = temp_code - data->temp_error1 + EXYNOS_FIRST_POINT_TRIM;
>   break;
>   default:
>   WARN_ON(1);
> + ret = -EINVAL;

ditto

>   break;
>   }
>  
> - return temp;
> + return ret;
>  }

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

Re: [GIT PULL] Thermal management updates for v4.17-rc1

[Zhang Rui]

Hi, Eduardo,

On 四, 2018-04-12 at 21:08 -0700, Eduardo Valentin wrote:
> Hello,
> 
> On Thu, Apr 12, 2018 at 09:55:19AM -0700, Linus Torvalds wrote:
> > 
> > On Wed, Apr 11, 2018 at 10:08 PM, Zhang Rui 
> > wrote:
> > > 
> > > 
> > > could you please illustrate me what the kconfig & warning is?
> > Just "make allmodconfig" and the warning is about a uninitialized
> > variable.
> > 
> > Line 304 in drivers/thermal/samsung/exynos_tmu.c if my shell
> > history
> > is to be believed.
> > 
> > Linus
> Yeah, this has also passed my local compilation error. Somehow my
> gcc4.9
> is not catching it. Using an older gcc (gcc4.6) does catch it.
> 
> Anyways, given that the conversion functions are written to cover
> for unexpected cal_type, the right way of fixing this is to rewrite
> the conversion functions to allow for returning error codes and
> adjusting the callers as expected.
> 
> Rui, bzolnier, please consider the following fix:
> 
as it is late in this merge window, I'd prefer to
1. drop all the thermal-soc material in the first pull request which I
will send out soon.
2. you can prepare another pull request containing the thermal-soc
materials except the exynos fixes
3. exynos fixes with the problem solved can be queued for -rc2 or
later.

thanks,
rui

> From 2aaf94f80c0021a21b4122c9f4197acff08ea398 Mon Sep 17 00:00:00
> 2001
> From: Eduardo Valentin 
> Date: Thu, 12 Apr 2018 21:00:48 -0700
> Subject: [PATCH 1/1] thermal: exynos: fix compilation warning around
>  conversion functions
> 
> In order to fix the warns:
> drivers/thermal/samsung/exynos_tmu.c:931:37: warning: 'temp' may be
> used uninitialized in this function [-Wmaybe-uninitialized]
> drivers/thermal/samsung/exynos_tmu.c:304:9: warning: 'temp_code' may
> be used uninitialized in this function [-Wmaybe-uninitialized]
> 
> the conversion functions should allow return error codes
> and the not mix the converted value with error code.
> 
> This patch change the conversion functions to return
> error code or success and adjusts the callers accordingly.
> 
> Signed-off-by: Eduardo Valentin 
> ---
>  drivers/thermal/samsung/exynos_tmu.c | 120 -
> --
>  1 file changed, 84 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/thermal/samsung/exynos_tmu.c
> b/drivers/thermal/samsung/exynos_tmu.c
> index 2ec8548..b3f0704 100644
> --- a/drivers/thermal/samsung/exynos_tmu.c
> +++ b/drivers/thermal/samsung/exynos_tmu.c
> @@ -282,52 +282,54 @@ static void exynos_report_trigger(struct
> exynos_tmu_data *p)
>   * TMU treats temperature as a mapped temperature code.
>   * The temperature is converted differently depending on the
> calibration type.
>   */
> -static int temp_to_code(struct exynos_tmu_data *data, u8 temp)
> +static int temp_to_code(struct exynos_tmu_data *data, u8 temp, int
> *temp_code)
>  {
> - int temp_code;
> + int ret = 0;
>  
>   switch (data->cal_type) {
>   case TYPE_TWO_POINT_TRIMMING:
> - temp_code = (temp - EXYNOS_FIRST_POINT_TRIM) *
> + *temp_code = (temp - EXYNOS_FIRST_POINT_TRIM) *
>   (data->temp_error2 - data->temp_error1) /
>   (EXYNOS_SECOND_POINT_TRIM -
> EXYNOS_FIRST_POINT_TRIM) +
>   data->temp_error1;
>   break;
>   case TYPE_ONE_POINT_TRIMMING:
> - temp_code = temp + data->temp_error1 -
> EXYNOS_FIRST_POINT_TRIM;
> + *temp_code = temp + data->temp_error1 -
> EXYNOS_FIRST_POINT_TRIM;
>   break;
>   default:
>   WARN_ON(1);
> + ret = -EINVAL;
>   break;
>   }
>  
> - return temp_code;
> + return ret;
>  }
>  
>  /*
>   * Calculate a temperature value from a temperature code.
>   * The unit of the temperature is degree Celsius.
>   */
> -static int code_to_temp(struct exynos_tmu_data *data, u16 temp_code)
> +static int code_to_temp(struct exynos_tmu_data *data, u16 temp_code,
> int *temp)
>  {
> - int temp;
> + int ret = 0;
>  
>   switch (data->cal_type) {
>   case TYPE_TWO_POINT_TRIMMING:
> - temp = (temp_code - data->temp_error1) *
> + *temp = (temp_code - data->temp_error1) *
>   (EXYNOS_SECOND_POINT_TRIM -
> EXYNOS_FIRST_POINT_TRIM) /
>   (data->temp_error2 - data->temp_error1) +
>   EXYNOS_FIRST_POINT_TRIM;
>   break;
>   case TYPE_ONE_POINT_TRIMMING:
> - temp = temp_code - data->temp_error1 +
> EXYNOS_FIRST_POINT_TRIM;
> + *temp = temp_code - data->temp_error1 +
> EXYNOS_FIRST_POINT_TRIM;
>   break;
>   default:
>   WARN_ON(1);
> + ret = -EINVAL;
>   break;
>   }
>  
> - return temp;
> + return ret;
>  }
>  
>  static void sanitize_temp_error(struct exynos_tmu_data *data, u32
> trim_info)
> @@ -352,7 +354,7 @@ static u32 get_th_reg(struct exynos_tmu_data
> *data, u32 thr

Re: [GIT PULL] Thermal management updates for v4.17-rc1

[Zhang Rui]

On 四, 2018-04-12 at 21:08 -0700, Eduardo Valentin wrote:
> Hello,
> 
> On Thu, Apr 12, 2018 at 09:55:19AM -0700, Linus Torvalds wrote:
> > 
> > On Wed, Apr 11, 2018 at 10:08 PM, Zhang Rui 
> > wrote:
> > > 
> > > 
> > > could you please illustrate me what the kconfig & warning is?
> > Just "make allmodconfig" and the warning is about a uninitialized
> > variable.
> > 
> > Line 304 in drivers/thermal/samsung/exynos_tmu.c if my shell
> > history
> > is to be believed.
> > 
> > Linus
> Yeah, this has also passed my local compilation error. Somehow my
> gcc4.9
> is not catching it. Using an older gcc (gcc4.6) does catch it.
> 

I think there are two problems here

1. Actually, this error has been raised by 0-day earlier.
https://marc.info/?l=linux-pm&m=152107340117077&w=2
Don't know why it still goes into thermal-soc tree.

2. After pulled the thermal-soc changes, I also asked 0-day to run
build test, but I didn't get any warning report (email attached), CC
Philip and Shun to look at this issue.

thanks,
rui

bin_6r1piPKmQ.bin
Description: application/mbox

Re: [GIT PULL] Thermal management updates for v4.17-rc1

[Eduardo Valentin]

Hello,

On Thu, Apr 12, 2018 at 09:55:19AM -0700, Linus Torvalds wrote:
> On Wed, Apr 11, 2018 at 10:08 PM, Zhang Rui  wrote:
> >
> > could you please illustrate me what the kconfig & warning is?
> 
> Just "make allmodconfig" and the warning is about a uninitialized variable.
> 
> Line 304 in drivers/thermal/samsung/exynos_tmu.c if my shell history
> is to be believed.
> 
> Linus

Yeah, this has also passed my local compilation error. Somehow my gcc4.9
is not catching it. Using an older gcc (gcc4.6) does catch it.

Anyways, given that the conversion functions are written to cover
for unexpected cal_type, the right way of fixing this is to rewrite
the conversion functions to allow for returning error codes and
adjusting the callers as expected.

Rui, bzolnier, please consider the following fix:

>From 2aaf94f80c0021a21b4122c9f4197acff08ea398 Mon Sep 17 00:00:00 2001
From: Eduardo Valentin 
Date: Thu, 12 Apr 2018 21:00:48 -0700
Subject: [PATCH 1/1] thermal: exynos: fix compilation warning around
 conversion functions

In order to fix the warns:
drivers/thermal/samsung/exynos_tmu.c:931:37: warning: 'temp' may be used 
uninitialized in this function [-Wmaybe-uninitialized]
drivers/thermal/samsung/exynos_tmu.c:304:9: warning: 'temp_code' may be used 
uninitialized in this function [-Wmaybe-uninitialized]

the conversion functions should allow return error codes
and the not mix the converted value with error code.

This patch change the conversion functions to return
error code or success and adjusts the callers accordingly.

Signed-off-by: Eduardo Valentin 
---
 drivers/thermal/samsung/exynos_tmu.c | 120 ---
 1 file changed, 84 insertions(+), 36 deletions(-)

diff --git a/drivers/thermal/samsung/exynos_tmu.c 
b/drivers/thermal/samsung/exynos_tmu.c
index 2ec8548..b3f0704 100644
--- a/drivers/thermal/samsung/exynos_tmu.c
+++ b/drivers/thermal/samsung/exynos_tmu.c
@@ -282,52 +282,54 @@ static void exynos_report_trigger(struct exynos_tmu_data 
*p)
  * TMU treats temperature as a mapped temperature code.
  * The temperature is converted differently depending on the calibration type.
  */
-static int temp_to_code(struct exynos_tmu_data *data, u8 temp)
+static int temp_to_code(struct exynos_tmu_data *data, u8 temp, int *temp_code)
 {
-   int temp_code;
+   int ret = 0;
 
switch (data->cal_type) {
case TYPE_TWO_POINT_TRIMMING:
-   temp_code = (temp - EXYNOS_FIRST_POINT_TRIM) *
+   *temp_code = (temp - EXYNOS_FIRST_POINT_TRIM) *
(data->temp_error2 - data->temp_error1) /
(EXYNOS_SECOND_POINT_TRIM - EXYNOS_FIRST_POINT_TRIM) +
data->temp_error1;
break;
case TYPE_ONE_POINT_TRIMMING:
-   temp_code = temp + data->temp_error1 - EXYNOS_FIRST_POINT_TRIM;
+   *temp_code = temp + data->temp_error1 - EXYNOS_FIRST_POINT_TRIM;
break;
default:
WARN_ON(1);
+   ret = -EINVAL;
break;
}
 
-   return temp_code;
+   return ret;
 }
 
 /*
  * Calculate a temperature value from a temperature code.
  * The unit of the temperature is degree Celsius.
  */
-static int code_to_temp(struct exynos_tmu_data *data, u16 temp_code)
+static int code_to_temp(struct exynos_tmu_data *data, u16 temp_code, int *temp)
 {
-   int temp;
+   int ret = 0;
 
switch (data->cal_type) {
case TYPE_TWO_POINT_TRIMMING:
-   temp = (temp_code - data->temp_error1) *
+   *temp = (temp_code - data->temp_error1) *
(EXYNOS_SECOND_POINT_TRIM - EXYNOS_FIRST_POINT_TRIM) /
(data->temp_error2 - data->temp_error1) +
EXYNOS_FIRST_POINT_TRIM;
break;
case TYPE_ONE_POINT_TRIMMING:
-   temp = temp_code - data->temp_error1 + EXYNOS_FIRST_POINT_TRIM;
+   *temp = temp_code - data->temp_error1 + EXYNOS_FIRST_POINT_TRIM;
break;
default:
WARN_ON(1);
+   ret = -EINVAL;
break;
}
 
-   return temp;
+   return ret;
 }
 
 static void sanitize_temp_error(struct exynos_tmu_data *data, u32 trim_info)
@@ -352,7 +354,7 @@ static u32 get_th_reg(struct exynos_tmu_data *data, u32 
threshold, bool falling)
struct thermal_zone_device *tz = data->tzd;
const struct thermal_trip * const trips =
of_thermal_get_trip_points(tz);
-   unsigned long temp;
+   int temp;
int i;
 
if (!trips) {
@@ -362,6 +364,8 @@ static u32 get_th_reg(struct exynos_tmu_data *data, u32 
threshold, bool falling)
}
 
for (i = 0; i < of_thermal_get_ntrips(tz); i++) {
+   int val, ret;
+
if (trips[i].type == THERMAL_TRIP_CRITICAL)
continue;
 
@@ -371,7 +375,14 @@ static u32 get_t

Re: [GIT PULL] Thermal management updates for v4.17-rc1

[Daniel Lezcano]

On 12/04/2018 18:55, Linus Torvalds wrote:
> On Wed, Apr 11, 2018 at 10:08 PM, Zhang Rui  wrote:
>>
>> could you please illustrate me what the kconfig & warning is?
> 
> Just "make allmodconfig" and the warning is about a uninitialized variable.
> 
> Line 304 in drivers/thermal/samsung/exynos_tmu.c if my shell history
> is to be believed.
> 
> Linus

These couple of warnings were introduced by:

commit 480b5bfc16e17ef51ca1c55bfcebf55db8673ebf
Author: Bartlomiej Zolnierkiewicz 
Date:   Tue Mar 6 15:43:45 2018 +0100

thermal: exynos: remove parsing of samsung, tmu_default_temp_offset
property

Trimming (one point based or two points based) is always used for
the temperature calibration and the default non-trimming code should
never be reached.

Modify temp_to_code() and code_to_temp() accordingly (WARN_ON(1)
in the default cases) and then remove no longer needed parsing of
samsung,tmu_default_temp_offset property.

There should be no functional changes caused by this patch.

Signed-off-by: Bartlomiej Zolnierkiewicz 
Signed-off-by: Eduardo Valentin 


After digging into, there is no obvious fix. It returns effectively an
uninitialized value and the callers are assuming the value is always
correct, so it is also not possible to simply return an error.


-- 
  Linaro.org │ Open source software for ARM SoCs

Follow Linaro:   Facebook |
 Twitter |
 Blog

Re: [GIT PULL] Thermal management updates for v4.17-rc1

[Linus Torvalds]

On Wed, Apr 11, 2018 at 10:08 PM, Zhang Rui  wrote:
>
> could you please illustrate me what the kconfig & warning is?

Just "make allmodconfig" and the warning is about a uninitialized variable.

Line 304 in drivers/thermal/samsung/exynos_tmu.c if my shell history
is to be believed.

Linus

Re: [GIT PULL] Thermal management updates for v4.17-rc1

[Zhang Rui]

Hi, Linus,

On 三, 2018-04-11 at 17:01 -0700, Linus Torvalds wrote:
> On Wed, Apr 11, 2018 at 1:41 AM, Zhang Rui 
> wrote:
> > 
> > 
> > Please pull from
> >   git://git.kernel.org/pub/scm/linux/kernel/git/rzhang/linux.git
> > next
> Pulled, and then immediately unpulled again.
> 
> The code causes new compiler warnings, and the warnings are valid.
> 
> If people don't care enough about their code to even check the
> warnings, I'm not going to waste one second pulling the resulting
> garbage. It's that simple.
> 

I'm really sorry for this.
could you please illustrate me what the kconfig & warning is?
I didn't
get such warnings from 0-day.

thanks,
rui

Re: [GIT PULL] Thermal management updates for v4.17-rc1

[Linus Torvalds]

On Wed, Apr 11, 2018 at 1:41 AM, Zhang Rui  wrote:
>
> Please pull from
>   git://git.kernel.org/pub/scm/linux/kernel/git/rzhang/linux.git next

Pulled, and then immediately unpulled again.

The code causes new compiler warnings, and the warnings are valid.

If people don't care enough about their code to even check the
warnings, I'm not going to waste one second pulling the resulting
garbage. It's that simple.

Linus

[GIT PULL] Thermal management updates for v4.17-rc1

[Zhang Rui]

Hi, Linus,

Please pull from
  git://git.kernel.org/pub/scm/linux/kernel/git/rzhang/linux.git next

to receive the latest Thermal Management updates for v4.17-rc1 with
top-most commit f8837aac36cdc7430422cd65f4466071b42654bb:

  Merge branches 'thermal-core' and 'thermal-soc' into next (2018-04-02 
21:49:31 +0800)

on top of commit 0c8efd610b58cb23cefdfa12015799079aef94ae:

  Linux 4.16-rc5 (2018-03-11 17:25:09 -0700)

Specifics:

- Fix race condition in imx_thermal_probe(). (Mikhail Lappo)

- Add cooling device's statistics in sysfs. (Viresh Kumar)

- add support for i.MX7 thermal sensor in imx_thermal driver. (Anson
Huang)

- add support for MT7622 SoC in mtk_thermal driver. (Sean Wang)

- Remove unused min/max cpu cooling DT property. (Viresh Kumar).

- A series of fixes on exynos driver. (Bartlomiej Zolnierkiewicz,
Maciej Purski, Marek Szyprowski)

thanks,
rui




Anson Huang (1):
  thermal: imx: add i.MX7 thermal sensor support

Bartlomiej Zolnierkiewicz (10):
  thermal: exynos: remove unused "type" field from struct
exynos_tmu_platform_data
  thermal: exynos: remove parsing of samsung,
tmu_default_temp_offset property
  thermal: exynos: remove parsing of samsung, tmu_[first,
second]_point_trim properties
  thermal: exynos: remove parsing of samsung, tmu_noise_cancel_mode
property
  thermal: exynos: remove parsing of samsung, tmu[_min,
_max]_efuse_value properties
  thermal: exynos: remove parsing of samsung, tmu_reference_voltage
property
  thermal: exynos: remove parsing of samsung,tmu_gain property
  thermal: exynos: remove parsing of samsung, tmu_cal_type property
  thermal: exynos: remove separate exynos_tmu.h header file
  dt-bindings: thermal: remove no longer needed samsung thermal
properties

Maciej Purski (1):
  thermal: exynos: Read soc_type from match data

Marek Szyprowski (2):
  thermal: exynos: Reading temperature makes sense only when TMU is
turned on
  thermal: exynos: Propagate error value from tmu_read()

Mikhail Lappo (1):
  thermal: imx: Fix race condition in imx_thermal_probe()

Sean Wang (2):
  dt-bindings: thermal: add binding for MT7622 SoC
  thermal: mediatek: add support for MT7622 SoC

Viresh Kumar (2):
  dt-bindings: thermal: Remove "cooling-{min|max}-level" properties
  thermal: Add cooling device's statistics in sysfs

Zhang Rui (2):
  Merge branch 'linus' of git://git.kernel.org/.../evalenti/linux-
soc-thermal into thermal-soc
  Merge branches 'thermal-core' and 'thermal-soc' into next

 .../devicetree/bindings/thermal/exynos-thermal.txt |  23 +-
 .../devicetree/bindings/thermal/imx-thermal.txt|   9 +-
 .../bindings/thermal/mediatek-thermal.txt  |   1 +
 .../devicetree/bindings/thermal/thermal.txt|  16 +-
 Documentation/thermal/sysfs-api.txt|  31 +++
 drivers/thermal/Kconfig|   7 +
 drivers/thermal/imx_thermal.c  | 301
-
 drivers/thermal/mtk_thermal.c  |  35 +++
 drivers/thermal/samsung/exynos_tmu.c   | 268 +--
---
 drivers/thermal/samsung/exynos_tmu.h   |  75 -
 drivers/thermal/thermal_core.c |   3 +-
 drivers/thermal/thermal_core.h |  10 +
 drivers/thermal/thermal_helpers.c  |   5 +-
 drivers/thermal/thermal_sysfs.c| 225
+++
 include/linux/thermal.h|   1 +
 15 files changed, 706 insertions(+), 304 deletions(-)
 delete mode 100644 drivers/thermal/samsung/exynos_tmu.h