On Wed, Feb 24, 2016 at 6:22 PM, Masahiro Yamada <yamada.masah...@socionext.com> wrote: > Hi Olof, > > > 2016-02-25 9:26 GMT+09:00 Olof Johansson <o...@lixom.net>: >> Hi, >> >> On Tue, Feb 16, 2016 at 11:15:04AM +0900, Masahiro Yamada wrote: >> >>> diff --git a/arch/arm/mach-uniphier/platsmp.c >>> b/arch/arm/mach-uniphier/platsmp.c >>> index e1cfc1d..b53a8d9 100644 >>> --- a/arch/arm/mach-uniphier/platsmp.c >>> +++ b/arch/arm/mach-uniphier/platsmp.c >>> @@ -30,7 +30,7 @@ >>> * The secondary CPUs check this register from the boot ROM for the jump >>> * destination. After that, it can be reused as a scratch register. >>> */ >>> -#define UNIPHIER_SBC_ROM_BOOT_RSV2 0x1208 >>> +#define UNIPHIER_SMPCTRL_ROM_BOOT_RSV2 0x208 >>> >>> static void __iomem *uniphier_smp_rom_boot_rsv2; >>> static unsigned int uniphier_smp_max_cpus; >>> @@ -98,15 +98,14 @@ static int __init >>> uniphier_smp_prepare_trampoline(unsigned int max_cpus) >>> phys_addr_t rom_rsv2_phys; >>> int ret; >>> >>> - np = of_find_compatible_node(NULL, NULL, >>> - "socionext,uniphier-system-bus-controller"); >>> - ret = of_address_to_resource(np, 1, &res); >>> + np = of_find_compatible_node(NULL, NULL, >>> "socionext,uniphier-smpctrl"); >>> + ret = of_address_to_resource(np, 0, &res); >>> if (ret) { >>> - pr_err("failed to get resource of system-bus-controller\n"); >>> + pr_err("failed to get resource of uniphier-smpctrl\n"); >>> return ret; >>> } >>> >>> - rom_rsv2_phys = res.start + UNIPHIER_SBC_ROM_BOOT_RSV2; >>> + rom_rsv2_phys = res.start + UNIPHIER_SMPCTRL_ROM_BOOT_RSV2; >>> >>> ret = uniphier_smp_copy_trampoline(rom_rsv2_phys); >>> if (ret) >> >> The previous binding has already been released. You can update, but your >> driver >> should be able to handle the previous binding. >> >> So, you still need to keep the old code around. >> >> This has the benefit of breaking the dependency between the code change and >> the >> DT change, so you no longer have to change your platform code at the same >> time >> as the DT to avoid regressions. >> >> >> Please adjust and resend. I'll hold off applying the series until then, so we >> don't have a partially applied series. > > > > How long do I have to keep the support for the old binding?
You know your platform best -- how many users do you think you have out there that might have built DTS files based on the old binding? If there's a good chance there are none, or if you're in good contact with them and can ask them to update, then you can be more flexible. > [1] > Everyone makes mistakes. > The constraint for the DT-binding is really really painful. > > This is how it happened. > > At first, I implemented uniphier-system-bus.c based on the old binding. > Then, during the review, Mark suggested me to change the driver design: > http://lists.infradead.org/pipermail/linux-arm-kernel/2015-November/387938.html > > I followed his suggestion, but I needed to changed the DT-binding as well. > Before that time, the DT and other support code for UniPhier had been > partially merged > in the mainline. So, in the current tree exist two bindings that are > not compatible to > each other. This situation is really a mess. > I want to clean up the code as soon as possible. Yeah, I understand that it's hard to come up with perfect bindings from day one, and that's why we sometimes have to play by ear. It's not a bad idea to get practice on how to solve it -- in this case it wouldn't really bad that bad. If you use variables to hold the base addresses, and get them from either binding, you'll only special-case during probe and not anywhere else in the driver. The general idea of decoupling DT changes from code changes is also a good habit. > [2] > For now, DT is mostly developed in the kernel tree in practice, > while DT is not theoretically only for Linux. > Everybody (at least every user of UniPhier SoCs) uses the combination > of a DTB and a kernel image > generated from the same Linux tree. > I see no reason to use a new DTB + an old kernel image, or vice versa. We're not aiming to support new DTB + old kernel image. The main problem is if someone has a product DTB that's not yet merged, and you change the binding, then their DTB might no longer work. It's not a huge deal, and for most changes it's fairly harmless, but the general principle still applies. As I said earlier, you know the users of your platform the best (I hope :), so you'll have the best feel for whether this is a breakage they will hurt from or not. > [3] > This binding is UniPhier-specific. No impact on other SoC vendors. > Everything is under my control. > > > > For now, I will prepare the logic to support the old binding, > but for the reasons above, please let me drop the support for the old > one some time later. Yeah, I'm perfectly fine with not keeping it for a long time. For example, feel free to remove it next release if you think that will work for your downstream users. -Olof