On 07/11/2013 01:32 PM, Thierry Reding wrote: > On Thu, Jul 11, 2013 at 11:50:48AM -0600, Stephen Warren wrote: >> On 07/11/2013 09:36 AM, Thierry Reding wrote: >>> On Thu, Jul 11, 2013 at 04:37:48PM +0200, Laurent Pinchart >>> wrote: [...] >>>> diff --git >>>> a/Documentation/devicetree/bindings/pwm/atmel-tcb-pwm.txt >>>> b/Documentation/devicetree/bindings/pwm/atmel-tcb-pwm.txt >>>> index de0eaed..be09be4 100644 --- >>>> a/Documentation/devicetree/bindings/pwm/atmel-tcb-pwm.txt >>>> +++ b/Documentation/devicetree/bindings/pwm/atmel-tcb-pwm.txt >>>> @@ -4,9 +4,9 @@ Required properties: - compatible: should be >>>> "atmel,tcb-pwm" - #pwm-cells: Should be 3. The first cell >>>> specifies the per-chip index of the PWM to use, the second >>>> cell is the period in nanoseconds and - bit 0 in the third >>>> cell is used to encode the polarity of PWM output. - Set bit >>>> 0 of the third cell in PWM specifier to 1 for inverse >>>> polarity & - set to 0 for normal polarity. + the third cell >>>> is used to encode the polarity of PWM output. Set the + >>>> PWM_POLARITY_NORMAL flag for normal polarity or the >>>> PWM_POLARITY_INVERSED + flag for inverted polarity. PWM >>>> flags are defined in <dt-bindings/pwm/pwm.h>. - tc-block: The >>>> Timer Counter block to use as a PWM chip. >>>> >>>> Example: >>> >>> I'd prefer for the original text to stay in place and the >>> reference to the dt-bindings/pwm/pwm.h file to go below that >>> block. >> >> I disagree here. The whole point of creating header files for >> the constants in binding definitions was so that you wouldn't >> have to duplicate all the values into the binding definitions. >> Rather, you'd simply say "see <dt-bindings/xxx.h>". > > But that's not something that this patch solves. Well, if the comments I made on the patch re: that <linux/pwm.h> should simply #include <dt-bindings/pwm/pwm.h> instead of duplicating the constants, then yet this patch will solve that. There will be a single place where the constants are defined. > And it could be solved even in the absence of the header file > defining the symbolic constants. If all the standard flags that > dt-bindings/pwm/pwm.txt now specifies were to be listed in pwm.txt > (they actually are) then referring to that document as the > canonical source works equally well. If that's all the happens, then there will still be duplication between pwm.txt and <linux/pwm.h>. > If we can take both of the above for granted, then sure, let's > refer to the header from within the generic pwm.txt file and add a > reference to that in bindings for drivers that use the standard > flags. > >>> Another issue might be that people without access to recent >>> versions of DTC won't be able to use the new #include feature, >>> so keeping the documentation backwards compatible seems like a >>> good idea. >> >> The dtc source tree is duplicated into the kernel source tree, so >> that isn't an issue for now. >> >> Besides, the dtc version is an entirely unrelated issue to how >> the documentation is written. > > Well, not really. If the documentation specifies the binding in a > way that the DTC can't handle that's still a problem. People will > end up with a DTS that their DTC can't compile. I guess that can be > resolved by adding a note to the upstream device tree repository > about the minimum required version of DTC. Yes, the separate repository would obviously require a version of dtc that's able to compile the files there; i.e. a version equivalent to what's already in the kernel tree (upstream 1.4.0 specifically). Again, right now, all of the binding docs, the *.dts files, and the dtc required to use them are part of the kernel; a single package, so there's no scope for issues re: using dtc features that aren't supported. If those components get separated later, obviously there will be a requirement to install a specific version of dtc to use with the separated *.dts and binding files. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
