From mboxrd@z Thu Jan 1 00:00:00 1970 From: swarren@wwwdotorg.org (Stephen Warren) Date: Fri, 12 Jul 2013 08:42:41 -0600 Subject: [PATCH 2/2] pwm: Add PWM polarity flag macros for DT In-Reply-To: <2172485.AZTjvkLGcM@avalon> References: <1373553468-6564-1-git-send-email-laurent.pinchart+renesas@ideasonboard.com> <20130711193245.GB17212@dhcp-172-17-186-34.nvidia.com> <51DF1054.9000207@wwwdotorg.org> <2172485.AZTjvkLGcM@avalon> Message-ID: <51E015E1.2090906@wwwdotorg.org> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org 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 . - 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 ". >>> >>> But that's not something that this patch solves. >> >> Well, if the comments I made on the patch re: that should >> simply #include 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 . > > 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.