linux-kernel - Re: [PATCH v5 06/12] dt-bindings: pwm: add microchip corepwm binding

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 2 Feb 2022 12:35:44 +0000
From:   <conor.dooley@...rochip.com>
To:     <u.kleine-koenig@...gutronix.de>
CC:     <a.zummo@...ertech.it>, <alexandre.belloni@...tlin.com>,
        <aou@...s.berkeley.edu>, <atishp@...osinc.com>,
        <bin.meng@...driver.com>, <brgl@...ev.pl>,
        <conor.dooley@...rochip.com>, <daire.mcnamara@...rochip.com>,
        <devicetree@...r.kernel.org>, <geert@...ux-m68k.org>,
        <heiko@...ech.de>, <ivan.griffin@...rochip.com>,
        <jassisinghbrar@...il.com>, <krzysztof.kozlowski@...onical.com>,
        <lee.jones@...aro.org>, <lewis.hanly@...rochip.com>,
        <linus.walleij@...aro.org>, <linux-gpio@...r.kernel.org>,
        <linux-i2c@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        <linux-pwm@...r.kernel.org>, <linux-riscv@...ts.infradead.org>,
        <linux-rtc@...r.kernel.org>, <palmer@...belt.com>,
        <paul.walmsley@...ive.com>, <robh+dt@...nel.org>,
        <robh@...nel.org>, <thierry.reding@...il.com>
Subject: Re: [PATCH v5 06/12] dt-bindings: pwm: add microchip corepwm binding

>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@...rochip.com wrote:
>> From: Conor Dooley <conor.dooley@...rochip.com>
>>
>> Add device tree bindings for the Microchip fpga fabric based "core" PWM
>> controller.
>>
>> Reviewed-by: Rob Herring <robh@...nel.org>
>>
>> Signed-off-by: Conor Dooley <conor.dooley@...rochip.com>
>> ---
>> .../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.