| Message ID | 20250312195951.1579682-2-jthies@google.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Load cros_ec_ucsi from OF and ACPI definitions | expand |
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
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 > 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> > --- Please use subject prefixes matching the subsystem. You can get them for example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory your patch is touching. For bindings, the preferred subjects are explained here: https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patches.html#i-for-patch-submitters A nit, subject: drop second/last, redundant "device tree documentation". The "dt-bindings" prefix is already stating that these are "device tree documentation". See also: https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18 Best regards, Krzysztof
Hi Krzysztof, thank you for taking a look at this series. Clearly it needs some more work. I’ll follow up with a v2 addressing your comments. > > - 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? TCPCs and PDCs are different components which can be used for power delivery messaging on USB-C ports. On ChromeOS devices, they are mutually exclusive. This line is just saying which driver should be chosen based on the USB-C port hardware. But, I see that type of information isn’t really relevant for the device tree documentation. Thanks, Jameson
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. properties: compatible: - const: google,cros-ec-typec + oneOf: + - items: + - const: google,cros-ec-typec + - const: google,cros-ec-ucsi '#address-cells': const: 1
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 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> --- .../bindings/chrome/google,cros-ec-typec.yaml | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-)