* [PATCH 1/6] dt-bindings: iio: dac: ad5791: Add optional reset, clr and ldac gpios
2024-10-28 7:11 [PATCH 0/6] Improvements and Enhancements for AD5791 DAC Driver ahaslam
@ 2024-10-28 7:11 ` ahaslam
2024-10-28 7:42 ` Krzysztof Kozlowski
2024-10-28 7:11 ` [PATCH 2/6] dt-bindings: iio: dac: ad5791: Add required voltage supplies ahaslam
` (4 subsequent siblings)
5 siblings, 1 reply; 17+ messages in thread
From: ahaslam @ 2024-10-28 7:11 UTC (permalink / raw)
To: lars, Michael.Hennerich, jic23, robh, krzk+dt, conor+dt, nuno.sa,
dlechner
Cc: linux-iio, devicetree, linux-kernel, Axel Haslam
From: Axel Haslam <ahaslam@baylibre.com>
Depending on board layout, the ad57xx may need control of reset, clear,
and ldac pins by the host driver. Add optional bindings for these gpios.
Signed-off-by: Axel Haslam <ahaslam@baylibre.com>
---
.../devicetree/bindings/iio/dac/adi,ad5791.yaml | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/Documentation/devicetree/bindings/iio/dac/adi,ad5791.yaml b/Documentation/devicetree/bindings/iio/dac/adi,ad5791.yaml
index c81285d84db7..fe664378c966 100644
--- a/Documentation/devicetree/bindings/iio/dac/adi,ad5791.yaml
+++ b/Documentation/devicetree/bindings/iio/dac/adi,ad5791.yaml
@@ -31,6 +31,17 @@ properties:
gain of two configuration.
type: boolean
+ reset-gpios:
+ maxItems: 1
+
+ clear-gpios:
+ maxItems: 1
+
+ ldac-gpios:
+ description:
+ LDAC pin to be used as a hardware trigger to update the DAC channels.
+ maxItems: 1
+
required:
- compatible
- reg
@@ -44,6 +55,7 @@ unevaluatedProperties: false
examples:
- |
+ #include <dt-bindings/gpio/gpio.h>
spi {
#address-cells = <1>;
#size-cells = <0>;
@@ -53,6 +65,9 @@ examples:
reg = <0>;
vss-supply = <&dac_vss>;
vdd-supply = <&dac_vdd>;
+ reset-gpios = <&gpio_bd 16 GPIO_ACTIVE_LOW>;
+ clear-gpios = <&gpio_bd 17 GPIO_ACTIVE_LOW>;
+ ldac-gpios = <&gpio_bd 18 GPIO_ACTIVE_HIGH>;
};
};
...
--
2.34.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 1/6] dt-bindings: iio: dac: ad5791: Add optional reset, clr and ldac gpios
2024-10-28 7:11 ` [PATCH 1/6] dt-bindings: iio: dac: ad5791: Add optional reset, clr and ldac gpios ahaslam
@ 2024-10-28 7:42 ` Krzysztof Kozlowski
0 siblings, 0 replies; 17+ messages in thread
From: Krzysztof Kozlowski @ 2024-10-28 7:42 UTC (permalink / raw)
To: ahaslam
Cc: lars, Michael.Hennerich, jic23, robh, krzk+dt, conor+dt, nuno.sa,
dlechner, linux-iio, devicetree, linux-kernel
On Mon, Oct 28, 2024 at 08:11:13AM +0100, ahaslam@baylibre.com wrote:
> From: Axel Haslam <ahaslam@baylibre.com>
>
> Depending on board layout, the ad57xx may need control of reset, clear,
> and ldac pins by the host driver. Add optional bindings for these gpios.
>
> Signed-off-by: Axel Haslam <ahaslam@baylibre.com>
> ---
> .../devicetree/bindings/iio/dac/adi,ad5791.yaml | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 2/6] dt-bindings: iio: dac: ad5791: Add required voltage supplies
2024-10-28 7:11 [PATCH 0/6] Improvements and Enhancements for AD5791 DAC Driver ahaslam
2024-10-28 7:11 ` [PATCH 1/6] dt-bindings: iio: dac: ad5791: Add optional reset, clr and ldac gpios ahaslam
@ 2024-10-28 7:11 ` ahaslam
2024-10-28 8:06 ` Krzysztof Kozlowski
2024-10-29 6:27 ` Krzysztof Kozlowski
2024-10-28 7:11 ` [PATCH 3/6] iio: dac: ad5791: Include chip_info in device match tables ahaslam
` (3 subsequent siblings)
5 siblings, 2 replies; 17+ messages in thread
From: ahaslam @ 2024-10-28 7:11 UTC (permalink / raw)
To: lars, Michael.Hennerich, jic23, robh, krzk+dt, conor+dt, nuno.sa,
dlechner
Cc: linux-iio, devicetree, linux-kernel, Axel Haslam
From: Axel Haslam <ahaslam@baylibre.com>
Vcc, iovcc, vrefp, and vrefn are needed for the DAC to work.
Add them as required bindings for ad5791.
Signed-off-by: Axel Haslam <ahaslam@baylibre.com>
---
.../bindings/iio/dac/adi,ad5791.yaml | 24 +++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/Documentation/devicetree/bindings/iio/dac/adi,ad5791.yaml b/Documentation/devicetree/bindings/iio/dac/adi,ad5791.yaml
index fe664378c966..79cb4b78a88a 100644
--- a/Documentation/devicetree/bindings/iio/dac/adi,ad5791.yaml
+++ b/Documentation/devicetree/bindings/iio/dac/adi,ad5791.yaml
@@ -26,6 +26,22 @@ properties:
vdd-supply: true
vss-supply: true
+ vcc-supply:
+ description:
+ Supply that powers the chip.
+
+ iovcc-supply:
+ description:
+ Supply for the digital interface.
+
+ vrefp-supply:
+ description:
+ Positive referance input voltage range. From 5v to (vdd - 2.5)
+
+ vrefn-supply:
+ description:
+ Negative referance input voltage range. From (vss + 2.5) to 0.
+
adi,rbuf-gain2-en:
description: Specify to allow an external amplifier to be connected in a
gain of two configuration.
@@ -47,6 +63,10 @@ required:
- reg
- vdd-supply
- vss-supply
+ - vcc-supply
+ - iovcc-supply
+ - vrefp-supply
+ - vrefn-supply
allOf:
- $ref: /schemas/spi/spi-peripheral-props.yaml#
@@ -65,6 +85,10 @@ examples:
reg = <0>;
vss-supply = <&dac_vss>;
vdd-supply = <&dac_vdd>;
+ vcc-supply = <&dac_vcc>;
+ iovcc-supply = <&dac_iovcc>;
+ vrefp-supply = <&dac_vrefp>;
+ vrefn-supply = <&dac_vrefn>;
reset-gpios = <&gpio_bd 16 GPIO_ACTIVE_LOW>;
clear-gpios = <&gpio_bd 17 GPIO_ACTIVE_LOW>;
ldac-gpios = <&gpio_bd 18 GPIO_ACTIVE_HIGH>;
--
2.34.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 2/6] dt-bindings: iio: dac: ad5791: Add required voltage supplies
2024-10-28 7:11 ` [PATCH 2/6] dt-bindings: iio: dac: ad5791: Add required voltage supplies ahaslam
@ 2024-10-28 8:06 ` Krzysztof Kozlowski
2024-10-28 23:07 ` Axel Haslam
2024-10-29 6:27 ` Krzysztof Kozlowski
1 sibling, 1 reply; 17+ messages in thread
From: Krzysztof Kozlowski @ 2024-10-28 8:06 UTC (permalink / raw)
To: ahaslam
Cc: lars, Michael.Hennerich, jic23, robh, krzk+dt, conor+dt, nuno.sa,
dlechner, linux-iio, devicetree, linux-kernel
On Mon, Oct 28, 2024 at 08:11:14AM +0100, ahaslam@baylibre.com wrote:
> From: Axel Haslam <ahaslam@baylibre.com>
>
> Vcc, iovcc, vrefp, and vrefn are needed for the DAC to work.
> Add them as required bindings for ad5791.
>
> Signed-off-by: Axel Haslam <ahaslam@baylibre.com>
> ---
> .../bindings/iio/dac/adi,ad5791.yaml | 24 +++++++++++++++++++
> 1 file changed, 24 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/iio/dac/adi,ad5791.yaml b/Documentation/devicetree/bindings/iio/dac/adi,ad5791.yaml
> index fe664378c966..79cb4b78a88a 100644
> --- a/Documentation/devicetree/bindings/iio/dac/adi,ad5791.yaml
> +++ b/Documentation/devicetree/bindings/iio/dac/adi,ad5791.yaml
> @@ -26,6 +26,22 @@ properties:
> vdd-supply: true
> vss-supply: true
>
> + vcc-supply:
> + description:
> + Supply that powers the chip.
> +
> + iovcc-supply:
> + description:
> + Supply for the digital interface.
> +
> + vrefp-supply:
> + description:
> + Positive referance input voltage range. From 5v to (vdd - 2.5)
> +
> + vrefn-supply:
> + description:
> + Negative referance input voltage range. From (vss + 2.5) to 0.
> +
> adi,rbuf-gain2-en:
> description: Specify to allow an external amplifier to be connected in a
> gain of two configuration.
> @@ -47,6 +63,10 @@ required:
> - reg
> - vdd-supply
> - vss-supply
> + - vcc-supply
> + - iovcc-supply
> + - vrefp-supply
> + - vrefn-supply
So you have six required supplies?
Datasheet says "A voltage range of 2.7 V to 5.5 V *can* be connected",
so doesn't it mean this is optional? Although similar wording is for
other supplies, so maybe it's just imprecise language?
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/6] dt-bindings: iio: dac: ad5791: Add required voltage supplies
2024-10-28 8:06 ` Krzysztof Kozlowski
@ 2024-10-28 23:07 ` Axel Haslam
0 siblings, 0 replies; 17+ messages in thread
From: Axel Haslam @ 2024-10-28 23:07 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: lars, Michael.Hennerich, jic23, robh, krzk+dt, conor+dt, nuno.sa,
dlechner, linux-iio, devicetree, linux-kernel
On Mon, 28 Oct 2024 at 09:06, Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On Mon, Oct 28, 2024 at 08:11:14AM +0100, ahaslam@baylibre.com wrote:
> > From: Axel Haslam <ahaslam@baylibre.com>
> >
> > Vcc, iovcc, vrefp, and vrefn are needed for the DAC to work.
> > Add them as required bindings for ad5791.
> >
> > Signed-off-by: Axel Haslam <ahaslam@baylibre.com>
> > ---
> > .../bindings/iio/dac/adi,ad5791.yaml | 24 +++++++++++++++++++
> > 1 file changed, 24 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/iio/dac/adi,ad5791.yaml b/Documentation/devicetree/bindings/iio/dac/adi,ad5791.yaml
> > index fe664378c966..79cb4b78a88a 100644
> > --- a/Documentation/devicetree/bindings/iio/dac/adi,ad5791.yaml
> > +++ b/Documentation/devicetree/bindings/iio/dac/adi,ad5791.yaml
> > @@ -26,6 +26,22 @@ properties:
> > vdd-supply: true
> > vss-supply: true
> >
> > + vcc-supply:
> > + description:
> > + Supply that powers the chip.
> > +
> > + iovcc-supply:
> > + description:
> > + Supply for the digital interface.
> > +
> > + vrefp-supply:
> > + description:
> > + Positive referance input voltage range. From 5v to (vdd - 2.5)
> > +
> > + vrefn-supply:
> > + description:
> > + Negative referance input voltage range. From (vss + 2.5) to 0.
> > +
> > adi,rbuf-gain2-en:
> > description: Specify to allow an external amplifier to be connected in a
> > gain of two configuration.
> > @@ -47,6 +63,10 @@ required:
> > - reg
> > - vdd-supply
> > - vss-supply
> > + - vcc-supply
> > + - iovcc-supply
> > + - vrefp-supply
> > + - vrefn-supply
>
> So you have six required supplies?
>
> Datasheet says "A voltage range of 2.7 V to 5.5 V *can* be connected",
> so doesn't it mean this is optional? Although similar wording is for
> other supplies, so maybe it's just imprecise language?
looks like unfortunate wording. Like you said, Vdd, Vss are already required
and have the same *can* word in their description like all other supplies
which i think its meant for the voltage level options of the power supply.
Vcc: is mentioned as need to "power on" in the startup sequence
section of the datasheet,
iovcc: we can't interface the chip without this supply.
vrefp: minimum input of 5v.
vrefn: from vss up to 0 volts max.
so vcc, iovcc, and vrefp to me, look required for the hw to work.
but i have a small doubt about vrefn since it could potentially be 0V.
Does this mean it should be an optional binding where we assume its 0
if not present?
or is it ok to leave it as required?
Regards
Axel.
>
> Best regards,
> Krzysztof
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/6] dt-bindings: iio: dac: ad5791: Add required voltage supplies
2024-10-28 7:11 ` [PATCH 2/6] dt-bindings: iio: dac: ad5791: Add required voltage supplies ahaslam
2024-10-28 8:06 ` Krzysztof Kozlowski
@ 2024-10-29 6:27 ` Krzysztof Kozlowski
1 sibling, 0 replies; 17+ messages in thread
From: Krzysztof Kozlowski @ 2024-10-29 6:27 UTC (permalink / raw)
To: ahaslam, lars, Michael.Hennerich, jic23, robh, krzk+dt, conor+dt,
nuno.sa, dlechner
Cc: linux-iio, devicetree, linux-kernel
On 28/10/2024 08:11, ahaslam@baylibre.com wrote:
> From: Axel Haslam <ahaslam@baylibre.com>
>
> Vcc, iovcc, vrefp, and vrefn are needed for the DAC to work.
> Add them as required bindings for ad5791.
>
> Signed-off-by: Axel Haslam <ahaslam@baylibre.com>
> ---
> .../bindings/iio/dac/adi,ad5791.yaml | 24 +++++++++++++++++++
> 1 file changed, 24 insertions(+)
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 3/6] iio: dac: ad5791: Include chip_info in device match tables
2024-10-28 7:11 [PATCH 0/6] Improvements and Enhancements for AD5791 DAC Driver ahaslam
2024-10-28 7:11 ` [PATCH 1/6] dt-bindings: iio: dac: ad5791: Add optional reset, clr and ldac gpios ahaslam
2024-10-28 7:11 ` [PATCH 2/6] dt-bindings: iio: dac: ad5791: Add required voltage supplies ahaslam
@ 2024-10-28 7:11 ` ahaslam
2024-10-28 16:38 ` kernel test robot
2024-10-28 20:22 ` Jonathan Cameron
2024-10-28 7:11 ` [PATCH 4/6] iio: dac: ad5791: Add reset, clr and ldac gpios ahaslam
` (2 subsequent siblings)
5 siblings, 2 replies; 17+ messages in thread
From: ahaslam @ 2024-10-28 7:11 UTC (permalink / raw)
To: lars, Michael.Hennerich, jic23, robh, krzk+dt, conor+dt, nuno.sa,
dlechner
Cc: linux-iio, devicetree, linux-kernel, Axel Haslam
From: Axel Haslam <ahaslam@baylibre.com>
Include a chip info struct in device SPI and device OF match tables to
provide channel definitions for each particular ADC model and drop
device enum.
Suggested-by: Nuno Sa <nuno.sa@analog.com>
Signed-off-by: Axel Haslam <ahaslam@baylibre.com>
---
drivers/iio/dac/ad5791.c | 107 +++++++++++++++++++--------------------
1 file changed, 51 insertions(+), 56 deletions(-)
diff --git a/drivers/iio/dac/ad5791.c b/drivers/iio/dac/ad5791.c
index 553431bf0232..a11e81211669 100644
--- a/drivers/iio/dac/ad5791.c
+++ b/drivers/iio/dac/ad5791.c
@@ -65,7 +65,9 @@
*/
struct ad5791_chip_info {
- int (*get_lin_comp) (unsigned int span);
+ const char *name;
+ const struct iio_chan_spec channel;
+ int (*get_lin_comp)(unsigned int span);
};
/**
@@ -98,13 +100,6 @@ struct ad5791_state {
} data[3] __aligned(IIO_DMA_MINALIGN);
};
-enum ad5791_supported_device_ids {
- ID_AD5760,
- ID_AD5780,
- ID_AD5781,
- ID_AD5791,
-};
-
static int ad5791_spi_write(struct ad5791_state *st, u8 addr, u32 val)
{
st->data[0].d32 = cpu_to_be32(AD5791_CMD_WRITE |
@@ -228,20 +223,6 @@ static int ad5780_get_lin_comp(unsigned int span)
else
return AD5780_LINCOMP_10_20;
}
-static const struct ad5791_chip_info ad5791_chip_info_tbl[] = {
- [ID_AD5760] = {
- .get_lin_comp = ad5780_get_lin_comp,
- },
- [ID_AD5780] = {
- .get_lin_comp = ad5780_get_lin_comp,
- },
- [ID_AD5781] = {
- .get_lin_comp = ad5791_get_lin_comp,
- },
- [ID_AD5791] = {
- .get_lin_comp = ad5791_get_lin_comp,
- },
-};
static int ad5791_read_raw(struct iio_dev *indio_dev,
struct iio_chan_spec const *chan,
@@ -289,30 +270,34 @@ static const struct iio_chan_spec_ext_info ad5791_ext_info[] = {
{ },
};
-#define AD5791_CHAN(bits, _shift) { \
- .type = IIO_VOLTAGE, \
- .output = 1, \
- .indexed = 1, \
- .address = AD5791_ADDR_DAC0, \
- .channel = 0, \
- .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
- .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) | \
- BIT(IIO_CHAN_INFO_OFFSET), \
- .scan_type = { \
- .sign = 'u', \
- .realbits = (bits), \
- .storagebits = 24, \
- .shift = (_shift), \
- }, \
- .ext_info = ad5791_ext_info, \
+#define AD5791_DEFINE_CHIP_INFO(_name, bits, _shift, _lin_comp) \
+static const struct ad5791_chip_info _name##_chip_info = { \
+ .name = #_name, \
+ .get_lin_comp = &(_lin_comp), \
+ .channel = { \
+ .type = IIO_VOLTAGE, \
+ .output = 1, \
+ .indexed = 1, \
+ .address = AD5791_ADDR_DAC0, \
+ .channel = 0, \
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
+ .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) | \
+ BIT(IIO_CHAN_INFO_OFFSET), \
+ .scan_type = { \
+ .sign = 'u', \
+ .realbits = (bits), \
+ .storagebits = 24, \
+ .shift = (_shift), \
+ }, \
+ .ext_info = ad5791_ext_info, \
+ }, \
}
-static const struct iio_chan_spec ad5791_channels[] = {
- [ID_AD5760] = AD5791_CHAN(16, 4),
- [ID_AD5780] = AD5791_CHAN(18, 2),
- [ID_AD5781] = AD5791_CHAN(18, 2),
- [ID_AD5791] = AD5791_CHAN(20, 0)
-};
+AD5791_DEFINE_CHIP_INFO(ad5760, 16, 4, ad5780_get_lin_comp);
+AD5791_DEFINE_CHIP_INFO(ad5780, 18, 2, ad5780_get_lin_comp);
+AD5791_DEFINE_CHIP_INFO(ad5781, 18, 2, ad5791_get_lin_comp);
+AD5791_DEFINE_CHIP_INFO(ad5790, 20, 0, ad5791_get_lin_comp);
+AD5791_DEFINE_CHIP_INFO(ad5791, 20, 0, ad5791_get_lin_comp);
static int ad5791_write_raw(struct iio_dev *indio_dev,
struct iio_chan_spec const *chan,
@@ -400,9 +385,9 @@ static int ad5791_probe(struct spi_device *spi)
if (ret)
goto error_disable_reg_neg;
- st->chip_info = &ad5791_chip_info_tbl[spi_get_device_id(spi)
- ->driver_data];
-
+ st->chip_info = spi_get_device_match_data(spi);
+ if (!st->chip_info)
+ return dev_err_probe(&spi->dev, -EINVAL, "no chip info\n");
st->ctrl = AD5761_CTRL_LINCOMP(st->chip_info->get_lin_comp(st->vref_mv))
| (use_rbuf_gain2 ? 0 : AD5791_CTRL_RBUF) |
@@ -416,10 +401,9 @@ static int ad5791_probe(struct spi_device *spi)
spi_set_drvdata(spi, indio_dev);
indio_dev->info = &ad5791_info;
indio_dev->modes = INDIO_DIRECT_MODE;
- indio_dev->channels
- = &ad5791_channels[spi_get_device_id(spi)->driver_data];
+ indio_dev->channels = &st->chip_info->channel;
indio_dev->num_channels = 1;
- indio_dev->name = spi_get_device_id(st->spi)->name;
+ indio_dev->name = st->chip_info->name;
ret = iio_device_register(indio_dev);
if (ret)
goto error_disable_reg_neg;
@@ -448,19 +432,30 @@ static void ad5791_remove(struct spi_device *spi)
regulator_disable(st->reg_vss);
}
+static const struct of_device_id ad5791_of_match[] = {
+ { .compatible = "adi,ad5760", .data = &ad5760_chip_info },
+ { .compatible = "adi,ad5780", .data = &ad5780_chip_info },
+ { .compatible = "adi,ad5781", .data = &ad5781_chip_info },
+ { .compatible = "adi,ad5790", .data = &ad5790_chip_info },
+ { .compatible = "adi,ad5791", .data = &ad5791_chip_info },
+ { }
+};
+MODULE_DEVICE_TABLE(of, ad5791_of_match);
+
static const struct spi_device_id ad5791_id[] = {
- {"ad5760", ID_AD5760},
- {"ad5780", ID_AD5780},
- {"ad5781", ID_AD5781},
- {"ad5790", ID_AD5791},
- {"ad5791", ID_AD5791},
- {}
+ { "ad5760", (kernel_ulong_t)&ad5760_chip_info },
+ { "ad5780", (kernel_ulong_t)&ad5780_chip_info },
+ { "ad5781", (kernel_ulong_t)&ad5781_chip_info },
+ { "ad5790", (kernel_ulong_t)&ad5790_chip_info },
+ { "ad5791", (kernel_ulong_t)&ad5791_chip_info },
+ { }
};
MODULE_DEVICE_TABLE(spi, ad5791_id);
static struct spi_driver ad5791_driver = {
.driver = {
.name = "ad5791",
+ .of_match_table = ad5791_of_match,
},
.probe = ad5791_probe,
.remove = ad5791_remove,
--
2.34.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 3/6] iio: dac: ad5791: Include chip_info in device match tables
2024-10-28 7:11 ` [PATCH 3/6] iio: dac: ad5791: Include chip_info in device match tables ahaslam
@ 2024-10-28 16:38 ` kernel test robot
2024-10-28 20:22 ` Jonathan Cameron
1 sibling, 0 replies; 17+ messages in thread
From: kernel test robot @ 2024-10-28 16:38 UTC (permalink / raw)
To: ahaslam, lars, Michael.Hennerich, jic23, robh, krzk+dt, conor+dt,
nuno.sa, dlechner
Cc: oe-kbuild-all, linux-iio, devicetree, linux-kernel, Axel Haslam
Hi,
kernel test robot noticed the following build warnings:
[auto build test WARNING on jic23-iio/togreg]
[also build test WARNING on linus/master v6.12-rc5 next-20241028]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/ahaslam-baylibre-com/dt-bindings-iio-dac-ad5791-Add-optional-reset-clr-and-ldac-gpios/20241028-151319
base: https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
patch link: https://lore.kernel.org/r/20241028071118.699951-4-ahaslam%40baylibre.com
patch subject: [PATCH 3/6] iio: dac: ad5791: Include chip_info in device match tables
config: parisc-randconfig-r071-20241028 (https://download.01.org/0day-ci/archive/20241028/202410282349.YFq0jd85-lkp@intel.com/config)
compiler: hppa-linux-gcc (GCC) 14.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241028/202410282349.YFq0jd85-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202410282349.YFq0jd85-lkp@intel.com/
All warnings (new ones prefixed by >>):
>> drivers/iio/dac/ad5791.c:71: warning: Function parameter or struct member 'name' not described in 'ad5791_chip_info'
>> drivers/iio/dac/ad5791.c:71: warning: Function parameter or struct member 'channel' not described in 'ad5791_chip_info'
vim +71 drivers/iio/dac/ad5791.c
20374d1a36df3e drivers/staging/iio/dac/ad5791.c Lars-Peter Clausen 2012-06-04 61
20374d1a36df3e drivers/staging/iio/dac/ad5791.c Lars-Peter Clausen 2012-06-04 62 /**
20374d1a36df3e drivers/staging/iio/dac/ad5791.c Lars-Peter Clausen 2012-06-04 63 * struct ad5791_chip_info - chip specific information
20374d1a36df3e drivers/staging/iio/dac/ad5791.c Lars-Peter Clausen 2012-06-04 64 * @get_lin_comp: function pointer to the device specific function
20374d1a36df3e drivers/staging/iio/dac/ad5791.c Lars-Peter Clausen 2012-06-04 65 */
20374d1a36df3e drivers/staging/iio/dac/ad5791.c Lars-Peter Clausen 2012-06-04 66
20374d1a36df3e drivers/staging/iio/dac/ad5791.c Lars-Peter Clausen 2012-06-04 67 struct ad5791_chip_info {
18e83b303d6e05 drivers/iio/dac/ad5791.c Axel Haslam 2024-10-28 68 const char *name;
18e83b303d6e05 drivers/iio/dac/ad5791.c Axel Haslam 2024-10-28 69 const struct iio_chan_spec channel;
20374d1a36df3e drivers/staging/iio/dac/ad5791.c Lars-Peter Clausen 2012-06-04 70 int (*get_lin_comp)(unsigned int span);
20374d1a36df3e drivers/staging/iio/dac/ad5791.c Lars-Peter Clausen 2012-06-04 @71 };
20374d1a36df3e drivers/staging/iio/dac/ad5791.c Lars-Peter Clausen 2012-06-04 72
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 3/6] iio: dac: ad5791: Include chip_info in device match tables
2024-10-28 7:11 ` [PATCH 3/6] iio: dac: ad5791: Include chip_info in device match tables ahaslam
2024-10-28 16:38 ` kernel test robot
@ 2024-10-28 20:22 ` Jonathan Cameron
1 sibling, 0 replies; 17+ messages in thread
From: Jonathan Cameron @ 2024-10-28 20:22 UTC (permalink / raw)
To: ahaslam
Cc: lars, Michael.Hennerich, robh, krzk+dt, conor+dt, nuno.sa,
dlechner, linux-iio, devicetree, linux-kernel
On Mon, 28 Oct 2024 08:11:15 +0100
ahaslam@baylibre.com wrote:
> From: Axel Haslam <ahaslam@baylibre.com>
>
> Include a chip info struct in device SPI and device OF match tables to
> provide channel definitions for each particular ADC model and drop
> device enum.
>
> Suggested-by: Nuno Sa <nuno.sa@analog.com>
> Signed-off-by: Axel Haslam <ahaslam@baylibre.com>
> ---
> drivers/iio/dac/ad5791.c | 107 +++++++++++++++++++--------------------
> 1 file changed, 51 insertions(+), 56 deletions(-)
>
> diff --git a/drivers/iio/dac/ad5791.c b/drivers/iio/dac/ad5791.c
> index 553431bf0232..a11e81211669 100644
> --- a/drivers/iio/dac/ad5791.c
> +++ b/drivers/iio/dac/ad5791.c
> @@ -65,7 +65,9 @@
> */
Whilst adding the docs for the missing entries, please delete this
blank line!
Otherwise patch looks good to me.
Jonathan
>
> struct ad5791_chip_info {
> - int (*get_lin_comp) (unsigned int span);
> + const char *name;
> + const struct iio_chan_spec channel;
> + int (*get_lin_comp)(unsigned int span);
> };
>
> /**
> @@ -98,13 +100,6 @@ struct ad5791_state {
> } data[3] __aligned(IIO_DMA_MINALIGN);
> };
>
> -enum ad5791_supported_device_ids {
> - ID_AD5760,
> - ID_AD5780,
> - ID_AD5781,
> - ID_AD5791,
> -};
> -
> static int ad5791_spi_write(struct ad5791_state *st, u8 addr, u32 val)
> {
> st->data[0].d32 = cpu_to_be32(AD5791_CMD_WRITE |
> @@ -228,20 +223,6 @@ static int ad5780_get_lin_comp(unsigned int span)
> else
> return AD5780_LINCOMP_10_20;
> }
> -static const struct ad5791_chip_info ad5791_chip_info_tbl[] = {
> - [ID_AD5760] = {
> - .get_lin_comp = ad5780_get_lin_comp,
> - },
> - [ID_AD5780] = {
> - .get_lin_comp = ad5780_get_lin_comp,
> - },
> - [ID_AD5781] = {
> - .get_lin_comp = ad5791_get_lin_comp,
> - },
> - [ID_AD5791] = {
> - .get_lin_comp = ad5791_get_lin_comp,
> - },
> -};
>
> static int ad5791_read_raw(struct iio_dev *indio_dev,
> struct iio_chan_spec const *chan,
> @@ -289,30 +270,34 @@ static const struct iio_chan_spec_ext_info ad5791_ext_info[] = {
> { },
> };
>
> -#define AD5791_CHAN(bits, _shift) { \
> - .type = IIO_VOLTAGE, \
> - .output = 1, \
> - .indexed = 1, \
> - .address = AD5791_ADDR_DAC0, \
> - .channel = 0, \
> - .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
> - .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) | \
> - BIT(IIO_CHAN_INFO_OFFSET), \
> - .scan_type = { \
> - .sign = 'u', \
> - .realbits = (bits), \
> - .storagebits = 24, \
> - .shift = (_shift), \
> - }, \
> - .ext_info = ad5791_ext_info, \
> +#define AD5791_DEFINE_CHIP_INFO(_name, bits, _shift, _lin_comp) \
> +static const struct ad5791_chip_info _name##_chip_info = { \
> + .name = #_name, \
> + .get_lin_comp = &(_lin_comp), \
> + .channel = { \
> + .type = IIO_VOLTAGE, \
> + .output = 1, \
> + .indexed = 1, \
> + .address = AD5791_ADDR_DAC0, \
> + .channel = 0, \
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
> + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) | \
> + BIT(IIO_CHAN_INFO_OFFSET), \
> + .scan_type = { \
> + .sign = 'u', \
> + .realbits = (bits), \
> + .storagebits = 24, \
> + .shift = (_shift), \
> + }, \
> + .ext_info = ad5791_ext_info, \
> + }, \
> }
>
> -static const struct iio_chan_spec ad5791_channels[] = {
> - [ID_AD5760] = AD5791_CHAN(16, 4),
> - [ID_AD5780] = AD5791_CHAN(18, 2),
> - [ID_AD5781] = AD5791_CHAN(18, 2),
> - [ID_AD5791] = AD5791_CHAN(20, 0)
> -};
> +AD5791_DEFINE_CHIP_INFO(ad5760, 16, 4, ad5780_get_lin_comp);
> +AD5791_DEFINE_CHIP_INFO(ad5780, 18, 2, ad5780_get_lin_comp);
> +AD5791_DEFINE_CHIP_INFO(ad5781, 18, 2, ad5791_get_lin_comp);
> +AD5791_DEFINE_CHIP_INFO(ad5790, 20, 0, ad5791_get_lin_comp);
> +AD5791_DEFINE_CHIP_INFO(ad5791, 20, 0, ad5791_get_lin_comp);
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 4/6] iio: dac: ad5791: Add reset, clr and ldac gpios
2024-10-28 7:11 [PATCH 0/6] Improvements and Enhancements for AD5791 DAC Driver ahaslam
` (2 preceding siblings ...)
2024-10-28 7:11 ` [PATCH 3/6] iio: dac: ad5791: Include chip_info in device match tables ahaslam
@ 2024-10-28 7:11 ` ahaslam
2024-10-28 18:46 ` kernel test robot
2024-10-28 7:11 ` [PATCH 5/6] iio: dac: ad5791: Use devm_regulator_get_enable_read_voltage ahaslam
2024-10-28 7:11 ` [PATCH 6/6] iio: dac: ad5791: Use devm_iio_device_register ahaslam
5 siblings, 1 reply; 17+ messages in thread
From: ahaslam @ 2024-10-28 7:11 UTC (permalink / raw)
To: lars, Michael.Hennerich, jic23, robh, krzk+dt, conor+dt, nuno.sa,
dlechner
Cc: linux-iio, devicetree, linux-kernel, Axel Haslam
From: Axel Haslam <ahaslam@baylibre.com>
The ad7591 has reset, clr and ldac gpios. For the DAC to output data
continuously written to the data register the state of these gpios needs
to be set by the driver.
Add these gpios to the driver making them optional in case they are fixed
on the pcb.
Signed-off-by: Axel Haslam <ahaslam@baylibre.com>
---
drivers/iio/dac/ad5791.c | 31 ++++++++++++++++++++++++++++---
1 file changed, 28 insertions(+), 3 deletions(-)
diff --git a/drivers/iio/dac/ad5791.c b/drivers/iio/dac/ad5791.c
index a11e81211669..a7cf19346cf0 100644
--- a/drivers/iio/dac/ad5791.c
+++ b/drivers/iio/dac/ad5791.c
@@ -9,6 +9,7 @@
#include <linux/interrupt.h>
#include <linux/fs.h>
#include <linux/device.h>
+#include <linux/delay.h>
#include <linux/kernel.h>
#include <linux/spi/spi.h>
#include <linux/slab.h>
@@ -87,6 +88,9 @@ struct ad5791_state {
struct spi_device *spi;
struct regulator *reg_vdd;
struct regulator *reg_vss;
+ struct gpio_desc *gpio_reset;
+ struct gpio_desc *gpio_clear;
+ struct gpio_desc *gpio_ldac;
const struct ad5791_chip_info *chip_info;
unsigned short vref_mv;
unsigned int vref_neg_mv;
@@ -336,6 +340,22 @@ static int ad5791_probe(struct spi_device *spi)
if (!indio_dev)
return -ENOMEM;
st = iio_priv(indio_dev);
+
+ st->gpio_reset = devm_gpiod_get_optional(&spi->dev, "reset",
+ GPIOD_OUT_HIGH);
+ if (IS_ERR(st->gpio_reset))
+ return PTR_ERR(st->gpio_reset);
+
+ st->gpio_clear = devm_gpiod_get_optional(&spi->dev, "clear",
+ GPIOD_OUT_LOW);
+ if (IS_ERR(st->gpio_clear))
+ return PTR_ERR(st->gpio_clear);
+
+ st->gpio_ldac = devm_gpiod_get_optional(&spi->dev, "ldac",
+ GPIOD_OUT_HIGH);
+ if (IS_ERR(st->gpio_ldac))
+ return PTR_ERR(st->gpio_ldac);
+
st->reg_vdd = devm_regulator_get(&spi->dev, "vdd");
if (!IS_ERR(st->reg_vdd)) {
ret = regulator_enable(st->reg_vdd);
@@ -381,9 +401,14 @@ static int ad5791_probe(struct spi_device *spi)
dev_warn(&spi->dev, "reference voltage unspecified\n");
}
- ret = ad5791_spi_write(st, AD5791_ADDR_SW_CTRL, AD5791_SWCTRL_RESET);
- if (ret)
- goto error_disable_reg_neg;
+ if (st->gpio_reset) {
+ fsleep(20);
+ gpiod_set_value_cansleep(st->gpio_reset, 0);
+ } else {
+ ret = ad5791_spi_write(st, AD5791_ADDR_SW_CTRL, AD5791_SWCTRL_RESET);
+ if (ret)
+ goto error_disable_reg_neg;
+ }
st->chip_info = spi_get_device_match_data(spi);
if (!st->chip_info)
--
2.34.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 4/6] iio: dac: ad5791: Add reset, clr and ldac gpios
2024-10-28 7:11 ` [PATCH 4/6] iio: dac: ad5791: Add reset, clr and ldac gpios ahaslam
@ 2024-10-28 18:46 ` kernel test robot
0 siblings, 0 replies; 17+ messages in thread
From: kernel test robot @ 2024-10-28 18:46 UTC (permalink / raw)
To: ahaslam, lars, Michael.Hennerich, jic23, robh, krzk+dt, conor+dt,
nuno.sa, dlechner
Cc: oe-kbuild-all, linux-iio, devicetree, linux-kernel, Axel Haslam
Hi,
kernel test robot noticed the following build warnings:
[auto build test WARNING on jic23-iio/togreg]
[also build test WARNING on linus/master v6.12-rc5 next-20241028]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/ahaslam-baylibre-com/dt-bindings-iio-dac-ad5791-Add-optional-reset-clr-and-ldac-gpios/20241028-151319
base: https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
patch link: https://lore.kernel.org/r/20241028071118.699951-5-ahaslam%40baylibre.com
patch subject: [PATCH 4/6] iio: dac: ad5791: Add reset, clr and ldac gpios
config: parisc-randconfig-r071-20241028 (https://download.01.org/0day-ci/archive/20241029/202410290242.TjrDXcKG-lkp@intel.com/config)
compiler: hppa-linux-gcc (GCC) 14.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241029/202410290242.TjrDXcKG-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202410290242.TjrDXcKG-lkp@intel.com/
All warnings (new ones prefixed by >>):
drivers/iio/dac/ad5791.c:72: warning: Function parameter or struct member 'name' not described in 'ad5791_chip_info'
drivers/iio/dac/ad5791.c:72: warning: Function parameter or struct member 'channel' not described in 'ad5791_chip_info'
>> drivers/iio/dac/ad5791.c:105: warning: Function parameter or struct member 'gpio_reset' not described in 'ad5791_state'
>> drivers/iio/dac/ad5791.c:105: warning: Function parameter or struct member 'gpio_clear' not described in 'ad5791_state'
>> drivers/iio/dac/ad5791.c:105: warning: Function parameter or struct member 'gpio_ldac' not described in 'ad5791_state'
vim +105 drivers/iio/dac/ad5791.c
20374d1a36df3e drivers/staging/iio/dac/ad5791.c Lars-Peter Clausen 2012-06-04 73
20374d1a36df3e drivers/staging/iio/dac/ad5791.c Lars-Peter Clausen 2012-06-04 74 /**
20374d1a36df3e drivers/staging/iio/dac/ad5791.c Lars-Peter Clausen 2012-06-04 75 * struct ad5791_state - driver instance specific data
ff96bf519acdb3 drivers/iio/dac/ad5791.c Peter Meerwald 2014-12-06 76 * @spi: spi_device
20374d1a36df3e drivers/staging/iio/dac/ad5791.c Lars-Peter Clausen 2012-06-04 77 * @reg_vdd: positive supply regulator
20374d1a36df3e drivers/staging/iio/dac/ad5791.c Lars-Peter Clausen 2012-06-04 78 * @reg_vss: negative supply regulator
20374d1a36df3e drivers/staging/iio/dac/ad5791.c Lars-Peter Clausen 2012-06-04 79 * @chip_info: chip model specific constants
20374d1a36df3e drivers/staging/iio/dac/ad5791.c Lars-Peter Clausen 2012-06-04 80 * @vref_mv: actual reference voltage used
20374d1a36df3e drivers/staging/iio/dac/ad5791.c Lars-Peter Clausen 2012-06-04 81 * @vref_neg_mv: voltage of the negative supply
0071aa300271a4 drivers/iio/dac/ad5791.c zuoqilin 2021-01-28 82 * @ctrl: control register cache
3b1c0b129590d7 drivers/iio/dac/ad5791.c Lee Jones 2020-07-16 83 * @pwr_down_mode: current power down mode
3b1c0b129590d7 drivers/iio/dac/ad5791.c Lee Jones 2020-07-16 84 * @pwr_down: true if device is powered down
3b1c0b129590d7 drivers/iio/dac/ad5791.c Lee Jones 2020-07-16 85 * @data: spi transfer buffers
20374d1a36df3e drivers/staging/iio/dac/ad5791.c Lars-Peter Clausen 2012-06-04 86 */
20374d1a36df3e drivers/staging/iio/dac/ad5791.c Lars-Peter Clausen 2012-06-04 87 struct ad5791_state {
20374d1a36df3e drivers/staging/iio/dac/ad5791.c Lars-Peter Clausen 2012-06-04 88 struct spi_device *spi;
20374d1a36df3e drivers/staging/iio/dac/ad5791.c Lars-Peter Clausen 2012-06-04 89 struct regulator *reg_vdd;
20374d1a36df3e drivers/staging/iio/dac/ad5791.c Lars-Peter Clausen 2012-06-04 90 struct regulator *reg_vss;
fcc5d80fd09f4c drivers/iio/dac/ad5791.c Axel Haslam 2024-10-28 91 struct gpio_desc *gpio_reset;
fcc5d80fd09f4c drivers/iio/dac/ad5791.c Axel Haslam 2024-10-28 92 struct gpio_desc *gpio_clear;
fcc5d80fd09f4c drivers/iio/dac/ad5791.c Axel Haslam 2024-10-28 93 struct gpio_desc *gpio_ldac;
20374d1a36df3e drivers/staging/iio/dac/ad5791.c Lars-Peter Clausen 2012-06-04 94 const struct ad5791_chip_info *chip_info;
20374d1a36df3e drivers/staging/iio/dac/ad5791.c Lars-Peter Clausen 2012-06-04 95 unsigned short vref_mv;
20374d1a36df3e drivers/staging/iio/dac/ad5791.c Lars-Peter Clausen 2012-06-04 96 unsigned int vref_neg_mv;
20374d1a36df3e drivers/staging/iio/dac/ad5791.c Lars-Peter Clausen 2012-06-04 97 unsigned ctrl;
20374d1a36df3e drivers/staging/iio/dac/ad5791.c Lars-Peter Clausen 2012-06-04 98 unsigned pwr_down_mode;
20374d1a36df3e drivers/staging/iio/dac/ad5791.c Lars-Peter Clausen 2012-06-04 99 bool pwr_down;
791bb52a0cd2cc drivers/iio/dac/ad5791.c Lars-Peter Clausen 2013-11-25 100
791bb52a0cd2cc drivers/iio/dac/ad5791.c Lars-Peter Clausen 2013-11-25 101 union {
791bb52a0cd2cc drivers/iio/dac/ad5791.c Lars-Peter Clausen 2013-11-25 102 __be32 d32;
791bb52a0cd2cc drivers/iio/dac/ad5791.c Lars-Peter Clausen 2013-11-25 103 u8 d8[4];
b2d5e9de77c877 drivers/iio/dac/ad5791.c Jonathan Cameron 2022-05-08 104 } data[3] __aligned(IIO_DMA_MINALIGN);
20374d1a36df3e drivers/staging/iio/dac/ad5791.c Lars-Peter Clausen 2012-06-04 @105 };
20374d1a36df3e drivers/staging/iio/dac/ad5791.c Lars-Peter Clausen 2012-06-04 106
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 5/6] iio: dac: ad5791: Use devm_regulator_get_enable_read_voltage
2024-10-28 7:11 [PATCH 0/6] Improvements and Enhancements for AD5791 DAC Driver ahaslam
` (3 preceding siblings ...)
2024-10-28 7:11 ` [PATCH 4/6] iio: dac: ad5791: Add reset, clr and ldac gpios ahaslam
@ 2024-10-28 7:11 ` ahaslam
2024-10-28 18:32 ` kernel test robot
2024-10-28 7:11 ` [PATCH 6/6] iio: dac: ad5791: Use devm_iio_device_register ahaslam
5 siblings, 1 reply; 17+ messages in thread
From: ahaslam @ 2024-10-28 7:11 UTC (permalink / raw)
To: lars, Michael.Hennerich, jic23, robh, krzk+dt, conor+dt, nuno.sa,
dlechner
Cc: linux-iio, devicetree, linux-kernel, Axel Haslam
From: Axel Haslam <ahaslam@baylibre.com>
Simplify probe by using of the devm_regulator_get_enable_read_voltage.
Suggested-by: David Lechner <dlechner@baylibre.com>
Signed-off-by: Axel Haslam <ahaslam@baylibre.com>
---
drivers/iio/dac/ad5791.c | 56 +++++++++-------------------------------
1 file changed, 12 insertions(+), 44 deletions(-)
diff --git a/drivers/iio/dac/ad5791.c b/drivers/iio/dac/ad5791.c
index a7cf19346cf0..cf3d41a10c20 100644
--- a/drivers/iio/dac/ad5791.c
+++ b/drivers/iio/dac/ad5791.c
@@ -356,32 +356,6 @@ static int ad5791_probe(struct spi_device *spi)
if (IS_ERR(st->gpio_ldac))
return PTR_ERR(st->gpio_ldac);
- st->reg_vdd = devm_regulator_get(&spi->dev, "vdd");
- if (!IS_ERR(st->reg_vdd)) {
- ret = regulator_enable(st->reg_vdd);
- if (ret)
- return ret;
-
- ret = regulator_get_voltage(st->reg_vdd);
- if (ret < 0)
- goto error_disable_reg_pos;
-
- pos_voltage_uv = ret;
- }
-
- st->reg_vss = devm_regulator_get(&spi->dev, "vss");
- if (!IS_ERR(st->reg_vss)) {
- ret = regulator_enable(st->reg_vss);
- if (ret)
- goto error_disable_reg_pos;
-
- ret = regulator_get_voltage(st->reg_vss);
- if (ret < 0)
- goto error_disable_reg_neg;
-
- neg_voltage_uv = ret;
- }
-
st->pwr_down = true;
st->spi = spi;
@@ -391,7 +365,15 @@ static int ad5791_probe(struct spi_device *spi)
use_rbuf_gain2 = device_property_read_bool(&spi->dev,
"adi,rbuf-gain2-en");
- if (!IS_ERR(st->reg_vss) && !IS_ERR(st->reg_vdd)) {
+ pos_voltage_uv = devm_regulator_get_enable_read_voltage(&spi->dev, "vdd");
+ if (pos_voltage_uv < 0 && pos_voltage_uv != -ENODEV)
+ return dev_err_probe(&spi->dev, ret, "failed to get vdd voltage\n");
+
+ neg_voltage_uv = devm_regulator_get_enable_read_voltage(&spi->dev, "vss");
+ if (neg_voltage_uv < 0 && neg_voltage_uv != -ENODEV)
+ return dev_err_probe(&spi->dev, ret, "failed to get vss voltage\n");
+
+ if (neg_voltage_uv >= 0 && pos_voltage_uv >= 0) {
st->vref_mv = (pos_voltage_uv + neg_voltage_uv) / 1000;
st->vref_neg_mv = neg_voltage_uv / 1000;
} else if (pdata) {
@@ -407,7 +389,7 @@ static int ad5791_probe(struct spi_device *spi)
} else {
ret = ad5791_spi_write(st, AD5791_ADDR_SW_CTRL, AD5791_SWCTRL_RESET);
if (ret)
- goto error_disable_reg_neg;
+ return dev_err_probe(&spi->dev, ret, "fail to reset\n");
}
st->chip_info = spi_get_device_match_data(spi);
@@ -421,7 +403,7 @@ static int ad5791_probe(struct spi_device *spi)
ret = ad5791_spi_write(st, AD5791_ADDR_CTRL, st->ctrl |
AD5791_CTRL_OPGND | AD5791_CTRL_DACTRI);
if (ret)
- goto error_disable_reg_neg;
+ return dev_err_probe(&spi->dev, ret, "fail to write ctrl register\n");
spi_set_drvdata(spi, indio_dev);
indio_dev->info = &ad5791_info;
@@ -431,30 +413,16 @@ static int ad5791_probe(struct spi_device *spi)
indio_dev->name = st->chip_info->name;
ret = iio_device_register(indio_dev);
if (ret)
- goto error_disable_reg_neg;
+ return dev_err_probe(&spi->dev, ret, "unable to register iio device\n");
return 0;
-
-error_disable_reg_neg:
- if (!IS_ERR(st->reg_vss))
- regulator_disable(st->reg_vss);
-error_disable_reg_pos:
- if (!IS_ERR(st->reg_vdd))
- regulator_disable(st->reg_vdd);
- return ret;
}
static void ad5791_remove(struct spi_device *spi)
{
struct iio_dev *indio_dev = spi_get_drvdata(spi);
- struct ad5791_state *st = iio_priv(indio_dev);
iio_device_unregister(indio_dev);
- if (!IS_ERR(st->reg_vdd))
- regulator_disable(st->reg_vdd);
-
- if (!IS_ERR(st->reg_vss))
- regulator_disable(st->reg_vss);
}
static const struct of_device_id ad5791_of_match[] = {
--
2.34.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 5/6] iio: dac: ad5791: Use devm_regulator_get_enable_read_voltage
2024-10-28 7:11 ` [PATCH 5/6] iio: dac: ad5791: Use devm_regulator_get_enable_read_voltage ahaslam
@ 2024-10-28 18:32 ` kernel test robot
0 siblings, 0 replies; 17+ messages in thread
From: kernel test robot @ 2024-10-28 18:32 UTC (permalink / raw)
To: ahaslam, lars, Michael.Hennerich, jic23, robh, krzk+dt, conor+dt,
nuno.sa, dlechner
Cc: llvm, oe-kbuild-all, linux-iio, devicetree, linux-kernel,
Axel Haslam
Hi,
kernel test robot noticed the following build warnings:
[auto build test WARNING on jic23-iio/togreg]
[also build test WARNING on linus/master v6.12-rc5 next-20241028]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/ahaslam-baylibre-com/dt-bindings-iio-dac-ad5791-Add-optional-reset-clr-and-ldac-gpios/20241028-151319
base: https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
patch link: https://lore.kernel.org/r/20241028071118.699951-6-ahaslam%40baylibre.com
patch subject: [PATCH 5/6] iio: dac: ad5791: Use devm_regulator_get_enable_read_voltage
config: x86_64-buildonly-randconfig-004-20241028 (https://download.01.org/0day-ci/archive/20241029/202410290245.0RC0cDV4-lkp@intel.com/config)
compiler: clang version 19.1.2 (https://github.com/llvm/llvm-project 7ba7d8e2f7b6445b60679da826210cdde29eaf8b)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241029/202410290245.0RC0cDV4-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202410290245.0RC0cDV4-lkp@intel.com/
All warnings (new ones prefixed by >>):
In file included from drivers/iio/dac/ad5791.c:14:
In file included from include/linux/spi/spi.h:17:
In file included from include/linux/scatterlist.h:8:
In file included from include/linux/mm.h:2213:
include/linux/vmstat.h:518:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
518 | return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
| ~~~~~~~~~~~ ^ ~~~
>> drivers/iio/dac/ad5791.c:370:35: warning: variable 'ret' is uninitialized when used here [-Wuninitialized]
370 | return dev_err_probe(&spi->dev, ret, "failed to get vdd voltage\n");
| ^~~
drivers/iio/dac/ad5791.c:336:9: note: initialize the variable 'ret' to silence this warning
336 | int ret, pos_voltage_uv = 0, neg_voltage_uv = 0;
| ^
| = 0
2 warnings generated.
vim +/ret +370 drivers/iio/dac/ad5791.c
330
331 static int ad5791_probe(struct spi_device *spi)
332 {
333 const struct ad5791_platform_data *pdata = dev_get_platdata(&spi->dev);
334 struct iio_dev *indio_dev;
335 struct ad5791_state *st;
336 int ret, pos_voltage_uv = 0, neg_voltage_uv = 0;
337 bool use_rbuf_gain2;
338
339 indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
340 if (!indio_dev)
341 return -ENOMEM;
342 st = iio_priv(indio_dev);
343
344 st->gpio_reset = devm_gpiod_get_optional(&spi->dev, "reset",
345 GPIOD_OUT_HIGH);
346 if (IS_ERR(st->gpio_reset))
347 return PTR_ERR(st->gpio_reset);
348
349 st->gpio_clear = devm_gpiod_get_optional(&spi->dev, "clear",
350 GPIOD_OUT_LOW);
351 if (IS_ERR(st->gpio_clear))
352 return PTR_ERR(st->gpio_clear);
353
354 st->gpio_ldac = devm_gpiod_get_optional(&spi->dev, "ldac",
355 GPIOD_OUT_HIGH);
356 if (IS_ERR(st->gpio_ldac))
357 return PTR_ERR(st->gpio_ldac);
358
359 st->pwr_down = true;
360 st->spi = spi;
361
362 if (pdata)
363 use_rbuf_gain2 = pdata->use_rbuf_gain2;
364 else
365 use_rbuf_gain2 = device_property_read_bool(&spi->dev,
366 "adi,rbuf-gain2-en");
367
368 pos_voltage_uv = devm_regulator_get_enable_read_voltage(&spi->dev, "vdd");
369 if (pos_voltage_uv < 0 && pos_voltage_uv != -ENODEV)
> 370 return dev_err_probe(&spi->dev, ret, "failed to get vdd voltage\n");
371
372 neg_voltage_uv = devm_regulator_get_enable_read_voltage(&spi->dev, "vss");
373 if (neg_voltage_uv < 0 && neg_voltage_uv != -ENODEV)
374 return dev_err_probe(&spi->dev, ret, "failed to get vss voltage\n");
375
376 if (neg_voltage_uv >= 0 && pos_voltage_uv >= 0) {
377 st->vref_mv = (pos_voltage_uv + neg_voltage_uv) / 1000;
378 st->vref_neg_mv = neg_voltage_uv / 1000;
379 } else if (pdata) {
380 st->vref_mv = pdata->vref_pos_mv + pdata->vref_neg_mv;
381 st->vref_neg_mv = pdata->vref_neg_mv;
382 } else {
383 dev_warn(&spi->dev, "reference voltage unspecified\n");
384 }
385
386 if (st->gpio_reset) {
387 fsleep(20);
388 gpiod_set_value_cansleep(st->gpio_reset, 0);
389 } else {
390 ret = ad5791_spi_write(st, AD5791_ADDR_SW_CTRL, AD5791_SWCTRL_RESET);
391 if (ret)
392 return dev_err_probe(&spi->dev, ret, "fail to reset\n");
393 }
394
395 st->chip_info = spi_get_device_match_data(spi);
396 if (!st->chip_info)
397 return dev_err_probe(&spi->dev, -EINVAL, "no chip info\n");
398
399 st->ctrl = AD5761_CTRL_LINCOMP(st->chip_info->get_lin_comp(st->vref_mv))
400 | (use_rbuf_gain2 ? 0 : AD5791_CTRL_RBUF) |
401 AD5791_CTRL_BIN2SC;
402
403 ret = ad5791_spi_write(st, AD5791_ADDR_CTRL, st->ctrl |
404 AD5791_CTRL_OPGND | AD5791_CTRL_DACTRI);
405 if (ret)
406 return dev_err_probe(&spi->dev, ret, "fail to write ctrl register\n");
407
408 spi_set_drvdata(spi, indio_dev);
409 indio_dev->info = &ad5791_info;
410 indio_dev->modes = INDIO_DIRECT_MODE;
411 indio_dev->channels = &st->chip_info->channel;
412 indio_dev->num_channels = 1;
413 indio_dev->name = st->chip_info->name;
414 ret = iio_device_register(indio_dev);
415 if (ret)
416 return dev_err_probe(&spi->dev, ret, "unable to register iio device\n");
417
418 return 0;
419 }
420
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 6/6] iio: dac: ad5791: Use devm_iio_device_register
2024-10-28 7:11 [PATCH 0/6] Improvements and Enhancements for AD5791 DAC Driver ahaslam
` (4 preceding siblings ...)
2024-10-28 7:11 ` [PATCH 5/6] iio: dac: ad5791: Use devm_regulator_get_enable_read_voltage ahaslam
@ 2024-10-28 7:11 ` ahaslam
2024-10-28 20:25 ` Jonathan Cameron
5 siblings, 1 reply; 17+ messages in thread
From: ahaslam @ 2024-10-28 7:11 UTC (permalink / raw)
To: lars, Michael.Hennerich, jic23, robh, krzk+dt, conor+dt, nuno.sa,
dlechner
Cc: linux-iio, devicetree, linux-kernel, Axel Haslam
From: Axel Haslam <ahaslam@baylibre.com>
Use devm_iio_device_register to automatically free the iio device.
since this is the last remaining resource that was not automatically
freed, we can drop the ".remove" callback.
Suggested-by: David Lechner <dlechner@baylibre.com>
Signed-off-by: Axel Haslam <ahaslam@baylibre.com>
---
drivers/iio/dac/ad5791.c | 15 +--------------
1 file changed, 1 insertion(+), 14 deletions(-)
diff --git a/drivers/iio/dac/ad5791.c b/drivers/iio/dac/ad5791.c
index cf3d41a10c20..21332c9aca5d 100644
--- a/drivers/iio/dac/ad5791.c
+++ b/drivers/iio/dac/ad5791.c
@@ -405,24 +405,12 @@ static int ad5791_probe(struct spi_device *spi)
if (ret)
return dev_err_probe(&spi->dev, ret, "fail to write ctrl register\n");
- spi_set_drvdata(spi, indio_dev);
indio_dev->info = &ad5791_info;
indio_dev->modes = INDIO_DIRECT_MODE;
indio_dev->channels = &st->chip_info->channel;
indio_dev->num_channels = 1;
indio_dev->name = st->chip_info->name;
- ret = iio_device_register(indio_dev);
- if (ret)
- return dev_err_probe(&spi->dev, ret, "unable to register iio device\n");
-
- return 0;
-}
-
-static void ad5791_remove(struct spi_device *spi)
-{
- struct iio_dev *indio_dev = spi_get_drvdata(spi);
-
- iio_device_unregister(indio_dev);
+ return devm_iio_device_register(&spi->dev, indio_dev);
}
static const struct of_device_id ad5791_of_match[] = {
@@ -451,7 +439,6 @@ static struct spi_driver ad5791_driver = {
.of_match_table = ad5791_of_match,
},
.probe = ad5791_probe,
- .remove = ad5791_remove,
.id_table = ad5791_id,
};
module_spi_driver(ad5791_driver);
--
2.34.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 6/6] iio: dac: ad5791: Use devm_iio_device_register
2024-10-28 7:11 ` [PATCH 6/6] iio: dac: ad5791: Use devm_iio_device_register ahaslam
@ 2024-10-28 20:25 ` Jonathan Cameron
0 siblings, 0 replies; 17+ messages in thread
From: Jonathan Cameron @ 2024-10-28 20:25 UTC (permalink / raw)
To: ahaslam
Cc: lars, Michael.Hennerich, robh, krzk+dt, conor+dt, nuno.sa,
dlechner, linux-iio, devicetree, linux-kernel
On Mon, 28 Oct 2024 08:11:18 +0100
ahaslam@baylibre.com wrote:
> From: Axel Haslam <ahaslam@baylibre.com>
>
> Use devm_iio_device_register to automatically free the iio device.
> since this is the last remaining resource that was not automatically
> freed, we can drop the ".remove" callback.
>
> Suggested-by: David Lechner <dlechner@baylibre.com>
> Signed-off-by: Axel Haslam <ahaslam@baylibre.com>
Hi Axel,
The bot did a much better review job than me this time.
Other than the obvious solutions to the things it pointed out,
this series looks fine to me.
Jonathan
> ---
> drivers/iio/dac/ad5791.c | 15 +--------------
> 1 file changed, 1 insertion(+), 14 deletions(-)
>
> diff --git a/drivers/iio/dac/ad5791.c b/drivers/iio/dac/ad5791.c
> index cf3d41a10c20..21332c9aca5d 100644
> --- a/drivers/iio/dac/ad5791.c
> +++ b/drivers/iio/dac/ad5791.c
> @@ -405,24 +405,12 @@ static int ad5791_probe(struct spi_device *spi)
> if (ret)
> return dev_err_probe(&spi->dev, ret, "fail to write ctrl register\n");
>
> - spi_set_drvdata(spi, indio_dev);
> indio_dev->info = &ad5791_info;
> indio_dev->modes = INDIO_DIRECT_MODE;
> indio_dev->channels = &st->chip_info->channel;
> indio_dev->num_channels = 1;
> indio_dev->name = st->chip_info->name;
> - ret = iio_device_register(indio_dev);
> - if (ret)
> - return dev_err_probe(&spi->dev, ret, "unable to register iio device\n");
> -
> - return 0;
> -}
> -
> -static void ad5791_remove(struct spi_device *spi)
> -{
> - struct iio_dev *indio_dev = spi_get_drvdata(spi);
> -
> - iio_device_unregister(indio_dev);
> + return devm_iio_device_register(&spi->dev, indio_dev);
> }
>
> static const struct of_device_id ad5791_of_match[] = {
> @@ -451,7 +439,6 @@ static struct spi_driver ad5791_driver = {
> .of_match_table = ad5791_of_match,
> },
> .probe = ad5791_probe,
> - .remove = ad5791_remove,
> .id_table = ad5791_id,
> };
> module_spi_driver(ad5791_driver);
^ permalink raw reply [flat|nested] 17+ messages in thread