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

Add DT compatible string in
Documentation/devicetree/bindings/mfd/google,cros-ec.yaml

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(-)

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).

Thanks,
 Enric

> 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:
>

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:
> >

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:
>     >
>