[U-Boot] [PATCH v2 6/6] arm: mvebu: helios4: Reset uSOM onboard phy during board init

Aditya Prayoga aditya at kobol.io
Tue Dec 4 15:37:34 UTC 2018


Hi Baruch,

On Tue, Dec 4, 2018 at 9:40 PM Baruch Siach <baruch at tkos.co.il> wrote:
>
> Hi Aditya,
>
> On Mon, Dec 03, 2018 at 09:39:56AM +0700, Aditya Prayoga wrote:
> > On Fri, Nov 30, 2018 at 3:25 PM Stefan Roese <sr at denx.de> wrote:
> > > On 30.11.18 09:14, Aditya Prayoga wrote:
> > > > On Fri, Nov 30, 2018 at 2:44 PM Stefan Roese <sr at denx.de> wrote:
> > > >>
> > > >> On 30.11.18 03:54, Aditya Prayoga wrote:
> > > >>> Similar to Clearfog rev 2.1, GPIO 19 also used to reset onboard ethernet
> > > >>> PHY.
> > > >>>
> > > >>> Signed-off-by: Aditya Prayoga <aditya at kobol.io>
> > > >>> ---
> > > >>> v2:
> > > >>> * Use generic gpio_* API (Baruch Siach)
> > > >>> ---
> > > >>>    board/kobol/helios4/helios4.c | 31 +++++++++++++++++++++++++++++++
> > > >>>    1 file changed, 31 insertions(+)
> > > >>>
> > > >>> diff --git a/board/kobol/helios4/helios4.c b/board/kobol/helios4/helios4.c
> > > >>> index 37c46a5..e535d7b 100644
> > > >>> --- a/board/kobol/helios4/helios4.c
> > > >>> +++ b/board/kobol/helios4/helios4.c
> > > >>> @@ -8,6 +8,7 @@
> > > >>>    #include <i2c.h>
> > > >>>    #include <miiphy.h>
> > > >>>    #include <netdev.h>
> > > >>> +#include <asm/gpio.h>
> > > >>>    #include <asm/io.h>
> > > >>>    #include <asm/arch/cpu.h>
> > > >>>    #include <asm/arch/soc.h>
> > > >>> @@ -111,9 +112,39 @@ int board_early_init_f(void)
> > > >>>
> > > >>>    int board_init(void)
> > > >>>    {
> > > >>> +     struct udevice *gpio0;
> > > >>> +     struct gpio_desc phy_reset;
> > > >>> +     int ret;
> > > >>> +
> > > >>>        /* Address of boot parameters */
> > > >>>        gd->bd->bi_boot_params = mvebu_sdram_bar(0) + 0x100;
> > > >>>
> > > >
> > > >  From here
> > > >>> +     ret = uclass_get_device_by_name(UCLASS_GPIO,
> > > >>> +                                     "gpio at 18100",
> > > >>> +                                     &gpio0);
> > > >>> +     if (ret < 0) {
> > > >>> +             printf("Failed to find gpio at 18100 node.\n");
> > > >>> +             return ret;
> > > >>> +     }
> > > >>> +
> > > >>> +     phy_reset.dev = gpio0;
> > > >>> +
> > > >>> +     /* MPP19 controls the uSOM onboard phy reset */
> > > >>> +     phy_reset.offset = 19;
> > > >>> +
> > > > up until this line can be replaced by a single line
> > > > ret = dm_gpio_lookup_name("A19", &reset);
> > > >
> > > > but gpio "A19" does not correlate with any documentation
> > > > (datasheet, schematic, or device tree).
> > > > I also got this warning
> > > > "Device 'gpio at 18100': seq 0 is in use by 'gpio-expander at 20'"
> > > >
> > > >>> +     ret = dm_gpio_request(&phy_reset, "phy-reset");
> > > >>> +     if (ret)
> > > >>> +             return ret;
> > > >>> +
> > > >>> +     dm_gpio_set_dir_flags(&phy_reset,
> > > >>> +                           GPIOD_IS_OUT |
> > > >>> +                           GPIOD_ACTIVE_LOW |
> > > >>> +                           GPIOD_IS_OUT_ACTIVE);
> > > >>> +
> > > >>> +     mdelay(10);
> > > >>> +     dm_gpio_set_value(&phy_reset, 0);
> > > >>> +     mdelay(10);
> > > >>> +
> > > >>
> > > >> Hmm, this is a pretty complex and unusual way to use the GPIO.
> > > >> Please use the DT to describe the PHY reset GPIO instead. And since
> > > >> it seems to be a common issue to have such a PHY reset GPIO, it
> > > >> would be better to integrate this into the ethernet driver instead.
> > > >
> > > > well, I followed minnowmax implementation
> > > > (board/intel/minnowmax/minnowmax.c) and some other boards.
> > >
> > > This seems not to be a good example. It's pretty outdated, AFAICT.
> > >
> > > > Maybe I should use named gpios in the device tree.
> > >
> > > Yes. As mentioned above, please look at the mvpp2 driver. And e.g.
> > > at the armada-8040-clearfog-gt-8k.dts DT file:
> > >
> > > /* 1G SGMII */
> > > &cps_eth1 {
> > >         status = "okay";
> > >         phy-mode = "sgmii";
> > >         phy = <&phy0>;
> > >         phy-reset-gpios = <&cpm_gpio1 11 GPIO_ACTIVE_LOW>;
> > > };
> > >
> > > Here you see, how the GPIO is defined in the DT. It should not
> > > be too hard to get this implemented in the mvneta driver as well.
> > >
> > Yes, what I meant is to put "phy-reset-gpios" in the device tree but it
> > would still part of the system reset so the reset would be asserted in
> > board_init.
> >
> > If the reset implemented in driver (mvneta), the reset would only
> > asserted when u-boot going to use the network interface. This is not
> > desirable.
>
> Network device probe routine is always called even when not used. See this
> comment in eth_initialize():
>
>         /*
>          * Devices need to write the hwaddr even if not started so that Linux
>          * will have access to the hwaddr that u-boot stored for the device.
>          * This is accomplished by attempting to probe each device and calling
>          * their write_hwaddr() operation.
>          */
>
That is what I thought but that is not what I observed.
I tried to port that mvpp2 changes to mvneta
---
diff --git a/drivers/net/mvneta.c b/drivers/net/mvneta.c
index 8cb04b5..34f191d 100644
--- a/drivers/net/mvneta.c
+++ b/drivers/net/mvneta.c
@@ -27,6 +27,7 @@
 #include <asm/arch/soc.h>
 #include <linux/compat.h>
 #include <linux/mbus.h>
+#include <asm-generic/gpio.h>

 DECLARE_GLOBAL_DATA_PTR;

@@ -274,6 +275,9 @@ struct mvneta_port {
     int init;
     int phyaddr;
     struct phy_device *phydev;
+#ifdef CONFIG_DM_GPIO
+    struct gpio_desc phy_reset_gpio;
+#endif
     struct mii_dev *bus;
 };

@@ -1733,6 +1737,17 @@ static int mvneta_probe(struct udevice *dev)
         pp->phyaddr = fdtdec_get_int(blob, addr, "reg", 0);
     }

+#ifdef CONFIG_DM_GPIO
+    gpio_request_by_name(dev, "phy-reset-gpios", 0,
+                 &pp->phy_reset_gpio, GPIOD_IS_OUT);
+
+    if (dm_gpio_is_valid(&pp->phy_reset_gpio)) {
+        dm_gpio_set_value(&pp->phy_reset_gpio, 1);
+        mdelay(3000);
+        dm_gpio_set_value(&pp->phy_reset_gpio, 0);
+    }
+#endif
+
     bus = mdio_alloc();
     if (!bus) {
         printf("Failed to allocate MDIO bus\n");
---
I intentionally put that 3 seconds delay so i can observe the
RJ45 LED. After pressing the reset button, the LED never turned off.
It turned off only when i access the network, for example tftpboot.

here are snippet from the serial log
----
Net:
Warning: ethernet at 70000 (eth1) using random MAC address - ae:c1:5a:4e:ba:00
eth1: ethernet at 70000
Hit any key to stop autoboot:  0
=> tftpboot
ethernet at 70000 Waiting for PHY auto negotiation to complete....... done
*** ERROR: `serverip' not set
=>
----
The LED turned off during "Waiting for PHY auto negotiation"

Regards,
Aditya

> baruch
>
> --
>      http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
> =}------------------------------------------------ooO--U--Ooo------------{=
>    - baruch at tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -


More information about the U-Boot mailing list