| Message ID | 55929566.50205@redhat.com |
|---|---|
| State | Not Applicable |
| Headers | show |
On Tue, 2015-06-30 at 15:11 +0200, Hans de Goede wrote: [...] > The latest patch-series seems to do the exact reverse. More knowledge is > being moved away from a central place and into defconfig files. As said > ARCH_SUNXI builds must always have CONFIG_DM* set, and yet now all sunxi > defconfig files need to set that explicitly. CONFIG_DM* seems like the sort of thing which ought to be select'd not default y'd (it's never really an user "option", it's something something else needs), but the recent change seems like a step in the wrong direction either way. Ian.
Hi, On 30-06-15 15:30, Ian Campbell wrote: > On Tue, 2015-06-30 at 15:11 +0200, Hans de Goede wrote: > [...] >> The latest patch-series seems to do the exact reverse. More knowledge is >> being moved away from a central place and into defconfig files. As said >> ARCH_SUNXI builds must always have CONFIG_DM* set, and yet now all sunxi >> defconfig files need to set that explicitly. > > CONFIG_DM* seems like the sort of thing which ought to be select'd not > default y'd ACK. The problem is that there are some other not entirely clean Kconfig bits un the u-boot tree which causes Kconfig to explain about circular deps last time I tried to select them. Still I agree we need to move to selecting these and fix any other issues this triggers. Regards, Hans
Hi Ian, On Tue, Jun 30, 2015 at 8:30 AM, Ian Campbell <ijc@hellion.org.uk> wrote: > On Tue, 2015-06-30 at 15:11 +0200, Hans de Goede wrote: > [...] >> The latest patch-series seems to do the exact reverse. More knowledge is >> being moved away from a central place and into defconfig files. As said >> ARCH_SUNXI builds must always have CONFIG_DM* set, and yet now all sunxi >> defconfig files need to set that explicitly. > > CONFIG_DM* seems like the sort of thing which ought to be select'd not > default y'd (it's never really an user "option", it's something > something else needs), but the recent change seems like a step in the > wrong direction either way. I agree that the DM configs should be selects where there is no other option (that will usually, maybe always, be the case with those, right?). I can see where the recent changes appear to be a step in the wrong direction, but I believe they are a step out of an unmanageable local minima. We simply have to get away from defining configs in arch or board Kconfigs. Unfortunately, the current mechanism for changing defaults requires this. I do not believe that we should do nothing further to improve the way we use Kconfig, but I do believe strongly that we cannot allow this approach to defaults to proliferate. For your specific case, sunxi defconfigs should be no different if you decide to "select" the DM options that always apply. Perhaps you didn't look at the next patch which moved all default commands into the Kconfig where those configs are actually defined? Cheers, -Joe
On Tue, Jun 30, 2015 at 10:48:20AM -0500, Joe Hershberger wrote: > Hi Ian, > > On Tue, Jun 30, 2015 at 8:30 AM, Ian Campbell <ijc@hellion.org.uk> wrote: > > On Tue, 2015-06-30 at 15:11 +0200, Hans de Goede wrote: > > [...] > >> The latest patch-series seems to do the exact reverse. More knowledge is > >> being moved away from a central place and into defconfig files. As said > >> ARCH_SUNXI builds must always have CONFIG_DM* set, and yet now all sunxi > >> defconfig files need to set that explicitly. > > > > CONFIG_DM* seems like the sort of thing which ought to be select'd not > > default y'd (it's never really an user "option", it's something > > something else needs), but the recent change seems like a step in the > > wrong direction either way. > > I agree that the DM configs should be selects where there is no other > option (that will usually, maybe always, be the case with those, > right?). > > I can see where the recent changes appear to be a step in the wrong > direction, but I believe they are a step out of an unmanageable local > minima. We simply have to get away from defining configs in arch or > board Kconfigs. Unfortunately, the current mechanism for changing > defaults requires this. Both the Kconfig and DM transition mean that we'll probably end up with some ugly bits for a while. But I want to take a poke around again and see if it can be a bit better now, today. > I do not believe that we should do nothing further to improve the way > we use Kconfig, but I do believe strongly that we cannot allow this > approach to defaults to proliferate. > > For your specific case, sunxi defconfigs should be no different if you > decide to "select" the DM options that always apply. Perhaps you > didn't look at the next patch which moved all default commands into > the Kconfig where those configs are actually defined? No, checkout say configs/A10-OLinuXino-Lime_defconfig and see all of the DM_* stuff that's in there. Hans' point is that it shouldn't be since they must be set. Having them correct in the config files (IMHO) helps, but Hans' point I believe (And I see the validity there, there's _60_ sunxi boards today and more every week practically) is that making each new board set this is wrong on many levels.
--- a/board/sunxi/Kconfig +++ b/board/sunxi/Kconfig @@ -566,25 +566,4 @@ config GMAC_TX_DELAY ---help--- Set the GMAC Transmit Clock Delay Chain value. -config SYS_MALLOC_CLEAR_ON_INIT - default n - -config NETDEVICES - default y - -config DM_ETH - default y - -config DM_SERIAL - default y - -config DM_USB - default y if !USB_MUSB_SUNXI - -config CMD_SETEXPR - default y - -config CMD_NET - default y - endif Which result in the following "matching" change for 60+ sunxi defconfig's : --- a/configs/A10-OLinuXino-Lime_defconfig +++ b/configs/A10-OLinuXino-Lime_defconfig @@ -5,5 +5,13 @@ CONFIG_DRAM_CLK=480 CONFIG_DRAM_EMR1=4 CONFIG_SYS_CLK_FREQ=912000000 CONFIG_DEFAULT_DEVICE_TREE="sun4i-a10-olinuxino-lime" +# CONFIG_SYS_MALLOC_CLEAR_ON_INIT is not set CONFIG_SPL=y CONFIG_SYS_EXTRA_OPTIONS="AXP209_POWER,SUNXI_EMAC,AHCI,SATAPWR=SUNXI_GPC(3),USB_EHCI" +CONFIG_CMD_SETEXPR=y +CONFIG_CMD_NET=y +CONFIG_DM_ETH=y +CONFIG_NETDEVICES=y +CONFIG_DM_SERIAL=y +CONFIG_USB=y +CONFIG_DM_USB=y