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

30 Nov 2018

On 30.11.18 09:14, Aditya Prayoga wrote:
...
Hi Stefan,
On Fri, Nov 30, 2018 at 2:44 PM Stefan Roese sr@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@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@18100",


                                &gpio0);


if (ret < 0) {


        printf("Failed to find gpio@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@18100': seq 0 is in use by 'gpio-expander@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

    