From patchwork Thu Feb 11 11:27:21 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Viresh Kumar X-Patchwork-Id: 380898 Delivered-To: patch@linaro.org Received: by 2002:a02:b18a:0:0:0:0:0 with SMTP id t10csp1984583jah; Thu, 11 Feb 2021 03:31:40 -0800 (PST) X-Google-Smtp-Source: ABdhPJw0SVHY9mKYPsRLqnePqQmT5WGC9ck7Q9+9DS10wZaBkZVlOXEqtdMx6150CcKuX+8IuNjG X-Received: by 2002:a05:6402:1215:: with SMTP id c21mr7974316edw.310.1613043100161; Thu, 11 Feb 2021 03:31:40 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1613043100; cv=none; d=google.com; s=arc-20160816; b=Mmif5Vd7ctC1zSMeROYVSO1FT+TerJBPJOAizlpJSyMswAVk1INCr0t+hE6/1UDCat bkThrPJtaelkGvnSm+uLwZdTIheI88jWWpm3E497BJcCbCRj2ROvDGQXyz4aLoV4EtHo PwGq1T1BQTRCizv2espej/8CA00tCMhWcBHALgDZEDNurCLyh22rAg8h5yDY3X5SCNnS lPvK3bzY0A9iJbgt1BsiXHh44mfP3DuRWabeRMyu9ZAW8b2MJlalN18rFlBpU8nbeUJh yKO8H62Q/lezfdPbpL2MAvRt4sLdMLOEvlIVoPF59Elc5R18e4MMvBaJiD4C0BvfVuPq araQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :message-id:date:subject:cc:to:from:dkim-signature; bh=JU1gc/ALV1VkkX6EBW5FvgSlWlS/XoSIRN2HuCqDgyE=; b=hCfRxNyPdirFCYqk37t9ZLw/nUzEgILMHBUDn4S+xsMcasGtp0AmK0x1sheNM8Rcjj 8Vp7Z3o1EXzR0Y5OxSfSecCEKJu6Vc9I1ckmJlkcsO6GLSkMB2t9w4g0Wjk+unPKDzIT LebLewI+R85+A/s+wHgk7ACiX0naWQlmG+UMEkFBD3I3gT6bSWsAg6hov9avtJDGxIhH 163yhhyPca/+7Z7Hb02XcdIB5yYIExccolQTTHaN16A7l58rcgM9pQl7WEhgfBiP4mPl T4LeqaBhVL6D3NmSSMLFrcXDbEQNN18CzDH4trFM5VO02ntvoWu8VvmAIB/XgANhHcAl ZvZA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=VqH3nRtm; spf=pass (google.com: domain of devicetree-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=devicetree-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id r8si3815525eds.193.2021.02.11.03.31.39; Thu, 11 Feb 2021 03:31:40 -0800 (PST) Received-SPF: pass (google.com: domain of devicetree-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=VqH3nRtm; spf=pass (google.com: domain of devicetree-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=devicetree-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231616AbhBKLaX (ORCPT + 6 others); Thu, 11 Feb 2021 06:30:23 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:57700 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231351AbhBKL2R (ORCPT ); Thu, 11 Feb 2021 06:28:17 -0500 Received: from mail-pg1-x532.google.com (mail-pg1-x532.google.com [IPv6:2607:f8b0:4864:20::532]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 00A37C061788 for ; Thu, 11 Feb 2021 03:27:36 -0800 (PST) Received: by mail-pg1-x532.google.com with SMTP id o63so3650585pgo.6 for ; Thu, 11 Feb 2021 03:27:36 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=JU1gc/ALV1VkkX6EBW5FvgSlWlS/XoSIRN2HuCqDgyE=; b=VqH3nRtmV89x4jHYQhp7d1wniZooga5iMIAUwOyMk3NC1SwOwoh4WjBTc8P8LYu6V8 oMkY1ErorG2ioNOMI9zp/H9kczqiwVQvOGvLjT1sHWUysiIWErHT1YGz69SIBv9htmYG YOu4qJmPAifHAd2GLd1p/N1tuY0qfFr/eicv52bF7c9uVOKSRad9a+nSpuDmEr6zfS/Z 95T0cd2o8hWkyro9vjfXqhV9bvOcXy9gOIA3WHxCkvfhkHpE2CpXU12kV5ex+scvywOC hZOlQcQzBOgdTjA+dMomoCvaEdYLiLZKVL9+Rqd6hzASrXZUWV8N3w8F2b0OLVAgpXf5 Nv1Q== 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:mime-version :content-transfer-encoding; bh=JU1gc/ALV1VkkX6EBW5FvgSlWlS/XoSIRN2HuCqDgyE=; b=Yc1Am2yvtJok9N56L4ELLe65ukpX+foUO2j5rE7qWMk+woP3vClAJS5ORciOqpt0k7 MBv9w7vEG5sCRA5OAszvRie2fy3Bj5ltT7+eK2SiAQ07/QzYAJhi6PkAU3p+419SdUEV JNuLvMKwUyUECcaj4o+5UjPEDH+NDwEeMR62Ktx5ZKhOiE525rYqKxhWREZYUZ6WsVRx RyQN97DLLoQsMfU4sfdM4cRKJARd2EvGINERItrlMsarr5WwMRpnQAeMtee+1voZkm0s 1vjS7CeREc6tT0HuTmCPnLziDszO4gAWXplBfwjddka6CslwMHuw802oMUvbHXLGEbof QXCA== X-Gm-Message-State: AOAM530lgnVc8dWRx71NbjEnmHEL4KAqeCiwnx/B8lKfY1kNIQoA0E2E n25zi8caq/9sqLb9JYM1qL3qUA== X-Received: by 2002:a63:515:: with SMTP id 21mr7539230pgf.231.1613042856327; Thu, 11 Feb 2021 03:27:36 -0800 (PST) Received: from localhost ([122.172.59.240]) by smtp.gmail.com with ESMTPSA id t8sm4734834pjm.9.2021.02.11.03.27.35 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Thu, 11 Feb 2021 03:27:35 -0800 (PST) From: Viresh Kumar To: David Gibson Cc: Viresh Kumar , Vincent Guittot , Frank Rowand , Rob Herring , Bill Mills , devicetree@vger.kernel.org, Jon Loeliger , devicetree-compiler@vger.kernel.org Subject: [DTC][RFC] dtc: Allow better error reporting Date: Thu, 11 Feb 2021 16:57:21 +0530 Message-Id: <3950d7da35130a850ba9217ac7bfef781fa850b2.1613042485.git.viresh.kumar@linaro.org> X-Mailer: git-send-email 2.25.0.rc1.19.g042ed3e048af MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: devicetree@vger.kernel.org The dtc tool doesn't do very elaborate error reporting to keep the size of the executables small enough for all kind of applications. This patch tries to provide a way to do better error reporting, without increasing the size of the executables for such cases (where we don't want elaborate error reporting). The error reporting will only be enabled if 'VERBOSE' (as -DVERBOSE) flag is passed during compilation of the tools. This also updates the fdtoverlay tool to do better error reporting, which is required by the Linux Kernel for now. Signed-off-by: Viresh Kumar --- Hi David, Unfortunately I am not a core DT guy and don't understand most of the stuff going on here. I tried to do minimal changes to get more information out on errors and it may require someone who understands the code well to write better error logs. FWIW, I stumbled upon this because of the discussion that happened here: https://lore.kernel.org/lkml/74f8aa8f-ffab-3b0f-186f-31fb7395ebbb@gmail.com/ Thanks. --- dtc.h | 6 ++ fdtoverlay.c | 1 + libfdt/fdt_overlay.c | 156 ++++++++++++++++++++++++++++++++++--------- 3 files changed, 132 insertions(+), 31 deletions(-) -- 2.25.0.rc1.19.g042ed3e048af diff --git a/dtc.h b/dtc.h index d3e82fb8e3db..b8ffec155263 100644 --- a/dtc.h +++ b/dtc.h @@ -29,6 +29,12 @@ #define debug(...) #endif +#ifdef VERBOSE +#define dtc_err(fmt, ...) fprintf(stderr, "DTC: %s: %d: " fmt, __func__, __LINE__, ##__VA_ARGS__) +#else +#define dtc_err(fmt, ...) +#endif + #define DEFAULT_FDT_VERSION 17 /* diff --git a/fdtoverlay.c b/fdtoverlay.c index 5350af65679f..5f60ce4e4cea 100644 --- a/fdtoverlay.c +++ b/fdtoverlay.c @@ -16,6 +16,7 @@ #include +#include "dtc.h" #include "util.h" #define BUF_INCREMENT 65536 diff --git a/libfdt/fdt_overlay.c b/libfdt/fdt_overlay.c index d217e79b6722..b24286ac8c6c 100644 --- a/libfdt/fdt_overlay.c +++ b/libfdt/fdt_overlay.c @@ -10,6 +10,7 @@ #include #include "libfdt_internal.h" +#include "../dtc.h" /** * overlay_get_target_phandle - retrieves the target phandle of a fragment @@ -160,12 +161,16 @@ static int overlay_adjust_node_phandles(void *fdto, int node, int ret; ret = overlay_phandle_add_offset(fdto, node, "phandle", delta); - if (ret && ret != -FDT_ERR_NOTFOUND) + if (ret && ret != -FDT_ERR_NOTFOUND) { + dtc_err("Failed to add phandle offset (%d: %s)\n", node, fdt_strerror(ret)); return ret; + } ret = overlay_phandle_add_offset(fdto, node, "linux,phandle", delta); - if (ret && ret != -FDT_ERR_NOTFOUND) + if (ret && ret != -FDT_ERR_NOTFOUND) { + dtc_err("Failed to add linux,phandle offset (%d: %s)\n", node, fdt_strerror(ret)); return ret; + } fdt_for_each_subnode(child, fdto, node) { ret = overlay_adjust_node_phandles(fdto, child, delta); @@ -239,12 +244,17 @@ static int overlay_update_local_node_references(void *fdto, if (!fixup_val) return fixup_len; - if (fixup_len % sizeof(uint32_t)) + if (fixup_len % sizeof(uint32_t)) { + dtc_err("Invalid fixup length\n"); return -FDT_ERR_BADOVERLAY; + } fixup_len /= sizeof(uint32_t); tree_val = fdt_getprop(fdto, tree_node, name, &tree_len); if (!tree_val) { + dtc_err("Failed to get property: %s: %d\n", name, + tree_len); + if (tree_len == -FDT_ERR_NOTFOUND) return -FDT_ERR_BADOVERLAY; @@ -274,11 +284,17 @@ static int overlay_update_local_node_references(void *fdto, poffset, &adj_val, sizeof(adj_val)); - if (ret == -FDT_ERR_NOSPACE) + if (ret == -FDT_ERR_NOSPACE) { + dtc_err("Failed to update property's name: %s\n", + name); return -FDT_ERR_BADOVERLAY; + } - if (ret) + if (ret) { + dtc_err("Failed to update property's name: %s: %s\n", + name, fdt_strerror(ret)); return ret; + } } } @@ -289,10 +305,16 @@ static int overlay_update_local_node_references(void *fdto, tree_child = fdt_subnode_offset(fdto, tree_node, fixup_child_name); - if (tree_child == -FDT_ERR_NOTFOUND) + if (tree_child == -FDT_ERR_NOTFOUND) { + dtc_err("Failed to find subnode: %s\n", + fixup_child_name); return -FDT_ERR_BADOVERLAY; - if (tree_child < 0) + } + if (tree_child < 0) { + dtc_err("Failed to find subnode: %s: %s\n", + fixup_child_name, fdt_strerror(tree_child)); return tree_child; + } ret = overlay_update_local_node_references(fdto, tree_child, @@ -332,6 +354,8 @@ static int overlay_update_local_references(void *fdto, uint32_t delta) if (fixups == -FDT_ERR_NOTFOUND) return 0; + dtc_err("Failed to find local_fixups (%s)\n", + fdt_strerror(fixups)); return fixups; } @@ -435,6 +459,8 @@ static int overlay_fixup_phandle(void *fdt, void *fdto, int symbols_off, value = fdt_getprop_by_offset(fdto, property, &label, &len); if (!value) { + dtc_err("Failed to get property by offset (%s)\n", + fdt_strerror(len)); if (len == -FDT_ERR_NOTFOUND) return -FDT_ERR_INTERNAL; @@ -450,8 +476,10 @@ static int overlay_fixup_phandle(void *fdt, void *fdto, int symbols_off, int poffset, ret; fixup_end = memchr(value, '\0', len); - if (!fixup_end) + if (!fixup_end) { + dtc_err("fixup_end can't be 0: %s: %s\n", value, label); return -FDT_ERR_BADOVERLAY; + } fixup_len = fixup_end - fixup_str; len -= fixup_len + 1; @@ -459,32 +487,47 @@ static int overlay_fixup_phandle(void *fdt, void *fdto, int symbols_off, path = fixup_str; sep = memchr(fixup_str, ':', fixup_len); - if (!sep || *sep != ':') + if (!sep || *sep != ':') { + dtc_err("Missing ':' separator: %s: %s\n", value, + label); return -FDT_ERR_BADOVERLAY; + } path_len = sep - path; - if (path_len == (fixup_len - 1)) + if (path_len == (fixup_len - 1)) { + dtc_err("Invalid path_len: %s: %s\n", value, label); return -FDT_ERR_BADOVERLAY; + } fixup_len -= path_len + 1; name = sep + 1; sep = memchr(name, ':', fixup_len); - if (!sep || *sep != ':') + if (!sep || *sep != ':') { + dtc_err("Missing ':' separator: %s: %s\n", value, + label); return -FDT_ERR_BADOVERLAY; + } name_len = sep - name; - if (!name_len) + if (!name_len) { + dtc_err("name_len can't be 0: %s: %s\n", value, label); return -FDT_ERR_BADOVERLAY; + } poffset = strtoul(sep + 1, &endptr, 10); - if ((*endptr != '\0') || (endptr <= (sep + 1))) + if ((*endptr != '\0') || (endptr <= (sep + 1))) { + dtc_err("Invalid name string: %s: %s\n", value, label); return -FDT_ERR_BADOVERLAY; + } ret = overlay_fixup_one_phandle(fdt, fdto, symbols_off, path, path_len, name, name_len, poffset, label); - if (ret) + if (ret) { + dtc_err("failed to fixup one phandle: %s: %s: %s\n", + value, label, fdt_strerror(ret)); return ret; + } } while (len > 0); return 0; @@ -516,13 +559,19 @@ static int overlay_fixup_phandles(void *fdt, void *fdto) fixups_off = fdt_path_offset(fdto, "/__fixups__"); if (fixups_off == -FDT_ERR_NOTFOUND) return 0; /* nothing to do */ - if (fixups_off < 0) + if (fixups_off < 0) { + dtc_err("Failed to get fixups offset (%s)\n", + fdt_strerror(fixups_off)); return fixups_off; + } /* And base DTs without symbols */ symbols_off = fdt_path_offset(fdt, "/__symbols__"); - if ((symbols_off < 0 && (symbols_off != -FDT_ERR_NOTFOUND))) + if ((symbols_off < 0 && (symbols_off != -FDT_ERR_NOTFOUND))) { + dtc_err("Failed to get symbols offset (%s)\n", + fdt_strerror(symbols_off)); return symbols_off; + } fdt_for_each_property_offset(property, fdto, fixups_off) { int ret; @@ -633,16 +682,27 @@ static int overlay_merge(void *fdt, void *fdto) if (overlay == -FDT_ERR_NOTFOUND) continue; - if (overlay < 0) + if (overlay < 0) { + dtc_err("Failed to find __overlay__ tag (%d: %s)\n", + fragment, fdt_strerror(overlay)); return overlay; + } target = overlay_get_target(fdt, fdto, fragment, NULL); - if (target < 0) + if (target < 0) { + dtc_err("Failed to retrieve fragment's target (%d: %s)\n", + fragment, fdt_strerror(target)); return target; + } ret = overlay_apply_node(fdt, target, fdto, overlay); - if (ret) + if (ret) { + if (ret != -FDT_ERR_NOSPACE) { + dtc_err("Failed to apply overlay node (%d: %d: %s)\n", + fragment, target, fdt_strerror(ret)); + } return ret; + } } return 0; @@ -718,24 +778,35 @@ static int overlay_symbol_update(void *fdt, void *fdto) root_sym = fdt_add_subnode(fdt, 0, "__symbols__"); /* any error is fatal now */ - if (root_sym < 0) + if (root_sym < 0) { + dtc_err("Failed to get root __symbols__ (%s)\n", + fdt_strerror(root_sym)); return root_sym; + } /* iterate over each overlay symbol */ fdt_for_each_property_offset(prop, fdto, ov_sym) { path = fdt_getprop_by_offset(fdto, prop, &name, &path_len); - if (!path) + if (!path) { + dtc_err("Failed to get prop by offset (%s)\n", + fdt_strerror(path_len)); return path_len; + } /* verify it's a string property (terminated by a single \0) */ - if (path_len < 1 || memchr(path, '\0', path_len) != &path[path_len - 1]) + if (path_len < 1 || memchr(path, '\0', path_len) != &path[path_len - 1]) { + dtc_err("Invalid property (%d)\n", path_len); return -FDT_ERR_BADVALUE; + } /* keep end marker to avoid strlen() */ e = path + path_len; - if (*path != '/') + if (*path != '/') { + dtc_err("Path should start with '/' (%s : %s)\n", path, + name); return -FDT_ERR_BADVALUE; + } /* get fragment name first */ s = strchr(path + 1, '/'); @@ -769,26 +840,38 @@ static int overlay_symbol_update(void *fdt, void *fdto) ret = fdt_subnode_offset_namelen(fdto, 0, frag_name, frag_name_len); /* not found? */ - if (ret < 0) + if (ret < 0) { + dtc_err("Failed to find fragment index (%s: %s: %d)\n", + path, frag_name, ret); return -FDT_ERR_BADOVERLAY; + } fragment = ret; /* an __overlay__ subnode must exist */ ret = fdt_subnode_offset(fdto, fragment, "__overlay__"); - if (ret < 0) + if (ret < 0) { + dtc_err("Failed to find __overlay__ subnode (%s: %s: %d)\n", + path, frag_name, ret); return -FDT_ERR_BADOVERLAY; + } /* get the target of the fragment */ ret = overlay_get_target(fdt, fdto, fragment, &target_path); - if (ret < 0) + if (ret < 0) { + dtc_err("Failed to get target for the fragment (%s: %s: %d)\n", + path, frag_name, ret); return ret; + } target = ret; /* if we have a target path use */ if (!target_path) { ret = get_path_len(fdt, target); - if (ret < 0) + if (ret < 0) { + dtc_err("Failed to get path length (%s: %s: %d)\n", + path, name, ret); return ret; + } len = ret; } else { len = strlen(target_path); @@ -796,14 +879,20 @@ static int overlay_symbol_update(void *fdt, void *fdto) ret = fdt_setprop_placeholder(fdt, root_sym, name, len + (len > 1) + rel_path_len + 1, &p); - if (ret < 0) + if (ret < 0) { + dtc_err("Failed to set prop placeholder (%s: %s: %d)\n", + path, name, ret); return ret; + } if (!target_path) { /* again in case setprop_placeholder changed it */ ret = overlay_get_target(fdt, fdto, fragment, &target_path); - if (ret < 0) + if (ret < 0) { + dtc_err("Failed to get target (%s: %s: %d)\n", + path, name, ret); return ret; + } target = ret; } @@ -811,8 +900,11 @@ static int overlay_symbol_update(void *fdt, void *fdto) if (len > 1) { /* target is not root */ if (!target_path) { ret = fdt_get_path(fdt, target, buf, len + 1); - if (ret < 0) + if (ret < 0) { + dtc_err("Failed to get target path (%s: %s: %d)\n", + path, name, ret); return ret; + } } else memcpy(buf, target_path, len + 1); @@ -836,8 +928,10 @@ int fdt_overlay_apply(void *fdt, void *fdto) FDT_RO_PROBE(fdto); ret = fdt_find_max_phandle(fdt, &delta); - if (ret) + if (ret) { + dtc_err("Failed to find max phandle (%s)\n", fdt_strerror(ret)); goto err; + } ret = overlay_adjust_local_phandles(fdto, delta); if (ret)