linux-kernel - Re: [PATCH v3 3/3] pwm: hibvt: Add hi3559v100 support

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]

Message-ID: <CANVeeZyQU_NAKYaqKQStWzCOAmidTqLAph0SupfmD7zrH5K_9g@mail.gmail.com>
Date:   Fri, 22 Feb 2019 13:15:53 +0100
From:   Mathieu Othacehe <m.othacehe@...il.com>
To:     Uwe Kleine-König 
        <u.kleine-koenig@...gutronix.de>
Cc:     Thierry Reding <thierry.reding@...il.com>, robh+dt@...nel.org,
        mark.rutland@....com, linux-pwm@...r.kernel.org,
        devicetree@...r.kernel.org, LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3 3/3] pwm: hibvt: Add hi3559v100 support

Hi Uwe,

> The patch looks fine now. (If you have to do another round:
> s/again/twice/ in the commit log and s/once more/twice/ in the comment
> below.)

Ok.

>
> I wonder if this behaviour is a bug or a feature of the hardware. Is
> this additional enable needed to apply changes to both period and
> duty_cycle atomically? Is the 2nd enable needed independent of the PWM
> already running? Can you share the relevant part of the documentation
> without violating an NDA?

The same goes for the period, so I'll precise it in v4. This behaviour
is not documented in the SoC manual.
I discovered it by hitting the problem. Hisilicon support confirmed it
was needed without any
further details. It looks like this behaviour is "by design" because
there are 2 registers for duty cycle and period
and 2 state registers for duty cycle and period. When the first two
registers are edited, the state registers are
only updated on sending a new "1" in pwm enable.

I don't know if the two boards already supported by hibvt driver have
the same design but it seems quite likely.
Maybe this enable thing could be done for all boards and not
considered as a quirk.

Thanks,

Mathieu