2023-12-18 16:52:10

by Anshul Dalal

[permalink] [raw]
Subject: [PATCH v3 1/2] dt-bindings: iio: dac: add MCP4821

Adds support for MCP48xx series of DACs.

Datasheet: https://ww1.microchip.com/downloads/en/DeviceDoc/22244B.pdf #MCP48x1
Datasheet: https://ww1.microchip.com/downloads/en/DeviceDoc/20002249B.pdf #MCP48x2
Reviewed-by: Conor Dooley <[email protected]>
Signed-off-by: Anshul Dalal <[email protected]>

---

Changes for v3:
- Added gpios for ldac and shutdown pins
- Added spi-cpha and spi-cpol for the SPI mode 0 and 3

Changes for v2:
- Changed order in device table to numerical
- Made vdd_supply required
- Added 'Reviewed-by: Conor Dooley'

Previous versions:
v2: https://lore.kernel.org/lkml/[email protected]/
v1: https://lore.kernel.org/lkml/[email protected]/
---
.../bindings/iio/dac/microchip,mcp4821.yaml | 86 +++++++++++++++++++
1 file changed, 86 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/dac/microchip,mcp4821.yaml

diff --git a/Documentation/devicetree/bindings/iio/dac/microchip,mcp4821.yaml b/Documentation/devicetree/bindings/iio/dac/microchip,mcp4821.yaml
new file mode 100644
index 000000000000..5b1707d974e2
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/dac/microchip,mcp4821.yaml
@@ -0,0 +1,86 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/dac/microchip,mcp4821.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Microchip MCP4821 and similar DACs
+
+description: |
+ Supports MCP48x1 (single channel) and MCP48x2 (dual channel) series of DACs.
+ Device supports simplex communication over SPI in Mode 0 and Mode 3.
+
+ +---------+--------------+-------------+
+ | Device | Resolution | Channels |
+ |---------|--------------|-------------|
+ | MCP4801 | 8-bit | 1 |
+ | MCP4802 | 8-bit | 2 |
+ | MCP4811 | 10-bit | 1 |
+ | MCP4812 | 10-bit | 2 |
+ | MCP4821 | 12-bit | 1 |
+ | MCP4822 | 12-bit | 2 |
+ +---------+--------------+-------------+
+
+ Datasheet:
+ MCP48x1: https://ww1.microchip.com/downloads/en/DeviceDoc/22244B.pdf
+ MCP48x2: https://ww1.microchip.com/downloads/en/DeviceDoc/20002249B.pdf
+
+maintainers:
+ - Anshul Dalal <[email protected]>
+
+allOf:
+ - $ref: /schemas/spi/spi-peripheral-props.yaml#
+
+properties:
+ compatible:
+ enum:
+ - microchip,mcp4801
+ - microchip,mcp4802
+ - microchip,mcp4811
+ - microchip,mcp4812
+ - microchip,mcp4821
+ - microchip,mcp4822
+
+ reg:
+ maxItems: 1
+
+ vdd-supply: true
+
+ ldac-gpios:
+ description: |
+ Active Low LDAC (Latch DAC Input) pin used to update the DAC output.
+ maxItems: 1
+
+ shdn-gpios:
+ description: |
+ Active Low pin used to enter the shutdown mode.
+ maxItems: 1
+
+ spi-cpha: true
+ spi-cpol: true
+
+required:
+ - compatible
+ - reg
+ - vdd-supply
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/gpio/gpio.h>
+
+ spi {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ dac@0 {
+ compatible = "microchip,mcp4821";
+ reg = <0>;
+ vdd-supply = <&vdd_regulator>;
+ ldac-gpios = <&gpio0 1 GPIO_ACTIVE_HIGH>;
+ shdn-gpios = <&gpio0 2 GPIO_ACTIVE_HIGH>;
+ spi-cpha;
+ spi-cpol;
+ };
+ };
--
2.43.0



2023-12-19 08:00:48

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v3 1/2] dt-bindings: iio: dac: add MCP4821

On 18/12/2023 17:47, Anshul Dalal wrote:
> Adds support for MCP48xx series of DACs.
>
> Datasheet: https://ww1.microchip.com/downloads/en/DeviceDoc/22244B.pdf #MCP48x1
> Datasheet: https://ww1.microchip.com/downloads/en/DeviceDoc/20002249B.pdf #MCP48x2
> Reviewed-by: Conor Dooley <[email protected]>

Drop, you added properties after review! Adding properties after the
review invalidates given review. Especially if you add wrong properties.

> Signed-off-by: Anshul Dalal <[email protected]>
>
> ---
>
> Changes for v3:
> - Added gpios for ldac and shutdown pins
> - Added spi-cpha and spi-cpol for the SPI mode 0 and 3
>
> Changes for v2:
> - Changed order in device table to numerical
> - Made vdd_supply required
> - Added 'Reviewed-by: Conor Dooley'
>
> Previous versions:
> v2: https://lore.kernel.org/lkml/[email protected]/
> v1: https://lore.kernel.org/lkml/[email protected]/
> ---


> +
> + reg:
> + maxItems: 1
> +
> + vdd-supply: true
> +
> + ldac-gpios:
> + description: |
> + Active Low LDAC (Latch DAC Input) pin used to update the DAC output.
> + maxItems: 1
> +
> + shdn-gpios:

Open gpio-consumer-common.yaml and look at entries there.


Best regards,
Krzysztof


2023-12-19 08:45:25

by Anshul Dalal

[permalink] [raw]
Subject: Re: [PATCH v3 1/2] dt-bindings: iio: dac: add MCP4821

On 12/19/23 13:29, Krzysztof Kozlowski wrote:
> On 18/12/2023 17:47, Anshul Dalal wrote:
>> +
>> + reg:
>> + maxItems: 1
>> +
>> + vdd-supply: true
>> +
>> + ldac-gpios:
>> + description: |
>> + Active Low LDAC (Latch DAC Input) pin used to update the DAC output.
>> + maxItems: 1
>> +
>> + shdn-gpios:
>
> Open gpio-consumer-common.yaml and look at entries there.
>

Should I name the property `powerdown-gpios` instead of `shdn-gpios` as
specified in gpio-consumer-common.yaml?
Furthermore, do I need to add gpio-consumer-common.yaml as a ref?

Thanks for your time,
Anshul

2023-12-19 09:00:33

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v3 1/2] dt-bindings: iio: dac: add MCP4821

On 19/12/2023 09:42, Anshul Dalal wrote:
> On 12/19/23 13:29, Krzysztof Kozlowski wrote:
>> On 18/12/2023 17:47, Anshul Dalal wrote:
>>> +
>>> + reg:
>>> + maxItems: 1
>>> +
>>> + vdd-supply: true
>>> +
>>> + ldac-gpios:
>>> + description: |
>>> + Active Low LDAC (Latch DAC Input) pin used to update the DAC output.
>>> + maxItems: 1
>>> +
>>> + shdn-gpios:
>>
>> Open gpio-consumer-common.yaml and look at entries there.
>>
>
> Should I name the property `powerdown-gpios` instead of `shdn-gpios` as
> specified in gpio-consumer-common.yaml?

Yes. You can provide the name of actual pin in description.

> Furthermore, do I need to add gpio-consumer-common.yaml as a ref?

No.


Best regards,
Krzysztof