| Message ID | 20190925235544.11524-2-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | [v9,1/4] dt-bindings: display: xlnx: Add ZynqMP DP subsystem bindings | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/dt-meta-schema | fail | build log |
On Wed, Sep 25, 2019 at 6:56 PM Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > > From: Hyun Kwon <hyun.kwon@xilinx.com> > > The bindings describe the ZynqMP DP subsystem. They don't support the > interface with the programmable logic (FPGA) or audio yet. > > Signed-off-by: Hyun Kwon <hyun.kwon@xilinx.com> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > Changes since v8: > > - Convert to yaml > - Rename aclk to dp_apb_clk /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/display/xlnx/xlnx,zynqmp-dpsub.example.dt.yaml: display@fd4a0000: clock-names:2: 'dp_vtc_pixel_clk_in' was expected
Hi Rob, On Thu, Sep 26, 2019 at 09:15:01AM -0500, Rob Herring wrote: > On Wed, Sep 25, 2019 at 6:56 PM Laurent Pinchart wrote: > > > > From: Hyun Kwon <hyun.kwon@xilinx.com> > > > > The bindings describe the ZynqMP DP subsystem. They don't support the > > interface with the programmable logic (FPGA) or audio yet. > > > > Signed-off-by: Hyun Kwon <hyun.kwon@xilinx.com> > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > --- > > Changes since v8: > > > > - Convert to yaml > > - Rename aclk to dp_apb_clk > > /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/display/xlnx/xlnx,zynqmp-dpsub.example.dt.yaml: > display@fd4a0000: clock-names:2: 'dp_vtc_pixel_clk_in' was expected If you allow me to steal a bit of your brain time, could you help me expressing the clocks constraint ? clocks: description: The AXI clock and at least one video clock are mandatory, the audio clock optional. minItems: 2 maxItems: 4 items: - description: AXI clock - description: Audio clock - description: Non-live video clock (from Processing System) - description: Live video clock (from Programmable Logic) clock-names: minItems: 2 maxItems: 4 items: - const: dp_apb_clk - const: dp_aud_clk - const: dp_vtc_pixel_clk_in - const: dp_live_video_in_clk dp_apb_clk is required, dp_aud_clk is optional, and at least one of dp_vtc_pixel_clk_in and dp_live_video_in_clk is required.
On Thu, Sep 26, 2019 at 9:23 AM Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > > Hi Rob, > > On Thu, Sep 26, 2019 at 09:15:01AM -0500, Rob Herring wrote: > > On Wed, Sep 25, 2019 at 6:56 PM Laurent Pinchart wrote: > > > > > > From: Hyun Kwon <hyun.kwon@xilinx.com> > > > > > > The bindings describe the ZynqMP DP subsystem. They don't support the > > > interface with the programmable logic (FPGA) or audio yet. > > > > > > Signed-off-by: Hyun Kwon <hyun.kwon@xilinx.com> > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > --- > > > Changes since v8: > > > > > > - Convert to yaml > > > - Rename aclk to dp_apb_clk > > > > /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/display/xlnx/xlnx,zynqmp-dpsub.example.dt.yaml: > > display@fd4a0000: clock-names:2: 'dp_vtc_pixel_clk_in' was expected > > If you allow me to steal a bit of your brain time, could you help me > expressing the clocks constraint ? > > clocks: > description: > The AXI clock and at least one video clock are mandatory, the audio clock > optional. > minItems: 2 > maxItems: 4 > items: > - description: AXI clock > - description: Audio clock > - description: Non-live video clock (from Processing System) > - description: Live video clock (from Programmable Logic) > clock-names: > minItems: 2 > maxItems: 4 > items: > - const: dp_apb_clk > - const: dp_aud_clk > - const: dp_vtc_pixel_clk_in > - const: dp_live_video_in_clk > > dp_apb_clk is required, dp_aud_clk is optional, and at least one of > dp_vtc_pixel_clk_in and dp_live_video_in_clk is required. I'm hoping people's inability to express the schema will prevent complicated ones like this in the first place... clock-names: oneOf: - minItems: 3 maxItems: 4 items: - const: dp_apb_clk - const: dp_aud_clk - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] - minItems: 2 maxItems: 3 items: - const: dp_apb_clk - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] Strictly speaking, that leaves items clocks wrong, but 'description' doesn't do anything. So I'd just leave it as is. Rob
Hi Rob, On Thu, Sep 26, 2019 at 09:57:29AM -0500, Rob Herring wrote: > On Thu, Sep 26, 2019 at 9:23 AM Laurent Pinchart wrote: > > On Thu, Sep 26, 2019 at 09:15:01AM -0500, Rob Herring wrote: > > > On Wed, Sep 25, 2019 at 6:56 PM Laurent Pinchart wrote: > > > > > > > > From: Hyun Kwon <hyun.kwon@xilinx.com> > > > > > > > > The bindings describe the ZynqMP DP subsystem. They don't support the > > > > interface with the programmable logic (FPGA) or audio yet. > > > > > > > > Signed-off-by: Hyun Kwon <hyun.kwon@xilinx.com> > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > > --- > > > > Changes since v8: > > > > > > > > - Convert to yaml > > > > - Rename aclk to dp_apb_clk > > > > > > /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/display/xlnx/xlnx,zynqmp-dpsub.example.dt.yaml: > > > display@fd4a0000: clock-names:2: 'dp_vtc_pixel_clk_in' was expected > > > > If you allow me to steal a bit of your brain time, could you help me > > expressing the clocks constraint ? > > > > clocks: > > description: > > The AXI clock and at least one video clock are mandatory, the audio clock > > optional. > > minItems: 2 > > maxItems: 4 > > items: > > - description: AXI clock > > - description: Audio clock > > - description: Non-live video clock (from Processing System) > > - description: Live video clock (from Programmable Logic) > > clock-names: > > minItems: 2 > > maxItems: 4 > > items: > > - const: dp_apb_clk > > - const: dp_aud_clk > > - const: dp_vtc_pixel_clk_in > > - const: dp_live_video_in_clk > > > > dp_apb_clk is required, dp_aud_clk is optional, and at least one of > > dp_vtc_pixel_clk_in and dp_live_video_in_clk is required. > > I'm hoping people's inability to express the schema will prevent > complicated ones like this in the first place... > > clock-names: > oneOf: > - minItems: 3 > maxItems: 4 > items: > - const: dp_apb_clk > - const: dp_aud_clk > - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] > - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] > - minItems: 2 > maxItems: 3 > items: > - const: dp_apb_clk > - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] > - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] The above would make clock-names = "dp_apb_clk", "dp_vtc_pixel_clk_in", "dp_vtc_pixel_clk_in"; valid. I've investigated a little bit and found uniqueItems which solves my issue. Would the following simpler solution be acceptable ? clock-names: minItems: 2 maxItems: 4 items: - const: dp_apb_clk - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] - const: dp_aud_clk - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] uniqueItems: true > Strictly speaking, that leaves items clocks wrong, but 'description' > doesn't do anything. So I'd just leave it as is. Speaking of which, there doesn't seem to be anything that validates the size of clocks and clock-names being identical. Is that a known issue ?
Hi Rob, On Fri, Nov 08, 2019 at 04:07:33PM +0200, Laurent Pinchart wrote: > On Thu, Sep 26, 2019 at 09:57:29AM -0500, Rob Herring wrote: > > On Thu, Sep 26, 2019 at 9:23 AM Laurent Pinchart wrote: > >> On Thu, Sep 26, 2019 at 09:15:01AM -0500, Rob Herring wrote: > >>> On Wed, Sep 25, 2019 at 6:56 PM Laurent Pinchart wrote: > >>>> > >>>> From: Hyun Kwon <hyun.kwon@xilinx.com> > >>>> > >>>> The bindings describe the ZynqMP DP subsystem. They don't support the > >>>> interface with the programmable logic (FPGA) or audio yet. > >>>> > >>>> Signed-off-by: Hyun Kwon <hyun.kwon@xilinx.com> > >>>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > >>>> --- > >>>> Changes since v8: > >>>> > >>>> - Convert to yaml > >>>> - Rename aclk to dp_apb_clk > >>> > >>> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/display/xlnx/xlnx,zynqmp-dpsub.example.dt.yaml: > >>> display@fd4a0000: clock-names:2: 'dp_vtc_pixel_clk_in' was expected > >> > >> If you allow me to steal a bit of your brain time, could you help me > >> expressing the clocks constraint ? > >> > >> clocks: > >> description: > >> The AXI clock and at least one video clock are mandatory, the audio clock > >> optional. > >> minItems: 2 > >> maxItems: 4 > >> items: > >> - description: AXI clock > >> - description: Audio clock > >> - description: Non-live video clock (from Processing System) > >> - description: Live video clock (from Programmable Logic) > >> clock-names: > >> minItems: 2 > >> maxItems: 4 > >> items: > >> - const: dp_apb_clk > >> - const: dp_aud_clk > >> - const: dp_vtc_pixel_clk_in > >> - const: dp_live_video_in_clk > >> > >> dp_apb_clk is required, dp_aud_clk is optional, and at least one of > >> dp_vtc_pixel_clk_in and dp_live_video_in_clk is required. > > > > I'm hoping people's inability to express the schema will prevent > > complicated ones like this in the first place... > > > > clock-names: > > oneOf: > > - minItems: 3 > > maxItems: 4 > > items: > > - const: dp_apb_clk > > - const: dp_aud_clk > > - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] > > - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] > > - minItems: 2 > > maxItems: 3 > > items: > > - const: dp_apb_clk > > - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] > > - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] > > The above would make > > clock-names = "dp_apb_clk", "dp_vtc_pixel_clk_in", "dp_vtc_pixel_clk_in"; > > valid. I've investigated a little bit and found uniqueItems which solves > my issue. > > Would the following simpler solution be acceptable ? > > clock-names: > minItems: 2 > maxItems: 4 > items: > - const: dp_apb_clk > - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] > - const: dp_aud_clk > - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] > uniqueItems: true To give more context, clocks: description: The AXI clock and at least one video clock are mandatory, the audio clock is optional. minItems: 2 maxItems: 4 items: - description: dp_apb_clk is the AXI clock - description: dp_aud_clk is the Audio clock - description: dp_vtc_pixel_clk_in is the non-live video clock (from Processing System) - description: dp_live_video_in_clk is the live video clock (from Programmable Logic) clock-names: minItems: 2 maxItems: 4 items: - const: dp_apb_clk - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] - const: dp_aud_clk - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] uniqueItems: true > > Strictly speaking, that leaves items clocks wrong, but 'description' > > doesn't do anything. So I'd just leave it as is. > > Speaking of which, there doesn't seem to be anything that validates the > size of clocks and clock-names being identical. Is that a known issue ?
On Fri, Nov 08, 2019 at 04:10:40PM +0200, Laurent Pinchart wrote: > On Fri, Nov 08, 2019 at 04:07:33PM +0200, Laurent Pinchart wrote: > > On Thu, Sep 26, 2019 at 09:57:29AM -0500, Rob Herring wrote: > > > On Thu, Sep 26, 2019 at 9:23 AM Laurent Pinchart wrote: > > >> On Thu, Sep 26, 2019 at 09:15:01AM -0500, Rob Herring wrote: > > >>> On Wed, Sep 25, 2019 at 6:56 PM Laurent Pinchart wrote: > > >>>> > > >>>> From: Hyun Kwon <hyun.kwon@xilinx.com> > > >>>> > > >>>> The bindings describe the ZynqMP DP subsystem. They don't support the > > >>>> interface with the programmable logic (FPGA) or audio yet. > > >>>> > > >>>> Signed-off-by: Hyun Kwon <hyun.kwon@xilinx.com> > > >>>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > >>>> --- > > >>>> Changes since v8: > > >>>> > > >>>> - Convert to yaml > > >>>> - Rename aclk to dp_apb_clk > > >>> > > >>> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/display/xlnx/xlnx,zynqmp-dpsub.example.dt.yaml: > > >>> display@fd4a0000: clock-names:2: 'dp_vtc_pixel_clk_in' was expected > > >> > > >> If you allow me to steal a bit of your brain time, could you help me > > >> expressing the clocks constraint ? > > >> > > >> clocks: > > >> description: > > >> The AXI clock and at least one video clock are mandatory, the audio clock > > >> optional. > > >> minItems: 2 > > >> maxItems: 4 > > >> items: > > >> - description: AXI clock > > >> - description: Audio clock > > >> - description: Non-live video clock (from Processing System) > > >> - description: Live video clock (from Programmable Logic) > > >> clock-names: > > >> minItems: 2 > > >> maxItems: 4 > > >> items: > > >> - const: dp_apb_clk > > >> - const: dp_aud_clk > > >> - const: dp_vtc_pixel_clk_in > > >> - const: dp_live_video_in_clk > > >> > > >> dp_apb_clk is required, dp_aud_clk is optional, and at least one of > > >> dp_vtc_pixel_clk_in and dp_live_video_in_clk is required. > > > > > > I'm hoping people's inability to express the schema will prevent > > > complicated ones like this in the first place... > > > > > > clock-names: > > > oneOf: > > > - minItems: 3 > > > maxItems: 4 > > > items: > > > - const: dp_apb_clk > > > - const: dp_aud_clk > > > - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] > > > - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] > > > - minItems: 2 > > > maxItems: 3 > > > items: > > > - const: dp_apb_clk > > > - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] > > > - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] > > > > The above would make > > > > clock-names = "dp_apb_clk", "dp_vtc_pixel_clk_in", "dp_vtc_pixel_clk_in"; > > > > valid. I've investigated a little bit and found uniqueItems which solves > > my issue. > > > > Would the following simpler solution be acceptable ? > > > > clock-names: > > minItems: 2 > > maxItems: 4 > > items: > > - const: dp_apb_clk > > - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] > > - const: dp_aud_clk > > - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] > > uniqueItems: true > > To give more context, > > clocks: > description: > The AXI clock and at least one video clock are mandatory, the audio clock > is optional. > minItems: 2 > maxItems: 4 > items: > - description: dp_apb_clk is the AXI clock > - description: dp_aud_clk is the Audio clock > - description: > dp_vtc_pixel_clk_in is the non-live video clock (from Processing > System) > - description: > dp_live_video_in_clk is the live video clock (from Programmable > Logic) > clock-names: > minItems: 2 > maxItems: 4 > items: > - const: dp_apb_clk > - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] > - const: dp_aud_clk > - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] > uniqueItems: true There's something going on that I can't really understand... clock-names: minItems: 2 maxItems: 4 items: - const: dp_apb_clk - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] - const: dp_aud_clk - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] uniqueItems: true results in dt_mk_schema complaining about an invalid schema. However, the following works: clock-names: oneOf: - minItems: 2 maxItems: 4 items: - const: dp_apb_clk - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] - const: dp_aud_clk - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] uniqueItems: true I assume this is due to clock-names being a string-array, which already contains uniqueItems. However, if I leave uniqueItems out, an example with a duplicated clock-names validates fine. > > > Strictly speaking, that leaves items clocks wrong, but 'description' > > > doesn't do anything. So I'd just leave it as is. > > > > Speaking of which, there doesn't seem to be anything that validates the > > size of clocks and clock-names being identical. Is that a known issue ?
On Fri, Nov 8, 2019 at 8:32 AM Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > > On Fri, Nov 08, 2019 at 04:10:40PM +0200, Laurent Pinchart wrote: > > On Fri, Nov 08, 2019 at 04:07:33PM +0200, Laurent Pinchart wrote: > > > On Thu, Sep 26, 2019 at 09:57:29AM -0500, Rob Herring wrote: > > > > On Thu, Sep 26, 2019 at 9:23 AM Laurent Pinchart wrote: > > > >> On Thu, Sep 26, 2019 at 09:15:01AM -0500, Rob Herring wrote: > > > >>> On Wed, Sep 25, 2019 at 6:56 PM Laurent Pinchart wrote: > > > >>>> > > > >>>> From: Hyun Kwon <hyun.kwon@xilinx.com> > > > >>>> > > > >>>> The bindings describe the ZynqMP DP subsystem. They don't support the > > > >>>> interface with the programmable logic (FPGA) or audio yet. > > > >>>> > > > >>>> Signed-off-by: Hyun Kwon <hyun.kwon@xilinx.com> > > > >>>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > >>>> --- > > > >>>> Changes since v8: > > > >>>> > > > >>>> - Convert to yaml > > > >>>> - Rename aclk to dp_apb_clk > > > >>> > > > >>> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/display/xlnx/xlnx,zynqmp-dpsub.example.dt.yaml: > > > >>> display@fd4a0000: clock-names:2: 'dp_vtc_pixel_clk_in' was expected > > > >> > > > >> If you allow me to steal a bit of your brain time, could you help me > > > >> expressing the clocks constraint ? > > > >> > > > >> clocks: > > > >> description: > > > >> The AXI clock and at least one video clock are mandatory, the audio clock > > > >> optional. > > > >> minItems: 2 > > > >> maxItems: 4 > > > >> items: > > > >> - description: AXI clock > > > >> - description: Audio clock > > > >> - description: Non-live video clock (from Processing System) > > > >> - description: Live video clock (from Programmable Logic) > > > >> clock-names: > > > >> minItems: 2 > > > >> maxItems: 4 > > > >> items: > > > >> - const: dp_apb_clk > > > >> - const: dp_aud_clk > > > >> - const: dp_vtc_pixel_clk_in > > > >> - const: dp_live_video_in_clk > > > >> > > > >> dp_apb_clk is required, dp_aud_clk is optional, and at least one of > > > >> dp_vtc_pixel_clk_in and dp_live_video_in_clk is required. > > > > > > > > I'm hoping people's inability to express the schema will prevent > > > > complicated ones like this in the first place... > > > > > > > > clock-names: > > > > oneOf: > > > > - minItems: 3 > > > > maxItems: 4 > > > > items: > > > > - const: dp_apb_clk > > > > - const: dp_aud_clk > > > > - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] > > > > - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] > > > > - minItems: 2 > > > > maxItems: 3 > > > > items: > > > > - const: dp_apb_clk > > > > - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] > > > > - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] > > > > > > The above would make > > > > > > clock-names = "dp_apb_clk", "dp_vtc_pixel_clk_in", "dp_vtc_pixel_clk_in"; > > > > > > valid. I've investigated a little bit and found uniqueItems which solves > > > my issue. It wouldn't because uniqueItems is the default (not for json-schema, but DT schema string-arrays). > > > > > > Would the following simpler solution be acceptable ? > > > > > > clock-names: > > > minItems: 2 > > > maxItems: 4 > > > items: > > > - const: dp_apb_clk > > > - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] > > > - const: dp_aud_clk > > > - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] > > > uniqueItems: true > > > > To give more context, > > > > clocks: > > description: > > The AXI clock and at least one video clock are mandatory, the audio clock > > is optional. > > minItems: 2 > > maxItems: 4 > > items: > > - description: dp_apb_clk is the AXI clock > > - description: dp_aud_clk is the Audio clock > > - description: > > dp_vtc_pixel_clk_in is the non-live video clock (from Processing > > System) > > - description: > > dp_live_video_in_clk is the live video clock (from Programmable > > Logic) > > clock-names: > > minItems: 2 > > maxItems: 4 > > items: > > - const: dp_apb_clk > > - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] > > - const: dp_aud_clk > > - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] > > uniqueItems: true 'clock' and 'clock-names' don't match. dp_aud_clk is in the wrong spot. > > There's something going on that I can't really understand... > > clock-names: > minItems: 2 > maxItems: 4 > items: > - const: dp_apb_clk > - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] > - const: dp_aud_clk > - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] > uniqueItems: true > > results in dt_mk_schema complaining about an invalid schema. However, > the following works: Because 'uniqueItems' is not allowed because it's not needed. > > clock-names: > oneOf: > - minItems: 2 > maxItems: 4 > items: > - const: dp_apb_clk > - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] > - const: dp_aud_clk > - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] > uniqueItems: true > > I assume this is due to clock-names being a string-array, which already > contains uniqueItems. However, if I leave uniqueItems out, an example > with a duplicated clock-names validates fine. Are you using 'DT_SCHEMA_FILES' as that leaves out all the core schema? BTW, this case is probably allowed because we fail to apply the same meta-schemas for schema behind the oneOf. Rob
Hi Rob, On Fri, Nov 08, 2019 at 09:57:55AM -0600, Rob Herring wrote: > On Fri, Nov 8, 2019 at 8:32 AM Laurent Pinchart wrote: > > On Fri, Nov 08, 2019 at 04:10:40PM +0200, Laurent Pinchart wrote: > > > On Fri, Nov 08, 2019 at 04:07:33PM +0200, Laurent Pinchart wrote: > > > > On Thu, Sep 26, 2019 at 09:57:29AM -0500, Rob Herring wrote: > > > > > On Thu, Sep 26, 2019 at 9:23 AM Laurent Pinchart wrote: > > > > >> On Thu, Sep 26, 2019 at 09:15:01AM -0500, Rob Herring wrote: > > > > >>> On Wed, Sep 25, 2019 at 6:56 PM Laurent Pinchart wrote: > > > > >>>> > > > > >>>> From: Hyun Kwon <hyun.kwon@xilinx.com> > > > > >>>> > > > > >>>> The bindings describe the ZynqMP DP subsystem. They don't support the > > > > >>>> interface with the programmable logic (FPGA) or audio yet. > > > > >>>> > > > > >>>> Signed-off-by: Hyun Kwon <hyun.kwon@xilinx.com> > > > > >>>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > > >>>> --- > > > > >>>> Changes since v8: > > > > >>>> > > > > >>>> - Convert to yaml > > > > >>>> - Rename aclk to dp_apb_clk > > > > >>> > > > > >>> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/display/xlnx/xlnx,zynqmp-dpsub.example.dt.yaml: > > > > >>> display@fd4a0000: clock-names:2: 'dp_vtc_pixel_clk_in' was expected > > > > >> > > > > >> If you allow me to steal a bit of your brain time, could you help me > > > > >> expressing the clocks constraint ? > > > > >> > > > > >> clocks: > > > > >> description: > > > > >> The AXI clock and at least one video clock are mandatory, the audio clock > > > > >> optional. > > > > >> minItems: 2 > > > > >> maxItems: 4 > > > > >> items: > > > > >> - description: AXI clock > > > > >> - description: Audio clock > > > > >> - description: Non-live video clock (from Processing System) > > > > >> - description: Live video clock (from Programmable Logic) > > > > >> clock-names: > > > > >> minItems: 2 > > > > >> maxItems: 4 > > > > >> items: > > > > >> - const: dp_apb_clk > > > > >> - const: dp_aud_clk > > > > >> - const: dp_vtc_pixel_clk_in > > > > >> - const: dp_live_video_in_clk > > > > >> > > > > >> dp_apb_clk is required, dp_aud_clk is optional, and at least one of > > > > >> dp_vtc_pixel_clk_in and dp_live_video_in_clk is required. > > > > > > > > > > I'm hoping people's inability to express the schema will prevent > > > > > complicated ones like this in the first place... > > > > > > > > > > clock-names: > > > > > oneOf: > > > > > - minItems: 3 > > > > > maxItems: 4 > > > > > items: > > > > > - const: dp_apb_clk > > > > > - const: dp_aud_clk > > > > > - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] > > > > > - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] > > > > > - minItems: 2 > > > > > maxItems: 3 > > > > > items: > > > > > - const: dp_apb_clk > > > > > - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] > > > > > - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] > > > > > > > > The above would make > > > > > > > > clock-names = "dp_apb_clk", "dp_vtc_pixel_clk_in", "dp_vtc_pixel_clk_in"; > > > > > > > > valid. I've investigated a little bit and found uniqueItems which solves > > > > my issue. > > It wouldn't because uniqueItems is the default (not for json-schema, > but DT schema string-arrays). > > > > > Would the following simpler solution be acceptable ? > > > > > > > > clock-names: > > > > minItems: 2 > > > > maxItems: 4 > > > > items: > > > > - const: dp_apb_clk > > > > - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] > > > > - const: dp_aud_clk > > > > - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] > > > > uniqueItems: true > > > > > > To give more context, > > > > > > clocks: > > > description: > > > The AXI clock and at least one video clock are mandatory, the audio clock > > > is optional. > > > minItems: 2 > > > maxItems: 4 > > > items: > > > - description: dp_apb_clk is the AXI clock > > > - description: dp_aud_clk is the Audio clock > > > - description: > > > dp_vtc_pixel_clk_in is the non-live video clock (from Processing > > > System) > > > - description: > > > dp_live_video_in_clk is the live video clock (from Programmable > > > Logic) > > > clock-names: > > > minItems: 2 > > > maxItems: 4 > > > items: > > > - const: dp_apb_clk > > > - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] > > > - const: dp_aud_clk > > > - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] > > > uniqueItems: true > > 'clock' and 'clock-names' don't match. dp_aud_clk is in the wrong spot. You're right, and after thinking twice about it, this won't allow clock-names = "dp_apb_clk", "dp_vtc_pixel_clk_in", "dp_live_video_in_clk"; so your solution is the only one that will work. > > > > There's something going on that I can't really understand... > > > > clock-names: > > minItems: 2 > > maxItems: 4 > > items: > > - const: dp_apb_clk > > - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] > > - const: dp_aud_clk > > - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] > > uniqueItems: true > > > > results in dt_mk_schema complaining about an invalid schema. However, > > the following works: > > Because 'uniqueItems' is not allowed because it's not needed. Error messages could be improved :-) > > clock-names: > > oneOf: > > - minItems: 2 > > maxItems: 4 > > items: > > - const: dp_apb_clk > > - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] > > - const: dp_aud_clk > > - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] > > uniqueItems: true > > > > I assume this is due to clock-names being a string-array, which already > > contains uniqueItems. However, if I leave uniqueItems out, an example > > with a duplicated clock-names validates fine. > > Are you using 'DT_SCHEMA_FILES' as that leaves out all the core schema? Ah, that's probably why ! Thank you. > BTW, this case is probably allowed because we fail to apply the same > meta-schemas for schema behind the oneOf.
Hi Rob, On Fri, Nov 08, 2019 at 06:01:20PM +0200, Laurent Pinchart wrote: > On Fri, Nov 08, 2019 at 09:57:55AM -0600, Rob Herring wrote: > > On Fri, Nov 8, 2019 at 8:32 AM Laurent Pinchart wrote: > >> On Fri, Nov 08, 2019 at 04:10:40PM +0200, Laurent Pinchart wrote: > >>> On Fri, Nov 08, 2019 at 04:07:33PM +0200, Laurent Pinchart wrote: > >>>> On Thu, Sep 26, 2019 at 09:57:29AM -0500, Rob Herring wrote: > >>>>> On Thu, Sep 26, 2019 at 9:23 AM Laurent Pinchart wrote: > >>>>>> On Thu, Sep 26, 2019 at 09:15:01AM -0500, Rob Herring wrote: > >>>>>>> On Wed, Sep 25, 2019 at 6:56 PM Laurent Pinchart wrote: > >>>>>>>> > >>>>>>>> From: Hyun Kwon <hyun.kwon@xilinx.com> > >>>>>>>> > >>>>>>>> The bindings describe the ZynqMP DP subsystem. They don't support the > >>>>>>>> interface with the programmable logic (FPGA) or audio yet. > >>>>>>>> > >>>>>>>> Signed-off-by: Hyun Kwon <hyun.kwon@xilinx.com> > >>>>>>>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > >>>>>>>> --- > >>>>>>>> Changes since v8: > >>>>>>>> > >>>>>>>> - Convert to yaml > >>>>>>>> - Rename aclk to dp_apb_clk > >>>>>>> > >>>>>>> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/display/xlnx/xlnx,zynqmp-dpsub.example.dt.yaml: > >>>>>>> display@fd4a0000: clock-names:2: 'dp_vtc_pixel_clk_in' was expected > >>>>>> > >>>>>> If you allow me to steal a bit of your brain time, could you help me > >>>>>> expressing the clocks constraint ? > >>>>>> > >>>>>> clocks: > >>>>>> description: > >>>>>> The AXI clock and at least one video clock are mandatory, the audio clock > >>>>>> optional. > >>>>>> minItems: 2 > >>>>>> maxItems: 4 > >>>>>> items: > >>>>>> - description: AXI clock > >>>>>> - description: Audio clock > >>>>>> - description: Non-live video clock (from Processing System) > >>>>>> - description: Live video clock (from Programmable Logic) > >>>>>> clock-names: > >>>>>> minItems: 2 > >>>>>> maxItems: 4 > >>>>>> items: > >>>>>> - const: dp_apb_clk > >>>>>> - const: dp_aud_clk > >>>>>> - const: dp_vtc_pixel_clk_in > >>>>>> - const: dp_live_video_in_clk > >>>>>> > >>>>>> dp_apb_clk is required, dp_aud_clk is optional, and at least one of > >>>>>> dp_vtc_pixel_clk_in and dp_live_video_in_clk is required. > >>>>> > >>>>> I'm hoping people's inability to express the schema will prevent > >>>>> complicated ones like this in the first place... > >>>>> > >>>>> clock-names: > >>>>> oneOf: > >>>>> - minItems: 3 > >>>>> maxItems: 4 > >>>>> items: > >>>>> - const: dp_apb_clk > >>>>> - const: dp_aud_clk > >>>>> - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] > >>>>> - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] > >>>>> - minItems: 2 > >>>>> maxItems: 3 > >>>>> items: > >>>>> - const: dp_apb_clk > >>>>> - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] > >>>>> - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] > >>>> > >>>> The above would make > >>>> > >>>> clock-names = "dp_apb_clk", "dp_vtc_pixel_clk_in", "dp_vtc_pixel_clk_in"; > >>>> > >>>> valid. I've investigated a little bit and found uniqueItems which solves > >>>> my issue. > > > > It wouldn't because uniqueItems is the default (not for json-schema, > > but DT schema string-arrays). > > > >>>> Would the following simpler solution be acceptable ? > >>>> > >>>> clock-names: > >>>> minItems: 2 > >>>> maxItems: 4 > >>>> items: > >>>> - const: dp_apb_clk > >>>> - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] > >>>> - const: dp_aud_clk > >>>> - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] > >>>> uniqueItems: true > >>> > >>> To give more context, > >>> > >>> clocks: > >>> description: > >>> The AXI clock and at least one video clock are mandatory, the audio clock > >>> is optional. > >>> minItems: 2 > >>> maxItems: 4 > >>> items: > >>> - description: dp_apb_clk is the AXI clock > >>> - description: dp_aud_clk is the Audio clock > >>> - description: > >>> dp_vtc_pixel_clk_in is the non-live video clock (from Processing > >>> System) > >>> - description: > >>> dp_live_video_in_clk is the live video clock (from Programmable > >>> Logic) > >>> clock-names: > >>> minItems: 2 > >>> maxItems: 4 > >>> items: > >>> - const: dp_apb_clk > >>> - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] > >>> - const: dp_aud_clk > >>> - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] > >>> uniqueItems: true > > > > 'clock' and 'clock-names' don't match. dp_aud_clk is in the wrong spot. > > You're right, and after thinking twice about it, this won't allow > > clock-names = "dp_apb_clk", "dp_vtc_pixel_clk_in", "dp_live_video_in_clk"; > > so your solution is the only one that will work. > > >> > >> There's something going on that I can't really understand... > >> > >> clock-names: > >> minItems: 2 > >> maxItems: 4 > >> items: > >> - const: dp_apb_clk > >> - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] > >> - const: dp_aud_clk > >> - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] > >> uniqueItems: true > >> > >> results in dt_mk_schema complaining about an invalid schema. However, > >> the following works: > > > > Because 'uniqueItems' is not allowed because it's not needed. > > Error messages could be improved :-) > > >> clock-names: > >> oneOf: > >> - minItems: 2 > >> maxItems: 4 > >> items: > >> - const: dp_apb_clk > >> - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] > >> - const: dp_aud_clk > >> - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] > >> uniqueItems: true > >> > >> I assume this is due to clock-names being a string-array, which already > >> contains uniqueItems. However, if I leave uniqueItems out, an example > >> with a duplicated clock-names validates fine. > > > > Are you using 'DT_SCHEMA_FILES' as that leaves out all the core schema? > > Ah, that's probably why ! Thank you. I've now run dt_binding_check without DT_SCHEMA_FILES, and without a uniqueItems in each of the oneOf options, duplicate values are allowed. Is this expected ? > > BTW, this case is probably allowed because we fail to apply the same > > meta-schemas for schema behind the oneOf.
On Fri, Nov 08, 2019 at 06:12:26PM +0200, Laurent Pinchart wrote: > On Fri, Nov 08, 2019 at 06:01:20PM +0200, Laurent Pinchart wrote: > > On Fri, Nov 08, 2019 at 09:57:55AM -0600, Rob Herring wrote: > >> On Fri, Nov 8, 2019 at 8:32 AM Laurent Pinchart wrote: > >>> On Fri, Nov 08, 2019 at 04:10:40PM +0200, Laurent Pinchart wrote: > >>>> On Fri, Nov 08, 2019 at 04:07:33PM +0200, Laurent Pinchart wrote: > >>>>> On Thu, Sep 26, 2019 at 09:57:29AM -0500, Rob Herring wrote: > >>>>>> On Thu, Sep 26, 2019 at 9:23 AM Laurent Pinchart wrote: > >>>>>>> On Thu, Sep 26, 2019 at 09:15:01AM -0500, Rob Herring wrote: > >>>>>>>> On Wed, Sep 25, 2019 at 6:56 PM Laurent Pinchart wrote: > >>>>>>>>> > >>>>>>>>> From: Hyun Kwon <hyun.kwon@xilinx.com> > >>>>>>>>> > >>>>>>>>> The bindings describe the ZynqMP DP subsystem. They don't support the > >>>>>>>>> interface with the programmable logic (FPGA) or audio yet. > >>>>>>>>> > >>>>>>>>> Signed-off-by: Hyun Kwon <hyun.kwon@xilinx.com> > >>>>>>>>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > >>>>>>>>> --- > >>>>>>>>> Changes since v8: > >>>>>>>>> > >>>>>>>>> - Convert to yaml > >>>>>>>>> - Rename aclk to dp_apb_clk > >>>>>>>> > >>>>>>>> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/display/xlnx/xlnx,zynqmp-dpsub.example.dt.yaml: > >>>>>>>> display@fd4a0000: clock-names:2: 'dp_vtc_pixel_clk_in' was expected > >>>>>>> > >>>>>>> If you allow me to steal a bit of your brain time, could you help me > >>>>>>> expressing the clocks constraint ? > >>>>>>> > >>>>>>> clocks: > >>>>>>> description: > >>>>>>> The AXI clock and at least one video clock are mandatory, the audio clock > >>>>>>> optional. > >>>>>>> minItems: 2 > >>>>>>> maxItems: 4 > >>>>>>> items: > >>>>>>> - description: AXI clock > >>>>>>> - description: Audio clock > >>>>>>> - description: Non-live video clock (from Processing System) > >>>>>>> - description: Live video clock (from Programmable Logic) > >>>>>>> clock-names: > >>>>>>> minItems: 2 > >>>>>>> maxItems: 4 > >>>>>>> items: > >>>>>>> - const: dp_apb_clk > >>>>>>> - const: dp_aud_clk > >>>>>>> - const: dp_vtc_pixel_clk_in > >>>>>>> - const: dp_live_video_in_clk > >>>>>>> > >>>>>>> dp_apb_clk is required, dp_aud_clk is optional, and at least one of > >>>>>>> dp_vtc_pixel_clk_in and dp_live_video_in_clk is required. > >>>>>> > >>>>>> I'm hoping people's inability to express the schema will prevent > >>>>>> complicated ones like this in the first place... > >>>>>> > >>>>>> clock-names: > >>>>>> oneOf: > >>>>>> - minItems: 3 > >>>>>> maxItems: 4 > >>>>>> items: > >>>>>> - const: dp_apb_clk > >>>>>> - const: dp_aud_clk > >>>>>> - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] > >>>>>> - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] > >>>>>> - minItems: 2 > >>>>>> maxItems: 3 > >>>>>> items: > >>>>>> - const: dp_apb_clk > >>>>>> - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] > >>>>>> - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] > >>>>> > >>>>> The above would make > >>>>> > >>>>> clock-names = "dp_apb_clk", "dp_vtc_pixel_clk_in", "dp_vtc_pixel_clk_in"; > >>>>> > >>>>> valid. I've investigated a little bit and found uniqueItems which solves > >>>>> my issue. > >> > >> It wouldn't because uniqueItems is the default (not for json-schema, > >> but DT schema string-arrays). > >> > >>>>> Would the following simpler solution be acceptable ? > >>>>> > >>>>> clock-names: > >>>>> minItems: 2 > >>>>> maxItems: 4 > >>>>> items: > >>>>> - const: dp_apb_clk > >>>>> - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] > >>>>> - const: dp_aud_clk > >>>>> - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] > >>>>> uniqueItems: true > >>>> > >>>> To give more context, > >>>> > >>>> clocks: > >>>> description: > >>>> The AXI clock and at least one video clock are mandatory, the audio clock > >>>> is optional. > >>>> minItems: 2 > >>>> maxItems: 4 > >>>> items: > >>>> - description: dp_apb_clk is the AXI clock > >>>> - description: dp_aud_clk is the Audio clock > >>>> - description: > >>>> dp_vtc_pixel_clk_in is the non-live video clock (from Processing > >>>> System) > >>>> - description: > >>>> dp_live_video_in_clk is the live video clock (from Programmable > >>>> Logic) > >>>> clock-names: > >>>> minItems: 2 > >>>> maxItems: 4 > >>>> items: > >>>> - const: dp_apb_clk > >>>> - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] > >>>> - const: dp_aud_clk > >>>> - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] > >>>> uniqueItems: true > >> > >> 'clock' and 'clock-names' don't match. dp_aud_clk is in the wrong spot. > > > > You're right, and after thinking twice about it, this won't allow > > > > clock-names = "dp_apb_clk", "dp_vtc_pixel_clk_in", "dp_live_video_in_clk"; > > > > so your solution is the only one that will work. > > > >>> > >>> There's something going on that I can't really understand... > >>> > >>> clock-names: > >>> minItems: 2 > >>> maxItems: 4 > >>> items: > >>> - const: dp_apb_clk > >>> - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] > >>> - const: dp_aud_clk > >>> - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] > >>> uniqueItems: true > >>> > >>> results in dt_mk_schema complaining about an invalid schema. However, > >>> the following works: > >> > >> Because 'uniqueItems' is not allowed because it's not needed. > > > > Error messages could be improved :-) > > > >>> clock-names: > >>> oneOf: > >>> - minItems: 2 > >>> maxItems: 4 > >>> items: > >>> - const: dp_apb_clk > >>> - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] > >>> - const: dp_aud_clk > >>> - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] > >>> uniqueItems: true > >>> > >>> I assume this is due to clock-names being a string-array, which already > >>> contains uniqueItems. However, if I leave uniqueItems out, an example > >>> with a duplicated clock-names validates fine. > >> > >> Are you using 'DT_SCHEMA_FILES' as that leaves out all the core schema? > > > > Ah, that's probably why ! Thank you. > > I've now run dt_binding_check without DT_SCHEMA_FILES, and without a > uniqueItems in each of the oneOf options, duplicate values are allowed. > Is this expected ? Sorry, please scratch this, I made a mistake. Things now work as expected. Thank you for your help ! > >> BTW, this case is probably allowed because we fail to apply the same > >> meta-schemas for schema behind the oneOf.
On Fri, Nov 8, 2019 at 10:15 AM Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > > On Fri, Nov 08, 2019 at 06:12:26PM +0200, Laurent Pinchart wrote: > > On Fri, Nov 08, 2019 at 06:01:20PM +0200, Laurent Pinchart wrote: > > > On Fri, Nov 08, 2019 at 09:57:55AM -0600, Rob Herring wrote: > > >> On Fri, Nov 8, 2019 at 8:32 AM Laurent Pinchart wrote: > > >>> On Fri, Nov 08, 2019 at 04:10:40PM +0200, Laurent Pinchart wrote: > > >>>> On Fri, Nov 08, 2019 at 04:07:33PM +0200, Laurent Pinchart wrote: > > >>>>> On Thu, Sep 26, 2019 at 09:57:29AM -0500, Rob Herring wrote: > > >>>>>> On Thu, Sep 26, 2019 at 9:23 AM Laurent Pinchart wrote: > > >>>>>>> On Thu, Sep 26, 2019 at 09:15:01AM -0500, Rob Herring wrote: > > >>>>>>>> On Wed, Sep 25, 2019 at 6:56 PM Laurent Pinchart wrote: > > >>>>>>>>> > > >>>>>>>>> From: Hyun Kwon <hyun.kwon@xilinx.com> > > >>>>>>>>> > > >>>>>>>>> The bindings describe the ZynqMP DP subsystem. They don't support the > > >>>>>>>>> interface with the programmable logic (FPGA) or audio yet. > > >>>>>>>>> > > >>>>>>>>> Signed-off-by: Hyun Kwon <hyun.kwon@xilinx.com> > > >>>>>>>>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > >>>>>>>>> --- > > >>>>>>>>> Changes since v8: > > >>>>>>>>> > > >>>>>>>>> - Convert to yaml > > >>>>>>>>> - Rename aclk to dp_apb_clk > > >>>>>>>> > > >>>>>>>> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/display/xlnx/xlnx,zynqmp-dpsub.example.dt.yaml: > > >>>>>>>> display@fd4a0000: clock-names:2: 'dp_vtc_pixel_clk_in' was expected > > >>>>>>> > > >>>>>>> If you allow me to steal a bit of your brain time, could you help me > > >>>>>>> expressing the clocks constraint ? > > >>>>>>> > > >>>>>>> clocks: > > >>>>>>> description: > > >>>>>>> The AXI clock and at least one video clock are mandatory, the audio clock > > >>>>>>> optional. > > >>>>>>> minItems: 2 > > >>>>>>> maxItems: 4 > > >>>>>>> items: > > >>>>>>> - description: AXI clock > > >>>>>>> - description: Audio clock > > >>>>>>> - description: Non-live video clock (from Processing System) > > >>>>>>> - description: Live video clock (from Programmable Logic) > > >>>>>>> clock-names: > > >>>>>>> minItems: 2 > > >>>>>>> maxItems: 4 > > >>>>>>> items: > > >>>>>>> - const: dp_apb_clk > > >>>>>>> - const: dp_aud_clk > > >>>>>>> - const: dp_vtc_pixel_clk_in > > >>>>>>> - const: dp_live_video_in_clk > > >>>>>>> > > >>>>>>> dp_apb_clk is required, dp_aud_clk is optional, and at least one of > > >>>>>>> dp_vtc_pixel_clk_in and dp_live_video_in_clk is required. > > >>>>>> > > >>>>>> I'm hoping people's inability to express the schema will prevent > > >>>>>> complicated ones like this in the first place... > > >>>>>> > > >>>>>> clock-names: > > >>>>>> oneOf: > > >>>>>> - minItems: 3 > > >>>>>> maxItems: 4 > > >>>>>> items: > > >>>>>> - const: dp_apb_clk > > >>>>>> - const: dp_aud_clk > > >>>>>> - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] > > >>>>>> - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] > > >>>>>> - minItems: 2 > > >>>>>> maxItems: 3 > > >>>>>> items: > > >>>>>> - const: dp_apb_clk > > >>>>>> - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] > > >>>>>> - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] > > >>>>> > > >>>>> The above would make > > >>>>> > > >>>>> clock-names = "dp_apb_clk", "dp_vtc_pixel_clk_in", "dp_vtc_pixel_clk_in"; > > >>>>> > > >>>>> valid. I've investigated a little bit and found uniqueItems which solves > > >>>>> my issue. > > >> > > >> It wouldn't because uniqueItems is the default (not for json-schema, > > >> but DT schema string-arrays). > > >> > > >>>>> Would the following simpler solution be acceptable ? > > >>>>> > > >>>>> clock-names: > > >>>>> minItems: 2 > > >>>>> maxItems: 4 > > >>>>> items: > > >>>>> - const: dp_apb_clk > > >>>>> - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] > > >>>>> - const: dp_aud_clk > > >>>>> - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] > > >>>>> uniqueItems: true > > >>>> > > >>>> To give more context, > > >>>> > > >>>> clocks: > > >>>> description: > > >>>> The AXI clock and at least one video clock are mandatory, the audio clock > > >>>> is optional. > > >>>> minItems: 2 > > >>>> maxItems: 4 > > >>>> items: > > >>>> - description: dp_apb_clk is the AXI clock > > >>>> - description: dp_aud_clk is the Audio clock > > >>>> - description: > > >>>> dp_vtc_pixel_clk_in is the non-live video clock (from Processing > > >>>> System) > > >>>> - description: > > >>>> dp_live_video_in_clk is the live video clock (from Programmable > > >>>> Logic) > > >>>> clock-names: > > >>>> minItems: 2 > > >>>> maxItems: 4 > > >>>> items: > > >>>> - const: dp_apb_clk > > >>>> - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] > > >>>> - const: dp_aud_clk > > >>>> - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] > > >>>> uniqueItems: true > > >> > > >> 'clock' and 'clock-names' don't match. dp_aud_clk is in the wrong spot. > > > > > > You're right, and after thinking twice about it, this won't allow > > > > > > clock-names = "dp_apb_clk", "dp_vtc_pixel_clk_in", "dp_live_video_in_clk"; > > > > > > so your solution is the only one that will work. > > > > > >>> > > >>> There's something going on that I can't really understand... > > >>> > > >>> clock-names: > > >>> minItems: 2 > > >>> maxItems: 4 > > >>> items: > > >>> - const: dp_apb_clk > > >>> - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] > > >>> - const: dp_aud_clk > > >>> - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] > > >>> uniqueItems: true > > >>> > > >>> results in dt_mk_schema complaining about an invalid schema. However, > > >>> the following works: > > >> > > >> Because 'uniqueItems' is not allowed because it's not needed. > > > > > > Error messages could be improved :-) > > > > > >>> clock-names: > > >>> oneOf: > > >>> - minItems: 2 > > >>> maxItems: 4 > > >>> items: > > >>> - const: dp_apb_clk > > >>> - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] > > >>> - const: dp_aud_clk > > >>> - enum: [ dp_vtc_pixel_clk_in, dp_live_video_in_clk ] > > >>> uniqueItems: true > > >>> > > >>> I assume this is due to clock-names being a string-array, which already > > >>> contains uniqueItems. However, if I leave uniqueItems out, an example > > >>> with a duplicated clock-names validates fine. > > >> > > >> Are you using 'DT_SCHEMA_FILES' as that leaves out all the core schema? > > > > > > Ah, that's probably why ! Thank you. > > > > I've now run dt_binding_check without DT_SCHEMA_FILES, and without a > > uniqueItems in each of the oneOf options, duplicate values are allowed. > > Is this expected ? > > Sorry, please scratch this, I made a mistake. Things now work as > expected. Thank you for your help ! Oh good, I was puzzled how that didn't work... In any case, I've got a meta-schema fix to correctly check under a oneOf. Rob
diff --git a/Documentation/devicetree/bindings/display/xlnx/xlnx,zynqmp-dpsub.yaml b/Documentation/devicetree/bindings/display/xlnx/xlnx,zynqmp-dpsub.yaml new file mode 100644 index 000000000000..e72264a5215d --- /dev/null +++ b/Documentation/devicetree/bindings/display/xlnx/xlnx,zynqmp-dpsub.yaml @@ -0,0 +1,153 @@ +# SPDX-License-Identifier: GPL-2.0 +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/display/xlnx/xlnx,zynqmp-dpsub.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Xilinx ZynqMP DisplayPort Subsystem + +description: | + The DisplayPort subsystem of Xilinx ZynqMP (Zynq UltraScale+ MPSoC) + implements the display and audio pipelines based on the DisplayPort v1.2 + standard. The subsystem includes multiple functional blocks as below: + + +------------------------------------------------------------+ + +--------+ | +----------------+ +-----------+ | + | DPDMA | --->| | --> | Video | Video +-------------+ | + | 4x vid | | | | | Rendering | -+--> | | | +------+ + | 2x aud | | | Audio/Video | --> | Pipeline | | | DisplayPort |---> | PHY0 | + +--------+ | | Buffer Manager | +-----------+ | | Source | | +------+ + | | and STC | +-----------+ | | Controller | | +------+ + Live Video --->| | --> | Audio | Audio | |---> | PHY1 | + | | | | Mixer | --+-> | | | +------+ + Live Audio --->| | --> | | || +-------------+ | + | +----------------+ +-----------+ || | + +---------------------------------------||-------------------+ + vv + Blended Video and + Mixed Audio to PL + + The Buffer Manager interacts with external interface such as DMA engines or + live audio/video streams from the programmable logic. The Video Rendering + Pipeline blends the video and graphics layers and performs colorspace + conversion. The Audio Mixer mixes the incoming audio streams. The DisplayPort + Source Controller handles the DisplayPort protocol and connects to external + PHYs. + + The subsystem supports 2 video and 2 audio streams, and various pixel formats + and depths up to 4K@30 resolution. + + Please refer to "Zynq UltraScale+ Device Technical Reference Manual" + (https://www.xilinx.com/support/documentation/user_guides/ug1085-zynq-ultrascale-trm.pdf) + for more details. + +maintainers: + - Laurent Pinchart <laurent.pinchart@ideasonboard.com> + +properties: + compatible: + const: xlnx,zynqmp-dpsub-1.7 + + reg: + maxItems: 4 + reg-names: + items: + - const: dp + - const: blend + - const: av_buf + - const: aud + + interrupts: + maxItems: 1 + + clocks: + description: + The AXI clock and at least one video clock are mandatory, the audio clock + optional. + minItems: 2 + maxItems: 4 + items: + - description: AXI clock + - description: Audio clock + - description: Non-live video clock (from Processing System) + - description: Live video clock (from Programmable Logic) + clock-names: + minItems: 2 + maxItems: 4 + items: + - const: dp_apb_clk + - const: dp_aud_clk + - const: dp_vtc_pixel_clk_in + - const: dp_live_video_in_clk + + power-domains: + maxItems: 1 + + dmas: + maxItems: 4 + items: + - description: Video layer, plane 0 (RGB or luma) + - description: Video layer, plane 1 (U/V or U) + - description: Video layer, plane 2 (V) + - description: Graphics layer + dma-names: + items: + - const: vid0 + - const: vid1 + - const: vid2 + - const: gfx0 + + phys: + description: PHYs for the DP data lanes + minItems: 1 + maxItems: 2 + phy-names: + minItems: 1 + maxItems: 2 + items: + - const: dp-phy0 + - const: dp-phy1 + +required: + - compatible + - reg + - reg-names + - interrupts + - clocks + - clock-names + - power-domains + - dmas + - dma-names + - phys + - phy-names + +additionalProperties: false + +examples: + - | + display@fd4a0000 { + compatible = "xlnx,zynqmp-dpsub-1.7"; + reg = <0x0 0xfd4a0000 0x0 0x1000>, + <0x0 0xfd4aa000 0x0 0x1000>, + <0x0 0xfd4ab000 0x0 0x1000>, + <0x0 0xfd4ac000 0x0 0x1000>; + reg-names = "dp", "blend", "av_buf", "aud"; + interrupts = <0 119 4>; + interrupt-parent = <&gic>; + + clock-names = "dp_apb_clk", "dp_aud_clk", "dp_live_video_in_clk"; + clocks = <&dp_aclk>, <&clkc 17>, <&si570_1>; + + power-domains = <&pd_dp>; + + dma-names = "vid0", "vid1", "vid2", "gfx0"; + dmas = <&xlnx_dpdma 0>, + <&xlnx_dpdma 1>, + <&xlnx_dpdma 2>, + <&xlnx_dpdma 3>; + + phys = <&lane1>, <&lane0>; + phy-names = "dp-phy0", "dp-phy1"; + }; + +...