Re: [U-Boot] [PATCH 07/10] sunxi: Convert CONFIG_RGMII to Kconfig

[Maxime Ripard]

Hi Thomas,

On Mon, Feb 27, 2017 at 02:59:46PM +0100, Thomas Petazzoni wrote:
> Hello,
> 
> On Thu, 23 Feb 2017 16:44:16 +0100, Mylène Josserand wrote:
> > Convert CONFIG_RGMII to Kconfig. Thanks to that, it is possible to
> > update defconfig files of SYS_EXTRA_OPTIONS accordingly and
> > remove it when it is possible.
> > 
> > Signed-off-by: Mylène Josserand 
> 
> I'm not familiar with how Allwinner platforms are handled in U-Boot,
> but I have a question for the following patches:
> 
>   [PATCH 07/10] sunxi: Convert CONFIG_RGMII to Kconfig
>   [PATCH 08/10] sunxi: Convert CONFIG_SATAPWR to Kconfig
>   [PATCH 09/10] sunxi: Convert CONFIG_MACPWR to Kconfig
>   [PATCH 10/10] sunxi: Convert CONS_INDEX to Kconfig
> 
> To me, all these details are hardware specific details that are given
> in the Device Tree. For example, RGMII is given in:
> 
>  {
> pinctrl-names = "default";
> pinctrl-0 = <_pins_rgmii_a>;
> phy = <>;
> phy-mode = "rgmii";
> status = "okay";
> 
> phy1: ethernet-phy@1 {
> reg = <1>;
> };
> };
> 
> The SATA power GPIO is given by:
> 
> _ahci_5v {
> pinctrl-0 = <_pwr_pin_olinuxinolime>;
> gpio = < 2 3 GPIO_ACTIVE_HIGH>;
> status = "okay";
> };
> 
> The MAC power GPIO is given by:
> 
> reg_gmac_3v3: gmac-3v3 {
> compatible = "regulator-fixed";
> pinctrl-names = "default";
> pinctrl-0 = <_power_pin_bananapi>;
> regulator-name = "gmac-3v3";
> regulator-min-microvolt = <330>;
> regulator-max-microvolt = <330>;
> startup-delay-us = <10>;
> enable-active-high;
> gpio = < 7 23 GPIO_ACTIVE_HIGH>;
> };
> 
> So it seems weird to encode it in the defconfig if the information is
> already available in the Device Tree.
> 
> In addition, the option names are very generic: CONFIG_SATAPWR,
> CONFIG_MACPWR, even though they are completely sunxi specific. So if
> those options really need to exist, should be they named
> CONFIG_SUNXI_SATAPWR, CONFIG_SUNXI_MACPWR ?

You make some pretty good points, and I definitely agree with you on
these changes.

However, there is also a plan to remove CONFIG_SYS_EXTRA_OPTIONS, and
all the defconfig using it, in a very near future, hence why we did
this serie.

We should definitely address the changes you pointed at, but I really
don't want to hold back the changes Mylene did. But she is also very
welcome to fix those issues later :)

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com


signature.asc
Description: PGP signature
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH 07/10] sunxi: Convert CONFIG_RGMII to Kconfig

[Thomas Petazzoni]

Hello,

On Thu, 23 Feb 2017 16:44:16 +0100, Mylène Josserand wrote:
> Convert CONFIG_RGMII to Kconfig. Thanks to that, it is possible to
> update defconfig files of SYS_EXTRA_OPTIONS accordingly and
> remove it when it is possible.
> 
> Signed-off-by: Mylène Josserand 

I'm not familiar with how Allwinner platforms are handled in U-Boot,
but I have a question for the following patches:

  [PATCH 07/10] sunxi: Convert CONFIG_RGMII to Kconfig
  [PATCH 08/10] sunxi: Convert CONFIG_SATAPWR to Kconfig
  [PATCH 09/10] sunxi: Convert CONFIG_MACPWR to Kconfig
  [PATCH 10/10] sunxi: Convert CONS_INDEX to Kconfig

To me, all these details are hardware specific details that are given
in the Device Tree. For example, RGMII is given in:

 {
pinctrl-names = "default";
pinctrl-0 = <_pins_rgmii_a>;
phy = <>;
phy-mode = "rgmii";
status = "okay";

phy1: ethernet-phy@1 {
reg = <1>;
};
};

The SATA power GPIO is given by:

_ahci_5v {
pinctrl-0 = <_pwr_pin_olinuxinolime>;
gpio = < 2 3 GPIO_ACTIVE_HIGH>;
status = "okay";
};

The MAC power GPIO is given by:

reg_gmac_3v3: gmac-3v3 {
compatible = "regulator-fixed";
pinctrl-names = "default";
pinctrl-0 = <_power_pin_bananapi>;
regulator-name = "gmac-3v3";
regulator-min-microvolt = <330>;
regulator-max-microvolt = <330>;
startup-delay-us = <10>;
enable-active-high;
gpio = < 7 23 GPIO_ACTIVE_HIGH>;
};

So it seems weird to encode it in the defconfig if the information is
already available in the Device Tree.

In addition, the option names are very generic: CONFIG_SATAPWR,
CONFIG_MACPWR, even though they are completely sunxi specific. So if
those options really need to exist, should be they named
CONFIG_SUNXI_SATAPWR, CONFIG_SUNXI_MACPWR ?

Thanks,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH 07/10] sunxi: Convert CONFIG_RGMII to Kconfig

[Mylène Josserand]

Convert CONFIG_RGMII to Kconfig. Thanks to that, it is possible to
update defconfig files of SYS_EXTRA_OPTIONS accordingly and
remove it when it is possible.

Signed-off-by: Mylène Josserand 
---
 board/sunxi/Kconfig| 7 +++
 configs/A20-OLinuXino-Lime2_defconfig  | 3 ++-
 configs/A20-Olimex-SOM-EVB_defconfig   | 3 ++-
 configs/Bananapi_defconfig | 3 ++-
 configs/Bananapro_defconfig| 3 ++-
 configs/Colombus_defconfig | 2 +-
 configs/Cubietruck_defconfig   | 3 ++-
 configs/Hummingbird_A31_defconfig  | 2 +-
 configs/Lamobo_R1_defconfig| 3 ++-
 configs/Linksprite_pcDuino3_Nano_defconfig | 3 ++-
 configs/Orangepi_defconfig | 3 ++-
 configs/Orangepi_mini_defconfig| 3 ++-
 configs/Sinovoip_BPI_M2_defconfig  | 2 +-
 configs/Wits_Pro_A20_DKT_defconfig | 2 +-
 configs/mixtile_loftq_defconfig| 3 ++-
 15 files changed, 31 insertions(+), 14 deletions(-)

diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig
index ff11b98ddb..a51066f7c6 100644
--- a/board/sunxi/Kconfig
+++ b/board/sunxi/Kconfig
@@ -385,6 +385,13 @@ config INITIAL_USB_SCAN_DELAY
option to a non 0 value to add an extra delay before the first usb
bus scan.
 
+config RGMII
+   bool "Enable RGMII"
+   default n
+   ---help---
+   Enable the support of the Reduced Gigabit Media-Independent
+   Interface (RGMII).
+
 config USB0_VBUS_PIN
string "Vbus enable pin for usb0 (otg)"
default ""
diff --git a/configs/A20-OLinuXino-Lime2_defconfig 
b/configs/A20-OLinuXino-Lime2_defconfig
index 47a876082a..40468f1394 100644
--- a/configs/A20-OLinuXino-Lime2_defconfig
+++ b/configs/A20-OLinuXino-Lime2_defconfig
@@ -4,13 +4,14 @@ CONFIG_SPL_I2C_SUPPORT=y
 CONFIG_MACH_SUN7I=y
 CONFIG_DRAM_CLK=384
 CONFIG_MMC0_CD_PIN="PH1"
+CONFIG_RGMII=y
 CONFIG_USB0_VBUS_PIN="PC17"
 CONFIG_USB0_VBUS_DET="PH5"
 CONFIG_SUNXI_GMAC=y
 CONFIG_DEFAULT_DEVICE_TREE="sun7i-a20-olinuxino-lime2"
 CONFIG_AHCI=y
 # CONFIG_SYS_MALLOC_CLEAR_ON_INIT is not set
-CONFIG_SYS_EXTRA_OPTIONS="RGMII,SATAPWR=SUNXI_GPC(3)"
+CONFIG_SYS_EXTRA_OPTIONS="SATAPWR=SUNXI_GPC(3)"
 CONFIG_SPL=y
 # CONFIG_CMD_IMLS is not set
 # CONFIG_CMD_FLASH is not set
diff --git a/configs/A20-Olimex-SOM-EVB_defconfig 
b/configs/A20-Olimex-SOM-EVB_defconfig
index dd7b1ac3e9..03f99809ef 100644
--- a/configs/A20-Olimex-SOM-EVB_defconfig
+++ b/configs/A20-Olimex-SOM-EVB_defconfig
@@ -7,13 +7,14 @@ CONFIG_MMC0_CD_PIN="PH1"
 CONFIG_MMC3_CD_PIN="PH0"
 CONFIG_MMC3_PINS="PH"
 CONFIG_MMC_SUNXI_SLOT_EXTRA=3
+CONFIG_RGMII=y
 CONFIG_USB0_VBUS_PIN="PB9"
 CONFIG_USB0_VBUS_DET="PH5"
 CONFIG_SUNXI_GMAC=y
 CONFIG_DEFAULT_DEVICE_TREE="sun7i-a20-olimex-som-evb"
 CONFIG_AHCI=y
 # CONFIG_SYS_MALLOC_CLEAR_ON_INIT is not set
-CONFIG_SYS_EXTRA_OPTIONS="RGMII,SATAPWR=SUNXI_GPC(3)"
+CONFIG_SYS_EXTRA_OPTIONS="SATAPWR=SUNXI_GPC(3)"
 CONFIG_SPL=y
 # CONFIG_CMD_IMLS is not set
 # CONFIG_CMD_FLASH is not set
diff --git a/configs/Bananapi_defconfig b/configs/Bananapi_defconfig
index c6a9e88351..8751875d03 100644
--- a/configs/Bananapi_defconfig
+++ b/configs/Bananapi_defconfig
@@ -3,13 +3,14 @@ CONFIG_ARCH_SUNXI=y
 CONFIG_SPL_I2C_SUPPORT=y
 CONFIG_MACH_SUN7I=y
 CONFIG_DRAM_CLK=432
+CONFIG_RGMII=y
 CONFIG_VIDEO_COMPOSITE=y
 CONFIG_SUNXI_GMAC=y
 CONFIG_GMAC_TX_DELAY=3
 CONFIG_DEFAULT_DEVICE_TREE="sun7i-a20-bananapi"
 CONFIG_AHCI=y
 # CONFIG_SYS_MALLOC_CLEAR_ON_INIT is not set
-CONFIG_SYS_EXTRA_OPTIONS="RGMII,MACPWR=SUNXI_GPH(23)"
+CONFIG_SYS_EXTRA_OPTIONS="MACPWR=SUNXI_GPH(23)"
 CONFIG_SPL=y
 # CONFIG_CMD_IMLS is not set
 # CONFIG_CMD_FLASH is not set
diff --git a/configs/Bananapro_defconfig b/configs/Bananapro_defconfig
index 596eb2dd7a..e70dbb8730 100644
--- a/configs/Bananapro_defconfig
+++ b/configs/Bananapro_defconfig
@@ -3,6 +3,7 @@ CONFIG_ARCH_SUNXI=y
 CONFIG_SPL_I2C_SUPPORT=y
 CONFIG_MACH_SUN7I=y
 CONFIG_DRAM_CLK=432
+CONFIG_RGMII=y
 CONFIG_USB1_VBUS_PIN="PH0"
 CONFIG_USB2_VBUS_PIN="PH1"
 CONFIG_VIDEO_COMPOSITE=y
@@ -11,7 +12,7 @@ CONFIG_GMAC_TX_DELAY=3
 CONFIG_DEFAULT_DEVICE_TREE="sun7i-a20-bananapro"
 CONFIG_AHCI=y
 # CONFIG_SYS_MALLOC_CLEAR_ON_INIT is not set
-CONFIG_SYS_EXTRA_OPTIONS="RGMII,MACPWR=SUNXI_GPH(23)"
+CONFIG_SYS_EXTRA_OPTIONS="MACPWR=SUNXI_GPH(23)"
 CONFIG_SPL=y
 # CONFIG_CMD_IMLS is not set
 # CONFIG_CMD_FLASH is not set
diff --git a/configs/Colombus_defconfig b/configs/Colombus_defconfig
index f9919558d4..6055a0d319 100644
--- a/configs/Colombus_defconfig
+++ b/configs/Colombus_defconfig
@@ -3,6 +3,7 @@ CONFIG_ARCH_SUNXI=y
 CONFIG_MACH_SUN6I=y
 CONFIG_DRAM_CLK=240
 CONFIG_DRAM_ZQ=251
+CONFIG_RGMII=y
 CONFIG_USB1_VBUS_PIN=""
 CONFIG_I2C0_ENABLE=y
 CONFIG_AXP_GPIO=y
@@ -17,7 +18,6 @@ CONFIG_VIDEO_LCD_PANEL_EDP_4_LANE_1620M_VIA_ANX9804=y
 CONFIG_SUNXI_GMAC=y
 CONFIG_DEFAULT_DEVICE_TREE="sun6i-a31-colombus"
 # CONFIG_SYS_MALLOC_CLEAR_ON_INIT is not set
-CONFIG_SYS_EXTRA_OPTIONS="RGMII"
 

Re: [U-Boot] [PATCH 07/10] sunxi: Convert CONFIG_RGMII to Kconfig

[Maxime Ripard]

Hi,

On Thu, Feb 23, 2017 at 04:44:16PM +0100, Mylène Josserand wrote:
> Convert CONFIG_RGMII to Kconfig. Thanks to that, it is possible to
> update defconfig files of SYS_EXTRA_OPTIONS accordingly and
> remove it when it is possible.
> 
> Signed-off-by: Mylène Josserand 
> ---
>  board/sunxi/Kconfig| 7 +++
>  configs/A20-OLinuXino-Lime2_defconfig  | 3 ++-
>  configs/A20-Olimex-SOM-EVB_defconfig   | 3 ++-
>  configs/Bananapi_defconfig | 3 ++-
>  configs/Bananapro_defconfig| 3 ++-
>  configs/Colombus_defconfig | 2 +-
>  configs/Cubietruck_defconfig   | 3 ++-
>  configs/Hummingbird_A31_defconfig  | 2 +-
>  configs/Lamobo_R1_defconfig| 3 ++-
>  configs/Linksprite_pcDuino3_Nano_defconfig | 3 ++-
>  configs/Orangepi_defconfig | 3 ++-
>  configs/Orangepi_mini_defconfig| 3 ++-
>  configs/Sinovoip_BPI_M2_defconfig  | 2 +-
>  configs/Wits_Pro_A20_DKT_defconfig | 2 +-
>  configs/mixtile_loftq_defconfig| 3 ++-
>  15 files changed, 31 insertions(+), 14 deletions(-)
> 
> diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig
> index ff11b98ddb..a51066f7c6 100644
> --- a/board/sunxi/Kconfig
> +++ b/board/sunxi/Kconfig
> @@ -385,6 +385,13 @@ config INITIAL_USB_SCAN_DELAY
>   option to a non 0 value to add an extra delay before the first usb
>   bus scan.
>  
> +config RGMII
> + bool "Enable RGMII"

This should be in drivers/net too

> + default n

And this is already the default.

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com


signature.asc
Description: PGP signature
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot