devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Rob Herring <robh@kernel.org>
To: David Lechner <dlechner@baylibre.com>
Cc: Angelo Dureghello <adureghello@baylibre.com>,
	jic23@kernel.org, krzk+dt@kernel.org, conor+dt@kernel.org,
	nuno.sa@analog.com, lars@metafoo.de,
	Michael.Hennerich@analog.com, linux-iio@vger.kernel.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 3/3] dt-bindings: iio: dac: fix ad3552r gain parameter names
Date: Mon, 13 May 2024 13:52:31 -0500	[thread overview]
Message-ID: <20240513185231.GA2920495-robh@kernel.org> (raw)
In-Reply-To: <CAMknhBGU8bXg7obzyjzb7a4AUbjnw_0b+mqEAYJJekAK2CB-CQ@mail.gmail.com>

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

  reply	other threads:[~2024-05-13 18:52 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-05-10 14:18 [PATCH 1/3] dt-bindings: iio: dac: add ad35xxr single output variants Angelo Dureghello
2024-05-10 14:18 ` [PATCH 2/3] iio: dac: ad3552r: add support for ad3541r and ad3551r Angelo Dureghello
2024-05-10 15:42   ` David Lechner
2024-05-16 13:17     ` Angelo Dureghello
2024-05-10 14:18 ` [PATCH 3/3] dt-bindings: iio: dac: fix ad3552r gain parameter names Angelo Dureghello
2024-05-10 15:43   ` David Lechner
2024-05-13 18:52     ` Rob Herring [this message]
2024-05-13 19:03       ` David Lechner
2024-05-10 15:39 ` [PATCH 1/3] dt-bindings: iio: dac: add ad35xxr single output variants David Lechner
2024-05-11 16:05   ` Jonathan Cameron
2024-05-16 13:20     ` Angelo Dureghello
2024-05-11 16:04 ` Jonathan Cameron

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240513185231.GA2920495-robh@kernel.org \
    --to=robh@kernel.org \
    --cc=Michael.Hennerich@analog.com \
    --cc=adureghello@baylibre.com \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=dlechner@baylibre.com \
    --cc=jic23@kernel.org \
    --cc=krzk+dt@kernel.org \
    --cc=lars@metafoo.de \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nuno.sa@analog.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).