2023-07-27 11:14:59

by Antoniu Miclaus

[permalink] [raw]
Subject: [PATCH v2 1/2] dt-bindings:iio:frequency:admv1013: add vcc regs

Add bindings for the VCC regulators of the ADMV1013 microware
upconverter.

Signed-off-by: Antoniu Miclaus <[email protected]>
---
changes in v2:
- make the vcc regulators as required.
.../bindings/iio/frequency/adi,admv1013.yaml | 60 +++++++++++++++++++
1 file changed, 60 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/frequency/adi,admv1013.yaml b/Documentation/devicetree/bindings/iio/frequency/adi,admv1013.yaml
index fc813bcb6532..f2eb2287ed9e 100644
--- a/Documentation/devicetree/bindings/iio/frequency/adi,admv1013.yaml
+++ b/Documentation/devicetree/bindings/iio/frequency/adi,admv1013.yaml
@@ -39,6 +39,46 @@ properties:
description:
Analog voltage regulator.

+ vcc-drv-supply:
+ description:
+ RF Driver voltage regulator.
+
+ vcc2-drv-supply:
+ description:
+ RF predriver voltage regulator.
+
+ vcc-vva-supply:
+ description:
+ VVA Control Circuit voltage regulator.
+
+ vcc-amp1-supply:
+ description:
+ RF Amplifier 1 voltage regulator.
+
+ vcc-amp2-supply:
+ description:
+ RF Amplifier 2 voltage regulator.
+
+ vcc-env-supply:
+ description:
+ Envelope Detector voltage regulator.
+
+ vcc-bg-supply:
+ description:
+ Mixer Chip Band Gap Circuit voltage regulator.
+
+ vcc-bg2-supply:
+ description:
+ VGA Chip Band Gap Circuit voltage regulator.
+
+ vcc-mixer-supply:
+ description:
+ Mixer voltage regulator.
+
+ vcc-quad-supply:
+ description:
+ Quadruppler voltage regulator.
+
adi,detector-enable:
description:
Enable the Envelope Detector available at output pins VENV_P and
@@ -69,6 +109,16 @@ required:
- clocks
- clock-names
- vcm-supply
+ - vcc-drv-supply
+ - vcc2-drv-supply
+ - vcc-vva-supply
+ - vcc-amp1-supply
+ - vcc-amp2-supply
+ - vcc-env-supply
+ - vcc-bg-supply
+ - vcc-bg2-supply
+ - vcc-mixer-supply
+ - vcc-quad-supply

allOf:
- $ref: /schemas/spi/spi-peripheral-props.yaml#
@@ -87,6 +137,16 @@ examples:
clocks = <&admv1013_lo>;
clock-names = "lo_in";
vcm-supply = <&vcm>;
+ vcc-drv-supply = <&vcc_drv>;
+ vcc2-drv-supply = <&vcc2_drv>;
+ vcc-vva-supply = <&vcc_vva>;
+ vcc-amp1-supply = <&vcc_amp1>;
+ vcc-amp2-supply = <&vcc_amp2>;
+ vcc-env-supply = <&vcc_env>;
+ vcc-bg-supply = <&vcc_bg>;
+ vcc-bg2-supply = <&vcc_bg2>;
+ vcc-mixer-supply = <&vcc_mixer>;
+ vcc-quad-supply = <&vcc_quad>;
adi,quad-se-mode = "diff";
adi,detector-enable;
};
--
2.41.0



2023-07-27 11:31:38

by Antoniu Miclaus

[permalink] [raw]
Subject: [PATCH v2 2/2] drivers:iio:admv1013: add vcc regulators

Add regulators for the VCC supplies of the admv1013.

The patch aims to align the implementation with the current admv1014
driver where all the VCC supplies are handled as regulators.

Signed-off-by: Antoniu Miclaus <[email protected]>
---
no changes in v2.
drivers/iio/frequency/admv1013.c | 35 ++++++++++++++++++++++++++++++++
1 file changed, 35 insertions(+)

diff --git a/drivers/iio/frequency/admv1013.c b/drivers/iio/frequency/admv1013.c
index 9bf8337806fc..086e2f35b52c 100644
--- a/drivers/iio/frequency/admv1013.c
+++ b/drivers/iio/frequency/admv1013.c
@@ -73,6 +73,7 @@
#define ADMV1013_REG_ADDR_READ_MSK GENMASK(6, 1)
#define ADMV1013_REG_ADDR_WRITE_MSK GENMASK(22, 17)
#define ADMV1013_REG_DATA_MSK GENMASK(16, 1)
+#define ADMV1013_VCC_NUM_REGULATORS 10

enum {
ADMV1013_IQ_MODE,
@@ -96,6 +97,7 @@ struct admv1013_state {
/* Protect against concurrent accesses to the device and to data */
struct mutex lock;
struct regulator *reg;
+ struct regulator_bulk_data vcc_regs[ADMV1013_VCC_NUM_REGULATORS];
struct notifier_block nb;
unsigned int input_mode;
unsigned int quad_se_mode;
@@ -379,6 +381,11 @@ static const struct iio_info admv1013_info = {
.debugfs_reg_access = &admv1013_reg_access,
};

+static const char * const admv1013_reg_name[] = {
+ "vcc-drv", "vcc2-drv", "vcc-vva", "vcc-amp1", "vcc-amp2",
+ "vcc-env", "vcc-bg", "vcc-bg2", "vcc-mixer", "vcc-quad"
+};
+
static int admv1013_freq_change(struct notifier_block *nb, unsigned long action, void *data)
{
struct admv1013_state *st = container_of(nb, struct admv1013_state, nb);
@@ -495,6 +502,11 @@ static void admv1013_reg_disable(void *data)
regulator_disable(data);
}

+static void admv1013_vcc_reg_disable(void *data)
+{
+ regulator_bulk_disable(ADMV1013_VCC_NUM_REGULATORS, data);
+}
+
static void admv1013_powerdown(void *data)
{
unsigned int enable_reg, enable_reg_msk;
@@ -520,6 +532,7 @@ static void admv1013_powerdown(void *data)
static int admv1013_properties_parse(struct admv1013_state *st)
{
int ret;
+ unsigned int i;
const char *str;
struct spi_device *spi = st->spi;

@@ -554,6 +567,17 @@ static int admv1013_properties_parse(struct admv1013_state *st)
return dev_err_probe(&spi->dev, PTR_ERR(st->reg),
"failed to get the common-mode voltage\n");

+ for (i = 0; i < ADMV1013_VCC_NUM_REGULATORS; ++i)
+ st->vcc_regs[i].supply = admv1013_reg_name[i];
+
+ ret = devm_regulator_bulk_get(&st->spi->dev,
+ ADMV1013_VCC_NUM_REGULATORS,
+ st->vcc_regs);
+ if (ret) {
+ dev_err(&spi->dev, "Failed to request VCC regulators");
+ return ret;
+ }
+
return 0;
}

@@ -591,6 +615,17 @@ static int admv1013_probe(struct spi_device *spi)
if (ret)
return ret;

+ ret = regulator_bulk_enable(ADMV1013_VCC_NUM_REGULATORS, st->vcc_regs);
+ if (ret) {
+ dev_err(&spi->dev, "Failed to enable regulators");
+ return ret;
+ }
+
+ ret = devm_add_action_or_reset(&spi->dev, admv1013_vcc_reg_disable,
+ st->vcc_regs);
+ if (ret)
+ return ret;
+
st->clkin = devm_clk_get_enabled(&spi->dev, "lo_in");
if (IS_ERR(st->clkin))
return dev_err_probe(&spi->dev, PTR_ERR(st->clkin),
--
2.41.0


2023-07-27 11:58:49

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] drivers:iio:admv1013: add vcc regulators

On 27/07/2023 13:01, Antoniu Miclaus wrote:
> Add regulators for the VCC supplies of the admv1013.
>
> The patch aims to align the implementation with the current admv1014

...

> const char *str;
> struct spi_device *spi = st->spi;
>
> @@ -554,6 +567,17 @@ static int admv1013_properties_parse(struct admv1013_state *st)
> return dev_err_probe(&spi->dev, PTR_ERR(st->reg),
> "failed to get the common-mode voltage\n");
>
> + for (i = 0; i < ADMV1013_VCC_NUM_REGULATORS; ++i)
> + st->vcc_regs[i].supply = admv1013_reg_name[i];
> +
> + ret = devm_regulator_bulk_get(&st->spi->dev,
> + ADMV1013_VCC_NUM_REGULATORS,
> + st->vcc_regs);
> + if (ret) {
> + dev_err(&spi->dev, "Failed to request VCC regulators");
> + return ret;

This should be return dev_err_probe, unless this is not called from
probe path.

Best regards,
Krzysztof


2023-07-27 12:09:48

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] dt-bindings:iio:frequency:admv1013: add vcc regs

On 27/07/2023 13:01, Antoniu Miclaus wrote:
> Add bindings for the VCC regulators of the ADMV1013 microware
> upconverter.
>
> Signed-off-by: Antoniu Miclaus <[email protected]>
> ---
> changes in v2:
> - make the vcc regulators as required.

No improvements in the subject... Please wait some time before sending
new versions.

Best regards,
Krzysztof


2023-07-29 14:35:15

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] drivers:iio:admv1013: add vcc regulators

On Thu, 27 Jul 2023 14:01:21 +0300
Antoniu Miclaus <[email protected]> wrote:

> Add regulators for the VCC supplies of the admv1013.
>
> The patch aims to align the implementation with the current admv1014
> driver where all the VCC supplies are handled as regulators.
>
> Signed-off-by: Antoniu Miclaus <[email protected]>
Hi Antoniu

Use the devm_regulator_bulk_get_enable() route
to simplify things for these regulators. The vcm one needs to remain
handled separately as we need to get the voltage for that one and hence
need access to the struct regulator.

> ---
> no changes in v2.
> drivers/iio/frequency/admv1013.c | 35 ++++++++++++++++++++++++++++++++
> 1 file changed, 35 insertions(+)
>
> diff --git a/drivers/iio/frequency/admv1013.c b/drivers/iio/frequency/admv1013.c
> index 9bf8337806fc..086e2f35b52c 100644
> --- a/drivers/iio/frequency/admv1013.c
> +++ b/drivers/iio/frequency/admv1013.c
> @@ -73,6 +73,7 @@
> #define ADMV1013_REG_ADDR_READ_MSK GENMASK(6, 1)
> #define ADMV1013_REG_ADDR_WRITE_MSK GENMASK(22, 17)
> #define ADMV1013_REG_DATA_MSK GENMASK(16, 1)
> +#define ADMV1013_VCC_NUM_REGULATORS 10
>
> enum {
> ADMV1013_IQ_MODE,
> @@ -96,6 +97,7 @@ struct admv1013_state {
> /* Protect against concurrent accesses to the device and to data */
> struct mutex lock;
> struct regulator *reg;
> + struct regulator_bulk_data vcc_regs[ADMV1013_VCC_NUM_REGULATORS];
> struct notifier_block nb;
> unsigned int input_mode;
> unsigned int quad_se_mode;
> @@ -379,6 +381,11 @@ static const struct iio_info admv1013_info = {
> .debugfs_reg_access = &admv1013_reg_access,
> };
>
> +static const char * const admv1013_reg_name[] = {
> + "vcc-drv", "vcc2-drv", "vcc-vva", "vcc-amp1", "vcc-amp2",
> + "vcc-env", "vcc-bg", "vcc-bg2", "vcc-mixer", "vcc-quad"
> +};
> +
> static int admv1013_freq_change(struct notifier_block *nb, unsigned long action, void *data)
> {
> struct admv1013_state *st = container_of(nb, struct admv1013_state, nb);
> @@ -495,6 +502,11 @@ static void admv1013_reg_disable(void *data)
> regulator_disable(data);
> }
>
> +static void admv1013_vcc_reg_disable(void *data)
> +{
> + regulator_bulk_disable(ADMV1013_VCC_NUM_REGULATORS, data);
> +}
> +
> static void admv1013_powerdown(void *data)
> {
> unsigned int enable_reg, enable_reg_msk;
> @@ -520,6 +532,7 @@ static void admv1013_powerdown(void *data)
> static int admv1013_properties_parse(struct admv1013_state *st)
> {
> int ret;
> + unsigned int i;
> const char *str;
> struct spi_device *spi = st->spi;
>
> @@ -554,6 +567,17 @@ static int admv1013_properties_parse(struct admv1013_state *st)
> return dev_err_probe(&spi->dev, PTR_ERR(st->reg),
> "failed to get the common-mode voltage\n");
>
> + for (i = 0; i < ADMV1013_VCC_NUM_REGULATORS; ++i)
> + st->vcc_regs[i].supply = admv1013_reg_name[i];
> +
> + ret = devm_regulator_bulk_get(&st->spi->dev,
> + ADMV1013_VCC_NUM_REGULATORS,
> + st->vcc_regs);
> + if (ret) {
> + dev_err(&spi->dev, "Failed to request VCC regulators");
> + return ret;
> + }
> +
> return 0;
> }
>
> @@ -591,6 +615,17 @@ static int admv1013_probe(struct spi_device *spi)
> if (ret)
> return ret;
>
> + ret = regulator_bulk_enable(ADMV1013_VCC_NUM_REGULATORS, st->vcc_regs);
> + if (ret) {
> + dev_err(&spi->dev, "Failed to enable regulators");
> + return ret;

I don't think the driver needs explicit access to any of these regulators.

Thus you can use devm_regulator_bulk_get_enable() here and no need to get
them separately earlier.

> + }
> +
> + ret = devm_add_action_or_reset(&spi->dev, admv1013_vcc_reg_disable,
> + st->vcc_regs);
> + if (ret)
> + return ret;
> +
> st->clkin = devm_clk_get_enabled(&spi->dev, "lo_in");
> if (IS_ERR(st->clkin))
> return dev_err_probe(&spi->dev, PTR_ERR(st->clkin),