Re: [PATCH v4 1/7] dt-bindings: Add panel-timing subnode to simple-panel

[Rob Herring <robh+dt@kernel.org>]

On Thu, Mar 28, 2019 at 6:50 PM Doug Anderson <dianders@chromium.org> wrote:
>
> Hi,
>
>
> On Thu, Mar 28, 2019 at 1:27 PM Ezequiel Garcia <ezequiel@collabora.com> wrote:
> >
> > On Thu, 2019-03-28 at 10:17 -0700, Douglas Anderson wrote:
> > > From: Sean Paul <seanpaul@chromium.org>
> > >
> > > This patch adds a new subnode to simple-panel allowing us to override
> > > the typical timing expressed in the panel's display_timing.
> > >
> > > Changes in v2:
> > >  - Split out the binding into a new patch (Rob)
> > >  - display-timings is a new section (Rob)
> > >  - Use the full display-timings subnode instead of picking the timing
> > >    out (Rob/Thierry)
> > > Changes in v3:
> > >  - Go back to using the timing subnode directly, but rename to
> > >    panel-timing (Rob)
> > > Changes in v4:
> > >  - Simplify desc. for when override should be used (Thierry/Laurent)
> > >  - Removed Rob H review since it's been a year and wording changed
> > >
> > > Cc: Doug Anderson <dianders@chromium.org>
> > > Cc: Eric Anholt <eric@anholt.net>
> > > Cc: Heiko Stuebner <heiko@sntech.de>
> > > Cc: Jeffy Chen <jeffy.chen@rock-chips.com>
> > > Cc: Rob Herring <robh+dt@kernel.org>
> > > Cc: Stéphane Marchesin <marcheu@chromium.org>
> > > Cc: Thierry Reding <thierry.reding@gmail.com>
> > > Cc: devicetree@vger.kernel.org
> > > Cc: dri-devel@lists.freedesktop.org
> > > Cc: linux-rockchip@lists.infradead.org
> > > Signed-off-by: Sean Paul <seanpaul@chromium.org>
> > > Signed-off-by: Douglas Anderson <dianders@chromium.org>
> > > ---
> > >
> > >  .../bindings/display/panel/simple-panel.txt   | 24 +++++++++++++++++++
> > >  1 file changed, 24 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/display/panel/simple-panel.txt b/Documentation/devicetree/bindings/display/panel/simple-panel.txt
> > > index b2b872c710f2..6157f86ddce4 100644
> > > --- a/Documentation/devicetree/bindings/display/panel/simple-panel.txt
> > > +++ b/Documentation/devicetree/bindings/display/panel/simple-panel.txt
> > > @@ -15,6 +15,18 @@ Optional properties:
> > >    (hot plug detect) signal, but the signal isn't hooked up so we should
> > >    hardcode the max delay from the panel spec when powering up the panel.
> > >
> > > +panel-timing subnode
> > > +--------------------
> > > +
> > > +This optional subnode is for devices which require a mode differing
> > > +from the panel's "typical" display timing.  The panel timings provided
> > > +here will be ignored if they are found to be outside of allowable
> > > +ranges for the given panel.
> > > +
> >
> > Is it OK to put this comment about how the implementation
> > will behave when values are out of range, given this is just a binding
> > spec?
> >
> > Perhaps -if needed- this sentence can be rephrased to state that,
> > e.g. the OS may not be able to apply these values, if the controller
> > or device is unable to?
>
> I will defer to Rob H. on this one, but I'm happy to simply remove the
> last sentence.  I was trying to add a more OS-agnostic version of the
> bullet points from V3 but agree that we could just remove this from
> the bindings completely.

Following my opinion that it's not the kernel's job to validate
bindings, I would say it's fine for the OS to blindly apply them if it
chooses.

Plus with schema, you can provide the ranges of values and validate
DTs up front (unless you want to validate some result of math
operations).

Rob