Re: Re: [PATCH v6 1/2] dt-bindings: pwm: dwc: add optional reset

Re: Re: [PATCH v6 1/2] dt-bindings: pwm: dwc: add optional reset

[Xuyang Dong]

> 
> On 29/04/2026 11:30, Xuyang Dong wrote:
> >>>>>  
> >>>>> +allOf:
> >>>>> +  - $ref: pwm.yaml#
> >>>>> +
> >>>>> +  - if:
> >>>>> +      properties:
> >>>>> +        compatible:
> >>>>> +          contains:
> >>>>> +            const: eswin,eic7700-pwm
> >>>>
> >>>> Same problem as v3 which I commented. I do not understand why your new
> >>>> device has also 1 reset.
> >>>>
> >>>> Your commit msg MUST explain why 1 reset is valid.
> >>>>
> >>>
> >>> Hi Krzysztof,
> >>>
> >>> Although the PWM IP supports two clock domains, each requiring a reset, 
> >>> the EIC7700 implementation uses the same clock domain for both clock 
> >>> signals. Therefore, the eic7700-pwm only supports one reset.
> >>>
> >>
> >> If we speak about eic7700, explain why it has two resets now, according
> >> to schema, even though you say it has not.
> >>
> >> But I was speaking about dw-apb-timers-pwm, which has one reset as well!
> >> Why you are not having proper constraints? Please read writing bindings
> >> document.
> >>
> > 
> > Hi Krzysztof,
> > 
> > Let me clarify the reset signals.
> >   - snps,dw-apb-timers-pwm2: IP spec has 2 optional reset signals (one per
> > clock domain), SoC vendor decides whether to wire them — so maxItems: 2, 
> > optional in required.
> 
> Two reset signals but what is exactly optional? Each of them? Only the
> first? Binding does not allow the first to be optional.
> 

Hi Krzysztof,

Thank you for the review. 

For the generic snps,dw-apb-timers-pwm2 binding, both reset signals 
are now fully optional by not including resets in the required list.

When a single optional reset signal is used, the interface bus reset 
(index 0) is used by default.

Keep the YAML as follows:
+  resets:
+    minItems: 1
+    items:
+      - description: Interface bus reset
+      - description: PWM timer logic reset

Add the following description to the commit message:

Whether each signal is wired on a given SoC is a board integration 
decision, so the resets property is optional for snps,dw-apb-timers-pwm2. 
When present, up to two handles may be supplied: the bus reset is always 
at index 0 and the timer reset at index 1.

> >   - eswin,eic7700-pwm: SoC physically ties both signals to one reset — so
> > exactly 1, required.
> 
> Then two would not be right and you need to restrict that.
> 

For the specific eswin,eic7700-pwm binding, the reset signal is required 
and fixed to one via conditional schema (if:then:), with maxItems: 1 
and resets added to required. And add an example for eswin,eic7700-pwm.
The changes are as follows:

+allOf:
+  - $ref: pwm.yaml#
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: eswin,eic7700-pwm
+    then:
+      properties:
+        resets:
+          maxItems: 1
+      required:
+        - resets
+

+  - |
+    pwm@50818000 {
+      compatible = "eswin,eic7700-pwm";
+      reg = <0x50818000 0x4000>;
+      #pwm-cells = <3>;
+      clocks = <&bus>, <&timer>;
+      clock-names = "bus", "timer";
+      resets = <&reset>;
+    };

Then change the binding's subject from "dt-bindings: pwm: dwc: add optional 
reset" to "dt-bindings: pwm: dwc: add eswin,eic7700-pwm compatible and resets".

Do these changes look acceptable to you?

Best regards,
Xuyang Dong