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

[Tony Dinh]

Hi Pali and Stefan,

On Fri, Jan 21, 2022 at 2:01 AM Pali Rohár <pali at 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 at 72000";
> > +     char *eth0_path = "/ocp at f1000000/ethernet-controller at 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.

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