| Message ID | 20220214135840.168236-6-conor.dooley@microchip.com |
|---|---|
| State | New |
| Headers | show |
| Series | Update the Icicle Kit device tree | expand |
Hey Uwe, Could you take a look at this version & see if the descriptions are easier to understand? Thanks, Conor On 14/02/2022 13:58, conor.dooley@microchip.com wrote: > From: Conor Dooley <conor.dooley@microchip.com> > > Add device tree bindings for the Microchip fpga fabric based "core" PWM > controller. > > Reviewed-by: Rob Herring <robh@kernel.org> > Signed-off-by: Conor Dooley <conor.dooley@microchip.com> > Acked-by: Palmer Dabbelt <palmer@rivosinc.com> > --- > .../bindings/pwm/microchip,corepwm.yaml | 81 +++++++++++++++++++ > 1 file changed, 81 insertions(+) > create mode 100644 Documentation/devicetree/bindings/pwm/microchip,corepwm.yaml > > diff --git a/Documentation/devicetree/bindings/pwm/microchip,corepwm.yaml b/Documentation/devicetree/bindings/pwm/microchip,corepwm.yaml > new file mode 100644 > index 000000000000..a7fae1772a81 > --- /dev/null > +++ b/Documentation/devicetree/bindings/pwm/microchip,corepwm.yaml > @@ -0,0 +1,81 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > + > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/pwm/microchip,corepwm.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Microchip IP corePWM controller bindings > + > +maintainers: > + - Conor Dooley <conor.dooley@microchip.com> > + > +description: | > + corePWM is an 16 channel pulse width modulator FPGA IP > + > + https://www.microsemi.com/existing-parts/parts/152118 > + > +allOf: > + - $ref: pwm.yaml# > + > +properties: > + compatible: > + items: > + - const: microchip,corepwm-rtl-v4 > + > + reg: > + maxItems: 1 > + > + clocks: > + maxItems: 1 > + > + "#pwm-cells": > + const: 2 > + > + microchip,sync-update-mask: > + description: | > + Depending on how the IP is instantiated, there are two modes of operation. > + In synchronous mode, all channels are updated at the beginning of the PWM period, > + and in asynchronous mode updates happen as the control registers are written. > + A 16 bit wide "SHADOW_REG_EN" parameter of the IP core controls whether synchronous > + mode is possible for each channel, and is set by the bitstream programmed to the > + FPGA. If the IP core is instantiated with SHADOW_REG_ENx=1, both registers that > + control the duty cycle for channel x have a second "shadow"/buffer reg synthesised. > + At runtime a bit wide register exposed to APB can be used to toggle on/off > + synchronised mode for all channels it has been synthesised for. > + Each bit of "microchip,sync-update-mask" corresponds to a PWM channel & represents > + whether synchronous mode is possible for the PWM channel. > + > + $ref: /schemas/types.yaml#/definitions/uint32 > + default: 0 > + > + microchip,dac-mode-mask: > + description: | > + Optional, per-channel Low Ripple DAC mode is possible on this IP core. It creates > + a minimum period pulse train whose High/Low average is that of the chosen duty > + cycle. This "DAC" will have far better bandwidth and ripple performance than the > + standard PWM algorithm can achieve. A 16 bit DAC_MODE module parameter of the IP > + core, set at instantiation and by the bitstream programmed to the FPGA, determines > + whether a given channel operates in regular PWM or DAC mode. > + Each bit corresponds to a PWM channel & represents whether DAC mode is enabled > + for that channel. > + > + $ref: /schemas/types.yaml#/definitions/uint32 > + default: 0 > + > +required: > + - compatible > + - reg > + - clocks > + > +additionalProperties: false > + > +examples: > + - | > + pwm@41000000 { > + compatible = "microchip,corepwm-rtl-v4"; > + microchip,sync-update-mask = /bits/ 32 <0>; > + clocks = <&clkcfg 30>; > + reg = <0x41000000 0xF0>; > + #pwm-cells = <2>; > + };
On Mon, Feb 14, 2022 at 01:58:35PM +0000, conor.dooley@microchip.com wrote: > From: Conor Dooley <conor.dooley@microchip.com> > > Add device tree bindings for the Microchip fpga fabric based "core" PWM > controller. > > Reviewed-by: Rob Herring <robh@kernel.org> > Signed-off-by: Conor Dooley <conor.dooley@microchip.com> > Acked-by: Palmer Dabbelt <palmer@rivosinc.com> I like it: Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> nitpick: Put your S-o-b last in the commit log. (This doesn't justify a resend IMHO) Best regards Uwe
On 23/02/2022 07:20, Uwe Kleine-König wrote: > On Mon, Feb 14, 2022 at 01:58:35PM +0000, conor.dooley@microchip.com wrote: >> From: Conor Dooley <conor.dooley@microchip.com> >> >> Add device tree bindings for the Microchip fpga fabric based "core" PWM >> controller. >> >> Reviewed-by: Rob Herring <robh@kernel.org> >> Signed-off-by: Conor Dooley <conor.dooley@microchip.com> >> Acked-by: Palmer Dabbelt <palmer@rivosinc.com> > > I like it: > > Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > > nitpick: Put your S-o-b last in the commit log. (This doesn't justify a > resend IMHO) It should be the opposite - the first. First author signs the patch, then comes review and finally an ack. Putting SoB at then suggests that tags were accumulated before sending patch, out of mailing list. Best regards, Krzysztof
On Wed, Feb 23, 2022 at 08:12:49AM +0100, Krzysztof Kozlowski wrote: > On 23/02/2022 07:20, Uwe Kleine-König wrote: > > On Mon, Feb 14, 2022 at 01:58:35PM +0000, conor.dooley@microchip.com wrote: > >> From: Conor Dooley <conor.dooley@microchip.com> > >> > >> Add device tree bindings for the Microchip fpga fabric based "core" PWM > >> controller. > >> > >> Reviewed-by: Rob Herring <robh@kernel.org> > >> Signed-off-by: Conor Dooley <conor.dooley@microchip.com> > >> Acked-by: Palmer Dabbelt <palmer@rivosinc.com> > > > > I like it: > > > > Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > > > > nitpick: Put your S-o-b last in the commit log. (This doesn't justify a > > resend IMHO) > > It should be the opposite - the first. First author signs the patch, > then comes review and finally an ack. Putting SoB at then suggests that > tags were accumulated before sending patch, out of mailing list. well, or in an earlier revision of this patch as is the case here. One of the ideas of S-o-b is that the order shows the flow of the patch states and if this patch ends in git with: Referred-by: Rob Herring <robh@kernel.org> Singed-off-by: Conor Dooley <conor.dooley@microchip.com> Backed-by: Palmer Dabbelt <palmer@rivosinc.com> Singed-off-by: Peter Maintainer <pm@example.com> I'd expect that Backed-by was added by Peter, not Conor. (Modified the tags on purpose to not interfere with b4's tag pickup, I guess you humans still get the point.) Best regards Uwe
On 23/02/2022 08:20, Uwe Kleine-König wrote: > On Wed, Feb 23, 2022 at 08:12:49AM +0100, Krzysztof Kozlowski wrote: > > On 23/02/2022 07:20, Uwe Kleine-König wrote: > > > On Mon, Feb 14, 2022 at 01:58:35PM +0000, conor.dooley@microchip.com wrote: > > >> From: Conor Dooley <conor.dooley@microchip.com> > > >> > > >> Add device tree bindings for the Microchip fpga fabric based "core" PWM > > >> controller. > > >> > > >> Reviewed-by: Rob Herring <robh@kernel.org> > > >> Signed-off-by: Conor Dooley <conor.dooley@microchip.com> > > >> Acked-by: Palmer Dabbelt <palmer@rivosinc.com> > > > > > > I like it: > > > > > > Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > > > > > > nitpick: Put your S-o-b last in the commit log. (This doesn't justify a > > > resend IMHO) > > > > It should be the opposite - the first. First author signs the patch, > > then comes review and finally an ack. Putting SoB at then suggests that > > tags were accumulated before sending patch, out of mailing list. > > well, or in an earlier revision of this patch as is the case here. One > of the ideas of S-o-b is that the order shows the flow of the patch > states and if this patch ends in git with: > > Referred-by: Rob Herring <robh@kernel.org> > Singed-off-by: Conor Dooley <conor.dooley@microchip.com> > Backed-by: Palmer Dabbelt <palmer@rivosinc.com> > Singed-off-by: Peter Maintainer <pm@example.com> > > I'd expect that Backed-by was added by Peter, not Conor. > (Modified the tags on purpose to not interfere with b4's tag pickup, I > guess you humans still get the point.) I had put the acks after the S-o-B for patches I hadn't changed since the ack, but I think that may have been a misinterpretation of what was meant by Rob when he said tags should be in chronological order. Won't do it this way in the future. If the remaining patch gets a maintainer ack, the order will be fine I guess since it'll be Palmer taking it anyway. If there's a v8, I will fix the order. Thanks, Conor
On Wed, 23 Feb 2022, Uwe Kleine-König wrote: > On Wed, Feb 23, 2022 at 08:12:49AM +0100, Krzysztof Kozlowski wrote: > > On 23/02/2022 07:20, Uwe Kleine-König wrote: > > > On Mon, Feb 14, 2022 at 01:58:35PM +0000, conor.dooley@microchip.com wrote: > > >> From: Conor Dooley <conor.dooley@microchip.com> > > >> > > >> Add device tree bindings for the Microchip fpga fabric based "core" PWM > > >> controller. > > >> > > >> Reviewed-by: Rob Herring <robh@kernel.org> > > >> Signed-off-by: Conor Dooley <conor.dooley@microchip.com> > > >> Acked-by: Palmer Dabbelt <palmer@rivosinc.com> > > > > > > I like it: > > > > > > Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > > > > > > nitpick: Put your S-o-b last in the commit log. (This doesn't justify a > > > resend IMHO) > > > > It should be the opposite - the first. First author signs the patch, > > then comes review and finally an ack. Putting SoB at then suggests that > > tags were accumulated before sending patch, out of mailing list. > > well, or in an earlier revision of this patch as is the case here. One > of the ideas of S-o-b is that the order shows the flow of the patch > states and if this patch ends in git with: > > Referred-by: Rob Herring <robh@kernel.org> > Singed-off-by: Conor Dooley <conor.dooley@microchip.com> > Backed-by: Palmer Dabbelt <palmer@rivosinc.com> > Singed-off-by: Peter Maintainer <pm@example.com> > > I'd expect that Backed-by was added by Peter, not Conor. > (Modified the tags on purpose to not interfere with b4's tag pickup, I > guess you humans still get the point.) I tend to like *-by tags to appear chronologically. Suggested (suggested-by) Authored (signed-off-by) Co-Authored (signed-off-by/co-developed-by) Reviewed/Acked/Tested (reviewed-by/acked-by/tested-by) Committed (signed-off-by)
On Mon, Feb 14, 2022 at 01:58:35PM +0000, conor.dooley@microchip.com wrote: > From: Conor Dooley <conor.dooley@microchip.com> > > Add device tree bindings for the Microchip fpga fabric based "core" PWM > controller. > > Reviewed-by: Rob Herring <robh@kernel.org> > Signed-off-by: Conor Dooley <conor.dooley@microchip.com> > Acked-by: Palmer Dabbelt <palmer@rivosinc.com> > --- > .../bindings/pwm/microchip,corepwm.yaml | 81 +++++++++++++++++++ > 1 file changed, 81 insertions(+) > create mode 100644 Documentation/devicetree/bindings/pwm/microchip,corepwm.yaml Fine with me to go through the RISC-V tree: Acked-by: Thierry Reding <thierry.reding@gmail.com>
diff --git a/Documentation/devicetree/bindings/pwm/microchip,corepwm.yaml b/Documentation/devicetree/bindings/pwm/microchip,corepwm.yaml new file mode 100644 index 000000000000..a7fae1772a81 --- /dev/null +++ b/Documentation/devicetree/bindings/pwm/microchip,corepwm.yaml @@ -0,0 +1,81 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) + +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/pwm/microchip,corepwm.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Microchip IP corePWM controller bindings + +maintainers: + - Conor Dooley <conor.dooley@microchip.com> + +description: | + corePWM is an 16 channel pulse width modulator FPGA IP + + https://www.microsemi.com/existing-parts/parts/152118 + +allOf: + - $ref: pwm.yaml# + +properties: + compatible: + items: + - const: microchip,corepwm-rtl-v4 + + reg: + maxItems: 1 + + clocks: + maxItems: 1 + + "#pwm-cells": + const: 2 + + microchip,sync-update-mask: + description: | + Depending on how the IP is instantiated, there are two modes of operation. + In synchronous mode, all channels are updated at the beginning of the PWM period, + and in asynchronous mode updates happen as the control registers are written. + A 16 bit wide "SHADOW_REG_EN" parameter of the IP core controls whether synchronous + mode is possible for each channel, and is set by the bitstream programmed to the + FPGA. If the IP core is instantiated with SHADOW_REG_ENx=1, both registers that + control the duty cycle for channel x have a second "shadow"/buffer reg synthesised. + At runtime a bit wide register exposed to APB can be used to toggle on/off + synchronised mode for all channels it has been synthesised for. + Each bit of "microchip,sync-update-mask" corresponds to a PWM channel & represents + whether synchronous mode is possible for the PWM channel. + + $ref: /schemas/types.yaml#/definitions/uint32 + default: 0 + + microchip,dac-mode-mask: + description: | + Optional, per-channel Low Ripple DAC mode is possible on this IP core. It creates + a minimum period pulse train whose High/Low average is that of the chosen duty + cycle. This "DAC" will have far better bandwidth and ripple performance than the + standard PWM algorithm can achieve. A 16 bit DAC_MODE module parameter of the IP + core, set at instantiation and by the bitstream programmed to the FPGA, determines + whether a given channel operates in regular PWM or DAC mode. + Each bit corresponds to a PWM channel & represents whether DAC mode is enabled + for that channel. + + $ref: /schemas/types.yaml#/definitions/uint32 + default: 0 + +required: + - compatible + - reg + - clocks + +additionalProperties: false + +examples: + - | + pwm@41000000 { + compatible = "microchip,corepwm-rtl-v4"; + microchip,sync-update-mask = /bits/ 32 <0>; + clocks = <&clkcfg 30>; + reg = <0x41000000 0xF0>; + #pwm-cells = <2>; + };