Re: [PATCH v1 1/3] dt-bindings: Add cros-ec-ucsi to cros-ec-typec device tree documentation

Krzysztof Kozlowski
2025-03-13T07:10:19Z

On 12/03/2025 20:59, Jameson Thies wrote:
> Add documentation for the cros-ec-ucsi device tree definition. Defining
> this node will load the cros_ec_ucsi driver which is used for USB-C port

Your patch does not do it at all.

> control on PDC based ChromeOS systems. Additionally, update mantainers
> list to reflect changes to the ChromeOS USB owners.
>
> Signed-off-by: Jameson Thies <jthies@google.com>

You need to work on upstream, not downstream trees.

You CC-ed an address, which suggests you do not work on mainline kernel
or you do not use get_maintainers.pl/b4/patman. Please rebase and always
work on mainline or start using mentioned tools, so correct addresses
will be used.

> ---
> .../bindings/chrome/google,cros-ec-typec.yaml | 13 ++++++++++---
> 1 file changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/chrome/google,cros-ec-typec.yaml b/Documentation/devicetree/bindings/chrome/google,cros-ec-typec.yaml
> index 9f9816fbecbc..ab39c5280681 100644
> --- a/Documentation/devicetree/bindings/chrome/google,cros-ec-typec.yaml
> +++ b/Documentation/devicetree/bindings/chrome/google,cros-ec-typec.yaml
> @@ -8,17 +8,24 @@ title: Google Chrome OS EC(Embedded Controller) Type C port driver.
>
> maintainers:
> - Benson Leung <bleung@chromium.org>
> - - Prashant Malani <pmalani@chromium.org>
> + - Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
> + - Łukasz Bartosik <ukaszb@chromium.org>
> + - Jameson Thies <jthies@google.com>
> + - Andrei Kuchynski <akuchynski@chromium.org>
>
> description:
> Chrome OS devices have an Embedded Controller(EC) which has access to
> Type C port state. This node is intended to allow the host to read and
> control the Type C ports. The node for this device should be under a
> - cros-ec node like google,cros-ec-spi.
> + cros-ec node like google,cros-ec-spi. On TCPC systems, ChromeOS should
> + use cros-ec-typec. On PDC systems, ChromeOS should use cros-ec-ucsi.

What does it mean? How is it related to description?

>
> properties:
> compatible:
> - const: google,cros-ec-typec
> + oneOf:
> + - items:
> + - const: google,cros-ec-typec
> + - const: google,cros-ec-ucsi

I don't understand at all why you are growing now this with fallback.
And if you tested your patch, you would see it does not make any sense.

NAK, test your patches before posting.

Best regards,
Krzysztof