On Sat, 10 Sept 2022 at 11:45, Krzysztof Kozlowski
<krzysztof.kozlowski@xxxxxxxxxx> wrote:
>
> On 10/09/2022 00:23, Rob Herring wrote:
> >>>>
> >>>> This should be ref to dsi-controller-main.yaml... or based on previous
> >>>> Rob's feedback you dropped it everywhere in children?
> >>>
> >>> I don't think I said. I thought about it some, as yes, we normally have
> >>> done as you suggested. The downside is with a ref we'll do the whole
> >>> validation of the child node twice (unless the referenced schema has a
> >>> 'select: false') whereas here only 'compatible' is validated twice. This
> >>> way also complicates checking for unevaluatedProperties/additionalProperties.
> >>>
> >>> So maybe better to keep with the 'normal' way and make this a ref.
> >>
> >> Well.. I tried using $ref in the previous iteration, but then I faced
> >> the fact that I can not use it. Using the $ref the node gets validated
> >> even if it is disabled, and we do not want to do this. The nodes are
> >> usually split in way that regulators are specified in the board DT file.
> >> Thus disabled dsi/dsi-phy nodes do not get all the required regulators.
> >> And dt-validate happily dumps tons of warnings for disabled nodes. Thus
> >> I ended up with the compatible rather than $ref.
> >
> > Only warnings about required properties? Those are supposed to get
> > filtered out if the node is disabled. Maybe being in a $ref doesn't
> > work.
>
> This seems to break regardless it is in $ref or when you directly
> reference the schema.
>
> I tested display/msm/dpu-sc7180.yaml on modified DTB when a required
> property is missing (I removed reg-names) and:
>
> 1. When node is enabled:
>
> c7180-idp.dtb: display-controller@ae01000: 'reg-names' is a required
> property
>
> Documentation/devicetree/bindings/display/msm/dpu-sc7180.yaml
>
> rch/arm64/boot/dts/qcom/sc7180-idp.dtb: display-controller@ae01000:
> Unevaluated properties are not allowed ('interrupt-parent',
> 'interrupts', 'operating-points-v2', 'opp-table', 'ports',
> 'power-domains' were unexpected)
>
>
> 2. When node is disabled the first error disappears (so no clue which
> one is missing) but schema does not pass:
>
> qcom/sc7180-idp.dtb: display-controller@ae01000: Unevaluated properties
> are not allowed ('interrupt-parent', 'interrupts',
> 'operating-points-v2', 'opp-table', 'ports', 'power-domains' were
> unexpected)
>
> From schema: Documentation/devicetree/bindings/display/msm/dpu-sc7180.yaml
>
>
> However I think there is no such problem, as Dmitry said, that ref
> changes anything. There will be always failure - either from parent
> schema (using $ref) or from device child schema (the one which actually
> misses the property).
Initially I stumbled upon this issue with the dsi and dsi_phy nodes
for msm8996 devices. If I have $ref here, dsi1/dsi1_phy nodes will
emit warnings regarding the missing -supply properties despite nodes
being disabled. If I use `compatible' here, the schema checks pass.
Thus I'd prefer to leave `compatible' here. Not to mention that it
also allows specifying a tighter binding than just using the $ref.
--
With best wishes
Dmitry
