[PATCH v4 3/4] arm: kirkwood: Pogoplug-V4 : Add board implementation files
Tony Dinh
mibodhi at gmail.com
Sun Jan 23 02:46:19 CET 2022
Hi Pali,
On Fri, Jan 21, 2022 at 1:33 PM Tony Dinh <mibodhi at gmail.com> wrote:
>
> 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 = ð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.
>
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 at 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 at 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 at 72000 Waiting for PHY auto negotiation to
complete....... done
Using ethernet-controller at 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 |= (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);
> > > +}
More information about the U-Boot
mailing list