Re: [PATCH v2 1/2] dt-bindings: pwm: add atcpit100-pwm

[Ben Zong-You Xie <ben717@andestech.com>]

On Mon, Dec 02, 2024 at 08:40:22AM +0100, Krzysztof Kozlowski wrote:
> [EXTERNAL MAIL]
> 
> On Mon, Dec 02, 2024 at 02:01:46PM +0800, Ben Zong-You Xie wrote:
> > Document devicetree bindings for Andes atcpit100-pwm.
> >
> > Signed-off-by: Ben Zong-You Xie <ben717@andestech.com>
> > ---
> >  .../bindings/pwm/andestech,atcpit100-pwm.yaml | 51 +++++++++++++++++++
> >  MAINTAINERS                                   |  5 ++
> >  2 files changed, 56 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/pwm/andestech,atcpit100-pwm.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/pwm/andestech,atcpit100-pwm.yaml b/Documentation/devicetree/bindings/pwm/andestech,atcpit100-pwm.yaml
> > new file mode 100644
> > index 000000000000..4b707f32ad72
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/pwm/andestech,atcpit100-pwm.yaml
> > @@ -0,0 +1,51 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/pwm/andestech,atcpit100-pwm.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Andes atcpit100 PWM
> > +
> > +maintainers:
> > +  - Ben Zong-You Xie <ben717@andestech.com>
> > +
> > +allOf:
> > +  - $ref: pwm.yaml#
> > +
> > +properties:
> > +  compatible:
> > +    const: andestech,atcpit100-pwm
> 
> 
> Previously, before we removed it in 2022, this was just
> andestech,atcpit100, so questions:
> 
> 1. Why are you re-introducing it? Please address all the comments or
> aspects leading to removal.
> 2. Why are you using different compatible? Is this one device?
> 
> Best regards,
> Krzysztof
>

Hi Krzysztof,

1. You can first refer to the patch[1].
   The patch not only removes the support to nds32, but also removes
   Andes device driver. Though Andes now dedicates our effort on RISC-V,
   ATCPIT100 is still one of peripheral platform IPs, and that's why we are
   re-introducing it now.

2. Yes, they are the same device. ATCPIT100 is a set of compact
   multi-function timers, which can be used as PWMs or simple timers.
   I think the example in the YAML file is a little confusing because
   there are two ATCPIT100 nodes in our DTS file now:

	pit: timer@f0400000 {
		compatible = "andestech,atcpit100";
		...
		...
	};
	pwm: pwm@f0400000 {
		compatible = "andestech,atcpit100-pwm";
		...
		...
	};

   Is it better to modify our DTS file and the example in the YAML file
   like below?
	
	pit: pit@f0400000 {
		compatible = "andestech,atcpit100";
		reg = <0xf0400000, 0x1000>;
		clocks = <&smu 1>, <&smu 7>;
		clock-names = "ext", "apb";
		pwm: pwm {
			compatible = "andestech,atcpit100-pwm";
			#pwm-cells = <3>;
		};
	};

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=aec499c75cf8e0b599be4d559e6922b613085f8f

Best regards,
Ben