On 02/02/2022 13:28, Geert Uytterhoeven wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > Hi Conor, > > On Wed, Feb 2, 2022 at 1:33 PM <conor.dooley@xxxxxxxxxxxxx> wrote: >>> 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 +++++++++++++++++++ > >>>> + 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. > > Shouldn't it use a different compatible value instead? > Differentiation by properties is not recommended, as it's easy to > miss a difference. Either you have something in mind that I've not thought of, or I've done a bad job of explaining again. The buffer/"shadow" registers are synthesised on a per channel basis, so any combination of the 16 channels may have this capability. The same applies to the DAC mode, per channel there too. Conor.
