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

Hi Duncan,

On 11/10/20 18:35, Duncan Laurie wrote:

On Sun, Oct 11, 2020 at 4:59 AM Enric Balletbo i Serra
<enric.balletbo@collabora.com <mailto: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.

Right, but then you need to define a proper DT binding, with all the options,
not only the compatible name.

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.

Thank you.

  Enric

-duncan

 


    > Signed-off-by: Bhanu Prakash Maiya <bhanumaiya@chromium.org
    <mailto:bhanumaiya@chromium.org>>
    > Reviewed-by: Rob Herring <robh@kernel.org <mailto: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 <mailto:bleung@chromium.org>>
    >    - Enric Balletbo i Serra <enric.balletbo@collabora.com
    <mailto:enric.balletbo@collabora.com>>
    >    - Guenter Roeck <groeck@chromium.org <mailto:groeck@chromium.org>>
    > +  - Bhanu Prakash Maiya <bhanumaiya@chromium.org
    <mailto: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:
    >