RE: [PATCH V7 1/4] dt-bindings: fsl: scu: add thermal binding

[Anson Huang]

Best Regards!
Anson Huang

> -----Original Message-----
> From: Aisheng Dong
> Sent: 2019年2月20日 15:45
> To: Anson Huang <anson.huang@xxxxxxx>; mark.rutland@xxxxxxx;
> shawnguo@xxxxxxxxxx; s.hauer@xxxxxxxxxxxxxx; kernel@xxxxxxxxxxxxxx;
> festevam@xxxxxxxxx; catalin.marinas@xxxxxxx; will.deacon@xxxxxxx;
> rui.zhang@xxxxxxxxx; edubezval@xxxxxxxxx; daniel.lezcano@xxxxxxxxxx;
> ulf.hansson@xxxxxxxxxx; Daniel Baluta <daniel.baluta@xxxxxxx>;
> horms+renesas@xxxxxxxxxxxx; Andy Gross <andy.gross@xxxxxxxxxx>;
> heiko@xxxxxxxxx; arnd@xxxxxxxx; olof@xxxxxxxxx;
> bjorn.andersson@xxxxxxxxxx; jagan@xxxxxxxxxxxxxxxxxxxx;
> enric.balletbo@xxxxxxxxxxxxx; marc.w.gonzalez@xxxxxxx;
> devicetree@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; linux-arm-
> kernel@xxxxxxxxxxxxxxxxxxx; linux-pm@xxxxxxxxxxxxxxx
> Cc: dl-linux-imx <linux-imx@xxxxxxx>
> Subject: RE: [PATCH V7 1/4] dt-bindings: fsl: scu: add thermal binding
> 
> > From: Anson Huang
> > Sent: Wednesday, February 20, 2019 2:54 PM
> >
> > NXP i.MX8QXP is an ARMv8 SoC with a Cortex-M4 core inside as system
> > controller, the system controller is in charge of system power, clock
> > and thermal sensors etc. management, Linux kernel has to communicate
> > with system controller via MU (message unit) IPC to get temperature
> > from thermal sensors, this patch adds binding doc for i.MX system
> controller thermal driver.
> >
> > Signed-off-by: Anson Huang <Anson.Huang@xxxxxxx>
> > Reviewed-by: Rob Herring <robh@xxxxxxxxxx>
> > ---
> > Changes since V6:
> > 	- put "imx,sensor-resource-id" property in thermal zone nodes.
> > ---
> >  .../devicetree/bindings/arm/freescale/fsl,scu.txt  | 27
> > ++++++++++++++++++++++
> >  1 file changed, 27 insertions(+)
> >
> > diff --git
> > a/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt
> > b/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt
> > index 72d481c..ad96881 100644
> > --- a/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt
> > +++ b/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt
> > @@ -122,6 +122,27 @@ RTC bindings based on SCU Message Protocol
> > Required properties:
> >  - compatible: should be "fsl,imx8qxp-sc-rtc";
> >
> > +Thermal bindings based on SCU Message Protocol
> > +------------------------------------------------------------
> > +
> > +Required properties:
> > +- compatible : Must be "fsl,imx-sc-thermal";
> > +- tsens-num : Total number of thermal sensors supported;
> 
> We properly don't need this property if the number of sensor is fixed on a
> specific SoC.

After further check, I think we can use " for_each_available_child_of_node "
to get every thermal zone's HW resource ID and remove this property.

> 
> > +- #thermal-sensor-cells : Should be 1. See
> > +
> Documentation/devicetree/bindings/thermal/thermal.txt
> > +			  for a description.
> > +
> > +Required properties for thermal zone nodes:
> > +- imx,sensor-resource-id : This property should be defined in each
> > +thermal
> 
> After a further looking at it, I wonder this property may not needed either.
> 

If we do NOT have such property, we have to do the mapping in driver to get
resource ID, for different SoC, we will have to create different mapping table,
the thermal zone ID given by common thermal framework is virtual ID starting
from 0. Adding this property in thermal zone node is to make SC thermal driver
more generic for other SoCs.

> > zone
> > +			   of thermal-zones node, it passes each thermal
> zone's
> > +			   resource id for thermal driver to get temperature
> via
> > +			   SCU IPC.
> > +
> > +			   For thermal zone nodes, please refer to below
> binding
> > +			   doc for details:
> > +
> > +[1] Documentation/devicetree/bindings/thermal/thermal.txt
> > +
> >  Example (imx8qxp):
> >  -------------
> >  lsio_mu1: mailbox@5d1c0000 {
> > @@ -168,6 +189,12 @@ firmware {
> >  		rtc: rtc {
> >  			compatible = "fsl,imx8qxp-sc-rtc";
> >  		};
> > +
> > +		tsens: thermal-sensor {
> > +			compatible = "fsl,imx8qxp-sc-thermal";
> > +			tsens-num = <1>;
> > +			#thermal-sensor-cells = <1>;
> 
> This looks may not correct if sensor cells is 1 but the sensor number Is also 1.
> So we probably better remove tsens-num property.

Yes, this should be 0 if 1 thermal zone ONLY have 1 thermal sensor, later we will
add other thermal zone like DDR zone, currently there is still some issues so SCFW
does NOT provide support for this zone, after issue fixed, SCFW may add this thermal
zone's support. Will fix them in next version.

Thanks,
Anson.

> 
> Regards
> Dong Aisheng
> 
> > +		};
> >  	};
> >  };
> >
> > --
> > 2.7.4