Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20201009132732.v6.1.Ic98067cd92a0b7fed9cd3dfb7b4e736e76551cda@changeid>
 <20201009132732.v6.2.I8d7530d8372e4ef298ddaaaad612a2cdd24ed93e@changeid> <8cfa923c-cc7a-6dd3-9541-8a18b98091b7@collabora.com>
In-Reply-To: <8cfa923c-cc7a-6dd3-9541-8a18b98091b7@collabora.com>
From:   Duncan Laurie <dlaurie@google.com>
Date:   Sun, 11 Oct 2020 09:38:36 -0700
Message-ID: <CADv6+04sHeL9BVSqWVtFeUfuPxBGK+H-jq2+gA0YgpwpbBtYjg@mail.gmail.com>
Subject: Re: [PATCH v6 2/2] dt-bindings: mfd: Add DT compatible string "google,cros_ec_uart"
To:     Enric Balletbo i Serra <enric.balletbo@collabora.com>
Cc:     Bhanu Prakash Maiya <bhanumaiya@chromium.org>,
        LKML <linux-kernel@vger.kernel.org>,
        Raul E Rangel <rrangel@chromium.org>,
        devicetree@vger.kernel.org, Benson Leung <bleung@chromium.org>,
        Guenter Roeck <groeck@chromium.org>,
        Rob Herring <robh+dt@kernel.org>,
        Eric Peers <epeers@google.com>,
        Furquan Shaikh <furquan@chromium.org>,
        Lee Jones <lee.jones@linaro.org>, Rob Herring <robh@kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Sun, Oct 11, 2020 at 4:59 AM Enric Balletbo i Serra
<enric.balletbo@collabora.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@chromium.org>
> > Reviewed-by: Rob Herring <robh@kernel.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@chromium.org>
> >    - Enric Balletbo i Serra <enric.balletbo@collabora.com>
> >    - Guenter Roeck <groeck@chromium.org>
> > +  - Bhanu Prakash Maiya <bhanumaiya@chromium.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:
> >