Re: [PATCH v4 3/4] arm: kirkwood: Pogoplug-V4 : Add board implementation files

Hi Pali,

On Fri, Jan 21, 2022 at 1:33 PM Tony Dinh mibodhi@gmail.com wrote:

Hi Pali and Stefan,

On Fri, Jan 21, 2022 at 2:01 AM Pali Rohár pali@kernel.org 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 = &eth0;
    };

(just before memory {} node)

&eth0 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.

Here is the test using mvgbe uclass. I've added some debug printfs to see what the calling sequence is.

<BEGIN log> U-Boot 2022.01-00673-g70fe7827f2-dirty (Jan 22 2022 - 00:09:05 -0800) Pogoplug V4

SoC: Kirkwood 88F6281_A1 Model: Cloud Engines PogoPlug Series 4 DRAM: 128 MiB Core: 11 devices, 10 uclasses, devicetree: separate NAND: 128 MiB MMC: mvsdio@90000: 0 Loading Environment from NAND... OK In: serial Out: serial Err: serial pcie0.0: Link up Net: mvgbe_of_to_plat mvgbe_of_to_plat phy-mode 1 mvgbe_probe eth0: ethernet-controller@72000 Hit any key to stop autoboot: 0

Pogo_V4> ping 192.168.0.1 mvgbe_start UCLASS_ETH mvgbe mvgbe_start PHY addr 0 __mvgbe_phy_init phy_interface = gmii phy_config phy.c board_phy_config phy.c board_phy_config calling default phydev->drv->config phy.c genphy_config ethernet-controller@72000 Waiting for PHY auto negotiation to complete....... done Using ethernet-controller@72000 device host 192.168.0.1 is alive <END log>

So the network bringup is purely based on the DTS. None of the marvell.c driver config is involved (i.e genphy_config function is where the configuration occurs). It is currently working fine with gmii phy-mode. So to get rgmii, we'll just need to add a phy-mode = "rgmii" property to the eth0 node in the DTS.

All this to say is I'm happy that I can remove a lot of code in pogo_v4.c, and not have to patch anything in the common area like FDT or network PHY.

Thanks, Tony

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);
  

• 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);
  

+}