diff mbox series

[v1,1/5] dt-bindings: pwm: Add Loongson PWM controller

Message ID edad2bb5b0045c633734c1499fb163c3c6776121.1711953223.git.zhoubinbin@loongson.cn
State Changes Requested, archived
Headers show
Series pwm: Introduce pwm driver for the Loongson family chips | expand

Checks

Context Check Description
robh/checkpatch success
robh/patch-applied success
robh/dtbs-check warning build log
robh/dt-meta-schema success

Commit Message

Binbin Zhou April 2, 2024, 7:58 a.m. UTC 
Add Loongson PWM controller binding with DT schema format using
json-schema.

Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
---
 .../devicetree/bindings/pwm/pwm-loongson.yaml | 64 +++++++++++++++++++
 MAINTAINERS                                   |  6 ++
 2 files changed, 70 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pwm/pwm-loongson.yaml

Comments

Rob Herring (Arm) April 2, 2024, 5:40 p.m. UTC | #1 
On Tue, Apr 02, 2024 at 03:58:38PM +0800, Binbin Zhou wrote:
> Add Loongson PWM controller binding with DT schema format using
> json-schema.
> 
> Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
> ---
>  .../devicetree/bindings/pwm/pwm-loongson.yaml | 64 +++++++++++++++++++

Filename should match compatible.

>  MAINTAINERS                                   |  6 ++
>  2 files changed, 70 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pwm/pwm-loongson.yaml
> 
> diff --git a/Documentation/devicetree/bindings/pwm/pwm-loongson.yaml b/Documentation/devicetree/bindings/pwm/pwm-loongson.yaml
> new file mode 100644
> index 000000000000..d25904468353
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pwm/pwm-loongson.yaml
> @@ -0,0 +1,64 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pwm/pwm-loongson.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Loongson PWM Controller
> +
> +maintainers:
> +  - Binbin Zhou <zhoubinbin@loongson.cn>
> +
> +description:
> +  It is the generic PWM framework driver for Loongson family.

That's describing the driver. Not really relevant to the binding.


> +  Each PWM has one pulse width output signal and one pulse input
> +  signal to be measured.
> +  It can be found on Loongson-2K series cpus and Loongson LS7A bridge chips.
> +
> +allOf:
> +  - $ref: pwm.yaml#
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - const: loongson,ls7a-pwm
> +      - items:
> +          - enum:
> +              - loongson,ls2k0500-pwm
> +              - loongson,ls2k1000-pwm
> +              - loongson,ls2k2000-pwm
> +          - const: loongson,ls7a-pwm
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 1
> +
> +  '#pwm-cells':
> +    const: 3

Please define what is in each cell. If there's only 2 signals, then the 
first cell defines the output or input (what value for which one?).

Really, the PWM binding is only for outputs, so is a cell even needed? I 
suppose we could use it for inputs too, but that's really "input 
capture" type operation that timers often have. I'll defer to the PWM 
maintainers...

> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - clocks
> +  - '#pwm-cells'

pwm.yaml makes this required already.

Rob
Binbin Zhou April 3, 2024, 2:37 a.m. UTC | #2 
Hi Rob:

Thanks for your reply.

On Tue, Apr 2, 2024 at 11:40 PM Rob Herring <robh@kernel.org> wrote:
>
> On Tue, Apr 02, 2024 at 03:58:38PM +0800, Binbin Zhou wrote:
> > Add Loongson PWM controller binding with DT schema format using
> > json-schema.
> >
> > Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
> > ---
> >  .../devicetree/bindings/pwm/pwm-loongson.yaml | 64 +++++++++++++++++++
>
> Filename should match compatible.

Emm... How about renaming it as loongson, pwm.yaml?

>
> >  MAINTAINERS                                   |  6 ++
> >  2 files changed, 70 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/pwm/pwm-loongson.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/pwm/pwm-loongson.yaml b/Documentation/devicetree/bindings/pwm/pwm-loongson.yaml
> > new file mode 100644
> > index 000000000000..d25904468353
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/pwm/pwm-loongson.yaml
> > @@ -0,0 +1,64 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/pwm/pwm-loongson.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Loongson PWM Controller
> > +
> > +maintainers:
> > +  - Binbin Zhou <zhoubinbin@loongson.cn>
> > +
> > +description:
> > +  It is the generic PWM framework driver for Loongson family.
>
> That's describing the driver. Not really relevant to the binding.
>
Ok ,I will rewrite this part.

>
> > +  Each PWM has one pulse width output signal and one pulse input
> > +  signal to be measured.
> > +  It can be found on Loongson-2K series cpus and Loongson LS7A bridge chips.
> > +
> > +allOf:
> > +  - $ref: pwm.yaml#
> > +
> > +properties:
> > +  compatible:
> > +    oneOf:
> > +      - const: loongson,ls7a-pwm
> > +      - items:
> > +          - enum:
> > +              - loongson,ls2k0500-pwm
> > +              - loongson,ls2k1000-pwm
> > +              - loongson,ls2k2000-pwm
> > +          - const: loongson,ls7a-pwm
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  interrupts:
> > +    maxItems: 1
> > +
> > +  clocks:
> > +    maxItems: 1
> > +
> > +  '#pwm-cells':
> > +    const: 3
>
> Please define what is in each cell. If there's only 2 signals, then the
> first cell defines the output or input (what value for which one?).
>
> Really, the PWM binding is only for outputs, so is a cell even needed? I
> suppose we could use it for inputs too, but that's really "input
> capture" type operation that timers often have. I'll defer to the PWM
> maintainers...

Ok, I will try to add some description about it.

If I understand correctly, the meaning of each cell in "#pwm-cells"is
determined.
The first cell specifies the per-chip index of the PWM to use, the
second cell is the period in nanoseconds and the third cell is the
polarity.

>
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - interrupts
> > +  - clocks
> > +  - '#pwm-cells'
>
> pwm.yaml makes this required already.
>
Yes, this is unnecessary. I will drop it in the next version.

Thanks.
Binbin
> Rob
>
Binbin Zhou April 3, 2024, 3:16 a.m. UTC | #3 
On Wed, Apr 3, 2024 at 8:37 AM Binbin Zhou <zhoubb.aaron@gmail.com> wrote:
>
> Hi Rob:
>
> Thanks for your reply.
>
> On Tue, Apr 2, 2024 at 11:40 PM Rob Herring <robh@kernel.org> wrote:
> >
> > On Tue, Apr 02, 2024 at 03:58:38PM +0800, Binbin Zhou wrote:
> > > Add Loongson PWM controller binding with DT schema format using
> > > json-schema.
> > >
> > > Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
> > > ---
> > >  .../devicetree/bindings/pwm/pwm-loongson.yaml | 64 +++++++++++++++++++
> >
> > Filename should match compatible.
>
> Emm... How about renaming it as loongson, pwm.yaml?
>
> >
> > >  MAINTAINERS                                   |  6 ++
> > >  2 files changed, 70 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/pwm/pwm-loongson.yaml
> > >
> > > diff --git a/Documentation/devicetree/bindings/pwm/pwm-loongson.yaml b/Documentation/devicetree/bindings/pwm/pwm-loongson.yaml
> > > new file mode 100644
> > > index 000000000000..d25904468353
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/pwm/pwm-loongson.yaml
> > > @@ -0,0 +1,64 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/pwm/pwm-loongson.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Loongson PWM Controller
> > > +
> > > +maintainers:
> > > +  - Binbin Zhou <zhoubinbin@loongson.cn>
> > > +
> > > +description:
> > > +  It is the generic PWM framework driver for Loongson family.
> >
> > That's describing the driver. Not really relevant to the binding.
> >
> Ok ,I will rewrite this part.
>
> >
> > > +  Each PWM has one pulse width output signal and one pulse input
> > > +  signal to be measured.
> > > +  It can be found on Loongson-2K series cpus and Loongson LS7A bridge chips.
> > > +
> > > +allOf:
> > > +  - $ref: pwm.yaml#
> > > +
> > > +properties:
> > > +  compatible:
> > > +    oneOf:
> > > +      - const: loongson,ls7a-pwm
> > > +      - items:
> > > +          - enum:
> > > +              - loongson,ls2k0500-pwm
> > > +              - loongson,ls2k1000-pwm
> > > +              - loongson,ls2k2000-pwm
> > > +          - const: loongson,ls7a-pwm
> > > +
> > > +  reg:
> > > +    maxItems: 1
> > > +
> > > +  interrupts:
> > > +    maxItems: 1
> > > +
> > > +  clocks:
> > > +    maxItems: 1
> > > +
> > > +  '#pwm-cells':
> > > +    const: 3
> >
> > Please define what is in each cell. If there's only 2 signals, then the
> > first cell defines the output or input (what value for which one?).

Hi Rob:

Sorry, the previous email did not answer this question.
The first cell defines the output signal, and its value is 0.

Thanks.
Binbin
> >
> > Really, the PWM binding is only for outputs, so is a cell even needed? I
> > suppose we could use it for inputs too, but that's really "input
> > capture" type operation that timers often have. I'll defer to the PWM
> > maintainers...
>
> Ok, I will try to add some description about it.
>
> If I understand correctly, the meaning of each cell in "#pwm-cells"is
> determined.
> The first cell specifies the per-chip index of the PWM to use, the
> second cell is the period in nanoseconds and the third cell is the
> polarity.
>
> >
> > > +
> > > +required:
> > > +  - compatible
> > > +  - reg
> > > +  - interrupts
> > > +  - clocks
> > > +  - '#pwm-cells'
> >
> > pwm.yaml makes this required already.
> >
> Yes, this is unnecessary. I will drop it in the next version.
>
> Thanks.
> Binbin
> > Rob
> >
Krzysztof Kozlowski April 3, 2024, 7 a.m. UTC | #4 
On 03/04/2024 04:37, Binbin Zhou wrote:
> Hi Rob:
> 
> Thanks for your reply.
> 
> On Tue, Apr 2, 2024 at 11:40 PM Rob Herring <robh@kernel.org> wrote:
>>
>> On Tue, Apr 02, 2024 at 03:58:38PM +0800, Binbin Zhou wrote:
>>> Add Loongson PWM controller binding with DT schema format using
>>> json-schema.
>>>
>>> Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
>>> ---
>>>  .../devicetree/bindings/pwm/pwm-loongson.yaml | 64 +++++++++++++++++++
>>
>> Filename should match compatible.
> 
> Emm... How about renaming it as loongson, pwm.yaml?

Use the fallback, so loongson,ls7a-pwm.yaml

Best regards,
Krzysztof
Binbin Zhou April 3, 2024, 7:22 a.m. UTC | #5 
On Wed, Apr 3, 2024 at 1:00 PM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 03/04/2024 04:37, Binbin Zhou wrote:
> > Hi Rob:
> >
> > Thanks for your reply.
> >
> > On Tue, Apr 2, 2024 at 11:40 PM Rob Herring <robh@kernel.org> wrote:
> >>
> >> On Tue, Apr 02, 2024 at 03:58:38PM +0800, Binbin Zhou wrote:
> >>> Add Loongson PWM controller binding with DT schema format using
> >>> json-schema.
> >>>
> >>> Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
> >>> ---
> >>>  .../devicetree/bindings/pwm/pwm-loongson.yaml | 64 +++++++++++++++++++
> >>
> >> Filename should match compatible.
> >
> > Emm... How about renaming it as loongson, pwm.yaml?
>
> Use the fallback, so loongson,ls7a-pwm.yaml

Ok, I got it.

Thanks.
Binbin
>
> Best regards,
> Krzysztof
>
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/pwm/pwm-loongson.yaml b/Documentation/devicetree/bindings/pwm/pwm-loongson.yaml
new file mode 100644
index 000000000000..d25904468353
--- /dev/null
+++ b/Documentation/devicetree/bindings/pwm/pwm-loongson.yaml
@@ -0,0 +1,64 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pwm/pwm-loongson.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Loongson PWM Controller
+
+maintainers:
+  - Binbin Zhou <zhoubinbin@loongson.cn>
+
+description:
+  It is the generic PWM framework driver for Loongson family.
+  Each PWM has one pulse width output signal and one pulse input
+  signal to be measured.
+  It can be found on Loongson-2K series cpus and Loongson LS7A bridge chips.
+
+allOf:
+  - $ref: pwm.yaml#
+
+properties:
+  compatible:
+    oneOf:
+      - const: loongson,ls7a-pwm
+      - items:
+          - enum:
+              - loongson,ls2k0500-pwm
+              - loongson,ls2k1000-pwm
+              - loongson,ls2k2000-pwm
+          - const: loongson,ls7a-pwm
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  '#pwm-cells':
+    const: 3
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+  - '#pwm-cells'
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    #include <dt-bindings/clock/loongson,ls2k-clk.h>
+    pwm@1fe22000 {
+        compatible = "loongson,ls2k1000-pwm", "loongson,ls7a-pwm";
+        reg = <0x1fe22000 0x10>;
+        interrupt-parent = <&liointc0>;
+        interrupts = <24 IRQ_TYPE_LEVEL_HIGH>;
+        clocks = <&clk LOONGSON2_APB_CLK>;
+        #pwm-cells = <3>;
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index 96496dbcae6d..9c4f3186e7f4 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12753,6 +12753,12 @@  S:	Maintained
 F:	Documentation/devicetree/bindings/i2c/loongson,ls2x-i2c.yaml
 F:	drivers/i2c/busses/i2c-ls2x.c
 
+LOONGSON PWM DRIVER
+M:	Binbin Zhou <zhoubinbin@loongson.cn>
+L:	linux-pwm@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/pwm/pwm-loongson.yaml
+
 LOONGSON-2 SOC SERIES CLOCK DRIVER
 M:	Yinbo Zhu <zhuyinbo@loongson.cn>
 L:	linux-clk@vger.kernel.org