1. Patchew
  2. linux
  3. [PATCH v3 0/2] add MCP4728 I2C DAC driver​
  4. View patch

[PATCH v3 1/2] dt-bindings: iio: dac: add mcp4728.yaml

Andrea Collamati posted 2 patches 2 years, 6 months ago
There is a newer version of this series
[PATCH v3 1/2] dt-bindings: iio: dac: add mcp4728.yaml
Posted by Andrea Collamati 2 years, 6 months ago 
Add documentation for MCP4728

Signed-off-by: Andrea Collamati <andrea.collamati@gmail.com>
---
 .../bindings/iio/dac/microchip,mcp4728.yaml   | 48 +++++++++++++++++++
 1 file changed, 48 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/dac/microchip,mcp4728.yaml

diff --git a/Documentation/devicetree/bindings/iio/dac/microchip,mcp4728.yaml b/Documentation/devicetree/bindings/iio/dac/microchip,mcp4728.yaml
new file mode 100644
index 000000000000..6fd9be076245
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/dac/microchip,mcp4728.yaml
@@ -0,0 +1,48 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/dac/microchip,mcp4728.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Microchip MCP4728 DAC
+
+description:
+  MCP4728 is a quad channel, 12-bit voltage output
+  Digital-to-Analog Converter with non-volatile
+  memory and I2C compatible Serial Interface.
+  https://www.microchip.com/en-us/product/mcp4728
+
+maintainers:
+  - Andrea Collamati <andrea.collamati@gmail.com>
+
+properties:
+  compatible:
+    enum:
+      - microchip,mcp4728
+  reg:
+    maxItems: 1
+
+  vdd-supply:
+    description: |
+      Provides both power and acts as the reference supply on the MCP4728
+      when Internal Vref is not selected.
+
+required:
+  - compatible
+  - reg
+  - vdd-supply
+
+additionalProperties: false
+
+examples:
+  - |
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        mcp4728@60 {
+            compatible = "microchip,mcp4728";
+            reg = <0x60>;
+            vdd-supply = <&vdac_vdd>;
+        };
+    };
-- 
2.34.1
Re: [PATCH v3 1/2] dt-bindings: iio: dac: add mcp4728.yaml
Posted by Krzysztof Kozlowski 2 years, 6 months ago 
On 20/07/2023 17:40, Andrea Collamati wrote:
> Add documentation for MCP4728
> 
> Signed-off-by: Andrea Collamati <andrea.collamati@gmail.com>
> ---
>  .../bindings/iio/dac/microchip,mcp4728.yaml   | 48 +++++++++++++++++++
>  1 file changed, 48 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/dac/microchip,mcp4728.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/dac/microchip,mcp4728.yaml b/Documentation/devicetree/bindings/iio/dac/microchip,mcp4728.yaml
> new file mode 100644
> index 000000000000..6fd9be076245
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/dac/microchip,mcp4728.yaml
> @@ -0,0 +1,48 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/dac/microchip,mcp4728.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Microchip MCP4728 DAC
> +
> +description:
> +  MCP4728 is a quad channel, 12-bit voltage output
> +  Digital-to-Analog Converter with non-volatile
> +  memory and I2C compatible Serial Interface.
> +  https://www.microchip.com/en-us/product/mcp4728
> +
> +maintainers:
> +  - Andrea Collamati <andrea.collamati@gmail.com>
> +
> +properties:
> +  compatible:
> +    enum:
> +      - microchip,mcp4728

This is a friendly reminder during the review process.

It seems my previous comments were not fully addressed. Maybe my
feedback got lost between the quotes, maybe you just forgot to apply it.
Please go back to the previous discussion and either implement all
requested changes or keep discussing them.

Thank you.

> +  reg:
> +    maxItems: 1
> +
> +  vdd-supply:
> +    description: |
> +      Provides both power and acts as the reference supply on the MCP4728
> +      when Internal Vref is not selected.
> +
> +required:
> +  - compatible
> +  - reg
> +  - vdd-supply
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        mcp4728@60 {

The same... Probably more comments were ignored, so:

This is a friendly reminder during the review process.

It seems my previous comments were not fully addressed. Maybe my
feedback got lost between the quotes, maybe you just forgot to apply it.
Please go back to the previous discussion and either implement all
requested changes or keep discussing them.

Thank you.


Best regards,
Krzysztof
Re: [PATCH v3 1/2] dt-bindings: iio: dac: add mcp4728.yaml
Posted by Andrea Collamati 2 years, 6 months ago 
Hi Krzysztof,

On 7/21/23 10:21, Krzysztof Kozlowski wrote:
>> Add documentation for MCP4728
>>
>> Signed-off-by: Andrea Collamati <andrea.collamati@gmail.com>
>> ---
>>  .../bindings/iio/dac/microchip,mcp4728.yaml   | 48 +++++++++++++++++++
>>  1 file changed, 48 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/iio/dac/microchip,mcp4728.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/iio/dac/microchip,mcp4728.yaml b/Documentation/devicetree/bindings/iio/dac/microchip,mcp4728.yaml
>> new file mode 100644
>> index 000000000000..6fd9be076245
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/iio/dac/microchip,mcp4728.yaml
>> @@ -0,0 +1,48 @@
>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/iio/dac/microchip,mcp4728.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Microchip MCP4728 DAC
>> +
>> +description:
>> +  MCP4728 is a quad channel, 12-bit voltage output
>> +  Digital-to-Analog Converter with non-volatile
>> +  memory and I2C compatible Serial Interface.
>> +  https://www.microchip.com/en-us/product/mcp4728
>> +
>> +maintainers:
>> +  - Andrea Collamati <andrea.collamati@gmail.com>
>> +
>> +properties:
>> +  compatible:
>> +    enum:
>> +      - microchip,mcp4728
> This is a friendly reminder during the review process.

Sorry but I didn't understand all your requests:

- I changed in the title mcp4728 with MCP4728

- I added description

but I don't know which blank line or whitespaces should be removed.

Can you tell me please?
Re: [PATCH v3 1/2] dt-bindings: iio: dac: add mcp4728.yaml
Posted by Krzysztof Kozlowski 2 years, 6 months ago 
On 21/07/2023 13:58, Andrea Collamati wrote:
> Hi Krzysztof,
> 
> On 7/21/23 10:21, Krzysztof Kozlowski wrote:
>>> Add documentation for MCP4728
>>>
>>> Signed-off-by: Andrea Collamati <andrea.collamati@gmail.com>
>>> ---
>>>  .../bindings/iio/dac/microchip,mcp4728.yaml   | 48 +++++++++++++++++++
>>>  1 file changed, 48 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/iio/dac/microchip,mcp4728.yaml
>>>
>>> diff --git a/Documentation/devicetree/bindings/iio/dac/microchip,mcp4728.yaml b/Documentation/devicetree/bindings/iio/dac/microchip,mcp4728.yaml
>>> new file mode 100644
>>> index 000000000000..6fd9be076245
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/iio/dac/microchip,mcp4728.yaml
>>> @@ -0,0 +1,48 @@
>>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/iio/dac/microchip,mcp4728.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: Microchip MCP4728 DAC
>>> +
>>> +description:
>>> +  MCP4728 is a quad channel, 12-bit voltage output
>>> +  Digital-to-Analog Converter with non-volatile
>>> +  memory and I2C compatible Serial Interface.
>>> +  https://www.microchip.com/en-us/product/mcp4728
>>> +
>>> +maintainers:
>>> +  - Andrea Collamati <andrea.collamati@gmail.com>
>>> +
>>> +properties:
>>> +  compatible:
>>> +    enum:
>>> +      - microchip,mcp4728
>> This is a friendly reminder during the review process.
> 
> Sorry but I didn't understand all your requests:
> 
> - I changed in the title mcp4728 with MCP4728
> 
> - I added description
> 
> but I don't know which blank line or whitespaces should be removed.
> 
> Can you tell me please?

You forgot to add blank line. Open example-schema and compare.

Also, you had white-space errors. Editors should show it to you. Git
maybe as well.

Best regards,
Krzysztof
Re: [PATCH v3 1/2] dt-bindings: iio: dac: add mcp4728.yaml
Posted by Krzysztof Kozlowski 2 years, 6 months ago 
On 21/07/2023 10:21, Krzysztof Kozlowski wrote:
>> +  - |
>> +    i2c {
>> +        #address-cells = <1>;
>> +        #size-cells = <0>;
>> +
>> +        mcp4728@60 {
> 
> The same... Probably more comments were ignored, so:
> 
> This is a friendly reminder during the review process.
> 
> It seems my previous comments were not fully addressed. Maybe my
> feedback got lost between the quotes, maybe you just forgot to apply it.
> Please go back to the previous discussion and either implement all
> requested changes or keep discussing them.

Damn, this is my third comment about the same. Here was second:
https://lore.kernel.org/all/5e5d1a1e-f106-9dd6-c19e-f933e8e70dd4@kernel.org/

so you nicely ignore feedback. NAK.

Best regards,
Krzysztof
Re: [PATCH v3 1/2] dt-bindings: iio: dac: add mcp4728.yaml
Posted by Andrea Collamati 2 years, 6 months ago 
On 7/21/23 10:22, Krzysztof Kozlowski wrote:
> On 21/07/2023 10:21, Krzysztof Kozlowski wrote:
>>> +  - |
>>> +    i2c {
>>> +        #address-cells = <1>;
>>> +        #size-cells = <0>;
>>> +
>>> +        mcp4728@60 {
>> The same... Probably more comments were ignored, so:
>>
>> This is a friendly reminder during the review process.
>>
>> It seems my previous comments were not fully addressed. Maybe my
>> feedback got lost between the quotes, maybe you just forgot to apply it.
>> Please go back to the previous discussion and either implement all
>> requested changes or keep discussing them.

Sorry, you are right. I missed to change the node name.

{

        #address-cells = <1>;
        #size-cells = <0>;

        dac@60 {

could be ok?


Thank you

       Andrea
Re: [PATCH v3 1/2] dt-bindings: iio: dac: add mcp4728.yaml
Posted by Conor Dooley 2 years, 6 months ago 
Hey Andrea,

On Thu, Jul 20, 2023 at 05:40:02PM +0200, Andrea Collamati wrote:
> Add documentation for MCP4728
> 
> Signed-off-by: Andrea Collamati <andrea.collamati@gmail.com>
> ---
>  .../bindings/iio/dac/microchip,mcp4728.yaml   | 48 +++++++++++++++++++
>  1 file changed, 48 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/dac/microchip,mcp4728.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/dac/microchip,mcp4728.yaml b/Documentation/devicetree/bindings/iio/dac/microchip,mcp4728.yaml
> new file mode 100644
> index 000000000000..6fd9be076245
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/dac/microchip,mcp4728.yaml
> @@ -0,0 +1,48 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/dac/microchip,mcp4728.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Microchip MCP4728 DAC
> +
> +description:
> +  MCP4728 is a quad channel, 12-bit voltage output
> +  Digital-to-Analog Converter with non-volatile
> +  memory and I2C compatible Serial Interface.
> +  https://www.microchip.com/en-us/product/mcp4728
> +
> +maintainers:
> +  - Andrea Collamati <andrea.collamati@gmail.com>
> +
> +properties:
> +  compatible:
> +    enum:
> +      - microchip,mcp4728

This can just be
compatible:
  const: microchip,mcp47288
since you only have a single item in your enum.

Otherwise, this looks good to me.
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>

Despite the email address, I have no knowledge of the hardware in
question, I'm just reviewing the binding.

Thanks,
Conor.

> +  reg:
> +    maxItems: 1
> +
> +  vdd-supply:
> +    description: |
> +      Provides both power and acts as the reference supply on the MCP4728
> +      when Internal Vref is not selected.
> +
> +required:
> +  - compatible
> +  - reg
> +  - vdd-supply
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        mcp4728@60 {
> +            compatible = "microchip,mcp4728";
> +            reg = <0x60>;
> +            vdd-supply = <&vdac_vdd>;
> +        };
> +    };
> -- 
> 2.34.1
>
Re: [PATCH v3 1/2] dt-bindings: iio: dac: add mcp4728.yaml
Posted by Andrea Collamati 2 years, 6 months ago 
Hi Conor,

On 7/20/23 19:01, Conor Dooley wrote:
>> Add documentation for MCP4728
>>
>> Signed-off-by: Andrea Collamati <andrea.collamati@gmail.com>
>> ---
>>  .../bindings/iio/dac/microchip,mcp4728.yaml   | 48 +++++++++++++++++++
>>  1 file changed, 48 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/iio/dac/microchip,mcp4728.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/iio/dac/microchip,mcp4728.yaml b/Documentation/devicetree/bindings/iio/dac/microchip,mcp4728.yaml
>> new file mode 100644
>> index 000000000000..6fd9be076245
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/iio/dac/microchip,mcp4728.yaml
>> @@ -0,0 +1,48 @@
>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/iio/dac/microchip,mcp4728.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Microchip MCP4728 DAC
>> +
>> +description:
>> +  MCP4728 is a quad channel, 12-bit voltage output
>> +  Digital-to-Analog Converter with non-volatile
>> +  memory and I2C compatible Serial Interface.
>> +  https://www.microchip.com/en-us/product/mcp4728
>> +
>> +maintainers:
>> +  - Andrea Collamati <andrea.collamati@gmail.com>
>> +
>> +properties:
>> +  compatible:
>> +    enum:
>> +      - microchip,mcp4728
> This can just be
> compatible:
>   const: microchip,mcp47288
> since you only have a single item in your enum.

I will in include in v4.

Thank you.

               Andrea

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.