[PATCH 0/3] STM32 ADC analog switches supply control

[PATCH 0/3] STM32 ADC analog switches supply control

[Fabrice Gasnier]

This series adds support for SYSCFG bits that control ADC analog switches
supply on STM32MP1 and STM32H7.

The ADC inputs are multiplexed with analog switches which have reduced
performances when their supply is below 2.7V. Analog switches supply
can be controlled using SYSCFG bits, to reach full ADC performance.

Fabrice Gasnier (3):
  dt-bindings: iio: adc: stm32: add analog switches supply control
  iio: adc: stm32-adc: add analog switches supply control
  ARM: dts: stm32: add ADC analog switches syscfg on stm32mp157c

 .../devicetree/bindings/iio/adc/st,stm32-adc.txt   |   6 +
 arch/arm/boot/dts/stm32mp157c.dtsi                 |   1 +
 drivers/iio/adc/stm32-adc-core.c                   | 232 ++++++++++++++++++++-
 3 files changed, 237 insertions(+), 2 deletions(-)

-- 
2.7.4


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

[PATCH 1/3] dt-bindings: iio: adc: stm32: add analog switches supply control

[Fabrice Gasnier]

On stm32h7 and stm32mp1, the ADC inputs are multiplexed with analog
switches which have reduced performances when their supply is below 2.7V
(vdda by default).

Add documentation for optional vdda-supply & vdd-supply that can be used
to supply analog circuitry (controlled by syscfg bits).

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
---
 Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt b/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
index 8346bcb..3af48b9 100644
--- a/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
+++ b/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
@@ -46,6 +46,12 @@ Required properties:
 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.
+- vdda-supply: Phandle to the vdda input voltage. It can be used to supply ADC
+  analog inputs switches on stm32h7 and stm32mp1.
+- vdd-supply: Phandle to the vdd input voltage. It can be used to supply ADC
+  analog inputs switches on stm32mp1.
+- st,syscfg: Phandle to system configuration controller. It can be used to
+  control the analog circuitry on stm32h7 and stm32mp1.
 
 Contents of a stm32 adc child node:
 -----------------------------------
-- 
2.7.4


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

Re: [PATCH 1/3] dt-bindings: iio: adc: stm32: add analog switches supply control

[Jonathan Cameron]

On Wed, 12 Jun 2019 09:24:34 +0200
Fabrice Gasnier <fabrice.gasnier@st.com> wrote:

> On stm32h7 and stm32mp1, the ADC inputs are multiplexed with analog
> switches which have reduced performances when their supply is below 2.7V
> (vdda by default).
> 
> Add documentation for optional vdda-supply & vdd-supply that can be used
> to supply analog circuitry (controlled by syscfg bits).
> 
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
> ---
>  Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt b/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
> index 8346bcb..3af48b9 100644
> --- a/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
> +++ b/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt
> @@ -46,6 +46,12 @@ Required properties:
>  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.
> +- vdda-supply: Phandle to the vdda input voltage. It can be used to supply ADC
> +  analog inputs switches on stm32h7 and stm32mp1.

input switches

> +- vdd-supply: Phandle to the vdd input voltage. It can be used to supply ADC
> +  analog inputs switches on stm32mp1.
> +- st,syscfg: Phandle to system configuration controller. It can be used to
> +  control the analog circuitry on stm32h7 and stm32mp1.
>  
>  Contents of a stm32 adc child node:
>  -----------------------------------


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

[PATCH 2/3] iio: adc: stm32-adc: add analog switches supply control

[Fabrice Gasnier]

On stm32h7 and stm32mp1, the ADC inputs are multiplexed with analog
switches which have reduced performances when their supply is below 2.7V
(vdda by default):
- vdd supply can be selected if above 2.7V by setting ANASWVDD syscfg bit
  (STM32MP1 only).
- Voltage booster can be used, to get full ADC performances by setting
  BOOSTE/EN_BOOSTER syscfg bit (increases power consumption).

Make this optional, since this is a trade-off between analog performance
and power consumption.

Note: STM32H7 syscfg has a set and clear register for "BOOSTE" control.
STM32MP1 has separate set and clear registers pair to control EN_BOOSTER
and ANASWVDD bits.

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
---
 drivers/iio/adc/stm32-adc-core.c | 232 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 230 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/adc/stm32-adc-core.c b/drivers/iio/adc/stm32-adc-core.c
index 2327ec1..9d41b16 100644
--- a/drivers/iio/adc/stm32-adc-core.c
+++ b/drivers/iio/adc/stm32-adc-core.c
@@ -14,9 +14,11 @@
 #include <linux/irqchip/chained_irq.h>
 #include <linux/irqdesc.h>
 #include <linux/irqdomain.h>
+#include <linux/mfd/syscon.h>
 #include <linux/module.h>
 #include <linux/of_device.h>
 #include <linux/pm_runtime.h>
+#include <linux/regmap.h>
 #include <linux/regulator/consumer.h>
 #include <linux/slab.h>
 
@@ -51,6 +53,20 @@
 
 #define STM32_ADC_CORE_SLEEP_DELAY_MS	2000
 
+/* SYSCFG registers */
+#define STM32H7_SYSCFG_PMCR		0x04
+#define STM32MP1_SYSCFG_PMCSETR		0x04
+#define STM32MP1_SYSCFG_PMCCLRR		0x44
+
+/* SYSCFG bit fields */
+#define STM32H7_SYSCFG_BOOSTE_MASK	BIT(8)
+#define STM32MP1_SYSCFG_ANASWVDD_MASK	BIT(9)
+
+/* SYSCFG capability flags */
+#define HAS_VBOOSTER		BIT(0)
+#define HAS_ANASWVDD		BIT(1)
+#define HAS_CLEAR_REG		BIT(2)
+
 /**
  * stm32_adc_common_regs - stm32 common registers, compatible dependent data
  * @csr:	common status register offset
@@ -58,6 +74,11 @@
  * @eoc1:	adc1 end of conversion flag in @csr
  * @eoc2:	adc2 end of conversion flag in @csr
  * @eoc3:	adc3 end of conversion flag in @csr
+ * @has_syscfg: SYSCFG capability flags
+ * @pmcr:	SYSCFG_PMCSETR/SYSCFG_PMCR register offset
+ * @pmcc:	SYSCFG_PMCCLRR clear register offset
+ * @booste_msk:	SYSCFG BOOSTE / EN_BOOSTER bitmask in PMCR & PMCCLRR
+ * @anaswvdd_msk: SYSCFG ANASWVDD bitmask in PMCR & PMCCLRR
  */
 struct stm32_adc_common_regs {
 	u32 csr;
@@ -65,6 +86,11 @@ struct stm32_adc_common_regs {
 	u32 eoc1_msk;
 	u32 eoc2_msk;
 	u32 eoc3_msk;
+	unsigned int has_syscfg;
+	u32 pmcr;
+	u32 pmcc;
+	u32 booste_msk;
+	u32 anaswvdd_msk;
 };
 
 struct stm32_adc_priv;
@@ -87,20 +113,26 @@ struct stm32_adc_priv_cfg {
  * @domain:		irq domain reference
  * @aclk:		clock reference for the analog circuitry
  * @bclk:		bus clock common for all ADCs, depends on part used
+ * @vdd:		vdd supply reference
+ * @vdda:		vdda supply reference
  * @vref:		regulator reference
  * @cfg:		compatible configuration data
  * @common:		common data for all ADC instances
  * @ccr_bak:		backup CCR in low power mode
+ * @syscfg:		reference to syscon, system control registers
  */
 struct stm32_adc_priv {
 	int				irq[STM32_ADC_MAX_ADCS];
 	struct irq_domain		*domain;
 	struct clk			*aclk;
 	struct clk			*bclk;
+	struct regulator		*vdd;
+	struct regulator		*vdda;
 	struct regulator		*vref;
 	const struct stm32_adc_priv_cfg	*cfg;
 	struct stm32_adc_common		common;
 	u32				ccr_bak;
+	struct regmap			*syscfg;
 };
 
 static struct stm32_adc_priv *to_stm32_adc_priv(struct stm32_adc_common *com)
@@ -284,6 +316,22 @@ static const struct stm32_adc_common_regs stm32h7_adc_common_regs = {
 	.ccr = STM32H7_ADC_CCR,
 	.eoc1_msk = STM32H7_EOC_MST,
 	.eoc2_msk = STM32H7_EOC_SLV,
+	.has_syscfg = HAS_VBOOSTER,
+	.pmcr = STM32H7_SYSCFG_PMCR,
+	.booste_msk = STM32H7_SYSCFG_BOOSTE_MASK,
+};
+
+/* STM32MP1 common registers definitions */
+static const struct stm32_adc_common_regs stm32mp1_adc_common_regs = {
+	.csr = STM32H7_ADC_CSR,
+	.ccr = STM32H7_ADC_CCR,
+	.eoc1_msk = STM32H7_EOC_MST,
+	.eoc2_msk = STM32H7_EOC_SLV,
+	.has_syscfg =  HAS_VBOOSTER | HAS_ANASWVDD | HAS_CLEAR_REG,
+	.pmcr = STM32MP1_SYSCFG_PMCSETR,
+	.pmcc = STM32MP1_SYSCFG_PMCCLRR,
+	.booste_msk = STM32H7_SYSCFG_BOOSTE_MASK,
+	.anaswvdd_msk = STM32MP1_SYSCFG_ANASWVDD_MASK,
 };
 
 /* ADC common interrupt for all instances */
@@ -388,16 +436,145 @@ static void stm32_adc_irq_remove(struct platform_device *pdev,
 	}
 }
 
+static int stm32_adc_core_switches_supply_en(struct device *dev)
+{
+	struct stm32_adc_common *common = dev_get_drvdata(dev);
+	struct stm32_adc_priv *priv = to_stm32_adc_priv(common);
+	const struct stm32_adc_common_regs *regs = priv->cfg->regs;
+	int ret, vdda, vdd = 0;
+	u32 mask, clrmask, setmask = 0;
+
+	/*
+	 * On STM32H7 and STM32MP1, the ADC inputs are multiplexed with analog
+	 * switches (via PCSEL) which have reduced performances when their
+	 * supply is below 2.7V (vdda by default):
+	 * - Voltage booster can be used, to get full ADC performances
+	 *   (increases power consumption).
+	 * - Vdd can be used to supply them, if above 2.7V (STM32MP1 only).
+	 *
+	 * This is optional, as this is a trade-off between analog performance
+	 * and power consumption.
+	 */
+	if (!regs->has_syscfg || !priv->vdda || !priv->syscfg) {
+		dev_dbg(dev, "Not configuring analog switches\n");
+		return 0;
+	}
+
+	ret = regulator_enable(priv->vdda);
+	if (ret < 0) {
+		dev_err(dev, "vdda enable failed %d\n", ret);
+		return ret;
+	}
+
+	ret = regulator_get_voltage(priv->vdda);
+	if (ret < 0) {
+		dev_err(dev, "vdda get voltage failed %d\n", ret);
+		goto vdda_dis;
+	}
+	vdda = ret;
+
+	if (priv->vdd && regs->has_syscfg & HAS_ANASWVDD) {
+		ret = regulator_enable(priv->vdd);
+		if (ret < 0) {
+			dev_err(dev, "vdd enable failed %d\n", ret);
+			goto vdda_dis;
+		}
+
+		ret = regulator_get_voltage(priv->vdd);
+		if (ret < 0) {
+			dev_err(dev, "vdd get voltage failed %d\n", ret);
+			goto vdd_dis;
+		}
+		vdd = ret;
+	}
+
+	/*
+	 * Recommended settings for ANASWVDD and EN_BOOSTER:
+	 * - vdda < 2.7V but vdd > 2.7V: ANASWVDD = 1, EN_BOOSTER = 0 (stm32mp1)
+	 * - vdda < 2.7V and vdd < 2.7V: ANASWVDD = 0, EN_BOOSTER = 1
+	 * - vdda >= 2.7V:               ANASWVDD = 0, EN_BOOSTER = 0 (default)
+	 */
+	if (vdda < 2700000) {
+		if (vdd > 2700000) {
+			dev_dbg(dev, "analog switches supplied by vdd\n");
+			setmask = regs->anaswvdd_msk;
+			clrmask = regs->booste_msk;
+		} else {
+			dev_dbg(dev, "Enabling voltage booster\n");
+			setmask = regs->booste_msk;
+			clrmask = regs->anaswvdd_msk;
+		}
+	} else {
+		dev_dbg(dev, "analog switches supplied by vdda\n");
+		clrmask = regs->booste_msk | regs->anaswvdd_msk;
+	}
+
+	mask = regs->booste_msk | regs->anaswvdd_msk;
+	if (regs->has_syscfg & HAS_CLEAR_REG) {
+		ret = regmap_write(priv->syscfg, regs->pmcc, clrmask);
+		if (ret) {
+			dev_err(dev, "syscfg clear failed, %d\n", ret);
+			goto vdd_dis;
+		}
+		mask = setmask;
+	}
+
+	ret = regmap_update_bits(priv->syscfg, regs->pmcr, mask, setmask);
+	if (ret) {
+		dev_err(dev, "syscfg update failed, %d\n", ret);
+		goto vdd_dis;
+	}
+
+	/* Booster voltage can take up to 50 us to stabilize */
+	if (setmask & regs->booste_msk)
+		usleep_range(50, 100);
+
+	return ret;
+
+vdd_dis:
+	if (priv->vdd && (regs->has_syscfg & HAS_ANASWVDD))
+		regulator_disable(priv->vdd);
+vdda_dis:
+	regulator_disable(priv->vdda);
+
+	return ret;
+}
+
+static void stm32_adc_core_switches_supply_dis(struct device *dev)
+{
+	struct stm32_adc_common *common = dev_get_drvdata(dev);
+	struct stm32_adc_priv *priv = to_stm32_adc_priv(common);
+	const struct stm32_adc_common_regs *regs = priv->cfg->regs;
+	u32 mask = regs->booste_msk | regs->anaswvdd_msk;
+
+	if (!regs->has_syscfg || !priv->vdda || !priv->syscfg)
+		return;
+
+	if (regs->has_syscfg & HAS_CLEAR_REG)
+		regmap_write(priv->syscfg, regs->pmcc, mask);
+	else
+		regmap_update_bits(priv->syscfg, regs->pmcr, mask, 0);
+
+	if (priv->vdd && (regs->has_syscfg & HAS_ANASWVDD))
+		regulator_disable(priv->vdd);
+
+	regulator_disable(priv->vdda);
+}
+
 static int stm32_adc_core_hw_start(struct device *dev)
 {
 	struct stm32_adc_common *common = dev_get_drvdata(dev);
 	struct stm32_adc_priv *priv = to_stm32_adc_priv(common);
 	int ret;
 
+	ret = stm32_adc_core_switches_supply_en(dev);
+	if (ret < 0)
+		return ret;
+
 	ret = regulator_enable(priv->vref);
 	if (ret < 0) {
 		dev_err(dev, "vref enable failed\n");
-		return ret;
+		goto err_switches_disable;
 	}
 
 	if (priv->bclk) {
@@ -425,6 +602,8 @@ static int stm32_adc_core_hw_start(struct device *dev)
 		clk_disable_unprepare(priv->bclk);
 err_regulator_disable:
 	regulator_disable(priv->vref);
+err_switches_disable:
+	stm32_adc_core_switches_supply_dis(dev);
 
 	return ret;
 }
@@ -441,6 +620,24 @@ static void stm32_adc_core_hw_stop(struct device *dev)
 	if (priv->bclk)
 		clk_disable_unprepare(priv->bclk);
 	regulator_disable(priv->vref);
+	stm32_adc_core_switches_supply_dis(dev);
+}
+
+static int stm32_adc_core_syscfg_probe(struct device_node *np,
+				       struct stm32_adc_priv *priv)
+{
+	if (!priv->cfg->regs->has_syscfg)
+		return 0;
+
+	priv->syscfg = syscon_regmap_lookup_by_phandle(np, "st,syscfg");
+	if (IS_ERR(priv->syscfg)) {
+		/* Optional */
+		if (PTR_ERR(priv->syscfg) != -ENODEV)
+			return PTR_ERR(priv->syscfg);
+		priv->syscfg = NULL;
+	}
+
+	return 0;
 }
 
 static int stm32_adc_probe(struct platform_device *pdev)
@@ -475,6 +672,30 @@ static int stm32_adc_probe(struct platform_device *pdev)
 		return ret;
 	}
 
+	priv->vdda = devm_regulator_get_optional(&pdev->dev, "vdda");
+	if (IS_ERR(priv->vdda)) {
+		ret = PTR_ERR(priv->vdda);
+		if (ret != -ENODEV) {
+			if (ret != -EPROBE_DEFER)
+				dev_err(&pdev->dev, "vdda get failed, %d\n",
+					ret);
+			return ret;
+		}
+		priv->vdda = NULL;
+	}
+
+	priv->vdd = devm_regulator_get_optional(&pdev->dev, "vdd");
+	if (IS_ERR(priv->vdd)) {
+		ret = PTR_ERR(priv->vdd);
+		if (ret != -ENODEV) {
+			if (ret != -EPROBE_DEFER)
+				dev_err(&pdev->dev, "vdd get failed, %d\n",
+					ret);
+			return ret;
+		}
+		priv->vdd = NULL;
+	}
+
 	priv->aclk = devm_clk_get(&pdev->dev, "adc");
 	if (IS_ERR(priv->aclk)) {
 		ret = PTR_ERR(priv->aclk);
@@ -495,6 +716,13 @@ static int stm32_adc_probe(struct platform_device *pdev)
 		priv->bclk = NULL;
 	}
 
+	ret = stm32_adc_core_syscfg_probe(np, priv);
+	if (ret) {
+		if (ret != -EPROBE_DEFER)
+			dev_err(&pdev->dev, "Can't probe syscfg: %d\n", ret);
+		return ret;
+	}
+
 	pm_runtime_get_noresume(dev);
 	pm_runtime_set_active(dev);
 	pm_runtime_set_autosuspend_delay(dev, STM32_ADC_CORE_SLEEP_DELAY_MS);
@@ -595,7 +823,7 @@ static const struct stm32_adc_priv_cfg stm32h7_adc_priv_cfg = {
 };
 
 static const struct stm32_adc_priv_cfg stm32mp1_adc_priv_cfg = {
-	.regs = &stm32h7_adc_common_regs,
+	.regs = &stm32mp1_adc_common_regs,
 	.clk_sel = stm32h7_adc_clk_sel,
 	.max_clk_rate_hz = 40000000,
 };
-- 
2.7.4


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

Re: [PATCH 2/3] iio: adc: stm32-adc: add analog switches supply control

[Jonathan Cameron]

On Wed, 12 Jun 2019 09:24:35 +0200
Fabrice Gasnier <fabrice.gasnier@st.com> wrote:

> On stm32h7 and stm32mp1, the ADC inputs are multiplexed with analog
> switches which have reduced performances when their supply is below 2.7V
> (vdda by default):
> - vdd supply can be selected if above 2.7V by setting ANASWVDD syscfg bit
>   (STM32MP1 only).
> - Voltage booster can be used, to get full ADC performances by setting
>   BOOSTE/EN_BOOSTER syscfg bit (increases power consumption).
> 
> Make this optional, since this is a trade-off between analog performance
> and power consumption.
> 
> Note: STM32H7 syscfg has a set and clear register for "BOOSTE" control.
> STM32MP1 has separate set and clear registers pair to control EN_BOOSTER
> and ANASWVDD bits.
> 
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>

A few minor bits inline, but mostly seems fine to me.

Jonathan

> ---
>  drivers/iio/adc/stm32-adc-core.c | 232 ++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 230 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/adc/stm32-adc-core.c b/drivers/iio/adc/stm32-adc-core.c
> index 2327ec1..9d41b16 100644
> --- a/drivers/iio/adc/stm32-adc-core.c
> +++ b/drivers/iio/adc/stm32-adc-core.c
> @@ -14,9 +14,11 @@
>  #include <linux/irqchip/chained_irq.h>
>  #include <linux/irqdesc.h>
>  #include <linux/irqdomain.h>
> +#include <linux/mfd/syscon.h>
>  #include <linux/module.h>
>  #include <linux/of_device.h>
>  #include <linux/pm_runtime.h>
> +#include <linux/regmap.h>
>  #include <linux/regulator/consumer.h>
>  #include <linux/slab.h>
>  
> @@ -51,6 +53,20 @@
>  
>  #define STM32_ADC_CORE_SLEEP_DELAY_MS	2000
>  
> +/* SYSCFG registers */
> +#define STM32H7_SYSCFG_PMCR		0x04
> +#define STM32MP1_SYSCFG_PMCSETR		0x04
> +#define STM32MP1_SYSCFG_PMCCLRR		0x44
> +
> +/* SYSCFG bit fields */
> +#define STM32H7_SYSCFG_BOOSTE_MASK	BIT(8)
> +#define STM32MP1_SYSCFG_ANASWVDD_MASK	BIT(9)
> +
> +/* SYSCFG capability flags */
> +#define HAS_VBOOSTER		BIT(0)
> +#define HAS_ANASWVDD		BIT(1)
> +#define HAS_CLEAR_REG		BIT(2)
> +
>  /**
>   * stm32_adc_common_regs - stm32 common registers, compatible dependent data
>   * @csr:	common status register offset
> @@ -58,6 +74,11 @@
>   * @eoc1:	adc1 end of conversion flag in @csr
>   * @eoc2:	adc2 end of conversion flag in @csr
>   * @eoc3:	adc3 end of conversion flag in @csr
> + * @has_syscfg: SYSCFG capability flags
> + * @pmcr:	SYSCFG_PMCSETR/SYSCFG_PMCR register offset
> + * @pmcc:	SYSCFG_PMCCLRR clear register offset
> + * @booste_msk:	SYSCFG BOOSTE / EN_BOOSTER bitmask in PMCR & PMCCLRR
> + * @anaswvdd_msk: SYSCFG ANASWVDD bitmask in PMCR & PMCCLRR
>   */
>  struct stm32_adc_common_regs {
>  	u32 csr;
> @@ -65,6 +86,11 @@ struct stm32_adc_common_regs {
>  	u32 eoc1_msk;
>  	u32 eoc2_msk;
>  	u32 eoc3_msk;
> +	unsigned int has_syscfg;
> +	u32 pmcr;
> +	u32 pmcc;
> +	u32 booste_msk;
> +	u32 anaswvdd_msk;
>  };
>  
>  struct stm32_adc_priv;
> @@ -87,20 +113,26 @@ struct stm32_adc_priv_cfg {
>   * @domain:		irq domain reference
>   * @aclk:		clock reference for the analog circuitry
>   * @bclk:		bus clock common for all ADCs, depends on part used
> + * @vdd:		vdd supply reference
> + * @vdda:		vdda supply reference
>   * @vref:		regulator reference
>   * @cfg:		compatible configuration data
>   * @common:		common data for all ADC instances
>   * @ccr_bak:		backup CCR in low power mode
> + * @syscfg:		reference to syscon, system control registers
>   */
>  struct stm32_adc_priv {
>  	int				irq[STM32_ADC_MAX_ADCS];
>  	struct irq_domain		*domain;
>  	struct clk			*aclk;
>  	struct clk			*bclk;
> +	struct regulator		*vdd;
> +	struct regulator		*vdda;
>  	struct regulator		*vref;
>  	const struct stm32_adc_priv_cfg	*cfg;
>  	struct stm32_adc_common		common;
>  	u32				ccr_bak;
> +	struct regmap			*syscfg;
>  };
>  
>  static struct stm32_adc_priv *to_stm32_adc_priv(struct stm32_adc_common *com)
> @@ -284,6 +316,22 @@ static const struct stm32_adc_common_regs stm32h7_adc_common_regs = {
>  	.ccr = STM32H7_ADC_CCR,
>  	.eoc1_msk = STM32H7_EOC_MST,
>  	.eoc2_msk = STM32H7_EOC_SLV,
> +	.has_syscfg = HAS_VBOOSTER,
> +	.pmcr = STM32H7_SYSCFG_PMCR,
> +	.booste_msk = STM32H7_SYSCFG_BOOSTE_MASK,
> +};
> +
> +/* STM32MP1 common registers definitions */
> +static const struct stm32_adc_common_regs stm32mp1_adc_common_regs = {
> +	.csr = STM32H7_ADC_CSR,
> +	.ccr = STM32H7_ADC_CCR,
> +	.eoc1_msk = STM32H7_EOC_MST,
> +	.eoc2_msk = STM32H7_EOC_SLV,
> +	.has_syscfg =  HAS_VBOOSTER | HAS_ANASWVDD | HAS_CLEAR_REG,

Extra space after =


> +	.pmcr = STM32MP1_SYSCFG_PMCSETR,
> +	.pmcc = STM32MP1_SYSCFG_PMCCLRR,
> +	.booste_msk = STM32H7_SYSCFG_BOOSTE_MASK,
> +	.anaswvdd_msk = STM32MP1_SYSCFG_ANASWVDD_MASK,
>  };
>  
>  /* ADC common interrupt for all instances */
> @@ -388,16 +436,145 @@ static void stm32_adc_irq_remove(struct platform_device *pdev,
>  	}
>  }
>  
> +static int stm32_adc_core_switches_supply_en(struct device *dev)
> +{
> +	struct stm32_adc_common *common = dev_get_drvdata(dev);
> +	struct stm32_adc_priv *priv = to_stm32_adc_priv(common);
> +	const struct stm32_adc_common_regs *regs = priv->cfg->regs;
> +	int ret, vdda, vdd = 0;
> +	u32 mask, clrmask, setmask = 0;
> +
> +	/*
> +	 * On STM32H7 and STM32MP1, the ADC inputs are multiplexed with analog
> +	 * switches (via PCSEL) which have reduced performances when their
> +	 * supply is below 2.7V (vdda by default):
> +	 * - Voltage booster can be used, to get full ADC performances
> +	 *   (increases power consumption).
> +	 * - Vdd can be used to supply them, if above 2.7V (STM32MP1 only).
> +	 *
> +	 * This is optional, as this is a trade-off between analog performance
> +	 * and power consumption.
> +	 */
> +	if (!regs->has_syscfg || !priv->vdda || !priv->syscfg) {
> +		dev_dbg(dev, "Not configuring analog switches\n");
> +		return 0;
> +	}
> +
> +	ret = regulator_enable(priv->vdda);
> +	if (ret < 0) {
> +		dev_err(dev, "vdda enable failed %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = regulator_get_voltage(priv->vdda);
> +	if (ret < 0) {
> +		dev_err(dev, "vdda get voltage failed %d\n", ret);
> +		goto vdda_dis;
> +	}
> +	vdda = ret;
> +
We only need to do the following block if vdaa is too low.  Should probably
not turn on vdd if there is not chance we are going to use it?

> +	if (priv->vdd && regs->has_syscfg & HAS_ANASWVDD) {
> +		ret = regulator_enable(priv->vdd);
> +		if (ret < 0) {
> +			dev_err(dev, "vdd enable failed %d\n", ret);
> +			goto vdda_dis;
> +		}
> +
> +		ret = regulator_get_voltage(priv->vdd);
> +		if (ret < 0) {
> +			dev_err(dev, "vdd get voltage failed %d\n", ret);
> +			goto vdd_dis;
> +		}
> +		vdd = ret;
> +	}
> +
> +	/*
> +	 * Recommended settings for ANASWVDD and EN_BOOSTER:
> +	 * - vdda < 2.7V but vdd > 2.7V: ANASWVDD = 1, EN_BOOSTER = 0 (stm32mp1)
> +	 * - vdda < 2.7V and vdd < 2.7V: ANASWVDD = 0, EN_BOOSTER = 1
> +	 * - vdda >= 2.7V:               ANASWVDD = 0, EN_BOOSTER = 0 (default)
> +	 */
> +	if (vdda < 2700000) {
> +		if (vdd > 2700000) {
> +			dev_dbg(dev, "analog switches supplied by vdd\n");
> +			setmask = regs->anaswvdd_msk;
> +			clrmask = regs->booste_msk;
> +		} else {
> +			dev_dbg(dev, "Enabling voltage booster\n");
> +			setmask = regs->booste_msk;
> +			clrmask = regs->anaswvdd_msk;
> +		}
> +	} else {
> +		dev_dbg(dev, "analog switches supplied by vdda\n");
> +		clrmask = regs->booste_msk | regs->anaswvdd_msk;
> +	}
> +
> +	mask = regs->booste_msk | regs->anaswvdd_msk;
> +	if (regs->has_syscfg & HAS_CLEAR_REG) {
> +		ret = regmap_write(priv->syscfg, regs->pmcc, clrmask);
> +		if (ret) {
> +			dev_err(dev, "syscfg clear failed, %d\n", ret);
> +			goto vdd_dis;
> +		}
> +		mask = setmask;
> +	}
> +
> +	ret = regmap_update_bits(priv->syscfg, regs->pmcr, mask, setmask);
> +	if (ret) {
> +		dev_err(dev, "syscfg update failed, %d\n", ret);
> +		goto vdd_dis;
> +	}
> +
> +	/* Booster voltage can take up to 50 us to stabilize */
> +	if (setmask & regs->booste_msk)
> +		usleep_range(50, 100);
> +
> +	return ret;
> +
> +vdd_dis:
> +	if (priv->vdd && (regs->has_syscfg & HAS_ANASWVDD))
> +		regulator_disable(priv->vdd);
> +vdda_dis:
> +	regulator_disable(priv->vdda);
> +
> +	return ret;
> +}
> +
> +static void stm32_adc_core_switches_supply_dis(struct device *dev)
> +{
> +	struct stm32_adc_common *common = dev_get_drvdata(dev);
> +	struct stm32_adc_priv *priv = to_stm32_adc_priv(common);
> +	const struct stm32_adc_common_regs *regs = priv->cfg->regs;
> +	u32 mask = regs->booste_msk | regs->anaswvdd_msk;
> +
> +	if (!regs->has_syscfg || !priv->vdda || !priv->syscfg)
> +		return;
> +
> +	if (regs->has_syscfg & HAS_CLEAR_REG)
> +		regmap_write(priv->syscfg, regs->pmcc, mask);
> +	else
> +		regmap_update_bits(priv->syscfg, regs->pmcr, mask, 0);
> +
> +	if (priv->vdd && (regs->has_syscfg & HAS_ANASWVDD))
> +		regulator_disable(priv->vdd);
> +
> +	regulator_disable(priv->vdda);
> +}
> +
>  static int stm32_adc_core_hw_start(struct device *dev)
>  {
>  	struct stm32_adc_common *common = dev_get_drvdata(dev);
>  	struct stm32_adc_priv *priv = to_stm32_adc_priv(common);
>  	int ret;
>  
> +	ret = stm32_adc_core_switches_supply_en(dev);
> +	if (ret < 0)
> +		return ret;
> +
>  	ret = regulator_enable(priv->vref);
>  	if (ret < 0) {
>  		dev_err(dev, "vref enable failed\n");
> -		return ret;
> +		goto err_switches_disable;
>  	}
>  
>  	if (priv->bclk) {
> @@ -425,6 +602,8 @@ static int stm32_adc_core_hw_start(struct device *dev)
>  		clk_disable_unprepare(priv->bclk);
>  err_regulator_disable:
>  	regulator_disable(priv->vref);
> +err_switches_disable:
> +	stm32_adc_core_switches_supply_dis(dev);
>  
>  	return ret;
>  }
> @@ -441,6 +620,24 @@ static void stm32_adc_core_hw_stop(struct device *dev)
>  	if (priv->bclk)
>  		clk_disable_unprepare(priv->bclk);
>  	regulator_disable(priv->vref);
> +	stm32_adc_core_switches_supply_dis(dev);
> +}
> +
> +static int stm32_adc_core_syscfg_probe(struct device_node *np,
> +				       struct stm32_adc_priv *priv)
> +{
> +	if (!priv->cfg->regs->has_syscfg)
> +		return 0;
> +
> +	priv->syscfg = syscon_regmap_lookup_by_phandle(np, "st,syscfg");
> +	if (IS_ERR(priv->syscfg)) {
> +		/* Optional */
> +		if (PTR_ERR(priv->syscfg) != -ENODEV)
> +			return PTR_ERR(priv->syscfg);
> +		priv->syscfg = NULL;
> +	}
> +
> +	return 0;
>  }
>  
>  static int stm32_adc_probe(struct platform_device *pdev)
> @@ -475,6 +672,30 @@ static int stm32_adc_probe(struct platform_device *pdev)
>  		return ret;
>  	}
>  
> +	priv->vdda = devm_regulator_get_optional(&pdev->dev, "vdda");
> +	if (IS_ERR(priv->vdda)) {
> +		ret = PTR_ERR(priv->vdda);
> +		if (ret != -ENODEV) {
> +			if (ret != -EPROBE_DEFER)
> +				dev_err(&pdev->dev, "vdda get failed, %d\n",
> +					ret);
> +			return ret;
> +		}
> +		priv->vdda = NULL;
> +	}
> +
> +	priv->vdd = devm_regulator_get_optional(&pdev->dev, "vdd");
> +	if (IS_ERR(priv->vdd)) {
> +		ret = PTR_ERR(priv->vdd);
> +		if (ret != -ENODEV) {
> +			if (ret != -EPROBE_DEFER)
> +				dev_err(&pdev->dev, "vdd get failed, %d\n",
> +					ret);
> +			return ret;
> +		}
> +		priv->vdd = NULL;
> +	}
> +
>  	priv->aclk = devm_clk_get(&pdev->dev, "adc");
>  	if (IS_ERR(priv->aclk)) {
>  		ret = PTR_ERR(priv->aclk);
> @@ -495,6 +716,13 @@ static int stm32_adc_probe(struct platform_device *pdev)
>  		priv->bclk = NULL;
>  	}
>  
> +	ret = stm32_adc_core_syscfg_probe(np, priv);
> +	if (ret) {
> +		if (ret != -EPROBE_DEFER)
> +			dev_err(&pdev->dev, "Can't probe syscfg: %d\n", ret);
> +		return ret;
> +	}
> +
>  	pm_runtime_get_noresume(dev);
>  	pm_runtime_set_active(dev);
>  	pm_runtime_set_autosuspend_delay(dev, STM32_ADC_CORE_SLEEP_DELAY_MS);
> @@ -595,7 +823,7 @@ static const struct stm32_adc_priv_cfg stm32h7_adc_priv_cfg = {
>  };
>  
>  static const struct stm32_adc_priv_cfg stm32mp1_adc_priv_cfg = {
> -	.regs = &stm32h7_adc_common_regs,
> +	.regs = &stm32mp1_adc_common_regs,
>  	.clk_sel = stm32h7_adc_clk_sel,
>  	.max_clk_rate_hz = 40000000,
>  };


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

Re: [PATCH 2/3] iio: adc: stm32-adc: add analog switches supply control

[Fabrice Gasnier]

On 6/16/19 5:07 PM, Jonathan Cameron wrote:
> On Wed, 12 Jun 2019 09:24:35 +0200
> Fabrice Gasnier <fabrice.gasnier@st.com> wrote:
> 
>> On stm32h7 and stm32mp1, the ADC inputs are multiplexed with analog
>> switches which have reduced performances when their supply is below 2.7V
>> (vdda by default):
>> - vdd supply can be selected if above 2.7V by setting ANASWVDD syscfg bit
>>   (STM32MP1 only).
>> - Voltage booster can be used, to get full ADC performances by setting
>>   BOOSTE/EN_BOOSTER syscfg bit (increases power consumption).
>>
>> Make this optional, since this is a trade-off between analog performance
>> and power consumption.
>>
>> Note: STM32H7 syscfg has a set and clear register for "BOOSTE" control.
>> STM32MP1 has separate set and clear registers pair to control EN_BOOSTER
>> and ANASWVDD bits.
>>
>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
> 
> A few minor bits inline, but mostly seems fine to me.
> 
> Jonathan
> 
>> ---
>>  drivers/iio/adc/stm32-adc-core.c | 232 ++++++++++++++++++++++++++++++++++++++-
>>  1 file changed, 230 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/iio/adc/stm32-adc-core.c b/drivers/iio/adc/stm32-adc-core.c
>> index 2327ec1..9d41b16 100644
>> --- a/drivers/iio/adc/stm32-adc-core.c
>> +++ b/drivers/iio/adc/stm32-adc-core.c
>> @@ -14,9 +14,11 @@
>>  #include <linux/irqchip/chained_irq.h>
>>  #include <linux/irqdesc.h>
>>  #include <linux/irqdomain.h>
>> +#include <linux/mfd/syscon.h>
>>  #include <linux/module.h>
>>  #include <linux/of_device.h>
>>  #include <linux/pm_runtime.h>
>> +#include <linux/regmap.h>
>>  #include <linux/regulator/consumer.h>
>>  #include <linux/slab.h>
>>  
>> @@ -51,6 +53,20 @@
>>  
>>  #define STM32_ADC_CORE_SLEEP_DELAY_MS	2000
>>  
>> +/* SYSCFG registers */
>> +#define STM32H7_SYSCFG_PMCR		0x04
>> +#define STM32MP1_SYSCFG_PMCSETR		0x04
>> +#define STM32MP1_SYSCFG_PMCCLRR		0x44
>> +
>> +/* SYSCFG bit fields */
>> +#define STM32H7_SYSCFG_BOOSTE_MASK	BIT(8)
>> +#define STM32MP1_SYSCFG_ANASWVDD_MASK	BIT(9)
>> +
>> +/* SYSCFG capability flags */
>> +#define HAS_VBOOSTER		BIT(0)
>> +#define HAS_ANASWVDD		BIT(1)
>> +#define HAS_CLEAR_REG		BIT(2)
>> +
>>  /**
>>   * stm32_adc_common_regs - stm32 common registers, compatible dependent data
>>   * @csr:	common status register offset
>> @@ -58,6 +74,11 @@
>>   * @eoc1:	adc1 end of conversion flag in @csr
>>   * @eoc2:	adc2 end of conversion flag in @csr
>>   * @eoc3:	adc3 end of conversion flag in @csr
>> + * @has_syscfg: SYSCFG capability flags
>> + * @pmcr:	SYSCFG_PMCSETR/SYSCFG_PMCR register offset
>> + * @pmcc:	SYSCFG_PMCCLRR clear register offset
>> + * @booste_msk:	SYSCFG BOOSTE / EN_BOOSTER bitmask in PMCR & PMCCLRR
>> + * @anaswvdd_msk: SYSCFG ANASWVDD bitmask in PMCR & PMCCLRR
>>   */
>>  struct stm32_adc_common_regs {
>>  	u32 csr;
>> @@ -65,6 +86,11 @@ struct stm32_adc_common_regs {
>>  	u32 eoc1_msk;
>>  	u32 eoc2_msk;
>>  	u32 eoc3_msk;
>> +	unsigned int has_syscfg;
>> +	u32 pmcr;
>> +	u32 pmcc;
>> +	u32 booste_msk;
>> +	u32 anaswvdd_msk;
>>  };
>>  
>>  struct stm32_adc_priv;
>> @@ -87,20 +113,26 @@ struct stm32_adc_priv_cfg {
>>   * @domain:		irq domain reference
>>   * @aclk:		clock reference for the analog circuitry
>>   * @bclk:		bus clock common for all ADCs, depends on part used
>> + * @vdd:		vdd supply reference
>> + * @vdda:		vdda supply reference
>>   * @vref:		regulator reference
>>   * @cfg:		compatible configuration data
>>   * @common:		common data for all ADC instances
>>   * @ccr_bak:		backup CCR in low power mode
>> + * @syscfg:		reference to syscon, system control registers
>>   */
>>  struct stm32_adc_priv {
>>  	int				irq[STM32_ADC_MAX_ADCS];
>>  	struct irq_domain		*domain;
>>  	struct clk			*aclk;
>>  	struct clk			*bclk;
>> +	struct regulator		*vdd;
>> +	struct regulator		*vdda;
>>  	struct regulator		*vref;
>>  	const struct stm32_adc_priv_cfg	*cfg;
>>  	struct stm32_adc_common		common;
>>  	u32				ccr_bak;
>> +	struct regmap			*syscfg;
>>  };
>>  
>>  static struct stm32_adc_priv *to_stm32_adc_priv(struct stm32_adc_common *com)
>> @@ -284,6 +316,22 @@ static const struct stm32_adc_common_regs stm32h7_adc_common_regs = {
>>  	.ccr = STM32H7_ADC_CCR,
>>  	.eoc1_msk = STM32H7_EOC_MST,
>>  	.eoc2_msk = STM32H7_EOC_SLV,
>> +	.has_syscfg = HAS_VBOOSTER,
>> +	.pmcr = STM32H7_SYSCFG_PMCR,
>> +	.booste_msk = STM32H7_SYSCFG_BOOSTE_MASK,
>> +};
>> +
>> +/* STM32MP1 common registers definitions */
>> +static const struct stm32_adc_common_regs stm32mp1_adc_common_regs = {
>> +	.csr = STM32H7_ADC_CSR,
>> +	.ccr = STM32H7_ADC_CCR,
>> +	.eoc1_msk = STM32H7_EOC_MST,
>> +	.eoc2_msk = STM32H7_EOC_SLV,
>> +	.has_syscfg =  HAS_VBOOSTER | HAS_ANASWVDD | HAS_CLEAR_REG,
> 
> Extra space after =

Hi Jonathan,

Oops, I'll fix it in v2.

> 
> 
>> +	.pmcr = STM32MP1_SYSCFG_PMCSETR,
>> +	.pmcc = STM32MP1_SYSCFG_PMCCLRR,
>> +	.booste_msk = STM32H7_SYSCFG_BOOSTE_MASK,
>> +	.anaswvdd_msk = STM32MP1_SYSCFG_ANASWVDD_MASK,
>>  };
>>  
>>  /* ADC common interrupt for all instances */
>> @@ -388,16 +436,145 @@ static void stm32_adc_irq_remove(struct platform_device *pdev,
>>  	}
>>  }
>>  
>> +static int stm32_adc_core_switches_supply_en(struct device *dev)
>> +{
>> +	struct stm32_adc_common *common = dev_get_drvdata(dev);
>> +	struct stm32_adc_priv *priv = to_stm32_adc_priv(common);
>> +	const struct stm32_adc_common_regs *regs = priv->cfg->regs;
>> +	int ret, vdda, vdd = 0;
>> +	u32 mask, clrmask, setmask = 0;
>> +
>> +	/*
>> +	 * On STM32H7 and STM32MP1, the ADC inputs are multiplexed with analog
>> +	 * switches (via PCSEL) which have reduced performances when their
>> +	 * supply is below 2.7V (vdda by default):
>> +	 * - Voltage booster can be used, to get full ADC performances
>> +	 *   (increases power consumption).
>> +	 * - Vdd can be used to supply them, if above 2.7V (STM32MP1 only).
>> +	 *
>> +	 * This is optional, as this is a trade-off between analog performance
>> +	 * and power consumption.
>> +	 */
>> +	if (!regs->has_syscfg || !priv->vdda || !priv->syscfg) {
>> +		dev_dbg(dev, "Not configuring analog switches\n");
>> +		return 0;
>> +	}
>> +
>> +	ret = regulator_enable(priv->vdda);
>> +	if (ret < 0) {
>> +		dev_err(dev, "vdda enable failed %d\n", ret);
>> +		return ret;
>> +	}
>> +
>> +	ret = regulator_get_voltage(priv->vdda);
>> +	if (ret < 0) {
>> +		dev_err(dev, "vdda get voltage failed %d\n", ret);
>> +		goto vdda_dis;
>> +	}
>> +	vdda = ret;
>> +
> We only need to do the following block if vdaa is too low.  Should probably
> not turn on vdd if there is not chance we are going to use it?

You're right, then I probably need to move the regulator_get_voltage()
call at probe time, to avoid enabling it for nothing at runtime. (e.g.
to figure out it's not going to be used).
In fact, vdd is used also for other things on the platform (I/Os, other
supplies...), and is marked "always-on" in the device tree. But I
understand your point.

I'll rework this and send a v2.

Thanks,
Fabrice

> 
>> +	if (priv->vdd && regs->has_syscfg & HAS_ANASWVDD) {
>> +		ret = regulator_enable(priv->vdd);
>> +		if (ret < 0) {
>> +			dev_err(dev, "vdd enable failed %d\n", ret);
>> +			goto vdda_dis;
>> +		}
>> +
>> +		ret = regulator_get_voltage(priv->vdd);
>> +		if (ret < 0) {
>> +			dev_err(dev, "vdd get voltage failed %d\n", ret);
>> +			goto vdd_dis;
>> +		}
>> +		vdd = ret;
>> +	}
>> +
>> +	/*
>> +	 * Recommended settings for ANASWVDD and EN_BOOSTER:
>> +	 * - vdda < 2.7V but vdd > 2.7V: ANASWVDD = 1, EN_BOOSTER = 0 (stm32mp1)
>> +	 * - vdda < 2.7V and vdd < 2.7V: ANASWVDD = 0, EN_BOOSTER = 1
>> +	 * - vdda >= 2.7V:               ANASWVDD = 0, EN_BOOSTER = 0 (default)
>> +	 */
>> +	if (vdda < 2700000) {
>> +		if (vdd > 2700000) {
>> +			dev_dbg(dev, "analog switches supplied by vdd\n");
>> +			setmask = regs->anaswvdd_msk;
>> +			clrmask = regs->booste_msk;
>> +		} else {
>> +			dev_dbg(dev, "Enabling voltage booster\n");
>> +			setmask = regs->booste_msk;
>> +			clrmask = regs->anaswvdd_msk;
>> +		}
>> +	} else {
>> +		dev_dbg(dev, "analog switches supplied by vdda\n");
>> +		clrmask = regs->booste_msk | regs->anaswvdd_msk;
>> +	}
>> +
>> +	mask = regs->booste_msk | regs->anaswvdd_msk;
>> +	if (regs->has_syscfg & HAS_CLEAR_REG) {
>> +		ret = regmap_write(priv->syscfg, regs->pmcc, clrmask);
>> +		if (ret) {
>> +			dev_err(dev, "syscfg clear failed, %d\n", ret);
>> +			goto vdd_dis;
>> +		}
>> +		mask = setmask;
>> +	}
>> +
>> +	ret = regmap_update_bits(priv->syscfg, regs->pmcr, mask, setmask);
>> +	if (ret) {
>> +		dev_err(dev, "syscfg update failed, %d\n", ret);
>> +		goto vdd_dis;
>> +	}
>> +
>> +	/* Booster voltage can take up to 50 us to stabilize */
>> +	if (setmask & regs->booste_msk)
>> +		usleep_range(50, 100);
>> +
>> +	return ret;
>> +
>> +vdd_dis:
>> +	if (priv->vdd && (regs->has_syscfg & HAS_ANASWVDD))
>> +		regulator_disable(priv->vdd);
>> +vdda_dis:
>> +	regulator_disable(priv->vdda);
>> +
>> +	return ret;
>> +}
>> +
>> +static void stm32_adc_core_switches_supply_dis(struct device *dev)
>> +{
>> +	struct stm32_adc_common *common = dev_get_drvdata(dev);
>> +	struct stm32_adc_priv *priv = to_stm32_adc_priv(common);
>> +	const struct stm32_adc_common_regs *regs = priv->cfg->regs;
>> +	u32 mask = regs->booste_msk | regs->anaswvdd_msk;
>> +
>> +	if (!regs->has_syscfg || !priv->vdda || !priv->syscfg)
>> +		return;
>> +
>> +	if (regs->has_syscfg & HAS_CLEAR_REG)
>> +		regmap_write(priv->syscfg, regs->pmcc, mask);
>> +	else
>> +		regmap_update_bits(priv->syscfg, regs->pmcr, mask, 0);
>> +
>> +	if (priv->vdd && (regs->has_syscfg & HAS_ANASWVDD))
>> +		regulator_disable(priv->vdd);
>> +
>> +	regulator_disable(priv->vdda);
>> +}
>> +
>>  static int stm32_adc_core_hw_start(struct device *dev)
>>  {
>>  	struct stm32_adc_common *common = dev_get_drvdata(dev);
>>  	struct stm32_adc_priv *priv = to_stm32_adc_priv(common);
>>  	int ret;
>>  
>> +	ret = stm32_adc_core_switches_supply_en(dev);
>> +	if (ret < 0)
>> +		return ret;
>> +
>>  	ret = regulator_enable(priv->vref);
>>  	if (ret < 0) {
>>  		dev_err(dev, "vref enable failed\n");
>> -		return ret;
>> +		goto err_switches_disable;
>>  	}
>>  
>>  	if (priv->bclk) {
>> @@ -425,6 +602,8 @@ static int stm32_adc_core_hw_start(struct device *dev)
>>  		clk_disable_unprepare(priv->bclk);
>>  err_regulator_disable:
>>  	regulator_disable(priv->vref);
>> +err_switches_disable:
>> +	stm32_adc_core_switches_supply_dis(dev);
>>  
>>  	return ret;
>>  }
>> @@ -441,6 +620,24 @@ static void stm32_adc_core_hw_stop(struct device *dev)
>>  	if (priv->bclk)
>>  		clk_disable_unprepare(priv->bclk);
>>  	regulator_disable(priv->vref);
>> +	stm32_adc_core_switches_supply_dis(dev);
>> +}
>> +
>> +static int stm32_adc_core_syscfg_probe(struct device_node *np,
>> +				       struct stm32_adc_priv *priv)
>> +{
>> +	if (!priv->cfg->regs->has_syscfg)
>> +		return 0;
>> +
>> +	priv->syscfg = syscon_regmap_lookup_by_phandle(np, "st,syscfg");
>> +	if (IS_ERR(priv->syscfg)) {
>> +		/* Optional */
>> +		if (PTR_ERR(priv->syscfg) != -ENODEV)
>> +			return PTR_ERR(priv->syscfg);
>> +		priv->syscfg = NULL;
>> +	}
>> +
>> +	return 0;
>>  }
>>  
>>  static int stm32_adc_probe(struct platform_device *pdev)
>> @@ -475,6 +672,30 @@ static int stm32_adc_probe(struct platform_device *pdev)
>>  		return ret;
>>  	}
>>  
>> +	priv->vdda = devm_regulator_get_optional(&pdev->dev, "vdda");
>> +	if (IS_ERR(priv->vdda)) {
>> +		ret = PTR_ERR(priv->vdda);
>> +		if (ret != -ENODEV) {
>> +			if (ret != -EPROBE_DEFER)
>> +				dev_err(&pdev->dev, "vdda get failed, %d\n",
>> +					ret);
>> +			return ret;
>> +		}
>> +		priv->vdda = NULL;
>> +	}
>> +
>> +	priv->vdd = devm_regulator_get_optional(&pdev->dev, "vdd");
>> +	if (IS_ERR(priv->vdd)) {
>> +		ret = PTR_ERR(priv->vdd);
>> +		if (ret != -ENODEV) {
>> +			if (ret != -EPROBE_DEFER)
>> +				dev_err(&pdev->dev, "vdd get failed, %d\n",
>> +					ret);
>> +			return ret;
>> +		}
>> +		priv->vdd = NULL;
>> +	}
>> +
>>  	priv->aclk = devm_clk_get(&pdev->dev, "adc");
>>  	if (IS_ERR(priv->aclk)) {
>>  		ret = PTR_ERR(priv->aclk);
>> @@ -495,6 +716,13 @@ static int stm32_adc_probe(struct platform_device *pdev)
>>  		priv->bclk = NULL;
>>  	}
>>  
>> +	ret = stm32_adc_core_syscfg_probe(np, priv);
>> +	if (ret) {
>> +		if (ret != -EPROBE_DEFER)
>> +			dev_err(&pdev->dev, "Can't probe syscfg: %d\n", ret);
>> +		return ret;
>> +	}
>> +
>>  	pm_runtime_get_noresume(dev);
>>  	pm_runtime_set_active(dev);
>>  	pm_runtime_set_autosuspend_delay(dev, STM32_ADC_CORE_SLEEP_DELAY_MS);
>> @@ -595,7 +823,7 @@ static const struct stm32_adc_priv_cfg stm32h7_adc_priv_cfg = {
>>  };
>>  
>>  static const struct stm32_adc_priv_cfg stm32mp1_adc_priv_cfg = {
>> -	.regs = &stm32h7_adc_common_regs,
>> +	.regs = &stm32mp1_adc_common_regs,
>>  	.clk_sel = stm32h7_adc_clk_sel,
>>  	.max_clk_rate_hz = 40000000,
>>  };
> 

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

Re: [PATCH 2/3] iio: adc: stm32-adc: add analog switches supply control

[Fabrice Gasnier]

On 6/17/19 2:43 PM, Fabrice Gasnier wrote:
> On 6/16/19 5:07 PM, Jonathan Cameron wrote:
>> On Wed, 12 Jun 2019 09:24:35 +0200
>> Fabrice Gasnier <fabrice.gasnier@st.com> wrote:
>>
>>> On stm32h7 and stm32mp1, the ADC inputs are multiplexed with analog
>>> switches which have reduced performances when their supply is below 2.7V
>>> (vdda by default):
>>> - vdd supply can be selected if above 2.7V by setting ANASWVDD syscfg bit
>>>   (STM32MP1 only).
>>> - Voltage booster can be used, to get full ADC performances by setting
>>>   BOOSTE/EN_BOOSTER syscfg bit (increases power consumption).
>>>
>>> Make this optional, since this is a trade-off between analog performance
>>> and power consumption.
>>>
>>> Note: STM32H7 syscfg has a set and clear register for "BOOSTE" control.
>>> STM32MP1 has separate set and clear registers pair to control EN_BOOSTER
>>> and ANASWVDD bits.
>>>
>>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
>>
>> A few minor bits inline, but mostly seems fine to me.
>>
>> Jonathan
>>
>>> ---
>>>  drivers/iio/adc/stm32-adc-core.c | 232 ++++++++++++++++++++++++++++++++++++++-
>>>  1 file changed, 230 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/iio/adc/stm32-adc-core.c b/drivers/iio/adc/stm32-adc-core.c
>>> index 2327ec1..9d41b16 100644
>>> --- a/drivers/iio/adc/stm32-adc-core.c
>>> +++ b/drivers/iio/adc/stm32-adc-core.c
>>> @@ -14,9 +14,11 @@
>>>  #include <linux/irqchip/chained_irq.h>
>>>  #include <linux/irqdesc.h>
>>>  #include <linux/irqdomain.h>
>>> +#include <linux/mfd/syscon.h>
>>>  #include <linux/module.h>
>>>  #include <linux/of_device.h>
>>>  #include <linux/pm_runtime.h>
>>> +#include <linux/regmap.h>
>>>  #include <linux/regulator/consumer.h>
>>>  #include <linux/slab.h>
>>>  
>>> @@ -51,6 +53,20 @@
>>>  
>>>  #define STM32_ADC_CORE_SLEEP_DELAY_MS	2000
>>>  
>>> +/* SYSCFG registers */
>>> +#define STM32H7_SYSCFG_PMCR		0x04
>>> +#define STM32MP1_SYSCFG_PMCSETR		0x04
>>> +#define STM32MP1_SYSCFG_PMCCLRR		0x44
>>> +
>>> +/* SYSCFG bit fields */
>>> +#define STM32H7_SYSCFG_BOOSTE_MASK	BIT(8)
>>> +#define STM32MP1_SYSCFG_ANASWVDD_MASK	BIT(9)
>>> +
>>> +/* SYSCFG capability flags */
>>> +#define HAS_VBOOSTER		BIT(0)
>>> +#define HAS_ANASWVDD		BIT(1)
>>> +#define HAS_CLEAR_REG		BIT(2)
>>> +
>>>  /**
>>>   * stm32_adc_common_regs - stm32 common registers, compatible dependent data
>>>   * @csr:	common status register offset
>>> @@ -58,6 +74,11 @@
>>>   * @eoc1:	adc1 end of conversion flag in @csr
>>>   * @eoc2:	adc2 end of conversion flag in @csr
>>>   * @eoc3:	adc3 end of conversion flag in @csr
>>> + * @has_syscfg: SYSCFG capability flags
>>> + * @pmcr:	SYSCFG_PMCSETR/SYSCFG_PMCR register offset
>>> + * @pmcc:	SYSCFG_PMCCLRR clear register offset
>>> + * @booste_msk:	SYSCFG BOOSTE / EN_BOOSTER bitmask in PMCR & PMCCLRR
>>> + * @anaswvdd_msk: SYSCFG ANASWVDD bitmask in PMCR & PMCCLRR
>>>   */
>>>  struct stm32_adc_common_regs {
>>>  	u32 csr;
>>> @@ -65,6 +86,11 @@ struct stm32_adc_common_regs {
>>>  	u32 eoc1_msk;
>>>  	u32 eoc2_msk;
>>>  	u32 eoc3_msk;
>>> +	unsigned int has_syscfg;
>>> +	u32 pmcr;
>>> +	u32 pmcc;
>>> +	u32 booste_msk;
>>> +	u32 anaswvdd_msk;
>>>  };
>>>  
>>>  struct stm32_adc_priv;
>>> @@ -87,20 +113,26 @@ struct stm32_adc_priv_cfg {
>>>   * @domain:		irq domain reference
>>>   * @aclk:		clock reference for the analog circuitry
>>>   * @bclk:		bus clock common for all ADCs, depends on part used
>>> + * @vdd:		vdd supply reference
>>> + * @vdda:		vdda supply reference
>>>   * @vref:		regulator reference
>>>   * @cfg:		compatible configuration data
>>>   * @common:		common data for all ADC instances
>>>   * @ccr_bak:		backup CCR in low power mode
>>> + * @syscfg:		reference to syscon, system control registers
>>>   */
>>>  struct stm32_adc_priv {
>>>  	int				irq[STM32_ADC_MAX_ADCS];
>>>  	struct irq_domain		*domain;
>>>  	struct clk			*aclk;
>>>  	struct clk			*bclk;
>>> +	struct regulator		*vdd;
>>> +	struct regulator		*vdda;
>>>  	struct regulator		*vref;
>>>  	const struct stm32_adc_priv_cfg	*cfg;
>>>  	struct stm32_adc_common		common;
>>>  	u32				ccr_bak;
>>> +	struct regmap			*syscfg;
>>>  };
>>>  
>>>  static struct stm32_adc_priv *to_stm32_adc_priv(struct stm32_adc_common *com)
>>> @@ -284,6 +316,22 @@ static const struct stm32_adc_common_regs stm32h7_adc_common_regs = {
>>>  	.ccr = STM32H7_ADC_CCR,
>>>  	.eoc1_msk = STM32H7_EOC_MST,
>>>  	.eoc2_msk = STM32H7_EOC_SLV,
>>> +	.has_syscfg = HAS_VBOOSTER,
>>> +	.pmcr = STM32H7_SYSCFG_PMCR,
>>> +	.booste_msk = STM32H7_SYSCFG_BOOSTE_MASK,
>>> +};
>>> +
>>> +/* STM32MP1 common registers definitions */
>>> +static const struct stm32_adc_common_regs stm32mp1_adc_common_regs = {
>>> +	.csr = STM32H7_ADC_CSR,
>>> +	.ccr = STM32H7_ADC_CCR,
>>> +	.eoc1_msk = STM32H7_EOC_MST,
>>> +	.eoc2_msk = STM32H7_EOC_SLV,
>>> +	.has_syscfg =  HAS_VBOOSTER | HAS_ANASWVDD | HAS_CLEAR_REG,
>>
>> Extra space after =
> 
> Hi Jonathan,
> 
> Oops, I'll fix it in v2.
> 
>>
>>
>>> +	.pmcr = STM32MP1_SYSCFG_PMCSETR,
>>> +	.pmcc = STM32MP1_SYSCFG_PMCCLRR,
>>> +	.booste_msk = STM32H7_SYSCFG_BOOSTE_MASK,
>>> +	.anaswvdd_msk = STM32MP1_SYSCFG_ANASWVDD_MASK,
>>>  };
>>>  
>>>  /* ADC common interrupt for all instances */
>>> @@ -388,16 +436,145 @@ static void stm32_adc_irq_remove(struct platform_device *pdev,
>>>  	}
>>>  }
>>>  
>>> +static int stm32_adc_core_switches_supply_en(struct device *dev)
>>> +{
>>> +	struct stm32_adc_common *common = dev_get_drvdata(dev);
>>> +	struct stm32_adc_priv *priv = to_stm32_adc_priv(common);
>>> +	const struct stm32_adc_common_regs *regs = priv->cfg->regs;
>>> +	int ret, vdda, vdd = 0;
>>> +	u32 mask, clrmask, setmask = 0;
>>> +
>>> +	/*
>>> +	 * On STM32H7 and STM32MP1, the ADC inputs are multiplexed with analog
>>> +	 * switches (via PCSEL) which have reduced performances when their
>>> +	 * supply is below 2.7V (vdda by default):
>>> +	 * - Voltage booster can be used, to get full ADC performances
>>> +	 *   (increases power consumption).
>>> +	 * - Vdd can be used to supply them, if above 2.7V (STM32MP1 only).
>>> +	 *
>>> +	 * This is optional, as this is a trade-off between analog performance
>>> +	 * and power consumption.
>>> +	 */
>>> +	if (!regs->has_syscfg || !priv->vdda || !priv->syscfg) {
>>> +		dev_dbg(dev, "Not configuring analog switches\n");
>>> +		return 0;
>>> +	}
>>> +
>>> +	ret = regulator_enable(priv->vdda);
>>> +	if (ret < 0) {
>>> +		dev_err(dev, "vdda enable failed %d\n", ret);
>>> +		return ret;
>>> +	}
>>> +
>>> +	ret = regulator_get_voltage(priv->vdda);
>>> +	if (ret < 0) {
>>> +		dev_err(dev, "vdda get voltage failed %d\n", ret);
>>> +		goto vdda_dis;
>>> +	}
>>> +	vdda = ret;
>>> +
>> We only need to do the following block if vdaa is too low.  Should probably
>> not turn on vdd if there is not chance we are going to use it?
> 
> You're right, then I probably need to move the regulator_get_voltage()
> call at probe time, to avoid enabling it for nothing at runtime. (e.g.
> to figure out it's not going to be used).
> In fact, vdd is used also for other things on the platform (I/Os, other
> supplies...), and is marked "always-on" in the device tree. But I
> understand your point.
> 
> I'll rework this and send a v2.

Hi Jonathan,

When reworking this part, I figured out the vdda should be described as
required supply for the STM32 ADC. So I pushed out a fix series to
address this "Add missing vdda-supply to STM32 ADC". I'll resume v2 of
this series that has some dependencies on the fix series .

Thanks
Best Regards,
Fabrice

> 
> Thanks,
> Fabrice
> 
>>
>>> +	if (priv->vdd && regs->has_syscfg & HAS_ANASWVDD) {
>>> +		ret = regulator_enable(priv->vdd);
>>> +		if (ret < 0) {
>>> +			dev_err(dev, "vdd enable failed %d\n", ret);
>>> +			goto vdda_dis;
>>> +		}
>>> +
>>> +		ret = regulator_get_voltage(priv->vdd);
>>> +		if (ret < 0) {
>>> +			dev_err(dev, "vdd get voltage failed %d\n", ret);
>>> +			goto vdd_dis;
>>> +		}
>>> +		vdd = ret;
>>> +	}
>>> +
>>> +	/*
>>> +	 * Recommended settings for ANASWVDD and EN_BOOSTER:
>>> +	 * - vdda < 2.7V but vdd > 2.7V: ANASWVDD = 1, EN_BOOSTER = 0 (stm32mp1)
>>> +	 * - vdda < 2.7V and vdd < 2.7V: ANASWVDD = 0, EN_BOOSTER = 1
>>> +	 * - vdda >= 2.7V:               ANASWVDD = 0, EN_BOOSTER = 0 (default)
>>> +	 */
>>> +	if (vdda < 2700000) {
>>> +		if (vdd > 2700000) {
>>> +			dev_dbg(dev, "analog switches supplied by vdd\n");
>>> +			setmask = regs->anaswvdd_msk;
>>> +			clrmask = regs->booste_msk;
>>> +		} else {
>>> +			dev_dbg(dev, "Enabling voltage booster\n");
>>> +			setmask = regs->booste_msk;
>>> +			clrmask = regs->anaswvdd_msk;
>>> +		}
>>> +	} else {
>>> +		dev_dbg(dev, "analog switches supplied by vdda\n");
>>> +		clrmask = regs->booste_msk | regs->anaswvdd_msk;
>>> +	}
>>> +
>>> +	mask = regs->booste_msk | regs->anaswvdd_msk;
>>> +	if (regs->has_syscfg & HAS_CLEAR_REG) {
>>> +		ret = regmap_write(priv->syscfg, regs->pmcc, clrmask);
>>> +		if (ret) {
>>> +			dev_err(dev, "syscfg clear failed, %d\n", ret);
>>> +			goto vdd_dis;
>>> +		}
>>> +		mask = setmask;
>>> +	}
>>> +
>>> +	ret = regmap_update_bits(priv->syscfg, regs->pmcr, mask, setmask);
>>> +	if (ret) {
>>> +		dev_err(dev, "syscfg update failed, %d\n", ret);
>>> +		goto vdd_dis;
>>> +	}
>>> +
>>> +	/* Booster voltage can take up to 50 us to stabilize */
>>> +	if (setmask & regs->booste_msk)
>>> +		usleep_range(50, 100);
>>> +
>>> +	return ret;
>>> +
>>> +vdd_dis:
>>> +	if (priv->vdd && (regs->has_syscfg & HAS_ANASWVDD))
>>> +		regulator_disable(priv->vdd);
>>> +vdda_dis:
>>> +	regulator_disable(priv->vdda);
>>> +
>>> +	return ret;
>>> +}
>>> +
>>> +static void stm32_adc_core_switches_supply_dis(struct device *dev)
>>> +{
>>> +	struct stm32_adc_common *common = dev_get_drvdata(dev);
>>> +	struct stm32_adc_priv *priv = to_stm32_adc_priv(common);
>>> +	const struct stm32_adc_common_regs *regs = priv->cfg->regs;
>>> +	u32 mask = regs->booste_msk | regs->anaswvdd_msk;
>>> +
>>> +	if (!regs->has_syscfg || !priv->vdda || !priv->syscfg)
>>> +		return;
>>> +
>>> +	if (regs->has_syscfg & HAS_CLEAR_REG)
>>> +		regmap_write(priv->syscfg, regs->pmcc, mask);
>>> +	else
>>> +		regmap_update_bits(priv->syscfg, regs->pmcr, mask, 0);
>>> +
>>> +	if (priv->vdd && (regs->has_syscfg & HAS_ANASWVDD))
>>> +		regulator_disable(priv->vdd);
>>> +
>>> +	regulator_disable(priv->vdda);
>>> +}
>>> +
>>>  static int stm32_adc_core_hw_start(struct device *dev)
>>>  {
>>>  	struct stm32_adc_common *common = dev_get_drvdata(dev);
>>>  	struct stm32_adc_priv *priv = to_stm32_adc_priv(common);
>>>  	int ret;
>>>  
>>> +	ret = stm32_adc_core_switches_supply_en(dev);
>>> +	if (ret < 0)
>>> +		return ret;
>>> +
>>>  	ret = regulator_enable(priv->vref);
>>>  	if (ret < 0) {
>>>  		dev_err(dev, "vref enable failed\n");
>>> -		return ret;
>>> +		goto err_switches_disable;
>>>  	}
>>>  
>>>  	if (priv->bclk) {
>>> @@ -425,6 +602,8 @@ static int stm32_adc_core_hw_start(struct device *dev)
>>>  		clk_disable_unprepare(priv->bclk);
>>>  err_regulator_disable:
>>>  	regulator_disable(priv->vref);
>>> +err_switches_disable:
>>> +	stm32_adc_core_switches_supply_dis(dev);
>>>  
>>>  	return ret;
>>>  }
>>> @@ -441,6 +620,24 @@ static void stm32_adc_core_hw_stop(struct device *dev)
>>>  	if (priv->bclk)
>>>  		clk_disable_unprepare(priv->bclk);
>>>  	regulator_disable(priv->vref);
>>> +	stm32_adc_core_switches_supply_dis(dev);
>>> +}
>>> +
>>> +static int stm32_adc_core_syscfg_probe(struct device_node *np,
>>> +				       struct stm32_adc_priv *priv)
>>> +{
>>> +	if (!priv->cfg->regs->has_syscfg)
>>> +		return 0;
>>> +
>>> +	priv->syscfg = syscon_regmap_lookup_by_phandle(np, "st,syscfg");
>>> +	if (IS_ERR(priv->syscfg)) {
>>> +		/* Optional */
>>> +		if (PTR_ERR(priv->syscfg) != -ENODEV)
>>> +			return PTR_ERR(priv->syscfg);
>>> +		priv->syscfg = NULL;
>>> +	}
>>> +
>>> +	return 0;
>>>  }
>>>  
>>>  static int stm32_adc_probe(struct platform_device *pdev)
>>> @@ -475,6 +672,30 @@ static int stm32_adc_probe(struct platform_device *pdev)
>>>  		return ret;
>>>  	}
>>>  
>>> +	priv->vdda = devm_regulator_get_optional(&pdev->dev, "vdda");
>>> +	if (IS_ERR(priv->vdda)) {
>>> +		ret = PTR_ERR(priv->vdda);
>>> +		if (ret != -ENODEV) {
>>> +			if (ret != -EPROBE_DEFER)
>>> +				dev_err(&pdev->dev, "vdda get failed, %d\n",
>>> +					ret);
>>> +			return ret;
>>> +		}
>>> +		priv->vdda = NULL;
>>> +	}
>>> +
>>> +	priv->vdd = devm_regulator_get_optional(&pdev->dev, "vdd");
>>> +	if (IS_ERR(priv->vdd)) {
>>> +		ret = PTR_ERR(priv->vdd);
>>> +		if (ret != -ENODEV) {
>>> +			if (ret != -EPROBE_DEFER)
>>> +				dev_err(&pdev->dev, "vdd get failed, %d\n",
>>> +					ret);
>>> +			return ret;
>>> +		}
>>> +		priv->vdd = NULL;
>>> +	}
>>> +
>>>  	priv->aclk = devm_clk_get(&pdev->dev, "adc");
>>>  	if (IS_ERR(priv->aclk)) {
>>>  		ret = PTR_ERR(priv->aclk);
>>> @@ -495,6 +716,13 @@ static int stm32_adc_probe(struct platform_device *pdev)
>>>  		priv->bclk = NULL;
>>>  	}
>>>  
>>> +	ret = stm32_adc_core_syscfg_probe(np, priv);
>>> +	if (ret) {
>>> +		if (ret != -EPROBE_DEFER)
>>> +			dev_err(&pdev->dev, "Can't probe syscfg: %d\n", ret);
>>> +		return ret;
>>> +	}
>>> +
>>>  	pm_runtime_get_noresume(dev);
>>>  	pm_runtime_set_active(dev);
>>>  	pm_runtime_set_autosuspend_delay(dev, STM32_ADC_CORE_SLEEP_DELAY_MS);
>>> @@ -595,7 +823,7 @@ static const struct stm32_adc_priv_cfg stm32h7_adc_priv_cfg = {
>>>  };
>>>  
>>>  static const struct stm32_adc_priv_cfg stm32mp1_adc_priv_cfg = {
>>> -	.regs = &stm32h7_adc_common_regs,
>>> +	.regs = &stm32mp1_adc_common_regs,
>>>  	.clk_sel = stm32h7_adc_clk_sel,
>>>  	.max_clk_rate_hz = 40000000,
>>>  };
>>

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

Re: [PATCH 2/3] iio: adc: stm32-adc: add analog switches supply control

[Jonathan Cameron]

On Wed, 19 Jun 2019 14:38:42 +0200
Fabrice Gasnier <fabrice.gasnier@st.com> wrote:

> On 6/17/19 2:43 PM, Fabrice Gasnier wrote:
> > On 6/16/19 5:07 PM, Jonathan Cameron wrote:  
> >> On Wed, 12 Jun 2019 09:24:35 +0200
> >> Fabrice Gasnier <fabrice.gasnier@st.com> wrote:
> >>  
> >>> On stm32h7 and stm32mp1, the ADC inputs are multiplexed with analog
> >>> switches which have reduced performances when their supply is below 2.7V
> >>> (vdda by default):
> >>> - vdd supply can be selected if above 2.7V by setting ANASWVDD syscfg bit
> >>>   (STM32MP1 only).
> >>> - Voltage booster can be used, to get full ADC performances by setting
> >>>   BOOSTE/EN_BOOSTER syscfg bit (increases power consumption).
> >>>
> >>> Make this optional, since this is a trade-off between analog performance
> >>> and power consumption.
> >>>
> >>> Note: STM32H7 syscfg has a set and clear register for "BOOSTE" control.
> >>> STM32MP1 has separate set and clear registers pair to control EN_BOOSTER
> >>> and ANASWVDD bits.
> >>>
> >>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>  
> >>
> >> A few minor bits inline, but mostly seems fine to me.
> >>
> >> Jonathan
> >>  
> >>> ---
> >>>  drivers/iio/adc/stm32-adc-core.c | 232 ++++++++++++++++++++++++++++++++++++++-
> >>>  1 file changed, 230 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/drivers/iio/adc/stm32-adc-core.c b/drivers/iio/adc/stm32-adc-core.c
> >>> index 2327ec1..9d41b16 100644
> >>> --- a/drivers/iio/adc/stm32-adc-core.c
> >>> +++ b/drivers/iio/adc/stm32-adc-core.c
> >>> @@ -14,9 +14,11 @@
> >>>  #include <linux/irqchip/chained_irq.h>
> >>>  #include <linux/irqdesc.h>
> >>>  #include <linux/irqdomain.h>
> >>> +#include <linux/mfd/syscon.h>
> >>>  #include <linux/module.h>
> >>>  #include <linux/of_device.h>
> >>>  #include <linux/pm_runtime.h>
> >>> +#include <linux/regmap.h>
> >>>  #include <linux/regulator/consumer.h>
> >>>  #include <linux/slab.h>
> >>>  
> >>> @@ -51,6 +53,20 @@
> >>>  
> >>>  #define STM32_ADC_CORE_SLEEP_DELAY_MS	2000
> >>>  
> >>> +/* SYSCFG registers */
> >>> +#define STM32H7_SYSCFG_PMCR		0x04
> >>> +#define STM32MP1_SYSCFG_PMCSETR		0x04
> >>> +#define STM32MP1_SYSCFG_PMCCLRR		0x44
> >>> +
> >>> +/* SYSCFG bit fields */
> >>> +#define STM32H7_SYSCFG_BOOSTE_MASK	BIT(8)
> >>> +#define STM32MP1_SYSCFG_ANASWVDD_MASK	BIT(9)
> >>> +
> >>> +/* SYSCFG capability flags */
> >>> +#define HAS_VBOOSTER		BIT(0)
> >>> +#define HAS_ANASWVDD		BIT(1)
> >>> +#define HAS_CLEAR_REG		BIT(2)
> >>> +
> >>>  /**
> >>>   * stm32_adc_common_regs - stm32 common registers, compatible dependent data
> >>>   * @csr:	common status register offset
> >>> @@ -58,6 +74,11 @@
> >>>   * @eoc1:	adc1 end of conversion flag in @csr
> >>>   * @eoc2:	adc2 end of conversion flag in @csr
> >>>   * @eoc3:	adc3 end of conversion flag in @csr
> >>> + * @has_syscfg: SYSCFG capability flags
> >>> + * @pmcr:	SYSCFG_PMCSETR/SYSCFG_PMCR register offset
> >>> + * @pmcc:	SYSCFG_PMCCLRR clear register offset
> >>> + * @booste_msk:	SYSCFG BOOSTE / EN_BOOSTER bitmask in PMCR & PMCCLRR
> >>> + * @anaswvdd_msk: SYSCFG ANASWVDD bitmask in PMCR & PMCCLRR
> >>>   */
> >>>  struct stm32_adc_common_regs {
> >>>  	u32 csr;
> >>> @@ -65,6 +86,11 @@ struct stm32_adc_common_regs {
> >>>  	u32 eoc1_msk;
> >>>  	u32 eoc2_msk;
> >>>  	u32 eoc3_msk;
> >>> +	unsigned int has_syscfg;
> >>> +	u32 pmcr;
> >>> +	u32 pmcc;
> >>> +	u32 booste_msk;
> >>> +	u32 anaswvdd_msk;
> >>>  };
> >>>  
> >>>  struct stm32_adc_priv;
> >>> @@ -87,20 +113,26 @@ struct stm32_adc_priv_cfg {
> >>>   * @domain:		irq domain reference
> >>>   * @aclk:		clock reference for the analog circuitry
> >>>   * @bclk:		bus clock common for all ADCs, depends on part used
> >>> + * @vdd:		vdd supply reference
> >>> + * @vdda:		vdda supply reference
> >>>   * @vref:		regulator reference
> >>>   * @cfg:		compatible configuration data
> >>>   * @common:		common data for all ADC instances
> >>>   * @ccr_bak:		backup CCR in low power mode
> >>> + * @syscfg:		reference to syscon, system control registers
> >>>   */
> >>>  struct stm32_adc_priv {
> >>>  	int				irq[STM32_ADC_MAX_ADCS];
> >>>  	struct irq_domain		*domain;
> >>>  	struct clk			*aclk;
> >>>  	struct clk			*bclk;
> >>> +	struct regulator		*vdd;
> >>> +	struct regulator		*vdda;
> >>>  	struct regulator		*vref;
> >>>  	const struct stm32_adc_priv_cfg	*cfg;
> >>>  	struct stm32_adc_common		common;
> >>>  	u32				ccr_bak;
> >>> +	struct regmap			*syscfg;
> >>>  };
> >>>  
> >>>  static struct stm32_adc_priv *to_stm32_adc_priv(struct stm32_adc_common *com)
> >>> @@ -284,6 +316,22 @@ static const struct stm32_adc_common_regs stm32h7_adc_common_regs = {
> >>>  	.ccr = STM32H7_ADC_CCR,
> >>>  	.eoc1_msk = STM32H7_EOC_MST,
> >>>  	.eoc2_msk = STM32H7_EOC_SLV,
> >>> +	.has_syscfg = HAS_VBOOSTER,
> >>> +	.pmcr = STM32H7_SYSCFG_PMCR,
> >>> +	.booste_msk = STM32H7_SYSCFG_BOOSTE_MASK,
> >>> +};
> >>> +
> >>> +/* STM32MP1 common registers definitions */
> >>> +static const struct stm32_adc_common_regs stm32mp1_adc_common_regs = {
> >>> +	.csr = STM32H7_ADC_CSR,
> >>> +	.ccr = STM32H7_ADC_CCR,
> >>> +	.eoc1_msk = STM32H7_EOC_MST,
> >>> +	.eoc2_msk = STM32H7_EOC_SLV,
> >>> +	.has_syscfg =  HAS_VBOOSTER | HAS_ANASWVDD | HAS_CLEAR_REG,  
> >>
> >> Extra space after =  
> > 
> > Hi Jonathan,
> > 
> > Oops, I'll fix it in v2.
> >   
> >>
> >>  
> >>> +	.pmcr = STM32MP1_SYSCFG_PMCSETR,
> >>> +	.pmcc = STM32MP1_SYSCFG_PMCCLRR,
> >>> +	.booste_msk = STM32H7_SYSCFG_BOOSTE_MASK,
> >>> +	.anaswvdd_msk = STM32MP1_SYSCFG_ANASWVDD_MASK,
> >>>  };
> >>>  
> >>>  /* ADC common interrupt for all instances */
> >>> @@ -388,16 +436,145 @@ static void stm32_adc_irq_remove(struct platform_device *pdev,
> >>>  	}
> >>>  }
> >>>  
> >>> +static int stm32_adc_core_switches_supply_en(struct device *dev)
> >>> +{
> >>> +	struct stm32_adc_common *common = dev_get_drvdata(dev);
> >>> +	struct stm32_adc_priv *priv = to_stm32_adc_priv(common);
> >>> +	const struct stm32_adc_common_regs *regs = priv->cfg->regs;
> >>> +	int ret, vdda, vdd = 0;
> >>> +	u32 mask, clrmask, setmask = 0;
> >>> +
> >>> +	/*
> >>> +	 * On STM32H7 and STM32MP1, the ADC inputs are multiplexed with analog
> >>> +	 * switches (via PCSEL) which have reduced performances when their
> >>> +	 * supply is below 2.7V (vdda by default):
> >>> +	 * - Voltage booster can be used, to get full ADC performances
> >>> +	 *   (increases power consumption).
> >>> +	 * - Vdd can be used to supply them, if above 2.7V (STM32MP1 only).
> >>> +	 *
> >>> +	 * This is optional, as this is a trade-off between analog performance
> >>> +	 * and power consumption.
> >>> +	 */
> >>> +	if (!regs->has_syscfg || !priv->vdda || !priv->syscfg) {
> >>> +		dev_dbg(dev, "Not configuring analog switches\n");
> >>> +		return 0;
> >>> +	}
> >>> +
> >>> +	ret = regulator_enable(priv->vdda);
> >>> +	if (ret < 0) {
> >>> +		dev_err(dev, "vdda enable failed %d\n", ret);
> >>> +		return ret;
> >>> +	}
> >>> +
> >>> +	ret = regulator_get_voltage(priv->vdda);
> >>> +	if (ret < 0) {
> >>> +		dev_err(dev, "vdda get voltage failed %d\n", ret);
> >>> +		goto vdda_dis;
> >>> +	}
> >>> +	vdda = ret;
> >>> +  
> >> We only need to do the following block if vdaa is too low.  Should probably
> >> not turn on vdd if there is not chance we are going to use it?  
> > 
> > You're right, then I probably need to move the regulator_get_voltage()
> > call at probe time, to avoid enabling it for nothing at runtime. (e.g.
> > to figure out it's not going to be used).
> > In fact, vdd is used also for other things on the platform (I/Os, other
> > supplies...), and is marked "always-on" in the device tree. But I
> > understand your point.
> > 
> > I'll rework this and send a v2.  
> 
> Hi Jonathan,
> 
> When reworking this part, I figured out the vdda should be described as
> required supply for the STM32 ADC. So I pushed out a fix series to
> address this "Add missing vdda-supply to STM32 ADC". I'll resume v2 of
> this series that has some dependencies on the fix series .

Cool. Given timing  I'm taking fixes and new stuff through the togreg
branch anyway at the moment so dependencies on fixes are easier than
normal for the next week or so ;)

Jonathan

> 
> Thanks
> Best Regards,
> Fabrice
> 
> > 
> > Thanks,
> > Fabrice
> >   
> >>  
> >>> +	if (priv->vdd && regs->has_syscfg & HAS_ANASWVDD) {
> >>> +		ret = regulator_enable(priv->vdd);
> >>> +		if (ret < 0) {
> >>> +			dev_err(dev, "vdd enable failed %d\n", ret);
> >>> +			goto vdda_dis;
> >>> +		}
> >>> +
> >>> +		ret = regulator_get_voltage(priv->vdd);
> >>> +		if (ret < 0) {
> >>> +			dev_err(dev, "vdd get voltage failed %d\n", ret);
> >>> +			goto vdd_dis;
> >>> +		}
> >>> +		vdd = ret;
> >>> +	}
> >>> +
> >>> +	/*
> >>> +	 * Recommended settings for ANASWVDD and EN_BOOSTER:
> >>> +	 * - vdda < 2.7V but vdd > 2.7V: ANASWVDD = 1, EN_BOOSTER = 0 (stm32mp1)
> >>> +	 * - vdda < 2.7V and vdd < 2.7V: ANASWVDD = 0, EN_BOOSTER = 1
> >>> +	 * - vdda >= 2.7V:               ANASWVDD = 0, EN_BOOSTER = 0 (default)
> >>> +	 */
> >>> +	if (vdda < 2700000) {
> >>> +		if (vdd > 2700000) {
> >>> +			dev_dbg(dev, "analog switches supplied by vdd\n");
> >>> +			setmask = regs->anaswvdd_msk;
> >>> +			clrmask = regs->booste_msk;
> >>> +		} else {
> >>> +			dev_dbg(dev, "Enabling voltage booster\n");
> >>> +			setmask = regs->booste_msk;
> >>> +			clrmask = regs->anaswvdd_msk;
> >>> +		}
> >>> +	} else {
> >>> +		dev_dbg(dev, "analog switches supplied by vdda\n");
> >>> +		clrmask = regs->booste_msk | regs->anaswvdd_msk;
> >>> +	}
> >>> +
> >>> +	mask = regs->booste_msk | regs->anaswvdd_msk;
> >>> +	if (regs->has_syscfg & HAS_CLEAR_REG) {
> >>> +		ret = regmap_write(priv->syscfg, regs->pmcc, clrmask);
> >>> +		if (ret) {
> >>> +			dev_err(dev, "syscfg clear failed, %d\n", ret);
> >>> +			goto vdd_dis;
> >>> +		}
> >>> +		mask = setmask;
> >>> +	}
> >>> +
> >>> +	ret = regmap_update_bits(priv->syscfg, regs->pmcr, mask, setmask);
> >>> +	if (ret) {
> >>> +		dev_err(dev, "syscfg update failed, %d\n", ret);
> >>> +		goto vdd_dis;
> >>> +	}
> >>> +
> >>> +	/* Booster voltage can take up to 50 us to stabilize */
> >>> +	if (setmask & regs->booste_msk)
> >>> +		usleep_range(50, 100);
> >>> +
> >>> +	return ret;
> >>> +
> >>> +vdd_dis:
> >>> +	if (priv->vdd && (regs->has_syscfg & HAS_ANASWVDD))
> >>> +		regulator_disable(priv->vdd);
> >>> +vdda_dis:
> >>> +	regulator_disable(priv->vdda);
> >>> +
> >>> +	return ret;
> >>> +}
> >>> +
> >>> +static void stm32_adc_core_switches_supply_dis(struct device *dev)
> >>> +{
> >>> +	struct stm32_adc_common *common = dev_get_drvdata(dev);
> >>> +	struct stm32_adc_priv *priv = to_stm32_adc_priv(common);
> >>> +	const struct stm32_adc_common_regs *regs = priv->cfg->regs;
> >>> +	u32 mask = regs->booste_msk | regs->anaswvdd_msk;
> >>> +
> >>> +	if (!regs->has_syscfg || !priv->vdda || !priv->syscfg)
> >>> +		return;
> >>> +
> >>> +	if (regs->has_syscfg & HAS_CLEAR_REG)
> >>> +		regmap_write(priv->syscfg, regs->pmcc, mask);
> >>> +	else
> >>> +		regmap_update_bits(priv->syscfg, regs->pmcr, mask, 0);
> >>> +
> >>> +	if (priv->vdd && (regs->has_syscfg & HAS_ANASWVDD))
> >>> +		regulator_disable(priv->vdd);
> >>> +
> >>> +	regulator_disable(priv->vdda);
> >>> +}
> >>> +
> >>>  static int stm32_adc_core_hw_start(struct device *dev)
> >>>  {
> >>>  	struct stm32_adc_common *common = dev_get_drvdata(dev);
> >>>  	struct stm32_adc_priv *priv = to_stm32_adc_priv(common);
> >>>  	int ret;
> >>>  
> >>> +	ret = stm32_adc_core_switches_supply_en(dev);
> >>> +	if (ret < 0)
> >>> +		return ret;
> >>> +
> >>>  	ret = regulator_enable(priv->vref);
> >>>  	if (ret < 0) {
> >>>  		dev_err(dev, "vref enable failed\n");
> >>> -		return ret;
> >>> +		goto err_switches_disable;
> >>>  	}
> >>>  
> >>>  	if (priv->bclk) {
> >>> @@ -425,6 +602,8 @@ static int stm32_adc_core_hw_start(struct device *dev)
> >>>  		clk_disable_unprepare(priv->bclk);
> >>>  err_regulator_disable:
> >>>  	regulator_disable(priv->vref);
> >>> +err_switches_disable:
> >>> +	stm32_adc_core_switches_supply_dis(dev);
> >>>  
> >>>  	return ret;
> >>>  }
> >>> @@ -441,6 +620,24 @@ static void stm32_adc_core_hw_stop(struct device *dev)
> >>>  	if (priv->bclk)
> >>>  		clk_disable_unprepare(priv->bclk);
> >>>  	regulator_disable(priv->vref);
> >>> +	stm32_adc_core_switches_supply_dis(dev);
> >>> +}
> >>> +
> >>> +static int stm32_adc_core_syscfg_probe(struct device_node *np,
> >>> +				       struct stm32_adc_priv *priv)
> >>> +{
> >>> +	if (!priv->cfg->regs->has_syscfg)
> >>> +		return 0;
> >>> +
> >>> +	priv->syscfg = syscon_regmap_lookup_by_phandle(np, "st,syscfg");
> >>> +	if (IS_ERR(priv->syscfg)) {
> >>> +		/* Optional */
> >>> +		if (PTR_ERR(priv->syscfg) != -ENODEV)
> >>> +			return PTR_ERR(priv->syscfg);
> >>> +		priv->syscfg = NULL;
> >>> +	}
> >>> +
> >>> +	return 0;
> >>>  }
> >>>  
> >>>  static int stm32_adc_probe(struct platform_device *pdev)
> >>> @@ -475,6 +672,30 @@ static int stm32_adc_probe(struct platform_device *pdev)
> >>>  		return ret;
> >>>  	}
> >>>  
> >>> +	priv->vdda = devm_regulator_get_optional(&pdev->dev, "vdda");
> >>> +	if (IS_ERR(priv->vdda)) {
> >>> +		ret = PTR_ERR(priv->vdda);
> >>> +		if (ret != -ENODEV) {
> >>> +			if (ret != -EPROBE_DEFER)
> >>> +				dev_err(&pdev->dev, "vdda get failed, %d\n",
> >>> +					ret);
> >>> +			return ret;
> >>> +		}
> >>> +		priv->vdda = NULL;
> >>> +	}
> >>> +
> >>> +	priv->vdd = devm_regulator_get_optional(&pdev->dev, "vdd");
> >>> +	if (IS_ERR(priv->vdd)) {
> >>> +		ret = PTR_ERR(priv->vdd);
> >>> +		if (ret != -ENODEV) {
> >>> +			if (ret != -EPROBE_DEFER)
> >>> +				dev_err(&pdev->dev, "vdd get failed, %d\n",
> >>> +					ret);
> >>> +			return ret;
> >>> +		}
> >>> +		priv->vdd = NULL;
> >>> +	}
> >>> +
> >>>  	priv->aclk = devm_clk_get(&pdev->dev, "adc");
> >>>  	if (IS_ERR(priv->aclk)) {
> >>>  		ret = PTR_ERR(priv->aclk);
> >>> @@ -495,6 +716,13 @@ static int stm32_adc_probe(struct platform_device *pdev)
> >>>  		priv->bclk = NULL;
> >>>  	}
> >>>  
> >>> +	ret = stm32_adc_core_syscfg_probe(np, priv);
> >>> +	if (ret) {
> >>> +		if (ret != -EPROBE_DEFER)
> >>> +			dev_err(&pdev->dev, "Can't probe syscfg: %d\n", ret);
> >>> +		return ret;
> >>> +	}
> >>> +
> >>>  	pm_runtime_get_noresume(dev);
> >>>  	pm_runtime_set_active(dev);
> >>>  	pm_runtime_set_autosuspend_delay(dev, STM32_ADC_CORE_SLEEP_DELAY_MS);
> >>> @@ -595,7 +823,7 @@ static const struct stm32_adc_priv_cfg stm32h7_adc_priv_cfg = {
> >>>  };
> >>>  
> >>>  static const struct stm32_adc_priv_cfg stm32mp1_adc_priv_cfg = {
> >>> -	.regs = &stm32h7_adc_common_regs,
> >>> +	.regs = &stm32mp1_adc_common_regs,
> >>>  	.clk_sel = stm32h7_adc_clk_sel,
> >>>  	.max_clk_rate_hz = 40000000,
> >>>  };  
> >>  


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

Re: [PATCH 2/3] iio: adc: stm32-adc: add analog switches supply control

[Fabrice Gasnier]

On 6/22/19 1:05 PM, Jonathan Cameron wrote:
> On Wed, 19 Jun 2019 14:38:42 +0200
> Fabrice Gasnier <fabrice.gasnier@st.com> wrote:
> 
>> On 6/17/19 2:43 PM, Fabrice Gasnier wrote:
>>> On 6/16/19 5:07 PM, Jonathan Cameron wrote:  
>>>> On Wed, 12 Jun 2019 09:24:35 +0200
>>>> Fabrice Gasnier <fabrice.gasnier@st.com> wrote:
>>>>  
>>>>> On stm32h7 and stm32mp1, the ADC inputs are multiplexed with analog
>>>>> switches which have reduced performances when their supply is below 2.7V
>>>>> (vdda by default):
>>>>> - vdd supply can be selected if above 2.7V by setting ANASWVDD syscfg bit
>>>>>   (STM32MP1 only).
>>>>> - Voltage booster can be used, to get full ADC performances by setting
>>>>>   BOOSTE/EN_BOOSTER syscfg bit (increases power consumption).
>>>>>
>>>>> Make this optional, since this is a trade-off between analog performance
>>>>> and power consumption.
>>>>>
>>>>> Note: STM32H7 syscfg has a set and clear register for "BOOSTE" control.
>>>>> STM32MP1 has separate set and clear registers pair to control EN_BOOSTER
>>>>> and ANASWVDD bits.
>>>>>
>>>>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>  
>>>>
>>>> A few minor bits inline, but mostly seems fine to me.
>>>>
>>>> Jonathan
>>>>  
>>>>> ---
>>>>>  drivers/iio/adc/stm32-adc-core.c | 232 ++++++++++++++++++++++++++++++++++++++-
>>>>>  1 file changed, 230 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/drivers/iio/adc/stm32-adc-core.c b/drivers/iio/adc/stm32-adc-core.c
>>>>> index 2327ec1..9d41b16 100644
>>>>> --- a/drivers/iio/adc/stm32-adc-core.c
>>>>> +++ b/drivers/iio/adc/stm32-adc-core.c
>>>>> @@ -14,9 +14,11 @@
>>>>>  #include <linux/irqchip/chained_irq.h>
>>>>>  #include <linux/irqdesc.h>
>>>>>  #include <linux/irqdomain.h>
>>>>> +#include <linux/mfd/syscon.h>
>>>>>  #include <linux/module.h>
>>>>>  #include <linux/of_device.h>
>>>>>  #include <linux/pm_runtime.h>
>>>>> +#include <linux/regmap.h>
>>>>>  #include <linux/regulator/consumer.h>
>>>>>  #include <linux/slab.h>
>>>>>  
>>>>> @@ -51,6 +53,20 @@
>>>>>  
>>>>>  #define STM32_ADC_CORE_SLEEP_DELAY_MS	2000
>>>>>  
>>>>> +/* SYSCFG registers */
>>>>> +#define STM32H7_SYSCFG_PMCR		0x04
>>>>> +#define STM32MP1_SYSCFG_PMCSETR		0x04
>>>>> +#define STM32MP1_SYSCFG_PMCCLRR		0x44
>>>>> +
>>>>> +/* SYSCFG bit fields */
>>>>> +#define STM32H7_SYSCFG_BOOSTE_MASK	BIT(8)
>>>>> +#define STM32MP1_SYSCFG_ANASWVDD_MASK	BIT(9)
>>>>> +
>>>>> +/* SYSCFG capability flags */
>>>>> +#define HAS_VBOOSTER		BIT(0)
>>>>> +#define HAS_ANASWVDD		BIT(1)
>>>>> +#define HAS_CLEAR_REG		BIT(2)
>>>>> +
>>>>>  /**
>>>>>   * stm32_adc_common_regs - stm32 common registers, compatible dependent data
>>>>>   * @csr:	common status register offset
>>>>> @@ -58,6 +74,11 @@
>>>>>   * @eoc1:	adc1 end of conversion flag in @csr
>>>>>   * @eoc2:	adc2 end of conversion flag in @csr
>>>>>   * @eoc3:	adc3 end of conversion flag in @csr
>>>>> + * @has_syscfg: SYSCFG capability flags
>>>>> + * @pmcr:	SYSCFG_PMCSETR/SYSCFG_PMCR register offset
>>>>> + * @pmcc:	SYSCFG_PMCCLRR clear register offset
>>>>> + * @booste_msk:	SYSCFG BOOSTE / EN_BOOSTER bitmask in PMCR & PMCCLRR
>>>>> + * @anaswvdd_msk: SYSCFG ANASWVDD bitmask in PMCR & PMCCLRR
>>>>>   */
>>>>>  struct stm32_adc_common_regs {
>>>>>  	u32 csr;
>>>>> @@ -65,6 +86,11 @@ struct stm32_adc_common_regs {
>>>>>  	u32 eoc1_msk;
>>>>>  	u32 eoc2_msk;
>>>>>  	u32 eoc3_msk;
>>>>> +	unsigned int has_syscfg;
>>>>> +	u32 pmcr;
>>>>> +	u32 pmcc;
>>>>> +	u32 booste_msk;
>>>>> +	u32 anaswvdd_msk;
>>>>>  };
>>>>>  
>>>>>  struct stm32_adc_priv;
>>>>> @@ -87,20 +113,26 @@ struct stm32_adc_priv_cfg {
>>>>>   * @domain:		irq domain reference
>>>>>   * @aclk:		clock reference for the analog circuitry
>>>>>   * @bclk:		bus clock common for all ADCs, depends on part used
>>>>> + * @vdd:		vdd supply reference
>>>>> + * @vdda:		vdda supply reference
>>>>>   * @vref:		regulator reference
>>>>>   * @cfg:		compatible configuration data
>>>>>   * @common:		common data for all ADC instances
>>>>>   * @ccr_bak:		backup CCR in low power mode
>>>>> + * @syscfg:		reference to syscon, system control registers
>>>>>   */
>>>>>  struct stm32_adc_priv {
>>>>>  	int				irq[STM32_ADC_MAX_ADCS];
>>>>>  	struct irq_domain		*domain;
>>>>>  	struct clk			*aclk;
>>>>>  	struct clk			*bclk;
>>>>> +	struct regulator		*vdd;
>>>>> +	struct regulator		*vdda;
>>>>>  	struct regulator		*vref;
>>>>>  	const struct stm32_adc_priv_cfg	*cfg;
>>>>>  	struct stm32_adc_common		common;
>>>>>  	u32				ccr_bak;
>>>>> +	struct regmap			*syscfg;
>>>>>  };
>>>>>  
>>>>>  static struct stm32_adc_priv *to_stm32_adc_priv(struct stm32_adc_common *com)
>>>>> @@ -284,6 +316,22 @@ static const struct stm32_adc_common_regs stm32h7_adc_common_regs = {
>>>>>  	.ccr = STM32H7_ADC_CCR,
>>>>>  	.eoc1_msk = STM32H7_EOC_MST,
>>>>>  	.eoc2_msk = STM32H7_EOC_SLV,
>>>>> +	.has_syscfg = HAS_VBOOSTER,
>>>>> +	.pmcr = STM32H7_SYSCFG_PMCR,
>>>>> +	.booste_msk = STM32H7_SYSCFG_BOOSTE_MASK,
>>>>> +};
>>>>> +
>>>>> +/* STM32MP1 common registers definitions */
>>>>> +static const struct stm32_adc_common_regs stm32mp1_adc_common_regs = {
>>>>> +	.csr = STM32H7_ADC_CSR,
>>>>> +	.ccr = STM32H7_ADC_CCR,
>>>>> +	.eoc1_msk = STM32H7_EOC_MST,
>>>>> +	.eoc2_msk = STM32H7_EOC_SLV,
>>>>> +	.has_syscfg =  HAS_VBOOSTER | HAS_ANASWVDD | HAS_CLEAR_REG,  
>>>>
>>>> Extra space after =  
>>>
>>> Hi Jonathan,
>>>
>>> Oops, I'll fix it in v2.
>>>   
>>>>
>>>>  
>>>>> +	.pmcr = STM32MP1_SYSCFG_PMCSETR,
>>>>> +	.pmcc = STM32MP1_SYSCFG_PMCCLRR,
>>>>> +	.booste_msk = STM32H7_SYSCFG_BOOSTE_MASK,
>>>>> +	.anaswvdd_msk = STM32MP1_SYSCFG_ANASWVDD_MASK,
>>>>>  };
>>>>>  
>>>>>  /* ADC common interrupt for all instances */
>>>>> @@ -388,16 +436,145 @@ static void stm32_adc_irq_remove(struct platform_device *pdev,
>>>>>  	}
>>>>>  }
>>>>>  
>>>>> +static int stm32_adc_core_switches_supply_en(struct device *dev)
>>>>> +{
>>>>> +	struct stm32_adc_common *common = dev_get_drvdata(dev);
>>>>> +	struct stm32_adc_priv *priv = to_stm32_adc_priv(common);
>>>>> +	const struct stm32_adc_common_regs *regs = priv->cfg->regs;
>>>>> +	int ret, vdda, vdd = 0;
>>>>> +	u32 mask, clrmask, setmask = 0;
>>>>> +
>>>>> +	/*
>>>>> +	 * On STM32H7 and STM32MP1, the ADC inputs are multiplexed with analog
>>>>> +	 * switches (via PCSEL) which have reduced performances when their
>>>>> +	 * supply is below 2.7V (vdda by default):
>>>>> +	 * - Voltage booster can be used, to get full ADC performances
>>>>> +	 *   (increases power consumption).
>>>>> +	 * - Vdd can be used to supply them, if above 2.7V (STM32MP1 only).
>>>>> +	 *
>>>>> +	 * This is optional, as this is a trade-off between analog performance
>>>>> +	 * and power consumption.
>>>>> +	 */
>>>>> +	if (!regs->has_syscfg || !priv->vdda || !priv->syscfg) {
>>>>> +		dev_dbg(dev, "Not configuring analog switches\n");
>>>>> +		return 0;
>>>>> +	}
>>>>> +
>>>>> +	ret = regulator_enable(priv->vdda);
>>>>> +	if (ret < 0) {
>>>>> +		dev_err(dev, "vdda enable failed %d\n", ret);
>>>>> +		return ret;
>>>>> +	}
>>>>> +
>>>>> +	ret = regulator_get_voltage(priv->vdda);
>>>>> +	if (ret < 0) {
>>>>> +		dev_err(dev, "vdda get voltage failed %d\n", ret);
>>>>> +		goto vdda_dis;
>>>>> +	}
>>>>> +	vdda = ret;
>>>>> +  
>>>> We only need to do the following block if vdaa is too low.  Should probably
>>>> not turn on vdd if there is not chance we are going to use it?  
>>>
>>> You're right, then I probably need to move the regulator_get_voltage()
>>> call at probe time, to avoid enabling it for nothing at runtime. (e.g.
>>> to figure out it's not going to be used).
>>> In fact, vdd is used also for other things on the platform (I/Os, other
>>> supplies...), and is marked "always-on" in the device tree. But I
>>> understand your point.
>>>
>>> I'll rework this and send a v2.  
>>
>> Hi Jonathan,
>>
>> When reworking this part, I figured out the vdda should be described as
>> required supply for the STM32 ADC. So I pushed out a fix series to
>> address this "Add missing vdda-supply to STM32 ADC". I'll resume v2 of
>> this series that has some dependencies on the fix series .
> 
> Cool. Given timing  I'm taking fixes and new stuff through the togreg
> branch anyway at the moment so dependencies on fixes are easier than
> normal for the next week or so ;)

Hi Jonathan

I did some additional changes at my end. Just for reference, you can see
"regulator: add support for the STM32 ADC booster" series here:
https://lkml.org/lkml/2019/6/28/188

I think the proper way to handle the booster is to have it fully
described as a regulator (that's one). This will also simplify handling
it here.

I'll resume this series once the regulator series has been discussed.

Many thanks,
Best regards,
Fabrice

> 
> Jonathan
> 
>>
>> Thanks
>> Best Regards,
>> Fabrice
>>
>>>
>>> Thanks,
>>> Fabrice
>>>   
>>>>  
>>>>> +	if (priv->vdd && regs->has_syscfg & HAS_ANASWVDD) {
>>>>> +		ret = regulator_enable(priv->vdd);
>>>>> +		if (ret < 0) {
>>>>> +			dev_err(dev, "vdd enable failed %d\n", ret);
>>>>> +			goto vdda_dis;
>>>>> +		}
>>>>> +
>>>>> +		ret = regulator_get_voltage(priv->vdd);
>>>>> +		if (ret < 0) {
>>>>> +			dev_err(dev, "vdd get voltage failed %d\n", ret);
>>>>> +			goto vdd_dis;
>>>>> +		}
>>>>> +		vdd = ret;
>>>>> +	}
>>>>> +
>>>>> +	/*
>>>>> +	 * Recommended settings for ANASWVDD and EN_BOOSTER:
>>>>> +	 * - vdda < 2.7V but vdd > 2.7V: ANASWVDD = 1, EN_BOOSTER = 0 (stm32mp1)
>>>>> +	 * - vdda < 2.7V and vdd < 2.7V: ANASWVDD = 0, EN_BOOSTER = 1
>>>>> +	 * - vdda >= 2.7V:               ANASWVDD = 0, EN_BOOSTER = 0 (default)
>>>>> +	 */
>>>>> +	if (vdda < 2700000) {
>>>>> +		if (vdd > 2700000) {
>>>>> +			dev_dbg(dev, "analog switches supplied by vdd\n");
>>>>> +			setmask = regs->anaswvdd_msk;
>>>>> +			clrmask = regs->booste_msk;
>>>>> +		} else {
>>>>> +			dev_dbg(dev, "Enabling voltage booster\n");
>>>>> +			setmask = regs->booste_msk;
>>>>> +			clrmask = regs->anaswvdd_msk;
>>>>> +		}
>>>>> +	} else {
>>>>> +		dev_dbg(dev, "analog switches supplied by vdda\n");
>>>>> +		clrmask = regs->booste_msk | regs->anaswvdd_msk;
>>>>> +	}
>>>>> +
>>>>> +	mask = regs->booste_msk | regs->anaswvdd_msk;
>>>>> +	if (regs->has_syscfg & HAS_CLEAR_REG) {
>>>>> +		ret = regmap_write(priv->syscfg, regs->pmcc, clrmask);
>>>>> +		if (ret) {
>>>>> +			dev_err(dev, "syscfg clear failed, %d\n", ret);
>>>>> +			goto vdd_dis;
>>>>> +		}
>>>>> +		mask = setmask;
>>>>> +	}
>>>>> +
>>>>> +	ret = regmap_update_bits(priv->syscfg, regs->pmcr, mask, setmask);
>>>>> +	if (ret) {
>>>>> +		dev_err(dev, "syscfg update failed, %d\n", ret);
>>>>> +		goto vdd_dis;
>>>>> +	}
>>>>> +
>>>>> +	/* Booster voltage can take up to 50 us to stabilize */
>>>>> +	if (setmask & regs->booste_msk)
>>>>> +		usleep_range(50, 100);
>>>>> +
>>>>> +	return ret;
>>>>> +
>>>>> +vdd_dis:
>>>>> +	if (priv->vdd && (regs->has_syscfg & HAS_ANASWVDD))
>>>>> +		regulator_disable(priv->vdd);
>>>>> +vdda_dis:
>>>>> +	regulator_disable(priv->vdda);
>>>>> +
>>>>> +	return ret;
>>>>> +}
>>>>> +
>>>>> +static void stm32_adc_core_switches_supply_dis(struct device *dev)
>>>>> +{
>>>>> +	struct stm32_adc_common *common = dev_get_drvdata(dev);
>>>>> +	struct stm32_adc_priv *priv = to_stm32_adc_priv(common);
>>>>> +	const struct stm32_adc_common_regs *regs = priv->cfg->regs;
>>>>> +	u32 mask = regs->booste_msk | regs->anaswvdd_msk;
>>>>> +
>>>>> +	if (!regs->has_syscfg || !priv->vdda || !priv->syscfg)
>>>>> +		return;
>>>>> +
>>>>> +	if (regs->has_syscfg & HAS_CLEAR_REG)
>>>>> +		regmap_write(priv->syscfg, regs->pmcc, mask);
>>>>> +	else
>>>>> +		regmap_update_bits(priv->syscfg, regs->pmcr, mask, 0);
>>>>> +
>>>>> +	if (priv->vdd && (regs->has_syscfg & HAS_ANASWVDD))
>>>>> +		regulator_disable(priv->vdd);
>>>>> +
>>>>> +	regulator_disable(priv->vdda);
>>>>> +}
>>>>> +
>>>>>  static int stm32_adc_core_hw_start(struct device *dev)
>>>>>  {
>>>>>  	struct stm32_adc_common *common = dev_get_drvdata(dev);
>>>>>  	struct stm32_adc_priv *priv = to_stm32_adc_priv(common);
>>>>>  	int ret;
>>>>>  
>>>>> +	ret = stm32_adc_core_switches_supply_en(dev);
>>>>> +	if (ret < 0)
>>>>> +		return ret;
>>>>> +
>>>>>  	ret = regulator_enable(priv->vref);
>>>>>  	if (ret < 0) {
>>>>>  		dev_err(dev, "vref enable failed\n");
>>>>> -		return ret;
>>>>> +		goto err_switches_disable;
>>>>>  	}
>>>>>  
>>>>>  	if (priv->bclk) {
>>>>> @@ -425,6 +602,8 @@ static int stm32_adc_core_hw_start(struct device *dev)
>>>>>  		clk_disable_unprepare(priv->bclk);
>>>>>  err_regulator_disable:
>>>>>  	regulator_disable(priv->vref);
>>>>> +err_switches_disable:
>>>>> +	stm32_adc_core_switches_supply_dis(dev);
>>>>>  
>>>>>  	return ret;
>>>>>  }
>>>>> @@ -441,6 +620,24 @@ static void stm32_adc_core_hw_stop(struct device *dev)
>>>>>  	if (priv->bclk)
>>>>>  		clk_disable_unprepare(priv->bclk);
>>>>>  	regulator_disable(priv->vref);
>>>>> +	stm32_adc_core_switches_supply_dis(dev);
>>>>> +}
>>>>> +
>>>>> +static int stm32_adc_core_syscfg_probe(struct device_node *np,
>>>>> +				       struct stm32_adc_priv *priv)
>>>>> +{
>>>>> +	if (!priv->cfg->regs->has_syscfg)
>>>>> +		return 0;
>>>>> +
>>>>> +	priv->syscfg = syscon_regmap_lookup_by_phandle(np, "st,syscfg");
>>>>> +	if (IS_ERR(priv->syscfg)) {
>>>>> +		/* Optional */
>>>>> +		if (PTR_ERR(priv->syscfg) != -ENODEV)
>>>>> +			return PTR_ERR(priv->syscfg);
>>>>> +		priv->syscfg = NULL;
>>>>> +	}
>>>>> +
>>>>> +	return 0;
>>>>>  }
>>>>>  
>>>>>  static int stm32_adc_probe(struct platform_device *pdev)
>>>>> @@ -475,6 +672,30 @@ static int stm32_adc_probe(struct platform_device *pdev)
>>>>>  		return ret;
>>>>>  	}
>>>>>  
>>>>> +	priv->vdda = devm_regulator_get_optional(&pdev->dev, "vdda");
>>>>> +	if (IS_ERR(priv->vdda)) {
>>>>> +		ret = PTR_ERR(priv->vdda);
>>>>> +		if (ret != -ENODEV) {
>>>>> +			if (ret != -EPROBE_DEFER)
>>>>> +				dev_err(&pdev->dev, "vdda get failed, %d\n",
>>>>> +					ret);
>>>>> +			return ret;
>>>>> +		}
>>>>> +		priv->vdda = NULL;
>>>>> +	}
>>>>> +
>>>>> +	priv->vdd = devm_regulator_get_optional(&pdev->dev, "vdd");
>>>>> +	if (IS_ERR(priv->vdd)) {
>>>>> +		ret = PTR_ERR(priv->vdd);
>>>>> +		if (ret != -ENODEV) {
>>>>> +			if (ret != -EPROBE_DEFER)
>>>>> +				dev_err(&pdev->dev, "vdd get failed, %d\n",
>>>>> +					ret);
>>>>> +			return ret;
>>>>> +		}
>>>>> +		priv->vdd = NULL;
>>>>> +	}
>>>>> +
>>>>>  	priv->aclk = devm_clk_get(&pdev->dev, "adc");
>>>>>  	if (IS_ERR(priv->aclk)) {
>>>>>  		ret = PTR_ERR(priv->aclk);
>>>>> @@ -495,6 +716,13 @@ static int stm32_adc_probe(struct platform_device *pdev)
>>>>>  		priv->bclk = NULL;
>>>>>  	}
>>>>>  
>>>>> +	ret = stm32_adc_core_syscfg_probe(np, priv);
>>>>> +	if (ret) {
>>>>> +		if (ret != -EPROBE_DEFER)
>>>>> +			dev_err(&pdev->dev, "Can't probe syscfg: %d\n", ret);
>>>>> +		return ret;
>>>>> +	}
>>>>> +
>>>>>  	pm_runtime_get_noresume(dev);
>>>>>  	pm_runtime_set_active(dev);
>>>>>  	pm_runtime_set_autosuspend_delay(dev, STM32_ADC_CORE_SLEEP_DELAY_MS);
>>>>> @@ -595,7 +823,7 @@ static const struct stm32_adc_priv_cfg stm32h7_adc_priv_cfg = {
>>>>>  };
>>>>>  
>>>>>  static const struct stm32_adc_priv_cfg stm32mp1_adc_priv_cfg = {
>>>>> -	.regs = &stm32h7_adc_common_regs,
>>>>> +	.regs = &stm32mp1_adc_common_regs,
>>>>>  	.clk_sel = stm32h7_adc_clk_sel,
>>>>>  	.max_clk_rate_hz = 40000000,
>>>>>  };  
>>>>  
> 

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

[PATCH 3/3] ARM: dts: stm32: add ADC analog switches syscfg on stm32mp157c

[Fabrice Gasnier]

On stm32mp157c, the ADC inputs are multiplexed with analog switches which
have reduced performances when their supply is below 2.7V (vdda by
default).
Add syscfg registers that can be used on stm32mp157c, to get full ADC
analog performances.

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
---
 arch/arm/boot/dts/stm32mp157c.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/stm32mp157c.dtsi b/arch/arm/boot/dts/stm32mp157c.dtsi
index 2afeee6..64d71c9 100644
--- a/arch/arm/boot/dts/stm32mp157c.dtsi
+++ b/arch/arm/boot/dts/stm32mp157c.dtsi
@@ -856,6 +856,7 @@
 			clocks = <&rcc ADC12>, <&rcc ADC12_K>;
 			clock-names = "bus", "adc";
 			interrupt-controller;
+			st,syscfg = <&syscfg>;
 			#interrupt-cells = <1>;
 			#address-cells = <1>;
 			#size-cells = <0>;
-- 
2.7.4


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