[PATCH v2 0/2] iio: dac: ad5791: Add support for controlling RBUF via devicetree

[PATCH v2 0/2] iio: dac: ad5791: Add support for controlling RBUF via devicetree

[Nuno Sa]

Simple series to add support for an external amplifier to be connected
in again of two configuration.

v2:
 * removed .yaml suffix from commit title;
 * Don't use commit/patch in commit message.

---
Michael Hennerich (2):
      dt-bindings: adi,ad5791: Add support for controlling RBUF
      iio: dac: ad5791: Add support for controlling RBUF via devicetree

 Documentation/devicetree/bindings/iio/dac/adi,ad5791.yaml | 5 +++++
 drivers/iio/dac/ad5791.c                                  | 9 ++++++++-
 2 files changed, 13 insertions(+), 1 deletion(-)

Thanks!
- Nuno Sá

[PATCH v2 1/2] dt-bindings: adi,ad5791: Add support for controlling RBUF

[Nuno Sa]

From: Michael Hennerich <michael.hennerich@analog.com>

This change adds support for an external amplifier to be connected in a
gain of two configuration.

Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Nuno Sa <nuno.sa@analog.com>
---
 Documentation/devicetree/bindings/iio/dac/adi,ad5791.yaml | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/dac/adi,ad5791.yaml b/Documentation/devicetree/bindings/iio/dac/adi,ad5791.yaml
index 3a84739736f6..c81285d84db7 100644
--- a/Documentation/devicetree/bindings/iio/dac/adi,ad5791.yaml
+++ b/Documentation/devicetree/bindings/iio/dac/adi,ad5791.yaml
@@ -26,6 +26,11 @@ properties:
   vdd-supply: true
   vss-supply: true
 
+  adi,rbuf-gain2-en:
+    description: Specify to allow an external amplifier to be connected in a
+      gain of two configuration.
+    type: boolean
+
 required:
   - compatible
   - reg

-- 
2.43.0

Re: [PATCH v2 1/2] dt-bindings: adi,ad5791: Add support for controlling RBUF

[Krzysztof Kozlowski]

On 28/11/2023 16:26, Nuno Sa wrote:
> From: Michael Hennerich <michael.hennerich@analog.com>
> 
> This change adds support for an external amplifier to be connected in a

Nothing improved here.

I said "This commit/patch" so you replaced it to "change", really, read
the Submitting patches document.



Best regards,
Krzysztof

Re: [PATCH v2 1/2] dt-bindings: adi,ad5791: Add support for controlling RBUF

[Nuno Sá]

On Tue, 2023-11-28 at 16:58 +0100, Krzysztof Kozlowski wrote:
> On 28/11/2023 16:26, Nuno Sa wrote:
> > From: Michael Hennerich <michael.hennerich@analog.com>
> > 
> > This change adds support for an external amplifier to be connected in a
> 
> Nothing improved here.
> 
> I said "This commit/patch" so you replaced it to "change", really, read
> the Submitting patches document.
> 

And? Is this message so horrible? Yes, you did said "This commit/patch" but I thought
"commit/patch" was the issue because yes, I already saw other maintainers/reviewers
complaining about using those specific words but nothing really against "This change"
(and I do have some messages like that). Yeah, the submitting patches document
suggests to do it more like "Added a new property so bala bla ..." but I dunno every
word in that document is blindly followed. Is this such a big deal in here that you
had reply like I don't care? And even if you're technically right, there were better
ways to say that what I have was not exactly what you asked for...

Submitters should make maintainers/reviewers life easier but the other way around is
equally true and nitpicky things like this are not helpful. Really...

Anyways, I'll send the v3.

- Nuno Sá

Re: [PATCH v2 1/2] dt-bindings: adi,ad5791: Add support for controlling RBUF

[Krzysztof Kozlowski]

On 29/11/2023 09:28, Nuno Sá wrote:
> On Tue, 2023-11-28 at 16:58 +0100, Krzysztof Kozlowski wrote:
>> On 28/11/2023 16:26, Nuno Sa wrote:
>>> From: Michael Hennerich <michael.hennerich@analog.com>
>>>
>>> This change adds support for an external amplifier to be connected in a
>>
>> Nothing improved here.
>>
>> I said "This commit/patch" so you replaced it to "change", really, read
>> the Submitting patches document.
>>
> 
> And? Is this message so horrible? Yes, you did said "This commit/patch" but I thought
> "commit/patch" was the issue because yes, I already saw other maintainers/reviewers
> complaining about using those specific words but nothing really against "This change"
> (and I do have some messages like that). Yeah, the submitting patches document
> suggests to do it more like "Added a new property so bala bla ..." but I dunno every
> word in that document is blindly followed. Is this such a big deal in here that you
> had reply like I don't care? And even if you're technically right, there were better
> ways to say that what I have was not exactly what you asked for...
> 
> Submitters should make maintainers/reviewers life easier but the other way around is
> equally true and nitpicky things like this are not helpful. Really...

Nitpicking is caused by ignored review feedback and by not reading at
all attached/linked guideline. I gave quite explicit instruction,
including what I want the contributor to read. It was not read.

Best regards,
Krzysztof

[PATCH v2 2/2] iio: dac: ad5791: Add support for controlling RBUF via devicetree

[Nuno Sa]

From: Michael Hennerich <michael.hennerich@analog.com>

This patch adds support for an external amplifier to be connected in a
gain of two configuration.

Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
Signed-off-by: Nuno Sa <nuno.sa@analog.com>
---
 drivers/iio/dac/ad5791.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/dac/ad5791.c b/drivers/iio/dac/ad5791.c
index a4167454da81..75b549827e15 100644
--- a/drivers/iio/dac/ad5791.c
+++ b/drivers/iio/dac/ad5791.c
@@ -345,6 +345,7 @@ static int ad5791_probe(struct spi_device *spi)
 	struct iio_dev *indio_dev;
 	struct ad5791_state *st;
 	int ret, pos_voltage_uv = 0, neg_voltage_uv = 0;
+	bool use_rbuf_gain2;
 
 	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
 	if (!indio_dev)
@@ -379,6 +380,12 @@ static int ad5791_probe(struct spi_device *spi)
 	st->pwr_down = true;
 	st->spi = spi;
 
+	if (pdata)
+		use_rbuf_gain2 = pdata->use_rbuf_gain2;
+	else
+		use_rbuf_gain2 = device_property_read_bool(&spi->dev,
+							   "adi,rbuf-gain2-en");
+
 	if (!IS_ERR(st->reg_vss) && !IS_ERR(st->reg_vdd)) {
 		st->vref_mv = (pos_voltage_uv + neg_voltage_uv) / 1000;
 		st->vref_neg_mv = neg_voltage_uv / 1000;
@@ -398,7 +405,7 @@ static int ad5791_probe(struct spi_device *spi)
 
 
 	st->ctrl = AD5761_CTRL_LINCOMP(st->chip_info->get_lin_comp(st->vref_mv))
-		  | ((pdata && pdata->use_rbuf_gain2) ? 0 : AD5791_CTRL_RBUF) |
+		  | (use_rbuf_gain2 ? 0 : AD5791_CTRL_RBUF) |
 		  AD5791_CTRL_BIN2SC;
 
 	ret = ad5791_spi_write(st, AD5791_ADDR_CTRL, st->ctrl |

-- 
2.43.0