* [PATCH v6 01/12] tools lib traceevent: Convert remaining %p[fF] users to %p[sS]
2019-09-10 8:46 [PATCH v6 00/12] Device property improvements, add %pfw format specifier Sakari Ailus
@ 2019-09-10 8:46 ` Sakari Ailus
2019-09-10 9:22 ` Andy Shevchenko
2019-09-10 11:18 ` Steven Rostedt
2019-09-10 8:46 ` [PATCH v6 02/12] software node: Get reference to parent swnode in get_parent op Sakari Ailus
` (10 subsequent siblings)
11 siblings, 2 replies; 25+ messages in thread
From: Sakari Ailus @ 2019-09-10 8:46 UTC (permalink / raw)
To: Petr Mladek, linux-kernel, rafael
Cc: Andy Shevchenko, linux-acpi, devicetree, Rob Herring,
Heikki Krogerus, Joe Perches, Steven Rostedt,
Arnaldo Carvalho de Melo, Tzvetomir Stoyanov, linux-trace-devel,
Jiri Olsa, Namhyung Kim
There are no in-kernel %p[fF] users left. Convert the traceevent tool,
too, to align with the kernel.
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Tzvetomir Stoyanov <tstoyanov@vmware.com>
Cc: linux-trace-devel@vger.kernel.org
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
---
.../Documentation/libtraceevent-func_apis.txt | 10 +++++-----
tools/lib/traceevent/event-parse.c | 7 +++----
2 files changed, 8 insertions(+), 9 deletions(-)
diff --git a/tools/lib/traceevent/Documentation/libtraceevent-func_apis.txt b/tools/lib/traceevent/Documentation/libtraceevent-func_apis.txt
index 38bfea30a5f64..f6aca0df2151a 100644
--- a/tools/lib/traceevent/Documentation/libtraceevent-func_apis.txt
+++ b/tools/lib/traceevent/Documentation/libtraceevent-func_apis.txt
@@ -59,12 +59,12 @@ parser context.
The _tep_register_function()_ function registers a function name mapped to an
address and (optional) module. This mapping is used in case the function tracer
-or events have "%pF" or "%pS" parameter in its format string. It is common to
-pass in the kallsyms function names with their corresponding addresses with this
+or events have "%pS" parameter in its format string. It is common to pass in
+the kallsyms function names with their corresponding addresses with this
function. The _tep_ argument is the trace event parser context. The _name_ is
-the name of the function, the string is copied internally. The _addr_ is
-the start address of the function. The _mod_ is the kernel module
-the function may be in (NULL for none).
+the name of the function, the string is copied internally. The _addr_ is the
+start address of the function. The _mod_ is the kernel module the function may
+be in (NULL for none).
The _tep_register_print_string()_ function registers a string by the address
it was stored in the kernel. Some strings internal to the kernel with static
diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
index b36b536a9fcba..1d7927ff32660 100644
--- a/tools/lib/traceevent/event-parse.c
+++ b/tools/lib/traceevent/event-parse.c
@@ -4335,8 +4335,6 @@ static struct tep_print_arg *make_bprint_args(char *fmt, void *data, int size, s
switch (*ptr) {
case 's':
case 'S':
- case 'f':
- case 'F':
case 'x':
break;
default:
@@ -4455,12 +4453,13 @@ get_bprint_format(void *data, int size __maybe_unused,
printk = find_printk(tep, addr);
if (!printk) {
- if (asprintf(&format, "%%pf: (NO FORMAT FOUND at %llx)\n", addr) < 0)
+ if (asprintf(&format, "%%ps: (NO FORMAT FOUND at %llx)\n",
+ addr) < 0)
return NULL;
return format;
}
- if (asprintf(&format, "%s: %s", "%pf", printk->printk) < 0)
+ if (asprintf(&format, "%s: %s", "%ps", printk->printk) < 0)
return NULL;
return format;
--
2.20.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH v6 01/12] tools lib traceevent: Convert remaining %p[fF] users to %p[sS]
2019-09-10 8:46 ` [PATCH v6 01/12] tools lib traceevent: Convert remaining %p[fF] users to %p[sS] Sakari Ailus
@ 2019-09-10 9:22 ` Andy Shevchenko
2019-09-10 11:18 ` Steven Rostedt
1 sibling, 0 replies; 25+ messages in thread
From: Andy Shevchenko @ 2019-09-10 9:22 UTC (permalink / raw)
To: Sakari Ailus
Cc: Petr Mladek, linux-kernel, rafael, linux-acpi, devicetree,
Rob Herring, Heikki Krogerus, Joe Perches, Steven Rostedt,
Arnaldo Carvalho de Melo, Tzvetomir Stoyanov, linux-trace-devel,
Jiri Olsa, Namhyung Kim
On Tue, Sep 10, 2019 at 11:46:56AM +0300, Sakari Ailus wrote:
> There are no in-kernel %p[fF] users left. Convert the traceevent tool,
> too, to align with the kernel.
> function. The _tep_ argument is the trace event parser context. The _name_ is
> -the name of the function, the string is copied internally. The _addr_ is
> -the start address of the function. The _mod_ is the kernel module
> -the function may be in (NULL for none).
> +the name of the function, the string is copied internally. The _addr_ is the
> +start address of the function. The _mod_ is the kernel module the function may
> +be in (NULL for none).
This seems a churn of reformatting.
> - if (asprintf(&format, "%%pf: (NO FORMAT FOUND at %llx)\n", addr) < 0)
> + if (asprintf(&format, "%%ps: (NO FORMAT FOUND at %llx)\n",
> + addr) < 0)
Splitting line also seems a churn to me. But this one is up to maintainers.
Other than above, looks good to me, thanks!
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v6 01/12] tools lib traceevent: Convert remaining %p[fF] users to %p[sS]
2019-09-10 8:46 ` [PATCH v6 01/12] tools lib traceevent: Convert remaining %p[fF] users to %p[sS] Sakari Ailus
2019-09-10 9:22 ` Andy Shevchenko
@ 2019-09-10 11:18 ` Steven Rostedt
2019-09-10 17:18 ` Joe Perches
1 sibling, 1 reply; 25+ messages in thread
From: Steven Rostedt @ 2019-09-10 11:18 UTC (permalink / raw)
To: Sakari Ailus
Cc: Petr Mladek, linux-kernel, rafael, Andy Shevchenko, linux-acpi,
devicetree, Rob Herring, Heikki Krogerus, Joe Perches,
Arnaldo Carvalho de Melo, Tzvetomir Stoyanov, linux-trace-devel,
Jiri Olsa, Namhyung Kim
On Tue, 10 Sep 2019 11:46:56 +0300
Sakari Ailus <sakari.ailus@linux.intel.com> wrote:
> There are no in-kernel %p[fF] users left. Convert the traceevent tool,
> too, to align with the kernel.
>
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> Cc: Tzvetomir Stoyanov <tstoyanov@vmware.com>
> Cc: linux-trace-devel@vger.kernel.org
> Cc: Jiri Olsa <jolsa@redhat.com>
> Cc: Namhyung Kim <namhyung@kernel.org>
> ---
> .../Documentation/libtraceevent-func_apis.txt | 10 +++++-----
> tools/lib/traceevent/event-parse.c | 7 +++----
> 2 files changed, 8 insertions(+), 9 deletions(-)
>
> diff --git a/tools/lib/traceevent/Documentation/libtraceevent-func_apis.txt b/tools/lib/traceevent/Documentation/libtraceevent-func_apis.txt
> index 38bfea30a5f64..f6aca0df2151a 100644
> --- a/tools/lib/traceevent/Documentation/libtraceevent-func_apis.txt
> +++ b/tools/lib/traceevent/Documentation/libtraceevent-func_apis.txt
> @@ -59,12 +59,12 @@ parser context.
>
> The _tep_register_function()_ function registers a function name mapped to an
> address and (optional) module. This mapping is used in case the function tracer
> -or events have "%pF" or "%pS" parameter in its format string. It is common to
> -pass in the kallsyms function names with their corresponding addresses with this
> +or events have "%pS" parameter in its format string. It is common to pass in
> +the kallsyms function names with their corresponding addresses with this
> function. The _tep_ argument is the trace event parser context. The _name_ is
> -the name of the function, the string is copied internally. The _addr_ is
> -the start address of the function. The _mod_ is the kernel module
> -the function may be in (NULL for none).
> +the name of the function, the string is copied internally. The _addr_ is the
> +start address of the function. The _mod_ is the kernel module the function may
> +be in (NULL for none).
>
> The _tep_register_print_string()_ function registers a string by the address
> it was stored in the kernel. Some strings internal to the kernel with static
> diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
> index b36b536a9fcba..1d7927ff32660 100644
> --- a/tools/lib/traceevent/event-parse.c
> +++ b/tools/lib/traceevent/event-parse.c
> @@ -4335,8 +4335,6 @@ static struct tep_print_arg *make_bprint_args(char *fmt, void *data, int size, s
> switch (*ptr) {
> case 's':
> case 'S':
> - case 'f':
> - case 'F':
This file is used to parse output from older kernels, so remove this hunk.
It's not just for the lastest kernel. We must maintain backward
compatibility here too. If there use to be a usage of this, then we
must keep it until the kernels are no longer used (perhaps 7 years?)
> case 'x':
> break;
> default:
> @@ -4455,12 +4453,13 @@ get_bprint_format(void *data, int size
> __maybe_unused,
> printk = find_printk(tep, addr);
> if (!printk) {
> - if (asprintf(&format, "%%pf: (NO FORMAT FOUND at
> %llx)\n", addr) < 0)
> + if (asprintf(&format, "%%ps: (NO FORMAT FOUND at
> %llx)\n",
> + addr) < 0)
Remove the line break. I hate the 80 character limit especially when it
makes the code look worse. Like it does here.
-- Steve
> return NULL;
> return format;
> }
>
> - if (asprintf(&format, "%s: %s", "%pf", printk->printk) < 0)
> + if (asprintf(&format, "%s: %s", "%ps", printk->printk) < 0)
> return NULL;
>
> return format;
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH v6 01/12] tools lib traceevent: Convert remaining %p[fF] users to %p[sS]
2019-09-10 11:18 ` Steven Rostedt
@ 2019-09-10 17:18 ` Joe Perches
2019-09-10 18:26 ` Steven Rostedt
0 siblings, 1 reply; 25+ messages in thread
From: Joe Perches @ 2019-09-10 17:18 UTC (permalink / raw)
To: Steven Rostedt, Sakari Ailus
Cc: Petr Mladek, linux-kernel, rafael, Andy Shevchenko, linux-acpi,
devicetree, Rob Herring, Heikki Krogerus,
Arnaldo Carvalho de Melo, Tzvetomir Stoyanov, linux-trace-devel,
Jiri Olsa, Namhyung Kim
On Tue, 2019-09-10 at 07:18 -0400, Steven Rostedt wrote:
> On Tue, 10 Sep 2019 11:46:56 +0300
> Sakari Ailus <sakari.ailus@linux.intel.com> wrote:
>
> > There are no in-kernel %p[fF] users left. Convert the traceevent tool,
> > too, to align with the kernel.
[]
> > diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
[]
> > @@ -4335,8 +4335,6 @@ static struct tep_print_arg *make_bprint_args(char *fmt, void *data, int size, s
> > switch (*ptr) {
> > case 's':
> > case 'S':
> > - case 'f':
> > - case 'F':
>
> This file is used to parse output from older kernels, so remove this hunk.
>
> It's not just for the lastest kernel. We must maintain backward
> compatibility here too. If there use to be a usage of this, then we
> must keep it until the kernels are no longer used (perhaps 7 years?)
That argues for not using "%pfw" at all for some number of years.
Perhaps the '%pfw' should be '%pnfw' for 'name' and 'fwnode'
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH v6 01/12] tools lib traceevent: Convert remaining %p[fF] users to %p[sS]
2019-09-10 17:18 ` Joe Perches
@ 2019-09-10 18:26 ` Steven Rostedt
2019-09-10 18:42 ` Joe Perches
0 siblings, 1 reply; 25+ messages in thread
From: Steven Rostedt @ 2019-09-10 18:26 UTC (permalink / raw)
To: Joe Perches
Cc: Sakari Ailus, Petr Mladek, linux-kernel, rafael, Andy Shevchenko,
linux-acpi, devicetree, Rob Herring, Heikki Krogerus,
Arnaldo Carvalho de Melo, Tzvetomir Stoyanov, linux-trace-devel,
Jiri Olsa, Namhyung Kim
On Tue, 10 Sep 2019 10:18:44 -0700
Joe Perches <joe@perches.com> wrote:
> > It's not just for the lastest kernel. We must maintain backward
> > compatibility here too. If there use to be a usage of this, then we
> > must keep it until the kernels are no longer used (perhaps 7 years?)
>
> That argues for not using "%pfw" at all for some number of years.
>
> Perhaps the '%pfw' should be '%pnfw' for 'name' and 'fwnode'
-ENOCOMPREHENSION
-- Steve
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v6 01/12] tools lib traceevent: Convert remaining %p[fF] users to %p[sS]
2019-09-10 18:26 ` Steven Rostedt
@ 2019-09-10 18:42 ` Joe Perches
2019-09-10 19:03 ` Steven Rostedt
0 siblings, 1 reply; 25+ messages in thread
From: Joe Perches @ 2019-09-10 18:42 UTC (permalink / raw)
To: Steven Rostedt
Cc: Sakari Ailus, Petr Mladek, linux-kernel, rafael, Andy Shevchenko,
linux-acpi, devicetree, Rob Herring, Heikki Krogerus,
Arnaldo Carvalho de Melo, Tzvetomir Stoyanov, linux-trace-devel,
Jiri Olsa, Namhyung Kim
On Tue, 2019-09-10 at 14:26 -0400, Steven Rostedt wrote:
> On Tue, 10 Sep 2019 10:18:44 -0700
> Joe Perches <joe@perches.com> wrote:
>
> > > It's not just for the lastest kernel. We must maintain backward
> > > compatibility here too. If there use to be a usage of this, then we
> > > must keep it until the kernels are no longer used (perhaps 7 years?)
> >
> > That argues for not using "%pfw" at all for some number of years.
> >
> > Perhaps the '%pfw' should be '%pnfw' for 'name' and 'fwnode'
>
> -ENOCOMPREHENSION
Perhaps you were not copied on the whole series.
https://lore.kernel.org/lkml/20190910084707.18380-1-sakari.ailus@linux.intel.com/
As I understand it, Sakair Ailus is proposing to
obsolete the current vsprintf "%p[Ff]" extension
and replace the usage with a new "%pfw" extension
which would emit the name of a pointer to "struct fwnode {}".
https://lore.kernel.org/lkml/20190910084707.18380-10-sakari.ailus@linux.intel.com/
If reusing "%pf<foo>" is a problem, then instead
it might be reasonable to have a new "%pn<foo>" for
that use instead.
btw:
Is there kernel version information available in
trace output files?
If so, it might be reasonable to change the tooling
there instead.
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH v6 01/12] tools lib traceevent: Convert remaining %p[fF] users to %p[sS]
2019-09-10 18:42 ` Joe Perches
@ 2019-09-10 19:03 ` Steven Rostedt
2019-09-10 19:44 ` Joe Perches
0 siblings, 1 reply; 25+ messages in thread
From: Steven Rostedt @ 2019-09-10 19:03 UTC (permalink / raw)
To: Joe Perches
Cc: Sakari Ailus, Petr Mladek, linux-kernel, rafael, Andy Shevchenko,
linux-acpi, devicetree, Rob Herring, Heikki Krogerus,
Arnaldo Carvalho de Melo, Tzvetomir Stoyanov, linux-trace-devel,
Jiri Olsa, Namhyung Kim
On Tue, 10 Sep 2019 11:42:06 -0700
Joe Perches <joe@perches.com> wrote:
> On Tue, 2019-09-10 at 14:26 -0400, Steven Rostedt wrote:
> > On Tue, 10 Sep 2019 10:18:44 -0700
> > Joe Perches <joe@perches.com> wrote:
> >
> > > > It's not just for the lastest kernel. We must maintain backward
> > > > compatibility here too. If there use to be a usage of this, then we
> > > > must keep it until the kernels are no longer used (perhaps 7 years?)
> > >
> > > That argues for not using "%pfw" at all for some number of years.
> > >
> > > Perhaps the '%pfw' should be '%pnfw' for 'name' and 'fwnode'
> >
> > -ENOCOMPREHENSION
>
> Perhaps you were not copied on the whole series.
>
> https://lore.kernel.org/lkml/20190910084707.18380-1-sakari.ailus@linux.intel.com/
Thanks for the link.
>
> As I understand it, Sakair Ailus is proposing to
> obsolete the current vsprintf "%p[Ff]" extension
> and replace the usage with a new "%pfw" extension
> which would emit the name of a pointer to "struct fwnode {}".
>
> https://lore.kernel.org/lkml/20190910084707.18380-10-sakari.ailus@linux.intel.com/
>
> If reusing "%pf<foo>" is a problem, then instead
> it might be reasonable to have a new "%pn<foo>" for
> that use instead.
>
> btw:
>
> Is there kernel version information available in
> trace output files?
Not really. This is just a library that parses the trace event formats,
there's not kernel versions passed in, but we do use variations in
formats and such to determine what is supported.
>
> If so, it might be reasonable to change the tooling
> there instead.
>
Actually, I think we could just look to see if "%pfw" is used and fall
to that, otherwise consider it an older kernel and do it the original
way.
-- Steve
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH v6 01/12] tools lib traceevent: Convert remaining %p[fF] users to %p[sS]
2019-09-10 19:03 ` Steven Rostedt
@ 2019-09-10 19:44 ` Joe Perches
2019-09-16 11:41 ` Sakari Ailus
0 siblings, 1 reply; 25+ messages in thread
From: Joe Perches @ 2019-09-10 19:44 UTC (permalink / raw)
To: Steven Rostedt
Cc: Sakari Ailus, Petr Mladek, linux-kernel, rafael, Andy Shevchenko,
linux-acpi, devicetree, Rob Herring, Heikki Krogerus,
Arnaldo Carvalho de Melo, Tzvetomir Stoyanov, linux-trace-devel,
Jiri Olsa, Namhyung Kim
On Tue, 2019-09-10 at 15:03 -0400, Steven Rostedt wrote:
> On Tue, 10 Sep 2019 11:42:06 -0700
[]
> > btw:
> >
> > Is there kernel version information available in
> > trace output files?
>
> Not really. This is just a library that parses the trace event formats,
> there's not kernel versions passed in, but we do use variations in
> formats and such to determine what is supported.
>
> > If so, it might be reasonable to change the tooling
> > there instead.
> >
>
> Actually, I think we could just look to see if "%pfw" is used and fall
> to that, otherwise consider it an older kernel and do it the original
> way.
Well, if you think that works, OK great.
But could that work?
How would an individual trace record know if
another trace record used %pfw?
Perhaps not reusing %pf, marking it reserved
for a period of years, and using another unused
prefix %p<type> like %pnfw may be simpler.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v6 01/12] tools lib traceevent: Convert remaining %p[fF] users to %p[sS]
2019-09-10 19:44 ` Joe Perches
@ 2019-09-16 11:41 ` Sakari Ailus
2019-09-16 14:37 ` Steven Rostedt
0 siblings, 1 reply; 25+ messages in thread
From: Sakari Ailus @ 2019-09-16 11:41 UTC (permalink / raw)
To: Joe Perches
Cc: Steven Rostedt, Petr Mladek, linux-kernel, rafael,
Andy Shevchenko, linux-acpi, devicetree, Rob Herring,
Heikki Krogerus, Arnaldo Carvalho de Melo, Tzvetomir Stoyanov,
linux-trace-devel, Jiri Olsa, Namhyung Kim
Hi Joe, Steven,
On Tue, Sep 10, 2019 at 12:44:03PM -0700, Joe Perches wrote:
> On Tue, 2019-09-10 at 15:03 -0400, Steven Rostedt wrote:
> > On Tue, 10 Sep 2019 11:42:06 -0700
> []
> > > btw:
> > >
> > > Is there kernel version information available in
> > > trace output files?
> >
> > Not really. This is just a library that parses the trace event formats,
> > there's not kernel versions passed in, but we do use variations in
> > formats and such to determine what is supported.
> >
> > > If so, it might be reasonable to change the tooling
> > > there instead.
> > >
> >
> > Actually, I think we could just look to see if "%pfw" is used and fall
> > to that, otherwise consider it an older kernel and do it the original
> > way.
>
> Well, if you think that works, OK great.
>
> But could that work?
> How would an individual trace record know if
> another trace record used %pfw?
>
> Perhaps not reusing %pf, marking it reserved
> for a period of years, and using another unused
> prefix %p<type> like %pnfw may be simpler.
%p[Ff]w does not exist (I grepped for it) in older kernels since v3.0. So
kernel support for %p[fF] and %pfw are mutually exclusive. If you're ok
with that, I could change the patch to check %pf isn't followed by 'w',
in order to support %pf on older kernels.
Although that still does not address using older tooling on newer kernels
with support for %pfw.
If you think that's an issue, I'll opt for another extension than %pfw,
which I chose originally since it's memorable --- fw for fwnode (names,
paths, and probably more in the future).
--
Regards,
Sakari Ailus
sakari.ailus@linux.intel.com
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v6 01/12] tools lib traceevent: Convert remaining %p[fF] users to %p[sS]
2019-09-16 11:41 ` Sakari Ailus
@ 2019-09-16 14:37 ` Steven Rostedt
2019-09-18 13:08 ` Sakari Ailus
0 siblings, 1 reply; 25+ messages in thread
From: Steven Rostedt @ 2019-09-16 14:37 UTC (permalink / raw)
To: Sakari Ailus
Cc: Joe Perches, Petr Mladek, linux-kernel, rafael, Andy Shevchenko,
linux-acpi, devicetree, Rob Herring, Heikki Krogerus,
Arnaldo Carvalho de Melo, Tzvetomir Stoyanov, linux-trace-devel,
Jiri Olsa, Namhyung Kim
On Mon, 16 Sep 2019 14:41:59 +0300
Sakari Ailus <sakari.ailus@linux.intel.com> wrote:
> > Well, if you think that works, OK great.
> >
> > But could that work?
> > How would an individual trace record know if
> > another trace record used %pfw?
> >
> > Perhaps not reusing %pf, marking it reserved
> > for a period of years, and using another unused
> > prefix %p<type> like %pnfw may be simpler.
>
> %p[Ff]w does not exist (I grepped for it) in older kernels since v3.0. So
> kernel support for %p[fF] and %pfw are mutually exclusive. If you're ok
> with that, I could change the patch to check %pf isn't followed by 'w',
> in order to support %pf on older kernels.
I think that's what I suggested to do.
>
> Although that still does not address using older tooling on newer kernels
> with support for %pfw.
That should be fine. I don't think it will crash those tools, they will
just give out wrong information, and if people complain, we can try to
get them to use the newer version of those tools ;-) (hopefully they
don't complain to Linus).
>
> If you think that's an issue, I'll opt for another extension than %pfw,
> which I chose originally since it's memorable --- fw for fwnode (names,
> paths, and probably more in the future).
>
I'm fine with the switch, as long as newer tools know how to handle it.
Make sure we also add a comment in the Linux kernel code that states
that older kernels use to have 'f' and 'F' and that new tools look for
'fw' to denote that this isn't an older kernel. This way, people will
hopefully not add another 'fX' pointer name.
-- Steve
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v6 01/12] tools lib traceevent: Convert remaining %p[fF] users to %p[sS]
2019-09-16 14:37 ` Steven Rostedt
@ 2019-09-18 13:08 ` Sakari Ailus
0 siblings, 0 replies; 25+ messages in thread
From: Sakari Ailus @ 2019-09-18 13:08 UTC (permalink / raw)
To: Steven Rostedt
Cc: Joe Perches, Petr Mladek, linux-kernel, rafael, Andy Shevchenko,
linux-acpi, devicetree, Rob Herring, Heikki Krogerus,
Arnaldo Carvalho de Melo, Tzvetomir Stoyanov, linux-trace-devel,
Jiri Olsa, Namhyung Kim
Hi Steven,
On Mon, Sep 16, 2019 at 10:37:55AM -0400, Steven Rostedt wrote:
> > If you think that's an issue, I'll opt for another extension than %pfw,
> > which I chose originally since it's memorable --- fw for fwnode (names,
> > paths, and probably more in the future).
> >
>
> I'm fine with the switch, as long as newer tools know how to handle it.
>
> Make sure we also add a comment in the Linux kernel code that states
> that older kernels use to have 'f' and 'F' and that new tools look for
> 'fw' to denote that this isn't an older kernel. This way, people will
> hopefully not add another 'fX' pointer name.
Good point. I'll add a comment on this to make_bprint_args() in
tools/lib/traceevent/event-parse.c as well as in vsprintf.c.
--
Regards,
Sakari Ailus
sakari.ailus@linux.intel.com
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v6 02/12] software node: Get reference to parent swnode in get_parent op
2019-09-10 8:46 [PATCH v6 00/12] Device property improvements, add %pfw format specifier Sakari Ailus
2019-09-10 8:46 ` [PATCH v6 01/12] tools lib traceevent: Convert remaining %p[fF] users to %p[sS] Sakari Ailus
@ 2019-09-10 8:46 ` Sakari Ailus
2019-09-10 8:46 ` [PATCH v6 03/12] software node: Make argument to to_software_node const Sakari Ailus
` (9 subsequent siblings)
11 siblings, 0 replies; 25+ messages in thread
From: Sakari Ailus @ 2019-09-10 8:46 UTC (permalink / raw)
To: Petr Mladek, linux-kernel, rafael
Cc: Andy Shevchenko, linux-acpi, devicetree, Rob Herring,
Heikki Krogerus, Joe Perches
The software_node_get_parent() returned a pointer to the parent swnode,
but did not take a reference to it, leading the caller to put a reference
that was not taken. Take that reference now.
Fixes: 59abd83672f7 ("drivers: base: Introducing software nodes to the firmware node framework")
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/base/swnode.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c
index e7b3aa3bd55ad..a7cb41812cfda 100644
--- a/drivers/base/swnode.c
+++ b/drivers/base/swnode.c
@@ -520,7 +520,10 @@ software_node_get_parent(const struct fwnode_handle *fwnode)
{
struct swnode *swnode = to_swnode(fwnode);
- return swnode ? (swnode->parent ? &swnode->parent->fwnode : NULL) : NULL;
+ if (!swnode || !swnode->parent)
+ return NULL;
+
+ return fwnode_handle_get(&swnode->parent->fwnode);
}
static struct fwnode_handle *
--
2.20.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v6 03/12] software node: Make argument to to_software_node const
2019-09-10 8:46 [PATCH v6 00/12] Device property improvements, add %pfw format specifier Sakari Ailus
2019-09-10 8:46 ` [PATCH v6 01/12] tools lib traceevent: Convert remaining %p[fF] users to %p[sS] Sakari Ailus
2019-09-10 8:46 ` [PATCH v6 02/12] software node: Get reference to parent swnode in get_parent op Sakari Ailus
@ 2019-09-10 8:46 ` Sakari Ailus
2019-09-10 8:46 ` [PATCH v6 04/12] device property: Move fwnode_get_parent() up Sakari Ailus
` (8 subsequent siblings)
11 siblings, 0 replies; 25+ messages in thread
From: Sakari Ailus @ 2019-09-10 8:46 UTC (permalink / raw)
To: Petr Mladek, linux-kernel, rafael
Cc: Andy Shevchenko, linux-acpi, devicetree, Rob Herring,
Heikki Krogerus, Joe Perches
to_software_node() does not need to modify the fwnode_handle it operates
on; therefore make it const. This allows passing a const fwnode_handle to
to_software_node().
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
drivers/base/swnode.c | 4 ++--
include/linux/property.h | 3 ++-
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c
index a7cb41812cfda..951e7efd47c23 100644
--- a/drivers/base/swnode.c
+++ b/drivers/base/swnode.c
@@ -71,9 +71,9 @@ software_node_to_swnode(const struct software_node *node)
return swnode;
}
-const struct software_node *to_software_node(struct fwnode_handle *fwnode)
+const struct software_node *to_software_node(const struct fwnode_handle *fwnode)
{
- struct swnode *swnode = to_swnode(fwnode);
+ const struct swnode *swnode = to_swnode(fwnode);
return swnode ? swnode->node : NULL;
}
diff --git a/include/linux/property.h b/include/linux/property.h
index 5a910ad795910..421c76e53708d 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -418,7 +418,8 @@ struct software_node {
};
bool is_software_node(const struct fwnode_handle *fwnode);
-const struct software_node *to_software_node(struct fwnode_handle *fwnode);
+const struct software_node *
+to_software_node(const struct fwnode_handle *fwnode);
struct fwnode_handle *software_node_fwnode(const struct software_node *node);
int software_node_register_nodes(const struct software_node *nodes);
--
2.20.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v6 04/12] device property: Move fwnode_get_parent() up
2019-09-10 8:46 [PATCH v6 00/12] Device property improvements, add %pfw format specifier Sakari Ailus
` (2 preceding siblings ...)
2019-09-10 8:46 ` [PATCH v6 03/12] software node: Make argument to to_software_node const Sakari Ailus
@ 2019-09-10 8:46 ` Sakari Ailus
2019-09-10 8:47 ` [PATCH v6 05/12] device property: Add functions for accessing node's parents Sakari Ailus
` (7 subsequent siblings)
11 siblings, 0 replies; 25+ messages in thread
From: Sakari Ailus @ 2019-09-10 8:46 UTC (permalink / raw)
To: Petr Mladek, linux-kernel, rafael
Cc: Andy Shevchenko, linux-acpi, devicetree, Rob Herring,
Heikki Krogerus, Joe Perches
Move fwnode_get_parent() above fwnode_get_next_parent(), making the order
the same as in the header file.
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/base/property.c | 26 +++++++++++++-------------
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/drivers/base/property.c b/drivers/base/property.c
index 81bd01ed40427..3d9dffbe96378 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -556,6 +556,19 @@ int device_add_properties(struct device *dev,
}
EXPORT_SYMBOL_GPL(device_add_properties);
+/**
+ * fwnode_get_parent - Return parent firwmare node
+ * @fwnode: Firmware whose parent is retrieved
+ *
+ * Return parent firmware node of the given node if possible or %NULL if no
+ * parent was available.
+ */
+struct fwnode_handle *fwnode_get_parent(const struct fwnode_handle *fwnode)
+{
+ return fwnode_call_ptr_op(fwnode, get_parent);
+}
+EXPORT_SYMBOL_GPL(fwnode_get_parent);
+
/**
* fwnode_get_next_parent - Iterate to the node's parent
* @fwnode: Firmware whose parent is retrieved
@@ -577,19 +590,6 @@ struct fwnode_handle *fwnode_get_next_parent(struct fwnode_handle *fwnode)
}
EXPORT_SYMBOL_GPL(fwnode_get_next_parent);
-/**
- * fwnode_get_parent - Return parent firwmare node
- * @fwnode: Firmware whose parent is retrieved
- *
- * Return parent firmware node of the given node if possible or %NULL if no
- * parent was available.
- */
-struct fwnode_handle *fwnode_get_parent(const struct fwnode_handle *fwnode)
-{
- return fwnode_call_ptr_op(fwnode, get_parent);
-}
-EXPORT_SYMBOL_GPL(fwnode_get_parent);
-
/**
* fwnode_get_next_child_node - Return the next child node handle for a node
* @fwnode: Firmware node to find the next child node for.
--
2.20.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v6 05/12] device property: Add functions for accessing node's parents
2019-09-10 8:46 [PATCH v6 00/12] Device property improvements, add %pfw format specifier Sakari Ailus
` (3 preceding siblings ...)
2019-09-10 8:46 ` [PATCH v6 04/12] device property: Move fwnode_get_parent() up Sakari Ailus
@ 2019-09-10 8:47 ` Sakari Ailus
2019-09-10 8:47 ` [PATCH v6 06/12] device property: Add fwnode_get_name for returning the name of a node Sakari Ailus
` (6 subsequent siblings)
11 siblings, 0 replies; 25+ messages in thread
From: Sakari Ailus @ 2019-09-10 8:47 UTC (permalink / raw)
To: Petr Mladek, linux-kernel, rafael
Cc: Andy Shevchenko, linux-acpi, devicetree, Rob Herring,
Heikki Krogerus, Joe Perches
Add two convenience functions for accessing node's parents:
fwnode_count_parents() returns the number of parent nodes a given node
has. fwnode_get_nth_parent() returns node's parent at a given distance
from the node itself.
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/base/property.c | 46 ++++++++++++++++++++++++++++++++++++++++
include/linux/property.h | 3 +++
2 files changed, 49 insertions(+)
diff --git a/drivers/base/property.c b/drivers/base/property.c
index 3d9dffbe96378..f2e555e68b56a 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -590,6 +590,52 @@ struct fwnode_handle *fwnode_get_next_parent(struct fwnode_handle *fwnode)
}
EXPORT_SYMBOL_GPL(fwnode_get_next_parent);
+/**
+ * fwnode_count_parents - Return the number of parents a node has
+ * @fwnode: The node the parents of which are to be counted
+ *
+ * Returns the number of parents a node has.
+ */
+unsigned int fwnode_count_parents(const struct fwnode_handle *fwnode)
+{
+ struct fwnode_handle *__fwnode;
+ unsigned int count;
+
+ __fwnode = fwnode_get_parent(fwnode);
+
+ for (count = 0; __fwnode; count++)
+ __fwnode = fwnode_get_next_parent(__fwnode);
+
+ return count;
+}
+EXPORT_SYMBOL_GPL(fwnode_count_parents);
+
+/**
+ * fwnode_get_nth_parent - Return an nth parent of a node
+ * @fwnode: The node the parent of which is requested
+ * @depth: Distance of the parent from the node
+ *
+ * Returns the nth parent of a node. If there is no parent at the requested
+ * @depth, %NULL is returned. If @depth is 0, the functionality is equivalent to
+ * fwnode_handle_get(). For @depth == 1, it is fwnode_get_parent() and so on.
+ *
+ * The caller is responsible for calling fwnode_handle_put() for the returned
+ * node.
+ */
+struct fwnode_handle *fwnode_get_nth_parent(struct fwnode_handle *fwnode,
+ unsigned int depth)
+{
+ unsigned int i;
+
+ fwnode_handle_get(fwnode);
+
+ for (i = 0; i < depth && fwnode; i++)
+ fwnode = fwnode_get_next_parent(fwnode);
+
+ return fwnode;
+}
+EXPORT_SYMBOL_GPL(fwnode_get_nth_parent);
+
/**
* fwnode_get_next_child_node - Return the next child node handle for a node
* @fwnode: Firmware node to find the next child node for.
diff --git a/include/linux/property.h b/include/linux/property.h
index 421c76e53708d..5450e7ec219ac 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -83,6 +83,9 @@ struct fwnode_handle *fwnode_find_reference(const struct fwnode_handle *fwnode,
struct fwnode_handle *fwnode_get_parent(const struct fwnode_handle *fwnode);
struct fwnode_handle *fwnode_get_next_parent(
struct fwnode_handle *fwnode);
+unsigned int fwnode_count_parents(const struct fwnode_handle *fwn);
+struct fwnode_handle *fwnode_get_nth_parent(struct fwnode_handle *fwn,
+ unsigned int depth);
struct fwnode_handle *fwnode_get_next_child_node(
const struct fwnode_handle *fwnode, struct fwnode_handle *child);
struct fwnode_handle *fwnode_get_next_available_child_node(
--
2.20.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v6 06/12] device property: Add fwnode_get_name for returning the name of a node
2019-09-10 8:46 [PATCH v6 00/12] Device property improvements, add %pfw format specifier Sakari Ailus
` (4 preceding siblings ...)
2019-09-10 8:47 ` [PATCH v6 05/12] device property: Add functions for accessing node's parents Sakari Ailus
@ 2019-09-10 8:47 ` Sakari Ailus
2019-09-10 8:47 ` [PATCH v6 07/12] device property: Add a function to obtain a node's prefix Sakari Ailus
` (5 subsequent siblings)
11 siblings, 0 replies; 25+ messages in thread
From: Sakari Ailus @ 2019-09-10 8:47 UTC (permalink / raw)
To: Petr Mladek, linux-kernel, rafael
Cc: Andy Shevchenko, linux-acpi, devicetree, Rob Herring,
Heikki Krogerus, Joe Perches
The fwnode framework did not have means to obtain the name of a node. Add
that now, in form of the fwnode_get_name() function and a corresponding
get_name fwnode op. OF and ACPI support is included.
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Acked-by: Rob Herring <robh@kernel.org> (for OF)
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/acpi/property.c | 26 ++++++++++++++++++++++++++
drivers/base/property.c | 11 +++++++++++
drivers/base/swnode.c | 12 ++++++++++++
drivers/of/property.c | 6 ++++++
include/linux/fwnode.h | 2 ++
include/linux/property.h | 1 +
6 files changed, 58 insertions(+)
diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
index ea3d700da3ca6..5a9397a390f41 100644
--- a/drivers/acpi/property.c
+++ b/drivers/acpi/property.c
@@ -1311,6 +1311,31 @@ acpi_fwnode_get_reference_args(const struct fwnode_handle *fwnode,
args_count, args);
}
+static const char *acpi_fwnode_get_name(const struct fwnode_handle *fwnode)
+{
+ const struct acpi_device *adev;
+ struct fwnode_handle *parent;
+
+ /* Is this the root node? */
+ parent = fwnode_get_parent(fwnode);
+ if (!parent)
+ return "\\";
+
+ fwnode_handle_put(parent);
+
+ if (is_acpi_data_node(fwnode)) {
+ const struct acpi_data_node *dn = to_acpi_data_node(fwnode);
+
+ return dn->name;
+ }
+
+ adev = to_acpi_device_node(fwnode);
+ if (WARN_ON(!adev))
+ return NULL;
+
+ return acpi_device_bid(adev);
+}
+
static struct fwnode_handle *
acpi_fwnode_get_parent(struct fwnode_handle *fwnode)
{
@@ -1351,6 +1376,7 @@ acpi_fwnode_device_get_match_data(const struct fwnode_handle *fwnode,
.get_parent = acpi_node_get_parent, \
.get_next_child_node = acpi_get_next_subnode, \
.get_named_child_node = acpi_fwnode_get_named_child_node, \
+ .get_name = acpi_fwnode_get_name, \
.get_reference_args = acpi_fwnode_get_reference_args, \
.graph_get_next_endpoint = \
acpi_graph_get_next_endpoint, \
diff --git a/drivers/base/property.c b/drivers/base/property.c
index f2e555e68b56a..9b5ec88e72d8b 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -556,6 +556,17 @@ int device_add_properties(struct device *dev,
}
EXPORT_SYMBOL_GPL(device_add_properties);
+/**
+ * fwnode_get_name - Return the name of a node
+ * @fwnode: The firmware node
+ *
+ * Returns a pointer to the node name.
+ */
+const char *fwnode_get_name(const struct fwnode_handle *fwnode)
+{
+ return fwnode_call_ptr_op(fwnode, get_name);
+}
+
/**
* fwnode_get_parent - Return parent firwmare node
* @fwnode: Firmware whose parent is retrieved
diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c
index 951e7efd47c23..ccbc3d9d6c5f5 100644
--- a/drivers/base/swnode.c
+++ b/drivers/base/swnode.c
@@ -515,6 +515,17 @@ static int software_node_read_string_array(const struct fwnode_handle *fwnode,
propname, val, nval);
}
+static const char *
+software_node_get_name(const struct fwnode_handle *fwnode)
+{
+ const struct swnode *swnode = to_swnode(fwnode);
+
+ if (!swnode)
+ return "(null)";
+
+ return kobject_name(&swnode->kobj);
+}
+
static struct fwnode_handle *
software_node_get_parent(const struct fwnode_handle *fwnode)
{
@@ -615,6 +626,7 @@ static const struct fwnode_operations software_node_ops = {
.property_present = software_node_property_present,
.property_read_int_array = software_node_read_int_array,
.property_read_string_array = software_node_read_string_array,
+ .get_name = software_node_get_name,
.get_parent = software_node_get_parent,
.get_next_child_node = software_node_get_next_child,
.get_named_child_node = software_node_get_named_child_node,
diff --git a/drivers/of/property.c b/drivers/of/property.c
index d7fa75e31f224..5bed634551ea6 100644
--- a/drivers/of/property.c
+++ b/drivers/of/property.c
@@ -872,6 +872,11 @@ of_fwnode_property_read_string_array(const struct fwnode_handle *fwnode,
of_property_count_strings(node, propname);
}
+static const char *of_fwnode_get_name(const struct fwnode_handle *fwnode)
+{
+ return kbasename(to_of_node(fwnode)->full_name);
+}
+
static struct fwnode_handle *
of_fwnode_get_parent(const struct fwnode_handle *fwnode)
{
@@ -993,6 +998,7 @@ const struct fwnode_operations of_fwnode_ops = {
.property_present = of_fwnode_property_present,
.property_read_int_array = of_fwnode_property_read_int_array,
.property_read_string_array = of_fwnode_property_read_string_array,
+ .get_name = of_fwnode_get_name,
.get_parent = of_fwnode_get_parent,
.get_next_child_node = of_fwnode_get_next_child_node,
.get_named_child_node = of_fwnode_get_named_child_node,
diff --git a/include/linux/fwnode.h b/include/linux/fwnode.h
index a11c8c56c78b4..c331e0ef31e80 100644
--- a/include/linux/fwnode.h
+++ b/include/linux/fwnode.h
@@ -56,6 +56,7 @@ struct fwnode_reference_args {
* otherwise.
* @property_read_string_array: Read an array of string properties. Return zero
* on success, a negative error code otherwise.
+ * @get_name: Return the name of an fwnode.
* @get_parent: Return the parent of an fwnode.
* @get_next_child_node: Return the next child node in an iteration.
* @get_named_child_node: Return a child node with a given name.
@@ -82,6 +83,7 @@ struct fwnode_operations {
(*property_read_string_array)(const struct fwnode_handle *fwnode_handle,
const char *propname, const char **val,
size_t nval);
+ const char *(*get_name)(const struct fwnode_handle *fwnode);
struct fwnode_handle *(*get_parent)(const struct fwnode_handle *fwnode);
struct fwnode_handle *
(*get_next_child_node)(const struct fwnode_handle *fwnode,
diff --git a/include/linux/property.h b/include/linux/property.h
index 5450e7ec219ac..ebc5e2016bb66 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -80,6 +80,7 @@ struct fwnode_handle *fwnode_find_reference(const struct fwnode_handle *fwnode,
const char *name,
unsigned int index);
+const char *fwnode_get_name(const struct fwnode_handle *fwnode);
struct fwnode_handle *fwnode_get_parent(const struct fwnode_handle *fwnode);
struct fwnode_handle *fwnode_get_next_parent(
struct fwnode_handle *fwnode);
--
2.20.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v6 07/12] device property: Add a function to obtain a node's prefix
2019-09-10 8:46 [PATCH v6 00/12] Device property improvements, add %pfw format specifier Sakari Ailus
` (5 preceding siblings ...)
2019-09-10 8:47 ` [PATCH v6 06/12] device property: Add fwnode_get_name for returning the name of a node Sakari Ailus
@ 2019-09-10 8:47 ` Sakari Ailus
2019-09-10 8:47 ` [PATCH v6 08/12] lib/vsprintf: Remove support for %pF and %pf in favour of %pS and %ps Sakari Ailus
` (4 subsequent siblings)
11 siblings, 0 replies; 25+ messages in thread
From: Sakari Ailus @ 2019-09-10 8:47 UTC (permalink / raw)
To: Petr Mladek, linux-kernel, rafael
Cc: Andy Shevchenko, linux-acpi, devicetree, Rob Herring,
Heikki Krogerus, Joe Perches
The prefix is used for printing purpose before a node, and it also works
as a separator between two nodes.
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Acked-by: Rob Herring <robh@kernel.org> (for OF)
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/acpi/property.c | 22 ++++++++++++++++++++++
drivers/base/property.c | 12 ++++++++++++
drivers/base/swnode.c | 22 ++++++++++++++++++++++
drivers/of/property.c | 10 ++++++++++
include/linux/fwnode.h | 2 ++
include/linux/property.h | 1 +
6 files changed, 69 insertions(+)
diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
index 5a9397a390f41..466239d3bb345 100644
--- a/drivers/acpi/property.c
+++ b/drivers/acpi/property.c
@@ -1336,6 +1336,27 @@ static const char *acpi_fwnode_get_name(const struct fwnode_handle *fwnode)
return acpi_device_bid(adev);
}
+static const char *
+acpi_fwnode_get_name_prefix(const struct fwnode_handle *fwnode)
+{
+ struct fwnode_handle *parent;
+
+ /* Is this the root node? */
+ parent = fwnode_get_parent(fwnode);
+ if (!parent)
+ return "";
+
+ /* Is this 2nd node from the root? */
+ parent = fwnode_get_next_parent(parent);
+ if (!parent)
+ return "";
+
+ fwnode_handle_put(parent);
+
+ /* ACPI device or data node. */
+ return ".";
+}
+
static struct fwnode_handle *
acpi_fwnode_get_parent(struct fwnode_handle *fwnode)
{
@@ -1377,6 +1398,7 @@ acpi_fwnode_device_get_match_data(const struct fwnode_handle *fwnode,
.get_next_child_node = acpi_get_next_subnode, \
.get_named_child_node = acpi_fwnode_get_named_child_node, \
.get_name = acpi_fwnode_get_name, \
+ .get_name_prefix = acpi_fwnode_get_name_prefix, \
.get_reference_args = acpi_fwnode_get_reference_args, \
.graph_get_next_endpoint = \
acpi_graph_get_next_endpoint, \
diff --git a/drivers/base/property.c b/drivers/base/property.c
index 9b5ec88e72d8b..511f6d7acdfe4 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -567,6 +567,18 @@ const char *fwnode_get_name(const struct fwnode_handle *fwnode)
return fwnode_call_ptr_op(fwnode, get_name);
}
+/**
+ * fwnode_get_name_prefix - Return the prefix of node for printing purposes
+ * @fwnode: The firmware node
+ *
+ * Returns the prefix of a node, intended to be printed right before the node.
+ * The prefix works also as a separator between the nodes.
+ */
+const char *fwnode_get_name_prefix(const struct fwnode_handle *fwnode)
+{
+ return fwnode_call_ptr_op(fwnode, get_name_prefix);
+}
+
/**
* fwnode_get_parent - Return parent firwmare node
* @fwnode: Firmware whose parent is retrieved
diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c
index ccbc3d9d6c5f5..6a8c427355f8d 100644
--- a/drivers/base/swnode.c
+++ b/drivers/base/swnode.c
@@ -526,6 +526,27 @@ software_node_get_name(const struct fwnode_handle *fwnode)
return kobject_name(&swnode->kobj);
}
+static const char *
+software_node_get_name_prefix(const struct fwnode_handle *fwnode)
+{
+ struct fwnode_handle *parent;
+ const char *prefix;
+
+ parent = fwnode_get_parent(fwnode);
+ if (!parent)
+ return "";
+
+ /* Figure out the prefix from the parents. */
+ while (is_software_node(parent))
+ parent = fwnode_get_next_parent(parent);
+
+ prefix = fwnode_get_name_prefix(parent);
+ fwnode_handle_put(parent);
+
+ /* Guess something if prefix was NULL. */
+ return prefix ?: "/";
+}
+
static struct fwnode_handle *
software_node_get_parent(const struct fwnode_handle *fwnode)
{
@@ -627,6 +648,7 @@ static const struct fwnode_operations software_node_ops = {
.property_read_int_array = software_node_read_int_array,
.property_read_string_array = software_node_read_string_array,
.get_name = software_node_get_name,
+ .get_name_prefix = software_node_get_name_prefix,
.get_parent = software_node_get_parent,
.get_next_child_node = software_node_get_next_child,
.get_named_child_node = software_node_get_named_child_node,
diff --git a/drivers/of/property.c b/drivers/of/property.c
index 5bed634551ea6..e8202f61a5d93 100644
--- a/drivers/of/property.c
+++ b/drivers/of/property.c
@@ -877,6 +877,15 @@ static const char *of_fwnode_get_name(const struct fwnode_handle *fwnode)
return kbasename(to_of_node(fwnode)->full_name);
}
+static const char *of_fwnode_get_name_prefix(const struct fwnode_handle *fwnode)
+{
+ /* Root needs no prefix here (its name is "/"). */
+ if (!to_of_node(fwnode)->parent)
+ return "";
+
+ return "/";
+}
+
static struct fwnode_handle *
of_fwnode_get_parent(const struct fwnode_handle *fwnode)
{
@@ -999,6 +1008,7 @@ const struct fwnode_operations of_fwnode_ops = {
.property_read_int_array = of_fwnode_property_read_int_array,
.property_read_string_array = of_fwnode_property_read_string_array,
.get_name = of_fwnode_get_name,
+ .get_name_prefix = of_fwnode_get_name_prefix,
.get_parent = of_fwnode_get_parent,
.get_next_child_node = of_fwnode_get_next_child_node,
.get_named_child_node = of_fwnode_get_named_child_node,
diff --git a/include/linux/fwnode.h b/include/linux/fwnode.h
index c331e0ef31e80..755709703fe6f 100644
--- a/include/linux/fwnode.h
+++ b/include/linux/fwnode.h
@@ -57,6 +57,7 @@ struct fwnode_reference_args {
* @property_read_string_array: Read an array of string properties. Return zero
* on success, a negative error code otherwise.
* @get_name: Return the name of an fwnode.
+ * @get_name_prefix: Get a prefix for a node (for printing purposes).
* @get_parent: Return the parent of an fwnode.
* @get_next_child_node: Return the next child node in an iteration.
* @get_named_child_node: Return a child node with a given name.
@@ -84,6 +85,7 @@ struct fwnode_operations {
const char *propname, const char **val,
size_t nval);
const char *(*get_name)(const struct fwnode_handle *fwnode);
+ const char *(*get_name_prefix)(const struct fwnode_handle *fwnode);
struct fwnode_handle *(*get_parent)(const struct fwnode_handle *fwnode);
struct fwnode_handle *
(*get_next_child_node)(const struct fwnode_handle *fwnode,
diff --git a/include/linux/property.h b/include/linux/property.h
index ebc5e2016bb66..c40fcfc5774a7 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -81,6 +81,7 @@ struct fwnode_handle *fwnode_find_reference(const struct fwnode_handle *fwnode,
unsigned int index);
const char *fwnode_get_name(const struct fwnode_handle *fwnode);
+const char *fwnode_get_name_prefix(const struct fwnode_handle *fwnode);
struct fwnode_handle *fwnode_get_parent(const struct fwnode_handle *fwnode);
struct fwnode_handle *fwnode_get_next_parent(
struct fwnode_handle *fwnode);
--
2.20.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v6 08/12] lib/vsprintf: Remove support for %pF and %pf in favour of %pS and %ps
2019-09-10 8:46 [PATCH v6 00/12] Device property improvements, add %pfw format specifier Sakari Ailus
` (6 preceding siblings ...)
2019-09-10 8:47 ` [PATCH v6 07/12] device property: Add a function to obtain a node's prefix Sakari Ailus
@ 2019-09-10 8:47 ` Sakari Ailus
2019-09-10 8:47 ` [PATCH v6 09/12] lib/vsprintf: Make use of fwnode API to obtain node names and separators Sakari Ailus
` (3 subsequent siblings)
11 siblings, 0 replies; 25+ messages in thread
From: Sakari Ailus @ 2019-09-10 8:47 UTC (permalink / raw)
To: Petr Mladek, linux-kernel, rafael
Cc: Andy Shevchenko, linux-acpi, devicetree, Rob Herring,
Heikki Krogerus, Joe Perches
%pS and %ps are now the preferred conversion specifiers to print function
names. The functionality is equivalent; remove the old, deprecated %pF
and %pf support.
Depends-on: commit 2d44d165e939 ("scsi: lpfc: Convert existing %pf users to %ps")
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
Documentation/core-api/printk-formats.rst | 10 ----------
lib/vsprintf.c | 8 ++------
scripts/checkpatch.pl | 1 -
3 files changed, 2 insertions(+), 17 deletions(-)
diff --git a/Documentation/core-api/printk-formats.rst b/Documentation/core-api/printk-formats.rst
index c6224d039bcbe..922a29eb70e6c 100644
--- a/Documentation/core-api/printk-formats.rst
+++ b/Documentation/core-api/printk-formats.rst
@@ -86,8 +86,6 @@ Symbols/Function Pointers
%pS versatile_init+0x0/0x110
%ps versatile_init
- %pF versatile_init+0x0/0x110
- %pf versatile_init
%pSR versatile_init+0x9/0x110
(with __builtin_extract_return_addr() translation)
%pB prev_fn_of_versatile_init+0x88/0x88
@@ -97,14 +95,6 @@ The ``S`` and ``s`` specifiers are used for printing a pointer in symbolic
format. They result in the symbol name with (S) or without (s)
offsets. If KALLSYMS are disabled then the symbol address is printed instead.
-Note, that the ``F`` and ``f`` specifiers are identical to ``S`` (``s``)
-and thus deprecated. We have ``F`` and ``f`` because on ia64, ppc64 and
-parisc64 function pointers are indirect and, in fact, are function
-descriptors, which require additional dereferencing before we can lookup
-the symbol. As of now, ``S`` and ``s`` perform dereferencing on those
-platforms (when needed), so ``F`` and ``f`` exist for compatibility
-reasons only.
-
The ``B`` specifier results in the symbol name with offsets and should be
used when printing stack backtraces. The specifier takes into
consideration the effect of compiler optimisations which may occur
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index b0967cf17137d..b00b57f9f911f 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -909,7 +909,7 @@ char *symbol_string(char *buf, char *end, void *ptr,
#ifdef CONFIG_KALLSYMS
if (*fmt == 'B')
sprint_backtrace(sym, value);
- else if (*fmt != 'f' && *fmt != 's')
+ else if (*fmt != 's')
sprint_symbol(sym, value);
else
sprint_symbol_no_offset(sym, value);
@@ -2007,9 +2007,7 @@ static char *kobject_string(char *buf, char *end, void *ptr,
*
* - 'S' For symbolic direct pointers (or function descriptors) with offset
* - 's' For symbolic direct pointers (or function descriptors) without offset
- * - 'F' Same as 'S'
- * - 'f' Same as 's'
- * - '[FfSs]R' as above with __builtin_extract_return_addr() translation
+ * - '[Ss]R' as above with __builtin_extract_return_addr() translation
* - 'B' For backtraced symbolic direct pointers with offset
* - 'R' For decoded struct resource, e.g., [mem 0x0-0x1f 64bit pref]
* - 'r' For raw struct resource, e.g., [mem 0x0-0x1f flags 0x201]
@@ -2112,8 +2110,6 @@ char *pointer(const char *fmt, char *buf, char *end, void *ptr,
struct printf_spec spec)
{
switch (*fmt) {
- case 'F':
- case 'f':
case 'S':
case 's':
ptr = dereference_symbol_descriptor(ptr);
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 93a7edfe0f059..a60c241112cd4 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -6012,7 +6012,6 @@ sub process {
my $ext_type = "Invalid";
my $use = "";
if ($bad_specifier =~ /p[Ff]/) {
- $ext_type = "Deprecated";
$use = " - use %pS instead";
$use =~ s/pS/ps/ if ($bad_specifier =~ /pf/);
}
--
2.20.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v6 09/12] lib/vsprintf: Make use of fwnode API to obtain node names and separators
2019-09-10 8:46 [PATCH v6 00/12] Device property improvements, add %pfw format specifier Sakari Ailus
` (7 preceding siblings ...)
2019-09-10 8:47 ` [PATCH v6 08/12] lib/vsprintf: Remove support for %pF and %pf in favour of %pS and %ps Sakari Ailus
@ 2019-09-10 8:47 ` Sakari Ailus
2019-09-10 8:47 ` [PATCH v6 10/12] lib/vsprintf: OF nodes are first and foremost, struct device_nodes Sakari Ailus
` (2 subsequent siblings)
11 siblings, 0 replies; 25+ messages in thread
From: Sakari Ailus @ 2019-09-10 8:47 UTC (permalink / raw)
To: Petr Mladek, linux-kernel, rafael
Cc: Andy Shevchenko, linux-acpi, devicetree, Rob Herring,
Heikki Krogerus, Joe Perches
Instead of implementing our own means of discovering parent nodes, node
names or counting how many parents a node has, use the newly added
functions in the fwnode API to obtain that information.
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Petr Mladek <pmladek@suse.com>
---
lib/vsprintf.c | 39 +++++++++++++++++----------------------
1 file changed, 17 insertions(+), 22 deletions(-)
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index b00b57f9f911f..7a0006e0c3896 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -38,6 +38,7 @@
#include <net/addrconf.h>
#include <linux/siphash.h>
#include <linux/compiler.h>
+#include <linux/property.h>
#ifdef CONFIG_BLOCK
#include <linux/blkdev.h>
#endif
@@ -1863,32 +1864,25 @@ char *flags_string(char *buf, char *end, void *flags_ptr,
return format_flags(buf, end, flags, names);
}
-static const char *device_node_name_for_depth(const struct device_node *np, int depth)
-{
- for ( ; np && depth; depth--)
- np = np->parent;
-
- return kbasename(np->full_name);
-}
-
static noinline_for_stack
-char *device_node_gen_full_name(const struct device_node *np, char *buf, char *end)
+char *fwnode_full_name_string(struct fwnode_handle *fwnode, char *buf,
+ char *end)
{
int depth;
- const struct device_node *parent = np->parent;
- /* special case for root node */
- if (!parent)
- return string_nocheck(buf, end, "/", default_str_spec);
+ /* Loop starting from the root node to the current node. */
+ for (depth = fwnode_count_parents(fwnode); depth >= 0; depth--) {
+ struct fwnode_handle *__fwnode =
+ fwnode_get_nth_parent(fwnode, depth);
- for (depth = 0; parent->parent; depth++)
- parent = parent->parent;
-
- for ( ; depth >= 0; depth--) {
- buf = string_nocheck(buf, end, "/", default_str_spec);
- buf = string(buf, end, device_node_name_for_depth(np, depth),
+ buf = string(buf, end, fwnode_get_name_prefix(__fwnode),
+ default_str_spec);
+ buf = string(buf, end, fwnode_get_name(__fwnode),
default_str_spec);
+
+ fwnode_handle_put(__fwnode);
}
+
return buf;
}
@@ -1933,10 +1927,11 @@ char *device_node_string(char *buf, char *end, struct device_node *dn,
switch (*fmt) {
case 'f': /* full_name */
- buf = device_node_gen_full_name(dn, buf, end);
+ buf = fwnode_full_name_string(of_fwnode_handle(dn), buf,
+ end);
break;
case 'n': /* name */
- p = kbasename(of_node_full_name(dn));
+ p = fwnode_get_name(of_fwnode_handle(dn));
precision = str_spec.precision;
str_spec.precision = strchrnul(p, '@') - p;
buf = string(buf, end, p, str_spec);
@@ -1946,7 +1941,7 @@ char *device_node_string(char *buf, char *end, struct device_node *dn,
buf = number(buf, end, (unsigned int)dn->phandle, num_spec);
break;
case 'P': /* path-spec */
- p = kbasename(of_node_full_name(dn));
+ p = fwnode_get_name(of_fwnode_handle(dn));
if (!p[1])
p = "/";
buf = string(buf, end, p, str_spec);
--
2.20.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v6 10/12] lib/vsprintf: OF nodes are first and foremost, struct device_nodes
2019-09-10 8:46 [PATCH v6 00/12] Device property improvements, add %pfw format specifier Sakari Ailus
` (8 preceding siblings ...)
2019-09-10 8:47 ` [PATCH v6 09/12] lib/vsprintf: Make use of fwnode API to obtain node names and separators Sakari Ailus
@ 2019-09-10 8:47 ` Sakari Ailus
2019-09-10 8:47 ` [PATCH v6 11/12] lib/vsprintf: Add %pfw conversion specifier for printing fwnode names Sakari Ailus
2019-09-10 8:47 ` [PATCH v6 12/12] lib/test_printf: Add tests for %pfw printk modifier Sakari Ailus
11 siblings, 0 replies; 25+ messages in thread
From: Sakari Ailus @ 2019-09-10 8:47 UTC (permalink / raw)
To: Petr Mladek, linux-kernel, rafael
Cc: Andy Shevchenko, linux-acpi, devicetree, Rob Herring,
Heikki Krogerus, Joe Perches
Factor out static kobject_string() function that simply calls
device_node_string(), and thus remove references to kobjects (as these are
struct device_node).
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Petr Mladek <pmladek@suse.com>
---
lib/vsprintf.c | 16 ++++------------
1 file changed, 4 insertions(+), 12 deletions(-)
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 7a0006e0c3896..9c5b398d4a512 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -1906,6 +1906,9 @@ char *device_node_string(char *buf, char *end, struct device_node *dn,
struct printf_spec str_spec = spec;
str_spec.field_width = -1;
+ if (fmt[0] != 'F')
+ return error_string(buf, end, "(%pO?)", spec);
+
if (!IS_ENABLED(CONFIG_OF))
return error_string(buf, end, "(%pOF?)", spec);
@@ -1979,17 +1982,6 @@ char *device_node_string(char *buf, char *end, struct device_node *dn,
return widen_string(buf, buf - buf_start, end, spec);
}
-static char *kobject_string(char *buf, char *end, void *ptr,
- struct printf_spec spec, const char *fmt)
-{
- switch (fmt[1]) {
- case 'F':
- return device_node_string(buf, end, ptr, spec, fmt + 1);
- }
-
- return error_string(buf, end, "(%pO?)", spec);
-}
-
/*
* Show a '%p' thing. A kernel extension is that the '%p' is followed
* by an extra set of alphanumeric characters that are extended format
@@ -2168,7 +2160,7 @@ char *pointer(const char *fmt, char *buf, char *end, void *ptr,
case 'G':
return flags_string(buf, end, ptr, spec, fmt);
case 'O':
- return kobject_string(buf, end, ptr, spec, fmt);
+ return device_node_string(buf, end, ptr, spec, fmt + 1);
case 'x':
return pointer_string(buf, end, ptr, spec);
}
--
2.20.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v6 11/12] lib/vsprintf: Add %pfw conversion specifier for printing fwnode names
2019-09-10 8:46 [PATCH v6 00/12] Device property improvements, add %pfw format specifier Sakari Ailus
` (9 preceding siblings ...)
2019-09-10 8:47 ` [PATCH v6 10/12] lib/vsprintf: OF nodes are first and foremost, struct device_nodes Sakari Ailus
@ 2019-09-10 8:47 ` Sakari Ailus
2019-09-10 10:21 ` Joe Perches
2019-09-10 8:47 ` [PATCH v6 12/12] lib/test_printf: Add tests for %pfw printk modifier Sakari Ailus
11 siblings, 1 reply; 25+ messages in thread
From: Sakari Ailus @ 2019-09-10 8:47 UTC (permalink / raw)
To: Petr Mladek, linux-kernel, rafael
Cc: Andy Shevchenko, linux-acpi, devicetree, Rob Herring,
Heikki Krogerus, Joe Perches
Add support for %pfw conversion specifier (with "f" and "P" modifiers) to
support printing full path of the node, including its name ("f") and only
the node's name ("P") in the printk family of functions. The two flags
have equivalent functionality to existing %pOF with the same two modifiers
("f" and "P") on OF based systems. The ability to do the same on ACPI
based systems is added by this patch.
On ACPI based systems the resulting strings look like
\_SB.PCI0.CIO2.port@1.endpoint@0
where the nodes are separated by a dot (".") and the first three are
ACPI device nodes and the latter two ACPI data nodes.
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
Documentation/core-api/printk-formats.rst | 24 +++++++++++++++
lib/vsprintf.c | 36 +++++++++++++++++++++++
scripts/checkpatch.pl | 7 +++--
3 files changed, 65 insertions(+), 2 deletions(-)
diff --git a/Documentation/core-api/printk-formats.rst b/Documentation/core-api/printk-formats.rst
index 922a29eb70e6c..1ab3841686dc5 100644
--- a/Documentation/core-api/printk-formats.rst
+++ b/Documentation/core-api/printk-formats.rst
@@ -418,6 +418,30 @@ Examples::
Passed by reference.
+Fwnode handles
+--------------
+
+::
+
+ %pfw[fP]
+
+For printing information on fwnode handles. The default is to print the full
+node name, including the path. The modifiers are functionally equivalent to
+%pOF above.
+
+ - f - full name of the node, including the path
+ - P - the name of the node including an address (if there is one)
+
+Examples (ACPI)::
+
+ %pfwf \_SB.PCI0.CIO2.port@1.endpoint@0 - Full node name
+ %pfwP endpoint@0 - Node name
+
+Examples (OF)::
+
+ %pfwf /ocp@68000000/i2c@48072000/camera@10/port/endpoint - Full name
+ %pfwP endpoint - Node name
+
Time and date (struct rtc_time)
-------------------------------
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 9c5b398d4a512..51c81921598d8 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -1982,6 +1982,36 @@ char *device_node_string(char *buf, char *end, struct device_node *dn,
return widen_string(buf, buf - buf_start, end, spec);
}
+static noinline_for_stack
+char *fwnode_string(char *buf, char *end, struct fwnode_handle *fwnode,
+ struct printf_spec spec, const char *fmt)
+{
+ struct printf_spec str_spec = spec;
+ char *buf_start = buf;
+
+ str_spec.field_width = -1;
+
+ if (*fmt != 'w')
+ return error_string(buf, end, "(%pf?)", spec);
+
+ if (check_pointer(&buf, end, fwnode, spec))
+ return buf;
+
+ fmt++;
+
+ switch (*fmt) {
+ case 'P': /* name */
+ buf = string(buf, end, fwnode_get_name(fwnode), str_spec);
+ break;
+ case 'f': /* full_name */
+ default:
+ buf = fwnode_full_name_string(fwnode, buf, end);
+ break;
+ }
+
+ return widen_string(buf, buf - buf_start, end, spec);
+}
+
/*
* Show a '%p' thing. A kernel extension is that the '%p' is followed
* by an extra set of alphanumeric characters that are extended format
@@ -2084,6 +2114,10 @@ char *device_node_string(char *buf, char *end, struct device_node *dn,
* F device node flags
* c major compatible string
* C full compatible string
+ * - 'fw[fP]' For a firmware node (struct fwnode_handle) pointer
+ * Without an option prints the full name of the node
+ * f full name
+ * P node name, including a possible unit address
* - 'x' For printing the address. Equivalent to "%lx".
*
* ** When making changes please also update:
@@ -2161,6 +2195,8 @@ char *pointer(const char *fmt, char *buf, char *end, void *ptr,
return flags_string(buf, end, ptr, spec, fmt);
case 'O':
return device_node_string(buf, end, ptr, spec, fmt + 1);
+ case 'f':
+ return fwnode_string(buf, end, ptr, spec, fmt + 1);
case 'x':
return pointer_string(buf, end, ptr, spec);
}
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index a60c241112cd4..93470283ea243 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -5988,14 +5988,17 @@ sub process {
for (my $count = $linenr; $count <= $lc; $count++) {
my $specifier;
my $extension;
+ my $qualifier;
my $bad_specifier = "";
my $fmt = get_quoted_string($lines[$count - 1], raw_line($count, 0));
$fmt =~ s/%%//g;
- while ($fmt =~ /(\%[\*\d\.]*p(\w))/g) {
+ while ($fmt =~ /(\%[\*\d\.]*p(\w)(\w?))/g) {
$specifier = $1;
$extension = $2;
- if ($extension !~ /[SsBKRraEhMmIiUDdgVCbGNOxt]/) {
+ $qualifier = $3;
+ if ($extension !~ /[SsBKRraEhMmIiUDdgVCbGNOxtf]/ ||
+ ($extension eq "f" && $qualifier ne "w")) {
$bad_specifier = $specifier;
last;
}
--
2.20.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH v6 11/12] lib/vsprintf: Add %pfw conversion specifier for printing fwnode names
2019-09-10 8:47 ` [PATCH v6 11/12] lib/vsprintf: Add %pfw conversion specifier for printing fwnode names Sakari Ailus
@ 2019-09-10 10:21 ` Joe Perches
2019-09-18 13:11 ` Sakari Ailus
0 siblings, 1 reply; 25+ messages in thread
From: Joe Perches @ 2019-09-10 10:21 UTC (permalink / raw)
To: Sakari Ailus, Petr Mladek, linux-kernel, rafael
Cc: Andy Shevchenko, linux-acpi, devicetree, Rob Herring, Heikki Krogerus
On Tue, 2019-09-10 at 11:47 +0300, Sakari Ailus wrote:
> Add support for %pfw conversion specifier (with "f" and "P" modifiers) to
> support printing full path of the node, including its name ("f") and only
> the node's name ("P") in the printk family of functions. The two flags
> have equivalent functionality to existing %pOF with the same two modifiers
> ("f" and "P") on OF based systems. The ability to do the same on ACPI
> based systems is added by this patch.
checkpatch comment below:
A.
> diff --git a/Documentation/core-api/printk-formats.rst b/Documentation/core-api/printk-formats.rst
[]
> @@ -418,6 +418,30 @@ Examples::
>
> Passed by reference.
>
> +Fwnode handles
> +--------------
> +
> +::
> +
> + %pfw[fP]
> +
> +For printing information on fwnode handles. The default is to print the full
> +node name, including the path. The modifiers are functionally equivalent to
> +%pOF above.
> +
> + - f - full name of the node, including the path
> + - P - the name of the node including an address (if there is one)
> +
> +Examples (ACPI)::
> +
> + %pfwf \_SB.PCI0.CIO2.port@1.endpoint@0 - Full node name
> + %pfwP endpoint@0 - Node name
> +
> +Examples (OF)::
> +
> + %pfwf /ocp@68000000/i2c@48072000/camera@10/port/endpoint - Full name
> + %pfwP endpoint - Node name
[]
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
[]
> @@ -5988,14 +5988,17 @@ sub process {
> for (my $count = $linenr; $count <= $lc; $count++) {
> my $specifier;
> my $extension;
> + my $qualifier;
> my $bad_specifier = "";
> my $fmt = get_quoted_string($lines[$count - 1], raw_line($count, 0));
> $fmt =~ s/%%//g;
>
> - while ($fmt =~ /(\%[\*\d\.]*p(\w))/g) {
> + while ($fmt =~ /(\%[\*\d\.]*p(\w)(\w?))/g) {
This is not generic enough to allow all possible qualifiers
to be captured.
This should be something like:
while ($fmt =~ /(\%[\*\d\.]*p(\w)(\w*))/g) {
> $specifier = $1;
> $extension = $2;
> - if ($extension !~ /[SsBKRraEhMmIiUDdgVCbGNOxt]/) {
> + $qualifier = $3;
> + if ($extension !~ /[SsBKRraEhMmIiUDdgVCbGNOxtf]/ ||
> + ($extension eq "f" && $qualifier ne "w")) {
And this should likely test
(defined $qualifier &&
$extension eq 'f' && $qualifier =~ /^w/))
> $bad_specifier = $specifier;
> last;
> }
And here the 'f' isn't really a bad $specifier,
it does not have the correct 'w' $extension.
The valid extensions may change in the future and
this code isn't particularly manageable, so I
think the $specifier/$extension pair should likely
have some additional testing after validating
the $specifier.
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH v6 11/12] lib/vsprintf: Add %pfw conversion specifier for printing fwnode names
2019-09-10 10:21 ` Joe Perches
@ 2019-09-18 13:11 ` Sakari Ailus
0 siblings, 0 replies; 25+ messages in thread
From: Sakari Ailus @ 2019-09-18 13:11 UTC (permalink / raw)
To: Joe Perches
Cc: Petr Mladek, linux-kernel, rafael, Andy Shevchenko, linux-acpi,
devicetree, Rob Herring, Heikki Krogerus
Hi Joe,
On Tue, Sep 10, 2019 at 03:21:26AM -0700, Joe Perches wrote:
> > @@ -5988,14 +5988,17 @@ sub process {
> > for (my $count = $linenr; $count <= $lc; $count++) {
> > my $specifier;
> > my $extension;
> > + my $qualifier;
> > my $bad_specifier = "";
> > my $fmt = get_quoted_string($lines[$count - 1], raw_line($count, 0));
> > $fmt =~ s/%%//g;
> >
> > - while ($fmt =~ /(\%[\*\d\.]*p(\w))/g) {
> > + while ($fmt =~ /(\%[\*\d\.]*p(\w)(\w?))/g) {
>
> This is not generic enough to allow all possible qualifiers
> to be captured.
>
> This should be something like:
>
> while ($fmt =~ /(\%[\*\d\.]*p(\w)(\w*))/g) {
>
> > $specifier = $1;
> > $extension = $2;
> > - if ($extension !~ /[SsBKRraEhMmIiUDdgVCbGNOxt]/) {
> > + $qualifier = $3;
> > + if ($extension !~ /[SsBKRraEhMmIiUDdgVCbGNOxtf]/ ||
> > + ($extension eq "f" && $qualifier ne "w")) {
>
>
> And this should likely test
> (defined $qualifier &&
> $extension eq 'f' && $qualifier =~ /^w/))
>
> > $bad_specifier = $specifier;
> > last;
> > }
>
> And here the 'f' isn't really a bad $specifier,
> it does not have the correct 'w' $extension.
>
> The valid extensions may change in the future and
> this code isn't particularly manageable, so I
> think the $specifier/$extension pair should likely
> have some additional testing after validating
> the $specifier.
There are certainly possibilities of having qualifiers that aren't
supported but extensions are also allowed to be followed by any other
characters, so this is a bit grey area.
I'll make the qualifier one or more letters for v7 as you suggested.
--
Regards,
Sakari Ailus
sakari.ailus@linux.intel.com
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v6 12/12] lib/test_printf: Add tests for %pfw printk modifier
2019-09-10 8:46 [PATCH v6 00/12] Device property improvements, add %pfw format specifier Sakari Ailus
` (10 preceding siblings ...)
2019-09-10 8:47 ` [PATCH v6 11/12] lib/vsprintf: Add %pfw conversion specifier for printing fwnode names Sakari Ailus
@ 2019-09-10 8:47 ` Sakari Ailus
11 siblings, 0 replies; 25+ messages in thread
From: Sakari Ailus @ 2019-09-10 8:47 UTC (permalink / raw)
To: Petr Mladek, linux-kernel, rafael
Cc: Andy Shevchenko, linux-acpi, devicetree, Rob Herring,
Heikki Krogerus, Joe Perches
Add a test for the %pfw printk modifier using software nodes.
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
lib/test_printf.c | 32 ++++++++++++++++++++++++++++++++
1 file changed, 32 insertions(+)
diff --git a/lib/test_printf.c b/lib/test_printf.c
index 944eb50f38625..bb6a7d334084b 100644
--- a/lib/test_printf.c
+++ b/lib/test_printf.c
@@ -22,6 +22,8 @@
#include <linux/gfp.h>
#include <linux/mm.h>
+#include <linux/property.h>
+
#include "../tools/testing/selftests/kselftest_module.h"
#define BUF_SIZE 256
@@ -588,6 +590,35 @@ flags(void)
kfree(cmp_buffer);
}
+static void __init fwnode_pointer(void)
+{
+ const struct software_node softnodes[] = {
+ { .name = "first", },
+ { .name = "second", .parent = &softnodes[0], },
+ { .name = "third", .parent = &softnodes[1], },
+ { NULL /* Guardian */ }
+ };
+ const char * const full_name = "first/second/third";
+ const char * const full_name_second = "first/second";
+ const char * const second_name = "second";
+ const char * const third_name = "third";
+ int rval;
+
+ rval = software_node_register_nodes(softnodes);
+ if (rval) {
+ pr_warn("cannot register softnodes; rval %d\n", rval);
+ return;
+ }
+
+ test(full_name_second, "%pfw", software_node_fwnode(&softnodes[1]));
+ test(full_name, "%pfw", software_node_fwnode(&softnodes[2]));
+ test(full_name, "%pfwf", software_node_fwnode(&softnodes[2]));
+ test(second_name, "%pfwP", software_node_fwnode(&softnodes[1]));
+ test(third_name, "%pfwP", software_node_fwnode(&softnodes[2]));
+
+ software_node_unregister_nodes(softnodes);
+}
+
static void __init
test_pointer(void)
{
@@ -610,6 +641,7 @@ test_pointer(void)
bitmap();
netdev_features();
flags();
+ fwnode_pointer();
}
static void __init selftest(void)
--
2.20.1
^ permalink raw reply related [flat|nested] 25+ messages in thread