| Message ID | 1529310679-13482-2-git-send-email-srinath.mannam@broadcom.com |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | Stingray thermal driver support | expand |
On Mon, Jun 18, 2018 at 02:01:17PM +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. > + > +Example: > + tmons { > + compatible = "simple-bus"; > + #address-cells = <1>; > + #size-cells = <1>; > + ranges; > + > + tmon_ihost0: thermal@8f100000 { > + compatible = "brcm,sr-thermal"; > + reg = <0x8f100000 0x4>; > + }; You still haven't given me a compelling reason why you need a node per register. You have a single range of registers. Make this 1 node. Rob -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Rob, Please find my comments for the reason to have multiple DT nodes. On Thu, Jun 21, 2018 at 1:22 AM, Rob Herring <robh@kernel.org> wrote: > On Mon, Jun 18, 2018 at 02:01:17PM +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. >> + >> +Example: >> + tmons { >> + compatible = "simple-bus"; >> + #address-cells = <1>; >> + #size-cells = <1>; >> + ranges; >> + >> + tmon_ihost0: thermal@8f100000 { >> + compatible = "brcm,sr-thermal"; >> + reg = <0x8f100000 0x4>; >> + }; > > You still haven't given me a compelling reason why you need a node per > register. > > You have a single range of registers. Make this 1 node. > We Have two reasons to have multiple nodes.. 1. Our chip has multiple functional blocks. Each functional block has its own thermal zone. Functional blocks and their thermal zones enabled/disabled based on end product. Few functional blocks need to disabled for few products so thermal zones also need to disable. In that case, nodes of specific thermal zones are removed from DTS file of corresponding product. 2. Thermal framework provides sysfs interface to configure thermal zones and read temperature of thermal zone. To configure individual thermal zone, we need to have separate DT node. Same to read temperature of individual thermal zone. Ex: To read temperature of thermal zone 0. cat /sys/class/thermal/thermal_zone0/temp To configure trip temperature of thermal zone 0. echo 110000 > /sys/class/thermal/thermal_zone0/trip_point_0_temp Also to avoid driver source change for the multiple products it is clean to have multiple DT nodes. > Rob -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Rob, Kindly provide your feedback. Regards, Srinath. On Fri, Jun 22, 2018 at 11:21 AM, Srinath Mannam <srinath.mannam@broadcom.com> wrote: > Hi Rob, > > Please find my comments for the reason to have multiple DT nodes. > > On Thu, Jun 21, 2018 at 1:22 AM, Rob Herring <robh@kernel.org> wrote: >> On Mon, Jun 18, 2018 at 02:01:17PM +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. >>> + >>> +Example: >>> + tmons { >>> + compatible = "simple-bus"; >>> + #address-cells = <1>; >>> + #size-cells = <1>; >>> + ranges; >>> + >>> + tmon_ihost0: thermal@8f100000 { >>> + compatible = "brcm,sr-thermal"; >>> + reg = <0x8f100000 0x4>; >>> + }; >> >> You still haven't given me a compelling reason why you need a node per >> register. >> >> You have a single range of registers. Make this 1 node. >> > > We Have two reasons to have multiple nodes.. > 1. Our chip has multiple functional blocks. Each functional block has > its own thermal zone. > Functional blocks and their thermal zones enabled/disabled based on end product. > Few functional blocks need to disabled for few products so thermal > zones also need to disable. > In that case, nodes of specific thermal zones are removed from DTS > file of corresponding product. > > 2. Thermal framework provides sysfs interface to configure thermal > zones and read temperature of thermal zone. > To configure individual thermal zone, we need to have separate DT node. > Same to read temperature of individual thermal zone. > Ex: To read temperature of thermal zone 0. > cat /sys/class/thermal/thermal_zone0/temp > To configure trip temperature of thermal zone 0. > echo 110000 > /sys/class/thermal/thermal_zone0/trip_point_0_temp > > Also to avoid driver source change for the multiple products it is > clean to have multiple DT nodes. > >> Rob -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Rob, I have provided my inputs for the purpose of having multiple nodes. Please get back if you have any comments or suggestions. Regards, Srinath. On Tue, Jul 3, 2018 at 4:15 PM, Srinath Mannam <srinath.mannam@broadcom.com> wrote: > Hi Rob, > > Kindly provide your feedback. > > Regards, > Srinath. > > On Fri, Jun 22, 2018 at 11:21 AM, Srinath Mannam > <srinath.mannam@broadcom.com> wrote: >> Hi Rob, >> >> Please find my comments for the reason to have multiple DT nodes. >> >> On Thu, Jun 21, 2018 at 1:22 AM, Rob Herring <robh@kernel.org> wrote: >>> On Mon, Jun 18, 2018 at 02:01:17PM +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. >>>> + >>>> +Example: >>>> + tmons { >>>> + compatible = "simple-bus"; >>>> + #address-cells = <1>; >>>> + #size-cells = <1>; >>>> + ranges; >>>> + >>>> + tmon_ihost0: thermal@8f100000 { >>>> + compatible = "brcm,sr-thermal"; >>>> + reg = <0x8f100000 0x4>; >>>> + }; >>> >>> You still haven't given me a compelling reason why you need a node per >>> register. >>> >>> You have a single range of registers. Make this 1 node. >>> >> >> We Have two reasons to have multiple nodes.. >> 1. Our chip has multiple functional blocks. Each functional block has >> its own thermal zone. >> Functional blocks and their thermal zones enabled/disabled based on end product. >> Few functional blocks need to disabled for few products so thermal >> zones also need to disable. >> In that case, nodes of specific thermal zones are removed from DTS >> file of corresponding product. >> >> 2. Thermal framework provides sysfs interface to configure thermal >> zones and read temperature of thermal zone. >> To configure individual thermal zone, we need to have separate DT node. >> Same to read temperature of individual thermal zone. >> Ex: To read temperature of thermal zone 0. >> cat /sys/class/thermal/thermal_zone0/temp >> To configure trip temperature of thermal zone 0. >> echo 110000 > /sys/class/thermal/thermal_zone0/trip_point_0_temp >> >> Also to avoid driver source change for the multiple products it is >> clean to have multiple DT nodes. >> >>> Rob -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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>; + }; + };