[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 <mylene.josserand at free-electrons.com>
> 
> 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:
> 
> &gmac {
>         pinctrl-names = "default";
>         pinctrl-0 = <&gmac_pins_rgmii_a>;
>         phy = <&phy1>;
>         phy-mode = "rgmii";
>         status = "okay";
> 
>         phy1: ethernet-phy at 1 {
>                 reg = <1>;
>         };
> };
> 
> The SATA power GPIO is given by:
> 
> &reg_ahci_5v {
>         pinctrl-0 = <&ahci_pwr_pin_olinuxinolime>;
>         gpio = <&pio 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 = <&gmac_power_pin_bananapi>;
>                 regulator-name = "gmac-3v3";
>                 regulator-min-microvolt = <3300000>;
>                 regulator-max-microvolt = <3300000>;
>                 startup-delay-us = <100000>;
>                 enable-active-high;
>                 gpio = <&pio 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170301/b7c21089/attachment.sig>