Hi Olof,
2016-02-25 16:20 GMT+09:00 Olof Johansson <[email protected]>: > On Wed, Feb 24, 2016 at 6:22 PM, Masahiro Yamada > <[email protected]> wrote: >> Hi Olof, >> >> >> 2016-02-25 9:26 GMT+09:00 Olof Johansson <[email protected]>: >>> 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. OK, I split the series into two. (DT and non-DT updates) Thanks. -- Best Regards Masahiro Yamada
