[U-Boot] [PATCH 1/6] clk: add clk_valid()

[U-Boot] [PATCH 1/6] clk: add clk_valid()

[Fabrice Gasnier]

add clk_valid() to check for optional clocks are valid.

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
---

 include/clk.h | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/include/clk.h b/include/clk.h
index 9a35764..71679a9 100644
--- a/include/clk.h
+++ b/include/clk.h
@@ -294,4 +294,14 @@ int clk_disable_bulk(struct clk_bulk *bulk);
 
 int soc_clk_dump(void);
 
+/**
+ * clk_valid() - check if clk is valid
+ *
+ * @clk:	the clock to check
+ * @return TRUE if valid, or FALSE
+ */
+static inline bool clk_valid(struct clk *clk)
+{
+	return !!clk->dev;
+}
 #endif
-- 
1.9.1

[U-Boot] [PATCH 2/6] dm: adc: uclass: get reference regulator once

[Fabrice Gasnier]

device_get_supply_regulator() only needs to be called once.
But each time there's call to adc_vxx_value() for instance, it calls
adc_vxx_platdata_update() -> device_get_supply_regulator().

This also allows vdd_supply/vss_supply to be provided directly from
uc_pdata, e.g dt-binding variant like stm32-adc provide its own
'vref-supply'.

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
---

 drivers/adc/adc-uclass.c | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/drivers/adc/adc-uclass.c b/drivers/adc/adc-uclass.c
index 17c1a4e..70f4cde 100644
--- a/drivers/adc/adc-uclass.c
+++ b/drivers/adc/adc-uclass.c
@@ -264,10 +264,13 @@ static int adc_vdd_platdata_update(struct udevice *dev)
 	 * will bind before its supply regulator device, then the below 'get'
 	 * will return an error.
 	 */
-	ret = device_get_supply_regulator(dev, "vdd-supply",
-					  &uc_pdata->vdd_supply);
-	if (ret)
-		return ret;
+	if (!uc_pdata->vdd_supply) {
+		/* Only get vdd_supply once */
+		ret = device_get_supply_regulator(dev, "vdd-supply",
+						  &uc_pdata->vdd_supply);
+		if (ret)
+			return ret;
+	}
 
 	ret = regulator_get_value(uc_pdata->vdd_supply);
 	if (ret < 0)
@@ -283,10 +286,12 @@ static int adc_vss_platdata_update(struct udevice *dev)
 	struct adc_uclass_platdata *uc_pdata = dev_get_uclass_platdata(dev);
 	int ret;
 
-	ret = device_get_supply_regulator(dev, "vss-supply",
-					  &uc_pdata->vss_supply);
-	if (ret)
-		return ret;
+	if (!uc_pdata->vss_supply) {
+		ret = device_get_supply_regulator(dev, "vss-supply",
+						  &uc_pdata->vss_supply);
+		if (ret)
+			return ret;
+	}
 
 	ret = regulator_get_value(uc_pdata->vss_supply);
 	if (ret < 0)
-- 
1.9.1

[U-Boot] [PATCH 2/6] dm: adc: uclass: get reference regulator once

[Simon Glass]

Hi Fabrice,

On 23 July 2018 at 06:35, Fabrice Gasnier <fabrice.gasnier@st.com> wrote:
> device_get_supply_regulator() only needs to be called once.
> But each time there's call to adc_vxx_value() for instance, it calls
> adc_vxx_platdata_update() -> device_get_supply_regulator().
>
> This also allows vdd_supply/vss_supply to be provided directly from
> uc_pdata, e.g dt-binding variant like stm32-adc provide its own
> 'vref-supply'.
>
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
> ---
>
>  drivers/adc/adc-uclass.c | 21 +++++++++++++--------
>  1 file changed, 13 insertions(+), 8 deletions(-)

The original code doesn't look right to me.

Reading from the DT should happen in the ofdata_to_platdata() method,
except (as here) where we need to probe another device, iwc we can use
the probe() method.

So can you move this code into a new probe() method, so it just
happens once, when the device is probed?

>
> diff --git a/drivers/adc/adc-uclass.c b/drivers/adc/adc-uclass.c
> index 17c1a4e..70f4cde 100644
> --- a/drivers/adc/adc-uclass.c
> +++ b/drivers/adc/adc-uclass.c
> @@ -264,10 +264,13 @@ static int adc_vdd_platdata_update(struct udevice *dev)
>          * will bind before its supply regulator device, then the below 'get'
>          * will return an error.
>          */
> -       ret = device_get_supply_regulator(dev, "vdd-supply",
> -                                         &uc_pdata->vdd_supply);
> -       if (ret)
> -               return ret;
> +       if (!uc_pdata->vdd_supply) {
> +               /* Only get vdd_supply once */
> +               ret = device_get_supply_regulator(dev, "vdd-supply",
> +                                                 &uc_pdata->vdd_supply);
> +               if (ret)
> +                       return ret;
> +       }
>
>         ret = regulator_get_value(uc_pdata->vdd_supply);
>         if (ret < 0)
> @@ -283,10 +286,12 @@ static int adc_vss_platdata_update(struct udevice *dev)
>         struct adc_uclass_platdata *uc_pdata = dev_get_uclass_platdata(dev);
>         int ret;
>
> -       ret = device_get_supply_regulator(dev, "vss-supply",
> -                                         &uc_pdata->vss_supply);
> -       if (ret)
> -               return ret;
> +       if (!uc_pdata->vss_supply) {
> +               ret = device_get_supply_regulator(dev, "vss-supply",
> +                                                 &uc_pdata->vss_supply);
> +               if (ret)
> +                       return ret;
> +       }
>
>         ret = regulator_get_value(uc_pdata->vss_supply);
>         if (ret < 0)
> --
> 1.9.1
>

Regards,
Simon

[U-Boot] [PATCH 2/6] dm: adc: uclass: get reference regulator once

[Fabrice Gasnier]

On 07/24/2018 01:48 AM, Simon Glass wrote:
> Hi Fabrice,
> 
> On 23 July 2018 at 06:35, Fabrice Gasnier <fabrice.gasnier@st.com> wrote:
>> device_get_supply_regulator() only needs to be called once.
>> But each time there's call to adc_vxx_value() for instance, it calls
>> adc_vxx_platdata_update() -> device_get_supply_regulator().
>>
>> This also allows vdd_supply/vss_supply to be provided directly from
>> uc_pdata, e.g dt-binding variant like stm32-adc provide its own
>> 'vref-supply'.
>>
>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
>> ---
>>
>>  drivers/adc/adc-uclass.c | 21 +++++++++++++--------
>>  1 file changed, 13 insertions(+), 8 deletions(-)
> 
> The original code doesn't look right to me.
> 
> Reading from the DT should happen in the ofdata_to_platdata() method,
> except (as here) where we need to probe another device, iwc we can use
> the probe() method.
> 
> So can you move this code into a new probe() method, so it just
> happens once, when the device is probed?

Hi Simon,

I just sent an updated version of this patchset. I agree with you, so I
moved device_get_supply_regulator() call to pre_probe().

But I get confused about "a new probe()" as you mention above.
So, please take a look at v2, and tell me if this change is in line with
your view.

Many thanks for reviewing,
Best Regards,
Fabrice

> 
>>
>> diff --git a/drivers/adc/adc-uclass.c b/drivers/adc/adc-uclass.c
>> index 17c1a4e..70f4cde 100644
>> --- a/drivers/adc/adc-uclass.c
>> +++ b/drivers/adc/adc-uclass.c
>> @@ -264,10 +264,13 @@ static int adc_vdd_platdata_update(struct udevice *dev)
>>          * will bind before its supply regulator device, then the below 'get'
>>          * will return an error.
>>          */
>> -       ret = device_get_supply_regulator(dev, "vdd-supply",
>> -                                         &uc_pdata->vdd_supply);
>> -       if (ret)
>> -               return ret;
>> +       if (!uc_pdata->vdd_supply) {
>> +               /* Only get vdd_supply once */
>> +               ret = device_get_supply_regulator(dev, "vdd-supply",
>> +                                                 &uc_pdata->vdd_supply);
>> +               if (ret)
>> +                       return ret;
>> +       }
>>
>>         ret = regulator_get_value(uc_pdata->vdd_supply);
>>         if (ret < 0)
>> @@ -283,10 +286,12 @@ static int adc_vss_platdata_update(struct udevice *dev)
>>         struct adc_uclass_platdata *uc_pdata = dev_get_uclass_platdata(dev);
>>         int ret;
>>
>> -       ret = device_get_supply_regulator(dev, "vss-supply",
>> -                                         &uc_pdata->vss_supply);
>> -       if (ret)
>> -               return ret;
>> +       if (!uc_pdata->vss_supply) {
>> +               ret = device_get_supply_regulator(dev, "vss-supply",
>> +                                                 &uc_pdata->vss_supply);
>> +               if (ret)
>> +                       return ret;
>> +       }
>>
>>         ret = regulator_get_value(uc_pdata->vss_supply);
>>         if (ret < 0)
>> --
>> 1.9.1
>>
> 
> Regards,
> Simon
> 

[U-Boot] [PATCH 3/6] dt-bindings: Document STM32 ADC DT bindings

[Fabrice Gasnier]

This patch adds documentation of device tree bindings for the STM32 ADC.
It's based on linux-v4.18-rc* dt-bindings, at the time of writing:
- Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
---

 doc/device-tree-bindings/adc/st,stm32-adc.txt | 141 ++++++++++++++++++++++++++
 1 file changed, 141 insertions(+)
 create mode 100644 doc/device-tree-bindings/adc/st,stm32-adc.txt

diff --git a/doc/device-tree-bindings/adc/st,stm32-adc.txt b/doc/device-tree-bindings/adc/st,stm32-adc.txt
new file mode 100644
index 0000000..07fb6cd
--- /dev/null
+++ b/doc/device-tree-bindings/adc/st,stm32-adc.txt
@@ -0,0 +1,141 @@
+STMicroelectronics STM32 ADC device
+
+STM32 ADC is a successive approximation analog-to-digital converter.
+It has several multiplexed input channels. Conversions can be performed
+in single, continuous, scan or discontinuous mode. Result of the ADC is
+stored in a left-aligned or right-aligned 32-bit data register.
+Conversions can be launched in software or using hardware triggers.
+
+The analog watchdog feature allows the application to detect if the input
+voltage goes beyond the user-defined, higher or lower thresholds.
+
+Each STM32 ADC block can have up to 3 ADC instances.
+
+Each instance supports two contexts to manage conversions, each one has its
+own configurable sequence and trigger:
+- regular conversion can be done in sequence, running in background
+- injected conversions have higher priority, and so have the ability to
+  interrupt regular conversion sequence (either triggered in SW or HW).
+  Regular sequence is resumed, in case it has been interrupted.
+
+Contents of a stm32 adc root node:
+-----------------------------------
+Required properties:
+- compatible: Should be one of:
+  "st,stm32f4-adc-core"
+  "st,stm32h7-adc-core"
+  "st,stm32mp1-adc-core"
+- reg: Offset and length of the ADC block register set.
+- interrupts: One or more interrupts for ADC block. Some parts like stm32f4
+  and stm32h7 share a common ADC interrupt line. stm32mp1 has two separate
+  interrupt lines, one for each ADC within ADC block.
+- clocks: Core can use up to two clocks, depending on part used:
+  - "adc" clock: for the analog circuitry, common to all ADCs.
+    It's required on stm32f4.
+    It's optional on stm32h7.
+  - "bus" clock: for registers access, common to all ADCs.
+    It's not present on stm32f4.
+    It's required on stm32h7.
+- clock-names: Must be "adc" and/or "bus" depending on part used.
+- interrupt-controller: Identifies the controller node as interrupt-parent
+- vref-supply: Phandle to the vref input analog reference voltage.
+- #interrupt-cells = <1>;
+- #address-cells = <1>;
+- #size-cells = <0>;
+
+Optional properties:
+- A pinctrl state named "default" for each ADC channel may be defined to set
+  inX ADC pins in mode of operation for analog input on external pin.
+
+Contents of a stm32 adc child node:
+-----------------------------------
+An ADC block node should contain at least one subnode, representing an
+ADC instance available on the machine.
+
+Required properties:
+- compatible: Should be one of:
+  "st,stm32f4-adc"
+  "st,stm32h7-adc"
+  "st,stm32mp1-adc"
+- reg: Offset of ADC instance in ADC block (e.g. may be 0x0, 0x100, 0x200).
+- clocks: Input clock private to this ADC instance. It's required only on
+  stm32f4, that has per instance clock input for registers access.
+- interrupt-parent: Phandle to the parent interrupt controller.
+- interrupts: IRQ Line for the ADC (e.g. may be 0 for adc at 0, 1 for adc at 100 or
+  2 for adc at 200).
+- st,adc-channels: List of single-ended channels muxed for this ADC.
+  It can have up to 16 channels on stm32f4 or 20 channels on stm32h7, numbered
+  from 0 to 15 or 19 (resp. for in0..in15 or in0..in19).
+- st,adc-diff-channels: List of differential channels muxed for this ADC.
+  Depending on part used, some channels can be configured as differential
+  instead of single-ended (e.g. stm32h7). List here positive and negative
+  inputs pairs as <vinp vinn>, <vinp vinn>,... vinp and vinn are numbered
+  from 0 to 19 on stm32h7)
+  Note: At least one of "st,adc-channels" or "st,adc-diff-channels" is required.
+  Both properties can be used together. Some channels can be used as
+  single-ended and some other ones as differential (mixed). But channels
+  can't be configured both as single-ended and differential (invalid).
+- #io-channel-cells = <1>: See the IIO bindings section "IIO consumers" in
+  Documentation/devicetree/bindings/iio/iio-bindings.txt
+
+Optional properties:
+- dmas: Phandle to dma channel for this ADC instance.
+  See ../../dma/dma.txt for details.
+- dma-names: Must be "rx" when dmas property is being used.
+- assigned-resolution-bits: Resolution (bits) to use for conversions. Must
+  match device available resolutions:
+  * can be 6, 8, 10 or 12 on stm32f4
+  * can be 8, 10, 12, 14 or 16 on stm32h7
+  Default is maximum resolution if unset.
+- st,min-sample-time-nsecs: Minimum sampling time in nanoseconds.
+  Depending on hardware (board) e.g. high/low analog input source impedance,
+  fine tune of ADC sampling time may be recommended.
+  This can be either one value or an array that matches 'st,adc-channels' list,
+  to set sample time resp. for all channels, or independently for each channel.
+
+Example:
+	adc: adc at 40012000 {
+		compatible = "st,stm32f4-adc-core";
+		reg = <0x40012000 0x400>;
+		interrupts = <18>;
+		clocks = <&rcc 0 168>;
+		clock-names = "adc";
+		vref-supply = <&reg_vref>;
+		interrupt-controller;
+		pinctrl-names = "default";
+		pinctrl-0 = <&adc3_in8_pin>;
+
+		#interrupt-cells = <1>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		adc at 0 {
+			compatible = "st,stm32f4-adc";
+			#io-channel-cells = <1>;
+			reg = <0x0>;
+			clocks = <&rcc 0 168>;
+			interrupt-parent = <&adc>;
+			interrupts = <0>;
+			st,adc-channels = <8>;
+			dmas = <&dma2 0 0 0x400 0x0>;
+			dma-names = "rx";
+			assigned-resolution-bits = <8>;
+		};
+		...
+		other adc child nodes follow...
+	};
+
+Example to setup:
+- channel 1 as single-ended
+- channels 2 & 3 as differential (with resp. 6 & 7 negative inputs)
+
+	adc: adc at 40022000 {
+		compatible = "st,stm32h7-adc-core";
+		...
+		adc1: adc at 0 {
+			compatible = "st,stm32h7-adc";
+			...
+			st,adc-channels = <1>;
+			st,adc-diff-channels = <2 6>, <3 7>;
+		};
+	};
-- 
1.9.1

[U-Boot] [PATCH 3/6] dt-bindings: Document STM32 ADC DT bindings

[Simon Glass]

On 23 July 2018 at 06:35, Fabrice Gasnier <fabrice.gasnier@st.com> wrote:
> This patch adds documentation of device tree bindings for the STM32 ADC.
> It's based on linux-v4.18-rc* dt-bindings, at the time of writing:
> - Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
>
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
> ---
>
>  doc/device-tree-bindings/adc/st,stm32-adc.txt | 141 ++++++++++++++++++++++++++
>  1 file changed, 141 insertions(+)
>  create mode 100644 doc/device-tree-bindings/adc/st,stm32-adc.txt

Reviewed-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH 4/6] adc: Add driver for STM32 ADC

[Fabrice Gasnier]

This patch adds support for STMicroelectronics STM32 ADC (analog to
digital converter). It's originally based on Linux kernel v4.18-rcs
drivers/iio/adc/stm32-adc*. It's composed of:
- core driver (UCLASS_SIMPLE_BUS) manages common resources (clk, regu).
- child drivers (UCLASS_ADC) declare each ADC, channels and handle
  conversions.
This driver currently supports STM32H7 and STM32MP1 ADC.

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
---

 drivers/adc/Kconfig          |  16 +++
 drivers/adc/Makefile         |   1 +
 drivers/adc/stm32-adc-core.c | 209 +++++++++++++++++++++++++++++++++++
 drivers/adc/stm32-adc-core.h |  51 +++++++++
 drivers/adc/stm32-adc.c      | 257 +++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 534 insertions(+)
 create mode 100644 drivers/adc/stm32-adc-core.c
 create mode 100644 drivers/adc/stm32-adc-core.h
 create mode 100644 drivers/adc/stm32-adc.c

diff --git a/drivers/adc/Kconfig b/drivers/adc/Kconfig
index 93e27f1..e719c38 100644
--- a/drivers/adc/Kconfig
+++ b/drivers/adc/Kconfig
@@ -47,3 +47,19 @@ config SARADC_ROCKCHIP
 	  - 2~6 analog input channels
 	  - 1O or 12 bits resolution
 	  - Up to 1MSPS of sample rate
+
+config STM32_ADC
+	bool "Enable STMicroelectronics STM32 ADC driver"
+	depends on ADC && (STM32H7 || ARCH_STM32MP)
+	help
+	  This enables driver for STMicroelectronics STM32 analog-to-digital
+	  converter (ADC).
+	  A STM32 ADC block can be composed of several individual ADCs.
+	  Each has its own private registers, but shares some resources:
+	  - clock selection and prescaler
+	  - voltage reference
+	  - common registers area.
+	  STM32 ADC driver is composed of:
+	  - core driver to deal with common resources
+	  - child driver to deal with individual ADC resources (declare ADC
+	  device and associated channels, start/stop conversions)
diff --git a/drivers/adc/Makefile b/drivers/adc/Makefile
index 95c93d4..cca0fec 100644
--- a/drivers/adc/Makefile
+++ b/drivers/adc/Makefile
@@ -10,3 +10,4 @@ obj-$(CONFIG_ADC_EXYNOS) += exynos-adc.o
 obj-$(CONFIG_ADC_SANDBOX) += sandbox.o
 obj-$(CONFIG_SARADC_ROCKCHIP) += rockchip-saradc.o
 obj-$(CONFIG_SARADC_MESON) += meson-saradc.o
+obj-$(CONFIG_STM32_ADC) += stm32-adc.o stm32-adc-core.o
diff --git a/drivers/adc/stm32-adc-core.c b/drivers/adc/stm32-adc-core.c
new file mode 100644
index 0000000..a9aa143
--- /dev/null
+++ b/drivers/adc/stm32-adc-core.c
@@ -0,0 +1,209 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2018, STMicroelectronics - All Rights Reserved
+ * Author: Fabrice Gasnier <fabrice.gasnier@st.com>
+ *
+ * Originally based on the Linux kernel v4.18 drivers/iio/adc/stm32-adc-core.c.
+ */
+
+#include <common.h>
+#include <asm/io.h>
+#include <power/regulator.h>
+#include "stm32-adc-core.h"
+
+/* STM32H7 - common registers for all ADC instances */
+#define STM32H7_ADC_CCR			(STM32_ADCX_COMN_OFFSET + 0x08)
+
+/* STM32H7_ADC_CCR - bit fields */
+#define STM32H7_PRESC_SHIFT		18
+#define STM32H7_PRESC_MASK		GENMASK(21, 18)
+#define STM32H7_CKMODE_SHIFT		16
+#define STM32H7_CKMODE_MASK		GENMASK(17, 16)
+
+/* STM32 H7 maximum analog clock rate (from datasheet) */
+#define STM32H7_ADC_MAX_CLK_RATE	36000000
+
+/**
+ * struct stm32h7_adc_ck_spec - specification for stm32h7 adc clock
+ * @ckmode: ADC clock mode, Async or sync with prescaler.
+ * @presc: prescaler bitfield for async clock mode
+ * @div: prescaler division ratio
+ */
+struct stm32h7_adc_ck_spec {
+	u32 ckmode;
+	u32 presc;
+	int div;
+};
+
+static const struct stm32h7_adc_ck_spec stm32h7_adc_ckmodes_spec[] = {
+	/* 00: CK_ADC[1..3]: Asynchronous clock modes */
+	{ 0, 0, 1 },
+	{ 0, 1, 2 },
+	{ 0, 2, 4 },
+	{ 0, 3, 6 },
+	{ 0, 4, 8 },
+	{ 0, 5, 10 },
+	{ 0, 6, 12 },
+	{ 0, 7, 16 },
+	{ 0, 8, 32 },
+	{ 0, 9, 64 },
+	{ 0, 10, 128 },
+	{ 0, 11, 256 },
+	/* HCLK used: Synchronous clock modes (1, 2 or 4 prescaler) */
+	{ 1, 0, 1 },
+	{ 2, 0, 2 },
+	{ 3, 0, 4 },
+};
+
+static int stm32h7_adc_clk_sel(struct udevice *dev,
+			       struct stm32_adc_common *common)
+{
+	u32 ckmode, presc;
+	unsigned long rate;
+	int i, div;
+
+	/* stm32h7 bus clock is common for all ADC instances (mandatory) */
+	if (!clk_valid(&common->bclk)) {
+		dev_err(dev, "No bclk clock found\n");
+		return -ENOENT;
+	}
+
+	/*
+	 * stm32h7 can use either 'bus' or 'adc' clock for analog circuitry.
+	 * So, choice is to have bus clock mandatory and adc clock optional.
+	 * If optional 'adc' clock has been found, then try to use it first.
+	 */
+	if (clk_valid(&common->aclk)) {
+		/*
+		 * Asynchronous clock modes (e.g. ckmode == 0)
+		 * From spec: PLL output musn't exceed max rate
+		 */
+		rate = clk_get_rate(&common->aclk);
+		if (!rate) {
+			dev_err(dev, "Invalid aclk rate: 0\n");
+			return -EINVAL;
+		}
+
+		for (i = 0; i < ARRAY_SIZE(stm32h7_adc_ckmodes_spec); i++) {
+			ckmode = stm32h7_adc_ckmodes_spec[i].ckmode;
+			presc = stm32h7_adc_ckmodes_spec[i].presc;
+			div = stm32h7_adc_ckmodes_spec[i].div;
+
+			if (ckmode)
+				continue;
+
+			if ((rate / div) <= STM32H7_ADC_MAX_CLK_RATE)
+				goto out;
+		}
+	}
+
+	/* Synchronous clock modes (e.g. ckmode is 1, 2 or 3) */
+	rate = clk_get_rate(&common->bclk);
+	if (!rate) {
+		dev_err(dev, "Invalid bus clock rate: 0\n");
+		return -EINVAL;
+	}
+
+	for (i = 0; i < ARRAY_SIZE(stm32h7_adc_ckmodes_spec); i++) {
+		ckmode = stm32h7_adc_ckmodes_spec[i].ckmode;
+		presc = stm32h7_adc_ckmodes_spec[i].presc;
+		div = stm32h7_adc_ckmodes_spec[i].div;
+
+		if (!ckmode)
+			continue;
+
+		if ((rate / div) <= STM32H7_ADC_MAX_CLK_RATE)
+			goto out;
+	}
+
+	dev_err(dev, "clk selection failed\n");
+	return -EINVAL;
+
+out:
+	/* rate used later by each ADC instance to control BOOST mode */
+	common->rate = rate / div;
+
+	/* Set common clock mode and prescaler */
+	clrsetbits_le32(common->base + STM32H7_ADC_CCR,
+			STM32H7_CKMODE_MASK | STM32H7_PRESC_MASK,
+			ckmode << STM32H7_CKMODE_SHIFT |
+			presc << STM32H7_PRESC_SHIFT);
+
+	dev_dbg(dev, "Using %s clock/%d source@%ld kHz\n",
+		ckmode ? "bus" : "adc", div, common->rate / 1000);
+
+	return 0;
+}
+
+static int stm32_adc_core_probe(struct udevice *dev)
+{
+	struct stm32_adc_common *common = dev_get_priv(dev);
+	int ret;
+
+	common->base = dev_read_addr_ptr(dev);
+	if (!common->base) {
+		dev_err(dev, "can't get address\n");
+		return -ENOENT;
+	}
+
+	ret = device_get_supply_regulator(dev, "vref-supply", &common->vref);
+	if (ret) {
+		dev_err(dev, "can't get vref-supply: %d\n", ret);
+		return ret;
+	}
+
+	ret = regulator_get_value(common->vref);
+	if (ret < 0) {
+		dev_err(dev, "can't get vref-supply value: %d\n", ret);
+		return ret;
+	}
+	common->vref_uv = ret;
+
+	ret = clk_get_by_name(dev, "adc", &common->aclk);
+	if (!ret) {
+		ret = clk_enable(&common->aclk);
+		if (ret) {
+			dev_err(dev, "Can't enable aclk: %d\n", ret);
+			return ret;
+		}
+	}
+
+	ret = clk_get_by_name(dev, "bus", &common->bclk);
+	if (!ret) {
+		ret = clk_enable(&common->bclk);
+		if (ret) {
+			dev_err(dev, "Can't enable bclk: %d\n", ret);
+			goto err_aclk_disable;
+		}
+	}
+
+	ret = stm32h7_adc_clk_sel(dev, common);
+	if (ret)
+		goto err_bclk_disable;
+
+	return ret;
+
+err_bclk_disable:
+	if (clk_valid(&common->bclk))
+		clk_disable(&common->bclk);
+
+err_aclk_disable:
+	if (clk_valid(&common->aclk))
+		clk_disable(&common->aclk);
+
+	return ret;
+}
+
+static const struct udevice_id stm32_adc_core_ids[] = {
+	{ .compatible = "st,stm32h7-adc-core" },
+	{ .compatible = "st,stm32mp1-adc-core" },
+	{}
+};
+
+U_BOOT_DRIVER(stm32_adc_core) = {
+	.name  = "stm32-adc-core",
+	.id = UCLASS_SIMPLE_BUS,
+	.of_match = stm32_adc_core_ids,
+	.probe = stm32_adc_core_probe,
+	.priv_auto_alloc_size = sizeof(struct stm32_adc_common),
+};
diff --git a/drivers/adc/stm32-adc-core.h b/drivers/adc/stm32-adc-core.h
new file mode 100644
index 0000000..ba0e10e
--- /dev/null
+++ b/drivers/adc/stm32-adc-core.h
@@ -0,0 +1,51 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2018, STMicroelectronics - All Rights Reserved
+ * Author: Fabrice Gasnier <fabrice.gasnier@st.com>.
+ *
+ * Originally based on the Linux kernel v4.18 drivers/iio/adc/stm32-adc-core.h.
+ */
+
+#ifndef __STM32_ADC_H
+#define __STM32_ADC_H
+
+/*
+ * STM32 - ADC global register map
+ * ________________________________________________________
+ * | Offset |                 Register                    |
+ * --------------------------------------------------------
+ * | 0x000  |                Master ADC1                  |
+ * --------------------------------------------------------
+ * | 0x100  |                Slave ADC2                   |
+ * --------------------------------------------------------
+ * | 0x200  |                Slave ADC3                   |
+ * --------------------------------------------------------
+ * | 0x300  |         Master & Slave common regs          |
+ * --------------------------------------------------------
+ */
+#define STM32_ADC_MAX_ADCS		3
+#define STM32_ADCX_COMN_OFFSET		0x300
+
+#include <common.h>
+#include <clk.h>
+#include <dm.h>
+
+/**
+ * struct stm32_adc_common - stm32 ADC driver common data (for all instances)
+ * @base:		control registers base cpu addr
+ * @rate:		clock rate used for analog circuitry
+ * @aclk:		clock for the analog circuitry
+ * @bclk:		bus clock common for all ADCs
+ * @vref:		regulator reference
+ * @vref_uv:		reference supply voltage (uV)
+ */
+struct stm32_adc_common {
+	void __iomem *base;
+	unsigned long rate;
+	struct clk aclk;
+	struct clk bclk;
+	struct udevice *vref;
+	int vref_uv;
+};
+
+#endif
diff --git a/drivers/adc/stm32-adc.c b/drivers/adc/stm32-adc.c
new file mode 100644
index 0000000..e108062
--- /dev/null
+++ b/drivers/adc/stm32-adc.c
@@ -0,0 +1,257 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2018, STMicroelectronics - All Rights Reserved
+ * Author: Fabrice Gasnier <fabrice.gasnier@st.com>
+ *
+ * Originally based on the Linux kernel v4.18 drivers/iio/adc/stm32-adc.c.
+ */
+
+#include <common.h>
+#include <adc.h>
+#include <asm/io.h>
+#include <linux/iopoll.h>
+#include "stm32-adc-core.h"
+
+/* STM32H7 - Registers for each ADC instance */
+#define STM32H7_ADC_ISR			0x00
+#define STM32H7_ADC_CR			0x08
+#define STM32H7_ADC_CFGR		0x0C
+#define STM32H7_ADC_SMPR1		0x14
+#define STM32H7_ADC_SMPR2		0x18
+#define STM32H7_ADC_PCSEL		0x1C
+#define STM32H7_ADC_SQR1		0x30
+#define STM32H7_ADC_DR			0x40
+#define STM32H7_ADC_DIFSEL		0xC0
+
+/* STM32H7_ADC_ISR - bit fields */
+#define STM32MP1_VREGREADY		BIT(12)
+#define STM32H7_EOC			BIT(2)
+#define STM32H7_ADRDY			BIT(0)
+
+/* STM32H7_ADC_CR - bit fields */
+#define STM32H7_DEEPPWD			BIT(29)
+#define STM32H7_ADVREGEN		BIT(28)
+#define STM32H7_BOOST			BIT(8)
+#define STM32H7_ADSTART			BIT(2)
+#define STM32H7_ADDIS			BIT(1)
+#define STM32H7_ADEN			BIT(0)
+
+/* STM32H7_ADC_CFGR bit fields */
+#define STM32H7_EXTEN			GENMASK(11, 10)
+#define STM32H7_DMNGT			GENMASK(1, 0)
+
+/* STM32H7_ADC_SQR1 - bit fields */
+#define STM32H7_SQ1_SHIFT		6
+
+/* BOOST bit must be set on STM32H7 when ADC clock is above 20MHz */
+#define STM32H7_BOOST_CLKRATE		20000000UL
+
+#define STM32_ADC_CH_MAX		20	/* max number of channels */
+#define STM32_ADC_TIMEOUT_US		100000
+
+struct stm32_adc_cfg {
+	unsigned int max_channels;
+	unsigned int num_bits;
+	bool has_vregready;
+};
+
+struct stm32_adc {
+	void __iomem *regs;
+	int active_channel;
+	const struct stm32_adc_cfg *cfg;
+};
+
+static int stm32_adc_stop(struct udevice *dev)
+{
+	struct stm32_adc *adc = dev_get_priv(dev);
+
+	setbits_le32(adc->regs + STM32H7_ADC_CR, STM32H7_ADDIS);
+	clrbits_le32(adc->regs + STM32H7_ADC_CR, STM32H7_BOOST);
+	/* Setting DEEPPWD disables ADC vreg and clears ADVREGEN */
+	setbits_le32(adc->regs + STM32H7_ADC_CR, STM32H7_DEEPPWD);
+	adc->active_channel = -1;
+
+	return 0;
+}
+
+static int stm32_adc_start_channel(struct udevice *dev, int channel)
+{
+	struct adc_uclass_platdata *uc_pdata = dev_get_uclass_platdata(dev);
+	struct stm32_adc_common *common = dev_get_priv(dev_get_parent(dev));
+	struct stm32_adc *adc = dev_get_priv(dev);
+	int ret;
+	u32 val;
+
+	/* Exit deep power down, then enable ADC voltage regulator */
+	clrbits_le32(adc->regs + STM32H7_ADC_CR, STM32H7_DEEPPWD);
+	setbits_le32(adc->regs + STM32H7_ADC_CR, STM32H7_ADVREGEN);
+	if (common->rate > STM32H7_BOOST_CLKRATE)
+		setbits_le32(adc->regs + STM32H7_ADC_CR, STM32H7_BOOST);
+
+	/* Wait for startup time */
+	if (!adc->cfg->has_vregready) {
+		udelay(20);
+	} else {
+		ret = readl_poll_timeout(adc->regs + STM32H7_ADC_ISR, val,
+					 val & STM32MP1_VREGREADY,
+					 STM32_ADC_TIMEOUT_US);
+		if (ret < 0) {
+			stm32_adc_stop(dev);
+			dev_err(dev, "Failed to enable vreg: %d\n", ret);
+			return ret;
+		}
+	}
+
+	/* Only use single ended channels */
+	writel(0, adc->regs + STM32H7_ADC_DIFSEL);
+
+	/* Enable ADC, Poll for ADRDY to be set (after adc startup time) */
+	setbits_le32(adc->regs + STM32H7_ADC_CR, STM32H7_ADEN);
+	ret = readl_poll_timeout(adc->regs + STM32H7_ADC_ISR, val,
+				 val & STM32H7_ADRDY, STM32_ADC_TIMEOUT_US);
+	if (ret < 0) {
+		stm32_adc_stop(dev);
+		dev_err(dev, "Failed to enable ADC: %d\n", ret);
+		return ret;
+	}
+
+	/* Preselect channels */
+	writel(uc_pdata->channel_mask, adc->regs + STM32H7_ADC_PCSEL);
+
+	/* Set sampling time to max value by default */
+	writel(0xffffffff, adc->regs + STM32H7_ADC_SMPR1);
+	writel(0xffffffff, adc->regs + STM32H7_ADC_SMPR2);
+
+	/* Program regular sequence: chan in SQ1 & len = 0 for one channel */
+	writel(channel << STM32H7_SQ1_SHIFT, adc->regs + STM32H7_ADC_SQR1);
+
+	/* Trigger detection disabled (conversion can be launched in SW) */
+	clrbits_le32(adc->regs + STM32H7_ADC_CFGR, STM32H7_EXTEN |
+		     STM32H7_DMNGT);
+	adc->active_channel = channel;
+
+	return 0;
+}
+
+static int stm32_adc_channel_data(struct udevice *dev, int channel,
+				  unsigned int *data)
+{
+	struct stm32_adc *adc = dev_get_priv(dev);
+	int ret;
+	u32 val;
+
+	if (channel != adc->active_channel) {
+		dev_err(dev, "Requested channel is not active!\n");
+		return -EINVAL;
+	}
+
+	setbits_le32(adc->regs + STM32H7_ADC_CR, STM32H7_ADSTART);
+	ret = readl_poll_timeout(adc->regs + STM32H7_ADC_ISR, val,
+				 val & STM32H7_EOC, STM32_ADC_TIMEOUT_US);
+	if (ret < 0) {
+		dev_err(dev, "conversion timed out: %d\n", ret);
+		return ret;
+	}
+
+	*data = readl(adc->regs + STM32H7_ADC_DR);
+
+	return 0;
+}
+
+static int stm32_adc_chan_of_init(struct udevice *dev)
+{
+	struct adc_uclass_platdata *uc_pdata = dev_get_uclass_platdata(dev);
+	struct stm32_adc *adc = dev_get_priv(dev);
+	u32 chans[STM32_ADC_CH_MAX];
+	int i, num_channels, ret;
+
+	/* Retrieve single ended channels listed in device tree */
+	num_channels = dev_read_size(dev, "st,adc-channels");
+	if (num_channels < 0) {
+		dev_err(dev, "can't get st,adc-channels: %d\n", num_channels);
+		return num_channels;
+	}
+	num_channels /= sizeof(u32);
+
+	if (num_channels > adc->cfg->max_channels) {
+		dev_err(dev, "too many st,adc-channels: %d\n", num_channels);
+		return -EINVAL;
+	}
+
+	ret = dev_read_u32_array(dev, "st,adc-channels", chans, num_channels);
+	if (ret < 0) {
+		dev_err(dev, "can't read st,adc-channels: %d\n", ret);
+		return ret;
+	}
+
+	for (i = 0; i < num_channels; i++) {
+		if (chans[i] >= adc->cfg->max_channels) {
+			dev_err(dev, "bad channel %u\n", chans[i]);
+			return -EINVAL;
+		}
+		uc_pdata->channel_mask |= 1 << chans[i];
+	}
+
+	uc_pdata->data_mask = (1 << adc->cfg->num_bits) - 1;
+	uc_pdata->data_format = ADC_DATA_FORMAT_BIN;
+	uc_pdata->data_timeout_us = 100000;
+
+	return 0;
+}
+
+static int stm32_adc_probe(struct udevice *dev)
+{
+	struct adc_uclass_platdata *uc_pdata = dev_get_uclass_platdata(dev);
+	struct stm32_adc_common *common = dev_get_priv(dev_get_parent(dev));
+	struct stm32_adc *adc = dev_get_priv(dev);
+	int offset;
+
+	offset = dev_read_u32_default(dev, "reg", -ENODATA);
+	if (offset < 0) {
+		dev_err(dev, "Can't read reg property\n");
+		return offset;
+	}
+	adc->regs = common->base + offset;
+	adc->cfg = (const struct stm32_adc_cfg *)dev_get_driver_data(dev);
+
+	/* VDD supplied by common vref pin */
+	uc_pdata->vdd_supply = common->vref;
+	uc_pdata->vdd_microvolts = common->vref_uv;
+	uc_pdata->vss_microvolts = 0;
+
+	return stm32_adc_chan_of_init(dev);
+}
+
+static const struct adc_ops stm32_adc_ops = {
+	.start_channel = stm32_adc_start_channel,
+	.channel_data = stm32_adc_channel_data,
+	.stop = stm32_adc_stop,
+};
+
+static const struct stm32_adc_cfg stm32h7_adc_cfg = {
+	.num_bits = 16,
+	.max_channels = STM32_ADC_CH_MAX,
+};
+
+static const struct stm32_adc_cfg stm32mp1_adc_cfg = {
+	.num_bits = 16,
+	.max_channels = STM32_ADC_CH_MAX,
+	.has_vregready = true,
+};
+
+static const struct udevice_id stm32_adc_ids[] = {
+	{ .compatible = "st,stm32h7-adc",
+	  .data = (ulong)&stm32h7_adc_cfg },
+	{ .compatible = "st,stm32mp1-adc",
+	  .data = (ulong)&stm32mp1_adc_cfg },
+	{}
+};
+
+U_BOOT_DRIVER(stm32_adc) = {
+	.name  = "stm32-adc",
+	.id = UCLASS_ADC,
+	.of_match = stm32_adc_ids,
+	.probe = stm32_adc_probe,
+	.ops = &stm32_adc_ops,
+	.priv_auto_alloc_size = sizeof(struct stm32_adc),
+};
-- 
1.9.1

[U-Boot] [PATCH 4/6] adc: Add driver for STM32 ADC

[Simon Glass]

Hi Fabrice,

On 23 July 2018 at 06:35, Fabrice Gasnier <fabrice.gasnier@st.com> wrote:
> This patch adds support for STMicroelectronics STM32 ADC (analog to
> digital converter). It's originally based on Linux kernel v4.18-rcs
> drivers/iio/adc/stm32-adc*. It's composed of:
> - core driver (UCLASS_SIMPLE_BUS) manages common resources (clk, regu).
> - child drivers (UCLASS_ADC) declare each ADC, channels and handle
>   conversions.
> This driver currently supports STM32H7 and STM32MP1 ADC.
>
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
> ---
>
>  drivers/adc/Kconfig          |  16 +++
>  drivers/adc/Makefile         |   1 +
>  drivers/adc/stm32-adc-core.c | 209 +++++++++++++++++++++++++++++++++++
>  drivers/adc/stm32-adc-core.h |  51 +++++++++
>  drivers/adc/stm32-adc.c      | 257 +++++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 534 insertions(+)
>  create mode 100644 drivers/adc/stm32-adc-core.c
>  create mode 100644 drivers/adc/stm32-adc-core.h
>  create mode 100644 drivers/adc/stm32-adc.c

Reviewed-by: Simon Glass <sjg@chromium.org>

I do worry a bit about the code bloat caused by the dev_err() calls.
Don't you want to use debug() instead? Or perhaps log() and
log_msg_ret()?

Regards,
Simon

[U-Boot] [PATCH 5/6] configs: stm32mp15: enable ADC

[Fabrice Gasnier]

Enable ADC on stm32mp15.
- CONFIG_CMD_ADC
- CONFIG_STM32_ADC

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
---

 configs/stm32mp15_basic_defconfig | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/configs/stm32mp15_basic_defconfig b/configs/stm32mp15_basic_defconfig
index 3a94db5..bbb65b5 100644
--- a/configs/stm32mp15_basic_defconfig
+++ b/configs/stm32mp15_basic_defconfig
@@ -18,6 +18,7 @@ CONFIG_SYS_PROMPT="STM32MP> "
 # CONFIG_CMD_EXPORTENV is not set
 # CONFIG_CMD_IMPORTENV is not set
 CONFIG_CMD_MEMINFO=y
+CONFIG_CMD_ADC=y
 CONFIG_CMD_FUSE=y
 CONFIG_CMD_GPIO=y
 CONFIG_CMD_GPT=y
@@ -27,6 +28,7 @@ CONFIG_CMD_PMIC=y
 CONFIG_CMD_REGULATOR=y
 CONFIG_CMD_EXT4_WRITE=y
 # CONFIG_SPL_DOS_PARTITION is not set
+CONFIG_STM32_ADC=y
 CONFIG_DM_I2C=y
 CONFIG_SYS_I2C_STM32F7=y
 CONFIG_DM_MMC=y
@@ -35,7 +37,6 @@ CONFIG_STM32_SDMMC2=y
 CONFIG_DM_PMIC=y
 # CONFIG_SPL_PMIC_CHILDREN is not set
 CONFIG_PMIC_STPMU1=y
-CONFIG_DM_REGULATOR=y
 CONFIG_DM_REGULATOR_GPIO=y
 CONFIG_DM_REGULATOR_STM32_VREFBUF=y
 CONFIG_DM_REGULATOR_STPMU1=y
-- 
1.9.1

[U-Boot] [PATCH 5/6] configs: stm32mp15: enable ADC

[Simon Glass]

On 23 July 2018 at 06:35, Fabrice Gasnier <fabrice.gasnier@st.com> wrote:
> Enable ADC on stm32mp15.
> - CONFIG_CMD_ADC
> - CONFIG_STM32_ADC
>
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
> ---
>
>  configs/stm32mp15_basic_defconfig | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH 6/6] ARM: dts: stm32mp157: Add ADC DT node

[Fabrice Gasnier]

Add ADC device tree node. This allows to get analog conversions on
stm32mp157.

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
---

 arch/arm/dts/stm32mp157.dtsi | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/arch/arm/dts/stm32mp157.dtsi b/arch/arm/dts/stm32mp157.dtsi
index 2b89416..88b5460 100644
--- a/arch/arm/dts/stm32mp157.dtsi
+++ b/arch/arm/dts/stm32mp157.dtsi
@@ -86,6 +86,38 @@
 			status = "disabled";
 		};
 
+		adc: adc at 48003000 {
+			compatible = "st,stm32mp1-adc-core";
+			reg = <0x48003000 0x400>;
+			interrupts = <GIC_SPI 18 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 90 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&rcc_clk ADC12>, <&rcc_clk ADC12_K>;
+			clock-names = "bus", "adc";
+			interrupt-controller;
+			#interrupt-cells = <1>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			status = "disabled";
+
+			adc1: adc at 0 {
+				compatible = "st,stm32mp1-adc";
+				#io-channel-cells = <1>;
+				reg = <0x0>;
+				interrupt-parent = <&adc>;
+				interrupts = <0>;
+				status = "disabled";
+			};
+
+			adc2: adc at 100 {
+				compatible = "st,stm32mp1-adc";
+				#io-channel-cells = <1>;
+				reg = <0x100>;
+				interrupt-parent = <&adc>;
+				interrupts = <1>;
+				status = "disabled";
+			};
+		};
+
 		sdmmc3: sdmmc at 48004000 {
 			compatible = "st,stm32-sdmmc2";
 			reg = <0x48004000 0x400>, <0x48005000 0x400>;
-- 
1.9.1

[U-Boot] [PATCH 1/6] clk: add clk_valid()

[Simon Glass]

Hi Fabrice,

On 23 July 2018 at 06:35, Fabrice Gasnier <fabrice.gasnier@st.com> wrote:
>
> add clk_valid() to check for optional clocks are valid.
>
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
> ---
>
>  include/clk.h | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/include/clk.h b/include/clk.h
> index 9a35764..71679a9 100644
> --- a/include/clk.h
> +++ b/include/clk.h
> @@ -294,4 +294,14 @@ int clk_disable_bulk(struct clk_bulk *bulk);
>
>  int soc_clk_dump(void);
>
> +/**
> + * clk_valid() - check if clk is valid
> + *
> + * @clk:       the clock to check
> + * @return TRUE if valid, or FALSE

true / false

> + */
> +static inline bool clk_valid(struct clk *clk)
> +{
> +       return !!clk->dev;
> +}
>  #endif
> --
> 1.9.1
>

Please can you add a call to this from test/dm/clk.c ?

Regards,
Simon