On Thu, 6 Sep 2018 13:27:34 +0000
Manish Narani [off-list ref] wrote:

Hi Jonathan,

Thanks for the review!

quoted

-----Original Message-----
From: Jonathan Cameron [mailto:jic23 at kernel.org]
Sent: Sunday, September 2, 2018 11:45 PM
To: Manish Narani <redacted>
Cc: knaack.h at gmx.de; lars at metafoo.de; pmeerw at pmeerw.net;
robh+dt at kernel.org; mark.rutland at arm.com; Michal Simek
[off-list ref]; leoyang.li at nxp.com; sudeep.holla at arm.com;
amit.kucheria at linaro.org; broonie at kernel.org; arnaud.pouliquen at st.com;
geert at linux-m68k.org; eugen.hristev at microchip.com; rdunlap at infradead.org;
lukas at wunner.de; freeman.liu at spreadtrum.com; vilhelm.gray at gmail.com;
tglx at linutronix.de; baolin.wang at linaro.org; gregkh at linuxfoundation.org;
Srinivas Goud [off-list ref]; Anirudha Sarangi [off-list ref];
linux-iio at vger.kernel.org; devicetree at vger.kernel.org; linux-arm-
kernel at lists.infradead.org; linux-kernel at vger.kernel.org
Subject: Re: [PATCH 1/3] dt-bindings: iio: adc: Add Xilinx AMS binding
documentation

On Thu, 30 Aug 2018 15:52:17 +0530
Manish Narani [off-list ref] wrote:
  

quoted

Xilinx AMS have several ADC channels that can be used for measurement
of different voltages and temperatures. Document the same in the bindings.

Signed-off-by: Manish Narani <redacted>
---
 .../devicetree/bindings/iio/adc/xilinx-ams.txt     | 159  

+++++++++++++++++++++  

quoted

 1 file changed, 159 insertions(+)
 create mode 100644
Documentation/devicetree/bindings/iio/adc/xilinx-ams.txt

diff --git a/Documentation/devicetree/bindings/iio/adc/xilinx-ams.txt

b/Documentation/devicetree/bindings/iio/adc/xilinx-ams.txt
new file mode 100644
index 0000000..8cc96f0

--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/xilinx-ams.txt

@@ -0,0 +1,159 @@
+Xilinx AMS controller
+
+The AMS includes an ADC as well as on-chip sensors that can be used
+to sample external voltages and monitor on-die operating conditions,
+such as temperature and supply voltage levels. The AMS has two SYSMON  

blocks.  

quoted

+PL-SYSMON block is capable of monitoring off chip voltage and  

temperature.  

quoted

+PL-SYSMON block has DRP, JTAG and I2C interface to enable monitoring
+from external master. Out of this interface currently only DRP is supported.
+Other block PS-SYSMON is memory mapped to PS. Both of block has
+built-in alarm generation logic that is used to interrupt the
+processor based on condition set.  

I guess anyone reading this because they have the hardware would know what
PS and PL are, but it would still be nice to define those acronyms!  

Okay, I will rectify this in v2.

quoted

Google suggest, Programmable Logic (FPGA bit I guess) and Process Space
(Arm core bit?)  As I read this, from a driver point of view it doesn't really
matter - these are just blocks of channels that are there?  

Yes, these are just blocks of channels, which are enabled/disabled
via devicetree child nodes "xlnx,zynqmp-ams-ps" and
"xlnx,zynqmp-ams-pl".

Great, put please expand in here on the naming.  This may not
matter for people using this particular driver, but having clear
human readable descriptions is useful when we are looking for
similarities between this an future drivers etc.

quoted

This binding is complex enough I definitely want some DT binding
review.  

Sure!

quoted

quoted

+
+All designs should have AMS registers, but PS and PL are
optional. +The AMS controller can work with only PS, only PL and
both PS and PL +configurations. Please specify registers
according to your design. +Devicetree should always have AMS
module property. Providing PS & PL  

module is optional.  

quoted

+
+Required properties:
+	- compatible: Should be "xlnx,zynqmp-ams"
+	- reg:  Should specify AMS register space
+	- interrupts: Interrupt number for the AMS control
interface
+	- interrupt-names: Interrupt name, must be "ams-irq"
+	- clocks: Should contain a clock specifier for the device
+	- ranges: keep the property empty to map child address
space
+	          (for PS and/or PL) nodes 1:1 onto the parent
address
+	          space
+
+AMS device tree subnode:
+	- compatible: Should be "xlnx,zynqmp-ams-ps" or
"xlnx,zynqmp-ams-pl"
+	- reg:  Register space for PS or PL
+
+Optional properties:
+
+Following optional property only valid for PL.
+	- xlnx,ext-channels: List of external channels that are
connected to the
+	                     AMS PL module.
+
+	  The child nodes of this node represent the external
channels which  

are  

quoted

+	  connected to the AMS Module. If the property is not
present
+	  no external channels will be assumed to be connected.
+
+	  Each child node represents one channel and has the
following
+	  properties:
+		Required properties:
+			* reg: Pair of pins the channel is
connected to.
+				0: VP/VN  

Hmm. So we have this table here of the ones that may or may not be
connected with different numbering from the overall table.  Given
these indexes are arbitrary can we unify them?

I.e. use the index from below but state here which values can be
used?  

Okay. I guess you are suggesting to use the channel index from the
channel details table given in the end of this file and state here
which number (for 'reg' value) should be used for given channel. Is
this correct?

Exactly right.

quoted

quoted

+				1: VUSER0
+				2: VUSER1
+				3: VUSER3
+				4: VUSER4
+				5: VAUXP[0]/VAUXN[0]
+				6: VAUXP[1]/VAUXN[1]
+				...
+				20: VAUXP[15]/VAUXN[15]
+			  Note each channel number should only be used at  

most  

quoted

+			  once.
+		Optional properties:
+			* xlnx,bipolar: If set the channel is used in bipolar
+			  mode.
+
+
+Example:
+	xilinx_ams: ams at ffa50000 {
+		compatible = "xlnx,zynqmp-ams";
+		interrupt-parent = <&gic>;
+		interrupts = <0 56 4>;
+		interrupt-names = "ams-irq";
+		clocks = <&clkc 70>;
+		reg = <0x0 0xffa50000 0x0 0x800>;
+		reg-names = "ams-base";
+		#address-cells = <2>;
+		#size-cells = <2>;
+		ranges;
+
+		ams_ps: ams_ps at ffa50800 {
+			compatible = "xlnx,zynqmp-ams-ps";
+			reg = <0x0 0xffa50800 0x0 0x400>;
+		};
+
+		ams_pl: ams_pl at ffa50c00 {
+			compatible = "xlnx,zynqmp-ams-pl";
+			reg = <0x0 0xffa50c00 0x0 0x400>;
+			xlnx,ext-channels {
+				#address-cells = <1>;
+				#size-cells = <0>;
+				channel at 0 {
+					reg = <0>;
+					xlnx,bipolar;
+				};
+				channel at 1 {
+					reg = <1>;
+				};
+				channel at 8 {
+					reg = <8>;
+					xlnx,bipolar;
+				};
+			};
+		};
+	};
+
+AMS Channels Details:
+
+Sysmon Block	|Channel|			Details  

			|Measurement  

quoted

+		 Number  

			 Type  

quoted

+---------------------------------------------------------------------------------------------------  

------  

quoted

+AMS CTRL	|0	|System PLLs voltage measurement, VCC_PSPLL.  

		|Voltage  

quoted

+		|1	|Battery voltage measurement, VCC_PSBATT.  

		|Voltage  

quoted

+		|2	|PL Internal voltage measurement, VCCINT.  

		|Voltage  

quoted

+		|3	|Block RAM voltage measurement, VCCBRAM.  

		|Voltage  

quoted

+		|4	|PL Aux voltage measurement, VCCAUX.  

		|Voltage  

quoted

+		|5	|Voltage measurement for six DDR I/O PLLs,  

VCC_PSDDR_PLL.	|Voltage  

quoted

+		|6	|VCC_PSINTFP_DDR voltage measurement.  

			|Voltage  

quoted

+---------------------------------------------------------------------------------------------------  

------  

quoted

+PS Sysmon	|7	|LPD temperature measurement.  

			|Temperature  

quoted

+		|8	|FPD Temperature Measurment (REMOTE).  

			|Temperature  

quoted

+		|9	|VCC PS LPD voltage measurement (supply1).  

		|Voltage  

quoted

+		|10	|VCC PS FPD voltage measurement (supply2).  

		|Voltage  

quoted

+		|11	|PS Aux voltage reference (supply3).  

		|Voltage  

quoted

+		|12	|DDR I/O VCC voltage measurement.  

		|Voltage  

quoted

+		|13	|PS IO Bank 503 voltage measurement (supply5).  

			|Voltage  

quoted

+		|14	|PS IO Bank 500 voltage measurement (supply6).  

			|Voltage  

quoted

+		|15	|VCCO_PSIO1 voltage measurement.  

		|Voltage  

quoted

+		|16	|VCCO_PSIO2 voltage measurement.  

		|Voltage  

quoted

+		|17	|VCC_PS_GTR voltage measurement  

(VPS_MGTRAVCC).			|Voltage  

quoted

+		|18	|VTT_PS_GTR voltage measurement  

(VPS_MGTRAVTT).			|Voltage  

quoted

+		|19	|VCC_PSADC voltage measurement.  

			|Voltage  

quoted

+---------------------------------------------------------------------------------------------------  

------  

quoted

+PL Sysmon	|20	|PL Temperature measurement.  

			|Temperature  

quoted

+		|21	|PL Internal Voltage Voltage measurement, VCCINT.  

		|Voltage  

quoted

+		|22	|PL Auxiliary Voltage measurement, VCCAUX.  

		|Voltage  

quoted

+		|23	|ADC Reference P+ Voltage measurement.  

			|Voltage  

quoted

+		|24	|ADC Reference N- Voltage measurement.  

			|Voltage  

quoted

+		|25	|PL Block RAM Voltage measurement, VCCBRAM.  

			|Voltage  

quoted

+		|26	|LPD Internal Voltage measurement, VCC_PSINTLP  

(supply4).	|Voltage  

quoted

+		|27	|FPD Internal Voltage measurement, VCC_PSINTFP  

(supply5).	|Voltage  

quoted

+		|28	|PS Auxiliary Voltage measurement (supply6).  

		|Voltage  

quoted

+		|29	|PL VCCADC Voltage measurement (vccams).  

		|Voltage  

quoted

+		|30	|Differencial analog input signal Voltage measurment.  

		|Voltage  

quoted

+		|31	|VUser0 Voltage measurement (supply7).  

			|Voltage  

quoted

+		|32	|VUser1 Voltage measurement (supply8).  

			|Voltage  

quoted

+		|33	|VUser2 Voltage measurement (supply9).  

			|Voltage  

quoted

+		|34	|VUser3 Voltage measurement (supply10).  

			|Voltage  

quoted

+		|35	|Auxiliary ch 0 Voltage measurement (VAux0).  

		|Voltage  

quoted

+		|36	|Auxiliary ch 1 Voltage measurement (VAux1).  

		|Voltage  

quoted

+		|37	|Auxiliary ch 2 Voltage measurement (VAux2).  

		|Voltage  

quoted

+		|38	|Auxiliary ch 3 Voltage measurement (VAux3).  

		|Voltage  

quoted

+		|39	|Auxiliary ch 4 Voltage measurement (VAux4).  

		|Voltage  

quoted

+		|40	|Auxiliary ch 5 Voltage measurement (VAux5).  

		|Voltage  

quoted

+		|41	|Auxiliary ch 6 Voltage measurement (VAux6).  

		|Voltage  

quoted

+		|42	|Auxiliary ch 7 Voltage measurement (VAux7).  

		|Voltage  

quoted

+		|43	|Auxiliary ch 8 Voltage measurement (VAux8).  

		|Voltage  

quoted

+		|44	|Auxiliary ch 9 Voltage measurement (VAux9).  

		|Voltage  

quoted

+		|45	|Auxiliary ch 10 Voltage measurement (VAux10).  

			|Voltage  

quoted

+		|46	|Auxiliary ch 11 Voltage measurement (VAux11).  

			|Voltage  

quoted

+		|47	|Auxiliary ch 12 Voltage measurement (VAux12).  

			|Voltage  

quoted

+		|48	|Auxiliary ch 13 Voltage measurement (VAux13).  

			|Voltage  

quoted

+		|49	|Auxiliary ch 14 Voltage measurement (VAux14).  

			|Voltage  

quoted

+		|50	|Auxiliary ch 15 Voltage measurement (VAux15).  

			|Voltage  

quoted

+---------------------------------------------------------------------
+------------------------------------  

Thanks,

Jonathan  

Thanks,
Manish Narani