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 14:28:48 +0100
From:   Geert Uytterhoeven <geert@...ux-m68k.org>
To:     Conor Dooley <conor.dooley@...rochip.com>
Cc:     Uwe Kleine-König 
        <u.kleine-koenig@...gutronix.de>,
        Alessandro Zummo <a.zummo@...ertech.it>,
        Alexandre Belloni <alexandre.belloni@...tlin.com>,
        Albert Ou <aou@...s.berkeley.edu>,
        Atish Patra <atishp@...osinc.com>,
        Bin Meng <bin.meng@...driver.com>,
        Bartosz Golaszewski <brgl@...ev.pl>,
        daire.mcnamara@...rochip.com,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@...r.kernel.org>, Heiko Stuebner <heiko@...ech.de>,
        ivan.griffin@...rochip.com, Jassi Brar <jassisinghbrar@...il.com>,
        Krzysztof Kozlowski <krzysztof.kozlowski@...onical.com>,
        Lee Jones <lee.jones@...aro.org>,
        Lewis Hanly <lewis.hanly@...rochip.com>,
        Linus Walleij <linus.walleij@...aro.org>,
        "open list:GPIO SUBSYSTEM" <linux-gpio@...r.kernel.org>,
        Linux I2C <linux-i2c@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Linux PWM List <linux-pwm@...r.kernel.org>,
        linux-riscv <linux-riscv@...ts.infradead.org>,
        linux-rtc@...r.kernel.org, Palmer Dabbelt <palmer@...belt.com>,
        Paul Walmsley <paul.walmsley@...ive.com>,
        Rob Herring <robh+dt@...nel.org>,
        Rob Herring <robh@...nel.org>,
        Thierry Reding <thierry.reding@...il.com>
Subject: Re: [PATCH v5 06/12] dt-bindings: pwm: add microchip corepwm binding

Hi Conor,

On Wed, Feb 2, 2022 at 1:33 PM <conor.dooley@...rochip.com> 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@...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 +++++++++++++++++++

> >> +  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.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...ux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds