>On 01/02/2022 07:58, Uwe Kleine-König wrote: >> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe >> On Mon, Jan 31, 2022 at 11:47:21AM +0000, conor.dooley@xxxxxxxxxxxxx wrote: >> From: Conor Dooley <conor.dooley@xxxxxxxxxxxxx> >> >> Add device tree bindings for the Microchip fpga fabric based "core" PWM >> controller. >> >> Reviewed-by: Rob Herring <robh@xxxxxxxxxx> >> >> Signed-off-by: Conor Dooley <conor.dooley@xxxxxxxxxxxxx> >> --- >> .../bindings/pwm/microchip,corepwm.yaml | 75 +++++++++++++++++++ <snip> >> + microchip,sync-update: >> + description: | >> + In synchronous mode, all channels are updated at the beginning of the PWM period. >> + Asynchronous mode is relevant to applications such as LED control, where >> + synchronous updates are not required. Asynchronous mode lowers the area size, >> + reducing shadow register requirements. This can be set at run time, provided >> + SHADOW_REG_EN is asserted. SHADOW_REG_EN is set by the FPGA bitstream programmed >> + to the device. >> + Each bit corresponds to a PWM channel & represents whether synchronous mode is >> + possible for the PWM channel. >> + >> + $ref: /schemas/types.yaml#/definitions/uint16 >> + default: 0 > >I'm not sure I understand this correctly. This is a soft-core and you >can synthesize it either with or without the ability to do synchronous >updates or not, right? All 16 channels share the same period length and >in the simple implementation changing the duty cycle is done at once >(maybe introducing a glitch) and in the more expensive implementation >there is a register to implement both variants? Correct. If the IP is instantiated with SHADOW_REG_ENx=1, both registers that control the duty cycle for channel x have a second "shadow 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. I will reword this description since it is not clear. >> + microchip,dac-mode: >> + 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. >> + Each bit corresponds to a PWM channel & represents whether dac mode is enabled >> + that PWM channel. > >In the last sentence a "for" is missing? It is missing, thanks. >These two properties are not detectable in software? Unfortunately not. THe configuration for these options are only accessible in the fpga design. You make a good point however & they really should be visible to software. I'll suggest that for future revisions of this IP that both configurations are accessible over APB Thanks, Conor.