linux-kernel - Re: [PATCH v6 2/2] dt-bindings: mfd: Add DT compatible string "google,cros_ec

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:   Sun, 11 Oct 2020 09:38:36 -0700
From:   Duncan Laurie <dlaurie@...gle.com>
To:     Enric Balletbo i Serra <enric.balletbo@...labora.com>
Cc:     Bhanu Prakash Maiya <bhanumaiya@...omium.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Raul E Rangel <rrangel@...omium.org>,
        devicetree@...r.kernel.org, Benson Leung <bleung@...omium.org>,
        Guenter Roeck <groeck@...omium.org>,
        Rob Herring <robh+dt@...nel.org>,
        Eric Peers <epeers@...gle.com>,
        Furquan Shaikh <furquan@...omium.org>,
        Lee Jones <lee.jones@...aro.org>, Rob Herring <robh@...nel.org>
Subject: Re: [PATCH v6 2/2] dt-bindings: mfd: Add DT compatible string "google,cros_ec_uart"

On Sun, Oct 11, 2020 at 4:59 AM Enric Balletbo i Serra
<enric.balletbo@...labora.com> wrote:
>
> Hi Bhanu,
>
> Thank you for your patch.
>
> On 9/10/20 23:01, Bhanu Prakash Maiya wrote:
> > Add DT compatible string in
> > Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
> >
>
> The problem with this patchset continues being the same. You are using the trick
> of using a DT compatible string to instantiate an ACPI-only driver. You should
> have an ACPI ID for that device or use a DMI table to match the device and
> instantiate it (see for example the platform/chrome/cros_ec_lpc.c).
>

It isn't really meant to be an ACPI only driver, it just happens
to be an x86 system that uses the UART interface first.  We use
this PRP0001 instantiation method with a lot of different drivers
like MIPI cameras, fingerprint sensors, the cr50 TPM driver, etc.

That said it is true that the rest of the cros_ec drivers use
distinct ACPI IDs.  Bhanu I allocated GOOG0019 for the cros_ec
UART interface so you can switch to using that instead.
You'll need to update the coreboot interface as well and might
want to keep both devices in firmware to ease the transition.

-duncan

>
> > Signed-off-by: Bhanu Prakash Maiya <bhanumaiya@...omium.org>
> > Reviewed-by: Rob Herring <robh@...nel.org>
> > ---
> >
> > Changes in v6:
> > - No change
> >
> > Changes in v5:
> > - No change
> >
> > Changes in v4:
> > - Changes in commit message.
> >
> > Changes in v3:
> > - Rebased changes on google,cros-ec.yaml
> >
> > Changes in v2:
> > - No change
> >
> >  Documentation/devicetree/bindings/mfd/google,cros-ec.yaml | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml b/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
> > index 6a7279a85ec1c..552d1c9bf3de4 100644
> > --- a/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
> > +++ b/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
> > @@ -10,11 +10,12 @@ maintainers:
> >    - Benson Leung <bleung@...omium.org>
> >    - Enric Balletbo i Serra <enric.balletbo@...labora.com>
> >    - Guenter Roeck <groeck@...omium.org>
> > +  - Bhanu Prakash Maiya <bhanumaiya@...omium.org>
> >
> >  description:
> >    Google's ChromeOS EC is a microcontroller which talks to the AP and
> >    implements various functions such as keyboard and battery charging.
> > -  The EC can be connected through various interfaces (I2C, SPI, and others)
> > +  The EC can be connected through various interfaces (I2C, SPI, UART and others)
> >    and the compatible string specifies which interface is being used.
> >
> >  properties:
> > @@ -29,6 +30,9 @@ properties:
> >        - description:
> >            For implementations of the EC is connected through RPMSG.
> >          const: google,cros-ec-rpmsg
> > +      - description:
> > +          For implementations of the EC is connected through UART.
> > +        const: google,cros-ec-uart
> >
> >    google,cros-ec-spi-pre-delay:
> >      description:
> >