[V9] thermal: bcm2835: add thermal driver for bcm2835 soc

Commit Message

Martin Sperl Jan. 7, 2017, 4:55 p.m. UTC

From: Martin Sperl <[e-mail]>

Add basic thermal driver for bcm2835 SOC.

This driver currently relies on the firmware setting up the
tsense HW block and does not set it up itself.

Signed-off-by: Martin Sperl <[e-mail]>
Acked-by: Eric Anholt <[e-mail]>
Acked-by: Stefan Wahren <[e-mail]>

---
 drivers/thermal/Kconfig           |   8 +
 drivers/thermal/Makefile          |   1 +
 drivers/thermal/bcm2835_thermal.c | 354 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 363 insertions(+)
 create mode 100644 drivers/thermal/bcm2835_thermal.c

ChangeLog:
 V1 -> V2: added specific settings depending on compatiblity
	   added trip point based on register
	   setting up ctrl-register if HW is not enabled by firmware
	     as per recommendation of Eric (untested)
	   check that clock frequency is in range
	     (1.9 - 5MHz - as per comment in clk-bcm2835.c)
 V2 -> V4: moved back to thermal (not using bcm sub-directory)
       	   set polling interval to 1second (was 0ms, so interrupt driven)
 V5 -> V6: added correct depends in KConfig
	   removed defined default for RESET_DELAY
	   removed obvious comments
	   clarify HW setup comments if not set up by FW already
	   move clk_prepare_enable to an earlier stage and add error handling
	   clarify warning when TS-clock runs out of recommended range
	   clk_disable_unprepare added in bcm2835_thermal_remove
	   added comment on recommended temperature ranges for SOC
 V6 -> V7: removed depends on ARCH_BCM2836 || ARCH_BCM2837 in Kconfig
 V7 -> V8: rebased
 V8 -> V9: moved to use the thermal framework offset and slope in
	   thermal_zone_parameters as per request

A few additional notes on this latest patch:
* all the other portions of the patch (dt, bindings, defconf) have already
  been merged into 4.10.0-rc2 - this only leaves the driver itself which
  is now just a single patch to get applied.
* the driver has been moved to use thermal_zone_parameters->offset and slope
  making use of the thermal_zone_get_offset and thermal_zone_get_slope
  methods where relevant
  Note that we cannot use those during initial setup of the HW - this code
  section is typically not used anyway, as the FW sets up the device already.
  This so modified patch actually increases code size by 14 lines:
    drivers/thermal/bcm2835_thermal.c | 42 ++++++++++++++++++++++++++-------------
    1 file changed, 28 insertions(+), 14 deletions(-)
* /sys/class/thermal/*/ contains now slope and offset files
  and those expose the correct values
* for some reason on module load the kernel emits now the message:
    (NULL device *): hwmon_device_register() is deprecated. Please convert the driver to use hwmon_device_register_with_info().
  But this seems framework specific and needs to get addressed there.

--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Eduardo Valentin Jan. 20, 2017, 4:14 a.m. UTC | #1

Hello Martin,

On Sat, Jan 07, 2017 at 04:55:45PM +0000, [e-mail] wrote:
> From: Martin Sperl <[e-mail]>
> 
> Add basic thermal driver for bcm2835 SOC.
> 
> This driver currently relies on the firmware setting up the
> tsense HW block and does not set it up itself.
> 
> Signed-off-by: Martin Sperl <[e-mail]>
> Acked-by: Eric Anholt <[e-mail]>
> Acked-by: Stefan Wahren <[e-mail]>
> 

<cut>

> +
> +static const struct of_device_id bcm2835_thermal_of_match_table[] = {
> +	{
> +		.compatible = "brcm,bcm2835-thermal",
> +		.data = &(struct bcm2835_thermal_info) {
> +			.offset = 407000,
> +			.slope = -538,
> +			.trip_temp = 80000
> +		}
> +	},
> +	{
> +		.compatible = "brcm,bcm2836-thermal",
> +		.data = &(struct bcm2835_thermal_info) {
> +			.offset = 407000,
> +			.slope = -538,
> +			.trip_temp = 80000
> +		}
> +	},
> +	{
> +		.compatible = "brcm,bcm2837-thermal",
> +		.data = &(struct bcm2835_thermal_info) {
> +			/* the bcm2837 needs adjustment of +5C */
> +			.offset = 407000 + 5000,
> +			.slope = -538,
> +			.trip_temp = 80000
> +		}
> +	},
> +	{},

Just for the same of clarification, is there anything preventing this
driver of using of-thermal API? the above data (slope, offset, and
trip_temps) would be in DT the place where they are supposed to be,
instead of code.

> +};
> +MODULE_DEVICE_TABLE(of, bcm2835_thermal_of_match_table);
> +
> +static struct platform_driver bcm2835_thermal_driver = {
> +	.probe = bcm2835_thermal_probe,
> +	.remove = bcm2835_thermal_remove,
> +	.driver = {
> +		.name = "bcm2835_thermal",
> +		.of_match_table = bcm2835_thermal_of_match_table,
> +	},
> +};
> +module_platform_driver(bcm2835_thermal_driver);
> +
> +MODULE_AUTHOR("Martin Sperl");
> +MODULE_DESCRIPTION("Thermal driver for bcm2835 chip");
> +MODULE_LICENSE("GPL");
> --
> 2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Eduardo Valentin Jan. 20, 2017, 4:23 a.m. UTC | #2

On Thu, Jan 19, 2017 at 08:14:02PM -0800, Eduardo Valentin wrote:
> Hello Martin,
> 
> On Sat, Jan 07, 2017 at 04:55:45PM +0000, [e-mail] wrote:
> > From: Martin Sperl <[e-mail]>
> > 
> > Add basic thermal driver for bcm2835 SOC.
> > 
> > This driver currently relies on the firmware setting up the
> > tsense HW block and does not set it up itself.
> > 
> > Signed-off-by: Martin Sperl <[e-mail]>
> > Acked-by: Eric Anholt <[e-mail]>
> > Acked-by: Stefan Wahren <[e-mail]>
> > 
> 
> <cut>


Also, I am getting this warn from sparse:
drivers/thermal/bcm2835_thermal.c:110:16: warning: cast truncates bits
from constant value (3ffffffffff00 becomes ffffff00)
drivers/thermal/bcm2835_thermal.c:134:16: warning: cast truncates bits
from constant value (3ffffffffff becomes ffffffff)

Have you seen this?
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Martin Sperl Jan. 20, 2017, 7:54 a.m. UTC | #3

> On 20.01.2017, at 05:14, Eduardo Valentin <[e-mail]> wrote:
> 
> Hello Martin,
> 
> On Sat, Jan 07, 2017 at 04:55:45PM +0000, [e-mail] wrote:
>> From: Martin Sperl <[e-mail]>
>> 
>> Add basic thermal driver for bcm2835 SOC.
>> 
>> This driver currently relies on the firmware setting up the
>> tsense HW block and does not set it up itself.
>> 
>> Signed-off-by: Martin Sperl <[e-mail]>
>> Acked-by: Eric Anholt <[e-mail]>
>> Acked-by: Stefan Wahren <[e-mail]>
>> 
> 
> <cut>
> 
>> +
>> +static const struct of_device_id bcm2835_thermal_of_match_table[] = {
>> +	{
>> +		.compatible = "brcm,bcm2835-thermal",
>> +		.data = &(struct bcm2835_thermal_info) {
>> +			.offset = 407000,
>> +			.slope = -538,
>> +			.trip_temp = 80000
>> +		}
>> +	},
>> +	{
>> +		.compatible = "brcm,bcm2836-thermal",
>> +		.data = &(struct bcm2835_thermal_info) {
>> +			.offset = 407000,
>> +			.slope = -538,
>> +			.trip_temp = 80000
>> +		}
>> +	},
>> +	{
>> +		.compatible = "brcm,bcm2837-thermal",
>> +		.data = &(struct bcm2835_thermal_info) {
>> +			/* the bcm2837 needs adjustment of +5C */
>> +			.offset = 407000 + 5000,
>> +			.slope = -538,
>> +			.trip_temp = 80000
>> +		}
>> +	},
>> +	{},
> 
> Just for the same of clarification, is there anything preventing this
> driver of using of-thermal API? the above data (slope, offset, and
> trip_temps) would be in DT the place where they are supposed to be,
> instead of code.
> 

As the DT changes, that only define compatible strings, have already gone
in without any such properties set, we need to define defaults for the 
slope/offset and trip_temp values.

I guess (for newer SOC) you still can use the values in the DT,
as (I guess) these are parsed and set in thermal_zone_device_register
after the defaults are set in thermal_zone_params.

Martin
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Martin Sperl Jan. 20, 2017, 8:43 a.m. UTC | #4

> On 20.01.2017, at 05:23, Eduardo Valentin <[e-mail]> wrote:
> 
> On Thu, Jan 19, 2017 at 08:14:02PM -0800, Eduardo Valentin wrote:
>> Hello Martin,
>> 
>> On Sat, Jan 07, 2017 at 04:55:45PM +0000, [e-mail] wrote:
>>> From: Martin Sperl <[e-mail]>
>>> 
>>> Add basic thermal driver for bcm2835 SOC.
>>> 
>>> This driver currently relies on the firmware setting up the
>>> tsense HW block and does not set it up itself.
>>> 
>>> Signed-off-by: Martin Sperl <[e-mail]>
>>> Acked-by: Eric Anholt <[e-mail]>
>>> Acked-by: Stefan Wahren <[e-mail]>
>>> 
>> 
>> <cut>
> 
> 
> Also, I am getting this warn from sparse:
> drivers/thermal/bcm2835_thermal.c:110:16: warning: cast truncates bits
> from constant value (3ffffffffff00 becomes ffffff00)
> drivers/thermal/bcm2835_thermal.c:134:16: warning: cast truncates bits
> from constant value (3ffffffffff becomes ffffffff)
> 
> Have you seen this?

No, I have not checked sparse.

These values are defined via GENMASK on line 47 and 57 respectively
and should actually compute to the following values:
  for line 110 (line 47 has the define):
    GENMASK(BCM2835_TS_TSENSCTL_THOLD_BITS +     \
            BCM2835_TS_TSENSCTL_THOLD_SHIFT - 1, \
            BCM2835_TS_TSENSCTL_THOLD_SHIFT)
    = GENMASK(10 + 8 - 1, 8) 
    = GENMASK(17, 8)
    = (((~0UL) << (8)) & (~0UL >> (32 - 1 - (10 + 8 - 1))))
    = 0x3ff00
  for line 134 (line 57 has the define):
    GENMASK(BCM2835_TS_TSENSCTL_THOLD_BITS +     \
            BCM2835_TS_TSENSCTL_THOLD_SHIFT - 1, \
            BCM2835_TS_TSENSCTL_THOLD_SHIFT)
    = GENMASK(10 + 0 - 1, 0)
    = GENMASK(9, 0)
    = (((~0UL) << (0)) & (~0UL >> (32 - 1 - (10 + 0 - 1))))
    = 0x003ff

Note that the preprocessor expansions have been verified by
looking at the preprocessed driver source
(drivers/thermal/bcm2835_thermal.i)

I wonder why sparse is computing these GENMASK values as:
0x3ffffffffff00 and 0x3ffffffffff

Martin
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Eduardo Valentin Jan. 24, 2017, 9:26 a.m. UTC | #5

Hello Martin,

On Fri, Jan 20, 2017 at 09:43:02AM +0100, [e-mail] wrote:
> 
> > On 20.01.2017, at 05:23, Eduardo Valentin <[e-mail]> wrote:
> > 
> > On Thu, Jan 19, 2017 at 08:14:02PM -0800, Eduardo Valentin wrote:
> >> Hello Martin,
> >> 
> >> On Sat, Jan 07, 2017 at 04:55:45PM +0000, [e-mail] wrote:
> >>> From: Martin Sperl <[e-mail]>
> >>> 
> >>> Add basic thermal driver for bcm2835 SOC.
> >>> 
> >>> This driver currently relies on the firmware setting up the
> >>> tsense HW block and does not set it up itself.
> >>> 
> >>> Signed-off-by: Martin Sperl <[e-mail]>
> >>> Acked-by: Eric Anholt <[e-mail]>
> >>> Acked-by: Stefan Wahren <[e-mail]>
> >>> 
> >> 
> >> <cut>
> > 
> > 
> > Also, I am getting this warn from sparse:
> > drivers/thermal/bcm2835_thermal.c:110:16: warning: cast truncates bits
> > from constant value (3ffffffffff00 becomes ffffff00)
> > drivers/thermal/bcm2835_thermal.c:134:16: warning: cast truncates bits
> > from constant value (3ffffffffff becomes ffffffff)
> > 
> > Have you seen this?
> 
> No, I have not checked sparse.
> 
> These values are defined via GENMASK on line 47 and 57 respectively
> and should actually compute to the following values:
>   for line 110 (line 47 has the define):
>     GENMASK(BCM2835_TS_TSENSCTL_THOLD_BITS +     \
>             BCM2835_TS_TSENSCTL_THOLD_SHIFT - 1, \
>             BCM2835_TS_TSENSCTL_THOLD_SHIFT)
>     = GENMASK(10 + 8 - 1, 8) 
>     = GENMASK(17, 8)
>     = (((~0UL) << (8)) & (~0UL >> (32 - 1 - (10 + 8 - 1))))
>     = 0x3ff00
>   for line 134 (line 57 has the define):
>     GENMASK(BCM2835_TS_TSENSCTL_THOLD_BITS +     \
>             BCM2835_TS_TSENSCTL_THOLD_SHIFT - 1, \
>             BCM2835_TS_TSENSCTL_THOLD_SHIFT)
>     = GENMASK(10 + 0 - 1, 0)
>     = GENMASK(9, 0)
>     = (((~0UL) << (0)) & (~0UL >> (32 - 1 - (10 + 0 - 1))))
>     = 0x003ff
> 
> Note that the preprocessor expansions have been verified by
> looking at the preprocessed driver source
> (drivers/thermal/bcm2835_thermal.i)

OK then.

> 
> I wonder why sparse is computing these GENMASK values as:
> 0x3ffffffffff00 and 0x3ffffffffff

In the case you can confirm that the values are correct, I believe this
could be a false positive report on sparse, in this case.

> 
> Martin
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Eduardo Valentin Jan. 24, 2017, 9:31 a.m. UTC | #6

Hello Martin,

On Fri, Jan 20, 2017 at 08:54:07AM +0100, [e-mail] wrote:
> 
> > On 20.01.2017, at 05:14, Eduardo Valentin <[e-mail]> wrote:
> > 
> > Hello Martin,
> > 
> > On Sat, Jan 07, 2017 at 04:55:45PM +0000, [e-mail] wrote:
> >> From: Martin Sperl <[e-mail]>
> >> 
> >> Add basic thermal driver for bcm2835 SOC.
> >> 
> >> This driver currently relies on the firmware setting up the
> >> tsense HW block and does not set it up itself.
> >> 
> >> Signed-off-by: Martin Sperl <[e-mail]>
> >> Acked-by: Eric Anholt <[e-mail]>
> >> Acked-by: Stefan Wahren <[e-mail]>
> >> 
> > 
> > <cut>
> > 
> >> +
> >> +static const struct of_device_id bcm2835_thermal_of_match_table[] = {
> >> +	{
> >> +		.compatible = "brcm,bcm2835-thermal",
> >> +		.data = &(struct bcm2835_thermal_info) {
> >> +			.offset = 407000,
> >> +			.slope = -538,
> >> +			.trip_temp = 80000
> >> +		}
> >> +	},
> >> +	{
> >> +		.compatible = "brcm,bcm2836-thermal",
> >> +		.data = &(struct bcm2835_thermal_info) {
> >> +			.offset = 407000,
> >> +			.slope = -538,
> >> +			.trip_temp = 80000
> >> +		}
> >> +	},
> >> +	{
> >> +		.compatible = "brcm,bcm2837-thermal",
> >> +		.data = &(struct bcm2835_thermal_info) {
> >> +			/* the bcm2837 needs adjustment of +5C */
> >> +			.offset = 407000 + 5000,
> >> +			.slope = -538,
> >> +			.trip_temp = 80000
> >> +		}
> >> +	},
> >> +	{},
> > 
> > Just for the same of clarification, is there anything preventing this
> > driver of using of-thermal API? the above data (slope, offset, and
> > trip_temps) would be in DT the place where they are supposed to be,
> > instead of code.
> > 
> 
> As the DT changes, that only define compatible strings, have already gone
> in without any such properties set, we need to define defaults for the 
> slope/offset and trip_temp values.
> 

These properties won't go into the same node you are referring to. They
go into the thermal-zone node you would create, which would then refer
to the node you referred (already merged). Therefore, I do not see
anything blocking a proper of-thermal usage to cover for the above data.

> I guess (for newer SOC) you still can use the values in the DT,
> as (I guess) these are parsed and set in thermal_zone_device_register
> after the defaults are set in thermal_zone_params.

Not sure what you meant here, but these values, when correctly used in
DT, they would come as part of the thermal_zone_params and in the
thermal trips of the thermal zones, as the of-thermal code with already
deal with those for you.

Please have a look at:
a. Documentation/devicetree/bindings/thermal/thermal.txt
b. drivers/thermal/of-thermal.c

And let me know if you see anything that would prevent this driver of
using the correct API to describe hardware data with DT.

BR,

> 
> Martin
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Martin Sperl Jan. 24, 2017, 9:37 a.m. UTC | #7

> On 24.01.2017, at 10:26, Eduardo Valentin <[e-mail]> wrote:
> 
> Hello Martin,
> 
> On Fri, Jan 20, 2017 at 09:43:02AM +0100, [e-mail] wrote:
>> 
>>> On 20.01.2017, at 05:23, Eduardo Valentin <[e-mail]> wrote:
>>> 
>>> On Thu, Jan 19, 2017 at 08:14:02PM -0800, Eduardo Valentin wrote:
>>>> Hello Martin,
>>>> 
>>>> On Sat, Jan 07, 2017 at 04:55:45PM +0000, [e-mail] wrote:
>>>>> From: Martin Sperl <[e-mail]>
>>>>> 
>>>>> Add basic thermal driver for bcm2835 SOC.
>>>>> 
>>>>> This driver currently relies on the firmware setting up the
>>>>> tsense HW block and does not set it up itself.
>>>>> 
>>>>> Signed-off-by: Martin Sperl <[e-mail]>
>>>>> Acked-by: Eric Anholt <[e-mail]>
>>>>> Acked-by: Stefan Wahren <[e-mail]>
>>>>> 
>>>> 
>>>> <cut>
>>> 
>>> 
>>> Also, I am getting this warn from sparse:
>>> drivers/thermal/bcm2835_thermal.c:110:16: warning: cast truncates bits
>>> from constant value (3ffffffffff00 becomes ffffff00)
>>> drivers/thermal/bcm2835_thermal.c:134:16: warning: cast truncates bits
>>> from constant value (3ffffffffff becomes ffffffff)
>>> 
>>> Have you seen this?
>> 
>> No, I have not checked sparse.
>> 
>> These values are defined via GENMASK on line 47 and 57 respectively
>> and should actually compute to the following values:
>>  for line 110 (line 47 has the define):
>>    GENMASK(BCM2835_TS_TSENSCTL_THOLD_BITS +     \
>>            BCM2835_TS_TSENSCTL_THOLD_SHIFT - 1, \
>>            BCM2835_TS_TSENSCTL_THOLD_SHIFT)
>>    = GENMASK(10 + 8 - 1, 8) 
>>    = GENMASK(17, 8)
>>    = (((~0UL) << (8)) & (~0UL >> (32 - 1 - (10 + 8 - 1))))
>>    = 0x3ff00
>>  for line 134 (line 57 has the define):
>>    GENMASK(BCM2835_TS_TSENSCTL_THOLD_BITS +     \
>>            BCM2835_TS_TSENSCTL_THOLD_SHIFT - 1, \
>>            BCM2835_TS_TSENSCTL_THOLD_SHIFT)
>>    = GENMASK(10 + 0 - 1, 0)
>>    = GENMASK(9, 0)
>>    = (((~0UL) << (0)) & (~0UL >> (32 - 1 - (10 + 0 - 1))))
>>    = 0x003ff
>> 
>> Note that the preprocessor expansions have been verified by
>> looking at the preprocessed driver source
>> (drivers/thermal/bcm2835_thermal.i)
> 
> OK then.
> 
>> 
>> I wonder why sparse is computing these GENMASK values as:
>> 0x3ffffffffff00 and 0x3ffffffffff
> 
> In the case you can confirm that the values are correct, I believe this
> could be a false positive report on sparse, in this case.

The correct values are the ones in my email: 0x3ff00 and 0x003ff

The ones reported by sparse (0x3ffffffffff00 and 0x3ffffffffff) are 
not calculated correctly - to me it looks as if it is possibly some
sort of 64 bit issue of sparse in conjunction with the generic macro
GENMASK.


--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Martin Sperl Jan. 24, 2017, 9:52 a.m. UTC | #8

> On 24.01.2017, at 10:31, Eduardo Valentin <[e-mail]> wrote:
> 
> Hello Martin,
> 
> On Fri, Jan 20, 2017 at 08:54:07AM +0100, [e-mail] wrote:
>> 
>>> On 20.01.2017, at 05:14, Eduardo Valentin <[e-mail]> wrote:
>>> 
>>> Hello Martin,
>>> 
>>> On Sat, Jan 07, 2017 at 04:55:45PM +0000, [e-mail] wrote:
>>>> From: Martin Sperl <[e-mail]>
>>>> 
>>>> Add basic thermal driver for bcm2835 SOC.
>>>> 
>>>> This driver currently relies on the firmware setting up the
>>>> tsense HW block and does not set it up itself.
>>>> 
>>>> Signed-off-by: Martin Sperl <[e-mail]>
>>>> Acked-by: Eric Anholt <[e-mail]>
>>>> Acked-by: Stefan Wahren <[e-mail]>
>>>> 
>>> 
>>> <cut>
>>> 
>>>> +
>>>> +static const struct of_device_id bcm2835_thermal_of_match_table[] = {
>>>> +	{
>>>> +		.compatible = "brcm,bcm2835-thermal",
>>>> +		.data = &(struct bcm2835_thermal_info) {
>>>> +			.offset = 407000,
>>>> +			.slope = -538,
>>>> +			.trip_temp = 80000
>>>> +		}
>>>> +	},
>>>> +	{
>>>> +		.compatible = "brcm,bcm2836-thermal",
>>>> +		.data = &(struct bcm2835_thermal_info) {
>>>> +			.offset = 407000,
>>>> +			.slope = -538,
>>>> +			.trip_temp = 80000
>>>> +		}
>>>> +	},
>>>> +	{
>>>> +		.compatible = "brcm,bcm2837-thermal",
>>>> +		.data = &(struct bcm2835_thermal_info) {
>>>> +			/* the bcm2837 needs adjustment of +5C */
>>>> +			.offset = 407000 + 5000,
>>>> +			.slope = -538,
>>>> +			.trip_temp = 80000
>>>> +		}
>>>> +	},
>>>> +	{},
>>> 
>>> Just for the same of clarification, is there anything preventing this
>>> driver of using of-thermal API? the above data (slope, offset, and
>>> trip_temps) would be in DT the place where they are supposed to be,
>>> instead of code.
>>> 
>> 
>> As the DT changes, that only define compatible strings, have already gone
>> in without any such properties set, we need to define defaults for the 
>> slope/offset and trip_temp values.
>> 
> 
> These properties won't go into the same node you are referring to. They
> go into the thermal-zone node you would create, which would then refer
> to the node you referred (already merged). Therefore, I do not see
> anything blocking a proper of-thermal usage to cover for the above data.
> 
>> I guess (for newer SOC) you still can use the values in the DT,
>> as (I guess) these are parsed and set in thermal_zone_device_register
>> after the defaults are set in thermal_zone_params.
> 
> Not sure what you meant here, but these values, when correctly used in
> DT, they would come as part of the thermal_zone_params and in the
> thermal trips of the thermal zones, as the of-thermal code with already
> deal with those for you.
> 
> Please have a look at:
> a. Documentation/devicetree/bindings/thermal/thermal.txt
> b. drivers/thermal/of-thermal.c
> 
> And let me know if you see anything that would prevent this driver of
> using the correct API to describe hardware data with DT.


I guess you miss my point:
The argument is that we have DT in the 4.10.0-rc2 kernel right now that
look like this:
                thermal@7e212000 {
                        compatible = "brcm,bcm2835-thermal";
                        clocks = <0x6 0x1b>;
                        status = "okay";
                        reg = <0x7e212000 0x8>;
                }
so we still need to be compatible with those without any extra defines.

Hence we need to define those slopes and offsets in the driver itself
to stay compatible with those older device-trees.

As for if it works with the framework - I have to admit I do not
have the slightest clue - it looks way to complicated for the soc right
now.

As a note: afaiu the trip_temp register is the temperature at which the
soc will reboot on its own - similar to a watchdog, but for temperatures.
(main reason for the assumption is because there is no interrupt that
can get assigned a handler to catch this situation).

Martin


--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Eduardo Valentin Feb. 2, 2017, 4:29 a.m. UTC | #9

Hello Martin,

On Tue, Jan 24, 2017 at 10:52:43AM +0100, [e-mail] wrote:
> 
> > On 24.01.2017, at 10:31, Eduardo Valentin <[e-mail]> wrote:
> > 
> > Hello Martin,
> > 
> > On Fri, Jan 20, 2017 at 08:54:07AM +0100, [e-mail] wrote:
> >> 
> >>> On 20.01.2017, at 05:14, Eduardo Valentin <[e-mail]> wrote:
> >>> 
> >>> Hello Martin,
> >>> 
> >>> On Sat, Jan 07, 2017 at 04:55:45PM +0000, [e-mail] wrote:
> >>>> From: Martin Sperl <[e-mail]>
> >>>> 
> >>>> Add basic thermal driver for bcm2835 SOC.
> >>>> 
> >>>> This driver currently relies on the firmware setting up the
> >>>> tsense HW block and does not set it up itself.
> >>>> 
> >>>> Signed-off-by: Martin Sperl <[e-mail]>
> >>>> Acked-by: Eric Anholt <[e-mail]>
> >>>> Acked-by: Stefan Wahren <[e-mail]>
> >>>> 
> >>> 
> >>> <cut>
> >>> 
> >>>> +
> >>>> +static const struct of_device_id bcm2835_thermal_of_match_table[] = {
> >>>> +	{
> >>>> +		.compatible = "brcm,bcm2835-thermal",
> >>>> +		.data = &(struct bcm2835_thermal_info) {
> >>>> +			.offset = 407000,
> >>>> +			.slope = -538,
> >>>> +			.trip_temp = 80000
> >>>> +		}
> >>>> +	},
> >>>> +	{
> >>>> +		.compatible = "brcm,bcm2836-thermal",
> >>>> +		.data = &(struct bcm2835_thermal_info) {
> >>>> +			.offset = 407000,
> >>>> +			.slope = -538,
> >>>> +			.trip_temp = 80000
> >>>> +		}
> >>>> +	},
> >>>> +	{
> >>>> +		.compatible = "brcm,bcm2837-thermal",
> >>>> +		.data = &(struct bcm2835_thermal_info) {
> >>>> +			/* the bcm2837 needs adjustment of +5C */
> >>>> +			.offset = 407000 + 5000,
> >>>> +			.slope = -538,
> >>>> +			.trip_temp = 80000
> >>>> +		}
> >>>> +	},
> >>>> +	{},
> >>> 
> >>> Just for the same of clarification, is there anything preventing this
> >>> driver of using of-thermal API? the above data (slope, offset, and
> >>> trip_temps) would be in DT the place where they are supposed to be,
> >>> instead of code.
> >>> 
> >> 
> >> As the DT changes, that only define compatible strings, have already gone
> >> in without any such properties set, we need to define defaults for the 
> >> slope/offset and trip_temp values.
> >> 
> > 
> > These properties won't go into the same node you are referring to. They
> > go into the thermal-zone node you would create, which would then refer
> > to the node you referred (already merged). Therefore, I do not see
> > anything blocking a proper of-thermal usage to cover for the above data.
> > 
> >> I guess (for newer SOC) you still can use the values in the DT,
> >> as (I guess) these are parsed and set in thermal_zone_device_register
> >> after the defaults are set in thermal_zone_params.
> > 
> > Not sure what you meant here, but these values, when correctly used in
> > DT, they would come as part of the thermal_zone_params and in the
> > thermal trips of the thermal zones, as the of-thermal code with already
> > deal with those for you.
> > 
> > Please have a look at:
> > a. Documentation/devicetree/bindings/thermal/thermal.txt
> > b. drivers/thermal/of-thermal.c
> > 
> > And let me know if you see anything that would prevent this driver of
> > using the correct API to describe hardware data with DT.
> 
> 
> I guess you miss my point:

Maybe you missed, or did not read the doc I pointed you...

> The argument is that we have DT in the 4.10.0-rc2 kernel right now that
> look like this:
>                 thermal@7e212000 {
>                         compatible = "brcm,bcm2835-thermal";
>                         clocks = <0x6 0x1b>;
>                         status = "okay";
>                         reg = <0x7e212000 0x8>;
>                 }
> so we still need to be compatible with those without any extra defines.

Yes, but the above DT entry will still be valid if you add the correct
of-thermal support. In fact, you would add in your DTS a thermal-zones
node, and in one of the defined zone, you would then reference the node
you already got into mainline. Below is a copy of the doc I mentioned
before:


#include <dt-bindings/thermal/thermal.h>

ocp {
	...
	/*
	 * A simple IC with several bandgap temperature sensors.
	 */
	bandgap0: bandgap@0x0000ED00 {
		...
		#thermal-sensor-cells = <1>;
	};
};

thermal-zones {
	cpu_thermal: cpu-thermal {
		polling-delay-passive = <250>; /* milliseconds */
		polling-delay = <1000>; /* milliseconds */

				/* sensor       ID */
		thermal-sensors = <&bandgap0     0>;

		trips {
			/* each zone within the SoC may have its own trips */
			cpu_alert: cpu-alert {
				temperature = <100000>; /* millicelsius */
				hysteresis = <2000>; /* millicelsius */
				type = "passive";
			};
			cpu_crit: cpu-crit {
				temperature = <125000>; /* millicelsius */
				hysteresis = <2000>; /* millicelsius */
				type = "critical";
			};
		};

		cooling-maps {
			/* each zone within the SoC may have its own cooling */
			...
		};
	};

> 
> Hence we need to define those slopes and offsets in the driver itself
> to stay compatible with those older device-trees.

not really, as long as we do not merge the driver with the missing
of-thermal support, I see no need to have both supports in your driver,
i.e., if we start correct for the beggining there is no need to have
offsets and slopes data in your driver code.

> 
> As for if it works with the framework - I have to admit I do not
> have the slightest clue - it looks way to complicated for the soc right
> now.

Well, there is the documentation I mentioned, which several other
drivers used as base for their support. You can also look at other
thermal zones already defined in the existing DTS(I)'s.

> 
> As a note: afaiu the trip_temp register is the temperature at which the
> soc will reboot on its own - similar to a watchdog, but for temperatures.
> (main reason for the assumption is because there is no interrupt that
> can get assigned a handler to catch this situation).
> 

OK. But that does not prevent you to have other trips so your running
system can act before the shutdown trip is crossed.

> Martin
> 
> 


BR,

Eduardo Valentin
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Martin Sperl Feb. 4, 2017, 8:35 a.m. UTC | #10

> On 02.02.2017, at 05:29, Eduardo Valentin <[e-mail]> wrote:
> 
> Hello Martin,
> 
> On Tue, Jan 24, 2017 at 10:52:43AM +0100, [e-mail] wrote:
>> 
>>> On 24.01.2017, at 10:31, Eduardo Valentin <[e-mail]> wrote:
>>> 
>>> Hello Martin,
>>> 
>>> On Fri, Jan 20, 2017 at 08:54:07AM +0100, [e-mail] wrote:
>>>> 
>>>>> On 20.01.2017, at 05:14, Eduardo Valentin <[e-mail]> wrote:
>>>>> 
>>>>> Hello Martin,
>>>>> 
>>>>> On Sat, Jan 07, 2017 at 04:55:45PM +0000, [e-mail] wrote:
>>>>>> From: Martin Sperl <[e-mail]>
>>>>>> 
>>>>>> Add basic thermal driver for bcm2835 SOC.
>>>>>> 
>>>>>> This driver currently relies on the firmware setting up the
>>>>>> tsense HW block and does not set it up itself.
>>>>>> 
>>>>>> Signed-off-by: Martin Sperl <[e-mail]>
>>>>>> Acked-by: Eric Anholt <[e-mail]>
>>>>>> Acked-by: Stefan Wahren <[e-mail]>
>>>>>> 
>>>>> 
>>>>> <cut>
>>>>> 
>>>>>> +
>>>>>> +static const struct of_device_id bcm2835_thermal_of_match_table[] = {
>>>>>> +	{
>>>>>> +		.compatible = "brcm,bcm2835-thermal",
>>>>>> +		.data = &(struct bcm2835_thermal_info) {
>>>>>> +			.offset = 407000,
>>>>>> +			.slope = -538,
>>>>>> +			.trip_temp = 80000
>>>>>> +		}
>>>>>> +	},
>>>>>> +	{
>>>>>> +		.compatible = "brcm,bcm2836-thermal",
>>>>>> +		.data = &(struct bcm2835_thermal_info) {
>>>>>> +			.offset = 407000,
>>>>>> +			.slope = -538,
>>>>>> +			.trip_temp = 80000
>>>>>> +		}
>>>>>> +	},
>>>>>> +	{
>>>>>> +		.compatible = "brcm,bcm2837-thermal",
>>>>>> +		.data = &(struct bcm2835_thermal_info) {
>>>>>> +			/* the bcm2837 needs adjustment of +5C */
>>>>>> +			.offset = 407000 + 5000,
>>>>>> +			.slope = -538,
>>>>>> +			.trip_temp = 80000
>>>>>> +		}
>>>>>> +	},
>>>>>> +	{},
>>>>> 
>>>>> Just for the same of clarification, is there anything preventing this
>>>>> driver of using of-thermal API? the above data (slope, offset, and
>>>>> trip_temps) would be in DT the place where they are supposed to be,
>>>>> instead of code.
>>>>> 
>>>> 
>>>> As the DT changes, that only define compatible strings, have already gone
>>>> in without any such properties set, we need to define defaults for the 
>>>> slope/offset and trip_temp values.
>>>> 
>>> 
>>> These properties won't go into the same node you are referring to. They
>>> go into the thermal-zone node you would create, which would then refer
>>> to the node you referred (already merged). Therefore, I do not see
>>> anything blocking a proper of-thermal usage to cover for the above data.
>>> 
>>>> I guess (for newer SOC) you still can use the values in the DT,
>>>> as (I guess) these are parsed and set in thermal_zone_device_register
>>>> after the defaults are set in thermal_zone_params.
>>> 
>>> Not sure what you meant here, but these values, when correctly used in
>>> DT, they would come as part of the thermal_zone_params and in the
>>> thermal trips of the thermal zones, as the of-thermal code with already
>>> deal with those for you.
>>> 
>>> Please have a look at:
>>> a. Documentation/devicetree/bindings/thermal/thermal.txt
>>> b. drivers/thermal/of-thermal.c
>>> 
>>> And let me know if you see anything that would prevent this driver of
>>> using the correct API to describe hardware data with DT.
>> 
>> 
>> I guess you miss my point:
> 
> Maybe you missed, or did not read the doc I pointed you...
> 
>> The argument is that we have DT in the 4.10.0-rc2 kernel right now that
>> look like this:
>>                thermal@7e212000 {
>>                        compatible = "brcm,bcm2835-thermal";
>>                        clocks = <0x6 0x1b>;
>>                        status = "okay";
>>                        reg = <0x7e212000 0x8>;
>>                }
>> so we still need to be compatible with those without any extra defines.
> 
> Yes, but the above DT entry will still be valid if you add the correct
> of-thermal support. In fact, you would add in your DTS a thermal-zones
> node, and in one of the defined zone, you would then reference the node
> you already got into mainline. Below is a copy of the doc I mentioned
> before:
> 
> 
> #include <dt-bindings/thermal/thermal.h>
> 
> ocp {
> 	...
> 	/*
> 	 * A simple IC with several bandgap temperature sensors.
> 	 */
> 	bandgap0: bandgap@0x0000ED00 {
> 		...
> 		#thermal-sensor-cells = <1>;
> 	};
> };
> 
> thermal-zones {
> 	cpu_thermal: cpu-thermal {
> 		polling-delay-passive = <250>; /* milliseconds */
> 		polling-delay = <1000>; /* milliseconds */
> 
> 				/* sensor       ID */
> 		thermal-sensors = <&bandgap0     0>;
> 
> 		trips {
> 			/* each zone within the SoC may have its own trips */
> 			cpu_alert: cpu-alert {
> 				temperature = <100000>; /* millicelsius */
> 				hysteresis = <2000>; /* millicelsius */
> 				type = "passive";
> 			};
> 			cpu_crit: cpu-crit {
> 				temperature = <125000>; /* millicelsius */
> 				hysteresis = <2000>; /* millicelsius */
> 				type = "critical";
> 			};
> 		};
> 
> 		cooling-maps {
> 			/* each zone within the SoC may have its own cooling */
> 			...
> 		};
> 	};
> 
>> 
>> Hence we need to define those slopes and offsets in the driver itself
>> to stay compatible with those older device-trees.
> 
> not really, as long as we do not merge the driver with the missing
> of-thermal support, I see no need to have both supports in your driver,
> i.e., if we start correct for the beggining there is no need to have
> offsets and slopes data in your driver code.
> 
>> 
>> As for if it works with the framework - I have to admit I do not
>> have the slightest clue - it looks way to complicated for the soc right
>> now.
> 
> Well, there is the documentation I mentioned, which several other
> drivers used as base for their support. You can also look at other
> thermal zones already defined in the existing DTS(I)'s.
> 
>> 
>> As a note: afaiu the trip_temp register is the temperature at which the
>> soc will reboot on its own - similar to a watchdog, but for temperatures.
>> (main reason for the assumption is because there is no interrupt that
>> can get assigned a handler to catch this situation).
>> 
> 
> OK. But that does not prevent you to have other trips so your running
> system can act before the shutdown trip is crossed.
> 
>> Martin
>> 
>> 

So how does this change/impact the driver code itself?

Defining a thermal zone in the dts is just an additional feature
that only impacts the device tree.
The DT as is works fine and at least allows to read the current temperature.

So can we just merge the driver now and look into the DT separately?

Martin

--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Stefan Wahren Feb. 4, 2017, 9:36 a.m. UTC | #11

Hi Eduardo,

> Eduardo Valentin <[e-mail]> hat am 2. Februar 2017 um 05:29 geschrieben:
> 
> 
> Hello Martin,
> 
> ...
> 
> 
> #include <dt-bindings/thermal/thermal.h>
> 
> ocp {
> 	...
> 	/*
> 	 * A simple IC with several bandgap temperature sensors.
> 	 */
> 	bandgap0: bandgap@0x0000ED00 {
> 		...
> 		#thermal-sensor-cells = <1>;
> 	};
> };
> 
> thermal-zones {
> 	cpu_thermal: cpu-thermal {
> 		polling-delay-passive = <250>; /* milliseconds */
> 		polling-delay = <1000>; /* milliseconds */
> 
> 				/* sensor       ID */
> 		thermal-sensors = <&bandgap0     0>;
> 
> 		trips {
> 			/* each zone within the SoC may have its own trips */
> 			cpu_alert: cpu-alert {
> 				temperature = <100000>; /* millicelsius */
> 				hysteresis = <2000>; /* millicelsius */
> 				type = "passive";
> 			};
> 			cpu_crit: cpu-crit {
> 				temperature = <125000>; /* millicelsius */
> 				hysteresis = <2000>; /* millicelsius */
> 				type = "critical";
> 			};
> 		};
> 
> 		cooling-maps {
> 			/* each zone within the SoC may have its own cooling */
> 			...
> 		};
> 	};
> 

if i get it right the device tree binding requires also a cooling map. But how should we model this without a fan or a DVFS driver? Is there something like a placeholder? Do you have an example?

Regards
Stefan
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Eduardo Valentin Feb. 8, 2017, 4:31 a.m. UTC | #12

On Sat, Feb 04, 2017 at 09:35:47AM +0100, [e-mail] wrote:
> 

> So how does this change/impact the driver code itself?
> 
> Defining a thermal zone in the dts is just an additional feature
> that only impacts the device tree.
> The DT as is works fine and at least allows to read the current temperature.

Well, your driver is still half finished. It is a DT only driver, which
does not follow the DT spec on thermal. The impact on your code is that
it wont need to carry your hardware data in the source code.

Also, having the data described in DT allows you porting your driver
without patching it, in  case you need, or any other system engineer,
to have different thermal data, trips values, number or trips, offset
and slope per sensor, on different boards, or even on different chip
version.

Not to say that having the data described in DT also allows system
engineers to deploy different thermal zones, based on your
driver/sensor, to extrapolate different hotspots.

> Martin
> 
BR,

Eduardo Valentin
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Martin Sperl Feb. 8, 2017, 8:19 a.m. UTC | #13

> On 08.02.2017, at 05:31, Eduardo Valentin <[e-mail]> wrote:
> 
> On Sat, Feb 04, 2017 at 09:35:47AM +0100, [e-mail] wrote:
>> 
> 
>> So how does this change/impact the driver code itself?
>> 
>> Defining a thermal zone in the dts is just an additional feature
>> that only impacts the device tree.
>> The DT as is works fine and at least allows to read the current temperature.
> 
> Well, your driver is still half finished. It is a DT only driver, which
> does not follow the DT spec on thermal. The impact on your code is that
> it wont need to carry your hardware data in the source code.
> 
> Also, having the data described in DT allows you porting your driver
> without patching it, in  case you need, or any other system engineer,
> to have different thermal data, trips values, number or trips, offset
> and slope per sensor, on different boards, or even on different chip
> version.
> 
> Not to say that having the data described in DT also allows system
> engineers to deploy different thermal zones, based on your
> driver/sensor, to extrapolate different hotspots.
> 

That is all true and as far as I understand you can do all of that
using the current driver - see the patch-sets by Stefan Wahren that
incorporates this into the dts.

My point is still: there are dtb’s that come with 4.10-rcX
that do not have all of those defined 

But as the mantra for DT is: "it is an API”, we have to be backwards
compatible from the driver perspective.
So we need those values in the driver to ensure the older version
of dtb’s are working correctly. That is why it is still included.

Also the settings of the trip are for the HW trip point for the case
that a future version of the firmware does not set up the thermal
HW block.

Do you really want to break this Mantra of backwards compatibility
by having those compatiblity “defines” stripped out with all the
possible negative side-effects if things are not set up correctly?

Thanks,
	Martin

P.s: I find it strange that V3 of the driver was sent out in May 2016
(way before the DTS changes got merged) and you only started to 
give feedback in November only in V8 (that primarily incorporated
minor changes and a rebase to 4.9) by which time the changes to the
dts have already been merged.



--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
index c2c056c..18f2de6 100644
--- a/drivers/thermal/Kconfig
+++ b/drivers/thermal/Kconfig
@@ -436,4 +436,12 @@  depends on (ARCH_QCOM && OF) || COMPILE_TEST
 source "drivers/thermal/qcom/Kconfig"
 endmenu

+config BCM2835_THERMAL
+	tristate "Thermal sensors on bcm2835 SoC"
+	depends on ARCH_BCM2835 || COMPILE_TEST
+	depends on HAS_IOMEM
+	depends on OF
+	help
+	  Support for thermal sensors on Broadcom bcm2835 SoCs.
+
 endif
diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
index 6a3d7b5..677c6d9 100644
--- a/drivers/thermal/Makefile
+++ b/drivers/thermal/Makefile
@@ -56,3 +56,4 @@  obj-$(CONFIG_TEGRA_SOCTHERM)	+= tegra/
 obj-$(CONFIG_HISI_THERMAL)     += hisi_thermal.o
 obj-$(CONFIG_MTK_THERMAL)	+= mtk_thermal.o
 obj-$(CONFIG_GENERIC_ADC_THERMAL)	+= thermal-generic-adc.o
+obj-$(CONFIG_BCM2835_THERMAL)	+= bcm2835_thermal.o
diff --git a/drivers/thermal/bcm2835_thermal.c b/drivers/thermal/bcm2835_thermal.c
new file mode 100644
index 0000000..5e2fea9
--- /dev/null
+++ b/drivers/thermal/bcm2835_thermal.c
@@ -0,0 +1,354 @@ 
+/*
+ * Driver for Broadcom BCM2835 soc temperature sensor
+ *
+ * Copyright (C) 2016 Martin Sperl
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/clk.h>
+#include <linux/debugfs.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/thermal.h>
+
+#define BCM2835_TS_TSENSCTL			0x00
+#define BCM2835_TS_TSENSSTAT			0x04
+
+#define BCM2835_TS_TSENSCTL_PRWDW		BIT(0)
+#define BCM2835_TS_TSENSCTL_RSTB		BIT(1)
+#define BCM2835_TS_TSENSCTL_CTRL_BITS		3
+#define BCM2835_TS_TSENSCTL_CTRL_SHIFT		2
+#define BCM2835_TS_TSENSCTL_CTRL_MASK		    \
+	GENMASK(BCM2835_TS_TSENSCTL_CTRL_BITS +     \
+		BCM2835_TS_TSENSCTL_CTRL_SHIFT - 1, \
+		BCM2835_TS_TSENSCTL_CTRL_SHIFT)
+#define BCM2835_TS_TSENSCTL_CTRL_DEFAULT	1
+#define BCM2835_TS_TSENSCTL_EN_INT		BIT(5)
+#define BCM2835_TS_TSENSCTL_DIRECT		BIT(6)
+#define BCM2835_TS_TSENSCTL_CLR_INT		BIT(7)
+#define BCM2835_TS_TSENSCTL_THOLD_SHIFT		8
+#define BCM2835_TS_TSENSCTL_THOLD_BITS		10
+#define BCM2835_TS_TSENSCTL_THOLD_MASK		     \
+	GENMASK(BCM2835_TS_TSENSCTL_THOLD_BITS +     \
+		BCM2835_TS_TSENSCTL_THOLD_SHIFT - 1, \
+		BCM2835_TS_TSENSCTL_THOLD_SHIFT)
+#define BCM2835_TS_TSENSCTL_RSTDELAY_SHIFT	18
+#define BCM2835_TS_TSENSCTL_RSTDELAY_BITS	8
+#define BCM2835_TS_TSENSCTL_REGULEN		BIT(26)
+
+#define BCM2835_TS_TSENSSTAT_DATA_BITS		10
+#define BCM2835_TS_TSENSSTAT_DATA_SHIFT		0
+#define BCM2835_TS_TSENSSTAT_DATA_MASK		     \
+	GENMASK(BCM2835_TS_TSENSSTAT_DATA_BITS +     \
+		BCM2835_TS_TSENSSTAT_DATA_SHIFT - 1, \
+		BCM2835_TS_TSENSSTAT_DATA_SHIFT)
+#define BCM2835_TS_TSENSSTAT_VALID		BIT(10)
+#define BCM2835_TS_TSENSSTAT_INTERRUPT		BIT(11)
+
+struct bcm2835_thermal_info {
+	int offset;
+	int slope;
+	int trip_temp;
+};
+
+struct bcm2835_thermal_data {
+	const struct bcm2835_thermal_info *info;
+	void __iomem *regs;
+	struct clk *clk;
+	struct dentry *debugfsdir;
+};
+
+static int bcm2835_thermal_adc2temp(u32 adc, int offset, int slope)
+{
+	return offset + slope * adc;
+}
+
+static int bcm2835_thermal_temp2adc(int temp, int offset, int slope)
+{
+	temp -= offset;
+	temp /= slope;
+
+	if (temp < 0)
+		temp = 0;
+	if (temp >= BIT(BCM2835_TS_TSENSSTAT_DATA_BITS))
+		temp = BIT(BCM2835_TS_TSENSSTAT_DATA_BITS) - 1;
+
+	return temp;
+}
+
+static int bcm2835_thermal_get_trip_type(
+	struct thermal_zone_device *tz, int trip,
+	enum thermal_trip_type *type)
+{
+	*type = THERMAL_TRIP_CRITICAL;
+	return 0;
+}
+
+static int bcm2835_thermal_get_trip_temp(
+	struct thermal_zone_device *tz, int trip, int *temp)
+{
+	struct bcm2835_thermal_data *data = tz->devdata;
+	u32 val = readl(data->regs + BCM2835_TS_TSENSCTL);
+
+	/* get the THOLD bits */
+	val &= BCM2835_TS_TSENSCTL_THOLD_MASK;
+	val >>= BCM2835_TS_TSENSCTL_THOLD_SHIFT;
+
+	/* if it is zero then use the info value */
+	if (val)
+		*temp = bcm2835_thermal_adc2temp(
+			val,
+			thermal_zone_get_offset(tz),
+			thermal_zone_get_slope(tz));
+	else
+		*temp = data->info->trip_temp;
+
+	return 0;
+}
+
+static int bcm2835_thermal_get_temp(struct thermal_zone_device *tz,
+				    int *temp)
+{
+	struct bcm2835_thermal_data *data = tz->devdata;
+	u32 val = readl(data->regs + BCM2835_TS_TSENSSTAT);
+
+	if (!(val & BCM2835_TS_TSENSSTAT_VALID))
+		return -EIO;
+
+	val &= BCM2835_TS_TSENSSTAT_DATA_MASK;
+
+	*temp = bcm2835_thermal_adc2temp(
+		val,
+		thermal_zone_get_offset(tz),
+		thermal_zone_get_slope(tz));
+
+	return 0;
+}
+
+static const struct debugfs_reg32 bcm2835_thermal_regs[] = {
+	{
+		.name = "ctl",
+		.offset = 0
+	},
+	{
+		.name = "stat",
+		.offset = 4
+	}
+};
+
+static void bcm2835_thermal_debugfs(struct platform_device *pdev)
+{
+	struct thermal_zone_device *tz = platform_get_drvdata(pdev);
+	struct bcm2835_thermal_data *data = tz->devdata;
+	struct debugfs_regset32 *regset;
+
+	data->debugfsdir = debugfs_create_dir("bcm2835_thermal", NULL);
+	if (!data->debugfsdir)
+		return;
+
+	regset = devm_kzalloc(&pdev->dev, sizeof(*regset), GFP_KERNEL);
+	if (!regset)
+		return;
+
+	regset->regs = bcm2835_thermal_regs;
+	regset->nregs = ARRAY_SIZE(bcm2835_thermal_regs);
+	regset->base = data->regs;
+
+	debugfs_create_regset32("regset", S_IRUGO,
+				data->debugfsdir, regset);
+}
+
+static struct thermal_zone_device_ops bcm2835_thermal_ops  = {
+	.get_temp = bcm2835_thermal_get_temp,
+	.get_trip_temp = bcm2835_thermal_get_trip_temp,
+	.get_trip_type = bcm2835_thermal_get_trip_type,
+};
+
+static const struct of_device_id bcm2835_thermal_of_match_table[];
+static int bcm2835_thermal_probe(struct platform_device *pdev)
+{
+	const struct of_device_id *match;
+	struct thermal_zone_device *tz;
+	struct thermal_zone_params *tzp;
+	const struct bcm2835_thermal_info *ti;
+	struct bcm2835_thermal_data *data;
+	struct resource *res;
+	int err;
+	u32 val;
+	unsigned long rate;
+
+	data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	tzp = devm_kzalloc(&pdev->dev, sizeof(*tzp), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	match = of_match_device(bcm2835_thermal_of_match_table,
+				&pdev->dev);
+	if (!match)
+		return -EINVAL;
+	ti = match->data;
+	data->info = ti;
+	tzp->slope = ti->slope;
+	tzp->offset = ti->offset;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	data->regs = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(data->regs)) {
+		err = PTR_ERR(data->regs);
+		dev_err(&pdev->dev, "Could not get registers: %d\n", err);
+		return err;
+	}
+
+	data->clk = devm_clk_get(&pdev->dev, NULL);
+	if (IS_ERR(data->clk)) {
+		err = PTR_ERR(data->clk);
+		if (err != -EPROBE_DEFER)
+			dev_err(&pdev->dev, "Could not get clk: %d\n", err);
+		return err;
+	}
+
+	err = clk_prepare_enable(data->clk);
+	if (err)
+		return err;
+
+	rate = clk_get_rate(data->clk);
+	if ((rate < 1920000) || (rate > 5000000))
+		dev_warn(&pdev->dev,
+			 "Clock %pCn running at %pCr Hz is outside of the recommended range: 1.92 to 5MHz\n",
+			 data->clk, data->clk);
+
+	/*
+	 * right now the FW does set up the HW-block, so we are not
+	 * touching the configuration registers.
+	 * But if the HW is not enabled, then set it up
+	 * using "sane" values used by the firmware right now.
+	 */
+	val = readl(data->regs + BCM2835_TS_TSENSCTL);
+	if (!(val & BCM2835_TS_TSENSCTL_RSTB)) {
+		/* the basic required flags */
+		val = (BCM2835_TS_TSENSCTL_CTRL_DEFAULT <<
+		       BCM2835_TS_TSENSCTL_CTRL_SHIFT) |
+		      BCM2835_TS_TSENSCTL_REGULEN;
+
+		/*
+		 * reset delay using the current firmware value of 14
+		 * - units of time are unknown.
+		 */
+		val |= (14 << BCM2835_TS_TSENSCTL_RSTDELAY_SHIFT);
+
+		/*  trip_adc value from info */
+		val |= bcm2835_thermal_temp2adc(data->info->trip_temp,
+						data->info->offset,
+						data->info->slope)
+			<< BCM2835_TS_TSENSCTL_THOLD_SHIFT;
+
+		/* write the value back to the register as 2 steps */
+		writel(val, data->regs + BCM2835_TS_TSENSCTL);
+		val |= BCM2835_TS_TSENSCTL_RSTB;
+		writel(val, data->regs + BCM2835_TS_TSENSCTL);
+	}
+
+	/* register thermal zone with 1 trip point an 1s polling */
+	tz = thermal_zone_device_register("bcm2835_thermal",
+					  1, 0, data,
+					  &bcm2835_thermal_ops,
+					  tzp,
+					  0, 1000);
+	if (IS_ERR(tz)) {
+		clk_disable_unprepare(data->clk);
+		err = PTR_ERR(tz);
+		dev_err(&pdev->dev,
+			"Failed to register the thermal device: %d\n",
+			err);
+		return err;
+	}
+
+	platform_set_drvdata(pdev, tz);
+
+	bcm2835_thermal_debugfs(pdev);
+
+	return 0;
+}
+
+static int bcm2835_thermal_remove(struct platform_device *pdev)
+{
+	struct thermal_zone_device *tz = platform_get_drvdata(pdev);
+	struct bcm2835_thermal_data *data = tz->devdata;
+
+	debugfs_remove_recursive(data->debugfsdir);
+	thermal_zone_device_unregister(tz);
+	clk_disable_unprepare(data->clk);
+
+	return 0;
+}
+
+/*
+ * Note: as per Raspberry Foundation FAQ
+ * (https://www.raspberrypi.org/help/faqs/#performanceOperatingTemperature)
+ * the recommended temperature range for the SOC -40C to +85C
+ * so the trip limit is set to 80C.
+ * this applies to all the BCM283X SOC
+ */
+
+static const struct of_device_id bcm2835_thermal_of_match_table[] = {
+	{
+		.compatible = "brcm,bcm2835-thermal",
+		.data = &(struct bcm2835_thermal_info) {
+			.offset = 407000,
+			.slope = -538,
+			.trip_temp = 80000
+		}
+	},
+	{
+		.compatible = "brcm,bcm2836-thermal",
+		.data = &(struct bcm2835_thermal_info) {
+			.offset = 407000,
+			.slope = -538,
+			.trip_temp = 80000
+		}
+	},
+	{
+		.compatible = "brcm,bcm2837-thermal",
+		.data = &(struct bcm2835_thermal_info) {
+			/* the bcm2837 needs adjustment of +5C */
+			.offset = 407000 + 5000,
+			.slope = -538,
+			.trip_temp = 80000
+		}
+	},
+	{},
+};
+MODULE_DEVICE_TABLE(of, bcm2835_thermal_of_match_table);
+
+static struct platform_driver bcm2835_thermal_driver = {
+	.probe = bcm2835_thermal_probe,
+	.remove = bcm2835_thermal_remove,
+	.driver = {
+		.name = "bcm2835_thermal",
+		.of_match_table = bcm2835_thermal_of_match_table,
+	},
+};
+module_platform_driver(bcm2835_thermal_driver);
+
+MODULE_AUTHOR("Martin Sperl");
+MODULE_DESCRIPTION("Thermal driver for bcm2835 chip");
+MODULE_LICENSE("GPL");