On Mon, Apr 6, 2020 at 5:40 AM Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote:
>
> Hi Laurent,
>
> On Mon, Apr 6, 2020 at 1:09 PM Laurent Pinchart
> <laurent.pinchart@xxxxxxxxxxxxxxxx> wrote:
> > On Mon, Apr 06, 2020 at 10:47:37AM +0200, Geert Uytterhoeven wrote:
> > > On Mon, Apr 6, 2020 at 1:24 AM Laurent Pinchart wrote:
> > > > Convert the Renesas R-Car LVDS encoder text binding to YAML.
> > > >
> > > > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@xxxxxxxxxxxxxxxx>
>
> > > > +if:
> > > > + properties:
> > > > + compatible:
> > > > + enum:
> > > > + - renesas,r8a774c0-lvds
> > > > + - renesas,r8a77990-lvds
> > > > + - renesas,r8a77995-lvds
> > > > +then:
> > > > + properties:
> > > > + clocks:
> > > > + minItems: 1
> > > > + maxItems: 4
> > > > + items:
> > > > + - description: Functional clock
> > > > + - description: EXTAL input clock
> > > > + - description: DU_DOTCLKIN0 input clock
> > > > + - description: DU_DOTCLKIN1 input clock
> > > > +
> > > > + clock-names:
> > > > + minItems: 1
> > > > + maxItems: 4
> > > > + items:
> > > > + - const: fck
> > > > + # The LVDS encoder can use the EXTAL or DU_DOTCLKINx clocks.
> > > > + # These clocks are optional.
> > > > + - enum:
> > > > + - extal
> > > > + - dclkin.0
> > > > + - dclkin.1
> > > > + - enum:
> > > > + - extal
> > > > + - dclkin.0
> > > > + - dclkin.1
> > > > + - enum:
> > > > + - extal
> > > > + - dclkin.0
> > > > + - dclkin.1
> > >
> > > Can the duplication of the last 3 entries be avoided?
> > > Perhaps like in
> > > Documentation/devicetree/bindings/serial/renesas,scif.yaml?
> >
> > I'd love to, if you can tell me how to make sure the fck entry is
> > mandatory. The following
> >
> > minItems: 1
> > maxItems: 4
> > items:
> > enum:
> > - fck
> > - extal
> > - dclkin.0
> > - dclkin.1
> >
> > passes the checks, but would accept
> >
> > clock-names = "extal";
> >
> > which is not valid. Your
> > Documentation/devicetree/bindings/serial/renesas,scif.yaml bindings
> > suffer from the same problem :-)
>
> Hmm....
>
> > > > +examples:
> > > > + - |
> > > > + #include <dt-bindings/clock/renesas-cpg-mssr.h>
> > > > + #include <dt-bindings/power/r8a7795-sysc.h>
> > > > +
> > > > + lvds@feb90000 {
> > > > + compatible = "renesas,r8a7795-lvds";
> > > > + reg = <0 0xfeb90000 0 0x14>;
> > >
> > > Examples are built with #{address,size}-cells = <1>.
> >
> > Are they ? I don't get any failure from make dt_binding_check.
>
> Hmm... And you do have "reg: maxItems: 1"...
At first glance I was expecting an error too, but there isn't. As far
as the schema is concerned, it's valid because it's a single entry
(i.e. one entry in <>). And then dtc can only check that reg is a
multiple of 2. The size check does work where we have more constraints
like I2C.
If we enforce bracketing, then we should be able to check these.
Otherwise, knowing both the cell sizes and number of entries is a
problem. With bracketing, we can split those checks. I'd been thinking
checking cell sizes would be easier in dtc (we're already doing that
in lots of cases), but thinking about it some more there is a way to
do this with schema:
if:
properties:
'#address-cells':
const: 2
'#size-cells':
const: 2
required:
- '#address-cells'
- '#size-cells'
then:
patternProperties:
'@':
properties:
reg:
items:
minItems: 4
maxItems: 4
required:
- reg
...and copy-n-paste for each size combination.
I imagine implementing this will result in another set of fixes.
Rob
