On 09/09/2013 11:35 AM, Stephen Warren wrote: > On 09/09/2013 12:03 PM, Mike Dunn wrote: >> On 09/09/2013 05:08 AM, Thierry Reding wrote: >>> On Sun, Sep 08, 2013 at 12:40:18PM -0700, Mike Dunn wrote: >>>> This patch adds device tree support to the pxa's pwm driver. Only an OF match > >>>> diff --git a/Documentation/devicetree/bindings/pwm/pxa-pwm.txt b/Documentation/devicetree/bindings/pwm/pxa-pwm.txt > >>>> +Marvell pwm controller >>>> + >>>> +Required properties: >>>> +- compatible: >>>> + for pxa25x, pxa27x, pxa168, pxa910: must be compatible = "marvell,pxa250-pwm"; >>>> +- reg: physical base address and length of the registers used by the pwm channel >>> >>> "pwm" -> "PWM". There are a few more occurrences that I haven't >>> explicitly pointed out. >>> >>>> + NB: One device instance must be created for each pwm that is used, so the >>>> + length covers only the register window for one pwm output, not that of the >>>> + entire pwm controller. Currently length is 0x10 for all supported devices. >>> >>> I'm not sure I like that very much. It seems a wrong representation of >>> the hardware for the sake of modifying a smaller number of lines in the >>> driver. >> >> I don't like it either, but this is an existing driver defect that will need to >> be corrected. To do so, I will need to to survey all the supported processors >> to determine how many pwm outputs is posessed by each. And there may be some >> confusion in that regard; the driver says "pxa25x" has one, but my pxa255 >> developer's manual makes no mention of a pwm. The pxa270 I am testing on has >> four, but the driver says "pxa27x" has two, so currently (using platform_data >> with existing driver) two devices are instantiated, each with two pwms. It >> seems that there are many variations within a single generation of the processor >> family, so to correctly identify the number of pwms, processor identification >> will have to be made on a finer granularity than the current "pxa25x", for >> example. I'm guessing that the hardware designers had this >> one-device-instance-per-pwm approach in mind when they made the decision to >> segregate the register sets of each pwm. I really hope that this sin can be >> forgiven :) > > The DT binding is an ABI, so it needs to correctly represent the HW. As > such, I'd say that if the binding is wrong, we need to fix it even if it > means a lot of work. > > That said, if each PWM truly is a *completely* independent block, with > non-overlapping register spaces, there's certainly an argument that it's > perfectly acceptable to represent each PWM as a separate DT node... Thanks Stephen. Yes, each pwm output has its own registers with no common control registers. The only commonality is that they all share a clock in the clock unit. But this is handled in the clock manager. Thanks, Mike -- 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
