[U-Boot] [PATCH v2 0/6] Update support for Helios4 board

[U-Boot] [PATCH v2 0/6] Update support for Helios4 board

[Aditya Prayoga]

Hi everyone,

This series add some missing peripheral support and fixed that was
implemented on U-Boot 2013.01 Marvell version: 2015_T1.0p16
- helios4: mmc: move environment before 1M
- helios4: Enable I2C and IO Expander
- helios4: Reset uSOM onboard phy during board init

and added new peripheral support and improvement
- helios4: Enable mvebu GPIO
- helios4: Enable SPI flash support
- helios4: Reduce U-Boot image size

---
Notes
  checkpatch.pl emit "WARNING: added, moved or deleted file(s), does
    MAINTAINERS need updating?" on 0002-arm-mvebu-helios4-Enable-I2C-and-
    IO-Expander.patch due to new file, armada-388-helios4-u-boot.dtsi
    
---
Changes v1 -> v2:
  * Rearrange the patch, enabling I2C and convert I2C IO expander to DM
    should applied together.
  * Move u-boot specific properties into armada-388-helios4-u-boot.dtsi
    (Dennis Gilmore)
  * Use generic gpio_* API to do phy reset (Baruch Siach)
---
Aditya Prayoga (6):
  arm: mvebu: helios4: mmc: move environment before 1M
  arm: mvebu: helios4: Enable I2C and IO Expander
  arm: mvebu: helios4: Enable mvebu GPIO
  arm: mvebu: helios4: Reduce U-Boot image size
  arm: mvebu: helios4: Enable SPI flash support
  arm: mvebu: helios4: Reset uSOM onboard phy during board init

 arch/arm/dts/armada-388-helios4-u-boot.dtsi | 18 +++++++++
 board/kobol/helios4/helios4.c               | 57 ++++++++++++++---------------
 configs/helios4_defconfig                   | 13 ++++---
 include/configs/helios4.h                   | 48 +++++++++++++++++-------
 4 files changed, 88 insertions(+), 48 deletions(-)
 create mode 100644 arch/arm/dts/armada-388-helios4-u-boot.dtsi

-- 
2.7.4

[U-Boot] [PATCH v2 1/6] arm: mvebu: helios4: mmc: move environment before 1M

[Aditya Prayoga]

Store the environment before 1M into the block device.
This constant is easier to remember, saves a little space,
and is in line with what SolidRun's 2018.01-based fork does for the
clearfog.

Signed-off-by: Josua Mayer <josua.mayer97@gmail.com>
[ Aditya Prayoga:
* Port forward]
Signed-off-by: Aditya Prayoga <aditya@kobol.io>
---
 include/configs/helios4.h | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/include/configs/helios4.h b/include/configs/helios4.h
index ce912ea..c71055f 100644
--- a/include/configs/helios4.h
+++ b/include/configs/helios4.h
@@ -46,14 +46,9 @@
 /* Environment in MMC */
 #define CONFIG_SYS_MMC_ENV_DEV		0
 #define CONFIG_ENV_SECT_SIZE		0x200
-#define CONFIG_ENV_SIZE			0x10000
-/*
- * For SD - reserve 1 LBA for MBR + 1M for u-boot image. The MMC/eMMC
- * boot image starts @ LBA-0.
- * As result in MMC/eMMC case it will be a 1 sector gap between u-boot
- * image and environment
- */
-#define CONFIG_ENV_OFFSET		0xf0000
+#define CONFIG_ENV_SIZE			0x2000
+/* stay within first 1M */
+#define CONFIG_ENV_OFFSET		(SZ_1M - CONFIG_ENV_SIZE)
 #define CONFIG_ENV_ADDR			CONFIG_ENV_OFFSET
 
 #define CONFIG_PHY_MARVELL		/* there is a marvell phy */
-- 
2.7.4

[U-Boot] [PATCH v2 2/6] arm: mvebu: helios4: Enable I2C and IO Expander

[Aditya Prayoga]

Enable Marvell I2C driver and I2C IO expander. Set default bus to
external I2C bus. Define I2C aliases in device tree so it can be
recognized by the driver.

Signed-off-by: Aditya Prayoga <aditya@kobol.io>
---
v2:
* Rearrange the patch, enabling I2C and convert I2C IO expander to DM
    should applied together.
* Move u-boot specific properties into armada-388-helios4-u-boot.dtsi
    (Dennis Gilmore)
---
 arch/arm/dts/armada-388-helios4-u-boot.dtsi |  8 ++++++++
 board/kobol/helios4/helios4.c               | 32 -----------------------------
 configs/helios4_defconfig                   |  4 ++++
 3 files changed, 12 insertions(+), 32 deletions(-)
 create mode 100644 arch/arm/dts/armada-388-helios4-u-boot.dtsi

diff --git a/arch/arm/dts/armada-388-helios4-u-boot.dtsi b/arch/arm/dts/armada-388-helios4-u-boot.dtsi
new file mode 100644
index 0000000..bd8c6ce
--- /dev/null
+++ b/arch/arm/dts/armada-388-helios4-u-boot.dtsi
@@ -0,0 +1,8 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/ {
+	aliases {
+		i2c0 = &i2c0;
+		i2c1 = &i2c1;
+	};
+};
diff --git a/board/kobol/helios4/helios4.c b/board/kobol/helios4/helios4.c
index 3416783..37c46a5 100644
--- a/board/kobol/helios4/helios4.c
+++ b/board/kobol/helios4/helios4.c
@@ -33,18 +33,6 @@ DECLARE_GLOBAL_DATA_PTR;
 #define BOARD_GPP_POL_LOW	0x0
 #define BOARD_GPP_POL_MID	0x0
 
-/* IO expander on Marvell GP board includes e.g. fan enabling */
-struct marvell_io_exp {
-	u8 addr;
-	u8 val;
-};
-
-static struct marvell_io_exp io_exp[] = {
-	{6, 0xf9},
-	{2, 0x46}, /* Assert reset signals and enable USB3 current limiter */
-	{6, 0xb9}
-};
-
 static struct serdes_map board_serdes_map[] = {
 	{SATA0, SERDES_SPEED_6_GBPS, SERDES_DEFAULT_MODE, 0, 0},
 	{USB3_HOST0, SERDES_SPEED_5_GBPS, SERDES_DEFAULT_MODE, 0, 0},
@@ -123,29 +111,9 @@ int board_early_init_f(void)
 
 int board_init(void)
 {
-	int i;
-
 	/* Address of boot parameters */
 	gd->bd->bi_boot_params = mvebu_sdram_bar(0) + 0x100;
 
-	/* Init I2C IO expanders */
-	for (i = 0; i < ARRAY_SIZE(io_exp); i++) {
-		struct udevice *dev;
-		int ret;
-
-		ret = i2c_get_chip_for_busnum(0, io_exp[i].addr, 1, &dev);
-		if (ret) {
-			printf("Cannot find I2C: %d\n", ret);
-			return 0;
-		}
-
-		ret = dm_i2c_write(dev, io_exp[i].val, &io_exp[i].val, 1);
-		if (ret) {
-			printf("Failed to set IO expander via I2C\n");
-			return -EIO;
-		}
-	}
-
 	return 0;
 }
 
diff --git a/configs/helios4_defconfig b/configs/helios4_defconfig
index e30eb3d..6471fa3 100644
--- a/configs/helios4_defconfig
+++ b/configs/helios4_defconfig
@@ -38,7 +38,11 @@ CONFIG_ENV_IS_IN_MMC=y
 CONFIG_NET_RANDOM_ETHADDR=y
 CONFIG_SPL_OF_TRANSLATE=y
 CONFIG_SCSI_AHCI=y
+CONFIG_DM_PCA953X=y
 CONFIG_DM_I2C=y
+CONFIG_SYS_I2C_MVTWSI=y
+CONFIG_I2C_SET_DEFAULT_BUS_NUM=y
+CONFIG_I2C_DEFAULT_BUS_NUMBER=0x1
 CONFIG_MMC_SDHCI=y
 CONFIG_MMC_SDHCI_SDMA=y
 CONFIG_MMC_SDHCI_MV=y
-- 
2.7.4

[U-Boot] [PATCH v2 3/6] arm: mvebu: helios4: Enable mvebu GPIO

[Aditya Prayoga]

Make use of U-Boot's GPIO DM to control native GPIO and I2C IO expander.

Signed-off-by: Aditya Prayoga <aditya@kobol.io>
---
v2:
* Rearrange the patch, move I2C IO expander DM convertion together
    with I2C.
---
 configs/helios4_defconfig | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/configs/helios4_defconfig b/configs/helios4_defconfig
index 6471fa3..69dd774 100644
--- a/configs/helios4_defconfig
+++ b/configs/helios4_defconfig
@@ -22,6 +22,7 @@ CONFIG_DISPLAY_BOARDINFO_LATE=y
 CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR=0x141
 CONFIG_SPL_I2C_SUPPORT=y
 # CONFIG_CMD_FLASH is not set
+CONFIG_CMD_GPIO=y
 CONFIG_CMD_I2C=y
 CONFIG_CMD_MMC=y
 CONFIG_CMD_PCI=y
@@ -38,6 +39,8 @@ CONFIG_ENV_IS_IN_MMC=y
 CONFIG_NET_RANDOM_ETHADDR=y
 CONFIG_SPL_OF_TRANSLATE=y
 CONFIG_SCSI_AHCI=y
+CONFIG_DM_GPIO=y
+CONFIG_MVEBU_GPIO=y
 CONFIG_DM_PCA953X=y
 CONFIG_DM_I2C=y
 CONFIG_SYS_I2C_MVTWSI=y
-- 
2.7.4

[U-Boot] [PATCH v2 4/6] arm: mvebu: helios4: Reduce U-Boot image size

[Aditya Prayoga]

Remove unused I2C support in SPL and use simple_malloc functions to
reduce SPL image size.
Since Helios4 does not have any PCIe allocated on SerDes, remove PCI
support. MTD layer on top of SPI flash is not needed, remove it also.

Signed-off-by: Aditya Prayoga <aditya@kobol.io>
---
 configs/helios4_defconfig | 5 +----
 include/configs/helios4.h | 6 ------
 2 files changed, 1 insertion(+), 10 deletions(-)

diff --git a/configs/helios4_defconfig b/configs/helios4_defconfig
index 69dd774..53995fe 100644
--- a/configs/helios4_defconfig
+++ b/configs/helios4_defconfig
@@ -8,6 +8,7 @@ CONFIG_TARGET_HELIOS4=y
 CONFIG_MVEBU_SPL_BOOT_DEVICE_MMC=y
 CONFIG_SPL_MMC_SUPPORT=y
 CONFIG_SPL_SERIAL_SUPPORT=y
+CONFIG_SPL_SYS_MALLOC_SIMPLE=y
 CONFIG_SPL=y
 CONFIG_DEBUG_UART_BASE=0xd0012000
 CONFIG_DEBUG_UART_CLOCK=250000000
@@ -20,12 +21,10 @@ CONFIG_SYS_CONSOLE_INFO_QUIET=y
 # CONFIG_DISPLAY_BOARDINFO is not set
 CONFIG_DISPLAY_BOARDINFO_LATE=y
 CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR=0x141
-CONFIG_SPL_I2C_SUPPORT=y
 # CONFIG_CMD_FLASH is not set
 CONFIG_CMD_GPIO=y
 CONFIG_CMD_I2C=y
 CONFIG_CMD_MMC=y
-CONFIG_CMD_PCI=y
 CONFIG_CMD_SF=y
 CONFIG_CMD_SPI=y
 CONFIG_CMD_USB=y
@@ -52,11 +51,9 @@ CONFIG_MMC_SDHCI_MV=y
 CONFIG_SPI_FLASH=y
 CONFIG_SPI_FLASH_BAR=y
 CONFIG_SPI_FLASH_WINBOND=y
-CONFIG_SPI_FLASH_MTD=y
 CONFIG_PHY_GIGE=y
 CONFIG_MVNETA=y
 CONFIG_MII=y
-CONFIG_PCI=y
 CONFIG_SCSI=y
 CONFIG_DEBUG_UART_SHIFT=2
 CONFIG_SYS_NS16550=y
diff --git a/include/configs/helios4.h b/include/configs/helios4.h
index c71055f..e665340 100644
--- a/include/configs/helios4.h
+++ b/include/configs/helios4.h
@@ -54,12 +54,6 @@
 #define CONFIG_PHY_MARVELL		/* there is a marvell phy */
 #define PHY_ANEG_TIMEOUT	8000	/* PHY needs a longer aneg time */
 
-/* PCIe support */
-#ifndef CONFIG_SPL_BUILD
-#define CONFIG_PCI_MVEBU
-#define CONFIG_PCI_SCAN_SHOW
-#endif
-
 /* Keep device tree and initrd in lower memory so the kernel can access them */
 #define RELOCATION_LIMITS_ENV_SETTINGS	\
 	"fdt_high=0x10000000\0"		\
-- 
2.7.4

[U-Boot] [PATCH v2 5/6] arm: mvebu: helios4: Enable SPI flash support

[Aditya Prayoga]

Enable SPI flash support under U-Boot and SPL. The ENV size and offset,
ported from U-Boot 2013.01 Marvell version: 2015_T1.0p16

To create U-Boot image for SPI flash, user would need to replace
* CONFIG_MVEBU_SPL_BOOT_DEVICE_MMC with CONFIG_MVEBU_SPL_BOOT_DEVICE_SPI
* CONFIG_ENV_IS_IN_MMC with CONFIG_ENV_IS_IN_SPI_FLASH

Signed-off-by: Aditya Prayoga <aditya@kobol.io>
---
v2:
* Move u-boot specific properties into armada-388-helios4-u-boot.dtsi
    (Dennis Gilmore)
---
 arch/arm/dts/armada-388-helios4-u-boot.dtsi | 10 ++++++++++
 configs/helios4_defconfig                   |  1 -
 include/configs/helios4.h                   | 31 +++++++++++++++++++++++++++++
 3 files changed, 41 insertions(+), 1 deletion(-)

diff --git a/arch/arm/dts/armada-388-helios4-u-boot.dtsi b/arch/arm/dts/armada-388-helios4-u-boot.dtsi
index bd8c6ce..4bc00ae 100644
--- a/arch/arm/dts/armada-388-helios4-u-boot.dtsi
+++ b/arch/arm/dts/armada-388-helios4-u-boot.dtsi
@@ -4,5 +4,15 @@
 	aliases {
 		i2c0 = &i2c0;
 		i2c1 = &i2c1;
+		spi1 = &spi1;
 	};
 };
+
+&spi1 {
+	u-boot,dm-spl;
+};
+
+&w25q32 {
+	status = "okay";
+	u-boot,dm-spl;
+};
diff --git a/configs/helios4_defconfig b/configs/helios4_defconfig
index 53995fe..3bb4622 100644
--- a/configs/helios4_defconfig
+++ b/configs/helios4_defconfig
@@ -6,7 +6,6 @@ CONFIG_SPL_LIBGENERIC_SUPPORT=y
 CONFIG_SYS_MALLOC_F_LEN=0x2000
 CONFIG_TARGET_HELIOS4=y
 CONFIG_MVEBU_SPL_BOOT_DEVICE_MMC=y
-CONFIG_SPL_MMC_SUPPORT=y
 CONFIG_SPL_SERIAL_SUPPORT=y
 CONFIG_SPL_SYS_MALLOC_SIMPLE=y
 CONFIG_SPL=y
diff --git a/include/configs/helios4.h b/include/configs/helios4.h
index e665340..3157225 100644
--- a/include/configs/helios4.h
+++ b/include/configs/helios4.h
@@ -6,6 +6,8 @@
 #ifndef _CONFIG_HELIOS4_H
 #define _CONFIG_HELIOS4_H
 
+#include <linux/sizes.h>
+
 /*
  * High Level Configuration Options (easy to change)
  */
@@ -23,6 +25,9 @@
 
 /* SPI NOR flash default params, used by sf commands */
 #define CONFIG_SF_DEFAULT_BUS		1
+#define CONFIG_SF_DEFAULT_CS		0
+#define CONFIG_SF_DEFAULT_SPEED		104000000
+#define CONFIG_SF_DEFAULT_MODE		SPI_MODE_3
 
 /*
  * SDIO/MMC Card Configuration
@@ -43,6 +48,21 @@
 #define CONFIG_SYS_SCSI_MAX_DEVICE      (CONFIG_SYS_SCSI_MAX_SCSI_ID * \
 					CONFIG_SYS_SCSI_MAX_LUN)
 
+#ifdef CONFIG_MVEBU_SPL_BOOT_DEVICE_SPI
+/*
+ * SPI Flash configuration for the environment access
+ */
+#define CONFIG_ENV_SPI_BUS		CONFIG_SF_DEFAULT_BUS
+#define CONFIG_ENV_SPI_CS		CONFIG_SF_DEFAULT_CS
+#define CONFIG_ENV_SPI_MAX_HZ		CONFIG_SF_DEFAULT_SPEED
+
+/* Environment in SPI NOR flash */
+#define CONFIG_ENV_SECT_SIZE		SZ_64K
+#define CONFIG_ENV_SIZE			CONFIG_ENV_SECT_SIZE
+#define CONFIG_ENV_OFFSET		SZ_1M
+#endif
+
+#ifdef CONFIG_MVEBU_SPL_BOOT_DEVICE_MMC
 /* Environment in MMC */
 #define CONFIG_SYS_MMC_ENV_DEV		0
 #define CONFIG_ENV_SECT_SIZE		0x200
@@ -50,6 +70,7 @@
 /* stay within first 1M */
 #define CONFIG_ENV_OFFSET		(SZ_1M - CONFIG_ENV_SIZE)
 #define CONFIG_ENV_ADDR			CONFIG_ENV_OFFSET
+#endif
 
 #define CONFIG_PHY_MARVELL		/* there is a marvell phy */
 #define PHY_ANEG_TIMEOUT	8000	/* PHY needs a longer aneg time */
@@ -69,7 +90,13 @@
  */
 #define SPL_BOOT_SPI_NOR_FLASH		1
 #define SPL_BOOT_SDIO_MMC_CARD		2
+
+#ifdef CONFIG_MVEBU_SPL_BOOT_DEVICE_SPI
+#define CONFIG_SPL_BOOT_DEVICE		SPL_BOOT_SPI_NOR_FLASH
+#endif
+#ifdef CONFIG_MVEBU_SPL_BOOT_DEVICE_MMC
 #define CONFIG_SPL_BOOT_DEVICE		SPL_BOOT_SDIO_MMC_CARD
+#endif
 
 /* Defines for SPL */
 #define CONFIG_SPL_SIZE			(140 << 10)
@@ -88,12 +115,16 @@
 
 #if CONFIG_SPL_BOOT_DEVICE == SPL_BOOT_SPI_NOR_FLASH
 /* SPL related SPI defines */
+#define CONFIG_SPL_SPI_FLASH_SUPPORT
+#define CONFIG_SPL_SPI_LOAD
+#define CONFIG_SPL_SPI_SUPPORT
 #define CONFIG_SYS_SPI_U_BOOT_OFFS	0x20000
 #define CONFIG_SYS_U_BOOT_OFFS		CONFIG_SYS_SPI_U_BOOT_OFFS
 #endif
 
 #if CONFIG_SPL_BOOT_DEVICE == SPL_BOOT_SDIO_MMC_CARD
 /* SPL related MMC defines */
+#define CONFIG_SPL_MMC_SUPPORT
 #define CONFIG_SYS_MMC_U_BOOT_OFFS		(160 << 10)
 #define CONFIG_SYS_U_BOOT_OFFS			CONFIG_SYS_MMC_U_BOOT_OFFS
 #ifdef CONFIG_SPL_BUILD
-- 
2.7.4

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

[Aditya Prayoga]

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;
 
+	ret = uclass_get_device_by_name(UCLASS_GPIO,
+					"gpio at 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;
+
+	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);
+
 	return 0;
 }
 
-- 
2.7.4

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

[Stefan Roese]

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;
>   
> +	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;
> +
> +	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.
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

[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@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 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

[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@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 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

[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 3:25 PM Stefan Roese <sr@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 <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 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.

Regards,
Aditya

> Thanks,
> Stefan

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

[Baruch Siach]

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@denx.de> wrote:
> > On 30.11.18 09:14, Aditya Prayoga wrote:
> > > 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 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.
         */

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 -

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

[Aditya Prayoga]

Hi Baruch,

On Tue, Dec 4, 2018 at 9:40 PM Baruch Siach <baruch@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@denx.de> wrote:
> > > On 30.11.18 09:14, Aditya Prayoga wrote:
> > > > 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 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 -

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

[Stefan Roese]

On 04.12.18 16:37, Aditya Prayoga wrote:

<snip>

>>>> 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

This is not correct. The probe function is *not* always called - at least
not in newer DM drivers.

>> 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.
>>           */
>>

These are special hooks that are always called to write the MAC address,
even when the device is not used in U-Boot. IIRC, the bind() function
is also always called.

> 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"

This should work. Did you correctly add the GPIO to your DT? Is the
GPIO correctly referenced? Did you check this in the GPIO driver?

Thanks,
Stefan

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

[Aditya Prayoga]

Hi Stefan,

On Tue, Dec 4, 2018 at 10:51 PM Stefan Roese <sr@denx.de> wrote:
>
> On 04.12.18 16:37, Aditya Prayoga wrote:
>
> <snip>
>
> >>>> 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
>
> This is not correct. The probe function is *not* always called - at least
> not in newer DM drivers.
>
> >> 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.
> >>           */
> >>
>
> These are special hooks that are always called to write the MAC address,
> even when the device is not used in U-Boot. IIRC, the bind() function
> is also always called.
>
> > 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"
>
> This should work. Did you correctly add the GPIO to your DT? Is the
> GPIO correctly referenced? Did you check this in the GPIO driver?
>
You're right. My fault, I put the gpio under phy node not the ethernet node.
After move the gpio into ethernet node, it works.
I will update this series and send the mvneta changes as separate patch.

Regards,
Aditya

> Thanks,
> Stefan