dt: dt-extract-compatibles: Extract compatibles from function parameters

[PATCH] dt: dt-extract-compatibles: Extract compatibles from function parameters

Posted by Rob Herring (Arm) 1 year, 3 months ago

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@collabora.com>
Signed-off-by: Rob Herring (Arm) <robh@kernel.org>
---
 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 """
+	match_table_list = []
+	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")
+		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

Re: [PATCH] dt: dt-extract-compatibles: Extract compatibles from function parameters

Posted by Nícolas F. R. A. Prado 1 year, 3 months ago

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@collabora.com>
> Signed-off-by: Rob Herring (Arm) <robh@kernel.org>
> ---
>  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.

"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.

> +		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@collabora.com>

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@collabora.com
or if that patch didn't make it to you somehow.

Thanks,
Nícolas

Re: [PATCH] dt: dt-extract-compatibles: Extract compatibles from function parameters

Posted by Rob Herring 1 year, 3 months ago

On Wed, Sep 4, 2024 at 3:42 PM Nícolas F. R. A. Prado
<nfraprado@collabora.com> 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@collabora.com>
> > Signed-off-by: Rob Herring (Arm) <robh@kernel.org>
> > ---
> >  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@collabora.com>

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@collabora.com
> 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

Re: [PATCH] dt: dt-extract-compatibles: Extract compatibles from function parameters

Posted by Saravana Kannan 1 year, 3 months ago

On Tue, Sep 3, 2024 at 1:07 PM Rob Herring (Arm) <robh@kernel.org> 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@collabora.com>
> Signed-off-by: Rob Herring (Arm) <robh@kernel.org>

Acked-by: Saravana Kannan <saravanak@google.com>

-Saravana

> ---
>  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 """
> +       match_table_list = []
> +       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")
> +               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
>