[PATCH 2/2] DT: pwm: Add NXP LPC18xx PWM/SCT binding documentation

[Ariel D'Alessandro]

Ezequiel, Joachim:

El 31/07/15 a las 15:09, Ezequiel Garcia escibió:
> Joachim,
> 
> On 30 July 2015 at 20:30, Joachim  Eastwood <manabian at gmail.com> wrote:
>> On 29 July 2015 at 15:47, Ezequiel Garcia <ezequiel at vanguardiasur.com.ar> wrote:
>>> +devicetree guys
>>>
>>> On 28 July 2015 at 20:22, Joachim  Eastwood <manabian at gmail.com> wrote:
>>>> On 29 July 2015 at 00:45, Ezequiel Garcia <ezequiel at vanguardiasur.com.ar> wrote:
>>>>> On 28 July 2015 at 19:37, Joachim  Eastwood <manabian at gmail.com> wrote:
>>>>>> On 27 July 2015 at 06:45, Ariel D'Alessandro <ariel at vanguardiasur.com.ar> wrote:
>>>>>>> Add the devicetree binding document for NXP LPC18xx PWM/SCT.
>>>>>>>
>>>>>>> Signed-off-by: Ariel D'Alessandro <ariel at vanguardiasur.com.ar>
>>>>>>> ---
>>>>>>>  .../devicetree/bindings/pwm/lpc1850-pwm.txt          | 20 ++++++++++++++++++++
>>>>>>>  1 file changed, 20 insertions(+)
>>>>>>>  create mode 100644 Documentation/devicetree/bindings/pwm/lpc1850-pwm.txt
>>>>>>>
>>>>>>> diff --git a/Documentation/devicetree/bindings/pwm/lpc1850-pwm.txt b/Documentation/devicetree/bindings/pwm/lpc1850-pwm.txt
>>>>>>> new file mode 100644
>>>>>>> index 0000000..3055429
>>>>>>> --- /dev/null
>>>>>>> +++ b/Documentation/devicetree/bindings/pwm/lpc1850-pwm.txt
>>>>>>> @@ -0,0 +1,20 @@
>>>>>>> +* NXP LPC18xx Pulse Width Modulator driver
>>>>>>> +
>>>>>>> +Required properties:
>>>>>>> +  - compatible: Should be "nxp,lpc1850-pwm"
>>>>>>> +  - reg: Should contain physical base address and length of pwm registers.
>>>>>>> +  - clocks: Must contain an entry for each entry in clock-names.
>>>>>>> +    See ../clock/clock-bindings.txt for details.
>>>>>>> +  - clock-names: Must include the following entries.
>>>>>>> +    - pwm: PWM operating clock.
>>>>>>> +  - #pwm-cells: Should be 3. See pwm.txt in this directory for the description
>>>>>>> +    of the cells format.
>>>>>>> +
>>>>>>> +Example:
>>>>>>> +  pwm: pwm at 40000000 {
>>>>>>> +    compatible = "nxp,lpc1850-pwm";
>>>>>>
>>>>>> I would prefer "nxp,lpc1850-sct" for a couple of reasons. SCT is the
>>>>>> name of hardware block as described in the user manual and while PWM
>>>>>> is the most obvious usage for this block on Linux, the hardware is not
>>>>>> limited to just doing that. So as a bit of future proofing if someone
>>>>>> wants to use this block for more than PWM I would prefer SCT.
>>>>>>
>>>>>
>>>>> Shouldn't we use something like "nxp,lpc1850-sct-pwm"?
>>>>>
>>>>> Sounds like the word PWM should be in the compatible as it describes
>>>>> not only the device, but the device used in a certain way.
>>>>>
>>>>> Otherwise, how would we use "nxp,lpc1850-sct" to distinguish a PWM from
>>>>> e.g. a clocksource/clockevents (we can also use SCT for that)?
>>>>
>>>> I not sure how to best handle dt bindings with such flexible hardware blocks.
>>>>
>>>> But are you suggestion to have multiple drivers for the same hw block?
>>>> ie.: "nxp,lpc1850-sct-pwm" for PWM and for example
>>>> "nxp,lpc1850-sct-clocksource" for clocksource and then use/enable one
>>>> or the other.
>>>>
>>>
>>> Yup, I'm suggesting just that. If the hardware is capable, I can't see
>>> why we can't have different drivers for it.
>>
>> Well, I think it would be nice if we could have one driver per hw
>> block. But then you need a way to select whether you want to use the
>> counter in the SCT block for PWM or, for example, clocksource. Sort of
>> mode selection I guess.
>>
>> But then again if someone has some very custom and application
>> specific usage for the SCT block that maybe better served by a
>> standalone driver anyway.
>>
> 
> Technically speaking, I believe we can use "nxp,lpc1850-sct" to expose
> both a PWM interface and a clocksource in a single driver. Not too
> clean, though.
> 
> Alternatively, we should be able to have a multifunction device (MFD) to mux
> the SCT resources to be consumed by different kind of drivers.
> 
> Given there's no counter API to feed SCT, and given we use the other
> timer hw block to feed clocksource and clockevents for the LPC18/43xx,
> I think currently the simplest choice is to have "nxp,lpc1850-sct-pwm"
> describing this PWM SCT driver.
> 
> If we later want to change completely the PWM SCT driver, we can do
> that without any bindings change.
> 
>> So for now I am fine with "nxp,lpc1850-sct-pwm". I shouldn't be too
>> hard to change the bindings if we see other users of this block, I
>> believe.
>>
> 
> Well, that would break DT backward compatibility, so it's not an ideal.
> Like I said, "nxp,lpc1850-sct-pwm" seems to be specific enough to
> describe this driver.
> 
> I'd like to know what Thierry thinks here.
> 

OK then. Until we have a better idea, I'll set the compatible string to
"nxp,lpc1850-sct-pwm".