Hi Stephen, On Friday 12 July 2013 08:42:41 Stephen Warren wrote: > On 07/12/2013 05:01 AM, Laurent Pinchart wrote: > > 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. I wasn't referring to HW-specific flags, but rather to system-wide flags that might not be supported by all drivers. If we later add new system-wide flags I think the individual DT bindings should explicitly document which flags they support. -- Regards, Laurent Pinchart -- 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
