linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v11 0/5] Add Xilinx AMS Driver
@ 2021-11-24 22:54 Anand Ashok Dumbre
  2021-11-24 22:54 ` [PATCH v11 1/5] device property: Add fwnode_iomap() Anand Ashok Dumbre
                   ` (4 more replies)
  0 siblings, 5 replies; 19+ messages in thread
From: Anand Ashok Dumbre @ 2021-11-24 22:54 UTC (permalink / raw)
  To: linux-kernel, jic23, lars, linux-iio, git, michal.simek, gregkh,
	rafael, linux-acpi, andriy.shevchenko, heikki.krogerus
  Cc: Anand Ashok Dumbre

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 will be used by iio-hwmon to repport voltages and temperatures
across the system by using various channel interfaces.
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

Changes in v3:
	- Updated bindings document with the suggested modification in v2 review
	- Removed documentation for sysfs
	- Removed extended names for channels in the Xilinx AMS driver
	- Modified dts to use ranges for child nodes
	- Reduced address and size cells to 32-bit instead of 64-bit

Changes in v4:
	- Updated bindings document with the suggested modification in v3 review
	- Changed the Device Tree property 'ranges' for child nodes
	- Used Channel Numbers as 'reg' value in DT to avoid confusion
	- Removed unused NULL arguments as suggested in v3 patch review
	- Addressed comments on Device Tree property naming

Changes in v5:
	- Updated bindings document to the YAML format
	- Updated bindings document with the suggested modification in v4 review
	- Renamed iio_pl_info struct to iio_ams_info in Xilinx AMS driver
	- Updated the Xilinx AMS driver to not use iio_priv_to_dev function
	- Updated Xilinx AMS node to reflect the changes in bindings document
	- Update MAINTAINERS file

Changes in v6:
	- Removed all tabs from bindings document.
	- Removed the xlnx,ext-channels node from the device tree since
	  it is not neeeded.
	- Fixed unit addresses for ps-ams and pl-ams.
	- Removed the names property from bindings.
	- Fixed warnings from checkpatch.pl in the driver.
	- devm_add_action_or_reset() used for exit/error path.
	- devm_request_irq() for managed irq request instead of
	  request_irq()

Changes in v7:
	- Added use of FIELD_PREP and FIELD_GET.
	- Added the spinlocks back the v1 which were removed in v2 for
	  no justifiable reason and replaced with the same mutex. This
	  caused deadlocks.
	- Removed the buffered mode information from channel config.
	- Usage of wrapper functions for devm_add_action_or_reset
	  callbacks to avoid typecasting functions.
	- Usage of devm_platform_iremap_resource().
	- Handled platform_get_irq() return values.
	- Removed the remove() callback.
	- Fixed the dt-bindings.

Changes in v8:
	- Replaced *_of_() APIs with fwnode.
	- Added missing headers.
	- Fixed documentation.
	- Added devm_add_action_or_reset() for iounmap.
	- Restructured read_raw function.
	- Added helper functions.
	- Usage of GENMASK for all masks.
	- Added defaults for most switch cases. Some can't be added
	  since the default will never occur.

Changes in v9:
	- Added a fwnode_iomap().
	- Fixed Kconfig indentation.
	- Added the overflow checks before memory allocation.
	- Usage of fwnode_iomap() instead of iomap().
	- Rename ams_parse_dt() to ams_parse_firmware().

Changes in v10:
	- Fixed licence in zynqmp.dtsi.
	- Changed the macros to use BIT().
	- Realign some code to fit within 100 chars.
	- Modified readl_poll_timeout usage.
	- Usage of array_size() instead of check_mul_overflow().
	- Usage of dev_err_probe() instead of dev_err().
	- Usage of kcalloc instead of kzalloc()

Changes in v11:
	- Added missing bitfield.h.
	- Fixed AMS_ALARM_THR_MIN macro.
	- Added terminators to enums where necessary.
	- Added explicit values as suggested to enums.
	- Removed redundant macros.
	- Added delay value for readl_poll_timeout.
	- Corrected shadowed error return.
	- Corrected formatting errors.
	- Added default cases where missing.
	- Made ams_parse_firmware() a single parameter functions.
	- Usage of devm_kcalloc() and devm_krealloc().
	- Directly returning from dev_err_probe().
	- Renamed masked_alarm to current_masked_alarm.


Anand Ashok Dumbre (5):
  device property: Add fwnode_iomap()
  arm64: zynqmp: DT: Add Xilinx AMS node
  iio: adc: Add Xilinx AMS driver
  dt-bindings: iio: adc: Add Xilinx AMS binding documentation
  MAINTAINERS: Add maintainer for xilinx-ams

 .../bindings/iio/adc/xlnx,zynqmp-ams.yaml     |  227 +++
 MAINTAINERS                                   |    7 +
 arch/arm64/boot/dts/xilinx/zynqmp.dtsi        |   24 +
 drivers/base/property.c                       |   16 +
 drivers/iio/adc/Kconfig                       |   15 +
 drivers/iio/adc/Makefile                      |    1 +
 drivers/iio/adc/xilinx-ams.c                  | 1425 +++++++++++++++++
 include/linux/property.h                      |    2 +
 8 files changed, 1717 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/xlnx,zynqmp-ams.yaml
 create mode 100644 drivers/iio/adc/xilinx-ams.c

-- 
2.17.1


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

* [PATCH v11 1/5] device property: Add fwnode_iomap()
  2021-11-24 22:54 [PATCH v11 0/5] Add Xilinx AMS Driver Anand Ashok Dumbre
@ 2021-11-24 22:54 ` Anand Ashok Dumbre
  2021-11-25 11:42   ` Andy Shevchenko
  2021-11-27 11:54   ` kernel test robot
  2021-11-24 22:54 ` [PATCH v11 2/5] arm64: zynqmp: DT: Add Xilinx AMS node Anand Ashok Dumbre
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 19+ messages in thread
From: Anand Ashok Dumbre @ 2021-11-24 22:54 UTC (permalink / raw)
  To: linux-kernel, jic23, lars, linux-iio, git, michal.simek, gregkh,
	rafael, linux-acpi, andriy.shevchenko, heikki.krogerus
  Cc: Anand Ashok Dumbre

This patch introduces a new helper routine - fwnode_iomap(), which
allows to map the memory mapped IO for a given device node.

This implementation does not cover the ACPI case and may be expanded
in the future. The main purpose here is to be able to develop resource
provider agnostic drivers.

Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Anand Ashok Dumbre <anand.ashok.dumbre@xilinx.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/base/property.c  | 16 ++++++++++++++++
 include/linux/property.h |  2 ++
 2 files changed, 18 insertions(+)

diff --git a/drivers/base/property.c b/drivers/base/property.c
index f1f35b48ab8b..f2e38be3a999 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -958,6 +958,22 @@ int fwnode_irq_get(const struct fwnode_handle *fwnode, unsigned int index)
 }
 EXPORT_SYMBOL(fwnode_irq_get);
 
+/**
+ * fwnode_iomap - Maps the memory mapped IO for a given fwnode
+ * @fwnode:	Pointer to the firmware node
+ * @index:	Index of the IO range
+ *
+ * Returns a pointer to the mapped memory.
+ */
+void __iomem *fwnode_iomap(struct fwnode_handle *fwnode, int index)
+{
+	if (is_of_node(fwnode))
+		return of_iomap(to_of_node(fwnode), index);
+
+	return NULL;
+}
+EXPORT_SYMBOL(fwnode_iomap);
+
 /**
  * fwnode_graph_get_next_endpoint - Get next endpoint firmware node
  * @fwnode: Pointer to the parent firmware node
diff --git a/include/linux/property.h b/include/linux/property.h
index 88fa726a76df..6670d5a1ec2a 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -122,6 +122,8 @@ void fwnode_handle_put(struct fwnode_handle *fwnode);
 
 int fwnode_irq_get(const struct fwnode_handle *fwnode, unsigned int index);
 
+void __iomem *fwnode_iomap(struct fwnode_handle *fwnode, int index);
+
 unsigned int device_get_child_node_count(struct device *dev);
 
 static inline bool device_property_read_bool(struct device *dev,
-- 
2.17.1


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

* [PATCH v11 2/5] arm64: zynqmp: DT: Add Xilinx AMS node
  2021-11-24 22:54 [PATCH v11 0/5] Add Xilinx AMS Driver Anand Ashok Dumbre
  2021-11-24 22:54 ` [PATCH v11 1/5] device property: Add fwnode_iomap() Anand Ashok Dumbre
@ 2021-11-24 22:54 ` Anand Ashok Dumbre
  2021-11-24 22:54 ` [PATCH v11 3/5] iio: adc: Add Xilinx AMS driver Anand Ashok Dumbre
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 19+ messages in thread
From: Anand Ashok Dumbre @ 2021-11-24 22:54 UTC (permalink / raw)
  To: linux-kernel, jic23, lars, linux-iio, git, michal.simek, gregkh,
	rafael, linux-acpi, andriy.shevchenko, heikki.krogerus
  Cc: Anand Ashok Dumbre, Manish Narani

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.

Co-developed-by: Manish Narani <manish.narani@xilinx.com>
Signed-off-by: Manish Narani <manish.narani@xilinx.com>
Signed-off-by: Anand Ashok Dumbre <anand.ashok.dumbre@xilinx.com>
---
 arch/arm64/boot/dts/xilinx/zynqmp.dtsi | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
index 74e66443e4ce..ab96708fe65e 100644
--- a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
+++ b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
@@ -878,6 +878,30 @@
 			timeout-sec = <10>;
 		};
 
+		xilinx_ams: ams@ffa50000 {
+			compatible = "xlnx,zynqmp-ams";
+			status = "disabled";
+			interrupt-parent = <&gic>;
+			interrupts = <0 56 4>;
+			reg = <0x0 0xffa50000 0x0 0x800>;
+			#address-cells = <1>;
+			#size-cells = <1>;
+			#io-channel-cells = <1>;
+			ranges = <0 0 0xffa50800 0x800>;
+
+			ams_ps: ams-ps@0 {
+				compatible = "xlnx,zynqmp-ams-ps";
+				status = "disabled";
+				reg = <0 0x400>;
+			};
+
+			ams_pl: ams-pl@400 {
+				compatible = "xlnx,zynqmp-ams-pl";
+				status = "disabled";
+				reg = <0x400 0x400>;
+			};
+		};
+
 		zynqmp_dpdma: dma-controller@fd4c0000 {
 			compatible = "xlnx,zynqmp-dpdma";
 			status = "disabled";
-- 
2.17.1


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

* [PATCH v11 3/5] iio: adc: Add Xilinx AMS driver
  2021-11-24 22:54 [PATCH v11 0/5] Add Xilinx AMS Driver Anand Ashok Dumbre
  2021-11-24 22:54 ` [PATCH v11 1/5] device property: Add fwnode_iomap() Anand Ashok Dumbre
  2021-11-24 22:54 ` [PATCH v11 2/5] arm64: zynqmp: DT: Add Xilinx AMS node Anand Ashok Dumbre
@ 2021-11-24 22:54 ` Anand Ashok Dumbre
  2021-11-25 12:14   ` Andy Shevchenko
                     ` (2 more replies)
  2021-11-24 22:54 ` [PATCH v11 4/5] dt-bindings: iio: adc: Add Xilinx AMS binding documentation Anand Ashok Dumbre
  2021-11-24 22:54 ` [PATCH v11 5/5] MAINTAINERS: Add maintainer for xilinx-ams Anand Ashok Dumbre
  4 siblings, 3 replies; 19+ messages in thread
From: Anand Ashok Dumbre @ 2021-11-24 22:54 UTC (permalink / raw)
  To: linux-kernel, jic23, lars, linux-iio, git, michal.simek, gregkh,
	rafael, linux-acpi, andriy.shevchenko, heikki.krogerus
  Cc: Anand Ashok Dumbre, Manish Narani

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 an external master. Out of these interfaces 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.

Co-developed-by: Manish Narani <manish.narani@xilinx.com>
Signed-off-by: Manish Narani <manish.narani@xilinx.com>
Signed-off-by: Anand Ashok Dumbre <anand.ashok.dumbre@xilinx.com>
---
 drivers/iio/adc/Kconfig      |   15 +
 drivers/iio/adc/Makefile     |    1 +
 drivers/iio/adc/xilinx-ams.c | 1425 ++++++++++++++++++++++++++++++++++
 3 files changed, 1441 insertions(+)
 create mode 100644 drivers/iio/adc/xilinx-ams.c

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 8bf5b62a73f4..f6efe11ea1a8 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -1288,4 +1288,19 @@ 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 for Ultrascale/Ultrascale+
+	  System Monitor. With this you can measure and monitor the Voltages and
+	  Temperature values on the SOC.
+
+	  The driver supports Voltage and Temperature monitoring on Xilinx Ultrascale
+	  devices.
+
+	  The driver can also be built 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 d3f53549720c..4a8f1833993b 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -115,4 +115,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 000000000000..568e60d547ba
--- /dev/null
+++ b/drivers/iio/adc/xilinx-ams.c
@@ -0,0 +1,1425 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Xilinx AMS driver
+ *
+ *  Copyright (C) 2021 Xilinx, Inc.
+ *
+ *  Manish Narani <mnarani@xilinx.com>
+ *  Rajnikant Bhojani <rajnikant.bhojani@xilinx.com>
+ */
+
+#include <linux/bits.h>
+#include <linux/bitfield.h>
+#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/mod_devicetable.h>
+#include <linux/overflow.h>
+#include <linux/platform_device.h>
+#include <linux/property.h>
+#include <linux/slab.h>
+
+#include <linux/iio/events.h>
+#include <linux/iio/iio.h>
+
+/* 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_CONFIG4			0x110
+#define AMS_REG_SEQ_CH0			0x120
+#define AMS_REG_SEQ_CH1			0x124
+#define AMS_REG_SEQ_CH2			0x118
+
+#define AMS_VUSER0_MASK			BIT(0)
+#define AMS_VUSER1_MASK			BIT(1)
+#define AMS_VUSER2_MASK			BIT(2)
+#define AMS_VUSER3_MASK			BIT(3)
+
+#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		0xFFFF
+#define AMS_PL_RESET_VALUE		0xFFFF
+
+#define AMS_CONF0_CHANNEL_NUM_MASK	GENMASK(6, 0)
+
+#define AMS_CONF1_SEQ_MASK		GENMASK(15, 12)
+#define AMS_CONF1_SEQ_DEFAULT		FIELD_PREP(AMS_CONF1_SEQ_MASK, 0)
+#define AMS_CONF1_SEQ_CONTINUOUS	FIELD_PREP(AMS_CONF1_SEQ_MASK, 1)
+#define AMS_CONF1_SEQ_SINGLE_CHANNEL	FIELD_PREP(AMS_CONF1_SEQ_MASK, 2)
+
+#define AMS_REG_SEQ0_MASK		GENMASK(15, 0)
+#define AMS_REG_SEQ2_MASK		GENMASK(21, 16)
+#define AMS_REG_SEQ1_MASK		GENMASK(37, 22)
+
+#define AMS_PS_SEQ_MASK			GENMASK(21, 0)
+#define AMS_PL_SEQ_MASK			GENMASK(59, 22)
+
+#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	BIT(1)
+#define AMS_ALARM_THR_MIN		0x0000
+#define AMS_ALARM_THR_MAX		(BIT(16) - 1)
+
+#define AMS_ALARM_MASK			GENMASK(63, 0)
+#define AMS_NO_OF_ALARMS		32
+#define AMS_PL_ALARM_START		16
+#define AMS_PL_ALARM_MASK		GENMASK(31, 16)
+#define AMS_ISR0_ALARM_MASK		GENMASK(31, 0)
+#define AMS_ISR1_ALARM_MASK		(GENMASK(31, 29) | GENMASK(4, 0))
+#define AMS_ISR1_EOC_MASK		BIT(3)
+#define AMS_ISR1_INTR_MASK		GENMASK(63, 32)
+#define AMS_ISR0_ALARM_2_TO_0_MASK	GENMASK(2, 0)
+#define AMS_ISR0_ALARM_6_TO_3_MASK	GENMASK(6, 3)
+#define AMS_ISR0_ALARM_12_TO_7_MASK	GENMASK(13, 8)
+#define AMS_CONF1_ALARM_2_TO_0_MASK	GENMASK(3, 1)
+#define AMS_CONF1_ALARM_6_TO_3_MASK	GENMASK(11, 8)
+#define AMS_CONF1_ALARM_12_TO_7_MASK	GENMASK(5, 0)
+#define AMS_REGCFG1_ALARM_MASK  \
+	(AMS_CONF1_ALARM_2_TO_0_MASK | AMS_CONF1_ALARM_6_TO_3_MASK | BIT(0))
+#define AMS_REGCFG3_ALARM_MASK		AMS_CONF1_ALARM_12_TO_7_MASK
+
+#define AMS_PS_CSTS_PS_READY		(BIT(27) | BIT(16))
+#define AMS_PL_CSTS_ACCESS_MASK		BIT(1)
+
+#define AMS_PL_MAX_FIXED_CHANNEL	10
+#define AMS_PL_MAX_EXT_CHANNEL		20
+
+#define AMS_INIT_POLL_TIME		200
+#define AMS_INIT_TIMEOUT_US		10000
+#define AMS_UNMASK_TIMEOUT_MS		500
+
+/*
+ * 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			-((280230LL << 16) / 509314)
+
+enum ams_alarm_bit {
+	AMS_ALARM_BIT_TEMP = 0,
+	AMS_ALARM_BIT_SUPPLY1 = 1,
+	AMS_ALARM_BIT_SUPPLY2 = 2,
+	AMS_ALARM_BIT_SUPPLY3 = 3,
+	AMS_ALARM_BIT_SUPPLY4 = 4,
+	AMS_ALARM_BIT_SUPPLY5 = 5,
+	AMS_ALARM_BIT_SUPPLY6 = 6,
+	AMS_ALARM_BIT_RESERVED = 7,
+	AMS_ALARM_BIT_SUPPLY7 = 8,
+	AMS_ALARM_BIT_SUPPLY8 = 9,
+	AMS_ALARM_BIT_SUPPLY9 = 10,
+	AMS_ALARM_BIT_SUPPLY10 = 11,
+	AMS_ALARM_BIT_VCCAMS = 12,
+	AMS_ALARM_BIT_TEMP_REMOTE = 13,
+};
+
+enum ams_seq {
+	AMS_SEQ_VCC_PSPLL = 0,
+	AMS_SEQ_VCC_PSBATT = 1,
+	AMS_SEQ_VCCINT = 2,
+	AMS_SEQ_VCCBRAM = 3,
+	AMS_SEQ_VCCAUX = 4,
+	AMS_SEQ_PSDDRPLL = 5,
+	AMS_SEQ_INTDDR = 6,
+};
+
+enum ams_ps_pl_seq {
+	AMS_SEQ_CALIB = 0,
+	AMS_SEQ_RSVD_1 = 1,
+	AMS_SEQ_RSVD_2 = 2,
+	AMS_SEQ_TEST = 3,
+	AMS_SEQ_RSVD_4 = 4,
+	AMS_SEQ_SUPPLY4 = 5,
+	AMS_SEQ_SUPPLY5 = 6,
+	AMS_SEQ_SUPPLY6 = 7,
+	AMS_SEQ_TEMP = 8,
+	AMS_SEQ_SUPPLY2 = 9,
+	AMS_SEQ_SUPPLY1 = 10,
+	AMS_SEQ_VP_VN = 11,
+	AMS_SEQ_VREFP = 12,
+	AMS_SEQ_VREFN = 13,
+	AMS_SEQ_SUPPLY3 = 14,
+	AMS_SEQ_CURRENT_MON = 15,
+	AMS_SEQ_SUPPLY7 = 16,
+	AMS_SEQ_SUPPLY8 = 17,
+	AMS_SEQ_SUPPLY9 = 18,
+	AMS_SEQ_SUPPLY10 = 19,
+	AMS_SEQ_VCCAMS = 20,
+	AMS_SEQ_TEMP_REMOTE = 21,
+	AMS_SEQ_MAX = 22
+};
+
+#define AMS_PS_SEQ_MAX		AMS_SEQ_MAX
+#define AMS_SEQ(x)		(AMS_SEQ_MAX + (x))
+#define PS_SEQ(x)		(x)
+#define PL_SEQ(x)		(AMS_PS_SEQ_MAX + (x))
+#define AMS_CTRL_SEQ_BASE	(AMS_PS_SEQ_MAX * 3)
+
+#define AMS_CHAN_TEMP(_scan_index, _addr) { \
+	.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), \
+	.event_spec = ams_temp_events, \
+	.scan_index = _scan_index, \
+	.num_event_specs = ARRAY_SIZE(ams_temp_events), \
+}
+
+#define AMS_CHAN_VOLTAGE(_scan_index, _addr, _alarm) { \
+	.type = IIO_VOLTAGE, \
+	.indexed = 1, \
+	.address = (_addr), \
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
+		BIT(IIO_CHAN_INFO_SCALE), \
+	.event_spec = (_alarm) ? ams_voltage_events : NULL, \
+	.scan_index = _scan_index, \
+	.num_event_specs = (_alarm) ? ARRAY_SIZE(ams_voltage_events) : 0, \
+}
+
+#define AMS_PS_CHAN_TEMP(_scan_index, _addr) \
+	AMS_CHAN_TEMP(PS_SEQ(_scan_index), _addr)
+#define AMS_PS_CHAN_VOLTAGE(_scan_index, _addr) \
+	AMS_CHAN_VOLTAGE(PS_SEQ(_scan_index), _addr, true)
+
+#define AMS_PL_CHAN_TEMP(_scan_index, _addr) \
+	AMS_CHAN_TEMP(PL_SEQ(_scan_index), _addr)
+#define AMS_PL_CHAN_VOLTAGE(_scan_index, _addr, _alarm) \
+	AMS_CHAN_VOLTAGE(PL_SEQ(_scan_index), _addr, _alarm)
+#define AMS_PL_AUX_CHAN_VOLTAGE(_auxno) \
+	AMS_CHAN_VOLTAGE(PL_SEQ(AMS_SEQ(_auxno)), \
+			AMS_REG_VAUX(_auxno), false)
+#define AMS_CTRL_CHAN_VOLTAGE(_scan_index, _addr) \
+	AMS_CHAN_VOLTAGE(PL_SEQ(AMS_SEQ(AMS_SEQ(_scan_index))), \
+			_addr, false)
+
+/**
+ * struct ams - Driver data for xilinx-ams
+ * @base: physical base address of device
+ * @ps_base: physical base address of PS device
+ * @pl_base: physical base address of PL device
+ * @clk: clocks associated with the device
+ * @dev: pointer to device struct
+ * @lock: to handle multiple user interaction
+ * @intr_lock: to protect interrupt mask values
+ * @alarm_mask: alarm configuration
+ * @current_masked_alarm: currently masked due to alarm
+ * @intr_mask: interrupt configuration
+ * @ams_unmask_work: re-enables event once the event condition disappears
+ *
+ * This structure contains necessary state for Sysmon driver to operate
+ */
+struct ams {
+	void __iomem *base;
+	void __iomem *ps_base;
+	void __iomem *pl_base;
+	struct clk *clk;
+	struct device *dev;
+	struct mutex lock;
+	spinlock_t intr_lock;
+	unsigned int alarm_mask;
+	unsigned int current_masked_alarm;
+	u64 intr_mask;
+	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, regval;
+
+	val = readl(ams->ps_base + offset);
+	regval = (val & ~mask) | (data & mask);
+	writel(regval, ams->ps_base + offset);
+}
+
+static inline void ams_pl_update_reg(struct ams *ams, unsigned int offset,
+				     u32 mask, u32 data)
+{
+	u32 val, regval;
+
+	val = readl(ams->pl_base + offset);
+	regval = (val & ~mask) | (data & mask);
+	writel(regval, ams->pl_base + offset);
+}
+
+static void ams_update_intrmask(struct ams *ams, u64 mask, u64 val)
+{
+	u32 regval;
+
+	ams->intr_mask = (ams->intr_mask & ~mask) | (val & mask);
+
+	regval = ~(ams->intr_mask | ams->current_masked_alarm);
+	writel(regval, ams->base + AMS_IER_0);
+
+	regval = ~(FIELD_GET(AMS_ISR1_INTR_MASK, ams->intr_mask));
+	writel(regval, ams->base + AMS_IER_1);
+
+	regval = ams->intr_mask | ams->current_masked_alarm;
+	writel(regval, ams->base + AMS_IDR_0);
+
+	regval = FIELD_GET(AMS_ISR1_INTR_MASK, ams->intr_mask);
+	writel(regval, 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 ams_update_ps_alarm(struct ams *ams, unsigned long alarm_mask)
+{
+	u32 cfg;
+	u32 val;
+
+	val = FIELD_GET(AMS_ISR0_ALARM_2_TO_0_MASK, alarm_mask);
+	cfg = ~(FIELD_PREP(AMS_CONF1_ALARM_2_TO_0_MASK, val));
+
+	val = FIELD_GET(AMS_ISR0_ALARM_6_TO_3_MASK, alarm_mask);
+	cfg &= ~(FIELD_PREP(AMS_CONF1_ALARM_6_TO_3_MASK, val));
+
+	ams_ps_update_reg(ams, AMS_REG_CONFIG1, AMS_REGCFG1_ALARM_MASK, cfg);
+
+	val = FIELD_GET(AMS_ISR0_ALARM_12_TO_7_MASK, alarm_mask);
+	cfg = ~(FIELD_PREP(AMS_CONF1_ALARM_12_TO_7_MASK, val));
+	ams_ps_update_reg(ams, AMS_REG_CONFIG3, AMS_REGCFG3_ALARM_MASK, cfg);
+}
+
+static void ams_update_pl_alarm(struct ams *ams, unsigned long alarm_mask)
+{
+	unsigned long pl_alarm_mask;
+	u32 cfg;
+	u32 val;
+
+	pl_alarm_mask = FIELD_GET(AMS_PL_ALARM_MASK, alarm_mask);
+
+	val = FIELD_GET(AMS_ISR0_ALARM_2_TO_0_MASK, pl_alarm_mask);
+	cfg = ~(FIELD_PREP(AMS_CONF1_ALARM_2_TO_0_MASK, val));
+
+	val = FIELD_GET(AMS_ISR0_ALARM_6_TO_3_MASK, pl_alarm_mask);
+	cfg &= ~(FIELD_PREP(AMS_CONF1_ALARM_6_TO_3_MASK, val));
+
+	ams_pl_update_reg(ams, AMS_REG_CONFIG1, AMS_REGCFG1_ALARM_MASK, cfg);
+
+	val = FIELD_GET(AMS_ISR0_ALARM_12_TO_7_MASK, pl_alarm_mask);
+	cfg = ~(FIELD_PREP(AMS_CONF1_ALARM_12_TO_7_MASK, val));
+	ams_pl_update_reg(ams, AMS_REG_CONFIG3, AMS_REGCFG3_ALARM_MASK, cfg);
+}
+
+static void ams_update_alarm(struct ams *ams, unsigned long alarm_mask)
+{
+	unsigned long flags;
+
+	if (ams->ps_base)
+		ams_update_ps_alarm(ams, alarm_mask);
+
+	if (ams->pl_base)
+		ams_update_pl_alarm(ams, alarm_mask);
+
+	spin_lock_irqsave(&ams->intr_lock, flags);
+	ams_update_intrmask(ams, AMS_ISR0_ALARM_MASK, ~alarm_mask);
+	spin_unlock_irqrestore(&ams->intr_lock, flags);
+}
+
+static void ams_enable_channel_sequence(struct iio_dev *indio_dev)
+{
+	struct ams *ams = iio_priv(indio_dev);
+	unsigned long long scan_mask;
+	int i;
+	u32 regval;
+
+	/*
+	 * Enable channel sequence. First 22 bits of scan_mask represent
+	 * PS channels, and next remaining bits represent PL channels.
+	 */
+
+	/* Run calibration of PS & PL as part of the sequence */
+	scan_mask = BIT(0) | BIT(AMS_PS_SEQ_MAX);
+	for (i = 0; i < indio_dev->num_channels; i++)
+		scan_mask |= BIT_ULL(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 */
+		regval = FIELD_GET(AMS_REG_SEQ0_MASK, scan_mask);
+		writel(regval, ams->ps_base + AMS_REG_SEQ_CH0);
+
+		regval = FIELD_GET(AMS_REG_SEQ2_MASK, scan_mask);
+		writel(regval, 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 = FIELD_GET(AMS_PL_SEQ_MASK, scan_mask);
+
+		regval = FIELD_GET(AMS_REG_SEQ0_MASK, scan_mask);
+		writel(regval, ams->pl_base + AMS_REG_SEQ_CH0);
+
+		regval = FIELD_GET(AMS_REG_SEQ1_MASK, scan_mask);
+		writel(regval, ams->pl_base + AMS_REG_SEQ_CH1);
+
+		regval = FIELD_GET(AMS_REG_SEQ2_MASK, scan_mask);
+		writel(regval, ams->pl_base + AMS_REG_SEQ_CH2);
+
+		/* set continuous sequence mode */
+		ams_pl_update_reg(ams, AMS_REG_CONFIG1, AMS_CONF1_SEQ_MASK,
+				  AMS_CONF1_SEQ_CONTINUOUS);
+	}
+}
+
+static int ams_init_device(struct ams *ams)
+{
+	u32 reg, value;
+	u32 expect = AMS_PS_CSTS_PS_READY;
+	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 & expect), AMS_INIT_POLL_TIME,
+					 AMS_INIT_TIMEOUT_US);
+		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) {
+		value = readl(ams->base + AMS_PL_CSTS);
+		if (value == 0)
+			return 0;
+
+		writel(AMS_PL_RESET_VALUE, ams->pl_base + AMS_VP_VN);
+
+		/* 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, AMS_ALARM_MASK, AMS_ALARM_MASK);
+
+	/* 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;
+
+	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;
+	u32 expect = AMS_ISR1_EOC_MASK;
+	int ret;
+
+	ret = ams_enable_single_channel(ams, offset);
+	if (ret)
+		return ret;
+
+	ret = readl_poll_timeout(ams->base + AMS_ISR_1, reg,
+				 (reg & expect), AMS_INIT_POLL_TIME, AMS_INIT_TIMEOUT_US);
+	if (ret)
+		return ret;
+
+	*data = readl(ams->base + offset);
+
+	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;
+	int regval;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		mutex_lock(&ams->lock);
+		if (chan->scan_index >= AMS_CTRL_SEQ_BASE) {
+			ret = ams_read_vcc_reg(ams, chan->address, val);
+			if (ret) {
+				mutex_unlock(&ams->lock);
+				return ret;
+			}
+			ams_enable_channel_sequence(indio_dev);
+		} else if (chan->scan_index >= AMS_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:
+			if (chan->scan_index < AMS_PS_SEQ_MAX) {
+				switch (chan->address) {
+				case AMS_SUPPLY1:
+				case AMS_SUPPLY2:
+				case AMS_SUPPLY3:
+				case AMS_SUPPLY4:
+				case AMS_SUPPLY9:
+				case AMS_SUPPLY10:
+				case AMS_VCCAMS:
+					*val = AMS_SUPPLY_SCALE_3VOLT;
+					break;
+				case AMS_SUPPLY5:
+				case AMS_SUPPLY6:
+				case AMS_SUPPLY7:
+				case AMS_SUPPLY8:
+					*val = AMS_SUPPLY_SCALE_6VOLT;
+					break;
+				default:
+					*val = AMS_SUPPLY_SCALE_1VOLT;
+					break;
+				}
+			} else if (chan->scan_index >= AMS_PS_SEQ_MAX &&
+				   chan->scan_index < AMS_CTRL_SEQ_BASE) {
+				switch (chan->address) {
+				case AMS_SUPPLY1:
+				case AMS_SUPPLY2:
+				case AMS_SUPPLY3:
+				case AMS_SUPPLY4:
+				case AMS_SUPPLY5:
+				case AMS_SUPPLY6:
+				case AMS_VCCAMS:
+				case AMS_VREFP:
+				case AMS_VREFN:
+					*val = AMS_SUPPLY_SCALE_3VOLT;
+					break;
+				case AMS_SUPPLY7:
+					regval = readl(ams->pl_base + AMS_REG_CONFIG4);
+					if (FIELD_GET(AMS_VUSER0_MASK, regval))
+						*val = AMS_SUPPLY_SCALE_6VOLT;
+					else
+						*val = AMS_SUPPLY_SCALE_3VOLT;
+					break;
+				case AMS_SUPPLY8:
+					regval = readl(ams->pl_base + AMS_REG_CONFIG4);
+					if (FIELD_GET(AMS_VUSER1_MASK, regval))
+						*val = AMS_SUPPLY_SCALE_6VOLT;
+					else
+						*val = AMS_SUPPLY_SCALE_3VOLT;
+					break;
+				case AMS_SUPPLY9:
+					regval = readl(ams->pl_base + AMS_REG_CONFIG4);
+					if (FIELD_GET(AMS_VUSER2_MASK, regval))
+						*val = AMS_SUPPLY_SCALE_6VOLT;
+					else
+						*val = AMS_SUPPLY_SCALE_3VOLT;
+					break;
+				case AMS_SUPPLY10:
+					regval = readl(ams->pl_base + AMS_REG_CONFIG4);
+					if (FIELD_GET(AMS_VUSER3_MASK, regval))
+						*val = AMS_SUPPLY_SCALE_6VOLT;
+					else
+						*val = AMS_SUPPLY_SCALE_3VOLT;
+					break;
+				case AMS_VP_VN:
+				case AMS_REG_VAUX(0) ... AMS_REG_VAUX(15):
+					*val = AMS_SUPPLY_SCALE_1VOLT;
+					break;
+				default:
+					*val = AMS_SUPPLY_SCALE_1VOLT;
+					break;
+				}
+			} else {
+				switch (chan->address) {
+				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;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int ams_get_alarm_offset(int scan_index, enum iio_event_direction dir)
+{
+	int offset;
+
+	if (scan_index >= AMS_PS_SEQ_MAX)
+		scan_index -= AMS_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;
+	} else {
+		offset = 0;
+	}
+
+	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;
+	default:
+		return 0;
+	}
+}
+
+static const struct iio_chan_spec *ams_event_to_channel(struct iio_dev *dev,
+							u32 event)
+{
+	int scan_index = 0, i;
+
+	if (event >= AMS_PL_ALARM_START) {
+		event -= AMS_PL_ALARM_START;
+		scan_index = AMS_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;
+	default:
+		break;
+	}
+
+	for (i = 0; i < dev->num_channels; i++)
+		if (dev->channels[i].scan_index == scan_index)
+			break;
+
+	return &dev->channels[i];
+}
+
+static int ams_get_alarm_mask(int scan_index)
+{
+	int bit = 0;
+
+	if (scan_index >= AMS_PS_SEQ_MAX) {
+		bit = AMS_PL_ALARM_START;
+		scan_index -= AMS_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);
+	default:
+		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));
+}
+
+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;
+
+	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 >= AMS_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 >= AMS_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 >= AMS_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;
+
+	spin_lock_irq(&ams->intr_lock);
+
+	status = readl(ams->base + AMS_ISR_0);
+
+	/* Clear those bits which are not active anymore */
+	unmask = (ams->current_masked_alarm ^ status) & ams->current_masked_alarm;
+
+	/* Clear status of disabled alarm */
+	unmask |= ams->intr_mask;
+
+	ams->current_masked_alarm &= status;
+
+	/* Also clear those which are masked out anyway */
+	ams->current_masked_alarm &= ~ams->intr_mask;
+
+	/* Clear the interrupts before we unmask them */
+	writel(unmask, ams->base + AMS_ISR_0);
+
+	ams_update_intrmask(ams, ~AMS_ALARM_MASK, ~AMS_ALARM_MASK);
+
+	spin_unlock_irq(&ams->intr_lock);
+
+	/* If still pending some alarm re-trigger the timer */
+	if (ams->current_masked_alarm)
+		schedule_delayed_work(&ams->ams_unmask_work,
+				      msecs_to_jiffies(AMS_UNMASK_TIMEOUT_MS));
+}
+
+static irqreturn_t ams_irq(int irq, void *data)
+{
+	struct iio_dev *indio_dev = data;
+	struct ams *ams = iio_priv(indio_dev);
+	u32 isr0;
+
+	spin_lock(&ams->intr_lock);
+
+	isr0 = readl(ams->base + AMS_ISR_0);
+
+	/* Only process alarms that are not masked */
+	isr0 &= ~((ams->intr_mask & AMS_ISR0_ALARM_MASK) | ams->current_masked_alarm);
+	if (!isr0) {
+		spin_unlock(&ams->intr_lock);
+		return IRQ_NONE;
+	}
+
+	/* Clear interrupt */
+	writel(isr0, ams->base + AMS_ISR_0);
+
+	/* Mask the alarm interrupts until cleared */
+	ams->current_masked_alarm |= isr0;
+	ams_update_intrmask(ams, ~AMS_ALARM_MASK, ~AMS_ALARM_MASK);
+
+	ams_handle_events(indio_dev, isr0);
+
+	schedule_delayed_work(&ams->ams_unmask_work,
+			      msecs_to_jiffies(AMS_UNMASK_TIMEOUT_MS));
+
+	spin_unlock(&ams->intr_lock);
+
+	return IRQ_HANDLED;
+}
+
+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),
+	AMS_PS_CHAN_TEMP(AMS_SEQ_TEMP_REMOTE, AMS_TEMP_REMOTE),
+	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY1, AMS_SUPPLY1),
+	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY2, AMS_SUPPLY2),
+	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY3, AMS_SUPPLY3),
+	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY4, AMS_SUPPLY4),
+	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY5, AMS_SUPPLY5),
+	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY6, AMS_SUPPLY6),
+	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY7, AMS_SUPPLY7),
+	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY8, AMS_SUPPLY8),
+	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY9, AMS_SUPPLY9),
+	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_SUPPLY10, AMS_SUPPLY10),
+	AMS_PS_CHAN_VOLTAGE(AMS_SEQ_VCCAMS, AMS_VCCAMS),
+};
+
+static const struct iio_chan_spec ams_pl_channels[] = {
+	AMS_PL_CHAN_TEMP(AMS_SEQ_TEMP, AMS_TEMP),
+	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY1, AMS_SUPPLY1, true),
+	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY2, AMS_SUPPLY2, true),
+	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_VREFP, AMS_VREFP, false),
+	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_VREFN, AMS_VREFN, false),
+	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY3, AMS_SUPPLY3, true),
+	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY4, AMS_SUPPLY4, true),
+	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY5, AMS_SUPPLY5, true),
+	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY6, AMS_SUPPLY6, true),
+	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_VCCAMS, AMS_VCCAMS, true),
+	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_VP_VN, AMS_VP_VN, false),
+	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY7, AMS_SUPPLY7, true),
+	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY8, AMS_SUPPLY8, true),
+	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY9, AMS_SUPPLY9, true),
+	AMS_PL_CHAN_VOLTAGE(AMS_SEQ_SUPPLY10, AMS_SUPPLY10, true),
+	AMS_PL_AUX_CHAN_VOLTAGE(0),
+	AMS_PL_AUX_CHAN_VOLTAGE(1),
+	AMS_PL_AUX_CHAN_VOLTAGE(2),
+	AMS_PL_AUX_CHAN_VOLTAGE(3),
+	AMS_PL_AUX_CHAN_VOLTAGE(4),
+	AMS_PL_AUX_CHAN_VOLTAGE(5),
+	AMS_PL_AUX_CHAN_VOLTAGE(6),
+	AMS_PL_AUX_CHAN_VOLTAGE(7),
+	AMS_PL_AUX_CHAN_VOLTAGE(8),
+	AMS_PL_AUX_CHAN_VOLTAGE(9),
+	AMS_PL_AUX_CHAN_VOLTAGE(10),
+	AMS_PL_AUX_CHAN_VOLTAGE(11),
+	AMS_PL_AUX_CHAN_VOLTAGE(12),
+	AMS_PL_AUX_CHAN_VOLTAGE(13),
+	AMS_PL_AUX_CHAN_VOLTAGE(14),
+	AMS_PL_AUX_CHAN_VOLTAGE(15),
+};
+
+static const struct iio_chan_spec ams_ctrl_channels[] = {
+	AMS_CTRL_CHAN_VOLTAGE(AMS_SEQ_VCC_PSPLL, AMS_VCC_PSPLL0),
+	AMS_CTRL_CHAN_VOLTAGE(AMS_SEQ_VCC_PSBATT, AMS_VCC_PSPLL3),
+	AMS_CTRL_CHAN_VOLTAGE(AMS_SEQ_VCCINT, AMS_VCCINT),
+	AMS_CTRL_CHAN_VOLTAGE(AMS_SEQ_VCCBRAM, AMS_VCCBRAM),
+	AMS_CTRL_CHAN_VOLTAGE(AMS_SEQ_VCCAUX, AMS_VCCAUX),
+	AMS_CTRL_CHAN_VOLTAGE(AMS_SEQ_PSDDRPLL, AMS_PSDDRPLL),
+	AMS_CTRL_CHAN_VOLTAGE(AMS_SEQ_INTDDR, AMS_PSINTFPDDR),
+};
+
+static int ams_get_ext_chan(struct fwnode_handle *chan_node,
+			    struct iio_chan_spec *channels, int num_channels)
+{
+	struct iio_chan_spec *chan;
+	struct fwnode_handle *child;
+	unsigned int reg, ext_chan;
+	int ret;
+
+	fwnode_for_each_child_node(chan_node, child) {
+		ret = fwnode_property_read_u32(child, "reg", &reg);
+		if (ret || reg > AMS_PL_MAX_EXT_CHANNEL + 30)
+			continue;
+
+		chan = &channels[num_channels];
+		ext_chan = reg + AMS_PL_MAX_FIXED_CHANNEL - 30;
+		memcpy(chan, &ams_pl_channels[ext_chan], sizeof(*channels));
+
+		if (fwnode_property_read_bool(child, "xlnx,bipolar"))
+			chan->scan_type.sign =	's';
+
+		num_channels++;
+	}
+
+	return num_channels;
+}
+
+static void ams_iounmap(void *data)
+{
+	iounmap(data);
+}
+
+static int ams_init_module(struct iio_dev *indio_dev,
+			   struct fwnode_handle *fwnode,
+			   struct iio_chan_spec *channels)
+{
+	struct device *dev = indio_dev->dev.parent;
+	struct ams *ams = iio_priv(indio_dev);
+	int num_channels = 0;
+	int ret;
+
+	if (fwnode_property_match_string(fwnode, "compatible",
+					 "xlnx,zynqmp-ams-ps") == 0) {
+		ams->ps_base = fwnode_iomap(fwnode, 0);
+		if (!ams->ps_base)
+			return -ENXIO;
+		ret = devm_add_action_or_reset(dev, ams_iounmap, ams->ps_base);
+		if (ret < 0)
+			return ret;
+
+		/* add PS channels to iio device channels */
+		memcpy(channels, ams_ps_channels,
+		       sizeof(ams_ps_channels));
+		num_channels += ARRAY_SIZE(ams_ps_channels);
+	} else if (fwnode_property_match_string(fwnode, "compatible",
+						"xlnx,zynqmp-ams-pl") == 0) {
+		ams->pl_base = fwnode_iomap(fwnode, 0);
+		if (!ams->pl_base)
+			return -ENXIO;
+
+		ret = devm_add_action_or_reset(dev, ams_iounmap, ams->pl_base);
+		if (ret < 0)
+			return ret;
+
+		/* Copy only first 10 fix channels */
+		memcpy(channels, ams_pl_channels,
+		       AMS_PL_MAX_FIXED_CHANNEL * sizeof(*channels));
+		num_channels += AMS_PL_MAX_FIXED_CHANNEL;
+
+		num_channels = ams_get_ext_chan(fwnode, channels,
+						num_channels);
+	} else if (fwnode_property_match_string(fwnode, "compatible",
+						"xlnx,zynqmp-ams") == 0) {
+		/* add AMS channels to iio device channels */
+		memcpy(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_firmware(struct iio_dev *indio_dev)
+{
+	struct ams *ams = iio_priv(indio_dev);
+	struct iio_chan_spec *ams_channels, *dev_channels;
+	struct device *dev = indio_dev->dev.parent;
+	struct fwnode_handle *child = NULL;
+	struct fwnode_handle *fwnode = dev_fwnode(dev);
+	size_t ams_size, dev_size;
+	int ret, ch_cnt = 0, i, rising_off, falling_off;
+	unsigned int num_channels = 0;
+
+	ams_size = ARRAY_SIZE(ams_ps_channels) + ARRAY_SIZE(ams_pl_channels) +
+		ARRAY_SIZE(ams_ctrl_channels);
+
+	/* Initialize buffer for channel specification */
+	ams_channels = devm_kcalloc(dev, ams_size, sizeof(*ams_channels), GFP_KERNEL);
+	if (!ams_channels)
+		return -ENOMEM;
+
+	if (fwnode_device_is_available(fwnode)) {
+		ret = ams_init_module(indio_dev, fwnode, ams_channels);
+		if (ret < 0)
+			return ret;
+
+		num_channels += ret;
+	}
+
+	fwnode_for_each_child_node(fwnode, child) {
+		if (fwnode_device_is_available(child)) {
+			ret = ams_init_module(indio_dev, child,
+					      ams_channels + num_channels);
+			if (ret < 0) {
+				fwnode_handle_put(child);
+				return ret;
+			}
+
+			num_channels += ret;
+		}
+	}
+
+	for (i = 0; i < num_channels; i++) {
+		ams_channels[i].channel = ch_cnt++;
+
+		if (ams_channels[i].scan_index < AMS_CTRL_SEQ_BASE) {
+			/* 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 >= AMS_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_size = sizeof(*dev_channels) * num_channels;
+	dev_channels = devm_krealloc(dev, ams_channels, dev_size, GFP_KERNEL);
+	if (!dev_channels)
+		ret = -ENOMEM;
+
+	indio_dev->channels = dev_channels;
+	indio_dev->num_channels = num_channels;
+
+	return 0;
+}
+
+static const struct iio_info iio_ams_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 void ams_clk_disable_unprepare(void *data)
+{
+	clk_disable_unprepare(data);
+}
+
+static void ams_cancel_delayed_work(void *data)
+{
+	cancel_delayed_work(data);
+}
+
+static int ams_probe(struct platform_device *pdev)
+{
+	struct iio_dev *indio_dev;
+	struct ams *ams;
+	int ret;
+	int irq;
+
+	indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*ams));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	ams = iio_priv(indio_dev);
+	mutex_init(&ams->lock);
+	spin_lock_init(&ams->intr_lock);
+
+	indio_dev->name = "xilinx-ams";
+
+	indio_dev->info = &iio_ams_info;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+
+	ams->base = devm_platform_ioremap_resource(pdev, 0);
+	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);
+
+	ret = clk_prepare_enable(ams->clk);
+	if (ret < 0)
+		return ret;
+
+	ret = devm_add_action_or_reset(&pdev->dev, ams_clk_disable_unprepare, ams->clk);
+	if (ret < 0)
+		return ret;
+
+	INIT_DELAYED_WORK(&ams->ams_unmask_work, ams_unmask_worker);
+	ret = devm_add_action_or_reset(&pdev->dev, ams_cancel_delayed_work,
+				       &ams->ams_unmask_work);
+	if (ret < 0)
+		return ret;
+
+	ret = ams_parse_firmware(indio_dev);
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret, "failure in parsing DT\n");
+
+	ret = ams_init_device(ams);
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret, "failed to initialize AMS\n");
+
+	ams_enable_channel_sequence(indio_dev);
+
+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0)
+		return ret;
+
+	ret = devm_request_irq(&pdev->dev, irq, &ams_irq, 0, "ams-irq",
+			       indio_dev);
+	if (ret < 0)
+		return dev_err_probe(&pdev->dev, ret, "failed to register interrupt\n");
+
+	platform_set_drvdata(pdev, indio_dev);
+
+	return devm_iio_device_register(&pdev->dev, indio_dev);
+}
+
+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));
+
+	return clk_prepare_enable(ams->clk);
+}
+
+static SIMPLE_DEV_PM_OPS(ams_pm_ops, ams_suspend, ams_resume);
+
+static struct platform_driver ams_driver = {
+	.probe = ams_probe,
+	.driver = {
+		.name = "xilinx-ams",
+		.pm = &ams_pm_ops,
+		.of_match_table = ams_of_match_table,
+	},
+};
+module_platform_driver(ams_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Xilinx, Inc.");
-- 
2.17.1


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

* [PATCH v11 4/5] dt-bindings: iio: adc: Add Xilinx AMS binding documentation
  2021-11-24 22:54 [PATCH v11 0/5] Add Xilinx AMS Driver Anand Ashok Dumbre
                   ` (2 preceding siblings ...)
  2021-11-24 22:54 ` [PATCH v11 3/5] iio: adc: Add Xilinx AMS driver Anand Ashok Dumbre
@ 2021-11-24 22:54 ` Anand Ashok Dumbre
  2021-11-24 22:54 ` [PATCH v11 5/5] MAINTAINERS: Add maintainer for xilinx-ams Anand Ashok Dumbre
  4 siblings, 0 replies; 19+ messages in thread
From: Anand Ashok Dumbre @ 2021-11-24 22:54 UTC (permalink / raw)
  To: linux-kernel, jic23, lars, linux-iio, git, michal.simek, gregkh,
	rafael, linux-acpi, andriy.shevchenko, heikki.krogerus
  Cc: Anand Ashok Dumbre

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: Anand Ashok Dumbre <anand.ashok.dumbre@xilinx.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 .../bindings/iio/adc/xlnx,zynqmp-ams.yaml     | 227 ++++++++++++++++++
 1 file changed, 227 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/xlnx,zynqmp-ams.yaml

diff --git a/Documentation/devicetree/bindings/iio/adc/xlnx,zynqmp-ams.yaml b/Documentation/devicetree/bindings/iio/adc/xlnx,zynqmp-ams.yaml
new file mode 100644
index 000000000000..87992db389b2
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/xlnx,zynqmp-ams.yaml
@@ -0,0 +1,227 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/adc/xlnx,zynqmp-ams.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Xilinx Zynq Ultrascale AMS controller
+
+maintainers:
+  - Anand Ashok Dumbre <anand.ashok.dumbre@xilinx.com>
+
+description: |
+  The AMS (Analog Monitoring System) 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.
+
+  AMS Channel 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 measurement (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 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     |Differential 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
+  --------------------------------------------------------------------------------------------------------
+
+properties:
+  compatible:
+    enum:
+      - xlnx,zynqmp-ams
+
+  interrupts:
+    maxItems: 1
+
+  reg:
+    description: AMS Controller register space
+    maxItems: 1
+
+  ranges:
+    description:
+      Maps the child address space for PS and/or PL.
+    maxItems: 1
+
+  '#address-cells':
+    const: 1
+
+  '#size-cells':
+    const: 1
+
+  '#io-channel-cells':
+    const: 1
+
+  ams-ps@0:
+    type: object
+    description: |
+      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.
+
+    properties:
+      compatible:
+        enum:
+          - xlnx,zynqmp-ams-ps
+
+      reg:
+        description: Register Space for PS-SYSMON
+        maxItems: 1
+
+    required:
+      - compatible
+      - reg
+
+    additionalProperties: false
+
+  ams-pl@400:
+    type: object
+    description:
+      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.
+
+    properties:
+      compatible:
+        items:
+          - enum:
+              - xlnx,zynqmp-ams-pl
+
+      reg:
+        description: Register Space for PL-SYSMON.
+        maxItems: 1
+
+      '#address-cells':
+        const: 1
+
+      '#size-cells':
+        const: 0
+
+    patternProperties:
+      "^channel@([2-4][0-9]|50)$":
+        type: object
+        description:
+          Describes the external channels connected.
+
+        properties:
+          reg:
+            description:
+              Pair of pins the channel is connected to. This value is
+              same as Channel Number for a particular channel.
+            minimum: 20
+            maximum: 50
+
+          xlnx,bipolar:
+            $ref: /schemas/types.yaml#/definitions/flag
+            type: boolean
+            description:
+              If the set channel is used in bipolar mode.
+
+        required:
+          - reg
+
+        additionalProperties: false
+
+required:
+  - compatible
+  - reg
+  - ranges
+
+additionalProperties: false
+
+examples:
+  - |
+    bus {
+        #address-cells = <2>;
+        #size-cells = <2>;
+
+        xilinx_ams: ams@ffa50000 {
+            compatible = "xlnx,zynqmp-ams";
+            interrupt-parent = <&gic>;
+            interrupts = <0 56 4>;
+            reg = <0x0 0xffa50000 0x0 0x800>;
+            #address-cells = <1>;
+            #size-cells = <1>;
+            #io-channel-cells = <1>;
+            ranges = <0 0 0xffa50800 0x800>;
+
+            ams_ps: ams-ps@0 {
+                compatible = "xlnx,zynqmp-ams-ps";
+                reg = <0 0x400>;
+            };
+
+            ams_pl: ams-pl@400 {
+                compatible = "xlnx,zynqmp-ams-pl";
+                reg = <0x400 0x400>;
+                #address-cells = <1>;
+                #size-cells = <0>;
+                channel@30 {
+                    reg = <30>;
+                    xlnx,bipolar;
+                };
+                channel@31 {
+                    reg = <31>;
+                };
+                channel@38 {
+                    reg = <38>;
+                    xlnx,bipolar;
+                };
+            };
+        };
+    };
-- 
2.17.1


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

* [PATCH v11 5/5] MAINTAINERS: Add maintainer for xilinx-ams
  2021-11-24 22:54 [PATCH v11 0/5] Add Xilinx AMS Driver Anand Ashok Dumbre
                   ` (3 preceding siblings ...)
  2021-11-24 22:54 ` [PATCH v11 4/5] dt-bindings: iio: adc: Add Xilinx AMS binding documentation Anand Ashok Dumbre
@ 2021-11-24 22:54 ` Anand Ashok Dumbre
  4 siblings, 0 replies; 19+ messages in thread
From: Anand Ashok Dumbre @ 2021-11-24 22:54 UTC (permalink / raw)
  To: linux-kernel, jic23, lars, linux-iio, git, michal.simek, gregkh,
	rafael, linux-acpi, andriy.shevchenko, heikki.krogerus
  Cc: Anand Ashok Dumbre

Add maintaner entry for xilinx-ams driver.

Signed-off-by: Anand Ashok Dumbre <anand.ashok.dumbre@xilinx.com>
---
 MAINTAINERS | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 7a2345ce8521..64d6a06b22f0 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -20862,6 +20862,13 @@ F:	fs/xfs/
 F:	include/uapi/linux/dqblk_xfs.h
 F:	include/uapi/linux/fsmap.h
 
+XILINX AMS DRIVER
+M:	Anand Ashok Dumbre <anand.ashok.dumbre@xilinx.com>
+L:	linux-iio@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/iio/adc/xlnx,zynqmp-ams.yaml
+F:	drivers/iio/adc/xilinx-ams.c
+
 XILINX AXI ETHERNET DRIVER
 M:	Radhey Shyam Pandey <radhey.shyam.pandey@xilinx.com>
 S:	Maintained
-- 
2.17.1


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

* Re: [PATCH v11 1/5] device property: Add fwnode_iomap()
  2021-11-24 22:54 ` [PATCH v11 1/5] device property: Add fwnode_iomap() Anand Ashok Dumbre
@ 2021-11-25 11:42   ` Andy Shevchenko
  2021-11-30 21:58     ` Anand Ashok Dumbre
  2021-11-27 11:54   ` kernel test robot
  1 sibling, 1 reply; 19+ messages in thread
From: Andy Shevchenko @ 2021-11-25 11:42 UTC (permalink / raw)
  To: Anand Ashok Dumbre
  Cc: linux-kernel, jic23, lars, linux-iio, git, michal.simek, gregkh,
	rafael, linux-acpi, heikki.krogerus

On Wed, Nov 24, 2021 at 10:54:03PM +0000, Anand Ashok Dumbre wrote:
> This patch introduces a new helper routine - fwnode_iomap(), which
> allows to map the memory mapped IO for a given device node.
> 
> This implementation does not cover the ACPI case and may be expanded
> in the future. The main purpose here is to be able to develop resource
> provider agnostic drivers.

...

> +void __iomem *fwnode_iomap(struct fwnode_handle *fwnode, int index)
> +{

> +	if (is_of_node(fwnode))
> +		return of_iomap(to_of_node(fwnode), index);

It seems this part should be wrapped in some ifdeffery according to kbuild bot
report.

> +	return NULL;
> +}
> +EXPORT_SYMBOL(fwnode_iomap);

-- 
With Best Regards,
Andy Shevchenko



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

* Re: [PATCH v11 3/5] iio: adc: Add Xilinx AMS driver
  2021-11-24 22:54 ` [PATCH v11 3/5] iio: adc: Add Xilinx AMS driver Anand Ashok Dumbre
@ 2021-11-25 12:14   ` Andy Shevchenko
  2021-12-02 16:32     ` Anand Ashok Dumbre
  2021-11-27  2:43   ` kernel test robot
  2021-11-27  5:16   ` kernel test robot
  2 siblings, 1 reply; 19+ messages in thread
From: Andy Shevchenko @ 2021-11-25 12:14 UTC (permalink / raw)
  To: Anand Ashok Dumbre
  Cc: linux-kernel, jic23, lars, linux-iio, git, michal.simek, gregkh,
	rafael, linux-acpi, heikki.krogerus, Manish Narani

On Wed, Nov 24, 2021 at 10:54:05PM +0000, Anand Ashok Dumbre 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 an external master. Out of these interfaces 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.

...

> +#define AMS_IDR_1			0x02c
...
> +#define AMS_VCC_PSPLL3			0x06C
...
> +#define AMS_VCCBRAM			0x07C
...
> +#define AMS_PSINTFPDDR			0x09C
...and so on

Be consistent with the capitalization in the hex values.

...

> +#define AMS_INIT_POLL_TIME		200

Does it need unit?

> +#define AMS_SUPPLY_SCALE_1VOLT		1000
> +#define AMS_SUPPLY_SCALE_3VOLT		3000
> +#define AMS_SUPPLY_SCALE_6VOLT		6000

I would rather make units with these:

#define AMS_SUPPLY_SCALE_1VOLT_mV		1000
#define AMS_SUPPLY_SCALE_3VOLT_mV		3000
#define AMS_SUPPLY_SCALE_6VOLT_mV		6000

...

> +#define AMS_PL_AUX_CHAN_VOLTAGE(_auxno) \

> +	AMS_CHAN_VOLTAGE(PL_SEQ(AMS_SEQ(_auxno)), \
> +			AMS_REG_VAUX(_auxno), false)

One line?

> +#define AMS_CTRL_CHAN_VOLTAGE(_scan_index, _addr) \

> +	AMS_CHAN_VOLTAGE(PL_SEQ(AMS_SEQ(AMS_SEQ(_scan_index))), \
> +			_addr, false)

Ditto.

...

> +/**
> + * struct ams - Driver data for xilinx-ams
> + * @base: physical base address of device
> + * @ps_base: physical base address of PS device
> + * @pl_base: physical base address of PL device
> + * @clk: clocks associated with the device
> + * @dev: pointer to device struct
> + * @lock: to handle multiple user interaction
> + * @intr_lock: to protect interrupt mask values
> + * @alarm_mask: alarm configuration
> + * @current_masked_alarm: currently masked due to alarm
> + * @intr_mask: interrupt configuration
> + * @ams_unmask_work: re-enables event once the event condition disappears

> + * This structure contains necessary state for Sysmon driver to operate

Shouldn't be this "state for Sysmon driver to operate" a summary above?

> + */

...

> +	u32 reg, value;
> +	u32 expect = AMS_PS_CSTS_PS_READY;
> +	int ret;

	u32 expect = AMS_PS_CSTS_PS_READY;
	u32 reg, value;
	int ret;

...

> +	u32 reg;
> +	u32 expect = AMS_ISR1_EOC_MASK;
> +	int ret;

Ditto.

...

> +	ret = readl_poll_timeout(ams->base + AMS_ISR_1, reg,
> +				 (reg & expect), AMS_INIT_POLL_TIME, AMS_INIT_TIMEOUT_US);

Something wrong with line lengths... There is enough space on previous line for
one parameter.

> +	if (ret)
> +		return ret;

...

> +	case IIO_CHAN_INFO_RAW:
> +		mutex_lock(&ams->lock);
> +		if (chan->scan_index >= AMS_CTRL_SEQ_BASE) {
> +			ret = ams_read_vcc_reg(ams, chan->address, val);
> +			if (ret) {

> +				mutex_unlock(&ams->lock);
> +				return ret;

Can it be
				goto out_unlock;

> +			}
> +			ams_enable_channel_sequence(indio_dev);
> +		} else if (chan->scan_index >= AMS_PS_SEQ_MAX)
> +			*val = readl(ams->pl_base + chan->address);
> +		else
> +			*val = readl(ams->ps_base + chan->address);

		ret = IIO_VAL_INT;
out_unlock:

> +		mutex_unlock(&ams->lock);
> +
> +		return IIO_VAL_INT;

		mutex_unlock(&ams->lock);
		return ret;

?

Also the question, why mutex only against INFO_RAW case?

...

> +		switch (chan->type) {
> +		case IIO_VOLTAGE:
> +			if (chan->scan_index < AMS_PS_SEQ_MAX) {
> +				switch (chan->address) {
> +				case AMS_SUPPLY1:
> +				case AMS_SUPPLY2:
> +				case AMS_SUPPLY3:
> +				case AMS_SUPPLY4:
> +				case AMS_SUPPLY9:
> +				case AMS_SUPPLY10:
> +				case AMS_VCCAMS:
> +					*val = AMS_SUPPLY_SCALE_3VOLT;
> +					break;
> +				case AMS_SUPPLY5:
> +				case AMS_SUPPLY6:
> +				case AMS_SUPPLY7:
> +				case AMS_SUPPLY8:
> +					*val = AMS_SUPPLY_SCALE_6VOLT;
> +					break;
> +				default:
> +					*val = AMS_SUPPLY_SCALE_1VOLT;
> +					break;
> +				}
> +			} else if (chan->scan_index >= AMS_PS_SEQ_MAX &&
> +				   chan->scan_index < AMS_CTRL_SEQ_BASE) {
> +				switch (chan->address) {
> +				case AMS_SUPPLY1:
> +				case AMS_SUPPLY2:
> +				case AMS_SUPPLY3:
> +				case AMS_SUPPLY4:
> +				case AMS_SUPPLY5:
> +				case AMS_SUPPLY6:
> +				case AMS_VCCAMS:
> +				case AMS_VREFP:
> +				case AMS_VREFN:
> +					*val = AMS_SUPPLY_SCALE_3VOLT;
> +					break;
> +				case AMS_SUPPLY7:
> +					regval = readl(ams->pl_base + AMS_REG_CONFIG4);
> +					if (FIELD_GET(AMS_VUSER0_MASK, regval))
> +						*val = AMS_SUPPLY_SCALE_6VOLT;
> +					else
> +						*val = AMS_SUPPLY_SCALE_3VOLT;
> +					break;
> +				case AMS_SUPPLY8:
> +					regval = readl(ams->pl_base + AMS_REG_CONFIG4);
> +					if (FIELD_GET(AMS_VUSER1_MASK, regval))
> +						*val = AMS_SUPPLY_SCALE_6VOLT;
> +					else
> +						*val = AMS_SUPPLY_SCALE_3VOLT;
> +					break;
> +				case AMS_SUPPLY9:
> +					regval = readl(ams->pl_base + AMS_REG_CONFIG4);
> +					if (FIELD_GET(AMS_VUSER2_MASK, regval))
> +						*val = AMS_SUPPLY_SCALE_6VOLT;
> +					else
> +						*val = AMS_SUPPLY_SCALE_3VOLT;
> +					break;
> +				case AMS_SUPPLY10:
> +					regval = readl(ams->pl_base + AMS_REG_CONFIG4);
> +					if (FIELD_GET(AMS_VUSER3_MASK, regval))
> +						*val = AMS_SUPPLY_SCALE_6VOLT;
> +					else
> +						*val = AMS_SUPPLY_SCALE_3VOLT;
> +					break;
> +				case AMS_VP_VN:
> +				case AMS_REG_VAUX(0) ... AMS_REG_VAUX(15):
> +					*val = AMS_SUPPLY_SCALE_1VOLT;
> +					break;
> +				default:
> +					*val = AMS_SUPPLY_SCALE_1VOLT;
> +					break;
> +				}
> +			} else {
> +				switch (chan->address) {
> +				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;
> +		}

Isn't it a bit too looong for a single switch case?

...

> +/**
> + * ams_unmask_worker - ams alarm interrupt unmask worker

> + * @work :		work to be done

Be consistent with a style on how you describe parameters in the kernel doc.

> + * 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.
> + */

...

> +	fwnode_for_each_child_node(chan_node, child) {
> +		ret = fwnode_property_read_u32(child, "reg", &reg);
> +		if (ret || reg > AMS_PL_MAX_EXT_CHANNEL + 30)
> +			continue;
> +
> +		chan = &channels[num_channels];
> +		ext_chan = reg + AMS_PL_MAX_FIXED_CHANNEL - 30;
> +		memcpy(chan, &ams_pl_channels[ext_chan], sizeof(*channels));
> +
> +		if (fwnode_property_read_bool(child, "xlnx,bipolar"))
> +			chan->scan_type.sign =	's';

Needless double spacing.

> +		num_channels++;
> +	}

...

> +		/* add PS channels to iio device channels */
> +		memcpy(channels, ams_ps_channels,
> +		       sizeof(ams_ps_channels));

One line.

...

> +		/* Copy only first 10 fix channels */

Be consistent with one line comments (pay attention to the capitalization,
compare to the above).

> +		memcpy(channels, ams_pl_channels,
> +		       AMS_PL_MAX_FIXED_CHANNEL * sizeof(*channels));

One line?

...

> +		/* add AMS channels to iio device channels */
> +		memcpy(channels, ams_ctrl_channels,
> +		       sizeof(ams_ctrl_channels));

One line.

...

> +	fwnode_for_each_child_node(fwnode, child) {
> +		if (fwnode_device_is_available(child)) {

> +			ret = ams_init_module(indio_dev, child,
> +					      ams_channels + num_channels);

One line?

> +			if (ret < 0) {
> +				fwnode_handle_put(child);
> +				return ret;
> +			}
> +
> +			num_channels += ret;
> +		}
> +	}

...

> +	dev_size = sizeof(*dev_channels) * num_channels;

Here you need to have an array_size(). Or introduce a devm_krealloc_array().

> +	dev_channels = devm_krealloc(dev, ams_channels, dev_size, GFP_KERNEL);
> +	if (!dev_channels)
> +		ret = -ENOMEM;


-- 
With Best Regards,
Andy Shevchenko



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

* Re: [PATCH v11 3/5] iio: adc: Add Xilinx AMS driver
  2021-11-24 22:54 ` [PATCH v11 3/5] iio: adc: Add Xilinx AMS driver Anand Ashok Dumbre
  2021-11-25 12:14   ` Andy Shevchenko
@ 2021-11-27  2:43   ` kernel test robot
  2021-11-27 17:50     ` Jonathan Cameron
  2021-11-27  5:16   ` kernel test robot
  2 siblings, 1 reply; 19+ messages in thread
From: kernel test robot @ 2021-11-27  2:43 UTC (permalink / raw)
  To: Anand Ashok Dumbre, linux-kernel, jic23, lars, linux-iio, git,
	michal.simek, gregkh, rafael, linux-acpi, andriy.shevchenko
  Cc: kbuild-all

Hi Anand,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on jic23-iio/togreg]
[also build test WARNING on linux/master linus/master v5.16-rc2 next-20211126]
[cannot apply to xilinx-xlnx/master]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Anand-Ashok-Dumbre/Add-Xilinx-AMS-Driver/20211125-065614
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
config: powerpc64-randconfig-s032-20211126 (https://download.01.org/0day-ci/archive/20211127/202111271041.Cdu0In2E-lkp@intel.com/config)
compiler: powerpc64-linux-gcc (GCC) 11.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # apt-get install sparse
        # sparse version: v0.6.4-dirty
        # https://github.com/0day-ci/linux/commit/9b07fe52c07c2e9f6eccd2f2050f69558904ed64
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Anand-Ashok-Dumbre/Add-Xilinx-AMS-Driver/20211125-065614
        git checkout 9b07fe52c07c2e9f6eccd2f2050f69558904ed64
        # save the config file to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=powerpc SHELL=/bin/bash drivers/clk/ drivers/iio/adc/ drivers/pci/controller/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>


sparse warnings: (new ones prefixed by >>)
>> drivers/iio/adc/xilinx-ams.c:1175:17: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected void volatile [noderef] __iomem *addr @@     got void *data @@
   drivers/iio/adc/xilinx-ams.c:1175:17: sparse:     expected void volatile [noderef] __iomem *addr
   drivers/iio/adc/xilinx-ams.c:1175:17: sparse:     got void *data
>> drivers/iio/adc/xilinx-ams.c:1192:69: sparse: sparse: incorrect type in argument 3 (different address spaces) @@     expected void *data @@     got void [noderef] __iomem *ps_base @@
   drivers/iio/adc/xilinx-ams.c:1192:69: sparse:     expected void *data
   drivers/iio/adc/xilinx-ams.c:1192:69: sparse:     got void [noderef] __iomem *ps_base
>> drivers/iio/adc/xilinx-ams.c:1206:69: sparse: sparse: incorrect type in argument 3 (different address spaces) @@     expected void *data @@     got void [noderef] __iomem *pl_base @@
   drivers/iio/adc/xilinx-ams.c:1206:69: sparse:     expected void *data
   drivers/iio/adc/xilinx-ams.c:1206:69: sparse:     got void [noderef] __iomem *pl_base

vim +1175 drivers/iio/adc/xilinx-ams.c

  1172	
  1173	static void ams_iounmap(void *data)
  1174	{
> 1175		iounmap(data);
  1176	}
  1177	
  1178	static int ams_init_module(struct iio_dev *indio_dev,
  1179				   struct fwnode_handle *fwnode,
  1180				   struct iio_chan_spec *channels)
  1181	{
  1182		struct device *dev = indio_dev->dev.parent;
  1183		struct ams *ams = iio_priv(indio_dev);
  1184		int num_channels = 0;
  1185		int ret;
  1186	
  1187		if (fwnode_property_match_string(fwnode, "compatible",
  1188						 "xlnx,zynqmp-ams-ps") == 0) {
  1189			ams->ps_base = fwnode_iomap(fwnode, 0);
  1190			if (!ams->ps_base)
  1191				return -ENXIO;
> 1192			ret = devm_add_action_or_reset(dev, ams_iounmap, ams->ps_base);
  1193			if (ret < 0)
  1194				return ret;
  1195	
  1196			/* add PS channels to iio device channels */
  1197			memcpy(channels, ams_ps_channels,
  1198			       sizeof(ams_ps_channels));
  1199			num_channels += ARRAY_SIZE(ams_ps_channels);
  1200		} else if (fwnode_property_match_string(fwnode, "compatible",
  1201							"xlnx,zynqmp-ams-pl") == 0) {
  1202			ams->pl_base = fwnode_iomap(fwnode, 0);
  1203			if (!ams->pl_base)
  1204				return -ENXIO;
  1205	
> 1206			ret = devm_add_action_or_reset(dev, ams_iounmap, ams->pl_base);
  1207			if (ret < 0)
  1208				return ret;
  1209	
  1210			/* Copy only first 10 fix channels */
  1211			memcpy(channels, ams_pl_channels,
  1212			       AMS_PL_MAX_FIXED_CHANNEL * sizeof(*channels));
  1213			num_channels += AMS_PL_MAX_FIXED_CHANNEL;
  1214	
  1215			num_channels = ams_get_ext_chan(fwnode, channels,
  1216							num_channels);
  1217		} else if (fwnode_property_match_string(fwnode, "compatible",
  1218							"xlnx,zynqmp-ams") == 0) {
  1219			/* add AMS channels to iio device channels */
  1220			memcpy(channels, ams_ctrl_channels,
  1221			       sizeof(ams_ctrl_channels));
  1222			num_channels += ARRAY_SIZE(ams_ctrl_channels);
  1223		} else {
  1224			return -EINVAL;
  1225		}
  1226	
  1227		return num_channels;
  1228	}
  1229	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

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

* Re: [PATCH v11 3/5] iio: adc: Add Xilinx AMS driver
  2021-11-24 22:54 ` [PATCH v11 3/5] iio: adc: Add Xilinx AMS driver Anand Ashok Dumbre
  2021-11-25 12:14   ` Andy Shevchenko
  2021-11-27  2:43   ` kernel test robot
@ 2021-11-27  5:16   ` kernel test robot
  2 siblings, 0 replies; 19+ messages in thread
From: kernel test robot @ 2021-11-27  5:16 UTC (permalink / raw)
  To: Anand Ashok Dumbre, linux-kernel, jic23, lars, linux-iio, git,
	michal.simek, gregkh, rafael, linux-acpi, andriy.shevchenko
  Cc: llvm, kbuild-all

Hi Anand,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on jic23-iio/togreg]
[also build test WARNING on linux/master linus/master v5.16-rc2 next-20211126]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Anand-Ashok-Dumbre/Add-Xilinx-AMS-Driver/20211125-065614
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
config: hexagon-randconfig-r014-20211126 (https://download.01.org/0day-ci/archive/20211127/202111271321.WU0i14KG-lkp@intel.com/config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 5162b558d8c0b542e752b037e72a69d5fd51eb1e)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/9b07fe52c07c2e9f6eccd2f2050f69558904ed64
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Anand-Ashok-Dumbre/Add-Xilinx-AMS-Driver/20211125-065614
        git checkout 9b07fe52c07c2e9f6eccd2f2050f69558904ed64
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash drivers/iio/adc/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/iio/adc/xilinx-ams.c:322:23: warning: shift count >= width of type [-Wshift-count-overflow]
           regval = ~(FIELD_GET(AMS_ISR1_INTR_MASK, ams->intr_mask));
                      ~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/iio/adc/xilinx-ams.c:131:29: note: expanded from macro 'AMS_ISR1_INTR_MASK'
   #define AMS_ISR1_INTR_MASK              GENMASK(63, 32)
                                           ^
   include/linux/bits.h:38:31: note: expanded from macro 'GENMASK'
           (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))
                                        ^
   include/linux/bits.h:35:22: note: expanded from macro '__GENMASK'
           (((~UL(0)) - (UL(1) << (l)) + 1) & \
                               ^
   note: (skipping 3 expansions in backtrace; use -fmacro-backtrace-limit=0 to see all)
   include/linux/compiler_types.h:335:22: note: expanded from macro 'compiletime_assert'
           _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
           ~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler_types.h:323:23: note: expanded from macro '_compiletime_assert'
           __compiletime_assert(condition, msg, prefix, suffix)
           ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler_types.h:315:9: note: expanded from macro '__compiletime_assert'
                   if (!(condition))                                       \
                         ^~~~~~~~~
>> drivers/iio/adc/xilinx-ams.c:322:23: warning: shift count is negative [-Wshift-count-negative]
           regval = ~(FIELD_GET(AMS_ISR1_INTR_MASK, ams->intr_mask));
                      ~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/iio/adc/xilinx-ams.c:131:29: note: expanded from macro 'AMS_ISR1_INTR_MASK'
   #define AMS_ISR1_INTR_MASK              GENMASK(63, 32)
                                           ^
   include/linux/bits.h:38:31: note: expanded from macro 'GENMASK'
           (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))
                                        ^
   include/linux/bits.h:36:11: note: expanded from macro '__GENMASK'
            (~UL(0) >> (BITS_PER_LONG - 1 - (h))))
                    ^
   note: (skipping 3 expansions in backtrace; use -fmacro-backtrace-limit=0 to see all)
   include/linux/compiler_types.h:335:22: note: expanded from macro 'compiletime_assert'
           _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
           ~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler_types.h:323:23: note: expanded from macro '_compiletime_assert'
           __compiletime_assert(condition, msg, prefix, suffix)
           ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler_types.h:315:9: note: expanded from macro '__compiletime_assert'
                   if (!(condition))                                       \
                         ^~~~~~~~~
>> drivers/iio/adc/xilinx-ams.c:322:23: warning: shift count >= width of type [-Wshift-count-overflow]
           regval = ~(FIELD_GET(AMS_ISR1_INTR_MASK, ams->intr_mask));
                      ~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/iio/adc/xilinx-ams.c:131:29: note: expanded from macro 'AMS_ISR1_INTR_MASK'
   #define AMS_ISR1_INTR_MASK              GENMASK(63, 32)
                                           ^
   include/linux/bits.h:38:31: note: expanded from macro 'GENMASK'
           (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))
                                        ^
   include/linux/bits.h:35:22: note: expanded from macro '__GENMASK'
           (((~UL(0)) - (UL(1) << (l)) + 1) & \
                               ^
   note: (skipping 3 expansions in backtrace; use -fmacro-backtrace-limit=0 to see all)
   include/linux/compiler_types.h:335:22: note: expanded from macro 'compiletime_assert'
           _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
           ~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler_types.h:323:23: note: expanded from macro '_compiletime_assert'
           __compiletime_assert(condition, msg, prefix, suffix)
           ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler_types.h:315:9: note: expanded from macro '__compiletime_assert'
                   if (!(condition))                                       \
                         ^~~~~~~~~
>> drivers/iio/adc/xilinx-ams.c:322:23: warning: shift count is negative [-Wshift-count-negative]
           regval = ~(FIELD_GET(AMS_ISR1_INTR_MASK, ams->intr_mask));
                      ~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/iio/adc/xilinx-ams.c:131:29: note: expanded from macro 'AMS_ISR1_INTR_MASK'
   #define AMS_ISR1_INTR_MASK              GENMASK(63, 32)
                                           ^
   include/linux/bits.h:38:31: note: expanded from macro 'GENMASK'
           (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))
                                        ^
   include/linux/bits.h:36:11: note: expanded from macro '__GENMASK'
            (~UL(0) >> (BITS_PER_LONG - 1 - (h))))
                    ^
   note: (skipping 3 expansions in backtrace; use -fmacro-backtrace-limit=0 to see all)
   include/linux/compiler_types.h:335:22: note: expanded from macro 'compiletime_assert'
           _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
           ~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler_types.h:323:23: note: expanded from macro '_compiletime_assert'
           __compiletime_assert(condition, msg, prefix, suffix)
           ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler_types.h:315:9: note: expanded from macro '__compiletime_assert'
                   if (!(condition))                                       \
                         ^~~~~~~~~
>> drivers/iio/adc/xilinx-ams.c:322:23: warning: shift count >= width of type [-Wshift-count-overflow]
           regval = ~(FIELD_GET(AMS_ISR1_INTR_MASK, ams->intr_mask));
                      ~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/iio/adc/xilinx-ams.c:131:29: note: expanded from macro 'AMS_ISR1_INTR_MASK'
   #define AMS_ISR1_INTR_MASK              GENMASK(63, 32)
                                           ^
   include/linux/bits.h:38:31: note: expanded from macro 'GENMASK'
           (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))
                                        ^
   include/linux/bits.h:35:22: note: expanded from macro '__GENMASK'
           (((~UL(0)) - (UL(1) << (l)) + 1) & \
                               ^
   note: (skipping 3 expansions in backtrace; use -fmacro-backtrace-limit=0 to see all)
   include/linux/compiler_types.h:335:22: note: expanded from macro 'compiletime_assert'
           _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
           ~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler_types.h:323:23: note: expanded from macro '_compiletime_assert'
           __compiletime_assert(condition, msg, prefix, suffix)
           ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler_types.h:315:9: note: expanded from macro '__compiletime_assert'
                   if (!(condition))                                       \
                         ^~~~~~~~~
>> drivers/iio/adc/xilinx-ams.c:322:23: warning: shift count is negative [-Wshift-count-negative]
           regval = ~(FIELD_GET(AMS_ISR1_INTR_MASK, ams->intr_mask));
                      ~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/iio/adc/xilinx-ams.c:131:29: note: expanded from macro 'AMS_ISR1_INTR_MASK'
   #define AMS_ISR1_INTR_MASK              GENMASK(63, 32)
                                           ^
   include/linux/bits.h:38:31: note: expanded from macro 'GENMASK'
           (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))
                                        ^
   include/linux/bits.h:36:11: note: expanded from macro '__GENMASK'
            (~UL(0) >> (BITS_PER_LONG - 1 - (h))))
                    ^
   note: (skipping 3 expansions in backtrace; use -fmacro-backtrace-limit=0 to see all)
   include/linux/compiler_types.h:335:22: note: expanded from macro 'compiletime_assert'
           _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
           ~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler_types.h:323:23: note: expanded from macro '_compiletime_assert'
           __compiletime_assert(condition, msg, prefix, suffix)
           ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler_types.h:315:9: note: expanded from macro '__compiletime_assert'
                   if (!(condition))                                       \
                         ^~~~~~~~~
>> drivers/iio/adc/xilinx-ams.c:322:23: warning: shift count >= width of type [-Wshift-count-overflow]
           regval = ~(FIELD_GET(AMS_ISR1_INTR_MASK, ams->intr_mask));
                      ~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/iio/adc/xilinx-ams.c:131:29: note: expanded from macro 'AMS_ISR1_INTR_MASK'
   #define AMS_ISR1_INTR_MASK              GENMASK(63, 32)
                                           ^
   include/linux/bits.h:38:31: note: expanded from macro 'GENMASK'
           (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))
                                        ^
   include/linux/bits.h:35:22: note: expanded from macro '__GENMASK'
           (((~UL(0)) - (UL(1) << (l)) + 1) & \
                               ^
   note: (skipping 4 expansions in backtrace; use -fmacro-backtrace-limit=0 to see all)
   include/linux/compiler_types.h:335:22: note: expanded from macro 'compiletime_assert'
           _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
           ~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler_types.h:323:23: note: expanded from macro '_compiletime_assert'
           __compiletime_assert(condition, msg, prefix, suffix)
           ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler_types.h:315:9: note: expanded from macro '__compiletime_assert'
                   if (!(condition))                                       \
                         ^~~~~~~~~
>> drivers/iio/adc/xilinx-ams.c:322:23: warning: shift count is negative [-Wshift-count-negative]
           regval = ~(FIELD_GET(AMS_ISR1_INTR_MASK, ams->intr_mask));
                      ~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/iio/adc/xilinx-ams.c:131:29: note: expanded from macro 'AMS_ISR1_INTR_MASK'
   #define AMS_ISR1_INTR_MASK              GENMASK(63, 32)
                                           ^
   include/linux/bits.h:38:31: note: expanded from macro 'GENMASK'
           (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))
                                        ^
   include/linux/bits.h:36:11: note: expanded from macro '__GENMASK'
            (~UL(0) >> (BITS_PER_LONG - 1 - (h))))
                    ^
   note: (skipping 4 expansions in backtrace; use -fmacro-backtrace-limit=0 to see all)
   include/linux/compiler_types.h:335:22: note: expanded from macro 'compiletime_assert'
           _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
           ~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler_types.h:323:23: note: expanded from macro '_compiletime_assert'
           __compiletime_assert(condition, msg, prefix, suffix)
           ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler_types.h:315:9: note: expanded from macro '__compiletime_assert'
                   if (!(condition))                                       \
                         ^~~~~~~~~
>> drivers/iio/adc/xilinx-ams.c:322:23: warning: shift count >= width of type [-Wshift-count-overflow]
           regval = ~(FIELD_GET(AMS_ISR1_INTR_MASK, ams->intr_mask));
                      ~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/iio/adc/xilinx-ams.c:131:29: note: expanded from macro 'AMS_ISR1_INTR_MASK'
   #define AMS_ISR1_INTR_MASK              GENMASK(63, 32)
                                           ^
   include/linux/bits.h:38:31: note: expanded from macro 'GENMASK'
           (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))
                                        ^
   include/linux/bits.h:35:22: note: expanded from macro '__GENMASK'
           (((~UL(0)) - (UL(1) << (l)) + 1) & \
                               ^
   note: (skipping 3 expansions in backtrace; use -fmacro-backtrace-limit=0 to see all)
   include/linux/compiler_types.h:335:22: note: expanded from macro 'compiletime_assert'
           _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
           ~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler_types.h:323:23: note: expanded from macro '_compiletime_assert'
           __compiletime_assert(condition, msg, prefix, suffix)
           ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler_types.h:315:9: note: expanded from macro '__compiletime_assert'
                   if (!(condition))                                       \
                         ^~~~~~~~~
>> drivers/iio/adc/xilinx-ams.c:322:23: warning: shift count is negative [-Wshift-count-negative]
           regval = ~(FIELD_GET(AMS_ISR1_INTR_MASK, ams->intr_mask));
                      ~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/iio/adc/xilinx-ams.c:131:29: note: expanded from macro 'AMS_ISR1_INTR_MASK'
   #define AMS_ISR1_INTR_MASK              GENMASK(63, 32)
                                           ^
   include/linux/bits.h:38:31: note: expanded from macro 'GENMASK'
           (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))
                                        ^
   include/linux/bits.h:36:11: note: expanded from macro '__GENMASK'
            (~UL(0) >> (BITS_PER_LONG - 1 - (h))))
                    ^
   note: (skipping 3 expansions in backtrace; use -fmacro-backtrace-limit=0 to see all)
   include/linux/compiler_types.h:335:22: note: expanded from macro 'compiletime_assert'
           _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
           ~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler_types.h:323:23: note: expanded from macro '_compiletime_assert'
           __compiletime_assert(condition, msg, prefix, suffix)
           ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler_types.h:315:9: note: expanded from macro '__compiletime_assert'
                   if (!(condition))                                       \
                         ^~~~~~~~~
>> drivers/iio/adc/xilinx-ams.c:322:23: warning: shift count >= width of type [-Wshift-count-overflow]
           regval = ~(FIELD_GET(AMS_ISR1_INTR_MASK, ams->intr_mask));
                      ~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/iio/adc/xilinx-ams.c:131:29: note: expanded from macro 'AMS_ISR1_INTR_MASK'
   #define AMS_ISR1_INTR_MASK              GENMASK(63, 32)
                                           ^
   include/linux/bits.h:38:31: note: expanded from macro 'GENMASK'
           (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))
                                        ^
   include/linux/bits.h:35:22: note: expanded from macro '__GENMASK'
           (((~UL(0)) - (UL(1) << (l)) + 1) & \
                               ^
   note: (skipping 5 expansions in backtrace; use -fmacro-backtrace-limit=0 to see all)
   include/linux/compiler_types.h:335:22: note: expanded from macro 'compiletime_assert'
           _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
           ~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler_types.h:323:23: note: expanded from macro '_compiletime_assert'
           __compiletime_assert(condition, msg, prefix, suffix)
           ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler_types.h:315:9: note: expanded from macro '__compiletime_assert'
                   if (!(condition))                                       \
                         ^~~~~~~~~
>> drivers/iio/adc/xilinx-ams.c:322:23: warning: shift count is negative [-Wshift-count-negative]
           regval = ~(FIELD_GET(AMS_ISR1_INTR_MASK, ams->intr_mask));
                      ~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/iio/adc/xilinx-ams.c:131:29: note: expanded from macro 'AMS_ISR1_INTR_MASK'
   #define AMS_ISR1_INTR_MASK              GENMASK(63, 32)
                                           ^
   include/linux/bits.h:38:31: note: expanded from macro 'GENMASK'
           (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))
                                        ^
   include/linux/bits.h:36:11: note: expanded from macro '__GENMASK'
            (~UL(0) >> (BITS_PER_LONG - 1 - (h))))
                    ^
   note: (skipping 5 expansions in backtrace; use -fmacro-backtrace-limit=0 to see all)
   include/linux/compiler_types.h:335:22: note: expanded from macro 'compiletime_assert'
           _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
           ~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler_types.h:323:23: note: expanded from macro '_compiletime_assert'
           __compiletime_assert(condition, msg, prefix, suffix)
           ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler_types.h:315:9: note: expanded from macro '__compiletime_assert'
                   if (!(condition))                                       \
                         ^~~~~~~~~
>> drivers/iio/adc/xilinx-ams.c:322:23: warning: shift count >= width of type [-Wshift-count-overflow]
           regval = ~(FIELD_GET(AMS_ISR1_INTR_MASK, ams->intr_mask));
                      ~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/iio/adc/xilinx-ams.c:131:29: note: expanded from macro 'AMS_ISR1_INTR_MASK'
   #define AMS_ISR1_INTR_MASK              GENMASK(63, 32)
                                           ^
   include/linux/bits.h:38:31: note: expanded from macro 'GENMASK'
           (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))
                                        ^
   include/linux/bits.h:35:22: note: expanded from macro '__GENMASK'
           (((~UL(0)) - (UL(1) << (l)) + 1) & \
                               ^
   note: (skipping 6 expansions in backtrace; use -fmacro-backtrace-limit=0 to see all)
   include/linux/compiler_types.h:335:22: note: expanded from macro 'compiletime_assert'
           _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
           ~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler_types.h:323:23: note: expanded from macro '_compiletime_assert'
           __compiletime_assert(condition, msg, prefix, suffix)
           ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler_types.h:315:9: note: expanded from macro '__compiletime_assert'
                   if (!(condition))                                       \
                         ^~~~~~~~~
>> drivers/iio/adc/xilinx-ams.c:322:23: warning: shift count is negative [-Wshift-count-negative]
           regval = ~(FIELD_GET(AMS_ISR1_INTR_MASK, ams->intr_mask));
                      ~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/iio/adc/xilinx-ams.c:131:29: note: expanded from macro 'AMS_ISR1_INTR_MASK'
   #define AMS_ISR1_INTR_MASK              GENMASK(63, 32)
                                           ^
   include/linux/bits.h:38:31: note: expanded from macro 'GENMASK'
           (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))
                                        ^
   include/linux/bits.h:36:11: note: expanded from macro '__GENMASK'
            (~UL(0) >> (BITS_PER_LONG - 1 - (h))))
                    ^
   note: (skipping 6 expansions in backtrace; use -fmacro-backtrace-limit=0 to see all)
   include/linux/compiler_types.h:335:22: note: expanded from macro 'compiletime_assert'
           _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
           ~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler_types.h:323:23: note: expanded from macro '_compiletime_assert'
           __compiletime_assert(condition, msg, prefix, suffix)
           ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler_types.h:315:9: note: expanded from macro '__compiletime_assert'
                   if (!(condition))                                       \
                         ^~~~~~~~~
>> drivers/iio/adc/xilinx-ams.c:322:23: warning: shift count >= width of type [-Wshift-count-overflow]
           regval = ~(FIELD_GET(AMS_ISR1_INTR_MASK, ams->intr_mask));
                      ~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/iio/adc/xilinx-ams.c:131:29: note: expanded from macro 'AMS_ISR1_INTR_MASK'
   #define AMS_ISR1_INTR_MASK              GENMASK(63, 32)
                                           ^
   include/linux/bits.h:38:31: note: expanded from macro 'GENMASK'
           (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))
                                        ^
   include/linux/bits.h:35:22: note: expanded from macro '__GENMASK'
           (((~UL(0)) - (UL(1) << (l)) + 1) & \
                               ^
   note: (skipping 5 expansions in backtrace; use -fmacro-backtrace-limit=0 to see all)
   include/linux/compiler_types.h:335:22: note: expanded from macro 'compiletime_assert'
           _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
           ~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler_types.h:323:23: note: expanded from macro '_compiletime_assert'
           __compiletime_assert(condition, msg, prefix, suffix)
           ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler_types.h:315:9: note: expanded from macro '__compiletime_assert'
                   if (!(condition))                                       \
                         ^~~~~~~~~
>> drivers/iio/adc/xilinx-ams.c:322:23: warning: shift count is negative [-Wshift-count-negative]
           regval = ~(FIELD_GET(AMS_ISR1_INTR_MASK, ams->intr_mask));
                      ~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/iio/adc/xilinx-ams.c:131:29: note: expanded from macro 'AMS_ISR1_INTR_MASK'
   #define AMS_ISR1_INTR_MASK              GENMASK(63, 32)
                                           ^
   include/linux/bits.h:38:31: note: expanded from macro 'GENMASK'
           (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))
                                        ^
   include/linux/bits.h:36:11: note: expanded from macro '__GENMASK'
            (~UL(0) >> (BITS_PER_LONG - 1 - (h))))
                    ^
   note: (skipping 5 expansions in backtrace; use -fmacro-backtrace-limit=0 to see all)
   include/linux/compiler_types.h:335:22: note: expanded from macro 'compiletime_assert'
           _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
           ~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler_types.h:323:23: note: expanded from macro '_compiletime_assert'
           __compiletime_assert(condition, msg, prefix, suffix)
           ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler_types.h:315:9: note: expanded from macro '__compiletime_assert'
                   if (!(condition))                                       \
                         ^~~~~~~~~
>> drivers/iio/adc/xilinx-ams.c:322:23: warning: shift count >= width of type [-Wshift-count-overflow]
           regval = ~(FIELD_GET(AMS_ISR1_INTR_MASK, ams->intr_mask));
                      ~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/iio/adc/xilinx-ams.c:131:29: note: expanded from macro 'AMS_ISR1_INTR_MASK'
   #define AMS_ISR1_INTR_MASK              GENMASK(63, 32)
                                           ^
   include/linux/bits.h:38:31: note: expanded from macro 'GENMASK'
           (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))
                                        ^
   include/linux/bits.h:35:22: note: expanded from macro '__GENMASK'
           (((~UL(0)) - (UL(1) << (l)) + 1) & \
                               ^
   note: (skipping 6 expansions in backtrace; use -fmacro-backtrace-limit=0 to see all)
   include/linux/compiler_types.h:335:22: note: expanded from macro 'compiletime_assert'
           _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
           ~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler_types.h:323:23: note: expanded from macro '_compiletime_assert'
           __compiletime_assert(condition, msg, prefix, suffix)
           ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler_types.h:315:9: note: expanded from macro '__compiletime_assert'
                   if (!(condition))                                       \
                         ^~~~~~~~~
>> drivers/iio/adc/xilinx-ams.c:322:23: warning: shift count is negative [-Wshift-count-negative]
           regval = ~(FIELD_GET(AMS_ISR1_INTR_MASK, ams->intr_mask));
                      ~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/iio/adc/xilinx-ams.c:131:29: note: expanded from macro 'AMS_ISR1_INTR_MASK'
   #define AMS_ISR1_INTR_MASK              GENMASK(63, 32)
                                           ^
   include/linux/bits.h:38:31: note: expanded from macro 'GENMASK'
           (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))
                                        ^
   include/linux/bits.h:36:11: note: expanded from macro '__GENMASK'
            (~UL(0) >> (BITS_PER_LONG - 1 - (h))))
                    ^
   note: (skipping 6 expansions in backtrace; use -fmacro-backtrace-limit=0 to see all)
   include/linux/compiler_types.h:335:22: note: expanded from macro 'compiletime_assert'
           _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
           ~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler_types.h:323:23: note: expanded from macro '_compiletime_assert'
           __compiletime_assert(condition, msg, prefix, suffix)
           ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler_types.h:315:9: note: expanded from macro '__compiletime_assert'
                   if (!(condition))                                       \
                         ^~~~~~~~~
>> drivers/iio/adc/xilinx-ams.c:322:23: warning: shift count >= width of type [-Wshift-count-overflow]
           regval = ~(FIELD_GET(AMS_ISR1_INTR_MASK, ams->intr_mask));
                      ~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/iio/adc/xilinx-ams.c:131:29: note: expanded from macro 'AMS_ISR1_INTR_MASK'
   #define AMS_ISR1_INTR_MASK              GENMASK(63, 32)
                                           ^
   include/linux/bits.h:38:31: note: expanded from macro 'GENMASK'
           (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))
                                        ^
   include/linux/bits.h:35:22: note: expanded from macro '__GENMASK'
           (((~UL(0)) - (UL(1) << (l)) + 1) & \
                               ^
   include/linux/bitfield.h:109:30: note: expanded from macro 'FIELD_GET'
                   (typeof(_mask))(((_reg) & (_mask)) >> __bf_shf(_mask)); \
                                              ^~~~~
>> drivers/iio/adc/xilinx-ams.c:322:23: warning: shift count is negative [-Wshift-count-negative]
           regval = ~(FIELD_GET(AMS_ISR1_INTR_MASK, ams->intr_mask));
                      ~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/iio/adc/xilinx-ams.c:131:29: note: expanded from macro 'AMS_ISR1_INTR_MASK'
   #define AMS_ISR1_INTR_MASK              GENMASK(63, 32)
                                           ^
   include/linux/bits.h:38:31: note: expanded from macro 'GENMASK'
           (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))
                                        ^
   include/linux/bits.h:36:11: note: expanded from macro '__GENMASK'
            (~UL(0) >> (BITS_PER_LONG - 1 - (h))))
                    ^
   include/linux/bitfield.h:109:30: note: expanded from macro 'FIELD_GET'
                   (typeof(_mask))(((_reg) & (_mask)) >> __bf_shf(_mask)); \
                                              ^~~~~
   drivers/iio/adc/xilinx-ams.c:322:23: warning: shift count >= width of type [-Wshift-count-overflow]
           regval = ~(FIELD_GET(AMS_ISR1_INTR_MASK, ams->intr_mask));
                      ~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/iio/adc/xilinx-ams.c:131:29: note: expanded from macro 'AMS_ISR1_INTR_MASK'
   #define AMS_ISR1_INTR_MASK              GENMASK(63, 32)
                                           ^
   include/linux/bits.h:38:31: note: expanded from macro 'GENMASK'
           (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))
                                        ^
   include/linux/bits.h:35:22: note: expanded from macro '__GENMASK'
           (((~UL(0)) - (UL(1) << (l)) + 1) & \
                               ^
   include/linux/bitfield.h:109:50: note: expanded from macro 'FIELD_GET'
                   (typeof(_mask))(((_reg) & (_mask)) >> __bf_shf(_mask)); \
                                                         ~~~~~~~~~^~~~~~
   include/linux/bitfield.h:42:38: note: expanded from macro '__bf_shf'
   #define __bf_shf(x) (__builtin_ffsll(x) - 1)
                                        ^
   drivers/iio/adc/xilinx-ams.c:322:23: warning: shift count is negative [-Wshift-count-negative]
           regval = ~(FIELD_GET(AMS_ISR1_INTR_MASK, ams->intr_mask));
                      ~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/iio/adc/xilinx-ams.c:131:29: note: expanded from macro 'AMS_ISR1_INTR_MASK'
   #define AMS_ISR1_INTR_MASK              GENMASK(63, 32)
                                           ^
   include/linux/bits.h:38:31: note: expanded from macro 'GENMASK'
           (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))
                                        ^
   include/linux/bits.h:36:11: note: expanded from macro '__GENMASK'
            (~UL(0) >> (BITS_PER_LONG - 1 - (h))))
                    ^
   include/linux/bitfield.h:109:50: note: expanded from macro 'FIELD_GET'
                   (typeof(_mask))(((_reg) & (_mask)) >> __bf_shf(_mask)); \
                                                         ~~~~~~~~~^~~~~~
   include/linux/bitfield.h:42:38: note: expanded from macro '__bf_shf'
   #define __bf_shf(x) (__builtin_ffsll(x) - 1)
                                        ^
   drivers/iio/adc/xilinx-ams.c:328:21: warning: shift count >= width of type [-Wshift-count-overflow]
           regval = FIELD_GET(AMS_ISR1_INTR_MASK, ams->intr_mask);
                    ~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/iio/adc/xilinx-ams.c:131:29: note: expanded from macro 'AMS_ISR1_INTR_MASK'
   #define AMS_ISR1_INTR_MASK              GENMASK(63, 32)
                                           ^
   include/linux/bits.h:38:31: note: expanded from macro 'GENMASK'
           (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))
                                        ^
   include/linux/bits.h:35:22: note: expanded from macro '__GENMASK'
           (((~UL(0)) - (UL(1) << (l)) + 1) & \
                               ^
   note: (skipping 3 expansions in backtrace; use -fmacro-backtrace-limit=0 to see all)
   include/linux/compiler_types.h:335:22: note: expanded from macro 'compiletime_assert'
           _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
           ~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler_types.h:323:23: note: expanded from macro '_compiletime_assert'
           __compiletime_assert(condition, msg, prefix, suffix)
           ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler_types.h:315:9: note: expanded from macro '__compiletime_assert'
                   if (!(condition))                                       \
                         ^~~~~~~~~
   drivers/iio/adc/xilinx-ams.c:328:21: warning: shift count is negative [-Wshift-count-negative]
           regval = FIELD_GET(AMS_ISR1_INTR_MASK, ams->intr_mask);
                    ~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/iio/adc/xilinx-ams.c:131:29: note: expanded from macro 'AMS_ISR1_INTR_MASK'
   #define AMS_ISR1_INTR_MASK              GENMASK(63, 32)
                                           ^
   include/linux/bits.h:38:31: note: expanded from macro 'GENMASK'
           (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))
                                        ^
   include/linux/bits.h:36:11: note: expanded from macro '__GENMASK'
            (~UL(0) >> (BITS_PER_LONG - 1 - (h))))
                    ^
   note: (skipping 3 expansions in backtrace; use -fmacro-backtrace-limit=0 to see all)
   include/linux/compiler_types.h:335:22: note: expanded from macro 'compiletime_assert'
           _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
           ~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler_types.h:323:23: note: expanded from macro '_compiletime_assert'
           __compiletime_assert(condition, msg, prefix, suffix)
           ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler_types.h:315:9: note: expanded from macro '__compiletime_assert'
                   if (!(condition))                                       \
                         ^~~~~~~~~
   drivers/iio/adc/xilinx-ams.c:328:21: warning: shift count >= width of type [-Wshift-count-overflow]
           regval = FIELD_GET(AMS_ISR1_INTR_MASK, ams->intr_mask);
                    ~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/iio/adc/xilinx-ams.c:131:29: note: expanded from macro 'AMS_ISR1_INTR_MASK'
   #define AMS_ISR1_INTR_MASK              GENMASK(63, 32)
                                           ^


vim +322 drivers/iio/adc/xilinx-ams.c

   312	
   313	static void ams_update_intrmask(struct ams *ams, u64 mask, u64 val)
   314	{
   315		u32 regval;
   316	
   317		ams->intr_mask = (ams->intr_mask & ~mask) | (val & mask);
   318	
   319		regval = ~(ams->intr_mask | ams->current_masked_alarm);
   320		writel(regval, ams->base + AMS_IER_0);
   321	
 > 322		regval = ~(FIELD_GET(AMS_ISR1_INTR_MASK, ams->intr_mask));
   323		writel(regval, ams->base + AMS_IER_1);
   324	
   325		regval = ams->intr_mask | ams->current_masked_alarm;
   326		writel(regval, ams->base + AMS_IDR_0);
   327	
   328		regval = FIELD_GET(AMS_ISR1_INTR_MASK, ams->intr_mask);
   329		writel(regval, ams->base + AMS_IDR_1);
   330	}
   331	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

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

* Re: [PATCH v11 1/5] device property: Add fwnode_iomap()
  2021-11-24 22:54 ` [PATCH v11 1/5] device property: Add fwnode_iomap() Anand Ashok Dumbre
  2021-11-25 11:42   ` Andy Shevchenko
@ 2021-11-27 11:54   ` kernel test robot
  1 sibling, 0 replies; 19+ messages in thread
From: kernel test robot @ 2021-11-27 11:54 UTC (permalink / raw)
  To: Anand Ashok Dumbre, linux-kernel, jic23, lars, linux-iio, git,
	michal.simek, gregkh, rafael, linux-acpi, andriy.shevchenko
  Cc: kbuild-all

Hi Anand,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on jic23-iio/togreg]
[also build test ERROR on xilinx-xlnx/master linux/master linus/master v5.16-rc2 next-20211126]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Anand-Ashok-Dumbre/Add-Xilinx-AMS-Driver/20211125-065614
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
config: s390-randconfig-r044-20211126 (https://download.01.org/0day-ci/archive/20211127/202111271953.N5Nm1MIG-lkp@intel.com/config)
compiler: s390-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/b9acba7c202b47024781ea7a6f85b787df15f29b
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Anand-Ashok-Dumbre/Add-Xilinx-AMS-Driver/20211125-065614
        git checkout b9acba7c202b47024781ea7a6f85b787df15f29b
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=s390 SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   s390-linux-ld: kernel/dma/coherent.o: in function `dma_init_coherent_memory':
   coherent.c:(.text+0x6c): undefined reference to `memremap'
   s390-linux-ld: coherent.c:(.text+0x106): undefined reference to `memunmap'
   s390-linux-ld: kernel/dma/coherent.o: in function `dma_declare_coherent_memory':
   coherent.c:(.text+0x436): undefined reference to `memunmap'
   s390-linux-ld: drivers/irqchip/irq-al-fic.o: in function `al_fic_init_dt':
   irq-al-fic.c:(.init.text+0x28): undefined reference to `of_iomap'
   s390-linux-ld: irq-al-fic.c:(.init.text+0x1f4): undefined reference to `iounmap'
   s390-linux-ld: drivers/dma/idma64.o: in function `idma64_platform_probe':
   idma64.c:(.text+0x14cc): undefined reference to `devm_ioremap_resource'
   s390-linux-ld: drivers/tty/ipwireless/main.o: in function `ipwireless_detach':
   main.c:(.text+0x8a): undefined reference to `iounmap'
   s390-linux-ld: main.c:(.text+0xd0): undefined reference to `iounmap'
   s390-linux-ld: drivers/tty/ipwireless/main.o: in function `ipwireless_probe':
   main.c:(.text+0x258): undefined reference to `ioremap'
   s390-linux-ld: main.c:(.text+0x30c): undefined reference to `ioremap'
   s390-linux-ld: main.c:(.text+0x35e): undefined reference to `iounmap'
   s390-linux-ld: main.c:(.text+0x3a0): undefined reference to `iounmap'
   s390-linux-ld: drivers/tty/ipwireless/main.o: in function `config_ipwireless':
   main.c:(.text+0x5a0): undefined reference to `iounmap'
   s390-linux-ld: main.c:(.text+0x5e0): undefined reference to `iounmap'
   s390-linux-ld: drivers/base/property.o: in function `fwnode_iomap':
>> property.c:(.text+0xf36): undefined reference to `of_iomap'
   s390-linux-ld: drivers/net/arcnet/arc-rimi.o: in function `arc_rimi_exit':
   arc-rimi.c:(.exit.text+0x34): undefined reference to `iounmap'
   s390-linux-ld: drivers/net/arcnet/arc-rimi.o: in function `arcrimi_found':
   arc-rimi.c:(.init.text+0xce): undefined reference to `ioremap'
   s390-linux-ld: arc-rimi.c:(.init.text+0x14c): undefined reference to `iounmap'
   s390-linux-ld: arc-rimi.c:(.init.text+0x2b0): undefined reference to `iounmap'
   s390-linux-ld: arc-rimi.c:(.init.text+0x332): undefined reference to `ioremap'
   s390-linux-ld: arc-rimi.c:(.init.text+0x3f4): undefined reference to `iounmap'
   s390-linux-ld: drivers/net/arcnet/arc-rimi.o: in function `check_mirror':
   arc-rimi.c:(.text.unlikely+0x4e): undefined reference to `ioremap'
   s390-linux-ld: arc-rimi.c:(.text.unlikely+0x74): undefined reference to `iounmap'
   s390-linux-ld: drivers/net/ethernet/8390/pcnet_cs.o: in function `pcnet_release':
   pcnet_cs.c:(.text+0x6b8): undefined reference to `iounmap'
   s390-linux-ld: drivers/net/ethernet/8390/pcnet_cs.o: in function `setup_shmem_window':
   pcnet_cs.c:(.text+0xb44): undefined reference to `ioremap'
   s390-linux-ld: pcnet_cs.c:(.text+0xbd8): undefined reference to `iounmap'
   s390-linux-ld: drivers/net/ethernet/8390/pcnet_cs.o: in function `get_hwinfo':
   pcnet_cs.c:(.text+0x1558): undefined reference to `ioremap'
   s390-linux-ld: pcnet_cs.c:(.text+0x1656): undefined reference to `iounmap'
   s390-linux-ld: drivers/net/ethernet/xircom/xirc2ps_cs.o: in function `xirc2ps_release':
   xirc2ps_cs.c:(.text+0x518): undefined reference to `iounmap'
   s390-linux-ld: drivers/net/ethernet/xircom/xirc2ps_cs.o: in function `xirc2ps_config':
   xirc2ps_cs.c:(.text+0x1584): undefined reference to `ioremap'
   s390-linux-ld: drivers/pcmcia/cistpl.o: in function `set_cis_map':
   cistpl.c:(.text+0x5c6): undefined reference to `ioremap'
   s390-linux-ld: cistpl.c:(.text+0x5fc): undefined reference to `iounmap'
   s390-linux-ld: cistpl.c:(.text+0x62a): undefined reference to `iounmap'
   s390-linux-ld: cistpl.c:(.text+0x63c): undefined reference to `ioremap'
   s390-linux-ld: drivers/pcmcia/cistpl.o: in function `release_cis_mem':
   cistpl.c:(.text+0xf80): undefined reference to `iounmap'

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

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

* Re: [PATCH v11 3/5] iio: adc: Add Xilinx AMS driver
  2021-11-27  2:43   ` kernel test robot
@ 2021-11-27 17:50     ` Jonathan Cameron
  0 siblings, 0 replies; 19+ messages in thread
From: Jonathan Cameron @ 2021-11-27 17:50 UTC (permalink / raw)
  To: kernel test robot
  Cc: Anand Ashok Dumbre, linux-kernel, lars, linux-iio, git,
	michal.simek, gregkh, rafael, linux-acpi, andriy.shevchenko,
	kbuild-all

On Sat, 27 Nov 2021 10:43:11 +0800
kernel test robot <lkp@intel.com> wrote:

> Hi Anand,
> 
> Thank you for the patch! Perhaps something to improve:
> 
> [auto build test WARNING on jic23-iio/togreg]
> [also build test WARNING on linux/master linus/master v5.16-rc2 next-20211126]
> [cannot apply to xilinx-xlnx/master]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch]
> 
> url:    https://github.com/0day-ci/linux/commits/Anand-Ashok-Dumbre/Add-Xilinx-AMS-Driver/20211125-065614
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
> config: powerpc64-randconfig-s032-20211126 (https://download.01.org/0day-ci/archive/20211127/202111271041.Cdu0In2E-lkp@intel.com/config)
> compiler: powerpc64-linux-gcc (GCC) 11.2.0
> reproduce:
>         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # apt-get install sparse
>         # sparse version: v0.6.4-dirty
>         # https://github.com/0day-ci/linux/commit/9b07fe52c07c2e9f6eccd2f2050f69558904ed64
>         git remote add linux-review https://github.com/0day-ci/linux
>         git fetch --no-tags linux-review Anand-Ashok-Dumbre/Add-Xilinx-AMS-Driver/20211125-065614
>         git checkout 9b07fe52c07c2e9f6eccd2f2050f69558904ed64
>         # save the config file to linux build tree
>         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=powerpc SHELL=/bin/bash drivers/clk/ drivers/iio/adc/ drivers/pci/controller/
> 
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
> 
> 
> sparse warnings: (new ones prefixed by >>)
> >> drivers/iio/adc/xilinx-ams.c:1175:17: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected void volatile [noderef] __iomem *addr @@     got void *data @@  
>    drivers/iio/adc/xilinx-ams.c:1175:17: sparse:     expected void volatile [noderef] __iomem *addr
>    drivers/iio/adc/xilinx-ams.c:1175:17: sparse:     got void *data
> >> drivers/iio/adc/xilinx-ams.c:1192:69: sparse: sparse: incorrect type in argument 3 (different address spaces) @@     expected void *data @@     got void [noderef] __iomem *ps_base @@  
>    drivers/iio/adc/xilinx-ams.c:1192:69: sparse:     expected void *data
>    drivers/iio/adc/xilinx-ams.c:1192:69: sparse:     got void [noderef] __iomem *ps_base
> >> drivers/iio/adc/xilinx-ams.c:1206:69: sparse: sparse: incorrect type in argument 3 (different address spaces) @@     expected void *data @@     got void [noderef] __iomem *pl_base @@  
>    drivers/iio/adc/xilinx-ams.c:1206:69: sparse:     expected void *data
>    drivers/iio/adc/xilinx-ams.c:1206:69: sparse:     got void [noderef] __iomem *pl_base
> 
> vim +1175 drivers/iio/adc/xilinx-ams.c
> 
>   1172	
>   1173	static void ams_iounmap(void *data)
>   1174	{
> > 1175		iounmap(data);  
I guess you probably already fixed this, but I'd do it by passing the
containing structure in as the *data and then use
ams->pl_base to access it here.

It's a bit more code than we'd normally need but cleaner than force
changes to and from iomem via forcing the pointer conversions.

>   1176	}
>   1177	
>   1178	static int ams_init_module(struct iio_dev *indio_dev,
>   1179				   struct fwnode_handle *fwnode,
>   1180				   struct iio_chan_spec *channels)
>   1181	{
>   1182		struct device *dev = indio_dev->dev.parent;
>   1183		struct ams *ams = iio_priv(indio_dev);
>   1184		int num_channels = 0;
>   1185		int ret;
>   1186	
>   1187		if (fwnode_property_match_string(fwnode, "compatible",
>   1188						 "xlnx,zynqmp-ams-ps") == 0) {
>   1189			ams->ps_base = fwnode_iomap(fwnode, 0);
>   1190			if (!ams->ps_base)
>   1191				return -ENXIO;
> > 1192			ret = devm_add_action_or_reset(dev, ams_iounmap, ams->ps_base);  
>   1193			if (ret < 0)
>   1194				return ret;
>   1195	
>   1196			/* add PS channels to iio device channels */
>   1197			memcpy(channels, ams_ps_channels,
>   1198			       sizeof(ams_ps_channels));
>   1199			num_channels += ARRAY_SIZE(ams_ps_channels);
>   1200		} else if (fwnode_property_match_string(fwnode, "compatible",
>   1201							"xlnx,zynqmp-ams-pl") == 0) {
>   1202			ams->pl_base = fwnode_iomap(fwnode, 0);
>   1203			if (!ams->pl_base)
>   1204				return -ENXIO;
>   1205	
> > 1206			ret = devm_add_action_or_reset(dev, ams_iounmap, ams->pl_base);  
>   1207			if (ret < 0)
>   1208				return ret;
>   1209	
>   1210			/* Copy only first 10 fix channels */
>   1211			memcpy(channels, ams_pl_channels,
>   1212			       AMS_PL_MAX_FIXED_CHANNEL * sizeof(*channels));
>   1213			num_channels += AMS_PL_MAX_FIXED_CHANNEL;
>   1214	
>   1215			num_channels = ams_get_ext_chan(fwnode, channels,
>   1216							num_channels);
>   1217		} else if (fwnode_property_match_string(fwnode, "compatible",
>   1218							"xlnx,zynqmp-ams") == 0) {
>   1219			/* add AMS channels to iio device channels */
>   1220			memcpy(channels, ams_ctrl_channels,
>   1221			       sizeof(ams_ctrl_channels));
>   1222			num_channels += ARRAY_SIZE(ams_ctrl_channels);
>   1223		} else {
>   1224			return -EINVAL;
>   1225		}
>   1226	
>   1227		return num_channels;
>   1228	}
>   1229	
> 
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org


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

* RE: [PATCH v11 1/5] device property: Add fwnode_iomap()
  2021-11-25 11:42   ` Andy Shevchenko
@ 2021-11-30 21:58     ` Anand Ashok Dumbre
  2021-12-02  9:10       ` Anand Ashok Dumbre
  0 siblings, 1 reply; 19+ messages in thread
From: Anand Ashok Dumbre @ 2021-11-30 21:58 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: linux-kernel, jic23, lars, linux-iio, git, Michal Simek, gregkh,
	rafael, linux-acpi, heikki.krogerus

Hi Andy,

Thanks for the review.

> -----Original Message-----
> From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Sent: Thursday 25 November 2021 11:42 AM
> To: Anand Ashok Dumbre <ANANDASH@xilinx.com>
> Cc: linux-kernel@vger.kernel.org; jic23@kernel.org; lars@metafoo.de; linux-
> iio@vger.kernel.org; git <git@xilinx.com>; Michal Simek
> <michals@xilinx.com>; gregkh@linuxfoundation.org; rafael@kernel.org;
> linux-acpi@vger.kernel.org; heikki.krogerus@linux.intel.com
> Subject: Re: [PATCH v11 1/5] device property: Add fwnode_iomap()
> 
> On Wed, Nov 24, 2021 at 10:54:03PM +0000, Anand Ashok Dumbre wrote:
> > This patch introduces a new helper routine - fwnode_iomap(), which
> > allows to map the memory mapped IO for a given device node.
> >
> > This implementation does not cover the ACPI case and may be expanded
> > in the future. The main purpose here is to be able to develop resource
> > provider agnostic drivers.
> 
> ...
> 
> > +void __iomem *fwnode_iomap(struct fwnode_handle *fwnode, int
> index) {
> 
> > +	if (is_of_node(fwnode))
> > +		return of_iomap(to_of_node(fwnode), index);
> 
> It seems this part should be wrapped in some ifdeffery according to kbuild
> bot report.

I see that of_iomap is wrapped in #ifdef
I will fix that and send a new patch.

> 
> > +	return NULL;
> > +}
> > +EXPORT_SYMBOL(fwnode_iomap);
> 
> --
> With Best Regards,
> Andy Shevchenko
> 

Thanks,
Anand 


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

* RE: [PATCH v11 1/5] device property: Add fwnode_iomap()
  2021-11-30 21:58     ` Anand Ashok Dumbre
@ 2021-12-02  9:10       ` Anand Ashok Dumbre
  2021-12-02  9:37         ` Andy Shevchenko
  0 siblings, 1 reply; 19+ messages in thread
From: Anand Ashok Dumbre @ 2021-12-02  9:10 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: linux-kernel, jic23, lars, linux-iio, git, Michal Simek, gregkh,
	rafael, linux-acpi, heikki.krogerus

Hi Andy,

> -----Original Message-----
> From: Anand Ashok Dumbre <ANANDASH@xilinx.com>
> Sent: Tuesday 30 November 2021 9:58 PM
> To: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Cc: linux-kernel@vger.kernel.org; jic23@kernel.org; lars@metafoo.de; linux-
> iio@vger.kernel.org; git <git@xilinx.com>; Michal Simek
> <michals@xilinx.com>; gregkh@linuxfoundation.org; rafael@kernel.org;
> linux-acpi@vger.kernel.org; heikki.krogerus@linux.intel.com
> Subject: RE: [PATCH v11 1/5] device property: Add fwnode_iomap()
> 
> Hi Andy,
> 
> Thanks for the review.
> 
> > -----Original Message-----
> > From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > Sent: Thursday 25 November 2021 11:42 AM
> > To: Anand Ashok Dumbre <ANANDASH@xilinx.com>
> > Cc: linux-kernel@vger.kernel.org; jic23@kernel.org; lars@metafoo.de;
> > linux- iio@vger.kernel.org; git <git@xilinx.com>; Michal Simek
> > <michals@xilinx.com>; gregkh@linuxfoundation.org; rafael@kernel.org;
> > linux-acpi@vger.kernel.org; heikki.krogerus@linux.intel.com
> > Subject: Re: [PATCH v11 1/5] device property: Add fwnode_iomap()
> >
> > On Wed, Nov 24, 2021 at 10:54:03PM +0000, Anand Ashok Dumbre wrote:
> > > This patch introduces a new helper routine - fwnode_iomap(), which
> > > allows to map the memory mapped IO for a given device node.
> > >
> > > This implementation does not cover the ACPI case and may be expanded
> > > in the future. The main purpose here is to be able to develop
> > > resource provider agnostic drivers.
> >
> > ...
> >
> > > +void __iomem *fwnode_iomap(struct fwnode_handle *fwnode, int
> > index) {
> >
> > > +	if (is_of_node(fwnode))
> > > +		return of_iomap(to_of_node(fwnode), index);
> >
> > It seems this part should be wrapped in some ifdeffery according to
> > kbuild bot report.
> 
> I see that of_iomap is wrapped in #ifdef I will fix that and send a new patch.

I am unable to reproduce the conditions for the error shown by the kernel bot.
Not sure if I am doing something wrong. Any help/suggestion would be appreciated to fix this issue.

> 
> >
> > > +	return NULL;
> > > +}
> > > +EXPORT_SYMBOL(fwnode_iomap);
> >
> > --
> > With Best Regards,
> > Andy Shevchenko
> >
> 
> Thanks,
> Anand

Thanks and Regards,
Anand

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

* Re: [PATCH v11 1/5] device property: Add fwnode_iomap()
  2021-12-02  9:10       ` Anand Ashok Dumbre
@ 2021-12-02  9:37         ` Andy Shevchenko
  2021-12-02 11:46           ` Anand Ashok Dumbre
  0 siblings, 1 reply; 19+ messages in thread
From: Andy Shevchenko @ 2021-12-02  9:37 UTC (permalink / raw)
  To: Anand Ashok Dumbre
  Cc: linux-kernel, jic23, lars, linux-iio, git, Michal Simek, gregkh,
	rafael, linux-acpi, heikki.krogerus

On Thu, Dec 02, 2021 at 09:10:02AM +0000, Anand Ashok Dumbre wrote:
> > From: Anand Ashok Dumbre <ANANDASH@xilinx.com>
> > Sent: Tuesday 30 November 2021 9:58 PM
> > > From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > > Sent: Thursday 25 November 2021 11:42 AM
> > > On Wed, Nov 24, 2021 at 10:54:03PM +0000, Anand Ashok Dumbre wrote:

...

> > > > +void __iomem *fwnode_iomap(struct fwnode_handle *fwnode, int
> > > index) {
> > >
> > > > +	if (is_of_node(fwnode))
> > > > +		return of_iomap(to_of_node(fwnode), index);
> > >
> > > It seems this part should be wrapped in some ifdeffery according to
> > > kbuild bot report.
> > 
> > I see that of_iomap is wrapped in #ifdef I will fix that and send a new patch.
> 
> I am unable to reproduce the conditions for the error shown by the kernel bot.
> Not sure if I am doing something wrong. Any help/suggestion would be appreciated to fix this issue.

Kbuild bot gives you a config file and command line with which it tried to
build. It's quite rare that it gives you false positives (and here it's not
the case, because you need to have ifdeffery like other APIs in this category
have).

> > > > +	return NULL;
> > > > +}


-- 
With Best Regards,
Andy Shevchenko



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

* RE: [PATCH v11 1/5] device property: Add fwnode_iomap()
  2021-12-02  9:37         ` Andy Shevchenko
@ 2021-12-02 11:46           ` Anand Ashok Dumbre
  2021-12-02 12:34             ` Anand Ashok Dumbre
  0 siblings, 1 reply; 19+ messages in thread
From: Anand Ashok Dumbre @ 2021-12-02 11:46 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: linux-kernel, jic23, lars, linux-iio, git, Michal Simek, gregkh,
	rafael, linux-acpi, heikki.krogerus

> ...
> 
> > > > > +void __iomem *fwnode_iomap(struct fwnode_handle *fwnode, int
> > > > index) {
> > > >
> > > > > +	if (is_of_node(fwnode))
> > > > > +		return of_iomap(to_of_node(fwnode), index);
> > > >
> > > > It seems this part should be wrapped in some ifdeffery according
> > > > to kbuild bot report.
> > >
> > > I see that of_iomap is wrapped in #ifdef I will fix that and send a new
> patch.
> >
> > I am unable to reproduce the conditions for the error shown by the kernel
> bot.
> > Not sure if I am doing something wrong. Any help/suggestion would be
> appreciated to fix this issue.
> 
> Kbuild bot gives you a config file and command line with which it tried to
> build. It's quite rare that it gives you false positives (and here it's not the
> case, because you need to have ifdeffery like other APIs in this category
> have).
> 

The problem is at the config file itself. I am unable to point to compiler correctly while running,
make ARCH=s390 test_defconfig
s390-linux-gcc: unknown compiler
scripts/Kconfig.include:44: Sorry, this compiler is not supported.
scripts/kconfig/Makefile:94: recipe for target 'test_defconfig' failed
make[1]: *** [test_defconfig] Error 1
Makefile:619: recipe for target 'test_defconfig' failed
make: *** [test_defconfig] Error 2

I have added the compiler binaries to the patch and set CROSS_COMPILE=s390-linux-

> > > > > +	return NULL;
> > > > > +}
> 
> 
> --
> With Best Regards,
> Andy Shevchenko
> 	

Thanks,
Anand


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

* RE: [PATCH v11 1/5] device property: Add fwnode_iomap()
  2021-12-02 11:46           ` Anand Ashok Dumbre
@ 2021-12-02 12:34             ` Anand Ashok Dumbre
  0 siblings, 0 replies; 19+ messages in thread
From: Anand Ashok Dumbre @ 2021-12-02 12:34 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: linux-kernel, jic23, lars, linux-iio, git, Michal Simek, gregkh,
	rafael, linux-acpi, heikki.krogerus



> -----Original Message-----
> From: Anand Ashok Dumbre <ANANDASH@xilinx.com>
> Sent: Thursday 2 December 2021 11:47 AM
> To: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Cc: linux-kernel@vger.kernel.org; jic23@kernel.org; lars@metafoo.de; linux-
> iio@vger.kernel.org; git <git@xilinx.com>; Michal Simek
> <michals@xilinx.com>; gregkh@linuxfoundation.org; rafael@kernel.org;
> linux-acpi@vger.kernel.org; heikki.krogerus@linux.intel.com
> Subject: RE: [PATCH v11 1/5] device property: Add fwnode_iomap()
> 
> > ...
> >
> > > > > > +void __iomem *fwnode_iomap(struct fwnode_handle *fwnode,
> int
> > > > > index) {
> > > > >
> > > > > > +	if (is_of_node(fwnode))
> > > > > > +		return of_iomap(to_of_node(fwnode), index);
> > > > >
> > > > > It seems this part should be wrapped in some ifdeffery according
> > > > > to kbuild bot report.
> > > >
> > > > I see that of_iomap is wrapped in #ifdef I will fix that and send
> > > > a new
> > patch.
> > >
> > > I am unable to reproduce the conditions for the error shown by the
> > > kernel
> > bot.
> > > Not sure if I am doing something wrong. Any help/suggestion would be
> > appreciated to fix this issue.
> >
> > Kbuild bot gives you a config file and command line with which it
> > tried to build. It's quite rare that it gives you false positives (and
> > here it's not the case, because you need to have ifdeffery like other
> > APIs in this category have).
> >
> 
> The problem is at the config file itself. I am unable to point to compiler
> correctly while running, make ARCH=s390 test_defconfig
> s390-linux-gcc: unknown compiler
> scripts/Kconfig.include:44: Sorry, this compiler is not supported.
> scripts/kconfig/Makefile:94: recipe for target 'test_defconfig' failed
> make[1]: *** [test_defconfig] Error 1
> Makefile:619: recipe for target 'test_defconfig' failed
> make: *** [test_defconfig] Error 2
> 
> I have added the compiler binaries to the patch and set
> CROSS_COMPILE=s390-linux-

I am able to build now, one of the build options was causing the problems.

> 
> > > > > > +	return NULL;
> > > > > > +}
> >
> >
> > --
> > With Best Regards,
> > Andy Shevchenko
> >
> 
> Thanks,
> Anand


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

* RE: [PATCH v11 3/5] iio: adc: Add Xilinx AMS driver
  2021-11-25 12:14   ` Andy Shevchenko
@ 2021-12-02 16:32     ` Anand Ashok Dumbre
  2021-12-02 16:51       ` Andy Shevchenko
  0 siblings, 1 reply; 19+ messages in thread
From: Anand Ashok Dumbre @ 2021-12-02 16:32 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: linux-kernel, jic23, lars, linux-iio, git, Michal Simek, gregkh,
	rafael, linux-acpi, heikki.krogerus, Manish Narani

Hi Andy,

Thanks for the review.

...
> 
> > +#define AMS_IDR_1			0x02c
> ...
> > +#define AMS_VCC_PSPLL3			0x06C
> ...
> > +#define AMS_VCCBRAM			0x07C
> ...
> > +#define AMS_PSINTFPDDR			0x09C
> ...and so on
> 
> Be consistent with the capitalization in the hex values.

Yes. Will fix all instances in next patch.

> 
> ...
> 
> > +#define AMS_INIT_POLL_TIME		200
> 
> Does it need unit?
> 
> > +#define AMS_SUPPLY_SCALE_1VOLT		1000
> > +#define AMS_SUPPLY_SCALE_3VOLT		3000
> > +#define AMS_SUPPLY_SCALE_6VOLT		6000
> 
> I would rather make units with these:
> 
> #define AMS_SUPPLY_SCALE_1VOLT_mV		1000
> #define AMS_SUPPLY_SCALE_3VOLT_mV		3000
> #define AMS_SUPPLY_SCALE_6VOLT_mV		6000
> 

Will do. 

> ...
> 
> > +#define AMS_PL_AUX_CHAN_VOLTAGE(_auxno) \
> 
> > +	AMS_CHAN_VOLTAGE(PL_SEQ(AMS_SEQ(_auxno)), \
> > +			AMS_REG_VAUX(_auxno), false)
> 
> One line?
> 
> > +#define AMS_CTRL_CHAN_VOLTAGE(_scan_index, _addr) \
> 
> > +
> 	AMS_CHAN_VOLTAGE(PL_SEQ(AMS_SEQ(AMS_SEQ(_scan_index)))
> , \
> > +			_addr, false)
> 
> Ditto.

Will do.

> 
> ...
> 
> > +/**
> > + * struct ams - Driver data for xilinx-ams
> > + * @base: physical base address of device
> > + * @ps_base: physical base address of PS device
> > + * @pl_base: physical base address of PL device
> > + * @clk: clocks associated with the device
> > + * @dev: pointer to device struct
> > + * @lock: to handle multiple user interaction
> > + * @intr_lock: to protect interrupt mask values
> > + * @alarm_mask: alarm configuration
> > + * @current_masked_alarm: currently masked due to alarm
> > + * @intr_mask: interrupt configuration
> > + * @ams_unmask_work: re-enables event once the event condition
> > +disappears
> 
> > + * This structure contains necessary state for Sysmon driver to
> > + operate
> 
> Shouldn't be this "state for Sysmon driver to operate" a summary above?

I don't understand. 

> 
> > + */
> 
> ...
> 
> > +	u32 reg, value;
> > +	u32 expect = AMS_PS_CSTS_PS_READY;
> > +	int ret;
> 
> 	u32 expect = AMS_PS_CSTS_PS_READY;
> 	u32 reg, value;
> 	int ret;
> 
> ...
> 
> > +	u32 reg;
> > +	u32 expect = AMS_ISR1_EOC_MASK;
> > +	int ret;
> 
> Ditto.
> 

Will fix.

> ...
> 
> > +	ret = readl_poll_timeout(ams->base + AMS_ISR_1, reg,
> > +				 (reg & expect), AMS_INIT_POLL_TIME,
> AMS_INIT_TIMEOUT_US);
> 
> Something wrong with line lengths... There is enough space on previous line
> for one parameter.

Accepted.

> 
> > +	if (ret)
> > +		return ret;
> 
> ...
> 
> > +	case IIO_CHAN_INFO_RAW:
> > +		mutex_lock(&ams->lock);
> > +		if (chan->scan_index >= AMS_CTRL_SEQ_BASE) {
> > +			ret = ams_read_vcc_reg(ams, chan->address, val);
> > +			if (ret) {
> 
> > +				mutex_unlock(&ams->lock);
> > +				return ret;
> 
> Can it be
> 				goto out_unlock;
> 
> > +			}
> > +			ams_enable_channel_sequence(indio_dev);
> > +		} else if (chan->scan_index >= AMS_PS_SEQ_MAX)
> > +			*val = readl(ams->pl_base + chan->address);
> > +		else
> > +			*val = readl(ams->ps_base + chan->address);
> 
> 		ret = IIO_VAL_INT;
> out_unlock:
> 
> > +		mutex_unlock(&ams->lock);
> > +
> > +		return IIO_VAL_INT;
> 
> 		mutex_unlock(&ams->lock);
> 		return ret;
> 
> ?

Sure. That looks good.

> 
> Also the question, why mutex only against INFO_RAW case?

All other cases return static values and don't have register configuration involved.

> 
> ...
> 
> > +		switch (chan->type) {
> > +		case IIO_VOLTAGE:
> > +			if (chan->scan_index < AMS_PS_SEQ_MAX) {
> > +				switch (chan->address) {
> > +				case AMS_SUPPLY1:
> > +				case AMS_SUPPLY2:
> > +				case AMS_SUPPLY3:
> > +				case AMS_SUPPLY4:
> > +				case AMS_SUPPLY9:
> > +				case AMS_SUPPLY10:
> > +				case AMS_VCCAMS:
> > +					*val = AMS_SUPPLY_SCALE_3VOLT;
> > +					break;
> > +				case AMS_SUPPLY5:
> > +				case AMS_SUPPLY6:
> > +				case AMS_SUPPLY7:
> > +				case AMS_SUPPLY8:
> > +					*val = AMS_SUPPLY_SCALE_6VOLT;
> > +					break;
> > +				default:
> > +					*val = AMS_SUPPLY_SCALE_1VOLT;
> > +					break;
> > +				}
> > +			} else if (chan->scan_index >= AMS_PS_SEQ_MAX
> &&
> > +				   chan->scan_index < AMS_CTRL_SEQ_BASE)
> {
> > +				switch (chan->address) {
> > +				case AMS_SUPPLY1:
> > +				case AMS_SUPPLY2:
> > +				case AMS_SUPPLY3:
> > +				case AMS_SUPPLY4:
> > +				case AMS_SUPPLY5:
> > +				case AMS_SUPPLY6:
> > +				case AMS_VCCAMS:
> > +				case AMS_VREFP:
> > +				case AMS_VREFN:
> > +					*val = AMS_SUPPLY_SCALE_3VOLT;
> > +					break;
> > +				case AMS_SUPPLY7:
> > +					regval = readl(ams->pl_base +
> AMS_REG_CONFIG4);
> > +					if (FIELD_GET(AMS_VUSER0_MASK,
> regval))
> > +						*val =
> AMS_SUPPLY_SCALE_6VOLT;
> > +					else
> > +						*val =
> AMS_SUPPLY_SCALE_3VOLT;
> > +					break;
> > +				case AMS_SUPPLY8:
> > +					regval = readl(ams->pl_base +
> AMS_REG_CONFIG4);
> > +					if (FIELD_GET(AMS_VUSER1_MASK,
> regval))
> > +						*val =
> AMS_SUPPLY_SCALE_6VOLT;
> > +					else
> > +						*val =
> AMS_SUPPLY_SCALE_3VOLT;
> > +					break;
> > +				case AMS_SUPPLY9:
> > +					regval = readl(ams->pl_base +
> AMS_REG_CONFIG4);
> > +					if (FIELD_GET(AMS_VUSER2_MASK,
> regval))
> > +						*val =
> AMS_SUPPLY_SCALE_6VOLT;
> > +					else
> > +						*val =
> AMS_SUPPLY_SCALE_3VOLT;
> > +					break;
> > +				case AMS_SUPPLY10:
> > +					regval = readl(ams->pl_base +
> AMS_REG_CONFIG4);
> > +					if (FIELD_GET(AMS_VUSER3_MASK,
> regval))
> > +						*val =
> AMS_SUPPLY_SCALE_6VOLT;
> > +					else
> > +						*val =
> AMS_SUPPLY_SCALE_3VOLT;
> > +					break;
> > +				case AMS_VP_VN:
> > +				case AMS_REG_VAUX(0) ...
> AMS_REG_VAUX(15):
> > +					*val = AMS_SUPPLY_SCALE_1VOLT;
> > +					break;
> > +				default:
> > +					*val = AMS_SUPPLY_SCALE_1VOLT;
> > +					break;
> > +				}
> > +			} else {
> > +				switch (chan->address) {
> > +				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;
> > +		}
> 
> Isn't it a bit too looong for a single switch case?

I agree. Will move them to smaller functions.

> 
> ...
> 
> > +/**
> > + * ams_unmask_worker - ams alarm interrupt unmask worker
> 
> > + * @work :		work to be done
> 
> Be consistent with a style on how you describe parameters in the kernel doc.

Will fix it.

> 
> > + * 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.
> > + */
> 
> ...
> 
> > +	fwnode_for_each_child_node(chan_node, child) {
> > +		ret = fwnode_property_read_u32(child, "reg", &reg);
> > +		if (ret || reg > AMS_PL_MAX_EXT_CHANNEL + 30)
> > +			continue;
> > +
> > +		chan = &channels[num_channels];
> > +		ext_chan = reg + AMS_PL_MAX_FIXED_CHANNEL - 30;
> > +		memcpy(chan, &ams_pl_channels[ext_chan],
> sizeof(*channels));
> > +
> > +		if (fwnode_property_read_bool(child, "xlnx,bipolar"))
> > +			chan->scan_type.sign =	's';
> 
> Needless double spacing.
> 

Agreed.

> > +		num_channels++;
> > +	}
> 
> ...
> 
> > +		/* add PS channels to iio device channels */
> > +		memcpy(channels, ams_ps_channels,
> > +		       sizeof(ams_ps_channels));
> 
> One line.
> 
> ...
> 
> > +		/* Copy only first 10 fix channels */
> 
> Be consistent with one line comments (pay attention to the capitalization,
> compare to the above).
> 
> > +		memcpy(channels, ams_pl_channels,
> > +		       AMS_PL_MAX_FIXED_CHANNEL * sizeof(*channels));
> 
> One line?
> 
> ...
> 
> > +		/* add AMS channels to iio device channels */
> > +		memcpy(channels, ams_ctrl_channels,
> > +		       sizeof(ams_ctrl_channels));
> 
> One line.
> 
> ...
> 
> > +	fwnode_for_each_child_node(fwnode, child) {
> > +		if (fwnode_device_is_available(child)) {
> 
> > +			ret = ams_init_module(indio_dev, child,
> > +					      ams_channels + num_channels);
> 
> One line?
> 
> > +			if (ret < 0) {
> > +				fwnode_handle_put(child);
> > +				return ret;
> > +			}
> > +
> > +			num_channels += ret;
> > +		}
> > +	}
> 

Will fix as many one liners as I can see in the code.

> ...
> 
> > +	dev_size = sizeof(*dev_channels) * num_channels;
> 
> Here you need to have an array_size(). Or introduce a
> devm_krealloc_array().

Oh yes, you are right.

> 
> > +	dev_channels = devm_krealloc(dev, ams_channels, dev_size,
> GFP_KERNEL);
> > +	if (!dev_channels)
> > +		ret = -ENOMEM;
> 
> 
> --
> With Best Regards,
> Andy Shevchenko
> 

Thanks,
Anand


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

* Re: [PATCH v11 3/5] iio: adc: Add Xilinx AMS driver
  2021-12-02 16:32     ` Anand Ashok Dumbre
@ 2021-12-02 16:51       ` Andy Shevchenko
  0 siblings, 0 replies; 19+ messages in thread
From: Andy Shevchenko @ 2021-12-02 16:51 UTC (permalink / raw)
  To: Anand Ashok Dumbre
  Cc: linux-kernel, jic23, lars, linux-iio, git, Michal Simek, gregkh,
	rafael, linux-acpi, heikki.krogerus, Manish Narani

On Thu, Dec 02, 2021 at 04:32:33PM +0000, Anand Ashok Dumbre wrote:

...

> > > +/**

> > > + * struct ams - Driver data for xilinx-ams

(1)

> > > + * @base: physical base address of device
> > > + * @ps_base: physical base address of PS device
> > > + * @pl_base: physical base address of PL device
> > > + * @clk: clocks associated with the device
> > > + * @dev: pointer to device struct
> > > + * @lock: to handle multiple user interaction
> > > + * @intr_lock: to protect interrupt mask values
> > > + * @alarm_mask: alarm configuration
> > > + * @current_masked_alarm: currently masked due to alarm
> > > + * @intr_mask: interrupt configuration
> > > + * @ams_unmask_work: re-enables event once the event condition
> > > +disappears
> > 
> > > + * This structure contains necessary state for Sysmon driver to
> > > + operate
> > 
> > Shouldn't be this "state for Sysmon driver to operate" a summary above?
> 
> I don't understand.

(1) is not so informative, this one is better.

> > > + */

-- 
With Best Regards,
Andy Shevchenko



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

end of thread, other threads:[~2021-12-02 16:52 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-24 22:54 [PATCH v11 0/5] Add Xilinx AMS Driver Anand Ashok Dumbre
2021-11-24 22:54 ` [PATCH v11 1/5] device property: Add fwnode_iomap() Anand Ashok Dumbre
2021-11-25 11:42   ` Andy Shevchenko
2021-11-30 21:58     ` Anand Ashok Dumbre
2021-12-02  9:10       ` Anand Ashok Dumbre
2021-12-02  9:37         ` Andy Shevchenko
2021-12-02 11:46           ` Anand Ashok Dumbre
2021-12-02 12:34             ` Anand Ashok Dumbre
2021-11-27 11:54   ` kernel test robot
2021-11-24 22:54 ` [PATCH v11 2/5] arm64: zynqmp: DT: Add Xilinx AMS node Anand Ashok Dumbre
2021-11-24 22:54 ` [PATCH v11 3/5] iio: adc: Add Xilinx AMS driver Anand Ashok Dumbre
2021-11-25 12:14   ` Andy Shevchenko
2021-12-02 16:32     ` Anand Ashok Dumbre
2021-12-02 16:51       ` Andy Shevchenko
2021-11-27  2:43   ` kernel test robot
2021-11-27 17:50     ` Jonathan Cameron
2021-11-27  5:16   ` kernel test robot
2021-11-24 22:54 ` [PATCH v11 4/5] dt-bindings: iio: adc: Add Xilinx AMS binding documentation Anand Ashok Dumbre
2021-11-24 22:54 ` [PATCH v11 5/5] MAINTAINERS: Add maintainer for xilinx-ams Anand Ashok Dumbre

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).