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

[Aditya Prayoga]

Hi Stefan,

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.
Maybe I should use named gpios in the device tree.

Aditya

> Here an example from the mvpp2 driver:
>
> https://patchwork.ozlabs.org/patch/799654/
>
> Could you try to integrate something similar into the mvneta
> driver?
>
> Thanks,
> Stefan