On Wed, Sep 25, 2013 at 2:34 PM, Olof Johansson <olof@xxxxxxxxx> wrote: > On Wed, Sep 25, 2013 at 12:20 PM, Rob Herring <robherring2@xxxxxxxxx> wrote: >> On Wed, Sep 25, 2013 at 12:15 PM, Olof Johansson <olof@xxxxxxxxx> wrote: >>> On Fri, Sep 20, 2013 at 3:20 AM, Mark Rutland <mark.rutland@xxxxxxx> wrote: >>>> On Fri, Sep 20, 2013 at 05:36:51AM +0100, Olof Johansson wrote: >>>>> Hi, >>>> >>>> Hi Olof, >>>> >>>>> >>>>> Just a quick drive-by. Sorry, I don't know the history of previous >>>>> review cycles. >>>>> >>>>> >>>>> On Thu, Sep 19, 2013 at 2:24 PM, Rob Herring <robherring2@xxxxxxxxx> wrote: >>>>> >>>>> > Main node optional properties: >>>>> > >>>>> > - - cpu_suspend : Function ID for CPU_SUSPEND operation >>>>> > + - cpu_suspend[-<32|64] : Function ID for CPU_SUSPEND operation >>>>> > + >>>>> > + - cpu_off : Function ID for CPU_OFF operation >>>>> > + >>>>> > + - cpu_on[-<32|64] : Function ID for CPU_ON operation >>>>> > + >>>>> > + - affinity_info[-<32|64] : Function ID for AFFINITY_INFO operation >>>>> > >>>>> > - - cpu_off : Function ID for CPU_OFF operation >>>>> > + - migrate[-<32|64] : Function ID for MIGRATE operation >>>>> > >>>>> > - - cpu_on : Function ID for CPU_ON operation >>>>> > + - migrate_info_type : Function ID for MIGRATE_INFO_TYPE operation >>>>> > >>>>> > - - migrate : Function ID for MIGRATE operation >>>>> > + - migrate_info_up_cpu[-<32|64] : Function ID for MIGRATE_INFO_UP_CPU operation >>>>> > >>>>> > + - system_reset : Function ID for SYSTEM_RESET operation >>>>> > + >>>>> > + - system_off : Function ID for SYSTEM_OFF operation >>>>> >>>>> All of these should use dashes instead of underscores. >>>> >>>> I also don't like the underscores, but they're an unfortunate historical >>>> artifact that we can't get rid of (at least for cpu_on, cpu_off, and >>>> cpu_suspend). >>> >>> We need to keep those three old ones supported in the driver, but the >>> newer binding can use dashes instead, as long as the kernel also >>> handles the old binding. >>> >>>> Both Xen and kvmtool are already passing DTs to guests which have >>>> underscores in the property names, so it's both a boot ABI and a >>>> userspace ABI. >>> >>> Again, as mentioned on IRC: We've said that bindings are not yet >>> stable until we lock them down, so we will need to adjust accordingly. >>> Once we lock them down, they are stable and part of ABI. But we really >>> don't want to deal with the bad bindings that people came up with >>> without much review, and that's what the whole idea behind cleaning up >>> and then locking them down is all about. >> >> For some of us, bindings are already in production. Until we have some >> way to document stable vs. unstable, it is not up to anyone but the >> users of a binding to define the stability. If the users of a binding >> can tolerate the change, then it is unstable and can change. > > Ok, so why review bindings at all then? Just implement what you want, > ship it and force it on the rest of us for ever. Dealing with 2 versions of PSCI bindings rather than simply extending the existing binding will be more pain for the kernel, kvmtool, qemu and xen than any benefit of using a hyphen will provide. This binding was written by Will Deacon and acked by Arnd and Nicolas not to mention the follow-up patches to use it over the last 9 months. Hardly a bunch of newbies that don't understand DT. That binding had 3 postings. No one complained about the underscores then. It is precisely because it has been reviewed that I don't think we should be changing it. I don't see simply adding additional function ids as grounds for redoing the binding. The 32/64 bit calling convention handling may be a different discussion which is fine with me. Dealing with that was not my goal here. >> All this boils down to using '_' vs. '-', right? Come on. I'm all for >> consistency, but let's be practical. There are plenty of documented >> examples that use '_' starting with device_type. > > Next property I add will be called Device_Type then, if we're willing > to not give a shit about best practices any more. Thanks for setting > the direction and holding the bar high, Rob! That's fine, just mark it unstable and we can change it later. I'll sit quiet for 9 months first and then complain about it. I'll be waiting for your patch to change all the '_' to '-' in all the binding docs and dts files. Since everything is unstable, that shouldn't be a problem to change. :) Rob -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
