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

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

[Krzysztof Kozlowski]

On 11/05/2026 09:10, Xuyang Dong wrote:
>>
>> 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:

We speak about hardware, not binding. I asked, why your new device has
only one reset.

> 
> 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?

So two resets or one reset? I am completely confused what you are
replying to.

Please read writing bindings document.

Best regards,
Krzysztof