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

[Ezequiel Garcia]

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.
-- 
Ezequiel García, VanguardiaSur
www.vanguardiasur.com.ar