linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/4] Add Xilinx AMS Driver
@ 2018-09-14  7:18 Manish Narani
  2018-09-14  7:18 ` [PATCH v2 1/4] dt-bindings: iio: adc: Add Xilinx AMS binding documentation Manish Narani
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: Manish Narani @ 2018-09-14  7:18 UTC (permalink / raw)
  To: jic23, knaack.h, lars, pmeerw, michal.simek, robh+dt,
	mark.rutland, manish.narani, sudeep.holla, amit.kucheria,
	leoyang.li, broonie, arnaud.pouliquen, eugen.hristev, rdunlap,
	geert, ak, freeman.liu, lukas, vilhelm.gray, gregkh, kstewart
  Cc: sgoud, anirudh, linux-iio, linux-arm-kernel, linux-kernel, devicetree

Add Xilinx AMS driver which is used for Xilinx's ZynqMP AMS controller.
This AMS driver is used to report various interface voltages and temperatures
across the system.
This driver handles AMS module including PS-Sysmon & PL-Sysmon. The binding
documentation is added for understanding of AMS, PS, PL Sysmon Channels.

Changes in v2:
	- Added documentation for sysfs (Patch-2)
	- Addressed code style review comments
	- Patch-2 (Now it is Patch-3)
		- Arranged the includes in alphabetical order
		- Removed the wrapper 'ams_pl_write_reg()' and used writel
		  instead
		- Removed the unnecessary delay of 1ms and used polling of EOC
		  instead
		- Removed spin_lock and used mutex only.
		- Used request_irq() instead of devm_request_irq() and handled
		  respective error conditions
		- Moved contents of xilinx-ams.h to inline with xilinx-ams.c
	- Patch-1
		- Addressed Documentation style comments

Manish Narani (4):
  dt-bindings: iio: adc: Add Xilinx AMS binding documentation
  iio: Documentation: Add Xilinx AMS sysfs documentation
  iio: adc: Add Xilinx AMS driver
  arm64: zynqmp: DT: Add Xilinx AMS node

 .../ABI/testing/sysfs-bus-iio-adc-xilinx-ams       |  246 ++++
 .../devicetree/bindings/iio/adc/xilinx-ams.txt     |  180 +++
 arch/arm64/boot/dts/xilinx/zynqmp.dtsi             |   26 +
 drivers/iio/adc/Kconfig                            |   10 +
 drivers/iio/adc/Makefile                           |    1 +
 drivers/iio/adc/xilinx-ams.c                       | 1337 ++++++++++++++++++++
 6 files changed, 1800 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-adc-xilinx-ams
 create mode 100644 Documentation/devicetree/bindings/iio/adc/xilinx-ams.txt
 create mode 100644 drivers/iio/adc/xilinx-ams.c

-- 
2.1.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2 1/4] dt-bindings: iio: adc: Add Xilinx AMS binding documentation
  2018-09-14  7:18 [PATCH v2 0/4] Add Xilinx AMS Driver Manish Narani
@ 2018-09-14  7:18 ` Manish Narani
  2018-09-14  7:18 ` [PATCH v2 2/4] iio: Documentation: Add Xilinx AMS sysfs documentation Manish Narani
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 12+ messages in thread
From: Manish Narani @ 2018-09-14  7:18 UTC (permalink / raw)
  To: jic23, knaack.h, lars, pmeerw, michal.simek, robh+dt,
	mark.rutland, manish.narani, sudeep.holla, amit.kucheria,
	leoyang.li, broonie, arnaud.pouliquen, eugen.hristev, rdunlap,
	geert, ak, freeman.liu, lukas, vilhelm.gray, gregkh, kstewart
  Cc: sgoud, anirudh, linux-iio, linux-arm-kernel, linux-kernel, devicetree

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 <manish.narani@xilinx.com>
---
 .../devicetree/bindings/iio/adc/xilinx-ams.txt     | 180 +++++++++++++++++++++
 1 file changed, 180 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..d0958a1
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/xilinx-ams.txt
@@ -0,0 +1,180 @@
+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 which are PL (Programmable Logic) SYSMON and
+PS (Processing System) SYSMON.
+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.
+
+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 sub-nodes:
+		- ams_ps : Used as PS-SYSMON node
+		- ams_pl : Used as PL-SYSMON node
+
+
+
+AMS PS-SYSMON
+`````````````
+PS (Processing System) SYSMON is memory mapped to PS. This block has built-in
+alarm generation logic that is used to interrupt the processor based on
+condition set.
+
+Required properties:
+	- compatible: Should be "xlnx,zynqmp-ams-ps"
+	- reg: Register space for PS-SYSMON
+
+
+
+AMS PL-SYSMON
+`````````````
+PL-SYSMON is capable of monitoring off chip voltage and temperature. PL-SYSMON
+block has DRP, JTAG and I2C interface to enable monitoring from external master.
+Out of this interface currently only DRP is supported. This block has alarm
+generation logic that is used to interrupt the processor based on condition set.
+
+Required properties:
+	- compatible: Should be "xlnx,zynqmp-ams-pl"
+	- reg: Register space for PL-SYSMON
+
+PL-SYSMON optional sub-nodes:
+	- xlnx,ext-channels: List of external channels that are connected to the
+	                     AMS PL module.
+
+	  The child nodes of PL-SYSMON represent the external channels which are
+	  connected to this 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.
+
+		    'reg' value		Channel Name		Channel Number
+		    -----------		------------		--------------
+			 0		VP/VN			      30
+			 1		VUSER0			      31
+			 2		VUSER1			      32
+			 3		VUSER3			      33
+			 4		VUSER4			      34
+			 5		VAUXP[0]/VAUXN[0]	      35
+			 6		VAUXP[1]/VAUXN[1]	      36
+			...
+			20		VAUXP[15]/VAUXN[15]	      50
+
+		Each channel number should only be used at most once. For
+		more details on channels, refer table given at the end.
+
+	Optional properties:
+		* xlnx,bipolar: If set the channel is used in bipolar
+		  mode.
+
+
+Example:
+	xilinx_ams: ams@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";
+		ranges;
+
+		ams_ps: ams_ps@ffa50800 {
+			compatible = "xlnx,zynqmp-ams-ps";
+			reg = <0x0 0xffa50800 0x0 0x400>;
+		};
+
+		ams_pl: ams_pl@ffa50c00 {
+			compatible = "xlnx,zynqmp-ams-pl";
+			reg = <0x0 0xffa50c00 0x0 0x400>;
+			xlnx,ext-channels {
+				channel@0 {
+					reg = <0>;
+					xlnx,bipolar;
+				};
+				channel@1 {
+					reg = <1>;
+				};
+				channel@8 {
+					reg = <8>;
+					xlnx,bipolar;
+				};
+			};
+		};
+	};
+
+
+AMS Channels Details
+````````````````````
+Sysmon Block	|Channel|			Details					|Measurement
+		 Number									 Type
+---------------------------------------------------------------------------------------------------------
+AMS CTRL	|0	|System PLLs voltage measurement, VCC_PSPLL.			|Voltage
+		|1	|Battery voltage measurement, VCC_PSBATT.			|Voltage
+		|2	|PL Internal voltage measurement, VCCINT.			|Voltage
+		|3	|Block RAM voltage measurement, VCCBRAM.			|Voltage
+		|4	|PL Aux voltage measurement, VCCAUX.				|Voltage
+		|5	|Voltage measurement for six DDR I/O PLLs, VCC_PSDDR_PLL.	|Voltage
+		|6	|VCC_PSINTFP_DDR voltage measurement.				|Voltage
+---------------------------------------------------------------------------------------------------------
+PS Sysmon	|7	|LPD temperature measurement.					|Temperature
+		|8	|FPD Temperature Measurment (REMOTE).				|Temperature
+		|9	|VCC PS LPD voltage measurement (supply1).			|Voltage
+		|10	|VCC PS FPD voltage measurement (supply2).			|Voltage
+		|11	|PS Aux voltage reference (supply3).				|Voltage
+		|12	|DDR I/O VCC voltage measurement.				|Voltage
+		|13	|PS IO Bank 503 voltage measurement (supply5).			|Voltage
+		|14	|PS IO Bank 500 voltage measurement (supply6).			|Voltage
+		|15	|VCCO_PSIO1 voltage measurement.				|Voltage
+		|16	|VCCO_PSIO2 voltage measurement.				|Voltage
+		|17	|VCC_PS_GTR voltage measurement (VPS_MGTRAVCC).			|Voltage
+		|18	|VTT_PS_GTR voltage measurement (VPS_MGTRAVTT).			|Voltage
+		|19	|VCC_PSADC voltage measurement.					|Voltage
+---------------------------------------------------------------------------------------------------------
+PL Sysmon	|20	|PL Temperature measurement.					|Temperature
+		|21	|PL Internal Voltage Voltage measurement, VCCINT.		|Voltage
+		|22	|PL Auxiliary Voltage measurement, VCCAUX.			|Voltage
+		|23	|ADC Reference P+ Voltage measurement.				|Voltage
+		|24	|ADC Reference N- Voltage measurement.				|Voltage
+		|25	|PL Block RAM Voltage measurement, VCCBRAM.			|Voltage
+		|26	|LPD Internal Voltage measurement, VCC_PSINTLP (supply4).	|Voltage
+		|27	|FPD Internal Voltage measurement, VCC_PSINTFP (supply5).	|Voltage
+		|28	|PS Auxiliary Voltage measurement (supply6).			|Voltage
+		|29	|PL VCCADC Voltage measurement (vccams).			|Voltage
+		|30	|Differencial analog input signal Voltage measurment.		|Voltage
+		|31	|VUser0 Voltage measurement (supply7).				|Voltage
+		|32	|VUser1 Voltage measurement (supply8).				|Voltage
+		|33	|VUser2 Voltage measurement (supply9).				|Voltage
+		|34	|VUser3 Voltage measurement (supply10).				|Voltage
+		|35	|Auxiliary ch 0 Voltage measurement (VAux0).			|Voltage
+		|36	|Auxiliary ch 1 Voltage measurement (VAux1).			|Voltage
+		|37	|Auxiliary ch 2 Voltage measurement (VAux2).			|Voltage
+		|38	|Auxiliary ch 3 Voltage measurement (VAux3).			|Voltage
+		|39	|Auxiliary ch 4 Voltage measurement (VAux4).			|Voltage
+		|40	|Auxiliary ch 5 Voltage measurement (VAux5).			|Voltage
+		|41	|Auxiliary ch 6 Voltage measurement (VAux6).			|Voltage
+		|42	|Auxiliary ch 7 Voltage measurement (VAux7).			|Voltage
+		|43	|Auxiliary ch 8 Voltage measurement (VAux8).			|Voltage
+		|44	|Auxiliary ch 9 Voltage measurement (VAux9).			|Voltage
+		|45	|Auxiliary ch 10 Voltage measurement (VAux10).			|Voltage
+		|46	|Auxiliary ch 11 Voltage measurement (VAux11).			|Voltage
+		|47	|Auxiliary ch 12 Voltage measurement (VAux12).			|Voltage
+		|48	|Auxiliary ch 13 Voltage measurement (VAux13).			|Voltage
+		|49	|Auxiliary ch 14 Voltage measurement (VAux14).			|Voltage
+		|50	|Auxiliary ch 15 Voltage measurement (VAux15).			|Voltage
+---------------------------------------------------------------------------------------------------------
-- 
2.1.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 2/4] iio: Documentation: Add Xilinx AMS sysfs documentation
  2018-09-14  7:18 [PATCH v2 0/4] Add Xilinx AMS Driver Manish Narani
  2018-09-14  7:18 ` [PATCH v2 1/4] dt-bindings: iio: adc: Add Xilinx AMS binding documentation Manish Narani
@ 2018-09-14  7:18 ` Manish Narani
  2018-09-16 10:12   ` Jonathan Cameron
  2018-09-14  7:18 ` [PATCH v2 3/4] iio: adc: Add Xilinx AMS driver Manish Narani
  2018-09-14  7:18 ` [PATCH v2 4/4] arm64: zynqmp: DT: Add Xilinx AMS node Manish Narani
  3 siblings, 1 reply; 12+ messages in thread
From: Manish Narani @ 2018-09-14  7:18 UTC (permalink / raw)
  To: jic23, knaack.h, lars, pmeerw, michal.simek, robh+dt,
	mark.rutland, manish.narani, sudeep.holla, amit.kucheria,
	leoyang.li, broonie, arnaud.pouliquen, eugen.hristev, rdunlap,
	geert, ak, freeman.liu, lukas, vilhelm.gray, gregkh, kstewart
  Cc: sgoud, anirudh, linux-iio, linux-arm-kernel, linux-kernel, devicetree

Add documentation for xilinx-ams driver. This contains information about
various voltages and temperatures on PS (Processing System), PL
(Programmable Logic) and AMS Control Block.

Signed-off-by: Manish Narani <manish.narani@xilinx.com>
---
 .../ABI/testing/sysfs-bus-iio-adc-xilinx-ams       | 246 +++++++++++++++++++++
 1 file changed, 246 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-adc-xilinx-ams

diff --git a/Documentation/ABI/testing/sysfs-bus-iio-adc-xilinx-ams b/Documentation/ABI/testing/sysfs-bus-iio-adc-xilinx-ams
new file mode 100644
index 0000000..589c389
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-iio-adc-xilinx-ams
@@ -0,0 +1,246 @@
+What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_vccpsintlp
+Date:		September 2018
+KernelVersion:	4.19.0
+Contact:	mnarani@xilinx.com
+Description:
+		Voltage input for PS (Processing System) LPD (Low Power Domain)
+		power supply.
+
+
+What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_vccpsintfp
+Date:		September 2018
+KernelVersion:	4.19.0
+Contact:	mnarani@xilinx.com
+Description:
+		Voltage input for PS (Processing System) FPD (Full Power Domain)
+		power supply.
+
+
+What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_vccpsaux
+Date:		September 2018
+KernelVersion:	4.19.0
+Contact:	mnarani@xilinx.com
+Description:
+		Voltage input for PS (Processing System) Auxiliary power
+		supply.
+
+
+What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_vccpsddr
+Date:		September 2018
+KernelVersion:	4.19.0
+Contact:	mnarani@xilinx.com
+Description:
+		Voltage input for PS (Processing System) I/O bank 504 (DDR
+		PHY) power supply.
+
+
+What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_vccpsio3
+Date:		September 2018
+KernelVersion:	4.19.0
+Contact:	mnarani@xilinx.com
+Description:
+		Voltage input for PS (Processing System) I/O bank 503
+		(boot, config, JTAG, SRST, POR) power supply.
+
+
+What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_vccpsio0
+Date:		September 2018
+KernelVersion:	4.19.0
+Contact:	mnarani@xilinx.com
+Description:
+		Voltage input for PS (Processing System) I/O bank 500
+		(MIO[0:25]) power supply.
+
+
+What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_vccpsio1
+Date:		September 2018
+KernelVersion:	4.19.0
+Contact:	mnarani@xilinx.com
+Description:
+		Voltage input for PS (Processing System) I/O bank 501
+		(MIO[26:51]) power supply.
+
+
+What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_vccpsio2
+Date:		September 2018
+KernelVersion:	4.19.0
+Contact:	mnarani@xilinx.com
+Description:
+		Voltage input for PS (Processing System) I/O bank 502
+		(MIO[52:77]) power supply.
+
+
+What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_psmgtravcc
+Date:		September 2018
+KernelVersion:	4.19.0
+Contact:	mnarani@xilinx.com
+Description:
+		Voltage input for PS (Processing System) GTR SerDes I/O
+		power supply.
+
+
+What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_psmgtravtt
+Date:		September 2018
+KernelVersion:	4.19.0
+Contact:	mnarani@xilinx.com
+Description:
+		Voltage input for PS (Processing System) GTR SerDes
+		terminators power supply.
+
+
+What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_vccams
+Date:		September 2018
+KernelVersion:	4.19.0
+Contact:	mnarani@xilinx.com
+Description:
+		Voltage input for PS (Processing System) SYSMON ADC circuitry
+		power supply.
+
+
+What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_vccint
+Date:		September 2018
+KernelVersion:	4.19.0
+Contact:	mnarani@xilinx.com
+Description:
+		Voltage input for PL (Programmable Logic) internal power supply.
+
+
+What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_vccvrefp
+Date:		September 2018
+KernelVersion:	4.19.0
+Contact:	mnarani@xilinx.com
+Description:
+		Voltage input for PL (Programmable Logic) ADC positive V
+		reference power supply.
+
+
+What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_vccvrefn
+Date:		September 2018
+KernelVersion:	4.19.0
+Contact:	mnarani@xilinx.com
+Description:
+		Voltage input for PL (Programmable Logic) ADC negative V
+		reference power supply.
+
+
+What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_vccplintlp
+Date:		September 2018
+KernelVersion:	4.19.0
+Contact:	mnarani@xilinx.com
+Description:
+		Voltage input for PL (Programmable Logic) LPD (Low Power Domain)
+		power supply.
+
+
+What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_vccplintfp
+Date:		September 2018
+KernelVersion:	4.19.0
+Contact:	mnarani@xilinx.com
+Description:
+		Voltage input for PL (Programmable Logic) FPD (Full Power
+		Domain) power supply.
+
+
+What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_vccplaux
+Date:		September 2018
+KernelVersion:	4.19.0
+Contact:	mnarani@xilinx.com
+Description:
+		Voltage input for PL (Programmable Logic) Auxiliary power
+		supply.
+
+
+What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_vccvpvn
+Date:		September 2018
+KernelVersion:	4.19.0
+Contact:	mnarani@xilinx.com
+Description:
+		Voltage input for PL (Programmable Logic) Analog Input Pins
+		power supply.
+
+
+What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_vuserZ
+Date:		September 2018
+KernelVersion:	4.19.0
+Contact:	mnarani@xilinx.com
+Description:
+		Voltage input for PL (Programmable Logic) Fabric Analog Wires 
+		power supply. Z is the number in range of 0 to 3.
+
+
+What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_vcc_pspll0
+Date:		September 2018
+KernelVersion:	4.19.0
+Contact:	mnarani@xilinx.com
+Description:
+		Voltage input for AMS (Analog Monitoring System) Systems PLLs
+		power supply.
+
+
+What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_vcc_psbatt
+Date:		September 2018
+KernelVersion:	4.19.0
+Contact:	mnarani@xilinx.com
+Description:
+		Voltage input for AMS (Analog Monitoring System) Battery power
+		supply.
+
+
+What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_vccbram
+Date:		September 2018
+KernelVersion:	4.19.0
+Contact:	mnarani@xilinx.com
+Description:
+		Voltage input for AMS (Analog Monitoring System) Block RAM power
+		supply.
+
+
+What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_vccaux
+Date:		September 2018
+KernelVersion:	4.19.0
+Contact:	mnarani@xilinx.com
+Description:
+		Voltage input for AMS (Analog Monitoring System) Auxiliary power
+		supply.
+
+
+What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_psddrpll
+Date:		September 2018
+KernelVersion:	4.19.0
+Contact:	mnarani@xilinx.com
+Description:
+		Voltage input for AMS (Analog Monitoring System) DDR I/O
+		PLLs [0:5] power supply.
+
+
+What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_psintfpddr
+Date:		September 2018
+KernelVersion:	4.19.0
+Contact:	mnarani@xilinx.com
+Description:
+		Voltage input for AMS (Analog Monitoring System) DDR controller
+		power supply.
+
+
+What:		/sys/bus/iio/devices/iio:deviceX/in_tempY_ps_temp
+Date:		September 2018
+KernelVersion:	4.19.0
+Contact:	mnarani@xilinx.com
+Description:
+		Temperature input for PS (Processing System) RPU MPCore.
+
+
+What:		/sys/bus/iio/devices/iio:deviceX/in_tempY_remote_temp
+Date:		September 2018
+KernelVersion:	4.19.0
+Contact:	mnarani@xilinx.com
+Description:
+		Temperature input for PS (Processing System) APU MPCore.
+
+
+What:		/sys/bus/iio/devices/iio:deviceX/in_tempY_pl_temp
+Date:		September 2018
+KernelVersion:	4.19.0
+Contact:	mnarani@xilinx.com
+Description:
+		Temperature input for PL (Programmable Logic) SYSMON.
-- 
2.1.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 3/4] iio: adc: Add Xilinx AMS driver
  2018-09-14  7:18 [PATCH v2 0/4] Add Xilinx AMS Driver Manish Narani
  2018-09-14  7:18 ` [PATCH v2 1/4] dt-bindings: iio: adc: Add Xilinx AMS binding documentation Manish Narani
  2018-09-14  7:18 ` [PATCH v2 2/4] iio: Documentation: Add Xilinx AMS sysfs documentation Manish Narani
@ 2018-09-14  7:18 ` Manish Narani
  2018-09-14 11:59   ` Himanshu Jha
                     ` (2 more replies)
  2018-09-14  7:18 ` [PATCH v2 4/4] arm64: zynqmp: DT: Add Xilinx AMS node Manish Narani
  3 siblings, 3 replies; 12+ messages in thread
From: Manish Narani @ 2018-09-14  7:18 UTC (permalink / raw)
  To: jic23, knaack.h, lars, pmeerw, michal.simek, robh+dt,
	mark.rutland, manish.narani, sudeep.holla, amit.kucheria,
	leoyang.li, broonie, arnaud.pouliquen, eugen.hristev, rdunlap,
	geert, ak, freeman.liu, lukas, vilhelm.gray, gregkh, kstewart
  Cc: sgoud, anirudh, linux-iio, linux-arm-kernel, linux-kernel, devicetree

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.
PL-SYSMON block is capable of monitoring off chip voltage and
temperature.
PL-SYSMON block has DRP, JTAG and I2C interface to enable monitoring
from external master. Out of these interface currently only DRP is
supported.
Other block PS-SYSMON is memory mapped to PS.
The AMS can use internal channels to monitor voltage and temperature as
well as one primary and up to 16 auxiliary channels for measuring
external voltages.
The voltage and temperature monitoring channels also have event
capability which allows to generate an interrupt when their value falls
below or raises above a set threshold.

Signed-off-by: Manish Narani <manish.narani@xilinx.com>
---
 drivers/iio/adc/Kconfig      |   10 +
 drivers/iio/adc/Makefile     |    1 +
 drivers/iio/adc/xilinx-ams.c | 1337 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 1348 insertions(+)
 create mode 100644 drivers/iio/adc/xilinx-ams.c

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 4a75492..405ea00 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -941,4 +941,14 @@ config XILINX_XADC
 	  The driver can also be build as a module. If so, the module will be called
 	  xilinx-xadc.
 
+config XILINX_AMS
+	tristate "Xilinx AMS driver"
+	depends on ARCH_ZYNQMP || COMPILE_TEST
+	depends on HAS_IOMEM
+	help
+	  Say yes here to have support for the Xilinx AMS.
+
+	  The driver can also be build as a module. If so, the module will be called
+	  xilinx-ams.
+
 endmenu
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index 03db7b5..fbfcc45 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -85,4 +85,5 @@ obj-$(CONFIG_VF610_ADC) += vf610_adc.o
 obj-$(CONFIG_VIPERBOARD_ADC) += viperboard_adc.o
 xilinx-xadc-y := xilinx-xadc-core.o xilinx-xadc-events.o
 obj-$(CONFIG_XILINX_XADC) += xilinx-xadc.o
+obj-$(CONFIG_XILINX_AMS) += xilinx-ams.o
 obj-$(CONFIG_SD_ADC_MODULATOR) += sd_adc_modulator.o
diff --git a/drivers/iio/adc/xilinx-ams.c b/drivers/iio/adc/xilinx-ams.c
new file mode 100644
index 0000000..c239a02
--- /dev/null
+++ b/drivers/iio/adc/xilinx-ams.c
@@ -0,0 +1,1337 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Xilinx AMS driver
+ *
+ *  Copyright (C) 2017-2018 Xilinx, Inc.
+ *
+ *  Manish Narani <mnarani@xilinx.com>
+ *  Rajnikant Bhojani <rajnikant.bhojani@xilinx.com>
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/iopoll.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+
+#include <linux/iio/events.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+
+static const unsigned int AMS_UNMASK_TIMEOUT_MS = 500;
+
+/* AMS registers definitions */
+#define AMS_ISR_0			0x010
+#define AMS_ISR_1			0x014
+#define AMS_IER_0			0x020
+#define AMS_IER_1			0x024
+#define AMS_IDR_0			0x028
+#define AMS_IDR_1			0x02c
+#define AMS_PS_CSTS			0x040
+#define AMS_PL_CSTS			0x044
+
+#define AMS_VCC_PSPLL0			0x060
+#define AMS_VCC_PSPLL3			0x06C
+#define AMS_VCCINT			0x078
+#define AMS_VCCBRAM			0x07C
+#define AMS_VCCAUX			0x080
+#define AMS_PSDDRPLL			0x084
+#define AMS_PSINTFPDDR			0x09C
+
+#define AMS_VCC_PSPLL0_CH		48
+#define AMS_VCC_PSPLL3_CH		51
+#define AMS_VCCINT_CH			54
+#define AMS_VCCBRAM_CH			55
+#define AMS_VCCAUX_CH			56
+#define AMS_PSDDRPLL_CH			57
+#define AMS_PSINTFPDDR_CH		63
+
+#define AMS_REG_CONFIG0			0x100
+#define AMS_REG_CONFIG1			0x104
+#define AMS_REG_CONFIG3			0x10C
+#define AMS_REG_SEQ_CH0			0x120
+#define AMS_REG_SEQ_CH1			0x124
+#define AMS_REG_SEQ_CH2			0x118
+
+#define AMS_TEMP			0x000
+#define AMS_SUPPLY1			0x004
+#define AMS_SUPPLY2			0x008
+#define AMS_VP_VN			0x00c
+#define AMS_VREFP			0x010
+#define AMS_VREFN			0x014
+#define AMS_SUPPLY3			0x018
+#define AMS_SUPPLY4			0x034
+#define AMS_SUPPLY5			0x038
+#define AMS_SUPPLY6			0x03c
+#define AMS_SUPPLY7			0x200
+#define AMS_SUPPLY8			0x204
+#define AMS_SUPPLY9			0x208
+#define AMS_SUPPLY10			0x20c
+#define AMS_VCCAMS			0x210
+#define AMS_TEMP_REMOTE			0x214
+
+#define AMS_REG_VAUX(x)			(0x40 + (4*(x)))
+
+#define AMS_PS_RESET_VALUE		0xFFFFU
+#define AMS_PL_RESET_VALUE		0xFFFFU
+
+#define AMS_CONF0_CHANNEL_NUM_MASK	(0x3f << 0)
+
+#define AMS_CONF1_SEQ_MASK		(0xf << 12)
+#define AMS_CONF1_SEQ_DEFAULT		(0 << 12)
+#define AMS_CONF1_SEQ_CONTINUOUS	(2 << 12)
+#define AMS_CONF1_SEQ_SINGLE_CHANNEL	(3 << 12)
+
+#define AMS_REG_SEQ0_MASK		0xFFFF
+#define AMS_REG_SEQ2_MASK		0x3F
+#define AMS_REG_SEQ1_MASK		0xFFFF
+#define AMS_REG_SEQ2_MASK_SHIFT		16
+#define AMS_REG_SEQ1_MASK_SHIFT		22
+
+#define AMS_REGCFG1_ALARM_MASK		0xF0F
+#define AMS_REGCFG3_ALARM_MASK		0x3F
+
+#define AMS_ALARM_TEMP			0x140
+#define AMS_ALARM_SUPPLY1		0x144
+#define AMS_ALARM_SUPPLY2		0x148
+#define AMS_ALARM_SUPPLY3		0x160
+#define AMS_ALARM_SUPPLY4		0x164
+#define AMS_ALARM_SUPPLY5		0x168
+#define AMS_ALARM_SUPPLY6		0x16c
+#define AMS_ALARM_SUPPLY7		0x180
+#define AMS_ALARM_SUPPLY8		0x184
+#define AMS_ALARM_SUPPLY9		0x188
+#define AMS_ALARM_SUPPLY10		0x18c
+#define AMS_ALARM_VCCAMS		0x190
+#define AMS_ALARM_TEMP_REMOTE		0x194
+#define AMS_ALARM_THRESHOLD_OFF_10	0x10
+#define AMS_ALARM_THRESHOLD_OFF_20	0x20
+
+#define AMS_ALARM_THR_DIRECT_MASK	0x01
+#define AMS_ALARM_THR_MIN		0x0000
+#define AMS_ALARM_THR_MAX		0xffff
+
+#define AMS_NO_OF_ALARMS		32
+#define AMS_PL_ALARM_START		16
+#define AMS_ISR0_ALARM_MASK		0xFFFFFFFFU
+#define AMS_ISR1_ALARM_MASK		0xE000001FU
+#define AMS_ISR1_EOC_MASK		0x00000008U
+#define AMS_ISR1_INTR_MASK_SHIFT	32
+#define AMS_ISR0_ALARM_2_TO_0_MASK	0x07
+#define AMS_ISR0_ALARM_6_TO_3_MASK	0x78
+#define AMS_ISR0_ALARM_12_TO_7_MASK	0x3F
+#define AMS_CONF1_ALARM_2_TO_0_SHIFT	1
+#define AMS_CONF1_ALARM_6_TO_3_SHIFT	5
+#define AMS_CONF3_ALARM_12_TO_7_SHIFT	8
+
+#define AMS_PS_CSTS_PS_READY		0x08010000U
+#define AMS_PL_CSTS_ACCESS_MASK		0x00000001U
+
+#define AMS_PL_MAX_FIXED_CHANNEL	10
+#define AMS_PL_MAX_EXT_CHANNEL		20
+
+#define AMS_INIT_TIMEOUT		10000
+
+/*
+ * Following scale and offset value is derived from
+ * UG580 (v1.7) December 20, 2016
+ */
+#define AMS_SUPPLY_SCALE_1VOLT		1000
+#define AMS_SUPPLY_SCALE_3VOLT		3000
+#define AMS_SUPPLY_SCALE_6VOLT		6000
+#define AMS_SUPPLY_SCALE_DIV_BIT	16
+
+#define AMS_TEMP_SCALE			509314
+#define AMS_TEMP_SCALE_DIV_BIT		16
+#define AMS_TEMP_OFFSET			-((280230L << 16) / 509314)
+
+enum ams_alarm_bit {
+	AMS_ALARM_BIT_TEMP,
+	AMS_ALARM_BIT_SUPPLY1,
+	AMS_ALARM_BIT_SUPPLY2,
+	AMS_ALARM_BIT_SUPPLY3,
+	AMS_ALARM_BIT_SUPPLY4,
+	AMS_ALARM_BIT_SUPPLY5,
+	AMS_ALARM_BIT_SUPPLY6,
+	AMS_ALARM_BIT_RESERVED,
+	AMS_ALARM_BIT_SUPPLY7,
+	AMS_ALARM_BIT_SUPPLY8,
+	AMS_ALARM_BIT_SUPPLY9,
+	AMS_ALARM_BIT_SUPPLY10,
+	AMS_ALARM_BIT_VCCAMS,
+	AMS_ALARM_BIT_TEMP_REMOTE
+};
+
+enum ams_seq {
+	AMS_SEQ_VCC_PSPLL,
+	AMS_SEQ_VCC_PSBATT,
+	AMS_SEQ_VCCINT,
+	AMS_SEQ_VCCBRAM,
+	AMS_SEQ_VCCAUX,
+	AMS_SEQ_PSDDRPLL,
+	AMS_SEQ_INTDDR
+};
+
+enum ams_ps_pl_seq {
+	AMS_SEQ_CALIB,
+	AMS_SEQ_RSVD_1,
+	AMS_SEQ_RSVD_2,
+	AMS_SEQ_TEST,
+	AMS_SEQ_RSVD_4,
+	AMS_SEQ_SUPPLY4,
+	AMS_SEQ_SUPPLY5,
+	AMS_SEQ_SUPPLY6,
+	AMS_SEQ_TEMP,
+	AMS_SEQ_SUPPLY2,
+	AMS_SEQ_SUPPLY1,
+	AMS_SEQ_VP_VN,
+	AMS_SEQ_VREFP,
+	AMS_SEQ_VREFN,
+	AMS_SEQ_SUPPLY3,
+	AMS_SEQ_CURRENT_MON,
+	AMS_SEQ_SUPPLY7,
+	AMS_SEQ_SUPPLY8,
+	AMS_SEQ_SUPPLY9,
+	AMS_SEQ_SUPPLY10,
+	AMS_SEQ_VCCAMS,
+	AMS_SEQ_TEMP_REMOTE,
+	AMS_SEQ_MAX
+};
+
+#define AMS_SEQ(x)          (AMS_SEQ_MAX + (x))
+#define AMS_VAUX_SEQ(x)     (AMS_SEQ_MAX + (x))
+
+#define PS_SEQ_MAX          AMS_SEQ_MAX
+#define PS_SEQ(x)           (x)
+#define PL_SEQ(x)           (PS_SEQ_MAX + x)
+
+#define AMS_CHAN_TEMP(_scan_index, _addr, _ext) { \
+	.type = IIO_TEMP, \
+	.indexed = 1, \
+	.address = (_addr), \
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
+		BIT(IIO_CHAN_INFO_SCALE) | \
+		BIT(IIO_CHAN_INFO_OFFSET), \
+	.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ), \
+	.event_spec = ams_temp_events, \
+	.num_event_specs = ARRAY_SIZE(ams_temp_events), \
+	.scan_index = (_scan_index), \
+	.scan_type = { \
+		.sign = 'u', \
+		.realbits = 12, \
+		.storagebits = 16, \
+		.shift = 4, \
+		.endianness = IIO_CPU, \
+	}, \
+	.extend_name = _ext, \
+}
+
+#define AMS_CHAN_VOLTAGE(_scan_index, _addr, _ext, _alarm) { \
+	.type = IIO_VOLTAGE, \
+	.indexed = 1, \
+	.address = (_addr), \
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
+		BIT(IIO_CHAN_INFO_SCALE), \
+	.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ), \
+	.event_spec = (_alarm) ? ams_voltage_events : NULL, \
+	.num_event_specs = (_alarm) ? ARRAY_SIZE(ams_voltage_events) : 0, \
+	.scan_index = (_scan_index), \
+	.scan_type = { \
+		.realbits = 10, \
+		.storagebits = 16, \
+		.shift = 6, \
+		.endianness = IIO_CPU, \
+	}, \
+	.extend_name = _ext, \
+}
+
+#define AMS_PS_CHAN_TEMP(_scan_index, _addr, _ext) \
+	AMS_CHAN_TEMP(PS_SEQ(_scan_index), _addr, _ext)
+#define AMS_PS_CHAN_VOLTAGE(_scan_index, _addr, _ext) \
+	AMS_CHAN_VOLTAGE(PS_SEQ(_scan_index), _addr, _ext, true)
+
+#define AMS_PL_CHAN_TEMP(_scan_index, _addr, _ext) \
+	AMS_CHAN_TEMP(PL_SEQ(_scan_index), _addr, _ext)
+#define AMS_PL_CHAN_VOLTAGE(_scan_index, _addr, _ext, _alarm) \
+	AMS_CHAN_VOLTAGE(PL_SEQ(_scan_index), _addr, _ext, _alarm)
+#define AMS_PL_AUX_CHAN_VOLTAGE(_auxno, _ext) \
+	AMS_CHAN_VOLTAGE(PL_SEQ(AMS_VAUX_SEQ(_auxno)), \
+			AMS_REG_VAUX(_auxno), _ext, false)
+#define AMS_CTRL_CHAN_VOLTAGE(_scan_index, _addr, _ext) \
+	AMS_CHAN_VOLTAGE(PL_SEQ(AMS_VAUX_SEQ(AMS_SEQ(_scan_index))), \
+			_addr, _ext, false)
+
+struct ams {
+	void __iomem *base;
+	void __iomem *ps_base;
+	void __iomem *pl_base;
+	struct clk *clk;
+	struct device *dev;
+	struct mutex lock;
+	unsigned int alarm_mask;
+	unsigned int masked_alarm;
+	u64 intr_mask;
+	int irq;
+	struct delayed_work ams_unmask_work;
+};
+
+static inline void ams_ps_update_reg(struct ams *ams, unsigned int offset,
+				     u32 mask, u32 data)
+{
+	u32 val;
+
+	val = readl(ams->ps_base + offset);
+	writel((val & ~mask) | (data & mask), ams->ps_base + offset);
+}
+
+static inline void ams_pl_update_reg(struct ams *ams, unsigned int offset,
+					 u32 mask, u32 data)
+{
+	u32 val;
+
+	val = readl(ams->pl_base + offset);
+	writel((val & ~mask) | (data & mask), ams->pl_base + offset);
+}
+
+static void ams_update_intrmask(struct ams *ams, u64 mask, u64 val)
+{
+	ams->intr_mask &= ~mask;
+	ams->intr_mask |= (val & mask);
+
+	writel(~(ams->intr_mask | ams->masked_alarm), ams->base + AMS_IER_0);
+	writel(~(ams->intr_mask >> AMS_ISR1_INTR_MASK_SHIFT),
+			ams->base + AMS_IER_1);
+	writel(ams->intr_mask | ams->masked_alarm, ams->base + AMS_IDR_0);
+	writel(ams->intr_mask >> AMS_ISR1_INTR_MASK_SHIFT,
+			ams->base + AMS_IDR_1);
+}
+
+static void ams_disable_all_alarms(struct ams *ams)
+{
+	/* disable PS module alarm */
+	if (ams->ps_base) {
+		ams_ps_update_reg(ams, AMS_REG_CONFIG1, AMS_REGCFG1_ALARM_MASK,
+				  AMS_REGCFG1_ALARM_MASK);
+		ams_ps_update_reg(ams, AMS_REG_CONFIG3, AMS_REGCFG3_ALARM_MASK,
+				  AMS_REGCFG3_ALARM_MASK);
+	}
+
+	/* disable PL module alarm */
+	if (ams->pl_base) {
+		ams_pl_update_reg(ams, AMS_REG_CONFIG1,
+				    AMS_REGCFG1_ALARM_MASK,
+				    AMS_REGCFG1_ALARM_MASK);
+		ams_pl_update_reg(ams, AMS_REG_CONFIG3,
+				    AMS_REGCFG3_ALARM_MASK,
+				    AMS_REGCFG3_ALARM_MASK);
+	}
+}
+
+static void iio_ams_update_alarm(struct ams *ams, unsigned long alarm_mask)
+{
+	u32 cfg;
+	unsigned long pl_alarm_mask;
+
+	if (ams->ps_base) {
+		/* Configuring PS alarm enable */
+		cfg = ~((alarm_mask & AMS_ISR0_ALARM_2_TO_0_MASK) <<
+			       AMS_CONF1_ALARM_2_TO_0_SHIFT);
+		cfg &= ~((alarm_mask & AMS_ISR0_ALARM_6_TO_3_MASK) <<
+				AMS_CONF1_ALARM_6_TO_3_SHIFT);
+		ams_ps_update_reg(ams, AMS_REG_CONFIG1, AMS_REGCFG1_ALARM_MASK,
+				  cfg);
+
+		cfg = ~((alarm_mask >> AMS_CONF3_ALARM_12_TO_7_SHIFT) &
+				AMS_ISR0_ALARM_12_TO_7_MASK);
+		ams_ps_update_reg(ams, AMS_REG_CONFIG3, AMS_REGCFG3_ALARM_MASK,
+				  cfg);
+	}
+
+	if (ams->pl_base) {
+		pl_alarm_mask = (alarm_mask >> AMS_PL_ALARM_START);
+		/* Configuring PL alarm enable */
+		cfg = ~((pl_alarm_mask & AMS_ISR0_ALARM_2_TO_0_MASK) <<
+			       AMS_CONF1_ALARM_2_TO_0_SHIFT);
+		cfg &= ~((pl_alarm_mask & AMS_ISR0_ALARM_6_TO_3_MASK) <<
+				AMS_CONF1_ALARM_6_TO_3_SHIFT);
+		ams_pl_update_reg(ams, AMS_REG_CONFIG1,
+				AMS_REGCFG1_ALARM_MASK, cfg);
+
+		cfg = ~((pl_alarm_mask >> AMS_CONF3_ALARM_12_TO_7_SHIFT) &
+				AMS_ISR0_ALARM_12_TO_7_MASK);
+		ams_pl_update_reg(ams, AMS_REG_CONFIG3,
+				AMS_REGCFG3_ALARM_MASK, cfg);
+	}
+
+	mutex_lock(&ams->lock);
+	ams_update_intrmask(ams, AMS_ISR0_ALARM_MASK, ~alarm_mask);
+	mutex_unlock(&ams->lock);
+}
+
+static void ams_enable_channel_sequence(struct ams *ams)
+{
+	int i;
+	unsigned long long scan_mask;
+	struct iio_dev *indio_dev = iio_priv_to_dev(ams);
+
+	/*
+	 * Enable channel sequence. First 22 bit of scan_mask represent
+	 * PS channels, and next remaining bit represents PL channels.
+	 */
+
+	/* Run calibration of PS & PL as part of the sequence */
+	scan_mask = 0x1 | BIT(PS_SEQ_MAX);
+	for (i = 0; i < indio_dev->num_channels; i++)
+		scan_mask |= BIT(indio_dev->channels[i].scan_index);
+
+	if (ams->ps_base) {
+		/* put sysmon in a soft reset to change the sequence */
+		ams_ps_update_reg(ams, AMS_REG_CONFIG1, AMS_CONF1_SEQ_MASK,
+				  AMS_CONF1_SEQ_DEFAULT);
+
+		/* configure basic channels */
+		writel(scan_mask & AMS_REG_SEQ0_MASK,
+				ams->ps_base + AMS_REG_SEQ_CH0);
+		writel(AMS_REG_SEQ2_MASK &
+			(scan_mask >> AMS_REG_SEQ2_MASK_SHIFT),
+			ams->ps_base + AMS_REG_SEQ_CH2);
+
+		/* set continuous sequence mode */
+		ams_ps_update_reg(ams, AMS_REG_CONFIG1, AMS_CONF1_SEQ_MASK,
+				  AMS_CONF1_SEQ_CONTINUOUS);
+	}
+
+	if (ams->pl_base) {
+		/* put sysmon in a soft reset to change the sequence */
+		ams_pl_update_reg(ams, AMS_REG_CONFIG1, AMS_CONF1_SEQ_MASK,
+				    AMS_CONF1_SEQ_DEFAULT);
+
+		/* configure basic channels */
+		scan_mask = scan_mask >> PS_SEQ_MAX;
+		writel(scan_mask & AMS_REG_SEQ0_MASK,
+				ams->pl_base + AMS_REG_SEQ_CH0);
+		writel(AMS_REG_SEQ2_MASK &
+				(scan_mask >> AMS_REG_SEQ2_MASK_SHIFT),
+				ams->pl_base + AMS_REG_SEQ_CH2);
+		writel(AMS_REG_SEQ1_MASK &
+				(scan_mask >> AMS_REG_SEQ1_MASK_SHIFT),
+				ams->pl_base + AMS_REG_SEQ_CH1);
+
+		/* set continuous sequence mode */
+		ams_pl_update_reg(ams, AMS_REG_CONFIG1, AMS_CONF1_SEQ_MASK,
+				AMS_CONF1_SEQ_CONTINUOUS);
+	}
+}
+
+static int iio_ams_init_device(struct ams *ams)
+{
+	u32 reg;
+	int ret;
+
+	/* reset AMS */
+	if (ams->ps_base) {
+		writel(AMS_PS_RESET_VALUE, ams->ps_base + AMS_VP_VN);
+
+		ret = readl_poll_timeout(ams->base + AMS_PS_CSTS, reg,
+					 (reg & AMS_PS_CSTS_PS_READY) ==
+					 AMS_PS_CSTS_PS_READY, 0,
+					 AMS_INIT_TIMEOUT);
+		if (ret)
+			return ret;
+
+		/* put sysmon in a default state */
+		ams_ps_update_reg(ams, AMS_REG_CONFIG1, AMS_CONF1_SEQ_MASK,
+				  AMS_CONF1_SEQ_DEFAULT);
+	}
+
+	if (ams->pl_base) {
+		writel(AMS_PL_RESET_VALUE, ams->pl_base + AMS_VP_VN);
+
+		ret = readl_poll_timeout(ams->base + AMS_PL_CSTS, reg,
+					 (reg & AMS_PL_CSTS_ACCESS_MASK) ==
+					 AMS_PL_CSTS_ACCESS_MASK, 0,
+					 AMS_INIT_TIMEOUT);
+		if (ret)
+			return ret;
+
+		/* put sysmon in a default state */
+		ams_pl_update_reg(ams, AMS_REG_CONFIG1, AMS_CONF1_SEQ_MASK,
+				    AMS_CONF1_SEQ_DEFAULT);
+	}
+
+	ams_disable_all_alarms(ams);
+
+	/* Disable interrupt */
+	ams_update_intrmask(ams, ~0, ~0);
+
+	/* Clear any pending interrupt */
+	writel(AMS_ISR0_ALARM_MASK, ams->base + AMS_ISR_0);
+	writel(AMS_ISR1_ALARM_MASK, ams->base + AMS_ISR_1);
+
+	return 0;
+}
+
+static int ams_enable_single_channel(struct ams *ams, unsigned int offset)
+{
+	u8 channel_num = 0;
+
+	switch (offset) {
+	case AMS_VCC_PSPLL0:
+		channel_num = AMS_VCC_PSPLL0_CH;
+		break;
+	case AMS_VCC_PSPLL3:
+		channel_num = AMS_VCC_PSPLL3_CH;
+		break;
+	case AMS_VCCINT:
+		channel_num = AMS_VCCINT_CH;
+		break;
+	case AMS_VCCBRAM:
+		channel_num = AMS_VCCBRAM_CH;
+		break;
+	case AMS_VCCAUX:
+		channel_num = AMS_VCCAUX_CH;
+		break;
+	case AMS_PSDDRPLL:
+		channel_num = AMS_PSDDRPLL_CH;
+		break;
+	case AMS_PSINTFPDDR:
+		channel_num = AMS_PSINTFPDDR_CH;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	/* set single channel, sequencer off mode */
+	ams_ps_update_reg(ams, AMS_REG_CONFIG1, AMS_CONF1_SEQ_MASK,
+			AMS_CONF1_SEQ_SINGLE_CHANNEL);
+
+	/* write the channel number */
+	ams_ps_update_reg(ams, AMS_REG_CONFIG0, AMS_CONF0_CHANNEL_NUM_MASK,
+			channel_num);
+	return 0;
+}
+
+static int ams_read_vcc_reg(struct ams *ams, unsigned int offset, u32 *data)
+{
+	u32 reg;
+	int ret;
+
+	ret = ams_enable_single_channel(ams, offset);
+	if (ret)
+		return ret;
+
+	ret = readl_poll_timeout(ams->base + AMS_ISR_1, reg,
+				 (reg & AMS_ISR1_EOC_MASK) == AMS_ISR1_EOC_MASK,
+				 0, AMS_INIT_TIMEOUT);
+	if (ret)
+		return ret;
+
+	*data = readl(ams->base + offset);
+	ams_enable_channel_sequence(ams);
+
+	return 0;
+}
+
+static int ams_read_raw(struct iio_dev *indio_dev,
+			struct iio_chan_spec const *chan,
+			int *val, int *val2, long mask)
+{
+	struct ams *ams = iio_priv(indio_dev);
+	int ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		mutex_lock(&ams->lock);
+		if (chan->scan_index >= (PS_SEQ_MAX * 3)) {
+			ret = ams_read_vcc_reg(ams, chan->address, val);
+			if (ret) {
+				mutex_unlock(&ams->lock);
+				return -EINVAL;
+			}
+		} else if (chan->scan_index >= PS_SEQ_MAX)
+			*val = readl(ams->pl_base + chan->address);
+		else
+			*val = readl(ams->ps_base + chan->address);
+		mutex_unlock(&ams->lock);
+
+		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_SCALE:
+		switch (chan->type) {
+		case IIO_VOLTAGE:
+			switch (chan->address) {
+			case AMS_SUPPLY1:
+			case AMS_SUPPLY2:
+			case AMS_SUPPLY3:
+			case AMS_SUPPLY4:
+				*val = AMS_SUPPLY_SCALE_3VOLT;
+				break;
+			case AMS_SUPPLY5:
+			case AMS_SUPPLY6:
+				if (chan->scan_index < PS_SEQ_MAX)
+					*val = AMS_SUPPLY_SCALE_6VOLT;
+				else
+					*val = AMS_SUPPLY_SCALE_3VOLT;
+				break;
+			case AMS_SUPPLY7:
+			case AMS_SUPPLY8:
+				*val = AMS_SUPPLY_SCALE_6VOLT;
+				break;
+			case AMS_SUPPLY9:
+			case AMS_SUPPLY10:
+				if (chan->scan_index < PS_SEQ_MAX)
+					*val = AMS_SUPPLY_SCALE_3VOLT;
+				else
+					*val = AMS_SUPPLY_SCALE_6VOLT;
+				break;
+			case AMS_VCC_PSPLL0:
+			case AMS_VCC_PSPLL3:
+			case AMS_VCCINT:
+			case AMS_VCCBRAM:
+			case AMS_VCCAUX:
+			case AMS_PSDDRPLL:
+			case AMS_PSINTFPDDR:
+				*val = AMS_SUPPLY_SCALE_3VOLT;
+				break;
+			default:
+				*val = AMS_SUPPLY_SCALE_1VOLT;
+				break;
+			}
+			*val2 = AMS_SUPPLY_SCALE_DIV_BIT;
+			return IIO_VAL_FRACTIONAL_LOG2;
+		case IIO_TEMP:
+			*val = AMS_TEMP_SCALE;
+			*val2 = AMS_TEMP_SCALE_DIV_BIT;
+			return IIO_VAL_FRACTIONAL_LOG2;
+		default:
+			return -EINVAL;
+		}
+	case IIO_CHAN_INFO_OFFSET:
+		/* Only the temperature channel has an offset */
+		*val = AMS_TEMP_OFFSET;
+		return IIO_VAL_INT;
+	}
+
+	return -EINVAL;
+}
+
+static int ams_get_alarm_offset(int scan_index, enum iio_event_direction dir)
+{
+	int offset = 0;
+
+	if (scan_index >= PS_SEQ_MAX)
+		scan_index -= PS_SEQ_MAX;
+
+	if (dir == IIO_EV_DIR_FALLING) {
+		if (scan_index < AMS_SEQ_SUPPLY7)
+			offset = AMS_ALARM_THRESHOLD_OFF_10;
+		else
+			offset = AMS_ALARM_THRESHOLD_OFF_20;
+	}
+
+	switch (scan_index) {
+	case AMS_SEQ_TEMP:
+		return AMS_ALARM_TEMP + offset;
+	case AMS_SEQ_SUPPLY1:
+		return AMS_ALARM_SUPPLY1 + offset;
+	case AMS_SEQ_SUPPLY2:
+		return AMS_ALARM_SUPPLY2 + offset;
+	case AMS_SEQ_SUPPLY3:
+		return AMS_ALARM_SUPPLY3 + offset;
+	case AMS_SEQ_SUPPLY4:
+		return AMS_ALARM_SUPPLY4 + offset;
+	case AMS_SEQ_SUPPLY5:
+		return AMS_ALARM_SUPPLY5 + offset;
+	case AMS_SEQ_SUPPLY6:
+		return AMS_ALARM_SUPPLY6 + offset;
+	case AMS_SEQ_SUPPLY7:
+		return AMS_ALARM_SUPPLY7 + offset;
+	case AMS_SEQ_SUPPLY8:
+		return AMS_ALARM_SUPPLY8 + offset;
+	case AMS_SEQ_SUPPLY9:
+		return AMS_ALARM_SUPPLY9 + offset;
+	case AMS_SEQ_SUPPLY10:
+		return AMS_ALARM_SUPPLY10 + offset;
+	case AMS_SEQ_VCCAMS:
+		return AMS_ALARM_VCCAMS + offset;
+	case AMS_SEQ_TEMP_REMOTE:
+		return AMS_ALARM_TEMP_REMOTE + offset;
+	}
+
+	return 0;
+}
+
+static const struct iio_chan_spec *ams_event_to_channel(
+		struct iio_dev *indio_dev, u32 event)
+{
+	int scan_index = 0, i;
+
+	if (event >= AMS_PL_ALARM_START) {
+		event -= AMS_PL_ALARM_START;
+		scan_index = PS_SEQ_MAX;
+	}
+
+	switch (event) {
+	case AMS_ALARM_BIT_TEMP:
+		scan_index += AMS_SEQ_TEMP;
+		break;
+	case AMS_ALARM_BIT_SUPPLY1:
+		scan_index += AMS_SEQ_SUPPLY1;
+		break;
+	case AMS_ALARM_BIT_SUPPLY2:
+		scan_index += AMS_SEQ_SUPPLY2;
+		break;
+	case AMS_ALARM_BIT_SUPPLY3:
+		scan_index += AMS_SEQ_SUPPLY3;
+		break;
+	case AMS_ALARM_BIT_SUPPLY4:
+		scan_index += AMS_SEQ_SUPPLY4;
+		break;
+	case AMS_ALARM_BIT_SUPPLY5:
+		scan_index += AMS_SEQ_SUPPLY5;
+		break;
+	case AMS_ALARM_BIT_SUPPLY6:
+		scan_index += AMS_SEQ_SUPPLY6;
+		break;
+	case AMS_ALARM_BIT_SUPPLY7:
+		scan_index += AMS_SEQ_SUPPLY7;
+		break;
+	case AMS_ALARM_BIT_SUPPLY8:
+		scan_index += AMS_SEQ_SUPPLY8;
+		break;
+	case AMS_ALARM_BIT_SUPPLY9:
+		scan_index += AMS_SEQ_SUPPLY9;
+		break;
+	case AMS_ALARM_BIT_SUPPLY10:
+		scan_index += AMS_SEQ_SUPPLY10;
+		break;
+	case AMS_ALARM_BIT_VCCAMS:
+		scan_index += AMS_SEQ_VCCAMS;
+		break;
+	case AMS_ALARM_BIT_TEMP_REMOTE:
+		scan_index += AMS_SEQ_TEMP_REMOTE;
+		break;
+	}
+
+	for (i = 0; i < indio_dev->num_channels; i++)
+		if (indio_dev->channels[i].scan_index == scan_index)
+			break;
+
+	return &indio_dev->channels[i];
+}
+
+static int ams_get_alarm_mask(int scan_index)
+{
+	int bit = 0;
+
+	if (scan_index >= PS_SEQ_MAX) {
+		bit = AMS_PL_ALARM_START;
+		scan_index -= PS_SEQ_MAX;
+	}
+
+	switch (scan_index) {
+	case AMS_SEQ_TEMP:
+		return BIT(AMS_ALARM_BIT_TEMP + bit);
+	case AMS_SEQ_SUPPLY1:
+		return BIT(AMS_ALARM_BIT_SUPPLY1 + bit);
+	case AMS_SEQ_SUPPLY2:
+		return BIT(AMS_ALARM_BIT_SUPPLY2 + bit);
+	case AMS_SEQ_SUPPLY3:
+		return BIT(AMS_ALARM_BIT_SUPPLY3 + bit);
+	case AMS_SEQ_SUPPLY4:
+		return BIT(AMS_ALARM_BIT_SUPPLY4 + bit);
+	case AMS_SEQ_SUPPLY5:
+		return BIT(AMS_ALARM_BIT_SUPPLY5 + bit);
+	case AMS_SEQ_SUPPLY6:
+		return BIT(AMS_ALARM_BIT_SUPPLY6 + bit);
+	case AMS_SEQ_SUPPLY7:
+		return BIT(AMS_ALARM_BIT_SUPPLY7 + bit);
+	case AMS_SEQ_SUPPLY8:
+		return BIT(AMS_ALARM_BIT_SUPPLY8 + bit);
+	case AMS_SEQ_SUPPLY9:
+		return BIT(AMS_ALARM_BIT_SUPPLY9 + bit);
+	case AMS_SEQ_SUPPLY10:
+		return BIT(AMS_ALARM_BIT_SUPPLY10 + bit);
+	case AMS_SEQ_VCCAMS:
+		return BIT(AMS_ALARM_BIT_VCCAMS + bit);
+	case AMS_SEQ_TEMP_REMOTE:
+		return BIT(AMS_ALARM_BIT_TEMP_REMOTE + bit);
+	}
+
+	return 0;
+}
+
+static int ams_read_event_config(struct iio_dev *indio_dev,
+				 const struct iio_chan_spec *chan,
+				 enum iio_event_type type,
+				 enum iio_event_direction dir)
+{
+	struct ams *ams = iio_priv(indio_dev);
+
+	return (ams->alarm_mask & ams_get_alarm_mask(chan->scan_index)) ? 1 : 0;
+}
+
+static int ams_write_event_config(struct iio_dev *indio_dev,
+				  const struct iio_chan_spec *chan,
+				  enum iio_event_type type,
+				  enum iio_event_direction dir,
+				  int state)
+{
+	struct ams *ams = iio_priv(indio_dev);
+	unsigned int alarm;
+
+	alarm = ams_get_alarm_mask(chan->scan_index);
+
+	mutex_lock(&ams->lock);
+
+	if (state)
+		ams->alarm_mask |= alarm;
+	else
+		ams->alarm_mask &= ~alarm;
+
+	iio_ams_update_alarm(ams, ams->alarm_mask);
+
+	mutex_unlock(&ams->lock);
+
+	return 0;
+}
+
+static int ams_read_event_value(struct iio_dev *indio_dev,
+				const struct iio_chan_spec *chan,
+				enum iio_event_type type,
+				enum iio_event_direction dir,
+				enum iio_event_info info, int *val, int *val2)
+{
+	struct ams *ams = iio_priv(indio_dev);
+	unsigned int offset = ams_get_alarm_offset(chan->scan_index, dir);
+
+	mutex_lock(&ams->lock);
+
+	if (chan->scan_index >= PS_SEQ_MAX)
+		*val = readl(ams->pl_base + offset);
+	else
+		*val = readl(ams->ps_base + offset);
+
+	mutex_unlock(&ams->lock);
+
+	return IIO_VAL_INT;
+}
+
+static int ams_write_event_value(struct iio_dev *indio_dev,
+				 const struct iio_chan_spec *chan,
+				 enum iio_event_type type,
+				 enum iio_event_direction dir,
+				 enum iio_event_info info, int val, int val2)
+{
+	struct ams *ams = iio_priv(indio_dev);
+	unsigned int offset;
+
+	mutex_lock(&ams->lock);
+
+	/* Set temperature channel threshold to direct threshold */
+	if (chan->type == IIO_TEMP) {
+		offset = ams_get_alarm_offset(chan->scan_index,
+					      IIO_EV_DIR_FALLING);
+
+		if (chan->scan_index >= PS_SEQ_MAX)
+			ams_pl_update_reg(ams, offset,
+					    AMS_ALARM_THR_DIRECT_MASK,
+					    AMS_ALARM_THR_DIRECT_MASK);
+		else
+			ams_ps_update_reg(ams, offset,
+					  AMS_ALARM_THR_DIRECT_MASK,
+					  AMS_ALARM_THR_DIRECT_MASK);
+	}
+
+	offset = ams_get_alarm_offset(chan->scan_index, dir);
+	if (chan->scan_index >= PS_SEQ_MAX)
+		writel(val, ams->pl_base + offset);
+	else
+		writel(val, ams->ps_base + offset);
+
+	mutex_unlock(&ams->lock);
+
+	return 0;
+}
+
+static void ams_handle_event(struct iio_dev *indio_dev, u32 event)
+{
+	const struct iio_chan_spec *chan;
+
+	chan = ams_event_to_channel(indio_dev, event);
+
+	if (chan->type == IIO_TEMP) {
+		/*
+		 * The temperature channel only supports over-temperature
+		 * events
+		 */
+		iio_push_event(indio_dev,
+			       IIO_UNMOD_EVENT_CODE(chan->type, chan->channel,
+						    IIO_EV_TYPE_THRESH,
+						    IIO_EV_DIR_RISING),
+			iio_get_time_ns(indio_dev));
+	} else {
+		/*
+		 * For other channels we don't know whether it is a upper or
+		 * lower threshold event. Userspace will have to check the
+		 * channel value if it wants to know.
+		 */
+		iio_push_event(indio_dev,
+			       IIO_UNMOD_EVENT_CODE(chan->type, chan->channel,
+						    IIO_EV_TYPE_THRESH,
+						    IIO_EV_DIR_EITHER),
+			iio_get_time_ns(indio_dev));
+	}
+}
+
+static void ams_handle_events(struct iio_dev *indio_dev, unsigned long events)
+{
+	unsigned int bit;
+
+	for_each_set_bit(bit, &events, AMS_NO_OF_ALARMS)
+		ams_handle_event(indio_dev, bit);
+}
+
+/**
+ * ams_unmask_worker - ams alarm interrupt unmask worker
+ * @work :		work to be done
+ *
+ * The ZynqMP threshold interrupts are level sensitive. Since we can't make the
+ * threshold condition go way from within the interrupt handler, this means as
+ * soon as a threshold condition is present we would enter the interrupt handler
+ * again and again. To work around this we mask all active threshold interrupts
+ * in the interrupt handler and start a timer. In this timer we poll the
+ * interrupt status and only if the interrupt is inactive we unmask it again.
+ */
+static void ams_unmask_worker(struct work_struct *work)
+{
+	struct ams *ams = container_of(work, struct ams, ams_unmask_work.work);
+	unsigned int status, unmask;
+
+	mutex_lock(&ams->lock);
+
+	status = readl(ams->base + AMS_ISR_0);
+
+	/* Clear those bits which are not active anymore */
+	unmask = (ams->masked_alarm ^ status) & ams->masked_alarm;
+
+	/* clear status of disabled alarm */
+	unmask |= ams->intr_mask;
+
+	ams->masked_alarm &= status;
+
+	/* Also clear those which are masked out anyway */
+	ams->masked_alarm &= ~ams->intr_mask;
+
+	/* Clear the interrupts before we unmask them */
+	writel(unmask, ams->base + AMS_ISR_0);
+
+	ams_update_intrmask(ams, 0, 0);
+
+	mutex_unlock(&ams->lock);
+
+	/* if still pending some alarm re-trigger the timer */
+	if (ams->masked_alarm)
+		schedule_delayed_work(&ams->ams_unmask_work,
+				      msecs_to_jiffies(AMS_UNMASK_TIMEOUT_MS));
+}
+
+static irqreturn_t ams_iio_irq(int irq, void *data)
+{
+	struct iio_dev *indio_dev = data;
+	struct ams *ams = iio_priv(indio_dev);
+	irqreturn_t irq_status = IRQ_NONE;
+	u32 isr0;
+
+	isr0 = readl(ams->base + AMS_ISR_0);
+
+	/* only process alarms that are not masked */
+	isr0 &= ~((ams->intr_mask & AMS_ISR0_ALARM_MASK) | ams->masked_alarm);
+
+	if (isr0) {
+		/* clear interrupt */
+		writel(isr0, ams->base + AMS_ISR_0);
+
+		/* Once the alarm interrupt occurred, mask until get cleared */
+		ams->masked_alarm |= isr0;
+		ams_update_intrmask(ams, 0, 0);
+
+		ams_handle_events(indio_dev, isr0);
+
+		schedule_delayed_work(&ams->ams_unmask_work,
+				      msecs_to_jiffies(AMS_UNMASK_TIMEOUT_MS));
+		irq_status = IRQ_HANDLED;
+	}
+
+	return irq_status;
+}
+
+static const struct iio_event_spec ams_temp_events[] = {
+	{
+		.type = IIO_EV_TYPE_THRESH,
+		.dir = IIO_EV_DIR_RISING,
+		.mask_separate = BIT(IIO_EV_INFO_ENABLE) |
+				BIT(IIO_EV_INFO_VALUE),
+	},
+};
+
+static const struct iio_event_spec ams_voltage_events[] = {
+	{
+		.type = IIO_EV_TYPE_THRESH,
+		.dir = IIO_EV_DIR_RISING,
+		.mask_separate = BIT(IIO_EV_INFO_VALUE),
+	},
+	{
+		.type = IIO_EV_TYPE_THRESH,
+		.dir = IIO_EV_DIR_FALLING,
+		.mask_separate = BIT(IIO_EV_INFO_VALUE),
+	},
+	{
+		.type = IIO_EV_TYPE_THRESH,
+		.dir = IIO_EV_DIR_EITHER,
+		.mask_separate = BIT(IIO_EV_INFO_ENABLE),
+	},
+};
+
+static const struct iio_chan_spec ams_ps_channels[] = {
+	AMS_PS_CHAN_TEMP(AMS_SEQ_TEMP, AMS_TEMP, "ps_temp"),
+	AMS_PS_CHAN_TEMP(AMS_SEQ_TEMP_REMOTE, AMS_TEMP_REMOTE, "remote_temp"),
+	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY1, AMS_SUPPLY1, "vccpsintlp"),
+	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY2, AMS_SUPPLY2, "vccpsintfp"),
+	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY3, AMS_SUPPLY3, "vccpsaux"),
+	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY4, AMS_SUPPLY4, "vccpsddr"),
+	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY5, AMS_SUPPLY5, "vccpsio3"),
+	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY6, AMS_SUPPLY6, "vccpsio0"),
+	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY7, AMS_SUPPLY7, "vccpsio1"),
+	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY8, AMS_SUPPLY8, "vccpsio2"),
+	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY9, AMS_SUPPLY9, "psmgtravcc"),
+	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY10, AMS_SUPPLY10, "psmgtravtt"),
+	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_VCCAMS, AMS_VCCAMS, "vccams"),
+};
+
+static const struct iio_chan_spec ams_pl_channels[] = {
+	AMS_PL_CHAN_TEMP(AMS_SEQ_TEMP, AMS_TEMP, "pl_temp"),
+	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY1, AMS_SUPPLY1, "vccint", true),
+	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY2, AMS_SUPPLY2, "vccaux", true),
+	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_VREFP, AMS_VREFP, "vccvrefp", false),
+	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_VREFN, AMS_VREFN, "vccvrefn", false),
+	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY3, AMS_SUPPLY3, "vccbram", true),
+	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY4, AMS_SUPPLY4, "vccplintlp", true),
+	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY5, AMS_SUPPLY5, "vccplintfp", true),
+	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY6, AMS_SUPPLY6, "vccplaux", true),
+	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_VCCAMS, AMS_VCCAMS, "vccams", true),
+	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_VP_VN, AMS_VP_VN, "vccvpvn", false),
+	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY7, AMS_SUPPLY7, "vuser0", true),
+	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY8, AMS_SUPPLY8, "vuser1", true),
+	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY9, AMS_SUPPLY9, "vuser2", true),
+	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY10, AMS_SUPPLY10, "vuser3", true),
+	AMS_PL_AUX_CHAN_VOLTAGE(0, NULL),
+	AMS_PL_AUX_CHAN_VOLTAGE(1, NULL),
+	AMS_PL_AUX_CHAN_VOLTAGE(2, NULL),
+	AMS_PL_AUX_CHAN_VOLTAGE(3, NULL),
+	AMS_PL_AUX_CHAN_VOLTAGE(4, NULL),
+	AMS_PL_AUX_CHAN_VOLTAGE(5, NULL),
+	AMS_PL_AUX_CHAN_VOLTAGE(6, NULL),
+	AMS_PL_AUX_CHAN_VOLTAGE(7, NULL),
+	AMS_PL_AUX_CHAN_VOLTAGE(8, NULL),
+	AMS_PL_AUX_CHAN_VOLTAGE(9, NULL),
+	AMS_PL_AUX_CHAN_VOLTAGE(10, NULL),
+	AMS_PL_AUX_CHAN_VOLTAGE(11, NULL),
+	AMS_PL_AUX_CHAN_VOLTAGE(12, NULL),
+	AMS_PL_AUX_CHAN_VOLTAGE(13, NULL),
+	AMS_PL_AUX_CHAN_VOLTAGE(14, NULL),
+	AMS_PL_AUX_CHAN_VOLTAGE(15, NULL),
+};
+
+static const struct iio_chan_spec ams_ctrl_channels[] = {
+	AMS_CTRL_CHAN_VOLTAGE(AMS_SEQ_VCC_PSPLL, AMS_VCC_PSPLL0, "vcc_pspll0"),
+	AMS_CTRL_CHAN_VOLTAGE(AMS_SEQ_VCC_PSBATT, AMS_VCC_PSPLL3, "vcc_psbatt"),
+	AMS_CTRL_CHAN_VOLTAGE(AMS_SEQ_VCCINT, AMS_VCCINT, "vccint"),
+	AMS_CTRL_CHAN_VOLTAGE(AMS_SEQ_VCCBRAM, AMS_VCCBRAM, "vccbram"),
+	AMS_CTRL_CHAN_VOLTAGE(AMS_SEQ_VCCAUX, AMS_VCCAUX, "vccaux"),
+	AMS_CTRL_CHAN_VOLTAGE(AMS_SEQ_PSDDRPLL, AMS_PSDDRPLL, "psddrpll"),
+	AMS_CTRL_CHAN_VOLTAGE(AMS_SEQ_INTDDR, AMS_PSINTFPDDR, "psintfpddr"),
+};
+
+static int ams_init_module(struct iio_dev *indio_dev, struct device_node *np,
+			   struct iio_chan_spec *channels)
+{
+	struct ams *ams = iio_priv(indio_dev);
+	struct device_node *chan_node, *child;
+	int ret, num_channels = 0;
+	unsigned int reg;
+
+	if (of_device_is_compatible(np, "xlnx,zynqmp-ams-ps")) {
+		ams->ps_base = of_iomap(np, 0);
+		if (!ams->ps_base)
+			return -ENXIO;
+
+		/* add PS channels to iio device channels */
+		memcpy(channels + num_channels, ams_ps_channels,
+		       sizeof(ams_ps_channels));
+		num_channels += ARRAY_SIZE(ams_ps_channels);
+	} else if (of_device_is_compatible(np, "xlnx,zynqmp-ams-pl")) {
+		ams->pl_base = of_iomap(np, 0);
+		if (!ams->pl_base)
+			return -ENXIO;
+
+		/* Copy only first 10 fix channels */
+		memcpy(channels + num_channels, ams_pl_channels,
+		       AMS_PL_MAX_FIXED_CHANNEL * sizeof(*channels));
+		num_channels += AMS_PL_MAX_FIXED_CHANNEL;
+
+		chan_node = of_get_child_by_name(np, "xlnx,ext-channels");
+		if (chan_node) {
+			for_each_child_of_node(chan_node, child) {
+				ret = of_property_read_u32(child, "reg", &reg);
+				if (ret || reg > AMS_PL_MAX_EXT_CHANNEL)
+					continue;
+
+				memcpy(&channels[num_channels],
+				       &ams_pl_channels[reg +
+				       AMS_PL_MAX_FIXED_CHANNEL],
+				       sizeof(*channels));
+
+				if (of_property_read_bool(child,
+							  "xlnx,bipolar"))
+					channels[num_channels].scan_type.sign =
+						's';
+
+				num_channels++;
+			}
+		}
+		of_node_put(chan_node);
+	} else if (of_device_is_compatible(np, "xlnx,zynqmp-ams")) {
+		/* add AMS channels to iio device channels */
+		memcpy(channels + num_channels, ams_ctrl_channels,
+				sizeof(ams_ctrl_channels));
+		num_channels += ARRAY_SIZE(ams_ctrl_channels);
+	} else {
+		return -EINVAL;
+	}
+
+	return num_channels;
+}
+
+static int ams_parse_dt(struct iio_dev *indio_dev, struct platform_device *pdev)
+{
+	struct ams *ams = iio_priv(indio_dev);
+	struct iio_chan_spec *ams_channels, *dev_channels;
+	struct device_node *child_node = NULL, *np = pdev->dev.of_node;
+	int ret, vol_ch_cnt = 0, temp_ch_cnt = 0, i, rising_off, falling_off;
+	unsigned int num_channels = 0;
+
+	/* Initialize buffer for channel specification */
+	ams_channels = kzalloc(sizeof(ams_ps_channels) +
+			       sizeof(ams_pl_channels) +
+			       sizeof(ams_ctrl_channels), GFP_KERNEL);
+	if (!ams_channels)
+		return -ENOMEM;
+
+	if (of_device_is_available(np)) {
+		ret = ams_init_module(indio_dev, np, ams_channels);
+		if (ret < 0)
+			goto err;
+
+		num_channels += ret;
+	}
+
+	for_each_child_of_node(np, child_node) {
+		if (of_device_is_available(child_node)) {
+			ret = ams_init_module(indio_dev, child_node,
+					      ams_channels + num_channels);
+			if (ret < 0)
+				goto err;
+
+			num_channels += ret;
+		}
+	}
+
+	for (i = 0; i < num_channels; i++) {
+		if (ams_channels[i].type == IIO_VOLTAGE)
+			ams_channels[i].channel = vol_ch_cnt++;
+		else
+			ams_channels[i].channel = temp_ch_cnt++;
+
+		if (ams_channels[i].scan_index < (PS_SEQ_MAX * 3)) {
+			/* set threshold to max and min for each channel */
+			falling_off = ams_get_alarm_offset(
+					ams_channels[i].scan_index,
+					IIO_EV_DIR_FALLING);
+			rising_off = ams_get_alarm_offset(
+					ams_channels[i].scan_index,
+					IIO_EV_DIR_RISING);
+			if (ams_channels[i].scan_index >= PS_SEQ_MAX) {
+				writel(AMS_ALARM_THR_MIN,
+						ams->pl_base + falling_off);
+				writel(AMS_ALARM_THR_MAX,
+						ams->pl_base + rising_off);
+			} else {
+				writel(AMS_ALARM_THR_MIN,
+						ams->ps_base + falling_off);
+				writel(AMS_ALARM_THR_MAX,
+						ams->ps_base + rising_off);
+			}
+		}
+	}
+
+	dev_channels = devm_kzalloc(&pdev->dev, sizeof(*dev_channels) *
+				    num_channels, GFP_KERNEL);
+	if (!dev_channels) {
+		ret = -ENOMEM;
+		goto err;
+	}
+
+	memcpy(dev_channels, ams_channels,
+	       sizeof(*ams_channels) * num_channels);
+	indio_dev->channels = dev_channels;
+	indio_dev->num_channels = num_channels;
+
+	ret = 0;
+err:
+	kfree(ams_channels);
+
+	return ret;
+}
+
+static const struct iio_info iio_pl_info = {
+	.read_raw = &ams_read_raw,
+	.read_event_config = &ams_read_event_config,
+	.write_event_config = &ams_write_event_config,
+	.read_event_value = &ams_read_event_value,
+	.write_event_value = &ams_write_event_value,
+};
+
+static const struct of_device_id ams_of_match_table[] = {
+	{ .compatible = "xlnx,zynqmp-ams" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, ams_of_match_table);
+
+static int ams_probe(struct platform_device *pdev)
+{
+	struct iio_dev *indio_dev;
+	struct ams *ams;
+	struct resource *res;
+	int ret;
+
+	if (!pdev->dev.of_node)
+		return -ENODEV;
+
+	indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*ams));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	ams = iio_priv(indio_dev);
+	mutex_init(&ams->lock);
+
+	indio_dev->dev.parent = &pdev->dev;
+	indio_dev->dev.of_node = pdev->dev.of_node;
+	indio_dev->name = "xilinx-ams";
+
+	indio_dev->info = &iio_pl_info;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "ams-base");
+	ams->base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(ams->base))
+		return PTR_ERR(ams->base);
+
+	ams->clk = devm_clk_get(&pdev->dev, NULL);
+	if (IS_ERR(ams->clk))
+		return PTR_ERR(ams->clk);
+	clk_prepare_enable(ams->clk);
+
+	INIT_DELAYED_WORK(&ams->ams_unmask_work, ams_unmask_worker);
+
+	ret = iio_ams_init_device(ams);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to initialize AMS\n");
+		goto err_probe;
+	}
+
+	ret = ams_parse_dt(indio_dev, pdev);
+	if (ret) {
+		dev_err(&pdev->dev, "failure in parsing DT\n");
+		goto err_probe;
+	}
+
+	ams_enable_channel_sequence(ams);
+
+	ams->irq = platform_get_irq_byname(pdev, "ams-irq");
+	ret = request_irq(ams->irq, &ams_iio_irq, 0, "ams-irq", indio_dev);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "failed to register interrupt\n");
+		goto err_probe;
+	}
+
+	platform_set_drvdata(pdev, indio_dev);
+
+	ret = iio_device_register(indio_dev);
+	if (ret)
+		goto err_irq_free;
+
+	return 0;
+
+err_irq_free:
+	free_irq(ams->irq, indio_dev);
+
+err_probe:
+	cancel_delayed_work(&ams->ams_unmask_work);
+	clk_disable_unprepare(ams->clk);
+
+	return ret;
+}
+
+static int ams_remove(struct platform_device *pdev)
+{
+	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
+	struct ams *ams = iio_priv(indio_dev);
+
+	cancel_delayed_work(&ams->ams_unmask_work);
+
+	free_irq(ams->irq, indio_dev);
+	/* Unregister the device */
+	iio_device_unregister(indio_dev);
+	clk_disable_unprepare(ams->clk);
+
+	return 0;
+}
+
+static int __maybe_unused ams_suspend(struct device *dev)
+{
+	struct ams *ams = iio_priv(dev_get_drvdata(dev));
+
+	clk_disable_unprepare(ams->clk);
+
+	return 0;
+}
+
+static int __maybe_unused ams_resume(struct device *dev)
+{
+	struct ams *ams = iio_priv(dev_get_drvdata(dev));
+
+	clk_prepare_enable(ams->clk);
+
+	return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(ams_pm_ops, ams_suspend, ams_resume);
+
+static struct platform_driver ams_driver = {
+	.probe = ams_probe,
+	.remove = ams_remove,
+	.driver = {
+		.name = "xilinx-ams",
+		.pm = &ams_pm_ops,
+		.of_match_table = ams_of_match_table,
+	},
+};
+module_platform_driver(ams_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Xilinx, Inc.");
-- 
2.1.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 4/4] arm64: zynqmp: DT: Add Xilinx AMS node
  2018-09-14  7:18 [PATCH v2 0/4] Add Xilinx AMS Driver Manish Narani
                   ` (2 preceding siblings ...)
  2018-09-14  7:18 ` [PATCH v2 3/4] iio: adc: Add Xilinx AMS driver Manish Narani
@ 2018-09-14  7:18 ` Manish Narani
  3 siblings, 0 replies; 12+ messages in thread
From: Manish Narani @ 2018-09-14  7:18 UTC (permalink / raw)
  To: jic23, knaack.h, lars, pmeerw, michal.simek, robh+dt,
	mark.rutland, manish.narani, sudeep.holla, amit.kucheria,
	leoyang.li, broonie, arnaud.pouliquen, eugen.hristev, rdunlap,
	geert, ak, freeman.liu, lukas, vilhelm.gray, gregkh, kstewart
  Cc: sgoud, anirudh, linux-iio, linux-arm-kernel, linux-kernel, devicetree

The Xilinx AMS includes an ADC as well as on-chip sensors that can be
used to sample external and monitor on-die operating conditions, such as
temperature and supply voltage levels.

Signed-off-by: Manish Narani <manish.narani@xilinx.com>
---
 arch/arm64/boot/dts/xilinx/zynqmp.dtsi | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
index 29ce234..6e42ca2 100644
--- a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
+++ b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
@@ -617,5 +617,31 @@
 			reg = <0x0 0xfd4d0000 0x0 0x1000>;
 			timeout-sec = <10>;
 		};
+
+		xilinx_ams: ams@ffa50000 {
+			compatible = "xlnx,zynqmp-ams";
+			status = "disabled";
+			interrupt-parent = <&gic>;
+			interrupts = <0 56 4>;
+			interrupt-names = "ams-irq";
+			reg = <0x0 0xffa50000 0x0 0x800>;
+			reg-names = "ams-base";
+			#address-cells = <2>;
+			#size-cells = <2>;
+			#io-channel-cells = <1>;
+			ranges;
+
+			ams_ps: ams_ps@ffa50800 {
+				compatible = "xlnx,zynqmp-ams-ps";
+				status = "disabled";
+				reg = <0x0 0xffa50800 0x0 0x400>;
+			};
+
+			ams_pl: ams_pl@ffa50c00 {
+				compatible = "xlnx,zynqmp-ams-pl";
+				status = "disabled";
+				reg = <0x0 0xffa50c00 0x0 0x400>;
+			};
+		};
 	};
 };
-- 
2.1.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 3/4] iio: adc: Add Xilinx AMS driver
  2018-09-14  7:18 ` [PATCH v2 3/4] iio: adc: Add Xilinx AMS driver Manish Narani
@ 2018-09-14 11:59   ` Himanshu Jha
  2018-09-14 21:30   ` Randy Dunlap
  2018-09-16 10:34   ` Jonathan Cameron
  2 siblings, 0 replies; 12+ messages in thread
From: Himanshu Jha @ 2018-09-14 11:59 UTC (permalink / raw)
  To: Manish Narani
  Cc: jic23, knaack.h, lars, pmeerw, michal.simek, robh+dt,
	mark.rutland, sudeep.holla, amit.kucheria, leoyang.li, broonie,
	arnaud.pouliquen, eugen.hristev, rdunlap, geert, ak, freeman.liu,
	lukas, vilhelm.gray, gregkh, kstewart, sgoud, anirudh, linux-iio,
	linux-arm-kernel, linux-kernel, devicetree, gustavo

Hi Manish,

On Fri, Sep 14, 2018 at 12:48:29PM +0530, Manish Narani wrote:
> 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.
> PL-SYSMON block is capable of monitoring off chip voltage and
> temperature.
> PL-SYSMON block has DRP, JTAG and I2C interface to enable monitoring
> from external master. Out of these interface currently only DRP is
> supported.
> Other block PS-SYSMON is memory mapped to PS.
> The AMS can use internal channels to monitor voltage and temperature as
> well as one primary and up to 16 auxiliary channels for measuring
> external voltages.
> The voltage and temperature monitoring channels also have event
> capability which allows to generate an interrupt when their value falls
> below or raises above a set threshold.
> 
> Signed-off-by: Manish Narani <manish.narani@xilinx.com>
> ---
[]
> +// SPDX-License-Identifier: GPL-2.0

License Identifier seems inconsistent as below you
mentioned "GPL" and not "GPLv2".


Please check once.
Documentation/process/license-rules.rst


> +	case IIO_CHAN_INFO_SCALE:
> +		switch (chan->type) {
> +		case IIO_VOLTAGE:
> +			switch (chan->address) {
> +			case AMS_SUPPLY1:
			/* fall through */
> +			case AMS_SUPPLY2:
			/* fall through */


Similarly to others as well.
There is a plan to enable "-Wimplicit-fallthrough" gcc flag
https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html

Also, Gustavo nearly cleaned all the cases and would save
his effort of doing it again :)
https://lore.kernel.org/lkml/20180903183618.GA6905@embeddedor.com/

> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Xilinx, Inc.");


Thanks
-- 
Himanshu Jha
Undergraduate Student
Department of Electronics & Communication
Guru Tegh Bahadur Institute of Technology

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 3/4] iio: adc: Add Xilinx AMS driver
  2018-09-14  7:18 ` [PATCH v2 3/4] iio: adc: Add Xilinx AMS driver Manish Narani
  2018-09-14 11:59   ` Himanshu Jha
@ 2018-09-14 21:30   ` Randy Dunlap
  2018-09-16 10:34   ` Jonathan Cameron
  2 siblings, 0 replies; 12+ messages in thread
From: Randy Dunlap @ 2018-09-14 21:30 UTC (permalink / raw)
  To: Manish Narani, jic23, knaack.h, lars, pmeerw, michal.simek,
	robh+dt, mark.rutland, sudeep.holla, amit.kucheria, leoyang.li,
	broonie, arnaud.pouliquen, eugen.hristev, geert, ak, freeman.liu,
	lukas, vilhelm.gray, gregkh, kstewart
  Cc: sgoud, anirudh, linux-iio, linux-arm-kernel, linux-kernel, devicetree

Hi,

On 9/14/18 12:18 AM, Manish Narani wrote:
> 
> Signed-off-by: Manish Narani <manish.narani@xilinx.com>
> ---
>  drivers/iio/adc/Kconfig      |   10 +
>  drivers/iio/adc/Makefile     |    1 +
>  drivers/iio/adc/xilinx-ams.c | 1337 ++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 1348 insertions(+)
>  create mode 100644 drivers/iio/adc/xilinx-ams.c
> 
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 4a75492..405ea00 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -941,4 +941,14 @@ config XILINX_XADC
>  	  The driver can also be build as a module. If so, the module will be called

oops (some other driver:)
	                         built

>  	  xilinx-xadc.
>  
> +config XILINX_AMS
> +	tristate "Xilinx AMS driver"
> +	depends on ARCH_ZYNQMP || COMPILE_TEST
> +	depends on HAS_IOMEM
> +	help
> +	  Say yes here to have support for the Xilinx AMS.
> +
> +	  The driver can also be build as a module. If so, the module will be called

	                         built

> +	  xilinx-ams.
> +
>  endmenu


-- 
~Randy

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 2/4] iio: Documentation: Add Xilinx AMS sysfs documentation
  2018-09-14  7:18 ` [PATCH v2 2/4] iio: Documentation: Add Xilinx AMS sysfs documentation Manish Narani
@ 2018-09-16 10:12   ` Jonathan Cameron
  2018-09-17  9:56     ` Michal Simek
  0 siblings, 1 reply; 12+ messages in thread
From: Jonathan Cameron @ 2018-09-16 10:12 UTC (permalink / raw)
  To: Manish Narani
  Cc: knaack.h, lars, pmeerw, michal.simek, robh+dt, mark.rutland,
	sudeep.holla, amit.kucheria, leoyang.li, broonie,
	arnaud.pouliquen, eugen.hristev, rdunlap, geert, ak, freeman.liu,
	lukas, vilhelm.gray, gregkh, kstewart, sgoud, anirudh, linux-iio,
	linux-arm-kernel, linux-kernel, devicetree

On Fri, 14 Sep 2018 12:48:28 +0530
Manish Narani <manish.narani@xilinx.com> wrote:

> Add documentation for xilinx-ams driver. This contains information about
> various voltages and temperatures on PS (Processing System), PL
> (Programmable Logic) and AMS Control Block.
> 
> Signed-off-by: Manish Narani <manish.narani@xilinx.com>
The more I look at this device the more I'm convinced it is very much a dedicated
hardware monitoring function, not a generic ADC sensing unit at all.

Hmm.  It is still fine to have it in IIO but you need to think in detail
about how you are going to interface this to hwmon via the iio-hwmon bridge.

Some of the interface complexity should only really be apparent when we hit
hwmon perhaps rather than having so many different custom interfaces in IIO.

Please also loop in the maintainers and lists for hwmon in the next
version so we can get their input.

Thanks,

Jonathan
> ---
>  .../ABI/testing/sysfs-bus-iio-adc-xilinx-ams       | 246 +++++++++++++++++++++
>  1 file changed, 246 insertions(+)
>  create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-adc-xilinx-ams
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-adc-xilinx-ams b/Documentation/ABI/testing/sysfs-bus-iio-adc-xilinx-ams
> new file mode 100644
> index 0000000..589c389
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-bus-iio-adc-xilinx-ams
> @@ -0,0 +1,246 @@
> +What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_vccpsintlp
> +Date:		September 2018
> +KernelVersion:	4.19.0
> +Contact:	mnarani@xilinx.com
> +Description:
> +		Voltage input for PS (Processing System) LPD (Low Power Domain)
> +		power supply.
> +
> +
> +What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_vccpsintfp
> +Date:		September 2018
> +KernelVersion:	4.19.0
> +Contact:	mnarani@xilinx.com
> +Description:
> +		Voltage input for PS (Processing System) FPD (Full Power Domain)
> +		power supply.
> +
> +
> +What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_vccpsaux
> +Date:		September 2018
> +KernelVersion:	4.19.0
> +Contact:	mnarani@xilinx.com
> +Description:
> +		Voltage input for PS (Processing System) Auxiliary power
> +		supply.
> +
> +
> +What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_vccpsddr
> +Date:		September 2018
> +KernelVersion:	4.19.0
> +Contact:	mnarani@xilinx.com
> +Description:
> +		Voltage input for PS (Processing System) I/O bank 504 (DDR
> +		PHY) power supply.
> +
> +
> +What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_vccpsio3
> +Date:		September 2018
> +KernelVersion:	4.19.0
> +Contact:	mnarani@xilinx.com
> +Description:
> +		Voltage input for PS (Processing System) I/O bank 503
> +		(boot, config, JTAG, SRST, POR) power supply.
> +
> +
> +What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_vccpsio0
> +Date:		September 2018
> +KernelVersion:	4.19.0
> +Contact:	mnarani@xilinx.com
> +Description:
> +		Voltage input for PS (Processing System) I/O bank 500
> +		(MIO[0:25]) power supply.
> +
> +
> +What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_vccpsio1
> +Date:		September 2018
> +KernelVersion:	4.19.0
> +Contact:	mnarani@xilinx.com
> +Description:
> +		Voltage input for PS (Processing System) I/O bank 501
> +		(MIO[26:51]) power supply.
> +
> +
> +What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_vccpsio2
> +Date:		September 2018
> +KernelVersion:	4.19.0
> +Contact:	mnarani@xilinx.com
> +Description:
> +		Voltage input for PS (Processing System) I/O bank 502
> +		(MIO[52:77]) power supply.
> +
> +
> +What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_psmgtravcc
> +Date:		September 2018
> +KernelVersion:	4.19.0
> +Contact:	mnarani@xilinx.com
> +Description:
> +		Voltage input for PS (Processing System) GTR SerDes I/O
> +		power supply.
> +
> +
> +What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_psmgtravtt
> +Date:		September 2018
> +KernelVersion:	4.19.0
> +Contact:	mnarani@xilinx.com
> +Description:
> +		Voltage input for PS (Processing System) GTR SerDes
> +		terminators power supply.
> +
> +
> +What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_vccams
> +Date:		September 2018
> +KernelVersion:	4.19.0
> +Contact:	mnarani@xilinx.com
> +Description:
> +		Voltage input for PS (Processing System) SYSMON ADC circuitry
> +		power supply.
> +
> +
> +What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_vccint
> +Date:		September 2018
> +KernelVersion:	4.19.0
> +Contact:	mnarani@xilinx.com
> +Description:
> +		Voltage input for PL (Programmable Logic) internal power supply.
> +
> +
> +What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_vccvrefp
> +Date:		September 2018
> +KernelVersion:	4.19.0
> +Contact:	mnarani@xilinx.com
> +Description:
> +		Voltage input for PL (Programmable Logic) ADC positive V
> +		reference power supply.
> +
> +
> +What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_vccvrefn
> +Date:		September 2018
> +KernelVersion:	4.19.0
> +Contact:	mnarani@xilinx.com
> +Description:
> +		Voltage input for PL (Programmable Logic) ADC negative V
> +		reference power supply.
> +
> +
> +What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_vccplintlp
> +Date:		September 2018
> +KernelVersion:	4.19.0
> +Contact:	mnarani@xilinx.com
> +Description:
> +		Voltage input for PL (Programmable Logic) LPD (Low Power Domain)
> +		power supply.
> +
> +
> +What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_vccplintfp
> +Date:		September 2018
> +KernelVersion:	4.19.0
> +Contact:	mnarani@xilinx.com
> +Description:
> +		Voltage input for PL (Programmable Logic) FPD (Full Power
> +		Domain) power supply.
> +
> +
> +What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_vccplaux
> +Date:		September 2018
> +KernelVersion:	4.19.0
> +Contact:	mnarani@xilinx.com
> +Description:
> +		Voltage input for PL (Programmable Logic) Auxiliary power
> +		supply.
> +
> +
> +What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_vccvpvn
> +Date:		September 2018
> +KernelVersion:	4.19.0
> +Contact:	mnarani@xilinx.com
> +Description:
> +		Voltage input for PL (Programmable Logic) Analog Input Pins
> +		power supply.
> +
> +
> +What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_vuserZ
> +Date:		September 2018
> +KernelVersion:	4.19.0
> +Contact:	mnarani@xilinx.com
> +Description:
> +		Voltage input for PL (Programmable Logic) Fabric Analog Wires 
> +		power supply. Z is the number in range of 0 to 3.
This one superficially sounds like a general purpose input?  The vuser part
doesn't mean anything to the user.  If you care about the pin mapping use the
datasheet name fields.

> +
> +
> +What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_vcc_pspll0
> +Date:		September 2018
> +KernelVersion:	4.19.0
> +Contact:	mnarani@xilinx.com
> +Description:
> +		Voltage input for AMS (Analog Monitoring System) Systems PLLs
> +		power supply.
> +
> +
> +What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_vcc_psbatt
> +Date:		September 2018
> +KernelVersion:	4.19.0
> +Contact:	mnarani@xilinx.com
> +Description:
> +		Voltage input for AMS (Analog Monitoring System) Battery power
> +		supply.

Why does this one merit a ps in it's name and the extra underscore by the vccbram
doesn't?  These need to be consistent.

> +
> +
> +What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_vccbram
> +Date:		September 2018
> +KernelVersion:	4.19.0
> +Contact:	mnarani@xilinx.com
> +Description:
> +		Voltage input for AMS (Analog Monitoring System) Block RAM power
> +		supply.
> +
> +
> +What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_vccaux
> +Date:		September 2018
> +KernelVersion:	4.19.0
> +Contact:	mnarani@xilinx.com
> +Description:
> +		Voltage input for AMS (Analog Monitoring System) Auxiliary power
> +		supply.
> +
> +
> +What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_psddrpll
> +Date:		September 2018
> +KernelVersion:	4.19.0
> +Contact:	mnarani@xilinx.com
> +Description:
> +		Voltage input for AMS (Analog Monitoring System) DDR I/O
> +		PLLs [0:5] power supply.
> +
> +
> +What:		/sys/bus/iio/devices/iio:deviceX/in_voltageY_psintfpddr

That is not a human readable bit of naming...
What is the intf part about for example?  Perhaps use some underscores to break
up the name. Stringing acronyms into one doesn't make it easy to parse in my
head!

> +Date:		September 2018
> +KernelVersion:	4.19.0
> +Contact:	mnarani@xilinx.com
> +Description:
> +		Voltage input for AMS (Analog Monitoring System) DDR controller
> +		power supply.

> +
> +
> +What:		/sys/bus/iio/devices/iio:deviceX/in_tempY_ps_temp
> +Date:		September 2018
> +KernelVersion:	4.19.0
> +Contact:	mnarani@xilinx.com
> +Description:
> +		Temperature input for PS (Processing System) RPU MPCore.
> +
> +
> +What:		/sys/bus/iio/devices/iio:deviceX/in_tempY_remote_temp
> +Date:		September 2018
> +KernelVersion:	4.19.0
> +Contact:	mnarani@xilinx.com
> +Description:
> +		Temperature input for PS (Processing System) APU MPCore.

This presumably is a separate external sensor attached to some pins?
If that's the case then perhaps it needs describing as something like...

Remote temperature sensor attached to the PS (procesisng system) APU MPCore.

For all these temperatures, do you intend to expose them as hwmon?
(which would be more conventional) via the iio-hmwon bridge/

> +
> +
> +What:		/sys/bus/iio/devices/iio:deviceX/in_tempY_pl_temp
> +Date:		September 2018
> +KernelVersion:	4.19.0
> +Contact:	mnarani@xilinx.com
> +Description:
> +		Temperature input for PL (Programmable Logic) SYSMON.


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 3/4] iio: adc: Add Xilinx AMS driver
  2018-09-14  7:18 ` [PATCH v2 3/4] iio: adc: Add Xilinx AMS driver Manish Narani
  2018-09-14 11:59   ` Himanshu Jha
  2018-09-14 21:30   ` Randy Dunlap
@ 2018-09-16 10:34   ` Jonathan Cameron
  2 siblings, 0 replies; 12+ messages in thread
From: Jonathan Cameron @ 2018-09-16 10:34 UTC (permalink / raw)
  To: Manish Narani
  Cc: knaack.h, lars, pmeerw, michal.simek, robh+dt, mark.rutland,
	sudeep.holla, amit.kucheria, leoyang.li, broonie,
	arnaud.pouliquen, eugen.hristev, rdunlap, geert, ak, freeman.liu,
	lukas, vilhelm.gray, gregkh, kstewart, sgoud, anirudh, linux-iio,
	linux-arm-kernel, linux-kernel, devicetree

On Fri, 14 Sep 2018 12:48:29 +0530
Manish Narani <manish.narani@xilinx.com> wrote:

> 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.
> PL-SYSMON block is capable of monitoring off chip voltage and
> temperature.
> PL-SYSMON block has DRP, JTAG and I2C interface to enable monitoring
> from external master. Out of these interface currently only DRP is
> supported.
> Other block PS-SYSMON is memory mapped to PS.
> The AMS can use internal channels to monitor voltage and temperature as
> well as one primary and up to 16 auxiliary channels for measuring
> external voltages.
> The voltage and temperature monitoring channels also have event
> capability which allows to generate an interrupt when their value falls
> below or raises above a set threshold.
> 
> Signed-off-by: Manish Narani <manish.narani@xilinx.com>
A few additional comments from me inline.

Thanks,

Jonathan

> ---
>  drivers/iio/adc/Kconfig      |   10 +
>  drivers/iio/adc/Makefile     |    1 +
>  drivers/iio/adc/xilinx-ams.c | 1337 ++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 1348 insertions(+)
>  create mode 100644 drivers/iio/adc/xilinx-ams.c
> 
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 4a75492..405ea00 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -941,4 +941,14 @@ config XILINX_XADC
>  	  The driver can also be build as a module. If so, the module will be called
>  	  xilinx-xadc.
>  
> +config XILINX_AMS
> +	tristate "Xilinx AMS driver"
> +	depends on ARCH_ZYNQMP || COMPILE_TEST
> +	depends on HAS_IOMEM
> +	help
> +	  Say yes here to have support for the Xilinx AMS.
> +
> +	  The driver can also be build as a module. If so, the module will be called
> +	  xilinx-ams.
> +
>  endmenu
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index 03db7b5..fbfcc45 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -85,4 +85,5 @@ obj-$(CONFIG_VF610_ADC) += vf610_adc.o
>  obj-$(CONFIG_VIPERBOARD_ADC) += viperboard_adc.o
>  xilinx-xadc-y := xilinx-xadc-core.o xilinx-xadc-events.o
>  obj-$(CONFIG_XILINX_XADC) += xilinx-xadc.o
> +obj-$(CONFIG_XILINX_AMS) += xilinx-ams.o
>  obj-$(CONFIG_SD_ADC_MODULATOR) += sd_adc_modulator.o
> diff --git a/drivers/iio/adc/xilinx-ams.c b/drivers/iio/adc/xilinx-ams.c
> new file mode 100644
> index 0000000..c239a02
> --- /dev/null
> +++ b/drivers/iio/adc/xilinx-ams.c
> @@ -0,0 +1,1337 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Xilinx AMS driver
> + *
> + *  Copyright (C) 2017-2018 Xilinx, Inc.
> + *
> + *  Manish Narani <mnarani@xilinx.com>
> + *  Rajnikant Bhojani <rajnikant.bhojani@xilinx.com>
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/iopoll.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of_address.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +
> +#include <linux/iio/events.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +
> +static const unsigned int AMS_UNMASK_TIMEOUT_MS = 500;
> +
> +/* AMS registers definitions */
> +#define AMS_ISR_0			0x010
> +#define AMS_ISR_1			0x014
> +#define AMS_IER_0			0x020
> +#define AMS_IER_1			0x024
> +#define AMS_IDR_0			0x028
> +#define AMS_IDR_1			0x02c
> +#define AMS_PS_CSTS			0x040
> +#define AMS_PL_CSTS			0x044
> +
> +#define AMS_VCC_PSPLL0			0x060
> +#define AMS_VCC_PSPLL3			0x06C
> +#define AMS_VCCINT			0x078
> +#define AMS_VCCBRAM			0x07C
> +#define AMS_VCCAUX			0x080
> +#define AMS_PSDDRPLL			0x084
> +#define AMS_PSINTFPDDR			0x09C
> +
> +#define AMS_VCC_PSPLL0_CH		48
> +#define AMS_VCC_PSPLL3_CH		51
> +#define AMS_VCCINT_CH			54
> +#define AMS_VCCBRAM_CH			55
> +#define AMS_VCCAUX_CH			56
> +#define AMS_PSDDRPLL_CH			57
> +#define AMS_PSINTFPDDR_CH		63
> +
> +#define AMS_REG_CONFIG0			0x100
> +#define AMS_REG_CONFIG1			0x104
> +#define AMS_REG_CONFIG3			0x10C
> +#define AMS_REG_SEQ_CH0			0x120
> +#define AMS_REG_SEQ_CH1			0x124
> +#define AMS_REG_SEQ_CH2			0x118
> +
> +#define AMS_TEMP			0x000
> +#define AMS_SUPPLY1			0x004
> +#define AMS_SUPPLY2			0x008
> +#define AMS_VP_VN			0x00c
> +#define AMS_VREFP			0x010
> +#define AMS_VREFN			0x014
> +#define AMS_SUPPLY3			0x018
> +#define AMS_SUPPLY4			0x034
> +#define AMS_SUPPLY5			0x038
> +#define AMS_SUPPLY6			0x03c
> +#define AMS_SUPPLY7			0x200
> +#define AMS_SUPPLY8			0x204
> +#define AMS_SUPPLY9			0x208
> +#define AMS_SUPPLY10			0x20c
> +#define AMS_VCCAMS			0x210
> +#define AMS_TEMP_REMOTE			0x214
> +
> +#define AMS_REG_VAUX(x)			(0x40 + (4*(x)))
> +
> +#define AMS_PS_RESET_VALUE		0xFFFFU
> +#define AMS_PL_RESET_VALUE		0xFFFFU
> +
> +#define AMS_CONF0_CHANNEL_NUM_MASK	(0x3f << 0)
> +
> +#define AMS_CONF1_SEQ_MASK		(0xf << 12)
> +#define AMS_CONF1_SEQ_DEFAULT		(0 << 12)
> +#define AMS_CONF1_SEQ_CONTINUOUS	(2 << 12)
> +#define AMS_CONF1_SEQ_SINGLE_CHANNEL	(3 << 12)
> +
> +#define AMS_REG_SEQ0_MASK		0xFFFF
> +#define AMS_REG_SEQ2_MASK		0x3F
> +#define AMS_REG_SEQ1_MASK		0xFFFF
> +#define AMS_REG_SEQ2_MASK_SHIFT		16
> +#define AMS_REG_SEQ1_MASK_SHIFT		22
> +
> +#define AMS_REGCFG1_ALARM_MASK		0xF0F
> +#define AMS_REGCFG3_ALARM_MASK		0x3F
> +
> +#define AMS_ALARM_TEMP			0x140
> +#define AMS_ALARM_SUPPLY1		0x144
> +#define AMS_ALARM_SUPPLY2		0x148
> +#define AMS_ALARM_SUPPLY3		0x160
> +#define AMS_ALARM_SUPPLY4		0x164
> +#define AMS_ALARM_SUPPLY5		0x168
> +#define AMS_ALARM_SUPPLY6		0x16c
> +#define AMS_ALARM_SUPPLY7		0x180
> +#define AMS_ALARM_SUPPLY8		0x184
> +#define AMS_ALARM_SUPPLY9		0x188
> +#define AMS_ALARM_SUPPLY10		0x18c
> +#define AMS_ALARM_VCCAMS		0x190
> +#define AMS_ALARM_TEMP_REMOTE		0x194
> +#define AMS_ALARM_THRESHOLD_OFF_10	0x10
> +#define AMS_ALARM_THRESHOLD_OFF_20	0x20
> +
> +#define AMS_ALARM_THR_DIRECT_MASK	0x01
> +#define AMS_ALARM_THR_MIN		0x0000
> +#define AMS_ALARM_THR_MAX		0xffff
> +
> +#define AMS_NO_OF_ALARMS		32
> +#define AMS_PL_ALARM_START		16
> +#define AMS_ISR0_ALARM_MASK		0xFFFFFFFFU
> +#define AMS_ISR1_ALARM_MASK		0xE000001FU
> +#define AMS_ISR1_EOC_MASK		0x00000008U
> +#define AMS_ISR1_INTR_MASK_SHIFT	32
> +#define AMS_ISR0_ALARM_2_TO_0_MASK	0x07
> +#define AMS_ISR0_ALARM_6_TO_3_MASK	0x78
> +#define AMS_ISR0_ALARM_12_TO_7_MASK	0x3F
> +#define AMS_CONF1_ALARM_2_TO_0_SHIFT	1
> +#define AMS_CONF1_ALARM_6_TO_3_SHIFT	5
> +#define AMS_CONF3_ALARM_12_TO_7_SHIFT	8
> +
> +#define AMS_PS_CSTS_PS_READY		0x08010000U
> +#define AMS_PL_CSTS_ACCESS_MASK		0x00000001U
> +
> +#define AMS_PL_MAX_FIXED_CHANNEL	10
> +#define AMS_PL_MAX_EXT_CHANNEL		20
> +
> +#define AMS_INIT_TIMEOUT		10000
> +
> +/*
> + * Following scale and offset value is derived from
> + * UG580 (v1.7) December 20, 2016
> + */
> +#define AMS_SUPPLY_SCALE_1VOLT		1000
> +#define AMS_SUPPLY_SCALE_3VOLT		3000
> +#define AMS_SUPPLY_SCALE_6VOLT		6000
> +#define AMS_SUPPLY_SCALE_DIV_BIT	16
> +
> +#define AMS_TEMP_SCALE			509314
> +#define AMS_TEMP_SCALE_DIV_BIT		16
> +#define AMS_TEMP_OFFSET			-((280230L << 16) / 509314)
> +
> +enum ams_alarm_bit {
> +	AMS_ALARM_BIT_TEMP,
> +	AMS_ALARM_BIT_SUPPLY1,
> +	AMS_ALARM_BIT_SUPPLY2,
> +	AMS_ALARM_BIT_SUPPLY3,
> +	AMS_ALARM_BIT_SUPPLY4,
> +	AMS_ALARM_BIT_SUPPLY5,
> +	AMS_ALARM_BIT_SUPPLY6,
> +	AMS_ALARM_BIT_RESERVED,
> +	AMS_ALARM_BIT_SUPPLY7,
> +	AMS_ALARM_BIT_SUPPLY8,
> +	AMS_ALARM_BIT_SUPPLY9,
> +	AMS_ALARM_BIT_SUPPLY10,
> +	AMS_ALARM_BIT_VCCAMS,
> +	AMS_ALARM_BIT_TEMP_REMOTE
> +};
> +
> +enum ams_seq {
> +	AMS_SEQ_VCC_PSPLL,
> +	AMS_SEQ_VCC_PSBATT,
> +	AMS_SEQ_VCCINT,
> +	AMS_SEQ_VCCBRAM,
> +	AMS_SEQ_VCCAUX,
> +	AMS_SEQ_PSDDRPLL,
> +	AMS_SEQ_INTDDR
> +};
> +
> +enum ams_ps_pl_seq {
> +	AMS_SEQ_CALIB,
> +	AMS_SEQ_RSVD_1,
> +	AMS_SEQ_RSVD_2,
> +	AMS_SEQ_TEST,
> +	AMS_SEQ_RSVD_4,
> +	AMS_SEQ_SUPPLY4,
> +	AMS_SEQ_SUPPLY5,
> +	AMS_SEQ_SUPPLY6,
> +	AMS_SEQ_TEMP,
> +	AMS_SEQ_SUPPLY2,
> +	AMS_SEQ_SUPPLY1,
> +	AMS_SEQ_VP_VN,
> +	AMS_SEQ_VREFP,
> +	AMS_SEQ_VREFN,
> +	AMS_SEQ_SUPPLY3,
> +	AMS_SEQ_CURRENT_MON,
> +	AMS_SEQ_SUPPLY7,
> +	AMS_SEQ_SUPPLY8,
> +	AMS_SEQ_SUPPLY9,
> +	AMS_SEQ_SUPPLY10,
> +	AMS_SEQ_VCCAMS,
> +	AMS_SEQ_TEMP_REMOTE,
> +	AMS_SEQ_MAX
> +};
> +
> +#define AMS_SEQ(x)          (AMS_SEQ_MAX + (x))
> +#define AMS_VAUX_SEQ(x)     (AMS_SEQ_MAX + (x))
> +
> +#define PS_SEQ_MAX          AMS_SEQ_MAX
> +#define PS_SEQ(x)           (x)
> +#define PL_SEQ(x)           (PS_SEQ_MAX + x)
> +
> +#define AMS_CHAN_TEMP(_scan_index, _addr, _ext) { \
> +	.type = IIO_TEMP, \
> +	.indexed = 1, \
> +	.address = (_addr), \
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
> +		BIT(IIO_CHAN_INFO_SCALE) | \
> +		BIT(IIO_CHAN_INFO_OFFSET), \
> +	.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ), \
> +	.event_spec = ams_temp_events, \
> +	.num_event_specs = ARRAY_SIZE(ams_temp_events), \
> +	.scan_index = (_scan_index), \
> +	.scan_type = { \
> +		.sign = 'u', \
> +		.realbits = 12, \
> +		.storagebits = 16, \
> +		.shift = 4, \
> +		.endianness = IIO_CPU, \
> +	}, \
> +	.extend_name = _ext, \
> +}
> +
> +#define AMS_CHAN_VOLTAGE(_scan_index, _addr, _ext, _alarm) { \
> +	.type = IIO_VOLTAGE, \
> +	.indexed = 1, \
> +	.address = (_addr), \
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
> +		BIT(IIO_CHAN_INFO_SCALE), \
> +	.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ), \
> +	.event_spec = (_alarm) ? ams_voltage_events : NULL, \
> +	.num_event_specs = (_alarm) ? ARRAY_SIZE(ams_voltage_events) : 0, \
> +	.scan_index = (_scan_index), \
> +	.scan_type = { \
> +		.realbits = 10, \
> +		.storagebits = 16, \
> +		.shift = 6, \
> +		.endianness = IIO_CPU, \
> +	}, \
> +	.extend_name = _ext, \
> +}
> +
> +#define AMS_PS_CHAN_TEMP(_scan_index, _addr, _ext) \
> +	AMS_CHAN_TEMP(PS_SEQ(_scan_index), _addr, _ext)
> +#define AMS_PS_CHAN_VOLTAGE(_scan_index, _addr, _ext) \
> +	AMS_CHAN_VOLTAGE(PS_SEQ(_scan_index), _addr, _ext, true)
> +
> +#define AMS_PL_CHAN_TEMP(_scan_index, _addr, _ext) \
> +	AMS_CHAN_TEMP(PL_SEQ(_scan_index), _addr, _ext)
> +#define AMS_PL_CHAN_VOLTAGE(_scan_index, _addr, _ext, _alarm) \
> +	AMS_CHAN_VOLTAGE(PL_SEQ(_scan_index), _addr, _ext, _alarm)
> +#define AMS_PL_AUX_CHAN_VOLTAGE(_auxno, _ext) \
> +	AMS_CHAN_VOLTAGE(PL_SEQ(AMS_VAUX_SEQ(_auxno)), \
> +			AMS_REG_VAUX(_auxno), _ext, false)
> +#define AMS_CTRL_CHAN_VOLTAGE(_scan_index, _addr, _ext) \
> +	AMS_CHAN_VOLTAGE(PL_SEQ(AMS_VAUX_SEQ(AMS_SEQ(_scan_index))), \
> +			_addr, _ext, false)
> +
> +struct ams {
> +	void __iomem *base;
> +	void __iomem *ps_base;
> +	void __iomem *pl_base;
> +	struct clk *clk;
> +	struct device *dev;
> +	struct mutex lock;
> +	unsigned int alarm_mask;
> +	unsigned int masked_alarm;
> +	u64 intr_mask;
> +	int irq;
> +	struct delayed_work ams_unmask_work;
> +};
> +
> +static inline void ams_ps_update_reg(struct ams *ams, unsigned int offset,
> +				     u32 mask, u32 data)
> +{
> +	u32 val;
> +
> +	val = readl(ams->ps_base + offset);
> +	writel((val & ~mask) | (data & mask), ams->ps_base + offset);
> +}
> +
> +static inline void ams_pl_update_reg(struct ams *ams, unsigned int offset,
> +					 u32 mask, u32 data)
> +{
> +	u32 val;
> +
> +	val = readl(ams->pl_base + offset);
> +	writel((val & ~mask) | (data & mask), ams->pl_base + offset);
> +}
> +
> +static void ams_update_intrmask(struct ams *ams, u64 mask, u64 val)
> +{
> +	ams->intr_mask &= ~mask;
> +	ams->intr_mask |= (val & mask);
> +
> +	writel(~(ams->intr_mask | ams->masked_alarm), ams->base + AMS_IER_0);
> +	writel(~(ams->intr_mask >> AMS_ISR1_INTR_MASK_SHIFT),
> +			ams->base + AMS_IER_1);
> +	writel(ams->intr_mask | ams->masked_alarm, ams->base + AMS_IDR_0);
> +	writel(ams->intr_mask >> AMS_ISR1_INTR_MASK_SHIFT,
> +			ams->base + AMS_IDR_1);
> +}
> +
> +static void ams_disable_all_alarms(struct ams *ams)
> +{
> +	/* disable PS module alarm */
> +	if (ams->ps_base) {
> +		ams_ps_update_reg(ams, AMS_REG_CONFIG1, AMS_REGCFG1_ALARM_MASK,
> +				  AMS_REGCFG1_ALARM_MASK);
> +		ams_ps_update_reg(ams, AMS_REG_CONFIG3, AMS_REGCFG3_ALARM_MASK,
> +				  AMS_REGCFG3_ALARM_MASK);
> +	}
> +
> +	/* disable PL module alarm */
> +	if (ams->pl_base) {
> +		ams_pl_update_reg(ams, AMS_REG_CONFIG1,
> +				    AMS_REGCFG1_ALARM_MASK,
> +				    AMS_REGCFG1_ALARM_MASK);
> +		ams_pl_update_reg(ams, AMS_REG_CONFIG3,
> +				    AMS_REGCFG3_ALARM_MASK,
> +				    AMS_REGCFG3_ALARM_MASK);
> +	}
> +}
> +
> +static void iio_ams_update_alarm(struct ams *ams, unsigned long alarm_mask)
> +{
> +	u32 cfg;
> +	unsigned long pl_alarm_mask;
> +
> +	if (ams->ps_base) {
> +		/* Configuring PS alarm enable */
> +		cfg = ~((alarm_mask & AMS_ISR0_ALARM_2_TO_0_MASK) <<
> +			       AMS_CONF1_ALARM_2_TO_0_SHIFT);
> +		cfg &= ~((alarm_mask & AMS_ISR0_ALARM_6_TO_3_MASK) <<
> +				AMS_CONF1_ALARM_6_TO_3_SHIFT);
> +		ams_ps_update_reg(ams, AMS_REG_CONFIG1, AMS_REGCFG1_ALARM_MASK,
> +				  cfg);
> +
> +		cfg = ~((alarm_mask >> AMS_CONF3_ALARM_12_TO_7_SHIFT) &
> +				AMS_ISR0_ALARM_12_TO_7_MASK);
> +		ams_ps_update_reg(ams, AMS_REG_CONFIG3, AMS_REGCFG3_ALARM_MASK,
> +				  cfg);
> +	}
> +
> +	if (ams->pl_base) {
> +		pl_alarm_mask = (alarm_mask >> AMS_PL_ALARM_START);
> +		/* Configuring PL alarm enable */
> +		cfg = ~((pl_alarm_mask & AMS_ISR0_ALARM_2_TO_0_MASK) <<
> +			       AMS_CONF1_ALARM_2_TO_0_SHIFT);
> +		cfg &= ~((pl_alarm_mask & AMS_ISR0_ALARM_6_TO_3_MASK) <<
> +				AMS_CONF1_ALARM_6_TO_3_SHIFT);
> +		ams_pl_update_reg(ams, AMS_REG_CONFIG1,
> +				AMS_REGCFG1_ALARM_MASK, cfg);
> +
> +		cfg = ~((pl_alarm_mask >> AMS_CONF3_ALARM_12_TO_7_SHIFT) &
> +				AMS_ISR0_ALARM_12_TO_7_MASK);
> +		ams_pl_update_reg(ams, AMS_REG_CONFIG3,
> +				AMS_REGCFG3_ALARM_MASK, cfg);
> +	}
> +
> +	mutex_lock(&ams->lock);
> +	ams_update_intrmask(ams, AMS_ISR0_ALARM_MASK, ~alarm_mask);
> +	mutex_unlock(&ams->lock);
> +}
> +
> +static void ams_enable_channel_sequence(struct ams *ams)
> +{
> +	int i;
> +	unsigned long long scan_mask;
> +	struct iio_dev *indio_dev = iio_priv_to_dev(ams);
> +
> +	/*
> +	 * Enable channel sequence. First 22 bit of scan_mask represent
> +	 * PS channels, and next remaining bit represents PL channels.
> +	 */
> +
> +	/* Run calibration of PS & PL as part of the sequence */
> +	scan_mask = 0x1 | BIT(PS_SEQ_MAX);
> +	for (i = 0; i < indio_dev->num_channels; i++)
> +		scan_mask |= BIT(indio_dev->channels[i].scan_index);
> +
> +	if (ams->ps_base) {
> +		/* put sysmon in a soft reset to change the sequence */
> +		ams_ps_update_reg(ams, AMS_REG_CONFIG1, AMS_CONF1_SEQ_MASK,
> +				  AMS_CONF1_SEQ_DEFAULT);
> +
> +		/* configure basic channels */
> +		writel(scan_mask & AMS_REG_SEQ0_MASK,
> +				ams->ps_base + AMS_REG_SEQ_CH0);
> +		writel(AMS_REG_SEQ2_MASK &
> +			(scan_mask >> AMS_REG_SEQ2_MASK_SHIFT),
> +			ams->ps_base + AMS_REG_SEQ_CH2);
> +
> +		/* set continuous sequence mode */
> +		ams_ps_update_reg(ams, AMS_REG_CONFIG1, AMS_CONF1_SEQ_MASK,
> +				  AMS_CONF1_SEQ_CONTINUOUS);
> +	}
> +
> +	if (ams->pl_base) {
> +		/* put sysmon in a soft reset to change the sequence */
> +		ams_pl_update_reg(ams, AMS_REG_CONFIG1, AMS_CONF1_SEQ_MASK,
> +				    AMS_CONF1_SEQ_DEFAULT);
> +
> +		/* configure basic channels */
> +		scan_mask = scan_mask >> PS_SEQ_MAX;
> +		writel(scan_mask & AMS_REG_SEQ0_MASK,
> +				ams->pl_base + AMS_REG_SEQ_CH0);
> +		writel(AMS_REG_SEQ2_MASK &
> +				(scan_mask >> AMS_REG_SEQ2_MASK_SHIFT),
> +				ams->pl_base + AMS_REG_SEQ_CH2);
> +		writel(AMS_REG_SEQ1_MASK &
> +				(scan_mask >> AMS_REG_SEQ1_MASK_SHIFT),
> +				ams->pl_base + AMS_REG_SEQ_CH1);
> +
> +		/* set continuous sequence mode */
> +		ams_pl_update_reg(ams, AMS_REG_CONFIG1, AMS_CONF1_SEQ_MASK,
> +				AMS_CONF1_SEQ_CONTINUOUS);
> +	}
> +}
> +
> +static int iio_ams_init_device(struct ams *ams)
> +{
> +	u32 reg;
> +	int ret;
> +
> +	/* reset AMS */
> +	if (ams->ps_base) {
> +		writel(AMS_PS_RESET_VALUE, ams->ps_base + AMS_VP_VN);
> +
> +		ret = readl_poll_timeout(ams->base + AMS_PS_CSTS, reg,
> +					 (reg & AMS_PS_CSTS_PS_READY) ==
> +					 AMS_PS_CSTS_PS_READY, 0,
> +					 AMS_INIT_TIMEOUT);
> +		if (ret)
> +			return ret;
> +
> +		/* put sysmon in a default state */
> +		ams_ps_update_reg(ams, AMS_REG_CONFIG1, AMS_CONF1_SEQ_MASK,
> +				  AMS_CONF1_SEQ_DEFAULT);
> +	}
> +
> +	if (ams->pl_base) {
> +		writel(AMS_PL_RESET_VALUE, ams->pl_base + AMS_VP_VN);
> +
> +		ret = readl_poll_timeout(ams->base + AMS_PL_CSTS, reg,
> +					 (reg & AMS_PL_CSTS_ACCESS_MASK) ==
> +					 AMS_PL_CSTS_ACCESS_MASK, 0,
> +					 AMS_INIT_TIMEOUT);
> +		if (ret)
> +			return ret;
> +
> +		/* put sysmon in a default state */
> +		ams_pl_update_reg(ams, AMS_REG_CONFIG1, AMS_CONF1_SEQ_MASK,
> +				    AMS_CONF1_SEQ_DEFAULT);
> +	}
> +
> +	ams_disable_all_alarms(ams);
> +
> +	/* Disable interrupt */
> +	ams_update_intrmask(ams, ~0, ~0);
> +
> +	/* Clear any pending interrupt */
> +	writel(AMS_ISR0_ALARM_MASK, ams->base + AMS_ISR_0);
> +	writel(AMS_ISR1_ALARM_MASK, ams->base + AMS_ISR_1);
> +
> +	return 0;
> +}
> +
> +static int ams_enable_single_channel(struct ams *ams, unsigned int offset)
> +{
> +	u8 channel_num = 0;
> +
> +	switch (offset) {
> +	case AMS_VCC_PSPLL0:
> +		channel_num = AMS_VCC_PSPLL0_CH;
> +		break;
> +	case AMS_VCC_PSPLL3:
> +		channel_num = AMS_VCC_PSPLL3_CH;
> +		break;
> +	case AMS_VCCINT:
> +		channel_num = AMS_VCCINT_CH;
> +		break;
> +	case AMS_VCCBRAM:
> +		channel_num = AMS_VCCBRAM_CH;
> +		break;
> +	case AMS_VCCAUX:
> +		channel_num = AMS_VCCAUX_CH;
> +		break;
> +	case AMS_PSDDRPLL:
> +		channel_num = AMS_PSDDRPLL_CH;
> +		break;
> +	case AMS_PSINTFPDDR:
> +		channel_num = AMS_PSINTFPDDR_CH;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	/* set single channel, sequencer off mode */
> +	ams_ps_update_reg(ams, AMS_REG_CONFIG1, AMS_CONF1_SEQ_MASK,
> +			AMS_CONF1_SEQ_SINGLE_CHANNEL);
> +
> +	/* write the channel number */
> +	ams_ps_update_reg(ams, AMS_REG_CONFIG0, AMS_CONF0_CHANNEL_NUM_MASK,
> +			channel_num);
> +	return 0;
> +}
> +
> +static int ams_read_vcc_reg(struct ams *ams, unsigned int offset, u32 *data)
> +{
> +	u32 reg;
> +	int ret;
> +
> +	ret = ams_enable_single_channel(ams, offset);
> +	if (ret)
> +		return ret;
> +
> +	ret = readl_poll_timeout(ams->base + AMS_ISR_1, reg,
> +				 (reg & AMS_ISR1_EOC_MASK) == AMS_ISR1_EOC_MASK,
> +				 0, AMS_INIT_TIMEOUT);
> +	if (ret)
> +		return ret;
> +
> +	*data = readl(ams->base + offset);
> +	ams_enable_channel_sequence(ams);
> +
> +	return 0;
> +}
> +
> +static int ams_read_raw(struct iio_dev *indio_dev,
> +			struct iio_chan_spec const *chan,
> +			int *val, int *val2, long mask)
> +{
> +	struct ams *ams = iio_priv(indio_dev);
> +	int ret;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		mutex_lock(&ams->lock);
> +		if (chan->scan_index >= (PS_SEQ_MAX * 3)) {
> +			ret = ams_read_vcc_reg(ams, chan->address, val);
> +			if (ret) {
> +				mutex_unlock(&ams->lock);
> +				return -EINVAL;
> +			}
> +		} else if (chan->scan_index >= PS_SEQ_MAX)
> +			*val = readl(ams->pl_base + chan->address);
> +		else
> +			*val = readl(ams->ps_base + chan->address);
> +		mutex_unlock(&ams->lock);
> +
> +		return IIO_VAL_INT;
> +	case IIO_CHAN_INFO_SCALE:
> +		switch (chan->type) {
> +		case IIO_VOLTAGE:
> +			switch (chan->address) {
> +			case AMS_SUPPLY1:
> +			case AMS_SUPPLY2:
> +			case AMS_SUPPLY3:
> +			case AMS_SUPPLY4:
> +				*val = AMS_SUPPLY_SCALE_3VOLT;
> +				break;
> +			case AMS_SUPPLY5:
> +			case AMS_SUPPLY6:
> +				if (chan->scan_index < PS_SEQ_MAX)
> +					*val = AMS_SUPPLY_SCALE_6VOLT;
> +				else
> +					*val = AMS_SUPPLY_SCALE_3VOLT;
> +				break;
> +			case AMS_SUPPLY7:
> +			case AMS_SUPPLY8:
> +				*val = AMS_SUPPLY_SCALE_6VOLT;
> +				break;
> +			case AMS_SUPPLY9:
> +			case AMS_SUPPLY10:
> +				if (chan->scan_index < PS_SEQ_MAX)
> +					*val = AMS_SUPPLY_SCALE_3VOLT;
> +				else
> +					*val = AMS_SUPPLY_SCALE_6VOLT;
> +				break;
> +			case AMS_VCC_PSPLL0:
> +			case AMS_VCC_PSPLL3:
> +			case AMS_VCCINT:
> +			case AMS_VCCBRAM:
> +			case AMS_VCCAUX:
> +			case AMS_PSDDRPLL:
> +			case AMS_PSINTFPDDR:
> +				*val = AMS_SUPPLY_SCALE_3VOLT;
> +				break;
> +			default:
> +				*val = AMS_SUPPLY_SCALE_1VOLT;
> +				break;
> +			}
> +			*val2 = AMS_SUPPLY_SCALE_DIV_BIT;
> +			return IIO_VAL_FRACTIONAL_LOG2;
> +		case IIO_TEMP:
> +			*val = AMS_TEMP_SCALE;
> +			*val2 = AMS_TEMP_SCALE_DIV_BIT;
> +			return IIO_VAL_FRACTIONAL_LOG2;
> +		default:
> +			return -EINVAL;
> +		}
> +	case IIO_CHAN_INFO_OFFSET:
> +		/* Only the temperature channel has an offset */
> +		*val = AMS_TEMP_OFFSET;
> +		return IIO_VAL_INT;
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static int ams_get_alarm_offset(int scan_index, enum iio_event_direction dir)
> +{
> +	int offset = 0;
> +
> +	if (scan_index >= PS_SEQ_MAX)
> +		scan_index -= PS_SEQ_MAX;
> +
> +	if (dir == IIO_EV_DIR_FALLING) {
> +		if (scan_index < AMS_SEQ_SUPPLY7)
> +			offset = AMS_ALARM_THRESHOLD_OFF_10;
> +		else
> +			offset = AMS_ALARM_THRESHOLD_OFF_20;
> +	}
> +
> +	switch (scan_index) {
> +	case AMS_SEQ_TEMP:
> +		return AMS_ALARM_TEMP + offset;
> +	case AMS_SEQ_SUPPLY1:
> +		return AMS_ALARM_SUPPLY1 + offset;
> +	case AMS_SEQ_SUPPLY2:
> +		return AMS_ALARM_SUPPLY2 + offset;
> +	case AMS_SEQ_SUPPLY3:
> +		return AMS_ALARM_SUPPLY3 + offset;
> +	case AMS_SEQ_SUPPLY4:
> +		return AMS_ALARM_SUPPLY4 + offset;
> +	case AMS_SEQ_SUPPLY5:
> +		return AMS_ALARM_SUPPLY5 + offset;
> +	case AMS_SEQ_SUPPLY6:
> +		return AMS_ALARM_SUPPLY6 + offset;
> +	case AMS_SEQ_SUPPLY7:
> +		return AMS_ALARM_SUPPLY7 + offset;
> +	case AMS_SEQ_SUPPLY8:
> +		return AMS_ALARM_SUPPLY8 + offset;
> +	case AMS_SEQ_SUPPLY9:
> +		return AMS_ALARM_SUPPLY9 + offset;
> +	case AMS_SEQ_SUPPLY10:
> +		return AMS_ALARM_SUPPLY10 + offset;
> +	case AMS_SEQ_VCCAMS:
> +		return AMS_ALARM_VCCAMS + offset;
> +	case AMS_SEQ_TEMP_REMOTE:
> +		return AMS_ALARM_TEMP_REMOTE + offset;
> +	}
> +
> +	return 0;
> +}
> +
> +static const struct iio_chan_spec *ams_event_to_channel(
> +		struct iio_dev *indio_dev, u32 event)
> +{
> +	int scan_index = 0, i;
> +
> +	if (event >= AMS_PL_ALARM_START) {
> +		event -= AMS_PL_ALARM_START;
> +		scan_index = PS_SEQ_MAX;
> +	}
> +
> +	switch (event) {
> +	case AMS_ALARM_BIT_TEMP:
> +		scan_index += AMS_SEQ_TEMP;
> +		break;
> +	case AMS_ALARM_BIT_SUPPLY1:
> +		scan_index += AMS_SEQ_SUPPLY1;
> +		break;
> +	case AMS_ALARM_BIT_SUPPLY2:
> +		scan_index += AMS_SEQ_SUPPLY2;
> +		break;
> +	case AMS_ALARM_BIT_SUPPLY3:
> +		scan_index += AMS_SEQ_SUPPLY3;
> +		break;
> +	case AMS_ALARM_BIT_SUPPLY4:
> +		scan_index += AMS_SEQ_SUPPLY4;
> +		break;
> +	case AMS_ALARM_BIT_SUPPLY5:
> +		scan_index += AMS_SEQ_SUPPLY5;
> +		break;
> +	case AMS_ALARM_BIT_SUPPLY6:
> +		scan_index += AMS_SEQ_SUPPLY6;
> +		break;
> +	case AMS_ALARM_BIT_SUPPLY7:
> +		scan_index += AMS_SEQ_SUPPLY7;
> +		break;
> +	case AMS_ALARM_BIT_SUPPLY8:
> +		scan_index += AMS_SEQ_SUPPLY8;
> +		break;
> +	case AMS_ALARM_BIT_SUPPLY9:
> +		scan_index += AMS_SEQ_SUPPLY9;
> +		break;
> +	case AMS_ALARM_BIT_SUPPLY10:
> +		scan_index += AMS_SEQ_SUPPLY10;
> +		break;
> +	case AMS_ALARM_BIT_VCCAMS:
> +		scan_index += AMS_SEQ_VCCAMS;
> +		break;
> +	case AMS_ALARM_BIT_TEMP_REMOTE:
> +		scan_index += AMS_SEQ_TEMP_REMOTE;
> +		break;
> +	}
> +
> +	for (i = 0; i < indio_dev->num_channels; i++)
> +		if (indio_dev->channels[i].scan_index == scan_index)
> +			break;
> +
> +	return &indio_dev->channels[i];
> +}
> +
> +static int ams_get_alarm_mask(int scan_index)
> +{
> +	int bit = 0;
> +
> +	if (scan_index >= PS_SEQ_MAX) {
> +		bit = AMS_PL_ALARM_START;
> +		scan_index -= PS_SEQ_MAX;
> +	}
> +
> +	switch (scan_index) {
> +	case AMS_SEQ_TEMP:
> +		return BIT(AMS_ALARM_BIT_TEMP + bit);
> +	case AMS_SEQ_SUPPLY1:
> +		return BIT(AMS_ALARM_BIT_SUPPLY1 + bit);
> +	case AMS_SEQ_SUPPLY2:
> +		return BIT(AMS_ALARM_BIT_SUPPLY2 + bit);
> +	case AMS_SEQ_SUPPLY3:
> +		return BIT(AMS_ALARM_BIT_SUPPLY3 + bit);
> +	case AMS_SEQ_SUPPLY4:
> +		return BIT(AMS_ALARM_BIT_SUPPLY4 + bit);
> +	case AMS_SEQ_SUPPLY5:
> +		return BIT(AMS_ALARM_BIT_SUPPLY5 + bit);
> +	case AMS_SEQ_SUPPLY6:
> +		return BIT(AMS_ALARM_BIT_SUPPLY6 + bit);
> +	case AMS_SEQ_SUPPLY7:
> +		return BIT(AMS_ALARM_BIT_SUPPLY7 + bit);
> +	case AMS_SEQ_SUPPLY8:
> +		return BIT(AMS_ALARM_BIT_SUPPLY8 + bit);
> +	case AMS_SEQ_SUPPLY9:
> +		return BIT(AMS_ALARM_BIT_SUPPLY9 + bit);
> +	case AMS_SEQ_SUPPLY10:
> +		return BIT(AMS_ALARM_BIT_SUPPLY10 + bit);
> +	case AMS_SEQ_VCCAMS:
> +		return BIT(AMS_ALARM_BIT_VCCAMS + bit);
> +	case AMS_SEQ_TEMP_REMOTE:
> +		return BIT(AMS_ALARM_BIT_TEMP_REMOTE + bit);
> +	}
> +
> +	return 0;
> +}
> +
> +static int ams_read_event_config(struct iio_dev *indio_dev,
> +				 const struct iio_chan_spec *chan,
> +				 enum iio_event_type type,
> +				 enum iio_event_direction dir)
> +{
> +	struct ams *ams = iio_priv(indio_dev);
> +
> +	return (ams->alarm_mask & ams_get_alarm_mask(chan->scan_index)) ? 1 : 0;
> +}
> +
> +static int ams_write_event_config(struct iio_dev *indio_dev,
> +				  const struct iio_chan_spec *chan,
> +				  enum iio_event_type type,
> +				  enum iio_event_direction dir,
> +				  int state)
> +{
> +	struct ams *ams = iio_priv(indio_dev);
> +	unsigned int alarm;
> +
> +	alarm = ams_get_alarm_mask(chan->scan_index);
> +
> +	mutex_lock(&ams->lock);
> +
> +	if (state)
> +		ams->alarm_mask |= alarm;
> +	else
> +		ams->alarm_mask &= ~alarm;
> +
> +	iio_ams_update_alarm(ams, ams->alarm_mask);
> +
> +	mutex_unlock(&ams->lock);
> +
> +	return 0;
> +}
> +
> +static int ams_read_event_value(struct iio_dev *indio_dev,
> +				const struct iio_chan_spec *chan,
> +				enum iio_event_type type,
> +				enum iio_event_direction dir,
> +				enum iio_event_info info, int *val, int *val2)
> +{
> +	struct ams *ams = iio_priv(indio_dev);
> +	unsigned int offset = ams_get_alarm_offset(chan->scan_index, dir);
> +
> +	mutex_lock(&ams->lock);
> +
> +	if (chan->scan_index >= PS_SEQ_MAX)
> +		*val = readl(ams->pl_base + offset);
> +	else
> +		*val = readl(ams->ps_base + offset);
> +
> +	mutex_unlock(&ams->lock);
> +
> +	return IIO_VAL_INT;
> +}
> +
> +static int ams_write_event_value(struct iio_dev *indio_dev,
> +				 const struct iio_chan_spec *chan,
> +				 enum iio_event_type type,
> +				 enum iio_event_direction dir,
> +				 enum iio_event_info info, int val, int val2)
> +{
> +	struct ams *ams = iio_priv(indio_dev);
> +	unsigned int offset;
> +
> +	mutex_lock(&ams->lock);
> +
> +	/* Set temperature channel threshold to direct threshold */
> +	if (chan->type == IIO_TEMP) {
> +		offset = ams_get_alarm_offset(chan->scan_index,
> +					      IIO_EV_DIR_FALLING);
> +
> +		if (chan->scan_index >= PS_SEQ_MAX)
> +			ams_pl_update_reg(ams, offset,
> +					    AMS_ALARM_THR_DIRECT_MASK,
> +					    AMS_ALARM_THR_DIRECT_MASK);
> +		else
> +			ams_ps_update_reg(ams, offset,
> +					  AMS_ALARM_THR_DIRECT_MASK,
> +					  AMS_ALARM_THR_DIRECT_MASK);
> +	}
> +
> +	offset = ams_get_alarm_offset(chan->scan_index, dir);
> +	if (chan->scan_index >= PS_SEQ_MAX)
> +		writel(val, ams->pl_base + offset);
> +	else
> +		writel(val, ams->ps_base + offset);
> +
> +	mutex_unlock(&ams->lock);
> +
> +	return 0;
> +}
> +
> +static void ams_handle_event(struct iio_dev *indio_dev, u32 event)
> +{
> +	const struct iio_chan_spec *chan;
> +
> +	chan = ams_event_to_channel(indio_dev, event);
> +
> +	if (chan->type == IIO_TEMP) {
> +		/*
> +		 * The temperature channel only supports over-temperature
> +		 * events
> +		 */
> +		iio_push_event(indio_dev,
> +			       IIO_UNMOD_EVENT_CODE(chan->type, chan->channel,
> +						    IIO_EV_TYPE_THRESH,
> +						    IIO_EV_DIR_RISING),
> +			iio_get_time_ns(indio_dev));
> +	} else {
> +		/*
> +		 * For other channels we don't know whether it is a upper or
> +		 * lower threshold event. Userspace will have to check the
> +		 * channel value if it wants to know.
> +		 */
> +		iio_push_event(indio_dev,
> +			       IIO_UNMOD_EVENT_CODE(chan->type, chan->channel,
> +						    IIO_EV_TYPE_THRESH,
> +						    IIO_EV_DIR_EITHER),
> +			iio_get_time_ns(indio_dev));
> +	}
> +}
> +
> +static void ams_handle_events(struct iio_dev *indio_dev, unsigned long events)
> +{
> +	unsigned int bit;
> +
> +	for_each_set_bit(bit, &events, AMS_NO_OF_ALARMS)
> +		ams_handle_event(indio_dev, bit);
> +}
> +
> +/**
> + * ams_unmask_worker - ams alarm interrupt unmask worker
> + * @work :		work to be done
> + *
> + * The ZynqMP threshold interrupts are level sensitive. Since we can't make the
> + * threshold condition go way from within the interrupt handler, this means as
> + * soon as a threshold condition is present we would enter the interrupt handler
> + * again and again. To work around this we mask all active threshold interrupts
> + * in the interrupt handler and start a timer. In this timer we poll the
> + * interrupt status and only if the interrupt is inactive we unmask it again.
> + */
> +static void ams_unmask_worker(struct work_struct *work)
> +{
> +	struct ams *ams = container_of(work, struct ams, ams_unmask_work.work);
> +	unsigned int status, unmask;
> +
> +	mutex_lock(&ams->lock);
> +
> +	status = readl(ams->base + AMS_ISR_0);
> +
> +	/* Clear those bits which are not active anymore */
> +	unmask = (ams->masked_alarm ^ status) & ams->masked_alarm;
> +
> +	/* clear status of disabled alarm */
> +	unmask |= ams->intr_mask;
> +
> +	ams->masked_alarm &= status;
> +
> +	/* Also clear those which are masked out anyway */
> +	ams->masked_alarm &= ~ams->intr_mask;
> +
> +	/* Clear the interrupts before we unmask them */
> +	writel(unmask, ams->base + AMS_ISR_0);
> +
> +	ams_update_intrmask(ams, 0, 0);
> +
> +	mutex_unlock(&ams->lock);
> +
> +	/* if still pending some alarm re-trigger the timer */
> +	if (ams->masked_alarm)
> +		schedule_delayed_work(&ams->ams_unmask_work,
> +				      msecs_to_jiffies(AMS_UNMASK_TIMEOUT_MS));
> +}
> +
> +static irqreturn_t ams_iio_irq(int irq, void *data)
> +{
> +	struct iio_dev *indio_dev = data;
> +	struct ams *ams = iio_priv(indio_dev);
> +	irqreturn_t irq_status = IRQ_NONE;
> +	u32 isr0;
> +
> +	isr0 = readl(ams->base + AMS_ISR_0);
> +
> +	/* only process alarms that are not masked */
> +	isr0 &= ~((ams->intr_mask & AMS_ISR0_ALARM_MASK) | ams->masked_alarm);
> +
> +	if (isr0) {
Flip the logic and this will cleaner.

if (!isr0)
	return IRQ_NONE;

/* clear interrupt */
...

> +		/* clear interrupt */
> +		writel(isr0, ams->base + AMS_ISR_0);
> +
> +		/* Once the alarm interrupt occurred, mask until get cleared */
> +		ams->masked_alarm |= isr0;
> +		ams_update_intrmask(ams, 0, 0);
> +
> +		ams_handle_events(indio_dev, isr0);
> +
> +		schedule_delayed_work(&ams->ams_unmask_work,
> +				      msecs_to_jiffies(AMS_UNMASK_TIMEOUT_MS));
> +		irq_status = IRQ_HANDLED;
> +	}
> +
> +	return irq_status;
> +}
> +
> +static const struct iio_event_spec ams_temp_events[] = {
> +	{
> +		.type = IIO_EV_TYPE_THRESH,
> +		.dir = IIO_EV_DIR_RISING,
> +		.mask_separate = BIT(IIO_EV_INFO_ENABLE) |
> +				BIT(IIO_EV_INFO_VALUE),
> +	},
> +};
> +
> +static const struct iio_event_spec ams_voltage_events[] = {
> +	{
> +		.type = IIO_EV_TYPE_THRESH,
> +		.dir = IIO_EV_DIR_RISING,
> +		.mask_separate = BIT(IIO_EV_INFO_VALUE),
> +	},
> +	{
> +		.type = IIO_EV_TYPE_THRESH,
> +		.dir = IIO_EV_DIR_FALLING,
> +		.mask_separate = BIT(IIO_EV_INFO_VALUE),
> +	},
> +	{
> +		.type = IIO_EV_TYPE_THRESH,
> +		.dir = IIO_EV_DIR_EITHER,
> +		.mask_separate = BIT(IIO_EV_INFO_ENABLE),
> +	},
> +};
> +
> +static const struct iio_chan_spec ams_ps_channels[] = {
> +	AMS_PS_CHAN_TEMP(AMS_SEQ_TEMP, AMS_TEMP, "ps_temp"),
> +	AMS_PS_CHAN_TEMP(AMS_SEQ_TEMP_REMOTE, AMS_TEMP_REMOTE, "remote_temp"),
> +	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY1, AMS_SUPPLY1, "vccpsintlp"),
> +	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY2, AMS_SUPPLY2, "vccpsintfp"),
> +	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY3, AMS_SUPPLY3, "vccpsaux"),
> +	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY4, AMS_SUPPLY4, "vccpsddr"),
> +	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY5, AMS_SUPPLY5, "vccpsio3"),
> +	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY6, AMS_SUPPLY6, "vccpsio0"),
> +	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY7, AMS_SUPPLY7, "vccpsio1"),
> +	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY8, AMS_SUPPLY8, "vccpsio2"),
> +	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY9, AMS_SUPPLY9, "psmgtravcc"),
> +	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY10, AMS_SUPPLY10, "psmgtravtt"),
> +	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_VCCAMS, AMS_VCCAMS, "vccams"),
> +};
> +
> +static const struct iio_chan_spec ams_pl_channels[] = {
> +	AMS_PL_CHAN_TEMP(AMS_SEQ_TEMP, AMS_TEMP, "pl_temp"),
> +	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY1, AMS_SUPPLY1, "vccint", true),
> +	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY2, AMS_SUPPLY2, "vccaux", true),
> +	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_VREFP, AMS_VREFP, "vccvrefp", false),
> +	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_VREFN, AMS_VREFN, "vccvrefn", false),
> +	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY3, AMS_SUPPLY3, "vccbram", true),
> +	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY4, AMS_SUPPLY4, "vccplintlp", true),
> +	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY5, AMS_SUPPLY5, "vccplintfp", true),
> +	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY6, AMS_SUPPLY6, "vccplaux", true),
> +	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_VCCAMS, AMS_VCCAMS, "vccams", true),
> +	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_VP_VN, AMS_VP_VN, "vccvpvn", false),
> +	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY7, AMS_SUPPLY7, "vuser0", true),
> +	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY8, AMS_SUPPLY8, "vuser1", true),
> +	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY9, AMS_SUPPLY9, "vuser2", true),
> +	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY10, AMS_SUPPLY10, "vuser3", true),
> +	AMS_PL_AUX_CHAN_VOLTAGE(0, NULL),
> +	AMS_PL_AUX_CHAN_VOLTAGE(1, NULL),
> +	AMS_PL_AUX_CHAN_VOLTAGE(2, NULL),
> +	AMS_PL_AUX_CHAN_VOLTAGE(3, NULL),
> +	AMS_PL_AUX_CHAN_VOLTAGE(4, NULL),
> +	AMS_PL_AUX_CHAN_VOLTAGE(5, NULL),
> +	AMS_PL_AUX_CHAN_VOLTAGE(6, NULL),
> +	AMS_PL_AUX_CHAN_VOLTAGE(7, NULL),
> +	AMS_PL_AUX_CHAN_VOLTAGE(8, NULL),
> +	AMS_PL_AUX_CHAN_VOLTAGE(9, NULL),
> +	AMS_PL_AUX_CHAN_VOLTAGE(10, NULL),
> +	AMS_PL_AUX_CHAN_VOLTAGE(11, NULL),
> +	AMS_PL_AUX_CHAN_VOLTAGE(12, NULL),
> +	AMS_PL_AUX_CHAN_VOLTAGE(13, NULL),
> +	AMS_PL_AUX_CHAN_VOLTAGE(14, NULL),
> +	AMS_PL_AUX_CHAN_VOLTAGE(15, NULL),
> +};
> +
> +static const struct iio_chan_spec ams_ctrl_channels[] = {
> +	AMS_CTRL_CHAN_VOLTAGE(AMS_SEQ_VCC_PSPLL, AMS_VCC_PSPLL0, "vcc_pspll0"),
> +	AMS_CTRL_CHAN_VOLTAGE(AMS_SEQ_VCC_PSBATT, AMS_VCC_PSPLL3, "vcc_psbatt"),
> +	AMS_CTRL_CHAN_VOLTAGE(AMS_SEQ_VCCINT, AMS_VCCINT, "vccint"),
> +	AMS_CTRL_CHAN_VOLTAGE(AMS_SEQ_VCCBRAM, AMS_VCCBRAM, "vccbram"),
> +	AMS_CTRL_CHAN_VOLTAGE(AMS_SEQ_VCCAUX, AMS_VCCAUX, "vccaux"),
> +	AMS_CTRL_CHAN_VOLTAGE(AMS_SEQ_PSDDRPLL, AMS_PSDDRPLL, "psddrpll"),
> +	AMS_CTRL_CHAN_VOLTAGE(AMS_SEQ_INTDDR, AMS_PSINTFPDDR, "psintfpddr"),
> +};
> +
> +static int ams_init_module(struct iio_dev *indio_dev, struct device_node *np,
> +			   struct iio_chan_spec *channels)
> +{
> +	struct ams *ams = iio_priv(indio_dev);
> +	struct device_node *chan_node, *child;
> +	int ret, num_channels = 0;
> +	unsigned int reg;
> +
> +	if (of_device_is_compatible(np, "xlnx,zynqmp-ams-ps")) {
> +		ams->ps_base = of_iomap(np, 0);
> +		if (!ams->ps_base)
> +			return -ENXIO;
> +
> +		/* add PS channels to iio device channels */
> +		memcpy(channels + num_channels, ams_ps_channels,
> +		       sizeof(ams_ps_channels));
> +		num_channels += ARRAY_SIZE(ams_ps_channels);
> +	} else if (of_device_is_compatible(np, "xlnx,zynqmp-ams-pl")) {
> +		ams->pl_base = of_iomap(np, 0);
> +		if (!ams->pl_base)
> +			return -ENXIO;
> +
> +		/* Copy only first 10 fix channels */
> +		memcpy(channels + num_channels, ams_pl_channels,
> +		       AMS_PL_MAX_FIXED_CHANNEL * sizeof(*channels));
> +		num_channels += AMS_PL_MAX_FIXED_CHANNEL;
> +
> +		chan_node = of_get_child_by_name(np, "xlnx,ext-channels");
> +		if (chan_node) {

There is some very deep nesting in here. I haven't thought about it in detail
but perhaps you can break some of this out into a a different function then
call that here?


> +			for_each_child_of_node(chan_node, child) {
> +				ret = of_property_read_u32(child, "reg", &reg);
> +				if (ret || reg > AMS_PL_MAX_EXT_CHANNEL)
> +					continue;
> +
> +				memcpy(&channels[num_channels],
> +				       &ams_pl_channels[reg +
> +				       AMS_PL_MAX_FIXED_CHANNEL],
> +				       sizeof(*channels));
> +
> +				if (of_property_read_bool(child,
> +							  "xlnx,bipolar"))
> +					channels[num_channels].scan_type.sign =
> +						's';
> +
> +				num_channels++;
> +			}
> +		}
> +		of_node_put(chan_node);
> +	} else if (of_device_is_compatible(np, "xlnx,zynqmp-ams")) {
> +		/* add AMS channels to iio device channels */
> +		memcpy(channels + num_channels, ams_ctrl_channels,
> +				sizeof(ams_ctrl_channels));
> +		num_channels += ARRAY_SIZE(ams_ctrl_channels);
> +	} else {
> +		return -EINVAL;
> +	}
> +
> +	return num_channels;
> +}
> +
> +static int ams_parse_dt(struct iio_dev *indio_dev, struct platform_device *pdev)
> +{
> +	struct ams *ams = iio_priv(indio_dev);
> +	struct iio_chan_spec *ams_channels, *dev_channels;
> +	struct device_node *child_node = NULL, *np = pdev->dev.of_node;
> +	int ret, vol_ch_cnt = 0, temp_ch_cnt = 0, i, rising_off, falling_off;
> +	unsigned int num_channels = 0;
> +
> +	/* Initialize buffer for channel specification */
> +	ams_channels = kzalloc(sizeof(ams_ps_channels) +
> +			       sizeof(ams_pl_channels) +
> +			       sizeof(ams_ctrl_channels), GFP_KERNEL);
> +	if (!ams_channels)
> +		return -ENOMEM;
> +
> +	if (of_device_is_available(np)) {
> +		ret = ams_init_module(indio_dev, np, ams_channels);
> +		if (ret < 0)
> +			goto err;
> +
> +		num_channels += ret;
> +	}
> +
> +	for_each_child_of_node(np, child_node) {
> +		if (of_device_is_available(child_node)) {
> +			ret = ams_init_module(indio_dev, child_node,
> +					      ams_channels + num_channels);
> +			if (ret < 0)
> +				goto err;
> +
> +			num_channels += ret;
> +		}
> +	}
> +
> +	for (i = 0; i < num_channels; i++) {
> +		if (ams_channels[i].type == IIO_VOLTAGE)
> +			ams_channels[i].channel = vol_ch_cnt++;
> +		else
> +			ams_channels[i].channel = temp_ch_cnt++;
> +
> +		if (ams_channels[i].scan_index < (PS_SEQ_MAX * 3)) {
> +			/* set threshold to max and min for each channel */
> +			falling_off = ams_get_alarm_offset(
> +					ams_channels[i].scan_index,
> +					IIO_EV_DIR_FALLING);
> +			rising_off = ams_get_alarm_offset(
> +					ams_channels[i].scan_index,
> +					IIO_EV_DIR_RISING);
> +			if (ams_channels[i].scan_index >= PS_SEQ_MAX) {
> +				writel(AMS_ALARM_THR_MIN,
> +						ams->pl_base + falling_off);
> +				writel(AMS_ALARM_THR_MAX,
> +						ams->pl_base + rising_off);
> +			} else {
> +				writel(AMS_ALARM_THR_MIN,
> +						ams->ps_base + falling_off);
> +				writel(AMS_ALARM_THR_MAX,
> +						ams->ps_base + rising_off);
> +			}
> +		}
> +	}
> +
> +	dev_channels = devm_kzalloc(&pdev->dev, sizeof(*dev_channels) *
> +				    num_channels, GFP_KERNEL);
> +	if (!dev_channels) {
> +		ret = -ENOMEM;
> +		goto err;
> +	}
> +
> +	memcpy(dev_channels, ams_channels,
> +	       sizeof(*ams_channels) * num_channels);
> +	indio_dev->channels = dev_channels;
> +	indio_dev->num_channels = num_channels;
> +
> +	ret = 0;
> +err:
> +	kfree(ams_channels);
> +
> +	return ret;
> +}
> +
> +static const struct iio_info iio_pl_info = {
> +	.read_raw = &ams_read_raw,
> +	.read_event_config = &ams_read_event_config,
> +	.write_event_config = &ams_write_event_config,
> +	.read_event_value = &ams_read_event_value,
> +	.write_event_value = &ams_write_event_value,
> +};
> +
> +static const struct of_device_id ams_of_match_table[] = {
> +	{ .compatible = "xlnx,zynqmp-ams" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, ams_of_match_table);
> +
> +static int ams_probe(struct platform_device *pdev)
> +{
> +	struct iio_dev *indio_dev;
> +	struct ams *ams;
> +	struct resource *res;
> +	int ret;
> +
> +	if (!pdev->dev.of_node)
> +		return -ENODEV;
> +
> +	indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*ams));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	ams = iio_priv(indio_dev);
> +	mutex_init(&ams->lock);
> +
> +	indio_dev->dev.parent = &pdev->dev;
> +	indio_dev->dev.of_node = pdev->dev.of_node;
> +	indio_dev->name = "xilinx-ams";
> +
> +	indio_dev->info = &iio_pl_info;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +
> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "ams-base");
> +	ams->base = devm_ioremap_resource(&pdev->dev, res);
> +	if (IS_ERR(ams->base))
> +		return PTR_ERR(ams->base);
> +
> +	ams->clk = devm_clk_get(&pdev->dev, NULL);
> +	if (IS_ERR(ams->clk))
> +		return PTR_ERR(ams->clk);
> +	clk_prepare_enable(ams->clk);
> +
> +	INIT_DELAYED_WORK(&ams->ams_unmask_work, ams_unmask_worker);
> +
> +	ret = iio_ams_init_device(ams);
> +	if (ret) {
> +		dev_err(&pdev->dev, "failed to initialize AMS\n");
> +		goto err_probe;
> +	}
> +
> +	ret = ams_parse_dt(indio_dev, pdev);
> +	if (ret) {
> +		dev_err(&pdev->dev, "failure in parsing DT\n");
> +		goto err_probe;
> +	}
> +
> +	ams_enable_channel_sequence(ams);
> +
> +	ams->irq = platform_get_irq_byname(pdev, "ams-irq");
> +	ret = request_irq(ams->irq, &ams_iio_irq, 0, "ams-irq", indio_dev);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "failed to register interrupt\n");
> +		goto err_probe;
> +	}
> +
> +	platform_set_drvdata(pdev, indio_dev);
> +
> +	ret = iio_device_register(indio_dev);
> +	if (ret)
> +		goto err_irq_free;
> +
> +	return 0;
> +
> +err_irq_free:
> +	free_irq(ams->irq, indio_dev);
> +
> +err_probe:
> +	cancel_delayed_work(&ams->ams_unmask_work);
> +	clk_disable_unprepare(ams->clk);
> +
> +	return ret;
> +}
> +
> +static int ams_remove(struct platform_device *pdev)
> +{
> +	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> +	struct ams *ams = iio_priv(indio_dev);
> +
> +	cancel_delayed_work(&ams->ams_unmask_work);
> +
> +	free_irq(ams->irq, indio_dev);
> +	/* Unregister the device */

I don't think this comment adds any information given the name
of the function.

> +	iio_device_unregister(indio_dev);

This ordering worries me a bit.  You disable the irq before
disabling the userspace and in kernel interfaces.  Immediately
makes me think there is a potential race here...

Also, the ordering here is not the reverse of that in probe which
often indicates a problem and also makes it harder for a reviewer
to be sure it is correct.


> +	clk_disable_unprepare(ams->clk);
> +
> +	return 0;
> +}
> +
> +static int __maybe_unused ams_suspend(struct device *dev)
> +{
> +	struct ams *ams = iio_priv(dev_get_drvdata(dev));
> +
> +	clk_disable_unprepare(ams->clk);
> +
> +	return 0;
> +}
> +
> +static int __maybe_unused ams_resume(struct device *dev)
> +{
> +	struct ams *ams = iio_priv(dev_get_drvdata(dev));
> +
> +	clk_prepare_enable(ams->clk);
> +
> +	return 0;
> +}
> +
> +static SIMPLE_DEV_PM_OPS(ams_pm_ops, ams_suspend, ams_resume);
> +
> +static struct platform_driver ams_driver = {
> +	.probe = ams_probe,
> +	.remove = ams_remove,
> +	.driver = {
> +		.name = "xilinx-ams",
> +		.pm = &ams_pm_ops,
> +		.of_match_table = ams_of_match_table,
> +	},
> +};
> +module_platform_driver(ams_driver);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Xilinx, Inc.");


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 2/4] iio: Documentation: Add Xilinx AMS sysfs documentation
  2018-09-16 10:12   ` Jonathan Cameron
@ 2018-09-17  9:56     ` Michal Simek
  2018-09-17 10:06       ` Jonathan Cameron
  0 siblings, 1 reply; 12+ messages in thread
From: Michal Simek @ 2018-09-17  9:56 UTC (permalink / raw)
  To: Jonathan Cameron, Manish Narani
  Cc: knaack.h, lars, pmeerw, michal.simek, robh+dt, mark.rutland,
	sudeep.holla, amit.kucheria, leoyang.li, broonie,
	arnaud.pouliquen, eugen.hristev, rdunlap, geert, ak, freeman.liu,
	lukas, vilhelm.gray, gregkh, kstewart, sgoud, anirudh, linux-iio,
	linux-arm-kernel, linux-kernel, devicetree

On 16.9.2018 12:12, Jonathan Cameron wrote:
> On Fri, 14 Sep 2018 12:48:28 +0530
> Manish Narani <manish.narani@xilinx.com> wrote:
> 
>> Add documentation for xilinx-ams driver. This contains information about
>> various voltages and temperatures on PS (Processing System), PL
>> (Programmable Logic) and AMS Control Block.
>>
>> Signed-off-by: Manish Narani <manish.narani@xilinx.com>
> The more I look at this device the more I'm convinced it is very much a dedicated
> hardware monitoring function, not a generic ADC sensing unit at all.
> 
> Hmm.  It is still fine to have it in IIO but you need to think in detail
> about how you are going to interface this to hwmon via the iio-hwmon bridge.
> 
> Some of the interface complexity should only really be apparent when we hit
> hwmon perhaps rather than having so many different custom interfaces in IIO.
> 
> Please also loop in the maintainers and lists for hwmon in the next
> version so we can get their input.

Isn't there iio-hwmon driver for this?

Thanks,
Michal

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 2/4] iio: Documentation: Add Xilinx AMS sysfs documentation
  2018-09-17  9:56     ` Michal Simek
@ 2018-09-17 10:06       ` Jonathan Cameron
  2018-09-17 10:23         ` Michal Simek
  0 siblings, 1 reply; 12+ messages in thread
From: Jonathan Cameron @ 2018-09-17 10:06 UTC (permalink / raw)
  To: Michal Simek
  Cc: Jonathan Cameron, Manish Narani, knaack.h, lars, pmeerw, robh+dt,
	mark.rutland, sudeep.holla, amit.kucheria, leoyang.li, broonie,
	arnaud.pouliquen, eugen.hristev, rdunlap, geert, ak, freeman.liu,
	lukas, vilhelm.gray, gregkh, kstewart, sgoud, anirudh, linux-iio,
	linux-arm-kernel, linux-kernel, devicetree

On Mon, 17 Sep 2018 11:56:08 +0200
Michal Simek <michal.simek@xilinx.com> wrote:

> On 16.9.2018 12:12, Jonathan Cameron wrote:
> > On Fri, 14 Sep 2018 12:48:28 +0530
> > Manish Narani <manish.narani@xilinx.com> wrote:
> >   
> >> Add documentation for xilinx-ams driver. This contains information about
> >> various voltages and temperatures on PS (Processing System), PL
> >> (Programmable Logic) and AMS Control Block.
> >>
> >> Signed-off-by: Manish Narani <manish.narani@xilinx.com>  
> > The more I look at this device the more I'm convinced it is very much a dedicated
> > hardware monitoring function, not a generic ADC sensing unit at all.
> > 
> > Hmm.  It is still fine to have it in IIO but you need to think in detail
> > about how you are going to interface this to hwmon via the iio-hwmon bridge.
> > 
> > Some of the interface complexity should only really be apparent when we hit
> > hwmon perhaps rather than having so many different custom interfaces in IIO.
> > 
> > Please also loop in the maintainers and lists for hwmon in the next
> > version so we can get their input.  
> 
> Isn't there iio-hwmon driver for this?
> 
> Thanks,
> Michal

Absolutely.  The interesting bit is that if we are planning to actually expose
the many monitoring channels via iio-hwmon IIRC it won't use the extended names
at all (I may have missremembered this thogh).  As such we may want to reduced
the amount of custom ABI in IIO in favour of a level of opaqueness with the
'real' interface provided by the iio-hwmon bridge driver.  A quick glance
suggested we may need to increase the information exposed by the iio-hwmon
driver to make this work.

When we have run into cases like this (a very much hardware monitoring oriented
device with a few general purpose channels) in the past we have always gotten
agreement from the hwmon maintainers that they are happy with using an IIO provider
and the iio-hwmon driver route. It is just nice to keep everyone in agreement
and have no surprises!

Jonathan


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 2/4] iio: Documentation: Add Xilinx AMS sysfs documentation
  2018-09-17 10:06       ` Jonathan Cameron
@ 2018-09-17 10:23         ` Michal Simek
  0 siblings, 0 replies; 12+ messages in thread
From: Michal Simek @ 2018-09-17 10:23 UTC (permalink / raw)
  To: Jonathan Cameron, Michal Simek
  Cc: Jonathan Cameron, Manish Narani, knaack.h, lars, pmeerw, robh+dt,
	mark.rutland, sudeep.holla, amit.kucheria, leoyang.li, broonie,
	arnaud.pouliquen, eugen.hristev, rdunlap, geert, ak, freeman.liu,
	lukas, vilhelm.gray, gregkh, kstewart, sgoud, anirudh, linux-iio,
	linux-arm-kernel, linux-kernel, devicetree

On 17.9.2018 12:06, Jonathan Cameron wrote:
> On Mon, 17 Sep 2018 11:56:08 +0200
> Michal Simek <michal.simek@xilinx.com> wrote:
> 
>> On 16.9.2018 12:12, Jonathan Cameron wrote:
>>> On Fri, 14 Sep 2018 12:48:28 +0530
>>> Manish Narani <manish.narani@xilinx.com> wrote:
>>>   
>>>> Add documentation for xilinx-ams driver. This contains information about
>>>> various voltages and temperatures on PS (Processing System), PL
>>>> (Programmable Logic) and AMS Control Block.
>>>>
>>>> Signed-off-by: Manish Narani <manish.narani@xilinx.com>  
>>> The more I look at this device the more I'm convinced it is very much a dedicated
>>> hardware monitoring function, not a generic ADC sensing unit at all.
>>>
>>> Hmm.  It is still fine to have it in IIO but you need to think in detail
>>> about how you are going to interface this to hwmon via the iio-hwmon bridge.
>>>
>>> Some of the interface complexity should only really be apparent when we hit
>>> hwmon perhaps rather than having so many different custom interfaces in IIO.
>>>
>>> Please also loop in the maintainers and lists for hwmon in the next
>>> version so we can get their input.  
>>
>> Isn't there iio-hwmon driver for this?
>>
>> Thanks,
>> Michal
> 
> Absolutely.  The interesting bit is that if we are planning to actually expose
> the many monitoring channels via iio-hwmon IIRC it won't use the extended names
> at all (I may have missremembered this thogh).  As such we may want to reduced
> the amount of custom ABI in IIO in favour of a level of opaqueness with the
> 'real' interface provided by the iio-hwmon bridge driver.  A quick glance
> suggested we may need to increase the information exposed by the iio-hwmon
> driver to make this work.

ok.


> When we have run into cases like this (a very much hardware monitoring oriented
> device with a few general purpose channels) in the past we have always gotten
> agreement from the hwmon maintainers that they are happy with using an IIO provider
> and the iio-hwmon driver route. It is just nice to keep everyone in agreement
> and have no surprises!

Definitely I agree with this.

Thanks,
Michal



^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2018-09-17 10:26 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-14  7:18 [PATCH v2 0/4] Add Xilinx AMS Driver Manish Narani
2018-09-14  7:18 ` [PATCH v2 1/4] dt-bindings: iio: adc: Add Xilinx AMS binding documentation Manish Narani
2018-09-14  7:18 ` [PATCH v2 2/4] iio: Documentation: Add Xilinx AMS sysfs documentation Manish Narani
2018-09-16 10:12   ` Jonathan Cameron
2018-09-17  9:56     ` Michal Simek
2018-09-17 10:06       ` Jonathan Cameron
2018-09-17 10:23         ` Michal Simek
2018-09-14  7:18 ` [PATCH v2 3/4] iio: adc: Add Xilinx AMS driver Manish Narani
2018-09-14 11:59   ` Himanshu Jha
2018-09-14 21:30   ` Randy Dunlap
2018-09-16 10:34   ` Jonathan Cameron
2018-09-14  7:18 ` [PATCH v2 4/4] arm64: zynqmp: DT: Add Xilinx AMS node Manish Narani

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).