Re: [[PATCH v2] 1/9] dt-bindings: pwm: Document Synopsys DesignWare snps,pwm

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 27/07/2022 13:02, Krzysztof Kozlowski wrote:
On 27/07/2022 12:32, Ben Dooks wrote:
On 26/07/2022 12:05, Krzysztof Kozlowski wrote:
On 26/07/2022 12:12, Ben Dooks wrote:
On 26/07/2022 11:05, Krzysztof Kozlowski wrote:
On 25/07/2022 23:21, Ben Dooks wrote:
Add documentation for the bindings for Synopsys' DesignWare PWM block
as we will be adding DT/platform support to the Linux driver soon.

Signed-off-by: Ben Dooks <ben.dooks@xxxxxxxxxx>
--

This is not proper delimiter and causes the changelog to end up in commit.

Correct also wrong formatting of subject PATCH.

I realised that once sent and forgot the cover letter.
Maybe I'll try some more post covid recovery.

v2:
- fix #pwm-cells to be 3
- fix indentation and ordering issues
---
    .../devicetree/bindings/pwm/snps,pwm.yaml     | 40 +++++++++++++++++++
    1 file changed, 40 insertions(+)
    create mode 100644 Documentation/devicetree/bindings/pwm/snps,pwm.yaml

diff --git a/Documentation/devicetree/bindings/pwm/snps,pwm.yaml b/Documentation/devicetree/bindings/pwm/snps,pwm.yaml
new file mode 100644
index 000000000000..594085e5e26f
--- /dev/null
+++ b/Documentation/devicetree/bindings/pwm/snps,pwm.yaml
@@ -0,0 +1,40 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright (C) 2022 SiFive, Inc.
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pwm/snps,pwm.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Synopsys PWM controller
+
+maintainers:
+  - Ben Dooks <ben.dooks@xxxxxxxxxx>
+
+allOf:
+  - $ref: pwm.yaml#
+
+properties:
+  compatible:
+    const: snps,pwm

This is very generic compatible. I doubt that you cover here all
Synopsys PWM designs, past and future. You need a specific compatible.

   From what I can get from the documentation (2.13a) there hasn't been
a huge external interface change and what has been added is all part
of synthesis time options.

But you have some specific version, right? Usually these blocks are
versioned, so you must include it. I would even argue that such generic
compatible should not be used as fallback at all, because it is simply
to generic (PWM is not some model name but common acronym),

I suppose dw-apb-timers is the actual document name, but that's already
been used for the timer mode in a number of SoCs so probably isn't going
to be useful. dw-apb-timers-pwm might be a better prefix if snps,pwm is
not going to be acceptable. (Yes, the block can be built as either a
PWM or a generic interrupt generating timer at IP generation time)

The first thing I'd like to get sorted is should we rename this to
snps,dw-apb-timers-pwm so we can rename the file and the compatible
that goes with it.

As for the version numbers, we could have the -v.vv suffix for these
blocks, but the v2.xx log has 22 entries already and only one feature
for programming (which is also a configurable one so can't be just
enabled by default - it's the 0/100 mode flag in the control registers).

I'm not sure what the v1.xx timers had, but I don't have access to this
information and we're getting these documents as second-generation so I
am not sure if we can get a v1.xx at-all (I suspect this is also going
to have a number of revisions and about 1 useful register api change
which would be the "new mode" double counter method which we currently
rely on having being implicitly enabled by the IP builder (again this
feature is still something that can be configured on IP genaration))

But why would you need v1.xx documentation?

I believe the driver should cover a large part of the v1.xx cores
as well, we just don't have any documentation for these to verify
this.


Given the configurability of the core, the version numbers might be
usable at some point, but it does seem to be a lot of churn for what
currently can be described by one boolean for the 0/100 feature that
might-be available. Is there a way of saying the compatible string
can be dw-apb-timers-pwm-2.[0-9][0-9][a-z] ?

I don't understand why. Aren't you documenting here only v2.13a version?

The document as-such should cover everything I have a log for, we've not
had time to test the extension for 0or100% which was introduced in 2.11a
spec. The earliest history I have is 2.02d. I will go and see if I can
find someone who can go look for anything earlier.

As a note, it does look like all the v2.xx cores have the IP version
register in them so we can auto-detect the version from that, at least
for the DT/platform case.

Best regards,
Krzysztof




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux