Re: [PATCH v4 1/3] dt-bindings: pwm: Add Xilinx AXI Timer

[Sean Anderson]

On 6/30/21 9:58 AM, Michal Simek wrote:
>
>
> On 6/30/21 3:47 PM, Michal Simek wrote:
>>
>>
>> On 5/28/21 11:45 PM, Sean Anderson wrote:
>>> This adds a binding for the Xilinx LogiCORE IP AXI Timer. This device is
>>> a "soft" block, so it has many parameters which would not be
>>> configurable in most hardware. This binding is usually automatically
>>> generated by Xilinx's tools, so the names and values of some properties
>>> must be kept as they are. Replacement properties have been provided for
>>> new device trees.
>>>
>>> Because we need to init timer devices so early in boot, the easiest way
>>> to configure things is to use a device tree property. For the moment
>>> this is 'xlnx,pwm', but this could be extended/renamed/etc. in the
>>> future if these is a need for a generic property.
>>>
>>> Signed-off-by: Sean Anderson <sean.anderson@xxxxxxxx>
>>> ---
>>>
>>> Changes in v4:
>>> - Remove references to generate polarity so this can get merged
>>> - Predicate PWM driver on the presence of #pwm-cells
>>> - Make some properties optional for clocksource drivers
>>>
>>> Changes in v3:
>>> - Mark all boolean-as-int properties as deprecated
>>> - Add xlnx,pwm and xlnx,gen?-active-low properties.
>>> - Make newer replacement properties mutually-exclusive with what they
>>>   replace
>>> - Add an example with non-deprecated properties only.
>>>
>>> Changes in v2:
>>> - Use 32-bit addresses for example binding
>>>
>>>  .../bindings/pwm/xlnx,axi-timer.yaml          | 85 +++++++++++++++++++
>>>  1 file changed, 85 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/pwm/xlnx,axi-timer.yaml
>>>
>>> diff --git a/Documentation/devicetree/bindings/pwm/xlnx,axi-timer.yaml b/Documentation/devicetree/bindings/pwm/xlnx,axi-timer.yaml
>>> new file mode 100644
>>> index 000000000000..48a280f96e63
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/pwm/xlnx,axi-timer.yaml
>>
>> I don't think this is the right location for this.
>>
>> I have done some grepping and I think this should be done in a different
>> way. I pretty much like solution around "ti,omap3430-timer" which is
>> calling dmtimer_systimer_select_best() and later dmtimer_is_preferred()
>> which in this case would allow us to get rid of cases which are not
>> suitable for clocksource and clockevent.
>>
>> And there is drivers/pwm/pwm-omap-dmtimer.c which has link to timer
>> which is providing functions for it's functionality.
>>
>> I have also looked at
>> Documentation/devicetree/bindings/timer/nxp,tpm-timer.yaml which is also
>> the same device.
>>
>> And sort of curious if you look at
>> https://www.xilinx.com/support/documentation/ip_documentation/axi_timer/v2_0/pg079-axi-timer.pdf
>> ( Figure 1-1)
>> that PWM is taking input from generate out 0 and generate out 1 which is
>> maybe can be modeled is any output and pwm driver can register inputs
>> for pwm driver.
>>
>>
>>> @@ -0,0 +1,85 @@
>>> +# SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/pwm/xlnx,axi-timer.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: Xilinx LogiCORE IP AXI Timer Device Tree Binding
>>> +
>>> +maintainers:
>>> +  - Sean Anderson <sean.anderson@xxxxxxxx>
>>> +
>>> +properties:
>>> +  compatible:
>>> +    oneOf:
>>> +      - items:
>>> +         - const: xlnx,axi-timer-2.0
>
> I am not quite sure if make sense also to list 2.0 version.
> There were likely also 1.0 version which is compatible with origin xps
> version which IIRC was PLB based. And the same driver was using in past
> with OPB bus.

It's required to list all compatible properties which may be used in a
binding. And AFAIK it is good practice to add a new compatible string
for new releases of an IP, in case incompatibilities are discovered.

--Sean