Hi Stefan,
On Fri, Nov 30, 2018 at 3:25 PM Stefan Roese <s...@denx.de> wrote:
>
> On 30.11.18 09:14, Aditya Prayoga wrote:
> > Hi Stefan,
> >
> > On Fri, Nov 30, 2018 at 2:44 PM Stefan Roese <s...@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 <adi...@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.
>
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.
Regards,
Aditya
> Thanks,
> Stefan
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
