From patchwork Fri Jun 23 17:30:53 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Rob Herring X-Patchwork-Id: 106277 Delivered-To: patch@linaro.org Received: by 10.182.135.102 with SMTP id pr6csp147634obb; Fri, 23 Jun 2017 10:31:11 -0700 (PDT) X-Received: by 10.98.38.129 with SMTP id m123mr9213248pfm.183.1498239071412; Fri, 23 Jun 2017 10:31:11 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1498239071; cv=none; d=google.com; s=arc-20160816; b=LDO5FlwMASIWILLfzZOwttxeHefauuzpP3Y67yY2hYUG0MxtqZFB+pg36g3qcBIsPX HU6NIWUoUL5axik1vu9UdTFwHfpS2aHSo072DwjEer/41D36CUBgkug9n5t/S+HZAq8J IPrsw8D1MrgXMGxFsvSicbS1GMdm7m1bYLqzU5UXaWyvuqmMtzsk0OOPFiDYqPRYSCJY it2SswR5Qos4HQ57D9Vjj88gB4wSR9tFYlfwKNKB9FfOHDuZOARB663OtaUU5TeFWJJW JIncvPIrYKGMiqZzsXG4IZychNChZu8Pa04tUIKCkl5IrpKSSLKTGwg1KVsjT5oRWFhv UbBQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:references:in-reply-to:message-id:date :subject:cc:to:from:arc-authentication-results; bh=7Jp1NKVIiFX3P8BduYjXvdoKswxEcIX7WygmJPu9n0Y=; b=dpiqQffVKtfamhadX5+Ery5U6zZphAplBMOR1TqcodULK+7MrJDnbQXJvNiuq+Oo4u k5iom9KSwf2h6JY4xv8DFFwZetjGKfSk9WTul3MGt1oA/s3HqbK91shLh2PYlyBMoCkc bqJ6WloiPMM/O6B5cAjPHNnHm176ExdVU7h47h7bQkwZS3ZLk3MGqVLFRpLaFRdQQIXO 1+PvBLw61TyFrbm1byOXMV01LYvxZwmBhF79s5k3UJxGK7MGOetTMO76YWDMixbxN4Hd /HvQpltzkO0t3nMGGjyi44xgjfMOWlSFd4GMc7Vay3EOB7RI13GhCicpWgIqWTj3qvZf O9gg== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id k33si4095158pld.481.2017.06.23.10.31.11; Fri, 23 Jun 2017 10:31:11 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754760AbdFWRbB (ORCPT + 25 others); Fri, 23 Jun 2017 13:31:01 -0400 Received: from mail-it0-f65.google.com ([209.85.214.65]:35640 "EHLO mail-it0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754625AbdFWRa6 (ORCPT ); Fri, 23 Jun 2017 13:30:58 -0400 Received: by mail-it0-f65.google.com with SMTP id f20so7859995itb.2; Fri, 23 Jun 2017 10:30:57 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=7Jp1NKVIiFX3P8BduYjXvdoKswxEcIX7WygmJPu9n0Y=; b=BzWnASy3XpMucKABY9NiBpcSh5toF8WE7VXuO9A0G6hiZTv0c4mW62J+IXKD0BQUwC TESqvxWWOYbjp6uNv9/T4eC7pEAs0v4b1Mg46fbrnxwdjQFKsSSB6CHn5124AHbtI/A5 ztH4G2wtgFIDBsmGwHD/uMd9Regx882vrneyr8E2fJNXtyEDwPwMLWPWt5lV8S//G8Mg 2seh55MX/+P2hItdV+0z/1u58T3yrHre00QB525EVyV0QjmxFFDyMPiCvjqEvPNRsQqb YkLyJiaxFKXYQAlNArHnDls7YIliAQJZ8b87AP9SqBWERpmjMzTkiSnk/hduzc8Zae48 IN9A== X-Gm-Message-State: AKS2vOzIwZKx7hLP7zrK04WQ90M0qPdxKuy+gAHbyoVLydOb0BEnvYkQ IV2vAO5bvP/AHA== X-Received: by 10.36.203.135 with SMTP id u129mr8325035itg.3.1498239056723; Fri, 23 Jun 2017 10:30:56 -0700 (PDT) Received: from localhost.localdomain (24-223-123-72.static.usa-companies.net. [24.223.123.72]) by smtp.googlemail.com with ESMTPSA id d188sm3353368ioe.52.2017.06.23.10.30.55 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 23 Jun 2017 10:30:56 -0700 (PDT) From: Rob Herring To: Frank Rowand , Mark Rutland , Joe Perches , Randy Dunlap , Grant Likely Cc: Pantelis Antoniou , devicetree@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH v3] vsprintf: Add %p extension "%pOF" for device tree Date: Fri, 23 Jun 2017 12:30:53 -0500 Message-Id: <20170623173053.636-1-robh@kernel.org> X-Mailer: git-send-email 2.11.0 In-Reply-To: <20170622204445.14930-1-robh@kernel.org> References: <20170622204445.14930-1-robh@kernel.org> Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org From: Pantelis Antoniou 90% of the usage of device node's full_name is printing it out in a kernel message. However, storing the full path for every node is wasteful and redundant. With a custom format specifier, we can generate the full path at run-time and eventually remove the full path from every node. For instance typical use is: pr_info("Frobbing node %s\n", node->full_name); Which can be written now as: pr_info("Frobbing node %pOF\n", node); '%pO' is the base specifier to represent kobjects with '%pOF' representing struct device_node. Currently, struct device_node is the only supported type of kobject. More fine-grained control of formatting includes printing the name, flags, path-spec name and others, explained in the documentation entry. Originally written by Pantelis, but pretty much rewrote the core function using existing string/number functions. The 2 passes were unnecessary and have been removed. Also, updated the checkpatch.pl check. The unittest code was written by Grant Likely. Signed-off-by: Pantelis Antoniou Signed-off-by: Rob Herring --- v3: - Fix missing documentation updates using '%pOF' as the device_node specifier. - Update the commit msg and documentation to clearly define '%pO' is the base specifier for kobjects. - Rework device_node_gen_full_name() to avoid creating an array of node pointers on the stack. v2: - Change subject - Rewrite device_node_gen_full_name() to avoid recursion. - Avoid using sprintf. - Add unittests Grant L. wrote. - Drop ref count printing (from discussion 2 years ago). - Remove fmtp local var. Documentation/printk-formats.txt | 36 +++++++ drivers/of/unittest-data/tests-platform.dtsi | 4 +- drivers/of/unittest.c | 58 ++++++++++++ lib/vsprintf.c | 135 +++++++++++++++++++++++++++ scripts/checkpatch.pl | 2 +- 5 files changed, 233 insertions(+), 2 deletions(-) -- 2.11.0 diff --git a/Documentation/printk-formats.txt b/Documentation/printk-formats.txt index 5962949944fd..619cdffa5d44 100644 --- a/Documentation/printk-formats.txt +++ b/Documentation/printk-formats.txt @@ -275,6 +275,42 @@ struct va_format: Passed by reference. +kobjects: + %pO + + Base specifier for kobject based structs. Must be followed with + character for specific type of kobject as listed below: + + Device tree nodes: + + %pOF[fnpPcCF] + + For printing device tree nodes. The optional arguments are: + f device node full_name + n device node name + p device node phandle + P device node path spec (name + @unit) + F device node flags + c major compatible string + C full compatible string + Without any arguments prints full_name (same as %pOFf) + The separator when using multiple arguments is ':' + + Examples: + + %pOF /foo/bar@0 - Node full name + %pOFf /foo/bar@0 - Same as above + %pOFfp /foo/bar@0:10 - Node full name + phandle + %pOFfcF /foo/bar@0:foo,device:--P- - Node full name + + major compatible string + + node flags + D - dynamic + d - detached + P - Populated + B - Populated bus + + Passed by reference. + struct clk: %pC pll1 diff --git a/drivers/of/unittest-data/tests-platform.dtsi b/drivers/of/unittest-data/tests-platform.dtsi index eb20eeb2b062..a0c93822aee3 100644 --- a/drivers/of/unittest-data/tests-platform.dtsi +++ b/drivers/of/unittest-data/tests-platform.dtsi @@ -26,7 +26,9 @@ #size-cells = <0>; dev@100 { - compatible = "test-sub-device"; + compatible = "test-sub-device", + "test-compat2", + "test-compat3"; reg = <0x100>; }; }; diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c index 987a1530282a..8f611e9844db 100644 --- a/drivers/of/unittest.c +++ b/drivers/of/unittest.c @@ -239,6 +239,63 @@ static void __init of_unittest_check_tree_linkage(void) pr_debug("allnodes list size (%i); sibling lists size (%i)\n", allnode_count, child_count); } +static void __init of_unittest_printf_one(struct device_node *np, const char *fmt, + const char *expected) +{ + char buf[strlen(expected)+10]; + int size, i; + + /* Baseline; check conversion with a large size limit */ + memset(buf, 0xff, sizeof(buf)); + size = snprintf(buf, sizeof(buf) - 2, fmt, np); + + /* use strcmp() instead of strncmp() here to be absolutely sure strings match */ + unittest((strcmp(buf, expected) == 0) && (buf[size+1] == 0xff), + "sprintf failed; fmt='%s' expected='%s' rslt='%s'\n", + fmt, expected, buf); + + /* Make sure length limits work */ + size++; + for (i = 0; i < 2; i++, size--) { + /* Clear the buffer, and make sure it works correctly still */ + memset(buf, 0xff, sizeof(buf)); + snprintf(buf, size+1, fmt, np); + unittest(strncmp(buf, expected, size) == 0 && (buf[size+1] == 0xff), + "snprintf failed; size=%i fmt='%s' expected='%s' rslt='%s'\n", + size, fmt, expected, buf); + } +} + +static void __init of_unittest_printf(void) +{ + struct device_node *np; + const char *full_name = "/testcase-data/platform-tests/test-device@1/dev@100"; + char phandle_str[16] = ""; + + np = of_find_node_by_path(full_name); + if (!np) { + unittest(np, "testcase data missing\n"); + return; + } + + num_to_str(phandle_str, sizeof(phandle_str), np->phandle); + + of_unittest_printf_one(np, "%pOF", full_name); + of_unittest_printf_one(np, "%pOFf", full_name); + of_unittest_printf_one(np, "%pOFp", phandle_str); + of_unittest_printf_one(np, "%pOFP", "dev@100"); + of_unittest_printf_one(np, "ABC %pOFP ABC", "ABC dev@100 ABC"); + of_unittest_printf_one(np, "%10pOFP", " dev@100"); + of_unittest_printf_one(np, "%-10pOFP", "dev@100 "); + of_unittest_printf_one(of_root, "%pOFP", "/"); + of_unittest_printf_one(np, "%pOFF", "----"); + of_unittest_printf_one(np, "%pOFPF", "dev@100:----"); + of_unittest_printf_one(np, "%pOFPFPc", "dev@100:----:dev@100:test-sub-device"); + of_unittest_printf_one(np, "%pOFc", "test-sub-device"); + of_unittest_printf_one(np, "%pOFC", + "\"test-sub-device\",\"test-compat2\",\"test-compat3\""); +} + struct node_hash { struct hlist_node node; struct device_node *np; @@ -2269,6 +2326,7 @@ static int __init of_unittest(void) of_unittest_find_node_by_name(); of_unittest_dynamic(); of_unittest_parse_phandle_with_args(); + of_unittest_printf(); of_unittest_property_string(); of_unittest_property_copy(); of_unittest_changeset(); diff --git a/lib/vsprintf.c b/lib/vsprintf.c index 2d41de3f98a1..9e2e9306fd24 100644 --- a/lib/vsprintf.c +++ b/lib/vsprintf.c @@ -31,6 +31,7 @@ #include #include #include +#include #include #ifdef CONFIG_BLOCK #include @@ -1470,6 +1471,125 @@ char *flags_string(char *buf, char *end, void *flags_ptr, const char *fmt) 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) +{ + int depth; + const struct device_node *parent = np->parent; + static const struct printf_spec strspec = { + .field_width = -1, + .precision = -1, + }; + + /* special case for root node */ + if (!parent) + return string(buf, end, "/", strspec); + + for (depth = 0; parent->parent; depth++) + parent = parent->parent; + + for ( ; depth >= 0; depth--) { + buf = string(buf, end, "/", strspec); + buf = string(buf, end, device_node_name_for_depth(np, depth), + strspec); + } + return buf; +} + +static noinline_for_stack +char *device_node_string(char *buf, char *end, struct device_node *dn, + struct printf_spec spec, const char *fmt) +{ + char tbuf[sizeof("xxxx") + 1]; + const char *p; + int ret; + char *buf_start = buf; + struct property *prop; + bool has_mult, pass; + static const struct printf_spec num_spec = { + .flags = SMALL, + .field_width = -1, + .precision = -1, + .base = 10, + }; + + struct printf_spec str_spec = spec; + str_spec.field_width = -1; + + if (!IS_ENABLED(CONFIG_OF)) + return string(buf, end, "(!OF)", spec); + + if ((unsigned long)dn < PAGE_SIZE) + return string(buf, end, "(null)", spec); + + /* simple case without anything any more format specifiers */ + fmt++; + if (fmt[0] == '\0' || strcspn(fmt,"fnpPFcC") > 0) + fmt = "f"; + + for (pass = false; strspn(fmt,"fnpPFcC"); fmt++, pass = true) { + if (pass) { + if (buf < end) + *buf = ':'; + buf++; + } + + switch (*fmt) { + case 'f': /* full_name */ + buf = device_node_gen_full_name(dn, buf, end); + break; + case 'n': /* name */ + buf = string(buf, end, dn->name, str_spec); + break; + case 'p': /* phandle */ + buf = number(buf, end, (unsigned int)dn->phandle, num_spec); + break; + case 'P': /* path-spec */ + p = kbasename(of_node_full_name(dn)); + if (!p[1]) + p = "/"; + buf = string(buf, end, p, str_spec); + break; + case 'F': /* flags */ + tbuf[0] = of_node_check_flag(dn, OF_DYNAMIC) ? 'D' : '-'; + tbuf[1] = of_node_check_flag(dn, OF_DETACHED) ? 'd' : '-'; + tbuf[2] = of_node_check_flag(dn, OF_POPULATED) ? 'P' : '-'; + tbuf[3] = of_node_check_flag(dn, OF_POPULATED_BUS) ? 'B' : '-'; + buf = string(buf, end, tbuf, str_spec); + break; + case 'c': /* major compatible string */ + ret = of_property_read_string(dn, "compatible", &p); + if (!ret) + buf = string(buf, end, p, str_spec); + break; + case 'C': /* full compatible string */ + has_mult = false; + of_property_for_each_string(dn, "compatible", prop, p) { + if (has_mult) + buf = string(buf, end, ",", str_spec); + buf = string(buf, end, "\"", str_spec); + buf = string(buf, end, p, str_spec); + buf = string(buf, end, "\"", str_spec); + + has_mult = true; + } + break; + default: + break; + } + } + + return widen_string(buf, buf - buf_start, end, spec); +} + int kptr_restrict __read_mostly; /* @@ -1566,6 +1686,16 @@ int kptr_restrict __read_mostly; * p page flags (see struct page) given as pointer to unsigned long * g gfp flags (GFP_* and __GFP_*) given as pointer to gfp_t * v vma flags (VM_*) given as pointer to unsigned long + * - 'O' For a kobject based struct. Must be one of the following: + * - 'OF[fnpPcCF]' For a device tree object + * Without any optional arguments prints the full_name + * f device node full_name + * n device node name + * p device node phandle + * P device node path spec (name + @unit) + * F device node flags + * c major compatible string + * C full compatible string * * ** Please update also Documentation/printk-formats.txt when making changes ** * @@ -1721,6 +1851,11 @@ char *pointer(const char *fmt, char *buf, char *end, void *ptr, case 'G': return flags_string(buf, end, ptr, fmt); + case 'O': + switch (fmt[1]) { + case 'F': + return device_node_string(buf, end, ptr, spec, fmt + 1); + } } spec.flags |= SMALL; if (spec.field_width == -1) { diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 4b9569fa931b..411f2098fa6b 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -5709,7 +5709,7 @@ sub process { for (my $count = $linenr; $count <= $lc; $count++) { my $fmt = get_quoted_string($lines[$count - 1], raw_line($count, 0)); $fmt =~ s/%%//g; - if ($fmt =~ /(\%[\*\d\.]*p(?![\WFfSsBKRraEhMmIiUDdgVCbGN]).)/) { + if ($fmt =~ /(\%[\*\d\.]*p(?![\WFfSsBKRraEhMmIiUDdgVCbGNO]).)/) { $bad_extension = $1; last; }