Hi Rob Herring, Please find my comments in line..
On Fri, Jun 1, 2018 at 7:59 PM, Rob Herring <[email protected]> wrote: > On Fri, Jun 1, 2018 at 3:51 AM, Srinath Mannam > <[email protected]> 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 <[email protected]> wrote: >>> On Mon, May 28, 2018 at 11:11:22AM +0530, Srinath Mannam wrote: >>>> From: Pramod Kumar <[email protected]> >>>> >>>> Add binding document for supported thermal implementation >>>> in Stingray. >>>> >>>> Signed-off-by: Pramod Kumar <[email protected]> >>>> Reviewed-by: Ray Jui <[email protected]> >>>> Reviewed-by: Scott Branden <[email protected]> >>>> Reviewed-by: Srinath Mannam <[email protected]> >>>> --- >>>> .../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
