* Re: [PATCH] dt-bindings: iio: dac: mcp47feb02: Fix binding issues
2026-04-16 13:33 [PATCH] dt-bindings: iio: dac: mcp47feb02: Fix binding issues Ariana Lazar
@ 2026-04-16 15:15 ` Conor Dooley
0 siblings, 0 replies; 2+ messages in thread
From: Conor Dooley @ 2026-04-16 15:15 UTC (permalink / raw)
To: Ariana Lazar
Cc: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Jonathan Cameron,
Conor Dooley, linux-iio, devicetree, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 8563 bytes --]
Hey,
On Thu, Apr 16, 2026 at 04:33:36PM +0300, Ariana Lazar wrote:
> Change maxItems value from 8 to 1 for the channel number reg property.
> Change example reg value from 0 to 0x60.
> Fix a few typos in property descriptions.
> Sort the part numbers in the enum list lexicographically.
Hmm, this looks like 3 or 4 patches to me. The maxItems change probably
qualifies for a fixes tag, none of the rest do.
>
> Fixes: 4ba12d304175 ("dt-bindings: iio: dac: adding support for Microchip MCP47FEB02")
> Reported-by: Conor Dooley <conor@kernel.org>
> Closes: https://lore.kernel.org/all/20260403-speed-childless-1360de358229@spud/
I didn't report anything, I just commented on something that you had
already "reported" when you fixed it yourself.
> Reported-by: David Lechner <dlechner@baylibre.com>
> Closes: https://lore.kernel.org/all/dd0dbadb-604b-4f12-8674-268b7db096fd@baylibre.com/
David didn't report anything either, he just gave you review feedback on
the change you made to the example.
> Signed-off-by: Ariana Lazar <ariana.lazar@microchip.com>
> ---
> .../bindings/iio/dac/microchip,mcp47feb02.yaml | 57 +++++++++++-----------
> 1 file changed, 28 insertions(+), 29 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/iio/dac/microchip,mcp47feb02.yaml b/Documentation/devicetree/bindings/iio/dac/microchip,mcp47feb02.yaml
> index d2466aa6bda2106a8b695347a0edf38462294d03..88a1495f2967a3d821ada7e7e9a7fbb466401040 100644
> --- a/Documentation/devicetree/bindings/iio/dac/microchip,mcp47feb02.yaml
> +++ b/Documentation/devicetree/bindings/iio/dac/microchip,mcp47feb02.yaml
> @@ -61,29 +61,29 @@ properties:
> compatible:
> enum:
> - microchip,mcp47feb01
> - - microchip,mcp47feb11
> - - microchip,mcp47feb21
> - microchip,mcp47feb02
> + - microchip,mcp47feb04
> + - microchip,mcp47feb08
> + - microchip,mcp47feb11
> - microchip,mcp47feb12
> + - microchip,mcp47feb14
> + - microchip,mcp47feb18
> + - microchip,mcp47feb21
> - microchip,mcp47feb22
> + - microchip,mcp47feb24
> + - microchip,mcp47feb28
> - microchip,mcp47fvb01
> - - microchip,mcp47fvb11
> - - microchip,mcp47fvb21
> - microchip,mcp47fvb02
> - - microchip,mcp47fvb12
> - - microchip,mcp47fvb22
> - microchip,mcp47fvb04
> - - microchip,mcp47fvb14
> - - microchip,mcp47fvb24
> - microchip,mcp47fvb08
> + - microchip,mcp47fvb11
> + - microchip,mcp47fvb12
> + - microchip,mcp47fvb14
> - microchip,mcp47fvb18
> + - microchip,mcp47fvb21
> + - microchip,mcp47fvb22
> + - microchip,mcp47fvb24
> - microchip,mcp47fvb28
> - - microchip,mcp47feb04
> - - microchip,mcp47feb14
> - - microchip,mcp47feb24
> - - microchip,mcp47feb08
> - - microchip,mcp47feb18
> - - microchip,mcp47feb28
>
> reg:
> maxItems: 1
> @@ -111,13 +111,13 @@ properties:
> - for single-channel device: Vout0;
> - for dual-channel device: Vout0, Vout1;
> - for quad-channel device: Vout0, Vout2;
> - - for octal-channel device: Vout0, Vout2, Vout6, Vout8;
> + - for octal-channel device: Vout0, Vout2, Vout4, Vout6;
>
> vref1-supply:
> description: |
> Vref1 pin may be used as a voltage reference when this supply is specified.
> The internal reference will be taken into account for voltage reference
> - beside VDD if this supply does not exist.
> + besides VDD if this supply does not exist.
ngl, I don't think this is a typo, the sentence doesn't make sense
either before or after the change. Should this be something like "The
internal reference and VDD will be used as the voltage reference if this
supply does not exist?
>
> This supply will be voltage reference for the following outputs:
> - for quad-channel device: Vout1, Vout3;
> @@ -141,7 +141,7 @@ properties:
> description:
> Enable buffering of the external Vref/Vref0 pin in cases where the
> external reference voltage does not have sufficient current capability in
> - order not to drop it’s voltage when connected to the internal resistor
> + order not to drop its voltage when connected to the internal resistor
> ladder circuit.
>
> microchip,vref1-buffered:
> @@ -149,7 +149,7 @@ properties:
> description:
> Enable buffering of the external Vref1 pin in cases where the external
> reference voltage does not have sufficient current capability in order not
> - to drop it’s voltage when connected to the internal resistor ladder
> + to drop its voltage when connected to the internal resistor ladder
> circuit.
>
> patternProperties:
> @@ -161,8 +161,7 @@ patternProperties:
> properties:
> reg:
> description: The channel number.
> - minItems: 1
> - maxItems: 8
> + maxItems: 1
>
> label:
> description: Unique name to identify which channel this is.
> @@ -227,12 +226,12 @@ allOf:
> compatible:
> contains:
> enum:
> - - microchip,mcp47fvb04
> - - microchip,mcp47fvb14
> - - microchip,mcp47fvb24
> - microchip,mcp47feb04
> - microchip,mcp47feb14
> - microchip,mcp47feb24
> + - microchip,mcp47fvb04
> + - microchip,mcp47fvb14
> + - microchip,mcp47fvb24
> then:
> patternProperties:
> "^channel@[0-3]$":
> @@ -245,12 +244,12 @@ allOf:
> compatible:
> contains:
> enum:
> - - microchip,mcp47fvb08
> - - microchip,mcp47fvb18
> - - microchip,mcp47fvb28
> - microchip,mcp47feb08
> - microchip,mcp47feb18
> - microchip,mcp47feb28
> + - microchip,mcp47fvb08
> + - microchip,mcp47fvb18
> + - microchip,mcp47fvb28
> then:
> patternProperties:
> "^channel@[0-7]$":
> @@ -280,9 +279,9 @@ examples:
>
> #address-cells = <1>;
> #size-cells = <0>;
> - dac@0 {
> + dac@60 {
> compatible = "microchip,mcp47feb02";
> - reg = <0>;
> + reg = <0x60>;
> vdd-supply = <&vdac_vdd>;
> vref-supply = <&vref_reg>;
>
> @@ -297,6 +296,6 @@ examples:
> reg = <0x1>;
> label = "Adjustable_voltage_ch1";
> };
> - };
> + };
> };
This whitespace change looks odd. You've aligned the opening and closing
braces, but the indent is not consistent as there's mixed 4- and 2-space
indentation used in this example. I think you need to do:
diff --git a/Documentation/devicetree/bindings/iio/dac/microchip,mcp47feb02.yaml b/Documentation/devicetree/bindings/iio/dac/microchip,mcp47feb02.yaml
index d2466aa6bda21..a36874c4ae276 100644
--- a/Documentation/devicetree/bindings/iio/dac/microchip,mcp47feb02.yaml
+++ b/Documentation/devicetree/bindings/iio/dac/microchip,mcp47feb02.yaml
@@ -278,25 +278,25 @@ examples:
- |
i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ dac@0 {
+ compatible = "microchip,mcp47feb02";
+ reg = <0>;
+ vdd-supply = <&vdac_vdd>;
+ vref-supply = <&vref_reg>;
+
#address-cells = <1>;
#size-cells = <0>;
- dac@0 {
- compatible = "microchip,mcp47feb02";
+ channel@0 {
reg = <0>;
- vdd-supply = <&vdac_vdd>;
- vref-supply = <&vref_reg>;
+ label = "Adjustable_voltage_ch0";
+ };
- #address-cells = <1>;
- #size-cells = <0>;
- channel@0 {
- reg = <0>;
- label = "Adjustable_voltage_ch0";
- };
-
- channel@1 {
- reg = <0x1>;
- label = "Adjustable_voltage_ch1";
- };
+ channel@1 {
+ reg = <0x1>;
+ label = "Adjustable_voltage_ch1";
+ };
};
};
...
Cheers,
Conor.
> ...
>
> ---
> base-commit: d2a4ec19d2a2e54c23b5180e939994d3da4a6b91
> change-id: 20260416-mcp47feb02-fix5-26994c5b428c
>
> Best regards,
> --
> Ariana Lazar <ariana.lazar@microchip.com>
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply related [flat|nested] 2+ messages in thread