[PATCH 2/3] Add fdt network helper functions
Stefan Roese
sr at denx.de
Thu Aug 12 08:15:39 CEST 2021
Hi Tony,
a few nits...
On 06.08.21 06:49, Tony Dinh wrote:
> Add fdt network helper functions common/fdt_support_net.c
>
> Signed-off-by: Tony Dinh <mibodhi at gmail.com>
> ---
>
> common/fdt_support_net.c | 46 ++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 46 insertions(+)
> create mode 100644 common/fdt_support_net.c
>
> diff --git a/common/fdt_support_net.c b/common/fdt_support_net.c
> new file mode 100644
> index 0000000000..06fa2175b4
> --- /dev/null
> +++ b/common/fdt_support_net.c
> @@ -0,0 +1,46 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2021 Tony Dinh <mibodhi at gmail.com>
> + */
> +
> +#include <common.h>
Including "common.h" is deprecated. Please remove it and include the
missing other headers instead (if any).
> +#include <linux/libfdt.h>
> +#include <fdt_support.h>
> +#include <asm/global_data.h>
> +#include <fdt_support_net.h>
Sorting would be good as well.
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +int fdt_get_phy_addr(const char *path)
> +{
> + const void *fdt = gd->fdt_blob;
> + const u32 *reg;
> + const u32 *val;
> + int node, phandle, addr;
> +
> + /* Find the node by its full path */
> + node = fdt_path_offset(fdt, path);
> + if (node >= 0) {
> + /* Look up phy-handle */
> + val = fdt_getprop(fdt, node, "phy-handle", NULL);
> + if (!val)
> + /* Look up phy (deprecated property for phy handle) */
> + val = fdt_getprop(fdt, node, "phy", NULL);
Above is a multi-line statement which is better included in
parentheses.
And I personally would add an empty line here.
> + if (val) {
> + phandle = fdt32_to_cpu(*val);
> + if (!phandle)
> + return -1;
Could you instead of returning "-1" return some matching error code
defines?
And I personally would add an empty line here.
> + /* Follow it to its node */
> + node = fdt_node_offset_by_phandle(fdt, phandle);
> + if (node) {
> + /* Look up reg */
> + reg = fdt_getprop(fdt, node, "reg", NULL);
> + if (reg) {
> + addr = fdt32_to_cpu(*reg);
> + return addr;
> + }
> + }
> + }
> + }
> + return -1;
Again, please change to some matching error define here.
Thanks,
Stefan
More information about the U-Boot
mailing list