Hi Pali and Stefan, On Fri, Jan 21, 2022 at 2:01 AM Pali Rohár <[email protected]> wrote: > > On Thursday 20 January 2022 17:50:54 Tony Dinh wrote: > > diff --git a/board/cloudengines/pogo_v4/pogo_v4.c > > b/board/cloudengines/pogo_v4/pogo_v4.c > > new file mode 100644 > > index 0000000000..c85de0b22e > > --- /dev/null > > +++ b/board/cloudengines/pogo_v4/pogo_v4.c > > +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); > > + } > > + if (val) { > > + phandle = fdt32_to_cpu(*val); > > + if (!phandle) > > + return -FDT_ERR_NOTFOUND; > > + > > + /* 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 -FDT_ERR_NOTFOUND; > > +} > > + > > +#if defined(CONFIG_RESET_PHY_R) > > +/* Configure and initialize PHY */ > > +void reset_phy(void) > > +{ > > + u16 reg; > > + int phyaddr; > > + char *name = "ethernet-controller@72000"; > > + char *eth0_path = "/ocp@f1000000/ethernet-controller@72000"; > > Hello! I would suggest to avoid hardcoding DT node names and DT node > paths into source code. > > How to avoid it? > > In DTS file define alias "ethernet0" as alias to eth0 node: > > aliases { > ethernet0 = ð0; > }; > > (just before memory {} node) > > ð0 label is already defined in arch/arm/dts/kirkwood.dtsi. > > Function fdt_path_offset() understands either full path or alias. So you > can call fdt_get_phy_addr("ethernet0"). > > And to get node name you can use function fdt_get_name() which takes > node offset - return value of fdt_path_offset(). > > And maybe... to avoid calling fdt_path_offset() two times, you may > adjust fdt_get_phy_addr() to directly take node offset (not path/alias).
Pali, Thanks for your suggestion and reviewing this fdt_get_phy_addr() function. However, this function will be removed. See my followup post in the V3 patch thread about the uclass MVGBE. https://lists.denx.de/pipermail/u-boot/2022-January/472691.html Stefan, Perhaps I should send in a v5 patch series for this board now (instead of reworking this file in a separate patch)? I've tested the uclass MVGBE in this board and it is working as I expected. Thank, Tony > > So something like this: > > node = fdt_path_offset(gd->fdt_blob, "ethernet0"); > ... error checking ... > name = fdt_get_name(gd->fdt_blob, node, NULL); > ... error checking ... > miiphy_set_current_dev(name); > ... error checking ... > fdt_get_phy_addr(node); > ... error checking ... > > > > + if (miiphy_set_current_dev(name)) > > + return; > > + > > + phyaddr = fdt_get_phy_addr(eth0_path); > > + if (phyaddr < 0) > > + return; > > + > > + /* > > + * Enable RGMII delay on Tx and Rx for CPU port > > + * Ref: sec 4.7.2 of chip datasheet > > + */ > > + miiphy_write(name, phyaddr, MV88E1116_PGADR_REG, 2); > > + miiphy_read(name, phyaddr, MV88E1116_MAC_CTRL_REG, ®); > > + reg |= (MV88E1116_RGMII_RXTM_CTRL | MV88E1116_RGMII_TXTM_CTRL); > > + miiphy_write(name, phyaddr, MV88E1116_MAC_CTRL_REG, reg); > > + miiphy_write(name, phyaddr, MV88E1116_PGADR_REG, 0); > > + > > + /* reset the phy */ > > + miiphy_reset(name, phyaddr); > > + > > + printf("88E1116 Initialized on %s\n", name); > > +}

