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

[Stefan Roese]

On 30.11.18 09:14, Aditya Prayoga wrote:
> 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.

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.

Thanks,
Stefan