On Fri, Mar 22, 2019 at 05:29:30PM +0200, 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.
Do we encourage people to use it instead of %pOF cases where it is suitable?
> 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.
Do we support swnode here?
> +static noinline_for_stack
> +char *fwnode_string(char *buf, char *end, struct fwnode_handle *fwnode,
> + struct printf_spec spec, const char *fmt)
> +{
> + const char * const modifiers = "fP";
> + struct printf_spec str_spec = spec;
> + char *buf_start = buf;
> + bool pass;
> +
> + str_spec.field_width = -1;
> +
> + if ((unsigned long)fwnode < PAGE_SIZE)
> + return string(buf, end, "(null)", spec);
Just put there a NULL pointer, we would not like to maintain duplicated strings
over the kernel.
I remember Petr has a patch series related to address space check, though I
don't remember the status of affairs.
> +
> + /* simple case without anything any more format specifiers */
> + fmt++;
> + if (fmt[0] == '\0' || strcspn(fmt, modifiers) > 0)
> + fmt = "f";
> +
> + for (pass = false; strspn(fmt, modifiers); fmt++, pass = true) {
I don't see test cases.
What would we get out of %pfwfffPPPfff?
Hint: I'm expecting above to be equivalent to %pfwf
> + if (pass) {
> + if (buf < end)
> + *buf = ':';
> + buf++;
> + }
> +
> + switch (*fmt) {
> + case 'f': /* full_name */
> + buf = fwnode_gen_full_name(fwnode, buf, end);
> + break;
> + case 'P': /* name */
> + buf = string(buf, end, fwnode_get_name(fwnode),
> + str_spec);
> + break;
> + default:
> + break;
> + }
> + }
> +
> + return widen_string(buf, buf - buf_start, end, spec);
> +}
--
With Best Regards,
Andy Shevchenko
