| Message ID | 20231020103741.557735-2-william.qiu@starfivetech.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | StarFive's Pulse Width Modulation driver support | expand |
| Context | Check | Description |
|---|---|---|
| conchuod/vmtest-fixes-PR | fail | merge-conflict |
Krzysztof, William, On Fri, Oct 20, 2023 at 06:37:38PM +0800, William Qiu wrote: > Add documentation to describe OpenCores Pulse Width Modulation > controller driver. > > Signed-off-by: William Qiu <william.qiu@starfivetech.com> > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > Reviewed-by: Hal Feng <hal.feng@starfivetech.com> > --- > .../bindings/pwm/opencores,pwm-ocores.yaml | 53 +++++++++++++++++++ > 1 file changed, 53 insertions(+) > create mode 100644 Documentation/devicetree/bindings/pwm/opencores,pwm-ocores.yaml > > diff --git a/Documentation/devicetree/bindings/pwm/opencores,pwm-ocores.yaml b/Documentation/devicetree/bindings/pwm/opencores,pwm-ocores.yaml > new file mode 100644 > index 000000000000..0f6a3434f155 > --- /dev/null > +++ b/Documentation/devicetree/bindings/pwm/opencores,pwm-ocores.yaml > @@ -0,0 +1,53 @@ > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/pwm/opencores,pwm-ocores.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: OpenCores PWM controller > + > +maintainers: > + - William Qiu <william.qiu@starfivetech.com> > + > +description: > + OpenCores PTC ip core contains a PWM controller. When operating in PWM mode, the PTC core > + generates binary signal with user-programmable low and high periods. All PTC counters and > + registers are 32-bit. > + > +allOf: > + - $ref: pwm.yaml# > + > +properties: > + compatible: > + enum: > + - opencores,pwm-ocores What does the extra "ocores" suffix add, when it just repeats the vendor prefix? > + - starfive,jh71x0-pwm Krzysztof, did you approve this generic compatible? And the whole thing looks like it should really be something like items: - enum: - starfive,jh7100-pwm - starfive,jh7110-pwm - const: opencores,pwm Cheers, Conor. > + > + reg: > + maxItems: 1 > + > + clocks: > + maxItems: 1 > + > + resets: > + maxItems: 1 > + > + "#pwm-cells": > + const: 3 > + > +required: > + - compatible > + - reg > + - clocks > + > +additionalProperties: false > + > +examples: > + - | > + pwm@12490000 { > + compatible = "opencores,pwm-ocores"; > + reg = <0x12490000 0x10000>; > + clocks = <&clkgen 181>; > + resets = <&rstgen 109>; > + #pwm-cells = <3>; > + }; > -- > 2.34.1 >
On Fri, Oct 20, 2023 at 03:21:15PM +0100, Conor Dooley wrote: > Krzysztof, William, > > On Fri, Oct 20, 2023 at 06:37:38PM +0800, William Qiu wrote: > > Add documentation to describe OpenCores Pulse Width Modulation > > controller driver. > > > > Signed-off-by: William Qiu <william.qiu@starfivetech.com> > > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > > Reviewed-by: Hal Feng <hal.feng@starfivetech.com> > > --- > > .../bindings/pwm/opencores,pwm-ocores.yaml | 53 +++++++++++++++++++ > > 1 file changed, 53 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/pwm/opencores,pwm-ocores.yaml > > > > diff --git a/Documentation/devicetree/bindings/pwm/opencores,pwm-ocores.yaml b/Documentation/devicetree/bindings/pwm/opencores,pwm-ocores.yaml > > new file mode 100644 > > index 000000000000..0f6a3434f155 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/pwm/opencores,pwm-ocores.yaml > > @@ -0,0 +1,53 @@ > > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/pwm/opencores,pwm-ocores.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: OpenCores PWM controller > > + > > +maintainers: > > + - William Qiu <william.qiu@starfivetech.com> > > + > > +description: > > + OpenCores PTC ip core contains a PWM controller. When operating in PWM mode, the PTC core > > + generates binary signal with user-programmable low and high periods. All PTC counters and > > + registers are 32-bit. > > + > > +allOf: > > + - $ref: pwm.yaml# > > + > > +properties: > > + compatible: > > + enum: > > + - opencores,pwm-ocores > > What does the extra "ocores" suffix add, when it just repeats the vendor > prefix? > > > + - starfive,jh71x0-pwm > > Krzysztof, did you approve this generic compatible? > > And the whole thing looks like it should really be something like > > items: > - enum: > - starfive,jh7100-pwm > - starfive,jh7110-pwm > - const: opencores,pwm (assuming that the opencores,pwm compatible represents a subset of what is implemented on the jh7100 series)
On 20/10/2023 12:37, William Qiu wrote: > Add documentation to describe OpenCores Pulse Width Modulation > controller driver. > > Signed-off-by: William Qiu <william.qiu@starfivetech.com> > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Please point me where this patch got review? > Reviewed-by: Hal Feng <hal.feng@starfivetech.com> > --- > .../bindings/pwm/opencores,pwm-ocores.yaml | 53 +++++++++++++++++++ > 1 file changed, 53 insertions(+) > create mode 100644 Documentation/devicetree/bindings/pwm/opencores,pwm-ocores.yaml > > diff --git a/Documentation/devicetree/bindings/pwm/opencores,pwm-ocores.yaml b/Documentation/devicetree/bindings/pwm/opencores,pwm-ocores.yaml > new file mode 100644 > index 000000000000..0f6a3434f155 > --- /dev/null > +++ b/Documentation/devicetree/bindings/pwm/opencores,pwm-ocores.yaml > @@ -0,0 +1,53 @@ > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/pwm/opencores,pwm-ocores.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: OpenCores PWM controller > + > +maintainers: > + - William Qiu <william.qiu@starfivetech.com> > + > +description: > + OpenCores PTC ip core contains a PWM controller. When operating in PWM mode, the PTC core > + generates binary signal with user-programmable low and high periods. All PTC counters and > + registers are 32-bit. > + > +allOf: > + - $ref: pwm.yaml# > + > +properties: > + compatible: > + enum: > + - opencores,pwm-ocores NAK. This is not something which received my review. Best regards, Krzysztof
On 20/10/2023 16:22, Conor Dooley wrote: > On Fri, Oct 20, 2023 at 03:21:15PM +0100, Conor Dooley wrote: >> Krzysztof, William, >> >> On Fri, Oct 20, 2023 at 06:37:38PM +0800, William Qiu wrote: >>> Add documentation to describe OpenCores Pulse Width Modulation >>> controller driver. >>> >>> Signed-off-by: William Qiu <william.qiu@starfivetech.com> >>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> >>> Reviewed-by: Hal Feng <hal.feng@starfivetech.com> >>> --- >>> .../bindings/pwm/opencores,pwm-ocores.yaml | 53 +++++++++++++++++++ >>> 1 file changed, 53 insertions(+) >>> create mode 100644 Documentation/devicetree/bindings/pwm/opencores,pwm-ocores.yaml >>> >>> diff --git a/Documentation/devicetree/bindings/pwm/opencores,pwm-ocores.yaml b/Documentation/devicetree/bindings/pwm/opencores,pwm-ocores.yaml >>> new file mode 100644 >>> index 000000000000..0f6a3434f155 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/pwm/opencores,pwm-ocores.yaml >>> @@ -0,0 +1,53 @@ >>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause >>> +%YAML 1.2 >>> +--- >>> +$id: http://devicetree.org/schemas/pwm/opencores,pwm-ocores.yaml# >>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>> + >>> +title: OpenCores PWM controller >>> + >>> +maintainers: >>> + - William Qiu <william.qiu@starfivetech.com> >>> + >>> +description: >>> + OpenCores PTC ip core contains a PWM controller. When operating in PWM mode, the PTC core >>> + generates binary signal with user-programmable low and high periods. All PTC counters and >>> + registers are 32-bit. >>> + >>> +allOf: >>> + - $ref: pwm.yaml# >>> + >>> +properties: >>> + compatible: >>> + enum: >>> + - opencores,pwm-ocores >> >> What does the extra "ocores" suffix add, when it just repeats the vendor >> prefix? >> >>> + - starfive,jh71x0-pwm >> >> Krzysztof, did you approve this generic compatible? Patch was quite different than reviewed by me. This obviously does not make any sense. Thanks for spotting. I guess carrying tags should not be trusted. >> >> And the whole thing looks like it should really be something like >> >> items: >> - enum: >> - starfive,jh7100-pwm >> - starfive,jh7110-pwm >> - const: opencores,pwm > > (assuming that the opencores,pwm compatible represents a subset of what > is implemented on the jh7100 series) Best regards, Krzysztof
On 2023/10/20 22:21, Conor Dooley wrote: > Krzysztof, William, > > On Fri, Oct 20, 2023 at 06:37:38PM +0800, William Qiu wrote: >> Add documentation to describe OpenCores Pulse Width Modulation >> controller driver. >> >> Signed-off-by: William Qiu <william.qiu@starfivetech.com> >> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> >> Reviewed-by: Hal Feng <hal.feng@starfivetech.com> >> --- >> .../bindings/pwm/opencores,pwm-ocores.yaml | 53 +++++++++++++++++++ >> 1 file changed, 53 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/pwm/opencores,pwm-ocores.yaml >> >> diff --git a/Documentation/devicetree/bindings/pwm/opencores,pwm-ocores.yaml b/Documentation/devicetree/bindings/pwm/opencores,pwm-ocores.yaml >> new file mode 100644 >> index 000000000000..0f6a3434f155 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/pwm/opencores,pwm-ocores.yaml >> @@ -0,0 +1,53 @@ >> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/pwm/opencores,pwm-ocores.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: OpenCores PWM controller >> + >> +maintainers: >> + - William Qiu <william.qiu@starfivetech.com> >> + >> +description: >> + OpenCores PTC ip core contains a PWM controller. When operating in PWM mode, the PTC core >> + generates binary signal with user-programmable low and high periods. All PTC counters and >> + registers are 32-bit. >> + >> +allOf: >> + - $ref: pwm.yaml# >> + >> +properties: >> + compatible: >> + enum: >> + - opencores,pwm-ocores > > What does the extra "ocores" suffix add, when it just repeats the vendor > prefix? > >> + - starfive,jh71x0-pwm > > Krzysztof, did you approve this generic compatible? > > And the whole thing looks like it should really be something like > > items: > - enum: > - starfive,jh7100-pwm > - starfive,jh7110-pwm > - const: opencores,pwm > > Cheers, > Conor. > I'm going to use this format. Thanks, William >> + >> + reg: >> + maxItems: 1 >> + >> + clocks: >> + maxItems: 1 >> + >> + resets: >> + maxItems: 1 >> + >> + "#pwm-cells": >> + const: 3 >> + >> +required: >> + - compatible >> + - reg >> + - clocks >> + >> +additionalProperties: false >> + >> +examples: >> + - | >> + pwm@12490000 { >> + compatible = "opencores,pwm-ocores"; >> + reg = <0x12490000 0x10000>; >> + clocks = <&clkgen 181>; >> + resets = <&rstgen 109>; >> + #pwm-cells = <3>; >> + }; >> -- >> 2.34.1 >>
On 2023/10/21 2:01, Krzysztof Kozlowski wrote: > On 20/10/2023 12:37, William Qiu wrote: >> Add documentation to describe OpenCores Pulse Width Modulation >> controller driver. >> >> Signed-off-by: William Qiu <william.qiu@starfivetech.com> >> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > > Please point me where this patch got review? > This is my mistake. After making extensive changes, the tag should have been deleted Best regards, William >> Reviewed-by: Hal Feng <hal.feng@starfivetech.com> >> --- >> .../bindings/pwm/opencores,pwm-ocores.yaml | 53 +++++++++++++++++++ >> 1 file changed, 53 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/pwm/opencores,pwm-ocores.yaml >> >> diff --git a/Documentation/devicetree/bindings/pwm/opencores,pwm-ocores.yaml b/Documentation/devicetree/bindings/pwm/opencores,pwm-ocores.yaml >> new file mode 100644 >> index 000000000000..0f6a3434f155 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/pwm/opencores,pwm-ocores.yaml >> @@ -0,0 +1,53 @@ >> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/pwm/opencores,pwm-ocores.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: OpenCores PWM controller >> + >> +maintainers: >> + - William Qiu <william.qiu@starfivetech.com> >> + >> +description: >> + OpenCores PTC ip core contains a PWM controller. When operating in PWM mode, the PTC core >> + generates binary signal with user-programmable low and high periods. All PTC counters and >> + registers are 32-bit. >> + >> +allOf: >> + - $ref: pwm.yaml# >> + >> +properties: >> + compatible: >> + enum: >> + - opencores,pwm-ocores > > NAK. This is not something which received my review. > > > Best regards, > Krzysztof >
diff --git a/Documentation/devicetree/bindings/pwm/opencores,pwm-ocores.yaml b/Documentation/devicetree/bindings/pwm/opencores,pwm-ocores.yaml new file mode 100644 index 000000000000..0f6a3434f155 --- /dev/null +++ b/Documentation/devicetree/bindings/pwm/opencores,pwm-ocores.yaml @@ -0,0 +1,53 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/pwm/opencores,pwm-ocores.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: OpenCores PWM controller + +maintainers: + - William Qiu <william.qiu@starfivetech.com> + +description: + OpenCores PTC ip core contains a PWM controller. When operating in PWM mode, the PTC core + generates binary signal with user-programmable low and high periods. All PTC counters and + registers are 32-bit. + +allOf: + - $ref: pwm.yaml# + +properties: + compatible: + enum: + - opencores,pwm-ocores + - starfive,jh71x0-pwm + + reg: + maxItems: 1 + + clocks: + maxItems: 1 + + resets: + maxItems: 1 + + "#pwm-cells": + const: 3 + +required: + - compatible + - reg + - clocks + +additionalProperties: false + +examples: + - | + pwm@12490000 { + compatible = "opencores,pwm-ocores"; + reg = <0x12490000 0x10000>; + clocks = <&clkgen 181>; + resets = <&rstgen 109>; + #pwm-cells = <3>; + };