| Message ID | 20240510141836.1624009-3-adureghello@baylibre.org (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [1/3] dt-bindings: iio: dac: add ad35xxr single output variants | expand |
On Fri, May 10, 2024 at 9:19 AM Angelo Dureghello <adureghello@baylibre.com> wrote: > > From: Angelo Dureghello <adureghello@baylibre.com> > > The adi,gain-scaling-p/n values are an inverted log2, > so initial naiming was set correct, but the driver uses just > adi,gain-scaling-p/n, so uniforming documentation, that seems > a less-risk fix for future rebases, and still conformant to datasheet. > > Signed-off-by: Angelo Dureghello <adureghello@baylibre.com> > --- > .../devicetree/bindings/iio/dac/adi,ad3552r.yaml | 16 ++++++++-------- > 1 file changed, 8 insertions(+), 8 deletions(-) > > diff --git a/Documentation/devicetree/bindings/iio/dac/adi,ad3552r.yaml b/Documentation/devicetree/bindings/iio/dac/adi,ad3552r.yaml > index 17442cdfbe27..9e3dbf890bfa 100644 > --- a/Documentation/devicetree/bindings/iio/dac/adi,ad3552r.yaml > +++ b/Documentation/devicetree/bindings/iio/dac/adi,ad3552r.yaml > @@ -94,13 +94,13 @@ patternProperties: > maximum: 511 > minimum: -511 > > - adi,gain-scaling-p-inv-log2: > - description: GainP = 1 / ( 2 ^ adi,gain-scaling-p-inv-log2) > + adi,gain-scaling-p: > + description: GainP = 1 / ( 2 ^ adi,gain-scaling-p) > $ref: /schemas/types.yaml#/definitions/uint32 > enum: [0, 1, 2, 3] > > - adi,gain-scaling-n-inv-log2: > - description: GainN = 1 / ( 2 ^ adi,gain-scaling-n-inv-log2) > + adi,gain-scaling-n: > + description: GainN = 1 / ( 2 ^ adi,gain-scaling-n) > $ref: /schemas/types.yaml#/definitions/uint32 > enum: [0, 1, 2, 3] > > @@ -109,8 +109,8 @@ patternProperties: > > required: > - adi,gain-offset > - - adi,gain-scaling-p-inv-log2 > - - adi,gain-scaling-n-inv-log2 > + - adi,gain-scaling-p > + - adi,gain-scaling-n > - adi,rfb-ohms > > required: > @@ -214,8 +214,8 @@ examples: > reg = <1>; > custom-output-range-config { > adi,gain-offset = <5>; > - adi,gain-scaling-p-inv-log2 = <1>; > - adi,gain-scaling-n-inv-log2 = <2>; > + adi,gain-scaling-p = <1>; > + adi,gain-scaling-n = <2>; > adi,rfb-ohms = <1>; > }; > }; > -- > 2.45.0.rc1 > > The DT bindings are generally considered immutable. So unless we can prove that no one has ever put adi,gain-scaling-n-inv-log2 in a .dtb file, we probably need to fix this in the driver rather than in the bindings. (The driver can still handle adi,gain-scaling-p in the driver for backwards compatibility but the official binding should be what was already accepted in the .yaml file)
On Fri, May 10, 2024 at 10:43:18AM -0500, David Lechner wrote: > On Fri, May 10, 2024 at 9:19 AM Angelo Dureghello > <adureghello@baylibre.com> wrote: > > > > From: Angelo Dureghello <adureghello@baylibre.com> > > > > The adi,gain-scaling-p/n values are an inverted log2, > > so initial naiming was set correct, but the driver uses just > > adi,gain-scaling-p/n, so uniforming documentation, that seems > > a less-risk fix for future rebases, and still conformant to datasheet. > > > > Signed-off-by: Angelo Dureghello <adureghello@baylibre.com> > > --- > > .../devicetree/bindings/iio/dac/adi,ad3552r.yaml | 16 ++++++++-------- > > 1 file changed, 8 insertions(+), 8 deletions(-) > > > > diff --git a/Documentation/devicetree/bindings/iio/dac/adi,ad3552r.yaml b/Documentation/devicetree/bindings/iio/dac/adi,ad3552r.yaml > > index 17442cdfbe27..9e3dbf890bfa 100644 > > --- a/Documentation/devicetree/bindings/iio/dac/adi,ad3552r.yaml > > +++ b/Documentation/devicetree/bindings/iio/dac/adi,ad3552r.yaml > > @@ -94,13 +94,13 @@ patternProperties: > > maximum: 511 > > minimum: -511 > > > > - adi,gain-scaling-p-inv-log2: > > - description: GainP = 1 / ( 2 ^ adi,gain-scaling-p-inv-log2) > > + adi,gain-scaling-p: > > + description: GainP = 1 / ( 2 ^ adi,gain-scaling-p) > > $ref: /schemas/types.yaml#/definitions/uint32 > > enum: [0, 1, 2, 3] > > > > - adi,gain-scaling-n-inv-log2: > > - description: GainN = 1 / ( 2 ^ adi,gain-scaling-n-inv-log2) > > + adi,gain-scaling-n: > > + description: GainN = 1 / ( 2 ^ adi,gain-scaling-n) > > $ref: /schemas/types.yaml#/definitions/uint32 > > enum: [0, 1, 2, 3] > > > > @@ -109,8 +109,8 @@ patternProperties: > > > > required: > > - adi,gain-offset > > - - adi,gain-scaling-p-inv-log2 > > - - adi,gain-scaling-n-inv-log2 > > + - adi,gain-scaling-p > > + - adi,gain-scaling-n > > - adi,rfb-ohms > > > > required: > > @@ -214,8 +214,8 @@ examples: > > reg = <1>; > > custom-output-range-config { > > adi,gain-offset = <5>; > > - adi,gain-scaling-p-inv-log2 = <1>; > > - adi,gain-scaling-n-inv-log2 = <2>; > > + adi,gain-scaling-p = <1>; > > + adi,gain-scaling-n = <2>; > > adi,rfb-ohms = <1>; > > }; > > }; > > -- > > 2.45.0.rc1 > > > > > > The DT bindings are generally considered immutable. So unless we can > prove that no one has ever put adi,gain-scaling-n-inv-log2 in a .dtb > file, You can't ever prove that. > we probably need to fix this in the driver rather than in the > bindings. (The driver can still handle adi,gain-scaling-p in the > driver for backwards compatibility but the official binding should be > what was already accepted in the .yaml file) If we can reasonable assume that the Linux driver is the only consumer, there are no upstream dts users (in kernel or other opensource projects), and/or the property is somewhat recent, then that's good enough IMO. Rob
On Mon, May 13, 2024 at 1:52 PM Rob Herring <robh@kernel.org> wrote: > > On Fri, May 10, 2024 at 10:43:18AM -0500, David Lechner wrote: > > On Fri, May 10, 2024 at 9:19 AM Angelo Dureghello > > <adureghello@baylibre.com> wrote: > > > > > > From: Angelo Dureghello <adureghello@baylibre.com> > > > > > > The adi,gain-scaling-p/n values are an inverted log2, > > > so initial naiming was set correct, but the driver uses just > > > adi,gain-scaling-p/n, so uniforming documentation, that seems > > > a less-risk fix for future rebases, and still conformant to datasheet. > > > > > > Signed-off-by: Angelo Dureghello <adureghello@baylibre.com> > > > --- > > > .../devicetree/bindings/iio/dac/adi,ad3552r.yaml | 16 ++++++++-------- > > > 1 file changed, 8 insertions(+), 8 deletions(-) > > > > > > diff --git a/Documentation/devicetree/bindings/iio/dac/adi,ad3552r.yaml b/Documentation/devicetree/bindings/iio/dac/adi,ad3552r.yaml > > > index 17442cdfbe27..9e3dbf890bfa 100644 > > > --- a/Documentation/devicetree/bindings/iio/dac/adi,ad3552r.yaml > > > +++ b/Documentation/devicetree/bindings/iio/dac/adi,ad3552r.yaml > > > @@ -94,13 +94,13 @@ patternProperties: > > > maximum: 511 > > > minimum: -511 > > > > > > - adi,gain-scaling-p-inv-log2: > > > - description: GainP = 1 / ( 2 ^ adi,gain-scaling-p-inv-log2) > > > + adi,gain-scaling-p: > > > + description: GainP = 1 / ( 2 ^ adi,gain-scaling-p) > > > $ref: /schemas/types.yaml#/definitions/uint32 > > > enum: [0, 1, 2, 3] > > > > > > - adi,gain-scaling-n-inv-log2: > > > - description: GainN = 1 / ( 2 ^ adi,gain-scaling-n-inv-log2) > > > + adi,gain-scaling-n: > > > + description: GainN = 1 / ( 2 ^ adi,gain-scaling-n) > > > $ref: /schemas/types.yaml#/definitions/uint32 > > > enum: [0, 1, 2, 3] > > > > > > @@ -109,8 +109,8 @@ patternProperties: > > > > > > required: > > > - adi,gain-offset > > > - - adi,gain-scaling-p-inv-log2 > > > - - adi,gain-scaling-n-inv-log2 > > > + - adi,gain-scaling-p > > > + - adi,gain-scaling-n > > > - adi,rfb-ohms > > > > > > required: > > > @@ -214,8 +214,8 @@ examples: > > > reg = <1>; > > > custom-output-range-config { > > > adi,gain-offset = <5>; > > > - adi,gain-scaling-p-inv-log2 = <1>; > > > - adi,gain-scaling-n-inv-log2 = <2>; > > > + adi,gain-scaling-p = <1>; > > > + adi,gain-scaling-n = <2>; > > > adi,rfb-ohms = <1>; > > > }; > > > }; > > > -- > > > 2.45.0.rc1 > > > > > > > > > > The DT bindings are generally considered immutable. So unless we can > > prove that no one has ever put adi,gain-scaling-n-inv-log2 in a .dtb > > file, > > You can't ever prove that. > > > we probably need to fix this in the driver rather than in the > > bindings. (The driver can still handle adi,gain-scaling-p in the > > driver for backwards compatibility but the official binding should be > > what was already accepted in the .yaml file) > > If we can reasonable assume that the Linux driver is the only consumer, > there are no upstream dts users (in kernel or other opensource > projects), and/or the property is somewhat recent, then that's good > enough IMO. > > Rob Ack. I stand corrected then.
diff --git a/Documentation/devicetree/bindings/iio/dac/adi,ad3552r.yaml b/Documentation/devicetree/bindings/iio/dac/adi,ad3552r.yaml index 17442cdfbe27..9e3dbf890bfa 100644 --- a/Documentation/devicetree/bindings/iio/dac/adi,ad3552r.yaml +++ b/Documentation/devicetree/bindings/iio/dac/adi,ad3552r.yaml @@ -94,13 +94,13 @@ patternProperties: maximum: 511 minimum: -511 - adi,gain-scaling-p-inv-log2: - description: GainP = 1 / ( 2 ^ adi,gain-scaling-p-inv-log2) + adi,gain-scaling-p: + description: GainP = 1 / ( 2 ^ adi,gain-scaling-p) $ref: /schemas/types.yaml#/definitions/uint32 enum: [0, 1, 2, 3] - adi,gain-scaling-n-inv-log2: - description: GainN = 1 / ( 2 ^ adi,gain-scaling-n-inv-log2) + adi,gain-scaling-n: + description: GainN = 1 / ( 2 ^ adi,gain-scaling-n) $ref: /schemas/types.yaml#/definitions/uint32 enum: [0, 1, 2, 3] @@ -109,8 +109,8 @@ patternProperties: required: - adi,gain-offset - - adi,gain-scaling-p-inv-log2 - - adi,gain-scaling-n-inv-log2 + - adi,gain-scaling-p + - adi,gain-scaling-n - adi,rfb-ohms required: @@ -214,8 +214,8 @@ examples: reg = <1>; custom-output-range-config { adi,gain-offset = <5>; - adi,gain-scaling-p-inv-log2 = <1>; - adi,gain-scaling-n-inv-log2 = <2>; + adi,gain-scaling-p = <1>; + adi,gain-scaling-n = <2>; adi,rfb-ohms = <1>; }; };