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

Tony Dinh mibodhi at gmail.com
Wed Jan 19 02:31:30 CET 2022


Hi Stefan,

On Mon, Jan 17, 2022 at 11:54 PM Stefan Roese <sr at denx.de> wrote:
>
> On 1/18/22 07:58, Tony Dinh wrote:
> > Note: currently the fdt_get_phy_addr function in this file is
> > duplicate in this board and many other Kirkwood boards
> > (eg. Sheevaplug, GoFlex Home, etc.). This function is being
> > factored out into common area by another patch. And because it
> > was written for flattree only, the patch is being rewritten to
> > use livetree calls.
> >
> > Signed-off-by: Tony Dinh <mibodhi at gmail.com>
> > ---
> >
> > Changes in v3:
> > - Squash board file small patches into one patch
> >
> > Changes in v2:
> > - Move constants to .c file and remove header file
> >
> >   board/cloudengines/pogo_v4/MAINTAINERS  |   6 +
> >   board/cloudengines/pogo_v4/Makefile     |  10 ++
> >   board/cloudengines/pogo_v4/kwbimage.cfg | 148 ++++++++++++++++
> >   board/cloudengines/pogo_v4/pogo_v4.c    | 220 ++++++++++++++++++++++++
> >   4 files changed, 384 insertions(+)
> >   create mode 100644 board/cloudengines/pogo_v4/MAINTAINERS
> >   create mode 100644 board/cloudengines/pogo_v4/Makefile
> >   create mode 100644 board/cloudengines/pogo_v4/kwbimage.cfg
> >   create mode 100644 board/cloudengines/pogo_v4/pogo_v4.c
> >
> > diff --git a/board/cloudengines/pogo_v4/MAINTAINERS b/board/cloudengines/pogo_v4/MAINTAINERS
> > new file mode 100644
> > index 0000000000..35fd7858b7
> > --- /dev/null
> > +++ b/board/cloudengines/pogo_v4/MAINTAINERS
> > @@ -0,0 +1,6 @@
> > +POGO_V4 BOARD
> > +M:   Tony Dinh <mibodhi at gmail.com>
> > +S:   Maintained
> > +F:   board/cloudengines/pogo_v4/
> > +F:   include/configs/pogo_v4.h
> > +F:   configs/pogo_v4_defconfig
> > diff --git a/board/cloudengines/pogo_v4/Makefile b/board/cloudengines/pogo_v4/Makefile
> > new file mode 100644
> > index 0000000000..511bf5ff7e
> > --- /dev/null
> > +++ b/board/cloudengines/pogo_v4/Makefile
> > @@ -0,0 +1,10 @@
> > +# SPDX-License-Identifier: GPL-2.0+
> > +#
> > +# (C) Copyright 2014-2021 Tony Dinh <mibodhi at gmail.com>
> > +#
> > +# Based on
> > +# Marvell Semiconductor <www.marvell.com>
> > +# Written-by: Prafulla Wadaskar <prafulla at marvell.com>
> > +#
> > +
> > +obj-y        := pogo_v4.o
> > diff --git a/board/cloudengines/pogo_v4/kwbimage.cfg b/board/cloudengines/pogo_v4/kwbimage.cfg
> > new file mode 100644
> > index 0000000000..f6294fe313
> > --- /dev/null
> > +++ b/board/cloudengines/pogo_v4/kwbimage.cfg
> > @@ -0,0 +1,148 @@
> > +# SPDX-License-Identifier: GPL-2.0+
> > +#
> > +# Copyright (C) 2012
> > +# David Purdy <david.c.purdy at gmail.com>
> > +#
> > +# Based on Kirkwood support:
> > +# (C) Copyright 2009
> > +# Marvell Semiconductor <www.marvell.com>
> > +# Written-by: Prafulla Wadaskar <prafulla <at> marvell.com>
> > +
> > +# Boot Media configurations   (DONE)
> > +BOOT_FROM    nand
> > +NAND_ECC_MODE        default
> > +NAND_PAGE_SIZE       0x0800
> > +
> > +# SOC registers configuration using bootrom header extension
> > +# Maximum KWBIMAGE_MAX_CONFIG configurations allowed
> > +
> > +# Configure RGMII-0 interface pad voltage to 1.8V (SHOULD BE SAME)
> > +DATA 0xffd100e0 0x1b1b1b9b
> > +
> > +#Dram initalization for SINGLE x16 CL=3 @ 200MHz   (need CL=3 @ 200MHz?)
> > +DATA 0xffd01400 0x43000618   # DDR Configuration register
> > +# bit13-0:  0x200 (200 DDR2 clks refresh rate)
> > +# bit23-14: zero
> > +# bit24: 1= enable exit self refresh mode on DDR access
> > +# bit25: 1 required
> > +# bit29-26: zero
> > +# bit31-30: 01
> > +
> > +DATA 0xffd01404 0x34143000   # DDR Controller Control Low
> > +# bit 4:    0=addr/cmd in smame cycle
> > +# bit 5:    0=clk is driven during self refresh, we don't care for APX
> > +# bit 6:    0=use recommended falling edge of clk for addr/cmd
> > +# bit14:    0=input buffer always powered up
> > +# bit18:    1=cpu lock transaction enabled
> > +# bit23-20: 3=recommended value for CL=3 and STARTBURST_DEL disabled bit31=0
> > +# bit27-24: 6= CL+3, STARTBURST sample stages, for freqs 400MHz, unbuffered DIMM
> > +# bit30-28: 3 required
> > +# bit31:    0=no additional STARTBURST delay
> > +
> > +DATA 0xffd01408 0x11012227   # DDR Timing (Low) (active cycles value +1)
> > +# bit3-0:   TRAS lsbs
> > +# bit7-4:   TRCD
> > +# bit11- 8: TRP
> > +# bit15-12: TWR
> > +# bit19-16: TWTR
> > +# bit20:    TRAS msb
> > +# bit23-21: 0x0
> > +# bit27-24: TRRD
> > +# bit31-28: TRTP
> > +
> > +DATA 0xffd0140c 0x00000819   #  DDR Timing (High)
> > +# bit6-0:   TRFC
> > +# bit8-7:   TR2R
> > +# bit10-9:  TR2W
> > +# bit12-11: TW2W
> > +# bit31-13: zero required
> > +
> > +DATA 0xffd01410 0x00000001   #  DDR Address Control  (changed to Dockstar vals)
> > +# bit1-0:   00, Cs0width=x16
> > +# bit3-2:   10, Cs0size=512Mb
> > +# bit5-4:   00, Cs2width=nonexistent
> > +# bit7-6:   00, Cs1size =nonexistent
> > +# bit9-8:   00, Cs2width=nonexistent
> > +# bit11-10: 00, Cs2size =nonexistent
> > +# bit13-12: 00, Cs3width=nonexistent
> > +# bit15-14: 00, Cs3size =nonexistent
> > +# bit16:    0,  Cs0AddrSel
> > +# bit17:    0,  Cs1AddrSel
> > +# bit18:    0,  Cs2AddrSel
> > +# bit19:    0,  Cs3AddrSel
> > +# bit31-20: 0 required
> > +
> > +DATA 0xffd01414 0x00000000   #  DDR Open Pages Control
> > +# bit0:    0,  OpenPage enabled
> > +# bit31-1: 0 required
> > +
> > +DATA 0xffd01418 0x00000000   #  DDR Operation
> > +# bit3-0:   0x0, DDR cmd
> > +# bit31-4:  0 required
> > +
> > +DATA 0xffd0141c 0x00000632   #  DDR Mode
> > +# bit2-0:   2, BurstLen=2 required
> > +# bit3:     0, BurstType=0 required
> > +# bit6-4:   4, CL=5                          (<===== change to CL=3 ?)
> > +# bit7:     0, TestMode=0 normal
> > +# bit8:     0, DLL reset=0 normal
> > +# bit11-9:  6, auto-precharge write recovery ????????????
> > +# bit12:    0, PD must be zero
> > +# bit31-13: 0 required
> > +
> > +DATA 0xffd01420 0x00000040   #  DDR Extended Mode
> > +# bit0:    0,  DDR DLL enabled
> > +# bit1:    0,  DDR drive strenght normal
> > +# bit2:    0,  DDR ODT control lsd (disabled)
> > +# bit5-3:  000, required
> > +# bit6:    1,  DDR ODT control msb, (disabled)
> > +# bit9-7:  000, required
> > +# bit10:   0,  differential DQS enabled
> > +# bit11:   0, required
> > +# bit12:   0, DDR output buffer enabled
> > +# bit31-13: 0 required
> > +
> > +DATA 0xffd01424 0x0000F07F   #  DDR Controller Control High
> > +# bit2-0:  111, required
> > +# bit3  :  1  , MBUS Burst Chop disabled
> > +# bit6-4:  111, required
> > +# bit7  :  0
> > +# bit8  :  0  , no sample stage
> > +# bit9  :  0  , no half clock cycle addition to dataout
> > +# bit10 :  0  , 1/4 clock cycle skew enabled for addr/ctl signals
> > +# bit11 :  0  , 1/4 clock cycle skew disabled for write mesh
> > +# bit15-12: 1111 required
> > +# bit31-16: 0    required
> > +
> > +DATA 0xffd01428 0x00085520   # DDR2 ODT Read Timing (default values)
> > +DATA 0xffd0147c 0x00008552   # DDR2 ODT Write Timing (default values)
> > +
> > +DATA 0xFFD01500 0x00000000   # CS[0]n Base address to 0x0
> > +DATA 0xFFD01504 0x07FFFFF1   # CS[0]n Size
> > +# bit0:    1,  Window enabled
> > +# bit1:    0,  Write Protect disabled
> > +# bit3-2:  00, CS0 hit selected
> > +# bit23-4: ones, required
> > +# bit31-24: 0x07, Size (i.e. 128MB)
> > +
> > +DATA 0xFFD0150C 0x00000000   # CS[1]n Size, window disabled
> > +DATA 0xFFD01514 0x00000000   # CS[2]n Size, window disabled
> > +DATA 0xFFD0151C 0x00000000   # CS[3]n Size, window disabled
> > +
> > +DATA 0xffd01494 0x00030000   #  DDR ODT Control (Low)                 (DONE)
> > +# bit3-0:  2, ODT0Rd, MODT[0] asserted during read from DRAM CS1
> > +# bit7-4:  1, ODT0Rd, MODT[0] asserted during read from DRAM CS0
> > +# bit19-16:2, ODT0Wr, MODT[0] asserted during write to DRAM CS1
> > +# bit23-20:1, ODT0Wr, MODT[0] asserted during write to DRAM CS0
> > +
> > +DATA 0xffd01498 0x00000000   #  DDR ODT Control (High)  (DONE)
> > +# bit1-0:  00, ODT0 controlled by ODT Control (low) register above
> > +# bit3-2:  01, ODT1 active NEVER!
> > +# bit31-4: zero, required
> > +
> > +DATA 0xffd0149c 0x0000e803   # CPU ODT Control        (DONE)
> > +DATA 0xffd01480 0x00000001   # DDR Initialization Control     (DONE)
> > +#bit0=1, enable DDR init upon this register write
> > +
> > +# End of Header extension
> > +DATA 0x0 0x0
> > 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
> > @@ -0,0 +1,220 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright (C) 2014-2021 Tony Dinh <mibodhi at gmail.com>
> > + *
> > + * Based on
> > + * Copyright (C) 2012 David Purdy <david.c.purdy at gmail.com>
> > + *
> > + * Based on Kirkwood support:
> > + * (C) Copyright 2009
> > + * Marvell Semiconductor <www.marvell.com>
> > + * Written-by: Prafulla Wadaskar <prafulla at marvell.com>
> > + */
> > +
> > +#include <common.h>
> > +#include <miiphy.h>
> > +#include <asm/arch/cpu.h>
> > +#include <asm/arch/soc.h>
> > +#include <asm/arch/mpp.h>
> > +#include <asm/io.h>
> > +#include <asm/arch/gpio.h>
> > +#include <asm/mach-types.h>
> > +#include <bootstage.h>
> > +#include <command.h>
> > +#include <init.h>
> > +#include <linux/bitops.h>
> > +
> > +DECLARE_GLOBAL_DATA_PTR;
> > +
> > +/* GPIO configuration */
> > +#define POGO_V4_OE_LOW                               (~(0))
> > +#define POGO_V4_OE_HIGH                              (~(0))
> > +#define POGO_V4_OE_VAL_LOW                   BIT(29)
> > +#define POGO_V4_OE_VAL_HIGH                  0
> > +
> > +/* PHY related */
> > +#define MV88E1116_LED_FCTRL_REG                      10
> > +#define MV88E1116_CPRSP_CR3_REG                      21
> > +#define MV88E1116_MAC_CTRL_REG                       21
> > +#define MV88E1116_PGADR_REG                  22
> > +#define MV88E1116_RGMII_TXTM_CTRL            BIT(4)
> > +#define MV88E1116_RGMII_RXTM_CTRL            BIT(5)
> > +
> > +/* button */
> > +#define BTN_EJECT                            29
> > +
> > +int board_early_init_f(void)
> > +{
> > +     /*
> > +      * default gpio configuration
> > +      * There are maximum 64 gpios controlled through 2 sets of registers
> > +      * the  below configuration configures mainly initial LED status
> > +      */
> > +     mvebu_config_gpio(POGO_V4_OE_VAL_LOW,
> > +                       POGO_V4_OE_VAL_HIGH,
> > +                       POGO_V4_OE_LOW, POGO_V4_OE_HIGH);
> > +
> > +     /* Multi-Purpose Pins Functionality configuration */
> > +     u32 kwmpp_config[] = {
> > +             MPP0_NF_IO2,
> > +             MPP1_NF_IO3,
> > +             MPP2_NF_IO4,
> > +             MPP3_NF_IO5,
> > +             MPP4_NF_IO6,
> > +             MPP5_NF_IO7,
> > +             MPP6_SYSRST_OUTn,
> > +             MPP7_GPO,
> > +             MPP8_TW_SDA,
> > +             MPP9_TW_SCK,
> > +             MPP10_UART0_TXD,
> > +             MPP11_UART0_RXD,
> > +             MPP12_SD_CLK,
> > +             MPP13_SD_CMD,
> > +             MPP14_SD_D0,
> > +             MPP15_SD_D1,
> > +             MPP16_SD_D2,
> > +             MPP17_SD_D3,
> > +             MPP18_NF_IO0,
> > +             MPP19_NF_IO1,
> > +             MPP20_SATA1_ACTn,
> > +             MPP21_SATA0_ACTn,
> > +             MPP22_GPIO,     /* Green LED */
> > +             MPP23_GPIO,
> > +             MPP24_GPIO,     /* Red LED */
> > +             MPP25_GPIO,
> > +             MPP26_GPIO,
> > +             MPP27_GPIO,
> > +             MPP28_GPIO,
> > +             MPP29_GPIO,     /* Eject button */
> > +             MPP30_GPIO,
> > +             MPP31_GPIO,
> > +             MPP32_GPIO,
> > +             MPP33_GPIO,
> > +             MPP34_GPIO,
> > +             MPP35_GPIO,     /* FR6192 has only 36 GPIOs */
> > +             0
> > +     };
> > +     kirkwood_mpp_conf(kwmpp_config, NULL);
> > +
> > +     return 0;
> > +}
> > +
> > +int board_late_init(void)
> > +{
> > +     /* Do late init to ensure successful enumeration of XHCI devices */
> > +     pci_init();
> > +     return 0;
> > +}
> > +
> > +int board_init(void)
> > +{
> > +     /* Boot parameters address */
> > +     gd->bd->bi_boot_params = mvebu_sdram_bar(0) + 0x100;
> > +
> > +     return 0;
> > +}
> > +
> > +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";
> > +
> > +     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);
>
> Please take a look at drivers/net/phy/marvell.c / m88e1310_config().
> Here you will find (amongst others):
>
>         /* Set RGMII delay */
>         phy_write(phydev, MDIO_DEVAD_NONE, MIIM_88E1310_PHY_PAGE, 0x0002);
>         reg = phy_read(phydev, MDIO_DEVAD_NONE, MIIM_88E1310_PHY_RGMII_CTRL);
>         reg |= 0x0030;
>         phy_write(phydev, MDIO_DEVAD_NONE, MIIM_88E1310_PHY_RGMII_CTRL, reg);
>
> Can't you use the Marvell PHY driver instead of this ad-hoc
> implementation? I didn't check the compatibilty of your PHY and this
> driver though.

Thanks for the advice. Marek had a similar comment regarding this code.

https://lists.denx.de/pipermail/u-boot/2021-December/470019.html

Marek said,
"There the m88e1118_config() method already does one thing of what you
are doing here: enabling rgmii delays. It also sets LED config, but
does not reset the PHY. You can add call to phy_reset() there..."

Currently, all other Kirkwood boards also use this old ad-hoc
implementation. And the PHY addr is different in some of these boards.
If we add phy_reset() in m88e1118_config(), we would need to make sure
all existing boards that use the driver will work... OTOH, perhaps we
can keep the phy_reset() call in each Kirkwood board...

Sounds like an approach that will potentially touch a common area, so
I think it is best that I will do the modernization and more testing
in a separate patch series, after we're done with this Pogo V4 board.

Does that sound reasonable?

Thanks,
Tony

> Thanks,
> Stefan
>
> > +
> > +     /* reset the phy */
> > +     miiphy_reset(name, phyaddr);
> > +
> > +     printf("88E1116 Initialized on %s\n", name);
> > +}
> > +#endif /* CONFIG_RESET_PHY_R */
> > +
> > +#if CONFIG_IS_ENABLED(BOOTSTAGE)
> > +#define GREEN_LED    BIT(22)
> > +#define RED_LED              BIT(24)
> > +#define BOTH_LEDS    (GREEN_LED | RED_LED)
> > +#define NEITHER_LED  0
> > +
> > +static void set_leds(u32 leds, u32 blinking)
> > +{
> > +     struct kwgpio_registers *r;
> > +     u32 oe;
> > +     u32 bl;
> > +
> > +     r = (struct kwgpio_registers *)MVEBU_GPIO0_BASE;
> > +     oe = readl(&r->oe) | BOTH_LEDS;
> > +     writel(oe & ~leds, &r->oe);     /* active low */
> > +     bl = readl(&r->blink_en) & ~BOTH_LEDS;
> > +     writel(bl | blinking, &r->blink_en);
> > +}
> > +
> > +void show_boot_progress(int val)
> > +{
> > +     switch (val) {
> > +     case BOOTSTAGE_ID_RUN_OS:               /* booting Linux */
> > +             set_leds(BOTH_LEDS, NEITHER_LED);
> > +             break;
> > +     case BOOTSTAGE_ID_NET_ETH_START:        /* Ethernet initialization */
> > +             set_leds(GREEN_LED, GREEN_LED);
> > +             break;
> > +     default:
> > +             if (val < 0)    /* error */
> > +                     set_leds(RED_LED, RED_LED);
> > +             break;
> > +     }
> > +}
> > +#endif
> >
>
> Viele Grüße,
> Stefan Roese
>
> --
> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de


More information about the U-Boot mailing list