[PATCH v2 0/3] Adds support of ADC for FSD SoC

[PATCH v2 0/3] Adds support of ADC for FSD SoC

[Tamseel Shams]

This series of 3 patches adds support to ADC driver for
FSD SoC, and also adds nodes in device tree. These patches
were a part of large set of patches related to FSD SoC, but
these are posted now separately as per review comments received.

M Tamseel Shams (3):
  dt-bindings: iio: adc: Add FSD-HW variant
  iio: adc: exynos-adc: Add support for ADC FSD-HW controller
  arm64: dts: fsd: Add ADC device tree node

 .../bindings/iio/adc/samsung,exynos-adc.yaml  |  1 +
 arch/arm64/boot/dts/tesla/fsd-evb.dts         |  4 ++
 arch/arm64/boot/dts/tesla/fsd.dtsi            | 11 ++++
 drivers/iio/adc/exynos_adc.c                  | 55 +++++++++++++++++++
 4 files changed, 71 insertions(+)

-- 
2.17.1

[PATCH v2 0/3] Adds support of ADC for FSD SoC

[Tamseel Shams]

This series of 3 patches adds support to ADC driver for
FSD SoC, and also adds nodes in device tree. These patches
were a part of large set of patches related to FSD SoC, but
these are posted now separately as per review comments received.

M Tamseel Shams (3):
  dt-bindings: iio: adc: Add FSD-HW variant
  iio: adc: exynos-adc: Add support for ADC FSD-HW controller
  arm64: dts: fsd: Add ADC device tree node

 .../bindings/iio/adc/samsung,exynos-adc.yaml  |  1 +
 arch/arm64/boot/dts/tesla/fsd-evb.dts         |  4 ++
 arch/arm64/boot/dts/tesla/fsd.dtsi            | 11 ++++
 drivers/iio/adc/exynos_adc.c                  | 55 +++++++++++++++++++
 4 files changed, 71 insertions(+)

-- 
2.17.1


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

[PATCH v2 1/3] dt-bindings: iio: adc: Add FSD-HW variant

[Tamseel Shams]

From: Alim Akhtar <alim.akhtar@samsung.com>

This patch adds a new compatible string for FSD-HW's
ADC variant

Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
Signed-off-by: Tamseel Shams <m.shams@samsung.com>
---
- Changes since v1
* Addressed Krzysztof's comment to use soc-specific compatible

 .../devicetree/bindings/iio/adc/samsung,exynos-adc.yaml          | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/iio/adc/samsung,exynos-adc.yaml b/Documentation/devicetree/bindings/iio/adc/samsung,exynos-adc.yaml
index 81c87295912c..ded0e3f85991 100644
--- a/Documentation/devicetree/bindings/iio/adc/samsung,exynos-adc.yaml
+++ b/Documentation/devicetree/bindings/iio/adc/samsung,exynos-adc.yaml
@@ -14,6 +14,7 @@ properties:
     enum:
       - samsung,exynos-adc-v1                 # Exynos5250
       - samsung,exynos-adc-v2
+      - samsung,exynos-adc-fsd-hw
       - samsung,exynos3250-adc
       - samsung,exynos4212-adc                # Exynos4212 and Exynos4412
       - samsung,exynos7-adc
-- 
2.17.1

[PATCH v2 1/3] dt-bindings: iio: adc: Add FSD-HW variant

[Tamseel Shams]

From: Alim Akhtar <alim.akhtar@samsung.com>

This patch adds a new compatible string for FSD-HW's
ADC variant

Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
Signed-off-by: Tamseel Shams <m.shams@samsung.com>
---
- Changes since v1
* Addressed Krzysztof's comment to use soc-specific compatible

 .../devicetree/bindings/iio/adc/samsung,exynos-adc.yaml          | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/iio/adc/samsung,exynos-adc.yaml b/Documentation/devicetree/bindings/iio/adc/samsung,exynos-adc.yaml
index 81c87295912c..ded0e3f85991 100644
--- a/Documentation/devicetree/bindings/iio/adc/samsung,exynos-adc.yaml
+++ b/Documentation/devicetree/bindings/iio/adc/samsung,exynos-adc.yaml
@@ -14,6 +14,7 @@ properties:
     enum:
       - samsung,exynos-adc-v1                 # Exynos5250
       - samsung,exynos-adc-v2
+      - samsung,exynos-adc-fsd-hw
       - samsung,exynos3250-adc
       - samsung,exynos4212-adc                # Exynos4212 and Exynos4412
       - samsung,exynos7-adc
-- 
2.17.1


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

[PATCH v2 2/3] iio: adc: exynos-adc: Add support for ADC FSD-HW controller

[Tamseel Shams]

From: Alim Akhtar <alim.akhtar@samsung.com>

Exynos's ADC-FSD-HW has some difference in registers set, number of
programmable channels (16 channel) etc. This patch adds support for
ADC-FSD-HW controller version.

Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
Signed-off-by: Tamseel Shams <m.shams@samsung.com>
---
- Changes since v1
* Addressed Jonathan's comment by using already provided isr handle

 drivers/iio/adc/exynos_adc.c | 55 ++++++++++++++++++++++++++++++++++++
 1 file changed, 55 insertions(+)

diff --git a/drivers/iio/adc/exynos_adc.c b/drivers/iio/adc/exynos_adc.c
index cff1ba57fb16..183ae591327a 100644
--- a/drivers/iio/adc/exynos_adc.c
+++ b/drivers/iio/adc/exynos_adc.c
@@ -55,6 +55,11 @@
 #define ADC_V2_INT_ST(x)	((x) + 0x14)
 #define ADC_V2_VER(x)		((x) + 0x20)
 
+/* ADC_FSD_HW register definitions */
+#define ADC_FSD_DAT(x)			((x) + 0x08)
+#define ADC_FSD_DAT_SUM(x)		((x) + 0x0C)
+#define ADC_FSD_DBG_DATA(x)		((x) + 0x1C)
+
 /* Bit definitions for ADC_V1 */
 #define ADC_V1_CON_RES		(1u << 16)
 #define ADC_V1_CON_PRSCEN	(1u << 14)
@@ -92,6 +97,7 @@
 
 /* Bit definitions for ADC_V2 */
 #define ADC_V2_CON1_SOFT_RESET	(1u << 2)
+#define ADC_V2_CON1_SOFT_NON_RESET	(1u << 1)
 
 #define ADC_V2_CON2_OSEL	(1u << 10)
 #define ADC_V2_CON2_ESEL	(1u << 9)
@@ -100,6 +106,7 @@
 #define ADC_V2_CON2_ACH_SEL(x)	(((x) & 0xF) << 0)
 #define ADC_V2_CON2_ACH_MASK	0xF
 
+#define MAX_ADC_FSD_CHANNELS		16
 #define MAX_ADC_V2_CHANNELS		10
 #define MAX_ADC_V1_CHANNELS		8
 #define MAX_EXYNOS3250_ADC_CHANNELS	2
@@ -484,6 +491,43 @@ static const struct exynos_adc_data exynos7_adc_data = {
 	.start_conv	= exynos_adc_v2_start_conv,
 };
 
+static void exynos_adc_fsd_init_hw(struct exynos_adc *info)
+{
+	u32 con2;
+
+	writel(ADC_V2_CON1_SOFT_RESET, ADC_V2_CON1(info->regs));
+
+	writel(ADC_V2_CON1_SOFT_NON_RESET, ADC_V2_CON1(info->regs));
+
+	con2 = ADC_V2_CON2_C_TIME(6);
+	writel(con2, ADC_V2_CON2(info->regs));
+
+	/* Enable interrupts */
+	writel(1, ADC_V2_INT_EN(info->regs));
+}
+
+static void exynos_adc_fsd_exit_hw(struct exynos_adc *info)
+{
+	u32 con2;
+
+	con2 = readl(ADC_V2_CON2(info->regs));
+	con2 &= ~ADC_V2_CON2_C_TIME(7);
+	writel(con2, ADC_V2_CON2(info->regs));
+
+	/* Disable interrupts */
+	writel(0, ADC_V2_INT_EN(info->regs));
+}
+
+static const struct exynos_adc_data fsd_hw_adc_data = {
+	.num_channels	= MAX_ADC_FSD_CHANNELS,
+	.mask		= ADC_DATX_MASK, /* 12 bit ADC resolution */
+
+	.init_hw	= exynos_adc_fsd_init_hw,
+	.exit_hw	= exynos_adc_fsd_exit_hw,
+	.clear_irq	= exynos_adc_v2_clear_irq,
+	.start_conv	= exynos_adc_v2_start_conv,
+};
+
 static const struct of_device_id exynos_adc_match[] = {
 	{
 		.compatible = "samsung,s3c2410-adc",
@@ -518,6 +562,9 @@ static const struct of_device_id exynos_adc_match[] = {
 	}, {
 		.compatible = "samsung,exynos7-adc",
 		.data = &exynos7_adc_data,
+	}, {
+		.compatible = "samsung,exynos-adc-fsd-hw",
+		.data = &fsd_hw_adc_data,
 	},
 	{},
 };
@@ -626,6 +673,8 @@ static irqreturn_t exynos_adc_isr(int irq, void *dev_id)
 		info->ts_x = readl(ADC_V1_DATX(info->regs));
 		info->ts_y = readl(ADC_V1_DATY(info->regs));
 		writel(ADC_TSC_WAIT4INT | ADC_S3C2443_TSC_UD_SEN, ADC_V1_TSC(info->regs));
+	} else if (of_device_is_compatible(info->dev->of_node, "samsung,exynos-adc-fsd-hw")) {
+		info->value = readl(ADC_FSD_DAT(info->regs)) & mask;
 	} else {
 		info->value = readl(ADC_V1_DATX(info->regs)) & mask;
 	}
@@ -719,6 +768,12 @@ static const struct iio_chan_spec exynos_adc_iio_channels[] = {
 	ADC_CHANNEL(7, "adc7"),
 	ADC_CHANNEL(8, "adc8"),
 	ADC_CHANNEL(9, "adc9"),
+	ADC_CHANNEL(10, "adc10"),
+	ADC_CHANNEL(11, "adc11"),
+	ADC_CHANNEL(12, "adc12"),
+	ADC_CHANNEL(13, "adc13"),
+	ADC_CHANNEL(14, "adc14"),
+	ADC_CHANNEL(15, "adc15"),
 };
 
 static int exynos_adc_remove_devices(struct device *dev, void *c)
-- 
2.17.1

[PATCH v2 2/3] iio: adc: exynos-adc: Add support for ADC FSD-HW controller

[Tamseel Shams]

From: Alim Akhtar <alim.akhtar@samsung.com>

Exynos's ADC-FSD-HW has some difference in registers set, number of
programmable channels (16 channel) etc. This patch adds support for
ADC-FSD-HW controller version.

Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
Signed-off-by: Tamseel Shams <m.shams@samsung.com>
---
- Changes since v1
* Addressed Jonathan's comment by using already provided isr handle

 drivers/iio/adc/exynos_adc.c | 55 ++++++++++++++++++++++++++++++++++++
 1 file changed, 55 insertions(+)

diff --git a/drivers/iio/adc/exynos_adc.c b/drivers/iio/adc/exynos_adc.c
index cff1ba57fb16..183ae591327a 100644
--- a/drivers/iio/adc/exynos_adc.c
+++ b/drivers/iio/adc/exynos_adc.c
@@ -55,6 +55,11 @@
 #define ADC_V2_INT_ST(x)	((x) + 0x14)
 #define ADC_V2_VER(x)		((x) + 0x20)
 
+/* ADC_FSD_HW register definitions */
+#define ADC_FSD_DAT(x)			((x) + 0x08)
+#define ADC_FSD_DAT_SUM(x)		((x) + 0x0C)
+#define ADC_FSD_DBG_DATA(x)		((x) + 0x1C)
+
 /* Bit definitions for ADC_V1 */
 #define ADC_V1_CON_RES		(1u << 16)
 #define ADC_V1_CON_PRSCEN	(1u << 14)
@@ -92,6 +97,7 @@
 
 /* Bit definitions for ADC_V2 */
 #define ADC_V2_CON1_SOFT_RESET	(1u << 2)
+#define ADC_V2_CON1_SOFT_NON_RESET	(1u << 1)
 
 #define ADC_V2_CON2_OSEL	(1u << 10)
 #define ADC_V2_CON2_ESEL	(1u << 9)
@@ -100,6 +106,7 @@
 #define ADC_V2_CON2_ACH_SEL(x)	(((x) & 0xF) << 0)
 #define ADC_V2_CON2_ACH_MASK	0xF
 
+#define MAX_ADC_FSD_CHANNELS		16
 #define MAX_ADC_V2_CHANNELS		10
 #define MAX_ADC_V1_CHANNELS		8
 #define MAX_EXYNOS3250_ADC_CHANNELS	2
@@ -484,6 +491,43 @@ static const struct exynos_adc_data exynos7_adc_data = {
 	.start_conv	= exynos_adc_v2_start_conv,
 };
 
+static void exynos_adc_fsd_init_hw(struct exynos_adc *info)
+{
+	u32 con2;
+
+	writel(ADC_V2_CON1_SOFT_RESET, ADC_V2_CON1(info->regs));
+
+	writel(ADC_V2_CON1_SOFT_NON_RESET, ADC_V2_CON1(info->regs));
+
+	con2 = ADC_V2_CON2_C_TIME(6);
+	writel(con2, ADC_V2_CON2(info->regs));
+
+	/* Enable interrupts */
+	writel(1, ADC_V2_INT_EN(info->regs));
+}
+
+static void exynos_adc_fsd_exit_hw(struct exynos_adc *info)
+{
+	u32 con2;
+
+	con2 = readl(ADC_V2_CON2(info->regs));
+	con2 &= ~ADC_V2_CON2_C_TIME(7);
+	writel(con2, ADC_V2_CON2(info->regs));
+
+	/* Disable interrupts */
+	writel(0, ADC_V2_INT_EN(info->regs));
+}
+
+static const struct exynos_adc_data fsd_hw_adc_data = {
+	.num_channels	= MAX_ADC_FSD_CHANNELS,
+	.mask		= ADC_DATX_MASK, /* 12 bit ADC resolution */
+
+	.init_hw	= exynos_adc_fsd_init_hw,
+	.exit_hw	= exynos_adc_fsd_exit_hw,
+	.clear_irq	= exynos_adc_v2_clear_irq,
+	.start_conv	= exynos_adc_v2_start_conv,
+};
+
 static const struct of_device_id exynos_adc_match[] = {
 	{
 		.compatible = "samsung,s3c2410-adc",
@@ -518,6 +562,9 @@ static const struct of_device_id exynos_adc_match[] = {
 	}, {
 		.compatible = "samsung,exynos7-adc",
 		.data = &exynos7_adc_data,
+	}, {
+		.compatible = "samsung,exynos-adc-fsd-hw",
+		.data = &fsd_hw_adc_data,
 	},
 	{},
 };
@@ -626,6 +673,8 @@ static irqreturn_t exynos_adc_isr(int irq, void *dev_id)
 		info->ts_x = readl(ADC_V1_DATX(info->regs));
 		info->ts_y = readl(ADC_V1_DATY(info->regs));
 		writel(ADC_TSC_WAIT4INT | ADC_S3C2443_TSC_UD_SEN, ADC_V1_TSC(info->regs));
+	} else if (of_device_is_compatible(info->dev->of_node, "samsung,exynos-adc-fsd-hw")) {
+		info->value = readl(ADC_FSD_DAT(info->regs)) & mask;
 	} else {
 		info->value = readl(ADC_V1_DATX(info->regs)) & mask;
 	}
@@ -719,6 +768,12 @@ static const struct iio_chan_spec exynos_adc_iio_channels[] = {
 	ADC_CHANNEL(7, "adc7"),
 	ADC_CHANNEL(8, "adc8"),
 	ADC_CHANNEL(9, "adc9"),
+	ADC_CHANNEL(10, "adc10"),
+	ADC_CHANNEL(11, "adc11"),
+	ADC_CHANNEL(12, "adc12"),
+	ADC_CHANNEL(13, "adc13"),
+	ADC_CHANNEL(14, "adc14"),
+	ADC_CHANNEL(15, "adc15"),
 };
 
 static int exynos_adc_remove_devices(struct device *dev, void *c)
-- 
2.17.1


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

[PATCH v2 3/3] arm64: dts: fsd: Add ADC device tree node

[Tamseel Shams]

From: Alim Akhtar <alim.akhtar@samsung.com>

This patch adds ADC device tree node and enables the same
on fsd platform.

Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
Signed-off-by: Tamseel Shams <m.shams@samsung.com>
---
- Changes since v1
* Addressed Krzysztof's comment for corrections in dt files

 arch/arm64/boot/dts/tesla/fsd-evb.dts |  4 ++++
 arch/arm64/boot/dts/tesla/fsd.dtsi    | 11 +++++++++++
 2 files changed, 15 insertions(+)

diff --git a/arch/arm64/boot/dts/tesla/fsd-evb.dts b/arch/arm64/boot/dts/tesla/fsd-evb.dts
index 5af560c1b5e6..63f26a85053a 100644
--- a/arch/arm64/boot/dts/tesla/fsd-evb.dts
+++ b/arch/arm64/boot/dts/tesla/fsd-evb.dts
@@ -34,6 +34,10 @@
 	clock-frequency = <24000000>;
 };
 
+&adc {
+	status = "okay";
+};
+
 &serial_0 {
 	status = "okay";
 };
diff --git a/arch/arm64/boot/dts/tesla/fsd.dtsi b/arch/arm64/boot/dts/tesla/fsd.dtsi
index 9a652abcbcac..f63624f2f414 100644
--- a/arch/arm64/boot/dts/tesla/fsd.dtsi
+++ b/arch/arm64/boot/dts/tesla/fsd.dtsi
@@ -748,6 +748,17 @@
 			clocks = <&fin_pll>, <&clock_imem IMEM_MCT_PCLK>;
 			clock-names = "fin_pll", "mct";
 		};
+
+		adc: adc@141a0000 {
+			compatible = "samsung,exynos-adc-fsd-hw";
+			reg = <0x0 0x141a0000 0x0 0x100>;
+			interrupts = <GIC_SPI 173 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&clock_peric PERIC_PCLK_ADCIF>;
+			clock-names = "adc";
+			#io-channel-cells = <1>;
+			io-channel-ranges;
+			status = "disabled";
+		};
 	};
 };
 
-- 
2.17.1

[PATCH v2 3/3] arm64: dts: fsd: Add ADC device tree node

[Tamseel Shams]

From: Alim Akhtar <alim.akhtar@samsung.com>

This patch adds ADC device tree node and enables the same
on fsd platform.

Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
Signed-off-by: Tamseel Shams <m.shams@samsung.com>
---
- Changes since v1
* Addressed Krzysztof's comment for corrections in dt files

 arch/arm64/boot/dts/tesla/fsd-evb.dts |  4 ++++
 arch/arm64/boot/dts/tesla/fsd.dtsi    | 11 +++++++++++
 2 files changed, 15 insertions(+)

diff --git a/arch/arm64/boot/dts/tesla/fsd-evb.dts b/arch/arm64/boot/dts/tesla/fsd-evb.dts
index 5af560c1b5e6..63f26a85053a 100644
--- a/arch/arm64/boot/dts/tesla/fsd-evb.dts
+++ b/arch/arm64/boot/dts/tesla/fsd-evb.dts
@@ -34,6 +34,10 @@
 	clock-frequency = <24000000>;
 };
 
+&adc {
+	status = "okay";
+};
+
 &serial_0 {
 	status = "okay";
 };
diff --git a/arch/arm64/boot/dts/tesla/fsd.dtsi b/arch/arm64/boot/dts/tesla/fsd.dtsi
index 9a652abcbcac..f63624f2f414 100644
--- a/arch/arm64/boot/dts/tesla/fsd.dtsi
+++ b/arch/arm64/boot/dts/tesla/fsd.dtsi
@@ -748,6 +748,17 @@
 			clocks = <&fin_pll>, <&clock_imem IMEM_MCT_PCLK>;
 			clock-names = "fin_pll", "mct";
 		};
+
+		adc: adc@141a0000 {
+			compatible = "samsung,exynos-adc-fsd-hw";
+			reg = <0x0 0x141a0000 0x0 0x100>;
+			interrupts = <GIC_SPI 173 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&clock_peric PERIC_PCLK_ADCIF>;
+			clock-names = "adc";
+			#io-channel-cells = <1>;
+			io-channel-ranges;
+			status = "disabled";
+		};
 	};
 };
 
-- 
2.17.1


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

Re: [PATCH v2 2/3] iio: adc: exynos-adc: Add support for ADC FSD-HW controller

[Jonathan Cameron]

On Fri, 20 May 2022 20:28:19 +0530
Tamseel Shams <m.shams@samsung.com> wrote:

> From: Alim Akhtar <alim.akhtar@samsung.com>
> 
> Exynos's ADC-FSD-HW has some difference in registers set, number of
> programmable channels (16 channel) etc. This patch adds support for
> ADC-FSD-HW controller version.
> 
> Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
> Signed-off-by: Tamseel Shams <m.shams@samsung.com>

Hi,

One suggestion inline, otherwise LGTM. Plenty of time to tidy this up as
this won't make the upcoming merge window - I'll be queuing it up for 5.20

Thanks,

Jonathan

> ---
> - Changes since v1
> * Addressed Jonathan's comment by using already provided isr handle
> 
>  drivers/iio/adc/exynos_adc.c | 55 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 55 insertions(+)
> 
> diff --git a/drivers/iio/adc/exynos_adc.c b/drivers/iio/adc/exynos_adc.c
> index cff1ba57fb16..183ae591327a 100644
> --- a/drivers/iio/adc/exynos_adc.c
> +++ b/drivers/iio/adc/exynos_adc.c
> @@ -55,6 +55,11 @@
>  #define ADC_V2_INT_ST(x)	((x) + 0x14)
>  #define ADC_V2_VER(x)		((x) + 0x20)
>  
> +/* ADC_FSD_HW register definitions */
> +#define ADC_FSD_DAT(x)			((x) + 0x08)

I mention this below, but these different register sets
should be in the struct exynos_adc_data to avoid the need
for an if "compatible" == check on each use of them.


> +#define ADC_FSD_DAT_SUM(x)		((x) + 0x0C)
> +#define ADC_FSD_DBG_DATA(x)		((x) + 0x1C)
> +
>  /* Bit definitions for ADC_V1 */
>  #define ADC_V1_CON_RES		(1u << 16)
>  #define ADC_V1_CON_PRSCEN	(1u << 14)
> @@ -92,6 +97,7 @@
>  
>  /* Bit definitions for ADC_V2 */
>  #define ADC_V2_CON1_SOFT_RESET	(1u << 2)
> +#define ADC_V2_CON1_SOFT_NON_RESET	(1u << 1)
>  
>  #define ADC_V2_CON2_OSEL	(1u << 10)
>  #define ADC_V2_CON2_ESEL	(1u << 9)
> @@ -100,6 +106,7 @@
>  #define ADC_V2_CON2_ACH_SEL(x)	(((x) & 0xF) << 0)
>  #define ADC_V2_CON2_ACH_MASK	0xF
>  
> +#define MAX_ADC_FSD_CHANNELS		16
>  #define MAX_ADC_V2_CHANNELS		10
>  #define MAX_ADC_V1_CHANNELS		8
>  #define MAX_EXYNOS3250_ADC_CHANNELS	2
> @@ -484,6 +491,43 @@ static const struct exynos_adc_data exynos7_adc_data = {
>  	.start_conv	= exynos_adc_v2_start_conv,
>  };
>  
> +static void exynos_adc_fsd_init_hw(struct exynos_adc *info)
> +{
> +	u32 con2;
> +
> +	writel(ADC_V2_CON1_SOFT_RESET, ADC_V2_CON1(info->regs));
> +
> +	writel(ADC_V2_CON1_SOFT_NON_RESET, ADC_V2_CON1(info->regs));
> +
> +	con2 = ADC_V2_CON2_C_TIME(6);
> +	writel(con2, ADC_V2_CON2(info->regs));
> +
> +	/* Enable interrupts */
> +	writel(1, ADC_V2_INT_EN(info->regs));
> +}
> +
> +static void exynos_adc_fsd_exit_hw(struct exynos_adc *info)
> +{
> +	u32 con2;
> +
> +	con2 = readl(ADC_V2_CON2(info->regs));
> +	con2 &= ~ADC_V2_CON2_C_TIME(7);
> +	writel(con2, ADC_V2_CON2(info->regs));
> +
> +	/* Disable interrupts */
> +	writel(0, ADC_V2_INT_EN(info->regs));
> +}
> +
> +static const struct exynos_adc_data fsd_hw_adc_data = {
> +	.num_channels	= MAX_ADC_FSD_CHANNELS,
> +	.mask		= ADC_DATX_MASK, /* 12 bit ADC resolution */
> +
> +	.init_hw	= exynos_adc_fsd_init_hw,
> +	.exit_hw	= exynos_adc_fsd_exit_hw,
> +	.clear_irq	= exynos_adc_v2_clear_irq,
> +	.start_conv	= exynos_adc_v2_start_conv,
> +};
> +
>  static const struct of_device_id exynos_adc_match[] = {
>  	{
>  		.compatible = "samsung,s3c2410-adc",
> @@ -518,6 +562,9 @@ static const struct of_device_id exynos_adc_match[] = {
>  	}, {
>  		.compatible = "samsung,exynos7-adc",
>  		.data = &exynos7_adc_data,
> +	}, {
> +		.compatible = "samsung,exynos-adc-fsd-hw",
> +		.data = &fsd_hw_adc_data,
>  	},
>  	{},
>  };
> @@ -626,6 +673,8 @@ static irqreturn_t exynos_adc_isr(int irq, void *dev_id)
>  		info->ts_x = readl(ADC_V1_DATX(info->regs));
>  		info->ts_y = readl(ADC_V1_DATY(info->regs));
>  		writel(ADC_TSC_WAIT4INT | ADC_S3C2443_TSC_UD_SEN, ADC_V1_TSC(info->regs));
> +	} else if (of_device_is_compatible(info->dev->of_node, "samsung,exynos-adc-fsd-hw")) {

Rather than a fairly expensive look up into a device tree node, why not add
the information to the struct exynos_adc_adc in some fashion?  Maybe as an offset
for the register block?

 
> +		info->value = readl(ADC_FSD_DAT(info->regs)) & mask;
>  	} else {
>  		info->value = readl(ADC_V1_DATX(info->regs)) & mask;
>  	}
> @@ -719,6 +768,12 @@ static const struct iio_chan_spec exynos_adc_iio_channels[] = {
>  	ADC_CHANNEL(7, "adc7"),
>  	ADC_CHANNEL(8, "adc8"),
>  	ADC_CHANNEL(9, "adc9"),
> +	ADC_CHANNEL(10, "adc10"),
> +	ADC_CHANNEL(11, "adc11"),
> +	ADC_CHANNEL(12, "adc12"),
> +	ADC_CHANNEL(13, "adc13"),
> +	ADC_CHANNEL(14, "adc14"),
> +	ADC_CHANNEL(15, "adc15"),
>  };
>  
>  static int exynos_adc_remove_devices(struct device *dev, void *c)

Re: [PATCH v2 2/3] iio: adc: exynos-adc: Add support for ADC FSD-HW controller

[Jonathan Cameron]

On Fri, 20 May 2022 20:28:19 +0530
Tamseel Shams <m.shams@samsung.com> wrote:

> From: Alim Akhtar <alim.akhtar@samsung.com>
> 
> Exynos's ADC-FSD-HW has some difference in registers set, number of
> programmable channels (16 channel) etc. This patch adds support for
> ADC-FSD-HW controller version.
> 
> Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
> Signed-off-by: Tamseel Shams <m.shams@samsung.com>

Hi,

One suggestion inline, otherwise LGTM. Plenty of time to tidy this up as
this won't make the upcoming merge window - I'll be queuing it up for 5.20

Thanks,

Jonathan

> ---
> - Changes since v1
> * Addressed Jonathan's comment by using already provided isr handle
> 
>  drivers/iio/adc/exynos_adc.c | 55 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 55 insertions(+)
> 
> diff --git a/drivers/iio/adc/exynos_adc.c b/drivers/iio/adc/exynos_adc.c
> index cff1ba57fb16..183ae591327a 100644
> --- a/drivers/iio/adc/exynos_adc.c
> +++ b/drivers/iio/adc/exynos_adc.c
> @@ -55,6 +55,11 @@
>  #define ADC_V2_INT_ST(x)	((x) + 0x14)
>  #define ADC_V2_VER(x)		((x) + 0x20)
>  
> +/* ADC_FSD_HW register definitions */
> +#define ADC_FSD_DAT(x)			((x) + 0x08)

I mention this below, but these different register sets
should be in the struct exynos_adc_data to avoid the need
for an if "compatible" == check on each use of them.


> +#define ADC_FSD_DAT_SUM(x)		((x) + 0x0C)
> +#define ADC_FSD_DBG_DATA(x)		((x) + 0x1C)
> +
>  /* Bit definitions for ADC_V1 */
>  #define ADC_V1_CON_RES		(1u << 16)
>  #define ADC_V1_CON_PRSCEN	(1u << 14)
> @@ -92,6 +97,7 @@
>  
>  /* Bit definitions for ADC_V2 */
>  #define ADC_V2_CON1_SOFT_RESET	(1u << 2)
> +#define ADC_V2_CON1_SOFT_NON_RESET	(1u << 1)
>  
>  #define ADC_V2_CON2_OSEL	(1u << 10)
>  #define ADC_V2_CON2_ESEL	(1u << 9)
> @@ -100,6 +106,7 @@
>  #define ADC_V2_CON2_ACH_SEL(x)	(((x) & 0xF) << 0)
>  #define ADC_V2_CON2_ACH_MASK	0xF
>  
> +#define MAX_ADC_FSD_CHANNELS		16
>  #define MAX_ADC_V2_CHANNELS		10
>  #define MAX_ADC_V1_CHANNELS		8
>  #define MAX_EXYNOS3250_ADC_CHANNELS	2
> @@ -484,6 +491,43 @@ static const struct exynos_adc_data exynos7_adc_data = {
>  	.start_conv	= exynos_adc_v2_start_conv,
>  };
>  
> +static void exynos_adc_fsd_init_hw(struct exynos_adc *info)
> +{
> +	u32 con2;
> +
> +	writel(ADC_V2_CON1_SOFT_RESET, ADC_V2_CON1(info->regs));
> +
> +	writel(ADC_V2_CON1_SOFT_NON_RESET, ADC_V2_CON1(info->regs));
> +
> +	con2 = ADC_V2_CON2_C_TIME(6);
> +	writel(con2, ADC_V2_CON2(info->regs));
> +
> +	/* Enable interrupts */
> +	writel(1, ADC_V2_INT_EN(info->regs));
> +}
> +
> +static void exynos_adc_fsd_exit_hw(struct exynos_adc *info)
> +{
> +	u32 con2;
> +
> +	con2 = readl(ADC_V2_CON2(info->regs));
> +	con2 &= ~ADC_V2_CON2_C_TIME(7);
> +	writel(con2, ADC_V2_CON2(info->regs));
> +
> +	/* Disable interrupts */
> +	writel(0, ADC_V2_INT_EN(info->regs));
> +}
> +
> +static const struct exynos_adc_data fsd_hw_adc_data = {
> +	.num_channels	= MAX_ADC_FSD_CHANNELS,
> +	.mask		= ADC_DATX_MASK, /* 12 bit ADC resolution */
> +
> +	.init_hw	= exynos_adc_fsd_init_hw,
> +	.exit_hw	= exynos_adc_fsd_exit_hw,
> +	.clear_irq	= exynos_adc_v2_clear_irq,
> +	.start_conv	= exynos_adc_v2_start_conv,
> +};
> +
>  static const struct of_device_id exynos_adc_match[] = {
>  	{
>  		.compatible = "samsung,s3c2410-adc",
> @@ -518,6 +562,9 @@ static const struct of_device_id exynos_adc_match[] = {
>  	}, {
>  		.compatible = "samsung,exynos7-adc",
>  		.data = &exynos7_adc_data,
> +	}, {
> +		.compatible = "samsung,exynos-adc-fsd-hw",
> +		.data = &fsd_hw_adc_data,
>  	},
>  	{},
>  };
> @@ -626,6 +673,8 @@ static irqreturn_t exynos_adc_isr(int irq, void *dev_id)
>  		info->ts_x = readl(ADC_V1_DATX(info->regs));
>  		info->ts_y = readl(ADC_V1_DATY(info->regs));
>  		writel(ADC_TSC_WAIT4INT | ADC_S3C2443_TSC_UD_SEN, ADC_V1_TSC(info->regs));
> +	} else if (of_device_is_compatible(info->dev->of_node, "samsung,exynos-adc-fsd-hw")) {

Rather than a fairly expensive look up into a device tree node, why not add
the information to the struct exynos_adc_adc in some fashion?  Maybe as an offset
for the register block?

 
> +		info->value = readl(ADC_FSD_DAT(info->regs)) & mask;
>  	} else {
>  		info->value = readl(ADC_V1_DATX(info->regs)) & mask;
>  	}
> @@ -719,6 +768,12 @@ static const struct iio_chan_spec exynos_adc_iio_channels[] = {
>  	ADC_CHANNEL(7, "adc7"),
>  	ADC_CHANNEL(8, "adc8"),
>  	ADC_CHANNEL(9, "adc9"),
> +	ADC_CHANNEL(10, "adc10"),
> +	ADC_CHANNEL(11, "adc11"),
> +	ADC_CHANNEL(12, "adc12"),
> +	ADC_CHANNEL(13, "adc13"),
> +	ADC_CHANNEL(14, "adc14"),
> +	ADC_CHANNEL(15, "adc15"),
>  };
>  
>  static int exynos_adc_remove_devices(struct device *dev, void *c)


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

Re: [PATCH v2 1/3] dt-bindings: iio: adc: Add FSD-HW variant

[Krzysztof Kozlowski]

On 20/05/2022 16:58, Tamseel Shams wrote:
> From: Alim Akhtar <alim.akhtar@samsung.com>
> 
> This patch adds a new compatible string for FSD-HW's
> ADC variant
> 
> Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
> Signed-off-by: Tamseel Shams <m.shams@samsung.com>
> ---
> - Changes since v1
> * Addressed Krzysztof's comment to use soc-specific compatible
> 
>  .../devicetree/bindings/iio/adc/samsung,exynos-adc.yaml          | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/samsung,exynos-adc.yaml b/Documentation/devicetree/bindings/iio/adc/samsung,exynos-adc.yaml
> index 81c87295912c..ded0e3f85991 100644
> --- a/Documentation/devicetree/bindings/iio/adc/samsung,exynos-adc.yaml
> +++ b/Documentation/devicetree/bindings/iio/adc/samsung,exynos-adc.yaml
> @@ -14,6 +14,7 @@ properties:
>      enum:
>        - samsung,exynos-adc-v1                 # Exynos5250
>        - samsung,exynos-adc-v2
> +      - samsung,exynos-adc-fsd-hw

The soc is fsd, so compatible should rather be:
tesla,fsd-adc

What does the "HW" mean?

>        - samsung,exynos3250-adc
>        - samsung,exynos4212-adc                # Exynos4212 and Exynos4412
>        - samsung,exynos7-adc


Best regards,
Krzysztof

Re: [PATCH v2 1/3] dt-bindings: iio: adc: Add FSD-HW variant

[Krzysztof Kozlowski]

On 20/05/2022 16:58, Tamseel Shams wrote:
> From: Alim Akhtar <alim.akhtar@samsung.com>
> 
> This patch adds a new compatible string for FSD-HW's
> ADC variant
> 
> Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
> Signed-off-by: Tamseel Shams <m.shams@samsung.com>
> ---
> - Changes since v1
> * Addressed Krzysztof's comment to use soc-specific compatible
> 
>  .../devicetree/bindings/iio/adc/samsung,exynos-adc.yaml          | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/samsung,exynos-adc.yaml b/Documentation/devicetree/bindings/iio/adc/samsung,exynos-adc.yaml
> index 81c87295912c..ded0e3f85991 100644
> --- a/Documentation/devicetree/bindings/iio/adc/samsung,exynos-adc.yaml
> +++ b/Documentation/devicetree/bindings/iio/adc/samsung,exynos-adc.yaml
> @@ -14,6 +14,7 @@ properties:
>      enum:
>        - samsung,exynos-adc-v1                 # Exynos5250
>        - samsung,exynos-adc-v2
> +      - samsung,exynos-adc-fsd-hw

The soc is fsd, so compatible should rather be:
tesla,fsd-adc

What does the "HW" mean?

>        - samsung,exynos3250-adc
>        - samsung,exynos4212-adc                # Exynos4212 and Exynos4412
>        - samsung,exynos7-adc


Best regards,
Krzysztof

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

Re: [PATCH v2 2/3] iio: adc: exynos-adc: Add support for ADC FSD-HW controller

[Krzysztof Kozlowski]

On 20/05/2022 16:58, Tamseel Shams wrote:
> From: Alim Akhtar <alim.akhtar@samsung.com>
> 
> Exynos's ADC-FSD-HW has some difference in registers set, number of
> programmable channels (16 channel) etc. This patch adds support for
> ADC-FSD-HW controller version.
> 
> Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
> Signed-off-by: Tamseel Shams <m.shams@samsung.com>

The compatible needs changing (as commented in bindings).

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH v2 2/3] iio: adc: exynos-adc: Add support for ADC FSD-HW controller

[Krzysztof Kozlowski]

On 20/05/2022 16:58, Tamseel Shams wrote:
> From: Alim Akhtar <alim.akhtar@samsung.com>
> 
> Exynos's ADC-FSD-HW has some difference in registers set, number of
> programmable channels (16 channel) etc. This patch adds support for
> ADC-FSD-HW controller version.
> 
> Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
> Signed-off-by: Tamseel Shams <m.shams@samsung.com>

The compatible needs changing (as commented in bindings).

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

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

Re: [PATCH v2 3/3] arm64: dts: fsd: Add ADC device tree node

[Krzysztof Kozlowski]

On 20/05/2022 16:58, Tamseel Shams wrote:
> From: Alim Akhtar <alim.akhtar@samsung.com>
> 
> This patch adds ADC device tree node and enables the same
> on fsd platform.
> 
> Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
> Signed-off-by: Tamseel Shams <m.shams@samsung.com>
> ---
> - Changes since v1
> * Addressed Krzysztof's comment for corrections in dt files
> 
>  arch/arm64/boot/dts/tesla/fsd-evb.dts |  4 ++++
>  arch/arm64/boot/dts/tesla/fsd.dtsi    | 11 +++++++++++
>  2 files changed, 15 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/tesla/fsd-evb.dts b/arch/arm64/boot/dts/tesla/fsd-evb.dts
> index 5af560c1b5e6..63f26a85053a 100644
> --- a/arch/arm64/boot/dts/tesla/fsd-evb.dts
> +++ b/arch/arm64/boot/dts/tesla/fsd-evb.dts
> @@ -34,6 +34,10 @@
>  	clock-frequency = <24000000>;
>  };
>  
> +&adc {
> +	status = "okay";

Please test your bindings (`make dtbs_check`). There is no way it passes
since you miss at least supply (maybe more?).




Best regards,
Krzysztof

Re: [PATCH v2 3/3] arm64: dts: fsd: Add ADC device tree node

[Krzysztof Kozlowski]

On 20/05/2022 16:58, Tamseel Shams wrote:
> From: Alim Akhtar <alim.akhtar@samsung.com>
> 
> This patch adds ADC device tree node and enables the same
> on fsd platform.
> 
> Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
> Signed-off-by: Tamseel Shams <m.shams@samsung.com>
> ---
> - Changes since v1
> * Addressed Krzysztof's comment for corrections in dt files
> 
>  arch/arm64/boot/dts/tesla/fsd-evb.dts |  4 ++++
>  arch/arm64/boot/dts/tesla/fsd.dtsi    | 11 +++++++++++
>  2 files changed, 15 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/tesla/fsd-evb.dts b/arch/arm64/boot/dts/tesla/fsd-evb.dts
> index 5af560c1b5e6..63f26a85053a 100644
> --- a/arch/arm64/boot/dts/tesla/fsd-evb.dts
> +++ b/arch/arm64/boot/dts/tesla/fsd-evb.dts
> @@ -34,6 +34,10 @@
>  	clock-frequency = <24000000>;
>  };
>  
> +&adc {
> +	status = "okay";

Please test your bindings (`make dtbs_check`). There is no way it passes
since you miss at least supply (maybe more?).




Best regards,
Krzysztof

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

RE: [PATCH v2 1/3] dt-bindings: iio: adc: Add FSD-HW variant

[m.shams]

Hi Krzysztof,

On 20/05/2022 16:58, Tamseel Shams wrote:
>> From: Alim Akhtar <alim.akhtar@samsung.com>
>> 
>> This patch adds a new compatible string for FSD-HW's ADC variant
>> 
>> Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
>> Signed-off-by: Tamseel Shams <m.shams@samsung.com>
>> ---
>> - Changes since v1
>> * Addressed Krzysztof's comment to use soc-specific compatible
>> 
>>  .../devicetree/bindings/iio/adc/samsung,exynos-adc.yaml          | 1 +
>>  1 file changed, 1 insertion(+)
>> 
>> diff --git 
>> a/Documentation/devicetree/bindings/iio/adc/samsung,exynos-adc.yaml 
>> b/Documentation/devicetree/bindings/iio/adc/samsung,exynos-adc.yaml
>> index 81c87295912c..ded0e3f85991 100644
>> --- 
>> a/Documentation/devicetree/bindings/iio/adc/samsung,exynos-adc.yaml
>> +++ b/Documentation/devicetree/bindings/iio/adc/samsung,exynos-adc.yam
>> +++ l
>> @@ -14,6 +14,7 @@ properties:
>>      enum:
>>        - samsung,exynos-adc-v1                 # Exynos5250
>>        - samsung,exynos-adc-v2
>> +      - samsung,exynos-adc-fsd-hw

> The soc is fsd, so compatible should rather be:
> tesla,fsd-adc

> What does the "HW" mean?

Will change the compatible to "tesla,fsd-adc" in the next version.
By HW, I meant Hardware


Thanks & Regards,
Tamseel Shams

RE: [PATCH v2 1/3] dt-bindings: iio: adc: Add FSD-HW variant

[m.shams]

Hi Krzysztof,

On 20/05/2022 16:58, Tamseel Shams wrote:
>> From: Alim Akhtar <alim.akhtar@samsung.com>
>> 
>> This patch adds a new compatible string for FSD-HW's ADC variant
>> 
>> Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
>> Signed-off-by: Tamseel Shams <m.shams@samsung.com>
>> ---
>> - Changes since v1
>> * Addressed Krzysztof's comment to use soc-specific compatible
>> 
>>  .../devicetree/bindings/iio/adc/samsung,exynos-adc.yaml          | 1 +
>>  1 file changed, 1 insertion(+)
>> 
>> diff --git 
>> a/Documentation/devicetree/bindings/iio/adc/samsung,exynos-adc.yaml 
>> b/Documentation/devicetree/bindings/iio/adc/samsung,exynos-adc.yaml
>> index 81c87295912c..ded0e3f85991 100644
>> --- 
>> a/Documentation/devicetree/bindings/iio/adc/samsung,exynos-adc.yaml
>> +++ b/Documentation/devicetree/bindings/iio/adc/samsung,exynos-adc.yam
>> +++ l
>> @@ -14,6 +14,7 @@ properties:
>>      enum:
>>        - samsung,exynos-adc-v1                 # Exynos5250
>>        - samsung,exynos-adc-v2
>> +      - samsung,exynos-adc-fsd-hw

> The soc is fsd, so compatible should rather be:
> tesla,fsd-adc

> What does the "HW" mean?

Will change the compatible to "tesla,fsd-adc" in the next version.
By HW, I meant Hardware


Thanks & Regards,
Tamseel Shams


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

RE: [PATCH v2 2/3] iio: adc: exynos-adc: Add support for ADC FSD-HW controller

[m.shams]

Hi Krzysztof,

On 20/05/2022 16:58, Tamseel Shams wrote:
>> From: Alim Akhtar <alim.akhtar@samsung.com>
>> 
>> Exynos's ADC-FSD-HW has some difference in registers set, number of 
>> programmable channels (16 channel) etc. This patch adds support for 
>> ADC-FSD-HW controller version.
>> 
>> Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
>> Signed-off-by: Tamseel Shams <m.shams@samsung.com>

>The compatible needs changing (as commented in bindings).

Sure, will change in next version 


Thanks & Regards,
Tamseel Shams

RE: [PATCH v2 2/3] iio: adc: exynos-adc: Add support for ADC FSD-HW controller

[m.shams]

Hi Krzysztof,

On 20/05/2022 16:58, Tamseel Shams wrote:
>> From: Alim Akhtar <alim.akhtar@samsung.com>
>> 
>> Exynos's ADC-FSD-HW has some difference in registers set, number of 
>> programmable channels (16 channel) etc. This patch adds support for 
>> ADC-FSD-HW controller version.
>> 
>> Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
>> Signed-off-by: Tamseel Shams <m.shams@samsung.com>

>The compatible needs changing (as commented in bindings).

Sure, will change in next version 


Thanks & Regards,
Tamseel Shams


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

RE: [PATCH v2 3/3] arm64: dts: fsd: Add ADC device tree node

[m.shams]

Hi Krzysztof,

On 20/05/2022 16:58, Tamseel Shams wrote:
>> From: Alim Akhtar <alim.akhtar@samsung.com>
>> 
>> This patch adds ADC device tree node and enables the same on fsd 
>> platform.
>> 
>> Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
>> Signed-off-by: Tamseel Shams <m.shams@samsung.com>
>> ---
>> - Changes since v1
>> * Addressed Krzysztof's comment for corrections in dt files
>> 
>>  arch/arm64/boot/dts/tesla/fsd-evb.dts |  4 ++++
>>  arch/arm64/boot/dts/tesla/fsd.dtsi    | 11 +++++++++++
>>  2 files changed, 15 insertions(+)
>> 
>> diff --git a/arch/arm64/boot/dts/tesla/fsd-evb.dts 
>> b/arch/arm64/boot/dts/tesla/fsd-evb.dts
>> index 5af560c1b5e6..63f26a85053a 100644
>> --- a/arch/arm64/boot/dts/tesla/fsd-evb.dts
>> +++ b/arch/arm64/boot/dts/tesla/fsd-evb.dts
>> @@ -34,6 +34,10 @@
>>  	clock-frequency = <24000000>;
>>  };
>>  
>> +&adc {
>> +	status = "okay";
>
> Please test your bindings (`make dtbs_check`). There is no way it passes since you miss at least supply (maybe more?).

Will add the "supply" field in dtsi file. Also will run dtbs check to see for any other missing field.


Thanks & Regards,
Tamseel Shams

RE: [PATCH v2 3/3] arm64: dts: fsd: Add ADC device tree node

[m.shams]

Hi Krzysztof,

On 20/05/2022 16:58, Tamseel Shams wrote:
>> From: Alim Akhtar <alim.akhtar@samsung.com>
>> 
>> This patch adds ADC device tree node and enables the same on fsd 
>> platform.
>> 
>> Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
>> Signed-off-by: Tamseel Shams <m.shams@samsung.com>
>> ---
>> - Changes since v1
>> * Addressed Krzysztof's comment for corrections in dt files
>> 
>>  arch/arm64/boot/dts/tesla/fsd-evb.dts |  4 ++++
>>  arch/arm64/boot/dts/tesla/fsd.dtsi    | 11 +++++++++++
>>  2 files changed, 15 insertions(+)
>> 
>> diff --git a/arch/arm64/boot/dts/tesla/fsd-evb.dts 
>> b/arch/arm64/boot/dts/tesla/fsd-evb.dts
>> index 5af560c1b5e6..63f26a85053a 100644
>> --- a/arch/arm64/boot/dts/tesla/fsd-evb.dts
>> +++ b/arch/arm64/boot/dts/tesla/fsd-evb.dts
>> @@ -34,6 +34,10 @@
>>  	clock-frequency = <24000000>;
>>  };
>>  
>> +&adc {
>> +	status = "okay";
>
> Please test your bindings (`make dtbs_check`). There is no way it passes since you miss at least supply (maybe more?).

Will add the "supply" field in dtsi file. Also will run dtbs check to see for any other missing field.


Thanks & Regards,
Tamseel Shams


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

RE: [PATCH v2 2/3] iio: adc: exynos-adc: Add support for ADC FSD-HW controller

[m.shams]

Hi Jonathan,

On Fri, 20 May 2022 20:28:19 +0530
Tamseel Shams <m.shams@samsung.com> wrote:

>> From: Alim Akhtar <alim.akhtar@samsung.com>
>> 
>> Exynos's ADC-FSD-HW has some difference in registers set, number of 
>> programmable channels (16 channel) etc. This patch adds support for 
>> ADC-FSD-HW controller version.
>> 
>> Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
>> Signed-off-by: Tamseel Shams <m.shams@samsung.com>
>
> Hi,
>
> One suggestion inline, otherwise LGTM. Plenty of time to tidy this up as
this won't make the upcoming merge window - I'll be queuing it up for 5.20
>
> Thanks,
>
> Jonathan
>

Okay, Thanks for reviewing.

>> ---
>> - Changes since v1
>> * Addressed Jonathan's comment by using already provided isr handle
>> 
>>  drivers/iio/adc/exynos_adc.c | 55 
>> ++++++++++++++++++++++++++++++++++++
>>  1 file changed, 55 insertions(+)
>> 
>> diff --git a/drivers/iio/adc/exynos_adc.c 
>> b/drivers/iio/adc/exynos_adc.c index cff1ba57fb16..183ae591327a 100644
>> --- a/drivers/iio/adc/exynos_adc.c
>> +++ b/drivers/iio/adc/exynos_adc.c
>> @@ -55,6 +55,11 @@
>>  #define ADC_V2_INT_ST(x)	((x) + 0x14)
>>  #define ADC_V2_VER(x)		((x) + 0x20)
>>  
>> +/* ADC_FSD_HW register definitions */
>> +#define ADC_FSD_DAT(x)			((x) + 0x08)
>
> I mention this below, but these different register sets should be in the
struct exynos_adc_data to avoid the need for an if "compatible" == check on
each use of > them.
>

Can you clarify on how exactly you want me to add these register sets to
struct exynos_adc_data?
Do you mean just for these registers or other registers too which are
defined in this way only?


Thanks & Regards,
Tamseel Shams

RE: [PATCH v2 2/3] iio: adc: exynos-adc: Add support for ADC FSD-HW controller

[m.shams]

Hi Jonathan,

On Fri, 20 May 2022 20:28:19 +0530
Tamseel Shams <m.shams@samsung.com> wrote:

>> From: Alim Akhtar <alim.akhtar@samsung.com>
>> 
>> Exynos's ADC-FSD-HW has some difference in registers set, number of 
>> programmable channels (16 channel) etc. This patch adds support for 
>> ADC-FSD-HW controller version.
>> 
>> Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
>> Signed-off-by: Tamseel Shams <m.shams@samsung.com>
>
> Hi,
>
> One suggestion inline, otherwise LGTM. Plenty of time to tidy this up as
this won't make the upcoming merge window - I'll be queuing it up for 5.20
>
> Thanks,
>
> Jonathan
>

Okay, Thanks for reviewing.

>> ---
>> - Changes since v1
>> * Addressed Jonathan's comment by using already provided isr handle
>> 
>>  drivers/iio/adc/exynos_adc.c | 55 
>> ++++++++++++++++++++++++++++++++++++
>>  1 file changed, 55 insertions(+)
>> 
>> diff --git a/drivers/iio/adc/exynos_adc.c 
>> b/drivers/iio/adc/exynos_adc.c index cff1ba57fb16..183ae591327a 100644
>> --- a/drivers/iio/adc/exynos_adc.c
>> +++ b/drivers/iio/adc/exynos_adc.c
>> @@ -55,6 +55,11 @@
>>  #define ADC_V2_INT_ST(x)	((x) + 0x14)
>>  #define ADC_V2_VER(x)		((x) + 0x20)
>>  
>> +/* ADC_FSD_HW register definitions */
>> +#define ADC_FSD_DAT(x)			((x) + 0x08)
>
> I mention this below, but these different register sets should be in the
struct exynos_adc_data to avoid the need for an if "compatible" == check on
each use of > them.
>

Can you clarify on how exactly you want me to add these register sets to
struct exynos_adc_data?
Do you mean just for these registers or other registers too which are
defined in this way only?


Thanks & Regards,
Tamseel Shams


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

Re: [PATCH v2 2/3] iio: adc: exynos-adc: Add support for ADC FSD-HW controller

[Jonathan Cameron]

On Tue, 31 May 2022 14:12:46 +0530
"m.shams" <m.shams@samsung.com> wrote:

> Hi Jonathan,
> 
> On Fri, 20 May 2022 20:28:19 +0530
> Tamseel Shams <m.shams@samsung.com> wrote:
> 
> >> From: Alim Akhtar <alim.akhtar@samsung.com>
> >> 
> >> Exynos's ADC-FSD-HW has some difference in registers set, number of 
> >> programmable channels (16 channel) etc. This patch adds support for 
> >> ADC-FSD-HW controller version.
> >> 
> >> Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
> >> Signed-off-by: Tamseel Shams <m.shams@samsung.com>  
> >
> > Hi,
> >
> > One suggestion inline, otherwise LGTM. Plenty of time to tidy this up as  
> this won't make the upcoming merge window - I'll be queuing it up for 5.20
> >
> > Thanks,
> >
> > Jonathan
> >  
> 
> Okay, Thanks for reviewing.
> 
> >> ---
> >> - Changes since v1
> >> * Addressed Jonathan's comment by using already provided isr handle
> >> 
> >>  drivers/iio/adc/exynos_adc.c | 55 
> >> ++++++++++++++++++++++++++++++++++++
> >>  1 file changed, 55 insertions(+)
> >> 
> >> diff --git a/drivers/iio/adc/exynos_adc.c 
> >> b/drivers/iio/adc/exynos_adc.c index cff1ba57fb16..183ae591327a 100644
> >> --- a/drivers/iio/adc/exynos_adc.c
> >> +++ b/drivers/iio/adc/exynos_adc.c
> >> @@ -55,6 +55,11 @@
> >>  #define ADC_V2_INT_ST(x)	((x) + 0x14)
> >>  #define ADC_V2_VER(x)		((x) + 0x20)
> >>  
> >> +/* ADC_FSD_HW register definitions */
> >> +#define ADC_FSD_DAT(x)			((x) + 0x08)  
> >
> > I mention this below, but these different register sets should be in the  
> struct exynos_adc_data to avoid the need for an if "compatible" == check on
> each use of > them.
> >  
> 
> Can you clarify on how exactly you want me to add these register sets to
> struct exynos_adc_data?
> Do you mean just for these registers or other registers too which are
> defined in this way only?

Any registers addresses that are different for the different chip variants
supported by the driver.

In cases where the only difference between versions is a register address then
define something like
#define ADC_FSD_DAT_BASE 0x08

In the structure have a

dat_addr = ADC_FSD_DAT_BASE

and use dat_addr + x to access.

If things are more complex (and I haven't looked closely so that may apply to
the example give above, the wrap the different access sequence and register
addresses in a callback similar to already done for clear_irq.


Jonathan


> 
> 
> Thanks & Regards,
> Tamseel Shams
> 

Re: [PATCH v2 2/3] iio: adc: exynos-adc: Add support for ADC FSD-HW controller

[Jonathan Cameron]

On Tue, 31 May 2022 14:12:46 +0530
"m.shams" <m.shams@samsung.com> wrote:

> Hi Jonathan,
> 
> On Fri, 20 May 2022 20:28:19 +0530
> Tamseel Shams <m.shams@samsung.com> wrote:
> 
> >> From: Alim Akhtar <alim.akhtar@samsung.com>
> >> 
> >> Exynos's ADC-FSD-HW has some difference in registers set, number of 
> >> programmable channels (16 channel) etc. This patch adds support for 
> >> ADC-FSD-HW controller version.
> >> 
> >> Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
> >> Signed-off-by: Tamseel Shams <m.shams@samsung.com>  
> >
> > Hi,
> >
> > One suggestion inline, otherwise LGTM. Plenty of time to tidy this up as  
> this won't make the upcoming merge window - I'll be queuing it up for 5.20
> >
> > Thanks,
> >
> > Jonathan
> >  
> 
> Okay, Thanks for reviewing.
> 
> >> ---
> >> - Changes since v1
> >> * Addressed Jonathan's comment by using already provided isr handle
> >> 
> >>  drivers/iio/adc/exynos_adc.c | 55 
> >> ++++++++++++++++++++++++++++++++++++
> >>  1 file changed, 55 insertions(+)
> >> 
> >> diff --git a/drivers/iio/adc/exynos_adc.c 
> >> b/drivers/iio/adc/exynos_adc.c index cff1ba57fb16..183ae591327a 100644
> >> --- a/drivers/iio/adc/exynos_adc.c
> >> +++ b/drivers/iio/adc/exynos_adc.c
> >> @@ -55,6 +55,11 @@
> >>  #define ADC_V2_INT_ST(x)	((x) + 0x14)
> >>  #define ADC_V2_VER(x)		((x) + 0x20)
> >>  
> >> +/* ADC_FSD_HW register definitions */
> >> +#define ADC_FSD_DAT(x)			((x) + 0x08)  
> >
> > I mention this below, but these different register sets should be in the  
> struct exynos_adc_data to avoid the need for an if "compatible" == check on
> each use of > them.
> >  
> 
> Can you clarify on how exactly you want me to add these register sets to
> struct exynos_adc_data?
> Do you mean just for these registers or other registers too which are
> defined in this way only?

Any registers addresses that are different for the different chip variants
supported by the driver.

In cases where the only difference between versions is a register address then
define something like
#define ADC_FSD_DAT_BASE 0x08

In the structure have a

dat_addr = ADC_FSD_DAT_BASE

and use dat_addr + x to access.

If things are more complex (and I haven't looked closely so that may apply to
the example give above, the wrap the different access sequence and register
addresses in a callback similar to already done for clear_irq.


Jonathan


> 
> 
> Thanks & Regards,
> Tamseel Shams
> 


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