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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 1 Jul 2021 11:38:47 -0400
From:   Sean Anderson <sean.anderson@...o.com>
To:     Michal Simek <michal.simek@...inx.com>, linux-pwm@...r.kernel.org,
        devicetree@...r.kernel.org,
        Uwe Kleine-König <u.kleine-koenig@...gutronix.de>,
        Sascha Hauer <s.hauer@...gutronix.de>
Cc:     linux-kernel@...r.kernel.org,
        Alvaro Gamez <alvaro.gamez@...ent.com>,
        linux-arm-kernel@...ts.infradead.org, Rob Herring <robh@...nel.org>
Subject: Re: [PATCH v4 1/3] dt-bindings: pwm: Add Xilinx AXI Timer



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@...o.com>
 >>> ---
 >>>
 >>> 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@...o.com>
 >>> +
 >>> +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