On Wed, Sep 4, 2024 at 3:42 PM Nícolas F. R. A. Prado
<nfraprado@xxxxxxxxxxxxx> wrote:
>
> On Tue, Sep 03, 2024 at 03:07:52PM -0500, Rob Herring (Arm) wrote:
> > Various DT and fwnode functions take a compatible string as a parameter.
> > These are often used in cases which don't have a driver, so they've been
> > missed.
> >
> > The additional checks add about 400 more undocumented compatible
> > strings.
> >
> > Cc: Nícolas F. R. A. Prado <nfraprado@xxxxxxxxxxxxx>
> > Signed-off-by: Rob Herring (Arm) <robh@xxxxxxxxxx>
> > ---
> > scripts/dtc/dt-extract-compatibles | 13 +++++++++++++
> > 1 file changed, 13 insertions(+)
> >
> > diff --git a/scripts/dtc/dt-extract-compatibles b/scripts/dtc/dt-extract-compatibles
> > index 5ffb2364409b..13ea66d49e6c 100755
> > --- a/scripts/dtc/dt-extract-compatibles
> > +++ b/scripts/dtc/dt-extract-compatibles
> > @@ -46,6 +46,15 @@ def parse_of_match_table(data):
> > return match_table_list
> >
> >
> > +def parse_of_functions(data, func_name):
> > + """ Find all (device|machine)_is_compatible() arguments """
>
> This comment looks outdated.
Indeed...
>
> "Find all compatibles in the last argument of a given function"?
>
> > + match_table_list = []
>
> A better name would be compat_list like in the other functions that return lists
> of compatibles.
>
> > + for m in re.finditer(rf'{func_name}\(([a-zA-Z0-9_>\(\)"\-]+,\s)*"([a-zA-Z0-9_,-]+)"\)', data):
> > + match_table_list.append(m[2])
> > +
> > + return match_table_list
> > +
> > +
> > def parse_compatibles(file, compat_ignore_list):
> > with open(file, 'r', encoding='utf-8') as f:
> > data = f.read().replace('\n', '')
> > @@ -60,6 +69,10 @@ def parse_compatibles(file, compat_ignore_list):
> > else:
> > compat_list = parse_of_declare_macros(data)
> > compat_list += parse_of_device_id(data)
> > + compat_list += parse_of_functions(data, "_is_compatible")
>
> This pattern seems very broad and bound to generate false-positives. That said,
> I glanced over the results and didn't see any string clearly wrong, so I guess
> it's fine. There were a couple suspicious strings like "B5221" but they were
> indeed used in the of functions, so it's working correctly.
Originally I had it more specific, but ended up here. I even found one
I didn't know about. It matches of_flat_dt_is_compatible,
of_machine_is_compatible, (of_)?device_is_compatible, and
dt_is_compatible.
>
> > + compat_list += parse_of_functions(data, "of_find_compatible_node")
> > + compat_list += parse_of_functions(data, "for_each_compatible_node")
> > + compat_list += parse_of_functions(data, "of_get_compatible_child")
> >
> > return compat_list
> >
> > --
> > 2.45.2
> >
>
> Reviewed-by: Nícolas F. R. A. Prado <nfraprado@xxxxxxxxxxxxx>
Thanks.
>
> On a related note, I'm wondering if you haven't had time to see
> https://lore.kernel.org/all/20240729-dt-kselftest-parent-disabled-v2-1-d7a001c4930d@xxxxxxxxxxxxx
> or if that patch didn't make it to you somehow.
Sorry, it's outside what patchwork catches and I tend to miss those.
Rob
