[1/3] dt-bindings: thermal: Add binding document for SR thermal
diff mbox series

Message ID 1527486084-4636-2-git-send-email-srinath.mannam@broadcom.com
State New, archived
Headers show
Series
  • Stingray thermal driver support
Related show

Commit Message

Srinath Mannam May 28, 2018, 5:41 a.m. UTC
From: Pramod Kumar <pramod.kumar@broadcom.com>

Add binding document for supported thermal implementation
in Stingray.

Signed-off-by: Pramod Kumar <pramod.kumar@broadcom.com>
Reviewed-by: Ray Jui <ray.jui@broadcom.com>
Reviewed-by: Scott Branden <scott.branden@broadcom.com>
Reviewed-by: Srinath Mannam <srinath.mannam@broadcom.com>
---
 .../bindings/thermal/brcm,sr-thermal.txt           | 45 ++++++++++++++++++++++
 1 file changed, 45 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/thermal/brcm,sr-thermal.txt

Comments

Rob Herring May 31, 2018, 4:48 p.m. UTC | #1
On Mon, May 28, 2018 at 11:11:22AM +0530, Srinath Mannam wrote:
> From: Pramod Kumar <pramod.kumar@broadcom.com>
> 
> Add binding document for supported thermal implementation
> in Stingray.
> 
> Signed-off-by: Pramod Kumar <pramod.kumar@broadcom.com>
> Reviewed-by: Ray Jui <ray.jui@broadcom.com>
> Reviewed-by: Scott Branden <scott.branden@broadcom.com>
> Reviewed-by: Srinath Mannam <srinath.mannam@broadcom.com>
> ---
>  .../bindings/thermal/brcm,sr-thermal.txt           | 45 ++++++++++++++++++++++
>  1 file changed, 45 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/thermal/brcm,sr-thermal.txt
> 
> diff --git a/Documentation/devicetree/bindings/thermal/brcm,sr-thermal.txt b/Documentation/devicetree/bindings/thermal/brcm,sr-thermal.txt
> new file mode 100644
> index 0000000..33f9e11
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/thermal/brcm,sr-thermal.txt
> @@ -0,0 +1,45 @@
> +* Broadcom Stingray Thermal
> +
> +This binding describes thermal sensors that is part of Stingray SoCs.
> +
> +Required properties:
> +- compatible : Must be "brcm,sr-thermal"
> +- reg : memory where tmon data will be available.

What type of memory is this?

> +
> +Example:
> +	tmons {
> +		compatible = "simple-bus";
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		ranges;
> +
> +		tmon_ihost0: thermal@8f100000 {
> +			compatible = "brcm,sr-thermal";
> +			reg = <0x8f100000 0x4>;
> +		};

Convince me that you need a node per register. This can all be 
accomplished with a single node and either a single reg entry or a 
series of reg entries.

> +
> +		tmon_ihost1: thermal@8f100004 {
> +			compatible = "brcm,sr-thermal";
> +			reg = <0x8f100004 0x4>;
> +		};
> +
> +		tmon_ihost2: thermal@8f100008 {
> +			compatible = "brcm,sr-thermal";
> +			reg = <0x8f100008 0x4>;
> +		};
> +
> +		tmon_ihost3: thermal@8f10000c {
> +			compatible = "brcm,sr-thermal";
> +			reg = <0x8f10000c 0x4>;
> +		};
> +
> +		tmon_crmu: thermal@8f100010 {
> +			compatible = "brcm,sr-thermal";
> +			reg = <0x8f100010 0x4>;
> +		};
> +
> +		tmon_nitro: thermal@8f100014 {
> +			compatible = "brcm,sr-thermal";
> +			reg = <0x8f100014 0x4>;
> +		};
> +	};
> -- 
> 2.7.4
>
Srinath Mannam June 1, 2018, 8:51 a.m. UTC | #2
Hi Rob Herring,

Thank you for the review.
Please find my answers inline..

On Thu, May 31, 2018 at 10:18 PM, Rob Herring <robh@kernel.org> wrote:
> On Mon, May 28, 2018 at 11:11:22AM +0530, Srinath Mannam wrote:
>> From: Pramod Kumar <pramod.kumar@broadcom.com>
>>
>> Add binding document for supported thermal implementation
>> in Stingray.
>>
>> Signed-off-by: Pramod Kumar <pramod.kumar@broadcom.com>
>> Reviewed-by: Ray Jui <ray.jui@broadcom.com>
>> Reviewed-by: Scott Branden <scott.branden@broadcom.com>
>> Reviewed-by: Srinath Mannam <srinath.mannam@broadcom.com>
>> ---
>>  .../bindings/thermal/brcm,sr-thermal.txt           | 45 ++++++++++++++++++++++
>>  1 file changed, 45 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/thermal/brcm,sr-thermal.txt
>>
>> diff --git a/Documentation/devicetree/bindings/thermal/brcm,sr-thermal.txt b/Documentation/devicetree/bindings/thermal/brcm,sr-thermal.txt
>> new file mode 100644
>> index 0000000..33f9e11
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/thermal/brcm,sr-thermal.txt
>> @@ -0,0 +1,45 @@
>> +* Broadcom Stingray Thermal
>> +
>> +This binding describes thermal sensors that is part of Stingray SoCs.
>> +
>> +Required properties:
>> +- compatible : Must be "brcm,sr-thermal"
>> +- reg : memory where tmon data will be available.
>
> What type of memory is this?
This is shared memory(cache-able) which is shared between two micro controllers.
One micro controller update temperature of all thermal zones in this
shared memory.
thermal driver running on another micro controller monitors
temperature data of all thermal zones.
>
>> +
>> +Example:
>> +     tmons {
>> +             compatible = "simple-bus";
>> +             #address-cells = <1>;
>> +             #size-cells = <1>;
>> +             ranges;
>> +
>> +             tmon_ihost0: thermal@8f100000 {
>> +                     compatible = "brcm,sr-thermal";
>> +                     reg = <0x8f100000 0x4>;
>> +             };
>
> Convince me that you need a node per register. This can all be
> accomplished with a single node and either a single reg entry or a
> series of reg entries.
>
In our system we have 6 different thermal zones. each node belongs to
one thermal zone.
thermal driver detects each node as separate thermal zone and monitors
separately.
register entry is only 4byte memory which contains the temperature
value of respective thermal zone.
Based on requirement we can increase or decrease the number thermal
zones need to monitor.
>> +
>> +             tmon_ihost1: thermal@8f100004 {
>> +                     compatible = "brcm,sr-thermal";
>> +                     reg = <0x8f100004 0x4>;
>> +             };
>> +
>> +             tmon_ihost2: thermal@8f100008 {
>> +                     compatible = "brcm,sr-thermal";
>> +                     reg = <0x8f100008 0x4>;
>> +             };
>> +
>> +             tmon_ihost3: thermal@8f10000c {
>> +                     compatible = "brcm,sr-thermal";
>> +                     reg = <0x8f10000c 0x4>;
>> +             };
>> +
>> +             tmon_crmu: thermal@8f100010 {
>> +                     compatible = "brcm,sr-thermal";
>> +                     reg = <0x8f100010 0x4>;
>> +             };
>> +
>> +             tmon_nitro: thermal@8f100014 {
>> +                     compatible = "brcm,sr-thermal";
>> +                     reg = <0x8f100014 0x4>;
>> +             };
>> +     };
>> --
>> 2.7.4
>>
Rob Herring June 1, 2018, 2:29 p.m. UTC | #3
On Fri, Jun 1, 2018 at 3:51 AM, Srinath Mannam
<srinath.mannam@broadcom.com> wrote:
> Hi Rob Herring,
>
> Thank you for the review.
> Please find my answers inline..
>
> On Thu, May 31, 2018 at 10:18 PM, Rob Herring <robh@kernel.org> wrote:
>> On Mon, May 28, 2018 at 11:11:22AM +0530, Srinath Mannam wrote:
>>> From: Pramod Kumar <pramod.kumar@broadcom.com>
>>>
>>> Add binding document for supported thermal implementation
>>> in Stingray.
>>>
>>> Signed-off-by: Pramod Kumar <pramod.kumar@broadcom.com>
>>> Reviewed-by: Ray Jui <ray.jui@broadcom.com>
>>> Reviewed-by: Scott Branden <scott.branden@broadcom.com>
>>> Reviewed-by: Srinath Mannam <srinath.mannam@broadcom.com>
>>> ---
>>>  .../bindings/thermal/brcm,sr-thermal.txt           | 45 ++++++++++++++++++++++
>>>  1 file changed, 45 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/thermal/brcm,sr-thermal.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/thermal/brcm,sr-thermal.txt b/Documentation/devicetree/bindings/thermal/brcm,sr-thermal.txt
>>> new file mode 100644
>>> index 0000000..33f9e11
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/thermal/brcm,sr-thermal.txt
>>> @@ -0,0 +1,45 @@
>>> +* Broadcom Stingray Thermal
>>> +
>>> +This binding describes thermal sensors that is part of Stingray SoCs.
>>> +
>>> +Required properties:
>>> +- compatible : Must be "brcm,sr-thermal"
>>> +- reg : memory where tmon data will be available.
>>
>> What type of memory is this?
> This is shared memory(cache-able) which is shared between two micro controllers.

What else is in this shared memory? You should probably be
representing that as a whole somewhere in DT.

> One micro controller update temperature of all thermal zones in this
> shared memory.
> thermal driver running on another micro controller monitors
> temperature data of all thermal zones.
>>
>>> +
>>> +Example:
>>> +     tmons {
>>> +             compatible = "simple-bus";
>>> +             #address-cells = <1>;
>>> +             #size-cells = <1>;
>>> +             ranges;
>>> +
>>> +             tmon_ihost0: thermal@8f100000 {
>>> +                     compatible = "brcm,sr-thermal";
>>> +                     reg = <0x8f100000 0x4>;
>>> +             };
>>
>> Convince me that you need a node per register. This can all be
>> accomplished with a single node and either a single reg entry or a
>> series of reg entries.
>>
> In our system we have 6 different thermal zones. each node belongs to
> one thermal zone.
> thermal driver detects each node as separate thermal zone and monitors
> separately.
> register entry is only 4byte memory which contains the temperature
> value of respective thermal zone.
> Based on requirement we can increase or decrease the number thermal
> zones need to monitor.

Still, all this can be accomplished with a single node. Having a node
per register in DT doesn't scale.

Rob
Srinath Mannam June 4, 2018, 6:06 a.m. UTC | #4
Hi Rob Herring,

Please find my comments in line..

On Fri, Jun 1, 2018 at 7:59 PM, Rob Herring <robh@kernel.org> wrote:
> On Fri, Jun 1, 2018 at 3:51 AM, Srinath Mannam
> <srinath.mannam@broadcom.com> wrote:
>> Hi Rob Herring,
>>
>> Thank you for the review.
>> Please find my answers inline..
>>
>> On Thu, May 31, 2018 at 10:18 PM, Rob Herring <robh@kernel.org> wrote:
>>> On Mon, May 28, 2018 at 11:11:22AM +0530, Srinath Mannam wrote:
>>>> From: Pramod Kumar <pramod.kumar@broadcom.com>
>>>>
>>>> Add binding document for supported thermal implementation
>>>> in Stingray.
>>>>
>>>> Signed-off-by: Pramod Kumar <pramod.kumar@broadcom.com>
>>>> Reviewed-by: Ray Jui <ray.jui@broadcom.com>
>>>> Reviewed-by: Scott Branden <scott.branden@broadcom.com>
>>>> Reviewed-by: Srinath Mannam <srinath.mannam@broadcom.com>
>>>> ---
>>>>  .../bindings/thermal/brcm,sr-thermal.txt           | 45 ++++++++++++++++++++++
>>>>  1 file changed, 45 insertions(+)
>>>>  create mode 100644 Documentation/devicetree/bindings/thermal/brcm,sr-thermal.txt
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/thermal/brcm,sr-thermal.txt b/Documentation/devicetree/bindings/thermal/brcm,sr-thermal.txt
>>>> new file mode 100644
>>>> index 0000000..33f9e11
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/thermal/brcm,sr-thermal.txt
>>>> @@ -0,0 +1,45 @@
>>>> +* Broadcom Stingray Thermal
>>>> +
>>>> +This binding describes thermal sensors that is part of Stingray SoCs.
>>>> +
>>>> +Required properties:
>>>> +- compatible : Must be "brcm,sr-thermal"
>>>> +- reg : memory where tmon data will be available.
>>>
>>> What type of memory is this?
>> This is shared memory(cache-able) which is shared between two micro controllers.
>
> What else is in this shared memory? You should probably be
> representing that as a whole somewhere in DT.
Nothing else except 4 byte memory containing temperature value of one
thermal zone.
At present we configured 6 thermal zones. So six separate 4byte
memories provided in
shared memory.
>
>> One micro controller update temperature of all thermal zones in this
>> shared memory.
>> thermal driver running on another micro controller monitors
>> temperature data of all thermal zones.
>>>
>>>> +
>>>> +Example:
>>>> +     tmons {
>>>> +             compatible = "simple-bus";
>>>> +             #address-cells = <1>;
>>>> +             #size-cells = <1>;
>>>> +             ranges;
>>>> +
>>>> +             tmon_ihost0: thermal@8f100000 {
>>>> +                     compatible = "brcm,sr-thermal";
>>>> +                     reg = <0x8f100000 0x4>;
>>>> +             };
>>>
>>> Convince me that you need a node per register. This can all be
>>> accomplished with a single node and either a single reg entry or a
>>> series of reg entries.
>>>
>> In our system we have 6 different thermal zones. each node belongs to
>> one thermal zone.
>> thermal driver detects each node as separate thermal zone and monitors
>> separately.
>> register entry is only 4byte memory which contains the temperature
>> value of respective thermal zone.
>> Based on requirement we can increase or decrease the number thermal
>> zones need to monitor.
>
> Still, all this can be accomplished with a single node. Having a node
> per register in DT doesn't scale.
Our approach is like we have individual controllers have one driver
with multiple DT nodes.
In coming future we will add more thermal zones based on our
requirement. then we can just
add new DT node, instead of having driver changes.
With this approach we can configure each thermal zone at different trip values.
So we must have all thermal zones are independent.
>
> Rob
Srinath Mannam June 18, 2018, 7:53 a.m. UTC | #5
Hi,

Please let me know if you have any comments to this patch.
If not can you please accept this patch.

Regards,
Srinath.

On Mon, Jun 4, 2018 at 11:36 AM, Srinath Mannam
<srinath.mannam@broadcom.com> wrote:
> Hi Rob Herring,
>
> Please find my comments in line..
>
> On Fri, Jun 1, 2018 at 7:59 PM, Rob Herring <robh@kernel.org> wrote:
>> On Fri, Jun 1, 2018 at 3:51 AM, Srinath Mannam
>> <srinath.mannam@broadcom.com> wrote:
>>> Hi Rob Herring,
>>>
>>> Thank you for the review.
>>> Please find my answers inline..
>>>
>>> On Thu, May 31, 2018 at 10:18 PM, Rob Herring <robh@kernel.org> wrote:
>>>> On Mon, May 28, 2018 at 11:11:22AM +0530, Srinath Mannam wrote:
>>>>> From: Pramod Kumar <pramod.kumar@broadcom.com>
>>>>>
>>>>> Add binding document for supported thermal implementation
>>>>> in Stingray.
>>>>>
>>>>> Signed-off-by: Pramod Kumar <pramod.kumar@broadcom.com>
>>>>> Reviewed-by: Ray Jui <ray.jui@broadcom.com>
>>>>> Reviewed-by: Scott Branden <scott.branden@broadcom.com>
>>>>> Reviewed-by: Srinath Mannam <srinath.mannam@broadcom.com>
>>>>> ---
>>>>>  .../bindings/thermal/brcm,sr-thermal.txt           | 45 ++++++++++++++++++++++
>>>>>  1 file changed, 45 insertions(+)
>>>>>  create mode 100644 Documentation/devicetree/bindings/thermal/brcm,sr-thermal.txt
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/thermal/brcm,sr-thermal.txt b/Documentation/devicetree/bindings/thermal/brcm,sr-thermal.txt
>>>>> new file mode 100644
>>>>> index 0000000..33f9e11
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/thermal/brcm,sr-thermal.txt
>>>>> @@ -0,0 +1,45 @@
>>>>> +* Broadcom Stingray Thermal
>>>>> +
>>>>> +This binding describes thermal sensors that is part of Stingray SoCs.
>>>>> +
>>>>> +Required properties:
>>>>> +- compatible : Must be "brcm,sr-thermal"
>>>>> +- reg : memory where tmon data will be available.
>>>>
>>>> What type of memory is this?
>>> This is shared memory(cache-able) which is shared between two micro controllers.
>>
>> What else is in this shared memory? You should probably be
>> representing that as a whole somewhere in DT.
> Nothing else except 4 byte memory containing temperature value of one
> thermal zone.
> At present we configured 6 thermal zones. So six separate 4byte
> memories provided in
> shared memory.
>>
>>> One micro controller update temperature of all thermal zones in this
>>> shared memory.
>>> thermal driver running on another micro controller monitors
>>> temperature data of all thermal zones.
>>>>
>>>>> +
>>>>> +Example:
>>>>> +     tmons {
>>>>> +             compatible = "simple-bus";
>>>>> +             #address-cells = <1>;
>>>>> +             #size-cells = <1>;
>>>>> +             ranges;
>>>>> +
>>>>> +             tmon_ihost0: thermal@8f100000 {
>>>>> +                     compatible = "brcm,sr-thermal";
>>>>> +                     reg = <0x8f100000 0x4>;
>>>>> +             };
>>>>
>>>> Convince me that you need a node per register. This can all be
>>>> accomplished with a single node and either a single reg entry or a
>>>> series of reg entries.
>>>>
>>> In our system we have 6 different thermal zones. each node belongs to
>>> one thermal zone.
>>> thermal driver detects each node as separate thermal zone and monitors
>>> separately.
>>> register entry is only 4byte memory which contains the temperature
>>> value of respective thermal zone.
>>> Based on requirement we can increase or decrease the number thermal
>>> zones need to monitor.
>>
>> Still, all this can be accomplished with a single node. Having a node
>> per register in DT doesn't scale.
> Our approach is like we have individual controllers have one driver
> with multiple DT nodes.
> In coming future we will add more thermal zones based on our
> requirement. then we can just
> add new DT node, instead of having driver changes.
> With this approach we can configure each thermal zone at different trip values.
> So we must have all thermal zones are independent.
>>
>> Rob

Patch
diff mbox series

diff --git a/Documentation/devicetree/bindings/thermal/brcm,sr-thermal.txt b/Documentation/devicetree/bindings/thermal/brcm,sr-thermal.txt
new file mode 100644
index 0000000..33f9e11
--- /dev/null
+++ b/Documentation/devicetree/bindings/thermal/brcm,sr-thermal.txt
@@ -0,0 +1,45 @@ 
+* Broadcom Stingray Thermal
+
+This binding describes thermal sensors that is part of Stingray SoCs.
+
+Required properties:
+- compatible : Must be "brcm,sr-thermal"
+- reg : memory where tmon data will be available.
+
+Example:
+	tmons {
+		compatible = "simple-bus";
+		#address-cells = <1>;
+		#size-cells = <1>;
+		ranges;
+
+		tmon_ihost0: thermal@8f100000 {
+			compatible = "brcm,sr-thermal";
+			reg = <0x8f100000 0x4>;
+		};
+
+		tmon_ihost1: thermal@8f100004 {
+			compatible = "brcm,sr-thermal";
+			reg = <0x8f100004 0x4>;
+		};
+
+		tmon_ihost2: thermal@8f100008 {
+			compatible = "brcm,sr-thermal";
+			reg = <0x8f100008 0x4>;
+		};
+
+		tmon_ihost3: thermal@8f10000c {
+			compatible = "brcm,sr-thermal";
+			reg = <0x8f10000c 0x4>;
+		};
+
+		tmon_crmu: thermal@8f100010 {
+			compatible = "brcm,sr-thermal";
+			reg = <0x8f100010 0x4>;
+		};
+
+		tmon_nitro: thermal@8f100014 {
+			compatible = "brcm,sr-thermal";
+			reg = <0x8f100014 0x4>;
+		};
+	};