On Thu, Apr 13, 2023 at 7:44 AM Qun-wei Lin (林群崴)
<Qun-wei.Lin@xxxxxxxxxxxx> wrote:
>
> On Thu, 2023-03-09 at 09:12 -0600, Rob Herring wrote:
> > On Wed, Mar 8, 2023 at 3:17 AM Qun-Wei Lin <qun-wei.lin@xxxxxxxxxxxx>
> > wrote:
> > >
> > > The overlay fragment is a special case where some properties are
> > > not
> > > present in the overlay source file, but in the base file.
> > >
> > > example:
> > > +-----------------------------+--------------------+
> > > > base.dts | overlay.dts |
> > >
> > > +-----------------------------+--------------------+
> > > > /dts-v1/; | /dts-v1/; |
> > > > | /plugin/; |
> > > > /{ | |
> > > > parent: test { | &parent { |
> > > > #address-cells = <1>; | child@0 { |
> > > > #size-cells = <0>; | reg = <0x0>; |
> > > > }; | }; |
> > > > }; | }; |
> > >
> > > +-----------------------------+--------------------+
> > >
> > > It will cause the following false alarms when compiling the overlay
> > > dts.
> > >
> > > 1. /fragment@0/__overlay__: Character '_' not recommended in node
> > > name
> > > 2. /fragment@0/__overlay__: Relying on default #address-cells value
> > > 3. /fragment@0/__overlay__: Relying on default #size-cells value
> > > 4. /fragment@0/__overlay__:reg: property has invalid length (4
> > > bytes)
> > > (#address-cells == 2, #size-cells == 1)
> > >
> > > This workaround will fix them by skip checking for node named
> > > __overlay__.
> > >
> > > Signed-off-by: Qun-Wei Lin <qun-wei.lin@xxxxxxxxxxxx>
> > > ---
> > > V1 -> V2:
> > > - Add is_overlay_node() helper
> > > - Skip anything starting with "__" in
> > > check_node_name_chars_strict()
> > >
> > > checks.c | 18 ++++++++++++++++++
> > > 1 file changed, 18 insertions(+)
> >
> > Reviewed-by: Rob Herring <robh@xxxxxxxxxx>
> >
> > Though I do wonder if as a matter of policy on overlay structure, if
> > we should require an overlay to have the parent node with
> > #address-cells/#size-cells. In the end that would be duplicated data,
> > but without it there's no way to parse and validate reg/ranges in an
> > unapplied overlay. That's just one example issue in being able to
> > validate overlays.
> >
> > Rob
>
> Hi Rob,
>
> Thank you for your review.
>
> I think I've found another problem:
> +-------------------------+----------------+
> | base.dts | overlay.dts |
> +-------------------------+----------------+
> | /dts-v1/; | /dts-v1/; |
> | /{ | /plugin/; |
> | #address-cells = <1>; | |
> | #size-cells = <0>; | &test { |
> | test: example@0 { | reg = <0x1>; |
> | reg = <0x0>; | }; |
> | }; | |
> | }; | |
> +-------------------------+----------------+
>
> The following error message is printed when compiling:
> Warning (reg_format): /fragment@0/__overlay__:reg: property has invalid
> length (4 bytes) (#address-cells == 2, #size-cells == 1)
> Warning (unit_address_vs_reg): /fragment@0/__overlay__: node has a reg
> or ranges property, but no unit name
> Warning (avoid_default_addr_size): /fragment@0/__overlay__: Relying on
> default #address-cells value
> Warning (avoid_default_addr_size): /fragment@0/__overlay__: Relying on
> default #size-cells value
>
> We can't get the #address-cells/#size-cells of the parent of the
> example node in the overlay structure.
> Do you think we should change it to is_overlay_node(node) instead of
> is_overlay_node(node->parent)?
> Or we just need to skip checking for node names starting with "__" in
> check_node_name_chars_strict()?
I think this is the tip of the iceberg in terms of being able to
validate overlays. Turning off address checks just kicks the problem
to schema validation. Perhaps the overlay should be structured to
include the parent bus node with #address-cells and #size-cells.
Rob
