On Fri, Jun 01, 2018 at 10:16:32AM -0700, Martin Kelly wrote:
On 06/01/2018 04:05 AM, Maxime Ripard wrote:
On Thu, May 31, 2018 at 05:59:16PM -0700, Martin Kelly wrote:
Currently, sunxi-common.h ignores CONFIG_DEFAULT_FDT_FILE and assumes the kernel fdtfile and the u-boot devicetree names are the same. Although this is typically the case, sometimes you might want to customize one of these differently, so it's useful to allow them to be different.
Fix this to use only CONFIG_DEFAULT_FDT_FILE as other boards do, and set CONFIG_DEFAULT_FDT_FILE in board files to be the same as the current fdtfile= setting.
Signed-off-by: Martin Kelly mkelly@xevo.com
I'd really prefer not to do that. There's going to be a long trail of defconfig that will be sent at pretty much the same time that are not going to have it and that will go unnotice for some time.
And there's obviously the useless churn in all the defconfig that are not in the case where it makes sense to override DEFAULT_FDT_FILE.
Why not create a Kconfig boolean that allows to select whether or not you want DEFAULT_FDT_FILE to be DEFAULT_DEVICE_TREE + .dtb or a custom value.
I can see the issues with new defconfigs, but I'm not sure if it will really be that bad. If we apply this patch against sunxi master, then shouldn't new patches get tested and rebased against it? In that case, if they have not set DEFAULT_FDT_FILE, it will default to "", the boards won't boot, and the mistake must be fixed prior to merging.
Unless one has tested it with a version prior to your patch, and sends it. Not a lot of people are testing with the next branch in the various trees.
Alternatively if we add the Kconfig boolean, we need to worry about what happens when people have DEFAULT_FDT_FILE set already. I guess we would need to default the new Kconfig boolean to be custom in order to keep those configs from breaking. But if we do that, sunxi will break by default (since sunxi configs don't have the value set).
What would you suggest the default value of the new boolean to be?
config DEFAULT_FDT_FILE_USE_DEFAULT_DEVICE_TREE bool "whatever" default y if ARCH_ROCKCHIP default y if ARCH_SUNXI
and in the headers
#ifdef CONFIG_DEFAULT_FDT_FILE_USE_DEFAULT_DEVICE_TREE #define CONFIG_DEFAULT_FDT_FILE CONFIG_DEFAULT_DEVICE_TREE ".dtb" #endif
And that's done.
Personally, though I think it could be a bit painful through the next merge window, I think fixing this to operate the same way as the rest of the boards is the right long-term decision.
So far, I've seen two platforms using this, the two representing a significant part of the total defconfig number, using the exact same value for both. Surely that means there's room for improvement.
Maxime