Re: [PATCH v2] checks: Suppress warnings on overlay fragments

["Qun-wei Lin (林群崴)" <Qun-wei.Lin-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>]

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@mediatek.com>
> 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@mediatek.com>
> > ---
> > 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@kernel.org>
> 
> 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()?

Qun-Wei