All of lore.kernel.org
 help / color / mirror / Atom feed
From: Rob Herring <robh@kernel.org>
To: Calvin Zhang <calvinzhang.cool@gmail.com>
Cc: Vineet Gupta <vgupta@kernel.org>,
	Russell King <linux@armlinux.org.uk>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will@kernel.org>, Guo Ren <guoren@kernel.org>,
	Yoshinori Sato <ysato@users.sourceforge.jp>,
	Thomas Bogendoerfer <tsbogend@alpha.franken.de>,
	Nick Hu <nickhu@andestech.com>, Greentime Hu <green.hu@gmail.com>,
	Vincent Chen <deanbo422@gmail.com>,
	Dinh Nguyen <dinguyen@kernel.org>,
	Jonas Bonn <jonas@southpole.se>,
	Stefan Kristiansson <stefan.kristiansson@saunalahti.fi>,
	Stafford Horne <shorne@gmail.com>,
	Michael Ellerman <mpe@ellerman.id.au>,
	Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	Paul Mackerras <paulus@samba.org>,
	Paul Walmsley <paul.walmsley@sifive.com>,
	Palmer Dabbelt <palmer@dabbelt.com>,
	Albert Ou <aou@eecs.berkeley.edu>, Rich Felker <dalias@libc.org>,
	Chris Zankel <chris@zankel.net>,
	Max Filippov <jcmvbkbc@gmail.com>,
	Frank Rowand <frowand.list@gmail.com>,
	Mike Rapoport <rppt@kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	David Hildenbrand <david@redhat.com>,
	Kefeng Wang <wangkefeng.wang@huawei.com>,
	Vladimir Isaev <isaev@synopsys.com>,
	Arnd Bergmann <arnd@arndb.de>,
	"Russell King (Oracle)" <rmk+kernel@armlinux.org.uk>,
	Wolfram Sang <wsa+renesas@sang-engineering.com>,
	Guenter Roeck <linux@roeck-us.net>, Marc Zyngier <maz@kernel.org>,
	David Brazdil <dbrazdil@google.com>,
	Mark Rutland <mark.rutland@arm.com>,
	Andrey Konovalov <andreyknvl@gmail.com>,
	Anshuman Khandual <anshuman.khandual@arm.com>,
	Souptick Joarder <jrdr.linux@gmail.com>,
	Jinyang He <hejinyang@loongson.cn>,
	Alexander Sverdlin <alexander.sverdlin@nokia.com>,
	Serge Semin <Sergey.Semin@baikalelectronics.ru>,
	Geert Uytterhoeven <geert@linux-m68k.org>,
	Ley Foon Tan <ley.foon.tan@intel.com>,
	Andreas Oetken <andreas.oetken@siemens.com>,
	Randy Dunlap <rdunlap@infradead.org>,
	Christophe JAILLET <christophe.jaillet@wanadoo.fr>,
	Christophe Leroy <christophe.leroy@c-s.fr>,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	Zhang Yunkai <zhang.yunkai@zte.com.cn>,
	Markus Elfring <elfring@users.sourceforge.net>,
	Ganesh Goudar <ganeshgr@linux.ibm.com>,
	"Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com>,
	Atish Patra <atish.patra@wdc.com>,
	Anup Patel <anup.patel@wdc.com>,
	Nick Kossifidis <mick@ics.forth.gr>,
	Alexandre Ghiti <alex@ghiti.fr>,
	Vitaly Wool <vitaly.wool@konsulko.com>,
	"Rafael J. Wysocki" <rafael.j.wysocki@intel.com>,
	Guo Ren <guoren@linux.alibaba.com>,
	Tiezhu Yang <yangtiezhu@loongson.cn>,
	Mauri Sandberg <sandberg@mailfence.com>,
	Palmer Dabbelt <palmerdabbelt@google.com>,
	linux-snps-arc@lists.infradead.org, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, linux-csky@vger.kernel.org,
	uclinux-h8-devel@lists.sourceforge.jp,
	linux-mips@vger.kernel.org, openrisc@lists.librecores.org,
	linuxppc-dev@lists.ozlabs.org, linux-riscv@lists.infradead.org,
	linux-sh@vger.kernel.org, linux-xtensa@linux-xtensa.org,
	devicetree@vger.kernel.org
Subject: Re: [PATCH 1/2] of: Sort reserved_mem related code
Date: Mon, 29 Nov 2021 18:01:50 -0600	[thread overview]
Message-ID: <YaVp7ubpCLN1xWfF@robh.at.kernel.org> (raw)
In-Reply-To: <20211119075844.2902592-2-calvinzhang.cool@gmail.com>

On Fri, Nov 19, 2021 at 03:58:18PM +0800, Calvin Zhang wrote:
> Move code about parsing /reserved-memory and initializing of
> reserved_mems array to of_reserved_mem.c for better modularity.
> 
> Rename array name from reserved_mem to reserved_mems to distinguish
> from type definition.
> 
> Signed-off-by: Calvin Zhang <calvinzhang.cool@gmail.com>
> ---
>  drivers/of/fdt.c                | 108 +--------------------
>  drivers/of/of_private.h         |  12 ++-
>  drivers/of/of_reserved_mem.c    | 163 ++++++++++++++++++++++++++------
>  include/linux/of_reserved_mem.h |   4 +
>  4 files changed, 149 insertions(+), 138 deletions(-)
> 
> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> index bdca35284ceb..445af4e69300 100644
> --- a/drivers/of/fdt.c
> +++ b/drivers/of/fdt.c
> @@ -80,7 +80,7 @@ void __init of_fdt_limit_memory(int limit)
>  	}
>  }
>  
> -static bool of_fdt_device_is_available(const void *blob, unsigned long node)
> +bool of_fdt_device_is_available(const void *blob, unsigned long node)
>  {
>  	const char *status = fdt_getprop(blob, node, "status", NULL);
>  
> @@ -476,7 +476,7 @@ void *initial_boot_params __ro_after_init;
>  
>  static u32 of_fdt_crc32;
>  
> -static int __init early_init_dt_reserve_memory_arch(phys_addr_t base,
> +int __init early_init_dt_reserve_memory_arch(phys_addr_t base,
>  					phys_addr_t size, bool nomap)

I think you can move this function too if you change the nomap==false 
callers to just call memblock_reserve directly.


>  {
>  	if (nomap) {
> @@ -492,108 +492,6 @@ static int __init early_init_dt_reserve_memory_arch(phys_addr_t base,
>  	return memblock_reserve(base, size);
>  }
>  
> -/*
> - * __reserved_mem_reserve_reg() - reserve all memory described in 'reg' property
> - */
> -static int __init __reserved_mem_reserve_reg(unsigned long node,
> -					     const char *uname)
> -{
> -	int t_len = (dt_root_addr_cells + dt_root_size_cells) * sizeof(__be32);
> -	phys_addr_t base, size;
> -	int len;
> -	const __be32 *prop;
> -	int first = 1;
> -	bool nomap;
> -
> -	prop = of_get_flat_dt_prop(node, "reg", &len);
> -	if (!prop)
> -		return -ENOENT;
> -
> -	if (len && len % t_len != 0) {
> -		pr_err("Reserved memory: invalid reg property in '%s', skipping node.\n",
> -		       uname);
> -		return -EINVAL;
> -	}
> -
> -	nomap = of_get_flat_dt_prop(node, "no-map", NULL) != NULL;
> -
> -	while (len >= t_len) {
> -		base = dt_mem_next_cell(dt_root_addr_cells, &prop);
> -		size = dt_mem_next_cell(dt_root_size_cells, &prop);
> -
> -		if (size &&
> -		    early_init_dt_reserve_memory_arch(base, size, nomap) == 0)
> -			pr_debug("Reserved memory: reserved region for node '%s': base %pa, size %lu MiB\n",
> -				uname, &base, (unsigned long)(size / SZ_1M));
> -		else
> -			pr_info("Reserved memory: failed to reserve memory for node '%s': base %pa, size %lu MiB\n",
> -				uname, &base, (unsigned long)(size / SZ_1M));
> -
> -		len -= t_len;
> -		if (first) {
> -			fdt_reserved_mem_save_node(node, uname, base, size);
> -			first = 0;
> -		}
> -	}
> -	return 0;
> -}
> -
> -/*
> - * __reserved_mem_check_root() - check if #size-cells, #address-cells provided
> - * in /reserved-memory matches the values supported by the current implementation,
> - * also check if ranges property has been provided
> - */
> -static int __init __reserved_mem_check_root(unsigned long node)
> -{
> -	const __be32 *prop;
> -
> -	prop = of_get_flat_dt_prop(node, "#size-cells", NULL);
> -	if (!prop || be32_to_cpup(prop) != dt_root_size_cells)
> -		return -EINVAL;
> -
> -	prop = of_get_flat_dt_prop(node, "#address-cells", NULL);
> -	if (!prop || be32_to_cpup(prop) != dt_root_addr_cells)
> -		return -EINVAL;
> -
> -	prop = of_get_flat_dt_prop(node, "ranges", NULL);
> -	if (!prop)
> -		return -EINVAL;
> -	return 0;
> -}
> -
> -/*
> - * fdt_scan_reserved_mem() - scan a single FDT node for reserved memory
> - */
> -static int __init fdt_scan_reserved_mem(void)
> -{
> -	int node, child;
> -	const void *fdt = initial_boot_params;
> -
> -	node = fdt_path_offset(fdt, "/reserved-memory");
> -	if (node < 0)
> -		return -ENODEV;
> -
> -	if (__reserved_mem_check_root(node) != 0) {
> -		pr_err("Reserved memory: unsupported node format, ignoring\n");
> -		return -EINVAL;
> -	}
> -
> -	fdt_for_each_subnode(child, fdt, node) {
> -		const char *uname;
> -		int err;
> -
> -		if (!of_fdt_device_is_available(fdt, child))
> -			continue;
> -
> -		uname = fdt_get_name(fdt, child, NULL);
> -
> -		err = __reserved_mem_reserve_reg(child, uname);
> -		if (err == -ENOENT && of_get_flat_dt_prop(child, "size", NULL))
> -			fdt_reserved_mem_save_node(child, uname, 0, 0);
> -	}
> -	return 0;
> -}
> -
>  /*
>   * fdt_reserve_elfcorehdr() - reserves memory for elf core header
>   *
> @@ -642,7 +540,7 @@ void __init early_init_fdt_scan_reserved_mem(void)
>  	}
>  
>  	fdt_scan_reserved_mem();
> -	fdt_init_reserved_mem();
> +	of_reserved_mem_init();
>  	fdt_reserve_elfcorehdr();
>  }
>  
> diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h
> index 9324483397f6..88b67f8ed698 100644
> --- a/drivers/of/of_private.h
> +++ b/drivers/of/of_private.h
> @@ -163,8 +163,14 @@ static inline int of_dma_get_range(struct device_node *np,
>  }
>  #endif
>  
> -void fdt_init_reserved_mem(void);
> -void fdt_reserved_mem_save_node(unsigned long node, const char *uname,
> -			       phys_addr_t base, phys_addr_t size);
> +bool of_fdt_device_is_available(const void *blob, unsigned long node);
> +int early_init_dt_reserve_memory_arch(phys_addr_t base,
> +					phys_addr_t size, bool nomap);
> +#ifdef CONFIG_OF_RESERVED_MEM
> +int fdt_scan_reserved_mem(void);
> +#else
> +static inline int fdt_scan_reserved_mem(void) { }
> +#endif
> +
>  
>  #endif /* _LINUX_OF_PRIVATE_H */
> diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
> index 9c0fb962c22b..784cfc5cd251 100644
> --- a/drivers/of/of_reserved_mem.c
> +++ b/drivers/of/of_reserved_mem.c
> @@ -20,13 +20,14 @@
>  #include <linux/of_reserved_mem.h>
>  #include <linux/sort.h>
>  #include <linux/slab.h>
> +#include <linux/libfdt.h>
>  #include <linux/memblock.h>
>  #include <linux/kmemleak.h>
>  
>  #include "of_private.h"
>  
>  #define MAX_RESERVED_REGIONS	64
> -static struct reserved_mem reserved_mem[MAX_RESERVED_REGIONS];
> +static struct reserved_mem reserved_mems[MAX_RESERVED_REGIONS];

Would be a bit easier to review without the rename.


>  static int reserved_mem_count;
>  
>  static int __init early_init_dt_alloc_reserved_memory_arch(phys_addr_t size,
> @@ -56,12 +57,12 @@ static int __init early_init_dt_alloc_reserved_memory_arch(phys_addr_t size,
>  /*
>   * fdt_reserved_mem_save_node() - save fdt node for second pass initialization
>   */
> -void __init fdt_reserved_mem_save_node(unsigned long node, const char *uname,
> +static void __init fdt_reserved_mem_save_node(unsigned long node, const char *uname,
>  				      phys_addr_t base, phys_addr_t size)
>  {
> -	struct reserved_mem *rmem = &reserved_mem[reserved_mem_count];
> +	struct reserved_mem *rmem = &reserved_mems[reserved_mem_count];
>  
> -	if (reserved_mem_count == ARRAY_SIZE(reserved_mem)) {
> +	if (reserved_mem_count == ARRAY_SIZE(reserved_mems)) {
>  		pr_err("not enough space for all defined regions.\n");
>  		return;
>  	}
> @@ -173,29 +174,105 @@ static const struct of_device_id __rmem_of_table_sentinel
>  	__used __section("__reservedmem_of_table_end");
>  
>  /*
> - * __reserved_mem_init_node() - call region specific reserved memory init code
> + * __reserved_mem_check_root() - check if #size-cells, #address-cells provided
> + * in /reserved-memory matches the values supported by the current implementation,
> + * also check if ranges property has been provided
>   */
> -static int __init __reserved_mem_init_node(struct reserved_mem *rmem)
> +static int __init __reserved_mem_check_root(unsigned long node)
>  {
> -	extern const struct of_device_id __reservedmem_of_table[];
> -	const struct of_device_id *i;
> -	int ret = -ENOENT;
> +	const __be32 *prop;
>  
> -	for (i = __reservedmem_of_table; i < &__rmem_of_table_sentinel; i++) {
> -		reservedmem_of_init_fn initfn = i->data;
> -		const char *compat = i->compatible;
> +	prop = of_get_flat_dt_prop(node, "#size-cells", NULL);
> +	if (!prop || be32_to_cpup(prop) != dt_root_size_cells)
> +		return -EINVAL;
>  
> -		if (!of_flat_dt_is_compatible(rmem->fdt_node, compat))
> -			continue;
> +	prop = of_get_flat_dt_prop(node, "#address-cells", NULL);
> +	if (!prop || be32_to_cpup(prop) != dt_root_addr_cells)
> +		return -EINVAL;
>  
> -		ret = initfn(rmem);
> -		if (ret == 0) {
> -			pr_info("initialized node %s, compatible id %s\n",
> -				rmem->name, compat);
> -			break;
> +	prop = of_get_flat_dt_prop(node, "ranges", NULL);
> +	if (!prop)
> +		return -EINVAL;
> +	return 0;
> +}
> +
> +/*
> + * __reserved_mem_reserve_reg() - reserve all memory described in 'reg' property
> + */
> +static int __init __reserved_mem_reserve_reg(unsigned long node,
> +					     const char *uname)
> +{
> +	int t_len = (dt_root_addr_cells + dt_root_size_cells) * sizeof(__be32);
> +	phys_addr_t base, size;
> +	int len;
> +	const __be32 *prop;
> +	int first = 1;
> +	bool nomap;
> +
> +	prop = of_get_flat_dt_prop(node, "reg", &len);
> +	if (!prop)
> +		return -ENOENT;
> +
> +	if (len && len % t_len != 0) {
> +		pr_err("Reserved memory: invalid reg property in '%s', skipping node.\n",
> +		       uname);
> +		return -EINVAL;
> +	}
> +
> +	nomap = of_get_flat_dt_prop(node, "no-map", NULL) != NULL;
> +
> +	while (len >= t_len) {
> +		base = dt_mem_next_cell(dt_root_addr_cells, &prop);
> +		size = dt_mem_next_cell(dt_root_size_cells, &prop);
> +
> +		if (size &&
> +		    early_init_dt_reserve_memory_arch(base, size, nomap) == 0)
> +			pr_debug("Reserved memory: reserved region for node '%s': base %pa, size %lu MiB\n",
> +				uname, &base, (unsigned long)(size / SZ_1M));
> +		else
> +			pr_info("Reserved memory: failed to reserve memory for node '%s': base %pa, size %lu MiB\n",
> +				uname, &base, (unsigned long)(size / SZ_1M));
> +
> +		len -= t_len;
> +		if (first) {
> +			fdt_reserved_mem_save_node(node, uname, base, size);
> +			first = 0;
>  		}
>  	}
> -	return ret;
> +	return 0;
> +}
> +
> +/*
> + * fdt_scan_reserved_mem() - scan a single FDT node for reserved memory
> + */
> +int __init fdt_scan_reserved_mem(void)
> +{
> +	int node, child;
> +	const void *fdt = initial_boot_params;
> +
> +	node = fdt_path_offset(fdt, "/reserved-memory");
> +	if (node < 0)
> +		return -ENODEV;
> +
> +	if (__reserved_mem_check_root(node) != 0) {
> +		pr_err("Reserved memory: unsupported node format, ignoring\n");
> +		return -EINVAL;
> +	}
> +
> +	fdt_for_each_subnode(child, fdt, node) {
> +		const char *uname;
> +		int err;
> +
> +		if (!of_fdt_device_is_available(fdt, child))
> +			continue;
> +
> +		uname = fdt_get_name(fdt, child, NULL);
> +
> +		err = __reserved_mem_reserve_reg(child, uname);
> +		if (err == -ENOENT && of_get_flat_dt_prop(child, "size", NULL))
> +			fdt_reserved_mem_save_node(child, uname, 0, 0);
> +	}
> +	return 0;
>  }
>  
>  static int __init __rmem_cmp(const void *a, const void *b)
> @@ -228,13 +305,13 @@ static void __init __rmem_check_for_overlap(void)
>  	if (reserved_mem_count < 2)
>  		return;
>  
> -	sort(reserved_mem, reserved_mem_count, sizeof(reserved_mem[0]),
> +	sort(reserved_mems, reserved_mem_count, sizeof(reserved_mems[0]),
>  	     __rmem_cmp, NULL);
>  	for (i = 0; i < reserved_mem_count - 1; i++) {
>  		struct reserved_mem *this, *next;
>  
> -		this = &reserved_mem[i];
> -		next = &reserved_mem[i + 1];
> +		this = &reserved_mems[i];
> +		next = &reserved_mems[i + 1];
>  
>  		if (this->base + this->size > next->base) {
>  			phys_addr_t this_end, next_end;
> @@ -248,10 +325,36 @@ static void __init __rmem_check_for_overlap(void)
>  	}
>  }
>  
> +/*
> + * __reserved_mem_init_node() - call region specific reserved memory init code
> + */
> +static int __init __reserved_mem_init_node(struct reserved_mem *rmem)
> +{
> +	extern const struct of_device_id __reservedmem_of_table[];
> +	const struct of_device_id *i;
> +	int ret = -ENOENT;
> +
> +	for (i = __reservedmem_of_table; i < &__rmem_of_table_sentinel; i++) {
> +		reservedmem_of_init_fn initfn = i->data;
> +		const char *compat = i->compatible;
> +
> +		if (!of_flat_dt_is_compatible(rmem->fdt_node, compat))
> +			continue;
> +
> +		ret = initfn(rmem);
> +		if (ret == 0) {
> +			pr_info("initialized node %s, compatible id %s\n",
> +				rmem->name, compat);
> +			break;
> +		}
> +	}
> +	return ret;
> +}
> +
>  /**
> - * fdt_init_reserved_mem() - allocate and init all saved reserved memory regions
> + * of_reserved_mem_init() - allocate and init all saved reserved memory regions
>   */
> -void __init fdt_init_reserved_mem(void)
> +void __init of_reserved_mem_init(void)
>  {
>  	int i;
>  
> @@ -259,7 +362,7 @@ void __init fdt_init_reserved_mem(void)
>  	__rmem_check_for_overlap();
>  
>  	for (i = 0; i < reserved_mem_count; i++) {
> -		struct reserved_mem *rmem = &reserved_mem[i];
> +		struct reserved_mem *rmem = &reserved_mems[i];
>  		unsigned long node = rmem->fdt_node;
>  		int len;
>  		const __be32 *prop;
> @@ -299,8 +402,8 @@ static inline struct reserved_mem *__find_rmem(struct device_node *node)
>  		return NULL;
>  
>  	for (i = 0; i < reserved_mem_count; i++)
> -		if (reserved_mem[i].phandle == node->phandle)
> -			return &reserved_mem[i];
> +		if (reserved_mems[i].phandle == node->phandle)
> +			return &reserved_mems[i];
>  	return NULL;
>  }
>  
> @@ -442,8 +545,8 @@ struct reserved_mem *of_reserved_mem_lookup(struct device_node *np)
>  
>  	name = kbasename(np->full_name);
>  	for (i = 0; i < reserved_mem_count; i++)
> -		if (!strcmp(reserved_mem[i].name, name))
> -			return &reserved_mem[i];
> +		if (!strcmp(reserved_mems[i].name, name))
> +			return &reserved_mems[i];
>  
>  	return NULL;
>  }
> diff --git a/include/linux/of_reserved_mem.h b/include/linux/of_reserved_mem.h
> index 4de2a24cadc9..34e134bec606 100644
> --- a/include/linux/of_reserved_mem.h
> +++ b/include/linux/of_reserved_mem.h
> @@ -32,6 +32,8 @@ typedef int (*reservedmem_of_init_fn)(struct reserved_mem *rmem);
>  #define RESERVEDMEM_OF_DECLARE(name, compat, init)			\
>  	_OF_DECLARE(reservedmem, name, compat, init, reservedmem_of_init_fn)
>  
> +void of_reserved_mem_init(void);
> +
>  int of_reserved_mem_device_init_by_idx(struct device *dev,
>  				       struct device_node *np, int idx);
>  int of_reserved_mem_device_init_by_name(struct device *dev,
> @@ -45,6 +47,8 @@ struct reserved_mem *of_reserved_mem_lookup(struct device_node *np);
>  #define RESERVEDMEM_OF_DECLARE(name, compat, init)			\
>  	_OF_DECLARE_STUB(reservedmem, name, compat, init, reservedmem_of_init_fn)
>  
> +static inline void of_reserved_mem_init(void) { }
> +
>  static inline int of_reserved_mem_device_init_by_idx(struct device *dev,
>  					struct device_node *np, int idx)
>  {
> -- 
> 2.30.2
> 
> 

WARNING: multiple messages have this Message-ID (diff)
From: Rob Herring <robh@kernel.org>
To: Calvin Zhang <calvinzhang.cool@gmail.com>
Cc: Vineet Gupta <vgupta@kernel.org>,
	Russell King <linux@armlinux.org.uk>,
	 Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will@kernel.org>, Guo Ren <guoren@kernel.org>,
	Yoshinori Sato <ysato@users.sourceforge.jp>,
	 Thomas Bogendoerfer <tsbogend@alpha.franken.de>,
	Nick Hu <nickhu@andestech.com>, Greentime Hu <green.hu@gmail.com>,
	Vincent Chen <deanbo422@gmail.com>,
	 Dinh Nguyen <dinguyen@kernel.org>,
	Jonas Bonn <jonas@southpole.se>,
	 Stefan Kristiansson <stefan.kristiansson@saunalahti.fi>,
	Stafford Horne <shorne@gmail.com>,
	Michael Ellerman <mpe@ellerman.id.au>,
	Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	 Paul Mackerras <paulus@samba.org>,
	Paul Walmsley <paul.walmsley@sifive.com>,
	 Palmer Dabbelt <palmer@dabbelt.com>,
	Albert Ou <aou@eecs.berkeley.edu>,  Rich Felker <dalias@libc.org>,
	Chris Zankel <chris@zankel.net>,
	Max Filippov <jcmvbkbc@gmail.com>,
	 Frank Rowand <frowand.list@gmail.com>,
	Mike Rapoport <rppt@kernel.org>,
	 Andrew Morton <akpm@linux-foundation.org>,
	David Hildenbrand <david@redhat.com>,
	Kefeng Wang <wangkefeng.wang@huawei.com>,
	Vladimir Isaev <isaev@synopsys.com>,
	 Arnd Bergmann <arnd@arndb.de>,
	"Russell King (Oracle)" <rmk+kernel@armlinux.org.uk>,
	 Wolfram Sang <wsa+renesas@sang-engineering.com>,
	Guenter Roeck <linux@roeck-us.net>,
	 Marc Zyngier <maz@kernel.org>,
	David Brazdil <dbrazdil@google.com>,
	 Mark Rutland <mark.rutland@arm.com>,
	Andrey Konovalov <andreyknvl@gmail.com>,
	 Anshuman Khandual <anshuman.khandual@arm.com>,
	Souptick Joarder <jrdr.linux@gmail.com>,
	 Jinyang He <hejinyang@loongson.cn>,
	Alexander Sverdlin <alexander.sverdlin@nokia.com>,
	 Serge Semin <Sergey.Semin@baikalelectronics.ru>,
	 Geert Uytterhoeven <geert@linux-m68k.org>,
	Ley Foon Tan <ley.foon.tan@intel.com>,
	 Andreas Oetken <andreas.oetken@siemens.com>,
	Randy Dunlap <rdunlap@infradead.org>,
	 Christophe JAILLET <christophe.jaillet@wanadoo.fr>,
	Christophe Leroy <christophe.leroy@c-s.fr>,
	 Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	Zhang Yunkai <zhang.yunkai@zte.com.cn>,
	 Markus Elfring <elfring@users.sourceforge.net>,
	Ganesh Goudar <ganeshgr@linux.ibm.com>,
	 "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com>,
	Atish Patra <atish.patra@wdc.com>,
	 Anup Patel <anup.patel@wdc.com>,
	Nick Kossifidis <mick@ics.forth.gr>,
	Alexandre Ghiti <alex@ghiti.fr>,
	Vitaly Wool <vitaly.wool@konsulko.com>,
	"Rafael J. Wysocki" <rafael.j.wysocki@intel.com>,
	 Guo Ren <guoren@linux.alibaba.com>,
	Tiezhu Yang <yangtiezhu@loongson.cn>,
	 Mauri Sandberg <sandberg@mailfence.com>,
	Palmer Dabbelt <palmerdabbelt@google.com>,
	 linux-snps-arc@lists.infradead.org,
	linux-kernel@vger.kernel.org,
	 linux-arm-kernel@lists.infradead.org,
	linux-csky@vger.kernel.org,
	 uclinux-h8-devel@lists.sourceforge.jp,
	linux-mips@vger.kernel.org,  openrisc@lists.librecores.org,
	linuxppc-dev@lists.ozlabs.org,  linux-riscv@lists.infradead.org,
	linux-sh@vger.kernel.org,  linux-xtensa@linux-xtensa.org,
	devicetree@vger.kernel.org
Subject: Re: [PATCH 1/2] of: Sort reserved_mem related code
Date: Mon, 29 Nov 2021 18:01:50 -0600	[thread overview]
Message-ID: <YaVp7ubpCLN1xWfF@robh.at.kernel.org> (raw)
In-Reply-To: <20211119075844.2902592-2-calvinzhang.cool@gmail.com>

On Fri, Nov 19, 2021 at 03:58:18PM +0800, Calvin Zhang wrote:
> Move code about parsing /reserved-memory and initializing of
> reserved_mems array to of_reserved_mem.c for better modularity.
> 
> Rename array name from reserved_mem to reserved_mems to distinguish
> from type definition.
> 
> Signed-off-by: Calvin Zhang <calvinzhang.cool@gmail.com>
> ---
>  drivers/of/fdt.c                | 108 +--------------------
>  drivers/of/of_private.h         |  12 ++-
>  drivers/of/of_reserved_mem.c    | 163 ++++++++++++++++++++++++++------
>  include/linux/of_reserved_mem.h |   4 +
>  4 files changed, 149 insertions(+), 138 deletions(-)
> 
> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> index bdca35284ceb..445af4e69300 100644
> --- a/drivers/of/fdt.c
> +++ b/drivers/of/fdt.c
> @@ -80,7 +80,7 @@ void __init of_fdt_limit_memory(int limit)
>  	}
>  }
>  
> -static bool of_fdt_device_is_available(const void *blob, unsigned long node)
> +bool of_fdt_device_is_available(const void *blob, unsigned long node)
>  {
>  	const char *status = fdt_getprop(blob, node, "status", NULL);
>  
> @@ -476,7 +476,7 @@ void *initial_boot_params __ro_after_init;
>  
>  static u32 of_fdt_crc32;
>  
> -static int __init early_init_dt_reserve_memory_arch(phys_addr_t base,
> +int __init early_init_dt_reserve_memory_arch(phys_addr_t base,
>  					phys_addr_t size, bool nomap)

I think you can move this function too if you change the nomap==false 
callers to just call memblock_reserve directly.


>  {
>  	if (nomap) {
> @@ -492,108 +492,6 @@ static int __init early_init_dt_reserve_memory_arch(phys_addr_t base,
>  	return memblock_reserve(base, size);
>  }
>  
> -/*
> - * __reserved_mem_reserve_reg() - reserve all memory described in 'reg' property
> - */
> -static int __init __reserved_mem_reserve_reg(unsigned long node,
> -					     const char *uname)
> -{
> -	int t_len = (dt_root_addr_cells + dt_root_size_cells) * sizeof(__be32);
> -	phys_addr_t base, size;
> -	int len;
> -	const __be32 *prop;
> -	int first = 1;
> -	bool nomap;
> -
> -	prop = of_get_flat_dt_prop(node, "reg", &len);
> -	if (!prop)
> -		return -ENOENT;
> -
> -	if (len && len % t_len != 0) {
> -		pr_err("Reserved memory: invalid reg property in '%s', skipping node.\n",
> -		       uname);
> -		return -EINVAL;
> -	}
> -
> -	nomap = of_get_flat_dt_prop(node, "no-map", NULL) != NULL;
> -
> -	while (len >= t_len) {
> -		base = dt_mem_next_cell(dt_root_addr_cells, &prop);
> -		size = dt_mem_next_cell(dt_root_size_cells, &prop);
> -
> -		if (size &&
> -		    early_init_dt_reserve_memory_arch(base, size, nomap) == 0)
> -			pr_debug("Reserved memory: reserved region for node '%s': base %pa, size %lu MiB\n",
> -				uname, &base, (unsigned long)(size / SZ_1M));
> -		else
> -			pr_info("Reserved memory: failed to reserve memory for node '%s': base %pa, size %lu MiB\n",
> -				uname, &base, (unsigned long)(size / SZ_1M));
> -
> -		len -= t_len;
> -		if (first) {
> -			fdt_reserved_mem_save_node(node, uname, base, size);
> -			first = 0;
> -		}
> -	}
> -	return 0;
> -}
> -
> -/*
> - * __reserved_mem_check_root() - check if #size-cells, #address-cells provided
> - * in /reserved-memory matches the values supported by the current implementation,
> - * also check if ranges property has been provided
> - */
> -static int __init __reserved_mem_check_root(unsigned long node)
> -{
> -	const __be32 *prop;
> -
> -	prop = of_get_flat_dt_prop(node, "#size-cells", NULL);
> -	if (!prop || be32_to_cpup(prop) != dt_root_size_cells)
> -		return -EINVAL;
> -
> -	prop = of_get_flat_dt_prop(node, "#address-cells", NULL);
> -	if (!prop || be32_to_cpup(prop) != dt_root_addr_cells)
> -		return -EINVAL;
> -
> -	prop = of_get_flat_dt_prop(node, "ranges", NULL);
> -	if (!prop)
> -		return -EINVAL;
> -	return 0;
> -}
> -
> -/*
> - * fdt_scan_reserved_mem() - scan a single FDT node for reserved memory
> - */
> -static int __init fdt_scan_reserved_mem(void)
> -{
> -	int node, child;
> -	const void *fdt = initial_boot_params;
> -
> -	node = fdt_path_offset(fdt, "/reserved-memory");
> -	if (node < 0)
> -		return -ENODEV;
> -
> -	if (__reserved_mem_check_root(node) != 0) {
> -		pr_err("Reserved memory: unsupported node format, ignoring\n");
> -		return -EINVAL;
> -	}
> -
> -	fdt_for_each_subnode(child, fdt, node) {
> -		const char *uname;
> -		int err;
> -
> -		if (!of_fdt_device_is_available(fdt, child))
> -			continue;
> -
> -		uname = fdt_get_name(fdt, child, NULL);
> -
> -		err = __reserved_mem_reserve_reg(child, uname);
> -		if (err == -ENOENT && of_get_flat_dt_prop(child, "size", NULL))
> -			fdt_reserved_mem_save_node(child, uname, 0, 0);
> -	}
> -	return 0;
> -}
> -
>  /*
>   * fdt_reserve_elfcorehdr() - reserves memory for elf core header
>   *
> @@ -642,7 +540,7 @@ void __init early_init_fdt_scan_reserved_mem(void)
>  	}
>  
>  	fdt_scan_reserved_mem();
> -	fdt_init_reserved_mem();
> +	of_reserved_mem_init();
>  	fdt_reserve_elfcorehdr();
>  }
>  
> diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h
> index 9324483397f6..88b67f8ed698 100644
> --- a/drivers/of/of_private.h
> +++ b/drivers/of/of_private.h
> @@ -163,8 +163,14 @@ static inline int of_dma_get_range(struct device_node *np,
>  }
>  #endif
>  
> -void fdt_init_reserved_mem(void);
> -void fdt_reserved_mem_save_node(unsigned long node, const char *uname,
> -			       phys_addr_t base, phys_addr_t size);
> +bool of_fdt_device_is_available(const void *blob, unsigned long node);
> +int early_init_dt_reserve_memory_arch(phys_addr_t base,
> +					phys_addr_t size, bool nomap);
> +#ifdef CONFIG_OF_RESERVED_MEM
> +int fdt_scan_reserved_mem(void);
> +#else
> +static inline int fdt_scan_reserved_mem(void) { }
> +#endif
> +
>  
>  #endif /* _LINUX_OF_PRIVATE_H */
> diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
> index 9c0fb962c22b..784cfc5cd251 100644
> --- a/drivers/of/of_reserved_mem.c
> +++ b/drivers/of/of_reserved_mem.c
> @@ -20,13 +20,14 @@
>  #include <linux/of_reserved_mem.h>
>  #include <linux/sort.h>
>  #include <linux/slab.h>
> +#include <linux/libfdt.h>
>  #include <linux/memblock.h>
>  #include <linux/kmemleak.h>
>  
>  #include "of_private.h"
>  
>  #define MAX_RESERVED_REGIONS	64
> -static struct reserved_mem reserved_mem[MAX_RESERVED_REGIONS];
> +static struct reserved_mem reserved_mems[MAX_RESERVED_REGIONS];

Would be a bit easier to review without the rename.


>  static int reserved_mem_count;
>  
>  static int __init early_init_dt_alloc_reserved_memory_arch(phys_addr_t size,
> @@ -56,12 +57,12 @@ static int __init early_init_dt_alloc_reserved_memory_arch(phys_addr_t size,
>  /*
>   * fdt_reserved_mem_save_node() - save fdt node for second pass initialization
>   */
> -void __init fdt_reserved_mem_save_node(unsigned long node, const char *uname,
> +static void __init fdt_reserved_mem_save_node(unsigned long node, const char *uname,
>  				      phys_addr_t base, phys_addr_t size)
>  {
> -	struct reserved_mem *rmem = &reserved_mem[reserved_mem_count];
> +	struct reserved_mem *rmem = &reserved_mems[reserved_mem_count];
>  
> -	if (reserved_mem_count == ARRAY_SIZE(reserved_mem)) {
> +	if (reserved_mem_count == ARRAY_SIZE(reserved_mems)) {
>  		pr_err("not enough space for all defined regions.\n");
>  		return;
>  	}
> @@ -173,29 +174,105 @@ static const struct of_device_id __rmem_of_table_sentinel
>  	__used __section("__reservedmem_of_table_end");
>  
>  /*
> - * __reserved_mem_init_node() - call region specific reserved memory init code
> + * __reserved_mem_check_root() - check if #size-cells, #address-cells provided
> + * in /reserved-memory matches the values supported by the current implementation,
> + * also check if ranges property has been provided
>   */
> -static int __init __reserved_mem_init_node(struct reserved_mem *rmem)
> +static int __init __reserved_mem_check_root(unsigned long node)
>  {
> -	extern const struct of_device_id __reservedmem_of_table[];
> -	const struct of_device_id *i;
> -	int ret = -ENOENT;
> +	const __be32 *prop;
>  
> -	for (i = __reservedmem_of_table; i < &__rmem_of_table_sentinel; i++) {
> -		reservedmem_of_init_fn initfn = i->data;
> -		const char *compat = i->compatible;
> +	prop = of_get_flat_dt_prop(node, "#size-cells", NULL);
> +	if (!prop || be32_to_cpup(prop) != dt_root_size_cells)
> +		return -EINVAL;
>  
> -		if (!of_flat_dt_is_compatible(rmem->fdt_node, compat))
> -			continue;
> +	prop = of_get_flat_dt_prop(node, "#address-cells", NULL);
> +	if (!prop || be32_to_cpup(prop) != dt_root_addr_cells)
> +		return -EINVAL;
>  
> -		ret = initfn(rmem);
> -		if (ret == 0) {
> -			pr_info("initialized node %s, compatible id %s\n",
> -				rmem->name, compat);
> -			break;
> +	prop = of_get_flat_dt_prop(node, "ranges", NULL);
> +	if (!prop)
> +		return -EINVAL;
> +	return 0;
> +}
> +
> +/*
> + * __reserved_mem_reserve_reg() - reserve all memory described in 'reg' property
> + */
> +static int __init __reserved_mem_reserve_reg(unsigned long node,
> +					     const char *uname)
> +{
> +	int t_len = (dt_root_addr_cells + dt_root_size_cells) * sizeof(__be32);
> +	phys_addr_t base, size;
> +	int len;
> +	const __be32 *prop;
> +	int first = 1;
> +	bool nomap;
> +
> +	prop = of_get_flat_dt_prop(node, "reg", &len);
> +	if (!prop)
> +		return -ENOENT;
> +
> +	if (len && len % t_len != 0) {
> +		pr_err("Reserved memory: invalid reg property in '%s', skipping node.\n",
> +		       uname);
> +		return -EINVAL;
> +	}
> +
> +	nomap = of_get_flat_dt_prop(node, "no-map", NULL) != NULL;
> +
> +	while (len >= t_len) {
> +		base = dt_mem_next_cell(dt_root_addr_cells, &prop);
> +		size = dt_mem_next_cell(dt_root_size_cells, &prop);
> +
> +		if (size &&
> +		    early_init_dt_reserve_memory_arch(base, size, nomap) == 0)
> +			pr_debug("Reserved memory: reserved region for node '%s': base %pa, size %lu MiB\n",
> +				uname, &base, (unsigned long)(size / SZ_1M));
> +		else
> +			pr_info("Reserved memory: failed to reserve memory for node '%s': base %pa, size %lu MiB\n",
> +				uname, &base, (unsigned long)(size / SZ_1M));
> +
> +		len -= t_len;
> +		if (first) {
> +			fdt_reserved_mem_save_node(node, uname, base, size);
> +			first = 0;
>  		}
>  	}
> -	return ret;
> +	return 0;
> +}
> +
> +/*
> + * fdt_scan_reserved_mem() - scan a single FDT node for reserved memory
> + */
> +int __init fdt_scan_reserved_mem(void)
> +{
> +	int node, child;
> +	const void *fdt = initial_boot_params;
> +
> +	node = fdt_path_offset(fdt, "/reserved-memory");
> +	if (node < 0)
> +		return -ENODEV;
> +
> +	if (__reserved_mem_check_root(node) != 0) {
> +		pr_err("Reserved memory: unsupported node format, ignoring\n");
> +		return -EINVAL;
> +	}
> +
> +	fdt_for_each_subnode(child, fdt, node) {
> +		const char *uname;
> +		int err;
> +
> +		if (!of_fdt_device_is_available(fdt, child))
> +			continue;
> +
> +		uname = fdt_get_name(fdt, child, NULL);
> +
> +		err = __reserved_mem_reserve_reg(child, uname);
> +		if (err == -ENOENT && of_get_flat_dt_prop(child, "size", NULL))
> +			fdt_reserved_mem_save_node(child, uname, 0, 0);
> +	}
> +	return 0;
>  }
>  
>  static int __init __rmem_cmp(const void *a, const void *b)
> @@ -228,13 +305,13 @@ static void __init __rmem_check_for_overlap(void)
>  	if (reserved_mem_count < 2)
>  		return;
>  
> -	sort(reserved_mem, reserved_mem_count, sizeof(reserved_mem[0]),
> +	sort(reserved_mems, reserved_mem_count, sizeof(reserved_mems[0]),
>  	     __rmem_cmp, NULL);
>  	for (i = 0; i < reserved_mem_count - 1; i++) {
>  		struct reserved_mem *this, *next;
>  
> -		this = &reserved_mem[i];
> -		next = &reserved_mem[i + 1];
> +		this = &reserved_mems[i];
> +		next = &reserved_mems[i + 1];
>  
>  		if (this->base + this->size > next->base) {
>  			phys_addr_t this_end, next_end;
> @@ -248,10 +325,36 @@ static void __init __rmem_check_for_overlap(void)
>  	}
>  }
>  
> +/*
> + * __reserved_mem_init_node() - call region specific reserved memory init code
> + */
> +static int __init __reserved_mem_init_node(struct reserved_mem *rmem)
> +{
> +	extern const struct of_device_id __reservedmem_of_table[];
> +	const struct of_device_id *i;
> +	int ret = -ENOENT;
> +
> +	for (i = __reservedmem_of_table; i < &__rmem_of_table_sentinel; i++) {
> +		reservedmem_of_init_fn initfn = i->data;
> +		const char *compat = i->compatible;
> +
> +		if (!of_flat_dt_is_compatible(rmem->fdt_node, compat))
> +			continue;
> +
> +		ret = initfn(rmem);
> +		if (ret == 0) {
> +			pr_info("initialized node %s, compatible id %s\n",
> +				rmem->name, compat);
> +			break;
> +		}
> +	}
> +	return ret;
> +}
> +
>  /**
> - * fdt_init_reserved_mem() - allocate and init all saved reserved memory regions
> + * of_reserved_mem_init() - allocate and init all saved reserved memory regions
>   */
> -void __init fdt_init_reserved_mem(void)
> +void __init of_reserved_mem_init(void)
>  {
>  	int i;
>  
> @@ -259,7 +362,7 @@ void __init fdt_init_reserved_mem(void)
>  	__rmem_check_for_overlap();
>  
>  	for (i = 0; i < reserved_mem_count; i++) {
> -		struct reserved_mem *rmem = &reserved_mem[i];
> +		struct reserved_mem *rmem = &reserved_mems[i];
>  		unsigned long node = rmem->fdt_node;
>  		int len;
>  		const __be32 *prop;
> @@ -299,8 +402,8 @@ static inline struct reserved_mem *__find_rmem(struct device_node *node)
>  		return NULL;
>  
>  	for (i = 0; i < reserved_mem_count; i++)
> -		if (reserved_mem[i].phandle == node->phandle)
> -			return &reserved_mem[i];
> +		if (reserved_mems[i].phandle == node->phandle)
> +			return &reserved_mems[i];
>  	return NULL;
>  }
>  
> @@ -442,8 +545,8 @@ struct reserved_mem *of_reserved_mem_lookup(struct device_node *np)
>  
>  	name = kbasename(np->full_name);
>  	for (i = 0; i < reserved_mem_count; i++)
> -		if (!strcmp(reserved_mem[i].name, name))
> -			return &reserved_mem[i];
> +		if (!strcmp(reserved_mems[i].name, name))
> +			return &reserved_mems[i];
>  
>  	return NULL;
>  }
> diff --git a/include/linux/of_reserved_mem.h b/include/linux/of_reserved_mem.h
> index 4de2a24cadc9..34e134bec606 100644
> --- a/include/linux/of_reserved_mem.h
> +++ b/include/linux/of_reserved_mem.h
> @@ -32,6 +32,8 @@ typedef int (*reservedmem_of_init_fn)(struct reserved_mem *rmem);
>  #define RESERVEDMEM_OF_DECLARE(name, compat, init)			\
>  	_OF_DECLARE(reservedmem, name, compat, init, reservedmem_of_init_fn)
>  
> +void of_reserved_mem_init(void);
> +
>  int of_reserved_mem_device_init_by_idx(struct device *dev,
>  				       struct device_node *np, int idx);
>  int of_reserved_mem_device_init_by_name(struct device *dev,
> @@ -45,6 +47,8 @@ struct reserved_mem *of_reserved_mem_lookup(struct device_node *np);
>  #define RESERVEDMEM_OF_DECLARE(name, compat, init)			\
>  	_OF_DECLARE_STUB(reservedmem, name, compat, init, reservedmem_of_init_fn)
>  
> +static inline void of_reserved_mem_init(void) { }
> +
>  static inline int of_reserved_mem_device_init_by_idx(struct device *dev,
>  					struct device_node *np, int idx)
>  {
> -- 
> 2.30.2
> 
> 

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

WARNING: multiple messages have this Message-ID (diff)
From: Rob Herring <robh@kernel.org>
To: Calvin Zhang <calvinzhang.cool@gmail.com>
Cc: Vineet Gupta <vgupta@kernel.org>,
	Russell King <linux@armlinux.org.uk>,
	 Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will@kernel.org>, Guo Ren <guoren@kernel.org>,
	Yoshinori Sato <ysato@users.sourceforge.jp>,
	 Thomas Bogendoerfer <tsbogend@alpha.franken.de>,
	Nick Hu <nickhu@andestech.com>, Greentime Hu <green.hu@gmail.com>,
	Vincent Chen <deanbo422@gmail.com>,
	 Dinh Nguyen <dinguyen@kernel.org>,
	Jonas Bonn <jonas@southpole.se>,
	 Stefan Kristiansson <stefan.kristiansson@saunalahti.fi>,
	Stafford Horne <shorne@gmail.com>,
	Michael Ellerman <mpe@ellerman.id.au>,
	Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	 Paul Mackerras <paulus@samba.org>,
	Paul Walmsley <paul.walmsley@sifive.com>,
	 Palmer Dabbelt <palmer@dabbelt.com>,
	Albert Ou <aou@eecs.berkeley.edu>,  Rich Felker <dalias@libc.org>,
	Chris Zankel <chris@zankel.net>,
	Max Filippov <jcmvbkbc@gmail.com>,
	 Frank Rowand <frowand.list@gmail.com>,
	Mike Rapoport <rppt@kernel.org>,
	 Andrew Morton <akpm@linux-foundation.org>,
	David Hildenbrand <david@redhat.com>,
	Kefeng Wang <wangkefeng.wang@huawei.com>,
	Vladimir Isaev <isaev@synopsys.com>,
	 Arnd Bergmann <arnd@arndb.de>,
	"Russell King (Oracle)" <rmk+kernel@armlinux.org.uk>,
	 Wolfram Sang <wsa+renesas@sang-engineering.com>,
	Guenter Roeck <linux@roeck-us.net>,
	 Marc Zyngier <maz@kernel.org>,
	David Brazdil <dbrazdil@google.com>,
	 Mark Rutland <mark.rutland@arm.com>,
	Andrey Konovalov <andreyknvl@gmail.com>,
	 Anshuman Khandual <anshuman.khandual@arm.com>,
	Souptick Joarder <jrdr.linux@gmail.com>,
	 Jinyang He <hejinyang@loongson.cn>,
	Alexander Sverdlin <alexander.sverdlin@nokia.com>,
	 Serge Semin <Sergey.Semin@baikalelectronics.ru>,
	 Geert Uytterhoeven <geert@linux-m68k.org>,
	Ley Foon Tan <ley.foon.tan@intel.com>,
	 Andreas Oetken <andreas.oetken@siemens.com>,
	Randy Dunlap <rdunlap@infradead.org>,
	 Christophe JAILLET <christophe.jaillet@wanadoo.fr>,
	Christophe Leroy <christophe.leroy@c-s.fr>,
	 Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	Zhang Yunkai <zhang.yunkai@zte.com.cn>,
	 Markus Elfring <elfring@users.sourceforge.net>,
	Ganesh Goudar <ganeshgr@linux.ibm.com>,
	 "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com>,
	Atish Patra <atish.patra@wdc.com>,
	 Anup Patel <anup.patel@wdc.com>,
	Nick Kossifidis <mick@ics.forth.gr>,
	Alexandre Ghiti <alex@ghiti.fr>,
	Vitaly Wool <vitaly.wool@konsulko.com>,
	"Rafael J. Wysocki" <rafael.j.wysocki@intel.com>,
	 Guo Ren <guoren@linux.alibaba.com>,
	Tiezhu Yang <yangtiezhu@loongson.cn>,
	 Mauri Sandberg <sandberg@mailfence.com>,
	Palmer Dabbelt <palmerdabbelt@google.com>,
	 linux-snps-arc@lists.infradead.org,
	linux-kernel@vger.kernel.org,
	 linux-arm-kernel@lists.infradead.org,
	linux-csky@vger.kernel.org,
	 uclinux-h8-devel@lists.sourceforge.jp,
	linux-mips@vger.kernel.org,  openrisc@lists.librecores.org,
	linuxppc-dev@lists.ozlabs.org,  linux-riscv@lists.infradead.org,
	linux-sh@vger.kernel.org,  linux-xtensa@linux-xtensa.org,
	devicetree@vger.kernel.org
Subject: Re: [PATCH 1/2] of: Sort reserved_mem related code
Date: Mon, 29 Nov 2021 18:01:50 -0600	[thread overview]
Message-ID: <YaVp7ubpCLN1xWfF@robh.at.kernel.org> (raw)
In-Reply-To: <20211119075844.2902592-2-calvinzhang.cool@gmail.com>

On Fri, Nov 19, 2021 at 03:58:18PM +0800, Calvin Zhang wrote:
> Move code about parsing /reserved-memory and initializing of
> reserved_mems array to of_reserved_mem.c for better modularity.
> 
> Rename array name from reserved_mem to reserved_mems to distinguish
> from type definition.
> 
> Signed-off-by: Calvin Zhang <calvinzhang.cool@gmail.com>
> ---
>  drivers/of/fdt.c                | 108 +--------------------
>  drivers/of/of_private.h         |  12 ++-
>  drivers/of/of_reserved_mem.c    | 163 ++++++++++++++++++++++++++------
>  include/linux/of_reserved_mem.h |   4 +
>  4 files changed, 149 insertions(+), 138 deletions(-)
> 
> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> index bdca35284ceb..445af4e69300 100644
> --- a/drivers/of/fdt.c
> +++ b/drivers/of/fdt.c
> @@ -80,7 +80,7 @@ void __init of_fdt_limit_memory(int limit)
>  	}
>  }
>  
> -static bool of_fdt_device_is_available(const void *blob, unsigned long node)
> +bool of_fdt_device_is_available(const void *blob, unsigned long node)
>  {
>  	const char *status = fdt_getprop(blob, node, "status", NULL);
>  
> @@ -476,7 +476,7 @@ void *initial_boot_params __ro_after_init;
>  
>  static u32 of_fdt_crc32;
>  
> -static int __init early_init_dt_reserve_memory_arch(phys_addr_t base,
> +int __init early_init_dt_reserve_memory_arch(phys_addr_t base,
>  					phys_addr_t size, bool nomap)

I think you can move this function too if you change the nomap==false 
callers to just call memblock_reserve directly.


>  {
>  	if (nomap) {
> @@ -492,108 +492,6 @@ static int __init early_init_dt_reserve_memory_arch(phys_addr_t base,
>  	return memblock_reserve(base, size);
>  }
>  
> -/*
> - * __reserved_mem_reserve_reg() - reserve all memory described in 'reg' property
> - */
> -static int __init __reserved_mem_reserve_reg(unsigned long node,
> -					     const char *uname)
> -{
> -	int t_len = (dt_root_addr_cells + dt_root_size_cells) * sizeof(__be32);
> -	phys_addr_t base, size;
> -	int len;
> -	const __be32 *prop;
> -	int first = 1;
> -	bool nomap;
> -
> -	prop = of_get_flat_dt_prop(node, "reg", &len);
> -	if (!prop)
> -		return -ENOENT;
> -
> -	if (len && len % t_len != 0) {
> -		pr_err("Reserved memory: invalid reg property in '%s', skipping node.\n",
> -		       uname);
> -		return -EINVAL;
> -	}
> -
> -	nomap = of_get_flat_dt_prop(node, "no-map", NULL) != NULL;
> -
> -	while (len >= t_len) {
> -		base = dt_mem_next_cell(dt_root_addr_cells, &prop);
> -		size = dt_mem_next_cell(dt_root_size_cells, &prop);
> -
> -		if (size &&
> -		    early_init_dt_reserve_memory_arch(base, size, nomap) == 0)
> -			pr_debug("Reserved memory: reserved region for node '%s': base %pa, size %lu MiB\n",
> -				uname, &base, (unsigned long)(size / SZ_1M));
> -		else
> -			pr_info("Reserved memory: failed to reserve memory for node '%s': base %pa, size %lu MiB\n",
> -				uname, &base, (unsigned long)(size / SZ_1M));
> -
> -		len -= t_len;
> -		if (first) {
> -			fdt_reserved_mem_save_node(node, uname, base, size);
> -			first = 0;
> -		}
> -	}
> -	return 0;
> -}
> -
> -/*
> - * __reserved_mem_check_root() - check if #size-cells, #address-cells provided
> - * in /reserved-memory matches the values supported by the current implementation,
> - * also check if ranges property has been provided
> - */
> -static int __init __reserved_mem_check_root(unsigned long node)
> -{
> -	const __be32 *prop;
> -
> -	prop = of_get_flat_dt_prop(node, "#size-cells", NULL);
> -	if (!prop || be32_to_cpup(prop) != dt_root_size_cells)
> -		return -EINVAL;
> -
> -	prop = of_get_flat_dt_prop(node, "#address-cells", NULL);
> -	if (!prop || be32_to_cpup(prop) != dt_root_addr_cells)
> -		return -EINVAL;
> -
> -	prop = of_get_flat_dt_prop(node, "ranges", NULL);
> -	if (!prop)
> -		return -EINVAL;
> -	return 0;
> -}
> -
> -/*
> - * fdt_scan_reserved_mem() - scan a single FDT node for reserved memory
> - */
> -static int __init fdt_scan_reserved_mem(void)
> -{
> -	int node, child;
> -	const void *fdt = initial_boot_params;
> -
> -	node = fdt_path_offset(fdt, "/reserved-memory");
> -	if (node < 0)
> -		return -ENODEV;
> -
> -	if (__reserved_mem_check_root(node) != 0) {
> -		pr_err("Reserved memory: unsupported node format, ignoring\n");
> -		return -EINVAL;
> -	}
> -
> -	fdt_for_each_subnode(child, fdt, node) {
> -		const char *uname;
> -		int err;
> -
> -		if (!of_fdt_device_is_available(fdt, child))
> -			continue;
> -
> -		uname = fdt_get_name(fdt, child, NULL);
> -
> -		err = __reserved_mem_reserve_reg(child, uname);
> -		if (err == -ENOENT && of_get_flat_dt_prop(child, "size", NULL))
> -			fdt_reserved_mem_save_node(child, uname, 0, 0);
> -	}
> -	return 0;
> -}
> -
>  /*
>   * fdt_reserve_elfcorehdr() - reserves memory for elf core header
>   *
> @@ -642,7 +540,7 @@ void __init early_init_fdt_scan_reserved_mem(void)
>  	}
>  
>  	fdt_scan_reserved_mem();
> -	fdt_init_reserved_mem();
> +	of_reserved_mem_init();
>  	fdt_reserve_elfcorehdr();
>  }
>  
> diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h
> index 9324483397f6..88b67f8ed698 100644
> --- a/drivers/of/of_private.h
> +++ b/drivers/of/of_private.h
> @@ -163,8 +163,14 @@ static inline int of_dma_get_range(struct device_node *np,
>  }
>  #endif
>  
> -void fdt_init_reserved_mem(void);
> -void fdt_reserved_mem_save_node(unsigned long node, const char *uname,
> -			       phys_addr_t base, phys_addr_t size);
> +bool of_fdt_device_is_available(const void *blob, unsigned long node);
> +int early_init_dt_reserve_memory_arch(phys_addr_t base,
> +					phys_addr_t size, bool nomap);
> +#ifdef CONFIG_OF_RESERVED_MEM
> +int fdt_scan_reserved_mem(void);
> +#else
> +static inline int fdt_scan_reserved_mem(void) { }
> +#endif
> +
>  
>  #endif /* _LINUX_OF_PRIVATE_H */
> diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
> index 9c0fb962c22b..784cfc5cd251 100644
> --- a/drivers/of/of_reserved_mem.c
> +++ b/drivers/of/of_reserved_mem.c
> @@ -20,13 +20,14 @@
>  #include <linux/of_reserved_mem.h>
>  #include <linux/sort.h>
>  #include <linux/slab.h>
> +#include <linux/libfdt.h>
>  #include <linux/memblock.h>
>  #include <linux/kmemleak.h>
>  
>  #include "of_private.h"
>  
>  #define MAX_RESERVED_REGIONS	64
> -static struct reserved_mem reserved_mem[MAX_RESERVED_REGIONS];
> +static struct reserved_mem reserved_mems[MAX_RESERVED_REGIONS];

Would be a bit easier to review without the rename.


>  static int reserved_mem_count;
>  
>  static int __init early_init_dt_alloc_reserved_memory_arch(phys_addr_t size,
> @@ -56,12 +57,12 @@ static int __init early_init_dt_alloc_reserved_memory_arch(phys_addr_t size,
>  /*
>   * fdt_reserved_mem_save_node() - save fdt node for second pass initialization
>   */
> -void __init fdt_reserved_mem_save_node(unsigned long node, const char *uname,
> +static void __init fdt_reserved_mem_save_node(unsigned long node, const char *uname,
>  				      phys_addr_t base, phys_addr_t size)
>  {
> -	struct reserved_mem *rmem = &reserved_mem[reserved_mem_count];
> +	struct reserved_mem *rmem = &reserved_mems[reserved_mem_count];
>  
> -	if (reserved_mem_count == ARRAY_SIZE(reserved_mem)) {
> +	if (reserved_mem_count == ARRAY_SIZE(reserved_mems)) {
>  		pr_err("not enough space for all defined regions.\n");
>  		return;
>  	}
> @@ -173,29 +174,105 @@ static const struct of_device_id __rmem_of_table_sentinel
>  	__used __section("__reservedmem_of_table_end");
>  
>  /*
> - * __reserved_mem_init_node() - call region specific reserved memory init code
> + * __reserved_mem_check_root() - check if #size-cells, #address-cells provided
> + * in /reserved-memory matches the values supported by the current implementation,
> + * also check if ranges property has been provided
>   */
> -static int __init __reserved_mem_init_node(struct reserved_mem *rmem)
> +static int __init __reserved_mem_check_root(unsigned long node)
>  {
> -	extern const struct of_device_id __reservedmem_of_table[];
> -	const struct of_device_id *i;
> -	int ret = -ENOENT;
> +	const __be32 *prop;
>  
> -	for (i = __reservedmem_of_table; i < &__rmem_of_table_sentinel; i++) {
> -		reservedmem_of_init_fn initfn = i->data;
> -		const char *compat = i->compatible;
> +	prop = of_get_flat_dt_prop(node, "#size-cells", NULL);
> +	if (!prop || be32_to_cpup(prop) != dt_root_size_cells)
> +		return -EINVAL;
>  
> -		if (!of_flat_dt_is_compatible(rmem->fdt_node, compat))
> -			continue;
> +	prop = of_get_flat_dt_prop(node, "#address-cells", NULL);
> +	if (!prop || be32_to_cpup(prop) != dt_root_addr_cells)
> +		return -EINVAL;
>  
> -		ret = initfn(rmem);
> -		if (ret == 0) {
> -			pr_info("initialized node %s, compatible id %s\n",
> -				rmem->name, compat);
> -			break;
> +	prop = of_get_flat_dt_prop(node, "ranges", NULL);
> +	if (!prop)
> +		return -EINVAL;
> +	return 0;
> +}
> +
> +/*
> + * __reserved_mem_reserve_reg() - reserve all memory described in 'reg' property
> + */
> +static int __init __reserved_mem_reserve_reg(unsigned long node,
> +					     const char *uname)
> +{
> +	int t_len = (dt_root_addr_cells + dt_root_size_cells) * sizeof(__be32);
> +	phys_addr_t base, size;
> +	int len;
> +	const __be32 *prop;
> +	int first = 1;
> +	bool nomap;
> +
> +	prop = of_get_flat_dt_prop(node, "reg", &len);
> +	if (!prop)
> +		return -ENOENT;
> +
> +	if (len && len % t_len != 0) {
> +		pr_err("Reserved memory: invalid reg property in '%s', skipping node.\n",
> +		       uname);
> +		return -EINVAL;
> +	}
> +
> +	nomap = of_get_flat_dt_prop(node, "no-map", NULL) != NULL;
> +
> +	while (len >= t_len) {
> +		base = dt_mem_next_cell(dt_root_addr_cells, &prop);
> +		size = dt_mem_next_cell(dt_root_size_cells, &prop);
> +
> +		if (size &&
> +		    early_init_dt_reserve_memory_arch(base, size, nomap) == 0)
> +			pr_debug("Reserved memory: reserved region for node '%s': base %pa, size %lu MiB\n",
> +				uname, &base, (unsigned long)(size / SZ_1M));
> +		else
> +			pr_info("Reserved memory: failed to reserve memory for node '%s': base %pa, size %lu MiB\n",
> +				uname, &base, (unsigned long)(size / SZ_1M));
> +
> +		len -= t_len;
> +		if (first) {
> +			fdt_reserved_mem_save_node(node, uname, base, size);
> +			first = 0;
>  		}
>  	}
> -	return ret;
> +	return 0;
> +}
> +
> +/*
> + * fdt_scan_reserved_mem() - scan a single FDT node for reserved memory
> + */
> +int __init fdt_scan_reserved_mem(void)
> +{
> +	int node, child;
> +	const void *fdt = initial_boot_params;
> +
> +	node = fdt_path_offset(fdt, "/reserved-memory");
> +	if (node < 0)
> +		return -ENODEV;
> +
> +	if (__reserved_mem_check_root(node) != 0) {
> +		pr_err("Reserved memory: unsupported node format, ignoring\n");
> +		return -EINVAL;
> +	}
> +
> +	fdt_for_each_subnode(child, fdt, node) {
> +		const char *uname;
> +		int err;
> +
> +		if (!of_fdt_device_is_available(fdt, child))
> +			continue;
> +
> +		uname = fdt_get_name(fdt, child, NULL);
> +
> +		err = __reserved_mem_reserve_reg(child, uname);
> +		if (err == -ENOENT && of_get_flat_dt_prop(child, "size", NULL))
> +			fdt_reserved_mem_save_node(child, uname, 0, 0);
> +	}
> +	return 0;
>  }
>  
>  static int __init __rmem_cmp(const void *a, const void *b)
> @@ -228,13 +305,13 @@ static void __init __rmem_check_for_overlap(void)
>  	if (reserved_mem_count < 2)
>  		return;
>  
> -	sort(reserved_mem, reserved_mem_count, sizeof(reserved_mem[0]),
> +	sort(reserved_mems, reserved_mem_count, sizeof(reserved_mems[0]),
>  	     __rmem_cmp, NULL);
>  	for (i = 0; i < reserved_mem_count - 1; i++) {
>  		struct reserved_mem *this, *next;
>  
> -		this = &reserved_mem[i];
> -		next = &reserved_mem[i + 1];
> +		this = &reserved_mems[i];
> +		next = &reserved_mems[i + 1];
>  
>  		if (this->base + this->size > next->base) {
>  			phys_addr_t this_end, next_end;
> @@ -248,10 +325,36 @@ static void __init __rmem_check_for_overlap(void)
>  	}
>  }
>  
> +/*
> + * __reserved_mem_init_node() - call region specific reserved memory init code
> + */
> +static int __init __reserved_mem_init_node(struct reserved_mem *rmem)
> +{
> +	extern const struct of_device_id __reservedmem_of_table[];
> +	const struct of_device_id *i;
> +	int ret = -ENOENT;
> +
> +	for (i = __reservedmem_of_table; i < &__rmem_of_table_sentinel; i++) {
> +		reservedmem_of_init_fn initfn = i->data;
> +		const char *compat = i->compatible;
> +
> +		if (!of_flat_dt_is_compatible(rmem->fdt_node, compat))
> +			continue;
> +
> +		ret = initfn(rmem);
> +		if (ret == 0) {
> +			pr_info("initialized node %s, compatible id %s\n",
> +				rmem->name, compat);
> +			break;
> +		}
> +	}
> +	return ret;
> +}
> +
>  /**
> - * fdt_init_reserved_mem() - allocate and init all saved reserved memory regions
> + * of_reserved_mem_init() - allocate and init all saved reserved memory regions
>   */
> -void __init fdt_init_reserved_mem(void)
> +void __init of_reserved_mem_init(void)
>  {
>  	int i;
>  
> @@ -259,7 +362,7 @@ void __init fdt_init_reserved_mem(void)
>  	__rmem_check_for_overlap();
>  
>  	for (i = 0; i < reserved_mem_count; i++) {
> -		struct reserved_mem *rmem = &reserved_mem[i];
> +		struct reserved_mem *rmem = &reserved_mems[i];
>  		unsigned long node = rmem->fdt_node;
>  		int len;
>  		const __be32 *prop;
> @@ -299,8 +402,8 @@ static inline struct reserved_mem *__find_rmem(struct device_node *node)
>  		return NULL;
>  
>  	for (i = 0; i < reserved_mem_count; i++)
> -		if (reserved_mem[i].phandle == node->phandle)
> -			return &reserved_mem[i];
> +		if (reserved_mems[i].phandle == node->phandle)
> +			return &reserved_mems[i];
>  	return NULL;
>  }
>  
> @@ -442,8 +545,8 @@ struct reserved_mem *of_reserved_mem_lookup(struct device_node *np)
>  
>  	name = kbasename(np->full_name);
>  	for (i = 0; i < reserved_mem_count; i++)
> -		if (!strcmp(reserved_mem[i].name, name))
> -			return &reserved_mem[i];
> +		if (!strcmp(reserved_mems[i].name, name))
> +			return &reserved_mems[i];
>  
>  	return NULL;
>  }
> diff --git a/include/linux/of_reserved_mem.h b/include/linux/of_reserved_mem.h
> index 4de2a24cadc9..34e134bec606 100644
> --- a/include/linux/of_reserved_mem.h
> +++ b/include/linux/of_reserved_mem.h
> @@ -32,6 +32,8 @@ typedef int (*reservedmem_of_init_fn)(struct reserved_mem *rmem);
>  #define RESERVEDMEM_OF_DECLARE(name, compat, init)			\
>  	_OF_DECLARE(reservedmem, name, compat, init, reservedmem_of_init_fn)
>  
> +void of_reserved_mem_init(void);
> +
>  int of_reserved_mem_device_init_by_idx(struct device *dev,
>  				       struct device_node *np, int idx);
>  int of_reserved_mem_device_init_by_name(struct device *dev,
> @@ -45,6 +47,8 @@ struct reserved_mem *of_reserved_mem_lookup(struct device_node *np);
>  #define RESERVEDMEM_OF_DECLARE(name, compat, init)			\
>  	_OF_DECLARE_STUB(reservedmem, name, compat, init, reservedmem_of_init_fn)
>  
> +static inline void of_reserved_mem_init(void) { }
> +
>  static inline int of_reserved_mem_device_init_by_idx(struct device *dev,
>  					struct device_node *np, int idx)
>  {
> -- 
> 2.30.2
> 
> 

_______________________________________________
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

WARNING: multiple messages have this Message-ID (diff)
From: Rob Herring <robh@kernel.org>
To: openrisc@lists.librecores.org
Subject: [OpenRISC] [PATCH 1/2] of: Sort reserved_mem related code
Date: Mon, 29 Nov 2021 18:01:50 -0600	[thread overview]
Message-ID: <YaVp7ubpCLN1xWfF@robh.at.kernel.org> (raw)
In-Reply-To: <20211119075844.2902592-2-calvinzhang.cool@gmail.com>

On Fri, Nov 19, 2021 at 03:58:18PM +0800, Calvin Zhang wrote:
> Move code about parsing /reserved-memory and initializing of
> reserved_mems array to of_reserved_mem.c for better modularity.
> 
> Rename array name from reserved_mem to reserved_mems to distinguish
> from type definition.
> 
> Signed-off-by: Calvin Zhang <calvinzhang.cool@gmail.com>
> ---
>  drivers/of/fdt.c                | 108 +--------------------
>  drivers/of/of_private.h         |  12 ++-
>  drivers/of/of_reserved_mem.c    | 163 ++++++++++++++++++++++++++------
>  include/linux/of_reserved_mem.h |   4 +
>  4 files changed, 149 insertions(+), 138 deletions(-)
> 
> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> index bdca35284ceb..445af4e69300 100644
> --- a/drivers/of/fdt.c
> +++ b/drivers/of/fdt.c
> @@ -80,7 +80,7 @@ void __init of_fdt_limit_memory(int limit)
>  	}
>  }
>  
> -static bool of_fdt_device_is_available(const void *blob, unsigned long node)
> +bool of_fdt_device_is_available(const void *blob, unsigned long node)
>  {
>  	const char *status = fdt_getprop(blob, node, "status", NULL);
>  
> @@ -476,7 +476,7 @@ void *initial_boot_params __ro_after_init;
>  
>  static u32 of_fdt_crc32;
>  
> -static int __init early_init_dt_reserve_memory_arch(phys_addr_t base,
> +int __init early_init_dt_reserve_memory_arch(phys_addr_t base,
>  					phys_addr_t size, bool nomap)

I think you can move this function too if you change the nomap==false 
callers to just call memblock_reserve directly.


>  {
>  	if (nomap) {
> @@ -492,108 +492,6 @@ static int __init early_init_dt_reserve_memory_arch(phys_addr_t base,
>  	return memblock_reserve(base, size);
>  }
>  
> -/*
> - * __reserved_mem_reserve_reg() - reserve all memory described in 'reg' property
> - */
> -static int __init __reserved_mem_reserve_reg(unsigned long node,
> -					     const char *uname)
> -{
> -	int t_len = (dt_root_addr_cells + dt_root_size_cells) * sizeof(__be32);
> -	phys_addr_t base, size;
> -	int len;
> -	const __be32 *prop;
> -	int first = 1;
> -	bool nomap;
> -
> -	prop = of_get_flat_dt_prop(node, "reg", &len);
> -	if (!prop)
> -		return -ENOENT;
> -
> -	if (len && len % t_len != 0) {
> -		pr_err("Reserved memory: invalid reg property in '%s', skipping node.\n",
> -		       uname);
> -		return -EINVAL;
> -	}
> -
> -	nomap = of_get_flat_dt_prop(node, "no-map", NULL) != NULL;
> -
> -	while (len >= t_len) {
> -		base = dt_mem_next_cell(dt_root_addr_cells, &prop);
> -		size = dt_mem_next_cell(dt_root_size_cells, &prop);
> -
> -		if (size &&
> -		    early_init_dt_reserve_memory_arch(base, size, nomap) == 0)
> -			pr_debug("Reserved memory: reserved region for node '%s': base %pa, size %lu MiB\n",
> -				uname, &base, (unsigned long)(size / SZ_1M));
> -		else
> -			pr_info("Reserved memory: failed to reserve memory for node '%s': base %pa, size %lu MiB\n",
> -				uname, &base, (unsigned long)(size / SZ_1M));
> -
> -		len -= t_len;
> -		if (first) {
> -			fdt_reserved_mem_save_node(node, uname, base, size);
> -			first = 0;
> -		}
> -	}
> -	return 0;
> -}
> -
> -/*
> - * __reserved_mem_check_root() - check if #size-cells, #address-cells provided
> - * in /reserved-memory matches the values supported by the current implementation,
> - * also check if ranges property has been provided
> - */
> -static int __init __reserved_mem_check_root(unsigned long node)
> -{
> -	const __be32 *prop;
> -
> -	prop = of_get_flat_dt_prop(node, "#size-cells", NULL);
> -	if (!prop || be32_to_cpup(prop) != dt_root_size_cells)
> -		return -EINVAL;
> -
> -	prop = of_get_flat_dt_prop(node, "#address-cells", NULL);
> -	if (!prop || be32_to_cpup(prop) != dt_root_addr_cells)
> -		return -EINVAL;
> -
> -	prop = of_get_flat_dt_prop(node, "ranges", NULL);
> -	if (!prop)
> -		return -EINVAL;
> -	return 0;
> -}
> -
> -/*
> - * fdt_scan_reserved_mem() - scan a single FDT node for reserved memory
> - */
> -static int __init fdt_scan_reserved_mem(void)
> -{
> -	int node, child;
> -	const void *fdt = initial_boot_params;
> -
> -	node = fdt_path_offset(fdt, "/reserved-memory");
> -	if (node < 0)
> -		return -ENODEV;
> -
> -	if (__reserved_mem_check_root(node) != 0) {
> -		pr_err("Reserved memory: unsupported node format, ignoring\n");
> -		return -EINVAL;
> -	}
> -
> -	fdt_for_each_subnode(child, fdt, node) {
> -		const char *uname;
> -		int err;
> -
> -		if (!of_fdt_device_is_available(fdt, child))
> -			continue;
> -
> -		uname = fdt_get_name(fdt, child, NULL);
> -
> -		err = __reserved_mem_reserve_reg(child, uname);
> -		if (err == -ENOENT && of_get_flat_dt_prop(child, "size", NULL))
> -			fdt_reserved_mem_save_node(child, uname, 0, 0);
> -	}
> -	return 0;
> -}
> -
>  /*
>   * fdt_reserve_elfcorehdr() - reserves memory for elf core header
>   *
> @@ -642,7 +540,7 @@ void __init early_init_fdt_scan_reserved_mem(void)
>  	}
>  
>  	fdt_scan_reserved_mem();
> -	fdt_init_reserved_mem();
> +	of_reserved_mem_init();
>  	fdt_reserve_elfcorehdr();
>  }
>  
> diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h
> index 9324483397f6..88b67f8ed698 100644
> --- a/drivers/of/of_private.h
> +++ b/drivers/of/of_private.h
> @@ -163,8 +163,14 @@ static inline int of_dma_get_range(struct device_node *np,
>  }
>  #endif
>  
> -void fdt_init_reserved_mem(void);
> -void fdt_reserved_mem_save_node(unsigned long node, const char *uname,
> -			       phys_addr_t base, phys_addr_t size);
> +bool of_fdt_device_is_available(const void *blob, unsigned long node);
> +int early_init_dt_reserve_memory_arch(phys_addr_t base,
> +					phys_addr_t size, bool nomap);
> +#ifdef CONFIG_OF_RESERVED_MEM
> +int fdt_scan_reserved_mem(void);
> +#else
> +static inline int fdt_scan_reserved_mem(void) { }
> +#endif
> +
>  
>  #endif /* _LINUX_OF_PRIVATE_H */
> diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
> index 9c0fb962c22b..784cfc5cd251 100644
> --- a/drivers/of/of_reserved_mem.c
> +++ b/drivers/of/of_reserved_mem.c
> @@ -20,13 +20,14 @@
>  #include <linux/of_reserved_mem.h>
>  #include <linux/sort.h>
>  #include <linux/slab.h>
> +#include <linux/libfdt.h>
>  #include <linux/memblock.h>
>  #include <linux/kmemleak.h>
>  
>  #include "of_private.h"
>  
>  #define MAX_RESERVED_REGIONS	64
> -static struct reserved_mem reserved_mem[MAX_RESERVED_REGIONS];
> +static struct reserved_mem reserved_mems[MAX_RESERVED_REGIONS];

Would be a bit easier to review without the rename.


>  static int reserved_mem_count;
>  
>  static int __init early_init_dt_alloc_reserved_memory_arch(phys_addr_t size,
> @@ -56,12 +57,12 @@ static int __init early_init_dt_alloc_reserved_memory_arch(phys_addr_t size,
>  /*
>   * fdt_reserved_mem_save_node() - save fdt node for second pass initialization
>   */
> -void __init fdt_reserved_mem_save_node(unsigned long node, const char *uname,
> +static void __init fdt_reserved_mem_save_node(unsigned long node, const char *uname,
>  				      phys_addr_t base, phys_addr_t size)
>  {
> -	struct reserved_mem *rmem = &reserved_mem[reserved_mem_count];
> +	struct reserved_mem *rmem = &reserved_mems[reserved_mem_count];
>  
> -	if (reserved_mem_count == ARRAY_SIZE(reserved_mem)) {
> +	if (reserved_mem_count == ARRAY_SIZE(reserved_mems)) {
>  		pr_err("not enough space for all defined regions.\n");
>  		return;
>  	}
> @@ -173,29 +174,105 @@ static const struct of_device_id __rmem_of_table_sentinel
>  	__used __section("__reservedmem_of_table_end");
>  
>  /*
> - * __reserved_mem_init_node() - call region specific reserved memory init code
> + * __reserved_mem_check_root() - check if #size-cells, #address-cells provided
> + * in /reserved-memory matches the values supported by the current implementation,
> + * also check if ranges property has been provided
>   */
> -static int __init __reserved_mem_init_node(struct reserved_mem *rmem)
> +static int __init __reserved_mem_check_root(unsigned long node)
>  {
> -	extern const struct of_device_id __reservedmem_of_table[];
> -	const struct of_device_id *i;
> -	int ret = -ENOENT;
> +	const __be32 *prop;
>  
> -	for (i = __reservedmem_of_table; i < &__rmem_of_table_sentinel; i++) {
> -		reservedmem_of_init_fn initfn = i->data;
> -		const char *compat = i->compatible;
> +	prop = of_get_flat_dt_prop(node, "#size-cells", NULL);
> +	if (!prop || be32_to_cpup(prop) != dt_root_size_cells)
> +		return -EINVAL;
>  
> -		if (!of_flat_dt_is_compatible(rmem->fdt_node, compat))
> -			continue;
> +	prop = of_get_flat_dt_prop(node, "#address-cells", NULL);
> +	if (!prop || be32_to_cpup(prop) != dt_root_addr_cells)
> +		return -EINVAL;
>  
> -		ret = initfn(rmem);
> -		if (ret == 0) {
> -			pr_info("initialized node %s, compatible id %s\n",
> -				rmem->name, compat);
> -			break;
> +	prop = of_get_flat_dt_prop(node, "ranges", NULL);
> +	if (!prop)
> +		return -EINVAL;
> +	return 0;
> +}
> +
> +/*
> + * __reserved_mem_reserve_reg() - reserve all memory described in 'reg' property
> + */
> +static int __init __reserved_mem_reserve_reg(unsigned long node,
> +					     const char *uname)
> +{
> +	int t_len = (dt_root_addr_cells + dt_root_size_cells) * sizeof(__be32);
> +	phys_addr_t base, size;
> +	int len;
> +	const __be32 *prop;
> +	int first = 1;
> +	bool nomap;
> +
> +	prop = of_get_flat_dt_prop(node, "reg", &len);
> +	if (!prop)
> +		return -ENOENT;
> +
> +	if (len && len % t_len != 0) {
> +		pr_err("Reserved memory: invalid reg property in '%s', skipping node.\n",
> +		       uname);
> +		return -EINVAL;
> +	}
> +
> +	nomap = of_get_flat_dt_prop(node, "no-map", NULL) != NULL;
> +
> +	while (len >= t_len) {
> +		base = dt_mem_next_cell(dt_root_addr_cells, &prop);
> +		size = dt_mem_next_cell(dt_root_size_cells, &prop);
> +
> +		if (size &&
> +		    early_init_dt_reserve_memory_arch(base, size, nomap) == 0)
> +			pr_debug("Reserved memory: reserved region for node '%s': base %pa, size %lu MiB\n",
> +				uname, &base, (unsigned long)(size / SZ_1M));
> +		else
> +			pr_info("Reserved memory: failed to reserve memory for node '%s': base %pa, size %lu MiB\n",
> +				uname, &base, (unsigned long)(size / SZ_1M));
> +
> +		len -= t_len;
> +		if (first) {
> +			fdt_reserved_mem_save_node(node, uname, base, size);
> +			first = 0;
>  		}
>  	}
> -	return ret;
> +	return 0;
> +}
> +
> +/*
> + * fdt_scan_reserved_mem() - scan a single FDT node for reserved memory
> + */
> +int __init fdt_scan_reserved_mem(void)
> +{
> +	int node, child;
> +	const void *fdt = initial_boot_params;
> +
> +	node = fdt_path_offset(fdt, "/reserved-memory");
> +	if (node < 0)
> +		return -ENODEV;
> +
> +	if (__reserved_mem_check_root(node) != 0) {
> +		pr_err("Reserved memory: unsupported node format, ignoring\n");
> +		return -EINVAL;
> +	}
> +
> +	fdt_for_each_subnode(child, fdt, node) {
> +		const char *uname;
> +		int err;
> +
> +		if (!of_fdt_device_is_available(fdt, child))
> +			continue;
> +
> +		uname = fdt_get_name(fdt, child, NULL);
> +
> +		err = __reserved_mem_reserve_reg(child, uname);
> +		if (err == -ENOENT && of_get_flat_dt_prop(child, "size", NULL))
> +			fdt_reserved_mem_save_node(child, uname, 0, 0);
> +	}
> +	return 0;
>  }
>  
>  static int __init __rmem_cmp(const void *a, const void *b)
> @@ -228,13 +305,13 @@ static void __init __rmem_check_for_overlap(void)
>  	if (reserved_mem_count < 2)
>  		return;
>  
> -	sort(reserved_mem, reserved_mem_count, sizeof(reserved_mem[0]),
> +	sort(reserved_mems, reserved_mem_count, sizeof(reserved_mems[0]),
>  	     __rmem_cmp, NULL);
>  	for (i = 0; i < reserved_mem_count - 1; i++) {
>  		struct reserved_mem *this, *next;
>  
> -		this = &reserved_mem[i];
> -		next = &reserved_mem[i + 1];
> +		this = &reserved_mems[i];
> +		next = &reserved_mems[i + 1];
>  
>  		if (this->base + this->size > next->base) {
>  			phys_addr_t this_end, next_end;
> @@ -248,10 +325,36 @@ static void __init __rmem_check_for_overlap(void)
>  	}
>  }
>  
> +/*
> + * __reserved_mem_init_node() - call region specific reserved memory init code
> + */
> +static int __init __reserved_mem_init_node(struct reserved_mem *rmem)
> +{
> +	extern const struct of_device_id __reservedmem_of_table[];
> +	const struct of_device_id *i;
> +	int ret = -ENOENT;
> +
> +	for (i = __reservedmem_of_table; i < &__rmem_of_table_sentinel; i++) {
> +		reservedmem_of_init_fn initfn = i->data;
> +		const char *compat = i->compatible;
> +
> +		if (!of_flat_dt_is_compatible(rmem->fdt_node, compat))
> +			continue;
> +
> +		ret = initfn(rmem);
> +		if (ret == 0) {
> +			pr_info("initialized node %s, compatible id %s\n",
> +				rmem->name, compat);
> +			break;
> +		}
> +	}
> +	return ret;
> +}
> +
>  /**
> - * fdt_init_reserved_mem() - allocate and init all saved reserved memory regions
> + * of_reserved_mem_init() - allocate and init all saved reserved memory regions
>   */
> -void __init fdt_init_reserved_mem(void)
> +void __init of_reserved_mem_init(void)
>  {
>  	int i;
>  
> @@ -259,7 +362,7 @@ void __init fdt_init_reserved_mem(void)
>  	__rmem_check_for_overlap();
>  
>  	for (i = 0; i < reserved_mem_count; i++) {
> -		struct reserved_mem *rmem = &reserved_mem[i];
> +		struct reserved_mem *rmem = &reserved_mems[i];
>  		unsigned long node = rmem->fdt_node;
>  		int len;
>  		const __be32 *prop;
> @@ -299,8 +402,8 @@ static inline struct reserved_mem *__find_rmem(struct device_node *node)
>  		return NULL;
>  
>  	for (i = 0; i < reserved_mem_count; i++)
> -		if (reserved_mem[i].phandle == node->phandle)
> -			return &reserved_mem[i];
> +		if (reserved_mems[i].phandle == node->phandle)
> +			return &reserved_mems[i];
>  	return NULL;
>  }
>  
> @@ -442,8 +545,8 @@ struct reserved_mem *of_reserved_mem_lookup(struct device_node *np)
>  
>  	name = kbasename(np->full_name);
>  	for (i = 0; i < reserved_mem_count; i++)
> -		if (!strcmp(reserved_mem[i].name, name))
> -			return &reserved_mem[i];
> +		if (!strcmp(reserved_mems[i].name, name))
> +			return &reserved_mems[i];
>  
>  	return NULL;
>  }
> diff --git a/include/linux/of_reserved_mem.h b/include/linux/of_reserved_mem.h
> index 4de2a24cadc9..34e134bec606 100644
> --- a/include/linux/of_reserved_mem.h
> +++ b/include/linux/of_reserved_mem.h
> @@ -32,6 +32,8 @@ typedef int (*reservedmem_of_init_fn)(struct reserved_mem *rmem);
>  #define RESERVEDMEM_OF_DECLARE(name, compat, init)			\
>  	_OF_DECLARE(reservedmem, name, compat, init, reservedmem_of_init_fn)
>  
> +void of_reserved_mem_init(void);
> +
>  int of_reserved_mem_device_init_by_idx(struct device *dev,
>  				       struct device_node *np, int idx);
>  int of_reserved_mem_device_init_by_name(struct device *dev,
> @@ -45,6 +47,8 @@ struct reserved_mem *of_reserved_mem_lookup(struct device_node *np);
>  #define RESERVEDMEM_OF_DECLARE(name, compat, init)			\
>  	_OF_DECLARE_STUB(reservedmem, name, compat, init, reservedmem_of_init_fn)
>  
> +static inline void of_reserved_mem_init(void) { }
> +
>  static inline int of_reserved_mem_device_init_by_idx(struct device *dev,
>  					struct device_node *np, int idx)
>  {
> -- 
> 2.30.2
> 
> 

WARNING: multiple messages have this Message-ID (diff)
From: Rob Herring <robh@kernel.org>
To: Calvin Zhang <calvinzhang.cool@gmail.com>
Cc: Mark Rutland <mark.rutland@arm.com>,
	Kefeng Wang <wangkefeng.wang@huawei.com>,
	Rich Felker <dalias@libc.org>, Jinyang He <hejinyang@loongson.cn>,
	David Hildenbrand <david@redhat.com>,
	Palmer Dabbelt <palmerdabbelt@google.com>,
	linux-kernel@vger.kernel.org, Max Filippov <jcmvbkbc@gmail.com>,
	Anup Patel <anup.patel@wdc.com>,
	Guo Ren <guoren@linux.alibaba.com>, Guo Ren <guoren@kernel.org>,
	linux-csky@vger.kernel.org, Nick Kossifidis <mick@ics.forth.gr>,
	Vladimir Isaev <isaev@synopsys.com>,
	Tiezhu Yang <yangtiezhu@loongson.cn>,
	Vincent Chen <deanbo422@gmail.com>, Will Deacon <will@kernel.org>,
	Markus Elfring <elfring@users.sourceforge.net>,
	Vitaly Wool <vitaly.wool@konsulko.com>,
	Jonas Bonn <jonas@southpole.se>,
	devicetree@vger.kernel.org, linux-snps-arc@lists.infradead.org,
	Yoshinori Sato <ysato@users.sourceforge.jp>,
	linux-sh@vger.kernel.org,
	"Rafael J. Wysocki" <rafael.j.wysocki@intel.com>,
	Christophe JAILLET <christophe.jaillet@wanadoo.fr>,
	Russell King <linux@armlinux.org.uk>,
	Ley Foon Tan <ley.foon.tan@intel.com>,
	Geert Uytterhoeven <geert@linux-m68k.org>,
	"Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Ganesh Goudar <ganeshgr@linux.ibm.com>,
	David Brazdil <dbrazdil@google.com>,
	linux-riscv@lists.infradead.org,
	Guenter Roeck <linux@roeck-us.net>,
	uclinux-h8-devel@lists.sourceforge.jp,
	linux-xtensa@linux-xtensa.org, Albert Ou <aou@eecs.berkeley.edu>,
	Arnd Bergmann <arnd@arndb.de>,
	Anshuman Khandual <anshuman.khandual@arm.com>,
	Vineet Gupta <vgupta@kernel.org>,
	Andreas Oetken <andreas.oetken@siemens.com>,
	Stefan Kristiansson <stefan.kristiansson@saunalahti.fi>,
	"Russell King \(Oracle\)" <rmk+kernel@armlinux.org.uk>,
	openrisc@lists.librecores.org,
	Alexander Sverdlin <alexander.sverdlin@nokia.com>,
	Greentime Hu <green.hu@gmail.com>,
	Paul Walmsley <paul.walmsley@sifive.com>,
	Stafford Horne <shorne@gmail.com>,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	Atish Patra <atish.patra@wdc.com>,
	linux-arm-kernel@lists.infradead.org,
	Andrey Konovalov <andreyknvl@gmail.com>,
	Christophe Leroy <christophe.leroy@c-s.fr>,
	Chris Zankel <chris@zankel.net>,
	Thomas Bogendoerfer <tsbogend@alpha.franken.de>,
	Alexandre Ghiti <alex@ghiti.fr>, Nick Hu <nickhu@andestech.com>,
	Wolfram Sang <wsa+renesas@sang-engineering.com>,
	linux-mips@vger.kernel.org, Randy Dunlap <rdunlap@infradead.org>,
	Frank Rowand <frowand.list@gmail.com>,
	Serge Semin <Sergey.Semin@baikalelectronics.ru>,
	Dinh Nguyen <dinguyen@kernel.org>,
	Zhang Yunkai <zhang.yunkai@zte.com.cn>,
	Palmer Dabbelt <palmer@dabbelt.com>,
	Souptick Joarder <jrdr.linux@gmail.com>,
	Marc Zyngier <maz@kernel.org>,
	Mauri Sandberg <sandberg@mailfence.com>,
	Paul Mackerras <paulus@samba.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	linuxppc-dev@lists.ozlabs.org, Mike Rapoport <rppt@kernel.org>
Subject: Re: [PATCH 1/2] of: Sort reserved_mem related code
Date: Mon, 29 Nov 2021 18:01:50 -0600	[thread overview]
Message-ID: <YaVp7ubpCLN1xWfF@robh.at.kernel.org> (raw)
In-Reply-To: <20211119075844.2902592-2-calvinzhang.cool@gmail.com>

On Fri, Nov 19, 2021 at 03:58:18PM +0800, Calvin Zhang wrote:
> Move code about parsing /reserved-memory and initializing of
> reserved_mems array to of_reserved_mem.c for better modularity.
> 
> Rename array name from reserved_mem to reserved_mems to distinguish
> from type definition.
> 
> Signed-off-by: Calvin Zhang <calvinzhang.cool@gmail.com>
> ---
>  drivers/of/fdt.c                | 108 +--------------------
>  drivers/of/of_private.h         |  12 ++-
>  drivers/of/of_reserved_mem.c    | 163 ++++++++++++++++++++++++++------
>  include/linux/of_reserved_mem.h |   4 +
>  4 files changed, 149 insertions(+), 138 deletions(-)
> 
> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> index bdca35284ceb..445af4e69300 100644
> --- a/drivers/of/fdt.c
> +++ b/drivers/of/fdt.c
> @@ -80,7 +80,7 @@ void __init of_fdt_limit_memory(int limit)
>  	}
>  }
>  
> -static bool of_fdt_device_is_available(const void *blob, unsigned long node)
> +bool of_fdt_device_is_available(const void *blob, unsigned long node)
>  {
>  	const char *status = fdt_getprop(blob, node, "status", NULL);
>  
> @@ -476,7 +476,7 @@ void *initial_boot_params __ro_after_init;
>  
>  static u32 of_fdt_crc32;
>  
> -static int __init early_init_dt_reserve_memory_arch(phys_addr_t base,
> +int __init early_init_dt_reserve_memory_arch(phys_addr_t base,
>  					phys_addr_t size, bool nomap)

I think you can move this function too if you change the nomap==false 
callers to just call memblock_reserve directly.


>  {
>  	if (nomap) {
> @@ -492,108 +492,6 @@ static int __init early_init_dt_reserve_memory_arch(phys_addr_t base,
>  	return memblock_reserve(base, size);
>  }
>  
> -/*
> - * __reserved_mem_reserve_reg() - reserve all memory described in 'reg' property
> - */
> -static int __init __reserved_mem_reserve_reg(unsigned long node,
> -					     const char *uname)
> -{
> -	int t_len = (dt_root_addr_cells + dt_root_size_cells) * sizeof(__be32);
> -	phys_addr_t base, size;
> -	int len;
> -	const __be32 *prop;
> -	int first = 1;
> -	bool nomap;
> -
> -	prop = of_get_flat_dt_prop(node, "reg", &len);
> -	if (!prop)
> -		return -ENOENT;
> -
> -	if (len && len % t_len != 0) {
> -		pr_err("Reserved memory: invalid reg property in '%s', skipping node.\n",
> -		       uname);
> -		return -EINVAL;
> -	}
> -
> -	nomap = of_get_flat_dt_prop(node, "no-map", NULL) != NULL;
> -
> -	while (len >= t_len) {
> -		base = dt_mem_next_cell(dt_root_addr_cells, &prop);
> -		size = dt_mem_next_cell(dt_root_size_cells, &prop);
> -
> -		if (size &&
> -		    early_init_dt_reserve_memory_arch(base, size, nomap) == 0)
> -			pr_debug("Reserved memory: reserved region for node '%s': base %pa, size %lu MiB\n",
> -				uname, &base, (unsigned long)(size / SZ_1M));
> -		else
> -			pr_info("Reserved memory: failed to reserve memory for node '%s': base %pa, size %lu MiB\n",
> -				uname, &base, (unsigned long)(size / SZ_1M));
> -
> -		len -= t_len;
> -		if (first) {
> -			fdt_reserved_mem_save_node(node, uname, base, size);
> -			first = 0;
> -		}
> -	}
> -	return 0;
> -}
> -
> -/*
> - * __reserved_mem_check_root() - check if #size-cells, #address-cells provided
> - * in /reserved-memory matches the values supported by the current implementation,
> - * also check if ranges property has been provided
> - */
> -static int __init __reserved_mem_check_root(unsigned long node)
> -{
> -	const __be32 *prop;
> -
> -	prop = of_get_flat_dt_prop(node, "#size-cells", NULL);
> -	if (!prop || be32_to_cpup(prop) != dt_root_size_cells)
> -		return -EINVAL;
> -
> -	prop = of_get_flat_dt_prop(node, "#address-cells", NULL);
> -	if (!prop || be32_to_cpup(prop) != dt_root_addr_cells)
> -		return -EINVAL;
> -
> -	prop = of_get_flat_dt_prop(node, "ranges", NULL);
> -	if (!prop)
> -		return -EINVAL;
> -	return 0;
> -}
> -
> -/*
> - * fdt_scan_reserved_mem() - scan a single FDT node for reserved memory
> - */
> -static int __init fdt_scan_reserved_mem(void)
> -{
> -	int node, child;
> -	const void *fdt = initial_boot_params;
> -
> -	node = fdt_path_offset(fdt, "/reserved-memory");
> -	if (node < 0)
> -		return -ENODEV;
> -
> -	if (__reserved_mem_check_root(node) != 0) {
> -		pr_err("Reserved memory: unsupported node format, ignoring\n");
> -		return -EINVAL;
> -	}
> -
> -	fdt_for_each_subnode(child, fdt, node) {
> -		const char *uname;
> -		int err;
> -
> -		if (!of_fdt_device_is_available(fdt, child))
> -			continue;
> -
> -		uname = fdt_get_name(fdt, child, NULL);
> -
> -		err = __reserved_mem_reserve_reg(child, uname);
> -		if (err == -ENOENT && of_get_flat_dt_prop(child, "size", NULL))
> -			fdt_reserved_mem_save_node(child, uname, 0, 0);
> -	}
> -	return 0;
> -}
> -
>  /*
>   * fdt_reserve_elfcorehdr() - reserves memory for elf core header
>   *
> @@ -642,7 +540,7 @@ void __init early_init_fdt_scan_reserved_mem(void)
>  	}
>  
>  	fdt_scan_reserved_mem();
> -	fdt_init_reserved_mem();
> +	of_reserved_mem_init();
>  	fdt_reserve_elfcorehdr();
>  }
>  
> diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h
> index 9324483397f6..88b67f8ed698 100644
> --- a/drivers/of/of_private.h
> +++ b/drivers/of/of_private.h
> @@ -163,8 +163,14 @@ static inline int of_dma_get_range(struct device_node *np,
>  }
>  #endif
>  
> -void fdt_init_reserved_mem(void);
> -void fdt_reserved_mem_save_node(unsigned long node, const char *uname,
> -			       phys_addr_t base, phys_addr_t size);
> +bool of_fdt_device_is_available(const void *blob, unsigned long node);
> +int early_init_dt_reserve_memory_arch(phys_addr_t base,
> +					phys_addr_t size, bool nomap);
> +#ifdef CONFIG_OF_RESERVED_MEM
> +int fdt_scan_reserved_mem(void);
> +#else
> +static inline int fdt_scan_reserved_mem(void) { }
> +#endif
> +
>  
>  #endif /* _LINUX_OF_PRIVATE_H */
> diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
> index 9c0fb962c22b..784cfc5cd251 100644
> --- a/drivers/of/of_reserved_mem.c
> +++ b/drivers/of/of_reserved_mem.c
> @@ -20,13 +20,14 @@
>  #include <linux/of_reserved_mem.h>
>  #include <linux/sort.h>
>  #include <linux/slab.h>
> +#include <linux/libfdt.h>
>  #include <linux/memblock.h>
>  #include <linux/kmemleak.h>
>  
>  #include "of_private.h"
>  
>  #define MAX_RESERVED_REGIONS	64
> -static struct reserved_mem reserved_mem[MAX_RESERVED_REGIONS];
> +static struct reserved_mem reserved_mems[MAX_RESERVED_REGIONS];

Would be a bit easier to review without the rename.


>  static int reserved_mem_count;
>  
>  static int __init early_init_dt_alloc_reserved_memory_arch(phys_addr_t size,
> @@ -56,12 +57,12 @@ static int __init early_init_dt_alloc_reserved_memory_arch(phys_addr_t size,
>  /*
>   * fdt_reserved_mem_save_node() - save fdt node for second pass initialization
>   */
> -void __init fdt_reserved_mem_save_node(unsigned long node, const char *uname,
> +static void __init fdt_reserved_mem_save_node(unsigned long node, const char *uname,
>  				      phys_addr_t base, phys_addr_t size)
>  {
> -	struct reserved_mem *rmem = &reserved_mem[reserved_mem_count];
> +	struct reserved_mem *rmem = &reserved_mems[reserved_mem_count];
>  
> -	if (reserved_mem_count == ARRAY_SIZE(reserved_mem)) {
> +	if (reserved_mem_count == ARRAY_SIZE(reserved_mems)) {
>  		pr_err("not enough space for all defined regions.\n");
>  		return;
>  	}
> @@ -173,29 +174,105 @@ static const struct of_device_id __rmem_of_table_sentinel
>  	__used __section("__reservedmem_of_table_end");
>  
>  /*
> - * __reserved_mem_init_node() - call region specific reserved memory init code
> + * __reserved_mem_check_root() - check if #size-cells, #address-cells provided
> + * in /reserved-memory matches the values supported by the current implementation,
> + * also check if ranges property has been provided
>   */
> -static int __init __reserved_mem_init_node(struct reserved_mem *rmem)
> +static int __init __reserved_mem_check_root(unsigned long node)
>  {
> -	extern const struct of_device_id __reservedmem_of_table[];
> -	const struct of_device_id *i;
> -	int ret = -ENOENT;
> +	const __be32 *prop;
>  
> -	for (i = __reservedmem_of_table; i < &__rmem_of_table_sentinel; i++) {
> -		reservedmem_of_init_fn initfn = i->data;
> -		const char *compat = i->compatible;
> +	prop = of_get_flat_dt_prop(node, "#size-cells", NULL);
> +	if (!prop || be32_to_cpup(prop) != dt_root_size_cells)
> +		return -EINVAL;
>  
> -		if (!of_flat_dt_is_compatible(rmem->fdt_node, compat))
> -			continue;
> +	prop = of_get_flat_dt_prop(node, "#address-cells", NULL);
> +	if (!prop || be32_to_cpup(prop) != dt_root_addr_cells)
> +		return -EINVAL;
>  
> -		ret = initfn(rmem);
> -		if (ret == 0) {
> -			pr_info("initialized node %s, compatible id %s\n",
> -				rmem->name, compat);
> -			break;
> +	prop = of_get_flat_dt_prop(node, "ranges", NULL);
> +	if (!prop)
> +		return -EINVAL;
> +	return 0;
> +}
> +
> +/*
> + * __reserved_mem_reserve_reg() - reserve all memory described in 'reg' property
> + */
> +static int __init __reserved_mem_reserve_reg(unsigned long node,
> +					     const char *uname)
> +{
> +	int t_len = (dt_root_addr_cells + dt_root_size_cells) * sizeof(__be32);
> +	phys_addr_t base, size;
> +	int len;
> +	const __be32 *prop;
> +	int first = 1;
> +	bool nomap;
> +
> +	prop = of_get_flat_dt_prop(node, "reg", &len);
> +	if (!prop)
> +		return -ENOENT;
> +
> +	if (len && len % t_len != 0) {
> +		pr_err("Reserved memory: invalid reg property in '%s', skipping node.\n",
> +		       uname);
> +		return -EINVAL;
> +	}
> +
> +	nomap = of_get_flat_dt_prop(node, "no-map", NULL) != NULL;
> +
> +	while (len >= t_len) {
> +		base = dt_mem_next_cell(dt_root_addr_cells, &prop);
> +		size = dt_mem_next_cell(dt_root_size_cells, &prop);
> +
> +		if (size &&
> +		    early_init_dt_reserve_memory_arch(base, size, nomap) == 0)
> +			pr_debug("Reserved memory: reserved region for node '%s': base %pa, size %lu MiB\n",
> +				uname, &base, (unsigned long)(size / SZ_1M));
> +		else
> +			pr_info("Reserved memory: failed to reserve memory for node '%s': base %pa, size %lu MiB\n",
> +				uname, &base, (unsigned long)(size / SZ_1M));
> +
> +		len -= t_len;
> +		if (first) {
> +			fdt_reserved_mem_save_node(node, uname, base, size);
> +			first = 0;
>  		}
>  	}
> -	return ret;
> +	return 0;
> +}
> +
> +/*
> + * fdt_scan_reserved_mem() - scan a single FDT node for reserved memory
> + */
> +int __init fdt_scan_reserved_mem(void)
> +{
> +	int node, child;
> +	const void *fdt = initial_boot_params;
> +
> +	node = fdt_path_offset(fdt, "/reserved-memory");
> +	if (node < 0)
> +		return -ENODEV;
> +
> +	if (__reserved_mem_check_root(node) != 0) {
> +		pr_err("Reserved memory: unsupported node format, ignoring\n");
> +		return -EINVAL;
> +	}
> +
> +	fdt_for_each_subnode(child, fdt, node) {
> +		const char *uname;
> +		int err;
> +
> +		if (!of_fdt_device_is_available(fdt, child))
> +			continue;
> +
> +		uname = fdt_get_name(fdt, child, NULL);
> +
> +		err = __reserved_mem_reserve_reg(child, uname);
> +		if (err == -ENOENT && of_get_flat_dt_prop(child, "size", NULL))
> +			fdt_reserved_mem_save_node(child, uname, 0, 0);
> +	}
> +	return 0;
>  }
>  
>  static int __init __rmem_cmp(const void *a, const void *b)
> @@ -228,13 +305,13 @@ static void __init __rmem_check_for_overlap(void)
>  	if (reserved_mem_count < 2)
>  		return;
>  
> -	sort(reserved_mem, reserved_mem_count, sizeof(reserved_mem[0]),
> +	sort(reserved_mems, reserved_mem_count, sizeof(reserved_mems[0]),
>  	     __rmem_cmp, NULL);
>  	for (i = 0; i < reserved_mem_count - 1; i++) {
>  		struct reserved_mem *this, *next;
>  
> -		this = &reserved_mem[i];
> -		next = &reserved_mem[i + 1];
> +		this = &reserved_mems[i];
> +		next = &reserved_mems[i + 1];
>  
>  		if (this->base + this->size > next->base) {
>  			phys_addr_t this_end, next_end;
> @@ -248,10 +325,36 @@ static void __init __rmem_check_for_overlap(void)
>  	}
>  }
>  
> +/*
> + * __reserved_mem_init_node() - call region specific reserved memory init code
> + */
> +static int __init __reserved_mem_init_node(struct reserved_mem *rmem)
> +{
> +	extern const struct of_device_id __reservedmem_of_table[];
> +	const struct of_device_id *i;
> +	int ret = -ENOENT;
> +
> +	for (i = __reservedmem_of_table; i < &__rmem_of_table_sentinel; i++) {
> +		reservedmem_of_init_fn initfn = i->data;
> +		const char *compat = i->compatible;
> +
> +		if (!of_flat_dt_is_compatible(rmem->fdt_node, compat))
> +			continue;
> +
> +		ret = initfn(rmem);
> +		if (ret == 0) {
> +			pr_info("initialized node %s, compatible id %s\n",
> +				rmem->name, compat);
> +			break;
> +		}
> +	}
> +	return ret;
> +}
> +
>  /**
> - * fdt_init_reserved_mem() - allocate and init all saved reserved memory regions
> + * of_reserved_mem_init() - allocate and init all saved reserved memory regions
>   */
> -void __init fdt_init_reserved_mem(void)
> +void __init of_reserved_mem_init(void)
>  {
>  	int i;
>  
> @@ -259,7 +362,7 @@ void __init fdt_init_reserved_mem(void)
>  	__rmem_check_for_overlap();
>  
>  	for (i = 0; i < reserved_mem_count; i++) {
> -		struct reserved_mem *rmem = &reserved_mem[i];
> +		struct reserved_mem *rmem = &reserved_mems[i];
>  		unsigned long node = rmem->fdt_node;
>  		int len;
>  		const __be32 *prop;
> @@ -299,8 +402,8 @@ static inline struct reserved_mem *__find_rmem(struct device_node *node)
>  		return NULL;
>  
>  	for (i = 0; i < reserved_mem_count; i++)
> -		if (reserved_mem[i].phandle == node->phandle)
> -			return &reserved_mem[i];
> +		if (reserved_mems[i].phandle == node->phandle)
> +			return &reserved_mems[i];
>  	return NULL;
>  }
>  
> @@ -442,8 +545,8 @@ struct reserved_mem *of_reserved_mem_lookup(struct device_node *np)
>  
>  	name = kbasename(np->full_name);
>  	for (i = 0; i < reserved_mem_count; i++)
> -		if (!strcmp(reserved_mem[i].name, name))
> -			return &reserved_mem[i];
> +		if (!strcmp(reserved_mems[i].name, name))
> +			return &reserved_mems[i];
>  
>  	return NULL;
>  }
> diff --git a/include/linux/of_reserved_mem.h b/include/linux/of_reserved_mem.h
> index 4de2a24cadc9..34e134bec606 100644
> --- a/include/linux/of_reserved_mem.h
> +++ b/include/linux/of_reserved_mem.h
> @@ -32,6 +32,8 @@ typedef int (*reservedmem_of_init_fn)(struct reserved_mem *rmem);
>  #define RESERVEDMEM_OF_DECLARE(name, compat, init)			\
>  	_OF_DECLARE(reservedmem, name, compat, init, reservedmem_of_init_fn)
>  
> +void of_reserved_mem_init(void);
> +
>  int of_reserved_mem_device_init_by_idx(struct device *dev,
>  				       struct device_node *np, int idx);
>  int of_reserved_mem_device_init_by_name(struct device *dev,
> @@ -45,6 +47,8 @@ struct reserved_mem *of_reserved_mem_lookup(struct device_node *np);
>  #define RESERVEDMEM_OF_DECLARE(name, compat, init)			\
>  	_OF_DECLARE_STUB(reservedmem, name, compat, init, reservedmem_of_init_fn)
>  
> +static inline void of_reserved_mem_init(void) { }
> +
>  static inline int of_reserved_mem_device_init_by_idx(struct device *dev,
>  					struct device_node *np, int idx)
>  {
> -- 
> 2.30.2
> 
> 

  reply	other threads:[~2021-11-30  0:01 UTC|newest]

Thread overview: 54+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-19  7:58 [PATCH 0/2] of: remove reserved regions count restriction Calvin Zhang
2021-11-19  7:58 ` Calvin Zhang
2021-11-19  7:58 ` [OpenRISC] " Calvin Zhang
2021-11-19  7:58 ` Calvin Zhang
2021-11-19  7:58 ` Calvin Zhang
2021-11-19  7:58 ` [PATCH 1/2] of: Sort reserved_mem related code Calvin Zhang
2021-11-19  7:58   ` Calvin Zhang
2021-11-19  7:58   ` [OpenRISC] " Calvin Zhang
2021-11-19  7:58   ` Calvin Zhang
2021-11-19  7:58   ` Calvin Zhang
2021-11-30  0:01   ` Rob Herring [this message]
2021-11-30  0:01     ` Rob Herring
2021-11-30  0:01     ` [OpenRISC] " Rob Herring
2021-11-30  0:01     ` Rob Herring
2021-11-30  0:01     ` Rob Herring
2021-11-19  7:58 ` [PATCH 2/2] of: reserved_mem: Remove reserved regions count restriction Calvin Zhang
2021-11-19  7:58   ` Calvin Zhang
2021-11-19  7:58   ` [OpenRISC] " Calvin Zhang
2021-11-19  7:58   ` Calvin Zhang
2021-11-19  7:58   ` Calvin Zhang
2021-11-19  9:56   ` Andy Shevchenko
2021-11-19  9:56     ` Andy Shevchenko
2021-11-19  9:56     ` [OpenRISC] " Andy Shevchenko
2021-11-19  9:56     ` Andy Shevchenko
2021-11-19  9:56     ` Andy Shevchenko
2021-11-19 10:27     ` Calvin Zhang
2021-11-19 10:27       ` Calvin Zhang
2021-11-19 10:27       ` [OpenRISC] " Calvin Zhang
2021-11-19 10:27       ` Calvin Zhang
2021-11-19 10:30     ` Calvin Zhang
2021-11-19 10:30       ` Calvin Zhang
2021-11-19 10:30       ` [OpenRISC] " Calvin Zhang
2021-11-19 10:30       ` Calvin Zhang
2021-11-19 10:30       ` Calvin Zhang
2021-11-21  6:43 ` [PATCH 0/2] of: remove " Mike Rapoport
2021-11-21  6:43   ` Mike Rapoport
2021-11-21  6:43   ` [OpenRISC] " Mike Rapoport
2021-11-21  6:43   ` Mike Rapoport
2021-11-21  6:43   ` Mike Rapoport
2021-11-21  9:01   ` Calvin Zhang
2021-11-21  9:01     ` Calvin Zhang
2021-11-21  9:01     ` [OpenRISC] " Calvin Zhang
2021-11-21  9:01     ` Calvin Zhang
2021-11-21  9:01     ` Calvin Zhang
2021-11-30  0:08   ` Rob Herring
2021-11-30  0:08     ` Rob Herring
2021-11-30  0:08     ` [OpenRISC] " Rob Herring
2021-11-30  0:08     ` Rob Herring
2021-11-30  0:08     ` Rob Herring
2021-11-30 21:07     ` Mike Rapoport
2021-11-30 21:07       ` Mike Rapoport
2021-11-30 21:07       ` [OpenRISC] " Mike Rapoport
2021-11-30 21:07       ` Mike Rapoport
2021-11-30 21:07       ` Mike Rapoport

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YaVp7ubpCLN1xWfF@robh.at.kernel.org \
    --to=robh@kernel.org \
    --cc=Sergey.Semin@baikalelectronics.ru \
    --cc=akpm@linux-foundation.org \
    --cc=alex@ghiti.fr \
    --cc=alexander.sverdlin@nokia.com \
    --cc=andreas.oetken@siemens.com \
    --cc=andreyknvl@gmail.com \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=aneesh.kumar@linux.ibm.com \
    --cc=anshuman.khandual@arm.com \
    --cc=anup.patel@wdc.com \
    --cc=aou@eecs.berkeley.edu \
    --cc=arnd@arndb.de \
    --cc=atish.patra@wdc.com \
    --cc=benh@kernel.crashing.org \
    --cc=calvinzhang.cool@gmail.com \
    --cc=catalin.marinas@arm.com \
    --cc=chris@zankel.net \
    --cc=christophe.jaillet@wanadoo.fr \
    --cc=christophe.leroy@c-s.fr \
    --cc=dalias@libc.org \
    --cc=david@redhat.com \
    --cc=dbrazdil@google.com \
    --cc=deanbo422@gmail.com \
    --cc=devicetree@vger.kernel.org \
    --cc=dinguyen@kernel.org \
    --cc=elfring@users.sourceforge.net \
    --cc=frowand.list@gmail.com \
    --cc=ganeshgr@linux.ibm.com \
    --cc=geert@linux-m68k.org \
    --cc=green.hu@gmail.com \
    --cc=guoren@kernel.org \
    --cc=guoren@linux.alibaba.com \
    --cc=hejinyang@loongson.cn \
    --cc=isaev@synopsys.com \
    --cc=jcmvbkbc@gmail.com \
    --cc=jonas@southpole.se \
    --cc=jrdr.linux@gmail.com \
    --cc=ley.foon.tan@intel.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-csky@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mips@vger.kernel.org \
    --cc=linux-riscv@lists.infradead.org \
    --cc=linux-sh@vger.kernel.org \
    --cc=linux-snps-arc@lists.infradead.org \
    --cc=linux-xtensa@linux-xtensa.org \
    --cc=linux@armlinux.org.uk \
    --cc=linux@roeck-us.net \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=mark.rutland@arm.com \
    --cc=maz@kernel.org \
    --cc=mick@ics.forth.gr \
    --cc=mpe@ellerman.id.au \
    --cc=nickhu@andestech.com \
    --cc=openrisc@lists.librecores.org \
    --cc=palmer@dabbelt.com \
    --cc=palmerdabbelt@google.com \
    --cc=paul.walmsley@sifive.com \
    --cc=paulus@samba.org \
    --cc=rafael.j.wysocki@intel.com \
    --cc=rdunlap@infradead.org \
    --cc=rmk+kernel@armlinux.org.uk \
    --cc=rppt@kernel.org \
    --cc=sandberg@mailfence.com \
    --cc=shorne@gmail.com \
    --cc=stefan.kristiansson@saunalahti.fi \
    --cc=tsbogend@alpha.franken.de \
    --cc=uclinux-h8-devel@lists.sourceforge.jp \
    --cc=vgupta@kernel.org \
    --cc=vitaly.wool@konsulko.com \
    --cc=wangkefeng.wang@huawei.com \
    --cc=will@kernel.org \
    --cc=wsa+renesas@sang-engineering.com \
    --cc=yangtiezhu@loongson.cn \
    --cc=ysato@users.sourceforge.jp \
    --cc=zhang.yunkai@zte.com.cn \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.