[snip as the thread is getting long]
On 06/04/2018 01:21 AM, Maxime Ripard wrote:
On Fri, Jun 01, 2018 at 10:16:32AM -0700, Martin Kelly wrote:
On 06/01/2018 04:05 AM, Maxime Ripard wrote:
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.
I didn't know Kconfig can set different default values for each architecture like that; that does indeed solve the problem. However, I don't think it's a good idea to have sunxi use an alternate mechanism than the other boards.
To be clear, are you proposing a general config option that would apply to every board? In that case, the header logic would be in a global header rather than a board-specific one.
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