From patchwork Thu Aug 6 21:31:02 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stephen Warren X-Patchwork-Id: 504918 X-Patchwork-Delegate: sjg@chromium.org Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from theia.denx.de (theia.denx.de [85.214.87.163]) by ozlabs.org (Postfix) with ESMTP id 8F54B1402A7 for ; Fri, 7 Aug 2015 07:30:57 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by theia.denx.de (Postfix) with ESMTP id A3059B37CE; Thu, 6 Aug 2015 23:30:54 +0200 (CEST) Received: from theia.denx.de ([127.0.0.1]) by localhost (theia.denx.de [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id XHEewLi7sifR; Thu, 6 Aug 2015 23:30:54 +0200 (CEST) Received: from theia.denx.de (localhost [127.0.0.1]) by theia.denx.de (Postfix) with ESMTP id 3CA844BA57; Thu, 6 Aug 2015 23:30:54 +0200 (CEST) Received: from localhost (localhost [127.0.0.1]) by theia.denx.de (Postfix) with ESMTP id ABDBF4BA57 for ; Thu, 6 Aug 2015 23:30:50 +0200 (CEST) Received: from theia.denx.de ([127.0.0.1]) by localhost (theia.denx.de [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id hip0EXicqh1P for ; Thu, 6 Aug 2015 23:30:50 +0200 (CEST) X-policyd-weight: NOT_IN_SBL_XBL_SPAMHAUS=-1.5 NOT_IN_SPAMCOP=-1.5 NOT_IN_BL_NJABL=-1.5 (only DNSBL check requested) Received: from avon.wwwdotorg.org (avon.wwwdotorg.org [70.85.31.133]) by theia.denx.de (Postfix) with ESMTPS id 326BA4BA55 for ; Thu, 6 Aug 2015 23:30:47 +0200 (CEST) Received: from severn.wwwdotorg.org (unknown [192.168.65.5]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by avon.wwwdotorg.org (Postfix) with ESMTPS id 60ABB630D; Thu, 6 Aug 2015 15:30:45 -0600 (MDT) Received: from swarren-lx1.nvidia.com (localhost [127.0.0.1]) (using TLSv1 with cipher AES128-SHA (128/128 bits)) (No client certificate requested) by severn.wwwdotorg.org (Postfix) with ESMTPSA id 9F1DAE455C; Thu, 6 Aug 2015 15:30:43 -0600 (MDT) From: Stephen Warren To: Tom Rini Date: Thu, 6 Aug 2015 15:31:02 -0600 Message-Id: <1438896662-27510-1-git-send-email-swarren@wwwdotorg.org> X-Mailer: git-send-email 1.9.1 X-NVConfidentiality: public X-Virus-Scanned: clamav-milter 0.98.6 at avon.wwwdotorg.org X-Virus-Status: Clean Cc: u-boot@lists.denx.de, Thierry Reding , Stephen Warren Subject: [U-Boot] [PATCH] fdt: add new fdt address parsing functions X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.15 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" From: Stephen Warren fdtdec_get_addr_size() hard-codes the number of cells used to represent an address or size in DT. This is incorrect in many cases depending on the DT binding for a particular node or property (e.g. it is incorrect for the "reg" property). In most cases, DT parsing code must use the properties #address-cells and #size-cells to parse addres properties. This change splits up the implementation of fdtdec_get_addr_size() so that the core logic can be used for both hard-coded and non-hard-coded cases. Various wrapper functions are implemented that support cases where hard-coded cell counts should or should not be used, and where the client does and doesn't know the parent node ID that contains the properties #address-cells and #size-cells. dev_get_addr() is updated to use the new functions. Core functionality in fdtdec_get_addr_size_fixed() is widely tested via fdtdec_get_addr_size(). I tested fdtdec_get_addr_size_auto_noparent() and dev_get_addr() by manually modifying the Tegra I2C driver to invoke them. Much of the core implementation of fdtdec_get_addr_size_fixed(), fdtdec_get_addr_size_auto_parent(), and fdtdec_get_addr_size_auto_noparent() comes from Thierry Reding's previous commit "fdt: Fix fdtdec_get_addr_size() for 64-bit". Based-on-work-by: Thierry Reding Cc: Thierry Reding Cc: Simon Glass Cc: Michal Suchanek Signed-off-by: Stephen Warren Acked-by: Simon Glass Tested-by: Mugunthan V N --- For patch context, this patch relies on Thierry's "fdt: Fix fdtdec_get_addr_size() for 64-bit" having been reverted. --- drivers/core/device.c | 5 ++- include/fdtdec.h | 111 +++++++++++++++++++++++++++++++++++++++++++---- lib/fdtdec.c | 116 +++++++++++++++++++++++++++++++++++++++++--------- 3 files changed, 202 insertions(+), 30 deletions(-) diff --git a/drivers/core/device.c b/drivers/core/device.c index 51b1b44e5b0a..917dd85551d3 100644 --- a/drivers/core/device.c +++ b/drivers/core/device.c @@ -555,7 +555,10 @@ const char *dev_get_uclass_name(struct udevice *dev) #ifdef CONFIG_OF_CONTROL fdt_addr_t dev_get_addr(struct udevice *dev) { - return fdtdec_get_addr(gd->fdt_blob, dev->of_offset, "reg"); + return fdtdec_get_addr_size_auto_parent(gd->fdt_blob, + dev->parent->of_offset, + dev->of_offset, "reg", + 0, NULL); } #else fdt_addr_t dev_get_addr(struct udevice *dev) diff --git a/include/fdtdec.h b/include/fdtdec.h index 4b3f8d13c356..04fe16bf15d2 100644 --- a/include/fdtdec.h +++ b/include/fdtdec.h @@ -308,10 +308,90 @@ int fdtdec_next_compatible(const void *blob, int node, int fdtdec_next_compatible_subnode(const void *blob, int node, enum fdt_compat_id id, int *depthp); -/** - * Look up an address property in a node and return it as an address. - * The property must hold either one address with no trailing data or - * one address with a length. This is only tested on 32-bit machines. +/* + * Look up an address property in a node and return the parsed address, and + * optionally the parsed size. + * + * This variant assumes a known and fixed number of cells are used to + * represent the address and size. + * + * You probably don't want to use this function directly except to parse + * non-standard properties, and never to parse the "reg" property. Instead, + * use one of the "auto" variants below, which automatically honor the + * #address-cells and #size-cells properties in the parent node. + * + * @param blob FDT blob + * @param node node to examine + * @param prop_name name of property to find + * @param index which address to retrieve from a list of addresses. Often 0. + * @param na the number of cells used to represent an address + * @param ns the number of cells used to represent a size + * @param sizep a pointer to store the size into. Use NULL if not required + * @return address, if found, or FDT_ADDR_T_NONE if not + */ +fdt_addr_t fdtdec_get_addr_size_fixed(const void *blob, int node, + const char *prop_name, int index, int na, int ns, + fdt_size_t *sizep); + +/* + * Look up an address property in a node and return the parsed address, and + * optionally the parsed size. + * + * This variant automatically determines the number of cells used to represent + * the address and size by parsing the provided parent node's #address-cells + * and #size-cells properties. + * + * @param blob FDT blob + * @param parent parent node of @node + * @param node node to examine + * @param prop_name name of property to find + * @param index which address to retrieve from a list of addresses. Often 0. + * @param sizep a pointer to store the size into. Use NULL if not required + * @return address, if found, or FDT_ADDR_T_NONE if not + */ +fdt_addr_t fdtdec_get_addr_size_auto_parent(const void *blob, int parent, + int node, const char *prop_name, int index, fdt_size_t *sizep); + +/* + * Look up an address property in a node and return the parsed address, and + * optionally the parsed size. + * + * This variant automatically determines the number of cells used to represent + * the address and size by parsing the parent node's #address-cells + * and #size-cells properties. The parent node is automatically found. + * + * The automatic parent lookup implemented by this function is slow. + * Consequently, fdtdec_get_addr_size_auto_parent() should be used where + * possible. + * + * @param blob FDT blob + * @param parent parent node of @node + * @param node node to examine + * @param prop_name name of property to find + * @param index which address to retrieve from a list of addresses. Often 0. + * @param sizep a pointer to store the size into. Use NULL if not required + * @return address, if found, or FDT_ADDR_T_NONE if not + */ +fdt_addr_t fdtdec_get_addr_size_auto_noparent(const void *blob, int node, + const char *prop_name, int index, fdt_size_t *sizep); + +/* + * Look up an address property in a node and return the parsed address. + * + * This variant hard-codes the number of cells used to represent the address + * and size based on sizeof(fdt_addr_t) and sizeof(fdt_size_t). It also + * always returns the first address value in the property (index 0). + * + * Use of this function is not recommended due to the hard-coding of cell + * counts. There is no programmatic validation that these hard-coded values + * actually match the device tree content in any way at all. This assumption + * can be satisfied by manually ensuring CONFIG_PHYS_64BIT is appropriately + * set in the U-Boot build and exercising strict control over DT content to + * ensure use of matching #address-cells/#size-cells properties. However, this + * approach is error-prone; those familiar with DT will not expect the + * assumption to exist, and could easily invalidate it. If the assumption is + * invalidated, this function will not report the issue, and debugging will + * be required. Instead, use fdtdec_get_addr_size_auto_parent(). * * @param blob FDT blob * @param node node to examine @@ -321,14 +401,29 @@ int fdtdec_next_compatible_subnode(const void *blob, int node, fdt_addr_t fdtdec_get_addr(const void *blob, int node, const char *prop_name); -/** - * Look up an address property in a node and return it as an address. - * The property must hold one address with a length. This is only tested - * on 32-bit machines. +/* + * Look up an address property in a node and return the parsed address, and + * optionally the parsed size. + * + * This variant hard-codes the number of cells used to represent the address + * and size based on sizeof(fdt_addr_t) and sizeof(fdt_size_t). It also + * always returns the first address value in the property (index 0). + * + * Use of this function is not recommended due to the hard-coding of cell + * counts. There is no programmatic validation that these hard-coded values + * actually match the device tree content in any way at all. This assumption + * can be satisfied by manually ensuring CONFIG_PHYS_64BIT is appropriately + * set in the U-Boot build and exercising strict control over DT content to + * ensure use of matching #address-cells/#size-cells properties. However, this + * approach is error-prone; those familiar with DT will not expect the + * assumption to exist, and could easily invalidate it. If the assumption is + * invalidated, this function will not report the issue, and debugging will + * be required. Instead, use fdtdec_get_addr_size_auto_parent(). * * @param blob FDT blob * @param node node to examine * @param prop_name name of property to find + * @param sizep a pointer to store the size into. Use NULL if not required * @return address, if found, or FDT_ADDR_T_NONE if not */ fdt_addr_t fdtdec_get_addr_size(const void *blob, int node, diff --git a/lib/fdtdec.c b/lib/fdtdec.c index 95b59b586ff0..3afec045e9bd 100644 --- a/lib/fdtdec.c +++ b/lib/fdtdec.c @@ -1,3 +1,5 @@ +#define DEBUG + /* * Copyright (c) 2011 The Chromium OS Authors. * SPDX-License-Identifier: GPL-2.0+ @@ -87,32 +89,104 @@ const char *fdtdec_get_compatible(enum fdt_compat_id id) return compat_names[id]; } -fdt_addr_t fdtdec_get_addr_size(const void *blob, int node, - const char *prop_name, fdt_size_t *sizep) +fdt_addr_t fdtdec_get_addr_size_fixed(const void *blob, int node, + const char *prop_name, int index, int na, int ns, + fdt_size_t *sizep) { - const fdt_addr_t *cell; + const fdt32_t *prop, *prop_end; + const fdt32_t *prop_addr, *prop_size, *prop_after_size; int len; + fdt_addr_t addr; debug("%s: %s: ", __func__, prop_name); - cell = fdt_getprop(blob, node, prop_name, &len); - if (cell && ((!sizep && len == sizeof(fdt_addr_t)) || - len == sizeof(fdt_addr_t) * 2)) { - fdt_addr_t addr = fdt_addr_to_cpu(*cell); - if (sizep) { - const fdt_size_t *size; - - size = (fdt_size_t *)((char *)cell + - sizeof(fdt_addr_t)); - *sizep = fdt_size_to_cpu(*size); - debug("addr=%08lx, size=%llx\n", - (ulong)addr, (u64)*sizep); - } else { - debug("%08lx\n", (ulong)addr); - } - return addr; + + if (na > (sizeof(fdt_addr_t) / sizeof(fdt32_t))) { + debug("(na too large for fdt_addr_t type)\n"); + return FDT_ADDR_T_NONE; } - debug("(not found)\n"); - return FDT_ADDR_T_NONE; + + if (ns > (sizeof(fdt_size_t) / sizeof(fdt32_t))) { + debug("(ns too large for fdt_size_t type)\n"); + return FDT_ADDR_T_NONE; + } + + prop = fdt_getprop(blob, node, prop_name, &len); + if (!prop) { + debug("(not found)\n"); + return FDT_ADDR_T_NONE; + } + prop_end = prop + (len / sizeof(*prop)); + + prop_addr = prop + (index * (na + ns)); + prop_size = prop_addr + na; + prop_after_size = prop_size + ns; + if (prop_after_size > prop_end) { + debug("(not enough data: expected >= %d cells, got %d cells)\n", + (u32)(prop_after_size - prop), ((u32)(prop_end - prop))); + return FDT_ADDR_T_NONE; + } + + addr = fdtdec_get_number(prop_addr, na); + + if (sizep) { + *sizep = fdtdec_get_number(prop_size, ns); + debug("addr=%08llx, size=%llx\n", (u64)addr, (u64)*sizep); + } else { + debug("addr=%08llx\n", (u64)addr); + } + + return addr; +} + +fdt_addr_t fdtdec_get_addr_size_auto_parent(const void *blob, int parent, + int node, const char *prop_name, int index, fdt_size_t *sizep) +{ + int na, ns; + + debug("%s: ", __func__); + + na = fdt_address_cells(blob, parent); + if (na < 1) { + debug("(bad #address-cells)\n"); + return FDT_ADDR_T_NONE; + } + + ns = fdt_size_cells(blob, parent); + if (ns < 1) { + debug("(bad #size-cells)\n"); + return FDT_ADDR_T_NONE; + } + + debug("na=%d, ns=%d, ", na, ns); + + return fdtdec_get_addr_size_fixed(blob, node, prop_name, index, na, + ns, sizep); +} + +fdt_addr_t fdtdec_get_addr_size_auto_noparent(const void *blob, int node, + const char *prop_name, int index, fdt_size_t *sizep) +{ + int parent; + + debug("%s: ", __func__); + + parent = fdt_parent_offset(blob, node); + if (parent < 0) { + debug("(no parent found)\n"); + return FDT_ADDR_T_NONE; + } + + return fdtdec_get_addr_size_auto_parent(blob, parent, node, prop_name, + index, sizep); +} + +fdt_addr_t fdtdec_get_addr_size(const void *blob, int node, + const char *prop_name, fdt_size_t *sizep) +{ + return fdtdec_get_addr_size_fixed(blob, node, prop_name, 0, + sizeof(fdt_addr_t) / sizeof(fdt32_t), + sizeof(fdt_size_t) / sizeof(fdt32_t), + sizep); } fdt_addr_t fdtdec_get_addr(const void *blob, int node,