All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/4] Xilinx AMS fixes
@ 2022-01-20  1:02 ` Robert Hancock
  0 siblings, 0 replies; 30+ messages in thread
From: Robert Hancock @ 2022-01-20  1:02 UTC (permalink / raw)
  To: linux-iio
  Cc: robh+dt, michal.simek, anand.ashok.dumbre, jic23, lars,
	manish.narani, linux-arm-kernel, devicetree, Robert Hancock

Various fixes for the Xilinx AMS driver, and addition of the required
references in the ZynqMP device tree files to use it.

Robert Hancock (4):
  arm64: dts: zynqmp: add AMS driver to device tree
  iio: adc: xilinx-ams: Fixed missing PS channels
  iio: adc: xilinx-ams: Fixed wrong sequencer register settings
  iio: adc: xilinx-ams: Fix single channel switching sequence

 .../arm64/boot/dts/xilinx/zynqmp-clk-ccf.dtsi |  4 +++
 arch/arm64/boot/dts/xilinx/zynqmp.dtsi        | 26 +++++++++++++++++++
 drivers/iio/adc/xilinx-ams.c                  | 15 ++++++++---
 3 files changed, 41 insertions(+), 4 deletions(-)

-- 
2.31.1


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

* [PATCH 0/4] Xilinx AMS fixes
@ 2022-01-20  1:02 ` Robert Hancock
  0 siblings, 0 replies; 30+ messages in thread
From: Robert Hancock @ 2022-01-20  1:02 UTC (permalink / raw)
  To: linux-iio
  Cc: robh+dt, michal.simek, anand.ashok.dumbre, jic23, lars,
	manish.narani, linux-arm-kernel, devicetree, Robert Hancock

Various fixes for the Xilinx AMS driver, and addition of the required
references in the ZynqMP device tree files to use it.

Robert Hancock (4):
  arm64: dts: zynqmp: add AMS driver to device tree
  iio: adc: xilinx-ams: Fixed missing PS channels
  iio: adc: xilinx-ams: Fixed wrong sequencer register settings
  iio: adc: xilinx-ams: Fix single channel switching sequence

 .../arm64/boot/dts/xilinx/zynqmp-clk-ccf.dtsi |  4 +++
 arch/arm64/boot/dts/xilinx/zynqmp.dtsi        | 26 +++++++++++++++++++
 drivers/iio/adc/xilinx-ams.c                  | 15 ++++++++---
 3 files changed, 41 insertions(+), 4 deletions(-)

-- 
2.31.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* [PATCH 1/4] arm64: dts: zynqmp: add AMS driver to device tree
  2022-01-20  1:02 ` Robert Hancock
@ 2022-01-20  1:02   ` Robert Hancock
  -1 siblings, 0 replies; 30+ messages in thread
From: Robert Hancock @ 2022-01-20  1:02 UTC (permalink / raw)
  To: linux-iio
  Cc: robh+dt, michal.simek, anand.ashok.dumbre, jic23, lars,
	manish.narani, linux-arm-kernel, devicetree, Robert Hancock

Add an entry to the ZynqMP device tree to support the AMS device which
now has a driver in mainline.

Signed-off-by: Robert Hancock <robert.hancock@calian.com>
---
 .../arm64/boot/dts/xilinx/zynqmp-clk-ccf.dtsi |  4 +++
 arch/arm64/boot/dts/xilinx/zynqmp.dtsi        | 26 +++++++++++++++++++
 2 files changed, 30 insertions(+)

diff --git a/arch/arm64/boot/dts/xilinx/zynqmp-clk-ccf.dtsi b/arch/arm64/boot/dts/xilinx/zynqmp-clk-ccf.dtsi
index 1e0b1bca7c94..108592104a1b 100644
--- a/arch/arm64/boot/dts/xilinx/zynqmp-clk-ccf.dtsi
+++ b/arch/arm64/boot/dts/xilinx/zynqmp-clk-ccf.dtsi
@@ -239,6 +239,10 @@ &lpd_watchdog {
 	clocks = <&zynqmp_clk LPD_WDT>;
 };
 
+&xilinx_ams {
+	clocks = <&zynqmp_clk AMS_REF>;
+};
+
 &zynqmp_dpdma {
 	clocks = <&zynqmp_clk DPDMA_REF>;
 };
diff --git a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
index 74e66443e4ce..d1fe1e5b46c1 100644
--- a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
+++ b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
@@ -878,6 +878,32 @@ lpd_watchdog: watchdog@ff150000 {
 			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 = <0x0 0x400>;
+			};
+
+			ams_pl: ams_pl@400 {
+				compatible = "xlnx,zynqmp-ams-pl";
+				status = "disabled";
+				reg = <0x400 0x400>;
+				#address-cells = <1>;
+				#size-cells = <0>;
+			};
+		};
+
 		zynqmp_dpdma: dma-controller@fd4c0000 {
 			compatible = "xlnx,zynqmp-dpdma";
 			status = "disabled";
-- 
2.31.1


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

* [PATCH 1/4] arm64: dts: zynqmp: add AMS driver to device tree
@ 2022-01-20  1:02   ` Robert Hancock
  0 siblings, 0 replies; 30+ messages in thread
From: Robert Hancock @ 2022-01-20  1:02 UTC (permalink / raw)
  To: linux-iio
  Cc: robh+dt, michal.simek, anand.ashok.dumbre, jic23, lars,
	manish.narani, linux-arm-kernel, devicetree, Robert Hancock

Add an entry to the ZynqMP device tree to support the AMS device which
now has a driver in mainline.

Signed-off-by: Robert Hancock <robert.hancock@calian.com>
---
 .../arm64/boot/dts/xilinx/zynqmp-clk-ccf.dtsi |  4 +++
 arch/arm64/boot/dts/xilinx/zynqmp.dtsi        | 26 +++++++++++++++++++
 2 files changed, 30 insertions(+)

diff --git a/arch/arm64/boot/dts/xilinx/zynqmp-clk-ccf.dtsi b/arch/arm64/boot/dts/xilinx/zynqmp-clk-ccf.dtsi
index 1e0b1bca7c94..108592104a1b 100644
--- a/arch/arm64/boot/dts/xilinx/zynqmp-clk-ccf.dtsi
+++ b/arch/arm64/boot/dts/xilinx/zynqmp-clk-ccf.dtsi
@@ -239,6 +239,10 @@ &lpd_watchdog {
 	clocks = <&zynqmp_clk LPD_WDT>;
 };
 
+&xilinx_ams {
+	clocks = <&zynqmp_clk AMS_REF>;
+};
+
 &zynqmp_dpdma {
 	clocks = <&zynqmp_clk DPDMA_REF>;
 };
diff --git a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
index 74e66443e4ce..d1fe1e5b46c1 100644
--- a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
+++ b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
@@ -878,6 +878,32 @@ lpd_watchdog: watchdog@ff150000 {
 			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 = <0x0 0x400>;
+			};
+
+			ams_pl: ams_pl@400 {
+				compatible = "xlnx,zynqmp-ams-pl";
+				status = "disabled";
+				reg = <0x400 0x400>;
+				#address-cells = <1>;
+				#size-cells = <0>;
+			};
+		};
+
 		zynqmp_dpdma: dma-controller@fd4c0000 {
 			compatible = "xlnx,zynqmp-dpdma";
 			status = "disabled";
-- 
2.31.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* [PATCH 2/4] iio: adc: xilinx-ams: Fixed missing PS channels
  2022-01-20  1:02 ` Robert Hancock
@ 2022-01-20  1:02   ` Robert Hancock
  -1 siblings, 0 replies; 30+ messages in thread
From: Robert Hancock @ 2022-01-20  1:02 UTC (permalink / raw)
  To: linux-iio
  Cc: robh+dt, michal.simek, anand.ashok.dumbre, jic23, lars,
	manish.narani, linux-arm-kernel, devicetree, Robert Hancock

The code forgot to increment num_channels for the PS channel inputs,
resulting in them not being enabled as they should.

Fixes: d5c70627a794 ("iio: adc: Add Xilinx AMS driver")
Signed-off-by: Robert Hancock <robert.hancock@calian.com>
---
 drivers/iio/adc/xilinx-ams.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/iio/adc/xilinx-ams.c b/drivers/iio/adc/xilinx-ams.c
index 8343c5f74121..b93864362dac 100644
--- a/drivers/iio/adc/xilinx-ams.c
+++ b/drivers/iio/adc/xilinx-ams.c
@@ -1224,6 +1224,7 @@ static int ams_init_module(struct iio_dev *indio_dev,
 
 		/* 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);
-- 
2.31.1


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

* [PATCH 2/4] iio: adc: xilinx-ams: Fixed missing PS channels
@ 2022-01-20  1:02   ` Robert Hancock
  0 siblings, 0 replies; 30+ messages in thread
From: Robert Hancock @ 2022-01-20  1:02 UTC (permalink / raw)
  To: linux-iio
  Cc: robh+dt, michal.simek, anand.ashok.dumbre, jic23, lars,
	manish.narani, linux-arm-kernel, devicetree, Robert Hancock

The code forgot to increment num_channels for the PS channel inputs,
resulting in them not being enabled as they should.

Fixes: d5c70627a794 ("iio: adc: Add Xilinx AMS driver")
Signed-off-by: Robert Hancock <robert.hancock@calian.com>
---
 drivers/iio/adc/xilinx-ams.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/iio/adc/xilinx-ams.c b/drivers/iio/adc/xilinx-ams.c
index 8343c5f74121..b93864362dac 100644
--- a/drivers/iio/adc/xilinx-ams.c
+++ b/drivers/iio/adc/xilinx-ams.c
@@ -1224,6 +1224,7 @@ static int ams_init_module(struct iio_dev *indio_dev,
 
 		/* 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);
-- 
2.31.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* [PATCH 3/4] iio: adc: xilinx-ams: Fixed wrong sequencer register settings
  2022-01-20  1:02 ` Robert Hancock
@ 2022-01-20  1:02   ` Robert Hancock
  -1 siblings, 0 replies; 30+ messages in thread
From: Robert Hancock @ 2022-01-20  1:02 UTC (permalink / raw)
  To: linux-iio
  Cc: robh+dt, michal.simek, anand.ashok.dumbre, jic23, lars,
	manish.narani, linux-arm-kernel, devicetree, Robert Hancock

Register settings used for the sequencer configuration register
were incorrect, causing some inputs to not be read properly.

Fixes: d5c70627a794 ("iio: adc: Add Xilinx AMS driver")
Signed-off-by: Robert Hancock <robert.hancock@calian.com>
---
 drivers/iio/adc/xilinx-ams.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/adc/xilinx-ams.c b/drivers/iio/adc/xilinx-ams.c
index b93864362dac..199027c93cdc 100644
--- a/drivers/iio/adc/xilinx-ams.c
+++ b/drivers/iio/adc/xilinx-ams.c
@@ -91,8 +91,8 @@
 
 #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_CONF1_SEQ_CONTINUOUS	FIELD_PREP(AMS_CONF1_SEQ_MASK, 2)
+#define AMS_CONF1_SEQ_SINGLE_CHANNEL	FIELD_PREP(AMS_CONF1_SEQ_MASK, 3)
 
 #define AMS_REG_SEQ0_MASK		GENMASK(15, 0)
 #define AMS_REG_SEQ2_MASK		GENMASK(21, 16)
-- 
2.31.1


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

* [PATCH 3/4] iio: adc: xilinx-ams: Fixed wrong sequencer register settings
@ 2022-01-20  1:02   ` Robert Hancock
  0 siblings, 0 replies; 30+ messages in thread
From: Robert Hancock @ 2022-01-20  1:02 UTC (permalink / raw)
  To: linux-iio
  Cc: robh+dt, michal.simek, anand.ashok.dumbre, jic23, lars,
	manish.narani, linux-arm-kernel, devicetree, Robert Hancock

Register settings used for the sequencer configuration register
were incorrect, causing some inputs to not be read properly.

Fixes: d5c70627a794 ("iio: adc: Add Xilinx AMS driver")
Signed-off-by: Robert Hancock <robert.hancock@calian.com>
---
 drivers/iio/adc/xilinx-ams.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/adc/xilinx-ams.c b/drivers/iio/adc/xilinx-ams.c
index b93864362dac..199027c93cdc 100644
--- a/drivers/iio/adc/xilinx-ams.c
+++ b/drivers/iio/adc/xilinx-ams.c
@@ -91,8 +91,8 @@
 
 #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_CONF1_SEQ_CONTINUOUS	FIELD_PREP(AMS_CONF1_SEQ_MASK, 2)
+#define AMS_CONF1_SEQ_SINGLE_CHANNEL	FIELD_PREP(AMS_CONF1_SEQ_MASK, 3)
 
 #define AMS_REG_SEQ0_MASK		GENMASK(15, 0)
 #define AMS_REG_SEQ2_MASK		GENMASK(21, 16)
-- 
2.31.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* [PATCH 4/4] iio: adc: xilinx-ams: Fix single channel switching sequence
  2022-01-20  1:02 ` Robert Hancock
@ 2022-01-20  1:02   ` Robert Hancock
  -1 siblings, 0 replies; 30+ messages in thread
From: Robert Hancock @ 2022-01-20  1:02 UTC (permalink / raw)
  To: linux-iio
  Cc: robh+dt, michal.simek, anand.ashok.dumbre, jic23, lars,
	manish.narani, linux-arm-kernel, devicetree, Robert Hancock

Some of the AMS channels need to be read by switching into single-channel
mode from the normal polling sequence. There was a logic issue in this
switching code that could cause the first read of these channels to read
back as zero.

It appears that the sequencer should be set back to default mode before
changing the channel selection, and the channel should be set before
switching the sequencer back into single-channel mode.

Also, write 1 to the EOC bit in the status register to clear it before
waiting for it to become set, so that we actually wait for a new
conversion to complete, and don't proceed based on a previous conversion
completing.

Fixes: d5c70627a794 ("iio: adc: Add Xilinx AMS driver")
Signed-off-by: Robert Hancock <robert.hancock@calian.com>
---
 drivers/iio/adc/xilinx-ams.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/adc/xilinx-ams.c b/drivers/iio/adc/xilinx-ams.c
index 199027c93cdc..7bf097fa10cb 100644
--- a/drivers/iio/adc/xilinx-ams.c
+++ b/drivers/iio/adc/xilinx-ams.c
@@ -530,14 +530,18 @@ static int ams_enable_single_channel(struct ams *ams, unsigned int offset)
 		return -EINVAL;
 	}
 
-	/* set single channel, sequencer off mode */
+	/* 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_SINGLE_CHANNEL);
+			  AMS_CONF1_SEQ_DEFAULT);
 
 	/* write the channel number */
 	ams_ps_update_reg(ams, AMS_REG_CONFIG0, AMS_CONF0_CHANNEL_NUM_MASK,
 			  channel_num);
 
+	/* set single channel, sequencer off mode */
+	ams_ps_update_reg(ams, AMS_REG_CONFIG1, AMS_CONF1_SEQ_MASK,
+			  AMS_CONF1_SEQ_SINGLE_CHANNEL);
+
 	return 0;
 }
 
@@ -551,6 +555,8 @@ static int ams_read_vcc_reg(struct ams *ams, unsigned int offset, u32 *data)
 	if (ret)
 		return ret;
 
+	/* clear end-of-conversion flag, wait for next conversion to complete */
+	writel(expect, ams->base + AMS_ISR_1);
 	ret = readl_poll_timeout(ams->base + AMS_ISR_1, reg, (reg & expect),
 				 AMS_INIT_POLL_TIME_US, AMS_INIT_TIMEOUT_US);
 	if (ret)
-- 
2.31.1


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

* [PATCH 4/4] iio: adc: xilinx-ams: Fix single channel switching sequence
@ 2022-01-20  1:02   ` Robert Hancock
  0 siblings, 0 replies; 30+ messages in thread
From: Robert Hancock @ 2022-01-20  1:02 UTC (permalink / raw)
  To: linux-iio
  Cc: robh+dt, michal.simek, anand.ashok.dumbre, jic23, lars,
	manish.narani, linux-arm-kernel, devicetree, Robert Hancock

Some of the AMS channels need to be read by switching into single-channel
mode from the normal polling sequence. There was a logic issue in this
switching code that could cause the first read of these channels to read
back as zero.

It appears that the sequencer should be set back to default mode before
changing the channel selection, and the channel should be set before
switching the sequencer back into single-channel mode.

Also, write 1 to the EOC bit in the status register to clear it before
waiting for it to become set, so that we actually wait for a new
conversion to complete, and don't proceed based on a previous conversion
completing.

Fixes: d5c70627a794 ("iio: adc: Add Xilinx AMS driver")
Signed-off-by: Robert Hancock <robert.hancock@calian.com>
---
 drivers/iio/adc/xilinx-ams.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/adc/xilinx-ams.c b/drivers/iio/adc/xilinx-ams.c
index 199027c93cdc..7bf097fa10cb 100644
--- a/drivers/iio/adc/xilinx-ams.c
+++ b/drivers/iio/adc/xilinx-ams.c
@@ -530,14 +530,18 @@ static int ams_enable_single_channel(struct ams *ams, unsigned int offset)
 		return -EINVAL;
 	}
 
-	/* set single channel, sequencer off mode */
+	/* 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_SINGLE_CHANNEL);
+			  AMS_CONF1_SEQ_DEFAULT);
 
 	/* write the channel number */
 	ams_ps_update_reg(ams, AMS_REG_CONFIG0, AMS_CONF0_CHANNEL_NUM_MASK,
 			  channel_num);
 
+	/* set single channel, sequencer off mode */
+	ams_ps_update_reg(ams, AMS_REG_CONFIG1, AMS_CONF1_SEQ_MASK,
+			  AMS_CONF1_SEQ_SINGLE_CHANNEL);
+
 	return 0;
 }
 
@@ -551,6 +555,8 @@ static int ams_read_vcc_reg(struct ams *ams, unsigned int offset, u32 *data)
 	if (ret)
 		return ret;
 
+	/* clear end-of-conversion flag, wait for next conversion to complete */
+	writel(expect, ams->base + AMS_ISR_1);
 	ret = readl_poll_timeout(ams->base + AMS_ISR_1, reg, (reg & expect),
 				 AMS_INIT_POLL_TIME_US, AMS_INIT_TIMEOUT_US);
 	if (ret)
-- 
2.31.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH 2/4] iio: adc: xilinx-ams: Fixed missing PS channels
  2022-01-20  1:02   ` Robert Hancock
@ 2022-01-20  1:09     ` Robert Hancock
  -1 siblings, 0 replies; 30+ messages in thread
From: Robert Hancock @ 2022-01-20  1:09 UTC (permalink / raw)
  To: linux-iio
  Cc: lars, robh+dt, jic23, devicetree, michal.simek, manish.narani,
	linux-arm-kernel, m.tretter, anand.ashok.dumbre

On Wed, 2022-01-19 at 19:02 -0600, Robert Hancock wrote:
> The code forgot to increment num_channels for the PS channel inputs,
> resulting in them not being enabled as they should.
> 
> Fixes: d5c70627a794 ("iio: adc: Add Xilinx AMS driver")
> Signed-off-by: Robert Hancock <robert.hancock@calian.com>
> ---
>  drivers/iio/adc/xilinx-ams.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/iio/adc/xilinx-ams.c b/drivers/iio/adc/xilinx-ams.c
> index 8343c5f74121..b93864362dac 100644
> --- a/drivers/iio/adc/xilinx-ams.c
> +++ b/drivers/iio/adc/xilinx-ams.c
> @@ -1224,6 +1224,7 @@ static int ams_init_module(struct iio_dev *indio_dev,
>  
>  		/* 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);

Looks like this is the same change just submitted by Michael Tretter ("iio:
adc: xilinx-ams: Fix num_channels for PS channels").

-- 
Robert Hancock
Senior Hardware Designer, Calian Advanced Technologies
www.calian.com

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

* Re: [PATCH 2/4] iio: adc: xilinx-ams: Fixed missing PS channels
@ 2022-01-20  1:09     ` Robert Hancock
  0 siblings, 0 replies; 30+ messages in thread
From: Robert Hancock @ 2022-01-20  1:09 UTC (permalink / raw)
  To: linux-iio
  Cc: lars, robh+dt, jic23, devicetree, michal.simek, manish.narani,
	linux-arm-kernel, m.tretter, anand.ashok.dumbre

On Wed, 2022-01-19 at 19:02 -0600, Robert Hancock wrote:
> The code forgot to increment num_channels for the PS channel inputs,
> resulting in them not being enabled as they should.
> 
> Fixes: d5c70627a794 ("iio: adc: Add Xilinx AMS driver")
> Signed-off-by: Robert Hancock <robert.hancock@calian.com>
> ---
>  drivers/iio/adc/xilinx-ams.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/iio/adc/xilinx-ams.c b/drivers/iio/adc/xilinx-ams.c
> index 8343c5f74121..b93864362dac 100644
> --- a/drivers/iio/adc/xilinx-ams.c
> +++ b/drivers/iio/adc/xilinx-ams.c
> @@ -1224,6 +1224,7 @@ static int ams_init_module(struct iio_dev *indio_dev,
>  
>  		/* 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);

Looks like this is the same change just submitted by Michael Tretter ("iio:
adc: xilinx-ams: Fix num_channels for PS channels").

-- 
Robert Hancock
Senior Hardware Designer, Calian Advanced Technologies
www.calian.com
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH 1/4] arm64: dts: zynqmp: add AMS driver to device tree
  2022-01-20  1:02   ` Robert Hancock
@ 2022-01-25  8:02     ` Michael Tretter
  -1 siblings, 0 replies; 30+ messages in thread
From: Michael Tretter @ 2022-01-25  8:02 UTC (permalink / raw)
  To: Robert Hancock
  Cc: linux-iio, robh+dt, michal.simek, anand.ashok.dumbre, jic23,
	lars, manish.narani, linux-arm-kernel, devicetree, kernel

On Wed, 19 Jan 2022 19:02:43 -0600, Robert Hancock wrote:
> Add an entry to the ZynqMP device tree to support the AMS device which
> now has a driver in mainline.
> 
> Signed-off-by: Robert Hancock <robert.hancock@calian.com>

Reviewed-by: Michael Tretter <m.tretter@pengutronix.de>

> ---
>  .../arm64/boot/dts/xilinx/zynqmp-clk-ccf.dtsi |  4 +++
>  arch/arm64/boot/dts/xilinx/zynqmp.dtsi        | 26 +++++++++++++++++++
>  2 files changed, 30 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/xilinx/zynqmp-clk-ccf.dtsi b/arch/arm64/boot/dts/xilinx/zynqmp-clk-ccf.dtsi
> index 1e0b1bca7c94..108592104a1b 100644
> --- a/arch/arm64/boot/dts/xilinx/zynqmp-clk-ccf.dtsi
> +++ b/arch/arm64/boot/dts/xilinx/zynqmp-clk-ccf.dtsi
> @@ -239,6 +239,10 @@ &lpd_watchdog {
>  	clocks = <&zynqmp_clk LPD_WDT>;
>  };
>  
> +&xilinx_ams {
> +	clocks = <&zynqmp_clk AMS_REF>;
> +};
> +
>  &zynqmp_dpdma {
>  	clocks = <&zynqmp_clk DPDMA_REF>;
>  };
> diff --git a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
> index 74e66443e4ce..d1fe1e5b46c1 100644
> --- a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
> +++ b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
> @@ -878,6 +878,32 @@ lpd_watchdog: watchdog@ff150000 {
>  			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 = <0x0 0x400>;
> +			};
> +
> +			ams_pl: ams_pl@400 {
> +				compatible = "xlnx,zynqmp-ams-pl";
> +				status = "disabled";
> +				reg = <0x400 0x400>;
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +			};
> +		};
> +
>  		zynqmp_dpdma: dma-controller@fd4c0000 {
>  			compatible = "xlnx,zynqmp-dpdma";
>  			status = "disabled";
> -- 
> 2.31.1
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH 1/4] arm64: dts: zynqmp: add AMS driver to device tree
@ 2022-01-25  8:02     ` Michael Tretter
  0 siblings, 0 replies; 30+ messages in thread
From: Michael Tretter @ 2022-01-25  8:02 UTC (permalink / raw)
  To: Robert Hancock
  Cc: linux-iio, robh+dt, michal.simek, anand.ashok.dumbre, jic23,
	lars, manish.narani, linux-arm-kernel, devicetree, kernel

On Wed, 19 Jan 2022 19:02:43 -0600, Robert Hancock wrote:
> Add an entry to the ZynqMP device tree to support the AMS device which
> now has a driver in mainline.
> 
> Signed-off-by: Robert Hancock <robert.hancock@calian.com>

Reviewed-by: Michael Tretter <m.tretter@pengutronix.de>

> ---
>  .../arm64/boot/dts/xilinx/zynqmp-clk-ccf.dtsi |  4 +++
>  arch/arm64/boot/dts/xilinx/zynqmp.dtsi        | 26 +++++++++++++++++++
>  2 files changed, 30 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/xilinx/zynqmp-clk-ccf.dtsi b/arch/arm64/boot/dts/xilinx/zynqmp-clk-ccf.dtsi
> index 1e0b1bca7c94..108592104a1b 100644
> --- a/arch/arm64/boot/dts/xilinx/zynqmp-clk-ccf.dtsi
> +++ b/arch/arm64/boot/dts/xilinx/zynqmp-clk-ccf.dtsi
> @@ -239,6 +239,10 @@ &lpd_watchdog {
>  	clocks = <&zynqmp_clk LPD_WDT>;
>  };
>  
> +&xilinx_ams {
> +	clocks = <&zynqmp_clk AMS_REF>;
> +};
> +
>  &zynqmp_dpdma {
>  	clocks = <&zynqmp_clk DPDMA_REF>;
>  };
> diff --git a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
> index 74e66443e4ce..d1fe1e5b46c1 100644
> --- a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
> +++ b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
> @@ -878,6 +878,32 @@ lpd_watchdog: watchdog@ff150000 {
>  			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 = <0x0 0x400>;
> +			};
> +
> +			ams_pl: ams_pl@400 {
> +				compatible = "xlnx,zynqmp-ams-pl";
> +				status = "disabled";
> +				reg = <0x400 0x400>;
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +			};
> +		};
> +
>  		zynqmp_dpdma: dma-controller@fd4c0000 {
>  			compatible = "xlnx,zynqmp-dpdma";
>  			status = "disabled";
> -- 
> 2.31.1
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

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

* Re: [PATCH 2/4] iio: adc: xilinx-ams: Fixed missing PS channels
  2022-01-20  1:09     ` Robert Hancock
@ 2022-01-25  8:06       ` Michael Tretter
  -1 siblings, 0 replies; 30+ messages in thread
From: Michael Tretter @ 2022-01-25  8:06 UTC (permalink / raw)
  To: Robert Hancock
  Cc: linux-iio, lars, robh+dt, jic23, devicetree, michal.simek,
	manish.narani, linux-arm-kernel, anand.ashok.dumbre, kernel

On Thu, 20 Jan 2022 01:09:39 +0000, Robert Hancock wrote:
> On Wed, 2022-01-19 at 19:02 -0600, Robert Hancock wrote:
> > The code forgot to increment num_channels for the PS channel inputs,
> > resulting in them not being enabled as they should.
> > 
> > Fixes: d5c70627a794 ("iio: adc: Add Xilinx AMS driver")
> > Signed-off-by: Robert Hancock <robert.hancock@calian.com>
> > ---
> >  drivers/iio/adc/xilinx-ams.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/drivers/iio/adc/xilinx-ams.c b/drivers/iio/adc/xilinx-ams.c
> > index 8343c5f74121..b93864362dac 100644
> > --- a/drivers/iio/adc/xilinx-ams.c
> > +++ b/drivers/iio/adc/xilinx-ams.c
> > @@ -1224,6 +1224,7 @@ static int ams_init_module(struct iio_dev *indio_dev,
> >  
> >  		/* 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);
> 
> Looks like this is the same change just submitted by Michael Tretter ("iio:
> adc: xilinx-ams: Fix num_channels for PS channels").

Thanks for pointing me here.

Reviewed-by: Michael Tretter <m.tretter@pengutronix.de>

Michael

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH 2/4] iio: adc: xilinx-ams: Fixed missing PS channels
@ 2022-01-25  8:06       ` Michael Tretter
  0 siblings, 0 replies; 30+ messages in thread
From: Michael Tretter @ 2022-01-25  8:06 UTC (permalink / raw)
  To: Robert Hancock
  Cc: linux-iio, lars, robh+dt, jic23, devicetree, michal.simek,
	manish.narani, linux-arm-kernel, anand.ashok.dumbre, kernel

On Thu, 20 Jan 2022 01:09:39 +0000, Robert Hancock wrote:
> On Wed, 2022-01-19 at 19:02 -0600, Robert Hancock wrote:
> > The code forgot to increment num_channels for the PS channel inputs,
> > resulting in them not being enabled as they should.
> > 
> > Fixes: d5c70627a794 ("iio: adc: Add Xilinx AMS driver")
> > Signed-off-by: Robert Hancock <robert.hancock@calian.com>
> > ---
> >  drivers/iio/adc/xilinx-ams.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/drivers/iio/adc/xilinx-ams.c b/drivers/iio/adc/xilinx-ams.c
> > index 8343c5f74121..b93864362dac 100644
> > --- a/drivers/iio/adc/xilinx-ams.c
> > +++ b/drivers/iio/adc/xilinx-ams.c
> > @@ -1224,6 +1224,7 @@ static int ams_init_module(struct iio_dev *indio_dev,
> >  
> >  		/* 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);
> 
> Looks like this is the same change just submitted by Michael Tretter ("iio:
> adc: xilinx-ams: Fix num_channels for PS channels").

Thanks for pointing me here.

Reviewed-by: Michael Tretter <m.tretter@pengutronix.de>

Michael

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

* Re: [PATCH 3/4] iio: adc: xilinx-ams: Fixed wrong sequencer register settings
  2022-01-20  1:02   ` Robert Hancock
@ 2022-01-25  8:21     ` Michael Tretter
  -1 siblings, 0 replies; 30+ messages in thread
From: Michael Tretter @ 2022-01-25  8:21 UTC (permalink / raw)
  To: Robert Hancock
  Cc: linux-iio, robh+dt, michal.simek, anand.ashok.dumbre, jic23,
	lars, manish.narani, linux-arm-kernel, devicetree, kernel

On Wed, 19 Jan 2022 19:02:45 -0600, Robert Hancock wrote:
> Register settings used for the sequencer configuration register
> were incorrect, causing some inputs to not be read properly.
> 
> Fixes: d5c70627a794 ("iio: adc: Add Xilinx AMS driver")
> Signed-off-by: Robert Hancock <robert.hancock@calian.com>
> ---
>  drivers/iio/adc/xilinx-ams.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/adc/xilinx-ams.c b/drivers/iio/adc/xilinx-ams.c
> index b93864362dac..199027c93cdc 100644
> --- a/drivers/iio/adc/xilinx-ams.c
> +++ b/drivers/iio/adc/xilinx-ams.c
> @@ -91,8 +91,8 @@
>  
>  #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_CONF1_SEQ_CONTINUOUS	FIELD_PREP(AMS_CONF1_SEQ_MASK, 2)
> +#define AMS_CONF1_SEQ_SINGLE_CHANNEL	FIELD_PREP(AMS_CONF1_SEQ_MASK, 3)

The TRM states that Continuous Loop Mode is 2, but Single Pass Sequence Mode
is 1, not 3. Is there a reason, why you need to set both bits?

Michael

>  
>  #define AMS_REG_SEQ0_MASK		GENMASK(15, 0)
>  #define AMS_REG_SEQ2_MASK		GENMASK(21, 16)
> -- 
> 2.31.1
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH 3/4] iio: adc: xilinx-ams: Fixed wrong sequencer register settings
@ 2022-01-25  8:21     ` Michael Tretter
  0 siblings, 0 replies; 30+ messages in thread
From: Michael Tretter @ 2022-01-25  8:21 UTC (permalink / raw)
  To: Robert Hancock
  Cc: linux-iio, robh+dt, michal.simek, anand.ashok.dumbre, jic23,
	lars, manish.narani, linux-arm-kernel, devicetree, kernel

On Wed, 19 Jan 2022 19:02:45 -0600, Robert Hancock wrote:
> Register settings used for the sequencer configuration register
> were incorrect, causing some inputs to not be read properly.
> 
> Fixes: d5c70627a794 ("iio: adc: Add Xilinx AMS driver")
> Signed-off-by: Robert Hancock <robert.hancock@calian.com>
> ---
>  drivers/iio/adc/xilinx-ams.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/adc/xilinx-ams.c b/drivers/iio/adc/xilinx-ams.c
> index b93864362dac..199027c93cdc 100644
> --- a/drivers/iio/adc/xilinx-ams.c
> +++ b/drivers/iio/adc/xilinx-ams.c
> @@ -91,8 +91,8 @@
>  
>  #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_CONF1_SEQ_CONTINUOUS	FIELD_PREP(AMS_CONF1_SEQ_MASK, 2)
> +#define AMS_CONF1_SEQ_SINGLE_CHANNEL	FIELD_PREP(AMS_CONF1_SEQ_MASK, 3)

The TRM states that Continuous Loop Mode is 2, but Single Pass Sequence Mode
is 1, not 3. Is there a reason, why you need to set both bits?

Michael

>  
>  #define AMS_REG_SEQ0_MASK		GENMASK(15, 0)
>  #define AMS_REG_SEQ2_MASK		GENMASK(21, 16)
> -- 
> 2.31.1
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

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

* Re: [PATCH 1/4] arm64: dts: zynqmp: add AMS driver to device tree
  2022-01-20  1:02   ` Robert Hancock
@ 2022-01-25  9:42     ` Michal Simek
  -1 siblings, 0 replies; 30+ messages in thread
From: Michal Simek @ 2022-01-25  9:42 UTC (permalink / raw)
  To: Robert Hancock, linux-iio
  Cc: robh+dt, michal.simek, anand.ashok.dumbre, jic23, lars,
	manish.narani, linux-arm-kernel, devicetree



On 1/20/22 02:02, Robert Hancock wrote:
> Add an entry to the ZynqMP device tree to support the AMS device which
> now has a driver in mainline.
> 
> Signed-off-by: Robert Hancock <robert.hancock@calian.com>
> ---
>   .../arm64/boot/dts/xilinx/zynqmp-clk-ccf.dtsi |  4 +++
>   arch/arm64/boot/dts/xilinx/zynqmp.dtsi        | 26 +++++++++++++++++++
>   2 files changed, 30 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/xilinx/zynqmp-clk-ccf.dtsi b/arch/arm64/boot/dts/xilinx/zynqmp-clk-ccf.dtsi
> index 1e0b1bca7c94..108592104a1b 100644
> --- a/arch/arm64/boot/dts/xilinx/zynqmp-clk-ccf.dtsi
> +++ b/arch/arm64/boot/dts/xilinx/zynqmp-clk-ccf.dtsi
> @@ -239,6 +239,10 @@ &lpd_watchdog {
>   	clocks = <&zynqmp_clk LPD_WDT>;
>   };
>   
> +&xilinx_ams {
> +	clocks = <&zynqmp_clk AMS_REF>;
> +};

Please send this patch out of the series. It should go via soc tree not via iio 
tree.
And unfortunately clock is not listed in DT binding document and needs to be 
added there.
Can you please send that patch too? dtbs_check reports it as issue.

> +
>   &zynqmp_dpdma {
>   	clocks = <&zynqmp_clk DPDMA_REF>;
>   };
> diff --git a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
> index 74e66443e4ce..d1fe1e5b46c1 100644
> --- a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
> +++ b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
> @@ -878,6 +878,32 @@ lpd_watchdog: watchdog@ff150000 {
>   			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 = <0x0 0x400>;
> +			};
> +
> +			ams_pl: ams_pl@400 {
> +				compatible = "xlnx,zynqmp-ams-pl";
> +				status = "disabled";
> +				reg = <0x400 0x400>;
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +			};
> +		};
> +
>   		zynqmp_dpdma: dma-controller@fd4c0000 {
>   			compatible = "xlnx,zynqmp-dpdma";
>   			status = "disabled";

And this second piece is completely aligned with dt binding that's why it is fine.

Thanks,
Michal

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

* Re: [PATCH 1/4] arm64: dts: zynqmp: add AMS driver to device tree
@ 2022-01-25  9:42     ` Michal Simek
  0 siblings, 0 replies; 30+ messages in thread
From: Michal Simek @ 2022-01-25  9:42 UTC (permalink / raw)
  To: Robert Hancock, linux-iio
  Cc: robh+dt, michal.simek, anand.ashok.dumbre, jic23, lars,
	manish.narani, linux-arm-kernel, devicetree



On 1/20/22 02:02, Robert Hancock wrote:
> Add an entry to the ZynqMP device tree to support the AMS device which
> now has a driver in mainline.
> 
> Signed-off-by: Robert Hancock <robert.hancock@calian.com>
> ---
>   .../arm64/boot/dts/xilinx/zynqmp-clk-ccf.dtsi |  4 +++
>   arch/arm64/boot/dts/xilinx/zynqmp.dtsi        | 26 +++++++++++++++++++
>   2 files changed, 30 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/xilinx/zynqmp-clk-ccf.dtsi b/arch/arm64/boot/dts/xilinx/zynqmp-clk-ccf.dtsi
> index 1e0b1bca7c94..108592104a1b 100644
> --- a/arch/arm64/boot/dts/xilinx/zynqmp-clk-ccf.dtsi
> +++ b/arch/arm64/boot/dts/xilinx/zynqmp-clk-ccf.dtsi
> @@ -239,6 +239,10 @@ &lpd_watchdog {
>   	clocks = <&zynqmp_clk LPD_WDT>;
>   };
>   
> +&xilinx_ams {
> +	clocks = <&zynqmp_clk AMS_REF>;
> +};

Please send this patch out of the series. It should go via soc tree not via iio 
tree.
And unfortunately clock is not listed in DT binding document and needs to be 
added there.
Can you please send that patch too? dtbs_check reports it as issue.

> +
>   &zynqmp_dpdma {
>   	clocks = <&zynqmp_clk DPDMA_REF>;
>   };
> diff --git a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
> index 74e66443e4ce..d1fe1e5b46c1 100644
> --- a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
> +++ b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
> @@ -878,6 +878,32 @@ lpd_watchdog: watchdog@ff150000 {
>   			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 = <0x0 0x400>;
> +			};
> +
> +			ams_pl: ams_pl@400 {
> +				compatible = "xlnx,zynqmp-ams-pl";
> +				status = "disabled";
> +				reg = <0x400 0x400>;
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +			};
> +		};
> +
>   		zynqmp_dpdma: dma-controller@fd4c0000 {
>   			compatible = "xlnx,zynqmp-dpdma";
>   			status = "disabled";

And this second piece is completely aligned with dt binding that's why it is fine.

Thanks,
Michal

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH 3/4] iio: adc: xilinx-ams: Fixed wrong sequencer register settings
  2022-01-25  8:21     ` Michael Tretter
@ 2022-01-25 16:15       ` Robert Hancock
  -1 siblings, 0 replies; 30+ messages in thread
From: Robert Hancock @ 2022-01-25 16:15 UTC (permalink / raw)
  To: m.tretter
  Cc: lars, robh+dt, jic23, devicetree, michal.simek, linux-iio,
	manish.narani, linux-arm-kernel, kernel, anand.ashok.dumbre

On Tue, 2022-01-25 at 09:21 +0100, Michael Tretter wrote:
> On Wed, 19 Jan 2022 19:02:45 -0600, Robert Hancock wrote:
> > Register settings used for the sequencer configuration register
> > were incorrect, causing some inputs to not be read properly.
> > 
> > Fixes: d5c70627a794 ("iio: adc: Add Xilinx AMS driver")
> > Signed-off-by: Robert Hancock <robert.hancock@calian.com>
> > ---
> >  drivers/iio/adc/xilinx-ams.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/iio/adc/xilinx-ams.c b/drivers/iio/adc/xilinx-ams.c
> > index b93864362dac..199027c93cdc 100644
> > --- a/drivers/iio/adc/xilinx-ams.c
> > +++ b/drivers/iio/adc/xilinx-ams.c
> > @@ -91,8 +91,8 @@
> >  
> >  #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_CONF1_SEQ_CONTINUOUS	FIELD_PREP(AMS_CONF1_SEQ_MASK, 2)
> > +#define AMS_CONF1_SEQ_SINGLE_CHANNEL	FIELD_PREP(AMS_CONF1_SEQ_MASK,
> > 3)
> 
> The TRM states that Continuous Loop Mode is 2, but Single Pass Sequence Mode
> is 1, not 3. Is there a reason, why you need to set both bits?

Single pass sequence mode (1) just runs the same sequence only once. To read
these values it needs to switch to single channel mode (3).

The register bits are defined in Table 3-8 of 
https://www.xilinx.com/support/documentation/user_guides/ug580-ultrascale-sysmon.pdf
 .

> 
> Michael
> 
> >  
> >  #define AMS_REG_SEQ0_MASK		GENMASK(15, 0)
> >  #define AMS_REG_SEQ2_MASK		GENMASK(21, 16)
> > -- 
> > 2.31.1
> > 
> > 
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel@lists.infradead.org
> > https://urldefense.com/v3/__http://lists.infradead.org/mailman/listinfo/linux-arm-kernel__;!!IOGos0k!yGFEjSC1BL20lwurby914len0HCLXyzarwxKJP9Jx30qv_qrERSkRJUiVo_2MdusMVA$ 
> > 
-- 
Robert Hancock
Senior Hardware Designer, Calian Advanced Technologies
www.calian.com

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

* Re: [PATCH 3/4] iio: adc: xilinx-ams: Fixed wrong sequencer register settings
@ 2022-01-25 16:15       ` Robert Hancock
  0 siblings, 0 replies; 30+ messages in thread
From: Robert Hancock @ 2022-01-25 16:15 UTC (permalink / raw)
  To: m.tretter
  Cc: lars, robh+dt, jic23, devicetree, michal.simek, linux-iio,
	manish.narani, linux-arm-kernel, kernel, anand.ashok.dumbre

On Tue, 2022-01-25 at 09:21 +0100, Michael Tretter wrote:
> On Wed, 19 Jan 2022 19:02:45 -0600, Robert Hancock wrote:
> > Register settings used for the sequencer configuration register
> > were incorrect, causing some inputs to not be read properly.
> > 
> > Fixes: d5c70627a794 ("iio: adc: Add Xilinx AMS driver")
> > Signed-off-by: Robert Hancock <robert.hancock@calian.com>
> > ---
> >  drivers/iio/adc/xilinx-ams.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/iio/adc/xilinx-ams.c b/drivers/iio/adc/xilinx-ams.c
> > index b93864362dac..199027c93cdc 100644
> > --- a/drivers/iio/adc/xilinx-ams.c
> > +++ b/drivers/iio/adc/xilinx-ams.c
> > @@ -91,8 +91,8 @@
> >  
> >  #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_CONF1_SEQ_CONTINUOUS	FIELD_PREP(AMS_CONF1_SEQ_MASK, 2)
> > +#define AMS_CONF1_SEQ_SINGLE_CHANNEL	FIELD_PREP(AMS_CONF1_SEQ_MASK,
> > 3)
> 
> The TRM states that Continuous Loop Mode is 2, but Single Pass Sequence Mode
> is 1, not 3. Is there a reason, why you need to set both bits?

Single pass sequence mode (1) just runs the same sequence only once. To read
these values it needs to switch to single channel mode (3).

The register bits are defined in Table 3-8 of 
https://www.xilinx.com/support/documentation/user_guides/ug580-ultrascale-sysmon.pdf
 .

> 
> Michael
> 
> >  
> >  #define AMS_REG_SEQ0_MASK		GENMASK(15, 0)
> >  #define AMS_REG_SEQ2_MASK		GENMASK(21, 16)
> > -- 
> > 2.31.1
> > 
> > 
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel@lists.infradead.org
> > https://urldefense.com/v3/__http://lists.infradead.org/mailman/listinfo/linux-arm-kernel__;!!IOGos0k!yGFEjSC1BL20lwurby914len0HCLXyzarwxKJP9Jx30qv_qrERSkRJUiVo_2MdusMVA$ 
> > 
-- 
Robert Hancock
Senior Hardware Designer, Calian Advanced Technologies
www.calian.com
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH 3/4] iio: adc: xilinx-ams: Fixed wrong sequencer register settings
  2022-01-25 16:15       ` Robert Hancock
@ 2022-01-26  9:12         ` Michael Tretter
  -1 siblings, 0 replies; 30+ messages in thread
From: Michael Tretter @ 2022-01-26  9:12 UTC (permalink / raw)
  To: Robert Hancock
  Cc: lars, robh+dt, jic23, devicetree, michal.simek, linux-iio,
	manish.narani, linux-arm-kernel, kernel, anand.ashok.dumbre

On Tue, 25 Jan 2022 16:15:05 +0000, Robert Hancock wrote:
> On Tue, 2022-01-25 at 09:21 +0100, Michael Tretter wrote:
> > On Wed, 19 Jan 2022 19:02:45 -0600, Robert Hancock wrote:
> > > Register settings used for the sequencer configuration register
> > > were incorrect, causing some inputs to not be read properly.
> > > 
> > > Fixes: d5c70627a794 ("iio: adc: Add Xilinx AMS driver")
> > > Signed-off-by: Robert Hancock <robert.hancock@calian.com>
> > > ---
> > >  drivers/iio/adc/xilinx-ams.c | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/iio/adc/xilinx-ams.c b/drivers/iio/adc/xilinx-ams.c
> > > index b93864362dac..199027c93cdc 100644
> > > --- a/drivers/iio/adc/xilinx-ams.c
> > > +++ b/drivers/iio/adc/xilinx-ams.c
> > > @@ -91,8 +91,8 @@
> > >  
> > >  #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_CONF1_SEQ_CONTINUOUS	FIELD_PREP(AMS_CONF1_SEQ_MASK, 2)
> > > +#define AMS_CONF1_SEQ_SINGLE_CHANNEL	FIELD_PREP(AMS_CONF1_SEQ_MASK,
> > > 3)
> > 
> > The TRM states that Continuous Loop Mode is 2, but Single Pass Sequence Mode
> > is 1, not 3. Is there a reason, why you need to set both bits?
> 
> Single pass sequence mode (1) just runs the same sequence only once. To read
> these values it needs to switch to single channel mode (3).
> 
> The register bits are defined in Table 3-8 of 
> https://www.xilinx.com/support/documentation/user_guides/ug580-ultrascale-sysmon.pdf
>  .

Thanks for the clarification.

Reviewed-by: Michael Tretter <m.tretter@pengutronix.de>

> 
> > 
> > Michael
> > 
> > >  
> > >  #define AMS_REG_SEQ0_MASK		GENMASK(15, 0)
> > >  #define AMS_REG_SEQ2_MASK		GENMASK(21, 16)
> > > -- 
> > > 2.31.1
> > > 
> > > 
> > > _______________________________________________
> > > linux-arm-kernel mailing list
> > > linux-arm-kernel@lists.infradead.org
> > > https://urldefense.com/v3/__http://lists.infradead.org/mailman/listinfo/linux-arm-kernel__;!!IOGos0k!yGFEjSC1BL20lwurby914len0HCLXyzarwxKJP9Jx30qv_qrERSkRJUiVo_2MdusMVA$ 
> > > 
> -- 
> Robert Hancock
> Senior Hardware Designer, Calian Advanced Technologies
> www.calian.com

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

* Re: [PATCH 3/4] iio: adc: xilinx-ams: Fixed wrong sequencer register settings
@ 2022-01-26  9:12         ` Michael Tretter
  0 siblings, 0 replies; 30+ messages in thread
From: Michael Tretter @ 2022-01-26  9:12 UTC (permalink / raw)
  To: Robert Hancock
  Cc: lars, robh+dt, jic23, devicetree, michal.simek, linux-iio,
	manish.narani, linux-arm-kernel, kernel, anand.ashok.dumbre

On Tue, 25 Jan 2022 16:15:05 +0000, Robert Hancock wrote:
> On Tue, 2022-01-25 at 09:21 +0100, Michael Tretter wrote:
> > On Wed, 19 Jan 2022 19:02:45 -0600, Robert Hancock wrote:
> > > Register settings used for the sequencer configuration register
> > > were incorrect, causing some inputs to not be read properly.
> > > 
> > > Fixes: d5c70627a794 ("iio: adc: Add Xilinx AMS driver")
> > > Signed-off-by: Robert Hancock <robert.hancock@calian.com>
> > > ---
> > >  drivers/iio/adc/xilinx-ams.c | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/iio/adc/xilinx-ams.c b/drivers/iio/adc/xilinx-ams.c
> > > index b93864362dac..199027c93cdc 100644
> > > --- a/drivers/iio/adc/xilinx-ams.c
> > > +++ b/drivers/iio/adc/xilinx-ams.c
> > > @@ -91,8 +91,8 @@
> > >  
> > >  #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_CONF1_SEQ_CONTINUOUS	FIELD_PREP(AMS_CONF1_SEQ_MASK, 2)
> > > +#define AMS_CONF1_SEQ_SINGLE_CHANNEL	FIELD_PREP(AMS_CONF1_SEQ_MASK,
> > > 3)
> > 
> > The TRM states that Continuous Loop Mode is 2, but Single Pass Sequence Mode
> > is 1, not 3. Is there a reason, why you need to set both bits?
> 
> Single pass sequence mode (1) just runs the same sequence only once. To read
> these values it needs to switch to single channel mode (3).
> 
> The register bits are defined in Table 3-8 of 
> https://www.xilinx.com/support/documentation/user_guides/ug580-ultrascale-sysmon.pdf
>  .

Thanks for the clarification.

Reviewed-by: Michael Tretter <m.tretter@pengutronix.de>

> 
> > 
> > Michael
> > 
> > >  
> > >  #define AMS_REG_SEQ0_MASK		GENMASK(15, 0)
> > >  #define AMS_REG_SEQ2_MASK		GENMASK(21, 16)
> > > -- 
> > > 2.31.1
> > > 
> > > 
> > > _______________________________________________
> > > linux-arm-kernel mailing list
> > > linux-arm-kernel@lists.infradead.org
> > > https://urldefense.com/v3/__http://lists.infradead.org/mailman/listinfo/linux-arm-kernel__;!!IOGos0k!yGFEjSC1BL20lwurby914len0HCLXyzarwxKJP9Jx30qv_qrERSkRJUiVo_2MdusMVA$ 
> > > 
> -- 
> Robert Hancock
> Senior Hardware Designer, Calian Advanced Technologies
> www.calian.com

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH 2/4] iio: adc: xilinx-ams: Fixed missing PS channels
  2022-01-25  8:06       ` Michael Tretter
@ 2022-01-30 12:29         ` Jonathan Cameron
  -1 siblings, 0 replies; 30+ messages in thread
From: Jonathan Cameron @ 2022-01-30 12:29 UTC (permalink / raw)
  To: Michael Tretter
  Cc: Robert Hancock, linux-iio, lars, robh+dt, devicetree,
	michal.simek, manish.narani, linux-arm-kernel,
	anand.ashok.dumbre, kernel

On Tue, 25 Jan 2022 09:06:34 +0100
Michael Tretter <m.tretter@pengutronix.de> wrote:

> On Thu, 20 Jan 2022 01:09:39 +0000, Robert Hancock wrote:
> > On Wed, 2022-01-19 at 19:02 -0600, Robert Hancock wrote:  
> > > The code forgot to increment num_channels for the PS channel inputs,
> > > resulting in them not being enabled as they should.
> > > 
> > > Fixes: d5c70627a794 ("iio: adc: Add Xilinx AMS driver")
> > > Signed-off-by: Robert Hancock <robert.hancock@calian.com>
> > > ---
> > >  drivers/iio/adc/xilinx-ams.c | 1 +
> > >  1 file changed, 1 insertion(+)
> > > 
> > > diff --git a/drivers/iio/adc/xilinx-ams.c b/drivers/iio/adc/xilinx-ams.c
> > > index 8343c5f74121..b93864362dac 100644
> > > --- a/drivers/iio/adc/xilinx-ams.c
> > > +++ b/drivers/iio/adc/xilinx-ams.c
> > > @@ -1224,6 +1224,7 @@ static int ams_init_module(struct iio_dev *indio_dev,
> > >  
> > >  		/* 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);  
> > 
> > Looks like this is the same change just submitted by Michael Tretter ("iio:
> > adc: xilinx-ams: Fix num_channels for PS channels").  
> 
> Thanks for pointing me here.
> 
> Reviewed-by: Michael Tretter <m.tretter@pengutronix.de>

I picked up Michael's patch and will drop this one...
(based on random email reading order. I'd argue his was first in time, but
then someone would point out next time I pick a patch up that is later than
another one :)

Thanks,

> 
> Michael


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

* Re: [PATCH 2/4] iio: adc: xilinx-ams: Fixed missing PS channels
@ 2022-01-30 12:29         ` Jonathan Cameron
  0 siblings, 0 replies; 30+ messages in thread
From: Jonathan Cameron @ 2022-01-30 12:29 UTC (permalink / raw)
  To: Michael Tretter
  Cc: Robert Hancock, linux-iio, lars, robh+dt, devicetree,
	michal.simek, manish.narani, linux-arm-kernel,
	anand.ashok.dumbre, kernel

On Tue, 25 Jan 2022 09:06:34 +0100
Michael Tretter <m.tretter@pengutronix.de> wrote:

> On Thu, 20 Jan 2022 01:09:39 +0000, Robert Hancock wrote:
> > On Wed, 2022-01-19 at 19:02 -0600, Robert Hancock wrote:  
> > > The code forgot to increment num_channels for the PS channel inputs,
> > > resulting in them not being enabled as they should.
> > > 
> > > Fixes: d5c70627a794 ("iio: adc: Add Xilinx AMS driver")
> > > Signed-off-by: Robert Hancock <robert.hancock@calian.com>
> > > ---
> > >  drivers/iio/adc/xilinx-ams.c | 1 +
> > >  1 file changed, 1 insertion(+)
> > > 
> > > diff --git a/drivers/iio/adc/xilinx-ams.c b/drivers/iio/adc/xilinx-ams.c
> > > index 8343c5f74121..b93864362dac 100644
> > > --- a/drivers/iio/adc/xilinx-ams.c
> > > +++ b/drivers/iio/adc/xilinx-ams.c
> > > @@ -1224,6 +1224,7 @@ static int ams_init_module(struct iio_dev *indio_dev,
> > >  
> > >  		/* 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);  
> > 
> > Looks like this is the same change just submitted by Michael Tretter ("iio:
> > adc: xilinx-ams: Fix num_channels for PS channels").  
> 
> Thanks for pointing me here.
> 
> Reviewed-by: Michael Tretter <m.tretter@pengutronix.de>

I picked up Michael's patch and will drop this one...
(based on random email reading order. I'd argue his was first in time, but
then someone would point out next time I pick a patch up that is later than
another one :)

Thanks,

> 
> Michael


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH 3/4] iio: adc: xilinx-ams: Fixed wrong sequencer register settings
  2022-01-26  9:12         ` Michael Tretter
@ 2022-01-30 12:31           ` Jonathan Cameron
  -1 siblings, 0 replies; 30+ messages in thread
From: Jonathan Cameron @ 2022-01-30 12:31 UTC (permalink / raw)
  To: Michael Tretter
  Cc: Robert Hancock, lars, robh+dt, devicetree, michal.simek,
	linux-iio, manish.narani, linux-arm-kernel, kernel,
	anand.ashok.dumbre

On Wed, 26 Jan 2022 10:12:50 +0100
Michael Tretter <m.tretter@pengutronix.de> wrote:

> On Tue, 25 Jan 2022 16:15:05 +0000, Robert Hancock wrote:
> > On Tue, 2022-01-25 at 09:21 +0100, Michael Tretter wrote:  
> > > On Wed, 19 Jan 2022 19:02:45 -0600, Robert Hancock wrote:  
> > > > Register settings used for the sequencer configuration register
> > > > were incorrect, causing some inputs to not be read properly.
> > > > 
> > > > Fixes: d5c70627a794 ("iio: adc: Add Xilinx AMS driver")
> > > > Signed-off-by: Robert Hancock <robert.hancock@calian.com>
> > > > ---
> > > >  drivers/iio/adc/xilinx-ams.c | 4 ++--
> > > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/drivers/iio/adc/xilinx-ams.c b/drivers/iio/adc/xilinx-ams.c
> > > > index b93864362dac..199027c93cdc 100644
> > > > --- a/drivers/iio/adc/xilinx-ams.c
> > > > +++ b/drivers/iio/adc/xilinx-ams.c
> > > > @@ -91,8 +91,8 @@
> > > >  
> > > >  #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_CONF1_SEQ_CONTINUOUS	FIELD_PREP(AMS_CONF1_SEQ_MASK, 2)
> > > > +#define AMS_CONF1_SEQ_SINGLE_CHANNEL	FIELD_PREP(AMS_CONF1_SEQ_MASK,
> > > > 3)  
> > > 
> > > The TRM states that Continuous Loop Mode is 2, but Single Pass Sequence Mode
> > > is 1, not 3. Is there a reason, why you need to set both bits?  
> > 
> > Single pass sequence mode (1) just runs the same sequence only once. To read
> > these values it needs to switch to single channel mode (3).
> > 
> > The register bits are defined in Table 3-8 of 
> > https://www.xilinx.com/support/documentation/user_guides/ug580-ultrascale-sysmon.pdf
> >  .  
> 
> Thanks for the clarification.
> 
> Reviewed-by: Michael Tretter <m.tretter@pengutronix.de>

Applied to the fixes-togreg branch of iio.git

Thanks,

Jonathan

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

* Re: [PATCH 3/4] iio: adc: xilinx-ams: Fixed wrong sequencer register settings
@ 2022-01-30 12:31           ` Jonathan Cameron
  0 siblings, 0 replies; 30+ messages in thread
From: Jonathan Cameron @ 2022-01-30 12:31 UTC (permalink / raw)
  To: Michael Tretter
  Cc: Robert Hancock, lars, robh+dt, devicetree, michal.simek,
	linux-iio, manish.narani, linux-arm-kernel, kernel,
	anand.ashok.dumbre

On Wed, 26 Jan 2022 10:12:50 +0100
Michael Tretter <m.tretter@pengutronix.de> wrote:

> On Tue, 25 Jan 2022 16:15:05 +0000, Robert Hancock wrote:
> > On Tue, 2022-01-25 at 09:21 +0100, Michael Tretter wrote:  
> > > On Wed, 19 Jan 2022 19:02:45 -0600, Robert Hancock wrote:  
> > > > Register settings used for the sequencer configuration register
> > > > were incorrect, causing some inputs to not be read properly.
> > > > 
> > > > Fixes: d5c70627a794 ("iio: adc: Add Xilinx AMS driver")
> > > > Signed-off-by: Robert Hancock <robert.hancock@calian.com>
> > > > ---
> > > >  drivers/iio/adc/xilinx-ams.c | 4 ++--
> > > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/drivers/iio/adc/xilinx-ams.c b/drivers/iio/adc/xilinx-ams.c
> > > > index b93864362dac..199027c93cdc 100644
> > > > --- a/drivers/iio/adc/xilinx-ams.c
> > > > +++ b/drivers/iio/adc/xilinx-ams.c
> > > > @@ -91,8 +91,8 @@
> > > >  
> > > >  #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_CONF1_SEQ_CONTINUOUS	FIELD_PREP(AMS_CONF1_SEQ_MASK, 2)
> > > > +#define AMS_CONF1_SEQ_SINGLE_CHANNEL	FIELD_PREP(AMS_CONF1_SEQ_MASK,
> > > > 3)  
> > > 
> > > The TRM states that Continuous Loop Mode is 2, but Single Pass Sequence Mode
> > > is 1, not 3. Is there a reason, why you need to set both bits?  
> > 
> > Single pass sequence mode (1) just runs the same sequence only once. To read
> > these values it needs to switch to single channel mode (3).
> > 
> > The register bits are defined in Table 3-8 of 
> > https://www.xilinx.com/support/documentation/user_guides/ug580-ultrascale-sysmon.pdf
> >  .  
> 
> Thanks for the clarification.
> 
> Reviewed-by: Michael Tretter <m.tretter@pengutronix.de>

Applied to the fixes-togreg branch of iio.git

Thanks,

Jonathan

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH 4/4] iio: adc: xilinx-ams: Fix single channel switching sequence
  2022-01-20  1:02   ` Robert Hancock
@ 2022-01-30 12:34     ` Jonathan Cameron
  -1 siblings, 0 replies; 30+ messages in thread
From: Jonathan Cameron @ 2022-01-30 12:34 UTC (permalink / raw)
  To: Robert Hancock
  Cc: linux-iio, robh+dt, michal.simek, anand.ashok.dumbre, lars,
	manish.narani, linux-arm-kernel, devicetree, Michael Tretter

On Wed, 19 Jan 2022 19:02:46 -0600
Robert Hancock <robert.hancock@calian.com> wrote:

> Some of the AMS channels need to be read by switching into single-channel
> mode from the normal polling sequence. There was a logic issue in this
> switching code that could cause the first read of these channels to read
> back as zero.
> 
> It appears that the sequencer should be set back to default mode before
> changing the channel selection, and the channel should be set before
> switching the sequencer back into single-channel mode.
> 
> Also, write 1 to the EOC bit in the status register to clear it before
> waiting for it to become set, so that we actually wait for a new
> conversion to complete, and don't proceed based on a previous conversion
> completing.
> 
> Fixes: d5c70627a794 ("iio: adc: Add Xilinx AMS driver")
> Signed-off-by: Robert Hancock <robert.hancock@calian.com>

Looking for an Ack from Anand or someone else familiar with this device.

Thanks,

Jonathan


> ---
>  drivers/iio/adc/xilinx-ams.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/adc/xilinx-ams.c b/drivers/iio/adc/xilinx-ams.c
> index 199027c93cdc..7bf097fa10cb 100644
> --- a/drivers/iio/adc/xilinx-ams.c
> +++ b/drivers/iio/adc/xilinx-ams.c
> @@ -530,14 +530,18 @@ static int ams_enable_single_channel(struct ams *ams, unsigned int offset)
>  		return -EINVAL;
>  	}
>  
> -	/* set single channel, sequencer off mode */
> +	/* 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_SINGLE_CHANNEL);
> +			  AMS_CONF1_SEQ_DEFAULT);
>  
>  	/* write the channel number */
>  	ams_ps_update_reg(ams, AMS_REG_CONFIG0, AMS_CONF0_CHANNEL_NUM_MASK,
>  			  channel_num);
>  
> +	/* set single channel, sequencer off mode */
> +	ams_ps_update_reg(ams, AMS_REG_CONFIG1, AMS_CONF1_SEQ_MASK,
> +			  AMS_CONF1_SEQ_SINGLE_CHANNEL);
> +
>  	return 0;
>  }
>  
> @@ -551,6 +555,8 @@ static int ams_read_vcc_reg(struct ams *ams, unsigned int offset, u32 *data)
>  	if (ret)
>  		return ret;
>  
> +	/* clear end-of-conversion flag, wait for next conversion to complete */
> +	writel(expect, ams->base + AMS_ISR_1);
>  	ret = readl_poll_timeout(ams->base + AMS_ISR_1, reg, (reg & expect),
>  				 AMS_INIT_POLL_TIME_US, AMS_INIT_TIMEOUT_US);
>  	if (ret)


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

* Re: [PATCH 4/4] iio: adc: xilinx-ams: Fix single channel switching sequence
@ 2022-01-30 12:34     ` Jonathan Cameron
  0 siblings, 0 replies; 30+ messages in thread
From: Jonathan Cameron @ 2022-01-30 12:34 UTC (permalink / raw)
  To: Robert Hancock
  Cc: linux-iio, robh+dt, michal.simek, anand.ashok.dumbre, lars,
	manish.narani, linux-arm-kernel, devicetree, Michael Tretter

On Wed, 19 Jan 2022 19:02:46 -0600
Robert Hancock <robert.hancock@calian.com> wrote:

> Some of the AMS channels need to be read by switching into single-channel
> mode from the normal polling sequence. There was a logic issue in this
> switching code that could cause the first read of these channels to read
> back as zero.
> 
> It appears that the sequencer should be set back to default mode before
> changing the channel selection, and the channel should be set before
> switching the sequencer back into single-channel mode.
> 
> Also, write 1 to the EOC bit in the status register to clear it before
> waiting for it to become set, so that we actually wait for a new
> conversion to complete, and don't proceed based on a previous conversion
> completing.
> 
> Fixes: d5c70627a794 ("iio: adc: Add Xilinx AMS driver")
> Signed-off-by: Robert Hancock <robert.hancock@calian.com>

Looking for an Ack from Anand or someone else familiar with this device.

Thanks,

Jonathan


> ---
>  drivers/iio/adc/xilinx-ams.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/adc/xilinx-ams.c b/drivers/iio/adc/xilinx-ams.c
> index 199027c93cdc..7bf097fa10cb 100644
> --- a/drivers/iio/adc/xilinx-ams.c
> +++ b/drivers/iio/adc/xilinx-ams.c
> @@ -530,14 +530,18 @@ static int ams_enable_single_channel(struct ams *ams, unsigned int offset)
>  		return -EINVAL;
>  	}
>  
> -	/* set single channel, sequencer off mode */
> +	/* 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_SINGLE_CHANNEL);
> +			  AMS_CONF1_SEQ_DEFAULT);
>  
>  	/* write the channel number */
>  	ams_ps_update_reg(ams, AMS_REG_CONFIG0, AMS_CONF0_CHANNEL_NUM_MASK,
>  			  channel_num);
>  
> +	/* set single channel, sequencer off mode */
> +	ams_ps_update_reg(ams, AMS_REG_CONFIG1, AMS_CONF1_SEQ_MASK,
> +			  AMS_CONF1_SEQ_SINGLE_CHANNEL);
> +
>  	return 0;
>  }
>  
> @@ -551,6 +555,8 @@ static int ams_read_vcc_reg(struct ams *ams, unsigned int offset, u32 *data)
>  	if (ret)
>  		return ret;
>  
> +	/* clear end-of-conversion flag, wait for next conversion to complete */
> +	writel(expect, ams->base + AMS_ISR_1);
>  	ret = readl_poll_timeout(ams->base + AMS_ISR_1, reg, (reg & expect),
>  				 AMS_INIT_POLL_TIME_US, AMS_INIT_TIMEOUT_US);
>  	if (ret)


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

end of thread, other threads:[~2022-01-30 12:29 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-20  1:02 [PATCH 0/4] Xilinx AMS fixes Robert Hancock
2022-01-20  1:02 ` Robert Hancock
2022-01-20  1:02 ` [PATCH 1/4] arm64: dts: zynqmp: add AMS driver to device tree Robert Hancock
2022-01-20  1:02   ` Robert Hancock
2022-01-25  8:02   ` Michael Tretter
2022-01-25  8:02     ` Michael Tretter
2022-01-25  9:42   ` Michal Simek
2022-01-25  9:42     ` Michal Simek
2022-01-20  1:02 ` [PATCH 2/4] iio: adc: xilinx-ams: Fixed missing PS channels Robert Hancock
2022-01-20  1:02   ` Robert Hancock
2022-01-20  1:09   ` Robert Hancock
2022-01-20  1:09     ` Robert Hancock
2022-01-25  8:06     ` Michael Tretter
2022-01-25  8:06       ` Michael Tretter
2022-01-30 12:29       ` Jonathan Cameron
2022-01-30 12:29         ` Jonathan Cameron
2022-01-20  1:02 ` [PATCH 3/4] iio: adc: xilinx-ams: Fixed wrong sequencer register settings Robert Hancock
2022-01-20  1:02   ` Robert Hancock
2022-01-25  8:21   ` Michael Tretter
2022-01-25  8:21     ` Michael Tretter
2022-01-25 16:15     ` Robert Hancock
2022-01-25 16:15       ` Robert Hancock
2022-01-26  9:12       ` Michael Tretter
2022-01-26  9:12         ` Michael Tretter
2022-01-30 12:31         ` Jonathan Cameron
2022-01-30 12:31           ` Jonathan Cameron
2022-01-20  1:02 ` [PATCH 4/4] iio: adc: xilinx-ams: Fix single channel switching sequence Robert Hancock
2022-01-20  1:02   ` Robert Hancock
2022-01-30 12:34   ` Jonathan Cameron
2022-01-30 12:34     ` Jonathan Cameron

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.