On 07/12/2013 05:01 AM, Laurent Pinchart wrote: > Hi, > > On Thursday 11 July 2013 14:06:44 Stephen Warren wrote: >> 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. > > As explained in another reply, this would require replacing the enum with an > unsigned int. I can write a patch if we agree on this. > >>> 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>. > > I've explicitly mentioned the flags in individual DT bindings to ease adding > new flags in the future. At the moment the defined flags are either all > supported or not used at all by drivers. If we later add a new flag supported > by a subset of drivers only the driver bindings should list supported flags > for each driver. > > I'm fine with removing the explicit mentions of individual flags right now and > adding it back when needed if you think that's better. I think the values for any common system-wide flags should be defined once in some system-wide place, and the values for any HW-specific values should be defined only in the documentation for that specific HW. You could try and avoid conflicts by either: a) Allocating system-wide flags from bit 0 up, and HW-specific flags from bit 31 down. or: b) Using 1 cell for standard flags, and a separate cell for any HW-specific flags. Drivers can quite easily adapt to adding extra cells to #pwm-cells, thus making adding a HW-specific cell later backwards-compatible. -- 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
