linux-kernel - Re: [PATCH 1/2] dt-bindings: HID: i2c-hid: Add the ability to control a reset GPIO

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] [day] [month] [year] [list]

Date:   Mon, 19 Oct 2020 16:10:36 -0500
From:   Rob Herring <robh@...nel.org>
To:     Douglas Anderson <dianders@...omium.org>
Cc:     jkosina@...e.cz, benjamin.tissoires@...hat.com,
        gregkh@...uxfoundation.org, kai.heng.feng@...onical.com,
        linux-input@...r.kernel.org, hdegoede@...hat.com,
        andrea@...gia.bo.it, swboyd@...omium.org,
        Dmitry Torokhov <dmitry.torokhov@...il.com>,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] dt-bindings: HID: i2c-hid: Add the ability to
 control a reset GPIO

On Wed, Oct 14, 2020 at 04:42:20PM -0700, Douglas Anderson wrote:
> Apparently some devices connected via i2c-hid have timing requirements
> around when a reset GPIO should be asserted to them.  The diagram I
> have seen, which I believe is from a Goodix device, looked like this:
> 
>          +----------------------------------
>          |
> AVDD ----+
>                +------------------------------
>          | (a) |
> RESET ---------+
>                      +-------------
>                | (b) |
> I2C comm OK ---------+
> 
> Where (a) is 10 ms and (b) is 120 ms.
> 
> Let's add the ability to specify these timings to the devicetree
> bindings.
> 
> Signed-off-by: Douglas Anderson <dianders@...omium.org>
> ---
> I notice this bindings file is still a ".txt" file.  Some searching on
> The Internets(TM) shows that Rob has maybe started a conversion 5
> years ago [1], but that looks ancient.  I can try to put something
> together if need be, or we can just land this fix.  ;-)

That was attempt number 1 using YAML...

> Note that the .txt version of the bindings seems to indicate that
> anyone using one of the optional properties is supposed to declare
> their special compatible string.  I'm not sure if that's still
> considered important or not?  Once you manage to get these devices
> powered on and talking i2c they self-describe themselves...

This change is exactly why devices should have specific compatible 
strings.

> 
> [1] https://kernel.googlesource.com/pub/scm/linux/kernel/git/robh/linux/+/refs/heads/dt-yaml/Documentation/devicetree/bindings/hid/hid-over-i2c.yaml
> 
>  Documentation/devicetree/bindings/input/hid-over-i2c.txt | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/input/hid-over-i2c.txt b/Documentation/devicetree/bindings/input/hid-over-i2c.txt
> index c76bafaf98d2..6fca39aa8cc6 100644
> --- a/Documentation/devicetree/bindings/input/hid-over-i2c.txt
> +++ b/Documentation/devicetree/bindings/input/hid-over-i2c.txt
> @@ -32,6 +32,11 @@ device-specific compatible properties, which should be used in addition to the
>  - vdd-supply: phandle of the regulator that provides the supply voltage.
>  - post-power-on-delay-ms: time required by the device after enabling its regulators
>    or powering it on, before it is ready for communication.
> +- reset-gpios: GPIOs to assert to reset the device. This GPIO is asserted when
> +  the device is powered off and released post-power-on-delay-ms after
> +  enabling the regulators.
> +- post-gpio-reset-delay-ms: After deasserting reset we'll delay for this many
> +  more milliseconds.

IMO, you should imply this from the compatible string.

Rob